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

[PATCH v2 0/4] 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 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 (4):
      dt-bindings: platform: introduce EC for Dell XPS 13 9345
      platform: arm64: dell-xps-ec: new driver
      arm64: dts: qcom: hamoa-pmics: define VADC for pmk8550
      arm64: dts: qcom: x1e80100-dell-xps13-9345: introduce EC

 .../embedded-controller/dell,xps13-9345-ec.yaml    |  91 +++++++
 MAINTAINERS                                        |   6 +
 arch/arm64/boot/dts/qcom/hamoa-pmics.dtsi          |  26 ++
 .../boot/dts/qcom/x1e80100-dell-xps13-9345.dts     |  94 +++++++-
 drivers/platform/arm64/Kconfig                     |  12 +
 drivers/platform/arm64/Makefile                    |   1 +
 drivers/platform/arm64/dell-xps-ec.c               | 267 +++++++++++++++++++++
 7 files changed, 495 insertions(+), 2 deletions(-)
---
base-commit: ec07eff1fd1ed6c4dca399aee4e8da15856589f0
change-id: 20260331-dell-xps-9345-ec-e5f49d1bef61

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

[PATCH v2 1/4] 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 0000000000000000000000000000000000000000..e14dbf2f1a6af8cc7511890fbef08c6c717c0aa6
--- /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 <dt-bindings/iio/qcom,spmi-adc7-pm8350.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 PM8350_ADC7_GPIO3_100K_PU(1)>,
+                          <&pmk8550_vadc PM8350_ADC7_GPIO4_100K_PU(1)>,
+                          <&pmk8550_vadc PM8350_ADC7_AMUX_THM1_100K_PU(1)>,
+                          <&pmk8550_vadc PM8350_ADC7_AMUX_THM2_100K_PU(1)>,
+                          <&pmk8550_vadc PM8350_ADC7_AMUX_THM3_100K_PU(1)>,
+                          <&pmk8550_vadc PM8350_ADC7_AMUX_THM4_100K_PU(1)>,
+                          <&pmk8550_vadc PM8350_ADC7_AMUX_THM5_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 96e0781f2201b41b976dfa69efd44d62c4ff0058..a5d175559f4468dfe363b319a1b08d3425f4d712 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7236,6 +7236,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.53.0

[PATCH v2 2/4] 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 | 267 +++++++++++++++++++++++++++++++++++
 4 files changed, 281 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index a5d175559f4468dfe363b319a1b08d3425f4d712..c150f57b60706224e5b24b0dfb3d8a9b81f36398 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7240,6 +7240,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 10f905d7d6bfa5fad30a0689d3a20481268c781e..0bc8f016032bb05cb3a7cc50bdf1092da04153bc 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 60c131cff6a15bb51a49c9edab95badf513ee0f6..6768dc6c2310837374e67381cfc729bed1fdaaef 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 0000000000000000000000000000000000000000..bf1495fbe473ccdb82b95a66b56e8525f782cc8e
--- /dev/null
+++ b/drivers/platform/arm64/dell-xps-ec.c
@@ -0,0 +1,267 @@
+// 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");

-- 
2.53.0

[PATCH v2 3/4] arm64: dts: qcom: hamoa-pmics: define VADC for pmk8550

[Aleksandrs Vinarskis]

Follow pattern of pmk8350 to add missing pmk8550 VADC to hamoa.
Register address of 0x9000 matches example schema for spmi-adc5-gen3.

Signed-off-by: Aleksandrs Vinarskis <alex@vinarskis.com>
---
 arch/arm64/boot/dts/qcom/hamoa-pmics.dtsi | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/hamoa-pmics.dtsi b/arch/arm64/boot/dts/qcom/hamoa-pmics.dtsi
index 6a31a0adf8be472badea502a916cdbc9477e9f2b..cc69d299bc356d90aa1483f347f5eee43b853e45 100644
--- a/arch/arm64/boot/dts/qcom/hamoa-pmics.dtsi
+++ b/arch/arm64/boot/dts/qcom/hamoa-pmics.dtsi
@@ -218,6 +218,32 @@ pon_resin: resin {
 			};
 		};
 
+		pmk8550_vadc: adc@9000 {
+			compatible = "qcom,spmi-adc5-gen3";
+			reg = <0x9000>, <0x9100>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			interrupts = <0x0 0x90 0x1 IRQ_TYPE_EDGE_RISING>,
+				     <0x0 0x91 0x1 IRQ_TYPE_EDGE_RISING>;
+			#io-channel-cells = <1>;
+			#thermal-sensor-cells = <1>;
+
+			channel@3 {
+				reg = <0x3>;
+				label = "pmk8550_die_temp";
+				qcom,pre-scaling = <1 1>;
+			};
+
+			channel@44 {
+				reg = <0x44>;
+				label = "pmk8550_xo_therm";
+				qcom,pre-scaling = <1 1>;
+				qcom,ratiometric;
+				qcom,hw-settle-time = <200>;
+				qcom,adc-tm;
+			};
+		};
+
 		pmk8550_rtc: rtc@6100 {
 			compatible = "qcom,pmk8350-rtc";
 			reg = <0x6100>, <0x6200>;

-- 
2.53.0

[PATCH v2 4/4] 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     | 94 +++++++++++++++++++++-
 1 file changed, 92 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 ce7b10ea89b6dcb2a4a65c114037f4c90a4b0c6d..fe7e069f0ef56c6fdc3b495dd78dacd1b96c1c95 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts
+++ b/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts
@@ -7,6 +7,7 @@
 /dts-v1/;
 
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/iio/qcom,spmi-adc7-pm8350.h>
 #include <dt-bindings/input/gpio-keys.h>
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/leds/common.h>
@@ -759,8 +760,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 PM8350_ADC7_GPIO3_100K_PU(1)>,
+			      <&pmk8550_vadc PM8350_ADC7_GPIO4_100K_PU(1)>,
+			      <&pmk8550_vadc PM8350_ADC7_AMUX_THM1_100K_PU(1)>,
+			      <&pmk8550_vadc PM8350_ADC7_AMUX_THM2_100K_PU(1)>,
+			      <&pmk8550_vadc PM8350_ADC7_AMUX_THM3_100K_PU(1)>,
+			      <&pmk8550_vadc PM8350_ADC7_AMUX_THM4_100K_PU(1)>,
+			      <&pmk8550_vadc PM8350_ADC7_AMUX_THM5_100K_PU(1)>;
+		io-channel-names = "sys_therm0",
+				   "sys_therm1",
+				   "sys_therm2",
+				   "sys_therm3",
+				   "sys_therm4",
+				   "sys_therm5",
+				   "sys_therm6";
+	};
 };
 
 &i2c7 {
@@ -1025,6 +1050,64 @@ rtmr0_1p8_reg_en: rtmr0-1p8-reg-en-state {
 	};
 };
 
+&pmk8550_vadc {
+	/* Around DRAM */
+	channel@14c {
+		reg = <PM8350_ADC7_GPIO3_100K_PU(1)>;
+		qcom,hw-settle-time = <200>;
+		qcom,ratiometric;
+		label = "sys_therm0";
+	};
+
+	/* Around left Type-C charging controller */
+	channel@14d {
+		reg = <PM8350_ADC7_GPIO4_100K_PU(1)>;
+		qcom,hw-settle-time = <200>;
+		qcom,ratiometric;
+		label = "sys_therm1";
+	};
+
+	/* Around upper-left side of motherboard */
+	channel@144 {
+		reg = <PM8350_ADC7_AMUX_THM1_100K_PU(1)>;
+		qcom,hw-settle-time = <200>;
+		qcom,ratiometric;
+		label = "sys_therm2";
+	};
+
+	/* Around right Type-C charging controller */
+	channel@145 {
+		reg = <PM8350_ADC7_AMUX_THM2_100K_PU(1)>;
+		qcom,hw-settle-time = <200>;
+		qcom,ratiometric;
+		label = "sys_therm3";
+	};
+
+	/* Around SSD connector */
+	channel@146 {
+		reg = <PM8350_ADC7_AMUX_THM3_100K_PU(1)>;
+		qcom,hw-settle-time = <200>;
+		qcom,ratiometric;
+		label = "sys_therm4";
+	};
+
+	/* Around battery charging circuit */
+	channel@147 {
+		reg = <PM8350_ADC7_AMUX_THM4_100K_PU(1)>;
+		qcom,hw-settle-time = <200>;
+		qcom,ratiometric;
+		label = "sys_therm5";
+	};
+
+	/* Around keyboard */
+	channel@148 {
+		reg = <PM8350_ADC7_AMUX_THM5_100K_PU(1)>;
+		qcom,hw-settle-time = <200>;
+		qcom,ratiometric;
+		label = "sys_therm6";
+	};
+};
+
 &qupv3_0 {
 	status = "okay";
 };
@@ -1071,6 +1154,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 +1165,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.53.0

Re: [PATCH v2 3/4] arm64: dts: qcom: hamoa-pmics: define VADC for pmk8550

[Dmitry Baryshkov]

On Sat, Apr 04, 2026 at 02:55:16PM +0200, Aleksandrs Vinarskis wrote:
> Follow pattern of pmk8350 to add missing pmk8550 VADC to hamoa.
> Register address of 0x9000 matches example schema for spmi-adc5-gen3.
> 
> Signed-off-by: Aleksandrs Vinarskis <alex@vinarskis.com>
> ---
>  arch/arm64/boot/dts/qcom/hamoa-pmics.dtsi | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

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

[Dmitry Baryshkov]

On Sat, Apr 04, 2026 at 02:55:17PM +0200, Aleksandrs Vinarskis wrote:
> 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     | 94 +++++++++++++++++++++-
>  1 file changed, 92 insertions(+), 2 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

-- 
With best wishes
Dmitry

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

[Bryan O'Donoghue]

On 04/04/2026 13:55, Aleksandrs Vinarskis wrote:
> 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 0000000000000000000000000000000000000000..e14dbf2f1a6af8cc7511890fbef08c6c717c0aa6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/embedded-controller/dell,xps13-9345-ec.yaml

I believe the part name of this embedded controller is the "mec5200" so 
instead of calling it dell,xps13-9345-ec suggest "dell,mec5200"

> @@ -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

Ditto the compat - name it after the IC not the laptop its a "mec5200" 
or "mec5200-ec" - I suspect the -ec postfix is a tautology the ec bit in 
"mec" probably captures.

> +
> +  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 <dt-bindings/iio/qcom,spmi-adc7-pm8350.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 PM8350_ADC7_GPIO3_100K_PU(1)>,
> +                          <&pmk8550_vadc PM8350_ADC7_GPIO4_100K_PU(1)>,
> +                          <&pmk8550_vadc PM8350_ADC7_AMUX_THM1_100K_PU(1)>,
> +                          <&pmk8550_vadc PM8350_ADC7_AMUX_THM2_100K_PU(1)>,
> +                          <&pmk8550_vadc PM8350_ADC7_AMUX_THM3_100K_PU(1)>,
> +                          <&pmk8550_vadc PM8350_ADC7_AMUX_THM4_100K_PU(1)>,
> +                          <&pmk8550_vadc PM8350_ADC7_AMUX_THM5_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 96e0781f2201b41b976dfa69efd44d62c4ff0058..a5d175559f4468dfe363b319a1b08d3425f4d712 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7236,6 +7236,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
> 

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

[Bryan O'Donoghue]

On 04/04/2026 13:55, Aleksandrs Vinarskis wrote:
> +    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

I agree with the number of io-channels but, having fixed names is I feel 
not correct.

io-channel-names:
   minItems: 7
   maxItems: 7

Does the same thing but lets you name the channels at the source and the 
sync wrt what they do.

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

[Bryan O'Donoghue]

On 04/04/2026 13:55, Aleksandrs Vinarskis wrote:
> +		io-channel-names = "sys_therm0",
> +				   "sys_therm1",
> +				   "sys_therm2",
> +				   "sys_therm3",
> +				   "sys_therm4",
> +				   "sys_therm5",
> +				   "sys_therm6";

io-channels-names = "lpddr5x-therm", "charger-left-therm", 
"charger-right-therm", "ssd-therm", "keyboard-therm"

> +	};
>   };
>   
>   &i2c7 {
> @@ -1025,6 +1050,64 @@ rtmr0_1p8_reg_en: rtmr0-1p8-reg-en-state {
>   	};
>   };
>   
> +&pmk8550_vadc {
> +	/* Around DRAM */
> +	channel@14c {
> +		reg = <PM8350_ADC7_GPIO3_100K_PU(1)>;
> +		qcom,hw-settle-time = <200>;
> +		qcom,ratiometric;
> +		label = "sys_therm0";

You might as well use the same name for the label "lpddr5-therm"

> +	};

On thena the list is:

- "OPT temp"
- "CPU VR"
- "GPU VR"
- "Charging-1"
- "Charging-2"
- "WLAN"
- "WLAN (EE)"

So I think both the source and the sink should describe and be allowed 
to describe what it is io-channel-names = "fixed list" is too restrictive.

Much more useful to userspace to see a string "lpddr5-therm" than 
"sys_therm0".

---
bod

Re: [PATCH v2 2/4] platform: arm64: dell-xps-ec: new driver

[Bryan O'Donoghue]

On 04/04/2026 13:55, 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 | 267 +++++++++++++++++++++++++++++++++++
>   4 files changed, 281 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a5d175559f4468dfe363b319a1b08d3425f4d712..c150f57b60706224e5b24b0dfb3d8a9b81f36398 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7240,6 +7240,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 10f905d7d6bfa5fad30a0689d3a20481268c781e..0bc8f016032bb05cb3a7cc50bdf1092da04153bc 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 60c131cff6a15bb51a49c9edab95badf513ee0f6..6768dc6c2310837374e67381cfc729bed1fdaaef 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 0000000000000000000000000000000000000000..bf1495fbe473ccdb82b95a66b56e8525f782cc8e
> --- /dev/null
> +++ b/drivers/platform/arm64/dell-xps-ec.c
> @@ -0,0 +1,267 @@
> +// 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 },
> +};

You could probably retrieve these strings from the dt if you really need 
them.

I don't think you need static consts in your driver though you could 
just as easily do `sprintf("sys_therm%d\n", i) where you use 
ec_therms[i].name - the name is only used to print errors and you have 
the index of the channel when you do.

It would be nicer to get the strings from DT - certainly make the string 
names mandatory but, then let the DT specify those names.

Either that or just do the sprintf("sys_therm%d\n", i); for the index, 
whichever you wish yourself.

> +
> +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;
\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;
> +}
> +
> +/*
> + * 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");
> 

Re: [PATCH v2 2/4] platform: arm64: dell-xps-ec: new driver

[Aleksandrs Vinarskis]

On Sunday, April 5th, 2026 at 02:29, Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote:

> On 04/04/2026 13:55, 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 | 267 +++++++++++++++++++++++++++++++++++
> >   4 files changed, 281 insertions(+)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index a5d175559f4468dfe363b319a1b08d3425f4d712..c150f57b60706224e5b24b0dfb3d8a9b81f36398 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -7240,6 +7240,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 10f905d7d6bfa5fad30a0689d3a20481268c781e..0bc8f016032bb05cb3a7cc50bdf1092da04153bc 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 60c131cff6a15bb51a49c9edab95badf513ee0f6..6768dc6c2310837374e67381cfc729bed1fdaaef 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 0000000000000000000000000000000000000000..bf1495fbe473ccdb82b95a66b56e8525f782cc8e
> > --- /dev/null
> > +++ b/drivers/platform/arm64/dell-xps-ec.c
> > @@ -0,0 +1,267 @@
> > +// 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 },
> > +};
>
> You could probably retrieve these strings from the dt if you really need
> them.
>
> I don't think you need static consts in your driver though you could
> just as easily do `sprintf("sys_therm%d\n", i) where you use
> ec_therms[i].name - the name is only used to print errors and you have
> the index of the channel when you do.
>
> It would be nicer to get the strings from DT - certainly make the string
> names mandatory but, then let the DT specify those names.
>
> Either that or just do the sprintf("sys_therm%d\n", i); for the index,
> whichever you wish yourself.

Hi Bryan,

Will answer here to all three comments about `sys_thermX`.

The reason I have added them as static consts here, and defined them in
the schema is because the order of the channels matters:
1. On my XPS (UEFI v2.11.0) changes in sys_therm2 immediately result in
   changes in fan speeds. Other channels seemingly have no affect, at
   least when spoofed one by one, implying that EC cares which value
   is which.
2. As I do not know internals of the EC firmware, even if today the other
   thermistor channels ordering is seemingly not relevant, we cannot be
   sure it will not change with EC firmware upgrade.

I have reconstructed the order of channels by comparing i2c data dumps
and real-time temps on Windows, eg. sys_therm0 is sent to EC under id 0x02
and represents the TZ71 (around dram on XPS). There is no other reason to
have the names of the channels in this driver except for enforcing the
channel mapping, so `sprintf("sys_therm%d\n", i)` wouldn't be useful.

By allowing source and sink to define the names and not enforcing it in
schema we lose ability to force the correct order, there is no way of
knowing whether "lpddr5-therm" or "ssd-therm" goes first. By forcing
"sys_thermX" convention, one would need to figure which one is which,
for example by referring to laptop schematics. I assume, "thena"'s
schematics has thermistors labeled as "sys_thermX"?

I do agree that labels of the ADC nodes could be more useful for the
user. So far I followed the example of sc8280xp platforms that define
ADC channels with "sys_thermX". Perhaps, we could separate the
io-channel-names and ADC node labels then? eg:

+ io-channel-names = "sys_therm0",
+ 		     "sys_therm1",
...

+ &pmk8550_vadc {
+	sys_therm0: channel@14c {
+ 		reg = <PM8350_ADC7_GPIO3_100K_PU(1)>;
+ 		qcom,hw-settle-time = <200>;
+ 		qcom,ratiometric;
+ 		label = "lpddr5x-therm";

Though not sure if such approach is 'legal'?

Alex

>
> > +
> > +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;
> \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;
> > +}
> > +
> > +/*
> > + * 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");
> >
>
>

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

[Aleksandrs Vinarskis]

On Sunday, April 5th, 2026 at 02:05, Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote:

> On 04/04/2026 13:55, Aleksandrs Vinarskis wrote:
> > 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 0000000000000000000000000000000000000000..e14dbf2f1a6af8cc7511890fbef08c6c717c0aa6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/embedded-controller/dell,xps13-9345-ec.yaml
>
> I believe the part name of this embedded controller is the "mec5200" so
> instead of calling it dell,xps13-9345-ec suggest "dell,mec5200"

Correct, its Microchip MEC5200. I remember reading some series discussion
about not naming driver after IC its based on, but rather platform its
used for since driver depends on firmware which is platform specific...
cannot find that discussion now.

>
> > @@ -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
>
> Ditto the compat - name it after the IC not the laptop its a "mec5200"
> or "mec5200-ec" - I suspect the -ec postfix is a tautology the ec bit in
> "mec" probably captures.

I'm not sure I agree regarding the compatible, its supposed to be as exact as
possible. "dell,mec5200" will not allow us to differentiate between EC drivers
of "tributo" and "thena" for example.

Suggestion:
- Schema filename to be generalized "dell,mec5200-ec.yaml"
- Driver filename to be generalized "dell-mec5200-ec.c"
- Config to be generalized "EC_DELL_MEC5200"
- Compatible to stay specific "dell,xps13-9345-ec", with fallback to
  "dell,mec5200-ec".

I would also keep "-ec" to stay consistent with naming convention of
existing drivers and bindings.

Let me know if this would work for you.

Alex

>
> > +
> > +  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 <dt-bindings/iio/qcom,spmi-adc7-pm8350.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 PM8350_ADC7_GPIO3_100K_PU(1)>,
> > +                          <&pmk8550_vadc PM8350_ADC7_GPIO4_100K_PU(1)>,
> > +                          <&pmk8550_vadc PM8350_ADC7_AMUX_THM1_100K_PU(1)>,
> > +                          <&pmk8550_vadc PM8350_ADC7_AMUX_THM2_100K_PU(1)>,
> > +                          <&pmk8550_vadc PM8350_ADC7_AMUX_THM3_100K_PU(1)>,
> > +                          <&pmk8550_vadc PM8350_ADC7_AMUX_THM4_100K_PU(1)>,
> > +                          <&pmk8550_vadc PM8350_ADC7_AMUX_THM5_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 96e0781f2201b41b976dfa69efd44d62c4ff0058..a5d175559f4468dfe363b319a1b08d3425f4d712 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -7236,6 +7236,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
> >
>
>

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

[Bryan O'Donoghue]

On 05/04/2026 21:50, Aleksandrs Vinarskis wrote:
> On Sunday, April 5th, 2026 at 02:05, Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote:
> 
>> On 04/04/2026 13:55, Aleksandrs Vinarskis wrote:
>>> 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 0000000000000000000000000000000000000000..e14dbf2f1a6af8cc7511890fbef08c6c717c0aa6
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/embedded-controller/dell,xps13-9345-ec.yaml
>>
>> I believe the part name of this embedded controller is the "mec5200" so
>> instead of calling it dell,xps13-9345-ec suggest "dell,mec5200"
> 
> Correct, its Microchip MEC5200. I remember reading some series discussion
> about not naming driver after IC its based on, but rather platform its
> used for since driver depends on firmware which is platform specific...
> cannot find that discussion now.
> 
>>
>>> @@ -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
>>
>> Ditto the compat - name it after the IC not the laptop its a "mec5200"
>> or "mec5200-ec" - I suspect the -ec postfix is a tautology the ec bit in
>> "mec" probably captures.
> 
> I'm not sure I agree regarding the compatible, its supposed to be as exact as
> possible. "dell,mec5200" will not allow us to differentiate between EC drivers
> of "tributo" and "thena" for example.
> 
> Suggestion:
> - Schema filename to be generalized "dell,mec5200-ec.yaml"
> - Driver filename to be generalized "dell-mec5200-ec.c"
> - Config to be generalized "EC_DELL_MEC5200"
> - Compatible to stay specific "dell,xps13-9345-ec", with fallback to
>    "dell,mec5200-ec".
> 
> I would also keep "-ec" to stay consistent with naming convention of
> existing drivers and bindings.
> 
> Let me know if this would work for you.
> 
> Alex

To me including the laptop model in the IC name, when our best 
information is that this same chip is used on both Thena variants isn't 
very logical.

The thing is the IC not the platform it resides on.

But if you think the firmware is specific to each platform - something 
like dell-mec5200-ec, dell,mec5200-xps13-9345-ec makes sense to me.

> 
>>
>>> +
>>> +  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 <dt-bindings/iio/qcom,spmi-adc7-pm8350.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 PM8350_ADC7_GPIO3_100K_PU(1)>,
>>> +                          <&pmk8550_vadc PM8350_ADC7_GPIO4_100K_PU(1)>,
>>> +                          <&pmk8550_vadc PM8350_ADC7_AMUX_THM1_100K_PU(1)>,
>>> +                          <&pmk8550_vadc PM8350_ADC7_AMUX_THM2_100K_PU(1)>,
>>> +                          <&pmk8550_vadc PM8350_ADC7_AMUX_THM3_100K_PU(1)>,
>>> +                          <&pmk8550_vadc PM8350_ADC7_AMUX_THM4_100K_PU(1)>,
>>> +                          <&pmk8550_vadc PM8350_ADC7_AMUX_THM5_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 96e0781f2201b41b976dfa69efd44d62c4ff0058..a5d175559f4468dfe363b319a1b08d3425f4d712 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -7236,6 +7236,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
>>>
>>
>>

Re: [PATCH v2 2/4] platform: arm64: dell-xps-ec: new driver

[Bryan O'Donoghue]

On 05/04/2026 21:48, Aleksandrs Vinarskis wrote:
> On Sunday, April 5th, 2026 at 02:29, Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote:
> 
>> On 04/04/2026 13:55, 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 | 267 +++++++++++++++++++++++++++++++++++
>>>    4 files changed, 281 insertions(+)
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index a5d175559f4468dfe363b319a1b08d3425f4d712..c150f57b60706224e5b24b0dfb3d8a9b81f36398 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -7240,6 +7240,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 10f905d7d6bfa5fad30a0689d3a20481268c781e..0bc8f016032bb05cb3a7cc50bdf1092da04153bc 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 60c131cff6a15bb51a49c9edab95badf513ee0f6..6768dc6c2310837374e67381cfc729bed1fdaaef 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 0000000000000000000000000000000000000000..bf1495fbe473ccdb82b95a66b56e8525f782cc8e
>>> --- /dev/null
>>> +++ b/drivers/platform/arm64/dell-xps-ec.c
>>> @@ -0,0 +1,267 @@
>>> +// 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 },
>>> +};
>>
>> You could probably retrieve these strings from the dt if you really need
>> them.
>>
>> I don't think you need static consts in your driver though you could
>> just as easily do `sprintf("sys_therm%d\n", i) where you use
>> ec_therms[i].name - the name is only used to print errors and you have
>> the index of the channel when you do.
>>
>> It would be nicer to get the strings from DT - certainly make the string
>> names mandatory but, then let the DT specify those names.
>>
>> Either that or just do the sprintf("sys_therm%d\n", i); for the index,
>> whichever you wish yourself.
> 
> Hi Bryan,
> 
> Will answer here to all three comments about `sys_thermX`.
> 
> The reason I have added them as static consts here, and defined them in
> the schema is because the order of the channels matters:

Two different things.

You have an array of strings here which you only use to print two error 
messages, you always know the index, so you don't need those strings.

At the same time, you could read the assigned names in the DT if you 
_really_ care to print the names.

You don't match the static names in the .c file to the .dts so the 
consts here serve no purpose.

Ideally you'd read the names out of DT - which is what I recommend. You 
then print the name you read from the DT via an index.

> 1. On my XPS (UEFI v2.11.0) changes in sys_therm2 immediately result in
>     changes in fan speeds. Other channels seemingly have no affect, at
>     least when spoofed one by one, implying that EC cares which value
>     is which.
> 2. As I do not know internals of the EC firmware, even if today the other
>     thermistor channels ordering is seemingly not relevant, we cannot be
>     sure it will not change with EC firmware upgrade.

But the PCB maps certain ADC channels to certain thermistors the lpddr5 
thermistor always maps to the lpddr5 thermistor and adc input 0 on the 
EC, that won't change with a firmware upgrade, the PCB is the PCB.

> I have reconstructed the order of channels by comparing i2c data dumps
> and real-time temps on Windows, eg. sys_therm0 is sent to EC under id 0x02
> and represents the TZ71 (around dram on XPS). There is no other reason to
> have the names of the channels in this driver except for enforcing the
> channel mapping, so `sprintf("sys_therm%d\n", i)` wouldn't be useful.
> 
> By allowing source and sink to define the names and not enforcing it in
> schema we lose ability to force the correct order, there is no way of
> knowing whether "lpddr5-therm" or "ssd-therm" goes first. 

But there is, by looking at the schematics. You have the correct order 
now and it maps to what the schematics say, I checked.

By forcing
> "sys_thermX" convention, one would need to figure which one is which,
> for example by referring to laptop schematics. I assume, "thena"'s
> schematics has thermistors labeled as "sys_thermX"?
> 
> I do agree that labels of the ADC nodes could be more useful for the
> user. So far I followed the example of sc8280xp platforms that define
> ADC channels with "sys_thermX". Perhaps, we could separate the
> io-channel-names and ADC node labels then? eg:

I mean it doesn't matter a whole lot to me how the source and the sink 
connect in DT.

BTW if we denoted the source and the sink lpddr5 today and the firmware 
was changed the name of the mapping would be wrong but the functional 
conneciton would still be correct.

So I'm not very concerned about source/sink names, its a bike shed

> + io-channel-names = "sys_therm0",
> + 		     "sys_therm1",
> ...
> 
> + &pmk8550_vadc {
> +	sys_therm0: channel@14c {
> + 		reg = <PM8350_ADC7_GPIO3_100K_PU(1)>;
> + 		qcom,hw-settle-time = <200>;
> + 		qcom,ratiometric;
> + 		label = "lpddr5x-therm";
> 
> Though not sure if such approach is 'legal'?

I mean the important thing is what user-space "sees" right ? I believe 
the label here is that but, please check.

A list of sys_therm0 entries in the dropdown from waybar is 100% meh, 
but if I can see "CPU temp" spike I suddenly have useful information.


> 
> Alex
> 
>>
>>> +
>>> +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;
>> \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;
>>> +}
>>> +
>>> +/*
>>> + * 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");
>>>
>>
>>

Re: [PATCH v2 2/4] platform: arm64: dell-xps-ec: new driver

[Bjorn Andersson]

On Sun, Apr 05, 2026 at 08:48:25PM +0000, Aleksandrs Vinarskis wrote:
> On Sunday, April 5th, 2026 at 02:29, Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote:
> 
> > On 04/04/2026 13:55, 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 | 267 +++++++++++++++++++++++++++++++++++
> > >   4 files changed, 281 insertions(+)
> > >
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index a5d175559f4468dfe363b319a1b08d3425f4d712..c150f57b60706224e5b24b0dfb3d8a9b81f36398 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -7240,6 +7240,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 10f905d7d6bfa5fad30a0689d3a20481268c781e..0bc8f016032bb05cb3a7cc50bdf1092da04153bc 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 60c131cff6a15bb51a49c9edab95badf513ee0f6..6768dc6c2310837374e67381cfc729bed1fdaaef 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 0000000000000000000000000000000000000000..bf1495fbe473ccdb82b95a66b56e8525f782cc8e
> > > --- /dev/null
> > > +++ b/drivers/platform/arm64/dell-xps-ec.c
> > > @@ -0,0 +1,267 @@
> > > +// 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 },
> > > +};
> >
> > You could probably retrieve these strings from the dt if you really need
> > them.
> >
> > I don't think you need static consts in your driver though you could
> > just as easily do `sprintf("sys_therm%d\n", i) where you use
> > ec_therms[i].name - the name is only used to print errors and you have
> > the index of the channel when you do.
> >
> > It would be nicer to get the strings from DT - certainly make the string
> > names mandatory but, then let the DT specify those names.
> >
> > Either that or just do the sprintf("sys_therm%d\n", i); for the index,
> > whichever you wish yourself.
> 
> Hi Bryan,
> 
> Will answer here to all three comments about `sys_thermX`.
> 
> The reason I have added them as static consts here, and defined them in
> the schema is because the order of the channels matters:
> 1. On my XPS (UEFI v2.11.0) changes in sys_therm2 immediately result in
>    changes in fan speeds. Other channels seemingly have no affect, at
>    least when spoofed one by one, implying that EC cares which value
>    is which.
> 2. As I do not know internals of the EC firmware, even if today the other
>    thermistor channels ordering is seemingly not relevant, we cannot be
>    sure it will not change with EC firmware upgrade.
> 
> I have reconstructed the order of channels by comparing i2c data dumps
> and real-time temps on Windows, eg. sys_therm0 is sent to EC under id 0x02
> and represents the TZ71 (around dram on XPS). There is no other reason to
> have the names of the channels in this driver except for enforcing the
> channel mapping, so `sprintf("sys_therm%d\n", i)` wouldn't be useful.
> 
> By allowing source and sink to define the names and not enforcing it in
> schema we lose ability to force the correct order, there is no way of
> knowing whether "lpddr5-therm" or "ssd-therm" goes first. By forcing
> "sys_thermX" convention, one would need to figure which one is which,
> for example by referring to laptop schematics. I assume, "thena"'s
> schematics has thermistors labeled as "sys_thermX"?
> 
> I do agree that labels of the ADC nodes could be more useful for the
> user. So far I followed the example of sc8280xp platforms that define
> ADC channels with "sys_thermX". Perhaps, we could separate the
> io-channel-names and ADC node labels then? eg:
> 

The general guidance for such naming questions is to follow naming from
the schematics, whenever available.

> + io-channel-names = "sys_therm0",
> + 		     "sys_therm1",
> ...
> 
> + &pmk8550_vadc {
> +	sys_therm0: channel@14c {
> + 		reg = <PM8350_ADC7_GPIO3_100K_PU(1)>;
> + 		qcom,hw-settle-time = <200>;
> + 		qcom,ratiometric;
> + 		label = "lpddr5x-therm";
> 
> Though not sure if such approach is 'legal'?

I might be missing something, but that does look legal. Your node's
label follows (what I assume to be) the naming in the schematics and you
provide a human-friendly label.


PS. Once we have these adc channels in place, I presume there's also a
TM that would allow us to wire them up as cooling-maps, to throttle the
CPUs? Similar to "skin-temp-thermal" in x13s.

Regards,
Bjorn