| Message ID | 20200315151206.30909-6-spinal.by@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/15] power: supply: cpcap-battery: Fix battery full status reporting | expand |
* Arthur Demchenkov <spinal.by@gmail.com> [200315 15:15]: > If the user provides us with charge_full value (which it could save in a > permanent storage between reboots), initialize low and high counter_uah > with calculated values. Hmm I'm getting -EINVAL when I do echo 1600000 > charge_now but yet the value does get updated? And I'm still not seeing capacity show up after that though even with full battery.. I think we should be able to calculate it if either a high or low value has been seen? Also, we should have the driver default to using the charge_full_design value if nothing is written from userspace and we see a high or low value. Maybe some pessimistic estimate could be used instead of just using charge_full_design if no value is initialized from the userspace? Something like (charge_full_design / 4) * 3 maybe? Yes it will be off, but the driver should work also without user interaction :) Regards, Tony
Hi. > Hmm I'm getting -EINVAL when I do echo 1600000 > charge_now but yet the > value does get updated? Hm. I'm getting "Permission denied" when trying to do this. Anyway, charge_now value is not supposed to be initialized by the user. Only charge_full should be writable. > And I'm still not seeing capacity show up after that though even with > full battery.. I think we should be able to calculate it if either a > high or > low value has been seen? There are two steps needed to calibrate the battery: to hit the "fully charged" state and to hit "battery empty" state. When the both states were hit the driver will initialize charge_full value. And it will start reporting correct charge_now and capacity (battery charge percentage) values. Once the charge_full value is calculated it's recommended to be saved by the user to a permanent storage between reboots. Saving/restoring the value can be done in init scripts. For saving the calibration value just use the command like: cat /sys/class/power_supply/battery/charge_full > /battery_cf To restore (after device reboot or power on): cat /battery_cf > /sys/class/power_supply/battery/charge_full This will allow the kernel to do fast calibration. I.e. you will only need to fully charge _OR_ fully discharge the battery to start seeing correct charge_now and capacity values. > Also, we should have the driver default to using the charge_full_design > value if nothing is written from userspace and we see a high or low > value. I'd prefer to have charge_full value undefined until the battery is calibrated. This way the userspace can estimate current battery capacity using voltage as a fallback for uncalibrated battery. The voltage estimation will be way more accurate than having charge_full = charge_full_design on pretty old Droid 4 batteries. For example, my battery after calibration has about 1000 mAh, while charge_full_design is 1740000. > Maybe some pessimistic estimate could be used instead of just using > charge_full_design if no value is initialized from the userspace? > Something like (charge_full_design / 4) * 3 maybe? It's better to rely on voltage estimated percentage. In Maemo Leste we patched upower to use the following formula, which gives pretty good results: percentage = (voltage - voltage_empty) / (voltage_full - voltage_empty) * 100 Actually, the formula we use is a bit more complicated. If you're curious, here's how it's actually done: https://github.com/maemo-leste/upower/blob/master/src/linux/up-device-supply.c#L756 -- Best regards, Spinal
* Arthur D. <spinal.by@gmail.com> [200321 22:09]: > Hi. > > > Hmm I'm getting -EINVAL when I do echo 1600000 > charge_now but yet the > > value does get updated? > > Hm. I'm getting "Permission denied" when trying to do this. > Anyway, charge_now value is not supposed to be initialized by the user. > Only charge_full should be writable. Sorry I meant writing to charge_full :) Anyways, looks like echo -n works with no errors: # echo 1305000 > charge_full bash: echo: write error: Invalid argument # echo -n 1305000 > charge_full > > And I'm still not seeing capacity show up after that though even with > > full battery.. I think we should be able to calculate it if either a > > high or > > low value has been seen? > > There are two steps needed to calibrate the battery: to hit the "fully > charged" > state and to hit "battery empty" state. When the both states were hit the > driver > will initialize charge_full value. And it will start reporting correct > charge_now > and capacity (battery charge percentage) values. Hmm OK. So far no luck triggering that though. > Once the charge_full value is calculated it's recommended to be saved by the > user > to a permanent storage between reboots. > > Saving/restoring the value can be done in init scripts. > > For saving the calibration value just use the command like: > cat /sys/class/power_supply/battery/charge_full > /battery_cf > > To restore (after device reboot or power on): > cat /battery_cf > /sys/class/power_supply/battery/charge_full OK > This will allow the kernel to do fast calibration. I.e. you will > only need to fully charge _OR_ fully discharge the battery to start > seeing correct charge_now and capacity values. OK > > Also, we should have the driver default to using the charge_full_design > > value if nothing is written from userspace and we see a high or low > > value. > > I'd prefer to have charge_full value undefined until the battery is > calibrated. > This way the userspace can estimate current battery capacity using voltage > as a > fallback for uncalibrated battery. The voltage estimation will be way more > accurate than having charge_full = charge_full_design on pretty old Droid 4 > batteries. For example, my battery after calibration has about 1000 mAh, > while charge_full_design is 1740000. OK > > Maybe some pessimistic estimate could be used instead of just using > > charge_full_design if no value is initialized from the userspace? > > Something like (charge_full_design / 4) * 3 maybe? > > It's better to rely on voltage estimated percentage. > In Maemo Leste we patched upower to use the following formula, which gives > pretty > good results: > > percentage = (voltage - voltage_empty) / (voltage_full - voltage_empty) * > 100 > > Actually, the formula we use is a bit more complicated. OK > If you're curious, here's how it's actually done: > https://github.com/maemo-leste/upower/blob/master/src/linux/up-device-supply.c#L756 Thanks, I'll give the calibration another try. Maybe I did not wait low enough, I think I went down to 3.3V. Regards, Tony
diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c index db1b519e87c6..669ed1513201 100644 --- a/drivers/power/supply/cpcap-battery.c +++ b/drivers/power/supply/cpcap-battery.c @@ -434,7 +434,7 @@ static bool cpcap_battery_low(struct cpcap_battery_ddata *ddata) static int cpcap_battery_update_status(struct cpcap_battery_ddata *ddata) { - struct cpcap_battery_state_data state, *latest, *previous, *tmp; + struct cpcap_battery_state_data state, *latest, *previous, *low, *high; ktime_t now; int error; @@ -464,15 +464,40 @@ static int cpcap_battery_update_status(struct cpcap_battery_ddata *ddata) memcpy(latest, &state, sizeof(*latest)); if (cpcap_battery_full(ddata)) { - tmp = cpcap_battery_get_highest(ddata); + high = cpcap_battery_get_highest(ddata); /* Update highest charge seen? */ - if (latest->counter_uah <= tmp->counter_uah) - memcpy(tmp, latest, sizeof(*tmp)); + if (latest->counter_uah <= high->counter_uah || + !high->voltage) { + memcpy(high, latest, sizeof(*high)); + + low = cpcap_battery_get_lowest(ddata); + if (low->voltage && low->voltage != -1) + ddata->charge_full = + low->counter_uah - high->counter_uah; + else if (ddata->charge_full) { + /* Initialize with user provided data */ + low->counter_uah = + high->counter_uah + ddata->charge_full; + /* Mark it as initialized */ + low->voltage = -1; + } + } } else if (cpcap_battery_low(ddata)) { - tmp = cpcap_battery_get_lowest(ddata); + low = cpcap_battery_get_lowest(ddata); /* Update lowest charge seen? */ - if (latest->counter_uah >= tmp->counter_uah) - memcpy(tmp, latest, sizeof(*tmp)); + if (latest->counter_uah >= low->counter_uah || + !low->voltage) { + memcpy(low, latest, sizeof(*low)); + + high = cpcap_battery_get_highest(ddata); + if (high->voltage) + ddata->charge_full = + low->counter_uah - high->counter_uah; + else if (ddata->charge_full) + /* Initialize with user provided data */ + high->counter_uah = + low->counter_uah - ddata->charge_full; + } } return 0;
If the user provides us with charge_full value (which it could save in a permanent storage between reboots), initialize low and high counter_uah with calculated values. Signed-off-by: Arthur Demchenkov <spinal.by@gmail.com> --- drivers/power/supply/cpcap-battery.c | 39 +++++++++++++++++++++++++++++------- 1 file changed, 32 insertions(+), 7 deletions(-)