lkml.org 
[lkml]   [2023]   [Nov]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 1/3] hwmon: (nct6775) Fix incomplete register array
From
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

\
 
 \ /
  Last update: 2023-11-20 15:42    [W:0.215 / U:0.384 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site