linux-hwmon.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] hwmon: (nct6775) Directly call ASUS ACPI WMI method
@ 2023-01-09 13:35 Denis Pauk
  2023-01-09 13:35 ` [PATCH 2/2] hwmon: (nct6775) B650/B660/X670 ASUS boards support Denis Pauk
  2023-01-09 16:22 ` [PATCH 1/2] hwmon: (nct6775) Directly call ASUS ACPI WMI method Guenter Roeck
  0 siblings, 2 replies; 3+ messages in thread
From: Denis Pauk @ 2023-01-09 13:35 UTC (permalink / raw)
  Cc: ahmad, chunkeey, greg, hubert.banas, igor, jaap.dehaan, jdelvare,
	jeroen, jonfarr87, jwp, kdudka, kernel, kpietrzak, linux-hwmon,
	linux-kernel, linux, me, metalcaedes, michael,
	mikhail.v.gavrilov, mundanedefoliation, nephartyz, oleksandr,
	pauk.denis, pehlm, renedis, robert, sahan.h.fernando,
	sebastian.arnhold, sefoci9222, sst, to.eivind, torvic9

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>
---
 drivers/hwmon/Kconfig            |   2 +-
 drivers/hwmon/nct6775-platform.c | 100 ++++++++++++++++++++++---------
 2 files changed, 72 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..7f5b430e1e2c 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"
+
+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,53 @@ static const char * const asus_wmi_boards[] = {
 	"TUF GAMING Z490-PLUS (WI-FI)",
 };
 
+struct each_port_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 stop.
+ * return is an error to exit the loop
+ */
+static int nct6775_find_asus_acpi(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_port_arg *arg = data;
+
+	if (hid && !strcmp(hid, ASUSWMI_DEVICE_HID) &&
+		uid && !strcmp(uid, arg->match)) {
+		arg->adev = adev;
+		return -EEXIST;
+	}
+
+	return 0;
+}
+
+static enum sensor_access nct6775_determine_access(const char *device_uid)
+{
+	struct each_port_arg arg;
+	u8 tmp;
+
+	arg.match = device_uid;
+	acpi_bus_for_each_dev(nct6775_find_asus_acpi, &arg);
+	if (!arg.adev)
+		return access_direct;
+
+	asus_acpi_dev = arg.adev;
+	/* if reading chip id via ACPI succeeds, use WMI "WMBD" method for access */
+	if (!nct6775_asuswmi_read(0, NCT6775_PORT_CHIPID, &tmp) && tmp) {
+		pr_info("Using Asus ACPI to access %#x chip.\n", tmp);
+		return access_asuswmi;
+	}
+
+	pr_err("Can't read ChipID by Asus ACPI %s device.\n", device_uid);
+	return access_direct;
+}
+
 static int __init sensors_nct6775_platform_init(void)
 {
 	int i, err;
@@ -1109,7 +1159,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 +1171,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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH 2/2] hwmon: (nct6775) B650/B660/X670 ASUS boards support
  2023-01-09 13:35 [PATCH 1/2] hwmon: (nct6775) Directly call ASUS ACPI WMI method Denis Pauk
@ 2023-01-09 13:35 ` Denis Pauk
  2023-01-09 16:22 ` [PATCH 1/2] hwmon: (nct6775) Directly call ASUS ACPI WMI method Guenter Roeck
  1 sibling, 0 replies; 3+ messages in thread
From: Denis Pauk @ 2023-01-09 13:35 UTC (permalink / raw)
  Cc: ahmad, chunkeey, greg, hubert.banas, igor, jaap.dehaan, jdelvare,
	jeroen, jonfarr87, jwp, kdudka, kernel, kpietrzak, linux-hwmon,
	linux-kernel, linux, me, metalcaedes, michael,
	mikhail.v.gavrilov, mundanedefoliation, nephartyz, oleksandr,
	pauk.denis, pehlm, renedis, robert, sahan.h.fernando,
	sebastian.arnhold, sefoci9222, sst, to.eivind, torvic9

Boards such as:
  "EX-B660M-V5 PRO D4",
  "PRIME B650-PLUS",
  "PRIME B650M-A",
  "PRIME B650M-A AX",
  "PRIME B650M-A II",
  "PRIME B650M-A WIFI",
  "PRIME B650M-A WIFI II",
  "PRIME B660M-A D4",
  "PRIME B660M-A WIFI D4",
  "PRIME X670-P",
  "PRIME X670-P WIFI",
  "PRIME X670E-PRO WIFI",
  "Pro B660M-C-D4",
  "ProArt B660-CREATOR D4",
  "ProArt X670E-CREATOR WIFI",
  "ROG CROSSHAIR X670E EXTREME",
  "ROG CROSSHAIR X670E GENE",
  "ROG CROSSHAIR X670E HERO",
  "ROG MAXIMUS XIII EXTREME GLACIAL",
  "ROG MAXIMUS Z690 EXTREME",
  "ROG MAXIMUS Z690 EXTREME GLACIAL",
  "ROG STRIX B650-A GAMING WIFI",
  "ROG STRIX B650E-E GAMING WIFI",
  "ROG STRIX B650E-F GAMING WIFI",
  "ROG STRIX B650E-I GAMING WIFI",
  "ROG STRIX B660-A GAMING WIFI D4",
  "ROG STRIX B660-F GAMING WIFI",
  "ROG STRIX B660-G GAMING WIFI",
  "ROG STRIX B660-I GAMING WIFI",
  "ROG STRIX X670E-A GAMING WIFI",
  "ROG STRIX X670E-E GAMING WIFI",
  "ROG STRIX X670E-F GAMING WIFI",
  "ROG STRIX X670E-I GAMING WIFI",
  "ROG STRIX Z590-A GAMING WIFI II",
  "ROG STRIX Z690-A GAMING WIFI D4",
  "TUF GAMING B650-PLUS",
  "TUF GAMING B650-PLUS WIFI",
  "TUF GAMING B650M-PLUS",
  "TUF GAMING B650M-PLUS WIFI",
  "TUF GAMING B660M-PLUS WIFI",
  "TUF GAMING X670E-PLUS",
  "TUF GAMING X670E-PLUS WIFI",
  "TUF GAMING Z590-PLUS WIFI",
have got a nct6775 chip, but by default there's no use of it
because of resource conflict with WMI method.

This commit adds such boards to the monitoring list with new ACPI device 
UID. 

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>
Tested-by: Jeroen Beerstra <jeroen@beerstra.org>
Tested-by: Slawomir Stepien <sst@poczta.fm>

---

B650/B660/X670 boards use NCT6799D and code is checked with 

https://patchwork.kernel.org/project/linux-hwmon/patch/20221228135744.281752-1-linux@roeck-us.net/  

Feedback form Jeroen Beerstra https://bugzilla.kernel.org/show_bug.cgi?id=204807#c282

nct6799-isa-0290
Adapter: ISA adapter
in0:                        1.37 V  (min =  +0.00 V, max =  +1.74 V)
in1:                      1000.00 mV (min =  +0.00 V, max =  +0.00 V)  ALARM
in2:                        3.39 V  (min =  +0.00 V, max =  +0.00 V)  ALARM
in3:                        3.34 V  (min =  +0.00 V, max =  +0.00 V)  ALARM
in4:                        1.02 V  (min =  +0.00 V, max =  +0.00 V)  ALARM
in5:                        1.03 V  (min =  +0.00 V, max =  +0.00 V)  ALARM
in6:                      752.00 mV (min =  +0.00 V, max =  +0.00 V)  ALARM
in7:                        3.39 V  (min =  +0.00 V, max =  +0.00 V)  ALARM
in8:                        3.30 V  (min =  +0.00 V, max =  +0.00 V)  ALARM
in9:                        1.67 V  (min =  +0.00 V, max =  +0.00 V)  ALARM
in10:                     560.00 mV (min =  +0.00 V, max =  +0.00 V)  ALARM
in11:                     560.00 mV (min =  +0.00 V, max =  +0.00 V)  ALARM
in12:                       1.03 V  (min =  +0.00 V, max =  +0.00 V)  ALARM
in13:                     496.00 mV (min =  +0.00 V, max =  +0.00 V)  ALARM
in14:                     424.00 mV (min =  +0.00 V, max =  +0.00 V)  ALARM
fan1:                      755 RPM  (min =    0 RPM)
fan2:                      819 RPM  (min =    0 RPM)
fan3:                      712 RPM  (min =    0 RPM)
fan4:                        0 RPM  (min =    0 RPM)
fan5:                      674 RPM  (min =    0 RPM)
fan7:                        0 RPM  (min =    0 RPM)
SYSTIN:                    +31.0°C  (high = +80.0°C, hyst = +75.0°C)  sensor = thermistor
CPUTIN:                    +36.5°C  (high = +80.0°C, hyst = +75.0°C)  sensor = thermistor
AUXTIN0:                   +39.5°C    sensor = thermistor
AUXTIN1:                   +19.0°C    sensor = thermistor
AUXTIN2:                   +20.0°C    sensor = thermistor
AUXTIN3:                   +73.0°C    sensor = thermistor
PECI Agent 0 Calibration:  +54.0°C  
PCH_CHIP_CPU_MAX_TEMP:      +0.0°C  
PCH_CHIP_TEMP:              +0.0°C  
PCH_CPU_TEMP:               +0.0°C  
TSI0_TEMP:                 +64.5°C  
intrusion0:               ALARM
intrusion1:               ALARM
beep_enable:              disabled


---
 drivers/hwmon/nct6775-platform.c | 52 ++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/drivers/hwmon/nct6775-platform.c b/drivers/hwmon/nct6775-platform.c
index 7f5b430e1e2c..b0c6958809b9 100644
--- a/drivers/hwmon/nct6775-platform.c
+++ b/drivers/hwmon/nct6775-platform.c
@@ -114,6 +114,7 @@ struct nct6775_sio_data {
 #define ASUSWMI_UNSUPPORTED_METHOD	0xFFFFFFFE
 #define ASUSWMI_DEVICE_HID		"PNP0C14"
 #define ASUSWMI_DEVICE_UID		"ASUSWMI"
+#define ASUSMSI_DEVICE_UID		"AsusMbSwInterface"
 
 struct acpi_device *asus_acpi_dev;
 
@@ -1102,6 +1103,52 @@ static const char * const asus_wmi_boards[] = {
 	"TUF GAMING Z490-PLUS (WI-FI)",
 };
 
+static const char * const asus_msi_boards[] = {
+	"EX-B660M-V5 PRO D4",
+	"PRIME B650-PLUS",
+	"PRIME B650M-A",
+	"PRIME B650M-A AX",
+	"PRIME B650M-A II",
+	"PRIME B650M-A WIFI",
+	"PRIME B650M-A WIFI II",
+	"PRIME B660M-A D4",
+	"PRIME B660M-A WIFI D4",
+	"PRIME X670-P",
+	"PRIME X670-P WIFI",
+	"PRIME X670E-PRO WIFI",
+	"Pro B660M-C-D4",
+	"ProArt B660-CREATOR D4",
+	"ProArt X670E-CREATOR WIFI",
+	"ROG CROSSHAIR X670E EXTREME",
+	"ROG CROSSHAIR X670E GENE",
+	"ROG CROSSHAIR X670E HERO",
+	"ROG MAXIMUS XIII EXTREME GLACIAL",
+	"ROG MAXIMUS Z690 EXTREME",
+	"ROG MAXIMUS Z690 EXTREME GLACIAL",
+	"ROG STRIX B650-A GAMING WIFI",
+	"ROG STRIX B650E-E GAMING WIFI",
+	"ROG STRIX B650E-F GAMING WIFI",
+	"ROG STRIX B650E-I GAMING WIFI",
+	"ROG STRIX B660-A GAMING WIFI D4",
+	"ROG STRIX B660-F GAMING WIFI",
+	"ROG STRIX B660-G GAMING WIFI",
+	"ROG STRIX B660-I GAMING WIFI",
+	"ROG STRIX X670E-A GAMING WIFI",
+	"ROG STRIX X670E-E GAMING WIFI",
+	"ROG STRIX X670E-F GAMING WIFI",
+	"ROG STRIX X670E-I GAMING WIFI",
+	"ROG STRIX Z590-A GAMING WIFI II",
+	"ROG STRIX Z690-A GAMING WIFI D4",
+	"TUF GAMING B650-PLUS",
+	"TUF GAMING B650-PLUS WIFI",
+	"TUF GAMING B650M-PLUS",
+	"TUF GAMING B650M-PLUS WIFI",
+	"TUF GAMING B660M-PLUS WIFI",
+	"TUF GAMING X670E-PLUS",
+	"TUF GAMING X670E-PLUS WIFI",
+	"TUF GAMING Z590-PLUS WIFI",
+};
+
 struct each_port_arg {
 	struct acpi_device *adev;
 	const char *match;
@@ -1173,6 +1220,11 @@ static int __init sensors_nct6775_platform_init(void)
 				   board_name);
 		if (err >= 0)
 			access = nct6775_determine_access(ASUSWMI_DEVICE_UID);
+
+		err = match_string(asus_msi_boards, ARRAY_SIZE(asus_msi_boards),
+				   board_name);
+		if (err >= 0)
+			access = nct6775_determine_access(ASUSMSI_DEVICE_UID);
 	}
 
 	/*
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/2] hwmon: (nct6775) Directly call ASUS ACPI WMI method
  2023-01-09 13:35 [PATCH 1/2] hwmon: (nct6775) Directly call ASUS ACPI WMI method Denis Pauk
  2023-01-09 13:35 ` [PATCH 2/2] hwmon: (nct6775) B650/B660/X670 ASUS boards support Denis Pauk
@ 2023-01-09 16:22 ` Guenter Roeck
  1 sibling, 0 replies; 3+ messages in thread
From: Guenter Roeck @ 2023-01-09 16:22 UTC (permalink / raw)
  To: Denis Pauk
  Cc: ahmad, chunkeey, greg, hubert.banas, igor, jaap.dehaan, jdelvare,
	jeroen, jonfarr87, jwp, kdudka, kernel, kpietrzak, linux-hwmon,
	linux-kernel, me, metalcaedes, michael, mikhail.v.gavrilov,
	mundanedefoliation, nephartyz, oleksandr, pehlm, renedis, robert,
	sahan.h.fernando, sebastian.arnhold, sefoci9222, sst, to.eivind,
	torvic9

On 1/9/23 05:35, 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>
> ---
>   drivers/hwmon/Kconfig            |   2 +-
>   drivers/hwmon/nct6775-platform.c | 100 ++++++++++++++++++++++---------
>   2 files changed, 72 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..7f5b430e1e2c 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"
> +
> +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,53 @@ static const char * const asus_wmi_boards[] = {
>   	"TUF GAMING Z490-PLUS (WI-FI)",
>   };
>   
> +struct each_port_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 stop.
> + * return is an error to exit the loop

This is a match function, which does not and should not return error/0
but a match result (true / false or 1/0), and it should be named
accordingly.

> + */
> +static int nct6775_find_asus_acpi(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_port_arg *arg = data;
> +
> +	if (hid && !strcmp(hid, ASUSWMI_DEVICE_HID) &&
> +		uid && !strcmp(uid, arg->match)) {
> +		arg->adev = adev;
> +		return -EEXIST;

... and is most definitely should not return "File Exists".

> +	}
> +
> +	return 0;
> +}
> +
> +static enum sensor_access nct6775_determine_access(const char *device_uid)
> +{
> +	struct each_port_arg arg;
> +	u8 tmp;
> +
> +	arg.match = device_uid;
> +	acpi_bus_for_each_dev(nct6775_find_asus_acpi, &arg);
> +	if (!arg.adev)
> +		return access_direct;
> +
> +	asus_acpi_dev = arg.adev;
> +	/* if reading chip id via ACPI succeeds, use WMI "WMBD" method for access */
> +	if (!nct6775_asuswmi_read(0, NCT6775_PORT_CHIPID, &tmp) && tmp) {
> +		pr_info("Using Asus ACPI to access %#x chip.\n", tmp);

Please reduce the noise. Make this a debug message if you think you need it.

> +		return access_asuswmi;
> +	}
> +
> +	pr_err("Can't read ChipID by Asus ACPI %s device.\n", device_uid);

If this is an error, I expect the code to return an error and abort
the probe function. Otherwise make it a debug message. Also, even as
debug message this is misleading, because it suggests and error when
the result is really that the code uses direct access. There should
be something like "Using direct access" or "Accessing chip registers
directly" in the message.

> +	return access_direct;
> +}
> +
>   static int __init sensors_nct6775_platform_init(void)
>   {
>   	int i, err;
> @@ -1109,7 +1159,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 +1171,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");
> -			}

Ah, that noise was already there. Comments above still apply, though.

Thanks,
Guenter

> -		}
> +		if (err >= 0)
> +			access = nct6775_determine_access(ASUSWMI_DEVICE_UID);
>   	}
>   
>   	/*
> 
> base-commit: b0587c87abc891e313d63946ff8c9f4939d1ea1a


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-01-09 16:22 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-09 13:35 [PATCH 1/2] hwmon: (nct6775) Directly call ASUS ACPI WMI method Denis Pauk
2023-01-09 13:35 ` [PATCH 2/2] hwmon: (nct6775) B650/B660/X670 ASUS boards support Denis Pauk
2023-01-09 16:22 ` [PATCH 1/2] hwmon: (nct6775) Directly call ASUS ACPI WMI method Guenter Roeck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).