Messages in this thread | | | From | Guenter Roeck <> | Date | Thu, 25 Jul 2019 06:39:48 -0700 | Subject | Re: [PATCH 1/1] power/supply/sbs-battery: Fix confusing battery status when idle or empty |
| |
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 >
| |