linux-kernel - Re: [PATCH] power: supply: bq27xxx: fix error return in case of no bq27000 hdq battery

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <VI1PR02MB10076D58D8B86F8FB50E59AADF422A@VI1PR02MB10076.eurprd02.prod.outlook.com>
Date: Tue, 5 Aug 2025 08:53:12 +0000
From: Jerry Lv <Jerry.Lv@...s.com>
To: "H. Nikolaus Schaller" <hns@...delico.com>, Sebastian Reichel
	<sre@...nel.org>
CC: Pali Rohár <pali@...nel.org>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"letux-kernel@...nphoenux.org" <letux-kernel@...nphoenux.org>,
	"stable@...r.kernel.org" <stable@...r.kernel.org>, "kernel@...a-handheld.com"
	<kernel@...a-handheld.com>, "andreas@...nade.info" <andreas@...nade.info>,
	Hermes Zhang <Hermes.Zhang@...s.com>
Subject: Re: [PATCH] power: supply: bq27xxx: fix error return in case of no
 bq27000 hdq battery

________________________________________
From: H. Nikolaus Schaller <hns@...delico.com>
Sent: Monday, July 21, 2025 8:46 PM
To: Sebastian Reichel; Jerry Lv
Cc: Pali Rohár; linux-pm@...r.kernel.org; linux-kernel@...r.kernel.org; letux-kernel@...nphoenux.org; stable@...r.kernel.org; kernel@...a-handheld.com; andreas@...nade.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@...delico.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@...s.com>
Cc: stable@...r.kernel.org
Signed-off-by: H. Nikolaus Schaller <hns@...delico.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