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

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/omap3.dtsi#L502

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/omap3.dtsi#L502
> 
> 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/omap3.dtsi#L502
>> 
>> 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