From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7F4E2C76194 for ; Fri, 26 Jul 2019 02:46:40 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 5760A22C7C for ; Fri, 26 Jul 2019 02:46:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726102AbfGZCqj (ORCPT ); Thu, 25 Jul 2019 22:46:39 -0400 Received: from anchovy2.45ru.net.au ([203.30.46.146]:45084 "EHLO anchovy2.45ru.net.au" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725942AbfGZCqi (ORCPT ); Thu, 25 Jul 2019 22:46:38 -0400 X-Greylist: delayed 363 seconds by postgrey-1.27 at vger.kernel.org; Thu, 25 Jul 2019 22:46:38 EDT Received: (qmail 30833 invoked by uid 5089); 26 Jul 2019 02:46:34 -0000 Received: by simscan 1.2.0 ppid: 30700, pid: 30704, t: 0.4346s scanners: regex: 1.2.0 attach: 1.2.0 clamav: 0.88.3/m:40/d:1950 spam: 3.1.4 Received: from unknown (HELO ?192.168.0.34?) (rtresidd@electromag.com.au@203.59.235.95) by anchovy3.45ru.net.au with ESMTPA; 26 Jul 2019 02:46:33 -0000 Subject: Re: [PATCH 1/1] power/supply/sbs-battery: Fix confusing battery status when idle or empty To: Guenter Roeck Cc: Sebastian Reichel , Enric Balletbo i Serra , Nick Crews , andrew.smirnov@gmail.com, Guenter Roeck , david@lechnology.com, Thomas Gleixner , Kate Stewart , Greg Kroah-Hartman , rfontana@redhat.com, allison@lohutok.net, baolin.wang@linaro.org, Linux PM list , linux-kernel References: <1564043102-25298-1-git-send-email-rtresidd@electromag.com.au> From: Richard Tresidder Message-ID: <71a968f7-88c9-aa6c-6822-edfc12484d91@electromag.com.au> Date: Fri, 26 Jul 2019 10:46:27 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-AU Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > 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 >> --- >> >> 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 >> >