On 8/13/26 18:20, [email protected] wrote:
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?
There is no guarantee of a specific ordering of those flags. They can and will
change
position and/or ordering as new flags are added. The hwmon subsystem must not
be held
hostage by some driver making up their own rules. FWIW, I don't even understand
what the habanalabs driver is doing and what assumptions it is making about the
position
and ordering of the flags.
Guenter
