RFC: power: supply: cpcap-battery: Add helper for rough capacity

Get a rough battery charge estimate until we've seen a high or low battery level. After that we can use the coulomb counter to calculate the battery capacity.

Note that I should probably update this to support ocv-capacity-table before this makes sense to apply. With ocv-capacity-table we should be able to estimate battery state as described in the documentation for Documentation/devicetree/bindings/power/supply/battery.txt.

drivers/power/supply/cpcap-battery.c +12 -0

+struct cpcap_battery_capacity {
+	int capacity;
+	int voltage;
+	int percentage;
+};
+
+#define CPCAP_CAP(l, v, p)			\
+{						\
+	.capacity = (l),			\
+	.voltage = (v),				\
+	.percentage = (p),			\
+},

Hi!

Quoted 4 lines

Get a rough battery charge estimate until we've seen a high or low battery level. After that we can use the coulomb counter to calculate the battery capacity.

Quoted 4 lines

Note that I should probably update this to support ocv-capacity-table before this makes sense to apply. With ocv-capacity-table we should be able to estimate battery state as described in the documentation for Documentation/devicetree/bindings/power/supply/battery.txt.

Maybe we should let userspace do that?

https://github.com/pavelmachek/libbattery

Quoted 9 lines

Quoted 8 lines

Get a rough battery charge estimate until we've seen a high or low battery level. After that we can use the coulomb counter to calculate the battery capacity.

Note that I should probably update this to support ocv-capacity-table before this makes sense to apply. With ocv-capacity-table we should be able to estimate battery state as described in the documentation for Documentation/devicetree/bindings/power/supply/battery.txt.

Quoted 3 lines

Maybe we should let userspace do that?

https://github.com/pavelmachek/libbattery

Unless this gets integrated in UPower, My point of view is that userspace probably wants some uniform approach. Kernel interface definitely is one uniform interface that would also get picked up by UPower.

Hi!

Quoted 13 lines

Quoted 12 lines

Quoted 8 lines

Get a rough battery charge estimate until we've seen a high or low battery level. After that we can use the coulomb counter to calculate the battery capacity.

Note that I should probably update this to support ocv-capacity-table before this makes sense to apply. With ocv-capacity-table we should be able to estimate battery state as described in the documentation for Documentation/devicetree/bindings/power/supply/battery.txt.

Maybe we should let userspace do that?

https://github.com/pavelmachek/libbattery

Quoted 3 lines

Unless this gets integrated in UPower, My point of view is that userspace probably wants some uniform approach. Kernel interface definitely is one uniform interface that would also get picked up by UPower.

I see that it is convenient to have it in kernel. I just assumed Sebastian would not take that, because it does not _need_ to be in kernel...

And maybe it is easier in userspace, since userspace has access to persistent storage, so it can store detailed battery calibration parameters? (And Gnome/Mate even does something like that, see mate-power-statistics).

So... not my decision. Don't care too much either way. I have been doing this in userspace, but...

What I'd like to see is some kind of indication that "this attribute is not from hardware, kernel computed it for you" so clever userspace can ignore it/compute it in more advanced way.

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.

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.

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;

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.

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

Quoted 5 lines

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.

Quoted 5 lines

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.

Hi.

Quoted 10 lines

Quoted 5 lines

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.

1. How to configure this value? I tried:
echo 4350000 > /sys/class/power_supply/battery/constant_charge_voltage

...but it didn't change the fully charged battery voltage.

2. If the value is configurable, it probably shouldn't be hardcoded like this in the code:
PCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_FULL, 4200000 - 18000, 100)

Quoted 14 lines

Quoted 5 lines

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

Quoted 2 lines

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

My idea is to only have one extra property which will store the delta of counter values for full and empty battery states. In this case, it's going to be more accurate than allowing the userspace to store/restore both full and empty counters.

Some users may have a dual boot setup. In this case restoring full & empty counters after reboot can make the kernel to report wrong battery percentage
values.

Let's imagine the following situation:

1. The user charges the battery to the full state and discharges it to be almost depleted, so the kernel has "calibrated" values for counter_full and counter_empty.
2. The user decides to reboot to another OS. Let's assume he has a fully charged battery before doing the reboot.
For example the kernel provides the following values:
POWER_SUPPLY_PROP_CHARGE_COUNTER_FULL = -250000 POWER_SUPPLY_PROP_CHARGE_COUNTER_EMPTY = 950000 /sys/class/power_supply/battery/charge_counter = -250000
A daemon saves these values to a permanent storage.
3. The user boots another OS, works there for a while, the battery charge

becomes 50%.
4. The user reboots back to our system, we restore the values:
POWER_SUPPLY_PROP_CHARGE_COUNTER_FULL <- -250000 POWER_SUPPLY_PROP_CHARGE_COUNTER_EMPTY <- 950000
5. /sys/class/power_supply/battery/charge_counter is initialized to 0 or -250000, it doesn't matter, because in both cases we will get the wrong battery percentage. We should report 50%, remember? In any case it can't be calculated to this value.

And here's how I suggest it to be implemented:
1. Same as above
2. Same as above, except the kernel provides a single value that the user should store between reboots (it's the delta I mentioned above):
POWER_SUPPLY_PROP_CHARGE_FULL = 1200000
3. Same as above
4. The user reboots back to our system, we restore the value:
POWER_SUPPLY_PROP_CHARGE_FULL <- 1200000
5. The kernel reports -ENODATA as battery capacity value until the battery goes to a reference point (fully charged or almost discharged state). When the battery is fully charged or discharged, the kernel can immediately calculate the opposite counter value. And no matter what was the initial value of /sys/class/power_supply/battery/charge_counter when the system booted.

Of course, the value of the delta will not be constant, it should be recalculated over time. The good news is that this value will be changed slowly over time. That is, it can vary from 2% to 10% per year.