linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Richard Tresidder <rtresidd@electromag.com.au>
To: sre@kernel.org, enric.balletbo@collabora.com,
	ncrews@chromium.org, andrew.smirnov@gmail.com,
	groeck@chromium.org, david@lechnology.com,
	rtresidd@electromag.com.au, tglx@linutronix.de,
	kstewart@linuxfoundation.org, gregkh@linuxfoundation.org,
	rfontana@redhat.com, allison@lohutok.net, baolin.wang@linaro.org,
	linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH v3 1/1] power/supply/sbs-battery: Fix confusing battery status when idle or empty
Date: Mon, 29 Jul 2019 16:07:39 +0800	[thread overview]
Message-ID: <1564387659-62064-1-git-send-email-rtresidd@electromag.com.au> (raw)

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 NOT CHARGING.
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 NOT CHARGING.
    Have also added the additional check for the battery FULL DISCHARGED flag
    which will now flag a status of EMPTY.
    
    v2: Missed a later merge that should have been included in original patch
    v3: Refactor the sbs_status_correct function to capture all the states for
        normal operation rather than being spread across multile functions.

 drivers/power/supply/power_supply_sysfs.c |  2 +-
 drivers/power/supply/sbs-battery.c        | 44 +++++++++++--------------------
 include/linux/power_supply.h              |  1 +
 3 files changed, 18 insertions(+), 29 deletions(-)

diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
index f37ad4e..305e833 100644
--- a/drivers/power/supply/power_supply_sysfs.c
+++ b/drivers/power/supply/power_supply_sysfs.c
@@ -51,7 +51,7 @@
 };
 
 static const char * const power_supply_status_text[] = {
-	"Unknown", "Charging", "Discharging", "Not charging", "Full"
+	"Unknown", "Charging", "Discharging", "Not charging", "Full", "Empty"
 };
 
 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 048d205..b28402d 100644
--- a/drivers/power/supply/sbs-battery.c
+++ b/drivers/power/supply/sbs-battery.c
@@ -293,16 +293,18 @@ 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;
-		else if (ret < 0)
-			*intval = POWER_SUPPLY_STATUS_DISCHARGING;
+	if (ret > 0)
+		*intval = POWER_SUPPLY_STATUS_CHARGING;
+	else if (ret < 0)
+		*intval = POWER_SUPPLY_STATUS_DISCHARGING;
+	else {
+		/* Current is 0, so how full is the battery? */
+		if (*intval & BATTERY_FULL_CHARGED)
+			*intval = POWER_SUPPLY_STATUS_FULL;
+		else if (*intval & BATTERY_FULL_DISCHARGED)
+			*intval = POWER_SUPPLY_STATUS_EMPTY;
+		else
+			*intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
 	}
 
 	return 0;
@@ -421,14 +423,9 @@ static int sbs_get_battery_property(struct i2c_client *client,
 			return 0;
 		}
 
-		if (ret & BATTERY_FULL_CHARGED)
-			val->intval = POWER_SUPPLY_STATUS_FULL;
-		else if (ret & BATTERY_DISCHARGING)
-			val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
-		else
-			val->intval = POWER_SUPPLY_STATUS_CHARGING;
-
-		sbs_status_correct(client, &val->intval);
+		ret = sbs_status_correct(client, &val->intval);
+		if (ret < 0)
+			return ret;
 
 		if (chip->poll_time == 0)
 			chip->last_state = val->intval;
@@ -773,20 +770,11 @@ static void sbs_delayed_work(struct work_struct *work)
 
 	ret = sbs_read_word_data(chip->client, sbs_data[REG_STATUS].addr);
 	/* if the read failed, give up on this work */
-	if (ret < 0) {
+	if ((ret < 0) || (sbs_status_correct(chip->client, &ret) < 0)) {
 		chip->poll_time = 0;
 		return;
 	}
 
-	if (ret & BATTERY_FULL_CHARGED)
-		ret = POWER_SUPPLY_STATUS_FULL;
-	else if (ret & BATTERY_DISCHARGING)
-		ret = POWER_SUPPLY_STATUS_DISCHARGING;
-	else
-		ret = POWER_SUPPLY_STATUS_CHARGING;
-
-	sbs_status_correct(chip->client, &ret);
-
 	if (chip->last_state != ret) {
 		chip->poll_time = 0;
 		power_supply_changed(chip->power_supply);
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 28413f7..8fb10ec 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -37,6 +37,7 @@ enum {
 	POWER_SUPPLY_STATUS_DISCHARGING,
 	POWER_SUPPLY_STATUS_NOT_CHARGING,
 	POWER_SUPPLY_STATUS_FULL,
+	POWER_SUPPLY_STATUS_EMPTY,
 };
 
 /* What algorithm is the charger using? */
-- 
1.8.3.1


             reply	other threads:[~2019-07-29  8:08 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-29  8:07 Richard Tresidder [this message]
2019-07-29 13:24 ` [PATCH v3 1/1] power/supply/sbs-battery: Fix confusing battery status when idle or empty Guenter Roeck
2019-07-29 17:17 ` Nick Crews

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1564387659-62064-1-git-send-email-rtresidd@electromag.com.au \
    --to=rtresidd@electromag.com.au \
    --cc=allison@lohutok.net \
    --cc=andrew.smirnov@gmail.com \
    --cc=baolin.wang@linaro.org \
    --cc=david@lechnology.com \
    --cc=enric.balletbo@collabora.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=groeck@chromium.org \
    --cc=kstewart@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=ncrews@chromium.org \
    --cc=rfontana@redhat.com \
    --cc=sre@kernel.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).