[PATCH v3 0/3] Lenovo ThinkPad X13s embedded controller support

[PATCH v3 0/3] Lenovo ThinkPad X13s embedded controller support

[Alex Robinson]

Add support for the embedded controller found in the Lenovo ThinkPad X13s.

The X13s EC uses an I2C command transport similar to the ThinkPad T14s EC,
but its event, keyboard backlight, and power-management interfaces differ
enough to warrant a separate driver.

The EC event interrupt is provided by GPIO103. GPIO176 is a separate
host-controlled power-state signal used during low-power transitions. The
driver follows the EC command and GPIO sequencing described by the X13s
ACPI DSDT and defers EC event queries during suspend until the I2C
controller is available again on resume.

This initial series provides EC transport, power-management support, and
keyboard-backlight control. Other EC events have been left for follow-up
work rather than exposing unverified input mappings.

Disable EC wakeup by default, following the existing upstream ThinkPad
T14s EC driver policy. The EC interrupt multiplexes multiple events and
no selective X13s EC wake-event mask is currently known. In particular,
lid-close event 0x53 can wake the system, but the event type cannot be
determined until the system has resumed far enough to query the EC over
QUP8/I2C. Rather than attempting to filter an already-triggered wakeup,
disable EC wake broadly. Other EC events therefore cannot wake the system
by default either; another enabled wake source is needed. Feedback is
welcome if a selective X13s EC event-mask mechanism is known.

Normal runtime EC interrupts remain enabled. The low-power entry sequence
(0x80 <- 0x55, GPIO176 low), exit sequence (GPIO176 high, 10 ms delay,
0x80 <- 0xaa), deferred-event handling, and backlight behavior are unchanged
from v2. No speculative event filtering or EC register writes are added.

Retain wakeup-source in the DT node, binding, and example: it describes
hardware capability, not the driver's default wake policy. The I2C core
initializes wake capability and associates the IRQ before driver probe;
device_wakeup_disable() in probe disables policy without disabling normal
IRQ handling. Userspace can explicitly re-enable power/wakeup, with the
same risk of unwanted wakeups. This is a default-off policy, not a removal
of hardware wake capability.

This follows earlier X13s EC work by Konrad Dybcio and Steev Klimaszewski.

Development was assisted by an LLM, including analysis of the decompiled
X13s ACPI DSDT and review of the resulting implementation.

This series is based on Linux v7.3-rc4.

The v3 series was tested on a Lenovo ThinkPad X13s. With EC wake
disabled, runtime EC events continue to work and keyboard-backlight
state remains preserved across both manual s2idle and lid-triggered
suspend/resume. Closing the lid while the system is already suspended
does not wake it, while opening the lid still wakes the system through
the separate lid wake path. The QUP8 pin configuration was also
previously verified on hardware.

For v3, the package patches pass an application check and the driver
builds as an ARM64 module with W=1. Checkpatch reports no errors (only
the generic MAINTAINERS reminders; the series includes an entry).
A source comparison confirms that the only C changes from v2 are the
wakeup API include and the probe-time wake policy with its explanation.

Changes in v3:
- Disable EC system wakeup by default using device_wakeup_disable(), as
  in the upstream T14s EC driver, without disabling runtime IRQ handling.
- Explain why unknown selective event masking requires disabling EC wake
  broadly, and invite information about a selective X13s mechanism.
- Retain the DT wakeup-source capability description and all v2 low-power,
  deferred-event, and backlight behavior.
- Remove the Kconfig help text's claim of wake support.
- Verify on hardware that disabling EC wake prevents lid-close wake
  without breaking runtime EC events, lid-open wake, or backlight state
  restoration across suspend/resume.

Changes in v2:
- Make keyboard-backlight snapshot and EC power-management operations
  best-effort so EC failures do not prevent system suspend or resume.
- Save firmware-selected keyboard-backlight brightness in the EC on
  Fn+Space, following the DSDT's SCMS(0x20) operation.
- Save software-selected keyboard-backlight brightness in the EC so lid
  opening does not restore a stale setting.
- Restore the EC-saved keyboard-backlight brightness on lid open,
  following the DSDT's SCMS(0x21) operation.
- Process deferred EC events after the normal resume brightness restore
  so a lid-open restore takes precedence.
- Add the QUP8 pinctrl state for GPIO43 and GPIO44 using the configuration
  observed on the running hardware.
- Fix the placement and ordering of the i2c8 and EC pinctrl nodes.

Link: https://lore.kernel.org/all/20260925215358.33417-1-alex@ironrobin.net/

Alex Robinson (3):
  dt-bindings: embedded-controller: Add Lenovo ThinkPad X13s EC
  platform: arm64: Add Lenovo ThinkPad X13s EC driver
  arm64: dts: qcom: sc8280xp-x13s: Add embedded controller

[PATCH v3 1/3] dt-bindings: embedded-controller: Add Lenovo ThinkPad X13s EC

[Alex Robinson]

Add a standalone binding for the embedded controller in the Lenovo
ThinkPad X13s. Although its I2C command transport resembles that of the
T14s EC, its keyboard-backlight, event and power-management interfaces
differ.

Describe the event interrupt and the separate host-controlled power-state
signal, which is high during normal operation and low in the low-power
state. Require this GPIO and allow wakeup-source to describe wake
capability.

Assisted-by: LLM
Signed-off-by: Alex Robinson <alex@ironrobin.net>

---
diff --git a/Documentation/devicetree/bindings/embedded-controller/lenovo,thinkpad-x13s-ec.yaml b/Documentation/devicetree/bindings/embedded-controller/lenovo,thinkpad-x13s-ec.yaml
new file mode 100644
--- /dev/null
+++ b/Documentation/devicetree/bindings/embedded-controller/lenovo,thinkpad-x13s-ec.yaml
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/embedded-controller/lenovo,thinkpad-x13s-ec.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Lenovo ThinkPad X13s Embedded Controller
+
+maintainers:
+  - Alex Robinson <alex@ironrobin.net>
+
+description:
+  The Lenovo ThinkPad X13s has an I2C-connected embedded controller (EC)
+  providing keyboard backlight control and event notifications. In addition
+  to its event interrupt, the EC has a separate host-controlled power-state
+  input used during transitions between normal operation and low power.
+
+properties:
+  compatible:
+    const: lenovo,thinkpad-x13s-ec
+
+  reg:
+    const: 0x28
+
+  interrupts:
+    maxItems: 1
+
+  power-state-gpios:
+    maxItems: 1
+    description:
+      GPIO connected to the EC power-state input, driven by the host.
+      The signal is high during normal operation and low in the low-power
+      state. It is used together with I2C power-state commands, rather than
+      as a power-supply enable or reset signal. The GPIO is active high.
+
+  wakeup-source: true
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - power-state-gpios
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        embedded-controller@28 {
+            compatible = "lenovo,thinkpad-x13s-ec";
+            reg = <0x28>;
+            interrupts-extended = <&gpio 1 IRQ_TYPE_EDGE_FALLING>;
+            power-state-gpios = <&gpio 2 GPIO_ACTIVE_HIGH>;
+            pinctrl-0 = <&ec_int_n>, <&ec_power_state>;
+            pinctrl-names = "default";
+            wakeup-source;
+        };
+    };
+...

[PATCH v3 2/3] platform: arm64: Add Lenovo ThinkPad X13s EC driver

[Alex Robinson]

Add a standalone driver for the Lenovo ThinkPad X13s embedded controller,
providing keyboard-backlight control and notifications of firmware-driven
brightness changes. Keep the X13s-specific backlight, event and
power-management handling separate from the T14s driver.

Use STOP-separated command and response transfers and serialize EC
access, including keyboard-backlight read-modify-write operations.
Follow the EC command and power-state GPIO sequence described by the
X13s ACPI DSDT when entering and leaving low power.

Save the firmware-updated brightness in the EC on Fn+Space and restore
it on lid open, following the DSDT's SCMS(0x20) and SCMS(0x21) operations.
Also save software-selected brightness so lid opening does not reinstate
an older setting; this extends the DSDT's MLCS path, which does not
explicitly save it.

Defer event queries while suspended until normal resume, when the I2C
controller is usable again. Save the hardware backlight brightness before
entering low power and attempt to restore it on resume because the
transition can reset it. Process deferred events afterwards so a lid-open
restore takes precedence over a snapshot taken after lid closure blanked
the light.

Disable EC wakeup by default, following the upstream ThinkPad T14s EC
driver's policy. The interrupt multiplexes events and no selective X13s
EC wake-event mask is currently known. In particular, lid-close event
0x53 can wake the system, but identifying the event requires querying the
EC over QUP8/I2C after the system has resumed far enough to use the bus.
Disable wake broadly rather than trying to filter an already-triggered
wakeup. This also prevents other EC events from waking the system by
default; normal runtime interrupt handling remains enabled.

Keep wakeup-source in DT to describe hardware capability. The I2C core
sets up the wake IRQ before probe, and the driver disables wakeup policy
without removing that capability. Userspace can explicitly re-enable it.
Feedback on a selective X13s EC event-mask mechanism would be welcome.

This follows earlier X13s EC work by Konrad Dybcio and Steev Klimaszewski.

Assisted-by: LLM
Signed-off-by: Alex Robinson <alex@ironrobin.net>

---
diff --git a/MAINTAINERS b/MAINTAINERS
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -27177,6 +27177,12 @@ THINKPAD T14S EMBEDDED CONTROLLER DRIVER
 F:	Documentation/devicetree/bindings/embedded-controller/lenovo,thinkpad-t14s-ec.yaml
 F:	drivers/platform/arm64/lenovo-thinkpad-t14s.c
 
+THINKPAD X13S EMBEDDED CONTROLLER DRIVER
+M:	Alex Robinson <alex@ironrobin.net>
+S:	Maintained
+F:	Documentation/devicetree/bindings/embedded-controller/lenovo,thinkpad-x13s-ec.yaml
+F:	drivers/platform/arm64/lenovo-thinkpad-x13s.c
+
 THINKPAD LMI DRIVER
 M:	Mark Pearson <mpearson-lenovo@squebb.ca>
 L:	platform-driver-x86@vger.kernel.org
diff --git a/drivers/platform/arm64/Kconfig b/drivers/platform/arm64/Kconfig
index e32e01b2a9bdd970d6f2b2bf6b051472c9a16103..c20b2fb40962f1d46e896ea7f7d0b16028f92f96 100644
--- a/drivers/platform/arm64/Kconfig
+++ b/drivers/platform/arm64/Kconfig
@@ -90,6 +90,22 @@ config EC_LENOVO_THINKPAD_T14S
 
 	  Say M or Y here to include this support.
 
+config EC_LENOVO_THINKPAD_X13S
+	tristate "Lenovo ThinkPad X13s Embedded Controller driver"
+	depends on ARCH_QCOM || COMPILE_TEST
+	depends on I2C
+	depends on GPIOLIB
+	select NEW_LEDS
+	select LEDS_CLASS
+	select LEDS_BRIGHTNESS_HW_CHANGED
+	help
+	  Driver for the embedded controller in the Lenovo ThinkPad X13s.
+	  Provides keyboard backlight control, hardware brightness change
+	  notifications and system sleep power sequencing.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called lenovo-thinkpad-x13s.
+
 config EC_QCOM_HAMOA
 	tristate "Embedded Controller driver for Qualcomm Hamoa/Glymur reference devices"
 	depends on ARCH_QCOM || COMPILE_TEST
diff --git a/drivers/platform/arm64/Makefile b/drivers/platform/arm64/Makefile
index 7681be4a46e94e7cb80e50bcc5b724cb0961357b..23ed6eef7ce41a5f45d48f5f83b281ebbca94cbb 100644
--- a/drivers/platform/arm64/Makefile
+++ b/drivers/platform/arm64/Makefile
@@ -9,4 +9,5 @@ obj-$(CONFIG_EC_ACER_ASPIRE1)	+= acer-aspire1-ec.o
 obj-$(CONFIG_EC_HUAWEI_GAOKUN)	+= huawei-gaokun-ec.o
 obj-$(CONFIG_EC_LENOVO_YOGA_C630) += lenovo-yoga-c630.o
 obj-$(CONFIG_EC_LENOVO_THINKPAD_T14S) += lenovo-thinkpad-t14s.o
+obj-$(CONFIG_EC_LENOVO_THINKPAD_X13S) += lenovo-thinkpad-x13s.o
 obj-$(CONFIG_EC_QCOM_HAMOA) += qcom-hamoa-ec.o
diff --git a/drivers/platform/arm64/lenovo-thinkpad-x13s.c b/drivers/platform/arm64/lenovo-thinkpad-x13s.c
new file mode 100644
index 0000000000000000000000000000000000000000..c600a616c9b6e8f4259caf61fd5c16d129547c90
--- /dev/null
+++ b/drivers/platform/arm64/lenovo-thinkpad-x13s.c
@@ -0,0 +1,416 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Lenovo ThinkPad X13s embedded controller */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/container_of.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/leds.h>
+#include <linux/lockdep.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/pm.h>
+#include <linux/pm_wakeup.h>
+#include <linux/slab.h>
+
+#define X13S_EC_CMD_READ		0x02
+#define X13S_EC_CMD_WRITE		0x03
+#define X13S_EC_CMD_EVENT		0xf0
+#define X13S_EC_REG_POWER_STATE	0x80
+#define X13S_EC_POWER_STATE_ENTER	0x55
+#define X13S_EC_POWER_STATE_EXIT	0xaa
+#define X13S_EC_REG_KBD_BACKLIGHT	0xc0
+#define X13S_EC_BACKLIGHT_MASK	GENMASK(5, 4)
+#define X13S_EC_REG_KBD_BACKLIGHT_SAVED	0x15
+#define X13S_EC_BACKLIGHT_SAVED_MASK	GENMASK(2, 1)
+#define X13S_EC_EVENT_FN_SPACE	0x1f
+#define X13S_EC_EVENT_LID_OPEN	0x52
+
+struct x13s_ec {
+	struct i2c_client *client;
+	struct gpio_desc *power_state;
+	/* Serializes EC transactions, backlight RMW and suspend/event state. */
+	struct mutex lock;
+	struct led_classdev led;
+	enum led_brightness saved_brightness;
+	bool suspended;
+	bool event_pending;
+};
+
+/*
+ * Each single-message transfer ends in STOP. Hold the bus segment across
+ * both halves so another client cannot interleave them. Keep the 10 ms
+ * post-command settling delay, including on failure.
+ */
+static int x13s_read(struct x13s_ec *ec, u8 cmd, u8 reg, u8 *value)
+{
+	struct i2c_client *client = ec->client;
+	u8 buf[] = { cmd, reg, 0, 1 };
+	struct i2c_msg request = {
+		.addr = client->addr,
+		.flags = I2C_M_STOP,
+		.len = sizeof(buf),
+		.buf = buf,
+	};
+	struct i2c_msg response = {
+		.addr = client->addr,
+		.flags = I2C_M_RD,
+		.len = 1,
+		.buf = value,
+	};
+	int ret;
+
+	lockdep_assert_held(&ec->lock);
+	if (ec->suspended)
+		return -EBUSY;
+
+	i2c_lock_bus(client->adapter, I2C_LOCK_SEGMENT);
+	ret = __i2c_transfer(client->adapter, &request, 1);
+	if (ret == 1)
+		ret = __i2c_transfer(client->adapter, &response, 1);
+	ret = ret == 1 ? 0 : (ret < 0 ? ret : -EIO);
+	i2c_unlock_bus(client->adapter, I2C_LOCK_SEGMENT);
+	fsleep(10000);
+	return ret;
+}
+
+/* Also used by normal PM callbacks while ordinary EC access is blocked. */
+static int x13s_write(struct x13s_ec *ec, u8 reg, u8 value)
+{
+	u8 buf[] = { X13S_EC_CMD_WRITE, reg, 0, 1, value };
+	int ret;
+
+	lockdep_assert_held(&ec->lock);
+
+	ret = i2c_master_send(ec->client, buf, sizeof(buf));
+	ret = ret == sizeof(buf) ? 0 : (ret < 0 ? ret : -EIO);
+	fsleep(10000);
+	return ret;
+}
+
+static int x13s_brightness(struct x13s_ec *ec)
+{
+	u8 value;
+	int ret;
+
+	ret = x13s_read(ec, X13S_EC_CMD_READ, X13S_EC_REG_KBD_BACKLIGHT, &value);
+	if (ret)
+		return ret;
+
+	ret = FIELD_GET(X13S_EC_BACKLIGHT_MASK, value);
+	return ret == 3 ? -EINVAL : ret;
+}
+
+static enum led_brightness x13s_brightness_get(struct led_classdev *led)
+{
+	struct x13s_ec *ec = container_of(led, struct x13s_ec, led);
+	int ret;
+
+	mutex_lock(&ec->lock);
+	ret = x13s_brightness(ec);
+	mutex_unlock(&ec->lock);
+	return ret;
+}
+
+static int x13s_set_brightness_locked(struct x13s_ec *ec,
+				      enum led_brightness brightness)
+{
+	u8 old, new;
+	int ret;
+
+	lockdep_assert_held(&ec->lock);
+	if (brightness > 2)
+		return -EINVAL;
+
+	ret = x13s_read(ec, X13S_EC_CMD_READ, X13S_EC_REG_KBD_BACKLIGHT, &old);
+	if (ret)
+		return ret;
+	/* Never interpret or overwrite a reserved current brightness. */
+	if (FIELD_GET(X13S_EC_BACKLIGHT_MASK, old) == 3)
+		return -EINVAL;
+	new = (old & ~X13S_EC_BACKLIGHT_MASK) |
+		FIELD_PREP(X13S_EC_BACKLIGHT_MASK, brightness);
+	return x13s_write(ec, X13S_EC_REG_KBD_BACKLIGHT, new);
+}
+
+/* SCMS(0x20): save the brightness obtained from C0 in the EC. */
+static int x13s_save_brightness_locked(struct x13s_ec *ec,
+				       enum led_brightness brightness)
+{
+	u8 value;
+	int ret;
+
+	lockdep_assert_held(&ec->lock);
+	ret = x13s_read(ec, X13S_EC_CMD_READ, X13S_EC_REG_KBD_BACKLIGHT_SAVED,
+			&value);
+	if (ret)
+		return ret;
+
+	value &= ~X13S_EC_BACKLIGHT_SAVED_MASK;
+	value |= FIELD_PREP(X13S_EC_BACKLIGHT_SAVED_MASK, brightness);
+	return x13s_write(ec, X13S_EC_REG_KBD_BACKLIGHT_SAVED, value);
+}
+
+/* SCMS(0x21): restore C0 from the EC's saved brightness. */
+static int x13s_restore_brightness_locked(struct x13s_ec *ec)
+{
+	u8 value;
+	int brightness, ret;
+
+	lockdep_assert_held(&ec->lock);
+	ret = x13s_read(ec, X13S_EC_CMD_READ, X13S_EC_REG_KBD_BACKLIGHT_SAVED,
+			&value);
+	if (ret)
+		return ret;
+
+	brightness = FIELD_GET(X13S_EC_BACKLIGHT_SAVED_MASK, value);
+	ret = x13s_set_brightness_locked(ec, brightness);
+	return ret ? ret : brightness;
+}
+
+static int x13s_brightness_set(struct led_classdev *led,
+			       enum led_brightness brightness)
+{
+	struct x13s_ec *ec = container_of(led, struct x13s_ec, led);
+	int ret;
+
+	mutex_lock(&ec->lock);
+	ret = x13s_set_brightness_locked(ec, brightness);
+	/* Keep software selections across the EC's lid blank/restore cycle. */
+	if (!ret)
+		ret = x13s_save_brightness_locked(ec, brightness);
+	mutex_unlock(&ec->lock);
+	return ret;
+}
+
+static void x13s_refresh_brightness(struct x13s_ec *ec)
+{
+	int brightness, ret;
+
+	lockdep_assert_held(&ec->lock);
+	/* Firmware owns the transition; save its result as _Q1F does. */
+	brightness = x13s_brightness(ec);
+	if (brightness < 0) {
+		dev_err_ratelimited(&ec->client->dev,
+				    "Backlight refresh failed: %d\n", brightness);
+		return;
+	}
+	ret = x13s_save_brightness_locked(ec, brightness);
+	if (ret)
+		dev_err_ratelimited(&ec->client->dev,
+				    "Backlight save failed: %d\n", ret);
+	led_classdev_notify_brightness_hw_changed(&ec->led, brightness);
+}
+
+static void x13s_query_event(struct x13s_ec *ec)
+{
+	u8 event;
+	int ret;
+
+	lockdep_assert_held(&ec->lock);
+	/* One query per interrupt, with no event-draining loop or retry. */
+	ret = x13s_read(ec, X13S_EC_CMD_EVENT, 0, &event);
+	if (ret) {
+		dev_err_ratelimited(&ec->client->dev, "Event query failed: %d\n", ret);
+		return;
+	}
+	if (event == X13S_EC_EVENT_FN_SPACE) {
+		x13s_refresh_brightness(ec);
+	} else if (event == X13S_EC_EVENT_LID_OPEN) {
+		ret = x13s_restore_brightness_locked(ec);
+		if (ret < 0) {
+			dev_err_ratelimited(&ec->client->dev,
+					    "Backlight lid restore failed: %d\n", ret);
+			return;
+		}
+		ec->led.brightness = ret;
+		led_classdev_notify_brightness_hw_changed(&ec->led, ret);
+	}
+}
+
+static void x13s_process_pending_event(struct x13s_ec *ec)
+{
+	lockdep_assert_held(&ec->lock);
+	if (ec->event_pending) {
+		ec->event_pending = false;
+		x13s_query_event(ec);
+	}
+}
+
+static irqreturn_t x13s_irq(int irq, void *data)
+{
+	struct x13s_ec *ec = data;
+
+	mutex_lock(&ec->lock);
+	/* Coalesce suspended IRQs into one query; never access I2C here. */
+	if (ec->suspended)
+		ec->event_pending = true;
+	else
+		x13s_query_event(ec);
+	mutex_unlock(&ec->lock);
+	return IRQ_HANDLED;
+}
+
+static int x13s_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct x13s_ec *ec;
+	int ret;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
+		return -EOPNOTSUPP;
+	if (client->irq <= 0)
+		return dev_err_probe(dev, -EINVAL, "Missing event IRQ\n");
+	if (irq_get_trigger_type(client->irq) != IRQ_TYPE_EDGE_FALLING)
+		return dev_err_probe(dev, -EINVAL, "Expected falling-edge IRQ\n");
+
+	ec = devm_kzalloc(dev, sizeof(*ec), GFP_KERNEL);
+	if (!ec)
+		return -ENOMEM;
+	ec->client = client;
+	ret = devm_mutex_init(dev, &ec->lock);
+	if (ret)
+		return ret;
+	i2c_set_clientdata(client, ec);
+
+	ec->power_state = devm_gpiod_get(dev, "power-state", GPIOD_OUT_HIGH);
+	if (IS_ERR(ec->power_state))
+		return dev_err_probe(dev, PTR_ERR(ec->power_state),
+				     "Failed to acquire power-state GPIO high\n");
+
+	ec->led.name = "platform::kbd_backlight";
+	ec->led.max_brightness = 2;
+	ec->led.flags = LED_BRIGHT_HW_CHANGED | LED_RETAIN_AT_SHUTDOWN;
+	ec->led.brightness_get = x13s_brightness_get;
+	ec->led.brightness_set_blocking = x13s_brightness_set;
+	ret = devm_led_classdev_register(dev, &ec->led);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to register keyboard backlight\n");
+
+	/* Devres synchronizes the IRQ before unregistering the LED. */
+	ret = devm_request_threaded_irq(dev, client->irq, NULL, x13s_irq,
+					IRQF_ONESHOT, dev_name(dev), ec);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to request event IRQ\n");
+
+	/*
+	 * As on the T14s, disable wakeup by default since selective EC event
+	 * masking is not known. Keep runtime IRQ handling enabled.
+	 */
+	device_wakeup_disable(dev);
+
+	return 0;
+}
+
+static int x13s_power_gpio(struct x13s_ec *ec, int value)
+{
+	int ret;
+
+	lockdep_assert_held(&ec->lock);
+	ret = gpiod_set_value_cansleep(ec->power_state, value);
+	if (ret)
+		dev_err(&ec->client->dev, "Failed to set power-state GPIO to %d: %d\n",
+			value, ret);
+	return ret;
+}
+
+static void x13s_exit_low_power(struct x13s_ec *ec)
+{
+	int ret;
+
+	lockdep_assert_held(&ec->lock);
+	x13s_power_gpio(ec, 1);
+	/* DSDT: GPIO176 high, Sleep(10), then register 0x80 <- 0xaa. */
+	fsleep(10000);
+	ret = x13s_write(ec, X13S_EC_REG_POWER_STATE, X13S_EC_POWER_STATE_EXIT);
+	if (ret)
+		dev_err(&ec->client->dev, "Failed to exit low power: %d\n", ret);
+
+	/* Resume ordinary access even if the EC exit attempt failed. */
+	ec->suspended = false;
+}
+
+static int x13s_suspend(struct device *dev)
+{
+	struct x13s_ec *ec = dev_get_drvdata(dev);
+	int ret;
+
+	/* Finish in-flight access while the GENI parent is still usable. */
+	mutex_lock(&ec->lock);
+	if (ec->suspended)
+		goto out;
+	/* Keep the previous snapshot if reading the hardware fails. */
+	ret = x13s_brightness(ec);
+	if (ret < 0)
+		dev_err(dev, "Failed to save backlight brightness: %d\n", ret);
+	else
+		ec->saved_brightness = ret;
+
+	/* DSDT: register 0x80 <- 0x55, then GPIO176 low. */
+	ret = x13s_write(ec, X13S_EC_REG_POWER_STATE, X13S_EC_POWER_STATE_ENTER);
+	if (ret)
+		dev_err(dev, "Failed to enter low power: %d\n", ret);
+	else
+		x13s_power_gpio(ec, 0);
+	/* Defer event queries throughout host suspend, even on EC failure. */
+	ec->suspended = true;
+out:
+	mutex_unlock(&ec->lock);
+	return 0;
+}
+
+static int x13s_resume(struct device *dev)
+{
+	struct x13s_ec *ec = dev_get_drvdata(dev);
+	int ret;
+
+	/*
+	 * Normal resume runs after the adapter and its GENI parent. In
+	 * particular, device_resume_early() has re-enabled runtime PM:
+	 * GENI's resume_noirq() alone is not sufficient for I2C transfers.
+	 * Consume the pending query under the IRQ thread's mutex, without
+	 * a worker that could race a new IRQ or the next suspend.
+	 */
+	mutex_lock(&ec->lock);
+	x13s_exit_low_power(ec);
+	ret = x13s_set_brightness_locked(ec, ec->saved_brightness);
+	if (ret)
+		dev_err(dev, "Failed to restore backlight brightness: %d\n", ret);
+	else
+		/* A software restore is not a hardware brightness change. */
+		ec->led.brightness = ec->saved_brightness;
+
+	/* A deferred lid-open restore must supersede the PM snapshot. */
+	x13s_process_pending_event(ec);
+	mutex_unlock(&ec->lock);
+	return 0;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(x13s_pm_ops, x13s_suspend, x13s_resume);
+
+static const struct of_device_id x13s_of_match[] = {
+	{ .compatible = "lenovo,thinkpad-x13s-ec" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, x13s_of_match);
+
+static struct i2c_driver x13s_driver = {
+	.probe = x13s_probe,
+	.driver = {
+		.name = "thinkpad-x13s-ec",
+		.of_match_table = x13s_of_match,
+		.pm = pm_sleep_ptr(&x13s_pm_ops),
+	},
+};
+module_i2c_driver(x13s_driver);
+
+MODULE_DESCRIPTION("Lenovo ThinkPad X13s Embedded Controller");
+MODULE_LICENSE("GPL");

[PATCH v3 3/3] arm64: dts: qcom: sc8280xp-x13s: Add embedded controller

[Alex Robinson]

Enable the embedded controller at address 0x28 on i2c8, with GPIO103 as
its falling-edge, wake-capable event interrupt.

Configure the QUP8 bus on GPIO43 and GPIO44 with 2 mA drive strength and
pull-ups, matching the working firmware-established pin configuration.

Configure GPIO103 as an input without overriding its existing pull
configuration. Describe GPIO176 as the separate active-high power-state
output and configure it initially high with bias disabled.

Assisted-by: LLM
Signed-off-by: Alex Robinson <alex@ironrobin.net>

---
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
index 218573a97785b04528854b03a99c837558a33c1e..1b875809773dac3bb0d1dafb1c9bb41a88f5027f 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
@@ -806,6 +806,25 @@ &i2c4 {
 	};
 };
 
+&i2c8 {
+	clock-frequency = <400000>;
+
+	pinctrl-0 = <&i2c8_default>;
+	pinctrl-names = "default";
+
+	status = "okay";
+
+	embedded-controller@28 {
+		compatible = "lenovo,thinkpad-x13s-ec";
+		reg = <0x28>;
+		interrupts-extended = <&tlmm 103 IRQ_TYPE_EDGE_FALLING>;
+		power-state-gpios = <&tlmm 176 GPIO_ACTIVE_HIGH>;
+		pinctrl-0 = <&ec_int_n>, <&ec_power_state>;
+		pinctrl-names = "default";
+		wakeup-source;
+	};
+};
+
 &i2c11 {
 	clock-frequency = <400000>;
 
@@ -1571,6 +1590,21 @@ cam_rgb_default: cam-rgb-default-state {
 		};
 	};
 
+	ec_int_n: ec-int-n-state {
+		pins = "gpio103";
+		function = "gpio";
+		input-enable;
+		/* Preserve firmware pull configuration; no bias override. */
+	};
+
+	ec_power_state: ec-power-state {
+		pins = "gpio176";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+		output-high;
+	};
+
 	edp_reg_en: edp-reg-en-state {
 		pins = "gpio25";
 		function = "gpio";
@@ -1591,6 +1625,13 @@ i2c4_default: i2c4-default-state {
 		bias-disable;
 	};
 
+	i2c8_default: i2c8-default-state {
+		pins = "gpio43", "gpio44";
+		function = "qup8";
+		drive-strength = <2>;
+		bias-pull-up;
+	};
+
 	i2c11_default: i2c11-default-state {
 		pins = "gpio18", "gpio19";
 		function = "qup11";

Re: [PATCH v3 2/3] platform: arm64: Add Lenovo ThinkPad X13s EC driver

[Ilpo Järvinen]

On Mon, 28 Sep 2026, Alex Robinson wrote:

> Add a standalone driver for the Lenovo ThinkPad X13s embedded controller,
> providing keyboard-backlight control and notifications of firmware-driven
> brightness changes. Keep the X13s-specific backlight, event and
> power-management handling separate from the T14s driver.
> 
> Use STOP-separated command and response transfers and serialize EC
> access, including keyboard-backlight read-modify-write operations.
> Follow the EC command and power-state GPIO sequence described by the
> X13s ACPI DSDT when entering and leaving low power.
> 
> Save the firmware-updated brightness in the EC on Fn+Space and restore
> it on lid open, following the DSDT's SCMS(0x20) and SCMS(0x21) operations.
> Also save software-selected brightness so lid opening does not reinstate
> an older setting; this extends the DSDT's MLCS path, which does not
> explicitly save it.
> 
> Defer event queries while suspended until normal resume, when the I2C
> controller is usable again. Save the hardware backlight brightness before
> entering low power and attempt to restore it on resume because the
> transition can reset it. Process deferred events afterwards so a lid-open
> restore takes precedence over a snapshot taken after lid closure blanked
> the light.
> 
> Disable EC wakeup by default, following the upstream ThinkPad T14s EC
> driver's policy. The interrupt multiplexes events and no selective X13s
> EC wake-event mask is currently known. In particular, lid-close event
> 0x53 can wake the system, but identifying the event requires querying the
> EC over QUP8/I2C after the system has resumed far enough to use the bus.
> Disable wake broadly rather than trying to filter an already-triggered
> wakeup. This also prevents other EC events from waking the system by
> default; normal runtime interrupt handling remains enabled.
> 
> Keep wakeup-source in DT to describe hardware capability. The I2C core
> sets up the wake IRQ before probe, and the driver disables wakeup policy
> without removing that capability. Userspace can explicitly re-enable it.
> Feedback on a selective X13s EC event-mask mechanism would be welcome.
> 
> This follows earlier X13s EC work by Konrad Dybcio and Steev Klimaszewski.
> 
> Assisted-by: LLM
> Signed-off-by: Alex Robinson <alex@ironrobin.net>
> 
> ---
> diff --git a/MAINTAINERS b/MAINTAINERS
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -27177,6 +27177,12 @@ THINKPAD T14S EMBEDDED CONTROLLER DRIVER
>  F:	Documentation/devicetree/bindings/embedded-controller/lenovo,thinkpad-t14s-ec.yaml
>  F:	drivers/platform/arm64/lenovo-thinkpad-t14s.c
>  
> +THINKPAD X13S EMBEDDED CONTROLLER DRIVER
> +M:	Alex Robinson <alex@ironrobin.net>
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/embedded-controller/lenovo,thinkpad-x13s-ec.yaml
> +F:	drivers/platform/arm64/lenovo-thinkpad-x13s.c
> +
>  THINKPAD LMI DRIVER
>  M:	Mark Pearson <mpearson-lenovo@squebb.ca>
>  L:	platform-driver-x86@vger.kernel.org
> diff --git a/drivers/platform/arm64/Kconfig b/drivers/platform/arm64/Kconfig
> index e32e01b2a9bdd970d6f2b2bf6b051472c9a16103..c20b2fb40962f1d46e896ea7f7d0b16028f92f96 100644
> --- a/drivers/platform/arm64/Kconfig
> +++ b/drivers/platform/arm64/Kconfig
> @@ -90,6 +90,22 @@ config EC_LENOVO_THINKPAD_T14S
>  
>  	  Say M or Y here to include this support.
>  
> +config EC_LENOVO_THINKPAD_X13S
> +	tristate "Lenovo ThinkPad X13s Embedded Controller driver"
> +	depends on ARCH_QCOM || COMPILE_TEST
> +	depends on I2C
> +	depends on GPIOLIB
> +	select NEW_LEDS
> +	select LEDS_CLASS
> +	select LEDS_BRIGHTNESS_HW_CHANGED
> +	help
> +	  Driver for the embedded controller in the Lenovo ThinkPad X13s.
> +	  Provides keyboard backlight control, hardware brightness change
> +	  notifications and system sleep power sequencing.
> +
> +	  To compile this driver as a module, choose M here: the module will
> +	  be called lenovo-thinkpad-x13s.
> +
>  config EC_QCOM_HAMOA
>  	tristate "Embedded Controller driver for Qualcomm Hamoa/Glymur reference devices"
>  	depends on ARCH_QCOM || COMPILE_TEST
> diff --git a/drivers/platform/arm64/Makefile b/drivers/platform/arm64/Makefile
> index 7681be4a46e94e7cb80e50bcc5b724cb0961357b..23ed6eef7ce41a5f45d48f5f83b281ebbca94cbb 100644
> --- a/drivers/platform/arm64/Makefile
> +++ b/drivers/platform/arm64/Makefile
> @@ -9,4 +9,5 @@ obj-$(CONFIG_EC_ACER_ASPIRE1)	+= acer-aspire1-ec.o
>  obj-$(CONFIG_EC_HUAWEI_GAOKUN)	+= huawei-gaokun-ec.o
>  obj-$(CONFIG_EC_LENOVO_YOGA_C630) += lenovo-yoga-c630.o
>  obj-$(CONFIG_EC_LENOVO_THINKPAD_T14S) += lenovo-thinkpad-t14s.o
> +obj-$(CONFIG_EC_LENOVO_THINKPAD_X13S) += lenovo-thinkpad-x13s.o
>  obj-$(CONFIG_EC_QCOM_HAMOA) += qcom-hamoa-ec.o
> diff --git a/drivers/platform/arm64/lenovo-thinkpad-x13s.c b/drivers/platform/arm64/lenovo-thinkpad-x13s.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..c600a616c9b6e8f4259caf61fd5c16d129547c90
> --- /dev/null
> +++ b/drivers/platform/arm64/lenovo-thinkpad-x13s.c
> @@ -0,0 +1,416 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Lenovo ThinkPad X13s embedded controller */
> +
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/container_of.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/leds.h>
> +#include <linux/lockdep.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/pm.h>
> +#include <linux/pm_wakeup.h>
> +#include <linux/slab.h>
> +
> +#define X13S_EC_CMD_READ		0x02
> +#define X13S_EC_CMD_WRITE		0x03
> +#define X13S_EC_CMD_EVENT		0xf0
> +#define X13S_EC_REG_POWER_STATE	0x80
> +#define X13S_EC_POWER_STATE_ENTER	0x55
> +#define X13S_EC_POWER_STATE_EXIT	0xaa
> +#define X13S_EC_REG_KBD_BACKLIGHT	0xc0
> +#define X13S_EC_BACKLIGHT_MASK	GENMASK(5, 4)
> +#define X13S_EC_REG_KBD_BACKLIGHT_SAVED	0x15
> +#define X13S_EC_BACKLIGHT_SAVED_MASK	GENMASK(2, 1)
> +#define X13S_EC_EVENT_FN_SPACE	0x1f
> +#define X13S_EC_EVENT_LID_OPEN	0x52
> +
> +struct x13s_ec {
> +	struct i2c_client *client;
> +	struct gpio_desc *power_state;
> +	/* Serializes EC transactions, backlight RMW and suspend/event state. */
> +	struct mutex lock;
> +	struct led_classdev led;
> +	enum led_brightness saved_brightness;
> +	bool suspended;
> +	bool event_pending;
> +};
> +
> +/*
> + * Each single-message transfer ends in STOP. Hold the bus segment across
> + * both halves so another client cannot interleave them. Keep the 10 ms
> + * post-command settling delay, including on failure.
> + */
> +static int x13s_read(struct x13s_ec *ec, u8 cmd, u8 reg, u8 *value)
> +{
> +	struct i2c_client *client = ec->client;
> +	u8 buf[] = { cmd, reg, 0, 1 };
> +	struct i2c_msg request = {
> +		.addr = client->addr,
> +		.flags = I2C_M_STOP,
> +		.len = sizeof(buf),
> +		.buf = buf,
> +	};
> +	struct i2c_msg response = {
> +		.addr = client->addr,
> +		.flags = I2C_M_RD,
> +		.len = 1,
> +		.buf = value,
> +	};
> +	int ret;
> +
> +	lockdep_assert_held(&ec->lock);
> +	if (ec->suspended)
> +		return -EBUSY;
> +
> +	i2c_lock_bus(client->adapter, I2C_LOCK_SEGMENT);
> +	ret = __i2c_transfer(client->adapter, &request, 1);
> +	if (ret == 1)
> +		ret = __i2c_transfer(client->adapter, &response, 1);
> +	ret = ret == 1 ? 0 : (ret < 0 ? ret : -EIO);

This looks unnecessarily convoluted. Please move this to the end and split 
it to multiple if checks that return immediately. Preferrably make the 
error returning checks first and leave return 0 as the last.

> +	i2c_unlock_bus(client->adapter, I2C_LOCK_SEGMENT);
> +	fsleep(10000);
> +	return ret;
> +}
> +
> +/* Also used by normal PM callbacks while ordinary EC access is blocked. */
> +static int x13s_write(struct x13s_ec *ec, u8 reg, u8 value)
> +{
> +	u8 buf[] = { X13S_EC_CMD_WRITE, reg, 0, 1, value };
> +	int ret;
> +
> +	lockdep_assert_held(&ec->lock);
> +
> +	ret = i2c_master_send(ec->client, buf, sizeof(buf));
> +	ret = ret == sizeof(buf) ? 0 : (ret < 0 ? ret : -EIO);

Ditto.

> +	fsleep(10000);
> +	return ret;
> +}
> +
> +static int x13s_brightness(struct x13s_ec *ec)
> +{
> +	u8 value;
> +	int ret;
> +
> +	ret = x13s_read(ec, X13S_EC_CMD_READ, X13S_EC_REG_KBD_BACKLIGHT, &value);
> +	if (ret)
> +		return ret;
> +
> +	ret = FIELD_GET(X13S_EC_BACKLIGHT_MASK, value);
> +	return ret == 3 ? -EINVAL : ret;

Is that ret == 3 actually a > max_brightness check?

Add another variable that is called "brigthness", do not use a generic 
return variable for this.

> +}
> +
> +static enum led_brightness x13s_brightness_get(struct led_classdev *led)
> +{
> +	struct x13s_ec *ec = container_of(led, struct x13s_ec, led);
> +	int ret;
> +
> +	mutex_lock(&ec->lock);
> +	ret = x13s_brightness(ec);
> +	mutex_unlock(&ec->lock);
> +	return ret;
> +}
> +
> +static int x13s_set_brightness_locked(struct x13s_ec *ec,
> +				      enum led_brightness brightness)
> +{
> +	u8 old, new;
> +	int ret;
> +
> +	lockdep_assert_held(&ec->lock);
> +	if (brightness > 2)

> max_brightness ?

> +		return -EINVAL;
> +
> +	ret = x13s_read(ec, X13S_EC_CMD_READ, X13S_EC_REG_KBD_BACKLIGHT, &old);
> +	if (ret)
> +		return ret;
> +	/* Never interpret or overwrite a reserved current brightness. */
> +	if (FIELD_GET(X13S_EC_BACKLIGHT_MASK, old) == 3)

Here too?

> +		return -EINVAL;
> +	new = (old & ~X13S_EC_BACKLIGHT_MASK) |
> +		FIELD_PREP(X13S_EC_BACKLIGHT_MASK, brightness);

The indentation seems off by a bit but I think this could use 
FIELD_MODIFY() instead.

> +	return x13s_write(ec, X13S_EC_REG_KBD_BACKLIGHT, new);
> +}
> +
> +/* SCMS(0x20): save the brightness obtained from C0 in the EC. */
> +static int x13s_save_brightness_locked(struct x13s_ec *ec,
> +				       enum led_brightness brightness)
> +{
> +	u8 value;
> +	int ret;
> +
> +	lockdep_assert_held(&ec->lock);
> +	ret = x13s_read(ec, X13S_EC_CMD_READ, X13S_EC_REG_KBD_BACKLIGHT_SAVED,
> +			&value);
> +	if (ret)
> +		return ret;
> +
> +	value &= ~X13S_EC_BACKLIGHT_SAVED_MASK;
> +	value |= FIELD_PREP(X13S_EC_BACKLIGHT_SAVED_MASK, brightness);

FIELD_MODIFY()

> +	return x13s_write(ec, X13S_EC_REG_KBD_BACKLIGHT_SAVED, value);
> +}
> +
> +/* SCMS(0x21): restore C0 from the EC's saved brightness. */
> +static int x13s_restore_brightness_locked(struct x13s_ec *ec)
> +{
> +	u8 value;
> +	int brightness, ret;
> +
> +	lockdep_assert_held(&ec->lock);
> +	ret = x13s_read(ec, X13S_EC_CMD_READ, X13S_EC_REG_KBD_BACKLIGHT_SAVED,
> +			&value);
> +	if (ret)
> +		return ret;
> +
> +	brightness = FIELD_GET(X13S_EC_BACKLIGHT_SAVED_MASK, value);
> +	ret = x13s_set_brightness_locked(ec, brightness);
> +	return ret ? ret : brightness;
> +}
> +
> +static int x13s_brightness_set(struct led_classdev *led,
> +			       enum led_brightness brightness)
> +{
> +	struct x13s_ec *ec = container_of(led, struct x13s_ec, led);
> +	int ret;
> +
> +	mutex_lock(&ec->lock);
> +	ret = x13s_set_brightness_locked(ec, brightness);
> +	/* Keep software selections across the EC's lid blank/restore cycle. */
> +	if (!ret)

Please use guard() so that this can return error immediately (reverse 
the logic and handle error first).

> +		ret = x13s_save_brightness_locked(ec, brightness);
> +	mutex_unlock(&ec->lock);
> +	return ret;
> +}
> +
> +static void x13s_refresh_brightness(struct x13s_ec *ec)
> +{
> +	int brightness, ret;
> +
> +	lockdep_assert_held(&ec->lock);
> +	/* Firmware owns the transition; save its result as _Q1F does. */
> +	brightness = x13s_brightness(ec);
> +	if (brightness < 0) {
> +		dev_err_ratelimited(&ec->client->dev,
> +				    "Backlight refresh failed: %d\n", brightness);

Add include.

> +		return;
> +	}
> +	ret = x13s_save_brightness_locked(ec, brightness);
> +	if (ret)
> +		dev_err_ratelimited(&ec->client->dev,
> +				    "Backlight save failed: %d\n", ret);

Use braces for multiline constructs.

> +	led_classdev_notify_brightness_hw_changed(&ec->led, brightness);
> +}
> +
> +static void x13s_query_event(struct x13s_ec *ec)
> +{
> +	u8 event;
> +	int ret;
> +
> +	lockdep_assert_held(&ec->lock);
> +	/* One query per interrupt, with no event-draining loop or retry. */
> +	ret = x13s_read(ec, X13S_EC_CMD_EVENT, 0, &event);
> +	if (ret) {
> +		dev_err_ratelimited(&ec->client->dev, "Event query failed: %d\n", ret);
> +		return;
> +	}
> +	if (event == X13S_EC_EVENT_FN_SPACE) {
> +		x13s_refresh_brightness(ec);
> +	} else if (event == X13S_EC_EVENT_LID_OPEN) {
> +		ret = x13s_restore_brightness_locked(ec);
> +		if (ret < 0) {
> +			dev_err_ratelimited(&ec->client->dev,
> +					    "Backlight lid restore failed: %d\n", ret);
> +			return;
> +		}
> +		ec->led.brightness = ret;
> +		led_classdev_notify_brightness_hw_changed(&ec->led, ret);
> +	}
> +}
> +
> +static void x13s_process_pending_event(struct x13s_ec *ec)
> +{
> +	lockdep_assert_held(&ec->lock);
> +	if (ec->event_pending) {
> +		ec->event_pending = false;
> +		x13s_query_event(ec);
> +	}
> +}
> +
> +static irqreturn_t x13s_irq(int irq, void *data)
> +{
> +	struct x13s_ec *ec = data;
> +
> +	mutex_lock(&ec->lock);
> +	/* Coalesce suspended IRQs into one query; never access I2C here. */
> +	if (ec->suspended)
> +		ec->event_pending = true;
> +	else
> +		x13s_query_event(ec);
> +	mutex_unlock(&ec->lock);
> +	return IRQ_HANDLED;
> +}
> +
> +static int x13s_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct x13s_ec *ec;
> +	int ret;
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
> +		return -EOPNOTSUPP;
> +	if (client->irq <= 0)
> +		return dev_err_probe(dev, -EINVAL, "Missing event IRQ\n");
> +	if (irq_get_trigger_type(client->irq) != IRQ_TYPE_EDGE_FALLING)
> +		return dev_err_probe(dev, -EINVAL, "Expected falling-edge IRQ\n");
> +
> +	ec = devm_kzalloc(dev, sizeof(*ec), GFP_KERNEL);
> +	if (!ec)
> +		return -ENOMEM;
> +	ec->client = client;
> +	ret = devm_mutex_init(dev, &ec->lock);
> +	if (ret)
> +		return ret;
> +	i2c_set_clientdata(client, ec);
> +
> +	ec->power_state = devm_gpiod_get(dev, "power-state", GPIOD_OUT_HIGH);
> +	if (IS_ERR(ec->power_state))
> +		return dev_err_probe(dev, PTR_ERR(ec->power_state),
> +				     "Failed to acquire power-state GPIO high\n");

Braces.

> +
> +	ec->led.name = "platform::kbd_backlight";
> +	ec->led.max_brightness = 2;
> +	ec->led.flags = LED_BRIGHT_HW_CHANGED | LED_RETAIN_AT_SHUTDOWN;
> +	ec->led.brightness_get = x13s_brightness_get;
> +	ec->led.brightness_set_blocking = x13s_brightness_set;
> +	ret = devm_led_classdev_register(dev, &ec->led);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to register keyboard backlight\n");
> +
> +	/* Devres synchronizes the IRQ before unregistering the LED. */
> +	ret = devm_request_threaded_irq(dev, client->irq, NULL, x13s_irq,
> +					IRQF_ONESHOT, dev_name(dev), ec);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to request event IRQ\n");
> +
> +	/*
> +	 * As on the T14s, disable wakeup by default since selective EC event
> +	 * masking is not known. Keep runtime IRQ handling enabled.
> +	 */
> +	device_wakeup_disable(dev);
> +
> +	return 0;
> +}
> +
> +static int x13s_power_gpio(struct x13s_ec *ec, int value)
> +{
> +	int ret;
> +
> +	lockdep_assert_held(&ec->lock);
> +	ret = gpiod_set_value_cansleep(ec->power_state, value);
> +	if (ret)
> +		dev_err(&ec->client->dev, "Failed to set power-state GPIO to %d: %d\n",
> +			value, ret);

Braces.

> +	return ret;
> +}
> +
> +static void x13s_exit_low_power(struct x13s_ec *ec)
> +{
> +	int ret;
> +
> +	lockdep_assert_held(&ec->lock);
> +	x13s_power_gpio(ec, 1);
> +	/* DSDT: GPIO176 high, Sleep(10), then register 0x80 <- 0xaa. */
> +	fsleep(10000);
> +	ret = x13s_write(ec, X13S_EC_REG_POWER_STATE, X13S_EC_POWER_STATE_EXIT);
> +	if (ret)
> +		dev_err(&ec->client->dev, "Failed to exit low power: %d\n", ret);
> +
> +	/* Resume ordinary access even if the EC exit attempt failed. */
> +	ec->suspended = false;
> +}
> +
> +static int x13s_suspend(struct device *dev)
> +{
> +	struct x13s_ec *ec = dev_get_drvdata(dev);
> +	int ret;
> +
> +	/* Finish in-flight access while the GENI parent is still usable. */
> +	mutex_lock(&ec->lock);

Please convert to guard() so you can return immediately as needed.

> +	if (ec->suspended)
> +		goto out;
> +	/* Keep the previous snapshot if reading the hardware fails. */
> +	ret = x13s_brightness(ec);
> +	if (ret < 0)
> +		dev_err(dev, "Failed to save backlight brightness: %d\n", ret);
> +	else
> +		ec->saved_brightness = ret;
> +
> +	/* DSDT: register 0x80 <- 0x55, then GPIO176 low. */
> +	ret = x13s_write(ec, X13S_EC_REG_POWER_STATE, X13S_EC_POWER_STATE_ENTER);
> +	if (ret)
> +		dev_err(dev, "Failed to enter low power: %d\n", ret);
> +	else
> +		x13s_power_gpio(ec, 0);
> +	/* Defer event queries throughout host suspend, even on EC failure. */
> +	ec->suspended = true;
> +out:
> +	mutex_unlock(&ec->lock);
> +	return 0;
> +}
> +
> +static int x13s_resume(struct device *dev)
> +{
> +	struct x13s_ec *ec = dev_get_drvdata(dev);
> +	int ret;
> +
> +	/*
> +	 * Normal resume runs after the adapter and its GENI parent. In
> +	 * particular, device_resume_early() has re-enabled runtime PM:
> +	 * GENI's resume_noirq() alone is not sufficient for I2C transfers.
> +	 * Consume the pending query under the IRQ thread's mutex, without
> +	 * a worker that could race a new IRQ or the next suspend.
> +	 */
> +	mutex_lock(&ec->lock);
> +	x13s_exit_low_power(ec);
> +	ret = x13s_set_brightness_locked(ec, ec->saved_brightness);

What if it was not stored? Or ->saved_brightness is stale (from an 
earlier suspend than the latest one)?

> +	if (ret)
> +		dev_err(dev, "Failed to restore backlight brightness: %d\n", ret);
> +	else
> +		/* A software restore is not a hardware brightness change. */
> +		ec->led.brightness = ec->saved_brightness;

Braces needed.

> +
> +	/* A deferred lid-open restore must supersede the PM snapshot. */
> +	x13s_process_pending_event(ec);
> +	mutex_unlock(&ec->lock);
> +	return 0;
> +}
> +
> +static DEFINE_SIMPLE_DEV_PM_OPS(x13s_pm_ops, x13s_suspend, x13s_resume);
> +
> +static const struct of_device_id x13s_of_match[] = {
> +	{ .compatible = "lenovo,thinkpad-x13s-ec" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, x13s_of_match);
> +
> +static struct i2c_driver x13s_driver = {
> +	.probe = x13s_probe,
> +	.driver = {
> +		.name = "thinkpad-x13s-ec",
> +		.of_match_table = x13s_of_match,
> +		.pm = pm_sleep_ptr(&x13s_pm_ops),
> +	},
> +};
> +module_i2c_driver(x13s_driver);
> +
> +MODULE_DESCRIPTION("Lenovo ThinkPad X13s Embedded Controller");
> +MODULE_LICENSE("GPL");
> 

-- 
 i.

Re: [PATCH v3 1/3] dt-bindings: embedded-controller: Add Lenovo ThinkPad X13s EC

[Krzysztof Kozlowski]

On Mon, Sep 28, 2026 at 10:21:29PM +0000, Alex Robinson wrote:
> Add a standalone binding for the embedded controller in the Lenovo
> ThinkPad X13s. Although its I2C command transport resembles that of the
> T14s EC, its keyboard-backlight, event and power-management interfaces
> differ.
> 
> Describe the event interrupt and the separate host-controlled power-state
> signal, which is high during normal operation and low in the low-power
> state. Require this GPIO and allow wakeup-source to describe wake
> capability.
> 
> Assisted-by: LLM
> Signed-off-by: Alex Robinson <alex@ironrobin.net>
> 

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof