| 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
| ||
|
Message-ID: <CABXOdTdz=+P-HXaUbGAuLBjNE1GA0C8o4OPmF996DOrXxkQJAg@mail.gmail.com>
Date: Thu, 25 Jul 2019 06:39:48 -0700
From: Guenter Roeck <groeck@...gle.com>
To: Richard Tresidder <rtresidd@...ctromag.com.au>
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
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