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: <6cab565f05820eb2e1a1c55644be057427ecdf2e.camel@crawford.emu.id.au>
Date: Tue, 22 Oct 2024 21:13:42 +1100
From: Frank Crawford <frank@...wford.emu.id.au>
To: Ai Chao <aichao@...inos.cn>, jdelvare@...e.com, linux@...ck-us.net,
        linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] hwmon: (it87) Add support for IT8625E

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.

Even one of the most basic items is that it will report the wrong
chipID in the logs.

Regards
Frank