Charge counter on droid 4

Charge counter on droid 4

[Pavel Machek]

Hi!

Droid 4 has non-removable battery, yet the charge counter is reset to
near zero on each boot of linux.

Unfortunately, that makes charge counter pretty much useless on d4, as
the "battery full" and "battery empty" limits will be different during
each boot.

Is it possible that we reset the cpcap needlessly, or something like
that?

unicsy_demo (github.com:pavelmachek/unicsy_demo.git) was updated to
read the charge counter, and attempt to estimate battery percentages
from that. Use monitor/batmond... warning -- it provides a _lot_ of
information.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180615/57bfea84/attachment.sig>

Charge counter on droid 4

[Pavel Machek]

On Fri 2018-06-15 10:00:14, Pavel Machek wrote:
> Hi!
> 
> Droid 4 has non-removable battery, yet the charge counter is reset to
> near zero on each boot of linux.
> 
> Unfortunately, that makes charge counter pretty much useless on d4, as
> the "battery full" and "battery empty" limits will be different during
> each boot.

Hmm, and could we refrain from providing "power" values?

I was thinking great, we have hardware that does proper power
measuerement for us. No.... it is driver providing synthetic
values. As userland has enough information to do that itself, I
believe we should not do this in kernel.

diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
index 839e365..1610026 100644
--- a/drivers/power/supply/cpcap-battery.c
+++ b/drivers/power/supply/cpcap-battery.c
@@ -490,24 +490,6 @@ static int cpcap_battery_get_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_CHARGE_COUNTER:
 		val->intval = latest->counter_uah;
 		break;
-	case POWER_SUPPLY_PROP_POWER_NOW:
-		tmp = (latest->voltage / 10000) * latest->current_ua;
-		val->intval = div64_s64(tmp, 100);
-		break;
-	case POWER_SUPPLY_PROP_POWER_AVG:
-		if (cached) {
-			tmp = cpcap_battery_cc_get_avg_current(ddata);
-			tmp *= (latest->voltage / 10000);
-			val->intval = div64_s64(tmp, 100);
-			break;
-		}
-		sample = latest->cc.sample - previous->cc.sample;
-		accumulator = latest->cc.accumulator - previous->cc.accumulator;
-		tmp = cpcap_battery_cc_to_ua(ddata, sample, accumulator,
-					     latest->cc.offset);
-		tmp *= ((latest->voltage + previous->voltage) / 20000);
-		val->intval = div64_s64(tmp, 100);
-		break;
 	case POWER_SUPPLY_PROP_CAPACITY_LEVEL:
 		if (cpcap_battery_full(ddata))
 			val->intval = POWER_SUPPLY_CAPACITY_LEVEL_FULL;

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180615/2ce5a36e/attachment.sig>

Charge counter on droid 4

[Tony Lindgren]

* Pavel Machek <pavel@ucw.cz> [180615 08:34]:
> On Fri 2018-06-15 10:00:14, Pavel Machek wrote:
> > Hi!
> > 
> > Droid 4 has non-removable battery, yet the charge counter is reset to
> > near zero on each boot of linux.

Not sure if we actively do anything to reset it. I'm guessing
it's the Motorola bootloader that resets everything on boot.

> > Unfortunately, that makes charge counter pretty much useless on d4, as
> > the "battery full" and "battery empty" limits will be different during
> > each boot.

>From what I've seen also the stock kernel starts only with
voltage based estimate initially after a reboot?

> Hmm, and could we refrain from providing "power" values?
> 
> I was thinking great, we have hardware that does proper power
> measuerement for us. No.... it is driver providing synthetic
> values. As userland has enough information to do that itself, I
> believe we should not do this in kernel.

Hmm I don't follow you, why would we want to remove these as
they implement a standard sysfs interface?

I use the sysfs interface all the time to monitor the power
consumption and the output seems to match what I was seeing
with my power supply.

Note that we also have the IIO raw data also available if that
might help.

Regards,

Tony

Charge counter on droid 4

[Pavel Machek]

Hi!

> > > Droid 4 has non-removable battery, yet the charge counter is reset to
> > > near zero on each boot of linux.
> 
> Not sure if we actively do anything to reset it. I'm guessing
> it's the Motorola bootloader that resets everything on boot.

Too bad if that's the case :-(.

> > > Unfortunately, that makes charge counter pretty much useless on d4, as
> > > the "battery full" and "battery empty" limits will be different during
> > > each boot.
> 
> From what I've seen also the stock kernel starts only with
> voltage based estimate initially after a reboot?
> 
> > Hmm, and could we refrain from providing "power" values?
> > 
> > I was thinking great, we have hardware that does proper power
> > measuerement for us. No.... it is driver providing synthetic
> > values. As userland has enough information to do that itself, I
> > believe we should not do this in kernel.
> 
> Hmm I don't follow you, why would we want to remove these as
> they implement a standard sysfs interface?
> 
> I use the sysfs interface all the time to monitor the power
> consumption and the output seems to match what I was seeing
> with my power supply.

So... there are mA, mAh values. Those come from hardware, and I
believe we should keep them.

But there are also mW, mWh values, which are synthetic. Userland can
compute them from mV, mA values... and it is confusing that kernel
provides them. (My tendency was to start computing these synthetic
values in userland, to compare them with "real hardware" values from
kernel. But then I looked at kernel implementation, and realized they
are synthetic, tooo...)

Best regards,
								Pavel
								
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180618/14095185/attachment.sig>

Charge counter on droid 4

[Tony Lindgren]

* Pavel Machek <pavel@ucw.cz> [180618 07:43]:
> 
> So... there are mA, mAh values. Those come from hardware, and I
> believe we should keep them.
> 
> But there are also mW, mWh values, which are synthetic. Userland can
> compute them from mV, mA values... and it is confusing that kernel
> provides them. (My tendency was to start computing these synthetic
> values in userland, to compare them with "real hardware" values from
> kernel. But then I looked at kernel implementation, and realized they
> are synthetic, tooo...)

Hmm mWh value is based on the hardware sampled shunt
values and number of samples gathered between the
two readings. I'd rather call the calculated values
based on userland reading mV and mA values "synthetic" :)

Regards,

Tony

Charge counter on droid 4

[Pavel Machek]

On Mon 2018-06-18 01:28:58, Tony Lindgren wrote:
> * Pavel Machek <pavel@ucw.cz> [180618 07:43]:
> > 
> > So... there are mA, mAh values. Those come from hardware, and I
> > believe we should keep them.
> > 
> > But there are also mW, mWh values, which are synthetic. Userland can
> > compute them from mV, mA values... and it is confusing that kernel
> > provides them. (My tendency was to start computing these synthetic
> > values in userland, to compare them with "real hardware" values from
> > kernel. But then I looked at kernel implementation, and realized they
> > are synthetic, tooo...)
> 
> Hmm mWh value is based on the hardware sampled shunt
> values and number of samples gathered between the
> two readings. I'd rather call the calculated values
> based on userland reading mV and mA values "synthetic" :)

As far as I know, shunt resistors provide you with current (mA) not
power (mW) measurement... and cpcap-battery computes power_now as
voltage * current. I'd rather have kernel tell me "hardware can't
measure power" and do "voltage*current" computation in userspace.

So I'm proposing to apply patch below.

Best regards,
								Pavel


+++ b/drivers/power/supply/cpcap-battery.c
@@ -490,24 +490,6 @@ static int cpcap_battery_get_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_CHARGE_COUNTER:
 		val->intval = latest->counter_uah;
 		break;
-	case POWER_SUPPLY_PROP_POWER_NOW:
-		tmp = (latest->voltage / 10000) * latest->current_ua;
-		val->intval = div64_s64(tmp, 100);
-		break;
-	case POWER_SUPPLY_PROP_POWER_AVG:
-		if (cached) {
-			tmp = cpcap_battery_cc_get_avg_current(ddata);
-			tmp *= (latest->voltage / 10000);
-			val->intval = div64_s64(tmp, 100);
-			break;
-		}
-		sample = latest->cc.sample - previous->cc.sample;
-		accumulator = latest->cc.accumulator - previous->cc.accumulator;
-		tmp = cpcap_battery_cc_to_ua(ddata, sample, accumulator,
-					     latest->cc.offset);
-		tmp *= ((latest->voltage + previous->voltage) / 20000);
-		val->intval = div64_s64(tmp, 100);
-		break;
 	case POWER_SUPPLY_PROP_CAPACITY_LEVEL:
 		if (cpcap_battery_full(ddata))
 			val->intval = POWER_SUPPLY_CAPACITY_LEVEL_FULL;

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180618/be105e1c/attachment.sig>

Charge counter on droid 4

[Tony Lindgren]

* Pavel Machek <pavel@ucw.cz> [180618 09:37]:
> On Mon 2018-06-18 01:28:58, Tony Lindgren wrote:
> > * Pavel Machek <pavel@ucw.cz> [180618 07:43]:
> > > 
> > > So... there are mA, mAh values. Those come from hardware, and I
> > > believe we should keep them.
> > > 
> > > But there are also mW, mWh values, which are synthetic. Userland can
> > > compute them from mV, mA values... and it is confusing that kernel
> > > provides them. (My tendency was to start computing these synthetic
> > > values in userland, to compare them with "real hardware" values from
> > > kernel. But then I looked at kernel implementation, and realized they
> > > are synthetic, tooo...)
> > 
> > Hmm mWh value is based on the hardware sampled shunt
> > values and number of samples gathered between the
> > two readings. I'd rather call the calculated values
> > based on userland reading mV and mA values "synthetic" :)
> 
> As far as I know, shunt resistors provide you with current (mA) not
> power (mW) measurement... and cpcap-battery computes power_now as
> voltage * current. I'd rather have kernel tell me "hardware can't
> measure power" and do "voltage*current" computation in userspace.

Yup you are correct the hardware samples mA and we still need
to calculate mW based on the voltage.

But considering it works and seems to match the power supply
provided average power consumption numbers pretty well and at
least I'm using it.. What is your reasoning for removing such
a usable interface?

Regards,

Tony

Charge counter on droid 4

[Pavel Machek]

On Mon 2018-06-18 04:48:32, Tony Lindgren wrote:
> * Pavel Machek <pavel@ucw.cz> [180618 09:37]:
> > On Mon 2018-06-18 01:28:58, Tony Lindgren wrote:
> > > * Pavel Machek <pavel@ucw.cz> [180618 07:43]:
> > > > 
> > > > So... there are mA, mAh values. Those come from hardware, and I
> > > > believe we should keep them.
> > > > 
> > > > But there are also mW, mWh values, which are synthetic. Userland can
> > > > compute them from mV, mA values... and it is confusing that kernel
> > > > provides them. (My tendency was to start computing these synthetic
> > > > values in userland, to compare them with "real hardware" values from
> > > > kernel. But then I looked at kernel implementation, and realized they
> > > > are synthetic, tooo...)
> > > 
> > > Hmm mWh value is based on the hardware sampled shunt
> > > values and number of samples gathered between the
> > > two readings. I'd rather call the calculated values
> > > based on userland reading mV and mA values "synthetic" :)
> > 
> > As far as I know, shunt resistors provide you with current (mA) not
> > power (mW) measurement... and cpcap-battery computes power_now as
> > voltage * current. I'd rather have kernel tell me "hardware can't
> > measure power" and do "voltage*current" computation in userspace.
> 
> Yup you are correct the hardware samples mA and we still need
> to calculate mW based on the voltage.
> 
> But considering it works and seems to match the power supply
> provided average power consumption numbers pretty well and at
> least I'm using it.. What is your reasoning for removing such
> a usable interface?

Well, it is confusing for the userland, because it has no way of
knowing data is synthetic.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180704/cf8dcf60/attachment.sig>

Charge counter on droid 4

[Tony Lindgren]

* Pavel Machek <pavel@ucw.cz> [180704 19:59]:
> On Mon 2018-06-18 04:48:32, Tony Lindgren wrote:
> > * Pavel Machek <pavel@ucw.cz> [180618 09:37]:
> > > On Mon 2018-06-18 01:28:58, Tony Lindgren wrote:
> > > > * Pavel Machek <pavel@ucw.cz> [180618 07:43]:
> > > > > 
> > > > > So... there are mA, mAh values. Those come from hardware, and I
> > > > > believe we should keep them.
> > > > > 
> > > > > But there are also mW, mWh values, which are synthetic. Userland can
> > > > > compute them from mV, mA values... and it is confusing that kernel
> > > > > provides them. (My tendency was to start computing these synthetic
> > > > > values in userland, to compare them with "real hardware" values from
> > > > > kernel. But then I looked at kernel implementation, and realized they
> > > > > are synthetic, tooo...)
> > > > 
> > > > Hmm mWh value is based on the hardware sampled shunt
> > > > values and number of samples gathered between the
> > > > two readings. I'd rather call the calculated values
> > > > based on userland reading mV and mA values "synthetic" :)
> > > 
> > > As far as I know, shunt resistors provide you with current (mA) not
> > > power (mW) measurement... and cpcap-battery computes power_now as
> > > voltage * current. I'd rather have kernel tell me "hardware can't
> > > measure power" and do "voltage*current" computation in userspace.
> > 
> > Yup you are correct the hardware samples mA and we still need
> > to calculate mW based on the voltage.
> > 
> > But considering it works and seems to match the power supply
> > provided average power consumption numbers pretty well and at
> > least I'm using it.. What is your reasoning for removing such
> > a usable interface?
> 
> Well, it is confusing for the userland, because it has no way of
> knowing data is synthetic.

Also the average current is as synthetic. And we don't
want to rely on user space polling the voltage as that
will mess up measurements for low-power idle states.

If there is a problem let's fix it by adding a timer
to refresh average current and power once a minute or
something like that.

I'd rather not have to rely on user space apps for
any knowledge about how to use it.

Regards,

Tony