On Mon, Jan 09, 2023 at 11:15:07PM +0200, 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@xxxxxxxxx> > Co-developed-by: Ahmad Khalifa <ahmad@xxxxxxxxxx> > Signed-off-by: Ahmad Khalifa <ahmad@xxxxxxxxxx> > --- > 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 Why ? More on that below. > make asus_acpi_dev static > > 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..1f7885af524e 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,44 @@ 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" > + > +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 +1102,50 @@ static const char * const asus_wmi_boards[] = { > "TUF GAMING Z490-PLUS (WI-FI)", > }; > > +struct each_device_arg { > + struct acpi_device *adev; > + const char *match; > +}; > + > +/* > + * Callback for acpi_bus_for_each_dev() to find the right device > + * by _UID and _HID and store to each_device_arg. > + */ > +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); > + struct each_device_arg *arg = data; > + > + if (hid && !strcmp(hid, ASUSWMI_DEVICE_HID) && > + uid && !strcmp(uid, arg->match)) { > + arg->adev = adev; Why not return 1 for match here ? If there is a reason to look for the last match instead of the first match, it needs to be explained. > + } > + > + return 0; > +} > + > +static enum sensor_access nct6775_determine_access(const char *device_uid) > +{ > + struct each_device_arg arg; > + u8 tmp; > + > + arg.match = device_uid; > + acpi_bus_for_each_dev(nct6775_asuswmi_device_match, &arg); > + if (!arg.adev) > + return access_direct; > + > + asus_acpi_dev = arg.adev; The use of the static variable made me look into this further. Why all that complexity with struct each_device_arg and passing the structure and adev to/from the match function instead of just passing the match string as parameter and storing the matching adev directly in asus_acpi_dev instead of passing it back and storing it here ? 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 ? Thanks, Guenter > + /* 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; > + } > + > + 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 > -- > 2.39.0 >
