| 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: <71a968f7-88c9-aa6c-6822-edfc12484d91@electromag.com.au>
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