Re: [PATCH] hwmon: (nct6775) Add support for NCT5525

[Anton Lundin <glance@ac2.se>]

On 03 november, 2026 - Guenter Roeck wrote:

> On 11/3/26 05:29, Anton Lundin wrote:
> > Extend nct6775 driver to support NCT5525.
> > 
> > Assisted-by: Copilot:GPT-5.6-Terra
> > Signed-off-by: Anton Lundin <glance@ac2.se>
> 
> 
> I don't havea datasheet for this chip, but I do have to say
> that this seems to be off just form chip numbering perspective.
> From the registers it looks like the chip is mostly compatible
> with the NCT61xx series, but NCT55xx chips are typically scaled-down
> variants of MCT67xx series chips. I _will_ need to see a datasheet
> to confirm, or at the very least independent confirmation that the
> driver is correct.
> 
> Some more comments inline.

I've now reviewed all the comments and reworked the code. There are some
things I'm still thinking about and I kindly ask for some guidance.
 
> >   };
> >   /* Common and NCT6775 specific data */
> > @@ -840,6 +842,33 @@ static const u16 NCT6106_REG_TEMP_CRIT[32] = {
> >   static const u16 NCT6106_REG_TSI_TEMP[] = { 0x59, 0x5b, 0x5d, 0x5f, 0x61, 0x63, 0x65, 0x67 };
> > +/* NCT5525D specific data */
> > +
> > +static const u16 NCT5525_REG_IN_MAX[] = {
> > +	0x92, 0x94, 0x96, 0x9a, 0x9c, 0x9e, 0xa0, 0xa2 };
> > +static const u16 NCT5525_REG_IN_MIN[] = {
> > +	0x93, 0x95, 0x97, 0x9b, 0x9d, 0x9f, 0xa1, 0xa3 };
> > +static const u16 NCT5525_REG_IN[] = {
> > +	0x01, 0x02, 0x03, 0x05, 0x06, 0x07, 0x08, 0x09 };
> > +
> 
> The above are equivalent to the NCT6106 definitions.
> Please do not duplicate definitions unless necessary.

I've now remodeled this into:

+       case nct5525:
...
+               data->REG_VIN = &NCT6106_REG_IN[1];
+               data->REG_IN_MINMAX[0] = &NCT6106_REG_IN_MIN[1];
+               data->REG_IN_MINMAX[1] = &NCT6106_REG_IN_MAX[1];

It's not the prettiest, but it works. Is that acceptable?

> > +static const u16 NCT5525_REG_FAN[] = { 0x20, 0x22, 0x28 };
> > +static const u16 NCT5525_REG_FAN_MIN[] = { 0xe0, 0xe2, 0xe8 };
> > +static const u16 NCT5525_REG_FAN_PULSES[] = { 0xf6, 0xf6, 0xf5 };
> > +static const u16 NCT5525_FAN_PULSE_SHIFT[] = { 0, 2, 6 };
> > +
> 
> This suggests compatibility with NCT6116, with fans index 2 and 3 not populated.
> Would it be possible to model it that way ?

I'll rework it into that. The data sheet even calls it fan5 in one
place, HM_Rev_Pulse_Fan5_Sel.

All the registers aligns nicely with this, so I think it's the right
call.

The only interesting bit will be that the fan names are now 1 2 and 5.

In the data sheet they're SYSFAN, CPUFAN and AUXFAN.

> > @@ -1191,6 +1224,10 @@ bool nct6775_reg_is_word_sized(struct nct6775_data *data, u16 reg)
> >   		  (reg >= 0x59 && reg < 0x69 && (reg & 1)) ||
> >   		  reg == 0xe0 || reg == 0xe2 || reg == 0xe4 ||
> >   		  reg == 0x111 || reg == 0x121 || reg == 0x131;
> > +	case nct5525:
> > +		return reg == 0x20 || reg == 0x22 || reg == 0x28 ||
> > +		  reg == 0x59 || reg == 0x5b || reg == 0xe0 || reg == 0xe2 ||
> > +		  reg == 0xe8 || reg == 0x111 || reg == 0x121;
> 
> Sashiko has some valid concerns here.

This is the same as for nct6106, nct6116 and nct6126.

As far as I see it, we would need to look if we're in speed-cruise mode
or thermal-cruise mode, and adjust if we read word sized or not.

Any preference on how that should be done or should I just make
something up?


//Anton