Re: [PATCH 1/2] power: supply: sbs-battery: Reject all-zero readings as battery absent

[Sebastian Reichel <sebastian.reichel@collabora.com>]

[-- Attachment #1: Type: text/plain, Size: 2431 bytes --]

Hi,

On Wed, Dec 31, 2025 at 09:31:51AM +0000, LI Qingwu wrote:
> The driver reports battery present when status register read succeeds,
> without checking the actual register values. Some systems return all
> zeros when no battery is connected, causing false presence detection.
> 
> Add validation: when status reads zero, cross-check voltage and capacity.
> Report battery absent only if all three registers return zero.
> 
> Tested on i.MX 8M Plus platform with SBS-compliant battery.
> 
> Signed-off-by: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>
> ---

Can you provide some more details about the platform / SBS compliant
battery?

The SBS battery chip is supposed to be part of the battery [*], so
by removing the battery you are removing the I2C device. Accessing a
non-existing device surely should not give you 0, but an error. That
suggests either your hardware design is broken and you disconnect
the battery cells from the IC or your I2C/SMbus driver is buggy and
should be fixed.

[*] This is the whole point of calling it a 'smart battery'. The
    battery cells and the chip are supposed to be one unit. This
    allows the chip to use a coloumb counter to measure energy
    stored/taken out of the battery and learn how the battery
    behaves.

Greetings,

-- Sebastian

>  drivers/power/supply/sbs-battery.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/power/supply/sbs-battery.c b/drivers/power/supply/sbs-battery.c
> index 943c82ee978f..0b9ecfc1f3f7 100644
> --- a/drivers/power/supply/sbs-battery.c
> +++ b/drivers/power/supply/sbs-battery.c
> @@ -594,9 +594,19 @@ static int sbs_get_battery_presence_and_health(
>  		return ret;
>  	}
>  
> -	if (psp == POWER_SUPPLY_PROP_PRESENT)
> +	if (psp == POWER_SUPPLY_PROP_PRESENT) {
>  		val->intval = 1; /* battery present */
> -	else { /* POWER_SUPPLY_PROP_HEALTH */
> +		if (ret == 0) {
> +			int voltage, capacity;
> +
> +			voltage = sbs_read_word_data(
> +				client, sbs_data[REG_VOLTAGE].addr);
> +			capacity = sbs_read_word_data(
> +				client, sbs_data[REG_CAPACITY].addr);
> +			if (voltage == 0 && capacity == 0)
> +				val->intval = 0;
> +		}
> +	} else { /* POWER_SUPPLY_PROP_HEALTH */
>  		if (sbs_bat_needs_calibration(client)) {
>  			val->intval = POWER_SUPPLY_HEALTH_CALIBRATION_REQUIRED;
>  		} else {
> -- 
> 2.43.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]