mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 0/4] platform: arm64: thinkpad-t14s-ec: fixes and suspend support
@ 2025-11-19  0:41 Sebastian Reichel
  2025-11-19  0:41 ` [PATCH v2 1/4] platform: arm64: thinkpad-t14s-ec: fix IRQ race condition Sebastian Reichel
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Sebastian Reichel @ 2025-11-19  0:41 UTC (permalink / raw)
  To: Sebastian Reichel, Hans de Goede, Ilpo Järvinen,
	Bryan O'Donoghue
  Cc: platform-driver-x86, linux-kernel

I have a two fixes for the T14s Gen6 Snapdragon EC driver as well as
improved system suspend support. I've been using my Thinkpad with these
patches for a few weeks as daily driver doing regular system suspend and
things seem to run well.

If there is enough time after review the fixes are probably good
material for the 6.18 release cycle.

Changes in v2:
- Link to v1: https://patch.msgid.link/20251106-thinkpad-t14s-ec-improvements-v1-0-109548ae75c0@collabora.com
- Add dmesg call trace in first patch description
- Mention ECWS ACPI function in PM patch description
- Include linux/pm.h in second patch
- Mention Thinkpad ACPI driver behavior in last patch

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
Sebastian Reichel (4):
      platform: arm64: thinkpad-t14s-ec: fix IRQ race condition
      platform: arm64: thinkpad-t14s-ec: sleep after EC access
      platform: arm64: thinkpad-t14s-ec: add system PM hooks
      platform: arm64: thinkpad-t14s-ec: add suspend handler for keyboard backlight

 drivers/platform/arm64/lenovo-thinkpad-t14s.c | 74 ++++++++++++++++++++++-----
 1 file changed, 60 insertions(+), 14 deletions(-)
---
base-commit: 6a23ae0a96a600d1d12557add110e0bb6e32730c
change-id: 20251105-thinkpad-t14s-ec-improvements-44d4c38fef71

Best regards,
-- 
Sebastian Reichel <sre@kernel.org>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2 1/4] platform: arm64: thinkpad-t14s-ec: fix IRQ race condition
  2025-11-19  0:41 [PATCH v2 0/4] platform: arm64: thinkpad-t14s-ec: fixes and suspend support Sebastian Reichel
@ 2025-11-19  0:41 ` Sebastian Reichel
  2025-11-19  0:41 ` [PATCH v2 2/4] platform: arm64: thinkpad-t14s-ec: sleep after EC access Sebastian Reichel
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Sebastian Reichel @ 2025-11-19  0:41 UTC (permalink / raw)
  To: Sebastian Reichel, Hans de Goede, Ilpo Järvinen,
	Bryan O'Donoghue
  Cc: platform-driver-x86, linux-kernel

Fix a race condition, that an input key related interrupt might be
triggered before the input handler has been registered, which results
in a NULL pointer dereference. This can happen if the user enables
the keyboard backlight shortly before the driver is being probed.

This fixes the following backtrace visible in dmesg:

Unable to handle kernel NULL pointer dereference at virtual address 00000000000000e0
...
Call trace:
 sparse_keymap_report_event+0x2c/0x978 [sparse_keymap] (P)
 t14s_ec_irq_handler+0x190/0x3e8 [lenovo_thinkpad_t14s]
 irq_thread_fn+0x30/0xb8
 irq_thread+0x18c/0x3b0
 kthread+0x148/0x228
 ret_from_fork+0x10/0x20

Fixes: 60b7ab6ce030 ("platform: arm64: thinkpad-t14s-ec: new driver")
Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/platform/arm64/lenovo-thinkpad-t14s.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/arm64/lenovo-thinkpad-t14s.c b/drivers/platform/arm64/lenovo-thinkpad-t14s.c
index 1d5d11adaf32..c1c01b977f2b 100644
--- a/drivers/platform/arm64/lenovo-thinkpad-t14s.c
+++ b/drivers/platform/arm64/lenovo-thinkpad-t14s.c
@@ -557,12 +557,6 @@ static int t14s_ec_probe(struct i2c_client *client)
 		return dev_err_probe(dev, PTR_ERR(ec->regmap),
 				     "Failed to init regmap\n");
 
-	ret = devm_request_threaded_irq(dev, client->irq, NULL,
-					t14s_ec_irq_handler,
-					IRQF_ONESHOT, dev_name(dev), ec);
-	if (ret < 0)
-		return dev_err_probe(dev, ret, "Failed to get IRQ\n");
-
 	ret = t14s_leds_probe(ec);
 	if (ret < 0)
 		return ret;
@@ -579,6 +573,12 @@ static int t14s_ec_probe(struct i2c_client *client)
 	if (ret < 0)
 		return ret;
 
+	ret = devm_request_threaded_irq(dev, client->irq, NULL,
+					t14s_ec_irq_handler,
+					IRQF_ONESHOT, dev_name(dev), ec);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Failed to get IRQ\n");
+
 	/*
 	 * Disable wakeup support by default, because the driver currently does
 	 * not support masking any events and the laptop should not wake up when

-- 
2.51.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2 2/4] platform: arm64: thinkpad-t14s-ec: sleep after EC access
  2025-11-19  0:41 [PATCH v2 0/4] platform: arm64: thinkpad-t14s-ec: fixes and suspend support Sebastian Reichel
  2025-11-19  0:41 ` [PATCH v2 1/4] platform: arm64: thinkpad-t14s-ec: fix IRQ race condition Sebastian Reichel
@ 2025-11-19  0:41 ` Sebastian Reichel
  2025-11-19  0:41 ` [PATCH v2 3/4] platform: arm64: thinkpad-t14s-ec: add system PM hooks Sebastian Reichel
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Sebastian Reichel @ 2025-11-19  0:41 UTC (permalink / raw)
  To: Sebastian Reichel, Hans de Goede, Ilpo Järvinen,
	Bryan O'Donoghue
  Cc: platform-driver-x86, linux-kernel

The ACPI ECRD and ECWR functions have a 10ms sleep at the end. It turns
out, that this is sometimes needed to avoid I2C transmission failures,
especially for functions doing regmap_update_bits (and thus read + write
shortly after each other). This fixes problems like the following
appearing in the kernel log:

leds platform::micmute: Setting an LED's brightness failed (-6)
leds platform::kbd_backlight: Setting an LED's brightness failed (-6)

The ACPI QEVT function used to read the interrupt status register also
has a 10ms sleep at the end. Without that there are problems with
reading multiple events following directly after each other resulting
in the following error message being logged:

thinkpad-t14s-ec 4-0028: Failed to read event

Fixes: 60b7ab6ce030 ("platform: arm64: thinkpad-t14s-ec: new driver")
Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/platform/arm64/lenovo-thinkpad-t14s.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/platform/arm64/lenovo-thinkpad-t14s.c b/drivers/platform/arm64/lenovo-thinkpad-t14s.c
index c1c01b977f2b..cf6a1d3b2617 100644
--- a/drivers/platform/arm64/lenovo-thinkpad-t14s.c
+++ b/drivers/platform/arm64/lenovo-thinkpad-t14s.c
@@ -120,6 +120,7 @@ static int t14s_ec_write(void *context, unsigned int reg,
 	if (ret < 0)
 		return ret;
 
+	fsleep(10000);
 	return 0;
 }
 
@@ -157,6 +158,7 @@ static int t14s_ec_read(void *context, unsigned int reg,
 
 out:
 	i2c_unlock_bus(client->adapter, I2C_LOCK_SEGMENT);
+	fsleep(10000);
 	return ret;
 }
 
@@ -191,6 +193,8 @@ static int t14s_ec_read_evt(struct t14s_ec *ec, u8 *val)
 	if (ret < 0)
 		goto out;
 
+	fsleep(10000);
+
 	ret = 0;
 
 out:

-- 
2.51.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2 3/4] platform: arm64: thinkpad-t14s-ec: add system PM hooks
  2025-11-19  0:41 [PATCH v2 0/4] platform: arm64: thinkpad-t14s-ec: fixes and suspend support Sebastian Reichel
  2025-11-19  0:41 ` [PATCH v2 1/4] platform: arm64: thinkpad-t14s-ec: fix IRQ race condition Sebastian Reichel
  2025-11-19  0:41 ` [PATCH v2 2/4] platform: arm64: thinkpad-t14s-ec: sleep after EC access Sebastian Reichel
@ 2025-11-19  0:41 ` Sebastian Reichel
  2025-11-19  0:41 ` [PATCH v2 4/4] platform: arm64: thinkpad-t14s-ec: add suspend handler for keyboard backlight Sebastian Reichel
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Sebastian Reichel @ 2025-11-19  0:41 UTC (permalink / raw)
  To: Sebastian Reichel, Hans de Goede, Ilpo Järvinen,
	Bryan O'Donoghue
  Cc: platform-driver-x86, linux-kernel

Improve support for system suspend. The register information has been
extracted from the ACPI DSDT code handling Windows Modern Standby. I
took over the weird multi-write function from the ACPI DSDT code where
it is called ECWS.

In addition to writing to the 0xE0 register, the ACPI Windows Modern
Standby code also does some changes to the thermal configuration. This
part is not implemented.

After this patch the laptop's power and LID LEDs will switch into the
typical breathing animation when the system is suspended and enabled
normally again after resuming.

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/platform/arm64/lenovo-thinkpad-t14s.c | 54 +++++++++++++++++++++++----
 1 file changed, 46 insertions(+), 8 deletions(-)

diff --git a/drivers/platform/arm64/lenovo-thinkpad-t14s.c b/drivers/platform/arm64/lenovo-thinkpad-t14s.c
index cf6a1d3b2617..cf8eff50d710 100644
--- a/drivers/platform/arm64/lenovo-thinkpad-t14s.c
+++ b/drivers/platform/arm64/lenovo-thinkpad-t14s.c
@@ -20,19 +20,23 @@
 #include <linux/module.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
+#include <linux/pm.h>
 
 #define T14S_EC_CMD_ECRD	0x02
 #define T14S_EC_CMD_ECWR	0x03
 #define T14S_EC_CMD_EVT		0xf0
 
-#define T14S_EC_REG_LED		0x0c
-#define T14S_EC_REG_KBD_BL1	0x0d
-#define T14S_EC_REG_KBD_BL2	0xe1
-#define T14S_EC_KBD_BL1_MASK	GENMASK_U8(7, 6)
-#define T14S_EC_KBD_BL2_MASK	GENMASK_U8(3, 2)
-#define T14S_EC_REG_AUD		0x30
-#define T14S_EC_MIC_MUTE_LED	BIT(5)
-#define T14S_EC_SPK_MUTE_LED	BIT(6)
+#define T14S_EC_REG_LED				0x0c
+#define T14S_EC_REG_KBD_BL1			0x0d
+#define T14S_EC_REG_MODERN_STANDBY		0xe0
+#define T14S_EC_MODERN_STANDBY_ENTRY		BIT(1)
+#define T14S_EC_MODERN_STANDBY_EXIT		BIT(0)
+#define T14S_EC_REG_KBD_BL2			0xe1
+#define T14S_EC_KBD_BL1_MASK			GENMASK_U8(7, 6)
+#define T14S_EC_KBD_BL2_MASK			GENMASK_U8(3, 2)
+#define T14S_EC_REG_AUD				0x30
+#define T14S_EC_MIC_MUTE_LED			BIT(5)
+#define T14S_EC_SPK_MUTE_LED			BIT(6)
 
 #define T14S_EC_EVT_NONE			0x00
 #define T14S_EC_EVT_KEY_FN_4			0x13
@@ -202,6 +206,14 @@ static int t14s_ec_read_evt(struct t14s_ec *ec, u8 *val)
 	return ret;
 }
 
+static void t14s_ec_write_sequence(struct t14s_ec *ec, u8 reg, u8 val, u8 cnt)
+{
+	int i;
+
+	for (i = 0; i < cnt; i++)
+		regmap_write(ec->regmap, reg, val);
+}
+
 static int t14s_led_set_status(struct t14s_ec *ec,
 			       struct t14s_ec_led_classdev *led,
 			       const enum t14s_ec_led_status_t ledstatus)
@@ -554,6 +566,7 @@ static int t14s_ec_probe(struct i2c_client *client)
 		return -ENOMEM;
 
 	ec->dev = dev;
+	i2c_set_clientdata(client, ec);
 
 	ec->regmap = devm_regmap_init(dev, &t14s_ec_regmap_bus,
 				      ec, &t14s_ec_regmap_config);
@@ -593,6 +606,26 @@ static int t14s_ec_probe(struct i2c_client *client)
 	return 0;
 }
 
+static int t14s_ec_suspend(struct device *dev)
+{
+	struct t14s_ec *ec = dev_get_drvdata(dev);
+
+	t14s_ec_write_sequence(ec, T14S_EC_REG_MODERN_STANDBY,
+			       T14S_EC_MODERN_STANDBY_ENTRY, 3);
+
+	return 0;
+}
+
+static int t14s_ec_resume(struct device *dev)
+{
+	struct t14s_ec *ec = dev_get_drvdata(dev);
+
+	t14s_ec_write_sequence(ec, T14S_EC_REG_MODERN_STANDBY,
+			       T14S_EC_MODERN_STANDBY_EXIT, 3);
+
+	return 0;
+}
+
 static const struct of_device_id t14s_ec_of_match[] = {
 	{ .compatible = "lenovo,thinkpad-t14s-ec" },
 	{}
@@ -605,10 +638,15 @@ static const struct i2c_device_id t14s_ec_i2c_id_table[] = {
 };
 MODULE_DEVICE_TABLE(i2c, t14s_ec_i2c_id_table);
 
+static const struct dev_pm_ops t14s_ec_pm_ops = {
+	SYSTEM_SLEEP_PM_OPS(t14s_ec_suspend, t14s_ec_resume)
+};
+
 static struct i2c_driver t14s_ec_i2c_driver = {
 	.driver = {
 		.name = "thinkpad-t14s-ec",
 		.of_match_table = t14s_ec_of_match,
+		.pm = &t14s_ec_pm_ops,
 	},
 	.probe = t14s_ec_probe,
 	.id_table = t14s_ec_i2c_id_table,

-- 
2.51.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2 4/4] platform: arm64: thinkpad-t14s-ec: add suspend handler for keyboard backlight
  2025-11-19  0:41 [PATCH v2 0/4] platform: arm64: thinkpad-t14s-ec: fixes and suspend support Sebastian Reichel
                   ` (2 preceding siblings ...)
  2025-11-19  0:41 ` [PATCH v2 3/4] platform: arm64: thinkpad-t14s-ec: add system PM hooks Sebastian Reichel
@ 2025-11-19  0:41 ` Sebastian Reichel
  2025-11-24 13:40 ` [PATCH v2 0/4] platform: arm64: thinkpad-t14s-ec: fixes and suspend support Ilpo Järvinen
  2025-11-24 17:06 ` Ilpo Järvinen
  5 siblings, 0 replies; 7+ messages in thread
From: Sebastian Reichel @ 2025-11-19  0:41 UTC (permalink / raw)
  To: Sebastian Reichel, Hans de Goede, Ilpo Järvinen,
	Bryan O'Donoghue
  Cc: platform-driver-x86, linux-kernel

This ensures that the keyboard backlight state is restored after
a suspend-resume cycle. Otherwise the keyboard is automatically
disabled during suspend and then stays disabled after resume.

Note, that this adopts the same behavior as the existing Thinkpad
ACPI driver.

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/platform/arm64/lenovo-thinkpad-t14s.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/platform/arm64/lenovo-thinkpad-t14s.c b/drivers/platform/arm64/lenovo-thinkpad-t14s.c
index cf8eff50d710..5590302a5694 100644
--- a/drivers/platform/arm64/lenovo-thinkpad-t14s.c
+++ b/drivers/platform/arm64/lenovo-thinkpad-t14s.c
@@ -610,6 +610,8 @@ static int t14s_ec_suspend(struct device *dev)
 {
 	struct t14s_ec *ec = dev_get_drvdata(dev);
 
+	led_classdev_suspend(&ec->kbd_backlight);
+
 	t14s_ec_write_sequence(ec, T14S_EC_REG_MODERN_STANDBY,
 			       T14S_EC_MODERN_STANDBY_ENTRY, 3);
 
@@ -623,6 +625,8 @@ static int t14s_ec_resume(struct device *dev)
 	t14s_ec_write_sequence(ec, T14S_EC_REG_MODERN_STANDBY,
 			       T14S_EC_MODERN_STANDBY_EXIT, 3);
 
+	led_classdev_resume(&ec->kbd_backlight);
+
 	return 0;
 }
 

-- 
2.51.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 0/4] platform: arm64: thinkpad-t14s-ec: fixes and suspend support
  2025-11-19  0:41 [PATCH v2 0/4] platform: arm64: thinkpad-t14s-ec: fixes and suspend support Sebastian Reichel
                   ` (3 preceding siblings ...)
  2025-11-19  0:41 ` [PATCH v2 4/4] platform: arm64: thinkpad-t14s-ec: add suspend handler for keyboard backlight Sebastian Reichel
@ 2025-11-24 13:40 ` Ilpo Järvinen
  2025-11-24 17:06 ` Ilpo Järvinen
  5 siblings, 0 replies; 7+ messages in thread
From: Ilpo Järvinen @ 2025-11-24 13:40 UTC (permalink / raw)
  To: Hans de Goede, Bryan O'Donoghue, Sebastian Reichel
  Cc: platform-driver-x86, linux-kernel

On Wed, 19 Nov 2025 01:41:39 +0100, Sebastian Reichel wrote:

> I have a two fixes for the T14s Gen6 Snapdragon EC driver as well as
> improved system suspend support. I've been using my Thinkpad with these
> patches for a few weeks as daily driver doing regular system suspend and
> things seem to run well.
> 
> If there is enough time after review the fixes are probably good
> material for the 6.18 release cycle.
> 
> [...]


Thank you for your contribution, it has been applied to my local
review-ilpo-fixes branch. Note it will show up in the public
platform-drivers-x86/review-ilpo-fixes branch only once I've pushed my
local branch there, which might take a while.

The list of commits applied:
[1/4] platform: arm64: thinkpad-t14s-ec: fix IRQ race condition
      commit: 29be646f75c4406bdd8322572a6054ebd22bf63b
[2/4] platform: arm64: thinkpad-t14s-ec: sleep after EC access
      commit: 752630cb8c62abca0407f96cc7baa2e13335cd15
[3/4] platform: arm64: thinkpad-t14s-ec: add system PM hooks
      (no commit info)
[4/4] platform: arm64: thinkpad-t14s-ec: add suspend handler for keyboard backlight
      (no commit info)

--
 i.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 0/4] platform: arm64: thinkpad-t14s-ec: fixes and suspend support
  2025-11-19  0:41 [PATCH v2 0/4] platform: arm64: thinkpad-t14s-ec: fixes and suspend support Sebastian Reichel
                   ` (4 preceding siblings ...)
  2025-11-24 13:40 ` [PATCH v2 0/4] platform: arm64: thinkpad-t14s-ec: fixes and suspend support Ilpo Järvinen
@ 2025-11-24 17:06 ` Ilpo Järvinen
  5 siblings, 0 replies; 7+ messages in thread
From: Ilpo Järvinen @ 2025-11-24 17:06 UTC (permalink / raw)
  To: Hans de Goede, Bryan O'Donoghue, Sebastian Reichel
  Cc: platform-driver-x86, linux-kernel

On Wed, 19 Nov 2025 01:41:39 +0100, Sebastian Reichel wrote:

> I have a two fixes for the T14s Gen6 Snapdragon EC driver as well as
> improved system suspend support. I've been using my Thinkpad with these
> patches for a few weeks as daily driver doing regular system suspend and
> things seem to run well.
> 
> If there is enough time after review the fixes are probably good
> material for the 6.18 release cycle.
> 
> [...]


Thank you for your contribution, it has been applied to my local
review-ilpo-next branch. Note it will show up in the public
platform-drivers-x86/review-ilpo-next branch only once I've pushed my
local branch there, which might take a while.

The list of commits applied:
[1/4] platform: arm64: thinkpad-t14s-ec: fix IRQ race condition
      (no commit info)
[2/4] platform: arm64: thinkpad-t14s-ec: sleep after EC access
      (no commit info)
[3/4] platform: arm64: thinkpad-t14s-ec: add system PM hooks
      commit: fc62cb6a3e118a7633ca2217203b6293bb742a23
[4/4] platform: arm64: thinkpad-t14s-ec: add suspend handler for keyboard backlight
      commit: 3c79c2e28a24a86199fa83bb80dc1a1c33f996c5

--
 i.


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-11-24 17:07 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-19  0:41 [PATCH v2 0/4] platform: arm64: thinkpad-t14s-ec: fixes and suspend support Sebastian Reichel
2025-11-19  0:41 ` [PATCH v2 1/4] platform: arm64: thinkpad-t14s-ec: fix IRQ race condition Sebastian Reichel
2025-11-19  0:41 ` [PATCH v2 2/4] platform: arm64: thinkpad-t14s-ec: sleep after EC access Sebastian Reichel
2025-11-19  0:41 ` [PATCH v2 3/4] platform: arm64: thinkpad-t14s-ec: add system PM hooks Sebastian Reichel
2025-11-19  0:41 ` [PATCH v2 4/4] platform: arm64: thinkpad-t14s-ec: add suspend handler for keyboard backlight Sebastian Reichel
2025-11-24 13:40 ` [PATCH v2 0/4] platform: arm64: thinkpad-t14s-ec: fixes and suspend support Ilpo Järvinen
2025-11-24 17:06 ` Ilpo Järvinen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®