lkml.org 
[lkml]   [2019]   [Jul]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 1/1] power/supply/sbs-battery: Fix confusing battery status when idle or empty
From
Date
Hi Guenter
   Yep sorry there was a merge that I missed during that initial send
of the patch.
I sent a version 2 shortly after.

Regards
   Richard Tresidder

On 25/07/2019 9:39 pm, Guenter Roeck wrote:
> On Thu, Jul 25, 2019 at 1:25 AM Richard Tresidder
> <rtresidd@electromag.com.au> wrote:
>> When a battery or batteries in a system are in parallel then one or more
>> may not be providing any current to the system.
>> This fixes an incorrect
>> status indication of FULL for the battery simply because it wasn't
>> discharging at that point in time.
>> The battery will now be flagged as IDLE.
>> Have also added the additional check for the battery FULL DISCHARGED flag
>> which will now flag a status of EMPTY.
>>
>> Signed-off-by: Richard Tresidder <rtresidd@electromag.com.au>
>> ---
>>
>> Notes:
>> power/supply/sbs-battery: Fix confusing battery status when idle or empty
>>
>> When a battery or batteries in a system are in parallel then one or more
>> may not be providing any current to the system.
>> This fixes an incorrect
>> status indication of FULL for the battery simply because it wasn't
>> discharging at that point in time.
>> The battery will now be flagged as IDLE.
>> Have also added the additional check for the battery FULL DISCHARGED flag
>> which will now flag a status of EMPTY.
>>
>> drivers/power/supply/power_supply_sysfs.c | 3 ++-
>> drivers/power/supply/sbs-battery.c | 28 ++++++++++++++--------------
>> include/linux/power_supply.h | 2 ++
>> 3 files changed, 18 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
>> index ce6671c..68ec49d 100644
>> --- a/drivers/power/supply/power_supply_sysfs.c
>> +++ b/drivers/power/supply/power_supply_sysfs.c
>> @@ -51,7 +51,8 @@
>> };
>>
>> static const char * const power_supply_status_text[] = {
>> - "Unknown", "Charging", "Discharging", "Not charging", "Full"
>> + "Unknown", "Charging", "Discharging", "Not charging", "Full",
>> + "Empty", "Idle"
>> };
>>
>> static const char * const power_supply_charge_type_text[] = {
>> diff --git a/drivers/power/supply/sbs-battery.c b/drivers/power/supply/sbs-battery.c
>> index ea8ba3e..e6c636c 100644
>> --- a/drivers/power/supply/sbs-battery.c
>> +++ b/drivers/power/supply/sbs-battery.c
>> @@ -294,14 +294,10 @@ static int sbs_status_correct(struct i2c_client *client, int *intval)
>>
>> ret = (s16)ret;
>>
>> - /* Not drawing current means full (cannot be not charging) */
>> - if (ret == 0)
>> - *intval = POWER_SUPPLY_STATUS_FULL;
>> -
>> - if (*intval == POWER_SUPPLY_STATUS_FULL) {
>> - /* Drawing or providing current when full */
>> - if (ret > 0)
>> - *intval = POWER_SUPPLY_STATUS_CHARGING;
>> + if (*intval == POWER_SUPPLY_STATUS_DISCHARGING) {
>> + /* Charging indicator not set in battery */
>> + if (ret == 0)
>> + *intval = POWER_SUPPLY_STATUS_IDLE;
> But doesn't the above already indicate that it _is_ discharging ?
>
>> else if (ret < 0)
>> *intval = POWER_SUPPLY_STATUS_DISCHARGING;
> This doesn't make sense. *intval is already set to
> POWER_SUPPLY_STATUS_DISCHARGING
> in this situation.
>
>> }
>> @@ -424,10 +420,12 @@ static int sbs_get_battery_property(struct i2c_client *client,
>>
>> if (ret & BATTERY_FULL_CHARGED)
>> val->intval = POWER_SUPPLY_STATUS_FULL;
>> - else if (ret & BATTERY_DISCHARGING)
>> - val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
>> - else
>> + else if (ret & BATTERY_FULL_DISCHARGED)
>> + val->intval = POWER_SUPPLY_STATUS_EMPTY;
>> + else if (!(ret & BATTERY_DISCHARGING))
>> val->intval = POWER_SUPPLY_STATUS_CHARGING;
>> + else
>> + val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
>>
>> sbs_status_correct(client, &val->intval);
>>
>> @@ -781,10 +779,12 @@ static void sbs_delayed_work(struct work_struct *work)
>>
>> if (ret & BATTERY_FULL_CHARGED)
>> ret = POWER_SUPPLY_STATUS_FULL;
>> - else if (ret & BATTERY_DISCHARGING)
>> - ret = POWER_SUPPLY_STATUS_DISCHARGING;
>> - else
>> + else if (ret & BATTERY_FULL_DISCHARGED)
>> + ret = POWER_SUPPLY_STATUS_EMPTY;
>> + else if (!(ret & BATTERY_DISCHARGING))
>> ret = POWER_SUPPLY_STATUS_CHARGING;
>> + else
>> + ret = POWER_SUPPLY_STATUS_DISCHARGING;
>>
>> sbs_status_correct(chip->client, &ret);
>>
>> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
>> index 28413f7..c9f3347 100644
>> --- a/include/linux/power_supply.h
>> +++ b/include/linux/power_supply.h
>> @@ -37,6 +37,8 @@ enum {
>> POWER_SUPPLY_STATUS_DISCHARGING,
>> POWER_SUPPLY_STATUS_NOT_CHARGING,
>> POWER_SUPPLY_STATUS_FULL,
>> + POWER_SUPPLY_STATUS_EMPTY,
>> + POWER_SUPPLY_STATUS_IDLE,
>> };
>>
>> /* What algorithm is the charger using? */
>> --
>> 1.8.3.1
>>
>

\
 
 \ /
  Last update: 2019-07-26 04:47    [W:0.083 / U:0.108 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site