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

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] [day] [month] [year] [list]
Date:   Thu, 16 Sep 2021 15:24:30 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Denis Pauk <pauk.denis@...il.com>
Cc:     Bernhard Seibold <mail@...nhard-seibold.de>,
        Pär Ekholm <pehlm@...holm.org>,
        to.eivind@...il.com, "Artem S . Tashkinov" <aros@....com>,
        Vittorio Roberto Alfieri <me@...toor.com>,
        Sahan Fernando <sahan.h.fernando@...il.com>,
        Andy Shevchenko <andriy.shevchenko@...el.com>,
        Jean Delvare <jdelvare@...e.com>, linux-hwmon@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7 3/3] hwmon: (nct6775) Support access via Asus WMI

On 9/16/21 2:24 PM, Denis Pauk wrote:
> On Thu, 16 Sep 2021 14:10:49 -0700
> Guenter Roeck <linux@...ck-us.net> 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@...il.com>
>>> Co-developed-by: Bernhard Seibold <mail@...nhard-seibold.de>
>>> Signed-off-by: Bernhard Seibold <mail@...nhard-seibold.de>
>>> Tested-by: Pär Ekholm <pehlm@...holm.org>
>>> Tested-by: <to.eivind@...il.com>
>>> Tested-by: Artem S. Tashkinov <aros@....com>
>>> Tested-by: Vittorio Roberto Alfieri <me@...toor.com>
>>> Tested-by: Sahan Fernando <sahan.h.fernando@...il.com>
>>> Cc: Andy Shevchenko <andriy.shevchenko@...el.com>
>>> Cc: Guenter Roeck <linux@...ck-us.net>
>>>
>>> ---
>>> 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