linux-kernel - Re: [PATCH v2] hwmon: (it87) Add support for IT8625E

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <300383ee-3ead-439e-893e-895f3ed49805@roeck-us.net>
Date: Tue, 22 Oct 2024 06:40:57 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Frank Crawford <frank@...wford.emu.id.au>, Ai Chao <aichao@...inos.cn>,
 jdelvare@...e.com, linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] hwmon: (it87) Add support for IT8625E

On 10/22/24 03:13, Frank Crawford wrote:
> On Tue, 2024-10-22 at 17:13 +0800, Ai Chao wrote:
>> Add support for IT8625E on Centerm P410.
>>
>> Signed-off-by: Ai Chao <aichao@...inos.cn>
>> ---
>> change for v2
>>   - Move IT8625E_DEVID after IT8623E_DEVID
>> ---
>>   drivers/hwmon/it87.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
>> index e233aafa8856..4aeb09f3bfdf 100644
>> --- a/drivers/hwmon/it87.c
>> +++ b/drivers/hwmon/it87.c
>> @@ -15,6 +15,7 @@
>>    *            IT8620E  Super I/O chip w/LPC interface
>>    *            IT8622E  Super I/O chip w/LPC interface
>>    *            IT8623E  Super I/O chip w/LPC interface
>> + *            IT8625E  Super I/O chip w/LPC interface
>>    *            IT8628E  Super I/O chip w/LPC interface
>>    *            IT8705F  Super I/O chip w/LPC interface
>>    *            IT8712F  Super I/O chip w/LPC interface
>> @@ -161,6 +162,7 @@ static inline void superio_exit(int ioreg, bool noexit)
>>   #define IT8620E_DEVID 0x8620
>>   #define IT8622E_DEVID 0x8622
>>   #define IT8623E_DEVID 0x8623
>> +#define IT8625E_DEVID 0x8625
>>   #define IT8628E_DEVID 0x8628
>>   #define IT87952E_DEVID 0x8695
>>   
>> @@ -2782,6 +2784,7 @@ static int __init it87_find(int sioaddr, unsigned short *address,
>>    case IT8622E_DEVID:
>>    sio_data->type = it8622;
>>    break;
>> + case IT8625E_DEVID:
>>    case IT8628E_DEVID:
>>    sio_data->type = it8628;
>>    break;
> 
> Can I just add that it isn't a good idea to use the same type for
> different chips.  There are some specific differences between the
> chips, which mean that it should have its own entry in
> 
> static const struct it87_devices it87_devices[]
> 
> even if currently they are very similar.
> 

According to the information I have, the ADC voltage is different,
and 8628 supports PECI but 8625 doesn't. Most importantly, 8625
has multiple register banks. There are also some differences in
fan control; 8628 can explicitly turn fans off using register bits.

Just mapping the chip to it8628 may be convenient, but it is not
acceptable.

Guenter