Message-Id: <10174C85-591A-4DCB-A44E-95F2ACE75E99@goldelico.com>
Date: Thu, 21 Aug 2025 20:54:41 +0200
From: "H. Nikolaus Schaller" <hns@...delico.com>
To: Andreas Kemnade <andreas@...nade.info>
Cc: Sebastian Reichel <sre@...nel.org>,
 Jerry Lv <Jerry.Lv@...s.com>,
 Pali Rohár <pali@...nel.org>,
 linux-pm@...r.kernel.org,
 linux-kernel@...r.kernel.org,
 letux-kernel@...nphoenux.org,
 stable@...r.kernel.org,
 kernel@...a-handheld.com
Subject: Re: [PATCH] power: supply: bq27xxx: fix error return in case of no
 bq27000 hdq battery



> Am 21.08.2025 um 20:15 schrieb Andreas Kemnade <andreas@...nade.info>:
> 
> Hi,
> 
> Am Mon, 21 Jul 2025 14:46:09 +0200
> schrieb "H. Nikolaus Schaller" <hns@...delico.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

Powered by blists - more mailing lists