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

[Ahmad Khalifa <ahmad@khalifa.ws>]

On 22/10/2024 14:40, Guenter Roeck wrote:
> 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.
...
>> 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.

Side question here. The standard for an acceptable chip driver is pretty
high (and rightfully so). But a common use case centres around readonly
display of information: temp/fan/in readings. Even just 2-3 readings are
better than nothing.

Example, I still have to use Frank's out of tree it87 for my IT8688.
It works perfectly fine for me, but still not possible to merge that
device into hwmon's it87.
This IT8625 is another example. The NCT6701D-R will be one more shortly.

A readonly driver that is configurable from userspace would help in that
use case. It can be configured for known devices without datasheets or
for testing new devices. Wouldn't even need to access superio config
space.

Filesystem has fuse, i2c has i2c-dev, input has evdev, ...
Would something similar be acceptable for hwmon?


-- 
Regards,
Ahmad