| Message ID | 20230110224823.14524-1-pauk.denis@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3,1/2] hwmon: (nct6775) Directly call ASUS ACPI WMI method | expand |
On 1/10/23 14:48, Denis Pauk wrote: > New ASUS B650/B660/X670 boards firmware have not exposed WMI monitoring > GUID and entrypoint method WMBD could be implemented for different device > UID. > > Implement the direct call to entrypoint method for monitoring the device > UID of B550/X570 boards. > > BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=204807 > Signed-off-by: Denis Pauk <pauk.denis@gmail.com> > Co-developed-by: Ahmad Khalifa <ahmad@khalifa.ws> > Signed-off-by: Ahmad Khalifa <ahmad@khalifa.ws> > --- > Changes: > v1: > Rename each_port_arg to each_device_arg > Rename nct6775_find_asus_acpi to nct6775_asuswmi_device_match > Remove unrequired return -EEXIST, and iterate whole list of devices > Make asus_acpi_dev static > v2: > Restore break iteration logic in nct6775_asuswmi_device_match > Add config check if ACPI disabled and acpi_device_* are undefined > Pass device_id as data parameter of acpi_bus_for_each_dev > >>> Also, the use of a static variable makes me wonder: would asus_acpi_dev > be the same for both chips if there are two Super-IO chips in the system ? > > Available B550/X570/B650/B660/X670 firmwares have contained only one device > with WMBD method and provided proxy only to 0x0270 port. Difference between > board generation is only in name of the such device. > > drivers/hwmon/Kconfig | 2 +- > drivers/hwmon/nct6775-platform.c | 97 ++++++++++++++++++++++---------- > 2 files changed, 69 insertions(+), 30 deletions(-) > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > index 3176c33af6c6..300ce8115ce4 100644 > --- a/drivers/hwmon/Kconfig > +++ b/drivers/hwmon/Kconfig > @@ -1516,7 +1516,7 @@ config SENSORS_NCT6775_CORE > config SENSORS_NCT6775 > tristate "Platform driver for Nuvoton NCT6775F and compatibles" > depends on !PPC > - depends on ACPI_WMI || ACPI_WMI=n > + depends on ACPI || ACPI=n > select HWMON_VID > select SENSORS_NCT6775_CORE > help > diff --git a/drivers/hwmon/nct6775-platform.c b/drivers/hwmon/nct6775-platform.c > index bf43f73dc835..082f48785999 100644 > --- a/drivers/hwmon/nct6775-platform.c > +++ b/drivers/hwmon/nct6775-platform.c > @@ -17,7 +17,6 @@ > #include <linux/module.h> > #include <linux/platform_device.h> > #include <linux/regmap.h> > -#include <linux/wmi.h> 0-day reported various errors. I suspect those are seen with CONFIG_ACPI=n. Have you tried the build reported as problematic ? I get drivers/hwmon/nct6775-platform.c:122:28: error: ‘asus_acpi_dev’ defined but not used [-Werror=unused-variable] 122 | static struct acpi_device *asus_acpi_dev; if I try to build nct6775-platform.o with W=1 and CONFIG_ACPI=n. Overall the #ifdefs in the driver get a bit out of hand. I think it may be time to consolidate that. Not necessarily now, but sometime soon. Guenter > > #include "nct6775.h" > > @@ -107,40 +106,48 @@ struct nct6775_sio_data { > void (*sio_exit)(struct nct6775_sio_data *sio_data); > }; > > -#define ASUSWMI_MONITORING_GUID "466747A0-70EC-11DE-8A39-0800200C9A66" > +#define ASUSWMI_METHOD "WMBD" > #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 > +#define ASUSWMI_DEVICE_HID "PNP0C14" > +#define ASUSWMI_DEVICE_UID "ASUSWMI" > + > +/* > + * ASUS boards have only one device with WMI "WMBD" method and have provided > + * access to only one SuperIO chip at 0x0290. > + */ > +static struct acpi_device *asus_acpi_dev; > > static int nct6775_asuswmi_evaluate_method(u32 method_id, u8 bank, u8 reg, u8 val, u32 *retval) > { > -#if IS_ENABLED(CONFIG_ACPI_WMI) > +#if IS_ENABLED(CONFIG_ACPI) > + acpi_handle handle = acpi_device_handle(asus_acpi_dev); > u32 args = bank | (reg << 8) | (val << 16); > - struct acpi_buffer input = { (acpi_size) sizeof(args), &args }; > - struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL }; > + struct acpi_object_list input; > + union acpi_object params[3]; > + unsigned long long result; > acpi_status status; > - union acpi_object *obj; > - u32 tmp = ASUSWMI_UNSUPPORTED_METHOD; > - > - status = wmi_evaluate_method(ASUSWMI_MONITORING_GUID, 0, > - method_id, &input, &output); > > + params[0].type = ACPI_TYPE_INTEGER; > + params[0].integer.value = 0; > + params[1].type = ACPI_TYPE_INTEGER; > + params[1].integer.value = method_id; > + params[2].type = ACPI_TYPE_BUFFER; > + params[2].buffer.length = sizeof(args); > + params[2].buffer.pointer = (void *)&args; > + input.count = 3; > + input.pointer = params; > + > + status = acpi_evaluate_integer(handle, ASUSWMI_METHOD, &input, &result); > if (ACPI_FAILURE(status)) > return -EIO; > > - obj = output.pointer; > - if (obj && obj->type == ACPI_TYPE_INTEGER) > - tmp = obj->integer.value; > - > if (retval) > - *retval = tmp; > - > - kfree(obj); > + *retval = (u32)result & 0xFFFFFFFF; > > - if (tmp == ASUSWMI_UNSUPPORTED_METHOD) > - return -ENODEV; > return 0; > #else > return -EOPNOTSUPP; > @@ -1099,6 +1106,46 @@ static const char * const asus_wmi_boards[] = { > "TUF GAMING Z490-PLUS (WI-FI)", > }; > > +#if IS_ENABLED(CONFIG_ACPI) > +/* > + * Callback for acpi_bus_for_each_dev() to find the right device > + * by _UID and _HID and return 1 to stop iteration. > + */ > +static int nct6775_asuswmi_device_match(struct device *dev, void *data) > +{ > + struct acpi_device *adev = to_acpi_device(dev); > + const char *uid = acpi_device_uid(adev); > + const char *hid = acpi_device_hid(adev); > + > + if (hid && !strcmp(hid, ASUSWMI_DEVICE_HID) && > + uid && !strcmp(uid, data)) { > + asus_acpi_dev = adev; > + return 1; > + } > + > + return 0; > +} > +#endif > + > +static enum sensor_access nct6775_determine_access(const char *device_uid) > +{ > +#if IS_ENABLED(CONFIG_ACPI) > + u8 tmp; > + > + acpi_bus_for_each_dev(nct6775_asuswmi_device_match, (void *)device_uid); > + if (!asus_acpi_dev) > + return access_direct; > + > + /* if reading chip id via ACPI succeeds, use WMI "WMBD" method for access */ > + if (!nct6775_asuswmi_read(0, NCT6775_PORT_CHIPID, &tmp) && tmp) { > + pr_debug("Using Asus WMBD method of %s to access %#x chip.\n", device_uid, tmp); > + return access_asuswmi; > + } > +#endif > + > + return access_direct; > +} > + > static int __init sensors_nct6775_platform_init(void) > { > int i, err; > @@ -1109,7 +1156,6 @@ static int __init sensors_nct6775_platform_init(void) > int sioaddr[2] = { 0x2e, 0x4e }; > enum sensor_access access = access_direct; > const char *board_vendor, *board_name; > - u8 tmp; > > err = platform_driver_register(&nct6775_driver); > if (err) > @@ -1122,15 +1168,8 @@ static int __init sensors_nct6775_platform_init(void) > !strcmp(board_vendor, "ASUSTeK COMPUTER INC.")) { > err = match_string(asus_wmi_boards, ARRAY_SIZE(asus_wmi_boards), > board_name); > - if (err >= 0) { > - /* if reading chip id via WMI succeeds, use WMI */ > - if (!nct6775_asuswmi_read(0, NCT6775_PORT_CHIPID, &tmp) && tmp) { > - pr_info("Using Asus WMI to access %#x chip.\n", tmp); > - access = access_asuswmi; > - } else { > - pr_err("Can't read ChipID by Asus WMI.\n"); > - } > - } > + if (err >= 0) > + access = nct6775_determine_access(ASUSWMI_DEVICE_UID); > } > > /* > > base-commit: b0587c87abc891e313d63946ff8c9f4939d1ea1a
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 3176c33af6c6..300ce8115ce4 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -1516,7 +1516,7 @@ config SENSORS_NCT6775_CORE config SENSORS_NCT6775 tristate "Platform driver for Nuvoton NCT6775F and compatibles" depends on !PPC - depends on ACPI_WMI || ACPI_WMI=n + depends on ACPI || ACPI=n select HWMON_VID select SENSORS_NCT6775_CORE help diff --git a/drivers/hwmon/nct6775-platform.c b/drivers/hwmon/nct6775-platform.c index bf43f73dc835..082f48785999 100644 --- a/drivers/hwmon/nct6775-platform.c +++ b/drivers/hwmon/nct6775-platform.c @@ -17,7 +17,6 @@ #include <linux/module.h> #include <linux/platform_device.h> #include <linux/regmap.h> -#include <linux/wmi.h> #include "nct6775.h" @@ -107,40 +106,48 @@ struct nct6775_sio_data { void (*sio_exit)(struct nct6775_sio_data *sio_data); }; -#define ASUSWMI_MONITORING_GUID "466747A0-70EC-11DE-8A39-0800200C9A66" +#define ASUSWMI_METHOD "WMBD" #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 +#define ASUSWMI_DEVICE_HID "PNP0C14" +#define ASUSWMI_DEVICE_UID "ASUSWMI" + +/* + * ASUS boards have only one device with WMI "WMBD" method and have provided + * access to only one SuperIO chip at 0x0290. + */ +static struct acpi_device *asus_acpi_dev; static int nct6775_asuswmi_evaluate_method(u32 method_id, u8 bank, u8 reg, u8 val, u32 *retval) { -#if IS_ENABLED(CONFIG_ACPI_WMI) +#if IS_ENABLED(CONFIG_ACPI) + acpi_handle handle = acpi_device_handle(asus_acpi_dev); u32 args = bank | (reg << 8) | (val << 16); - struct acpi_buffer input = { (acpi_size) sizeof(args), &args }; - struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL }; + struct acpi_object_list input; + union acpi_object params[3]; + unsigned long long result; acpi_status status; - union acpi_object *obj; - u32 tmp = ASUSWMI_UNSUPPORTED_METHOD; - - status = wmi_evaluate_method(ASUSWMI_MONITORING_GUID, 0, - method_id, &input, &output); + params[0].type = ACPI_TYPE_INTEGER; + params[0].integer.value = 0; + params[1].type = ACPI_TYPE_INTEGER; + params[1].integer.value = method_id; + params[2].type = ACPI_TYPE_BUFFER; + params[2].buffer.length = sizeof(args); + params[2].buffer.pointer = (void *)&args; + input.count = 3; + input.pointer = params; + + status = acpi_evaluate_integer(handle, ASUSWMI_METHOD, &input, &result); if (ACPI_FAILURE(status)) return -EIO; - obj = output.pointer; - if (obj && obj->type == ACPI_TYPE_INTEGER) - tmp = obj->integer.value; - if (retval) - *retval = tmp; - - kfree(obj); + *retval = (u32)result & 0xFFFFFFFF; - if (tmp == ASUSWMI_UNSUPPORTED_METHOD) - return -ENODEV; return 0; #else return -EOPNOTSUPP; @@ -1099,6 +1106,46 @@ static const char * const asus_wmi_boards[] = { "TUF GAMING Z490-PLUS (WI-FI)", }; +#if IS_ENABLED(CONFIG_ACPI) +/* + * Callback for acpi_bus_for_each_dev() to find the right device + * by _UID and _HID and return 1 to stop iteration. + */ +static int nct6775_asuswmi_device_match(struct device *dev, void *data) +{ + struct acpi_device *adev = to_acpi_device(dev); + const char *uid = acpi_device_uid(adev); + const char *hid = acpi_device_hid(adev); + + if (hid && !strcmp(hid, ASUSWMI_DEVICE_HID) && + uid && !strcmp(uid, data)) { + asus_acpi_dev = adev; + return 1; + } + + return 0; +} +#endif + +static enum sensor_access nct6775_determine_access(const char *device_uid) +{ +#if IS_ENABLED(CONFIG_ACPI) + u8 tmp; + + acpi_bus_for_each_dev(nct6775_asuswmi_device_match, (void *)device_uid); + if (!asus_acpi_dev) + return access_direct; + + /* if reading chip id via ACPI succeeds, use WMI "WMBD" method for access */ + if (!nct6775_asuswmi_read(0, NCT6775_PORT_CHIPID, &tmp) && tmp) { + pr_debug("Using Asus WMBD method of %s to access %#x chip.\n", device_uid, tmp); + return access_asuswmi; + } +#endif + + return access_direct; +} + static int __init sensors_nct6775_platform_init(void) { int i, err; @@ -1109,7 +1156,6 @@ static int __init sensors_nct6775_platform_init(void) int sioaddr[2] = { 0x2e, 0x4e }; enum sensor_access access = access_direct; const char *board_vendor, *board_name; - u8 tmp; err = platform_driver_register(&nct6775_driver); if (err) @@ -1122,15 +1168,8 @@ static int __init sensors_nct6775_platform_init(void) !strcmp(board_vendor, "ASUSTeK COMPUTER INC.")) { err = match_string(asus_wmi_boards, ARRAY_SIZE(asus_wmi_boards), board_name); - if (err >= 0) { - /* if reading chip id via WMI succeeds, use WMI */ - if (!nct6775_asuswmi_read(0, NCT6775_PORT_CHIPID, &tmp) && tmp) { - pr_info("Using Asus WMI to access %#x chip.\n", tmp); - access = access_asuswmi; - } else { - pr_err("Can't read ChipID by Asus WMI.\n"); - } - } + if (err >= 0) + access = nct6775_determine_access(ASUSWMI_DEVICE_UID); } /*