[LEDE-DEV] Problem with commit "kernel: add hwmon for W83627EHF and family"

[Philip Prindeville]

> On May 21, 2017, at 10:03 AM, Daniel Golle <daniel at makrotopia.org> wrote:
> 
> Hi Rafal,
> 
> On Sun, May 21, 2017 at 05:02:44PM +0200, Rafał Miłecki wrote:
>> Hi,
>> 
>> I noticed commit 0dcc36fc7ddec ("kernel: add hwmon for W83627EHF and
>> family") in the LEDE tree that doesn't look OK to me.
>> 
>> 1) Package for hwmon-w83627ehf
>> Do we need it to be a package? Or could it be built-in into the kernel?
>> Do we need it to be a global package? Usually hwmon drivers are
>> designed for a specific device and it should be enough to have it in
>> target/linux/*/modules.mk
> 
> True, but all other x86-specific hwmon modules are in
> package/kernel/linux/modules as well and I'd rather wanted it to be
> consistent.
> 
>> 
>> 2) 800-hwmon-w83627ehf-dont-claim-nct677x.patch
>> I really don't like this one as it's totally unclear why we need this
>> change. What's wrong with NCT6775 and the w83627ehf driver? It should
>> be well described in the patch.
> 
> Sorry for the lack of a more detailed description.
> The problem here is that the W83627EHF driver also claims the NCT677x
> hardware but doesn't support all features (according to
> Philip Prindeville). I reckon he can provide more information regarding
> which features which are available when using the nct6776 driver are
> missing in the w83627ehf driver on NCT677x hardware.
> 
> Cheers
> 
> Daniel

To follow up on what Daniel said, yes, the older w83627 has a subset of functionality for the nct6775 chips, but the native nct6775 driver does a better job.

Quoting Documentation/hwmon/nct6775:

    Note
    ====

    This driver supersedes the NCT6775F and NCT6776F support in the W83627EHF
    driver.

and drivers/hwmon/Kconfig:

config SENSORS_NCT6775
        tristate "Nuvoton NCT6775F and compatibles"
        depends on !PPC
        select HWMON_VID
        help
          If you say yes here you get support for the hardware monitoring
          functionality of the Nuvoton NCT6106D, NCT6775F, NCT6776F, NCT6779D,
          NCT6791D, NCT6792D, NCT6793D, and compatible Super-I/O chips. This
          driver replaces the w83627ehf driver for NCT6775F and NCT6776F.

          This driver can also be built as a module.  If so, the module
          will be called nct6775.

...
config SENSORS_W83627EHF
        tristate "Winbond W83627EHF/EHG/DHG/UHG, W83667HG, NCT6775F, NCT6776F"
        depends on !PPC
        select HWMON_VID
        help
          If you say yes here you get support for the hardware
          monitoring functionality of the Winbond W83627EHF Super-I/O chip.

          This driver also supports the W83627EHG, which is the lead-free
          version of the W83627EHF, and the W83627DHG, which is a similar
          chip suited for specific Intel processors that use PECI such as
          the Core 2 Duo. And also the W83627UHG, which is a stripped down
          version of the W83627DHG (as far as hardware monitoring goes.)

          This driver also supports Nuvoton W83667HG, W83667HG-B, NCT6775F
          (also known as W83667HG-I), and NCT6776F.

          This driver can also be built as a module.  If so, the module
          will be called w83627ehf.



If you definitely have NCT6775F hardware, they you don’t want to build both drivers into your kernel.

If you want to build a kernel that supports both the NCT6775 and W83627HF drivers, then you need this patch so that ONLY the NCT6775 will detect the chipset.

It’s not clear why a few of the patches to add partial NCT6775 support to the W83627HF driver weren’t backed out after the NCT6775 driver was merged…  probably an oversight.

-Philip