[PATCH v7 0/3] (no cover subject)

[PATCH v7 0/3] (no cover subject)

[Aleksandrs Vinarskis]

gpg: Signature made Sun 25 Oct 2026 11:52:52 PM CET
gpg:                using EDDSA key 8BFCF5668AA29DAD00D728F6EDAE71A20F500310
gpg: Good signature from "Aleksandrs Vinarskis <alex@vinarskis.com>" [ultimate]
gpg:                 aka "Aleksandrs Vinarskis <alex.vinarskis@gmail.com>" [ultimate]
Introduce EC driver for Snapdragon X1E based Dell XPS 13 9345

This series adds Embedded Controller (EC) driver for Dell XPS 13 9345.
While EC appears to control most of device's peripherals, particular
driver addresses power and thermal managment issues. Key operational
principle involves initial thermistor constants configuration followed
by a periodic reporting of these onboard thermistor values from across
the motherboard to the EC. The latter then handles fan ramp-up/
ramp-down internally. Suspend/Resume must be likewise propagated to EC
for power management.

The driver was developed primarily by analyzing ACPI DSDT's _DSM and
i2c dumps of communication between SoC and EC during various stages of
operation (bootup, suspend, resume).

With EC driver in place, the following issues are addressed:
1. Fans were not properly cooling the laptop, would kick in late and
   spin lazily, resulting in heavy throttling. With EC driver fans
   start sooner and hit high RPM under heavy load.
2. Fans were not stopping once SoC temperature dropped, they would keep
   slowly spinning irrespective of suspend and/or closed lid until the
   next powercycle. With EC driver shortly after SoC temperature drops,
   thermistors temperature drops, and fans ramp-down.
3. Keyboard and touch row backlight were not turning off during
   suspend - only lid close would power off the touch row. With EC
   driver behavior matches that of Windows, suspending device with lid
   open powers off the peripherals.

As thermistor readout depends on pmic's ADCs, this series introduces
EC driver and its schema, adds missing ADC to hamoa-pmics, and finally
adds thermistor and EC nodes to x1e80100-dell-xps13-9345.dts.

Additional findings:
- Max fan speed depends on Dell's power mode settings, configurable in
  BIOS or using Windows app (relies on ACPI-WMI). It appears best
  cooling performance is achieved under 'Ultra Performance' profile.
- When the said power mode is changed using Windows app, EC IRQ is
  triggered. Windows performs what appears to be thermistor contants
  readout, though its not obvious what it is used for.
- Given similarities between Dell XPS 13 series (codename 'tributo')
  and Snapdragon-based Latitude, Inspiron ('thena'), including matching
  EC address and response to suspend/resume command the EC driver can
  be likely used for both, though in-depth testing on 'thena' is
  required.

This series depends on QCOM SPMI PMIC5 Gen3 ADC [1], which was added to
`linux-next` for v7.1, but must be picked if this series to be backported.

[1] https://lore.kernel.org/all/20260209105438.596339-1-jishnu.prakash@oss.qualcomm.com/

Signed-off-by: Aleksandrs Vinarskis <alex@vinarskis.com>
---
Changes in v7:
- Rebase on latest linux-next
- Fixed 1st, 2nd channels from AMUX3_GPIO, AMUX4_GPIO to AMUX1_GPIO,
  AMUX2_GPIO as per Michael Burch, Liviu Nicoara.
- Fixed sys_therm0..sys_therm6 to sys_therm1..sys_therm7. Initial
  convention was picked based on schematics. It is now changed to match
  most of the existing device-trees.
- Fixed dt-bindings schema to not use arch specific imports as per
  Rob Herring, sashiko.
- Adjusted thermistor label as per additional findings from reverse
  engineering investigation.
- Link to v6: https://lore.kernel.org/r/20261011-dell-xps-9345-ec-v6-0-2a66a364fe70@vinarskis.com

Changes in v6:
- Fix dts mistake due to rebase
- Link to v5: https://lore.kernel.org/r/20261011-dell-xps-9345-ec-v5-0-2ab1258b84c1@vinarskis.com

Changes in v5:
- Rebase on latest linux-next
- Re-schedule work if suspend fails as per Ilpo
- Fix comments as per Ilpo
- Link to v4: https://lore.kernel.org/r/20260809-dell-xps-9345-ec-v4-0-88306dbbc7aa@vinarskis.com

Changes in v4:
- Change to .data struct provided via compatible, which contains expected
  thermistor arrangement and magic init sequence to make it easier to
  support new systems as per Bryan
- Change milicelsius spelling, add newline, return as per Bryan
- Link to v3: https://lore.kernel.org/r/20260801-dell-xps-9345-ec-v3-0-9f4bdb5a5dad@vinarskis.com

Changes in v3:
- Rebase on latest linux-next
- Add Neil, Stephan to CC, as they worked on ADC for PMK8550
- Drop VADC for pmk8550 as it was already merged upstream
- Rename PMK8550 registers to use newly upstreamed headers for ADC5_GEN3
  in both dt-bindings, device-tree patches
- Rename ADC labels to have meaningful for user names as per Bryan
- Drop Dmitry's RB from last patch since it was almost completely changed
  now
- Link to v2: https://lore.kernel.org/r/20260404-dell-xps-9345-ec-v2-0-c977c3caa81f@vinarskis.com

Changes in v2:
- Update cover letter to indicate dependency on QCOM ADC series, only
  relevant for backporting
- Update description of dell_xps_ec_suspend: entering suspend does not
  necessarily ramp down the fans, if thermistors still report high temps
- Add die_temp/xo_therm to pmk8550 as per Konrad Dybcio
- Add missing header imports as per Ilpo Järvinen
- Add explanation for EC reset pin being reserved
- Fix device-tree: minor issues as per Konrad Dybcio
- Fix device-tree: alingment issues as per Konrad Dybcio
- Fix driver: alingment issues as per Bjorn Andersson
- Fix driver: handle temp value as 16bit register as per Ilpo Järvinen
- Fix bindigs: description and example as per Krzysztof Kozlowski 
- Link to v1: https://lore.kernel.org/r/20260401-dell-xps-9345-ec-v1-0-afa5cacd49be@vinarskis.com

---
Aleksandrs Vinarskis (3):
      dt-bindings: platform: introduce EC for Dell XPS 13 9345
      platform: arm64: dell-xps-ec: new driver
      arm64: dts: qcom: x1e80100-dell-xps13-9345: introduce EC

 .../embedded-controller/dell,xps13-9345-ec.yaml    |  90 ++++++
 MAINTAINERS                                        |   6 +
 .../boot/dts/qcom/x1e80100-dell-xps13-9345.dts     |  65 ++++-
 drivers/platform/arm64/Kconfig                     |  12 +
 drivers/platform/arm64/Makefile                    |   1 +
 drivers/platform/arm64/dell-xps-ec.c               | 308 +++++++++++++++++++++
 6 files changed, 480 insertions(+), 2 deletions(-)
---
base-commit: 77cb3534193b758eaecd662a610a3a1bffde43ed
change-id: 20260331-dell-xps-9345-ec-e5f49d1bef61

Best regards,
-- 
Aleksandrs Vinarskis <alex@vinarskis.com>

[PATCH v7 1/3] dt-bindings: platform: introduce EC for Dell XPS 13 9345

[Aleksandrs Vinarskis]

Add bindings for Embedded Controller (EC) in Dell XPS 13 9345 (platform
codename 'tributo'). It may be partially or fully compatible with EC
found in Snapdragon-based Dell Latitude, Inspiron ('thena').

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Aleksandrs Vinarskis <alex@vinarskis.com>
---
 .../embedded-controller/dell,xps13-9345-ec.yaml    | 90 ++++++++++++++++++++++
 MAINTAINERS                                        |  5 ++
 2 files changed, 95 insertions(+)

diff --git a/Documentation/devicetree/bindings/embedded-controller/dell,xps13-9345-ec.yaml b/Documentation/devicetree/bindings/embedded-controller/dell,xps13-9345-ec.yaml
new file mode 100644
index 000000000000..29cbb837a31b
--- /dev/null
+++ b/Documentation/devicetree/bindings/embedded-controller/dell,xps13-9345-ec.yaml
@@ -0,0 +1,90 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/embedded-controller/dell,xps13-9345-ec.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Dell XPS 13 9345 Embedded Controller
+
+maintainers:
+  - Aleksandrs Vinarskis <alex@vinarskis.com>
+
+description:
+  The Dell XPS 13 9345 has an Embedded Controller (EC) which handles thermal
+  and power management. It is communicating with SoC over multiple i2c busses.
+  Among other things, it handles fan speed control, thermal shutdown, peripheral
+  power supply including trackpad, touch-row, display. For these functions, it
+  requires frequently updated thermal readings from onboard thermistors.
+
+properties:
+  compatible:
+    const: dell,xps13-9345-ec
+
+  reg:
+    const: 0x3b
+
+  interrupts:
+    maxItems: 1
+
+  io-channels:
+    description:
+      ADC channels connected to the 7 onboard thermistors on PMK8550.
+      EC requires frequent thermal readings of these channels to perform
+      automated fan speed control.
+    items:
+      - description: ADC channel for sys_therm1
+      - description: ADC channel for sys_therm2
+      - description: ADC channel for sys_therm3
+      - description: ADC channel for sys_therm4
+      - description: ADC channel for sys_therm5
+      - description: ADC channel for sys_therm6
+      - description: ADC channel for sys_therm7
+
+  io-channel-names:
+    items:
+      - const: sys_therm1
+      - const: sys_therm2
+      - const: sys_therm3
+      - const: sys_therm4
+      - const: sys_therm5
+      - const: sys_therm6
+      - const: sys_therm7
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - io-channels
+  - io-channel-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        embedded-controller@3b {
+            compatible = "dell,xps13-9345-ec";
+            reg = <0x3b>;
+            interrupts-extended = <&tlmm 66 IRQ_TYPE_LEVEL_LOW>;
+
+            io-channels = <&pmk8550_vadc 0x14a>,
+                          <&pmk8550_vadc 0x14b>,
+                          <&pmk8550_vadc 0x144>,
+                          <&pmk8550_vadc 0x145>,
+                          <&pmk8550_vadc 0x146>,
+                          <&pmk8550_vadc 0x147>,
+                          <&pmk8550_vadc 0x148>,
+            io-channel-names = "sys_therm1",
+                               "sys_therm2",
+                               "sys_therm3",
+                               "sys_therm4",
+                               "sys_therm5",
+                               "sys_therm6",
+                               "sys_therm7";
+        };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 1e0d90fe2220..f20bb171e5ee 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7537,6 +7537,11 @@ S:	Maintained
 F:	Documentation/ABI/testing/sysfs-class-firmware-attributes
 F:	drivers/platform/x86/dell/dell-wmi-sysman/
 
+DELL XPS EMBEDDED CONTROLLER DRIVER
+M:	Aleksandrs Vinarskis <alex@vinarskis.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/embedded-controller/dell,xps13-9345-ec.yaml
+
 DELTA AHE-50DC FAN CONTROL MODULE DRIVER
 M:	Zev Weiss <zev@bewilderbeest.net>
 L:	linux-hwmon@vger.kernel.org

-- 
2.54.0

[PATCH v7 2/3] platform: arm64: dell-xps-ec: new driver

[Aleksandrs Vinarskis]

Introduce EC driver for Dell XPS 13 9345 (codename 'tributo') which may
partially of fully compatible with Snapdragon-based Dell Latitude,
Inspiron ('thena'). Primary function of this driver is unblock EC's
thermal management, specifically to provide it with necessary
information to control device fans, peripherals power.

The driver was developed primarily by analyzing ACPI DSDT's _DSM and
i2c dumps of communication between SoC and EC. Changes to Windows
driver's behavior include increasing temperature feed loop from ~50ms
to 100ms here.

While Xps's EC is rather complex and controls practically all device
peripherals including touch row's brightness and special keys such as
mic mute, these do not go over this particular i2c interface.

Not yet implemented features:
- On lid-close IRQ event is registered. Windows performs what to
  appears to be thermistor constants readout, though its not obvious
  what it used for.
- According to ACPI's _DSM there is a method to readout fans' RPM.
- Initial thermistor constants were sniffed from Windows, these can be
  likely fine tuned for better cooling performance.
- There is additional temperature reading that Windows sents to EC but
  more rare than others, likely SoC T_j / TZ98 or TZ4. This is the only
  thermal zone who's reading can exceed 115C without triggering thermal
  shutdown.
- Given similarities between 'tributo' and 'thena' platforms, including
  EC i2c address, driver can be potentially extended to support both.

Signed-off-by: Aleksandrs Vinarskis <alex@vinarskis.com>
---
 MAINTAINERS                          |   1 +
 drivers/platform/arm64/Kconfig       |  12 ++
 drivers/platform/arm64/Makefile      |   1 +
 drivers/platform/arm64/dell-xps-ec.c | 308 +++++++++++++++++++++++++++++++++++
 4 files changed, 322 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index f20bb171e5ee..791bc39af64f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7541,6 +7541,7 @@ DELL XPS EMBEDDED CONTROLLER DRIVER
 M:	Aleksandrs Vinarskis <alex@vinarskis.com>
 S:	Maintained
 F:	Documentation/devicetree/bindings/embedded-controller/dell,xps13-9345-ec.yaml
+F:	drivers/platform/arm64/dell-xps-ec.c
 
 DELTA AHE-50DC FAN CONTROL MODULE DRIVER
 M:	Zev Weiss <zev@bewilderbeest.net>
diff --git a/drivers/platform/arm64/Kconfig b/drivers/platform/arm64/Kconfig
index de45c5e101a4..63d53e05b502 100644
--- a/drivers/platform/arm64/Kconfig
+++ b/drivers/platform/arm64/Kconfig
@@ -33,6 +33,18 @@ config EC_ACER_ASPIRE1
 	  laptop where this information is not properly exposed via the
 	  standard ACPI devices.
 
+config EC_DELL_XPS
+	tristate "Dell XPS 9345 Embedded Controller driver"
+	depends on ARCH_QCOM || COMPILE_TEST
+	depends on I2C
+	depends on IIO
+	help
+	  Driver for the Embedded Controller in the Qualcomm Snapdragon-based
+	  Dell XPS 13 9345, which handles thermal management and fan speed
+	  control.
+
+	  Say M or Y here to include this support.
+
 config EC_HUAWEI_GAOKUN
 	tristate "Huawei Matebook E Go Embedded Controller driver"
 	depends on ARCH_QCOM || COMPILE_TEST
diff --git a/drivers/platform/arm64/Makefile b/drivers/platform/arm64/Makefile
index 605cca4d42a9..400a2c732410 100644
--- a/drivers/platform/arm64/Makefile
+++ b/drivers/platform/arm64/Makefile
@@ -6,6 +6,7 @@
 #
 
 obj-$(CONFIG_EC_ACER_ASPIRE1)	+= acer-aspire1-ec.o
+obj-$(CONFIG_EC_DELL_XPS)	+= dell-xps-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
diff --git a/drivers/platform/arm64/dell-xps-ec.c b/drivers/platform/arm64/dell-xps-ec.c
new file mode 100644
index 000000000000..20cb2a1b7bbd
--- /dev/null
+++ b/drivers/platform/arm64/dell-xps-ec.c
@@ -0,0 +1,308 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2026, Aleksandrs Vinarskis <alex@vinarskis.com>
+ */
+
+#include <linux/array_size.h>
+#include <linux/container_of.h>
+#include <linux/dev_printk.h>
+#include <linux/device.h>
+#include <linux/devm-helpers.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/iio/consumer.h>
+#include <linux/interrupt.h>
+#include <linux/jiffies.h>
+#include <linux/module.h>
+#include <linux/overflow.h>
+#include <linux/pm.h>
+#include <linux/types.h>
+#include <linux/unaligned.h>
+#include <linux/units.h>
+#include <linux/workqueue.h>
+
+#define DELL_XPS_EC_SUSPEND_CMD		0xb9
+#define DELL_XPS_EC_SUSPEND_MSG_LEN	64
+
+#define DELL_XPS_EC_TEMP_CMD0		0xfb
+#define DELL_XPS_EC_TEMP_CMD1		0x20
+#define DELL_XPS_EC_TEMP_CMD3		0x02
+#define DELL_XPS_EC_TEMP_MSG_LEN	6
+#define DELL_XPS_EC_TEMP_POLL_JIFFIES	msecs_to_jiffies(100)
+
+/*
+ * Mapping between IIO channel name (as per schematics) and EC command byte.
+ */
+struct dell_xps_ec_therm {
+	const char *name;
+	u8 cmd;
+};
+
+struct dell_xps_ec_data {
+	const struct dell_xps_ec_therm *therms;
+	unsigned int num_therms;
+	const u8 *profile;
+	size_t profile_len;
+};
+
+/*
+ * Format:
+ * - header/unknown (2 bytes)
+ * - per-thermistor entries (3 bytes): thermistor_id, param1, param2
+ */
+static const u8 dell_xps13_9345_thermistor_profile[] = {
+	0xff, 0x54,
+	0x01, 0x00, 0x2b,	/* sys_therm1 */
+	0x02, 0x44, 0x2a,	/* sys_therm2 */
+	0x03, 0x44, 0x2b,	/* sys_therm3 */
+	0x04, 0x44, 0x28,	/* sys_therm4 */
+	0x05, 0x55, 0x2a,	/* sys_therm5 */
+	0x06, 0x44, 0x26,	/* sys_therm6 */
+	0x07, 0x44, 0x2b,	/* sys_therm7 */
+};
+
+static const struct dell_xps_ec_therm dell_xps13_9345_therms[] = {
+	/* TODO: 0x01 is sent only occasionally, likely TZ98 or TZ4. */
+	{ "sys_therm1", 0x02 },
+	{ "sys_therm2", 0x03 },
+	{ "sys_therm3", 0x04 },
+	{ "sys_therm4", 0x05 },
+	{ "sys_therm5", 0x06 },
+	{ "sys_therm6", 0x07 },
+	{ "sys_therm7", 0x08 },
+};
+
+static const struct dell_xps_ec_data dell_xps13_9345_data = {
+	.therms = dell_xps13_9345_therms,
+	.num_therms = ARRAY_SIZE(dell_xps13_9345_therms),
+	.profile = dell_xps13_9345_thermistor_profile,
+	.profile_len = sizeof(dell_xps13_9345_thermistor_profile),
+};
+
+struct dell_xps_ec {
+	struct device *dev;
+	struct i2c_client *client;
+	const struct dell_xps_ec_data *data;
+	struct delayed_work temp_work;
+	unsigned int num_therms;
+	struct iio_channel *therm_channels[] __counted_by(num_therms);
+};
+
+static int dell_xps_ec_suspend_cmd(struct dell_xps_ec *ec, bool suspend)
+{
+	u8 buf[DELL_XPS_EC_SUSPEND_MSG_LEN] = {};
+	int ret;
+
+	buf[0] = DELL_XPS_EC_SUSPEND_CMD;
+	buf[1] = suspend ? 0x01 : 0x00;
+	/* bytes 2..63 remain zero */
+
+	ret = i2c_master_send(ec->client, buf, sizeof(buf));
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int dell_xps_ec_send_temp(struct dell_xps_ec *ec, u8 cmd_byte,
+				 int milli_celsius)
+{
+	u8 buf[DELL_XPS_EC_TEMP_MSG_LEN];
+	u16 deci_celsius;
+	int ret;
+
+	deci_celsius = milli_celsius / MILLIDEGREE_PER_DECIDEGREE;
+
+	buf[0] = DELL_XPS_EC_TEMP_CMD0;
+	buf[1] = DELL_XPS_EC_TEMP_CMD1;
+	buf[2] = cmd_byte;
+	buf[3] = DELL_XPS_EC_TEMP_CMD3;
+	put_unaligned_le16(deci_celsius, &buf[4]);
+
+	ret = i2c_master_send(ec->client, buf, sizeof(buf));
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static void dell_xps_ec_temp_work_fn(struct work_struct *work)
+{
+	struct dell_xps_ec *ec = container_of(work, struct dell_xps_ec,
+					      temp_work.work);
+	const struct dell_xps_ec_therm *therms = ec->data->therms;
+	int val, ret;
+	unsigned int i;
+
+	for (i = 0; i < ec->num_therms; i++) {
+		if (!ec->therm_channels[i])
+			continue;
+
+		ret = iio_read_channel_processed(ec->therm_channels[i], &val);
+		if (ret < 0) {
+			dev_err_ratelimited(ec->dev,
+					    "Failed to read thermistor %s: %d\n",
+					    therms[i].name, ret);
+			continue;
+		}
+
+		ret = dell_xps_ec_send_temp(ec, therms[i].cmd, val);
+		if (ret < 0) {
+			dev_err_ratelimited(ec->dev,
+					    "Failed to send temp for %s: %d\n",
+					    therms[i].name, ret);
+		}
+	}
+
+	schedule_delayed_work(&ec->temp_work, DELL_XPS_EC_TEMP_POLL_JIFFIES);
+}
+
+static irqreturn_t dell_xps_ec_irq_handler(int irq, void *data)
+{
+	struct dell_xps_ec *ec = data;
+
+	/*
+	 * TODO: IRQ is fired on lid-close. Follow Windows example to read out
+	 *       the thermistor thresholds and potentially fan speeds.
+	 */
+	dev_dbg(ec->dev, "IRQ triggered (irq=%d)\n", irq);
+
+	return IRQ_HANDLED;
+}
+
+static int dell_xps_ec_probe(struct i2c_client *client)
+{
+	const struct dell_xps_ec_data *data;
+	struct device *dev = &client->dev;
+	struct dell_xps_ec *ec;
+	unsigned int i;
+	int ret;
+
+	data = i2c_get_match_data(client);
+	if (!data)
+		return dev_err_probe(dev, -ENODEV, "No match data\n");
+
+	ec = devm_kzalloc(dev, struct_size(ec, therm_channels, data->num_therms),
+			  GFP_KERNEL);
+	if (!ec)
+		return -ENOMEM;
+
+	ec->dev = dev;
+	ec->client = client;
+	ec->data = data;
+	ec->num_therms = data->num_therms;
+	i2c_set_clientdata(client, ec);
+
+	/* Set default thermistor profile */
+	ret = i2c_master_send(client, data->profile, data->profile_len);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Failed to set thermistor profile\n");
+
+	/* Get IIO channels for thermistors */
+	for (i = 0; i < ec->num_therms; i++) {
+		ec->therm_channels[i] =
+			devm_iio_channel_get(dev, data->therms[i].name);
+		if (IS_ERR(ec->therm_channels[i])) {
+			ret = PTR_ERR(ec->therm_channels[i]);
+			ec->therm_channels[i] = NULL;
+			if (ret == -EPROBE_DEFER)
+				return ret;
+			dev_warn(dev, "Thermistor %s not available: %d\n",
+				 data->therms[i].name, ret);
+		}
+	}
+
+	/* Start periodic temperature reporting */
+	ret = devm_delayed_work_autocancel(dev, &ec->temp_work,
+					   dell_xps_ec_temp_work_fn);
+	if (ret)
+		return ret;
+	schedule_delayed_work(&ec->temp_work, DELL_XPS_EC_TEMP_POLL_JIFFIES);
+	dev_dbg(dev, "Started periodic temperature reporting to EC every %d ms\n",
+		jiffies_to_msecs(DELL_XPS_EC_TEMP_POLL_JIFFIES));
+
+	/* Request IRQ for EC events */
+	ret = devm_request_threaded_irq(dev, client->irq, NULL,
+					dell_xps_ec_irq_handler,
+					IRQF_ONESHOT, dev_name(dev), ec);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to request IRQ\n");
+
+	return 0;
+}
+
+/*
+ * Notify EC of suspend
+ *
+ * This will:
+ * - Cut power to display/trackpad/keyboard/touchrow, wake-up source still works.
+ */
+static int dell_xps_ec_suspend(struct device *dev)
+{
+	struct dell_xps_ec *ec = dev_get_drvdata(dev);
+
+	int ret;
+
+	cancel_delayed_work_sync(&ec->temp_work);
+
+	ret = dell_xps_ec_suspend_cmd(ec, true);
+	if (ret) {
+		schedule_delayed_work(&ec->temp_work,
+				      DELL_XPS_EC_TEMP_POLL_JIFFIES);
+		return ret;
+	}
+
+	return 0;
+}
+
+/*
+ * Notify EC of resume
+ *
+ * This will undo the suspend actions. Without the resume signal, device would
+ * wake up but be forced back into suspend by EC within seconds.
+ */
+static int dell_xps_ec_resume(struct device *dev)
+{
+	struct dell_xps_ec *ec = dev_get_drvdata(dev);
+	int ret;
+
+	ret = dell_xps_ec_suspend_cmd(ec, false);
+	if (ret)
+		return ret;
+
+	schedule_delayed_work(&ec->temp_work, DELL_XPS_EC_TEMP_POLL_JIFFIES);
+
+	return 0;
+}
+
+static const struct of_device_id dell_xps_ec_of_match[] = {
+	{ .compatible = "dell,xps13-9345-ec", .data = &dell_xps13_9345_data },
+	{}
+};
+MODULE_DEVICE_TABLE(of, dell_xps_ec_of_match);
+
+static const struct i2c_device_id dell_xps_ec_i2c_id[] = {
+	{ "dell-xps-ec", (kernel_ulong_t)&dell_xps13_9345_data },
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, dell_xps_ec_i2c_id);
+
+static const struct dev_pm_ops dell_xps_ec_pm_ops = {
+	SYSTEM_SLEEP_PM_OPS(dell_xps_ec_suspend, dell_xps_ec_resume)
+};
+
+static struct i2c_driver dell_xps_ec_driver = {
+	.driver = {
+		.name = "dell-xps-ec",
+		.of_match_table = dell_xps_ec_of_match,
+		.pm = &dell_xps_ec_pm_ops,
+	},
+	.probe = dell_xps_ec_probe,
+	.id_table = dell_xps_ec_i2c_id,
+};
+module_i2c_driver(dell_xps_ec_driver);
+
+MODULE_AUTHOR("Aleksandrs Vinarskis <alex@vinarskis.com>");
+MODULE_DESCRIPTION("Dell XPS 13 9345 Embedded Controller");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("IIO_CONSUMER");

-- 
2.54.0

[PATCH v7 3/3] arm64: dts: qcom: x1e80100-dell-xps13-9345: introduce EC

[Aleksandrs Vinarskis]

Describe embedded controller, its interrupt and required thermal zones.
Add user-friendly labels for know thermal zones.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Signed-off-by: Aleksandrs Vinarskis <alex@vinarskis.com>
---
 .../boot/dts/qcom/x1e80100-dell-xps13-9345.dts     | 65 +++++++++++++++++++++-
 1 file changed, 63 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts b/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts
index f48e6bc35006..3f0c17c845b3 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts
+++ b/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts
@@ -759,8 +759,32 @@ retimer_ss0_con_sbu_out: endpoint {
 
 &i2c5 {
 	clock-frequency = <100000>;
-	status = "disabled";
-	/* EC @0x3b */
+	status = "okay";
+
+	embedded-controller@3b {
+		compatible = "dell,xps13-9345-ec";
+		reg = <0x3b>;
+
+		interrupts-extended = <&tlmm 66 IRQ_TYPE_LEVEL_LOW>;
+
+		pinctrl-0 = <&ec_int_n_default>;
+		pinctrl-names = "default";
+
+		io-channels = <&pmk8550_vadc ADC5_GEN3_AMUX1_GPIO_100K_PU(1)>,
+			      <&pmk8550_vadc ADC5_GEN3_AMUX2_GPIO_100K_PU(1)>,
+			      <&pmk8550_vadc ADC5_GEN3_AMUX1_THM_100K_PU(1)>,
+			      <&pmk8550_vadc ADC5_GEN3_AMUX2_THM_100K_PU(1)>,
+			      <&pmk8550_vadc ADC5_GEN3_AMUX3_THM_100K_PU(1)>,
+			      <&pmk8550_vadc ADC5_GEN3_AMUX4_THM_100K_PU(1)>,
+			      <&pmk8550_vadc ADC5_GEN3_AMUX5_THM_100K_PU(1)>;
+		io-channel-names = "sys_therm1",
+				   "sys_therm2",
+				   "sys_therm3",
+				   "sys_therm4",
+				   "sys_therm5",
+				   "sys_therm6",
+				   "sys_therm7";
+	};
 };
 
 &i2c7 {
@@ -1025,6 +1049,37 @@ rtmr0_1p8_reg_en: rtmr0-1p8-reg-en-state {
 	};
 };
 
+&pmk8550_vadc {
+	channel@144 {
+		label = "vph_pwr_therm";
+	};
+
+	channel@145 {
+		label = "usb_charger_right_therm";
+	};
+
+	channel@146 {
+		label = "ssd_conn_therm";
+	};
+
+	channel@147 {
+		label = "vreg_rtmr0_1p8_therm";
+	};
+
+	channel@148 {
+		label = "keyboard_therm";
+	};
+
+	channel@14a {
+		label = "lpddr5_therm";
+	};
+
+	/* Between Left USB charging controller and SSD connector */
+	channel@14b {
+		label = "sys_therm2";
+	};
+};
+
 &qupv3_0 {
 	status = "okay";
 };
@@ -1103,6 +1158,12 @@ cam_indicator_en: cam-indicator-en-state {
 		bias-disable;
 	};
 
+	ec_int_n_default: ec-int-n-state {
+		pins = "gpio66";
+		function = "gpio";
+		bias-disable;
+	};
+
 	edp_bl_en: edp-bl-en-state {
 		pins = "gpio74";
 		function = "gpio";

-- 
2.54.0

Re: [PATCH v7 1/3] dt-bindings: platform: introduce EC for Dell XPS 13 9345

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] dt-bindings: syntax error in dell,xps13-9345-ec.yaml example io-channels
--

commit 6b26d06321d0162521ee363bfa4a27b82eb6cc2e
Author: Aleksandrs Vinarskis <alex@vinarskis.com>

dt-bindings: platform: introduce EC for Dell XPS 13 9345

This commit adds Device Tree bindings for the Embedded Controller (EC) in
the Dell XPS 13 9345 (platform codename tributo).

> diff --git a/Documentation/devicetree/bindings/embedded-controller/dell,xps13-9345-ec.yaml b/Documentation/devicetree/bindings/embedded-controller/dell,xps13-9345-ec.yaml
> new file mode 100644
> index 0000000000000..29cbb837a31b8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/embedded-controller/dell,xps13-9345-ec.yaml
> @@ -0,0 +1,90 @@

[ ... ]

> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        embedded-controller@3b {
> +            compatible = "dell,xps13-9345-ec";
> +            reg = <0x3b>;
> +            interrupts-extended = <&tlmm 66 IRQ_TYPE_LEVEL_LOW>;
> +
> +            io-channels = <&pmk8550_vadc 0x14a>,
> +                          <&pmk8550_vadc 0x14b>,
> +                          <&pmk8550_vadc 0x144>,
> +                          <&pmk8550_vadc 0x145>,
> +                          <&pmk8550_vadc 0x146>,
> +                          <&pmk8550_vadc 0x147>,
> +                          <&pmk8550_vadc 0x148>,
> +            io-channel-names = "sys_therm1",

[Severity: Low]
In the examples section of
Documentation/devicetree/bindings/embedded-controller/dell,xps13-9345-ec.yaml,
the last entry of io-channels in embedded-controller@3b ends with a comma
instead of a semicolon before io-channel-names.

Should <&pmk8550_vadc 0x148>, end with a semicolon instead so that
dt_binding_check does not fail with a dtc syntax error when compiling the
example?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026-dell-xps-9345-ec-v7-0-828ce301b839@vinarskis.com?part=1

Re: [PATCH v7 2/3] platform: arm64: dell-xps-ec: new driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] platform: arm64: dell-xps-ec: level-low IRQ storm in dell_xps_ec_irq_handler()
- [Medium] platform: arm64: dell-xps-ec: temp_work not rescheduled on resume error
--

commit 40d07e79bd3d5e1fdf49249741f6c1ece2a424aa
Author: Aleksandrs Vinarskis <alex@vinarskis.com>

platform: arm64: dell-xps-ec: new driver

This commit introduces an I2C embedded controller driver for the Dell XPS
13 9345 to configure thermistor profiles, periodically feed IIO thermistor
temperature readings to the EC, and notify the EC across system sleep
transitions.

> diff --git a/drivers/platform/arm64/dell-xps-ec.c b/drivers/platform/arm64/dell-xps-ec.c
> new file mode 100644
> index 0000000000000..20cb2a1b7bbd4
> --- /dev/null
> +++ b/drivers/platform/arm64/dell-xps-ec.c

[ ... ]

> +static irqreturn_t dell_xps_ec_irq_handler(int irq, void *data)
> +{
> +	struct dell_xps_ec *ec = data;
> +
> +	/*
> +	 * TODO: IRQ is fired on lid-close. Follow Windows example to read out
> +	 *       the thermistor thresholds and potentially fan speeds.
> +	 */
> +	dev_dbg(ec->dev, "IRQ triggered (irq=%d)\n", irq);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int dell_xps_ec_probe(struct i2c_client *client)
> +{

[ ... ]

> +	/* Request IRQ for EC events */
> +	ret = devm_request_threaded_irq(dev, client->irq, NULL,
> +					dell_xps_ec_irq_handler,
> +					IRQF_ONESHOT, dev_name(dev), ec);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to request IRQ\n");
> +
> +	return 0;
> +}

[Severity: High]
Will requesting client->irq here cause an unmasked interrupt storm when the
EC asserts its interrupt line (for example, on lid close)?

In Documentation/devicetree/bindings/embedded-controller/dell,xps13-9345-ec.yaml,
the interrupt is specified as level-triggered active-low:

    interrupts-extended = <&tlmm 66 IRQ_TYPE_LEVEL_LOW>;

When the EC asserts the line, dell_xps_ec_irq_handler() logs a debug message
and returns IRQ_HANDLED without performing an I2C transfer to clear or
acknowledge the event on the EC. Once irq_finalize_oneshot() unmasks the line,
the level-low interrupt will immediately fire again, and returning IRQ_HANDLED
prevents note_interrupt() in kernel/irq/spurious.c from detecting and
disabling the stuck interrupt.

Should requesting the IRQ be deferred until the EC event readout is
implemented, or should the interrupt be left disabled for now?

> +
> +/*
> + * Notify EC of suspend
> + *
> + * This will:
> + * - Cut power to display/trackpad/keyboard/touchrow, wake-up source still works.
> + */
> +static int dell_xps_ec_suspend(struct device *dev)
> +{
> +	struct dell_xps_ec *ec = dev_get_drvdata(dev);
> +
> +	int ret;
> +
> +	cancel_delayed_work_sync(&ec->temp_work);
> +
> +	ret = dell_xps_ec_suspend_cmd(ec, true);
> +	if (ret) {
> +		schedule_delayed_work(&ec->temp_work,
> +				      DELL_XPS_EC_TEMP_POLL_JIFFIES);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * Notify EC of resume
> + *
> + * This will undo the suspend actions. Without the resume signal, device would
> + * wake up but be forced back into suspend by EC within seconds.
> + */
> +static int dell_xps_ec_resume(struct device *dev)
> +{
> +	struct dell_xps_ec *ec = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = dell_xps_ec_suspend_cmd(ec, false);
> +	if (ret)
> +		return ret;
> +
> +	schedule_delayed_work(&ec->temp_work, DELL_XPS_EC_TEMP_POLL_JIFFIES);
> +
> +	return 0;
> +}

[Severity: Medium]
If dell_xps_ec_suspend_cmd(ec, false) fails in dell_xps_ec_resume(), does
returning early leave ec->temp_work permanently stopped after the system
resumes or thaws?

During suspend or hibernation freeze, dell_xps_ec_suspend() cancels
ec->temp_work via cancel_delayed_work_sync(&ec->temp_work). On resume, thaw,
or restore, the PM core logs errors from dell_xps_ec_resume() but continues
resuming the system without re-running the callback. Would it make sense to
still reschedule ec->temp_work even when dell_xps_ec_suspend_cmd() returns an
error?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026-dell-xps-9345-ec-v7-0-828ce301b839@vinarskis.com?part=2

Re: [PATCH v7 3/3] arm64: dts: qcom: x1e80100-dell-xps13-9345: introduce EC

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] arm64: dts: qcom: level-low EC IRQ with stub dell_xps_ec_irq_handler()
--

commit bdc3996f914e2f3bd3df5f76f45237ab26fd449b
Author: Aleksandrs Vinarskis <alex@vinarskis.com>

arm64: dts: qcom: x1e80100-dell-xps13-9345: introduce EC

This commit enables i2c5 and adds the embedded controller node along with its
interrupt and thermal ADC channels for the Dell XPS 13 9345. It also adds
labels for the known PMK8550 VADC thermal channels.

> diff --git a/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts b/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts
> index f48e6bc350063..3f0c17c845b3e 100644
> --- a/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts
> +++ b/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts
> @@ -759,8 +759,32 @@ retimer_ss0_con_sbu_out: endpoint {
>  
>  &i2c5 {
>  	clock-frequency = <100000>;
> -	status = "disabled";
> -	/* EC @0x3b */
> +	status = "okay";
> +
> +	embedded-controller@3b {
> +		compatible = "dell,xps13-9345-ec";
> +		reg = <0x3b>;
> +
> +		interrupts-extended = <&tlmm 66 IRQ_TYPE_LEVEL_LOW>;

[Severity: High]
Can configuring this interrupt as IRQ_TYPE_LEVEL_LOW cause an unrecoverable
interrupt storm when the EC asserts gpio66 low (such as on lid close)?

In drivers/platform/arm64/dell-xps-ec.c, dell_xps_ec_probe() requests and
enables client->irq as a oneshot threaded interrupt:

drivers/platform/arm64/dell-xps-ec.c:dell_xps_ec_probe() {
	...
	/* Request IRQ for EC events */
	ret = devm_request_threaded_irq(dev, client->irq, NULL,
					dell_xps_ec_irq_handler,
					IRQF_ONESHOT, dev_name(dev), ec);
	if (ret)
		return dev_err_probe(dev, ret, "Failed to request IRQ\n");
}

However, dell_xps_ec_irq_handler() is currently a stub that logs a debug
message and returns IRQ_HANDLED without performing an I2C read to acknowledge
or clear the pending event on the EC:

drivers/platform/arm64/dell-xps-ec.c:dell_xps_ec_irq_handler() {
	struct dell_xps_ec *ec = data;

	/*
	 * TODO: IRQ is fired on lid-close. Follow Windows example to read out
	 *       the thermistor thresholds and potentially fan speeds.
	 */
	dev_dbg(ec->dev, "IRQ triggered (irq=%d)\n", irq);

	return IRQ_HANDLED;
}

Because the interrupt is level-triggered (IRQ_TYPE_LEVEL_LOW) with
IRQF_ONESHOT, irq_finalize_oneshot() unmasks the line as soon as
dell_xps_ec_irq_handler() returns. If the EC continues holding gpio66 low
because the event was not cleared over I2C, the interrupt will immediately
re-fire in a tight loop.

In addition, because dell_xps_ec_irq_handler() returns IRQ_HANDLED rather
than IRQ_NONE, note_interrupt() in kernel/irq/spurious.c never increments
desc->irqs_unhandled, preventing the spurious IRQ detector from disabling the
interrupt line.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026-dell-xps-9345-ec-v7-0-828ce301b839@vinarskis.com?part=3