Since commit
commit f16d9fb6cf03 ("power: supply: bq27xxx: Retrieve again when busy")
the console log of some devices with hdq but no bq27000 battery (like the Pandaboard) is flooded with messages like:
[ 34.247833] power_supply bq27000-battery: driver failed to report 'status' property: -1
as soon as user-space is finding a /sys entry and trying to read the "status" property.
It turns out that the offending commit changes the logic to now return the value of cache.flags if it is <0. This is likely under the assumption that it is an error number. In normal errors from bq27xxx_read() this is indeed the case.
But there is special code to detect if no bq27000 is installed or accessible through hdq/1wire and wants to report this. In that case, the cache.flags are set (historically) to constant -1 which did make reading properties return -ENODEV. So everything appeared to be fine before the return value was fixed. Now the -1 is returned as -ENOPERM instead of -ENODEV, triggering the error condition in power_supply_format_property() which then floods the console log.
So we change the detection of missing bq27000 battery to simply set
cache.flags = -ENODEV
instead of -1.
Fixes: f16d9fb6cf03 ("power: supply: bq27xxx: Retrieve again when busy") Cc: Jerry Lv Jerry.Lv@axis.com Cc: stable@vger.kernel.org Signed-off-by: H. Nikolaus Schaller hns@goldelico.com --- drivers/power/supply/bq27xxx_battery.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c index 93dcebbe11417..efe02ad695a62 100644 --- a/drivers/power/supply/bq27xxx_battery.c +++ b/drivers/power/supply/bq27xxx_battery.c @@ -1920,7 +1920,7 @@ static void bq27xxx_battery_update_unlocked(struct bq27xxx_device_info *di)
cache.flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, has_singe_flag); if ((cache.flags & 0xff) == 0xff) - cache.flags = -1; /* read error */ + cache.flags = -ENODEV; /* read error */ if (cache.flags >= 0) { cache.capacity = bq27xxx_battery_read_soc(di);
________________________________________ From: H. Nikolaus Schaller hns@goldelico.com Sent: Monday, July 21, 2025 8:46 PM To: Sebastian Reichel; Jerry Lv Cc: Pali Rohár; linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org; letux-kernel@openphoenux.org; stable@vger.kernel.org; kernel@pyra-handheld.com; andreas@kemnade.info; H. Nikolaus Schaller Subject: [PATCH] power: supply: bq27xxx: fix error return in case of no bq27000 hdq battery
[You don't often get email from hns@goldelico.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
Since commit
commit f16d9fb6cf03 ("power: supply: bq27xxx: Retrieve again when busy")
the console log of some devices with hdq but no bq27000 battery (like the Pandaboard) is flooded with messages like:
[ 34.247833] power_supply bq27000-battery: driver failed to report 'status' property: -1
as soon as user-space is finding a /sys entry and trying to read the "status" property.
It turns out that the offending commit changes the logic to now return the value of cache.flags if it is <0. This is likely under the assumption that it is an error number. In normal errors from bq27xxx_read() this is indeed the case.
But there is special code to detect if no bq27000 is installed or accessible through hdq/1wire and wants to report this. In that case, the cache.flags are set (historically) to constant -1 which did make reading properties return -ENODEV. So everything appeared to be fine before the return value was fixed. Now the -1 is returned as -ENOPERM instead of -ENODEV, triggering the error condition in power_supply_format_property() which then floods the console log.
So we change the detection of missing bq27000 battery to simply set
cache.flags = -ENODEV
instead of -1.
Fixes: f16d9fb6cf03 ("power: supply: bq27xxx: Retrieve again when busy") Cc: Jerry Lv Jerry.Lv@axis.com Cc: stable@vger.kernel.org Signed-off-by: H. Nikolaus Schaller hns@goldelico.com --- drivers/power/supply/bq27xxx_battery.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c index 93dcebbe11417..efe02ad695a62 100644 --- a/drivers/power/supply/bq27xxx_battery.c +++ b/drivers/power/supply/bq27xxx_battery.c @@ -1920,7 +1920,7 @@ static void bq27xxx_battery_update_unlocked(struct bq27xxx_device_info *di)
cache.flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, has_singe_flag); if ((cache.flags & 0xff) == 0xff) - cache.flags = -1; /* read error */ + cache.flags = -ENODEV; /* read error */ if (cache.flags >= 0) { cache.capacity = bq27xxx_battery_read_soc(di);
-- 2.50.0
In our device, we use the I2C to get data from the gauge bq27z561. During our test, when try to get the status register by bq27xxx_read() in the bq27xxx_battery_update_unlocked(), we found sometimes the returned value is 0xFFFF, but it will update to some other value very quickly. So the returned 0xFFFF does not indicate "No such device", if we force to set the cache.flags to "-ENODEV" or "-1" manually in this case, the bq27xxx_battery_get_property() will just return the cache.flags until it is updated at lease 5 seconds later, it means we cannot get any property in these 5 seconds.
In fact, for the I2C driver, if no bq27000 is installed or accessible, the bq27xxx_battery_i2c_read() will return "-ENODEV" directly when no device, or the i2c_transfer() will return the negative error according to real case.
bq27xxx_battery_i2c_read() { ... if (!client->adapter) return -ENODEV; ... ret = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg)); ... if (ret < 0) return ret; ... }
But there is no similar check in the bq27xxx_battery_hdq_read() for the HDQ/1-wire driver. Could we do the same check in the bq27xxx_battery_hdq_read(), instead of changing the cache.flags manually when the last byte in the returned data is 0xFF? Or could we just force to set the returned value to "-ENODEV" only when the last byte get from bq27xxx_battery_hdq_read() is 0xFF?
Best Regards, Jerry Lv
Hi Jerry,
Am 05.08.2025 um 10:53 schrieb Jerry Lv Jerry.Lv@axis.com:
From: H. Nikolaus Schaller hns@goldelico.com Sent: Monday, July 21, 2025 8:46 PM To: Sebastian Reichel; Jerry Lv Cc: Pali Rohár; linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org; letux-kernel@openphoenux.org; stable@vger.kernel.org; kernel@pyra-handheld.com; andreas@kemnade.info; H. Nikolaus Schaller Subject: [PATCH] power: supply: bq27xxx: fix error return in case of no bq27000 hdq battery
[You don't often get email from hns@goldelico.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
Since commit
commit f16d9fb6cf03 ("power: supply: bq27xxx: Retrieve again when busy")
the console log of some devices with hdq but no bq27000 battery (like the Pandaboard) is flooded with messages like:
[ 34.247833] power_supply bq27000-battery: driver failed to report 'status' property: -1
as soon as user-space is finding a /sys entry and trying to read the "status" property.
It turns out that the offending commit changes the logic to now return the value of cache.flags if it is <0. This is likely under the assumption that it is an error number. In normal errors from bq27xxx_read() this is indeed the case.
But there is special code to detect if no bq27000 is installed or accessible through hdq/1wire and wants to report this. In that case, the cache.flags are set (historically) to constant -1 which did make reading properties return -ENODEV. So everything appeared to be fine before the return value was fixed. Now the -1 is returned as -ENOPERM instead of -ENODEV, triggering the error condition in power_supply_format_property() which then floods the console log.
So we change the detection of missing bq27000 battery to simply set
cache.flags = -ENODEVinstead of -1.
Fixes: f16d9fb6cf03 ("power: supply: bq27xxx: Retrieve again when busy") Cc: Jerry Lv Jerry.Lv@axis.com Cc: stable@vger.kernel.org Signed-off-by: H. Nikolaus Schaller hns@goldelico.com
drivers/power/supply/bq27xxx_battery.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c index 93dcebbe11417..efe02ad695a62 100644 --- a/drivers/power/supply/bq27xxx_battery.c +++ b/drivers/power/supply/bq27xxx_battery.c @@ -1920,7 +1920,7 @@ static void bq27xxx_battery_update_unlocked(struct bq27xxx_device_info *di)
cache.flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, has_singe_flag); if ((cache.flags & 0xff) == 0xff)
cache.flags = -1; /* read error */
cache.flags = -ENODEV; /* read error */ if (cache.flags >= 0) { cache.capacity = bq27xxx_battery_read_soc(di);-- 2.50.0
In our device, we use the I2C to get data from the gauge bq27z561. During our test, when try to get the status register by bq27xxx_read() in the bq27xxx_battery_update_unlocked(), we found sometimes the returned value is 0xFFFF, but it will update to some other value very quickly.
Strange. Do you have an idea if this is an I2C communication effect or really reported from the bq27z561 chip?
So the returned 0xFFFF does not indicate "No such device", if we force to set the cache.flags to "-ENODEV" or "-1" manually in this case, the bq27xxx_battery_get_property() will just return the cache.flags until it is updated at lease 5 seconds later, it means we cannot get any property in these 5 seconds.
Ok I see. So there should be a different rule for the bq27z561.
In fact, for the I2C driver, if no bq27000 is installed or accessible, the bq27xxx_battery_i2c_read() will return "-ENODEV" directly when no device, or the i2c_transfer() will return the negative error according to real case.
Yes, that is what I2C can easily report. But for AFAIK for HDQ there is no -ENODEV detection in the protocol. So the bq27000 has this special check.
bq27xxx_battery_i2c_read() { ... if (!client->adapter) return -ENODEV; ... ret = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg)); ... if (ret < 0) return ret; ... }But there is no similar check in the bq27xxx_battery_hdq_read() for the HDQ/1-wire driver.
Could we do the same check in the bq27xxx_battery_hdq_read(), instead of changing the cache.flags manually when the last byte in the returned data is 0xFF?
So your suggestion is to modify bq27xxx_battery_hdq_read to check for BQ27XXX_REG_FLAGS and value 0xff and convert to -ENODEV?
Well, it depends on the data that has been successfully reported. So making bq27xxx_battery_hdq_read() have some logic to evaluate the data seems to just move the problem to a different place. Especially as this is a generic function that can read any register it is counter-intuitive to analyse the data.
Or could we just force to set the returned value to "-ENODEV" only when the last byte get from bq27xxx_battery_hdq_read() is 0xFF?
In summary I am not sure if that improves anything. It just makes the existing code more difficult to understand.
What about checking bq27xxx_battery_update_unlocked() for
if (!(di->opts & BQ27Z561_O_BITS) && (cache.flags & 0xff) == 0xff)
to protect your driver from this logic?
This would not touch or break the well tested bq27000 logic and prevent the new bq27z561 driver to trigger a false positive?
BR and thanks, Nikolaus
Hello Nikolaus,
On 8/5/2025 5:28 PM, H. Nikolaus Schaller wrote:
Hi Jerry,
Am 05.08.2025 um 10:53 schrieb Jerry Lv Jerry.Lv@axis.com:
From: H. Nikolaus Schaller hns@goldelico.com Sent: Monday, July 21, 2025 8:46 PM To: Sebastian Reichel; Jerry Lv Cc: Pali Rohár; linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org; letux-kernel@openphoenux.org; stable@vger.kernel.org; kernel@pyra-handheld.com; andreas@kemnade.info; H. Nikolaus Schaller Subject: [PATCH] power: supply: bq27xxx: fix error return in case of no bq27000 hdq battery
[You don't often get email from hns@goldelico.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
Since commit
commit f16d9fb6cf03 ("power: supply: bq27xxx: Retrieve again when busy")
the console log of some devices with hdq but no bq27000 battery (like the Pandaboard) is flooded with messages like:
[ 34.247833] power_supply bq27000-battery: driver failed to report 'status' property: -1
as soon as user-space is finding a /sys entry and trying to read the "status" property.
It turns out that the offending commit changes the logic to now return the value of cache.flags if it is <0. This is likely under the assumption that it is an error number. In normal errors from bq27xxx_read() this is indeed the case.
But there is special code to detect if no bq27000 is installed or accessible through hdq/1wire and wants to report this. In that case, the cache.flags are set (historically) to constant -1 which did make reading properties return -ENODEV. So everything appeared to be fine before the return value was fixed. Now the -1 is returned as -ENOPERM instead of -ENODEV, triggering the error condition in power_supply_format_property() which then floods the console log.
So we change the detection of missing bq27000 battery to simply set
cache.flags = -ENODEVinstead of -1.
Fixes: f16d9fb6cf03 ("power: supply: bq27xxx: Retrieve again when busy") Cc: Jerry Lv Jerry.Lv@axis.com Cc: stable@vger.kernel.org Signed-off-by: H. Nikolaus Schaller hns@goldelico.com
drivers/power/supply/bq27xxx_battery.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c index 93dcebbe11417..efe02ad695a62 100644 --- a/drivers/power/supply/bq27xxx_battery.c +++ b/drivers/power/supply/bq27xxx_battery.c @@ -1920,7 +1920,7 @@ static void bq27xxx_battery_update_unlocked(struct bq27xxx_device_info *di)
cache.flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, has_singe_flag); if ((cache.flags & 0xff) == 0xff)
cache.flags = -1; /* read error */
cache.flags = -ENODEV; /* read error */ if (cache.flags >= 0) { cache.capacity = bq27xxx_battery_read_soc(di);-- 2.50.0
In our device, we use the I2C to get data from the gauge bq27z561. During our test, when try to get the status register by bq27xxx_read() in the bq27xxx_battery_update_unlocked(), we found sometimes the returned value is 0xFFFF, but it will update to some other value very quickly.
Strange. Do you have an idea if this is an I2C communication effect or really reported from the bq27z561 chip?
It's the data returned by i2c_transfer(). I have reported this issue to TI, and wait for their further investigation. Not sure whether other gauges behave like this or not.
So the returned 0xFFFF does not indicate "No such device", if we force to set the cache.flags to "-ENODEV" or "-1" manually in this case, the bq27xxx_battery_get_property() will just return the cache.flags until it is updated at lease 5 seconds later, it means we cannot get any property in these 5 seconds.
Ok I see. So there should be a different rule for the bq27z561.
This is not only for bq27z561, it's the general mechanism in the driver bq27xxx_battery.c for all gauges:
static int bq27xxx_battery_get_property() {
...
if (psp != POWER_SUPPLY_PROP_PRESENT && di->cache.flags < 0)
return di->cache.flags;
}
In fact, for the I2C driver, if no bq27000 is installed or accessible, the bq27xxx_battery_i2c_read() will return "-ENODEV" directly when no device, or the i2c_transfer() will return the negative error according to real case.
Yes, that is what I2C can easily report. But for AFAIK for HDQ there is no -ENODEV detection in the protocol. So the bq27000 has this special check.
Since this is the special check only needed for bq27000,
suggest to check the chip type before changing the cache.flags to -ENODEV manually, see my comments in later part.
bq27xxx_battery_i2c_read() { ... if (!client->adapter) return -ENODEV; ... ret = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg)); ... if (ret < 0) return ret; ... }But there is no similar check in the bq27xxx_battery_hdq_read() for the HDQ/1-wire driver.
Could we do the same check in the bq27xxx_battery_hdq_read(), instead of changing the cache.flags manually when the last byte in the returned data is 0xFF?
So your suggestion is to modify bq27xxx_battery_hdq_read to check for BQ27XXX_REG_FLAGS and value 0xff and convert to -ENODEV?
Well, it depends on the data that has been successfully reported. So making bq27xxx_battery_hdq_read() have some logic to evaluate the data seems to just move the problem to a different place. Especially as this is a generic function that can read any register it is counter-intuitive to analyse the data.
Or could we just force to set the returned value to "-ENODEV" only when the last byte get from bq27xxx_battery_hdq_read() is 0xFF?
In summary I am not sure if that improves anything. It just makes the existing code more difficult to understand.
What about checking bq27xxx_battery_update_unlocked() for
if (!(di->opts & BQ27Z561_O_BITS) && (cache.flags & 0xff) == 0xff)to protect your driver from this logic?
This would not touch or break the well tested bq27000 logic and prevent the new bq27z561 driver to trigger a false positive?
This change works for my device, but just as you said, this change makes the existing code more difficult to understand.
Since changing the cache.flags to -ENODEV manually is only needed for the bq27000 with the HDQ driver,
suggest to check the chip type first like below:
if ((di->chip == BQ27000) && (cache.flags & 0xff) == 0xff)
cache.flags = -ENODEV; /* read error */
This will not break the well tested bq27000 logic, and also works fine with other gauges, and it's more easy to understand. What's your opinion?
BR and thanks, Nikolaus
Best Regards,
Jerry Lv
Hi Jerry, sorry it appears that I have missed your mail.
Am 08.08.2025 um 11:13 schrieb Jerry Lv jerrylv@axis.com:
Hello Nikolaus,
On 8/5/2025 5:28 PM, H. Nikolaus Schaller wrote:
Hi Jerry,
Am 05.08.2025 um 10:53 schrieb Jerry Lv Jerry.Lv@axis.com:
From: H. Nikolaus Schaller hns@goldelico.com Sent: Monday, July 21, 2025 8:46 PM To: Sebastian Reichel; Jerry Lv Cc: Pali Rohár; linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org; letux-kernel@openphoenux.org; stable@vger.kernel.org; kernel@pyra-handheld.com; andreas@kemnade.info; H. Nikolaus Schaller Subject: [PATCH] power: supply: bq27xxx: fix error return in case of no bq27000 hdq battery
[You don't often get email from hns@goldelico.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
Since commit
commit f16d9fb6cf03 ("power: supply: bq27xxx: Retrieve again when busy")
the console log of some devices with hdq but no bq27000 battery (like the Pandaboard) is flooded with messages like:
[ 34.247833] power_supply bq27000-battery: driver failed to report 'status' property: -1
as soon as user-space is finding a /sys entry and trying to read the "status" property.
It turns out that the offending commit changes the logic to now return the value of cache.flags if it is <0. This is likely under the assumption that it is an error number. In normal errors from bq27xxx_read() this is indeed the case.
But there is special code to detect if no bq27000 is installed or accessible through hdq/1wire and wants to report this. In that case, the cache.flags are set (historically) to constant -1 which did make reading properties return -ENODEV. So everything appeared to be fine before the return value was fixed. Now the -1 is returned as -ENOPERM instead of -ENODEV, triggering the error condition in power_supply_format_property() which then floods the console log.
So we change the detection of missing bq27000 battery to simply set
cache.flags = -ENODEVinstead of -1.
Fixes: f16d9fb6cf03 ("power: supply: bq27xxx: Retrieve again when busy") Cc: Jerry Lv Jerry.Lv@axis.com Cc: stable@vger.kernel.org Signed-off-by: H. Nikolaus Schaller hns@goldelico.com
drivers/power/supply/bq27xxx_battery.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c index 93dcebbe11417..efe02ad695a62 100644 --- a/drivers/power/supply/bq27xxx_battery.c +++ b/drivers/power/supply/bq27xxx_battery.c @@ -1920,7 +1920,7 @@ static void bq27xxx_battery_update_unlocked(struct bq27xxx_device_info *di)
cache.flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, has_singe_flag); if ((cache.flags & 0xff) == 0xff)
cache.flags = -1; /* read error */
cache.flags = -ENODEV; /* read error */ if (cache.flags >= 0) { cache.capacity = bq27xxx_battery_read_soc(di);-- 2.50.0
In our device, we use the I2C to get data from the gauge bq27z561. During our test, when try to get the status register by bq27xxx_read() in the bq27xxx_battery_update_unlocked(), we found sometimes the returned value is 0xFFFF, but it will update to some other value very quickly.
Strange. Do you have an idea if this is an I2C communication effect or really reported from the bq27z561 chip?
It's the data returned by i2c_transfer(). I have reported this issue to TI, and wait for their further investigation. Not sure whether other gauges behave like this or not.
So the returned 0xFFFF does not indicate "No such device", if we force to set the cache.flags to "-ENODEV" or "-1" manually in this case, the bq27xxx_battery_get_property() will just return the cache.flags until it is updated at lease 5 seconds later, it means we cannot get any property in these 5 seconds.
Ok I see. So there should be a different rule for the bq27z561.
This is not only for bq27z561, it's the general mechanism in the driver bq27xxx_battery.c for all gauges:
But the "bug" is only for the bq27z561.
static int bq27xxx_battery_get_property() { ... if (psp != POWER_SUPPLY_PROP_PRESENT && di->cache.flags < 0) return di->cache.flags; }In fact, for the I2C driver, if no bq27000 is installed or accessible, the bq27xxx_battery_i2c_read() will return "-ENODEV" directly when no device, or the i2c_transfer() will return the negative error according to real case.
Yes, that is what I2C can easily report. But for AFAIK for HDQ there is no -ENODEV detection in the protocol. So the bq27000 has this special check.
Since this is the special check only needed for bq27000,
suggest to check the chip type before changing the cache.flags to -ENODEV manually, see my comments in later part.
bq27xxx_battery_i2c_read() { ... if (!client->adapter) return -ENODEV; ... ret = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg)); ... if (ret < 0) return ret; ... }But there is no similar check in the bq27xxx_battery_hdq_read() for the HDQ/1-wire driver.
Could we do the same check in the bq27xxx_battery_hdq_read(), instead of changing the cache.flags manually when the last byte in the returned data is 0xFF?
So your suggestion is to modify bq27xxx_battery_hdq_read to check for BQ27XXX_REG_FLAGS and value 0xff and convert to -ENODEV?
Well, it depends on the data that has been successfully reported. So making bq27xxx_battery_hdq_read() have some logic to evaluate the data seems to just move the problem to a different place. Especially as this is a generic function that can read any register it is counter-intuitive to analyse the data.
Or could we just force to set the returned value to "-ENODEV" only when the last byte get from bq27xxx_battery_hdq_read() is 0xFF?
In summary I am not sure if that improves anything. It just makes the existing code more difficult to understand.
What about checking bq27xxx_battery_update_unlocked() for
if (!(di->opts & BQ27Z561_O_BITS) && (cache.flags & 0xff) == 0xff)to protect your driver from this logic?
This would not touch or break the well tested bq27000 logic and prevent the new bq27z561 driver to trigger a false positive?
This change works for my device, but just as you said, this change makes the existing code more difficult to understand.
It may be a matter of a better code comment (see below)...
Since changing the cache.flags to -ENODEV manually is only needed for the bq27000 with the HDQ driver,
suggest to check the chip type first like below:
if ((di->chip == BQ27000) && (cache.flags & 0xff) == 0xff) cache.flags = -ENODEV; /* read error */
Ok, this depends on what the gauges != bq27000 and != bq27Z561 would expect. This is something I do not know, but maybe the community / maintainers.
And, there is a comment for the definition of BQ27000 that it is also meant for the bq27200, which has an i2c interface. That one is obsolete but still may be used somewhere...
But if it does never return 0xff for the status register it would be fine for me.
This will not break the well tested bq27000 logic, and also works fine with other gauges, and it's more easy to understand. What's your opinion?
I can't acccess the git repo at the moment, so here is what think this would mean:
@@ -1920,7 +1920,7 @@ static void bq27xxx_battery_update_unlocked(struct bq27xxx_device_info *di)
cache.flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, has_singe_flag); - if ((cache.flags & 0xff) == 0xff) + if ((di->chip == BQ27000) && (cache.flags & 0xff) == 0xff) - cache.flags = -1; /* read error */ + cache.flags = -ENODEV; /* bq27000 hdq read error */ if (cache.flags >= 0) { cache.capacity = bq27xxx_battery_read_soc(di);
With the changed comment it should be more clear what it is doing and why.
Maybe can you test this? As soon as I can test, I'll report and submit a v2.
BR and thanks, Nikolaus
Hi Jerry,
Am 05.08.2025 um 10:53 schrieb Jerry Lv Jerry.Lv@axis.com:
But there is no similar check in the bq27xxx_battery_hdq_read() for the HDQ/1-wire driver. Could we do the same check in the bq27xxx_battery_hdq_read(), instead of changing the cache.flags manually when the last byte in the returned data is 0xFF? Or could we just force to set the returned value to "-ENODEV" only when the last byte get from bq27xxx_battery_hdq_read() is 0xFF?
I tried to move the 0xff detection to bq27xxx_battery_hdq_read() and make it trigger only for register 0x0a (BQ27XXX_REG_FLAGS), but there are other locations where bq27xxx_read() is called for this register. And those emit error messages in case the battery is removed while user-space is polling.
So I'll post a v2 with two patches (for different bugs): a) set cache.flags to -ENODEV to fix the -EPERM bug b) restrict the check for the 0xff condition to the bq27000 to avoid false positives for your bq27z561
BR and thanks, Nikolaus
Hi,
Am Mon, 21 Jul 2025 14:46:09 +0200 schrieb "H. Nikolaus Schaller" hns@goldelico.com:
Since commit
commit f16d9fb6cf03 ("power: supply: bq27xxx: Retrieve again when busy")
the console log of some devices with hdq but no bq27000 battery (like the Pandaboard) is flooded with messages like:
[ 34.247833] power_supply bq27000-battery: driver failed to report 'status' property: -1
as soon as user-space is finding a /sys entry and trying to read the "status" property.
It turns out that the offending commit changes the logic to now return the value of cache.flags if it is <0. This is likely under the assumption that it is an error number. In normal errors from bq27xxx_read() this is indeed the case.
But there is special code to detect if no bq27000 is installed or accessible through hdq/1wire and wants to report this. In that case, the cache.flags are set (historically) to constant -1 which did make reading properties return -ENODEV. So everything appeared to be fine before the return value was fixed. Now the -1 is returned as -ENOPERM instead of -ENODEV, triggering the error condition in power_supply_format_property() which then floods the console log.
So we change the detection of missing bq27000 battery to simply set
cache.flags = -ENODEV
instead of -1.
This all is a bit inconsistent, the offending commit makes it worse. Normally devices appear only in /sys if they exist. Regarding stuff in /sys/class/power_supply, input power supplies might be there or not, but there you can argument that the entry in /sys/class/power_supply only means that there is a connector for connecting a supply. But having the battery entry everywhere looks like waste. If would expect the existence of a battery bay in the device where the common battery is one with a bq27xxx.
Regards, Andreas
Am 21.08.2025 um 20:15 schrieb Andreas Kemnade andreas@kemnade.info:
Hi,
Am Mon, 21 Jul 2025 14:46:09 +0200 schrieb "H. Nikolaus Schaller" hns@goldelico.com:
Since commit
commit f16d9fb6cf03 ("power: supply: bq27xxx: Retrieve again when busy")
the console log of some devices with hdq but no bq27000 battery (like the Pandaboard) is flooded with messages like:
[ 34.247833] power_supply bq27000-battery: driver failed to report 'status' property: -1
as soon as user-space is finding a /sys entry and trying to read the "status" property.
It turns out that the offending commit changes the logic to now return the value of cache.flags if it is <0. This is likely under the assumption that it is an error number. In normal errors from bq27xxx_read() this is indeed the case.
But there is special code to detect if no bq27000 is installed or accessible through hdq/1wire and wants to report this. In that case, the cache.flags are set (historically) to constant -1 which did make reading properties return -ENODEV. So everything appeared to be fine before the return value was fixed. Now the -1 is returned as -ENOPERM instead of -ENODEV, triggering the error condition in power_supply_format_property() which then floods the console log.
So we change the detection of missing bq27000 battery to simply set
cache.flags = -ENODEV
instead of -1.
This all is a bit inconsistent, the offending commit makes it worse. Normally devices appear only in /sys if they exist. Regarding stuff in /sys/class/power_supply, input power supplies might be there or not, but there you can argument that the entry in /sys/class/power_supply only means that there is a connector for connecting a supply.
Indeed. If there is an optional bq27000 hdq battery the entry exists.
But having the battery entry everywhere looks like waste. If would expect the existence of a battery bay in the device where the common battery is one with a bq27xxx.
I think the flaw you are mentioning is a completely diffent one. It comes from that the 1-wire or hdq interface of some omap processors is enabled in the .dtsi by default instead of disabling it like other interfaces (e.g. mcbsp1). E.g. for omap3 hdqw1w:
https://elixir.bootlin.com/linux/v6.16.1/source/arch/arm/boot/dts/ti/omap/om...
And we should have the dts for the boards enable it only if the hdq interface is really in use and there is a chance that a bq27000 can be connected. In that case the full /sys entry is prepared but returns -ENODEV if the battery is missing, which is then exactly the right error return (instead of -EPERM triggering the console message).
Or is there something related to power-management, so that the hdq silicon always needs the hdq driver to properly idle?
Anyways I think this is a different topic for separate cleanup. For the moment we should fix the hdq27000 missing battery detection which was broken by changing the return value logic.
BR, Nikolaus
Am Thu, 21 Aug 2025 20:54:41 +0200 schrieb "H. Nikolaus Schaller" hns@goldelico.com:
Am 21.08.2025 um 20:15 schrieb Andreas Kemnade andreas@kemnade.info:
Hi,
Am Mon, 21 Jul 2025 14:46:09 +0200 schrieb "H. Nikolaus Schaller" hns@goldelico.com:
Since commit
commit f16d9fb6cf03 ("power: supply: bq27xxx: Retrieve again when busy")
the console log of some devices with hdq but no bq27000 battery (like the Pandaboard) is flooded with messages like:
[ 34.247833] power_supply bq27000-battery: driver failed to report 'status' property: -1
as soon as user-space is finding a /sys entry and trying to read the "status" property.
It turns out that the offending commit changes the logic to now return the value of cache.flags if it is <0. This is likely under the assumption that it is an error number. In normal errors from bq27xxx_read() this is indeed the case.
But there is special code to detect if no bq27000 is installed or accessible through hdq/1wire and wants to report this. In that case, the cache.flags are set (historically) to constant -1 which did make reading properties return -ENODEV. So everything appeared to be fine before the return value was fixed. Now the -1 is returned as -ENOPERM instead of -ENODEV, triggering the error condition in power_supply_format_property() which then floods the console log.
So we change the detection of missing bq27000 battery to simply set
cache.flags = -ENODEV
instead of -1.
This all is a bit inconsistent, the offending commit makes it worse. Normally devices appear only in /sys if they exist. Regarding stuff in /sys/class/power_supply, input power supplies might be there or not, but there you can argument that the entry in /sys/class/power_supply only means that there is a connector for connecting a supply.
Indeed. If there is an optional bq27000 hdq battery the entry exists.
Which is the condition that there is an optional bq27000 battery? w1 might be enabled for other reasons. The bq27000 is not the only w1 chip in the world. BTW: I have removed the battery from my macbook and there is no battery entry in /sys/class/power_supply. Same with another laptop.
But having the battery entry everywhere looks like waste. If would expect the existence of a battery bay in the device where the common battery is one with a bq27xxx.
I think the flaw you are mentioning is a completely diffent one. It comes from that the 1-wire or hdq interface of some omap processors is enabled in the .dtsi by default instead of disabling it like other interfaces (e.g. mcbsp1). E.g. for omap3 hdqw1w:
https://elixir.bootlin.com/linux/v6.16.1/source/arch/arm/boot/dts/ti/omap/om...
And we should have the dts for the boards enable it only if the hdq interface is really in use and there is a chance that a bq27000 can be connected. In that case the full /sys entry is prepared but returns -ENODEV if the battery is missing, which is then exactly the right error return (instead of -EPERM triggering the console message).
And why do you think bq27000 should behave different than max1721x_battery or ds2780_battery or ds2781_battery? If I enable the drivers there is no additional battery in /sys/class/power_supply! Why should everyone have a bq27000 in /sys/class/power_supply if the driver is enabled and w1 is used for something? I wonder if the -ENODEV should be catched earlier.
Regards, Andreas
Hi,
Am 21.08.2025 um 22:05 schrieb Andreas Kemnade andreas@kemnade.info:
Am Thu, 21 Aug 2025 20:54:41 +0200 schrieb "H. Nikolaus Schaller" hns@goldelico.com:
Am 21.08.2025 um 20:15 schrieb Andreas Kemnade andreas@kemnade.info:
Hi,
Am Mon, 21 Jul 2025 14:46:09 +0200 schrieb "H. Nikolaus Schaller" hns@goldelico.com:
Since commit
commit f16d9fb6cf03 ("power: supply: bq27xxx: Retrieve again when busy")
the console log of some devices with hdq but no bq27000 battery (like the Pandaboard) is flooded with messages like:
[ 34.247833] power_supply bq27000-battery: driver failed to report 'status' property: -1
as soon as user-space is finding a /sys entry and trying to read the "status" property.
It turns out that the offending commit changes the logic to now return the value of cache.flags if it is <0. This is likely under the assumption that it is an error number. In normal errors from bq27xxx_read() this is indeed the case.
But there is special code to detect if no bq27000 is installed or accessible through hdq/1wire and wants to report this. In that case, the cache.flags are set (historically) to constant -1 which did make reading properties return -ENODEV. So everything appeared to be fine before the return value was fixed. Now the -1 is returned as -ENOPERM instead of -ENODEV, triggering the error condition in power_supply_format_property() which then floods the console log.
So we change the detection of missing bq27000 battery to simply set
cache.flags = -ENODEV
instead of -1.
This all is a bit inconsistent, the offending commit makes it worse. Normally devices appear only in /sys if they exist. Regarding stuff in /sys/class/power_supply, input power supplies might be there or not, but there you can argument that the entry in /sys/class/power_supply only means that there is a connector for connecting a supply.
Indeed. If there is an optional bq27000 hdq battery the entry exists.
Which is the condition that there is an optional bq27000 battery?
If there is no bq27000 battery, hdq reads 8 bits of 0xff (no client on the 1 bit serial bus with pull-up) as the "value" of the battery status register. If there is a battery connected, the value is defined and not 0xff.
See 3dd843e1c26a023dc8d776e5d984c635c642785f
w1 might be enabled for other reasons. The bq27000 is not the only w1 chip in the world.
In these cases the bq27xxx driver does not need to be present and does not interfere.
BTW: the bq27000 is unconditionally added to the hdq subsystem as soon as CONFIG_BATTERY_BQ27XXX_HDQ is configured. On every system. So the alternative to disabling hdq on the processor and enabling in the board specific DTB is to unconfigure CONFIG_BATTERY_BQ27XXX_HDQ if hdq is used for something else.
BTW: I have removed the battery from my macbook and there is no battery entry in /sys/class/power_supply. Same with another laptop.
Does the entry disappear if you remove the battery while powered from AC and come back on re-insertion of the battery?
Do they use hdq at all? With i2c it is easier to detect a "no response" during probe or operation. But still i2c assumes that a chip responds at boot or never (or user-space must run a timer to reprobe every now and then).
IMHO they are not prepared to handle the use case we have for the bq27000 and should therefore not be the role model.
But having the battery entry everywhere looks like waste. If would expect the existence of a battery bay in the device where the common battery is one with a bq27xxx.
I think the flaw you are mentioning is a completely diffent one. It comes from that the 1-wire or hdq interface of some omap processors is enabled in the .dtsi by default instead of disabling it like other interfaces (e.g. mcbsp1). E.g. for omap3 hdqw1w:
https://elixir.bootlin.com/linux/v6.16.1/source/arch/arm/boot/dts/ti/omap/om...
And we should have the dts for the boards enable it only if the hdq interface is really in use and there is a chance that a bq27000 can be connected. In that case the full /sys entry is prepared but returns -ENODEV if the battery is missing, which is then exactly the right error return (instead of -EPERM triggering the console message).
And why do you think bq27000 should behave different than max1721x_battery or ds2780_battery or ds2781_battery?
I have looked into the ds2780 code but do not understand how they handle the case that the battery is removed or swapped or inserted during operation (while on external power supply).
The max1721x is different. At least from looking into code it behaves exactly the same as the bq27000. There is a POWER_SUPPLY_PROP_PRESENT. Which can always be read. It does this by reading the MAX172XX_REG_STATUS and detecting that some bit (MAX172XX_BAT_PRESENT) is not set. This can either mean no battery connected to the chip or the chip (built into the battery case) is not connected to the hdq bus.
I can not test but would therefore assume the same for the max1721. As soon as you configure it for a hdq capable device/kernel there should be a /sys entry for it with present == 0.
BTW: all bq27xxx gauges have this property, not only the bq27000.
If I enable the drivers there is no additional battery in /sys/class/power_supply!
Which is surprising because then the max1721x can never report "no battery present". Unless it is always sitting on the main board and the battery is connected or not.
Or there is some special logic in the max1721x probe which can detect during boot that the chip is really connected. But then you can not remove a battery with built-in max1721x because it must be installed on first boot and can not be inserted later.
Why should everyone have a bq27000 in /sys/class/power_supply if the driver is enabled and w1 is used for something? I wonder if the -ENODEV should be catched earlier.
What do you mean with "catched earlier"? What is your proposal?
Well, as proposed by Jerry earlier, it appears as if it can also be handled in bq27xxx_battery_hdq_read() by detecting the register BQ27XXX_REG_FLAGS and the read value 0xff and return -ENODEV.
Then it would be constrained to the bq27000 - but still not solve your issue that boards may not have a bq27000 option on their hdq bus...
Anyways this discussion now goes far beyond fixing the regression introduced by
f16d9fb6cf03 ("power: supply: bq27xxx: Retrieve again when busy")
So I'd suggest to fix the regression first and then add new functions or changes (for handling removable chips and removable batteries reasonably) in a separate patch or series. Having separate patches improves bisectability and is easier to track what has been changed (for different reasons).
BR, Nikolaus
Hi,
Am 22.08.2025 um 08:51 schrieb H. Nikolaus Schaller hns@goldelico.com:
What do you mean with "catched earlier"? What is your proposal?
Well, as proposed by Jerry earlier, it appears as if it can also be handled in bq27xxx_battery_hdq_read() by detecting the register BQ27XXX_REG_FLAGS and the read value 0xff and return -ENODEV.
I tried this but there are more locations where BQ27XXX_REG_FLAGS are read and where the reading code is not prepared to receive an -ENODEV. This will for example emit
[ 293.389831] w1_slave_driver 01-000000000000: error reading flags
each time the battery is removed. And in some race cases (a read of the full /sys properties is already in progress), there may be more than one such message. That is not nice to replace one console message with another...
So I am not sure if it is a good idea to make the lowest layer ("catched earlier") read function detect -ENODEV based on the register number. It can not know what the next layer wants to do with the result.
BR, Nikolaus
linux-stable-mirror@lists.linaro.org
-
Andreas Kemnade -
H. Nikolaus Schaller -
Jerry Lv -
Jerry Lv