Re: [PATCH v2 1/3] platform: arm64:: lenovo-thinkpad-t14s-ec: Add hwmon support for temperatures

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Tue, 30 Jun 2026, Daniel Lezcano wrote:

> 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.
> 
> Signed-off-by: Daniel Lezcano daniel.lezcano@oss.qualcomm.com
> ---
>  drivers/platform/arm64/lenovo-thinkpad-t14s.c | 128 ++++++++++++++++++
>  1 file changed, 128 insertions(+)
> 
> diff --git a/drivers/platform/arm64/lenovo-thinkpad-t14s.c b/drivers/platform/arm64/lenovo-thinkpad-t14s.c
> index 5590302a5694..35a6f8b0cb6b 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>
> @@ -67,6 +68,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 +101,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 +573,112 @@ 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);

Add an empty line.

> +	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 * 1000;

Does this literal relate to something from units.h?

> +
> +			return 0;
> +		}
> +		break;
> +	default:
> +		break;

Nit, style is inconsistent with the one above.

> +	}
> +
> +	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(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 +714,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);
> 

-- 
 i.