On 7/15/23 10:03, Ahmad Khalifa wrote:
On 15/07/2023 17:48, Guenter Roeck wrote:
On 7/15/23 08:31, Ahmad Khalifa wrote:
nct6799 supports 18 voltage readings where this driver stops at 16.
You are adding three sets of registers, though, not just two. I think
you meant to say that the driver stops at 15.
Yes, currently 15 IN defined. It was influenced by the ALARM bits
comment. I'll change it.
+/* NCT6799 layout of alarm bits is indexed by the REG_VIN
+ * order, which is
+ * CPUVC, VIN1, AVSB, 3VCC, VIN0, VIN8, VIN4, 3VSB
+ * VBAT, VTT, VIN5, VIN6, VIN2, VIN3, VIN7, VIN9
+ * no space for 16-17: VHIF, VIN10 (bits 31, -1)
Why not use bit 31 ?
Well, this is the part that made me say "driver supports up to 16".
The ALARM bits have FAN_BASE starting at index 16, so the IN alarms
can only take up 0-15, unless all alarm bits have extra padding
added to push FAN_BASE/TEMP_BASE/INTRUSION_BASE up.
I took the easy option here and left out the 16 IN alarm.
Did I count this wrong?
nct6775_fan_is_visible()
`data->ALARM_BITS[FAN_ALARM_BASE + fan]`
The 16 was just for convenience when setting the alarm base values,
and to keep some space for voltages. That doesn't mean the driver as
a whole supports 16 voltage inputs (or 8 fan control inputs, for
that matter).
We need to revisit the entire alarm handling at some point.
nct6798 and nct6799 support 8 sets of temperature limits and
alarms, and that isn't currently supported either. 'alarms'
is already 64 bits wide, so it should be possible to shift
the bits around and make space for more alarms (such as 24
voltages, 8 fans, and 8 or even 16 temperatures).
Thanks,
Guenter
