On 9/16/23 09:43, Ahmad Khalifa wrote:
Thanks for the detailed report, Doug.
Would you be able to test out a fix please?
It's a basic guard line as _alarm attributes are assumed to be there when a sensor exists, but the device doesn't have an alarm for in5 (VIN8
on the device)
If you could also confirm that your /sys/class/hwmon/hwmon?/in5_alarm
file is there and showing 0, that would be great.
---
diff --git a/drivers/hwmon/nct6775-core.c b/drivers/hwmon/nct6775-core.c
index 02a7124..5470aff 100644
--- a/drivers/hwmon/nct6775-core.c
+++ b/drivers/hwmon/nct6775-core.c
@@ -1753,6 +1753,9 @@ nct6775_show_alarm(struct device *dev, struct device_attribute *attr, char *buf)
return PTR_ERR(data);
nr = data->ALARM_BITS[sattr->index];
+ if (nr < 0)
+ return sprintf(buf, "0\n");
+
return sprintf(buf, "%u\n",
(unsigned int)((data->alarms >> nr) & 0x01));
}
---
More details - if the above patch fixes the issue...
The update of nct6798/nct6799 for additional TEMP sensors required
separating them from the previous set of devices.
Both devices are very similar.
Both do not have an alarm bit for VIN8 (index 5) where the
shift-out-of-bounds check trips.
>> nct6775_core: doug: nr: 21 ; index 5
Previously in5_alarm was pointing at bit 21 (which is bit 5 in reg
0x45b) which is reserved and *probably* always 0. With the update, 6798
was pointed at the same ALARM_BITS as 6799 as the TEMP bits are
different from other devices.
This is correct and it doesn't need to point back at the old BITS again.
However, now I realise that in*_alarm attributes can't be switched off
separately from their sensor - in5_* are all there except ALARM.
Seems the shift-out-of-bounds check was there prior to this change.
I'm not exactly sure why this didn't come up in my testing on 6.5.0.
This is the only occurrence of non-existent ALARM in the middle of other
sensors, so a simple guard check seems to be the reasonable thing here
even though it may get evaluated more times than it should.
If it's confirmed to work, I can submit a formal patch later.
The proper fix should really be in nct6775_in_is_visible(),
which should drop the attribute if there is no alarm bit for it,
similar to, for example, nct6775_fan_is_visible().
Something like
int nr = index % 5; /* attribute index */
if (nr == 1 && data->ALARM_BITS[in] == -1)
return 0;
I think there is a also problem with beep attribute handling,
but that is a different bug unrelated to this one.
Guenter
