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

[Paul Fertser <fercerpav@gmail.com>]

Hi Tobias,

On Sat, Jun 19, 2021 at 11:52:34AM +0200, Tobias Schramm wrote:
> > Are you sure the chip reports current state of charge relative to the
> > full design capacity rather than to the latest auto-calibrated full
> > capacity? That would mean that over time as the cells wear out
> > cw_bat->soc (PROP_CAPACITY) is never going to be reaching 100; does
> > this match your experience?
> > 
> As far as I'm aware the gauge reports SoC relative to what it believes to be
> the the current, auto-calibrated full battery capacity.
> However, since I don't know how to extract that value from the gauge we just
> always assume it to be the design capacity [1].
> Since we inquire the gauge about current SoC directly [2] there is no way
> for any miscalculation on our end preventing it from reaching 100%.

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.

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

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

-- 
Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software!
mailto:fercerpav@gmail.com