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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <4ed34030-d5c1-4caa-919a-69c7c5aa77ef@roeck-us.net>
Date:   Mon, 20 Nov 2023 06:41:08 -0800
From:   Guenter Roeck <linux@...ck-us.net>
To:     "xingtong.wu" <xingtong_wu@....com>,
        Jean Delvare <jdelvare@...e.com>, linux-hwmon@...r.kernel.org,
        linux-kernel@...r.kernel.org
Cc:     xingtong.wu@...mens.com, tobias.schaffner@...mens.com,
        gerd.haeussler.ext@...mens.com
Subject: Re: [PATCH 1/3] hwmon: (nct6775) Fix incomplete register array

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@...mens.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@...mens.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