lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 26 Jul 2019 10:46:27 +0800 From: Richard Tresidder <rtresidd@...ctromag.com.au> To: Guenter Roeck <groeck@...gle.com> Cc: Sebastian Reichel <sre@...nel.org>, Enric Balletbo i Serra <enric.balletbo@...labora.com>, Nick Crews <ncrews@...omium.org>, andrew.smirnov@...il.com, Guenter Roeck <groeck@...omium.org>, david@...hnology.com, Thomas Gleixner <tglx@...utronix.de>, Kate Stewart <kstewart@...uxfoundation.org>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, rfontana@...hat.com, allison@...utok.net, baolin.wang@...aro.org, Linux PM list <linux-pm@...r.kernel.org>, linux-kernel <linux-kernel@...r.kernel.org> Subject: Re: [PATCH 1/1] power/supply/sbs-battery: Fix confusing battery status when idle or empty 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@...ctromag.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@...ctromag.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 >> >
Powered by blists - more mailing lists