mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v4 0/3] Lenovo ThinkPad X13s embedded controller support
@ 2026-09-29 19:29 Alex Robinson
  2026-09-29 19:29 ` [PATCH v4 1/3] dt-bindings: embedded-controller: Add Lenovo ThinkPad X13s EC Alex Robinson
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Alex Robinson @ 2026-09-29 19:29 UTC (permalink / raw)
  To: Hans de Goede, Ilpo Järvinen
  Cc: Bryan O'Donoghue, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Bjorn Andersson, Konrad Dybcio, Steev Klimaszewski,
	platform-driver-x86, linux-arm-msm, devicetree, linux-kernel

Add Device Tree support for the ThinkPad X13s embedded controller,
providing keyboard-backlight control and firmware-driven brightness
notifications. Its event, backlight and power-management interfaces
differ from the T14s EC and require a separate driver.

EC wakeup remains disabled by default because lid closure can trigger
an unwanted wakeup and no selective event mask is known. Userspace can
enable wakeup when desired. Other EC event mappings remain outside the
scope of this series.

This follows earlier X13s EC work by Konrad Dybcio and Steev Klimaszewski.
Development was assisted by an LLM, including analysis of the X13s ACPI
DSDT and review of the implementation.

Changes in v4:
- Set GPIO103 bias-pull-up explicitly, based on its firmware-inherited
  configuration measured at pinctrl probe before Linux claimed or
  configured the pin. CTL offset 0x67000 read 0x00000003 (pull field 3).
  The temporary measurement instrumentation is not included.
- Remove output-high from the GPIO176 pinctrl state. The driver already
  requests GPIOD_OUT_HIGH at probe and controls the line during sleep.
- Restore the suspend brightness snapshot only if it was successfully
  captured during that suspend, avoiding restoration of a stale value.
- Use FIELD_MODIFY(), validate brightness against the LED maximum, and
  simplify transfer error handling and selected mutex-protected paths.
- Include linux/ratelimit.h explicitly and adjust conditional formatting.
- Shorten commit messages to focus on rationale rather than the diff.

Changes in v3:
- Disable EC wakeup by default without disabling runtime IRQ handling.
- Retain wakeup-source as a description of hardware capability.
- Remove the Kconfig help text's claim of wake support.

Changes in v2:
- Make backlight snapshots and EC power-management operations best-effort.
- Save firmware- and software-selected brightness in the EC and restore
  the EC-saved brightness on lid open.
- Process deferred events after the normal resume brightness restore.
- Add QUP8 pin configuration verified on hardware and fix DT node ordering.

Testing:
- Confirmed GPIO103's firmware-inherited pull-up on a ThinkPad X13s using
  a vanilla kernel with temporary probe-time instrumentation.
- Verified that all three v4 patches apply in order to Linux v7.3-rc4
  without the separate PCI workaround.
- Earlier v2 hardware testing covered keyboard-backlight control,
  firmware brightness changes, s2idle, lid-close/open brightness
  restoration, and EC wake events. Those results are historical and do
  not establish validation of the complete v4 series or default-off
  wakeup policy. Full v4 build and runtime validation remain outstanding.

Earlier series:
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

base-commit: 93f51579e7df248780214094418f205253383cc5


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

* [PATCH v4 1/3] dt-bindings: embedded-controller: Add Lenovo ThinkPad X13s EC
  2026-09-29 19:29 [PATCH v4 0/3] Lenovo ThinkPad X13s embedded controller support Alex Robinson
@ 2026-09-29 19:29 ` Alex Robinson
  2026-09-29 19:29 ` [PATCH v4 2/3] platform: arm64: Add Lenovo ThinkPad X13s EC driver Alex Robinson
  2026-09-29 19:29 ` [PATCH v4 3/3] arm64: dts: qcom: sc8280xp-x13s: Add embedded controller Alex Robinson
  2 siblings, 0 replies; 5+ messages in thread
From: Alex Robinson @ 2026-09-29 19:29 UTC (permalink / raw)
  To: Hans de Goede, Ilpo Järvinen
  Cc: Bryan O'Donoghue, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Bjorn Andersson, Konrad Dybcio, Steev Klimaszewski,
	platform-driver-x86, linux-arm-msm, devicetree, linux-kernel

The ThinkPad X13s EC uses a command transport similar to the T14s EC,
but has different keyboard-backlight, event and power-management
interfaces.

Add a separate binding to describe the X13s EC.

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;
+        };
+    };
+...


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

* [PATCH v4 2/3] platform: arm64: Add Lenovo ThinkPad X13s EC driver
  2026-09-29 19:29 [PATCH v4 0/3] Lenovo ThinkPad X13s embedded controller support Alex Robinson
  2026-09-29 19:29 ` [PATCH v4 1/3] dt-bindings: embedded-controller: Add Lenovo ThinkPad X13s EC Alex Robinson
@ 2026-09-29 19:29 ` Alex Robinson
  2026-09-30  6:38   ` Ilpo Järvinen
  2026-09-29 19:29 ` [PATCH v4 3/3] arm64: dts: qcom: sc8280xp-x13s: Add embedded controller Alex Robinson
  2 siblings, 1 reply; 5+ messages in thread
From: Alex Robinson @ 2026-09-29 19:29 UTC (permalink / raw)
  To: Hans de Goede, Ilpo Järvinen
  Cc: Bryan O'Donoghue, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Bjorn Andersson, Konrad Dybcio, Steev Klimaszewski,
	platform-driver-x86, linux-arm-msm, devicetree, linux-kernel

Add support for the ThinkPad X13s embedded controller to expose keyboard
backlight control and firmware-driven brightness changes when booting
with Device Tree. Its backlight, event and power-management interfaces
differ from the T14s EC and require separate handling.

Preserve brightness across lid and system-sleep transitions using the
sequences described by the X13s ACPI firmware.

Leave EC wakeup disabled by default because the shared interrupt can
wake the system on lid closure and no selective event mask is known.
Userspace can enable wakeup when desired.

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
index cc3cae2e378b34aeb705cef654c720ca9f2ba223..86ae65ac93e7575815f34dbc7a690042ad9ca3b0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -27177,6 +27177,12 @@ S:	Maintained
 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..eb0b84e5a93733ca16fc871ea94eda30f5760b3c 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..4a7d5d03690955ba6fd3760a8af4e76b999a68bc
--- /dev/null
+++ b/drivers/platform/arm64/lenovo-thinkpad-x13s.c
@@ -0,0 +1,435 @@
+// 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/ratelimit.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 saved_brightness_valid;
+	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);
+	i2c_unlock_bus(client->adapter, I2C_LOCK_SEGMENT);
+	fsleep(10000);
+
+	if (ret < 0)
+		return ret;
+	if (ret != 1)
+		return -EIO;
+
+	return 0;
+}
+
+/* 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));
+	fsleep(10000);
+
+	if (ret < 0)
+		return ret;
+	if (ret != sizeof(buf))
+		return -EIO;
+
+	return 0;
+}
+
+static int x13s_brightness(struct x13s_ec *ec)
+{
+	u8 value;
+	int brightness, ret;
+
+	ret = x13s_read(ec, X13S_EC_CMD_READ, X13S_EC_REG_KBD_BACKLIGHT, &value);
+	if (ret)
+		return ret;
+
+	brightness = FIELD_GET(X13S_EC_BACKLIGHT_MASK, value);
+	if (brightness > ec->led.max_brightness)
+		return -EINVAL;
+
+	return brightness;
+}
+
+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 value;
+	int ret;
+
+	lockdep_assert_held(&ec->lock);
+	if (brightness > ec->led.max_brightness)
+		return -EINVAL;
+
+	ret = x13s_read(ec, X13S_EC_CMD_READ, X13S_EC_REG_KBD_BACKLIGHT, &value);
+	if (ret)
+		return ret;
+	/* Never interpret or overwrite a reserved current brightness. */
+	if (FIELD_GET(X13S_EC_BACKLIGHT_MASK, value) > ec->led.max_brightness)
+		return -EINVAL;
+	FIELD_MODIFY(X13S_EC_BACKLIGHT_MASK, &value, brightness);
+	return x13s_write(ec, X13S_EC_REG_KBD_BACKLIGHT, value);
+}
+
+/* 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;
+
+	FIELD_MODIFY(X13S_EC_BACKLIGHT_SAVED_MASK, &value, 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;
+
+	guard(mutex)(&ec->lock);
+	ret = x13s_set_brightness_locked(ec, brightness);
+	if (ret)
+		return ret;
+
+	/* Keep software selections across the EC's lid blank/restore cycle. */
+	return x13s_save_brightness_locked(ec, brightness);
+}
+
+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. */
+	guard(mutex)(&ec->lock);
+	if (ec->suspended)
+		return 0;
+	/* Only restore a snapshot captured during this suspend. */
+	ec->saved_brightness_valid = false;
+	ret = x13s_brightness(ec);
+	if (ret < 0) {
+		dev_err(dev, "Failed to save backlight brightness: %d\n", ret);
+	} else {
+		ec->saved_brightness = ret;
+		ec->saved_brightness_valid = true;
+	}
+
+	/* 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;
+	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);
+	if (ec->saved_brightness_valid) {
+		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;
+		}
+		ec->saved_brightness_valid = false;
+	}
+
+	/* 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");


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

* [PATCH v4 3/3] arm64: dts: qcom: sc8280xp-x13s: Add embedded controller
  2026-09-29 19:29 [PATCH v4 0/3] Lenovo ThinkPad X13s embedded controller support Alex Robinson
  2026-09-29 19:29 ` [PATCH v4 1/3] dt-bindings: embedded-controller: Add Lenovo ThinkPad X13s EC Alex Robinson
  2026-09-29 19:29 ` [PATCH v4 2/3] platform: arm64: Add Lenovo ThinkPad X13s EC driver Alex Robinson
@ 2026-09-29 19:29 ` Alex Robinson
  2 siblings, 0 replies; 5+ messages in thread
From: Alex Robinson @ 2026-09-29 19:29 UTC (permalink / raw)
  To: Hans de Goede, Ilpo Järvinen
  Cc: Bryan O'Donoghue, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Bjorn Andersson, Konrad Dybcio, Steev Klimaszewski,
	platform-driver-x86, linux-arm-msm, devicetree, linux-kernel

Enable the ThinkPad X13s embedded controller so its driver can provide
keyboard backlight support when booting with Device Tree.

Use a pull-up on the EC interrupt line to match the firmware-inherited
configuration measured before Linux configured the pin.

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

---
Review note: GPIO103 was measured at pinctrl probe before Linux
claimed/configured the pin. TLMM CTL offset 0x67000 read 0x00000003;
the pull field value of 3 decodes to pull-up on SC8280XP.

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..1e29bab37de6d192ec49dc407e24e1a3bc9496bd 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,20 @@ cam_rgb_default: cam-rgb-default-state {
 		};
 	};
 
+	ec_int_n: ec-int-n-state {
+		pins = "gpio103";
+		function = "gpio";
+		input-enable;
+		bias-pull-up;
+	};
+
+	ec_power_state: ec-power-state {
+		pins = "gpio176";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
 	edp_reg_en: edp-reg-en-state {
 		pins = "gpio25";
 		function = "gpio";
@@ -1591,6 +1624,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";


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

* Re: [PATCH v4 2/3] platform: arm64: Add Lenovo ThinkPad X13s EC driver
  2026-09-29 19:29 ` [PATCH v4 2/3] platform: arm64: Add Lenovo ThinkPad X13s EC driver Alex Robinson
@ 2026-09-30  6:38   ` Ilpo Järvinen
  0 siblings, 0 replies; 5+ messages in thread
From: Ilpo Järvinen @ 2026-09-30  6:38 UTC (permalink / raw)
  To: Alex Robinson
  Cc: Hans de Goede, Bryan O'Donoghue, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Konrad Dybcio, Steev Klimaszewski, platform-driver-x86,
	linux-arm-msm, devicetree, LKML

On Tue, 29 Sep 2026, Alex Robinson wrote:

> Add support for the ThinkPad X13s embedded controller to expose keyboard
> backlight control and firmware-driven brightness changes when booting
> with Device Tree. Its backlight, event and power-management interfaces
> differ from the T14s EC and require separate handling.
> 
> Preserve brightness across lid and system-sleep transitions using the
> sequences described by the X13s ACPI firmware.
> 
> Leave EC wakeup disabled by default because the shared interrupt can
> wake the system on lid closure and no selective event mask is known.
> Userspace can enable wakeup when desired.
> 
> 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
> index cc3cae2e378b34aeb705cef654c720ca9f2ba223..86ae65ac93e7575815f34dbc7a690042ad9ca3b0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -27177,6 +27177,12 @@ S:	Maintained
>  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..eb0b84e5a93733ca16fc871ea94eda30f5760b3c 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..4a7d5d03690955ba6fd3760a8af4e76b999a68bc
> --- /dev/null
> +++ b/drivers/platform/arm64/lenovo-thinkpad-x13s.c
> @@ -0,0 +1,435 @@
> +// 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/ratelimit.h>

I think this was wrong/unnecessary include to add, I was expecting 
dev_printk.h which has those dev_*_retelimited() (and dev_*() printing in 
general).

> +#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 saved_brightness_valid;
> +	bool suspended;

Why you need this, doesn't pm core already have this information for you?

> +	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);
> +	i2c_unlock_bus(client->adapter, I2C_LOCK_SEGMENT);
> +	fsleep(10000);
> +
> +	if (ret < 0)
> +		return ret;
> +	if (ret != 1)
> +		return -EIO;
> +
> +	return 0;

Much easier to read, thanks.

--
 i.

> +}
> +
> +/* 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));
> +	fsleep(10000);
> +
> +	if (ret < 0)
> +		return ret;
> +	if (ret != sizeof(buf))
> +		return -EIO;
> +
> +	return 0;
> +}
> +
> +static int x13s_brightness(struct x13s_ec *ec)
> +{
> +	u8 value;
> +	int brightness, ret;
> +
> +	ret = x13s_read(ec, X13S_EC_CMD_READ, X13S_EC_REG_KBD_BACKLIGHT, &value);
> +	if (ret)
> +		return ret;
> +
> +	brightness = FIELD_GET(X13S_EC_BACKLIGHT_MASK, value);
> +	if (brightness > ec->led.max_brightness)
> +		return -EINVAL;
> +
> +	return brightness;
> +}
> +
> +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 value;
> +	int ret;
> +
> +	lockdep_assert_held(&ec->lock);
> +	if (brightness > ec->led.max_brightness)
> +		return -EINVAL;
> +
> +	ret = x13s_read(ec, X13S_EC_CMD_READ, X13S_EC_REG_KBD_BACKLIGHT, &value);
> +	if (ret)
> +		return ret;
> +	/* Never interpret or overwrite a reserved current brightness. */
> +	if (FIELD_GET(X13S_EC_BACKLIGHT_MASK, value) > ec->led.max_brightness)
> +		return -EINVAL;
> +	FIELD_MODIFY(X13S_EC_BACKLIGHT_MASK, &value, brightness);
> +	return x13s_write(ec, X13S_EC_REG_KBD_BACKLIGHT, value);
> +}
> +
> +/* 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;
> +
> +	FIELD_MODIFY(X13S_EC_BACKLIGHT_SAVED_MASK, &value, 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;
> +
> +	guard(mutex)(&ec->lock);
> +	ret = x13s_set_brightness_locked(ec, brightness);
> +	if (ret)
> +		return ret;
> +
> +	/* Keep software selections across the EC's lid blank/restore cycle. */
> +	return x13s_save_brightness_locked(ec, brightness);
> +}
> +
> +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. */
> +	guard(mutex)(&ec->lock);
> +	if (ec->suspended)
> +		return 0;
> +	/* Only restore a snapshot captured during this suspend. */
> +	ec->saved_brightness_valid = false;
> +	ret = x13s_brightness(ec);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to save backlight brightness: %d\n", ret);
> +	} else {
> +		ec->saved_brightness = ret;
> +		ec->saved_brightness_valid = true;
> +	}
> +
> +	/* 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;
> +	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);
> +	if (ec->saved_brightness_valid) {
> +		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;
> +		}
> +		ec->saved_brightness_valid = false;
> +	}
> +
> +	/* 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");
> 

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

end of thread, other threads:[~2026-09-30  6:39 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-29 19:29 [PATCH v4 0/3] Lenovo ThinkPad X13s embedded controller support Alex Robinson
2026-09-29 19:29 ` [PATCH v4 1/3] dt-bindings: embedded-controller: Add Lenovo ThinkPad X13s EC Alex Robinson
2026-09-29 19:29 ` [PATCH v4 2/3] platform: arm64: Add Lenovo ThinkPad X13s EC driver Alex Robinson
2026-09-30  6:38   ` Ilpo Järvinen
2026-09-29 19:29 ` [PATCH v4 3/3] arm64: dts: qcom: sc8280xp-x13s: Add embedded controller Alex Robinson

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®