Devicetree
 help / color / mirror / Atom feed
* [PATCH v6 0/3] Lenovo ThinkPad T14s EC thermal monitoring and thermal zone integration
@ 2026-07-07 19:22 Daniel Lezcano
  2026-07-07 19:22 ` [PATCH v6 1/3] dt-bindings: embedded-controller: Add Lenovo ThinkPad T14s thermal sensor provider support Daniel Lezcano
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Daniel Lezcano @ 2026-07-07 19:22 UTC (permalink / raw)
  To: sre, hansg, ilpo.jarvinen, linux, andersson, konradybcio, robh,
	krzk+dt, conor+dt
  Cc: bryan.odonoghue, platform-driver-x86, linux-kernel, linux-hwmon,
	linux-arm-msm, devicetree, neil.armstrong, gaurav.kohli,
	manaf.pallikunhi, priyansh.jain

Hi,

This series extends the Lenovo ThinkPad T14s embedded controller driver
with environmental monitoring capabilities and integrates the exposed
sensors into the Linux thermal framework.

The EC provides access to several platform temperature sensors
covering the SoC, keyboard area, bottom cover, charging circuitry, QTM
module and SSD. These sensors are currently used by the firmware for
thermal management but are not exposed to Linux.

The first patch adds hwmon support for the EC temperature sensors.

The second patch exposes the EC as a thermal sensor provider in the
device tree and defines thermal zones for the keyboard skin
temperature and the charging circuitry temperature. This allows the
generic thermal framework to react to EC-reported temperatures and
apply standard Linux thermal mitigation policies.

As the EC protocol is not fully decoded, the passive trip points
get/set actions are missing, so it is not possible to program a
threshold and receive an interrupt when crossed the way up or
down. Consequently, the thermal zone related to the charging circuitry
is polled every two seconds until we can set the trip points in the
EC.

This series fixes critical thermal issues happening on this platform
where a kernel compilation, or heavy workloads, lead to a system
reboot.

Tested on a Lenovo ThinkPad T14s Gen 6 (Snapdragon X Elite).

Please make sure scmi-cpufreq.ko is loaded before testing

Thanks,

Daniel

---
 Changelog:
	v6:
	 - Added Reviewed-by tag (Dmitry Baryshkov)
	 - Removed required property for #thermal-sensor-cells (Krzysztof Kozlowski)
	v5:
	 - Added Reviewed-by tag (Ilpo Järvinen)
	 - Added the '#thermal-sensor-cells' property DT binding (Sashiko)
	v4:
 	 - Added the missing HWMON_C_REGISTER_TZ attribute (Sashiko)
	 - Fixed dependency with HWMON (Sashiko)
	 - Added Tested-by tag (Neil Armstrong)
	v3:
	 - Removed event based because trip point are not yet well supported
	 - Added an empty line after variable declaration (Ilpo Järvinen)
	 - Used MILLIDEGREE_PER_DEGREE from units.h (Ilpo Järvinen)
	 - Made switch consistent (Ilpo Järvinen)
	v2:
	 - Fixed patch 1 subject prefix
	 - Removed the fan information part
	 - Added HWMON_T_ALARM
	 - Fixed DT change description to reflect what it does really

Daniel Lezcano (3):
  dt-bindings: embedded-controller: Add Lenovo ThinkPad T14s thermal
    sensor provider support
  platform: arm64: lenovo-thinkpad-t14s-ec: Add hwmon support for
    temperatures
  arm64: dts: qcom: x1e78100-t14s: Add thermal zones for keyboard skin
    and charging sensors

 .../lenovo,thinkpad-t14s-ec.yaml              |   4 +
 .../qcom/x1e78100-lenovo-thinkpad-t14s.dtsi   |  67 ++++++++-
 drivers/platform/arm64/Kconfig                |   1 +
 drivers/platform/arm64/lenovo-thinkpad-t14s.c | 131 ++++++++++++++++++
 4 files changed, 202 insertions(+), 1 deletion(-)

-- 
2.53.0


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

* [PATCH v6 1/3] dt-bindings: embedded-controller: Add Lenovo ThinkPad T14s thermal sensor provider support
  2026-07-07 19:22 [PATCH v6 0/3] Lenovo ThinkPad T14s EC thermal monitoring and thermal zone integration Daniel Lezcano
@ 2026-07-07 19:22 ` Daniel Lezcano
  2026-07-07 19:30   ` sashiko-bot
  2026-07-07 19:22 ` [PATCH v6 2/3] platform: arm64: lenovo-thinkpad-t14s-ec: Add hwmon support for temperatures Daniel Lezcano
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 18+ messages in thread
From: Daniel Lezcano @ 2026-07-07 19:22 UTC (permalink / raw)
  To: sre, hansg, ilpo.jarvinen, linux, andersson, konradybcio, robh,
	krzk+dt, conor+dt
  Cc: bryan.odonoghue, platform-driver-x86, linux-kernel, linux-hwmon,
	linux-arm-msm, devicetree, neil.armstrong, gaurav.kohli,
	manaf.pallikunhi, priyansh.jain

Document the Lenovo ThinkPad T14s Embedded Controller as a thermal
sensor provider by adding the '#thermal-sensor-cells' property.

This allows the EC temperature sensors to be referenced from thermal
zones in the device tree.

Signed-off-by: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>
---
 .../bindings/embedded-controller/lenovo,thinkpad-t14s-ec.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/embedded-controller/lenovo,thinkpad-t14s-ec.yaml b/Documentation/devicetree/bindings/embedded-controller/lenovo,thinkpad-t14s-ec.yaml
index c87ccb5b3086..f677966b52a8 100644
--- a/Documentation/devicetree/bindings/embedded-controller/lenovo,thinkpad-t14s-ec.yaml
+++ b/Documentation/devicetree/bindings/embedded-controller/lenovo,thinkpad-t14s-ec.yaml
@@ -26,6 +26,9 @@ properties:
 
   wakeup-source: true
 
+  "#thermal-sensor-cells":
+    const: 1
+
 required:
   - compatible
   - reg
@@ -45,6 +48,7 @@ examples:
             reg = <0x28>;
             interrupts-extended = <&tlmm 66 IRQ_TYPE_LEVEL_LOW>;
             wakeup-source;
+            #thermal-sensor-cells = <1>;
         };
     };
 ...
-- 
2.53.0


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

* [PATCH v6 2/3] platform: arm64: lenovo-thinkpad-t14s-ec: Add hwmon support for temperatures
  2026-07-07 19:22 [PATCH v6 0/3] Lenovo ThinkPad T14s EC thermal monitoring and thermal zone integration Daniel Lezcano
  2026-07-07 19:22 ` [PATCH v6 1/3] dt-bindings: embedded-controller: Add Lenovo ThinkPad T14s thermal sensor provider support Daniel Lezcano
@ 2026-07-07 19:22 ` Daniel Lezcano
  2026-07-07 19:28   ` sashiko-bot
  2026-07-08  6:30   ` Krzysztof Kozlowski
  2026-07-07 19:22 ` [PATCH v6 3/3] arm64: dts: qcom: x1e78100-t14s: Add thermal zones for keyboard skin and charging sensors Daniel Lezcano
  2026-07-08 12:03 ` [PATCH v6 0/3] Lenovo ThinkPad T14s EC thermal monitoring and thermal zone integration Stephan Gerhold
  3 siblings, 2 replies; 18+ messages in thread
From: Daniel Lezcano @ 2026-07-07 19:22 UTC (permalink / raw)
  To: sre, hansg, ilpo.jarvinen, linux, andersson, konradybcio, robh,
	krzk+dt, conor+dt
  Cc: bryan.odonoghue, platform-driver-x86, linux-kernel, linux-hwmon,
	linux-arm-msm, devicetree, neil.armstrong, gaurav.kohli,
	manaf.pallikunhi, priyansh.jain

Expose the Lenovo ThinkPad T14s EC environmental sensors through
the hwmon subsystem.

The driver now registers a hwmon device providing access to six EC
temperature sensors corresponding to the SoC, keyboard area, base
cover, PMIC/charging circuitry, QTM module and SSD. Sensor labels
are exported to allow user space to identify each measurement.

This allows standard monitoring tools such as lm-sensors to report
platform temperatures.

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on T14s OLED
Signed-off-by: Daniel Lezcano daniel.lezcano@oss.qualcomm.com
---
 drivers/platform/arm64/Kconfig                |   1 +
 drivers/platform/arm64/lenovo-thinkpad-t14s.c | 131 ++++++++++++++++++
 2 files changed, 132 insertions(+)

diff --git a/drivers/platform/arm64/Kconfig b/drivers/platform/arm64/Kconfig
index 10f905d7d6bf..121043348740 100644
--- a/drivers/platform/arm64/Kconfig
+++ b/drivers/platform/arm64/Kconfig
@@ -75,6 +75,7 @@ config EC_LENOVO_THINKPAD_T14S
 	depends on ARCH_QCOM || COMPILE_TEST
 	depends on I2C
 	depends on INPUT
+	depends on HWMON
 	select INPUT_SPARSEKMAP
 	select LEDS_CLASS
 	select NEW_LEDS
diff --git a/drivers/platform/arm64/lenovo-thinkpad-t14s.c b/drivers/platform/arm64/lenovo-thinkpad-t14s.c
index 5590302a5694..fc480f093238 100644
--- a/drivers/platform/arm64/lenovo-thinkpad-t14s.c
+++ b/drivers/platform/arm64/lenovo-thinkpad-t14s.c
@@ -11,6 +11,7 @@
 #include <linux/delay.h>
 #include <linux/dev_printk.h>
 #include <linux/err.h>
+#include <linux/hwmon.h>
 #include <linux/i2c.h>
 #include <linux/input.h>
 #include <linux/input/sparse-keymap.h>
@@ -21,6 +22,7 @@
 #include <linux/regmap.h>
 #include <linux/slab.h>
 #include <linux/pm.h>
+#include <linux/units.h>
 
 #define T14S_EC_CMD_ECRD	0x02
 #define T14S_EC_CMD_ECWR	0x03
@@ -67,6 +69,13 @@
 #define T14S_EC_EVT_KEY_FN_F11			0x7a
 #define T14S_EC_EVT_KEY_FN_G			0x7e
 
+#define T14S_EC_SYS_THERM0 0x78 /* SoC (CPU+GPU)  */
+#define T14S_EC_SYS_THERM1 0x79 /* Keyboard       */
+#define T14S_EC_SYS_THERM2 0x7a /* Back cover     */
+#define T14S_EC_SYS_THERM3 0x7b /* Charger / PMIC */
+#define T14S_EC_SYS_THERM6 0x7c /* QTM West       */
+#define T14S_EC_SYS_THERM7 0x7d /* SSD            */
+
 /* Hardware LED blink rate is 1 Hz (500ms off, 500ms on) */
 #define T14S_EC_BLINK_RATE_ON_OFF_MS		500
 
@@ -93,9 +102,19 @@ struct t14s_ec_led_classdev {
 	struct t14s_ec *ec;
 };
 
+struct t14s_ec_hwmon_sys_thermx {
+	const char *label;
+	int reg;
+};
+
+struct t14s_ec_hwmon {
+	struct t14s_ec_hwmon_sys_thermx *sys_thermx;
+};
+
 struct t14s_ec {
 	struct regmap *regmap;
 	struct device *dev;
+	struct t14s_ec_hwmon ec_hwmon;
 	struct t14s_ec_led_classdev led_pwr_btn;
 	struct t14s_ec_led_classdev led_chrg_orange;
 	struct t14s_ec_led_classdev led_chrg_white;
@@ -555,6 +574,114 @@ static irqreturn_t t14s_ec_irq_handler(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+static umode_t t14s_ec_hwmon_is_visible(const void *drvdata,
+					enum hwmon_sensor_types type,
+					u32 attr, int channel)
+{
+	switch (type) {
+	case hwmon_temp:
+		if (attr == hwmon_temp_input ||
+		    attr == hwmon_temp_label)
+			return 0444;
+		break;
+	default:
+		return 0;
+	}
+
+	return 0;
+}
+
+static int t14s_ec_hwmon_read_string(struct device *dev, enum hwmon_sensor_types type,
+				     u32 attr, int channel, const char **str)
+{
+	struct t14s_ec *ec = dev_get_drvdata(dev);
+
+	switch (type) {
+	case hwmon_temp:
+		if (attr == hwmon_temp_label) {
+			*str = ec->ec_hwmon.sys_thermx[channel].label;
+			return 0;
+		}
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return -EOPNOTSUPP;
+}
+
+static int t14s_ec_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
+			      u32 attr, int channel, long *val)
+{
+	struct t14s_ec *ec = dev_get_drvdata(dev);
+	unsigned int value;
+	int ret;
+
+	switch (type) {
+	case hwmon_temp:
+		if (attr == hwmon_temp_input) {
+			ret = t14s_ec_read(ec, ec->ec_hwmon.sys_thermx[channel].reg, &value);
+			if (ret)
+				return ret;
+			*val = value * MILLIDEGREE_PER_DEGREE;
+
+			return 0;
+		}
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return -EOPNOTSUPP;
+}
+
+static const struct hwmon_ops t14s_ec_hwmon_ops = {
+	.is_visible = t14s_ec_hwmon_is_visible,
+	.read = t14s_ec_hwmon_read,
+	.read_string = t14s_ec_hwmon_read_string,
+};
+
+static const struct hwmon_channel_info *t14s_ec_hwmon_info[] = {
+	HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ),
+	HWMON_CHANNEL_INFO(temp,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL),
+	NULL
+};
+
+static const struct hwmon_chip_info t14s_ec_chip_info = {
+	.ops = &t14s_ec_hwmon_ops,
+	.info = t14s_ec_hwmon_info,
+};
+
+static int t14s_ec_hwmon_probe(struct t14s_ec *ec)
+{
+	struct device *dev;
+	struct t14s_ec_hwmon_sys_thermx sys_thermx[] = {
+		{ .label = "soc",	.reg = T14S_EC_SYS_THERM0 },
+		{ .label = "keyboard",	.reg = T14S_EC_SYS_THERM1 },
+		{ .label = "base",	.reg = T14S_EC_SYS_THERM2 },
+		{ .label = "charging",	.reg = T14S_EC_SYS_THERM3 },
+		{ .label = "qtm",	.reg = T14S_EC_SYS_THERM6 },
+		{ .label = "ssd",	.reg = T14S_EC_SYS_THERM7 },
+	};
+
+	ec->ec_hwmon.sys_thermx = devm_kmemdup_array(ec->dev, sys_thermx,
+						     ARRAY_SIZE(sys_thermx),
+						     sizeof(sys_thermx[0]), GFP_KERNEL);
+	if (!ec->ec_hwmon.sys_thermx)
+		return -ENOMEM;
+
+	dev = devm_hwmon_device_register_with_info(ec->dev, "t14s_ec", ec,
+						   &t14s_ec_chip_info, NULL);
+
+	return PTR_ERR_OR_ZERO(dev);
+}
+
 static int t14s_ec_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
@@ -590,6 +717,10 @@ static int t14s_ec_probe(struct i2c_client *client)
 	if (ret < 0)
 		return ret;
 
+	ret = t14s_ec_hwmon_probe(ec);
+	if (ret < 0)
+		return ret;
+
 	ret = devm_request_threaded_irq(dev, client->irq, NULL,
 					t14s_ec_irq_handler,
 					IRQF_ONESHOT, dev_name(dev), ec);
-- 
2.53.0


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

* [PATCH v6 3/3] arm64: dts: qcom: x1e78100-t14s: Add thermal zones for keyboard skin and charging sensors
  2026-07-07 19:22 [PATCH v6 0/3] Lenovo ThinkPad T14s EC thermal monitoring and thermal zone integration Daniel Lezcano
  2026-07-07 19:22 ` [PATCH v6 1/3] dt-bindings: embedded-controller: Add Lenovo ThinkPad T14s thermal sensor provider support Daniel Lezcano
  2026-07-07 19:22 ` [PATCH v6 2/3] platform: arm64: lenovo-thinkpad-t14s-ec: Add hwmon support for temperatures Daniel Lezcano
@ 2026-07-07 19:22 ` Daniel Lezcano
  2026-07-07 19:30   ` sashiko-bot
  2026-07-08 12:03 ` [PATCH v6 0/3] Lenovo ThinkPad T14s EC thermal monitoring and thermal zone integration Stephan Gerhold
  3 siblings, 1 reply; 18+ messages in thread
From: Daniel Lezcano @ 2026-07-07 19:22 UTC (permalink / raw)
  To: sre, hansg, ilpo.jarvinen, linux, andersson, konradybcio, robh,
	krzk+dt, conor+dt
  Cc: bryan.odonoghue, platform-driver-x86, linux-kernel, linux-hwmon,
	linux-arm-msm, devicetree, neil.armstrong, gaurav.kohli,
	manaf.pallikunhi, priyansh.jain

The Lenovo ThinkPad T14s embedded controller exposes several platform
temperature sensors that are already used by the firmware for thermal
management.

Expose the EC as a thermal sensor provider and describe the keyboard
skin and charging circuitry sensors as thermal zones in the device
tree.

The keyboard thermal zone defines passive and hot trip points, while
the charging thermal zone also associates a cooling map with the CPU
clusters, allowing the generic thermal framework to apply CPU
throttling when the charging circuitry temperature exceeds the passive
threshold.

This integrates the EC temperature sensors with the Linux thermal
framework and enables platform thermal management using standard
thermal zone definitions.

The EC protocol currently does not provide a mechanism to program trip
points from Linux. Consequently, the thermal zones rely on periodic
polling to detect threshold crossings.

Using the charging circuitry temperature for thermal mitigation provides
a conservative approximation of the platform thermal state and prevents
the platform from reaching critical temperatures under sustained heavy
CPU load.

Without this change the platform reaches a critical thermal condition
and resets under heavy load.

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on T14s OLED
Signed-off-by: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>
---
 .../qcom/x1e78100-lenovo-thinkpad-t14s.dtsi   | 67 ++++++++++++++++++-
 1 file changed, 66 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi
index 5d49df41be02..ed5ba3a428a8 100644
--- a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi
@@ -979,7 +979,7 @@ &i2c6 {
 
 	status = "okay";
 
-	embedded-controller@28 {
+	ec: embedded-controller@28 {
 		compatible = "lenovo,thinkpad-t14s-ec";
 		reg = <0x28>;
 
@@ -988,6 +988,8 @@ embedded-controller@28 {
 		pinctrl-0 = <&ec_int_n_default>;
 		pinctrl-names = "default";
 
+		#thermal-sensor-cells = <1>;
+
 		wakeup-source;
 	};
 };
@@ -1729,3 +1731,66 @@ &usb_mp_qmpphy1 {
 
 	status = "okay";
 };
+
+&thermal_zones {
+	ec-keyboard-thermal {
+		polling-delay = <5000>;
+		polling-delay-passive = <1000>;
+
+		thermal-sensors = <&ec 1>;
+
+		trips {
+			trip-point0 {
+				temperature = <55000>;
+				hysteresis = <2000>;
+				type = "passive";
+			};
+
+			trip-point1 {
+				temperature = <62000>;
+				hysteresis = <0>;
+				type = "hot";
+			};
+		};
+	};
+
+	ec-charging-thermal {
+		/* EC trip points cannot yet be programmed. */
+		polling-delay = <5000>;
+		polling-delay-passive = <2000>;
+
+		thermal-sensors = <&ec 3>;
+
+		trips {
+			ec_charging_psv0: trip-point0 {
+				temperature = <55000>;
+				hysteresis = <0>;
+				type = "passive";
+			};
+
+			ec_charging_alrt0: trip-point1 {
+				temperature = <63000>;
+				hysteresis = <0>;
+				type = "hot";
+			};
+		};
+
+		cooling-maps {
+			map0 {
+				trip = <&ec_charging_psv0>;
+				cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+						 <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+						 <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+						 <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+						 <&cpu4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+						 <&cpu5 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+						 <&cpu6 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+						 <&cpu7 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+						 <&cpu8 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+						 <&cpu9 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+						 <&cpu10 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+						 <&cpu11 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+			};
+		};
+	};
+};
-- 
2.53.0


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

* Re: [PATCH v6 2/3] platform: arm64: lenovo-thinkpad-t14s-ec: Add hwmon support for temperatures
  2026-07-07 19:22 ` [PATCH v6 2/3] platform: arm64: lenovo-thinkpad-t14s-ec: Add hwmon support for temperatures Daniel Lezcano
@ 2026-07-07 19:28   ` sashiko-bot
  2026-07-08  6:30   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 18+ messages in thread
From: sashiko-bot @ 2026-07-07 19:28 UTC (permalink / raw)
  To: Daniel Lezcano; +Cc: conor+dt, robh, devicetree, linux-hwmon

> Expose the Lenovo ThinkPad T14s EC environmental sensors through
> the hwmon subsystem.
> 
> The driver now registers a hwmon device providing access to six EC
> temperature sensors corresponding to the SoC, keyboard area, base
> cover, PMIC/charging circuitry, QTM module and SSD. Sensor labels
> are exported to allow user space to identify each measurement.
> 
> This allows standard monitoring tools such as lm-sensors to report
> platform temperatures.
> 
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on T14s OLED
> Signed-off-by: Daniel Lezcano daniel.lezcano@oss.qualcomm.com

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707192228.14647-1-daniel.lezcano@oss.qualcomm.com?part=2


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

* Re: [PATCH v6 1/3] dt-bindings: embedded-controller: Add Lenovo ThinkPad T14s thermal sensor provider support
  2026-07-07 19:22 ` [PATCH v6 1/3] dt-bindings: embedded-controller: Add Lenovo ThinkPad T14s thermal sensor provider support Daniel Lezcano
@ 2026-07-07 19:30   ` sashiko-bot
  2026-07-07 21:34     ` Daniel Lezcano
  0 siblings, 1 reply; 18+ messages in thread
From: sashiko-bot @ 2026-07-07 19:30 UTC (permalink / raw)
  To: Daniel Lezcano; +Cc: devicetree, robh, linux-hwmon, conor+dt

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly added '#thermal-sensor-cells' property is missing from the 'required' list.
--

--- Patch [1]: [PATCH v6 1/3] dt-bindings: embedded-controller: Add Lenovo ThinkPad T14s thermal sensor provider support ---
commit 48ae118884c307667e25a720a62e87b8e32443c5
Author: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>

dt-bindings: embedded-controller: Add Lenovo ThinkPad T14s thermal sensor provider support

This commit documents the Lenovo ThinkPad T14s Embedded Controller as a
thermal sensor provider by adding the #thermal-sensor-cells property. This
allows the EC temperature sensors to be referenced from thermal zones.

> diff --git a/Documentation/devicetree/bindings/embedded-controller/lenovo,thinkpad-t14s-ec.yaml b/Documentation/devicetree/bindings/embedded-controller/lenovo,thinkpad-t14s-ec.yaml
> --- a/Documentation/devicetree/bindings/embedded-controller/lenovo,thinkpad-t14s-ec.yaml
> +++ b/Documentation/devicetree/bindings/embedded-controller/lenovo,thinkpad-t14s-ec.yaml
> @@ -26,6 +26,9 @@ properties:
>  
>    wakeup-source: true
>  
> +  "#thermal-sensor-cells":
> +    const: 1
> +
>  required:
>    - compatible
>    - reg

[Severity: Medium]
Should #thermal-sensor-cells be added to this required list?

When hardware gains new provider capabilities, omitting the corresponding
standard properties from the required list allows incomplete device tree
nodes to pass schema validation. Dependent thermal zones could fail at
runtime if they attempt to reference the embedded controller and the property
is missing.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707192228.14647-1-daniel.lezcano@oss.qualcomm.com?part=1

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

* Re: [PATCH v6 3/3] arm64: dts: qcom: x1e78100-t14s: Add thermal zones for keyboard skin and charging sensors
  2026-07-07 19:22 ` [PATCH v6 3/3] arm64: dts: qcom: x1e78100-t14s: Add thermal zones for keyboard skin and charging sensors Daniel Lezcano
@ 2026-07-07 19:30   ` sashiko-bot
  0 siblings, 0 replies; 18+ messages in thread
From: sashiko-bot @ 2026-07-07 19:30 UTC (permalink / raw)
  To: Daniel Lezcano; +Cc: conor+dt, robh, linux-hwmon, devicetree

> The Lenovo ThinkPad T14s embedded controller exposes several platform
> temperature sensors that are already used by the firmware for thermal
> management.
> 
> Expose the EC as a thermal sensor provider and describe the keyboard
> skin and charging circuitry sensors as thermal zones in the device
> tree.
> 
> The keyboard thermal zone defines passive and hot trip points, while
> the charging thermal zone also associates a cooling map with the CPU
> clusters, allowing the generic thermal framework to apply CPU
> throttling when the charging circuitry temperature exceeds the passive
> threshold.
> 
> This integrates the EC temperature sensors with the Linux thermal
> [ ... ]
> 
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on T14s OLED
> Signed-off-by: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707192228.14647-1-daniel.lezcano@oss.qualcomm.com?part=3


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

* Re: [PATCH v6 1/3] dt-bindings: embedded-controller: Add Lenovo ThinkPad T14s thermal sensor provider support
  2026-07-07 19:30   ` sashiko-bot
@ 2026-07-07 21:34     ` Daniel Lezcano
  2026-07-08  6:31       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 18+ messages in thread
From: Daniel Lezcano @ 2026-07-07 21:34 UTC (permalink / raw)
  To: robh, conor+dt, Krzysztof Kozlowski; +Cc: devicetree, linux-hwmon

On 7/7/26 21:30, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The newly added '#thermal-sensor-cells' property is missing from the 'required' list.
> --
> 
> --- Patch [1]: [PATCH v6 1/3] dt-bindings: embedded-controller: Add Lenovo ThinkPad T14s thermal sensor provider support ---
> commit 48ae118884c307667e25a720a62e87b8e32443c5
> Author: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>
> 
> dt-bindings: embedded-controller: Add Lenovo ThinkPad T14s thermal sensor provider support
> 
> This commit documents the Lenovo ThinkPad T14s Embedded Controller as a
> thermal sensor provider by adding the #thermal-sensor-cells property. This
> allows the EC temperature sensors to be referenced from thermal zones.
> 
>> diff --git a/Documentation/devicetree/bindings/embedded-controller/lenovo,thinkpad-t14s-ec.yaml b/Documentation/devicetree/bindings/embedded-controller/lenovo,thinkpad-t14s-ec.yaml
>> --- a/Documentation/devicetree/bindings/embedded-controller/lenovo,thinkpad-t14s-ec.yaml
>> +++ b/Documentation/devicetree/bindings/embedded-controller/lenovo,thinkpad-t14s-ec.yaml
>> @@ -26,6 +26,9 @@ properties:
>>   
>>     wakeup-source: true
>>   
>> +  "#thermal-sensor-cells":
>> +    const: 1
>> +
>>   required:
>>     - compatible
>>     - reg
> 
> [Severity: Medium]
> Should #thermal-sensor-cells be added to this required list?
> 
> When hardware gains new provider capabilities, omitting the corresponding
> standard properties from the required list allows incomplete device tree
> nodes to pass schema validation. Dependent thermal zones could fail at
> runtime if they attempt to reference the embedded controller and the property
> is missing.
> 
> [ ... ]


If I'm not wrong it is a false positive


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

* Re: [PATCH v6 2/3] platform: arm64: lenovo-thinkpad-t14s-ec: Add hwmon support for temperatures
  2026-07-07 19:22 ` [PATCH v6 2/3] platform: arm64: lenovo-thinkpad-t14s-ec: Add hwmon support for temperatures Daniel Lezcano
  2026-07-07 19:28   ` sashiko-bot
@ 2026-07-08  6:30   ` Krzysztof Kozlowski
  2026-07-08  7:31     ` Daniel Lezcano
  1 sibling, 1 reply; 18+ messages in thread
From: Krzysztof Kozlowski @ 2026-07-08  6:30 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: sre, hansg, ilpo.jarvinen, linux, andersson, konradybcio, robh,
	krzk+dt, conor+dt, bryan.odonoghue, platform-driver-x86,
	linux-kernel, linux-hwmon, linux-arm-msm, devicetree,
	neil.armstrong, gaurav.kohli, manaf.pallikunhi, priyansh.jain

On Tue, Jul 07, 2026 at 09:22:27PM +0200, Daniel Lezcano wrote:
> +static const struct hwmon_ops t14s_ec_hwmon_ops = {
> +	.is_visible = t14s_ec_hwmon_is_visible,
> +	.read = t14s_ec_hwmon_read,
> +	.read_string = t14s_ec_hwmon_read_string,
> +};
> +
> +static const struct hwmon_channel_info *t14s_ec_hwmon_info[] = {
> +	HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ),
> +	HWMON_CHANNEL_INFO(temp,
> +			   HWMON_T_INPUT | HWMON_T_LABEL,
> +			   HWMON_T_INPUT | HWMON_T_LABEL,
> +			   HWMON_T_INPUT | HWMON_T_LABEL,
> +			   HWMON_T_INPUT | HWMON_T_LABEL,
> +			   HWMON_T_INPUT | HWMON_T_LABEL,
> +			   HWMON_T_INPUT | HWMON_T_LABEL),
> +	NULL
> +};
> +
> +static const struct hwmon_chip_info t14s_ec_chip_info = {
> +	.ops = &t14s_ec_hwmon_ops,
> +	.info = t14s_ec_hwmon_info,
> +};
> +
> +static int t14s_ec_hwmon_probe(struct t14s_ec *ec)
> +{
> +	struct device *dev;
> +	struct t14s_ec_hwmon_sys_thermx sys_thermx[] = {
> +		{ .label = "soc",	.reg = T14S_EC_SYS_THERM0 },
> +		{ .label = "keyboard",	.reg = T14S_EC_SYS_THERM1 },
> +		{ .label = "base",	.reg = T14S_EC_SYS_THERM2 },
> +		{ .label = "charging",	.reg = T14S_EC_SYS_THERM3 },
> +		{ .label = "qtm",	.reg = T14S_EC_SYS_THERM6 },
> +		{ .label = "ssd",	.reg = T14S_EC_SYS_THERM7 },
> +	};
> +
> +	ec->ec_hwmon.sys_thermx = devm_kmemdup_array(ec->dev, sys_thermx,
> +						     ARRAY_SIZE(sys_thermx),
> +						     sizeof(sys_thermx[0]), GFP_KERNEL);
> +	if (!ec->ec_hwmon.sys_thermx)
> +		return -ENOMEM;
> +
> +	dev = devm_hwmon_device_register_with_info(ec->dev, "t14s_ec", ec,
> +						   &t14s_ec_chip_info, NULL);

Last time I commented this looks like actual ABI break. You did not respond to
that, so I assume my finding was right, thus:
1. binding is not correct (although driver should be fixed, not binding)
2. the code still breaks users

Best regards,
Krzysztof


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

* Re: [PATCH v6 1/3] dt-bindings: embedded-controller: Add Lenovo ThinkPad T14s thermal sensor provider support
  2026-07-07 21:34     ` Daniel Lezcano
@ 2026-07-08  6:31       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 18+ messages in thread
From: Krzysztof Kozlowski @ 2026-07-08  6:31 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: robh, conor+dt, Krzysztof Kozlowski, devicetree, linux-hwmon

On Tue, Jul 07, 2026 at 11:34:53PM +0200, Daniel Lezcano wrote:
> On 7/7/26 21:30, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > - [Medium] The newly added '#thermal-sensor-cells' property is missing from the 'required' list.
> > --
> > 
> > --- Patch [1]: [PATCH v6 1/3] dt-bindings: embedded-controller: Add Lenovo ThinkPad T14s thermal sensor provider support ---
> > commit 48ae118884c307667e25a720a62e87b8e32443c5
> > Author: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>
> > 
> > dt-bindings: embedded-controller: Add Lenovo ThinkPad T14s thermal sensor provider support
> > 
> > This commit documents the Lenovo ThinkPad T14s Embedded Controller as a
> > thermal sensor provider by adding the #thermal-sensor-cells property. This
> > allows the EC temperature sensors to be referenced from thermal zones.
> > 
> > > diff --git a/Documentation/devicetree/bindings/embedded-controller/lenovo,thinkpad-t14s-ec.yaml b/Documentation/devicetree/bindings/embedded-controller/lenovo,thinkpad-t14s-ec.yaml
> > > --- a/Documentation/devicetree/bindings/embedded-controller/lenovo,thinkpad-t14s-ec.yaml
> > > +++ b/Documentation/devicetree/bindings/embedded-controller/lenovo,thinkpad-t14s-ec.yaml
> > > @@ -26,6 +26,9 @@ properties:
> > >     wakeup-source: true
> > > +  "#thermal-sensor-cells":
> > > +    const: 1
> > > +
> > >   required:
> > >     - compatible
> > >     - reg
> > 
> > [Severity: Medium]
> > Should #thermal-sensor-cells be added to this required list?
> > 
> > When hardware gains new provider capabilities, omitting the corresponding
> > standard properties from the required list allows incomplete device tree
> > nodes to pass schema validation. Dependent thermal zones could fail at
> > runtime if they attempt to reference the embedded controller and the property
> > is missing.
> > 
> > [ ... ]
> 
> 
> If I'm not wrong it is a false positive
> 

Looks valid, my previous comment was not addressed, so you did not
document here the ABI which was implemented (at least after quick look
at hwmon code).

Best regards,
Krzysztof


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

* Re: [PATCH v6 2/3] platform: arm64: lenovo-thinkpad-t14s-ec: Add hwmon support for temperatures
  2026-07-08  6:30   ` Krzysztof Kozlowski
@ 2026-07-08  7:31     ` Daniel Lezcano
  2026-07-08  8:24       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 18+ messages in thread
From: Daniel Lezcano @ 2026-07-08  7:31 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: sre, hansg, ilpo.jarvinen, linux, andersson, konradybcio, robh,
	krzk+dt, conor+dt, bryan.odonoghue, platform-driver-x86,
	linux-kernel, linux-hwmon, linux-arm-msm, devicetree,
	neil.armstrong, gaurav.kohli, manaf.pallikunhi, priyansh.jain

On 7/8/26 08:30, Krzysztof Kozlowski wrote:
> On Tue, Jul 07, 2026 at 09:22:27PM +0200, Daniel Lezcano wrote:
>> +static const struct hwmon_ops t14s_ec_hwmon_ops = {
>> +	.is_visible = t14s_ec_hwmon_is_visible,
>> +	.read = t14s_ec_hwmon_read,
>> +	.read_string = t14s_ec_hwmon_read_string,
>> +};
>> +
>> +static const struct hwmon_channel_info *t14s_ec_hwmon_info[] = {
>> +	HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ),
>> +	HWMON_CHANNEL_INFO(temp,
>> +			   HWMON_T_INPUT | HWMON_T_LABEL,
>> +			   HWMON_T_INPUT | HWMON_T_LABEL,
>> +			   HWMON_T_INPUT | HWMON_T_LABEL,
>> +			   HWMON_T_INPUT | HWMON_T_LABEL,
>> +			   HWMON_T_INPUT | HWMON_T_LABEL,
>> +			   HWMON_T_INPUT | HWMON_T_LABEL),
>> +	NULL
>> +};
>> +
>> +static const struct hwmon_chip_info t14s_ec_chip_info = {
>> +	.ops = &t14s_ec_hwmon_ops,
>> +	.info = t14s_ec_hwmon_info,
>> +};
>> +
>> +static int t14s_ec_hwmon_probe(struct t14s_ec *ec)
>> +{
>> +	struct device *dev;
>> +	struct t14s_ec_hwmon_sys_thermx sys_thermx[] = {
>> +		{ .label = "soc",	.reg = T14S_EC_SYS_THERM0 },
>> +		{ .label = "keyboard",	.reg = T14S_EC_SYS_THERM1 },
>> +		{ .label = "base",	.reg = T14S_EC_SYS_THERM2 },
>> +		{ .label = "charging",	.reg = T14S_EC_SYS_THERM3 },
>> +		{ .label = "qtm",	.reg = T14S_EC_SYS_THERM6 },
>> +		{ .label = "ssd",	.reg = T14S_EC_SYS_THERM7 },
>> +	};
>> +
>> +	ec->ec_hwmon.sys_thermx = devm_kmemdup_array(ec->dev, sys_thermx,
>> +						     ARRAY_SIZE(sys_thermx),
>> +						     sizeof(sys_thermx[0]), GFP_KERNEL);
>> +	if (!ec->ec_hwmon.sys_thermx)
>> +		return -ENOMEM;
>> +
>> +	dev = devm_hwmon_device_register_with_info(ec->dev, "t14s_ec", ec,
>> +						   &t14s_ec_chip_info, NULL);
> 
> Last time I commented this looks like actual ABI break. You did not respond to
> that, so I assume my finding was right, thus:
> 1. binding is not correct (although driver should be fixed, not binding)
> 2. the code still breaks users

Sorry I misunderstood it was a question expecting an answer but an 
affirmation.

By ABI break do you mean:

  * new driver with old DT will fail ?
  * new DT with old driver will break ?

* If no thermal-cells is specified, new code will fail with -EINVAL
   -> is that one a problem ?

* If thermal-cells is specified, but no thermal zone, new code does not fail

* If thermal-cells is specified, old code does not fail

* If thermal-cells is not specified, but a thermal zone is specified, 
schema is not respected

Can you clarify ?

Thanks




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

* Re: [PATCH v6 2/3] platform: arm64: lenovo-thinkpad-t14s-ec: Add hwmon support for temperatures
  2026-07-08  7:31     ` Daniel Lezcano
@ 2026-07-08  8:24       ` Krzysztof Kozlowski
  2026-07-08  8:38         ` Daniel Lezcano
  0 siblings, 1 reply; 18+ messages in thread
From: Krzysztof Kozlowski @ 2026-07-08  8:24 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: sre, hansg, ilpo.jarvinen, linux, andersson, konradybcio, robh,
	krzk+dt, conor+dt, bryan.odonoghue, platform-driver-x86,
	linux-kernel, linux-hwmon, linux-arm-msm, devicetree,
	neil.armstrong, gaurav.kohli, manaf.pallikunhi, priyansh.jain

On 08/07/2026 09:31, Daniel Lezcano wrote:
> On 7/8/26 08:30, Krzysztof Kozlowski wrote:
>> On Tue, Jul 07, 2026 at 09:22:27PM +0200, Daniel Lezcano wrote:
>>> +static const struct hwmon_ops t14s_ec_hwmon_ops = {
>>> +	.is_visible = t14s_ec_hwmon_is_visible,
>>> +	.read = t14s_ec_hwmon_read,
>>> +	.read_string = t14s_ec_hwmon_read_string,
>>> +};
>>> +
>>> +static const struct hwmon_channel_info *t14s_ec_hwmon_info[] = {
>>> +	HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ),
>>> +	HWMON_CHANNEL_INFO(temp,
>>> +			   HWMON_T_INPUT | HWMON_T_LABEL,
>>> +			   HWMON_T_INPUT | HWMON_T_LABEL,
>>> +			   HWMON_T_INPUT | HWMON_T_LABEL,
>>> +			   HWMON_T_INPUT | HWMON_T_LABEL,
>>> +			   HWMON_T_INPUT | HWMON_T_LABEL,
>>> +			   HWMON_T_INPUT | HWMON_T_LABEL),
>>> +	NULL
>>> +};
>>> +
>>> +static const struct hwmon_chip_info t14s_ec_chip_info = {
>>> +	.ops = &t14s_ec_hwmon_ops,
>>> +	.info = t14s_ec_hwmon_info,
>>> +};
>>> +
>>> +static int t14s_ec_hwmon_probe(struct t14s_ec *ec)
>>> +{
>>> +	struct device *dev;
>>> +	struct t14s_ec_hwmon_sys_thermx sys_thermx[] = {
>>> +		{ .label = "soc",	.reg = T14S_EC_SYS_THERM0 },
>>> +		{ .label = "keyboard",	.reg = T14S_EC_SYS_THERM1 },
>>> +		{ .label = "base",	.reg = T14S_EC_SYS_THERM2 },
>>> +		{ .label = "charging",	.reg = T14S_EC_SYS_THERM3 },
>>> +		{ .label = "qtm",	.reg = T14S_EC_SYS_THERM6 },
>>> +		{ .label = "ssd",	.reg = T14S_EC_SYS_THERM7 },
>>> +	};
>>> +
>>> +	ec->ec_hwmon.sys_thermx = devm_kmemdup_array(ec->dev, sys_thermx,
>>> +						     ARRAY_SIZE(sys_thermx),
>>> +						     sizeof(sys_thermx[0]), GFP_KERNEL);
>>> +	if (!ec->ec_hwmon.sys_thermx)
>>> +		return -ENOMEM;
>>> +
>>> +	dev = devm_hwmon_device_register_with_info(ec->dev, "t14s_ec", ec,
>>> +						   &t14s_ec_chip_info, NULL);
>>
>> Last time I commented this looks like actual ABI break. You did not respond to
>> that, so I assume my finding was right, thus:
>> 1. binding is not correct (although driver should be fixed, not binding)
>> 2. the code still breaks users
> 
> Sorry I misunderstood it was a question expecting an answer but an 
> affirmation.
> 
> By ABI break do you mean:

Existing ABI contract is changed in an incompatible way, which has
observable effects as below:
> 
>   * new driver with old DT will fail ?

^^^ this one.


Best regards,
Krzysztof

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

* Re: [PATCH v6 2/3] platform: arm64: lenovo-thinkpad-t14s-ec: Add hwmon support for temperatures
  2026-07-08  8:24       ` Krzysztof Kozlowski
@ 2026-07-08  8:38         ` Daniel Lezcano
  2026-07-08 11:48           ` Dmitry Baryshkov
  0 siblings, 1 reply; 18+ messages in thread
From: Daniel Lezcano @ 2026-07-08  8:38 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: sre, hansg, ilpo.jarvinen, linux, andersson, konradybcio, robh,
	krzk+dt, conor+dt, bryan.odonoghue, platform-driver-x86,
	linux-kernel, linux-hwmon, linux-arm-msm, devicetree,
	neil.armstrong, gaurav.kohli, manaf.pallikunhi, priyansh.jain



Le 08/07/2026 à 10:24, Krzysztof Kozlowski a écrit :
> On 08/07/2026 09:31, Daniel Lezcano wrote:
>> On 7/8/26 08:30, Krzysztof Kozlowski wrote:
>>> On Tue, Jul 07, 2026 at 09:22:27PM +0200, Daniel Lezcano wrote:
>>>> +static const struct hwmon_ops t14s_ec_hwmon_ops = {
>>>> +	.is_visible = t14s_ec_hwmon_is_visible,
>>>> +	.read = t14s_ec_hwmon_read,
>>>> +	.read_string = t14s_ec_hwmon_read_string,
>>>> +};
>>>> +
>>>> +static const struct hwmon_channel_info *t14s_ec_hwmon_info[] = {
>>>> +	HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ),
>>>> +	HWMON_CHANNEL_INFO(temp,
>>>> +			   HWMON_T_INPUT | HWMON_T_LABEL,
>>>> +			   HWMON_T_INPUT | HWMON_T_LABEL,
>>>> +			   HWMON_T_INPUT | HWMON_T_LABEL,
>>>> +			   HWMON_T_INPUT | HWMON_T_LABEL,
>>>> +			   HWMON_T_INPUT | HWMON_T_LABEL,
>>>> +			   HWMON_T_INPUT | HWMON_T_LABEL),
>>>> +	NULL
>>>> +};
>>>> +
>>>> +static const struct hwmon_chip_info t14s_ec_chip_info = {
>>>> +	.ops = &t14s_ec_hwmon_ops,
>>>> +	.info = t14s_ec_hwmon_info,
>>>> +};
>>>> +
>>>> +static int t14s_ec_hwmon_probe(struct t14s_ec *ec)
>>>> +{
>>>> +	struct device *dev;
>>>> +	struct t14s_ec_hwmon_sys_thermx sys_thermx[] = {
>>>> +		{ .label = "soc",	.reg = T14S_EC_SYS_THERM0 },
>>>> +		{ .label = "keyboard",	.reg = T14S_EC_SYS_THERM1 },
>>>> +		{ .label = "base",	.reg = T14S_EC_SYS_THERM2 },
>>>> +		{ .label = "charging",	.reg = T14S_EC_SYS_THERM3 },
>>>> +		{ .label = "qtm",	.reg = T14S_EC_SYS_THERM6 },
>>>> +		{ .label = "ssd",	.reg = T14S_EC_SYS_THERM7 },
>>>> +	};
>>>> +
>>>> +	ec->ec_hwmon.sys_thermx = devm_kmemdup_array(ec->dev, sys_thermx,
>>>> +						     ARRAY_SIZE(sys_thermx),
>>>> +						     sizeof(sys_thermx[0]), GFP_KERNEL);
>>>> +	if (!ec->ec_hwmon.sys_thermx)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	dev = devm_hwmon_device_register_with_info(ec->dev, "t14s_ec", ec,
>>>> +						   &t14s_ec_chip_info, NULL);
>>>
>>> Last time I commented this looks like actual ABI break. You did not respond to
>>> that, so I assume my finding was right, thus:
>>> 1. binding is not correct (although driver should be fixed, not binding)
>>> 2. the code still breaks users
>>
>> Sorry I misunderstood it was a question expecting an answer but an
>> affirmation.
>>
>> By ABI break do you mean:
> 
> Existing ABI contract is changed in an incompatible way, which has
> observable effects as below:
>>
>>    * new driver with old DT will fail ?
> 
> ^^^ this one.

When we are incrementally adding features to a platform and like here 
reverse engineering the device protocol of the EC. It is difficult to 
give a complete bindings as we are discovering things little by little. 
How to prevent handling all versions in the code when providing 
iterative completion in the bindings ?

Is it possible to have something like sysfs?
	Documentation/ABI/[testing|stable|removed|obsolete]

but for DT bindings ?



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

* Re: [PATCH v6 2/3] platform: arm64: lenovo-thinkpad-t14s-ec: Add hwmon support for temperatures
  2026-07-08  8:38         ` Daniel Lezcano
@ 2026-07-08 11:48           ` Dmitry Baryshkov
  0 siblings, 0 replies; 18+ messages in thread
From: Dmitry Baryshkov @ 2026-07-08 11:48 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Krzysztof Kozlowski, sre, hansg, ilpo.jarvinen, linux, andersson,
	konradybcio, robh, krzk+dt, conor+dt, bryan.odonoghue,
	platform-driver-x86, linux-kernel, linux-hwmon, linux-arm-msm,
	devicetree, neil.armstrong, gaurav.kohli, manaf.pallikunhi,
	priyansh.jain

On Wed, Jul 08, 2026 at 10:38:49AM +0200, Daniel Lezcano wrote:
> 
> 
> Le 08/07/2026 à 10:24, Krzysztof Kozlowski a écrit :
> > On 08/07/2026 09:31, Daniel Lezcano wrote:
> > > On 7/8/26 08:30, Krzysztof Kozlowski wrote:
> > > > On Tue, Jul 07, 2026 at 09:22:27PM +0200, Daniel Lezcano wrote:
> > > > > +static const struct hwmon_ops t14s_ec_hwmon_ops = {
> > > > > +	.is_visible = t14s_ec_hwmon_is_visible,
> > > > > +	.read = t14s_ec_hwmon_read,
> > > > > +	.read_string = t14s_ec_hwmon_read_string,
> > > > > +};
> > > > > +
> > > > > +static const struct hwmon_channel_info *t14s_ec_hwmon_info[] = {
> > > > > +	HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ),
> > > > > +	HWMON_CHANNEL_INFO(temp,
> > > > > +			   HWMON_T_INPUT | HWMON_T_LABEL,
> > > > > +			   HWMON_T_INPUT | HWMON_T_LABEL,
> > > > > +			   HWMON_T_INPUT | HWMON_T_LABEL,
> > > > > +			   HWMON_T_INPUT | HWMON_T_LABEL,
> > > > > +			   HWMON_T_INPUT | HWMON_T_LABEL,
> > > > > +			   HWMON_T_INPUT | HWMON_T_LABEL),
> > > > > +	NULL
> > > > > +};
> > > > > +
> > > > > +static const struct hwmon_chip_info t14s_ec_chip_info = {
> > > > > +	.ops = &t14s_ec_hwmon_ops,
> > > > > +	.info = t14s_ec_hwmon_info,
> > > > > +};
> > > > > +
> > > > > +static int t14s_ec_hwmon_probe(struct t14s_ec *ec)
> > > > > +{
> > > > > +	struct device *dev;
> > > > > +	struct t14s_ec_hwmon_sys_thermx sys_thermx[] = {
> > > > > +		{ .label = "soc",	.reg = T14S_EC_SYS_THERM0 },
> > > > > +		{ .label = "keyboard",	.reg = T14S_EC_SYS_THERM1 },
> > > > > +		{ .label = "base",	.reg = T14S_EC_SYS_THERM2 },
> > > > > +		{ .label = "charging",	.reg = T14S_EC_SYS_THERM3 },
> > > > > +		{ .label = "qtm",	.reg = T14S_EC_SYS_THERM6 },
> > > > > +		{ .label = "ssd",	.reg = T14S_EC_SYS_THERM7 },
> > > > > +	};
> > > > > +
> > > > > +	ec->ec_hwmon.sys_thermx = devm_kmemdup_array(ec->dev, sys_thermx,
> > > > > +						     ARRAY_SIZE(sys_thermx),
> > > > > +						     sizeof(sys_thermx[0]), GFP_KERNEL);
> > > > > +	if (!ec->ec_hwmon.sys_thermx)
> > > > > +		return -ENOMEM;
> > > > > +
> > > > > +	dev = devm_hwmon_device_register_with_info(ec->dev, "t14s_ec", ec,
> > > > > +						   &t14s_ec_chip_info, NULL);
> > > > 
> > > > Last time I commented this looks like actual ABI break. You did not respond to
> > > > that, so I assume my finding was right, thus:
> > > > 1. binding is not correct (although driver should be fixed, not binding)
> > > > 2. the code still breaks users
> > > 
> > > Sorry I misunderstood it was a question expecting an answer but an
> > > affirmation.
> > > 
> > > By ABI break do you mean:
> > 
> > Existing ABI contract is changed in an incompatible way, which has
> > observable effects as below:
> > > 
> > >    * new driver with old DT will fail ?
> > 
> > ^^^ this one.
> 
> When we are incrementally adding features to a platform and like here
> reverse engineering the device protocol of the EC. It is difficult to give a
> complete bindings as we are discovering things little by little. How to
> prevent handling all versions in the code when providing iterative
> completion in the bindings ?

Provide backwards compatibility. Here you can check for th property to
be present in DT.


-- 
With best wishes
Dmitry

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

* Re: [PATCH v6 0/3] Lenovo ThinkPad T14s EC thermal monitoring and thermal zone integration
  2026-07-07 19:22 [PATCH v6 0/3] Lenovo ThinkPad T14s EC thermal monitoring and thermal zone integration Daniel Lezcano
                   ` (2 preceding siblings ...)
  2026-07-07 19:22 ` [PATCH v6 3/3] arm64: dts: qcom: x1e78100-t14s: Add thermal zones for keyboard skin and charging sensors Daniel Lezcano
@ 2026-07-08 12:03 ` Stephan Gerhold
  2026-07-08 13:16   ` Daniel Lezcano
  3 siblings, 1 reply; 18+ messages in thread
From: Stephan Gerhold @ 2026-07-08 12:03 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: sre, hansg, ilpo.jarvinen, linux, andersson, konradybcio, robh,
	krzk+dt, conor+dt, bryan.odonoghue, platform-driver-x86,
	linux-kernel, linux-hwmon, linux-arm-msm, devicetree,
	neil.armstrong, gaurav.kohli, manaf.pallikunhi, priyansh.jain

On Tue, Jul 07, 2026 at 09:22:25PM +0200, Daniel Lezcano wrote:
> This series extends the Lenovo ThinkPad T14s embedded controller driver
> with environmental monitoring capabilities and integrates the exposed
> sensors into the Linux thermal framework.
> 
> The EC provides access to several platform temperature sensors
> covering the SoC, keyboard area, bottom cover, charging circuitry, QTM
> module and SSD. These sensors are currently used by the firmware for
> thermal management but are not exposed to Linux.
> 
> The first patch adds hwmon support for the EC temperature sensors.
> 
> The second patch exposes the EC as a thermal sensor provider in the
> device tree and defines thermal zones for the keyboard skin
> temperature and the charging circuitry temperature. This allows the
> generic thermal framework to react to EC-reported temperatures and
> apply standard Linux thermal mitigation policies.
> 
> As the EC protocol is not fully decoded, the passive trip points
> get/set actions are missing, so it is not possible to program a
> threshold and receive an interrupt when crossed the way up or
> down. Consequently, the thermal zone related to the charging circuitry
> is polled every two seconds until we can set the trip points in the
> EC.
> 
> This series fixes critical thermal issues happening on this platform
> where a kernel compilation, or heavy workloads, lead to a system
> reboot.
> 

Thanks for working on this! I have a few comments/questions about this:

1. EC vs PMIC temperature sensors

AFAIK, the T14s (and actually most X1E laptops) have two sets of
thermistors in each location: One is connected to the PMIC (called
SYS_THERMx), and the other set is connected to the EC.

The SYS_THERMx sensors connected to the PMIC have been enabled for the
T14s already over a year ago [1]. The reason this is not upstream is
that we now been waiting 3 years for the corresponding ADC/thermal code
to land upstream [2]. It seems pretty close now, the ADC part has landed
and there is only the thermal part left [3].

The PMIC thermistor setup is likely going to be similar for most X1E
laptops, so I think it would be preferable to use that instead of the EC
sensors to implement additional temperature throttling. It also supports
interrupts/trip points already, so it doesn't need polling.

The most recent proposed patch actually adds the SYS_THERMx thermal
zones to all X1E-based devices [4], although I'm not sure if it would be
better to keep that device-specific...

[1]: https://github.com/stephan-gh/linux/commit/c0ddc9fa96667d6b32d690ce6a3dcfc76aaabad6
[2]: https://lore.kernel.org/linux-arm-msm/20230708072835.3035398-1-quic_jprakash@quicinc.com/
[3]: https://lore.kernel.org/linux-arm-msm/20260705-gen3_adc_tm-v3-0-ac62f387dbce@oss.qualcomm.com/
[4]: https://lore.kernel.org/linux-arm-msm/20260614-adc5_gen3_dt-v2-4-32ec576c5865@oss.qualcomm.com/

2. EC sensor mapping vs PMIC sensor mapping

In PATCH 2/3 you define:

	{ .label = "soc",	.reg = T14S_EC_SYS_THERM0 },
	{ .label = "keyboard",	.reg = T14S_EC_SYS_THERM1 },
	{ .label = "base",	.reg = T14S_EC_SYS_THERM2 },
	{ .label = "charging",	.reg = T14S_EC_SYS_THERM3 },
	{ .label = "qtm",	.reg = T14S_EC_SYS_THERM6 },
	{ .label = "ssd",	.reg = T14S_EC_SYS_THERM7 },

I'm not sure if this is correct. When comparing this with the data read
from the sensors connected to the PMIC:

| Sensor          | PMIC Channel | EC (hwmon)   | PMIC (thermal)  | Delta    |
|-----------------|--------------|--------------|-----------------|----------|
| SOC             | SYS_THERM0   | 79.0°C       | 78.7°C          | +0.3°C   |
| Keyboard        | SYS_THERM1   | 68.0°C       | 70.1°C          | -2.1°C   |
| Base / Back     | SYS_THERM2   | 66.0°C       | 64.6°C          | +1.4°C   |
| Charging        | SYS_THERM3   | 73.0°C       | 73.8°C          | -0.8°C   |
| West / QTM      | SYS_THERM6   | 64.0°C       | 62.6°C          | +1.4°C   |
| SSD             | SYS_THERM7   | 31.0°C       | 67.1°C          | -36.1°C  |
| Modem           | SYS_THERM4   | N/A          | 31.6°C          | N/A      |
| East            | SYS_THERM5   | N/A          | 70.3°C          | N/A      |

The SSD delta of 36°C is definitely suspicious. I think
"ssd"/T14S_EC_SYS_THERM7 in your EC driver patch is actually the modem
sensor (SYS_THERM4).

If you look at a picture of the T14s mainboard
(https://www.notebookcheck.net/fileadmin/_processed_/d/c/csm_DSC_0003_aadae1ddd2.jpg)
and zoom in to the unpopulated modem sub-board left to the fan you can
see the two thermistors RT601 and RT301. The SSD on the other hand sits
almost directly next to the SoC on the right, so I wouldn't expect it to
stay > 30°C cooler than its surroundings.

However, there are also two thermistors next to the SSD, see e.g. this
close-up picture of the mainboard:
https://download.lenovo.com/Images/Parts/5B21P83385/5B21P83385_A.jpg
This means that the SSD is probably one of the other mappings. If the
thermistors are consecutively numbered in the EC firmware, the SSD
(RT8203) might be actually the third sensor ("base"/T14S_EC_SYS_THERM2).
I'm not sure how to figure out the proper mapping.

The back of the mainboard is completely covered with tape
(https://download.lenovo.com/Images/Parts/5B21P83377/5B21P83377_B.jpg)
so it's impossible to see anything there.

3. Active vs passive throttling

Are you matching the Windows cooling/throttling setup here? If not, have
you considered how this interacts with the fan control applied by the
EC? I'm a bit worried that this might lead to unexpected performance
regressions if we start throttling before the EC runs the fan at full
speed.

Thanks,
Stephan

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

* Re: [PATCH v6 0/3] Lenovo ThinkPad T14s EC thermal monitoring and thermal zone integration
  2026-07-08 12:03 ` [PATCH v6 0/3] Lenovo ThinkPad T14s EC thermal monitoring and thermal zone integration Stephan Gerhold
@ 2026-07-08 13:16   ` Daniel Lezcano
  2026-07-08 13:39     ` Stephan Gerhold
  0 siblings, 1 reply; 18+ messages in thread
From: Daniel Lezcano @ 2026-07-08 13:16 UTC (permalink / raw)
  To: Stephan Gerhold
  Cc: sre, hansg, ilpo.jarvinen, linux, andersson, konradybcio, robh,
	krzk+dt, conor+dt, bryan.odonoghue, platform-driver-x86,
	linux-kernel, linux-hwmon, linux-arm-msm, devicetree,
	neil.armstrong, gaurav.kohli, manaf.pallikunhi, priyansh.jain


Hi Stephan,

On 7/8/26 14:03, Stephan Gerhold wrote:
> On Tue, Jul 07, 2026 at 09:22:25PM +0200, Daniel Lezcano wrote:
>> This series extends the Lenovo ThinkPad T14s embedded controller driver
>> with environmental monitoring capabilities and integrates the exposed
>> sensors into the Linux thermal framework.
>>
>> The EC provides access to several platform temperature sensors
>> covering the SoC, keyboard area, bottom cover, charging circuitry, QTM
>> module and SSD. These sensors are currently used by the firmware for
>> thermal management but are not exposed to Linux.
>>
>> The first patch adds hwmon support for the EC temperature sensors.
>>
>> The second patch exposes the EC as a thermal sensor provider in the
>> device tree and defines thermal zones for the keyboard skin
>> temperature and the charging circuitry temperature. This allows the
>> generic thermal framework to react to EC-reported temperatures and
>> apply standard Linux thermal mitigation policies.
>>
>> As the EC protocol is not fully decoded, the passive trip points
>> get/set actions are missing, so it is not possible to program a
>> threshold and receive an interrupt when crossed the way up or
>> down. Consequently, the thermal zone related to the charging circuitry
>> is polled every two seconds until we can set the trip points in the
>> EC.
>>
>> This series fixes critical thermal issues happening on this platform
>> where a kernel compilation, or heavy workloads, lead to a system
>> reboot.
>>
> 
> Thanks for working on this! I have a few comments/questions about this:

Thanks for jumping in the discussion

> 1. EC vs PMIC temperature sensors
> 
> AFAIK, the T14s (and actually most X1E laptops) have two sets of
> thermistors in each location: One is connected to the PMIC (called
> SYS_THERMx), and the other set is connected to the EC.

 From the schematics I have in my possession, the SYS_THERMx are 
connected to the EC, at least this is how they are named. May be it is 
the other set with different numbers.

I'm curious to know why this mirroring ?

> The SYS_THERMx sensors connected to the PMIC have been enabled for the
> T14s already over a year ago [1]. The reason this is not upstream is
> that we now been waiting 3 years for the corresponding ADC/thermal code
> to land upstream [2]. It seems pretty close now, the ADC part has landed
> and there is only the thermal part left [3].

Right, I have to pick this one.

> The PMIC thermistor setup is likely going to be similar for most X1E
> laptops, so I think it would be preferable to use that instead of the EC
> sensors to implement additional temperature throttling. It also supports
> interrupts/trip points already, so it doesn't need polling.

I definitively second that

> The most recent proposed patch actually adds the SYS_THERMx thermal
> zones to all X1E-based devices [4], although I'm not sure if it would be
> better to keep that device-specific...
> 
> [1]: https://github.com/stephan-gh/linux/commit/c0ddc9fa96667d6b32d690ce6a3dcfc76aaabad6
> [2]: https://lore.kernel.org/linux-arm-msm/20230708072835.3035398-1-quic_jprakash@quicinc.com/
> [3]: https://lore.kernel.org/linux-arm-msm/20260705-gen3_adc_tm-v3-0-ac62f387dbce@oss.qualcomm.com/
> [4]: https://lore.kernel.org/linux-arm-msm/20260614-adc5_gen3_dt-v2-4-32ec576c5865@oss.qualcomm.com/
> 
> 2. EC sensor mapping vs PMIC sensor mapping
> 
> In PATCH 2/3 you define:
> 
> 	{ .label = "soc",	.reg = T14S_EC_SYS_THERM0 },
> 	{ .label = "keyboard",	.reg = T14S_EC_SYS_THERM1 },
> 	{ .label = "base",	.reg = T14S_EC_SYS_THERM2 },
> 	{ .label = "charging",	.reg = T14S_EC_SYS_THERM3 },
> 	{ .label = "qtm",	.reg = T14S_EC_SYS_THERM6 },
> 	{ .label = "ssd",	.reg = T14S_EC_SYS_THERM7 },
> 
> I'm not sure if this is correct. When comparing this with the data read
> from the sensors connected to the PMIC:
> 
> | Sensor          | PMIC Channel | EC (hwmon)   | PMIC (thermal)  | Delta    |
> |-----------------|--------------|--------------|-----------------|----------|
> | SOC             | SYS_THERM0   | 79.0°C       | 78.7°C          | +0.3°C   |
> | Keyboard        | SYS_THERM1   | 68.0°C       | 70.1°C          | -2.1°C   |
> | Base / Back     | SYS_THERM2   | 66.0°C       | 64.6°C          | +1.4°C   |
> | Charging        | SYS_THERM3   | 73.0°C       | 73.8°C          | -0.8°C   |
> | West / QTM      | SYS_THERM6   | 64.0°C       | 62.6°C          | +1.4°C   |
> | SSD             | SYS_THERM7   | 31.0°C       | 67.1°C          | -36.1°C  |
> | Modem           | SYS_THERM4   | N/A          | 31.6°C          | N/A      |
> | East            | SYS_THERM5   | N/A          | 70.3°C          | N/A      |
> 
> The SSD delta of 36°C is definitely suspicious. I think
> "ssd"/T14S_EC_SYS_THERM7 in your EC driver patch is actually the modem
> sensor (SYS_THERM4).

Yes probably, it was unclear from the documentation. I may have mixed 
some info.

The SoC sensor seems to be hotter than the Charging sensor. I'm a bit 
surprised because I've always seen charging hotter than the rest.


> If you look at a picture of the T14s mainboard
> (https://www.notebookcheck.net/fileadmin/_processed_/d/c/csm_DSC_0003_aadae1ddd2.jpg)
> and zoom in to the unpopulated modem sub-board left to the fan you can
> see the two thermistors RT601 and RT301. The SSD on the other hand sits
> almost directly next to the SoC on the right, so I wouldn't expect it to
> stay > 30°C cooler than its surroundings.
> 
> However, there are also two thermistors next to the SSD, see e.g. this
> close-up picture of the mainboard:
> https://download.lenovo.com/Images/Parts/5B21P83385/5B21P83385_A.jpg
> This means that the SSD is probably one of the other mappings. If the
> thermistors are consecutively numbered in the EC firmware, the SSD
> (RT8203) might be actually the third sensor ("base"/T14S_EC_SYS_THERM2).
> I'm not sure how to figure out the proper mapping.
> 
> The back of the mainboard is completely covered with tape
> (https://download.lenovo.com/Images/Parts/5B21P83377/5B21P83377_B.jpg)
> so it's impossible to see anything there.
> 
> 3. Active vs passive throttling
> 
> Are you matching the Windows cooling/throttling setup here? If not, have
> you considered how this interacts with the fan control applied by the
> EC? I'm a bit worried that this might lead to unexpected performance
> regressions if we start throttling before the EC runs the fan at full
> speed.

For the moment, I would say fixing the critical issue is the highest 
priority. With the 'charging' sensor with a 55°C trip point, AFAICT the 
fan is at full speed before this trip is reached.

Then we may want to take control of the fan and add active trip and 
passive trip.

This autonomous EC / PMIC mix sounds a bit strange to me :/



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

* Re: [PATCH v6 0/3] Lenovo ThinkPad T14s EC thermal monitoring and thermal zone integration
  2026-07-08 13:16   ` Daniel Lezcano
@ 2026-07-08 13:39     ` Stephan Gerhold
  2026-07-08 14:09       ` Daniel Lezcano
  0 siblings, 1 reply; 18+ messages in thread
From: Stephan Gerhold @ 2026-07-08 13:39 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: sre, hansg, ilpo.jarvinen, linux, andersson, konradybcio, robh,
	krzk+dt, conor+dt, bryan.odonoghue, platform-driver-x86,
	linux-kernel, linux-hwmon, linux-arm-msm, devicetree,
	neil.armstrong, gaurav.kohli, manaf.pallikunhi, priyansh.jain

On Wed, Jul 08, 2026 at 03:16:20PM +0200, Daniel Lezcano wrote:
> On 7/8/26 14:03, Stephan Gerhold wrote:
> > On Tue, Jul 07, 2026 at 09:22:25PM +0200, Daniel Lezcano wrote:
> > > This series extends the Lenovo ThinkPad T14s embedded controller driver
> > > with environmental monitoring capabilities and integrates the exposed
> > > sensors into the Linux thermal framework.
> > > 
> > > The EC provides access to several platform temperature sensors
> > > covering the SoC, keyboard area, bottom cover, charging circuitry, QTM
> > > module and SSD. These sensors are currently used by the firmware for
> > > thermal management but are not exposed to Linux.
> > > 
> > > The first patch adds hwmon support for the EC temperature sensors.
> > > 
> > > The second patch exposes the EC as a thermal sensor provider in the
> > > device tree and defines thermal zones for the keyboard skin
> > > temperature and the charging circuitry temperature. This allows the
> > > generic thermal framework to react to EC-reported temperatures and
> > > apply standard Linux thermal mitigation policies.
> > > 
> > > As the EC protocol is not fully decoded, the passive trip points
> > > get/set actions are missing, so it is not possible to program a
> > > threshold and receive an interrupt when crossed the way up or
> > > down. Consequently, the thermal zone related to the charging circuitry
> > > is polled every two seconds until we can set the trip points in the
> > > EC.
> > > 
> > > This series fixes critical thermal issues happening on this platform
> > > where a kernel compilation, or heavy workloads, lead to a system
> > > reboot.
> > > 
> > 
> > Thanks for working on this! I have a few comments/questions about this:
> 
> Thanks for jumping in the discussion
> 
> > 1. EC vs PMIC temperature sensors
> > 
> > AFAIK, the T14s (and actually most X1E laptops) have two sets of
> > thermistors in each location: One is connected to the PMIC (called
> > SYS_THERMx), and the other set is connected to the EC.
> 
> From the schematics I have in my possession, the SYS_THERMx are connected to
> the EC, at least this is how they are named. May be it is the other set with
> different numbers.
> 
> I'm curious to know why this mirroring ?
> 

AFAICT the SYS_THERMx thermistors all go to the PMC838C PMIC on the
T14s. The EC thermistors are on a separate page, search for "RT8203" for
example.

I'm not sure why this mirroring exists, but even QC's Hamoa/X1E CRD has
that. IIRC it is not always there though, the Purwa/X1P CRD has the
sensors only connected to the EC (maybe for some minor cost savings?).

I believe on some devices the OS (Windows/Linux) even needs to send
temperatures (e.g. CPU/GPU temperature) to the EC for it to work
correctly. Not sure if the T14s needs/uses that though. I was mostly
looking at the CRD thermal setup a year ago...

> > The SYS_THERMx sensors connected to the PMIC have been enabled for the
> > T14s already over a year ago [1]. The reason this is not upstream is
> > that we now been waiting 3 years for the corresponding ADC/thermal code
> > to land upstream [2]. It seems pretty close now, the ADC part has landed
> > and there is only the thermal part left [3].
> 
> Right, I have to pick this one.
> 
> > The PMIC thermistor setup is likely going to be similar for most X1E
> > laptops, so I think it would be preferable to use that instead of the EC
> > sensors to implement additional temperature throttling. It also supports
> > interrupts/trip points already, so it doesn't need polling.
> 
> I definitively second that
> 
> > The most recent proposed patch actually adds the SYS_THERMx thermal
> > zones to all X1E-based devices [4], although I'm not sure if it would be
> > better to keep that device-specific...
> > 
> > [1]: https://github.com/stephan-gh/linux/commit/c0ddc9fa96667d6b32d690ce6a3dcfc76aaabad6
> > [2]: https://lore.kernel.org/linux-arm-msm/20230708072835.3035398-1-quic_jprakash@quicinc.com/
> > [3]: https://lore.kernel.org/linux-arm-msm/20260705-gen3_adc_tm-v3-0-ac62f387dbce@oss.qualcomm.com/
> > [4]: https://lore.kernel.org/linux-arm-msm/20260614-adc5_gen3_dt-v2-4-32ec576c5865@oss.qualcomm.com/
> > 
> > 2. EC sensor mapping vs PMIC sensor mapping
> > 
> > In PATCH 2/3 you define:
> > 
> > 	{ .label = "soc",	.reg = T14S_EC_SYS_THERM0 },
> > 	{ .label = "keyboard",	.reg = T14S_EC_SYS_THERM1 },
> > 	{ .label = "base",	.reg = T14S_EC_SYS_THERM2 },
> > 	{ .label = "charging",	.reg = T14S_EC_SYS_THERM3 },
> > 	{ .label = "qtm",	.reg = T14S_EC_SYS_THERM6 },
> > 	{ .label = "ssd",	.reg = T14S_EC_SYS_THERM7 },
> > 
> > I'm not sure if this is correct. When comparing this with the data read
> > from the sensors connected to the PMIC:
> > 
> > | Sensor          | PMIC Channel | EC (hwmon)   | PMIC (thermal)  | Delta    |
> > |-----------------|--------------|--------------|-----------------|----------|
> > | SOC             | SYS_THERM0   | 79.0°C       | 78.7°C          | +0.3°C   |
> > | Keyboard        | SYS_THERM1   | 68.0°C       | 70.1°C          | -2.1°C   |
> > | Base / Back     | SYS_THERM2   | 66.0°C       | 64.6°C          | +1.4°C   |
> > | Charging        | SYS_THERM3   | 73.0°C       | 73.8°C          | -0.8°C   |
> > | West / QTM      | SYS_THERM6   | 64.0°C       | 62.6°C          | +1.4°C   |
> > | SSD             | SYS_THERM7   | 31.0°C       | 67.1°C          | -36.1°C  |
> > | Modem           | SYS_THERM4   | N/A          | 31.6°C          | N/A      |
> > | East            | SYS_THERM5   | N/A          | 70.3°C          | N/A      |
> > 
> > The SSD delta of 36°C is definitely suspicious. I think
> > "ssd"/T14S_EC_SYS_THERM7 in your EC driver patch is actually the modem
> > sensor (SYS_THERM4).
> 
> Yes probably, it was unclear from the documentation. I may have mixed some
> info.
> 
> The SoC sensor seems to be hotter than the Charging sensor. I'm a bit
> surprised because I've always seen charging hotter than the rest.
> 

I captured the above while actively stressing the CPU, so presumably the
SoC was heating up more quickly than the charging in that situation.
Just a few minutes earlier it was like this in idle state with charging
indeed warmer than SoC:

| Sensor          | EC (hwmon)   | PMIC (thermal)  | Delta    |
|-----------------|--------------|-----------------|----------|
| SOC             | 35.0°C       | 35.8°C          | -0.8°C   |
| Keyboard        | 32.0°C       | 33.0°C          | -1.0°C   |
| Base / Back     | 33.0°C       | 31.9°C          | +1.1°C   |
| Charging        | 37.0°C       | 38.2°C          | -1.2°C   |
| West / QTM      | 31.0°C       | 35.7°C          | -4.7°C   |
| SSD             | 25.0°C       | 34.0°C          | -9.0°C   |
| Modem           | N/A          | 26.2°C          | N/A      |
| East            | N/A          | 33.1°C          | N/A      |

> 
> > If you look at a picture of the T14s mainboard
> > (https://www.notebookcheck.net/fileadmin/_processed_/d/c/csm_DSC_0003_aadae1ddd2.jpg)
> > and zoom in to the unpopulated modem sub-board left to the fan you can
> > see the two thermistors RT601 and RT301. The SSD on the other hand sits
> > almost directly next to the SoC on the right, so I wouldn't expect it to
> > stay > 30°C cooler than its surroundings.
> > 
> > However, there are also two thermistors next to the SSD, see e.g. this
> > close-up picture of the mainboard:
> > https://download.lenovo.com/Images/Parts/5B21P83385/5B21P83385_A.jpg
> > This means that the SSD is probably one of the other mappings. If the
> > thermistors are consecutively numbered in the EC firmware, the SSD
> > (RT8203) might be actually the third sensor ("base"/T14S_EC_SYS_THERM2).
> > I'm not sure how to figure out the proper mapping.
> > 
> > The back of the mainboard is completely covered with tape
> > (https://download.lenovo.com/Images/Parts/5B21P83377/5B21P83377_B.jpg)
> > so it's impossible to see anything there.
> > 
> > 3. Active vs passive throttling
> > 
> > Are you matching the Windows cooling/throttling setup here? If not, have
> > you considered how this interacts with the fan control applied by the
> > EC? I'm a bit worried that this might lead to unexpected performance
> > regressions if we start throttling before the EC runs the fan at full
> > speed.
> 
> For the moment, I would say fixing the critical issue is the highest
> priority. With the 'charging' sensor with a 55°C trip point, AFAICT the fan
> is at full speed before this trip is reached.

Agreed.

I'm not sure about the fan speed, I never checked properly but my gut
feeling is that the fan runs faster in Windows and never runs full speed
in Linux. (Are there fan profiles maybe? Not sure. Maybe I'm imagining
it. :-))

> 
> Then we may want to take control of the fan and add active trip and passive
> trip.
> 
> This autonomous EC / PMIC mix sounds a bit strange to me :/
> 

Right, it's all quite complicated. :/ Ideally, we would be able to
figure out the exact thresholds for the fan control in the EC firmware,
so that we can set the throttling temperature to be shortly above that..

Thanks,
Stephan

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

* Re: [PATCH v6 0/3] Lenovo ThinkPad T14s EC thermal monitoring and thermal zone integration
  2026-07-08 13:39     ` Stephan Gerhold
@ 2026-07-08 14:09       ` Daniel Lezcano
  0 siblings, 0 replies; 18+ messages in thread
From: Daniel Lezcano @ 2026-07-08 14:09 UTC (permalink / raw)
  To: Stephan Gerhold
  Cc: sre, hansg, ilpo.jarvinen, linux, andersson, konradybcio, robh,
	krzk+dt, conor+dt, bryan.odonoghue, platform-driver-x86,
	linux-kernel, linux-hwmon, linux-arm-msm, devicetree,
	neil.armstrong, gaurav.kohli, manaf.pallikunhi, priyansh.jain

On 7/8/26 15:39, Stephan Gerhold wrote:
> On Wed, Jul 08, 2026 at 03:16:20PM +0200, Daniel Lezcano wrote:
>> On 7/8/26 14:03, Stephan Gerhold wrote:
>>> On Tue, Jul 07, 2026 at 09:22:25PM +0200, Daniel Lezcano wrote:
>>>> This series extends the Lenovo ThinkPad T14s embedded controller driver
>>>> with environmental monitoring capabilities and integrates the exposed
>>>> sensors into the Linux thermal framework.
>>>>
>>>> The EC provides access to several platform temperature sensors
>>>> covering the SoC, keyboard area, bottom cover, charging circuitry, QTM
>>>> module and SSD. These sensors are currently used by the firmware for
>>>> thermal management but are not exposed to Linux.
>>>>
>>>> The first patch adds hwmon support for the EC temperature sensors.
>>>>
>>>> The second patch exposes the EC as a thermal sensor provider in the
>>>> device tree and defines thermal zones for the keyboard skin
>>>> temperature and the charging circuitry temperature. This allows the
>>>> generic thermal framework to react to EC-reported temperatures and
>>>> apply standard Linux thermal mitigation policies.
>>>>
>>>> As the EC protocol is not fully decoded, the passive trip points
>>>> get/set actions are missing, so it is not possible to program a
>>>> threshold and receive an interrupt when crossed the way up or
>>>> down. Consequently, the thermal zone related to the charging circuitry
>>>> is polled every two seconds until we can set the trip points in the
>>>> EC.
>>>>
>>>> This series fixes critical thermal issues happening on this platform
>>>> where a kernel compilation, or heavy workloads, lead to a system
>>>> reboot.
>>>>
>>>
>>> Thanks for working on this! I have a few comments/questions about this:
>>
>> Thanks for jumping in the discussion
>>
>>> 1. EC vs PMIC temperature sensors
>>>
>>> AFAIK, the T14s (and actually most X1E laptops) have two sets of
>>> thermistors in each location: One is connected to the PMIC (called
>>> SYS_THERMx), and the other set is connected to the EC.
>>
>>  From the schematics I have in my possession, the SYS_THERMx are connected to
>> the EC, at least this is how they are named. May be it is the other set with
>> different numbers.
>>
>> I'm curious to know why this mirroring ?
>>
> 
> AFAICT the SYS_THERMx thermistors all go to the PMC838C PMIC on the
> T14s. The EC thermistors are on a separate page, search for "RT8203" for
> example.
> 
> I'm not sure why this mirroring exists, but even QC's Hamoa/X1E CRD has
> that. IIRC it is not always there though, the Purwa/X1P CRD has the
> sensors only connected to the EC (maybe for some minor cost savings?).
> 
> I believe on some devices the OS (Windows/Linux) even needs to send
> temperatures (e.g. CPU/GPU temperature) to the EC for it to work
> correctly. Not sure if the T14s needs/uses that though. I was mostly
> looking at the CRD thermal setup a year ago...
> 
>>> The SYS_THERMx sensors connected to the PMIC have been enabled for the
>>> T14s already over a year ago [1]. The reason this is not upstream is
>>> that we now been waiting 3 years for the corresponding ADC/thermal code
>>> to land upstream [2]. It seems pretty close now, the ADC part has landed
>>> and there is only the thermal part left [3].
>>
>> Right, I have to pick this one.
>>
>>> The PMIC thermistor setup is likely going to be similar for most X1E
>>> laptops, so I think it would be preferable to use that instead of the EC
>>> sensors to implement additional temperature throttling. It also supports
>>> interrupts/trip points already, so it doesn't need polling.
>>
>> I definitively second that
>>
>>> The most recent proposed patch actually adds the SYS_THERMx thermal
>>> zones to all X1E-based devices [4], although I'm not sure if it would be
>>> better to keep that device-specific...
>>>
>>> [1]: https://github.com/stephan-gh/linux/commit/c0ddc9fa96667d6b32d690ce6a3dcfc76aaabad6
>>> [2]: https://lore.kernel.org/linux-arm-msm/20230708072835.3035398-1-quic_jprakash@quicinc.com/
>>> [3]: https://lore.kernel.org/linux-arm-msm/20260705-gen3_adc_tm-v3-0-ac62f387dbce@oss.qualcomm.com/
>>> [4]: https://lore.kernel.org/linux-arm-msm/20260614-adc5_gen3_dt-v2-4-32ec576c5865@oss.qualcomm.com/
>>>
>>> 2. EC sensor mapping vs PMIC sensor mapping
>>>
>>> In PATCH 2/3 you define:
>>>
>>> 	{ .label = "soc",	.reg = T14S_EC_SYS_THERM0 },
>>> 	{ .label = "keyboard",	.reg = T14S_EC_SYS_THERM1 },
>>> 	{ .label = "base",	.reg = T14S_EC_SYS_THERM2 },
>>> 	{ .label = "charging",	.reg = T14S_EC_SYS_THERM3 },
>>> 	{ .label = "qtm",	.reg = T14S_EC_SYS_THERM6 },
>>> 	{ .label = "ssd",	.reg = T14S_EC_SYS_THERM7 },
>>>
>>> I'm not sure if this is correct. When comparing this with the data read
>>> from the sensors connected to the PMIC:
>>>
>>> | Sensor          | PMIC Channel | EC (hwmon)   | PMIC (thermal)  | Delta    |
>>> |-----------------|--------------|--------------|-----------------|----------|
>>> | SOC             | SYS_THERM0   | 79.0°C       | 78.7°C          | +0.3°C   |
>>> | Keyboard        | SYS_THERM1   | 68.0°C       | 70.1°C          | -2.1°C   |
>>> | Base / Back     | SYS_THERM2   | 66.0°C       | 64.6°C          | +1.4°C   |
>>> | Charging        | SYS_THERM3   | 73.0°C       | 73.8°C          | -0.8°C   |
>>> | West / QTM      | SYS_THERM6   | 64.0°C       | 62.6°C          | +1.4°C   |
>>> | SSD             | SYS_THERM7   | 31.0°C       | 67.1°C          | -36.1°C  |
>>> | Modem           | SYS_THERM4   | N/A          | 31.6°C          | N/A      |
>>> | East            | SYS_THERM5   | N/A          | 70.3°C          | N/A      |
>>>
>>> The SSD delta of 36°C is definitely suspicious. I think
>>> "ssd"/T14S_EC_SYS_THERM7 in your EC driver patch is actually the modem
>>> sensor (SYS_THERM4).
>>
>> Yes probably, it was unclear from the documentation. I may have mixed some
>> info.
>>
>> The SoC sensor seems to be hotter than the Charging sensor. I'm a bit
>> surprised because I've always seen charging hotter than the rest.
>>
> 
> I captured the above while actively stressing the CPU, so presumably the
> SoC was heating up more quickly than the charging in that situation.
> Just a few minutes earlier it was like this in idle state with charging
> indeed warmer than SoC:
> 
> | Sensor          | EC (hwmon)   | PMIC (thermal)  | Delta    |
> |-----------------|--------------|-----------------|----------|
> | SOC             | 35.0°C       | 35.8°C          | -0.8°C   |
> | Keyboard        | 32.0°C       | 33.0°C          | -1.0°C   |
> | Base / Back     | 33.0°C       | 31.9°C          | +1.1°C   |
> | Charging        | 37.0°C       | 38.2°C          | -1.2°C   |
> | West / QTM      | 31.0°C       | 35.7°C          | -4.7°C   |
> | SSD             | 25.0°C       | 34.0°C          | -9.0°C   |
> | Modem           | N/A          | 26.2°C          | N/A      |
> | East            | N/A          | 33.1°C          | N/A      |
> 
>>
>>> If you look at a picture of the T14s mainboard
>>> (https://www.notebookcheck.net/fileadmin/_processed_/d/c/csm_DSC_0003_aadae1ddd2.jpg)
>>> and zoom in to the unpopulated modem sub-board left to the fan you can
>>> see the two thermistors RT601 and RT301. The SSD on the other hand sits
>>> almost directly next to the SoC on the right, so I wouldn't expect it to
>>> stay > 30°C cooler than its surroundings.
>>>
>>> However, there are also two thermistors next to the SSD, see e.g. this
>>> close-up picture of the mainboard:
>>> https://download.lenovo.com/Images/Parts/5B21P83385/5B21P83385_A.jpg
>>> This means that the SSD is probably one of the other mappings. If the
>>> thermistors are consecutively numbered in the EC firmware, the SSD
>>> (RT8203) might be actually the third sensor ("base"/T14S_EC_SYS_THERM2).
>>> I'm not sure how to figure out the proper mapping.
>>>
>>> The back of the mainboard is completely covered with tape
>>> (https://download.lenovo.com/Images/Parts/5B21P83377/5B21P83377_B.jpg)
>>> so it's impossible to see anything there.
>>>
>>> 3. Active vs passive throttling
>>>
>>> Are you matching the Windows cooling/throttling setup here? If not, have
>>> you considered how this interacts with the fan control applied by the
>>> EC? I'm a bit worried that this might lead to unexpected performance
>>> regressions if we start throttling before the EC runs the fan at full
>>> speed.
>>
>> For the moment, I would say fixing the critical issue is the highest
>> priority. With the 'charging' sensor with a 55°C trip point, AFAICT the fan
>> is at full speed before this trip is reached.
> 
> Agreed.
> 
> I'm not sure about the fan speed, I never checked properly but my gut
> feeling is that the fan runs faster in Windows and never runs full speed
> in Linux. (Are there fan profiles maybe? Not sure. Maybe I'm imagining
> it. :-))

When I did some tests with the EC and the fan rpm count, it reports a 
constant augmentation of RPM up to 3600 when starting a CPU workload. So 
I guess 3600 is the max but may be it can go above. I don't know.

Or Windows has control of the fan and is more aggressive by going to 
3600 RPM more often so giving the illusion it turns faster.

Without technical details, it is hard to say.

>> Then we may want to take control of the fan and add active trip and passive
>> trip.
>>
>> This autonomous EC / PMIC mix sounds a bit strange to me :/
>>
> 
> Right, it's all quite complicated. :/ Ideally, we would be able to
> figure out the exact thresholds for the fan control in the EC firmware,
> so that we can set the throttling temperature to be shortly above that..

Perhaps, with a dual boot Windows / Linux, we can read the i2c transfer 
between windows and the EC to figure it out. Unfortunately, I don't have 
enough bandwidth to investigate that now


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

end of thread, other threads:[~2026-07-08 14:09 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-07 19:22 [PATCH v6 0/3] Lenovo ThinkPad T14s EC thermal monitoring and thermal zone integration Daniel Lezcano
2026-07-07 19:22 ` [PATCH v6 1/3] dt-bindings: embedded-controller: Add Lenovo ThinkPad T14s thermal sensor provider support Daniel Lezcano
2026-07-07 19:30   ` sashiko-bot
2026-07-07 21:34     ` Daniel Lezcano
2026-07-08  6:31       ` Krzysztof Kozlowski
2026-07-07 19:22 ` [PATCH v6 2/3] platform: arm64: lenovo-thinkpad-t14s-ec: Add hwmon support for temperatures Daniel Lezcano
2026-07-07 19:28   ` sashiko-bot
2026-07-08  6:30   ` Krzysztof Kozlowski
2026-07-08  7:31     ` Daniel Lezcano
2026-07-08  8:24       ` Krzysztof Kozlowski
2026-07-08  8:38         ` Daniel Lezcano
2026-07-08 11:48           ` Dmitry Baryshkov
2026-07-07 19:22 ` [PATCH v6 3/3] arm64: dts: qcom: x1e78100-t14s: Add thermal zones for keyboard skin and charging sensors Daniel Lezcano
2026-07-07 19:30   ` sashiko-bot
2026-07-08 12:03 ` [PATCH v6 0/3] Lenovo ThinkPad T14s EC thermal monitoring and thermal zone integration Stephan Gerhold
2026-07-08 13:16   ` Daniel Lezcano
2026-07-08 13:39     ` Stephan Gerhold
2026-07-08 14:09       ` Daniel Lezcano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox