Re: [PATCH v2] hwmon: Add support for currX_emergency and inX_[l]emergency attributes

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Internal ABI breakage in the habanalabs driver caused by inserting values into hwmon enums.
--

--- Patch [1]: [PATCH v2] hwmon: Add support for currX_emergency and inX_[l]emergency attributes ---
commit 618b70e656b029d918a4d90557dfadbd253324dd
Author: Guenter Roeck <[email protected]>

hwmon: Add support for currX_emergency and inX_[l]emergency attributes

This commit adds new emergency limit attributes for current and voltage to
the hardware monitoring subsystem. It updates the hwmon core code, the
enumerations, and the sysfs ABI documentation to support the new alarms.

> diff --git a/include/linux/hwmon.h b/include/linux/hwmon.h
> index dd713e193d0c3..a3a7d27f3b5ff 100644
> --- a/include/linux/hwmon.h
> +++ b/include/linux/hwmon.h
> @@ -134,6 +134,8 @@ enum hwmon_in_attributes {
>  	hwmon_in_max,
>  	hwmon_in_lcrit,
>  	hwmon_in_crit,
> +	hwmon_in_lemergency,
> +	hwmon_in_emergency,

[Severity: High]
Does inserting these elements here break downstream drivers that rely on the
historical integer mapping?

For example, in drivers/accel/habanalabs/common/hwmon.c:adjust_hwmon_flags(),
the driver performs a hardcoded bitwise shift assuming the Linux hwmon enum
values perfectly match the firmware API shifted by 1:

    flags = (cpucp_flags << 1) | HWMON_I_ENABLE;

By inserting elements into the middle of this enum, all subsequent values
(like hwmon_in_average) shift by 2. This causes the habanalabs driver to
incorrectly map firmware sensors to Linux hwmon attributes. For instance,
inX_highest will map to inX_average, and reading from these incorrectly
mapped attributes via sysfs will fail with -EINVAL.

Can these new enum values be added to the end of the enum instead, or does
habanalabs need to be updated to decouple it from the hwmon enum ordering?

>  	hwmon_in_average,
>  	hwmon_in_lowest,
>  	hwmon_in_highest,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1