Linux Hardware Monitor development
 help / color / mirror / Atom feed
From: Stephan Gerhold <stephan.gerhold@linaro.org>
To: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>
Cc: sre@kernel.org, hansg@kernel.org, ilpo.jarvinen@linux.intel.com,
	linux@roeck-us.net, andersson@kernel.org, konradybcio@kernel.org,
	robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
	bryan.odonoghue@linaro.org, platform-driver-x86@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-hwmon@vger.kernel.org,
	linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
	neil.armstrong@linaro.org, gaurav.kohli@oss.qualcomm.com,
	manaf.pallikunhi@oss.qualcomm.com,
	priyansh.jain@oss.qualcomm.com
Subject: Re: [PATCH v6 0/3] Lenovo ThinkPad T14s EC thermal monitoring and thermal zone integration
Date: Wed, 8 Jul 2026 14:03:14 +0200	[thread overview]
Message-ID: <ak48gsxhVW492C7H@linaro.org> (raw)
In-Reply-To: <20260707192228.14647-1-daniel.lezcano@oss.qualcomm.com>

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

  parent reply	other threads:[~2026-07-08 12:03 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 ` Stephan Gerhold [this message]
2026-07-08 13:16   ` [PATCH v6 0/3] Lenovo ThinkPad T14s EC thermal monitoring and thermal zone integration Daniel Lezcano
  -- strict thread matches above, loose matches on Subject: below --
2026-07-07 19:22 Daniel Lezcano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ak48gsxhVW492C7H@linaro.org \
    --to=stephan.gerhold@linaro.org \
    --cc=andersson@kernel.org \
    --cc=bryan.odonoghue@linaro.org \
    --cc=conor+dt@kernel.org \
    --cc=daniel.lezcano@oss.qualcomm.com \
    --cc=devicetree@vger.kernel.org \
    --cc=gaurav.kohli@oss.qualcomm.com \
    --cc=hansg@kernel.org \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=konradybcio@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=manaf.pallikunhi@oss.qualcomm.com \
    --cc=neil.armstrong@linaro.org \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=priyansh.jain@oss.qualcomm.com \
    --cc=robh@kernel.org \
    --cc=sre@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox