Re: [PATCH] power: supply: bq27xxx: do not report bogus zero values

[Sicelo <absicsz@gmail.com>]

Hi

On Thu, Feb 27, 2025 at 06:48:49PM +0100, H. Nikolaus Schaller wrote:
> > 
> > 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.

In the case of the HF08, the value will not get reset since the chip is
inside the battery as describe. However, the patch should be fine for
HF08 and N900, etc., since the EINVAL should only be emitted when it has
been reset. Unfortunately, I made the mistake below...

> 
> You write:
> 
>   Therefore, discard readings for these properties if their value is 0 while EDVF is unset.
> 
> but the 'val' is not checked at all. Only the error return value
> 'ret' of bq27xxx_simple_value() which is 0 if reading was ok.
> 
> So shouldn't that be:
> 
> + if (!val->intval && di->opts & BQ27XXX_O_ZERO &&
> +   !(di->cache.flags & BQ27000_FLAG_EDVF))
> + return -EINVAL;
> 
> This could explain why I always get an -EINVAL here since BQ27000_FLAG_EDVF is likely not set.

You are absolutely correct, Thank you. It seems I spent more time
testing the non-working condition than the working condition. Apologies.

Will you submit the fix, or I should?

Regards
Sicelo A. Mhlongo