Thread (7 messages) 7 messages, 3 authors, 2021-06-19

Re: [PATCH] power: supply: cw2015: Add CHARGE_NOW support

From: Tobias Schramm <t.schramm@manjaro.org>
Date: 2021-06-19 13:48:20

Hey Paul,

Am 19.06.21 um 12:21 schrieb Paul Fertser:
...
After reading the datasheet for the gauge I'm rather disappointed. Not
using a shunt resistor for real current measurements and instead
relying on some magic doesn't make any sense in my book.
I felt similarly when reading it for the first time. However, by now some
more well known manufacturers have started offering similar products [1].
It seems the manufacturers are relying on internal resistance of the 
battery to measure current draw.
In general it seems to work pretty well in the Pine64 Pinebook Pro for 
about two years now.
So to sum up, what you can get from the chip itself:

0. Battery voltage; pretty accurate.

1. Its idea of current state of charge; this is percentage relative to
the charge_full; I'd expect that to be reasonably accurate for not too
worn-out batteries.

2. Its idea of the remaining run time; involves secret magic to
somehow guess the current; I'd expect this to be relatively
inaccurate, would be interesting to learn how this performs for your
real life loads.
That is one of the characteristics I find very surprising. Even after 
over a year of use the remaining runtime estimation still seems to be 
pretty accurate. Maybe I got lucky but it works remarkably well :)
That's it. We can't learn charge_full and current_now.


Your driver code always reports charge_full equal to
charge_full_design which is plain incorrect IMHO. It's just wrong and
misleading, as after e.g. 2 years of usage the battery might lose half
its capacity; and people are used to get real data from power_supply
subsystem to be able to judge about battery health and performance.
It most definitely is. I'm not really happy with reporting them either.
However it seems that a lot of battery widgets get very confused if 
those stats are not reported.
Thus I went for some level of guestimation there. Please feel free to 
remove them and test whether all the usual battery monitoring tools 
still work. I'll be more than happy to remove them if it is not a 
problem anymore.
Now you're also using the same value to calculate the current
reported. But the CW2015 datasheet says that SOC is relative to the
full capacity, not full design capacity, so the current you report
might get wildly inaccurate too. Same about charge_now that Martin
added: you just can't rely on unknown values when doing calculations.


If I was using this driver I would certainly prefer it to _not_ report
any grossly inaccurate guessimations. So I propose to remove
POWER_SUPPLY_PROP_CHARGE_FULL, POWER_SUPPLY_PROP_CHARGE_NOW and
POWER_SUPPLY_PROP_CURRENT_NOW altogether since they can't be
meaningfully obtained from anywhere. Please do not fool userspace into
thinking they have some information when in fact it's pulled out of
thin air.

I'm not the authority here so it would be nice to hear the opinion of
the subsystem maintainers, adding Sebastian to Cc. Ready to be proven
wrong :)
Neither am I. The supported parameters were mostly chosen based on the 
Android
reference code provided to me by CellWise and user feedback (battery 
applet compatibility). I'd hate breaking the latter.

Cheers,
Tobias

[1] https://www.ti.com/lit/ds/symlink/bq27621-g1.pdf
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help