[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <e2329d97-3d9f-3579-f24f-b1e3d9660f9f@roeck-us.net>
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
Powered by blists - more mailing lists