Re: [PATCH 5/7] hwmon: Add driver for Marvel AlphaServer environmental sensors

[Matt Turner]

On Sat, Nov 14, 2026 at 6:00 PM Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
>> +``temp[1-*]_max_alarm``     Sensor is in the warning or fault state
>
> "fault": Sensor is broken.
> "max_alarm": Temperature is too high.
>
> That is distincly different. A faulty sensor should not be reported
> as "temperature too high".

My wording was the problem here, sorry. "Fault" was my name for what
the firmware calls the Failure state, and it does not mean the sensor is
broken. The AlphaServer Management Utility user guide [1] (section 3.16)
says the firmware keeps a warning limit and a failure limit for each
fan, voltage and temperature sensor, and puts the sensor in the Warning
or Failure state when a reading meets or exceeds the matching limit.
The status byte in the console's packet only says OK, Warning, Failure,
Unknown or Not present (Table B-1 of the same guide), so a broken
sensor cannot be told apart from a bad reading at all.

For temperatures, v2 reports Warning as temp_max_alarm and Failure as
temp_max_alarm plus temp_crit_alarm, with temp_max and temp_crit being
the warning and failure limits. Nothing is reported as a fault.

>> +``fan[1-*]_min_alarm``      Fan is in the warning or fault state
>
> This is wrong since there is also a separate fault attribute,
> and "fault" means defective, not "speed too low".

A fan in the Failure state has slowed to its failure limit, and the
firmware treats it as a failed fan (a drawer is powered off for having
too few running fans). v2 reports Warning as fan_min_alarm and Failure
as fan_min_alarm plus fan_fault. If you would rather keep fan_fault for
something else, I can report Failure as fan_min_alarm only, but then
nothing in sysfs separates a slow fan from a stopped one.

>> +``in[0-*]_alarm``           Sensor is in the warning or fault state
>
> Which one is it ? Is it distinct ? A "fault" means
> that the hardware is broken/defective. An alarm means that
> the voltage is too high or too low. A faulty channel
> should not be reported as alarm but as fault.

It is always the voltage being out of range: Warning is the reading
outside the warning limits (5% from nominal), Failure outside the
failure limits (10%). Both are reported as in_alarm in v2. The limits
are not in the console's packet, so they are not exported.

>> +``power[1-*]_alarm``        Power supply is in the warning or fault state
>
> "power" is supposed to report the system power, not
> a "power supply warning or fault state". If there is
> nothing else, this should be identical to the matching
> inX_alarm and/or inX_fault attribute.

There is nothing else: the firmware reports a supply only as OK or
Failure, with no reading. Failure means the supply itself has failed
(lost AC, power not OK, supply fail, over temperature or fan), so v2
reports each supply as an in channel with only in_fault and in_label.

I'll send v2 tomorrow.

Thanks a bunch for the review!
Matt

[1] https://ftp.zx.net.nz/pub/archive/ftp.digital.com/pub/DEC/Alpha/firmware/interim/ams/releases/Version_5_2_0/ams-user-guide.pdf