Re: [PATCH 2/2] hwmon: (lm90) Reject channel 2 on chips with only one remote sensor

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Setting the temperature offset for channel 2 during probe clobbers the device's hardware configuration register because `data->config` is used before it is initialized.
--

--- Patch [2]: [PATCH 2/2] hwmon: (lm90) Reject channel 2 on chips with only one remote sensor ---
commit dbe7fc3f39616fac014beb08a22b533d36d4c54b
Author: Flaviu Nistor <flaviu.nistor@gmail.com>
Subject: hwmon: (lm90) Reject channel 2 on chips with only one remote sensor

This patch validates firmware channel definitions against chip capabilities
and returns -EINVAL when channel 2 is configured on devices with 1 remote
channel.

> diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
> index 1c603272538aa..3faeb2c6ab01b 100644
> --- a/drivers/hwmon/lm90.c
> +++ b/drivers/hwmon/lm90.c
> @@ -2712,6 +2712,11 @@ static int lm90_probe_channel(struct i2c_client *client,
>  		return -EINVAL;
>  	}
>  
> +	if (id == 2 && !(data->flags & LM90_HAVE_TEMP3)) {
> +		dev_err(dev, "channel %d is not supported for this chip in %pfw\n", id, child);
> +		return -EINVAL;
> +	}
> +
[Severity: High]
This is a pre-existing issue, but I noticed a potential problem when channel 2
is valid and has a temperature offset configured.

In lm90_probe(), lm90_parse_channel_info() is called before
lm90_init_client(). If the device tree specifies the property
temperature-offset-millicelsius for channel 2, it invokes
lm90_set_temp_offset() further down in lm90_probe_channel().

For channel 2, lm90_set_temp_offset() calls lm90_select_remote_channel():

drivers/hwmon/lm90.c:lm90_set_temp_offset() {
        ...
        /* For ADT7481 we can use the same registers for remote channel 1 and 2 */
        if (channel > 1)
                lm90_select_remote_channel(data, true);
        ...
}

Because lm90_init_client() hasn't executed yet, data->config is still 0 from
its initial allocation. This causes the driver to write 0 directly to the
hardware configuration register, which might clobber the original
BIOS/bootloader configuration before it is ever read and saved.

Could this silently overwrite the chip's thermal alert masks or run states
during device probe on systems providing the offset property?

>  	err = fwnode_property_read_string(child, "label", &data->channel_label[id]);
>  	if (err == -ENODATA || err == -EILSEQ) {
>  		dev_err(dev, "invalid label property in %pfw\n", child);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824183900.8983-1-flaviu.nistor@gmail.com?part=2