Messages in this thread | | | Date | Mon, 20 Nov 2023 06:41:08 -0800 | Subject | Re: [PATCH 1/3] hwmon: (nct6775) Fix incomplete register array | From | 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@siemens.com> >>> >>> 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@siemens.com> >>> --- >>> 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
| |