Re: [PATCH v2 2/3] platform: arm64: lenovo-thinkpad-t14s: Wire EC thermal events to hwmon

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch adds HWMON_T_ALARM to the hwmon temperature channels to support thermal alarms but fails to expose or handle hwmon_temp_alarm in the is_visible and read callbacks, rendering the alarm attribute invisible and inaccessible.
--

commit 100b1ed0167b57e72fe6d235a080f1fbdce5fee9
Author: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>

platform: arm64: lenovo-thinkpad-t14s: Wire EC thermal events to hwmon

This commit wires up EC thermal zone status change notifications to the
hwmon notification framework. It associates hwmon temperature channels
with EC events and adds thermal alarm capabilities.

> diff --git a/drivers/platform/arm64/lenovo-thinkpad-t14s.c b/drivers/platform/arm64/lenovo-thinkpad-t14s.c
> index 35a6f8b0cb6b1..5fafb01a2b332 100644
> --- a/drivers/platform/arm64/lenovo-thinkpad-t14s.c
> +++ b/drivers/platform/arm64/lenovo-thinkpad-t14s.c

[ ... ]

> @@ -640,13 +653,14 @@ static const struct hwmon_ops t14s_ec_hwmon_ops = {
>  };
>  
>  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),
> +			   HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_ALARM,
> +			   HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_ALARM,
> +			   HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_ALARM,
> +			   HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_ALARM,
> +			   HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_ALARM,
> +			   HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_ALARM),
>  	NULL
>  };

[Severity: Medium]
Since HWMON_T_ALARM is added here, do t14s_ec_hwmon_is_visible() and
t14s_ec_hwmon_read() also need to be updated to handle hwmon_temp_alarm?

If t14s_ec_hwmon_is_visible() is not updated to support hwmon_temp_alarm,
it falls through to returning 0:

static umode_t t14s_ec_hwmon_is_visible(...)
{
        switch (type) {
        case hwmon_temp:
                if (attr == hwmon_temp_input ||
                    attr == hwmon_temp_label)
                        return 0444;
                break;
        default:
                return 0;
        }

        return 0;
}

Will this prevent the sysfs file from being created by the hwmon core,
meaning hwmon_notify_event() will attempt to notify on a non-existent file,
and userspace will not be able to read the alarm state?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630145307.10745-1-daniel.lezcano@oss.qualcomm.com?part=2