diff mbox series

power: supply: bq27xxx: do not report bogus zero values

Message ID 20250207220605.106768-1-absicsz@gmail.com
State Superseded
Headers show
Series power: supply: bq27xxx: do not report bogus zero values | expand

Commit Message

Sicelo A. Mhlongo Feb. 7, 2025, 9:51 p.m. UTC
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.

Signed-off-by: Sicelo A. Mhlongo <absicsz@gmail.com>
---
 drivers/power/supply/bq27xxx_battery.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

Comments

H. Nikolaus Schaller Feb. 27, 2025, 5:14 p.m. UTC | #1
Hi,


> Am 07.02.2025 um 22:51 schrieb Sicelo A. Mhlongo <absicsz@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@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
> 
>
H. Nikolaus Schaller Feb. 27, 2025, 5:48 p.m. UTC | #2
Another comment (in code section)

> Am 27.02.2025 um 18:14 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
> 
> Hi,
> 
> 
>> Am 07.02.2025 um 22:51 schrieb Sicelo A. Mhlongo <absicsz@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@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
>> 
>> 
> 
>
Sicelo A. Mhlongo Feb. 28, 2025, 8:02 a.m. UTC | #3
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
H. Nikolaus Schaller Feb. 28, 2025, 8:27 a.m. UTC | #4
Hi,

> Am 28.02.2025 um 09:02 schrieb 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.

Ah, ok. Of course it obviously will be reset if the chip becomes disconnected from all power.

> 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.

No worry. That is what review is good for.

> 
> Will you submit the fix, or I should?

I think the patch has not yet been merged anywhere (but I am not sure), so
you better can send a v2 of the series.

And, I think I have found another potential issue.

The function bq27xxx_simple_value() returns the value passed as the first
parameter if it is negative (i.e. read error). And, then, it is not copies
to val->intval.

In that case val->intval may still be 0 and an -EINVAL may be returned
instead of the error return from what is passed as the first parameter to
bq27xxx_simple_value().

So I think it could be better to not check for 

>> if (!val->intval && 

but

>> if (!di->cache.capacity && 

Alternatively if it should not rely on knowledge about the internaly of
bq27xxx_simple_value()

>> + if (!ret && !val->intval && di->opts & BQ27XXX_O_ZERO &&

Or even a third and maybe best alternative: do the check before calling
bq27xxx_simple_value(). Since setting val->intval and then returning an
error is not needed.


 case POWER_SUPPLY_PROP_CAPACITY:
	/* If 0 is reported, it is expected that EDVF is also set */
	if (!di->cache.capacity && di->opts & BQ27XXX_O_ZERO &&
		!(di->cache.flags & BQ27000_FLAG_EDVF))
		return -EINVAL;
 	ret = bq27xxx_simple_value(di->cache.capacity, val);
	break;

etc.

This should still report the ret error if di->cache.capacity < 0.

BR and thanks,
Nikolaus
Sicelo A. Mhlongo Feb. 28, 2025, 9:54 a.m. UTC | #5
Hi

On Fri, Feb 28, 2025 at 09:27:29AM +0100, H. Nikolaus Schaller wrote:
> > 
> > Will you submit the fix, or I should?
> 
> I think the patch has not yet been merged anywhere (but I am not sure), so
> you better can send a v2 of the series.

It is in linux-next [0]. Not sure if that counts? I guess the only option
now is to submit a follow-up fix?

> And, I think I have found another potential issue.

Thanks for the review. I will do more thorough testing over the weekend
and send the patch.


[0] https://web.git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/power/supply/bq27xxx_battery.c?h=next-20250228&id=f3974aca381e81c0b1418d8ecc12fa62e1e9d31f

Sincerely
Sicelo A. Mhlongo
H. Nikolaus Schaller Feb. 28, 2025, 10:01 a.m. UTC | #6
Hi,

> Am 28.02.2025 um 10:54 schrieb Sicelo <absicsz@gmail.com>:
> 
> Hi
> 
> On Fri, Feb 28, 2025 at 09:27:29AM +0100, H. Nikolaus Schaller wrote:
>>> 
>>> Will you submit the fix, or I should?
>> 
>> I think the patch has not yet been merged anywhere (but I am not sure), so
>> you better can send a v2 of the series.
> 
> It is in linux-next [0]. Not sure if that counts?


Ah, I didn't recognize that (and must admit that I did not search through
linux-next). Sometimes maintainers close a discussion by telling that it
has been merged.

Maybe Sebastian can replace it or recommends a fixup.

> I guess the only option
> now is to submit a follow-up fix?
> 
>> And, I think I have found another potential issue.
> 
> Thanks for the review. I will do more thorough testing over the weekend
> and send the patch.
> 
> 
> [0] https://web.git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/power/supply/bq27xxx_battery.c?h=next-20250228&id=f3974aca381e81c0b1418d8ecc12fa62e1e9d31f
> 
> Sincerely
> Sicelo A. Mhlongo

BR and thanks,
Nikolaus
Sicelo A. Mhlongo March 10, 2025, 5:26 p.m. UTC | #7
Hi Sebastian

On Fri, Feb 28, 2025 at 11:01:35AM +0100, H. Nikolaus Schaller wrote:
> > It is in linux-next [0]. Not sure if that counts?
> 
> Ah, I didn't recognize that (and must admit that I did not search through
> linux-next). Sometimes maintainers close a discussion by telling that it
> has been merged.
> 
> Maybe Sebastian can replace it or recommends a fixup.
> 
> > I guess the only option
> > now is to submit a follow-up fix?

How would you like me to correct the bugs I introduced with the patch in
the subject of this email?

I guess it will either be:

- submit v2 against linux-power-supply/master or
  linux-power-supply/fixes (so the bad commit will be removed from
  future history)
- submit a new patch against linux-power-supply/for-next (so the bad
  commit will always be in the history)
- use a different solution you suggest

I apologize for introducing the bug and have taken the time to more
thoroughly test the changes, with help from HNS. I have the patch ready
now, and just need to be sure how to submit it.

Kind Regards
Sicelo A. Mhlongo
Sebastian Reichel March 12, 2025, 7:19 a.m. UTC | #8
Hi,

On Mon, Mar 10, 2025 at 07:26:51PM +0200, Sicelo wrote:
> Hi Sebastian
> 
> On Fri, Feb 28, 2025 at 11:01:35AM +0100, H. Nikolaus Schaller wrote:
> > > It is in linux-next [0]. Not sure if that counts?
> > 
> > Ah, I didn't recognize that (and must admit that I did not search through
> > linux-next). Sometimes maintainers close a discussion by telling that it
> > has been merged.
> > 
> > Maybe Sebastian can replace it or recommends a fixup.
> > 
> > > I guess the only option now is to submit a follow-up fix?
> 
> How would you like me to correct the bugs I introduced with the patch in
> the subject of this email?
> 
> I guess it will either be:
> 
> - submit v2 against linux-power-supply/master or
>   linux-power-supply/fixes (so the bad commit will be removed from
>   future history)
> - submit a new patch against linux-power-supply/for-next (so the bad
>   commit will always be in the history)
> - use a different solution you suggest
> 
> I apologize for introducing the bug and have taken the time to more
> thoroughly test the changes, with help from HNS. I have the patch ready
> now, and just need to be sure how to submit it.
> 
> Kind Regards
> Sicelo A. Mhlongo

Sorry for the delayed response. You need to prepare a follow-up
patch against power-supply's for-next branch. Dropping the broken
patch would require rebasing the whole branch. The only options
you have are:

1. Revert the original broken patch with a proper long description,
   then create a new fix
2. Add a fix on top of the broken fix

Please don't forget to add

Fixes: f3974aca381e ("power: supply: bq27xxx: do not report bogus zero values")

Thanks,

-- Sebastian
Sicelo A. Mhlongo March 12, 2025, 12:20 p.m. UTC | #9
Hi

On Wed, Mar 12, 2025 at 08:19:44AM +0100, Sebastian Reichel wrote:
> Sorry for the delayed response. You need to prepare a follow-up
> patch against power-supply's for-next branch. Dropping the broken
> patch would require rebasing the whole branch. The only options
> you have are:
> 
> 1. Revert the original broken patch with a proper long description,
>    then create a new fix
> 2. Add a fix on top of the broken fix
> 
> Please don't forget to add
> 
> Fixes: f3974aca381e ("power: supply: bq27xxx: do not report bogus zero values")

Thank you for the guidance. I have taken option 1 in the new patch [0]

Regards
Sicelo


[0] https://lore.kernel.org/linux-pm/20250312121712.146109-1-absicsz@gmail.com/T/#t
Sicelo A. Mhlongo March 12, 2025, 12:28 p.m. UTC | #10
Hi Nikolaus

On Fri, Feb 28, 2025 at 11:01:35AM +0100, H. Nikolaus Schaller wrote:
> > 
> > Thanks for the review. I will do more thorough testing over the weekend
> > and send the patch.

I have made a follow-up patch [0], which produces the following values
on the N900:

bq27200 in 'normal' state:

   DEVTYPE=power_supply
   OF_NAME=bq27200
   OF_FULLNAME=/ocp@68000000/i2c@48072000/bq27200@55
   OF_COMPATIBLE_0=ti,bq27200
   OF_COMPATIBLE_N=1
   POWER_SUPPLY_NAME=bq27200-0
   POWER_SUPPLY_TYPE=Battery
   POWER_SUPPLY_STATUS=Discharging
   POWER_SUPPLY_HEALTH=Calibration required
   POWER_SUPPLY_PRESENT=1
   POWER_SUPPLY_TECHNOLOGY=Li-ion
   POWER_SUPPLY_CYCLE_COUNT=0
   POWER_SUPPLY_VOLTAGE_MAX_DESIGN=4064000
   POWER_SUPPLY_VOLTAGE_MIN_DESIGN=3000000
   POWER_SUPPLY_VOLTAGE_NOW=3536000
   POWER_SUPPLY_CURRENT_NOW=-432327
   POWER_SUPPLY_POWER_AVG=1146100
   POWER_SUPPLY_CHARGE_FULL_DESIGN=2056320
   POWER_SUPPLY_CHARGE_FULL=2050560
   POWER_SUPPLY_CHARGE_NOW=366680
   POWER_SUPPLY_ENERGY_NOW=1207420
   POWER_SUPPLY_CAPACITY=18
   POWER_SUPPLY_CAPACITY_LEVEL=Normal
   POWER_SUPPLY_TEMP=316
   POWER_SUPPLY_TIME_TO_EMPTY_NOW=4140
   POWER_SUPPLY_TIME_TO_EMPTY_AVG=3780
   POWER_SUPPLY_TYPE=Battery
   POWER_SUPPLY_MANUFACTURER=Texas Instruments

bq27200 in 'broken' state:

   DEVTYPE=power_supply
   OF_NAME=bq27200
   OF_FULLNAME=/ocp@68000000/i2c@48072000/bq27200@55
   OF_COMPATIBLE_0=ti,bq27200
   OF_COMPATIBLE_N=1
   POWER_SUPPLY_NAME=bq27200-0
   POWER_SUPPLY_TYPE=Battery
   POWER_SUPPLY_STATUS=Discharging
   POWER_SUPPLY_HEALTH=Calibration required
   POWER_SUPPLY_PRESENT=1
   POWER_SUPPLY_TECHNOLOGY=Li-ion
   POWER_SUPPLY_CYCLE_COUNT=0
   POWER_SUPPLY_VOLTAGE_MAX_DESIGN=4064000
   POWER_SUPPLY_VOLTAGE_MIN_DESIGN=3000000
   POWER_SUPPLY_VOLTAGE_NOW=3633000
   POWER_SUPPLY_CURRENT_NOW=-397341
   POWER_SUPPLY_POWER_AVG=1432260
   POWER_SUPPLY_CHARGE_FULL_DESIGN=2056320
   POWER_SUPPLY_CHARGE_FULL=2050560
   POWER_SUPPLY_CAPACITY_LEVEL=Normal
   POWER_SUPPLY_TEMP=299
   POWER_SUPPLY_TYPE=Battery
   POWER_SUPPLY_MANUFACTURER=Texas Instruments


Hope this also works fine on your devices.

Thanks and Regards
Sicelo



[0] https://lore.kernel.org/linux-pm/20250312121712.146109-1-absicsz@gmail.com/T/#t
diff mbox series

Patch

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);