All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Guenter Roeck" <linux@roeck-us.net>
Cc: linux-hwmon@vger.kernel.org
Subject: Re: [PATCH v2] hwmon: Add support for currX_emergency and inX_[l]emergency attributes
Date: Fri, 14 Aug 2026 01:20:59 +0000	[thread overview]
Message-ID: <20260814012059.CFC2F1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260813211433.879638-1-linux@roeck-us.net>

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 <linux@roeck-us.net>

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/20260813211433.879638-1-linux@roeck-us.net?part=1

  reply	other threads:[~2026-08-14  1:21 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-13 21:14 [PATCH v2] hwmon: Add support for currX_emergency and inX_[l]emergency attributes Guenter Roeck
2026-08-14  1:20 ` sashiko-bot [this message]
2026-08-14  1:54   ` Guenter Roeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260814012059.CFC2F1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.