[Letux-kernel] [PATCH] power: supply: bq27xxx: do not report bogus zero values

[H. Nikolaus Schaller]

Hi,


> Am 07.02.2025 um 22:51 schrieb Sicelo A. Mhlongo <absicsz at gmail.com>:
> 
> On the bq27x00 and bq27x10 variants, a number of conditions may reset the value
> stored in the NAC register. This will cause capacity, energy, and charge to
> report the value 0, even when the battery is full. On the other hand, the chip
> also provides a flag, EDVF, which accurately detects the true battery empty
> condition, when capacity, charge, and energy are really 0.  Therefore, discard
> readings for these properties if their value is 0 while EDVF is unset.
> 
> Tested on the Nokia N900 with bq27200.

I finally found time to test this patch on the GTA04 which uses an OpenMoko HF08
battery which comes with a built-in bq27000.

The result is that I can't read the /sys/class/power_supply/bq27000-battery/charge_now
at all and always get EINVAL. Independently of the charge level reported without
your patch.

If I remove this patch again, the values are reasonable as in all the years before.

What I don't know is how and to which values the EEPROM of the bq27000 has been
factory programmed so that the HF08 battery maybehave differently from yours.

Finally, I am not aware of reports from GTA04 users that the value stored in the NAC
register is being reset as you describe and capacity, energy and charge to be falsely
reported as 0.

So please restrict this patch to the N900.

BR and thanks,
Nikolaus

> Signed-off-by: Sicelo A. Mhlongo <absicsz at gmail.com>
> ---
> drivers/power/supply/bq27xxx_battery.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
> index 90a5bccfc6b9..df92f55c63f6 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -2115,6 +2115,10 @@ static int bq27xxx_battery_get_property(struct power_supply *psy,
> break;
> case POWER_SUPPLY_PROP_CAPACITY:
> ret = bq27xxx_simple_value(di->cache.capacity, val);
> + /* If 0 is reported, it is expected that EDVF is also set */
> + if (!ret && di->opts & BQ27XXX_O_ZERO &&
> +   !(di->cache.flags & BQ27000_FLAG_EDVF))
> + return -EINVAL;
> break;
> case POWER_SUPPLY_PROP_CAPACITY_LEVEL:
> ret = bq27xxx_battery_capacity_level(di, val);
> @@ -2138,10 +2142,15 @@ static int bq27xxx_battery_get_property(struct power_supply *psy,
> val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
> break;
> case POWER_SUPPLY_PROP_CHARGE_NOW:
> - if (di->regs[BQ27XXX_REG_NAC] != INVALID_REG_ADDR)
> + if (di->regs[BQ27XXX_REG_NAC] != INVALID_REG_ADDR) {
> ret = bq27xxx_battery_read_nac(di, val);
> - else
> + /* If 0 is reported, it is expected that EDVF is also set */
> + if (!ret && di->opts & BQ27XXX_O_ZERO &&
> +   !(di->cache.flags & BQ27000_FLAG_EDVF))
> + return -EINVAL;
> + } else {
> ret = bq27xxx_battery_read_rc(di, val);
> + }
> break;
> case POWER_SUPPLY_PROP_CHARGE_FULL:
> ret = bq27xxx_battery_read_fcc(di, val);
> @@ -2163,6 +2172,10 @@ static int bq27xxx_battery_get_property(struct power_supply *psy,
> break;
> case POWER_SUPPLY_PROP_ENERGY_NOW:
> ret = bq27xxx_battery_read_energy(di, val);
> + /* If 0 is reported, it is expected that EDVF is also set */
> + if (!ret && di->opts & BQ27XXX_O_ZERO &&
> +   !(di->cache.flags & BQ27000_FLAG_EDVF))
> + return -EINVAL;
> break;
> case POWER_SUPPLY_PROP_POWER_AVG:
> ret = bq27xxx_battery_pwr_avg(di, val);
> -- 
> 2.47.2
> 
>