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