Linux-PM Archive mirror
 help / color / mirror / Atom feed
From: "Nícolas F. R. A. Prado" <nfraprado@collabora.com>
To: Sebastian Reichel <sre@kernel.org>
Cc: kernel@collabora.com, linux-pm@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	"Pin-yen Lin" <treapking@chromium.org>,
	"Hsin-Te Yuan" <yuanhsinte@chromium.org>,
	"Nícolas F. R. A. Prado" <nfraprado@collabora.com>
Subject: [PATCH v3] power: supply: sbs-battery: Handle unsupported PROP_TIME_TO_EMPTY_NOW
Date: Thu, 18 Apr 2024 13:34:23 -0400	[thread overview]
Message-ID: <20240418-sbs-time-empty-now-error-v3-1-f286e29e3fca@collabora.com> (raw)

Despite the RunTimeToEmpty() (0x11) function being defined in the SBS
specification as required, it seems that not all batteries implement it.
On platforms with such batteries, reading the property will cause an
error to be printed:

power_supply sbs-8-000b: driver failed to report `time_to_empty_now' property: -5

This not only pollutes the log, distracting from real problems on the
device, but also prevents the uevent file from being read since it
contains all properties, including the faulty one.

The following table summarizes the findings for a handful of platforms:

Platform                                Status  Manufacturer    Model
------------------------------------------------------------------------
mt8186-corsola-steelix-sku131072        OK      BYD             L22B3PG0
mt8195-cherry-tomato-r2                 NOT OK  PANASON         AP16L5J
mt8192-asurada-spherion-r0              NOT OK  PANASON         AP15O5L
mt8183-kukui-jacuzzi-juniper-sku16      NOT OK  LGC KT0         AP16L8J
mt8173-elm-hana                         OK      Sunwoda         L18D3PG1
sc7180-trogdor-lazor-limozeen-nots-r5   NOT OK  Murata          AP18C4K
sc7180-trogdor-kingoftown               NOT OK  333-AC-0D-A     GG02047XL
rk3399-gru-kevin                        OK      SDI             4352D51

Detect if this is one of the quirky batteries during presence update, so
that hot-plugging works as expected, and if so report -ENODATA for
POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW, which removes it from uevent and
prevents throwing errors.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---
Changes in v3:
- Reordered variable declarations and removed unneeded initialization
- Link to v2: https://lore.kernel.org/r/20240415-sbs-time-empty-now-error-v2-1-32d8a747e308@collabora.com

Changes in v2:
- Reworked patch to lay down and use a proper quirk infrastructure, and
  update the quirks on the presence update callback so it works properly
  even when hot-plugging different batteries
- Link to v1: https://lore.kernel.org/r/20240307-sbs-time-empty-now-error-v1-1-18d0f8702330@collabora.com
---
 drivers/power/supply/sbs-battery.c | 55 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/drivers/power/supply/sbs-battery.c b/drivers/power/supply/sbs-battery.c
index a6c204c08232..2b1481b81b78 100644
--- a/drivers/power/supply/sbs-battery.c
+++ b/drivers/power/supply/sbs-battery.c
@@ -214,6 +214,7 @@ struct sbs_info {
 	struct delayed_work		work;
 	struct mutex			mode_lock;
 	u32				flags;
+	u32				quirks;
 	int				technology;
 	char				strings[NR_STRING_BUFFERS][I2C_SMBUS_BLOCK_MAX + 1];
 };
@@ -263,6 +264,54 @@ static void sbs_disable_charger_broadcasts(struct sbs_info *chip)
 		dev_dbg(&chip->client->dev, "%s\n", __func__);
 }
 
+/* Required by the spec, but missing in some implementations */
+#define SBS_QUIRK_BROKEN_TTE_NOW	BIT(0)
+
+struct sbs_quirk_entry {
+	const char *manufacturer;
+	const char *model;
+	u32 flags;
+};
+
+static const struct sbs_quirk_entry sbs_quirks[] = {
+	{"PANASON", "AP16L5J", SBS_QUIRK_BROKEN_TTE_NOW},
+	{"PANASON", "AP15O5L", SBS_QUIRK_BROKEN_TTE_NOW},
+	{"LGC KT0", "AP16L8J", SBS_QUIRK_BROKEN_TTE_NOW},
+	{"Murata", "AP18C4K", SBS_QUIRK_BROKEN_TTE_NOW},
+	{"333-AC-0D-A", "GG02047XL", SBS_QUIRK_BROKEN_TTE_NOW},
+};
+
+static const char *sbs_get_constant_string(struct sbs_info *chip,
+					   enum power_supply_property psp);
+
+static void sbs_update_quirks(struct sbs_info *chip)
+{
+	const char *manufacturer;
+	const char *model;
+	unsigned int i;
+
+	/* reset quirks from battery before the hot-plug event */
+	chip->quirks = 0;
+
+	manufacturer = sbs_get_constant_string(chip, POWER_SUPPLY_PROP_MANUFACTURER);
+	model = sbs_get_constant_string(chip, POWER_SUPPLY_PROP_MODEL_NAME);
+	if (IS_ERR(manufacturer) || IS_ERR(model)) {
+		dev_warn(&chip->client->dev, "Couldn't read manufacturer and model to set quirks\n");
+		return;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(sbs_quirks); i++) {
+		if (strcmp(manufacturer, sbs_quirks[i].manufacturer))
+			continue;
+		if (strcmp(model, sbs_quirks[i].model))
+			continue;
+		chip->quirks |= sbs_quirks[i].flags;
+	}
+
+	if (chip->quirks & SBS_QUIRK_BROKEN_TTE_NOW)
+		dev_info(&chip->client->dev, "Added quirk disabling TIME_TO_EMPTY_NOW\n");
+}
+
 static int sbs_update_presence(struct sbs_info *chip, bool is_present)
 {
 	struct i2c_client *client = chip->client;
@@ -323,6 +372,8 @@ static int sbs_update_presence(struct sbs_info *chip, bool is_present)
 	dev_dbg(&client->dev, "PEC: %s\n", (client->flags & I2C_CLIENT_PEC) ?
 		"enabled" : "disabled");
 
+	sbs_update_quirks(chip);
+
 	if (!chip->is_present && is_present && !chip->charger_broadcasts)
 		sbs_disable_charger_broadcasts(chip);
 
@@ -614,6 +665,10 @@ static int sbs_get_battery_property(struct i2c_client *client,
 	struct sbs_info *chip = i2c_get_clientdata(client);
 	s32 ret;
 
+	if (psp == POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW &&
+	    chip->quirks & SBS_QUIRK_BROKEN_TTE_NOW)
+		return -ENODATA;
+
 	ret = sbs_read_word_data(client, sbs_data[reg_offset].addr);
 	if (ret < 0)
 		return ret;

---
base-commit: 7b4f2bc91c15fdcf948bb2d9741a9d7d54303f8d
change-id: 20240307-sbs-time-empty-now-error-322bc074d3f2

Best regards,
-- 
Nícolas F. R. A. Prado <nfraprado@collabora.com>


             reply	other threads:[~2024-04-18 17:34 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-18 17:34 Nícolas F. R. A. Prado [this message]
2024-04-19  7:26 ` [PATCH v3] power: supply: sbs-battery: Handle unsupported PROP_TIME_TO_EMPTY_NOW AngeloGioacchino Del Regno
2024-04-19 13:25   ` Sebastian Reichel
2024-04-19 16:03 ` Nícolas F. R. A. Prado
2024-04-22  8:10   ` Hsin-Te Yuan
2024-05-09 15:25     ` Nícolas F. R. A. Prado
2024-05-09 15:43       ` AngeloGioacchino Del Regno
2024-05-13 13:27         ` Nícolas F. R. A. Prado

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=20240418-sbs-time-empty-now-error-v3-1-f286e29e3fca@collabora.com \
    --to=nfraprado@collabora.com \
    --cc=kernel@collabora.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=sre@kernel.org \
    --cc=treapking@chromium.org \
    --cc=yuanhsinte@chromium.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 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 read-only IMAP folder(s) and NNTP newsgroup(s).