Another comment (in code section)
> Am 27.02.2025 um 18:14 schrieb H. Nikolaus Schaller <h...@goldelico.com>:
>
> Hi,
>
>
>> Am 07.02.2025 um 22:51 schrieb Sicelo A. Mhlongo <absi...@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 <absi...@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;
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.
>> 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
>>
>>
>
>
