power: supply: cpcap-battery: Fix battery full status reporting
PATCH 1/15 power: supply: cpcap-battery: Fix battery full status reporting
Don't report that the battery is fully charged if the charging current exceeds 100 mA.
--- drivers/power/supply/cpcap-battery.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/power/supply/cpcap-battery.c +2 -1
@@ -408,7 +408,8 @@ static bool cpcap_battery_full(struct cpcap_battery_ddata *ddata)struct cpcap_battery_state_data *state =cpcap_battery_latest (ddata);if (state->voltage >= - (ddata->config.bat.constant_charge_voltage_max_uv -18000 )) + (ddata->config.bat.constant_charge_voltage_max_uv -18000 ) && + state->current_ua >-100000 )return true;return false;
Signature · 1 line
2.11.0
PATCH 2/15 power: supply: cpcap-battery: Improve battery full status detection
If the battery status is detected as full for the charging current that doesn't exceed 100 mA, it will then be reported as full for charging currents in the range of 100-150 mA. This is needed because charge_current value has a spread.
We don't use avg_current here because it can trigger wrong battery full status on charger connected event.
--- drivers/power/supply/cpcap-battery.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
drivers/power/supply/cpcap-battery.c +6 -3
@@ -406,13 +406,16 @@ static int cpcap_battery_cc_get_avg_current(struct cpcap_battery_ddata *ddata)static bool cpcap_battery_full (struct cpcap_battery_ddata *ddata) {struct cpcap_battery_state_data *state =cpcap_battery_latest (ddata); +static bool is_full;if (state->voltage >= (ddata->config.bat.constant_charge_voltage_max_uv -18000 ) && - state->current_ua >-100000 ) -return true; + state->current_ua > (is_full ?-150000 :-100000 )) + is_full = true; +else + is_full = false; -return false; +return is_full; }static bool cpcap_battery_low (struct cpcap_battery_ddata *ddata)
Signature · 1 line
2.11.0
Re: [PATCH 02/15] power: supply: cpcap-battery: Improve battery full status detection
Quoted 7 lines · Arthur Demchenkov
If the battery status is detected as full for the charging current that doesn't exceed 100 mA, it will then be reported as full for charging currents in the range of 100-150 mA. This is needed because
charge_currentvalue has a spread.We don't use
avg_currenthere because it can trigger wrong battery full status on charger connected event.
Hmm oh so this is against my earlier RFC patches. Care to respin the series against v5.6-rc?
Feel free tweak my patches to leave out the unnecessary stuff as we decided to do things in a better way :) Just add a note like:
[spinal: dropped out unusable foo and bar]
So we know what got changed compared to the RFC patches.
And then we might have a nice working set for merging in subject to others approving this approach of course :)
Regards,
Tony
PATCH 3/15 power: supply: cpcap-battery: Fix battery low status reporting
In reply to Arthur DemchenkovIf we hit battery low once, we should stick on reporting it until the charger is connected. This way low->counter_uah will be updated properly, and that will allow us to get more accurate charge_full value.
--- drivers/power/supply/cpcap-battery.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
drivers/power/supply/cpcap-battery.c +6 -3
@@ -421,11 +421,14 @@ static bool cpcap_battery_full(struct cpcap_battery_ddata *ddata)static bool cpcap_battery_low (struct cpcap_battery_ddata *ddata) {struct cpcap_battery_state_data *state =cpcap_battery_latest (ddata); +static bool is_low; -if (state->current_ua && state->voltage <=3300000 ) -return true; +if (state->current_ua >0 && (state->voltage <=3300000 || is_low)) + is_low = true; +else + is_low = false; -return false; +return is_low; }static int cpcap_battery_update_status (struct cpcap_battery_ddata *ddata)
Signature · 1 line
2.11.0
Re: [PATCH 03/15] power: supply: cpcap-battery: Fix battery low status reporting
Quoted 19 lines · Arthur Demchenkov
If we hit battery low once, we should stick on reporting it until the charger is connected. This way
low->counter_uahwill be updated properly, and that will allow us to get more accuratecharge_fullvalue.Signed-off-by Arthur Demchenkov <[email protected]>--- drivers/power/supply/cpcap-battery.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)drivers/power/supply/cpcap-battery.c +1
-0@@ -421,11 +421,14 @@ static bool cpcap_battery_full(struct cpcap_battery_ddata *ddata)static bool cpcap_battery_low (struct cpcap_battery_ddata *ddata) {struct cpcap_battery_state_data *state =cpcap_battery_latest (ddata); +static bool is_low;
drivers/power/supply/cpcap-battery.c static bool cpcap_battery_full(struct cpcap_battery_ddata *ddata) · line 432
Quoted 10 lines
drivers/power/supply/cpcap-battery.c +5
-3-if (state->current_ua && state->voltage <=3300000 ) -return true; +if (state->current_ua >0 && (state->voltage <=3300000 || is_low)) + is_low = true; +else + is_low = false; -return false; +return is_low; }
Please set up bitmask in ddata for unsigned int battery_low:1; instead of using a static variable here. If we ever had two instances of cpcap on the same device the static variable here would not work :)
Regards,
Tony
Re: [PATCH 03/15] power: supply: cpcap-battery: Fix battery low status reporting
Will do. Thanks!
Quoted 2 lines
Please set up bitmask in ddata for unsigned int battery_low:1; instead of
using a static variable here. If we ever had two instances of cpcap on the same device the static variable here would not work :)
PATCH 4/15 power: supply: cpcap-battery: Add charge_full property
In reply to Arthur DemchenkovAdd charge_full property.
--- drivers/power/supply/cpcap-battery.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
drivers/power/supply/cpcap-battery.c +17 -0
@@ -134,6 +134,7 @@ struct cpcap_battery_ddata {struct cpcap_battery_state_data state[CPCAP_BATTERY_STATE_NR ];u32 cc_lsb;/* μAms per LSB */ atomic_t active; +int charge_full;int status;u16 vendor; };
@@ -530,6 +531,7 @@ static enum power_supply_property cpcap_battery_props[] = {POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE ,POWER_SUPPLY_PROP_CURRENT_AVG ,POWER_SUPPLY_PROP_CURRENT_NOW , +POWER_SUPPLY_PROP_CHARGE_FULL ,POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN ,POWER_SUPPLY_PROP_CHARGE_COUNTER , POWER_SUPPLY_PROP_POWER_NOW,
@@ -649,6 +651,11 @@ static int cpcap_battery_get_property(struct power_supply *psy,case POWER_SUPPLY_PROP_CAPACITY_LEVEL : val->intval =cpcap_battery_get_rough_capacity (ddata);break ; +case POWER_SUPPLY_PROP_CHARGE_FULL : +if (!ddata->charge_full) +return -ENODATA ; + val->intval = ddata->charge_full; +break ;case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN : val->intval = ddata->config.info.charge_full_design;break ;
@@ -710,6 +717,15 @@ static int cpcap_battery_set_property(struct power_supply *psy, ddata->config.bat.constant_charge_voltage_max_uv = val->intval;return cpcap_battery_update_charger (ddata, val->intval); +case POWER_SUPPLY_PROP_CHARGE_FULL : +if (val->intval <0 ) +return -EINVAL ; +if (val->intval > ddata->config.info.charge_full_design) +return -EINVAL ; + + ddata->charge_full = val->intval; + +return 0 ;default :return -EINVAL ; }
@@ -722,6 +738,7 @@ static int cpcap_battery_property_is_writeable(struct power_supply *psy, {switch (psp) {case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE : +case POWER_SUPPLY_PROP_CHARGE_FULL :return 1 ;default :return 0 ;
Signature · 1 line
2.11.0
PATCH 5/15 power: supply: cpcap-battery: Add charge_now property
In reply to Arthur DemchenkovAdd charge_now property.
--- drivers/power/supply/cpcap-battery.c | 7 +++++++ 1 file changed, 7 insertions(+)
drivers/power/supply/cpcap-battery.c +7 -0
@@ -532,6 +532,7 @@ static enum power_supply_property cpcap_battery_props[] = {POWER_SUPPLY_PROP_CURRENT_AVG ,POWER_SUPPLY_PROP_CURRENT_NOW ,POWER_SUPPLY_PROP_CHARGE_FULL , +POWER_SUPPLY_PROP_CHARGE_NOW ,POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN ,POWER_SUPPLY_PROP_CHARGE_COUNTER , POWER_SUPPLY_PROP_POWER_NOW,
@@ -651,6 +652,12 @@ static int cpcap_battery_get_property(struct power_supply *psy,case POWER_SUPPLY_PROP_CAPACITY_LEVEL : val->intval =cpcap_battery_get_rough_capacity (ddata);break ; +case POWER_SUPPLY_PROP_CHARGE_NOW : + low =cpcap_battery_get_lowest (ddata); +if (!low->voltage) +return -ENODATA ; + val->intval = low->counter_uah - latest->counter_uah; +break ;case POWER_SUPPLY_PROP_CHARGE_FULL :if (!ddata->charge_full)return -ENODATA ;
Signature · 1 line
2.11.0
PATCH 6/15 power: supply: cpcap-battery: Initialize with user provided data
In reply to Arthur DemchenkovIf 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.
--- drivers/power/supply/cpcap-battery.c | 39 +++++++++++++++++++++++++++++------- 1 file changed, 32 insertions(+), 7 deletions(-)
drivers/power/supply/cpcap-battery.c +32 -7
@@ -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 ;
Signature · 1 line
2.11.0
Re: [PATCH 06/15] power: supply: cpcap-battery: Initialize with user provided data
Quoted 3 lines · Arthur Demchenkov
If the user provides us with
charge_fullvalue (which it could save in a permanent storage between reboots), initialize low and highcounter_uahwith 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
Re: [PATCH 06/15] power: supply: cpcap-battery: Initialize with user provided data
Hi.
Quoted 2 lines
Hmm I'm getting
-EINVALwhen I do echo 1600000 >charge_nowbut 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.
Quoted 2 lines
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.
Quoted 2 lines
Also, we should have the driver default to using the
charge_full_designvalue 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.
Quoted 3 lines
Maybe some pessimistic estimate could be used instead of just using
charge_full_designif 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
Re: [PATCH 06/15] power: supply: cpcap-battery: Initialize with user provided data
Quoted 8 lines · Arthur D.
Hi.
Quoted 2 lines
Hmm I'm getting
-EINVALwhen I do echo 1600000 >charge_nowbut yet the value does get updated?Hm. I'm getting "Permission denied" when trying to do this. Anyway,
charge_nowvalue is not supposed to be initialized by the user. Onlycharge_fullshould 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
Quoted 5 lines
Quoted 4 lines
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?
Quoted 7 lines
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 initializecharge_fullvalue. And it will start reporting correctcharge_now
and capacity (battery charge percentage) values.
Hmm OK. So far no luck triggering that though.
Quoted 11 lines
Once the
charge_fullvalue 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_cfTo restore (after device reboot or power on):
cat /battery_cf >/sys/class/power_supply/battery/charge_full
OK
Quoted 3 lines
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_nowand capacity values.
OK
Quoted 4 lines
Quoted 3 lines
Also, we should have the driver default to using the
charge_full_designvalue if nothing is written from userspace and we see a high or low value.
Quoted 8 lines
I'd prefer to have
charge_fullvalue 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 havingcharge_full=charge_full_designon pretty old Droid 4 batteries. For example, my battery after calibration has about 1000 mAh, whilecharge_full_designis 1740000.
OK
Quoted 9 lines
Quoted 3 lines
Maybe some pessimistic estimate could be used instead of just using
charge_full_designif 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:
Quoted 4 lines
percentage = (voltage -
voltage_empty) / (voltage_full-voltage_empty) * 100Actually, the formula we use is a bit more complicated.
OK
Quoted 2 lines
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
PATCH 7/15 power: supply: cpcap-battery: Rewrite capacity reporting
In reply to Arthur Demchenkov* Return -ENODATA instead of confusing rough values
* Calculate percentage using charge_full value provided
--- drivers/power/supply/cpcap-battery.c | 27 +++++++-------------------- 1 file changed, 7 insertions(+), 20 deletions(-)
drivers/power/supply/cpcap-battery.c +7 -20
@@ -573,10 +573,10 @@ static int cpcap_battery_get_property(struct power_supply *psy, union power_supply_propval *val) {struct cpcap_battery_ddata *ddata =power_supply_get_drvdata (psy); -struct cpcap_battery_state_data *latest, *previous, *low, *high; +struct cpcap_battery_state_data *latest, *previous, *low;u32 sample;s32 accumulator; -int cached, delta, est; +int cached;s64 tmp; cached =cpcap_battery_update_status (ddata);
@@ -654,25 +654,12 @@ static int cpcap_battery_get_property(struct power_supply *psy, val->intval =div64_s64 (tmp,100 );break ;case POWER_SUPPLY_PROP_CAPACITY : - est =cpcap_battery_get_rough_percentage (ddata); - high =cpcap_battery_get_highest (ddata); -if (high->voltage) { - delta = latest->counter_uah - high->counter_uah; - val->intval = (ddata->config.info.charge_full_design - - delta) *100 ; - val->intval /= ddata->config.info.charge_full_design; - delta =abs (val->intval - est); -break ; - } low =cpcap_battery_get_lowest (ddata); -if (low->voltage) { - delta = low->counter_uah - latest->counter_uah; - val->intval = (delta *100 ) / - ddata->config.info.charge_full_design; - delta =abs (val->intval - est); -break ; - } - val->intval = est; +if (!low->voltage || !ddata->charge_full) +return -ENODATA ; + val->intval = (low->counter_uah - + latest->counter_uah) *100 ; + val->intval /= ddata->charge_full;break ;case POWER_SUPPLY_PROP_CAPACITY_LEVEL : val->intval =cpcap_battery_get_rough_capacity (ddata);
Signature · 1 line
2.11.0
PATCH 8/15 power: supply: cpcap-battery: Get rid of rough capacity percentage
In reply to Arthur DemchenkovGet rid of rough capacity percentage.
--- drivers/power/supply/cpcap-battery.c | 46 +++++++++--------------------------- 1 file changed, 11 insertions(+), 35 deletions(-)
drivers/power/supply/cpcap-battery.c +11 -35
@@ -142,24 +142,22 @@ struct cpcap_battery_ddata {struct cpcap_battery_capacity {int capacity;int voltage; -int percentage; }; -#define CPCAP_CAP (l, v, p) \ +#define CPCAP_CAP (l, v) \ { \ .capacity = (l), \ .voltage = (v), \ - .percentage = (p), \ },/* Pessimistic battery capacity mapping before high or low value is seen */ static const struct cpcap_battery_capacity cpcap_battery_cap[] = { - CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_UNKNOWN, 0, 0) - CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_CRITICAL, 3100000, 0) - CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_LOW, 3300000, 2) - CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_NORMAL, 3700000, 50) - CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_HIGH, 4000000, 75) -CPCAP_CAP (POWER_SUPPLY_CAPACITY_LEVEL_FULL ,4200000 -18000 ,100 ) + CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_UNKNOWN, 0) + CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_CRITICAL, 3100000) + CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_LOW, 3300000) + CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_NORMAL, 3700000) + CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_HIGH, 4000000) +CPCAP_CAP (POWER_SUPPLY_CAPACITY_LEVEL_FULL ,4200000 -18000 ) };#define CPCAP_NO_BATTERY -400
@@ -503,8 +501,7 @@ static int cpcap_battery_update_status(struct cpcap_battery_ddata *ddata)return 0 ; } -static void cpcap_battery_get_rough (struct cpcap_battery_ddata *ddata, -int *level,int *percentage) +static int cpcap_battery_get_capacity_level (struct cpcap_battery_ddata *ddata) {struct cpcap_battery_state_data *latest;const struct cpcap_battery_capacity *cap =NULL ;
@@ -520,30 +517,9 @@ static void cpcap_battery_get_rough(struct cpcap_battery_ddata *ddata, }if (!cap) -return ; - -if (level) - *level = cap->capacity; -if (percentage) - *percentage = cap->percentage; -} - -static int cpcap_battery_get_rough_capacity (struct cpcap_battery_ddata *ddata) -{ -int capacity =0 ; - -cpcap_battery_get_rough (ddata, &capacity,NULL ); - -return capacity; -} - -static int cpcap_battery_get_rough_percentage (struct cpcap_battery_ddata *ddata) -{ -int percentage =0 ; - -cpcap_battery_get_rough (ddata,NULL , &percentage); +return 0 ; -return percentage; +return cap->capacity; }static enum power_supply_property cpcap_battery_props[] = {
@@ -662,7 +638,7 @@ static int cpcap_battery_get_property(struct power_supply *psy, val->intval /= ddata->charge_full;break ;case POWER_SUPPLY_PROP_CAPACITY_LEVEL : - val->intval =cpcap_battery_get_rough_capacity (ddata); + val->intval =cpcap_battery_get_capacity_level (ddata);break ;case POWER_SUPPLY_PROP_CHARGE_NOW : low =cpcap_battery_get_lowest (ddata);
Signature · 1 line
2.11.0
PATCH 9/15 power: supply: cpcap-battery: Increse low voltage bound
In reply to Arthur DemchenkovThus, the user will have more time to start charging when calibrating the charge_full value.
--- drivers/power/supply/cpcap-battery.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/power/supply/cpcap-battery.c +1 -1
@@ -422,7 +422,7 @@ static bool cpcap_battery_low(struct cpcap_battery_ddata *ddata)struct cpcap_battery_state_data *state =cpcap_battery_latest (ddata);static bool is_low; -if (state->current_ua >0 && (state->voltage <=3300000 || is_low)) +if (state->current_ua >0 && (state->voltage <=3350000 || is_low)) is_low = true;else is_low = false;
Signature · 1 line
2.11.0
PATCH 10/15 power: supply: cpcap-battery: Improve full status reporting
In reply to Arthur DemchenkovUse average current when detecting the fully charged state
--- drivers/power/supply/cpcap-battery.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
drivers/power/supply/cpcap-battery.c +9 -5
@@ -407,12 +407,16 @@ static bool cpcap_battery_full(struct cpcap_battery_ddata *ddata)struct cpcap_battery_state_data *state =cpcap_battery_latest (ddata);static bool is_full; -if (state->voltage >= - (ddata->config.bat.constant_charge_voltage_max_uv -18000 ) && - state->current_ua > (is_full ?-150000 :-100000 )) - is_full = true; -else +if (state->voltage < + (ddata->config.bat.constant_charge_voltage_max_uv -18000 )) { is_full = false; + }else if (is_full) { +if (state->current_ua <-150000 ) + is_full = false; + }else if (state->current_ua >=-150000 && +cpcap_battery_cc_get_avg_current (ddata) >=-100000 ) { + is_full = true; + }return is_full; }
Signature · 1 line
2.11.0
PATCH 11/15 power: supply: cpcap-battery: Rename properties, variables and functions
In reply to Arthur DemchenkovThis is a preparation for the following change which will stabilize battery full charge value when calibrating.
--- drivers/power/supply/cpcap-battery.c | 75 ++++++++++++++++++------------------ 1 file changed, 38 insertions(+), 37 deletions(-)
drivers/power/supply/cpcap-battery.c +38 -37
@@ -110,8 +110,8 @@ struct cpcap_coulomb_counter_data {enum cpcap_battery_state {CPCAP_BATTERY_STATE_PREVIOUS ,CPCAP_BATTERY_STATE_LATEST , -CPCAP_BATTERY_STATE_LOW , -CPCAP_BATTERY_STATE_HIGH , +CPCAP_BATTERY_STATE_EMPTY , +CPCAP_BATTERY_STATE_FULL ,CPCAP_BATTERY_STATE_NR , };
@@ -185,15 +185,15 @@ cpcap_battery_previous(struct cpcap_battery_ddata *ddata) }static struct cpcap_battery_state_data * -cpcap_battery_get_lowest (struct cpcap_battery_ddata *ddata) +cpcap_battery_get_empty (struct cpcap_battery_ddata *ddata) { -return cpcap_battery_get_state (ddata,CPCAP_BATTERY_STATE_LOW ); +return cpcap_battery_get_state (ddata,CPCAP_BATTERY_STATE_EMPTY ); }static struct cpcap_battery_state_data * -cpcap_battery_get_highest (struct cpcap_battery_ddata *ddata) +cpcap_battery_get_full (struct cpcap_battery_ddata *ddata) { -return cpcap_battery_get_state (ddata,CPCAP_BATTERY_STATE_HIGH ); +return cpcap_battery_get_state (ddata,CPCAP_BATTERY_STATE_FULL ); } static int cpcap_charger_battery_temperature(struct cpcap_battery_ddata *ddata,
@@ -436,7 +436,8 @@ 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, *low, *high; +struct cpcap_battery_state_data state, *latest, *previous, + *empty, *full;ktime_t now;int error;
@@ -466,39 +467,39 @@ static int cpcap_battery_update_status(struct cpcap_battery_ddata *ddata)memcpy (latest, &state,sizeof (*latest));if (cpcap_battery_full (ddata)) { - high =cpcap_battery_get_highest (ddata); -/* Update highest charge seen? */ -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 ) + full =cpcap_battery_get_full (ddata); +/* Update full state value? */ +if (latest->counter_uah <= full->counter_uah || + !full->voltage) { +memcpy (full, latest,sizeof (*full)); + + empty =cpcap_battery_get_empty (ddata); +if (empty->voltage && empty->voltage !=-1 ) ddata->charge_full = - low->counter_uah - high->counter_uah; + empty->counter_uah - full->counter_uah;else if (ddata->charge_full) {/* Initialize with user provided data */ - low->counter_uah = - high->counter_uah + ddata->charge_full; + empty->counter_uah = + full->counter_uah + ddata->charge_full;/* Mark it as initialized */ - low->voltage =-1 ; + empty->voltage =-1 ; } } }else if (cpcap_battery_low (ddata)) { - low =cpcap_battery_get_lowest (ddata); -/* Update lowest charge seen? */ -if (latest->counter_uah >= low->counter_uah || - !low->voltage) { -memcpy (low, latest,sizeof (*low)); - - high =cpcap_battery_get_highest (ddata); -if (high->voltage) + empty =cpcap_battery_get_empty (ddata); +/* Update empty state value? */ +if (latest->counter_uah >= empty->counter_uah || + !empty->voltage) { +memcpy (empty, latest,sizeof (*empty)); + + full =cpcap_battery_get_full (ddata); +if (full->voltage) ddata->charge_full = - low->counter_uah - high->counter_uah; + empty->counter_uah - full->counter_uah;else if (ddata->charge_full)/* Initialize with user provided data */ - high->counter_uah = - low->counter_uah - ddata->charge_full; + full->counter_uah = + empty->counter_uah - ddata->charge_full; } }
@@ -553,7 +554,7 @@ static int cpcap_battery_get_property(struct power_supply *psy, union power_supply_propval *val) {struct cpcap_battery_ddata *ddata =power_supply_get_drvdata (psy); -struct cpcap_battery_state_data *latest, *previous, *low; +struct cpcap_battery_state_data *latest, *previous, *empty;u32 sample;s32 accumulator;int cached;
@@ -634,10 +635,10 @@ static int cpcap_battery_get_property(struct power_supply *psy, val->intval =div64_s64 (tmp,100 );break ;case POWER_SUPPLY_PROP_CAPACITY : - low =cpcap_battery_get_lowest (ddata); -if (!low->voltage || !ddata->charge_full) + empty =cpcap_battery_get_empty (ddata); +if (!empty->voltage || !ddata->charge_full)return -ENODATA ; - val->intval = (low->counter_uah - + val->intval = (empty->counter_uah - latest->counter_uah) *100 ; val->intval /= ddata->charge_full;break ;
@@ -645,10 +646,10 @@ static int cpcap_battery_get_property(struct power_supply *psy, val->intval =cpcap_battery_get_capacity_level (ddata);break ;case POWER_SUPPLY_PROP_CHARGE_NOW : - low =cpcap_battery_get_lowest (ddata); -if (!low->voltage) + empty =cpcap_battery_get_empty (ddata); +if (!empty->voltage)return -ENODATA ; - val->intval = low->counter_uah - latest->counter_uah; + val->intval = empty->counter_uah - latest->counter_uah;break ;case POWER_SUPPLY_PROP_CHARGE_FULL :if (!ddata->charge_full)
Signature · 1 line
2.11.0
PATCH 12/15 power: supply: cpcap-battery: stabilize charge_full value
In reply to Arthur DemchenkovIt will not grow after the battery met "fully charged" state. This is needed because of how constant voltage charging works. If we want "fully charged" state to be reported in a user expected manner we are forced to do it when the charging current is low enough, but not a zero value.
So, we report "battery full" when the charging current is as low as 100 mA. But the actual charging continues until the user disconnects the charger. It means that a few mAh will be added to the reported charge_full value, which the user shouldn't see. With that purpose we clamp the charge_now value to not exceed charge_full.
--- drivers/power/supply/cpcap-battery.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-)
drivers/power/supply/cpcap-battery.c +15 -14
@@ -469,15 +469,15 @@ static int cpcap_battery_update_status(struct cpcap_battery_ddata *ddata)if (cpcap_battery_full (ddata)) { full =cpcap_battery_get_full (ddata);/* Update full state value? */ -if (latest->counter_uah <= full->counter_uah || - !full->voltage) { +if (!full->voltage) {memcpy (full, latest,sizeof (*full)); empty =cpcap_battery_get_empty (ddata); -if (empty->voltage && empty->voltage !=-1 ) +if (empty->voltage) { ddata->charge_full = empty->counter_uah - full->counter_uah; -else if (ddata->charge_full) { + empty->voltage =-1 ; + }else if (ddata->charge_full) {/* Initialize with user provided data */ empty->counter_uah = full->counter_uah + ddata->charge_full;
@@ -488,18 +488,15 @@ static int cpcap_battery_update_status(struct cpcap_battery_ddata *ddata) }else if (cpcap_battery_low (ddata)) { empty =cpcap_battery_get_empty (ddata);/* Update empty state value? */ -if (latest->counter_uah >= empty->counter_uah || - !empty->voltage) { +if (!empty->voltage || empty->voltage ==-1 ) {memcpy (empty, latest,sizeof (*empty)); full =cpcap_battery_get_full (ddata); -if (full->voltage) +if (full->voltage) { ddata->charge_full = empty->counter_uah - full->counter_uah; -else if (ddata->charge_full) -/* Initialize with user provided data */ - full->counter_uah = - empty->counter_uah - ddata->charge_full; + full->voltage =0 ; + } } }
@@ -638,9 +635,9 @@ static int cpcap_battery_get_property(struct power_supply *psy, empty =cpcap_battery_get_empty (ddata);if (!empty->voltage || !ddata->charge_full)return -ENODATA ; - val->intval = (empty->counter_uah - - latest->counter_uah) *100 ; - val->intval /= ddata->charge_full; + val->intval = empty->counter_uah - latest->counter_uah; + val->intval =clamp (val->intval,0 , ddata->charge_full); + val->intval = val->intval *100 / ddata->charge_full;break ;case POWER_SUPPLY_PROP_CAPACITY_LEVEL : val->intval =cpcap_battery_get_capacity_level (ddata);
@@ -650,6 +647,10 @@ static int cpcap_battery_get_property(struct power_supply *psy,if (!empty->voltage)return -ENODATA ; val->intval = empty->counter_uah - latest->counter_uah; +if (val->intval <0 ) + val->intval =0 ; +else if (ddata->charge_full && ddata->charge_full < val->intval) + val->intval = ddata->charge_full;break ;case POWER_SUPPLY_PROP_CHARGE_FULL :if (!ddata->charge_full)
Signature · 1 line
2.11.0
PATCH 13/15 power: supply: cpcap-battery: Fine tune end of charge current
In reply to Arthur DemchenkovFor some reason the charger device gets disconnected prior we have 100 mA current. Set the fully charged state detection current to 112 mA.
--- drivers/power/supply/cpcap-battery.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/power/supply/cpcap-battery.c +3 -3
@@ -411,10 +411,10 @@ static bool cpcap_battery_full(struct cpcap_battery_ddata *ddata) (ddata->config.bat.constant_charge_voltage_max_uv - 18000)) { is_full = false; } else if (is_full) { -if (state->current_ua <-150000 ) +if (state->current_ua <-170000 ) is_full = false; - } else if (state->current_ua >= -150000 && - cpcap_battery_cc_get_avg_current(ddata) >= -100000) { + } else if (state->current_ua >= -170000 && + cpcap_battery_cc_get_avg_current(ddata) >= -112000) { is_full = true; }
Signature · 1 line
2.11.0
PATCH 14/15 power: supply: cpcap-battery: Make it behave like bq27200
In reply to Arthur DemchenkovNow we begin to update the charge_now value immediately after connecting or disconnecting the charger. No more hidden mAh buffer when the battery is fully charged or discharged.
--- drivers/power/supply/cpcap-battery.c | 44 +++++++++++++++--------------------- 1 file changed, 18 insertions(+), 26 deletions(-)
drivers/power/supply/cpcap-battery.c +18 -26
@@ -468,35 +468,27 @@ static int cpcap_battery_update_status(struct cpcap_battery_ddata *ddata)if (cpcap_battery_full (ddata)) { full =cpcap_battery_get_full (ddata); -/* Update full state value? */ -if (!full->voltage) { -memcpy (full, latest,sizeof (*full)); - - empty =cpcap_battery_get_empty (ddata); -if (empty->voltage) { - ddata->charge_full = - empty->counter_uah - full->counter_uah; - empty->voltage =-1 ; - }else if (ddata->charge_full) { -/* Initialize with user provided data */ - empty->counter_uah = - full->counter_uah + ddata->charge_full; -/* Mark it as initialized */ - empty->voltage =-1 ; - } +memcpy (full, latest,sizeof (*full)); + + empty =cpcap_battery_get_empty (ddata); +if (empty->voltage && empty->voltage !=-1 ) { + empty->voltage =-1 ; + ddata->charge_full = + empty->counter_uah - full->counter_uah; + }else if (ddata->charge_full) { + empty->voltage =-1 ; + empty->counter_uah = + full->counter_uah + ddata->charge_full; } }else if (cpcap_battery_low (ddata)) { empty =cpcap_battery_get_empty (ddata); -/* Update empty state value? */ -if (!empty->voltage || empty->voltage ==-1 ) { -memcpy (empty, latest,sizeof (*empty)); - - full =cpcap_battery_get_full (ddata); -if (full->voltage) { - ddata->charge_full = - empty->counter_uah - full->counter_uah; - full->voltage =0 ; - } +memcpy (empty, latest,sizeof (*empty)); + + full =cpcap_battery_get_full (ddata); +if (full->voltage) { + full->voltage =0 ; + ddata->charge_full = + empty->counter_uah - full->counter_uah; } }
Signature · 1 line
2.11.0
PATCH 15/15 power: supply: cpcap-battery: Add rounding to capacity reporting
In reply to Arthur DemchenkovAdd rounding to capacity reporting.
--- drivers/power/supply/cpcap-battery.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/power/supply/cpcap-battery.c +3 -1
@@ -627,7 +627,9 @@ static int cpcap_battery_get_property(struct power_supply *psy, empty =cpcap_battery_get_empty (ddata);if (!empty->voltage || !ddata->charge_full)return -ENODATA ; - val->intval = empty->counter_uah - latest->counter_uah; +/* (ddata->charge_full / 200) is needed for rounding */ + val->intval = empty->counter_uah - latest->counter_uah + + ddata->charge_full /200 ; val->intval =clamp (val->intval,0 , ddata->charge_full); val->intval = val->intval *100 / ddata->charge_full;break ;
Signature · 1 line
2.11.0
Hi!
Quoted 2 lines
Don't report that the battery is fully charged if the charging current exceeds 100 mA.
Could you merge patches together for possibility of reasonable review?
drivers/power/supply/cpcap-battery.c static bool cpcap_battery_full(struct cpcap_battery_ddata *ddata) · line 412
Quoted 7 lines
drivers/power/supply/cpcap-battery.c +2
-1@@ -408,7 +408,8 @@ static bool cpcap_battery_full(struct cpcap_battery_ddata *ddata)struct cpcap_battery_state_data *state =cpcap_battery_latest (ddata);if (state->voltage >= - (ddata->config.bat.constant_charge_voltage_max_uv -18000 )) + (ddata->config.bat.constant_charge_voltage_max_uv -18000 ) && + state->current_ua >-100000 )
It seems that this 100mA threshold is changed about 3 times in the series :-(.
Plus, it might be better to place booleans into struct, rather than using "static bool" inside a function.
Could we get some kind of explanations for the whole series? 100mA is rather high current for end of charge. If you stop updating full capacity when "your" end of charge is met, you'll have battery charged to more than 100%. I... don't think that makes sense.
Best regards,
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Quoted 18 lines · Pavel Machek
Hi!
Quoted 2 lines
Don't report that the battery is fully charged if the charging current exceeds 100 mA.
Could you merge patches together for possibility of reasonable review?
Review static bool cpcap_battery_full(struct cpcap_battery_ddata *ddata) · line 412
Quoted 7 lines
(quoted diff) +2
-1@@ -408,7 +408,8 @@ static bool cpcap_battery_full(struct cpcap_battery_ddata *ddata) struct cpcap_battery_state_data *state = cpcap_battery_latest(ddata); if (state->voltage >= - (ddata->config.bat.constant_charge_voltage_max_uv - 18000)) + (ddata->config.bat.constant_charge_voltage_max_uv - 18000) && + state->current_ua > -100000)It seems that this 100mA threshold is changed about 3 times in the series :-(.
Quoted 7 lines
Plus, it might be better to place booleans into struct, rather than using "static bool" inside a function.
Could we get some kind of explanations for the whole series? 100mA is rather high current for end of charge. If you stop updating full capacity when "your" end of charge is met, you'll have battery charged to more than 100%. I... don't think that makes sense.
Yeh please compress out the unncessary parts of my RFC patches :)
For reference only, below is a diff of all the changes for cpcap-battery.c against v5.6-rc5.
I applied this series on top of a droid4-pending-v5.6 based branch that has the earlier RFC changes applied.
Regards,
Tony
8< -----------------
drivers/power/supply/cpcap-battery.c +150 -18
@@ -110,6 +110,8 @@ struct cpcap_coulomb_counter_data {enum cpcap_battery_state {CPCAP_BATTERY_STATE_PREVIOUS ,CPCAP_BATTERY_STATE_LATEST , +CPCAP_BATTERY_STATE_EMPTY , +CPCAP_BATTERY_STATE_FULL ,CPCAP_BATTERY_STATE_NR , };
@@ -132,10 +134,32 @@ struct cpcap_battery_ddata {struct cpcap_battery_state_data state[CPCAP_BATTERY_STATE_NR ];u32 cc_lsb;/* μAms per LSB */ atomic_t active; +int charge_full;int status;u16 vendor; }; +struct cpcap_battery_capacity { +int capacity; +int voltage; +}; + +#define CPCAP_CAP (l, v) \ +{ \ + .capacity = (l), \ + .voltage = (v), \ +}, + +/* Pessimistic battery capacity mapping before high or low value is seen */ +static const struct cpcap_battery_capacity cpcap_battery_cap[] = { + CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_UNKNOWN, 0) + CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_CRITICAL, 3100000) + CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_LOW, 3300000) + CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_NORMAL, 3700000) + CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_HIGH, 4000000) +CPCAP_CAP (POWER_SUPPLY_CAPACITY_LEVEL_FULL ,4200000 -18000 ) +}; +#define CPCAP_NO_BATTERY -400 static struct cpcap_battery_state_data *
@@ -160,6 +184,18 @@ cpcap_battery_previous(struct cpcap_battery_ddata *ddata)return cpcap_battery_get_state (ddata,CPCAP_BATTERY_STATE_PREVIOUS ); } +static struct cpcap_battery_state_data * +cpcap_battery_get_empty (struct cpcap_battery_ddata *ddata) +{ +return cpcap_battery_get_state (ddata,CPCAP_BATTERY_STATE_EMPTY ); +} + +static struct cpcap_battery_state_data * +cpcap_battery_get_full (struct cpcap_battery_ddata *ddata) +{ +return cpcap_battery_get_state (ddata,CPCAP_BATTERY_STATE_FULL ); +} +static int cpcap_charger_battery_temperature (struct cpcap_battery_ddata *ddata,int *value) {
@@ -369,17 +405,39 @@ static int cpcap_battery_cc_get_avg_current(struct cpcap_battery_ddata *ddata)static bool cpcap_battery_full (struct cpcap_battery_ddata *ddata) {struct cpcap_battery_state_data *state =cpcap_battery_latest (ddata); +static bool is_full; + +if (state->voltage < + (ddata->config.bat.constant_charge_voltage_max_uv -18000 )) { + is_full = false; + }else if (is_full) { +if (state->current_ua <-170000 ) + is_full = false; + }else if (state->current_ua >=-170000 && +cpcap_battery_cc_get_avg_current (ddata) >=-112000 ) { + is_full = true; + } + +return is_full; +} -if (state->voltage >= - (ddata->config.bat.constant_charge_voltage_max_uv -18000 )) -return true; +static bool cpcap_battery_low (struct cpcap_battery_ddata *ddata) +{ +struct cpcap_battery_state_data *state =cpcap_battery_latest (ddata); +static bool is_low; -return false; +if (state->current_ua >0 && (state->voltage <=3350000 || is_low)) + is_low = true; +else + is_low = false; + +return is_low; }static int cpcap_battery_update_status (struct cpcap_battery_ddata *ddata) { -struct cpcap_battery_state_data state, *latest, *previous; +struct cpcap_battery_state_data state, *latest, *previous, + *empty, *full;ktime_t now;int error;
@@ -408,9 +466,56 @@ static int cpcap_battery_update_status(struct cpcap_battery_ddata *ddata)memcpy (previous, latest,sizeof (*previous));memcpy (latest, &state,sizeof (*latest)); +if (cpcap_battery_full (ddata)) { + full =cpcap_battery_get_full (ddata); +memcpy (full, latest,sizeof (*full)); + + empty =cpcap_battery_get_empty (ddata); +if (empty->voltage && empty->voltage !=-1 ) { + empty->voltage =-1 ; + ddata->charge_full = + empty->counter_uah - full->counter_uah; + }else if (ddata->charge_full) { + empty->voltage =-1 ; + empty->counter_uah = + full->counter_uah + ddata->charge_full; + } + }else if (cpcap_battery_low (ddata)) { + empty =cpcap_battery_get_empty (ddata); +memcpy (empty, latest,sizeof (*empty)); + + full =cpcap_battery_get_full (ddata); +if (full->voltage) { + full->voltage =0 ; + ddata->charge_full = + empty->counter_uah - full->counter_uah; + } + } +return 0 ; } +static int cpcap_battery_get_capacity_level (struct cpcap_battery_ddata *ddata) +{ +struct cpcap_battery_state_data *latest; +const struct cpcap_battery_capacity *cap =NULL ; +int voltage, i; + + latest =cpcap_battery_latest (ddata); + voltage = latest->voltage; + +for (i =ARRAY_SIZE (cpcap_battery_cap) -1 ; i >=0 ; i--) { + cap = &cpcap_battery_cap[i]; +if (voltage >= cap->voltage) +break ; + } + +if (!cap) +return 0 ; + +return cap->capacity; +} +static enum power_supply_property cpcap_battery_props[] = {POWER_SUPPLY_PROP_STATUS ,POWER_SUPPLY_PROP_PRESENT ,
@@ -421,10 +526,13 @@ static enum power_supply_property cpcap_battery_props[] = {POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE ,POWER_SUPPLY_PROP_CURRENT_AVG ,POWER_SUPPLY_PROP_CURRENT_NOW , +POWER_SUPPLY_PROP_CHARGE_FULL , +POWER_SUPPLY_PROP_CHARGE_NOW ,POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN ,POWER_SUPPLY_PROP_CHARGE_COUNTER ,POWER_SUPPLY_PROP_POWER_NOW ,POWER_SUPPLY_PROP_POWER_AVG , +POWER_SUPPLY_PROP_CAPACITY ,POWER_SUPPLY_PROP_CAPACITY_LEVEL ,POWER_SUPPLY_PROP_SCOPE , POWER_SUPPLY_PROP_TEMP,
@@ -435,7 +543,7 @@ static int cpcap_battery_get_property(struct power_supply *psy, union power_supply_propval *val) {struct cpcap_battery_ddata *ddata =power_supply_get_drvdata (psy); -struct cpcap_battery_state_data *latest, *previous; +struct cpcap_battery_state_data *latest, *previous, *empty;u32 sample;s32 accumulator;int cached;
@@ -515,19 +623,33 @@ static int cpcap_battery_get_property(struct power_supply *psy, tmp *= ((latest->voltage + previous->voltage) /20000 ); val->intval =div64_s64 (tmp,100 );break ; +case POWER_SUPPLY_PROP_CAPACITY : + empty =cpcap_battery_get_empty (ddata); +if (!empty->voltage || !ddata->charge_full) +return -ENODATA ; +/* (ddata->charge_full / 200) is needed for rounding */ + val->intval = empty->counter_uah - latest->counter_uah + + ddata->charge_full /200 ; + val->intval =clamp (val->intval,0 , ddata->charge_full); + val->intval = val->intval *100 / ddata->charge_full; +break ;case POWER_SUPPLY_PROP_CAPACITY_LEVEL : -if (cpcap_battery_full (ddata)) - val->intval =POWER_SUPPLY_CAPACITY_LEVEL_FULL ; -else if (latest->voltage >=3750000 ) - val->intval =POWER_SUPPLY_CAPACITY_LEVEL_HIGH ; -else if (latest->voltage >=3300000 ) - val->intval =POWER_SUPPLY_CAPACITY_LEVEL_NORMAL ; -else if (latest->voltage >3100000 ) - val->intval =POWER_SUPPLY_CAPACITY_LEVEL_LOW ; -else if (latest->voltage <=3100000 ) - val->intval =POWER_SUPPLY_CAPACITY_LEVEL_CRITICAL ; -else - val->intval =POWER_SUPPLY_CAPACITY_LEVEL_UNKNOWN ; + val->intval =cpcap_battery_get_capacity_level (ddata); +break ; +case POWER_SUPPLY_PROP_CHARGE_NOW : + empty =cpcap_battery_get_empty (ddata); +if (!empty->voltage) +return -ENODATA ; + val->intval = empty->counter_uah - latest->counter_uah; +if (val->intval <0 ) + val->intval =0 ; +else if (ddata->charge_full && ddata->charge_full < val->intval) + val->intval = ddata->charge_full; +break ; +case POWER_SUPPLY_PROP_CHARGE_FULL : +if (!ddata->charge_full) +return -ENODATA ; + val->intval = ddata->charge_full;break ;case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN : val->intval = ddata->config.info.charge_full_design;
@@ -590,6 +712,15 @@ static int cpcap_battery_set_property(struct power_supply *psy, ddata->config.bat.constant_charge_voltage_max_uv = val->intval;return cpcap_battery_update_charger (ddata, val->intval); +case POWER_SUPPLY_PROP_CHARGE_FULL : +if (val->intval <0 ) +return -EINVAL ; +if (val->intval > ddata->config.info.charge_full_design) +return -EINVAL ; + + ddata->charge_full = val->intval; + +return 0 ;default :return -EINVAL ; }
@@ -602,6 +733,7 @@ static int cpcap_battery_property_is_writeable(struct power_supply *psy, {switch (psp) {case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE : +case POWER_SUPPLY_PROP_CHARGE_FULL :return 1 ;default :return 0 ;
Quoted 4 lines
Could we get some kind of explanations for the whole series? 100mA is rather high current for end of charge. If you stop updating full capacity when "your" end of charge is met, you'll have battery charged to more than 100%. I... don't think that makes sense.
The aim was to allow userspace to see accurate values for charge_now, charge_full and capacity (percentage) of the battery. It will allow the user to estimate how long the device can work on the battery with current power consumption.
For this the user will need to do a battery calibration. I.e. he will need to fully charge and then discharge the battery. Or vice versa: discharge -> charge. Once the user completes the calibration cycle, he will be able to see pretty accurate values for charge_now, charge_full and capacity. This is similar to how bq27200 IC from Nokia N900 works.
Also this patchset allows the userspace to restore the calibration value after reboot. By the calibration value I mean charge_full.
We can't rely on restoring charge_now value, because the user may have multiple operating systems installed and if he works in another OS for a while the charge_now value will become invalid.
So, after a reboot the user may want to restore the charge_full value, so the kernel will be able to estimate the percentage and capacity values without forcing the user to do a full calib- ration cycle again. The only thing the user will have to do is to fully charge OR fully discharge the battery at least once. And he will get all values set.
Now about the chosen limits. For some reason the charging is interrupted (and restarted after a while) when the following conditions are met:
1) the charging current is < 112 mA
2) the display backlight is off
This behaviour was observed in Maemo Leste with hildon-desktop running. I tested these patches for several days, so I picked up the parameters for optimal (from my point of view) work in practice taking into account the current "features" of Droid 4 drivers.
If we could somehow fix this behaviour (charging interruption), I'd reconsider the end of charge current value to be 50 mA.
Making it lower than 50 mA doesn't seem to make much sence because of the extended charging time visibility without giving significant improvement in charge_full accuracy.
Quoted 2 lines
If you stop updating full capacity when "your" end of charge is met, you'll have battery charged to more than 100%.
No worries. With the implemented algorithm, the user will not notice that the battery is more than 100% charged (which is just a convention here). And this situation gives an advantage in that it has a slightly pessimistic charge_full value, which in practice turns out to be good:
the user will be warned about low battery a little ahead of time.
--
Best regards, Spinal
Quoted 6 lines · Arthur D.
Now about the chosen limits. For some reason the charging is interrupted (and restarted after a while) when the following conditions are met:
1) the charging current is < 112 mA
2) the display backlight is off
Quoted 7 lines
This behaviour was observed in Maemo Leste with hildon-desktop running. I tested these patches for several days, so I picked up the parameters for optimal (from my point of view) work in practice taking into account the current "features" of Droid 4 drivers.
If we could somehow fix this behaviour (charging interruption), I'd reconsider the end of charge current value to be 50 mA.
Hmm well we do have two chargers, the usb charger and the unknown inductive charger for the pins on the back.
It would be best to keep cpcap-battery.c independent of the chargers to avoid depndencies as the chargers do usually start charging briefly always when connected.
Maybe just adding something like below would be enough of a check:
static int
cpcap_battery_get_counter_rate(struct cpcap_battery_ddata *ddata,
int poll_time_ms);
And then based on the value being negative or positive you would know if it's charging or not. I guess we could then use this also for POWER_SUPPLY_PROP_CHARGE_NOW with poll_time_ms value of 0. I think the charge counter is configure to poll at 250 ms right now.
Regards,
Tony
Hi, Tony.
It seems like a misunderstanding here. There's no problem in detecting if the charging is in progress. The green led is switched off and the battery current sign is changed from "-" to "+" (which means that the battery is being discharged). So there's no need in additional checks. For cpcap-battery this situation seems like a battery stopped charging. And it doesn't matter if that was a user who disconnected the charger or it was done somewhere in a driver/firmware/hardware.
The problem is that the charging current cant get to the point <100 mA, not talking about <50 mA. And that's why I set the value of 112 mA for the end of charge current: to help the kernel to detect this plateau and to stop the calibration cycle, so the userspace can get all the battery parameters I mentioned in the previous mail.
Please note, that the behaviour I mentioned was observed only when the conditions written in my last mail were met. The important one was:
Quoted 1 line
2) the display backlight is off
Because when I unlocked the display the charging current was able to go below 112 mA. Of course I couldn't rely on something like this:
the user should stay with backlight on to have the battery calibrated. Think about it: waiting for the charging current to drop from 100 mA to 50 mA can take dozens of minutes (it depends on the age of battery - the older the battery the longer it will take), and the user should force somehow the device to not switch off the display hightlight until the battery is calibrated.
Of course it's unacceptable, so I decided to set the end of charge current limit to 112 mA. Which allows the user to just put the device on a table and to wait until it's fully charged without a need to interfere the charging process with some action from the user.
--
Best regards, Spinal
Quoted 32 lines
Quoted 6 lines
Now about the chosen limits. For some reason the charging is interrupted (and restarted after a while) when the following conditions are met:
1) the charging current is < 112 mA
2) the display backlight is offQuoted 7 lines
This behaviour was observed in Maemo Leste with hildon-desktop running. I tested these patches for several days, so I picked up the parameters for optimal (from my point of view) work in practice taking into account the current "features" of Droid 4 drivers.
If we could somehow fix this behaviour (charging interruption), I'd reconsider the end of charge current value to be 50 mA.
Hmm well we do have two chargers, the usb charger and the unknown inductive charger for the pins on the back.
It would be best to keep
cpcap-battery.cindependent of the chargers to avoid depndencies as the chargers do usually start charging briefly always when connected.Maybe just adding something like below would be enough of a check:
static int
cpcap_battery_get_counter_rate(structcpcap_battery_ddata*ddata,int poll_time_ms);And then based on the value being negative or positive you would know if it's charging or not. I guess we could then use this also for
POWER_SUPPLY_PROP_CHARGE_NOWwithpoll_time_msvalue of 0. I think the charge counter is configure to poll at 250 ms right now.
Quoted 10 lines · Arthur D.
Hi, Tony.
It seems like a misunderstanding here. There's no problem in detecting if the charging is in progress. The green led is switched off and the battery current sign is changed from "-" to "+" (which means that the battery is being discharged). So there's no need in additional checks. For cpcap-battery this situation seems like a battery stopped charging. And it doesn't matter if that was a user who disconnected the charger or it was done somewhere in a driver/firmware/hardware.
Quoted 5 lines
The problem is that the charging current cant get to the point <100 mA, not talking about <50 mA. And that's why I set the value of 112 mA for the end of charge current: to help the kernel to detect this plateau and to stop the calibration cycle, so the userspace can get all the battery parameters I mentioned in the previous mail.
OK I guess that's easy to change if we figure out something better :) Maybe add some define for it like CPCAP_BATTERY_FULL_CHARGE_CURRENT or similar?
Quoted 13 lines
Please note, that the behaviour I mentioned was observed only when the conditions written in my last mail were met. The important one was:
Quoted 1 line
2) the display backlight is off
Because when I unlocked the display the charging current was able to go below 112 mA. Of course I couldn't rely on something like this:
the user should stay with backlight on to have the battery calibrated. Think about it: waiting for the charging current to drop from 100 mA to 50 mA can take dozens of minutes (it depends on the age of battery - the older the battery the longer it will take), and the user should force somehow the device to not switch off the display hightlight until the battery is calibrated.
Quoted 4 lines
Of course it's unacceptable, so I decided to set the end of charge current limit to 112 mA. Which allows the user to just put the device on a table and to wait until it's fully charged without a need to interfere the charging process with some action from the user.
Yeah OK thanks.
Tony