Re: [PATCH 1/3] hwmon: (nct6775) Fix incomplete register array

[Guenter Roeck]

On 11/19/23 19:30, xingtong.wu wrote:
> On 2023/11/17 09:35, Guenter Roeck wrote:
> > On 11/15/23 18:23, Xing Tong Wu wrote:
> > > From: Xing Tong Wu <xingtong.wu@xxxxxxxxxxx>
> > >
> > > The nct6116 specification actually includes 5 PWMs, but only 3
> > > PWMs are present in the array. To address this, the missing 2
> > > PWMs have been added to the array.
> > >
> > > Signed-off-by: Xing Tong Wu <xingtong.wu@xxxxxxxxxxx>
> > > ---
> > >    drivers/hwmon/nct6775-core.c | 4 ++--
> > >    1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/hwmon/nct6775-core.c b/drivers/hwmon/nct6775-core.c
> > > index d928eb8ae5a3..2111f0cd9787 100644
> > > --- a/drivers/hwmon/nct6775-core.c
> > > +++ b/drivers/hwmon/nct6775-core.c
> > > @@ -769,7 +769,7 @@ static const u16 NCT6106_FAN_PULSE_SHIFT[] = { 0, 2, 4 };
> > >      static const u8 NCT6106_REG_PWM_MODE[] = { 0xf3, 0xf3, 0xf3 };
> > >    static const u8 NCT6106_PWM_MODE_MASK[] = { 0x01, 0x02, 0x04 };
> > > -static const u16 NCT6106_REG_PWM_READ[] = { 0x4a, 0x4b, 0x4c };
> > > +static const u16 NCT6106_REG_PWM_READ[] = { 0x4a, 0x4b, 0x4c, 0xd8, 0xd9 };
> >
> > I have no idea where you got the above register addresses from. Looking at
> > the datasheet, NCT6116 doesn't use those registers at all, and neither does
> > NCT6106. The PWM registers for NCT6116 are
>
> I obtain these registers from the Nuvoton official specification
> "NCT6116D_Datasheet_V1_0.pdf". There is a table that describes the access for the
> PWM registers and corresponding fans:
>
> Fans: SYSFANOUT, CPUFANOUT, AUXFANOUT0, AUXFANOUT1, AUXFANOUT2
> PWM output duty (write): Bank1 Index19 bit[7:0], Bank1 Index29 bit[7:0], Bank1 Index39 bit[7:0], Bank1 Index99 bit[7:0], Bank1 IndexA9 bit[7:0]
> Current output value (read): Bank0 Index4A, Bank0 Index4B, Bank0 Index4C, Bank0 IndexD8, Bank0 IndexD9
>
> I have checked the NCT6106-NCT6126 series specification(These documents are not
> publicly available, so I cannot share them with you), there are only 3 fans in
> NCT6106: SYSFANOUT, CPUFANOUT, AUXFANOU0. However, for NCT6116D-NCT6126D, there
> are 2 additional fans: AUXFANOUT1, AUXFANOUT2. The registers for these fans are
> the same. I'll add a new array for NCT6116D's PWM read in my new patch.

I wasn't aware of NCT6126D, but I do have datasheets for NCT6102/4/6 and for NCT6112/4/6.

There is no need to add a separate array. This is good as-is since the added registers
are not used for NCT6102/4/6.

> > static const u16 NCT6116_REG_PWM[] = { 0x119, 0x129, 0x139, 0x199, 0x1a9 };
>
> Therefore, this array is only for writing, we need to add an array of registers for reading.

Ah, good point. I forgot that the read and write registers are different.

Still, you'll need to extend NCT6106_REG_PWM_MODE[] and NCT6106_PWM_MODE_MASK[] as well.
As far as I can see, aux1 and aux2 are always in pwm mode, so the register arrays
need to be extended with ", 0, 0".

Thanks,
Guenter