Re: [PATCH 2/3] platform: arm64: lenovo-thinkpad-t14s: Add Gen 7 EC support

[Hans de Goede <hansg@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1646 bytes --]

Hi Abel,

On 9-Nov-26 15:52, Hans de Goede wrote:

<snip>

> For easier review by you I've split the improvements into 4 patches (attached):

<snip>

> 4: Cleanup: has_kbd_bl2_and_standby and mic_mute_led_generic checks are
> really just checks for the original (gen6) support vs gen7 support
> drop the bools and directly check which variant ec->variant points
> at instead.

I accidentally left some debug code in patch 4, attached is a new version.


<snip>

> So about these differences:
> 
> 1: For the spk / mic mute LEDs, on a hunch I tried using std LED controls
> with a LED idx/id of 15 for the spk mute LED and that also works.
> 
> I did not find either way (REG_AUD vs REG_LED) for the spk mute LED in
> the DSDT, instead the DSDT ends up sending an ACPI notify event, relying
> on some windows driver to handle this.
> 
> How did you find that REG_AUD still works, did you just try the gen 6
> method ?
> 
> 2: For the kbd backlight, again the DSDT ends up sending an ACPI notify event
> and does not access the EC itself at all.
> 
> How did you find that T14S_EC_REG_KBD_BL1 also works on gen 7?

One more remark about the kbd-backlight the new mode which reports itself
as brightness 3 is an auto mode with brightness 2, but it turns off
the backlight after not using the keyboard for a while (and back on again
on the next keypress).

Newer x86 Thinkpads also have this and a whole new userspace API has just
landed to support this. Here is a patch adding support for this auto
mode to the x86 thinkpad_acpi driver:

https://lore.kernel.org/platform-driver-x86/20261101100123.28749-1-kmatzkaczor244@gmail.com/

Regards,

Hans



[-- Attachment #2: 0004-fixup-platform-arm64-lenovo-thinkpad-t14s-add-Gen-7-.patch --]
[-- Type: text/x-patch, Size: 3004 bytes --]

From 0e77c36ed2c9aba498723465f01ff6bb217abada Mon Sep 17 00:00:00 2001
From: Hans de Goede <johannes.goede@oss.qualcomm.com>
Date: Mon, 9 Nov 2026 14:29:37 +0100
Subject: [PATCH 4/4] fixup! platform: arm64: lenovo-thinkpad-t14s: add Gen 7
 EC support

Drop the xfer_lock mutex and instead use i2c_lock_bus() like
the existing t14s gen6 support does for multi-msg transactions.

Also drop the fsleep(5000) in t14s_gen7_ec_cmd_read() the DSDT code
has no delay between the transfers so this is not necessary.

Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
---
 drivers/platform/arm64/lenovo-thinkpad-t14s.c | 30 ++++++++-----------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/drivers/platform/arm64/lenovo-thinkpad-t14s.c b/drivers/platform/arm64/lenovo-thinkpad-t14s.c
index a24bf95268dc..45b5cde69e3e 100644
--- a/drivers/platform/arm64/lenovo-thinkpad-t14s.c
+++ b/drivers/platform/arm64/lenovo-thinkpad-t14s.c
@@ -132,8 +132,6 @@ struct t14s_ec {
 	struct regmap *regmap;
 	struct device *dev;
 	const struct t14s_ec_variant *variant;
-	/* serializes the multi-message Gen 7 EC transactions */
-	struct mutex xfer_lock;
 	enum led_brightness mic_mute_cache;
 	struct t14s_ec_led_classdev led_pwr_btn;
 	struct t14s_ec_led_classdev led_chrg_orange;
@@ -250,24 +248,28 @@ static int t14s_gen7_ec_cmd_read(struct t14s_ec *ec, u8 cmd, u8 arg, u8 *val)
 	struct i2c_client *client = to_i2c_client(ec->dev);
 	u8 req[2] = { cmd, arg };
 	struct i2c_msg msgs[2] = {
-		{ .addr = client->addr, .flags = 0, .len = 1, .buf = &req[0] },
+		{ .addr = client->addr, .flags = 0, .len = 2, .buf = &req[0] },
 		{ .addr = client->addr, .flags = I2C_M_RD, .len = 1, .buf = val },
 	};
 	int ret;
 
-	guard(mutex)(&ec->xfer_lock);
+	i2c_lock_bus(client->adapter, I2C_LOCK_SEGMENT);
 
-	ret = i2c_master_send(client, req, sizeof(req));
+	/* Send cmd + arg */
+	ret = __i2c_transfer(client->adapter, msgs, 1);
 	if (ret < 0)
-		return ret;
+		goto out;
 
-	fsleep(5000);
-
-	ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
+	/* Resend the cmd byte + read the value */
+	msgs[0].len = 1;
+	ret = __i2c_transfer(client->adapter, msgs, 2);
 	if (ret < 0)
-		return ret;
+		goto out;
 
-	return ret == ARRAY_SIZE(msgs) ? 0 : -EIO;
+	ret = ret == 2 ? 0 : EIO;
+out:
+	i2c_unlock_bus(client->adapter, I2C_LOCK_SEGMENT);
+	return ret;
 }
 
 static int t14s_gen7_ec_write(void *context, unsigned int reg,
@@ -278,8 +280,6 @@ static int t14s_gen7_ec_write(void *context, unsigned int reg,
 	u8 buf[3] = { T14S_GEN7_EC_CMD_WR, reg, val };
 	int ret;
 
-	guard(mutex)(&ec->xfer_lock);
-
 	ret = i2c_master_send(client, buf, sizeof(buf));
 	if (ret < 0)
 		return ret;
@@ -732,10 +732,6 @@ static int t14s_ec_probe(struct i2c_client *client)
 	if (!ec->variant)
 		ec->variant = &t14s_ec_variant;
 
-	ret = devm_mutex_init(dev, &ec->xfer_lock);
-	if (ret)
-		return ret;
-
 	i2c_set_clientdata(client, ec);
 
 	ec->regmap = devm_regmap_init(dev, ec->variant->regmap_bus,
-- 
2.55.0