Re: [PATCH v7 3/3] hwmon: (nct6775) Support access via Asus WMI

[Guenter Roeck]

On 9/16/21 2:24 PM, Denis Pauk wrote:
> On Thu, 16 Sep 2021 14:10:49 -0700
> Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
>
> > On 9/16/21 1:22 PM, Denis Pauk wrote:
> > > Support accessing the NCT677x via Asus WMI functions.
> > >
> > > On mainboards that support this way of accessing the chip, the
> > > driver will usually not work without this option since in these
> > > mainboards, ACPI will mark the I/O port as used.
> > >
> > > Code uses ACPI firmware interface to communicate with sensors with
> > > ASUS motherboards:
> > > * PRIME B460-PLUS,
> > > * ROG CROSSHAIR VIII IMPACT,
> > > * ROG STRIX B550-E GAMING,
> > > * ROG STRIX B550-F GAMING,
> > > * ROG STRIX B550-F GAMING (WI-FI),
> > > * ROG STRIX Z490-I GAMING,
> > > * TUF GAMING B550M-PLUS,
> > > * TUF GAMING B550M-PLUS (WI-FI),
> > > * TUF GAMING B550-PLUS,
> > > * TUF GAMING X570-PLUS,
> > > * TUF GAMING X570-PRO (WI-FI).
> > >
> > > BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=204807
> > > Signed-off-by: Denis Pauk <pauk.denis@xxxxxxxxx>
> > > Co-developed-by: Bernhard Seibold <mail@xxxxxxxxxxxxxxxxxxx>
> > > Signed-off-by: Bernhard Seibold <mail@xxxxxxxxxxxxxxxxxxx>
> > > Tested-by: Pär Ekholm <pehlm@xxxxxxxxxxx>
> > > Tested-by: <to.eivind@xxxxxxxxx>
> > > Tested-by: Artem S. Tashkinov <aros@xxxxxxx>
> > > Tested-by: Vittorio Roberto Alfieri <me@xxxxxxxxxxx>
> > > Tested-by: Sahan Fernando <sahan.h.fernando@xxxxxxxxx>
> > > Cc: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
> > > Cc: Guenter Roeck <linux@xxxxxxxxxxxx>
> > >
> > > ---
> > > Changes in v7:
> > >     - Remove unrequred & 0xff with int8 variables.
> > >     - Make ASUSWMI_UNSUPPORTED_METHOD as default value for WMI
> > > responce, before run wmi_evaluate_method().
> > >     - Rename ASUSWMI_MGMT2_GUID to ASUSWMI_MONITORING_GUID.
> > >     - Replace checks of 'err != -EINVAL' with 'err >= 0' for
> > > match_string result.
> > >
> > > Changes in v6:
> > >     - Minimaze codes inside code inside defined(CONFIG_ACPI_WMI).
> > >
> > > Changes in v5:
> > >     - Use IS_ENABLED(CONFIG_ACPI_WMI) instead
> > > defined(CONFIG_ACPI_WMI)
> > >
> > > Changes in v4:
> > >     - Fix build without ACPI WMI.
> > >
> > > Changes in v3:
> > >     - Remove unrequired type conversions.
> > >     - Save result of match_string before check.
> > >
> > > Changes in v2:
> > >     - Split changes to separate patches.
> > >     - Limit WMI usage by DMI_BOARD_NAME in checked ASUS motherboards.
> > > ---
> > >    drivers/hwmon/Kconfig   |   1 +
> > >    drivers/hwmon/nct6775.c | 230
> > > ++++++++++++++++++++++++++++++++++++---- 2 files changed, 210
> > > insertions(+), 21 deletions(-)
> > >
> > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > > index e3675377bc5d..9eefb1014b53 100644
> > > --- a/drivers/hwmon/Kconfig
> > > +++ b/drivers/hwmon/Kconfig
> > > @@ -1423,6 +1423,7 @@ config SENSORS_NCT6683
> > >    config SENSORS_NCT6775
> > >    	tristate "Nuvoton NCT6775F and compatibles"
> > >    	depends on !PPC
> > > +	depends on ACPI_WMI || ACPI_WMI=n
> > >    	select HWMON_VID
> > >    	help
> > >    	  If you say yes here you get support for the hardware
> > > monitoring diff --git a/drivers/hwmon/nct6775.c
> > > b/drivers/hwmon/nct6775.c index 4253eed7f5b0..46262d9d3bd9 100644
> > > --- a/drivers/hwmon/nct6775.c
> > > +++ b/drivers/hwmon/nct6775.c
> > > @@ -55,6 +55,7 @@
> > >    #include <linux/dmi.h>
> > >    #include <linux/io.h>
> > >    #include <linux/nospec.h>
> > > +#include <linux/wmi.h>
> > >    #include "lm75.h"
> > >    
> > >    #define USE_ALTERNATE
> > > @@ -132,10 +133,13 @@ MODULE_PARM_DESC(fan_debounce, "Enable
> > > debouncing for fan RPM signal"); #define SIO_ID_MASK
> > > 0xFFF8
> > >    enum pwm_enable { off, manual, thermal_cruise, speed_cruise, sf3,
> > > sf4 }; +enum sensor_access { access_direct, access_asuswmi };
> > >    
> > >    struct nct6775_sio_data {
> > >    	int sioreg;
> > > +	int ld;
> > >    	enum kinds kind;
> > > +	enum sensor_access access;
> > >    
> > >    	/* superio_() callbacks  */
> > >    	void (*sio_outb)(struct nct6775_sio_data *sio_data, int
> > > reg, int val); @@ -145,6 +149,90 @@ struct nct6775_sio_data {
> > >    	void (*sio_exit)(struct nct6775_sio_data *sio_data);
> > >    };
> > >    
> > > +#define ASUSWMI_MONITORING_GUID
> > > "466747A0-70EC-11DE-8A39-0800200C9A66" +#define
> > > ASUSWMI_METHODID_RSIO		0x5253494F +#define
> > > ASUSWMI_METHODID_WSIO		0x5753494F +#define
> > > ASUSWMI_METHODID_RHWM		0x5248574D +#define
> > > ASUSWMI_METHODID_WHWM		0x5748574D +#define
> > > ASUSWMI_UNSUPPORTED_METHOD	0xFFFFFFFE +
> > > +static int asuswmi_evaluate_method(u32 method_id, u8 bank, u8 reg,
> > > u8 val, u32 *retval) +{
> What do you thin about rename asuswmi_evaluate_method() to
> asuswmi_monitoring_method()? I have found that kernel already have
> asus_wmi_evaluate_method() that has used different method GUID, so
> looks as make sense to use different function name. It uses different
> constants with different names/values and does not intersect with this
> one.

nct6775_asuswmi_evaluate_method or similar, maybe, if you really
want to rename it.

Guenter