* [PATCH v6 0/3] Introduce EC driver for Snapdragon X1E based Dell XPS 13 9345
@ 2026-10-11 17:50 Aleksandrs Vinarskis
2026-10-11 17:50 ` [PATCH v6 1/3] dt-bindings: platform: introduce EC for " Aleksandrs Vinarskis
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Aleksandrs Vinarskis @ 2026-10-11 17:50 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Hans de Goede, Ilpo Järvinen,
Bryan O'Donoghue, Kees Cook, Gustavo A. R. Silva, Abel Vesa
Cc: Liviu Nicoara, linux-arm-msm, devicetree, linux-kernel,
platform-driver-x86, Abel Vesa, Tobias Heider, Val Packett,
Neil Armstrong, Stephan Gerhold, linux-hardening,
Krzysztof Kozlowski, Dmitry Baryshkov, Konrad Dybcio
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 v6:
- Fix dts mistake due to rebase
- Link to v5: https://lore.kernel.org/r/20261011-dell-xps-9345-ec-v5-0-2ab1258b84c1@vinarskis.com
Changes in v5:
- Rebase on latest linux-next
- Re-schedule work if suspend fails as per Ilpo
- Fix comments as per Ilpo
- Link to v4: https://lore.kernel.org/r/20260809-dell-xps-9345-ec-v4-0-88306dbbc7aa@vinarskis.com
Changes in v4:
- Change to .data struct provided via compatible, which contains expected
thermistor arrangement and magic init sequence to make it easier to
support new systems as per Bryan
- Change milicelsius spelling, add newline, return as per Bryan
- Link to v3: https://lore.kernel.org/r/20260801-dell-xps-9345-ec-v3-0-9f4bdb5a5dad@vinarskis.com
Changes in v3:
- Rebase on latest linux-next
- Add Neil, Stephan to CC, as they worked on ADC for PMK8550
- Drop VADC for pmk8550 as it was already merged upstream
- Rename PMK8550 registers to use newly upstreamed headers for ADC5_GEN3
in both dt-bindings, device-tree patches
- Rename ADC labels to have meaningful for user names as per Bryan
- Drop Dmitry's RB from last patch since it was almost completely changed
now
- Link to v2: https://lore.kernel.org/r/20260404-dell-xps-9345-ec-v2-0-c977c3caa81f@vinarskis.com
Changes in v2:
- Update cover letter to indicate dependency on QCOM ADC series, only
relevant for backporting
- Update description of dell_xps_ec_suspend: entering suspend does not
necessarily ramp down the fans, if thermistors still report high temps
- Add die_temp/xo_therm to pmk8550 as per Konrad Dybcio
- Add missing header imports as per Ilpo Järvinen
- Add explanation for EC reset pin being reserved
- Fix device-tree: minor issues as per Konrad Dybcio
- Fix device-tree: alingment issues as per Konrad Dybcio
- Fix driver: alingment issues as per Bjorn Andersson
- Fix driver: handle temp value as 16bit register as per Ilpo Järvinen
- Fix bindigs: description and example as per Krzysztof Kozlowski
- Link to v1: https://lore.kernel.org/r/20260401-dell-xps-9345-ec-v1-0-afa5cacd49be@vinarskis.com
---
Aleksandrs Vinarskis (3):
dt-bindings: platform: introduce EC for Dell XPS 13 9345
platform: arm64: dell-xps-ec: new driver
arm64: dts: qcom: x1e80100-dell-xps13-9345: introduce EC
.../embedded-controller/dell,xps13-9345-ec.yaml | 91 ++++++
MAINTAINERS | 6 +
.../boot/dts/qcom/x1e80100-dell-xps13-9345.dts | 85 +++++-
drivers/platform/arm64/Kconfig | 12 +
drivers/platform/arm64/Makefile | 1 +
drivers/platform/arm64/dell-xps-ec.c | 308 +++++++++++++++++++++
6 files changed, 501 insertions(+), 2 deletions(-)
---
base-commit: c74ef2fbf60ff7e7b956c023a881e87502c1852a
change-id: 20260331-dell-xps-9345-ec-e5f49d1bef61
Best regards,
--
Aleksandrs Vinarskis <alex@vinarskis.com>
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v6 1/3] dt-bindings: platform: introduce EC for Dell XPS 13 9345 2026-10-11 17:50 [PATCH v6 0/3] Introduce EC driver for Snapdragon X1E based Dell XPS 13 9345 Aleksandrs Vinarskis @ 2026-10-11 17:50 ` Aleksandrs Vinarskis 2026-10-11 17:59 ` sashiko-bot 2026-10-15 3:07 ` Rob Herring 2026-10-11 17:50 ` [PATCH v6 2/3] platform: arm64: dell-xps-ec: new driver Aleksandrs Vinarskis ` (2 subsequent siblings) 3 siblings, 2 replies; 12+ messages in thread From: Aleksandrs Vinarskis @ 2026-10-11 17:50 UTC (permalink / raw) To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Hans de Goede, Ilpo Järvinen, Bryan O'Donoghue, Kees Cook, Gustavo A. R. Silva, Abel Vesa Cc: Liviu Nicoara, linux-arm-msm, devicetree, linux-kernel, platform-driver-x86, Abel Vesa, Tobias Heider, Val Packett, Neil Armstrong, Stephan Gerhold, linux-hardening, Krzysztof Kozlowski Add bindings for Embedded Controller (EC) in Dell XPS 13 9345 (platform codename 'tributo'). It may be partially or fully compatible with EC found in Snapdragon-based Dell Latitude, Inspiron ('thena'). Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com> Signed-off-by: Aleksandrs Vinarskis <alex@vinarskis.com> --- .../embedded-controller/dell,xps13-9345-ec.yaml | 91 ++++++++++++++++++++++ MAINTAINERS | 5 ++ 2 files changed, 96 insertions(+) diff --git a/Documentation/devicetree/bindings/embedded-controller/dell,xps13-9345-ec.yaml b/Documentation/devicetree/bindings/embedded-controller/dell,xps13-9345-ec.yaml new file mode 100644 index 000000000000..3485117b505c --- /dev/null +++ b/Documentation/devicetree/bindings/embedded-controller/dell,xps13-9345-ec.yaml @@ -0,0 +1,91 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/embedded-controller/dell,xps13-9345-ec.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Dell XPS 13 9345 Embedded Controller + +maintainers: + - Aleksandrs Vinarskis <alex@vinarskis.com> + +description: + The Dell XPS 13 9345 has an Embedded Controller (EC) which handles thermal + and power management. It is communicating with SoC over multiple i2c busses. + Among other things, it handles fan speed control, thermal shutdown, peripheral + power supply including trackpad, touch-row, display. For these functions, it + requires frequently updated thermal readings from onboard thermistors. + +properties: + compatible: + const: dell,xps13-9345-ec + + reg: + const: 0x3b + + interrupts: + maxItems: 1 + + io-channels: + description: + ADC channels connected to the 7 onboard thermistors on PMK8550. + EC requires frequent thermal readings of these channels to perform + automated fan speed control. + items: + - description: ADC channel for sys_therm0 + - description: ADC channel for sys_therm1 + - description: ADC channel for sys_therm2 + - description: ADC channel for sys_therm3 + - description: ADC channel for sys_therm4 + - description: ADC channel for sys_therm5 + - description: ADC channel for sys_therm6 + + io-channel-names: + items: + - const: sys_therm0 + - const: sys_therm1 + - const: sys_therm2 + - const: sys_therm3 + - const: sys_therm4 + - const: sys_therm5 + - const: sys_therm6 + +required: + - compatible + - reg + - interrupts + - io-channels + - io-channel-names + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + #include <arm64/qcom/qcom-adc5-gen3.h> + i2c { + #address-cells = <1>; + #size-cells = <0>; + + embedded-controller@3b { + compatible = "dell,xps13-9345-ec"; + reg = <0x3b>; + interrupts-extended = <&tlmm 66 IRQ_TYPE_LEVEL_LOW>; + + io-channels = <&pmk8550_vadc ADC5_GEN3_AMUX3_GPIO_100K_PU(1)>, + <&pmk8550_vadc ADC5_GEN3_AMUX4_GPIO_100K_PU(1)>, + <&pmk8550_vadc ADC5_GEN3_AMUX1_THM_100K_PU(1)>, + <&pmk8550_vadc ADC5_GEN3_AMUX2_THM_100K_PU(1)>, + <&pmk8550_vadc ADC5_GEN3_AMUX3_THM_100K_PU(1)>, + <&pmk8550_vadc ADC5_GEN3_AMUX4_THM_100K_PU(1)>, + <&pmk8550_vadc ADC5_GEN3_AMUX5_THM_100K_PU(1)>; + io-channel-names = "sys_therm0", + "sys_therm1", + "sys_therm2", + "sys_therm3", + "sys_therm4", + "sys_therm5", + "sys_therm6"; + }; + }; +... diff --git a/MAINTAINERS b/MAINTAINERS index 252b6d7ee163..1aa035cc1c5b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7465,6 +7465,11 @@ S: Maintained F: Documentation/ABI/testing/sysfs-class-firmware-attributes F: drivers/platform/x86/dell/dell-wmi-sysman/ +DELL XPS EMBEDDED CONTROLLER DRIVER +M: Aleksandrs Vinarskis <alex@vinarskis.com> +S: Maintained +F: Documentation/devicetree/bindings/embedded-controller/dell,xps13-9345-ec.yaml + DELTA AHE-50DC FAN CONTROL MODULE DRIVER M: Zev Weiss <zev@bewilderbeest.net> L: linux-hwmon@vger.kernel.org -- 2.54.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v6 1/3] dt-bindings: platform: introduce EC for Dell XPS 13 9345 2026-10-11 17:50 ` [PATCH v6 1/3] dt-bindings: platform: introduce EC for " Aleksandrs Vinarskis @ 2026-10-11 17:59 ` sashiko-bot 2026-10-15 3:07 ` Rob Herring 1 sibling, 0 replies; 12+ messages in thread From: sashiko-bot @ 2026-10-11 17:59 UTC (permalink / raw) To: Aleksandrs Vinarskis; +Cc: robh, conor+dt, devicetree Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] dt-bindings: arch-specific include in schema example breaks external tools -- commit 6a2d14c54cd528a2a30745160408a04834bb3011 Author: Aleksandrs Vinarskis <alex@vinarskis.com> dt-bindings: platform: introduce EC for Dell XPS 13 9345 This commit adds device tree bindings for the Embedded Controller found in the Dell XPS 13 9345. It defines the required properties for communication over i2c, including thermal channel mappings. > diff --git a/Documentation/devicetree/bindings/embedded-controller/dell,xps13-9345-ec.yaml b/Documentation/devicetree/bindings/embedded-controller/dell,xps13-9345-ec.yaml > new file mode 100644 > index 0000000000000..3485117b505cc > --- /dev/null > +++ b/Documentation/devicetree/bindings/embedded-controller/dell,xps13-9345-ec.yaml [ ... ] > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/irq.h> > + #include <arm64/qcom/qcom-adc5-gen3.h> [Severity: Low] Will this architecture-specific include break external device tree validation tools? Out-of-tree schema validation tools or downstream projects using these bindings (like U-Boot or Zephyr) will typically fail to compile the example because they lack the Linux-specific arch/arm64/boot/dts/ tree and the associated Makefile symlinks. A search of Documentation/devicetree/bindings/ shows that other schema examples strictly rely on <dt-bindings/...> headers instead of architecture-specific paths. Could this be adjusted to avoid the architecture-specific include? > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + embedded-controller@3b { > + compatible = "dell,xps13-9345-ec"; -- Sashiko AI review · https://sashiko.dev/#/patchset/20261011-dell-xps-9345-ec-v6-0-2a66a364fe70@vinarskis.com?part=1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v6 1/3] dt-bindings: platform: introduce EC for Dell XPS 13 9345 2026-10-11 17:50 ` [PATCH v6 1/3] dt-bindings: platform: introduce EC for " Aleksandrs Vinarskis 2026-10-11 17:59 ` sashiko-bot @ 2026-10-15 3:07 ` Rob Herring 1 sibling, 0 replies; 12+ messages in thread From: Rob Herring @ 2026-10-15 3:07 UTC (permalink / raw) To: Aleksandrs Vinarskis Cc: Bjorn Andersson, Konrad Dybcio, Krzysztof Kozlowski, Conor Dooley, Hans de Goede, Ilpo Järvinen, Bryan O'Donoghue, Kees Cook, Gustavo A. R. Silva, Abel Vesa, Liviu Nicoara, linux-arm-msm, devicetree, linux-kernel, platform-driver-x86, Abel Vesa, Tobias Heider, Val Packett, Neil Armstrong, Stephan Gerhold, linux-hardening, Krzysztof Kozlowski On Sun, Oct 11, 2026 at 07:50:08PM +0200, 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(+) Please address sashiko comment. ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v6 2/3] platform: arm64: dell-xps-ec: new driver 2026-10-11 17:50 [PATCH v6 0/3] Introduce EC driver for Snapdragon X1E based Dell XPS 13 9345 Aleksandrs Vinarskis 2026-10-11 17:50 ` [PATCH v6 1/3] dt-bindings: platform: introduce EC for " Aleksandrs Vinarskis @ 2026-10-11 17:50 ` Aleksandrs Vinarskis 2026-10-11 17:59 ` sashiko-bot 2026-10-11 17:50 ` [PATCH v6 3/3] arm64: dts: qcom: x1e80100-dell-xps13-9345: introduce EC Aleksandrs Vinarskis 2026-10-11 21:35 ` [PATCH v6 0/3] Introduce EC driver for Snapdragon X1E based Dell XPS 13 9345 Liviu Nicoara 3 siblings, 1 reply; 12+ messages in thread From: Aleksandrs Vinarskis @ 2026-10-11 17:50 UTC (permalink / raw) To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Hans de Goede, Ilpo Järvinen, Bryan O'Donoghue, Kees Cook, Gustavo A. R. Silva, Abel Vesa Cc: Liviu Nicoara, linux-arm-msm, devicetree, linux-kernel, platform-driver-x86, Abel Vesa, Tobias Heider, Val Packett, Neil Armstrong, Stephan Gerhold, linux-hardening Introduce EC driver for Dell XPS 13 9345 (codename 'tributo') which may partially of fully compatible with Snapdragon-based Dell Latitude, Inspiron ('thena'). Primary function of this driver is unblock EC's thermal management, specifically to provide it with necessary information to control device fans, peripherals power. The driver was developed primarily by analyzing ACPI DSDT's _DSM and i2c dumps of communication between SoC and EC. Changes to Windows driver's behavior include increasing temperature feed loop from ~50ms to 100ms here. While Xps's EC is rather complex and controls practically all device peripherals including touch row's brightness and special keys such as mic mute, these do not go over this particular i2c interface. Not yet implemented features: - On lid-close IRQ event is registered. Windows performs what to appears to be thermistor constants readout, though its not obvious what it used for. - According to ACPI's _DSM there is a method to readout fans' RPM. - Initial thermistor constants were sniffed from Windows, these can be likely fine tuned for better cooling performance. - There is additional temperature reading that Windows sents to EC but more rare than others, likely SoC T_j / TZ98 or TZ4. This is the only thermal zone who's reading can exceed 115C without triggering thermal shutdown. - Given similarities between 'tributo' and 'thena' platforms, including EC i2c address, driver can be potentially extended to support both. Signed-off-by: Aleksandrs Vinarskis <alex@vinarskis.com> --- MAINTAINERS | 1 + drivers/platform/arm64/Kconfig | 12 ++ drivers/platform/arm64/Makefile | 1 + drivers/platform/arm64/dell-xps-ec.c | 308 +++++++++++++++++++++++++++++++++++ 4 files changed, 322 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 1aa035cc1c5b..29149a1b6ec9 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7469,6 +7469,7 @@ DELL XPS EMBEDDED CONTROLLER DRIVER M: Aleksandrs Vinarskis <alex@vinarskis.com> S: Maintained F: Documentation/devicetree/bindings/embedded-controller/dell,xps13-9345-ec.yaml +F: drivers/platform/arm64/dell-xps-ec.c DELTA AHE-50DC FAN CONTROL MODULE DRIVER M: Zev Weiss <zev@bewilderbeest.net> diff --git a/drivers/platform/arm64/Kconfig b/drivers/platform/arm64/Kconfig index e32e01b2a9bd..29416f8d7232 100644 --- a/drivers/platform/arm64/Kconfig +++ b/drivers/platform/arm64/Kconfig @@ -33,6 +33,18 @@ config EC_ACER_ASPIRE1 laptop where this information is not properly exposed via the standard ACPI devices. +config EC_DELL_XPS + tristate "Dell XPS 9345 Embedded Controller driver" + depends on ARCH_QCOM || COMPILE_TEST + depends on I2C + depends on IIO + help + Driver for the Embedded Controller in the Qualcomm Snapdragon-based + Dell XPS 13 9345, which handles thermal management and fan speed + control. + + Say M or Y here to include this support. + config EC_HUAWEI_GAOKUN tristate "Huawei Matebook E Go Embedded Controller driver" depends on ARCH_QCOM || COMPILE_TEST diff --git a/drivers/platform/arm64/Makefile b/drivers/platform/arm64/Makefile index 7681be4a46e9..669dc9e79afb 100644 --- a/drivers/platform/arm64/Makefile +++ b/drivers/platform/arm64/Makefile @@ -6,6 +6,7 @@ # obj-$(CONFIG_EC_ACER_ASPIRE1) += acer-aspire1-ec.o +obj-$(CONFIG_EC_DELL_XPS) += dell-xps-ec.o obj-$(CONFIG_EC_HUAWEI_GAOKUN) += huawei-gaokun-ec.o obj-$(CONFIG_EC_LENOVO_YOGA_C630) += lenovo-yoga-c630.o obj-$(CONFIG_EC_LENOVO_THINKPAD_T14S) += lenovo-thinkpad-t14s.o diff --git a/drivers/platform/arm64/dell-xps-ec.c b/drivers/platform/arm64/dell-xps-ec.c new file mode 100644 index 000000000000..cf2dd41b1aa6 --- /dev/null +++ b/drivers/platform/arm64/dell-xps-ec.c @@ -0,0 +1,308 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2026, Aleksandrs Vinarskis <alex@vinarskis.com> + */ + +#include <linux/array_size.h> +#include <linux/container_of.h> +#include <linux/dev_printk.h> +#include <linux/device.h> +#include <linux/devm-helpers.h> +#include <linux/err.h> +#include <linux/i2c.h> +#include <linux/iio/consumer.h> +#include <linux/interrupt.h> +#include <linux/jiffies.h> +#include <linux/module.h> +#include <linux/overflow.h> +#include <linux/pm.h> +#include <linux/types.h> +#include <linux/unaligned.h> +#include <linux/units.h> +#include <linux/workqueue.h> + +#define DELL_XPS_EC_SUSPEND_CMD 0xb9 +#define DELL_XPS_EC_SUSPEND_MSG_LEN 64 + +#define DELL_XPS_EC_TEMP_CMD0 0xfb +#define DELL_XPS_EC_TEMP_CMD1 0x20 +#define DELL_XPS_EC_TEMP_CMD3 0x02 +#define DELL_XPS_EC_TEMP_MSG_LEN 6 +#define DELL_XPS_EC_TEMP_POLL_JIFFIES msecs_to_jiffies(100) + +/* + * Mapping between IIO channel name (as per schematics) and EC command byte. + */ +struct dell_xps_ec_therm { + const char *name; + u8 cmd; +}; + +struct dell_xps_ec_data { + const struct dell_xps_ec_therm *therms; + unsigned int num_therms; + const u8 *profile; + size_t profile_len; +}; + +/* + * Format: + * - header/unknown (2 bytes) + * - per-thermistor entries (3 bytes): thermistor_id, param1, param2 + */ +static const u8 dell_xps13_9345_thermistor_profile[] = { + 0xff, 0x54, + 0x01, 0x00, 0x2b, /* sys_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 */ +}; + +static const struct dell_xps_ec_therm dell_xps13_9345_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 }, +}; + +static const struct dell_xps_ec_data dell_xps13_9345_data = { + .therms = dell_xps13_9345_therms, + .num_therms = ARRAY_SIZE(dell_xps13_9345_therms), + .profile = dell_xps13_9345_thermistor_profile, + .profile_len = sizeof(dell_xps13_9345_thermistor_profile), +}; + +struct dell_xps_ec { + struct device *dev; + struct i2c_client *client; + const struct dell_xps_ec_data *data; + struct delayed_work temp_work; + unsigned int num_therms; + struct iio_channel *therm_channels[] __counted_by(num_therms); +}; + +static int dell_xps_ec_suspend_cmd(struct dell_xps_ec *ec, bool suspend) +{ + u8 buf[DELL_XPS_EC_SUSPEND_MSG_LEN] = {}; + int ret; + + buf[0] = DELL_XPS_EC_SUSPEND_CMD; + buf[1] = suspend ? 0x01 : 0x00; + /* bytes 2..63 remain zero */ + + ret = i2c_master_send(ec->client, buf, sizeof(buf)); + if (ret < 0) + return ret; + + return 0; +} + +static int dell_xps_ec_send_temp(struct dell_xps_ec *ec, u8 cmd_byte, + int milli_celsius) +{ + u8 buf[DELL_XPS_EC_TEMP_MSG_LEN]; + u16 deci_celsius; + int ret; + + deci_celsius = milli_celsius / MILLIDEGREE_PER_DECIDEGREE; + + buf[0] = DELL_XPS_EC_TEMP_CMD0; + buf[1] = DELL_XPS_EC_TEMP_CMD1; + buf[2] = cmd_byte; + buf[3] = DELL_XPS_EC_TEMP_CMD3; + put_unaligned_le16(deci_celsius, &buf[4]); + + ret = i2c_master_send(ec->client, buf, sizeof(buf)); + if (ret < 0) + return ret; + + return 0; +} + +static void dell_xps_ec_temp_work_fn(struct work_struct *work) +{ + struct dell_xps_ec *ec = container_of(work, struct dell_xps_ec, + temp_work.work); + const struct dell_xps_ec_therm *therms = ec->data->therms; + int val, ret; + unsigned int i; + + for (i = 0; i < ec->num_therms; i++) { + if (!ec->therm_channels[i]) + continue; + + ret = iio_read_channel_processed(ec->therm_channels[i], &val); + if (ret < 0) { + dev_err_ratelimited(ec->dev, + "Failed to read thermistor %s: %d\n", + therms[i].name, ret); + continue; + } + + ret = dell_xps_ec_send_temp(ec, therms[i].cmd, val); + if (ret < 0) { + dev_err_ratelimited(ec->dev, + "Failed to send temp for %s: %d\n", + therms[i].name, ret); + } + } + + schedule_delayed_work(&ec->temp_work, DELL_XPS_EC_TEMP_POLL_JIFFIES); +} + +static irqreturn_t dell_xps_ec_irq_handler(int irq, void *data) +{ + struct dell_xps_ec *ec = data; + + /* + * TODO: IRQ is fired on lid-close. Follow Windows example to read out + * the thermistor thresholds and potentially fan speeds. + */ + dev_dbg(ec->dev, "IRQ triggered (irq=%d)\n", irq); + + return IRQ_HANDLED; +} + +static int dell_xps_ec_probe(struct i2c_client *client) +{ + const struct dell_xps_ec_data *data; + struct device *dev = &client->dev; + struct dell_xps_ec *ec; + unsigned int i; + int ret; + + data = i2c_get_match_data(client); + if (!data) + return dev_err_probe(dev, -ENODEV, "No match data\n"); + + ec = devm_kzalloc(dev, struct_size(ec, therm_channels, data->num_therms), + GFP_KERNEL); + if (!ec) + return -ENOMEM; + + ec->dev = dev; + ec->client = client; + ec->data = data; + ec->num_therms = data->num_therms; + i2c_set_clientdata(client, ec); + + /* Set default thermistor profile */ + ret = i2c_master_send(client, data->profile, data->profile_len); + if (ret < 0) + return dev_err_probe(dev, ret, "Failed to set thermistor profile\n"); + + /* Get IIO channels for thermistors */ + for (i = 0; i < ec->num_therms; i++) { + ec->therm_channels[i] = + devm_iio_channel_get(dev, data->therms[i].name); + if (IS_ERR(ec->therm_channels[i])) { + ret = PTR_ERR(ec->therm_channels[i]); + ec->therm_channels[i] = NULL; + if (ret == -EPROBE_DEFER) + return ret; + dev_warn(dev, "Thermistor %s not available: %d\n", + data->therms[i].name, ret); + } + } + + /* Start periodic temperature reporting */ + ret = devm_delayed_work_autocancel(dev, &ec->temp_work, + dell_xps_ec_temp_work_fn); + if (ret) + return ret; + schedule_delayed_work(&ec->temp_work, DELL_XPS_EC_TEMP_POLL_JIFFIES); + dev_dbg(dev, "Started periodic temperature reporting to EC every %d ms\n", + jiffies_to_msecs(DELL_XPS_EC_TEMP_POLL_JIFFIES)); + + /* Request IRQ for EC events */ + ret = devm_request_threaded_irq(dev, client->irq, NULL, + dell_xps_ec_irq_handler, + IRQF_ONESHOT, dev_name(dev), ec); + if (ret) + return dev_err_probe(dev, ret, "Failed to request IRQ\n"); + + return 0; +} + +/* + * Notify EC of suspend + * + * This will: + * - Cut power to display/trackpad/keyboard/touchrow, wake-up source still works. + */ +static int dell_xps_ec_suspend(struct device *dev) +{ + struct dell_xps_ec *ec = dev_get_drvdata(dev); + + int ret; + + cancel_delayed_work_sync(&ec->temp_work); + + ret = dell_xps_ec_suspend_cmd(ec, true); + if (ret) { + schedule_delayed_work(&ec->temp_work, + DELL_XPS_EC_TEMP_POLL_JIFFIES); + return ret; + } + + return 0; +} + +/* + * Notify EC of resume + * + * This will undo the suspend actions. Without the resume signal, device would + * wake up but be forced back into suspend by EC within seconds. + */ +static int dell_xps_ec_resume(struct device *dev) +{ + struct dell_xps_ec *ec = dev_get_drvdata(dev); + int ret; + + ret = dell_xps_ec_suspend_cmd(ec, false); + if (ret) + return ret; + + schedule_delayed_work(&ec->temp_work, DELL_XPS_EC_TEMP_POLL_JIFFIES); + + return 0; +} + +static const struct of_device_id dell_xps_ec_of_match[] = { + { .compatible = "dell,xps13-9345-ec", .data = &dell_xps13_9345_data }, + {} +}; +MODULE_DEVICE_TABLE(of, dell_xps_ec_of_match); + +static const struct i2c_device_id dell_xps_ec_i2c_id[] = { + { "dell-xps-ec", (kernel_ulong_t)&dell_xps13_9345_data }, + {} +}; +MODULE_DEVICE_TABLE(i2c, dell_xps_ec_i2c_id); + +static const struct dev_pm_ops dell_xps_ec_pm_ops = { + SYSTEM_SLEEP_PM_OPS(dell_xps_ec_suspend, dell_xps_ec_resume) +}; + +static struct i2c_driver dell_xps_ec_driver = { + .driver = { + .name = "dell-xps-ec", + .of_match_table = dell_xps_ec_of_match, + .pm = &dell_xps_ec_pm_ops, + }, + .probe = dell_xps_ec_probe, + .id_table = dell_xps_ec_i2c_id, +}; +module_i2c_driver(dell_xps_ec_driver); + +MODULE_AUTHOR("Aleksandrs Vinarskis <alex@vinarskis.com>"); +MODULE_DESCRIPTION("Dell XPS 13 9345 Embedded Controller"); +MODULE_LICENSE("GPL"); +MODULE_IMPORT_NS("IIO_CONSUMER"); -- 2.54.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v6 2/3] platform: arm64: dell-xps-ec: new driver 2026-10-11 17:50 ` [PATCH v6 2/3] platform: arm64: dell-xps-ec: new driver Aleksandrs Vinarskis @ 2026-10-11 17:59 ` sashiko-bot 0 siblings, 0 replies; 12+ messages in thread From: sashiko-bot @ 2026-10-11 17:59 UTC (permalink / raw) To: Aleksandrs Vinarskis; +Cc: devicetree, conor+dt, robh Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] platform/arm64: dell-xps-ec: interrupt storm due to un-cleared level IRQ - [Medium] pm: early return in resume on I2C failure permanently disables thermal polling - [Medium] platform: off-by-one mismatch between thermistor profile IDs and update IDs -- commit bd6f1bc93a902f49663602dd218ea3f2d7526203 Author: Aleksandrs Vinarskis <alex@vinarskis.com> platform: arm64: dell-xps-ec: new driver 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. > diff --git a/drivers/platform/arm64/dell-xps-ec.c b/drivers/platform/arm64/dell-xps-ec.c > new file mode 100644 > index 0000000000000..cf2dd41b1aa68 > --- /dev/null > +++ b/drivers/platform/arm64/dell-xps-ec.c > @@ -0,0 +1,308 @@ [ ... ] > +/* > + * Format: > + * - header/unknown (2 bytes) > + * - per-thermistor entries (3 bytes): thermistor_id, param1, param2 > + */ > +static const u8 dell_xps13_9345_thermistor_profile[] = { > + 0xff, 0x54, > + 0x01, 0x00, 0x2b, /* sys_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 */ > +}; > + > +static const struct dell_xps_ec_therm dell_xps13_9345_therms[] = { > + /* TODO: 0x01 is sent only occasionally, likely TZ98 or TZ4. */ > + { "sys_therm0", 0x02 }, > + { "sys_therm1", 0x03 }, > + { "sys_therm2", 0x04 }, > + { "sys_therm3", 0x05 }, > + { "sys_therm4", 0x06 }, > + { "sys_therm5", 0x07 }, > + { "sys_therm6", 0x08 }, > +}; [Severity: Medium] Does this thermistor ID mapping mismatch the profile? The dell_xps13_9345_thermistor_profile array configures 7 thermistor IDs from 0x01 through 0x07 (with comments sys_therm0 through sys_therm6). However, the dell_xps13_9345_therms array maps sys_therm0 through sys_therm6 to command bytes 0x02 through 0x08. This misalignment means that the thermal polling loop sends temperature updates to ID 0x08 (which is completely unconfigured in the EC profile) and fails to send updates to ID 0x01 (which is explicitly configured in the profile). Will this configuration mismatch result in incorrect or missing thermal data being fed to the EC? [ ... ] > +static irqreturn_t dell_xps_ec_irq_handler(int irq, void *data) > +{ > + struct dell_xps_ec *ec = data; > + > + /* > + * TODO: IRQ is fired on lid-close. Follow Windows example to read out > + * the thermistor thresholds and potentially fan speeds. > + */ > + dev_dbg(ec->dev, "IRQ triggered (irq=%d)\n", irq); > + > + return IRQ_HANDLED; > +} [Severity: High] Will this dummy IRQ handler cause an interrupt storm? The driver requests an EC interrupt which is defined as IRQ_TYPE_LEVEL_LOW in the device tree (commit 8c751badf45b). The threaded IRQ handler simply logs a debug message and unconditionally returns IRQ_HANDLED without issuing any I2C commands to acknowledge or clear the event on the EC hardware. Because the interrupt is requested with IRQF_ONESHOT in dell_xps_ec_probe(), the interrupt is unmasked after the threaded handler returns. Since the level-triggered interrupt is never cleared on the EC, it will remain asserted and immediately fire again, potentially causing an infinite interrupt storm that can consume 100% of a CPU core. [ ... ] > +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; [Severity: Medium] Does this early return permanently disable thermal polling if the EC is slow to wake? During system resume, if dell_xps_ec_suspend_cmd() fails (e.g., due to a transient bus error or the EC taking longer to wake up), the function returns early without calling schedule_delayed_work(). Because the PM core largely ignores individual device resume errors and continues bringing the system up, the device will remain in the 'resumed' state, but the periodic temperature polling workqueue will be permanently stopped. Will this starve the EC of the thermal updates required to spin the fans, potentially leading to SoC overheating? > + > + schedule_delayed_work(&ec->temp_work, DELL_XPS_EC_TEMP_POLL_JIFFIES); > + > + return 0; > +} -- Sashiko AI review · https://sashiko.dev/#/patchset/20261011-dell-xps-9345-ec-v6-0-2a66a364fe70@vinarskis.com?part=2 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v6 3/3] arm64: dts: qcom: x1e80100-dell-xps13-9345: introduce EC 2026-10-11 17:50 [PATCH v6 0/3] Introduce EC driver for Snapdragon X1E based Dell XPS 13 9345 Aleksandrs Vinarskis 2026-10-11 17:50 ` [PATCH v6 1/3] dt-bindings: platform: introduce EC for " Aleksandrs Vinarskis 2026-10-11 17:50 ` [PATCH v6 2/3] platform: arm64: dell-xps-ec: new driver Aleksandrs Vinarskis @ 2026-10-11 17:50 ` Aleksandrs Vinarskis 2026-10-11 17:58 ` sashiko-bot ` (2 more replies) 2026-10-11 21:35 ` [PATCH v6 0/3] Introduce EC driver for Snapdragon X1E based Dell XPS 13 9345 Liviu Nicoara 3 siblings, 3 replies; 12+ messages in thread From: Aleksandrs Vinarskis @ 2026-10-11 17:50 UTC (permalink / raw) To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Hans de Goede, Ilpo Järvinen, Bryan O'Donoghue, Kees Cook, Gustavo A. R. Silva, Abel Vesa Cc: Liviu Nicoara, linux-arm-msm, devicetree, linux-kernel, platform-driver-x86, Abel Vesa, Tobias Heider, Val Packett, Neil Armstrong, Stephan Gerhold, linux-hardening, Dmitry Baryshkov, Konrad Dybcio 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. Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> Signed-off-by: Aleksandrs Vinarskis <alex@vinarskis.com> --- .../boot/dts/qcom/x1e80100-dell-xps13-9345.dts | 85 +++++++++++++++++++++- 1 file changed, 83 insertions(+), 2 deletions(-) diff --git a/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts b/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts index f48e6bc35006..bb20cf33847e 100644 --- a/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts +++ b/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts @@ -759,8 +759,32 @@ retimer_ss0_con_sbu_out: endpoint { &i2c5 { clock-frequency = <100000>; - status = "disabled"; - /* EC @0x3b */ + status = "okay"; + + embedded-controller@3b { + compatible = "dell,xps13-9345-ec"; + reg = <0x3b>; + + interrupts-extended = <&tlmm 66 IRQ_TYPE_LEVEL_LOW>; + + pinctrl-0 = <&ec_int_n_default>; + pinctrl-names = "default"; + + io-channels = <&pmk8550_vadc ADC5_GEN3_AMUX3_GPIO_100K_PU(1)>, + <&pmk8550_vadc ADC5_GEN3_AMUX4_GPIO_100K_PU(1)>, + <&pmk8550_vadc ADC5_GEN3_AMUX1_THM_100K_PU(1)>, + <&pmk8550_vadc ADC5_GEN3_AMUX2_THM_100K_PU(1)>, + <&pmk8550_vadc ADC5_GEN3_AMUX3_THM_100K_PU(1)>, + <&pmk8550_vadc ADC5_GEN3_AMUX4_THM_100K_PU(1)>, + <&pmk8550_vadc ADC5_GEN3_AMUX5_THM_100K_PU(1)>; + io-channel-names = "sys_therm0", + "sys_therm1", + "sys_therm2", + "sys_therm3", + "sys_therm4", + "sys_therm5", + "sys_therm6"; + }; }; &i2c7 { @@ -1025,6 +1049,57 @@ rtmr0_1p8_reg_en: rtmr0-1p8-reg-en-state { }; }; +&pmk8550_vadc { + sys_therm0: channel@14c { + reg = <ADC5_GEN3_AMUX3_GPIO_100K_PU(1)>; + qcom,hw-settle-time = <200>; + qcom,ratiometric; + label = "lpddr5_therm"; + }; + + sys_therm1: channel@14d { + reg = <ADC5_GEN3_AMUX4_GPIO_100K_PU(1)>; + qcom,hw-settle-time = <200>; + qcom,ratiometric; + label = "usb_charger_left_therm"; + }; + + sys_therm2: channel@144 { + reg = <ADC5_GEN3_AMUX1_THM_100K_PU(1)>; + qcom,hw-settle-time = <200>; + qcom,ratiometric; + label = "vph_pwr_therm"; + }; + + sys_therm3: channel@145 { + reg = <ADC5_GEN3_AMUX2_THM_100K_PU(1)>; + qcom,hw-settle-time = <200>; + qcom,ratiometric; + label = "usb_charger_right_therm"; + }; + + sys_therm4: channel@146 { + reg = <ADC5_GEN3_AMUX3_THM_100K_PU(1)>; + qcom,hw-settle-time = <200>; + qcom,ratiometric; + label = "ssd_conn_therm"; + }; + + sys_therm5: channel@147 { + reg = <ADC5_GEN3_AMUX4_THM_100K_PU(1)>; + qcom,hw-settle-time = <200>; + qcom,ratiometric; + label = "sys_therm5"; + }; + + sys_therm6: channel@148 { + reg = <ADC5_GEN3_AMUX5_THM_100K_PU(1)>; + qcom,hw-settle-time = <200>; + qcom,ratiometric; + label = "keyboard_therm"; + }; +}; + &qupv3_0 { status = "okay"; }; @@ -1103,6 +1178,12 @@ cam_indicator_en: cam-indicator-en-state { bias-disable; }; + ec_int_n_default: ec-int-n-state { + pins = "gpio66"; + function = "gpio"; + bias-disable; + }; + edp_bl_en: edp-bl-en-state { pins = "gpio74"; function = "gpio"; -- 2.54.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v6 3/3] arm64: dts: qcom: x1e80100-dell-xps13-9345: introduce EC 2026-10-11 17:50 ` [PATCH v6 3/3] arm64: dts: qcom: x1e80100-dell-xps13-9345: introduce EC Aleksandrs Vinarskis @ 2026-10-11 17:58 ` sashiko-bot 2026-10-13 17:27 ` Michael Burch 2026-10-14 8:38 ` Abel Vesa 2 siblings, 0 replies; 12+ messages in thread From: sashiko-bot @ 2026-10-11 17:58 UTC (permalink / raw) To: Aleksandrs Vinarskis; +Cc: conor+dt, robh, devicetree Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] platform: arm64: dell-xps-ec: permanently stopped thermal polling on resume error - [Low] i2c: dell_xps_ec_i2c_id initialized without named initializers -- commit bd6f1bc93a902f49663602dd218ea3f2d7526203 Author: Aleksandrs Vinarskis <alex@vinarskis.com> platform: arm64: dell-xps-ec: new driver This commit introduces a new Embedded Controller driver for the Dell XPS 13 9345. Its primary function is to unblock thermal management by providing the EC with temperature readings needed to control fans and peripheral power. > diff --git a/drivers/platform/arm64/dell-xps-ec.c b/drivers/platform/arm64/dell-xps-ec.c > new file mode 100644 > index 0000000000000..cf2dd41b1aa68 > --- /dev/null > +++ b/drivers/platform/arm64/dell-xps-ec.c [ ... ] > +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; [Severity: Medium] If a transient I2C bus error occurs during resume, dell_xps_ec_resume() returns early here without calling schedule_delayed_work(). Does this mean the background temperature polling loop (ec->temp_work) is never restarted? If so, could this cause the Embedded Controller to stop receiving thermal updates for the remainder of the boot, potentially leading to incorrect fan curves or unexpected thermal shutdowns until the next suspend/resume cycle? > + > + schedule_delayed_work(&ec->temp_work, DELL_XPS_EC_TEMP_POLL_JIFFIES); > + > + return 0; > +} [ ... ] > +static const struct i2c_device_id dell_xps_ec_i2c_id[] = { > + { "dell-xps-ec", (kernel_ulong_t)&dell_xps13_9345_data }, [Severity: Low] This isn't a bug, but should the dell_xps_ec_i2c_id array use named initializers instead of positional ones? The I2C subsystem guidelines mandate that struct i2c_device_id arrays must use named initializers. > + {} > +}; > +MODULE_DEVICE_TABLE(i2c, dell_xps_ec_i2c_id); -- Sashiko AI review · https://sashiko.dev/#/patchset/20261011-dell-xps-9345-ec-v6-0-2a66a364fe70@vinarskis.com?part=3 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v6 3/3] arm64: dts: qcom: x1e80100-dell-xps13-9345: introduce EC 2026-10-11 17:50 ` [PATCH v6 3/3] arm64: dts: qcom: x1e80100-dell-xps13-9345: introduce EC Aleksandrs Vinarskis 2026-10-11 17:58 ` sashiko-bot @ 2026-10-13 17:27 ` Michael Burch 2026-10-13 19:53 ` Liviu Nicoara 2026-10-14 8:38 ` Abel Vesa 2 siblings, 1 reply; 12+ messages in thread From: Michael Burch @ 2026-10-13 17:27 UTC (permalink / raw) To: Aleksandrs Vinarskis Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Hans de Goede, Ilpo Järvinen, Bryan O'Donoghue, Kees Cook, Gustavo A. R. Silva, Abel Vesa, Liviu Nicoara, linux-arm-msm, devicetree, linux-kernel, platform-driver-x86, Abel Vesa, Tobias Heider, Val Packett, Neil Armstrong, Stephan Gerhold, linux-hardening, Dmitry Baryshkov, Konrad Dybcio On Sun, Oct 11, 2026 at 07:50:10PM +0200, Aleksandrs Vinarskis wrote: > + io-channels = <&pmk8550_vadc ADC5_GEN3_AMUX3_GPIO_100K_PU(1)>, > + <&pmk8550_vadc ADC5_GEN3_AMUX4_GPIO_100K_PU(1)>, These two should be AMUX1_GPIO and AMUX2_GPIO. My unit (BIOS 2.14.0) shows what Liviu reported [1]: 0x14c (lpddr5_therm) reads 130.048 C from boot on, and that is the value the EC is sent. The Windows ADC configuration answers Liviu's question, wrong channel or missing pin configuration. In ADC1 from your qcom-aeob-dumps (dell-xps-9345/ADC1.json), the channel table puts the seven board thermistors on SID 1, all with the 100k pull-up: SYS_THERM1 0x4a AMUX1_GPIO SYS_THERM2 0x4b AMUX2_GPIO SYS_THERM3 0x44 AMUX1_THM SYS_THERM4 0x45 AMUX2_THM SYS_THERM5 0x46 AMUX3_THM SYS_THERM6 0x47 AMUX4_THM SYS_THERM7 0x48 AMUX5_THM Its GPIO table sets up only two pins as ADC inputs: PM8550 GPIO3 on channels 0x0a/0x4a and GPIO4 on 0x0b/0x4b. hamoa-pmics.dtsi already puts those two pins in high impedance for the VADC and defines channel@14a and channel@14b. Nothing in ADC1 uses AMUX3 or AMUX4 GPIO. sys_therm2 to sys_therm6 here are SYS_THERM3 to SYS_THERM7, so sys_thermN is SYS_THERM(N+1), and sys_therm0/1 are 0x14a/0x14b. The labels fit too: Liviu found the left charger thermistor on AMUX2 GPIO [2], and sys_therm1 is usb_charger_left_therm. Readings on a 7.3-rc6 based kernel, idle: 0x14a 40.6 C 0x14c 130.0 C (constant) 0x14b 34.6 C 0x14d 37.0 C AMUX1-5 THM 34.6 to 40.5 C > + sys_therm0: channel@14c { > + reg = <ADC5_GEN3_AMUX3_GPIO_100K_PU(1)>; [...] > + sys_therm1: channel@14d { > + reg = <ADC5_GEN3_AMUX4_GPIO_100K_PU(1)>; With the change below on a v7.3-rc7 based kernel, sys_therm0 and sys_therm1 read 45.4 C and 41.0 C, next to 38.2 to 46.8 C on the THM channels: --- a/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts +++ b/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts @@ -1133,8 +1133,8 @@ embedded-controller@3b { pinctrl-0 = <&ec_int_n_default>; pinctrl-names = "default"; - io-channels = <&pmk8550_vadc ADC5_GEN3_AMUX3_GPIO_100K_PU(1)>, - <&pmk8550_vadc ADC5_GEN3_AMUX4_GPIO_100K_PU(1)>, + io-channels = <&pmk8550_vadc ADC5_GEN3_AMUX1_GPIO_100K_PU(1)>, + <&pmk8550_vadc ADC5_GEN3_AMUX2_GPIO_100K_PU(1)>, <&pmk8550_vadc ADC5_GEN3_AMUX1_THM_100K_PU(1)>, <&pmk8550_vadc ADC5_GEN3_AMUX2_THM_100K_PU(1)>, <&pmk8550_vadc ADC5_GEN3_AMUX3_THM_100K_PU(1)>, @@ -1439,15 +1439,15 @@ rtmr0_1p8_reg_en: rtmr0-1p8-reg-en-state { }; &pmk8550_vadc { - sys_therm0: channel@14c { - reg = <ADC5_GEN3_AMUX3_GPIO_100K_PU(1)>; + sys_therm0: channel@14a { + reg = <ADC5_GEN3_AMUX1_GPIO_100K_PU(1)>; qcom,hw-settle-time = <200>; qcom,ratiometric; label = "lpddr5_therm"; }; - sys_therm1: channel@14d { - reg = <ADC5_GEN3_AMUX4_GPIO_100K_PU(1)>; + sys_therm1: channel@14b { + reg = <ADC5_GEN3_AMUX2_GPIO_100K_PU(1)>; qcom,hw-settle-time = <200>; qcom,ratiometric; label = "usb_charger_left_therm"; [1] https://lore.kernel.org/r/20261011213528.13207-1-lnicoara@thinkoid.org [2] https://lore.kernel.org/r/20261006185557.23016-1-lnicoara@thinkoid.org Michael ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v6 3/3] arm64: dts: qcom: x1e80100-dell-xps13-9345: introduce EC 2026-10-13 17:27 ` Michael Burch @ 2026-10-13 19:53 ` Liviu Nicoara 0 siblings, 0 replies; 12+ messages in thread From: Liviu Nicoara @ 2026-10-13 19:53 UTC (permalink / raw) To: Michael Burch, Aleksandrs Vinarskis Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Hans de Goede, Ilpo Järvinen, Bryan O'Donoghue, Kees Cook, Gustavo A . R . Silva, Abel Vesa, linux-arm-msm, devicetree, linux-kernel, platform-driver-x86, Abel Vesa, Tobias Heider, Val Packett, Neil Armstrong, Stephan Gerhold, linux-hardening, Dmitry Baryshkov, Konrad Dybcio On Tue, 13 Oct 2026 12:27:16 -0500, Michael Burch wrote: > These two should be AMUX1_GPIO and AMUX2_GPIO. My unit (BIOS 2.14.0) > shows what Liviu reported [1]: 0x14c (lpddr5_therm) reads 130.048 C from > boot on, and that is the value the EC is sent. Same here, and the ADC1 table settles what my readings left open. On this unit (also BIOS 2.14.0), with all four GPIO channels defined in one DTB on a 7.2.6 kernel: 0x14a 38.6 C 0x14c 130.0 C (constant) 0x14b 32.7 C 0x14d 34.7 C A charger on the left USB-C port warmed 0x14b by 1.6 C in three minutes; 0x14d stayed within 0.1 C. So usb_charger_left_therm is AMUX2_GPIO, 0x14b, as your table says; 0x14a reads a board temperature and warms with the SoC. > + sys_therm0: channel@14a { > + reg = <ADC5_GEN3_AMUX1_GPIO_100K_PU(1)>; One note on the shape: hamoa-pmics.dtsi already defines channel@14a and channel@14b as pm8550_gpio_01 and pm8550_gpio_02, with gpio3 and gpio4 in high impedance for them. The EC's io-channels can reference those two directly; redefining the nodes in the board file only adds the labels, which the EC node does not need. Happy to test a v7 with the two channels moved. Liviu ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v6 3/3] arm64: dts: qcom: x1e80100-dell-xps13-9345: introduce EC 2026-10-11 17:50 ` [PATCH v6 3/3] arm64: dts: qcom: x1e80100-dell-xps13-9345: introduce EC Aleksandrs Vinarskis 2026-10-11 17:58 ` sashiko-bot 2026-10-13 17:27 ` Michael Burch @ 2026-10-14 8:38 ` Abel Vesa 2 siblings, 0 replies; 12+ messages in thread From: Abel Vesa @ 2026-10-14 8:38 UTC (permalink / raw) To: Aleksandrs Vinarskis Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Hans de Goede, Ilpo Järvinen, Bryan O'Donoghue, Kees Cook, Gustavo A. R. Silva, Abel Vesa, Liviu Nicoara, linux-arm-msm, devicetree, linux-kernel, platform-driver-x86, Tobias Heider, Val Packett, Neil Armstrong, Stephan Gerhold, linux-hardening, Dmitry Baryshkov, Konrad Dybcio On 26-10-11 19:50:10, 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. > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> > Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> > Signed-off-by: Aleksandrs Vinarskis <alex@vinarskis.com> Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v6 0/3] Introduce EC driver for Snapdragon X1E based Dell XPS 13 9345 2026-10-11 17:50 [PATCH v6 0/3] Introduce EC driver for Snapdragon X1E based Dell XPS 13 9345 Aleksandrs Vinarskis ` (2 preceding siblings ...) 2026-10-11 17:50 ` [PATCH v6 3/3] arm64: dts: qcom: x1e80100-dell-xps13-9345: introduce EC Aleksandrs Vinarskis @ 2026-10-11 21:35 ` Liviu Nicoara 3 siblings, 0 replies; 12+ messages in thread From: Liviu Nicoara @ 2026-10-11 21:35 UTC (permalink / raw) To: Aleksandrs Vinarskis, Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Hans de Goede, Ilpo Järvinen, Bryan O'Donoghue, Kees Cook, Gustavo A . R . Silva, Abel Vesa Cc: linux-arm-msm, devicetree, linux-kernel, platform-driver-x86, Abel Vesa, Tobias Heider, Val Packett, Neil Armstrong, Stephan Gerhold, linux-hardening, Krzysztof Kozlowski, Dmitry Baryshkov, Konrad Dybcio On Sun, 11 Oct 2026 19:50:07 +0200, Aleksandrs Vinarskis wrote: > 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. Tested v6 on a second XPS 13 9345 (BIOS 2.14.0), on 7.2.6 with the PMIC5 Gen3 ADC and ADC_TM drivers and the board thermal zones [1] backported. The driver probes, gets all seven channels and reports to the EC without an error. Two observations from this unit: 1. The two GPIO thermistor channels. Here AMUX3_GPIO (0x14c, lpddr5_therm) reads the top of the conversion table, ~130 C, from boot on, and AMUX4_GPIO (0x14d, usb_charger_left_therm) does not move when a charger warms the left port. AMUX2_GPIO (0x14b) does, 1.6 C in three minutes, and AMUX1_GPIO (0x14a) reads a plausible board temperature. Windows' ADC1.bin puts SYS_THERM1/2 on 0x14a/0x14b, and hamoa-pmics.dtsi in next defines those two. The patch configures no PMIC GPIO for AMUX3/4, where mainline sets pm8550 gpio3 and gpio4 for AMUX1/2; so either the two thermistors sit on 0x14a/0x14b on this board, or AMUX3/4 want the pin configuration. As it stands the EC is told the memory is at 130 C. The fans did not react to that here, at idle on battery. 2. Resume. With the driver loaded, a key press wakes the machine through the PMIC power key: the EC has powered the keyboard off, as the comment in the driver says, and the keyboard delivers no event for the key that woke the machine; instead pm8941-pwrkey reports a KEY_POWER press and release at resume. With logind's default HandlePowerKey=poweroff and no desktop holding the power-key inhibitor, the machine shuts down right after it wakes. With the inhibitor held, the resume is clean and the machine stays up, so the resume command does its job. Without the driver the keyboard wakes the host as an i2c-hid device and no power-key event appears. Reproduced twice; GNOME and KDE take that inhibitor, so they would not see it. [1] https://lore.kernel.org/all/20261006185557.23016-1-lnicoara@thinkoid.org/ Liviu ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2026-10-15 3:07 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-10-11 17:50 [PATCH v6 0/3] Introduce EC driver for Snapdragon X1E based Dell XPS 13 9345 Aleksandrs Vinarskis 2026-10-11 17:50 ` [PATCH v6 1/3] dt-bindings: platform: introduce EC for " Aleksandrs Vinarskis 2026-10-11 17:59 ` sashiko-bot 2026-10-15 3:07 ` Rob Herring 2026-10-11 17:50 ` [PATCH v6 2/3] platform: arm64: dell-xps-ec: new driver Aleksandrs Vinarskis 2026-10-11 17:59 ` sashiko-bot 2026-10-11 17:50 ` [PATCH v6 3/3] arm64: dts: qcom: x1e80100-dell-xps13-9345: introduce EC Aleksandrs Vinarskis 2026-10-11 17:58 ` sashiko-bot 2026-10-13 17:27 ` Michael Burch 2026-10-13 19:53 ` Liviu Nicoara 2026-10-14 8:38 ` Abel Vesa 2026-10-11 21:35 ` [PATCH v6 0/3] Introduce EC driver for Snapdragon X1E based Dell XPS 13 9345 Liviu Nicoara
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox