Re: [PATCH v3 1/1] hwmon: (asus-ec-sensors) detect unconnected physical sensors

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the zero-initialized `last_updated` timestamp causes hwmon to report stale 0 values for the first 5 minutes of system uptime.
--

--- Patch [1]: [PATCH v3 1/1] hwmon: (asus-ec-sensors) detect unconnected physical sensors ---
commit 5f0c212609da2bfbdab37b7fbc65fb23b0a4589a
Author: Eugene Shalygin <eugene.shalygin@gmail.com>

hwmon: (asus-ec-sensors) detect unconnected physical sensors

When there is no physical sensor connected to the motherboard socket,
reading the EC register returns one of several special values (-62, -60,
-40). This commit tests for these values and returns -ENODATA to hwmon.

> diff --git a/drivers/hwmon/asus-ec-sensors.c b/drivers/hwmon/asus-ec-sensors.c
> index 29a23484cbe7b..3936006adeb6c 100644
> --- a/drivers/hwmon/asus-ec-sensors.c
> +++ b/drivers/hwmon/asus-ec-sensors.c
[ ... ]
> @@ -1309,12 +1343,19 @@ static int asus_ec_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
>  	}
>  
>  	ret = get_cached_value_or_update(dev, sidx, state, &value);

[Severity: High]
This isn't a bug introduced by this patch, but doesn't this result in stale 0
values being returned for the first 5 minutes of system uptime?

During probe, struct ec_sensors_data is allocated via devm_kzalloc(), which
zero-initializes the last_updated field.

Because the kernel initializes jiffies to a large negative value (typically
-300 * HZ to catch wrap-around bugs), the timestamp check in
get_cached_value_or_update() effectively becomes:

    time_after(negative_val, HZ)

This evaluates to false, as (long)(HZ - negative_val) < 0 is false.

The condition remains false until jiffies wraps around and passes HZ,
which takes approximately 300 seconds. This can cause userspace fan control
daemons to disable cooling, leading to unexpected throttling.

Should last_updated be initialized to jiffies - HZ during allocation to
ensure the first sensor read correctly triggers an update?

> -	if (!ret) {
> -		*val = scale_sensor_value(value,
> -					  get_sensor_info(state, sidx)->type);
> +	if (ret)
> +		return ret;
> +
> +	sensor_info = get_sensor_info(state, sidx);
> +	if (sensor_info->type == hwmon_temp) {
> +		ec_sensor = get_ec_sensor_type(state, sidx);
> +		if ((environment_temp_sensors & BIT(ec_sensor)) &&
> +		    is_blank_temperature_value(value))
> +			return -ENODATA;
>  	}
> +	*val = scale_sensor_value(value, sensor_info->type);
>  
> -	return ret;
> +	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260711122937.594989-2-eugene.shalygin@gmail.com?part=1