linux-omap.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tony Lindgren <tony@atomide.com>
To: "Arthur D." <spinal.by@gmail.com>
Cc: Pavel Machek <pavel@ucw.cz>, Merlijn Wajer <merlijn@wizzup.org>,
	Sebastian Reichel <sre@kernel.org>,
	linux-pm@vger.kernel.org, linux-omap@vger.kernel.org
Subject: Re: [PATCH 1/3] RFC: power: supply: cpcap-battery: Add helper for rough capacity
Date: Tue, 10 Mar 2020 08:39:51 -0700	[thread overview]
Message-ID: <20200310153951.GR37466@atomide.com> (raw)
In-Reply-To: <op.0g8ykrhnhxa7s4@supervisor.net28>

* Arthur D. <spinal.by@gmail.com> [200310 04:55]:
> Hi, Tony.
> 
> I used the kernel with the patch serieas applied for a while.
> 
> What I expected from the userspace perspective was having
> /sys/class/power_supply/battery/capacity undefined until
> kernel calculates more or less accurate values for it.
> 
> Until then, the userspace should estimate percentage on its own
> using voltage and current values provided by the kernel.
> Like it's already done with bq27200 on Nokia N900.
> 
> Right now the values which the kernel provides with
> /sys/class/power_supply/battery/capacity after a system boot
> are confusing.

OK yeah I like the idea of not showing anything until userspace
as configured estimated high and low values.

> The user can see the battery plugin doesn't change its gauge
> gradually like it was done with previous kernel versions.
> Sometimes it gets suddenly empty, sometimes it changes from empty
> to half full. And it always reports "battery full" in advance -
> when the battery is being charged with relatively high current.
> 
> The following part of mail dedicated to what I think should be fixed
> in the commits.
> 
> 1) RFC: power: supply: cpcap-battery: Add helper for rough capacity
> 
> CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_FULL, 4200000 - 18000, 100)
>
> This line seems wrong, because Droid4 full battery is 4.35V, not 4.2V.

Hmm yeah this is now configurable and defaults to 4.2V so we
should use the confgured value. The reason we default to 4.2V
now is because we suspect that if left connected to the charger
at higher than 4.2V the battery ages faster.

> 2.1) RFC: power: supply: cpcap-battery: Implement capacity percentage
> 
> val->intval = (ddata->config.info.charge_full_design - delta) * 100;
> val->intval /= ddata->config.info.charge_full_design;
> 
> IMHO, charge_full_design should never be used in battery charge
> percentage calculations. Li-ion batteries loose their capacity
> with age. Therefore such calculations are likely to always be wrong.
> 
> I'd prefer to have the full charge value to be dynamically calculated by
> the kernel with a formula like this:
> charge_full = high->counter_uah - low->counter_uah;

OK makes sense. I guess we can assuem if both are set to the
the same number like 0 on boot, there's no estimate available.

> This which will give us accurate estimation for the battery charge, esti-
> mated time left on the battery and so on. It would be good, if we allow
> the userspace to store the full charge value between reboots and to feed
> it to the kernel after, so it could start providing accurated data faster:
> the battery should only go full or empty once and we are ready.

Yes userspace is the only place that can eventually generate accurate
estimate based on battery wear, temperature etc.

> I'd like to mention explicitly: I think the kernel should return -ENODATA
> for capacity values until the battery is "calibrated". By "calibration"
> I mean having high and low counter_uah values initialized with the data
> collected when the battery went a full charge or discharge cycle (from
> empty to full or vice versa).

Yes I like this idea. That leaves out the need for poor estimates
in the kernel.

I guess we should have the following new properties:

POWER_SUPPLY_PROP_CHARGE_COUNTER_FULL
POWER_SUPPLY_PROP_CHARGE_COUNTER_EMPTY

And these would be 0 on boot and then userspace can
update these based on battery data.

Regards,

Tony

  reply	other threads:[~2020-03-10 15:39 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-19 20:11 [PATCH 1/3] RFC: power: supply: cpcap-battery: Add helper for rough capacity Tony Lindgren
2020-01-19 20:11 ` [PATCH 2/3] RFC: power: supply: cpcap-battery: Save high and low states Tony Lindgren
2020-01-19 20:11 ` [PATCH 3/3] RFC: power: supply: cpcap-battery: Implement capacity percentage Tony Lindgren
2020-01-21  9:57 ` [PATCH 1/3] RFC: power: supply: cpcap-battery: Add helper for rough capacity Pavel Machek
2020-01-21 10:42   ` Merlijn Wajer
2020-01-21 22:25     ` Pavel Machek
2020-01-23 16:02       ` Tony Lindgren
2020-03-10  4:54         ` Arthur D.
2020-03-10 15:39           ` Tony Lindgren [this message]
2020-03-10 18:35             ` Arthur D.
2020-03-11 14:23               ` Tony Lindgren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200310153951.GR37466@atomide.com \
    --to=tony@atomide.com \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=merlijn@wizzup.org \
    --cc=pavel@ucw.cz \
    --cc=spinal.by@gmail.com \
    --cc=sre@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).