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

From: Guenter Roeck
Date: Thu Sep 09 2021 - 13:00:24 EST

On Thu, Sep 09, 2021 at 12:36:04AM +0300, 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.
>
> v2: split changes to separate patches
> limit WMI usage by DMI_BOARD_NAME in checked ASUS motherboards
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=204807
> Signed-off-by: Bernhard Seibold <mail@xxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Denis Pauk <pauk.denis@xxxxxxxxx>
> 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>
> ---
> drivers/hwmon/nct6775.c | 219 ++++++++++++++++++++++++++++++++++++----
> 1 file changed, 198 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/hwmon/nct6775.c b/drivers/hwmon/nct6775.c
> index 908521ce2ee8..ae0344811821 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;
> void (*outb)(struct nct6775_sio_data *sio_data, int reg, int val);
> int (*inb)(struct nct6775_sio_data *sio_data, int reg);
> void (*select)(struct nct6775_sio_data *sio_data, int ld);
> @@ -143,6 +147,92 @@ struct nct6775_sio_data {
> void (*exit)(struct nct6775_sio_data *sio_data);
> };
>
> +#define ASUSWMI_MGMT2_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)
> +{
> + u32 args = bank | (reg << 8) | (val << 16);
> + struct acpi_buffer input = { (acpi_size) sizeof(args), &args };
> + struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> + acpi_status status;
> + union acpi_object *obj;
> + u32 tmp = 0;
> +
> + status = wmi_evaluate_method(ASUSWMI_MGMT2_GUID, 0, method_id,
> + &input, &output);
> +
> + if (ACPI_FAILURE(status))
> + return -EIO;
> +
> + obj = (union acpi_object *)output.pointer;
> + if (obj && obj->type == ACPI_TYPE_INTEGER)
> + tmp = (u32) obj->integer.value;
> +
> + if (retval)
> + *retval = tmp;
> +
> + kfree(obj);
> +
> + if (tmp == ASUSWMI_UNSUPPORTED_METHOD)
> + return -ENODEV;
> + return 0;
> +}
> +
> +static inline int
> +nct6775_asuswmi_write(u8 bank, u8 reg, u8 val)
> +{
> + return asuswmi_evaluate_method(ASUSWMI_METHODID_WHWM, bank, reg, val, 0);
> +}
> +
> +static inline int
> +nct6775_asuswmi_read(u8 bank, u8 reg, u8 *val)
> +{
> + u32 tmp;
> + int ret = asuswmi_evaluate_method(ASUSWMI_METHODID_RHWM, bank, reg, 0, &tmp);
> + *val = tmp & 0xff;
> + return ret;
> +}
> +
> +static int
> +superio_wmi_inb(struct nct6775_sio_data *sio_data, int reg)
> +{
> + int tmp;
> +
> + asuswmi_evaluate_method(ASUSWMI_METHODID_RSIO,
> + sio_data->ld, reg, 0, &tmp);
> + return tmp;
> +}
> +
> +static void
> +superio_wmi_outb(struct nct6775_sio_data *sio_data, int reg, int val)
> +{
> + asuswmi_evaluate_method(ASUSWMI_METHODID_WSIO,
> + sio_data->ld, reg, val, 0);
> +}
> +
> +static void
> +superio_wmi_select(struct nct6775_sio_data *sio_data, int ld)
> +{
> + sio_data->ld = ld;
> +}
> +
> +static int
> +superio_wmi_enter(struct nct6775_sio_data *sio_data)
> +{
> + return 0;
> +}
> +
> +static void
> +superio_wmi_exit(struct nct6775_sio_data *sio_data)
> +{
> +}
> +
> static void
> superio_outb(struct nct6775_sio_data *sio_data, int reg, int val)
> {
> @@ -1090,6 +1180,7 @@ struct nct6775_data {
> int addr; /* IO base of hw monitor block */
> int sioreg; /* SIO register address */
> enum kinds kind;
> + enum sensor_access access;
> const char *name;
>
> const struct attribute_group *groups[6];
> @@ -1432,6 +1523,11 @@ static inline void nct6775_set_bank(struct nct6775_data *data, u16 reg)
> {
> u8 bank = reg >> 8;
>
> + if (data->access == access_asuswmi) {
> + data->bank = bank;
> + return;
> + }
> +

All that preparation, and then this is still needed. The accessor functions
(nct6775_set_bank, nct6775_read_value, nct6775_write_value) should be
implemented as pointers instead.

Guenter

> if (data->bank != bank) {
> outb_p(NCT6775_REG_BANK, data->addr + ADDR_REG_OFFSET);
> outb_p(bank, data->addr + DATA_REG_OFFSET);
> @@ -1442,8 +1538,21 @@ static inline void nct6775_set_bank(struct nct6775_data *data, u16 reg)
> static u16 nct6775_read_value(struct nct6775_data *data, u16 reg)
> {
> int res, word_sized = is_word_sized(data, reg);
> + u8 tmp;
>
> nct6775_set_bank(data, reg);
> +
> + if (data->access == access_asuswmi) {
> + nct6775_asuswmi_read(data->bank, reg, &tmp);
> + res = (tmp & 0xff);
> + if (word_sized) {
> + nct6775_asuswmi_read(data->bank,
> + (reg & 0xff) + 1, &tmp);
> + res = (res << 8) + (tmp & 0xff);
> + }
> + return res;
> + }
> +
> outb_p(reg & 0xff, data->addr + ADDR_REG_OFFSET);
> res = inb_p(data->addr + DATA_REG_OFFSET);
> if (word_sized) {
> @@ -1459,6 +1568,21 @@ static int nct6775_write_value(struct nct6775_data *data, u16 reg, u16 value)
> int word_sized = is_word_sized(data, reg);
>
> nct6775_set_bank(data, reg);
> +
> + if (data->access == access_asuswmi) {
> + if (word_sized) {
> + nct6775_asuswmi_write(data->bank, (reg & 0xff),
> + (value >> 8) & 0xff);
> + nct6775_asuswmi_write(data->bank, (reg & 0xff) + 1,
> + value & 0xff);
> + } else {
> + nct6775_asuswmi_write(data->bank, (reg & 0xff),
> + value);
> + }
> +
> + return 0;
> + }
> +
> outb_p(reg & 0xff, data->addr + ADDR_REG_OFFSET);
> if (word_sized) {
> outb_p(value >> 8, data->addr + DATA_REG_OFFSET);
> @@ -3821,13 +3945,15 @@ static int nct6775_probe(struct platform_device *pdev)
> struct device *hwmon_dev;
> int num_attr_groups = 0;
>
> - res = platform_get_resource(pdev, IORESOURCE_IO, 0);
> - if (!res)
> - return -EBUSY;
> + if (sio_data->access == access_direct) {
> + res = platform_get_resource(pdev, IORESOURCE_IO, 0);
> + if (!res)
> + return -EBUSY;
>
> - if (!devm_request_region(&pdev->dev, res->start, IOREGION_LENGTH,
> - DRVNAME))
> - return -EBUSY;
> + if (!devm_request_region(&pdev->dev, res->start, IOREGION_LENGTH,
> + DRVNAME))
> + return -EBUSY;
> + }
>
> data = devm_kzalloc(&pdev->dev, sizeof(struct nct6775_data),
> GFP_KERNEL);
> @@ -3836,7 +3962,11 @@ static int nct6775_probe(struct platform_device *pdev)
>
> data->kind = sio_data->kind;
> data->sioreg = sio_data->sioreg;
> - data->addr = res->start;
> + data->access = sio_data->access;
> + if (sio_data->access == access_direct)
> + data->addr = res->start;
> + else
> + data->addr = 0;
> mutex_init(&data->update_lock);
> data->name = nct6775_device_names[data->kind];
> data->bank = 0xff; /* Force initial bank selection */
> @@ -4747,6 +4877,7 @@ static int __init nct6775_find(int sioaddr, struct nct6775_sio_data *sio_data)
> int err;
> int addr;
>
> + sio_data->access = access_direct;
> sio_data->sioreg = sioaddr;
>
> err = sio_data->enter(sio_data);
> @@ -4841,6 +4972,21 @@ static int __init nct6775_find(int sioaddr, struct nct6775_sio_data *sio_data)
> */
> static struct platform_device *pdev[2];
>
> +#define NCT6775_REG_CHIPID 0x58
> +
> +static const char * const asus_wmi_boards[] = {
> + "PRIME B460-PLUS",
> + "ROG CROSSHAIR VIII IMPACT",
> + "ROG STRIX B550-E 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)"
> +};
> +
> static int __init sensors_nct6775_init(void)
> {
> int i, err;
> @@ -4849,11 +4995,29 @@ static int __init sensors_nct6775_init(void)
> struct resource res;
> struct nct6775_sio_data sio_data;
> int sioaddr[2] = { 0x2e, 0x4e };
> + const char *board_vendor, *board_name;
> + enum sensor_access access = access_direct;
> + u8 tmp;
>
> err = platform_driver_register(&nct6775_driver);
> if (err)
> return err;
>
> + board_vendor = dmi_get_system_info(DMI_BOARD_VENDOR);
> + board_name = dmi_get_system_info(DMI_BOARD_NAME);
> +
> + if (board_name && board_vendor &&
> + !strcmp(board_vendor, "ASUSTeK COMPUTER INC.")) {
> + if (match_string(asus_wmi_boards,
> + ARRAY_SIZE(asus_wmi_boards), board_name) != -EINVAL) {
> + /* if reading chip id via WMI succeeds, use WMI */
> + if (!nct6775_asuswmi_read(0, NCT6775_REG_CHIPID, &tmp)) {
> + pr_info("Using Asus WMI to access chip\n");
> + access = access_asuswmi;
> + }
> + }
> + }
> +
> /*
> * initialize sio_data->kind and sio_data->sioreg.
> *
> @@ -4874,6 +5038,17 @@ static int __init sensors_nct6775_init(void)
>
> found = true;
>
> + /* Update access method */
> + sio_data.access = access;
> +
> + if (access == access_asuswmi) {
> + sio_data.outb = superio_wmi_outb;
> + sio_data.inb = superio_wmi_inb;
> + sio_data.select = superio_wmi_select;
> + sio_data.enter = superio_wmi_enter;
> + sio_data.exit = superio_wmi_exit;
> + }
> +
> pdev[i] = platform_device_alloc(DRVNAME, address);
> if (!pdev[i]) {
> err = -ENOMEM;
> @@ -4885,23 +5060,25 @@ static int __init sensors_nct6775_init(void)
> if (err)
> goto exit_device_put;
>
> - memset(&res, 0, sizeof(res));
> - res.name = DRVNAME;
> - res.start = address + IOREGION_OFFSET;
> - res.end = address + IOREGION_OFFSET + IOREGION_LENGTH - 1;
> - res.flags = IORESOURCE_IO;
> + if (sio_data.access == access_direct) {
> + memset(&res, 0, sizeof(res));
> + res.name = DRVNAME;
> + res.start = address + IOREGION_OFFSET;
> + res.end = address + IOREGION_OFFSET + IOREGION_LENGTH - 1;
> + res.flags = IORESOURCE_IO;
> +
> + err = acpi_check_resource_conflict(&res);
> + if (err) {
> + platform_device_put(pdev[i]);
> + pdev[i] = NULL;
> + continue;
> + }
>
> - err = acpi_check_resource_conflict(&res);
> - if (err) {
> - platform_device_put(pdev[i]);
> - pdev[i] = NULL;
> - continue;
> + err = platform_device_add_resources(pdev[i], &res, 1);
> + if (err)
> + goto exit_device_put;
> }
>
> - err = platform_device_add_resources(pdev[i], &res, 1);
> - if (err)
> - goto exit_device_put;
> -
> /* platform_device_add calls probe() */
> err = platform_device_add(pdev[i]);
> if (err)
> --
> 2.33.0
>