All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: stable@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	patches@lists.linux.dev,
	Matthias Schiffer <matthias.schiffer@ew.tq-group.com>,
	Sebastian Reichel <sebastian.reichel@collabora.com>,
	Sasha Levin <sashal@kernel.org>
Subject: [PATCH 5.10 05/22] power: supply: bq27xxx: make status more robust
Date: Thu,  1 Jun 2023 14:21:03 +0100	[thread overview]
Message-ID: <20230601131933.968973306@linuxfoundation.org> (raw)
In-Reply-To: <20230601131933.727832920@linuxfoundation.org>

From: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>

[ Upstream commit c3a6d6a1dfc8a9bf12d79a0b1a30cb24c92a2ddf ]

There are multiple issues in bq27xxx_battery_status():

- On BQ28Q610 is was observed that the "full" flag may be set even while
  the battery is charging or discharging. With the current logic to make
  "full" override everything else, it look a very long time (>20min) for
  the status to change from "full" to "discharging" after unplugging the
  supply on a device with low power consumption
- The POWER_SUPPLY_STATUS_NOT_CHARGING check depends on
  power_supply_am_i_supplied(), which will not work when the supply
  doesn't exist as a separate device known to Linux

We can solve both issues by deriving the status from the current instead
of the flags field. The flags are now only used to distinguish "full"
from "not charging", and to determine the sign of the current on
BQ27XXX_O_ZERO devices.

Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Stable-dep-of: 35092c5819f8 ("power: supply: bq27xxx: Add cache parameter to bq27xxx_battery_current_and_status()")
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/power/supply/bq27xxx_battery.c | 88 +++++++++++++-------------
 1 file changed, 43 insertions(+), 45 deletions(-)

diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index d34f1fceadbb4..681fa81f4dbde 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -1754,14 +1754,27 @@ static void bq27xxx_battery_poll(struct work_struct *work)
 	bq27xxx_battery_update(di);
 }
 
+static bool bq27xxx_battery_is_full(struct bq27xxx_device_info *di, int flags)
+{
+	if (di->opts & BQ27XXX_O_ZERO)
+		return (flags & BQ27000_FLAG_FC);
+	else if (di->opts & BQ27Z561_O_BITS)
+		return (flags & BQ27Z561_FLAG_FC);
+	else
+		return (flags & BQ27XXX_FLAG_FC);
+}
+
 /*
- * Return the battery average current in µA
+ * Return the battery average current in µA and the status
  * Note that current can be negative signed as well
  * Or 0 if something fails.
  */
-static int bq27xxx_battery_current(struct bq27xxx_device_info *di,
-				   union power_supply_propval *val)
+static int bq27xxx_battery_current_and_status(
+	struct bq27xxx_device_info *di,
+	union power_supply_propval *val_curr,
+	union power_supply_propval *val_status)
 {
+	bool single_flags = (di->opts & BQ27XXX_O_ZERO);
 	int curr;
 	int flags;
 
@@ -1771,17 +1784,39 @@ static int bq27xxx_battery_current(struct bq27xxx_device_info *di,
 		return curr;
 	}
 
+	flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, single_flags);
+	if (flags < 0) {
+		dev_err(di->dev, "error reading flags\n");
+		return flags;
+	}
+
 	if (di->opts & BQ27XXX_O_ZERO) {
-		flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, true);
 		if (!(flags & BQ27000_FLAG_CHGS)) {
 			dev_dbg(di->dev, "negative current!\n");
 			curr = -curr;
 		}
 
-		val->intval = curr * BQ27XXX_CURRENT_CONSTANT / BQ27XXX_RS;
+		curr = curr * BQ27XXX_CURRENT_CONSTANT / BQ27XXX_RS;
 	} else {
 		/* Other gauges return signed value */
-		val->intval = (int)((s16)curr) * 1000;
+		curr = (int)((s16)curr) * 1000;
+	}
+
+	if (val_curr)
+		val_curr->intval = curr;
+
+	if (val_status) {
+		if (curr > 0) {
+			val_status->intval = POWER_SUPPLY_STATUS_CHARGING;
+		} else if (curr < 0) {
+			val_status->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+		} else {
+			if (bq27xxx_battery_is_full(di, flags))
+				val_status->intval = POWER_SUPPLY_STATUS_FULL;
+			else
+				val_status->intval =
+					POWER_SUPPLY_STATUS_NOT_CHARGING;
+		}
 	}
 
 	return 0;
@@ -1813,43 +1848,6 @@ static int bq27xxx_battery_pwr_avg(struct bq27xxx_device_info *di,
 	return 0;
 }
 
-static int bq27xxx_battery_status(struct bq27xxx_device_info *di,
-				  union power_supply_propval *val)
-{
-	int status;
-
-	if (di->opts & BQ27XXX_O_ZERO) {
-		if (di->cache.flags & BQ27000_FLAG_FC)
-			status = POWER_SUPPLY_STATUS_FULL;
-		else if (di->cache.flags & BQ27000_FLAG_CHGS)
-			status = POWER_SUPPLY_STATUS_CHARGING;
-		else
-			status = POWER_SUPPLY_STATUS_DISCHARGING;
-	} else if (di->opts & BQ27Z561_O_BITS) {
-		if (di->cache.flags & BQ27Z561_FLAG_FC)
-			status = POWER_SUPPLY_STATUS_FULL;
-		else if (di->cache.flags & BQ27Z561_FLAG_DIS_CH)
-			status = POWER_SUPPLY_STATUS_DISCHARGING;
-		else
-			status = POWER_SUPPLY_STATUS_CHARGING;
-	} else {
-		if (di->cache.flags & BQ27XXX_FLAG_FC)
-			status = POWER_SUPPLY_STATUS_FULL;
-		else if (di->cache.flags & BQ27XXX_FLAG_DSC)
-			status = POWER_SUPPLY_STATUS_DISCHARGING;
-		else
-			status = POWER_SUPPLY_STATUS_CHARGING;
-	}
-
-	if ((status == POWER_SUPPLY_STATUS_DISCHARGING) &&
-	    (power_supply_am_i_supplied(di->bat) > 0))
-		status = POWER_SUPPLY_STATUS_NOT_CHARGING;
-
-	val->intval = status;
-
-	return 0;
-}
-
 static int bq27xxx_battery_capacity_level(struct bq27xxx_device_info *di,
 					  union power_supply_propval *val)
 {
@@ -1935,7 +1933,7 @@ static int bq27xxx_battery_get_property(struct power_supply *psy,
 
 	switch (psp) {
 	case POWER_SUPPLY_PROP_STATUS:
-		ret = bq27xxx_battery_status(di, val);
+		ret = bq27xxx_battery_current_and_status(di, NULL, val);
 		break;
 	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
 		ret = bq27xxx_battery_voltage(di, val);
@@ -1944,7 +1942,7 @@ static int bq27xxx_battery_get_property(struct power_supply *psy,
 		val->intval = di->cache.flags < 0 ? 0 : 1;
 		break;
 	case POWER_SUPPLY_PROP_CURRENT_NOW:
-		ret = bq27xxx_battery_current(di, val);
+		ret = bq27xxx_battery_current_and_status(di, val, NULL);
 		break;
 	case POWER_SUPPLY_PROP_CAPACITY:
 		ret = bq27xxx_simple_value(di->cache.capacity, val);
-- 
2.39.2




  parent reply	other threads:[~2023-06-01 13:23 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-01 13:20 [PATCH 5.10 00/22] 5.10.182-rc1 review Greg Kroah-Hartman
2023-06-01 13:20 ` [PATCH 5.10 01/22] x86/cpu: Add Raptor Lake to Intel family Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.10 02/22] x86/cpu: Drop spurious underscore from RAPTOR_LAKE #define Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.10 03/22] power: supply: bq27xxx: fix polarity of current_now Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.10 04/22] power: supply: bq27xxx: fix sign of current_now for newer ICs Greg Kroah-Hartman
2023-06-01 13:21 ` Greg Kroah-Hartman [this message]
2023-06-01 13:21 ` [PATCH 5.10 06/22] power: supply: bq27xxx: Add cache parameter to bq27xxx_battery_current_and_status() Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.10 07/22] power: supply: bq27xxx: expose battery data when CI=1 Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.10 08/22] power: supply: bq27xxx: Move bq27xxx_battery_update() down Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.10 09/22] power: supply: bq27xxx: Ensure power_supply_changed() is called on current sign changes Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.10 10/22] power: supply: bq27xxx: After charger plug in/out wait 0.5s for things to stabilize Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.10 11/22] power: supply: core: Refactor power_supply_set_input_current_limit_from_supplier() Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.10 12/22] power: supply: bq24190: Call power_supply_changed() after updating input current Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.10 13/22] regulator: Add regmap helper for ramp-delay setting Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.10 14/22] regulator: pca9450: Convert to use regulator_set_ramp_delay_regmap Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.10 15/22] regulator: pca9450: Fix BUCK2 enable_mask Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.10 16/22] net/mlx5: devcom only supports 2 ports Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.10 17/22] net/mlx5: Devcom, serialize devcom registration Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.10 18/22] net: phy: mscc: enable VSC8501/2 RGMII RX clock Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.10 19/22] bluetooth: Add cmd validity checks at the start of hci_sock_ioctl() Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.10 20/22] binder: fix UAF caused by faulty buffer cleanup Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.10 21/22] ipv{4,6}/raw: fix output xfrm lookup wrt protocol Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.10 22/22] netfilter: ctnetlink: Support offloaded conntrack entry deletion Greg Kroah-Hartman
2023-06-01 16:22 ` [PATCH 5.10 00/22] 5.10.182-rc1 review Florian Fainelli
2023-06-01 20:47 ` Shuah Khan
2023-06-02  8:45 ` Jon Hunter
2023-06-02 10:21 ` Naresh Kamboju
2023-06-02 22:34 ` Guenter Roeck
2023-06-05  9:16 ` Chris Paterson

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=20230601131933.968973306@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=matthias.schiffer@ew.tq-group.com \
    --cc=patches@lists.linux.dev \
    --cc=sashal@kernel.org \
    --cc=sebastian.reichel@collabora.com \
    --cc=stable@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.