[PATCH v3 0/3] Introduce EC driver for Snapdragon X1E based Dell XPS 13 9345

[PATCH v3 0/3] Introduce EC driver for Snapdragon X1E based Dell XPS 13 9345

[Aleksandrs Vinarskis]

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 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    |  91 +++++++
 MAINTAINERS                                        |   6 +
 .../boot/dts/qcom/x1e80100-dell-xps13-9345.dts     |  86 ++++++-
 drivers/platform/arm64/Kconfig                     |  12 +
 drivers/platform/arm64/Makefile                    |   1 +
 drivers/platform/arm64/dell-xps-ec.c               | 268 +++++++++++++++++++++
 6 files changed, 462 insertions(+), 2 deletions(-)
---
base-commit: 9dba66b6217aacb924d997de3866e7e4c0dcf952
change-id: 20260331-dell-xps-9345-ec-e5f49d1bef61

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

[PATCH v3 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    | 91 ++++++++++++++++++++++
 MAINTAINERS                                        |  5 ++
 2 files changed, 96 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..3485117b505c
--- /dev/null
+++ b/Documentation/devicetree/bindings/embedded-controller/dell,xps13-9345-ec.yaml
@@ -0,0 +1,91 @@
+# 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_therm0
+      - 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
+
+  io-channel-names:
+    items:
+      - const: sys_therm0
+      - const: sys_therm1
+      - const: sys_therm2
+      - const: sys_therm3
+      - const: sys_therm4
+      - const: sys_therm5
+      - const: sys_therm6
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - io-channels
+  - io-channel-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <arm64/qcom/qcom-adc5-gen3.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 ADC5_GEN3_AMUX3_GPIO_100K_PU(1)>,
+                          <&pmk8550_vadc ADC5_GEN3_AMUX4_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_therm0",
+                               "sys_therm1",
+                               "sys_therm2",
+                               "sys_therm3",
+                               "sys_therm4",
+                               "sys_therm5",
+                               "sys_therm6";
+        };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index fe67f7bfa44c..9b5efed4c430 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7343,6 +7343,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

Re: [PATCH v3 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] The device tree binding example includes a nonexistent header and uses undefined parameterized macros, breaking schema validation.
--

--- Patch [1]: [PATCH v3 1/3] dt-bindings: platform: introduce EC for Dell XPS 13 9345 ---
commit 260bc940f745039b1b740f5fca09f58549473355
Author: Aleksandrs Vinarskis <alex@vinarskis.com>

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

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').

> 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
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/embedded-controller/dell,xps13-9345-ec.yaml

[ ... ]

> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <arm64/qcom/qcom-adc5-gen3.h>
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: Low]
Does this header exist in the source tree? It appears to be missing, which
would cause a fatal preprocessor error during schema validation when running
make dt_binding_check.

> +    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 ADC5_GEN3_AMUX3_GPIO_100K_PU(1)>,
                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: Low]
Are these macros defined anywhere? They appear to be undefined and used
incorrectly as parameterized macros, which would further contribute to
preprocessor and schema check errors.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260801-dell-xps-9345-ec-v3-0-9f4bdb5a5dad@vinarskis.com?part=1

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

[Aleksandrs Vinarskis]

On Saturday, August 1st, 2026 at 20:32, sashiko-bot@kernel.org <sashiko-bot@kernel.org> wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] The device tree binding example includes a nonexistent header and uses undefined parameterized macros, breaking schema validation.
> --
> 
> --- Patch [1]: [PATCH v3 1/3] dt-bindings: platform: introduce EC for Dell XPS 13 9345 ---
> commit 260bc940f745039b1b740f5fca09f58549473355
> Author: Aleksandrs Vinarskis <alex@vinarskis.com>
> 
> dt-bindings: platform: introduce EC for Dell XPS 13 9345
> 
> 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').
> 
> > 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
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/embedded-controller/dell,xps13-9345-ec.yaml
> 
> [ ... ]
> 
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +    #include <arm64/qcom/qcom-adc5-gen3.h>
>                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

It exist in linux-next, namely next-2026031, as per series dependency.

> 
> [Severity: Low]
> Does this header exist in the source tree? It appears to be missing, which
> would cause a fatal preprocessor error during schema validation when running
> make dt_binding_check.
> 
> > +    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 ADC5_GEN3_AMUX3_GPIO_100K_PU(1)>,
>                                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> [Severity: Low]
> Are these macros defined anywhere? They appear to be undefined and used
> incorrectly as parameterized macros, which would further contribute to
> preprocessor and schema check errors.

Exists in the import above. Confirmed to pass with:
`make dt_binding_check DT_SCHEMA_FILES=dell,xps13-9345-ec.yaml`

Alex

> 
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260801-dell-xps-9345-ec-v3-0-9f4bdb5a5dad@vinarskis.com?part=1
>

[PATCH v3 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 | 268 +++++++++++++++++++++++++++++++++++
 4 files changed, 282 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9b5efed4c430..64e143bc5129 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7347,6 +7347,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 e32e01b2a9bd..29416f8d7232 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 7681be4a46e9..669dc9e79afb 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..7758f5dd9342
--- /dev/null
+++ b/drivers/platform/arm64/dell-xps-ec.c
@@ -0,0 +1,268 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2026, Aleksandrs Vinarskis <alex@vinarskis.com>
+ */
+
+#include <linux/array_size.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/pm.h>
+#include <linux/unaligned.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)
+
+/*
+ * Format:
+ * - header/unknown (2 bytes)
+ * - per-thermistor entries (3 bytes): thermistor_id, param1, param2
+ */
+static const u8 dell_xps_ec_thermistor_profile[] = {
+	0xff, 0x54,
+	0x01, 0x00, 0x2b,	/* sys_therm0 */
+	0x02, 0x44, 0x2a,	/* sys_therm1 */
+	0x03, 0x44, 0x2b,	/* sys_therm2 */
+	0x04, 0x44, 0x28,	/* sys_therm3 */
+	0x05, 0x55, 0x2a,	/* sys_therm4 */
+	0x06, 0x44, 0x26,	/* sys_therm5 */
+	0x07, 0x44, 0x2b,	/* sys_therm6 */
+};
+
+/*
+ * Mapping from IIO channel name to EC command byte
+ */
+static const struct {
+	const char *name;
+	u8 cmd;
+} dell_xps_ec_therms[] = {
+	/* TODO: 0x01 is sent only occasionally, likely TZ98 or TZ4 */
+	{ "sys_therm0", 0x02 },
+	{ "sys_therm1", 0x03 },
+	{ "sys_therm2", 0x04 },
+	{ "sys_therm3", 0x05 },
+	{ "sys_therm4", 0x06 },
+	{ "sys_therm5", 0x07 },
+	{ "sys_therm6", 0x08 },
+};
+
+struct dell_xps_ec {
+	struct device *dev;
+	struct i2c_client *client;
+	struct iio_channel *therm_channels[ARRAY_SIZE(dell_xps_ec_therms)];
+	struct delayed_work temp_work;
+};
+
+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;
+
+	/* Convert milli-Celsius to deci-Celsius (Celsius * 10) */
+	deci_celsius = milli_celsius / 100;
+
+	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);
+	int val, ret, i;
+
+	for (i = 0; i < ARRAY_SIZE(dell_xps_ec_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",
+					    dell_xps_ec_therms[i].name, ret);
+			continue;
+		}
+
+		ret = dell_xps_ec_send_temp(ec, dell_xps_ec_therms[i].cmd, val);
+		if (ret < 0) {
+			dev_err_ratelimited(ec->dev,
+					    "Failed to send temp for %s: %d\n",
+					    dell_xps_ec_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_info_ratelimited(ec->dev, "IRQ triggered! (irq=%d)\n", irq);
+
+	return IRQ_HANDLED;
+}
+
+static int dell_xps_ec_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct dell_xps_ec *ec;
+	int ret, i;
+
+	ec = devm_kzalloc(dev, sizeof(*ec), GFP_KERNEL);
+	if (!ec)
+		return -ENOMEM;
+
+	ec->dev = dev;
+	ec->client = client;
+	i2c_set_clientdata(client, ec);
+
+	/* Set default thermistor profile */
+	ret = i2c_master_send(client, dell_xps_ec_thermistor_profile,
+			      sizeof(dell_xps_ec_thermistor_profile));
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Failed to set thermistor profile\n");
+
+	/* Get IIO channels for thermistors */
+	for (i = 0; i < ARRAY_SIZE(dell_xps_ec_therms); i++) {
+		ec->therm_channels[i] =
+			devm_iio_channel_get(dev, dell_xps_ec_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",
+				 dell_xps_ec_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 < 0)
+		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);
+
+	cancel_delayed_work_sync(&ec->temp_work);
+
+	return dell_xps_ec_suspend_cmd(ec, true);
+}
+
+/*
+ * 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" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, dell_xps_ec_of_match);
+
+static const struct i2c_device_id dell_xps_ec_i2c_id[] = {
+	{ "dell-xps-ec" },
+	{}
+};
+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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] System suspend and resume error paths permanently disable the thermal management polling loop.
- [High] Unacknowledged interrupt in dummy IRQ handler causes an interrupt storm.
- [Medium] Mismatch between thermistor profile initialization array and runtime temperature reporting commands.
- [Low] The `i2c_device_id` array uses a positional initializer instead of a named initializer.
--

--- Patch [2]: [PATCH v3 2/3] platform: arm64: dell-xps-ec: new driver ---
commit 552ca6978cc04ce4176db63b786af0637fffc44e
Author: Aleksandrs Vinarskis <alex@vinarskis.com>

platform: arm64: dell-xps-ec: new driver
    
Introduce EC driver for Dell XPS 13 9345 (codename 'tributo'). The driver
provides thermal management information to control fans and peripheral power.

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

[ ... ]

> +static const u8 dell_xps_ec_thermistor_profile[] = {
> +	0xff, 0x54,
> +	0x01, 0x00, 0x2b,	/* sys_therm0 */
> +	0x02, 0x44, 0x2a,	/* sys_therm1 */
> +	0x03, 0x44, 0x2b,	/* sys_therm2 */
> +	0x04, 0x44, 0x28,	/* sys_therm3 */
> +	0x05, 0x55, 0x2a,	/* sys_therm4 */
> +	0x06, 0x44, 0x26,	/* sys_therm5 */
> +	0x07, 0x44, 0x2b,	/* sys_therm6 */
> +};
> +
> +/*
> + * Mapping from IIO channel name to EC command byte
> + */
> +static const struct {
> +	const char *name;
> +	u8 cmd;
> +} dell_xps_ec_therms[] = {
> +	/* TODO: 0x01 is sent only occasionally, likely TZ98 or TZ4 */
> +	{ "sys_therm0", 0x02 },
> +	{ "sys_therm1", 0x03 },
> +	{ "sys_therm2", 0x04 },
> +	{ "sys_therm3", 0x05 },
> +	{ "sys_therm4", 0x06 },
> +	{ "sys_therm5", 0x07 },
> +	{ "sys_therm6", 0x08 },
> +};

[Severity: Medium]
Is there a mismatch between the thermistor profile initialization array and
the runtime commands?

The profile array configures thermistor IDs 0x01 through 0x07, but the
runtime temperature polling uses commands 0x02 through 0x08 for sys_therm0
through sys_therm6.

This leaves thermistor ID 0x08 uninitialized and seems to misalign the rest
of the IDs. Will this break the EC's cooling logic?

[ ... ]

> +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_info_ratelimited(ec->dev, "IRQ triggered! (irq=%d)\n", irq);
> +
> +	return IRQ_HANDLED;
> +}

[Severity: High]
Could this cause an interrupt storm on level-triggered lines?

The handler returns IRQ_HANDLED without touching the hardware to clear the
interrupt source.

For level-triggered interrupts, returning IRQ_HANDLED unmasks the interrupt
line at the controller, which can cause it to immediately fire again and hang
the CPU in softirq/IRQ handling context.

[ ... ]

> +static int dell_xps_ec_suspend(struct device *dev)
> +{
> +	struct dell_xps_ec *ec = dev_get_drvdata(dev);
> +
> +	cancel_delayed_work_sync(&ec->temp_work);
> +
> +	return dell_xps_ec_suspend_cmd(ec, true);
> +}

[Severity: High]
If dell_xps_ec_suspend_cmd() fails and aborts the system suspend, does this
permanently disable the thermal management polling loop?

The temp_work is cancelled but is not rescheduled on the error path. This
could lead to missing temperature updates to the EC, potentially causing
severe thermal throttling or overheating.

> +
> +/*
> + * 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: High]
Similarly, on resume, if dell_xps_ec_suspend_cmd() returns an error, the
function returns early and skips rescheduling the critical polling work.

Can this leave the device without thermal management after a transient
I2C error during power state transitions?

[ ... ]

> +static const struct i2c_device_id dell_xps_ec_i2c_id[] = {
> +	{ "dell-xps-ec" },
> +	{}
> +};

[Severity: Low]
Should this use a named initializer?

The I2C subsystem guidelines require that initialized arrays of type
struct i2c_device_id must use named initializers rather than positional ones.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260801-dell-xps-9345-ec-v3-0-9f4bdb5a5dad@vinarskis.com?part=2

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

[Bryan O'Donoghue]

On 01/08/2026 19:22, Aleksandrs Vinarskis wrote:
> 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 | 268 +++++++++++++++++++++++++++++++++++
>   4 files changed, 282 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9b5efed4c430..64e143bc5129 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7347,6 +7347,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 e32e01b2a9bd..29416f8d7232 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 7681be4a46e9..669dc9e79afb 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..7758f5dd9342
> --- /dev/null
> +++ b/drivers/platform/arm64/dell-xps-ec.c
> @@ -0,0 +1,268 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2026, Aleksandrs Vinarskis <alex@vinarskis.com>
> + */
> +
> +#include <linux/array_size.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/pm.h>
> +#include <linux/unaligned.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)
> +
> +/*
> + * Format:
> + * - header/unknown (2 bytes)
> + * - per-thermistor entries (3 bytes): thermistor_id, param1, param2
> + */
> +static const u8 dell_xps_ec_thermistor_profile[] = {
> +	0xff, 0x54,
> +	0x01, 0x00, 0x2b,	/* sys_therm0 */
> +	0x02, 0x44, 0x2a,	/* sys_therm1 */
> +	0x03, 0x44, 0x2b,	/* sys_therm2 */
> +	0x04, 0x44, 0x28,	/* sys_therm3 */
> +	0x05, 0x55, 0x2a,	/* sys_therm4 */
> +	0x06, 0x44, 0x26,	/* sys_therm5 */
> +	0x07, 0x44, 0x2b,	/* sys_therm6 */
> +};

You ready have intuited this as a data-structure so if you define it as 
such with __attribute(packed); it will look neater.

> +
> +/*
> + * Mapping from IIO channel name to EC command byte
> + */
> +static const struct {
> +	const char *name;
> +	u8 cmd;
> +} dell_xps_ec_therms[] = {
> +	/* TODO: 0x01 is sent only occasionally, likely TZ98 or TZ4 */
> +	{ "sys_therm0", 0x02 },
> +	{ "sys_therm1", 0x03 },
> +	{ "sys_therm2", 0x04 },
> +	{ "sys_therm3", 0x05 },
> +	{ "sys_therm4", 0x06 },
> +	{ "sys_therm5", 0x07 },
> +	{ "sys_therm6", 0x08 },
> +};
> +
> +struct dell_xps_ec {
> +	struct device *dev;
> +	struct i2c_client *client;
> +	struct iio_channel *therm_channels[ARRAY_SIZE(dell_xps_ec_therms)];

This should come from data supplied via the probe .data and compat string.

> +	struct delayed_work temp_work;
> +};
> +
> +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;
> +
> +	/* Convert milli-Celsius to deci-Celsius (Celsius * 10) */

milli-Celsius is very strange hyphenation just use the lower-case kernel 
terminology for it millicelsius.

https://www.kernel.org/doc/Documentation/devicetree/bindings/thermal/thermal.txt

> +	deci_celsius = milli_celsius / 100;
> +
> +	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);
> +	int val, ret, i;
> +
> +	for (i = 0; i < ARRAY_SIZE(dell_xps_ec_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",
> +					    dell_xps_ec_therms[i].name, ret);
> +			continue;

Is it really legitimate to continue on in this case ? I'll leave it up 
to you to think about that for yourself, perhaps yes is the answer but 
if term_channels[i] is true and a read of a channel fails - surely 
that's indicative of a bail-out situation not read the next channel 
situation

> +		}
> +
> +		ret = dell_xps_ec_send_temp(ec, dell_xps_ec_therms[i].cmd, val);
> +		if (ret < 0) {
> +			dev_err_ratelimited(ec->dev,
> +					    "Failed to send temp for %s: %d\n",
> +					    dell_xps_ec_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_info_ratelimited(ec->dev, "IRQ triggered! (irq=%d)\n", irq);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int dell_xps_ec_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct dell_xps_ec *ec;
> +	int ret, i;
> +
> +	ec = devm_kzalloc(dev, sizeof(*ec), GFP_KERNEL);
> +	if (!ec)
> +		return -ENOMEM;
> +
> +	ec->dev = dev;
> +	ec->client = client;
> +	i2c_set_clientdata(client, ec);
> +
> +	/* Set default thermistor profile */
> +	ret = i2c_master_send(client, dell_xps_ec_thermistor_profile,
> +			      sizeof(dell_xps_ec_thermistor_profile));
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "Failed to set thermistor profile\n");
> +
> +	/* Get IIO channels for thermistors */
> +	for (i = 0; i < ARRAY_SIZE(dell_xps_ec_therms); i++) {

This should be num_channels passed from your compat .data

> +		ec->therm_channels[i] =
> +			devm_iio_channel_get(dev, dell_xps_ec_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",
> +				 dell_xps_ec_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;
\n

> +	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 < 0)
> +		return dev_err_probe(dev, ret, "Failed to request IRQ\n");
> +
> +	return 0;

You aren't returning the ret.

> +}
> +
> +/*
> + * 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);
> +
> +	cancel_delayed_work_sync(&ec->temp_work);
> +
> +	return dell_xps_ec_suspend_cmd(ec, true);
> +}
> +
> +/*
> + * 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);
\n> +	return 0;
> +}
> +
> +static const struct of_device_id dell_xps_ec_of_match[] = {
> +	{ .compatible = "dell,xps13-9345-ec" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, dell_xps_ec_of_match);
> +
> +static const struct i2c_device_id dell_xps_ec_i2c_id[] = {
> +	{ "dell-xps-ec" },

A nice .data will allow extension of this to other systems with 
more/less channels.

Which is why you shouldn't use the ARRAY_SIZE() extents from statics 
declared up top directly.

> +	{}
> +};
> +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");
> 

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

[Aleksandrs Vinarskis]

On Saturday, August 1st, 2026 at 23:15, Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote:

Hi Bryan,

Thanks for review;

> On 01/08/2026 19:22, Aleksandrs Vinarskis wrote:
> > 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 | 268 +++++++++++++++++++++++++++++++++++
> >   4 files changed, 282 insertions(+)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 9b5efed4c430..64e143bc5129 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -7347,6 +7347,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 e32e01b2a9bd..29416f8d7232 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 7681be4a46e9..669dc9e79afb 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..7758f5dd9342
> > --- /dev/null
> > +++ b/drivers/platform/arm64/dell-xps-ec.c
> > @@ -0,0 +1,268 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (c) 2026, Aleksandrs Vinarskis <alex@vinarskis.com>
> > + */
> > +
> > +#include <linux/array_size.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/pm.h>
> > +#include <linux/unaligned.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)
> > +
> > +/*
> > + * Format:
> > + * - header/unknown (2 bytes)
> > + * - per-thermistor entries (3 bytes): thermistor_id, param1, param2
> > + */
> > +static const u8 dell_xps_ec_thermistor_profile[] = {
> > +	0xff, 0x54,
> > +	0x01, 0x00, 0x2b,	/* sys_therm0 */
> > +	0x02, 0x44, 0x2a,	/* sys_therm1 */
> > +	0x03, 0x44, 0x2b,	/* sys_therm2 */
> > +	0x04, 0x44, 0x28,	/* sys_therm3 */
> > +	0x05, 0x55, 0x2a,	/* sys_therm4 */
> > +	0x06, 0x44, 0x26,	/* sys_therm5 */
> > +	0x07, 0x44, 0x2b,	/* sys_therm6 */
> > +};
> 
> You ready have intuited this as a data-structure so if you define it as
> such with __attribute(packed); it will look neater.
> 

I looked into it again, and I think its safer to assume that the init
sequence is a 'magic byte sequence' instead. The order and total amount
of thermistors does not match the data structure that is being sent on
regular intervals (off by one, short by one). I am also not sure it
would be the same across different platforms.

> > +
> > +/*
> > + * Mapping from IIO channel name to EC command byte
> > + */
> > +static const struct {
> > +	const char *name;
> > +	u8 cmd;
> > +} dell_xps_ec_therms[] = {
> > +	/* TODO: 0x01 is sent only occasionally, likely TZ98 or TZ4 */
> > +	{ "sys_therm0", 0x02 },
> > +	{ "sys_therm1", 0x03 },
> > +	{ "sys_therm2", 0x04 },
> > +	{ "sys_therm3", 0x05 },
> > +	{ "sys_therm4", 0x06 },
> > +	{ "sys_therm5", 0x07 },
> > +	{ "sys_therm6", 0x08 },
> > +};
> > +
> > +struct dell_xps_ec {
> > +	struct device *dev;
> > +	struct i2c_client *client;
> > +	struct iio_channel *therm_channels[ARRAY_SIZE(dell_xps_ec_therms)];
> 
> This should come from data supplied via the probe .data and compat string.

Good idea, will rework.

> 
> > +	struct delayed_work temp_work;
> > +};
> > +
> > +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;
> > +
> > +	/* Convert milli-Celsius to deci-Celsius (Celsius * 10) */
> 
> milli-Celsius is very strange hyphenation just use the lower-case kernel
> terminology for it millicelsius.
> 
> https://www.kernel.org/doc/Documentation/devicetree/bindings/thermal/thermal.txt

Will fix.

> 
> > +	deci_celsius = milli_celsius / 100;
> > +
> > +	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);
> > +	int val, ret, i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(dell_xps_ec_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",
> > +					    dell_xps_ec_therms[i].name, ret);
> > +			continue;
> 
> Is it really legitimate to continue on in this case ? I'll leave it up
> to you to think about that for yourself, perhaps yes is the answer but
> if term_channels[i] is true and a read of a channel fails - surely
> that's indicative of a bail-out situation not read the next channel
> situation

I think, its still good to continue, since its probably better to have
some channels working than none. It is not failing silently, developer
will be able to see it.

> 
> > +		}
> > +
> > +		ret = dell_xps_ec_send_temp(ec, dell_xps_ec_therms[i].cmd, val);
> > +		if (ret < 0) {
> > +			dev_err_ratelimited(ec->dev,
> > +					    "Failed to send temp for %s: %d\n",
> > +					    dell_xps_ec_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_info_ratelimited(ec->dev, "IRQ triggered! (irq=%d)\n", irq);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static int dell_xps_ec_probe(struct i2c_client *client)
> > +{
> > +	struct device *dev = &client->dev;
> > +	struct dell_xps_ec *ec;
> > +	int ret, i;
> > +
> > +	ec = devm_kzalloc(dev, sizeof(*ec), GFP_KERNEL);
> > +	if (!ec)
> > +		return -ENOMEM;
> > +
> > +	ec->dev = dev;
> > +	ec->client = client;
> > +	i2c_set_clientdata(client, ec);
> > +
> > +	/* Set default thermistor profile */
> > +	ret = i2c_master_send(client, dell_xps_ec_thermistor_profile,
> > +			      sizeof(dell_xps_ec_thermistor_profile));
> > +	if (ret < 0)
> > +		return dev_err_probe(dev, ret, "Failed to set thermistor profile\n");
> > +
> > +	/* Get IIO channels for thermistors */
> > +	for (i = 0; i < ARRAY_SIZE(dell_xps_ec_therms); i++) {
> 
> This should be num_channels passed from your compat .data

Will fix.

> 
> > +		ec->therm_channels[i] =
> > +			devm_iio_channel_get(dev, dell_xps_ec_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",
> > +				 dell_xps_ec_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;
> \n
> 
> > +	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 < 0)
> > +		return dev_err_probe(dev, ret, "Failed to request IRQ\n");
> > +
> > +	return 0;
> 
> You aren't returning the ret.

Good catch, switched to `if (ret)` with `return 0;` in the end instead
to match existing implementations.

> 
> > +}
> > +
> > +/*
> > + * 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);
> > +
> > +	cancel_delayed_work_sync(&ec->temp_work);
> > +
> > +	return dell_xps_ec_suspend_cmd(ec, true);
> > +}
> > +
> > +/*
> > + * 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);
> \n> +	return 0;
> > +}
> > +
> > +static const struct of_device_id dell_xps_ec_of_match[] = {
> > +	{ .compatible = "dell,xps13-9345-ec" },
> > +	{}
> > +};
> > +MODULE_DEVICE_TABLE(of, dell_xps_ec_of_match);
> > +
> > +static const struct i2c_device_id dell_xps_ec_i2c_id[] = {
> > +	{ "dell-xps-ec" },
> 
> A nice .data will allow extension of this to other systems with
> more/less channels.
> 
> Which is why you shouldn't use the ARRAY_SIZE() extents from statics
> declared up top directly.

Indeed, will fix.

Thanks,
Alex

> 
> > +	{}
> > +};
> > +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");
> >
> 
>

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

[Aleksandrs Vinarskis]

Describe embedded controller, its interrupt and required thermal zones.
Add EC's reset GPIO to reserved range, as triggering it during device
operation leads to unrecoverable and unusable state.

Signed-off-by: Aleksandrs Vinarskis <alex@vinarskis.com>
---
 .../boot/dts/qcom/x1e80100-dell-xps13-9345.dts     | 86 +++++++++++++++++++++-
 1 file changed, 84 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 ce7b10ea89b6..8d8d8014049b 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_AMUX3_GPIO_100K_PU(1)>,
+			      <&pmk8550_vadc ADC5_GEN3_AMUX4_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_therm0",
+				   "sys_therm1",
+				   "sys_therm2",
+				   "sys_therm3",
+				   "sys_therm4",
+				   "sys_therm5",
+				   "sys_therm6";
+	};
 };
 
 &i2c7 {
@@ -1025,6 +1049,57 @@ rtmr0_1p8_reg_en: rtmr0-1p8-reg-en-state {
 	};
 };
 
+&pmk8550_vadc {
+	sys_therm0: channel@14c {
+		reg = <ADC5_GEN3_AMUX3_GPIO_100K_PU(1)>;
+		qcom,hw-settle-time = <200>;
+		qcom,ratiometric;
+		label = "lpddr5_therm";
+	};
+
+	sys_therm1: channel@14d {
+		reg = <ADC5_GEN3_AMUX4_GPIO_100K_PU(1)>;
+		qcom,hw-settle-time = <200>;
+		qcom,ratiometric;
+		label = "usb_charger_left_therm";
+	};
+
+	sys_therm2: channel@144 {
+		reg = <ADC5_GEN3_AMUX1_THM_100K_PU(1)>;
+		qcom,hw-settle-time = <200>;
+		qcom,ratiometric;
+		label = "vph_pwr_therm";
+	};
+
+	sys_therm3: channel@145 {
+		reg = <ADC5_GEN3_AMUX2_THM_100K_PU(1)>;
+		qcom,hw-settle-time = <200>;
+		qcom,ratiometric;
+		label = "usb_charger_right_therm";
+	};
+
+	sys_therm4: channel@146 {
+		reg = <ADC5_GEN3_AMUX3_THM_100K_PU(1)>;
+		qcom,hw-settle-time = <200>;
+		qcom,ratiometric;
+		label = "ssd_conn_therm";
+	};
+
+	sys_therm5: channel@147 {
+		reg = <ADC5_GEN3_AMUX4_THM_100K_PU(1)>;
+		qcom,hw-settle-time = <200>;
+		qcom,ratiometric;
+		label = "sys_therm5";
+	};
+
+	sys_therm6: channel@148 {
+		reg = <ADC5_GEN3_AMUX5_THM_100K_PU(1)>;
+		qcom,hw-settle-time = <200>;
+		qcom,ratiometric;
+		label = "keyboard_therm";
+	};
+};
+
 &qupv3_0 {
 	status = "okay";
 };
@@ -1071,6 +1146,7 @@ &smb2360_1_eusb2_repeater {
 
 &tlmm {
 	gpio-reserved-ranges = <44 4>,  /* SPI11 (TPM) */
+			       <65 1>,  /* EC Reset, accessible but yields system unusable */
 			       <76 4>,  /* SPI19 (TZ Protected) */
 			       <238 1>; /* UFS Reset */
 
@@ -1081,6 +1157,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 v3 3/3] arm64: dts: qcom: x1e80100-dell-xps13-9345: introduce EC

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The device tree patch extends and references a non-existent label `pmk8550_vadc`, which will cause a fatal `dtc` compilation error.
- [Low] The patch uses undefined function-like macros (e.g., `ADC5_GEN3_AMUX3_GPIO_100K_PU(1)`), which will cause a syntax error during device tree compilation.
--

--- Patch [3]: [PATCH v3 3/3] arm64: dts: qcom: x1e80100-dell-xps13-9345: introduce EC ---
commit e7703c5dfab63d67ae0d89c48a05547baa5abd7f
Author: Aleksandrs Vinarskis <alex@vinarskis.com>

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

Describe embedded controller, its interrupt and required thermal zones.
Add EC's reset GPIO to reserved range, as triggering it during device
operation leads to unrecoverable and unusable state.

> 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 ce7b10ea89b6d..8d8d8014049bb 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 {
[ ... ]
> +	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_AMUX3_GPIO_100K_PU(1)>,
[Severity: Low]
Does this file include the node definitions and bindings for pmk8550_vadc?

The pmk8550_vadc phandle does not appear to be defined in this file or its
typical includes, which will cause a dtc compilation error when the device
tree is built.

Additionally, is ADC5_GEN3_AMUX3_GPIO_100K_PU intended to be used as a
function-like macro taking an argument here?

The dt-bindings headers usually define this as a parameterless macro.
Supplying an argument like (1) will cause a preprocessor error during the
device tree compilation.

> +			      <&pmk8550_vadc ADC5_GEN3_AMUX4_GPIO_100K_PU(1)>,
[ ... ]
> @@ -1025,6 +1049,57 @@ rtmr0_1p8_reg_en: rtmr0-1p8-reg-en-state {
>  	};
>  };
>  
> +&pmk8550_vadc {
> +	sys_therm0: channel@14c {
> +		reg = <ADC5_GEN3_AMUX3_GPIO_100K_PU(1)>;
[Severity: Low]
Will appending to pmk8550_vadc work if the base node does not exist?

Similar to the io-channels property in the embedded-controller block,
attempting to append properties to an undefined pmk8550_vadc node will
result in a path not found error from the device tree compiler.

The reg assignment also uses the same function-like macro syntax which
will trigger a preprocessor failure.

> +		qcom,hw-settle-time = <200>;
> +		qcom,ratiometric;
> +		label = "lpddr5_therm";
> +	};
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260801-dell-xps-9345-ec-v3-0-9f4bdb5a5dad@vinarskis.com?part=3

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

[Aleksandrs Vinarskis]

On Saturday, August 1st, 2026 at 20:43, sashiko-bot@kernel.org <sashiko-bot@kernel.org> wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Low] The device tree patch extends and references a non-existent label `pmk8550_vadc`, which will cause a fatal `dtc` compilation error.
> - [Low] The patch uses undefined function-like macros (e.g., `ADC5_GEN3_AMUX3_GPIO_100K_PU(1)`), which will cause a syntax error during device tree compilation.
> --
> 
> --- Patch [3]: [PATCH v3 3/3] arm64: dts: qcom: x1e80100-dell-xps13-9345: introduce EC ---
> commit e7703c5dfab63d67ae0d89c48a05547baa5abd7f
> Author: Aleksandrs Vinarskis <alex@vinarskis.com>
> 
> arm64: dts: qcom: x1e80100-dell-xps13-9345: introduce EC
> 
> Describe embedded controller, its interrupt and required thermal zones.
> Add EC's reset GPIO to reserved range, as triggering it during device
> operation leads to unrecoverable and unusable state.
> 
> > 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 ce7b10ea89b6d..8d8d8014049bb 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 {
> [ ... ]
> > +	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_AMUX3_GPIO_100K_PU(1)>,
> [Severity: Low]
> Does this file include the node definitions and bindings for pmk8550_vadc?
> 
> The pmk8550_vadc phandle does not appear to be defined in this file or its
> typical includes, which will cause a dtc compilation error when the device
> tree is built.
> 
> Additionally, is ADC5_GEN3_AMUX3_GPIO_100K_PU intended to be used as a
> function-like macro taking an argument here?
> 
> The dt-bindings headers usually define this as a parameterless macro.
> Supplying an argument like (1) will cause a preprocessor error during the
> device tree compilation.

..

> 
> > +			      <&pmk8550_vadc ADC5_GEN3_AMUX4_GPIO_100K_PU(1)>,
> [ ... ]
> > @@ -1025,6 +1049,57 @@ rtmr0_1p8_reg_en: rtmr0-1p8-reg-en-state {
> >  	};
> >  };
> >
> > +&pmk8550_vadc {
> > +	sys_therm0: channel@14c {
> > +		reg = <ADC5_GEN3_AMUX3_GPIO_100K_PU(1)>;
> [Severity: Low]
> Will appending to pmk8550_vadc work if the base node does not exist?
> 
> Similar to the io-channels property in the embedded-controller block,
> attempting to append properties to an undefined pmk8550_vadc node will
> result in a path not found error from the device tree compiler.
> 
> The reg assignment also uses the same function-like macro syntax which
> will trigger a preprocessor failure.

Seems bot is not using latest linux-next as base. Nodes exist in
next-20260731, this series builds and works.

Alex

> 
> > +		qcom,hw-settle-time = <200>;
> > +		qcom,ratiometric;
> > +		label = "lpddr5_therm";
> > +	};
> [ ... ]
> 
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260801-dell-xps-9345-ec-v3-0-9f4bdb5a5dad@vinarskis.com?part=3
>