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

[Dmitry Baryshkov]

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