All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/3] hwmon: Support access to the NCT677x via Asus WMI.
@ 2021-09-16 20:22 Denis Pauk
  2021-09-16 20:22 ` [PATCH v7 " Denis Pauk
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Denis Pauk @ 2021-09-16 20:22 UTC (permalink / raw)
  Cc: pauk.denis, Bernhard Seibold, Pär Ekholm, to.eivind,
	Artem S . Tashkinov, Vittorio Roberto Alfieri, Sahan Fernando,
	Andy Shevchenko, Guenter Roeck, Jean Delvare, linux-hwmon,
	linux-kernel

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.

Could you please review?

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=204807
Signed-off-by: Denis Pauk <pauk.denis@gmail.com>
Co-developed-by: Bernhard Seibold <mail@bernhard-seibold.de>
Signed-off-by: Bernhard Seibold <mail@bernhard-seibold.de>
Tested-by: Pär Ekholm <pehlm@pekholm.org>
Tested-by: <to.eivind@gmail.com>
Tested-by: Artem S. Tashkinov <aros@gmx.com>
Tested-by: Vittorio Roberto Alfieri <me@rebtoor.com>
Tested-by: Sahan Fernando <sahan.h.fernando@gmail.com>
Cc: Andy Shevchenko <andriy.shevchenko@intel.com>
Cc: Guenter Roeck <linux@roeck-us.net>

---
Changes in v6:
 - Add dependency "ACPI_WMI || ACPI_WMI=n" to Kconfig, 
 - Minimize size of code under IS_ENABLED(CONFIG_ACPI_WMI),
 - Remove not requred check of platform_get_resource result,
 - Split function pointers patch to two separate patches,
 - Add more board ROG CROSSHAIR * names from bugzilla.
 
Changes in v5:
  - Use IS_ENABLED(CONFIG_ACPI_WMI) instead defined(CONFIG_ACPI_WMI)

Changes in v4:
  - Fix naming conflict with inb, outb by add sio prefix to callbacks in
    nct6775_sio_data.
  - Fix build without ACPI WMI.    
    
Changes in v3:
  - Remove unrequired type conversions.
  - Make function declarations one line.
  - Use nct6775 function pointers in struct nct6775_data instead direct calls.

Changes in v2:
  - Split changes to separate patches.
  - Limit WMI usage by DMI_BOARD_NAME in checked ASUS motherboards.
  - Rearrange code for directly use struct nct6775_sio_data in superio_*()
    functions.

Denis Pauk (2):
  hwmon: (nct6775) Use sio_data in superio_*().
  hwmon: (nct6775) Support access via Asus WMI

Denis Pauk (3):
  hwmon: (nct6775) Use superio_*() function pointers in sio_data.
  hwmon: (nct6775) Use nct6775_*() function pointers in nct6775_data.
  hwmon: (nct6775) Support access via Asus WMI

 drivers/hwmon/Kconfig   |   1 +
 drivers/hwmon/nct6775.c | 698 ++++++++++++++++++++++++++--------------
 2 files changed, 456 insertions(+), 243 deletions(-)

-- 
2.33.0


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

* [PATCH v7 0/3] hwmon: Support access to the NCT677x via Asus WMI.
  2021-09-16 20:22 [PATCH v6 0/3] hwmon: Support access to the NCT677x via Asus WMI Denis Pauk
@ 2021-09-16 20:22 ` Denis Pauk
  2021-09-16 20:22 ` [PATCH v7 1/3] hwmon: (nct6775) Use superio_*() function pointers in sio_data Denis Pauk
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Denis Pauk @ 2021-09-16 20:22 UTC (permalink / raw)
  Cc: pauk.denis, Bernhard Seibold, Pär Ekholm, to.eivind,
	Artem S . Tashkinov, Vittorio Roberto Alfieri, Sahan Fernando,
	Andy Shevchenko, Guenter Roeck, Jean Delvare, linux-hwmon,
	linux-kernel

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.

Could you please review?

@Andy Shevchenko: I have removed (tmp & 0xff) for i8 variable and changed 
'err != -EINVAL' to 'err >= 0'. Also I have changed default value of 
parameter for wmi_evaluate_method() and replaced ASUSWMI_MGMT2_GUID
to  ASUSWMI_MONITORING_GUID that looks more user friendly name as for me.

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=204807
Signed-off-by: Denis Pauk <pauk.denis@gmail.com>
Co-developed-by: Bernhard Seibold <mail@bernhard-seibold.de>
Signed-off-by: Bernhard Seibold <mail@bernhard-seibold.de>
Tested-by: Pär Ekholm <pehlm@pekholm.org>
Tested-by: <to.eivind@gmail.com>
Tested-by: Artem S. Tashkinov <aros@gmx.com>
Tested-by: Vittorio Roberto Alfieri <me@rebtoor.com>
Tested-by: Sahan Fernando <sahan.h.fernando@gmail.com>
Cc: Andy Shevchenko <andriy.shevchenko@intel.com>
Cc: Guenter Roeck <linux@roeck-us.net>

---
Changes in v7:
  - Remove unrequred & 0xff with i8 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:
 - Add dependency "ACPI_WMI || ACPI_WMI=n" to Kconfig, 
 - Minimize size of code under IS_ENABLED(CONFIG_ACPI_WMI),
 - Remove not requred check of platform_get_resource result,
 - Split function pointers patch to two separate patches,
 - Add more board ROG CROSSHAIR * names from bugzilla.
 
Changes in v5:
  - Use IS_ENABLED(CONFIG_ACPI_WMI) instead defined(CONFIG_ACPI_WMI)

Changes in v4:
  - Fix naming conflict with inb, outb by add sio prefix to callbacks in
    nct6775_sio_data.
  - Fix build without ACPI WMI.    
    
Changes in v3:
  - Remove unrequired type conversions.
  - Make function declarations one line.
  - Use nct6775 function pointers in struct nct6775_data instead direct calls.

Changes in v2:
  - Split changes to separate patches.
  - Limit WMI usage by DMI_BOARD_NAME in checked ASUS motherboards.
  - Rearrange code for directly use struct nct6775_sio_data in superio_*()
    functions.

Denis Pauk (2):
  hwmon: (nct6775) Use sio_data in superio_*().
  hwmon: (nct6775) Support access via Asus WMI

Denis Pauk (3):
  hwmon: (nct6775) Use superio_*() function pointers in sio_data.
  hwmon: (nct6775) Use nct6775_*() function pointers in nct6775_data.
  hwmon: (nct6775) Support access via Asus WMI

Denis Pauk (3):
  hwmon: (nct6775) Use superio_*() function pointers in sio_data.
  hwmon: (nct6775) Use nct6775_*() function pointers in nct6775_data.
  hwmon: (nct6775) Support access via Asus WMI

 drivers/hwmon/Kconfig   |   1 +
 drivers/hwmon/nct6775.c | 696 ++++++++++++++++++++++++++--------------
 2 files changed, 454 insertions(+), 243 deletions(-)

-- 
2.33.0


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

* [PATCH v7 1/3] hwmon: (nct6775) Use superio_*() function pointers in sio_data.
  2021-09-16 20:22 [PATCH v6 0/3] hwmon: Support access to the NCT677x via Asus WMI Denis Pauk
  2021-09-16 20:22 ` [PATCH v7 " Denis Pauk
@ 2021-09-16 20:22 ` Denis Pauk
  2021-09-16 20:22 ` [PATCH v7 2/3] hwmon: (nct6775) Use nct6775_*() function pointers in nct6775_data Denis Pauk
  2021-09-16 20:22 ` [PATCH v7 3/3] hwmon: (nct6775) Support access via Asus WMI Denis Pauk
  3 siblings, 0 replies; 8+ messages in thread
From: Denis Pauk @ 2021-09-16 20:22 UTC (permalink / raw)
  Cc: pauk.denis, Bernhard Seibold, Andy Shevchenko, Guenter Roeck,
	Jean Delvare, linux-hwmon, linux-kernel

Prepare for platform specific callbacks usage:
* Rearrange code for directly use struct nct6775_sio_data in superio_*()
  functions.
* Use superio function pointers in nct6775_sio_data struct instead direct
  calls.

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=204807
Signed-off-by: Denis Pauk <pauk.denis@gmail.com>
Co-developed-by: Bernhard Seibold <mail@bernhard-seibold.de>
Signed-off-by: Bernhard Seibold <mail@bernhard-seibold.de>
Cc: Andy Shevchenko <andriy.shevchenko@intel.com>
Cc: Guenter Roeck <linux@roeck-us.net>

---
Changes in v7:
  - Without changes.
  
Changes in v6:
  - Split patch with replace direct calls to function pointers.

Changes in v5:
  - Without changes.

Changes in v4:
  - Fix naming conflict with inb, outb by add sio prefix to callbacks in
    nct6775_sio_data.

Changes in v3:
  - Use nct6775 function pointers in struct nct6775_data instead direct calls.
  - make function declarations one line.

Changes in v2:
  - Split changes to separate patches.
  - Rearrange code for directly use struct nct6775_sio_data in superio_*()
    functions.
---
 drivers/hwmon/nct6775.c | 189 ++++++++++++++++++++++------------------
 1 file changed, 104 insertions(+), 85 deletions(-)

diff --git a/drivers/hwmon/nct6775.c b/drivers/hwmon/nct6775.c
index 5bd15622a85f..9b503c88e20d 100644
--- a/drivers/hwmon/nct6775.c
+++ b/drivers/hwmon/nct6775.c
@@ -133,30 +133,46 @@ MODULE_PARM_DESC(fan_debounce, "Enable debouncing for fan RPM signal");
 
 enum pwm_enable { off, manual, thermal_cruise, speed_cruise, sf3, sf4 };
 
-static inline void
-superio_outb(int ioreg, int reg, int val)
+struct nct6775_sio_data {
+	int sioreg;
+	enum kinds kind;
+
+	/* superio_() callbacks  */
+	void (*sio_outb)(struct nct6775_sio_data *sio_data, int reg, int val);
+	int (*sio_inb)(struct nct6775_sio_data *sio_data, int reg);
+	void (*sio_select)(struct nct6775_sio_data *sio_data, int ld);
+	int (*sio_enter)(struct nct6775_sio_data *sio_data);
+	void (*sio_exit)(struct nct6775_sio_data *sio_data);
+};
+
+static void superio_outb(struct nct6775_sio_data *sio_data, int reg, int val)
 {
+	int ioreg = sio_data->sioreg;
+
 	outb(reg, ioreg);
 	outb(val, ioreg + 1);
 }
 
-static inline int
-superio_inb(int ioreg, int reg)
+static int superio_inb(struct nct6775_sio_data *sio_data, int reg)
 {
+	int ioreg = sio_data->sioreg;
+
 	outb(reg, ioreg);
 	return inb(ioreg + 1);
 }
 
-static inline void
-superio_select(int ioreg, int ld)
+static void superio_select(struct nct6775_sio_data *sio_data, int ld)
 {
+	int ioreg = sio_data->sioreg;
+
 	outb(SIO_REG_LDSEL, ioreg);
 	outb(ld, ioreg + 1);
 }
 
-static inline int
-superio_enter(int ioreg)
+static int superio_enter(struct nct6775_sio_data *sio_data)
 {
+	int ioreg = sio_data->sioreg;
+
 	/*
 	 * Try to reserve <ioreg> and <ioreg + 1> for exclusive access.
 	 */
@@ -169,9 +185,10 @@ superio_enter(int ioreg)
 	return 0;
 }
 
-static inline void
-superio_exit(int ioreg)
+static void superio_exit(struct nct6775_sio_data *sio_data)
 {
+	int ioreg = sio_data->sioreg;
+
 	outb(0xaa, ioreg);
 	outb(0x02, ioreg);
 	outb(0x02, ioreg + 1);
@@ -1217,11 +1234,6 @@ struct nct6775_data {
 	u8 sio_reg_enable;
 };
 
-struct nct6775_sio_data {
-	int sioreg;
-	enum kinds kind;
-};
-
 struct sensor_device_template {
 	struct device_attribute dev_attr;
 	union {
@@ -3410,6 +3422,7 @@ clear_caseopen(struct device *dev, struct device_attribute *attr,
 	       const char *buf, size_t count)
 {
 	struct nct6775_data *data = dev_get_drvdata(dev);
+	struct nct6775_sio_data *sio_data = dev_get_platdata(dev);
 	int nr = to_sensor_dev_attr(attr)->index - INTRUSION_ALARM_BASE;
 	unsigned long val;
 	u8 reg;
@@ -3425,19 +3438,19 @@ clear_caseopen(struct device *dev, struct device_attribute *attr,
 	 * The CR registers are the same for all chips, and not all chips
 	 * support clearing the caseopen status through "regular" registers.
 	 */
-	ret = superio_enter(data->sioreg);
+	ret = sio_data->sio_enter(sio_data);
 	if (ret) {
 		count = ret;
 		goto error;
 	}
 
-	superio_select(data->sioreg, NCT6775_LD_ACPI);
-	reg = superio_inb(data->sioreg, NCT6775_REG_CR_CASEOPEN_CLR[nr]);
+	sio_data->sio_select(sio_data, NCT6775_LD_ACPI);
+	reg = sio_data->sio_inb(sio_data, NCT6775_REG_CR_CASEOPEN_CLR[nr]);
 	reg |= NCT6775_CR_CASEOPEN_CLR_MASK[nr];
-	superio_outb(data->sioreg, NCT6775_REG_CR_CASEOPEN_CLR[nr], reg);
+	sio_data->sio_outb(sio_data, NCT6775_REG_CR_CASEOPEN_CLR[nr], reg);
 	reg &= ~NCT6775_CR_CASEOPEN_CLR_MASK[nr];
-	superio_outb(data->sioreg, NCT6775_REG_CR_CASEOPEN_CLR[nr], reg);
-	superio_exit(data->sioreg);
+	sio_data->sio_outb(sio_data, NCT6775_REG_CR_CASEOPEN_CLR[nr], reg);
+	sio_data->sio_exit(sio_data);
 
 	data->valid = false;	/* Force cache refresh */
 error:
@@ -3542,29 +3555,28 @@ static inline void nct6775_init_device(struct nct6775_data *data)
 }
 
 static void
-nct6775_check_fan_inputs(struct nct6775_data *data)
+nct6775_check_fan_inputs(struct nct6775_data *data, struct nct6775_sio_data *sio_data)
 {
 	bool fan3pin = false, fan4pin = false, fan4min = false;
 	bool fan5pin = false, fan6pin = false, fan7pin = false;
 	bool pwm3pin = false, pwm4pin = false, pwm5pin = false;
 	bool pwm6pin = false, pwm7pin = false;
-	int sioreg = data->sioreg;
 
 	/* Store SIO_REG_ENABLE for use during resume */
-	superio_select(sioreg, NCT6775_LD_HWM);
-	data->sio_reg_enable = superio_inb(sioreg, SIO_REG_ENABLE);
+	sio_data->sio_select(sio_data, NCT6775_LD_HWM);
+	data->sio_reg_enable = sio_data->sio_inb(sio_data, SIO_REG_ENABLE);
 
 	/* fan4 and fan5 share some pins with the GPIO and serial flash */
 	if (data->kind == nct6775) {
-		int cr2c = superio_inb(sioreg, 0x2c);
+		int cr2c = sio_data->sio_inb(sio_data, 0x2c);
 
 		fan3pin = cr2c & BIT(6);
 		pwm3pin = cr2c & BIT(7);
 
 		/* On NCT6775, fan4 shares pins with the fdc interface */
-		fan4pin = !(superio_inb(sioreg, 0x2A) & 0x80);
+		fan4pin = !(sio_data->sio_inb(sio_data, 0x2A) & 0x80);
 	} else if (data->kind == nct6776) {
-		bool gpok = superio_inb(sioreg, 0x27) & 0x80;
+		bool gpok = sio_data->sio_inb(sio_data, 0x27) & 0x80;
 		const char *board_vendor, *board_name;
 
 		board_vendor = dmi_get_system_info(DMI_BOARD_VENDOR);
@@ -3580,7 +3592,7 @@ nct6775_check_fan_inputs(struct nct6775_data *data)
 			if (!strcmp(board_name, "Z77 Pro4-M")) {
 				if ((data->sio_reg_enable & 0xe0) != 0xe0) {
 					data->sio_reg_enable |= 0xe0;
-					superio_outb(sioreg, SIO_REG_ENABLE,
+					sio_data->sio_outb(sio_data, SIO_REG_ENABLE,
 						     data->sio_reg_enable);
 				}
 			}
@@ -3589,32 +3601,32 @@ nct6775_check_fan_inputs(struct nct6775_data *data)
 		if (data->sio_reg_enable & 0x80)
 			fan3pin = gpok;
 		else
-			fan3pin = !(superio_inb(sioreg, 0x24) & 0x40);
+			fan3pin = !(sio_data->sio_inb(sio_data, 0x24) & 0x40);
 
 		if (data->sio_reg_enable & 0x40)
 			fan4pin = gpok;
 		else
-			fan4pin = superio_inb(sioreg, 0x1C) & 0x01;
+			fan4pin = sio_data->sio_inb(sio_data, 0x1C) & 0x01;
 
 		if (data->sio_reg_enable & 0x20)
 			fan5pin = gpok;
 		else
-			fan5pin = superio_inb(sioreg, 0x1C) & 0x02;
+			fan5pin = sio_data->sio_inb(sio_data, 0x1C) & 0x02;
 
 		fan4min = fan4pin;
 		pwm3pin = fan3pin;
 	} else if (data->kind == nct6106) {
-		int cr24 = superio_inb(sioreg, 0x24);
+		int cr24 = sio_data->sio_inb(sio_data, 0x24);
 
 		fan3pin = !(cr24 & 0x80);
 		pwm3pin = cr24 & 0x08;
 	} else if (data->kind == nct6116) {
-		int cr1a = superio_inb(sioreg, 0x1a);
-		int cr1b = superio_inb(sioreg, 0x1b);
-		int cr24 = superio_inb(sioreg, 0x24);
-		int cr2a = superio_inb(sioreg, 0x2a);
-		int cr2b = superio_inb(sioreg, 0x2b);
-		int cr2f = superio_inb(sioreg, 0x2f);
+		int cr1a = sio_data->sio_inb(sio_data, 0x1a);
+		int cr1b = sio_data->sio_inb(sio_data, 0x1b);
+		int cr24 = sio_data->sio_inb(sio_data, 0x24);
+		int cr2a = sio_data->sio_inb(sio_data, 0x2a);
+		int cr2b = sio_data->sio_inb(sio_data, 0x2b);
+		int cr2f = sio_data->sio_inb(sio_data, 0x2f);
 
 		fan3pin = !(cr2b & 0x10);
 		fan4pin = (cr2b & 0x80) ||			// pin 1(2)
@@ -3630,24 +3642,24 @@ nct6775_check_fan_inputs(struct nct6775_data *data)
 		 * NCT6779D, NCT6791D, NCT6792D, NCT6793D, NCT6795D, NCT6796D,
 		 * NCT6797D, NCT6798D
 		 */
-		int cr1a = superio_inb(sioreg, 0x1a);
-		int cr1b = superio_inb(sioreg, 0x1b);
-		int cr1c = superio_inb(sioreg, 0x1c);
-		int cr1d = superio_inb(sioreg, 0x1d);
-		int cr2a = superio_inb(sioreg, 0x2a);
-		int cr2b = superio_inb(sioreg, 0x2b);
-		int cr2d = superio_inb(sioreg, 0x2d);
-		int cr2f = superio_inb(sioreg, 0x2f);
+		int cr1a = sio_data->sio_inb(sio_data, 0x1a);
+		int cr1b = sio_data->sio_inb(sio_data, 0x1b);
+		int cr1c = sio_data->sio_inb(sio_data, 0x1c);
+		int cr1d = sio_data->sio_inb(sio_data, 0x1d);
+		int cr2a = sio_data->sio_inb(sio_data, 0x2a);
+		int cr2b = sio_data->sio_inb(sio_data, 0x2b);
+		int cr2d = sio_data->sio_inb(sio_data, 0x2d);
+		int cr2f = sio_data->sio_inb(sio_data, 0x2f);
 		bool dsw_en = cr2f & BIT(3);
 		bool ddr4_en = cr2f & BIT(4);
 		int cre0;
 		int creb;
 		int cred;
 
-		superio_select(sioreg, NCT6775_LD_12);
-		cre0 = superio_inb(sioreg, 0xe0);
-		creb = superio_inb(sioreg, 0xeb);
-		cred = superio_inb(sioreg, 0xed);
+		sio_data->sio_select(sio_data, NCT6775_LD_12);
+		cre0 = sio_data->sio_inb(sio_data, 0xe0);
+		creb = sio_data->sio_inb(sio_data, 0xeb);
+		cred = sio_data->sio_inb(sio_data, 0xed);
 
 		fan3pin = !(cr1c & BIT(5));
 		fan4pin = !(cr1c & BIT(6));
@@ -4502,11 +4514,11 @@ static int nct6775_probe(struct platform_device *pdev)
 	/* Initialize the chip */
 	nct6775_init_device(data);
 
-	err = superio_enter(sio_data->sioreg);
+	err = sio_data->sio_enter(sio_data);
 	if (err)
 		return err;
 
-	cr2a = superio_inb(sio_data->sioreg, 0x2a);
+	cr2a = sio_data->sio_inb(sio_data, 0x2a);
 	switch (data->kind) {
 	case nct6775:
 		data->have_vid = (cr2a & 0x40);
@@ -4532,17 +4544,17 @@ static int nct6775_probe(struct platform_device *pdev)
 	 * We can get the VID input values directly at logical device D 0xe3.
 	 */
 	if (data->have_vid) {
-		superio_select(sio_data->sioreg, NCT6775_LD_VID);
-		data->vid = superio_inb(sio_data->sioreg, 0xe3);
+		sio_data->sio_select(sio_data, NCT6775_LD_VID);
+		data->vid = sio_data->sio_inb(sio_data, 0xe3);
 		data->vrm = vid_which_vrm();
 	}
 
 	if (fan_debounce) {
 		u8 tmp;
 
-		superio_select(sio_data->sioreg, NCT6775_LD_HWM);
-		tmp = superio_inb(sio_data->sioreg,
-				  NCT6775_REG_CR_FAN_DEBOUNCE);
+		sio_data->sio_select(sio_data, NCT6775_LD_HWM);
+		tmp = sio_data->sio_inb(sio_data,
+				    NCT6775_REG_CR_FAN_DEBOUNCE);
 		switch (data->kind) {
 		case nct6106:
 		case nct6116:
@@ -4565,15 +4577,15 @@ static int nct6775_probe(struct platform_device *pdev)
 			tmp |= 0x7e;
 			break;
 		}
-		superio_outb(sio_data->sioreg, NCT6775_REG_CR_FAN_DEBOUNCE,
+		sio_data->sio_outb(sio_data, NCT6775_REG_CR_FAN_DEBOUNCE,
 			     tmp);
 		dev_info(&pdev->dev, "Enabled fan debounce for chip %s\n",
 			 data->name);
 	}
 
-	nct6775_check_fan_inputs(data);
+	nct6775_check_fan_inputs(data, sio_data);
 
-	superio_exit(sio_data->sioreg);
+	sio_data->sio_exit(sio_data);
 
 	/* Read fan clock dividers immediately */
 	nct6775_init_fan_common(dev, data);
@@ -4613,15 +4625,15 @@ static int nct6775_probe(struct platform_device *pdev)
 	return PTR_ERR_OR_ZERO(hwmon_dev);
 }
 
-static void nct6791_enable_io_mapping(int sioaddr)
+static void nct6791_enable_io_mapping(struct nct6775_sio_data *sio_data)
 {
 	int val;
 
-	val = superio_inb(sioaddr, NCT6791_REG_HM_IO_SPACE_LOCK_ENABLE);
+	val = sio_data->sio_inb(sio_data, NCT6791_REG_HM_IO_SPACE_LOCK_ENABLE);
 	if (val & 0x10) {
 		pr_info("Enabling hardware monitor logical device mappings.\n");
-		superio_outb(sioaddr, NCT6791_REG_HM_IO_SPACE_LOCK_ENABLE,
-			     val & ~0x10);
+		sio_data->sio_outb(sio_data, NCT6791_REG_HM_IO_SPACE_LOCK_ENABLE,
+			       val & ~0x10);
 	}
 }
 
@@ -4643,29 +4655,29 @@ static int __maybe_unused nct6775_suspend(struct device *dev)
 static int __maybe_unused nct6775_resume(struct device *dev)
 {
 	struct nct6775_data *data = dev_get_drvdata(dev);
-	int sioreg = data->sioreg;
+	struct nct6775_sio_data *sio_data = dev_get_platdata(dev);
 	int i, j, err = 0;
 	u8 reg;
 
 	mutex_lock(&data->update_lock);
 	data->bank = 0xff;		/* Force initial bank selection */
 
-	err = superio_enter(sioreg);
+	err = sio_data->sio_enter(sio_data);
 	if (err)
 		goto abort;
 
-	superio_select(sioreg, NCT6775_LD_HWM);
-	reg = superio_inb(sioreg, SIO_REG_ENABLE);
+	sio_data->sio_select(sio_data, NCT6775_LD_HWM);
+	reg = sio_data->sio_inb(sio_data, SIO_REG_ENABLE);
 	if (reg != data->sio_reg_enable)
-		superio_outb(sioreg, SIO_REG_ENABLE, data->sio_reg_enable);
+		sio_data->sio_outb(sio_data, SIO_REG_ENABLE, data->sio_reg_enable);
 
 	if (data->kind == nct6791 || data->kind == nct6792 ||
 	    data->kind == nct6793 || data->kind == nct6795 ||
 	    data->kind == nct6796 || data->kind == nct6797 ||
 	    data->kind == nct6798)
-		nct6791_enable_io_mapping(sioreg);
+		nct6791_enable_io_mapping(sio_data);
 
-	superio_exit(sioreg);
+	sio_data->sio_exit(sio_data);
 
 	/* Restore limits */
 	for (i = 0; i < data->in_num; i++) {
@@ -4728,12 +4740,14 @@ static int __init nct6775_find(int sioaddr, struct nct6775_sio_data *sio_data)
 	int err;
 	int addr;
 
-	err = superio_enter(sioaddr);
+	sio_data->sioreg = sioaddr;
+
+	err = sio_data->sio_enter(sio_data);
 	if (err)
 		return err;
 
-	val = (superio_inb(sioaddr, SIO_REG_DEVID) << 8) |
-		superio_inb(sioaddr, SIO_REG_DEVID + 1);
+	val = (sio_data->sio_inb(sio_data, SIO_REG_DEVID) << 8) |
+		sio_data->sio_inb(sio_data, SIO_REG_DEVID + 1);
 	if (force_id && val != 0xffff)
 		val = force_id;
 
@@ -4777,38 +4791,37 @@ static int __init nct6775_find(int sioaddr, struct nct6775_sio_data *sio_data)
 	default:
 		if (val != 0xffff)
 			pr_debug("unsupported chip ID: 0x%04x\n", val);
-		superio_exit(sioaddr);
+		sio_data->sio_exit(sio_data);
 		return -ENODEV;
 	}
 
 	/* We have a known chip, find the HWM I/O address */
-	superio_select(sioaddr, NCT6775_LD_HWM);
-	val = (superio_inb(sioaddr, SIO_REG_ADDR) << 8)
-	    | superio_inb(sioaddr, SIO_REG_ADDR + 1);
+	sio_data->sio_select(sio_data, NCT6775_LD_HWM);
+	val = (sio_data->sio_inb(sio_data, SIO_REG_ADDR) << 8)
+	    | sio_data->sio_inb(sio_data, SIO_REG_ADDR + 1);
 	addr = val & IOREGION_ALIGNMENT;
 	if (addr == 0) {
 		pr_err("Refusing to enable a Super-I/O device with a base I/O port 0\n");
-		superio_exit(sioaddr);
+		sio_data->sio_exit(sio_data);
 		return -ENODEV;
 	}
 
 	/* Activate logical device if needed */
-	val = superio_inb(sioaddr, SIO_REG_ENABLE);
+	val = sio_data->sio_inb(sio_data, SIO_REG_ENABLE);
 	if (!(val & 0x01)) {
 		pr_warn("Forcibly enabling Super-I/O. Sensor is probably unusable.\n");
-		superio_outb(sioaddr, SIO_REG_ENABLE, val | 0x01);
+		sio_data->sio_outb(sio_data, SIO_REG_ENABLE, val | 0x01);
 	}
 
 	if (sio_data->kind == nct6791 || sio_data->kind == nct6792 ||
 	    sio_data->kind == nct6793 || sio_data->kind == nct6795 ||
 	    sio_data->kind == nct6796 || sio_data->kind == nct6797 ||
 	    sio_data->kind == nct6798)
-		nct6791_enable_io_mapping(sioaddr);
+		nct6791_enable_io_mapping(sio_data);
 
-	superio_exit(sioaddr);
+	sio_data->sio_exit(sio_data);
 	pr_info("Found %s or compatible chip at %#x:%#x\n",
 		nct6775_sio_names[sio_data->kind], sioaddr, addr);
-	sio_data->sioreg = sioaddr;
 
 	return addr;
 }
@@ -4842,6 +4855,12 @@ static int __init sensors_nct6775_init(void)
 	 * nct6775 hardware monitor, and call probe()
 	 */
 	for (i = 0; i < ARRAY_SIZE(pdev); i++) {
+		sio_data.sio_outb = superio_outb;
+		sio_data.sio_inb = superio_inb;
+		sio_data.sio_select = superio_select;
+		sio_data.sio_enter = superio_enter;
+		sio_data.sio_exit = superio_exit;
+
 		address = nct6775_find(sioaddr[i], &sio_data);
 		if (address <= 0)
 			continue;
-- 
2.33.0


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

* [PATCH v7 2/3] hwmon: (nct6775) Use nct6775_*() function pointers in nct6775_data.
  2021-09-16 20:22 [PATCH v6 0/3] hwmon: Support access to the NCT677x via Asus WMI Denis Pauk
  2021-09-16 20:22 ` [PATCH v7 " Denis Pauk
  2021-09-16 20:22 ` [PATCH v7 1/3] hwmon: (nct6775) Use superio_*() function pointers in sio_data Denis Pauk
@ 2021-09-16 20:22 ` Denis Pauk
  2021-09-16 20:22 ` [PATCH v7 3/3] hwmon: (nct6775) Support access via Asus WMI Denis Pauk
  3 siblings, 0 replies; 8+ messages in thread
From: Denis Pauk @ 2021-09-16 20:22 UTC (permalink / raw)
  Cc: pauk.denis, Bernhard Seibold, Andy Shevchenko, Guenter Roeck,
	Jean Delvare, linux-hwmon, linux-kernel

Prepare for platform specific callbacks usage:
* Use nct6775 function pointers in struct nct6775_data instead direct
  calls.

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=204807
Signed-off-by: Denis Pauk <pauk.denis@gmail.com>
Co-developed-by: Bernhard Seibold <mail@bernhard-seibold.de>
Signed-off-by: Bernhard Seibold <mail@bernhard-seibold.de>
Cc: Andy Shevchenko <andriy.shevchenko@intel.com>
Cc: Guenter Roeck <linux@roeck-us.net>

---
Changes in v7:
  - Without changes.
  
Changes in v6:
  - Split patch with replace direct calls to function pointers.

Changes in v5:
  - Without changes.

Changes in v4:
  - Fix naming conflict with inb, outb by add sio prefix to callbacks in
    nct6775_sio_data.

Changes in v3:
  - Use nct6775 function pointers in struct nct6775_data instead direct calls.
  - make function declarations one line.

Changes in v2:
  - Split changes to separate patches.
  - Rearrange code for directly use struct nct6775_sio_data in superio_*()
    functions.
---
 drivers/hwmon/nct6775.c | 283 ++++++++++++++++++++--------------------
 1 file changed, 143 insertions(+), 140 deletions(-)

diff --git a/drivers/hwmon/nct6775.c b/drivers/hwmon/nct6775.c
index 9b503c88e20d..4253eed7f5b0 100644
--- a/drivers/hwmon/nct6775.c
+++ b/drivers/hwmon/nct6775.c
@@ -1232,6 +1232,10 @@ struct nct6775_data {
 	u8 fandiv1;
 	u8 fandiv2;
 	u8 sio_reg_enable;
+
+	/* nct6775_*() callbacks  */
+	u16 (*read_value)(struct nct6775_data *data, u16 reg);
+	int (*write_value)(struct nct6775_data *data, u16 reg, u16 value);
 };
 
 struct sensor_device_template {
@@ -1471,7 +1475,7 @@ static u16 nct6775_read_temp(struct nct6775_data *data, u16 reg)
 {
 	u16 res;
 
-	res = nct6775_read_value(data, reg);
+	res = data->read_value(data, reg);
 	if (!is_word_sized(data, reg))
 		res <<= 8;
 
@@ -1482,7 +1486,7 @@ static int nct6775_write_temp(struct nct6775_data *data, u16 reg, u16 value)
 {
 	if (!is_word_sized(data, reg))
 		value >>= 8;
-	return nct6775_write_value(data, reg, value);
+	return data->write_value(data, reg, value);
 }
 
 /* This function assumes that the caller holds data->update_lock */
@@ -1492,24 +1496,24 @@ static void nct6775_write_fan_div(struct nct6775_data *data, int nr)
 
 	switch (nr) {
 	case 0:
-		reg = (nct6775_read_value(data, NCT6775_REG_FANDIV1) & 0x70)
+		reg = (data->read_value(data, NCT6775_REG_FANDIV1) & 0x70)
 		    | (data->fan_div[0] & 0x7);
-		nct6775_write_value(data, NCT6775_REG_FANDIV1, reg);
+		data->write_value(data, NCT6775_REG_FANDIV1, reg);
 		break;
 	case 1:
-		reg = (nct6775_read_value(data, NCT6775_REG_FANDIV1) & 0x7)
+		reg = (data->read_value(data, NCT6775_REG_FANDIV1) & 0x7)
 		    | ((data->fan_div[1] << 4) & 0x70);
-		nct6775_write_value(data, NCT6775_REG_FANDIV1, reg);
+		data->write_value(data, NCT6775_REG_FANDIV1, reg);
 		break;
 	case 2:
-		reg = (nct6775_read_value(data, NCT6775_REG_FANDIV2) & 0x70)
+		reg = (data->read_value(data, NCT6775_REG_FANDIV2) & 0x70)
 		    | (data->fan_div[2] & 0x7);
-		nct6775_write_value(data, NCT6775_REG_FANDIV2, reg);
+		data->write_value(data, NCT6775_REG_FANDIV2, reg);
 		break;
 	case 3:
-		reg = (nct6775_read_value(data, NCT6775_REG_FANDIV2) & 0x7)
+		reg = (data->read_value(data, NCT6775_REG_FANDIV2) & 0x7)
 		    | ((data->fan_div[3] << 4) & 0x70);
-		nct6775_write_value(data, NCT6775_REG_FANDIV2, reg);
+		data->write_value(data, NCT6775_REG_FANDIV2, reg);
 		break;
 	}
 }
@@ -1524,10 +1528,10 @@ static void nct6775_update_fan_div(struct nct6775_data *data)
 {
 	u8 i;
 
-	i = nct6775_read_value(data, NCT6775_REG_FANDIV1);
+	i = data->read_value(data, NCT6775_REG_FANDIV1);
 	data->fan_div[0] = i & 0x7;
 	data->fan_div[1] = (i & 0x70) >> 4;
-	i = nct6775_read_value(data, NCT6775_REG_FANDIV2);
+	i = data->read_value(data, NCT6775_REG_FANDIV2);
 	data->fan_div[2] = i & 0x7;
 	if (data->has_fan & BIT(3))
 		data->fan_div[3] = (i & 0x70) >> 4;
@@ -1575,11 +1579,11 @@ static void nct6775_init_fan_common(struct device *dev,
 	 */
 	for (i = 0; i < ARRAY_SIZE(data->fan_min); i++) {
 		if (data->has_fan_min & BIT(i)) {
-			reg = nct6775_read_value(data, data->REG_FAN_MIN[i]);
+			reg = data->read_value(data, data->REG_FAN_MIN[i]);
 			if (!reg)
-				nct6775_write_value(data, data->REG_FAN_MIN[i],
-						    data->has_fan_div ? 0xff
-								      : 0xff1f);
+				data->write_value(data, data->REG_FAN_MIN[i],
+						  data->has_fan_div ? 0xff
+								    : 0xff1f);
 		}
 	}
 }
@@ -1623,8 +1627,8 @@ static void nct6775_select_fan_div(struct device *dev,
 			}
 			if (fan_min != data->fan_min[nr]) {
 				data->fan_min[nr] = fan_min;
-				nct6775_write_value(data, data->REG_FAN_MIN[nr],
-						    fan_min);
+				data->write_value(data, data->REG_FAN_MIN[nr],
+						  fan_min);
 			}
 		}
 		data->fan_div[nr] = fan_div;
@@ -1644,16 +1648,15 @@ static void nct6775_update_pwm(struct device *dev)
 			continue;
 
 		duty_is_dc = data->REG_PWM_MODE[i] &&
-		  (nct6775_read_value(data, data->REG_PWM_MODE[i])
+		  (data->read_value(data, data->REG_PWM_MODE[i])
 		   & data->PWM_MODE_MASK[i]);
 		data->pwm_mode[i] = !duty_is_dc;
 
-		fanmodecfg = nct6775_read_value(data, data->REG_FAN_MODE[i]);
+		fanmodecfg = data->read_value(data, data->REG_FAN_MODE[i]);
 		for (j = 0; j < ARRAY_SIZE(data->REG_PWM); j++) {
 			if (data->REG_PWM[j] && data->REG_PWM[j][i]) {
-				data->pwm[j][i]
-				  = nct6775_read_value(data,
-						       data->REG_PWM[j][i]);
+				data->pwm[j][i] = data->read_value(data,
+								   data->REG_PWM[j][i]);
 			}
 		}
 
@@ -1668,17 +1671,17 @@ static void nct6775_update_pwm(struct device *dev)
 			u8 t = fanmodecfg & 0x0f;
 
 			if (data->REG_TOLERANCE_H) {
-				t |= (nct6775_read_value(data,
+				t |= (data->read_value(data,
 				      data->REG_TOLERANCE_H[i]) & 0x70) >> 1;
 			}
 			data->target_speed_tolerance[i] = t;
 		}
 
 		data->temp_tolerance[1][i] =
-			nct6775_read_value(data,
-					data->REG_CRITICAL_TEMP_TOLERANCE[i]);
+			data->read_value(data,
+					 data->REG_CRITICAL_TEMP_TOLERANCE[i]);
 
-		reg = nct6775_read_value(data, data->REG_TEMP_SEL[i]);
+		reg = data->read_value(data, data->REG_TEMP_SEL[i]);
 		data->pwm_temp_sel[i] = reg & 0x1f;
 		/* If fan can stop, report floor as 0 */
 		if (reg & 0x80)
@@ -1687,7 +1690,7 @@ static void nct6775_update_pwm(struct device *dev)
 		if (!data->REG_WEIGHT_TEMP_SEL[i])
 			continue;
 
-		reg = nct6775_read_value(data, data->REG_WEIGHT_TEMP_SEL[i]);
+		reg = data->read_value(data, data->REG_WEIGHT_TEMP_SEL[i]);
 		data->pwm_weight_temp_sel[i] = reg & 0x1f;
 		/* If weight is disabled, report weight source as 0 */
 		if (!(reg & 0x80))
@@ -1695,9 +1698,8 @@ static void nct6775_update_pwm(struct device *dev)
 
 		/* Weight temp data */
 		for (j = 0; j < ARRAY_SIZE(data->weight_temp); j++) {
-			data->weight_temp[j][i]
-			  = nct6775_read_value(data,
-					       data->REG_WEIGHT_TEMP[j][i]);
+			data->weight_temp[j][i] = data->read_value(data,
+								   data->REG_WEIGHT_TEMP[j][i]);
 		}
 	}
 }
@@ -1715,10 +1717,10 @@ static void nct6775_update_pwm_limits(struct device *dev)
 
 		for (j = 0; j < ARRAY_SIZE(data->fan_time); j++) {
 			data->fan_time[j][i] =
-			  nct6775_read_value(data, data->REG_FAN_TIME[j][i]);
+			  data->read_value(data, data->REG_FAN_TIME[j][i]);
 		}
 
-		reg_t = nct6775_read_value(data, data->REG_TARGET[i]);
+		reg_t = data->read_value(data, data->REG_TARGET[i]);
 		/* Update only in matching mode or if never updated */
 		if (!data->target_temp[i] ||
 		    data->pwm_enable[i] == thermal_cruise)
@@ -1726,7 +1728,7 @@ static void nct6775_update_pwm_limits(struct device *dev)
 		if (!data->target_speed[i] ||
 		    data->pwm_enable[i] == speed_cruise) {
 			if (data->REG_TOLERANCE_H) {
-				reg_t |= (nct6775_read_value(data,
+				reg_t |= (data->read_value(data,
 					data->REG_TOLERANCE_H[i]) & 0x0f) << 8;
 			}
 			data->target_speed[i] = reg_t;
@@ -1734,21 +1736,21 @@ static void nct6775_update_pwm_limits(struct device *dev)
 
 		for (j = 0; j < data->auto_pwm_num; j++) {
 			data->auto_pwm[i][j] =
-			  nct6775_read_value(data,
-					     NCT6775_AUTO_PWM(data, i, j));
+			  data->read_value(data,
+					   NCT6775_AUTO_PWM(data, i, j));
 			data->auto_temp[i][j] =
-			  nct6775_read_value(data,
-					     NCT6775_AUTO_TEMP(data, i, j));
+			  data->read_value(data,
+					   NCT6775_AUTO_TEMP(data, i, j));
 		}
 
 		/* critical auto_pwm temperature data */
 		data->auto_temp[i][data->auto_pwm_num] =
-			nct6775_read_value(data, data->REG_CRITICAL_TEMP[i]);
+			data->read_value(data, data->REG_CRITICAL_TEMP[i]);
 
 		switch (data->kind) {
 		case nct6775:
-			reg = nct6775_read_value(data,
-						 NCT6775_REG_CRITICAL_ENAB[i]);
+			reg = data->read_value(data,
+					       NCT6775_REG_CRITICAL_ENAB[i]);
 			data->auto_pwm[i][data->auto_pwm_num] =
 						(reg & 0x02) ? 0xff : 0x00;
 			break;
@@ -1765,10 +1767,10 @@ static void nct6775_update_pwm_limits(struct device *dev)
 		case nct6796:
 		case nct6797:
 		case nct6798:
-			reg = nct6775_read_value(data,
+			reg = data->read_value(data,
 					data->REG_CRITICAL_PWM_ENABLE[i]);
 			if (reg & data->CRITICAL_PWM_ENABLE_MASK)
-				reg = nct6775_read_value(data,
+				reg = data->read_value(data,
 					data->REG_CRITICAL_PWM[i]);
 			else
 				reg = 0xff;
@@ -1795,11 +1797,11 @@ static struct nct6775_data *nct6775_update_device(struct device *dev)
 			if (!(data->have_in & BIT(i)))
 				continue;
 
-			data->in[i][0] = nct6775_read_value(data,
-							    data->REG_VIN[i]);
-			data->in[i][1] = nct6775_read_value(data,
+			data->in[i][0] = data->read_value(data,
+							  data->REG_VIN[i]);
+			data->in[i][1] = data->read_value(data,
 					  data->REG_IN_MINMAX[0][i]);
-			data->in[i][2] = nct6775_read_value(data,
+			data->in[i][2] = data->read_value(data,
 					  data->REG_IN_MINMAX[1][i]);
 		}
 
@@ -1810,18 +1812,18 @@ static struct nct6775_data *nct6775_update_device(struct device *dev)
 			if (!(data->has_fan & BIT(i)))
 				continue;
 
-			reg = nct6775_read_value(data, data->REG_FAN[i]);
+			reg = data->read_value(data, data->REG_FAN[i]);
 			data->rpm[i] = data->fan_from_reg(reg,
 							  data->fan_div[i]);
 
 			if (data->has_fan_min & BIT(i))
-				data->fan_min[i] = nct6775_read_value(data,
+				data->fan_min[i] = data->read_value(data,
 					   data->REG_FAN_MIN[i]);
 
 			if (data->REG_FAN_PULSES[i]) {
 				data->fan_pulses[i] =
-				  (nct6775_read_value(data,
-						      data->REG_FAN_PULSES[i])
+				  (data->read_value(data,
+						    data->REG_FAN_PULSES[i])
 				   >> data->FAN_PULSE_SHIFT[i]) & 0x03;
 			}
 
@@ -1837,15 +1839,14 @@ static struct nct6775_data *nct6775_update_device(struct device *dev)
 				continue;
 			for (j = 0; j < ARRAY_SIZE(data->reg_temp); j++) {
 				if (data->reg_temp[j][i])
-					data->temp[j][i]
-					  = nct6775_read_temp(data,
-						data->reg_temp[j][i]);
+					data->temp[j][i] = nct6775_read_temp(data,
+									     data->reg_temp[j][i]);
 			}
 			if (i >= NUM_TEMP_FIXED ||
 			    !(data->have_temp_fixed & BIT(i)))
 				continue;
-			data->temp_offset[i]
-			  = nct6775_read_value(data, data->REG_TEMP_OFFSET[i]);
+			data->temp_offset[i] = data->read_value(data,
+								   data->REG_TEMP_OFFSET[i]);
 		}
 
 		data->alarms = 0;
@@ -1854,7 +1855,7 @@ static struct nct6775_data *nct6775_update_device(struct device *dev)
 
 			if (!data->REG_ALARM[i])
 				continue;
-			alarm = nct6775_read_value(data, data->REG_ALARM[i]);
+			alarm = data->read_value(data, data->REG_ALARM[i]);
 			data->alarms |= ((u64)alarm) << (i << 3);
 		}
 
@@ -1864,7 +1865,7 @@ static struct nct6775_data *nct6775_update_device(struct device *dev)
 
 			if (!data->REG_BEEP[i])
 				continue;
-			beep = nct6775_read_value(data, data->REG_BEEP[i]);
+			beep = data->read_value(data, data->REG_BEEP[i]);
 			data->beeps |= ((u64)beep) << (i << 3);
 		}
 
@@ -1906,8 +1907,8 @@ store_in_reg(struct device *dev, struct device_attribute *attr, const char *buf,
 		return err;
 	mutex_lock(&data->update_lock);
 	data->in[nr][index] = in_to_reg(val, nr);
-	nct6775_write_value(data, data->REG_IN_MINMAX[index - 1][nr],
-			    data->in[nr][index]);
+	data->write_value(data, data->REG_IN_MINMAX[index - 1][nr],
+			  data->in[nr][index]);
 	mutex_unlock(&data->update_lock);
 	return count;
 }
@@ -1931,8 +1932,8 @@ static int find_temp_source(struct nct6775_data *data, int index, int count)
 	for (nr = 0; nr < count; nr++) {
 		int src;
 
-		src = nct6775_read_value(data,
-					 data->REG_TEMP_SOURCE[nr]) & 0x1f;
+		src = data->read_value(data,
+				       data->REG_TEMP_SOURCE[nr]) & 0x1f;
 		if (src == source)
 			return nr;
 	}
@@ -1993,8 +1994,8 @@ store_beep(struct device *dev, struct device_attribute *attr, const char *buf,
 		data->beeps |= (1ULL << nr);
 	else
 		data->beeps &= ~(1ULL << nr);
-	nct6775_write_value(data, data->REG_BEEP[regindex],
-			    (data->beeps >> (regindex << 3)) & 0xff);
+	data->write_value(data, data->REG_BEEP[regindex],
+			  (data->beeps >> (regindex << 3)) & 0xff);
 	mutex_unlock(&data->update_lock);
 	return count;
 }
@@ -2049,8 +2050,8 @@ store_temp_beep(struct device *dev, struct device_attribute *attr,
 		data->beeps |= (1ULL << bit);
 	else
 		data->beeps &= ~(1ULL << bit);
-	nct6775_write_value(data, data->REG_BEEP[regindex],
-			    (data->beeps >> (regindex << 3)) & 0xff);
+	data->write_value(data, data->REG_BEEP[regindex],
+			  (data->beeps >> (regindex << 3)) & 0xff);
 	mutex_unlock(&data->update_lock);
 
 	return count;
@@ -2217,7 +2218,7 @@ store_fan_min(struct device *dev, struct device_attribute *attr,
 	}
 
 write_min:
-	nct6775_write_value(data, data->REG_FAN_MIN[nr], data->fan_min[nr]);
+	data->write_value(data, data->REG_FAN_MIN[nr], data->fan_min[nr]);
 	mutex_unlock(&data->update_lock);
 
 	return count;
@@ -2253,10 +2254,10 @@ store_fan_pulses(struct device *dev, struct device_attribute *attr,
 
 	mutex_lock(&data->update_lock);
 	data->fan_pulses[nr] = val & 3;
-	reg = nct6775_read_value(data, data->REG_FAN_PULSES[nr]);
+	reg = data->read_value(data, data->REG_FAN_PULSES[nr]);
 	reg &= ~(0x03 << data->FAN_PULSE_SHIFT[nr]);
 	reg |= (val & 3) << data->FAN_PULSE_SHIFT[nr];
-	nct6775_write_value(data, data->REG_FAN_PULSES[nr], reg);
+	data->write_value(data, data->REG_FAN_PULSES[nr], reg);
 	mutex_unlock(&data->update_lock);
 
 	return count;
@@ -2390,7 +2391,7 @@ store_temp_offset(struct device *dev, struct device_attribute *attr,
 
 	mutex_lock(&data->update_lock);
 	data->temp_offset[nr] = val;
-	nct6775_write_value(data, data->REG_TEMP_OFFSET[nr], val);
+	data->write_value(data, data->REG_TEMP_OFFSET[nr], val);
 	mutex_unlock(&data->update_lock);
 
 	return count;
@@ -2429,8 +2430,8 @@ store_temp_type(struct device *dev, struct device_attribute *attr,
 	data->temp_type[nr] = val;
 	vbit = 0x02 << nr;
 	dbit = data->DIODE_MASK << nr;
-	vbat = nct6775_read_value(data, data->REG_VBAT) & ~vbit;
-	diode = nct6775_read_value(data, data->REG_DIODE) & ~dbit;
+	vbat = data->read_value(data, data->REG_VBAT) & ~vbit;
+	diode = data->read_value(data, data->REG_DIODE) & ~dbit;
 	switch (val) {
 	case 1:	/* CPU diode (diode, current mode) */
 		vbat |= vbit;
@@ -2442,8 +2443,8 @@ store_temp_type(struct device *dev, struct device_attribute *attr,
 	case 4:	/* thermistor */
 		break;
 	}
-	nct6775_write_value(data, data->REG_VBAT, vbat);
-	nct6775_write_value(data, data->REG_DIODE, diode);
+	data->write_value(data, data->REG_VBAT, vbat);
+	data->write_value(data, data->REG_DIODE, diode);
 
 	mutex_unlock(&data->update_lock);
 	return count;
@@ -2567,11 +2568,11 @@ store_pwm_mode(struct device *dev, struct device_attribute *attr,
 
 	mutex_lock(&data->update_lock);
 	data->pwm_mode[nr] = val;
-	reg = nct6775_read_value(data, data->REG_PWM_MODE[nr]);
+	reg = data->read_value(data, data->REG_PWM_MODE[nr]);
 	reg &= ~data->PWM_MODE_MASK[nr];
 	if (!val)
 		reg |= data->PWM_MODE_MASK[nr];
-	nct6775_write_value(data, data->REG_PWM_MODE[nr], reg);
+	data->write_value(data, data->REG_PWM_MODE[nr], reg);
 	mutex_unlock(&data->update_lock);
 	return count;
 }
@@ -2590,7 +2591,7 @@ show_pwm(struct device *dev, struct device_attribute *attr, char *buf)
 	 * Otherwise, show the configured value.
 	 */
 	if (index == 0 && data->pwm_enable[nr] > manual)
-		pwm = nct6775_read_value(data, data->REG_PWM_READ[nr]);
+		pwm = data->read_value(data, data->REG_PWM_READ[nr]);
 	else
 		pwm = data->pwm[index][nr];
 
@@ -2619,13 +2620,13 @@ store_pwm(struct device *dev, struct device_attribute *attr, const char *buf,
 
 	mutex_lock(&data->update_lock);
 	data->pwm[index][nr] = val;
-	nct6775_write_value(data, data->REG_PWM[index][nr], val);
+	data->write_value(data, data->REG_PWM[index][nr], val);
 	if (index == 2)	{ /* floor: disable if val == 0 */
-		reg = nct6775_read_value(data, data->REG_TEMP_SEL[nr]);
+		reg = data->read_value(data, data->REG_TEMP_SEL[nr]);
 		reg &= 0x7f;
 		if (val)
 			reg |= 0x80;
-		nct6775_write_value(data, data->REG_TEMP_SEL[nr], reg);
+		data->write_value(data, data->REG_TEMP_SEL[nr], reg);
 	}
 	mutex_unlock(&data->update_lock);
 	return count;
@@ -2664,29 +2665,29 @@ static void pwm_update_registers(struct nct6775_data *data, int nr)
 	case manual:
 		break;
 	case speed_cruise:
-		reg = nct6775_read_value(data, data->REG_FAN_MODE[nr]);
+		reg = data->read_value(data, data->REG_FAN_MODE[nr]);
 		reg = (reg & ~data->tolerance_mask) |
 		  (data->target_speed_tolerance[nr] & data->tolerance_mask);
-		nct6775_write_value(data, data->REG_FAN_MODE[nr], reg);
-		nct6775_write_value(data, data->REG_TARGET[nr],
+		data->write_value(data, data->REG_FAN_MODE[nr], reg);
+		data->write_value(data, data->REG_TARGET[nr],
 				    data->target_speed[nr] & 0xff);
 		if (data->REG_TOLERANCE_H) {
 			reg = (data->target_speed[nr] >> 8) & 0x0f;
 			reg |= (data->target_speed_tolerance[nr] & 0x38) << 1;
-			nct6775_write_value(data,
-					    data->REG_TOLERANCE_H[nr],
-					    reg);
+			data->write_value(data,
+					  data->REG_TOLERANCE_H[nr],
+					  reg);
 		}
 		break;
 	case thermal_cruise:
-		nct6775_write_value(data, data->REG_TARGET[nr],
-				    data->target_temp[nr]);
+		data->write_value(data, data->REG_TARGET[nr],
+				  data->target_temp[nr]);
 		fallthrough;
 	default:
-		reg = nct6775_read_value(data, data->REG_FAN_MODE[nr]);
+		reg = data->read_value(data, data->REG_FAN_MODE[nr]);
 		reg = (reg & ~data->tolerance_mask) |
 		  data->temp_tolerance[0][nr];
-		nct6775_write_value(data, data->REG_FAN_MODE[nr], reg);
+		data->write_value(data, data->REG_FAN_MODE[nr], reg);
 		break;
 	}
 }
@@ -2734,13 +2735,13 @@ store_pwm_enable(struct device *dev, struct device_attribute *attr,
 		 * turn off pwm control: select manual mode, set pwm to maximum
 		 */
 		data->pwm[0][nr] = 255;
-		nct6775_write_value(data, data->REG_PWM[0][nr], 255);
+		data->write_value(data, data->REG_PWM[0][nr], 255);
 	}
 	pwm_update_registers(data, nr);
-	reg = nct6775_read_value(data, data->REG_FAN_MODE[nr]);
+	reg = data->read_value(data, data->REG_FAN_MODE[nr]);
 	reg &= 0x0f;
 	reg |= pwm_enable_to_reg(val) << 4;
-	nct6775_write_value(data, data->REG_FAN_MODE[nr], reg);
+	data->write_value(data, data->REG_FAN_MODE[nr], reg);
 	mutex_unlock(&data->update_lock);
 	return count;
 }
@@ -2793,10 +2794,10 @@ store_pwm_temp_sel(struct device *dev, struct device_attribute *attr,
 	mutex_lock(&data->update_lock);
 	src = data->temp_src[val - 1];
 	data->pwm_temp_sel[nr] = src;
-	reg = nct6775_read_value(data, data->REG_TEMP_SEL[nr]);
+	reg = data->read_value(data, data->REG_TEMP_SEL[nr]);
 	reg &= 0xe0;
 	reg |= src;
-	nct6775_write_value(data, data->REG_TEMP_SEL[nr], reg);
+	data->write_value(data, data->REG_TEMP_SEL[nr], reg);
 	mutex_unlock(&data->update_lock);
 
 	return count;
@@ -2838,15 +2839,15 @@ store_pwm_weight_temp_sel(struct device *dev, struct device_attribute *attr,
 	if (val) {
 		src = data->temp_src[val - 1];
 		data->pwm_weight_temp_sel[nr] = src;
-		reg = nct6775_read_value(data, data->REG_WEIGHT_TEMP_SEL[nr]);
+		reg = data->read_value(data, data->REG_WEIGHT_TEMP_SEL[nr]);
 		reg &= 0xe0;
 		reg |= (src | 0x80);
-		nct6775_write_value(data, data->REG_WEIGHT_TEMP_SEL[nr], reg);
+		data->write_value(data, data->REG_WEIGHT_TEMP_SEL[nr], reg);
 	} else {
 		data->pwm_weight_temp_sel[nr] = 0;
-		reg = nct6775_read_value(data, data->REG_WEIGHT_TEMP_SEL[nr]);
+		reg = data->read_value(data, data->REG_WEIGHT_TEMP_SEL[nr]);
 		reg &= 0x7f;
-		nct6775_write_value(data, data->REG_WEIGHT_TEMP_SEL[nr], reg);
+		data->write_value(data, data->REG_WEIGHT_TEMP_SEL[nr], reg);
 	}
 	mutex_unlock(&data->update_lock);
 
@@ -2958,9 +2959,9 @@ store_temp_tolerance(struct device *dev, struct device_attribute *attr,
 	if (index)
 		pwm_update_registers(data, nr);
 	else
-		nct6775_write_value(data,
-				    data->REG_CRITICAL_TEMP_TOLERANCE[nr],
-				    val);
+		data->write_value(data,
+				  data->REG_CRITICAL_TEMP_TOLERANCE[nr],
+				  val);
 	mutex_unlock(&data->update_lock);
 	return count;
 }
@@ -3083,7 +3084,7 @@ store_weight_temp(struct device *dev, struct device_attribute *attr,
 
 	mutex_lock(&data->update_lock);
 	data->weight_temp[index][nr] = val;
-	nct6775_write_value(data, data->REG_WEIGHT_TEMP[index][nr], val);
+	data->write_value(data, data->REG_WEIGHT_TEMP[index][nr], val);
 	mutex_unlock(&data->update_lock);
 	return count;
 }
@@ -3132,7 +3133,7 @@ store_fan_time(struct device *dev, struct device_attribute *attr,
 	val = step_time_to_reg(val, data->pwm_mode[nr]);
 	mutex_lock(&data->update_lock);
 	data->fan_time[index][nr] = val;
-	nct6775_write_value(data, data->REG_FAN_TIME[index][nr], val);
+	data->write_value(data, data->REG_FAN_TIME[index][nr], val);
 	mutex_unlock(&data->update_lock);
 	return count;
 }
@@ -3174,21 +3175,21 @@ store_auto_pwm(struct device *dev, struct device_attribute *attr,
 	mutex_lock(&data->update_lock);
 	data->auto_pwm[nr][point] = val;
 	if (point < data->auto_pwm_num) {
-		nct6775_write_value(data,
+		data->write_value(data,
 				    NCT6775_AUTO_PWM(data, nr, point),
 				    data->auto_pwm[nr][point]);
 	} else {
 		switch (data->kind) {
 		case nct6775:
 			/* disable if needed (pwm == 0) */
-			reg = nct6775_read_value(data,
-						 NCT6775_REG_CRITICAL_ENAB[nr]);
+			reg = data->read_value(data,
+					       NCT6775_REG_CRITICAL_ENAB[nr]);
 			if (val)
 				reg |= 0x02;
 			else
 				reg &= ~0x02;
-			nct6775_write_value(data, NCT6775_REG_CRITICAL_ENAB[nr],
-					    reg);
+			data->write_value(data, NCT6775_REG_CRITICAL_ENAB[nr],
+					  reg);
 			break;
 		case nct6776:
 			break; /* always enabled, nothing to do */
@@ -3202,17 +3203,17 @@ store_auto_pwm(struct device *dev, struct device_attribute *attr,
 		case nct6796:
 		case nct6797:
 		case nct6798:
-			nct6775_write_value(data, data->REG_CRITICAL_PWM[nr],
+			data->write_value(data, data->REG_CRITICAL_PWM[nr],
 					    val);
-			reg = nct6775_read_value(data,
+			reg = data->read_value(data,
 					data->REG_CRITICAL_PWM_ENABLE[nr]);
 			if (val == 255)
 				reg &= ~data->CRITICAL_PWM_ENABLE_MASK;
 			else
 				reg |= data->CRITICAL_PWM_ENABLE_MASK;
-			nct6775_write_value(data,
-					    data->REG_CRITICAL_PWM_ENABLE[nr],
-					    reg);
+			data->write_value(data,
+					  data->REG_CRITICAL_PWM_ENABLE[nr],
+					  reg);
 			break;
 		}
 	}
@@ -3255,11 +3256,11 @@ store_auto_temp(struct device *dev, struct device_attribute *attr,
 	mutex_lock(&data->update_lock);
 	data->auto_temp[nr][point] = DIV_ROUND_CLOSEST(val, 1000);
 	if (point < data->auto_pwm_num) {
-		nct6775_write_value(data,
+		data->write_value(data,
 				    NCT6775_AUTO_TEMP(data, nr, point),
 				    data->auto_temp[nr][point]);
 	} else {
-		nct6775_write_value(data, data->REG_CRITICAL_TEMP[nr],
+		data->write_value(data, data->REG_CRITICAL_TEMP[nr],
 				    data->auto_temp[nr][point]);
 	}
 	mutex_unlock(&data->update_lock);
@@ -3519,9 +3520,9 @@ static inline void nct6775_init_device(struct nct6775_data *data)
 
 	/* Start monitoring if needed */
 	if (data->REG_CONFIG) {
-		tmp = nct6775_read_value(data, data->REG_CONFIG);
+		tmp = data->read_value(data, data->REG_CONFIG);
 		if (!(tmp & 0x01))
-			nct6775_write_value(data, data->REG_CONFIG, tmp | 0x01);
+			data->write_value(data, data->REG_CONFIG, tmp | 0x01);
 	}
 
 	/* Enable temperature sensors if needed */
@@ -3530,18 +3531,18 @@ static inline void nct6775_init_device(struct nct6775_data *data)
 			continue;
 		if (!data->reg_temp_config[i])
 			continue;
-		tmp = nct6775_read_value(data, data->reg_temp_config[i]);
+		tmp = data->read_value(data, data->reg_temp_config[i]);
 		if (tmp & 0x01)
-			nct6775_write_value(data, data->reg_temp_config[i],
+			data->write_value(data, data->reg_temp_config[i],
 					    tmp & 0xfe);
 	}
 
 	/* Enable VBAT monitoring if needed */
-	tmp = nct6775_read_value(data, data->REG_VBAT);
+	tmp = data->read_value(data, data->REG_VBAT);
 	if (!(tmp & 0x01))
-		nct6775_write_value(data, data->REG_VBAT, tmp | 0x01);
+		data->write_value(data, data->REG_VBAT, tmp | 0x01);
 
-	diode = nct6775_read_value(data, data->REG_DIODE);
+	diode = data->read_value(data, data->REG_DIODE);
 
 	for (i = 0; i < data->temp_fixed_num; i++) {
 		if (!(data->have_temp_fixed & BIT(i)))
@@ -3786,7 +3787,7 @@ static void add_temp_sensors(struct nct6775_data *data, const u16 *regp,
 
 		if (!regp[i])
 			continue;
-		src = nct6775_read_value(data, regp[i]);
+		src = data->read_value(data, regp[i]);
 		src &= 0x1f;
 		if (!src || (*mask & BIT(src)))
 			continue;
@@ -3794,7 +3795,7 @@ static void add_temp_sensors(struct nct6775_data *data, const u16 *regp,
 			continue;
 
 		index = __ffs(*available);
-		nct6775_write_value(data, data->REG_TEMP_SOURCE[index], src);
+		data->write_value(data, data->REG_TEMP_SOURCE[index], src);
 		*available &= ~BIT(index);
 		*mask |= BIT(src);
 	}
@@ -3830,6 +3831,8 @@ static int nct6775_probe(struct platform_device *pdev)
 	data->kind = sio_data->kind;
 	data->sioreg = sio_data->sioreg;
 	data->addr = res->start;
+	data->read_value = nct6775_read_value;
+	data->write_value = nct6775_write_value;
 	mutex_init(&data->update_lock);
 	data->name = nct6775_device_names[data->kind];
 	data->bank = 0xff;		/* Force initial bank selection */
@@ -4349,7 +4352,7 @@ static int nct6775_probe(struct platform_device *pdev)
 		if (reg_temp[i] == 0)
 			continue;
 
-		src = nct6775_read_value(data, data->REG_TEMP_SOURCE[i]) & 0x1f;
+		src = data->read_value(data, data->REG_TEMP_SOURCE[i]) & 0x1f;
 		if (!src || (mask & BIT(src)))
 			available |= BIT(i);
 
@@ -4369,7 +4372,7 @@ static int nct6775_probe(struct platform_device *pdev)
 		if (reg_temp[i] == 0)
 			continue;
 
-		src = nct6775_read_value(data, data->REG_TEMP_SOURCE[i]) & 0x1f;
+		src = data->read_value(data, data->REG_TEMP_SOURCE[i]) & 0x1f;
 		if (!src || (mask & BIT(src)))
 			continue;
 
@@ -4429,7 +4432,7 @@ static int nct6775_probe(struct platform_device *pdev)
 		if (reg_temp_mon[i] == 0)
 			continue;
 
-		src = nct6775_read_value(data, data->REG_TEMP_SEL[i]) & 0x1f;
+		src = data->read_value(data, data->REG_TEMP_SEL[i]) & 0x1f;
 		if (!src)
 			continue;
 
@@ -4642,10 +4645,10 @@ static int __maybe_unused nct6775_suspend(struct device *dev)
 	struct nct6775_data *data = nct6775_update_device(dev);
 
 	mutex_lock(&data->update_lock);
-	data->vbat = nct6775_read_value(data, data->REG_VBAT);
+	data->vbat = data->read_value(data, data->REG_VBAT);
 	if (data->kind == nct6775) {
-		data->fandiv1 = nct6775_read_value(data, NCT6775_REG_FANDIV1);
-		data->fandiv2 = nct6775_read_value(data, NCT6775_REG_FANDIV2);
+		data->fandiv1 = data->read_value(data, NCT6775_REG_FANDIV1);
+		data->fandiv2 = data->read_value(data, NCT6775_REG_FANDIV2);
 	}
 	mutex_unlock(&data->update_lock);
 
@@ -4684,18 +4687,18 @@ static int __maybe_unused nct6775_resume(struct device *dev)
 		if (!(data->have_in & BIT(i)))
 			continue;
 
-		nct6775_write_value(data, data->REG_IN_MINMAX[0][i],
-				    data->in[i][1]);
-		nct6775_write_value(data, data->REG_IN_MINMAX[1][i],
-				    data->in[i][2]);
+		data->write_value(data, data->REG_IN_MINMAX[0][i],
+				  data->in[i][1]);
+		data->write_value(data, data->REG_IN_MINMAX[1][i],
+				  data->in[i][2]);
 	}
 
 	for (i = 0; i < ARRAY_SIZE(data->fan_min); i++) {
 		if (!(data->has_fan_min & BIT(i)))
 			continue;
 
-		nct6775_write_value(data, data->REG_FAN_MIN[i],
-				    data->fan_min[i]);
+		data->write_value(data, data->REG_FAN_MIN[i],
+				  data->fan_min[i]);
 	}
 
 	for (i = 0; i < NUM_TEMP; i++) {
@@ -4709,10 +4712,10 @@ static int __maybe_unused nct6775_resume(struct device *dev)
 	}
 
 	/* Restore other settings */
-	nct6775_write_value(data, data->REG_VBAT, data->vbat);
+	data->write_value(data, data->REG_VBAT, data->vbat);
 	if (data->kind == nct6775) {
-		nct6775_write_value(data, NCT6775_REG_FANDIV1, data->fandiv1);
-		nct6775_write_value(data, NCT6775_REG_FANDIV2, data->fandiv2);
+		data->write_value(data, NCT6775_REG_FANDIV1, data->fandiv1);
+		data->write_value(data, NCT6775_REG_FANDIV2, data->fandiv2);
 	}
 
 abort:
-- 
2.33.0


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

* [PATCH v7 3/3] hwmon: (nct6775) Support access via Asus WMI
  2021-09-16 20:22 [PATCH v6 0/3] hwmon: Support access to the NCT677x via Asus WMI Denis Pauk
                   ` (2 preceding siblings ...)
  2021-09-16 20:22 ` [PATCH v7 2/3] hwmon: (nct6775) Use nct6775_*() function pointers in nct6775_data Denis Pauk
@ 2021-09-16 20:22 ` Denis Pauk
  2021-09-16 21:10   ` Guenter Roeck
  3 siblings, 1 reply; 8+ messages in thread
From: Denis Pauk @ 2021-09-16 20:22 UTC (permalink / raw)
  Cc: pauk.denis, Bernhard Seibold, Pär Ekholm, to.eivind,
	Artem S . Tashkinov, Vittorio Roberto Alfieri, Sahan Fernando,
	Andy Shevchenko, Guenter Roeck, Jean Delvare, linux-hwmon,
	linux-kernel

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@gmail.com>
Co-developed-by: Bernhard Seibold <mail@bernhard-seibold.de>
Signed-off-by: Bernhard Seibold <mail@bernhard-seibold.de>
Tested-by: Pär Ekholm <pehlm@pekholm.org>
Tested-by: <to.eivind@gmail.com>
Tested-by: Artem S. Tashkinov <aros@gmx.com>
Tested-by: Vittorio Roberto Alfieri <me@rebtoor.com>
Tested-by: Sahan Fernando <sahan.h.fernando@gmail.com>
Cc: Andy Shevchenko <andriy.shevchenko@intel.com>
Cc: Guenter Roeck <linux@roeck-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)
+{
+#if IS_ENABLED(CONFIG_ACPI_WMI)
+	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 = ASUSWMI_UNSUPPORTED_METHOD;
+
+	status = wmi_evaluate_method(ASUSWMI_MONITORING_GUID, 0,
+				     method_id, &input, &output);
+
+	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);
+
+	if (tmp == ASUSWMI_UNSUPPORTED_METHOD)
+		return -ENODEV;
+	return 0;
+#else
+	return -EOPNOTSUPP;
+#endif
+}
+
+static inline int nct6775_asuswmi_write(u8 bank, u8 reg, u8 val)
+{
+	return asuswmi_evaluate_method(ASUSWMI_METHODID_WHWM, bank,
+							      reg, val, NULL);
+}
+
+static inline int nct6775_asuswmi_read(u8 bank, u8 reg, u8 *val)
+{
+	u32 tmp = 0;
+	int ret = asuswmi_evaluate_method(ASUSWMI_METHODID_RHWM, bank,
+				     reg, 0, &tmp);
+	*val = tmp;
+	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, NULL);
+}
+
+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)
 {
 	int ioreg = sio_data->sioreg;
@@ -207,6 +295,7 @@ static void superio_exit(struct nct6775_sio_data *sio_data)
 
 #define NCT6775_REG_BANK	0x4E
 #define NCT6775_REG_CONFIG	0x40
+#define NCT6775_PORT_CHIPID	0x58
 
 /*
  * Not currently used:
@@ -1423,6 +1512,45 @@ static bool is_word_sized(struct nct6775_data *data, u16 reg)
 	return false;
 }
 
+static inline void nct6775_wmi_set_bank(struct nct6775_data *data, u16 reg)
+{
+	u8 bank = reg >> 8;
+
+	data->bank = bank;
+}
+
+static u16 nct6775_wmi_read_value(struct nct6775_data *data, u16 reg)
+{
+	int res, word_sized = is_word_sized(data, reg);
+	u8 tmp;
+
+	nct6775_wmi_set_bank(data, reg);
+
+	nct6775_asuswmi_read(data->bank, reg, &tmp);
+	res = tmp;
+	if (word_sized) {
+		nct6775_asuswmi_read(data->bank, (reg & 0xff) + 1, &tmp);
+		res = (res << 8) + tmp;
+	}
+	return res;
+}
+
+static int nct6775_wmi_write_value(struct nct6775_data *data, u16 reg, u16 value)
+{
+	int word_sized = is_word_sized(data, reg);
+
+	nct6775_wmi_set_bank(data, reg);
+
+	if (word_sized) {
+		nct6775_asuswmi_write(data->bank, reg & 0xff, value >> 8);
+		nct6775_asuswmi_write(data->bank, (reg & 0xff) + 1, value);
+	} else {
+		nct6775_asuswmi_write(data->bank, reg & 0xff, value);
+	}
+
+	return 0;
+}
+
 /*
  * On older chips, only registers 0x50-0x5f are banked.
  * On more recent chips, all registers are banked.
@@ -3818,10 +3946,12 @@ 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 (!devm_request_region(&pdev->dev, res->start, IOREGION_LENGTH,
-				 DRVNAME))
-		return -EBUSY;
+	if (sio_data->access == access_direct) {
+		res = platform_get_resource(pdev, IORESOURCE_IO, 0);
+		if (!devm_request_region(&pdev->dev, res->start, IOREGION_LENGTH,
+					 DRVNAME))
+			return -EBUSY;
+	}
 
 	data = devm_kzalloc(&pdev->dev, sizeof(struct nct6775_data),
 			    GFP_KERNEL);
@@ -3830,9 +3960,16 @@ static int nct6775_probe(struct platform_device *pdev)
 
 	data->kind = sio_data->kind;
 	data->sioreg = sio_data->sioreg;
-	data->addr = res->start;
-	data->read_value = nct6775_read_value;
-	data->write_value = nct6775_write_value;
+
+	if (sio_data->access == access_direct) {
+		data->addr = res->start;
+		data->read_value = nct6775_read_value;
+		data->write_value = nct6775_write_value;
+	} else {
+		data->read_value = nct6775_wmi_read_value;
+		data->write_value = nct6775_wmi_write_value;
+	}
+
 	mutex_init(&data->update_lock);
 	data->name = nct6775_device_names[data->kind];
 	data->bank = 0xff;		/* Force initial bank selection */
@@ -4743,6 +4880,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->sio_enter(sio_data);
@@ -4837,6 +4975,23 @@ static int __init nct6775_find(int sioaddr, struct nct6775_sio_data *sio_data)
  */
 static struct platform_device *pdev[2];
 
+static const char * const asus_wmi_boards[] = {
+	"PRIME B460-PLUS",
+	"ROG CROSSHAIR VIII DARK HERO",
+	"ROG CROSSHAIR VIII HERO",
+	"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-PLUS (WI-FI)",
+	"TUF GAMING X570-PRO (WI-FI)",
+};
+
 static int __init sensors_nct6775_init(void)
 {
 	int i, err;
@@ -4845,11 +5000,32 @@ static int __init sensors_nct6775_init(void)
 	struct resource res;
 	struct nct6775_sio_data sio_data;
 	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)
 		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.")) {
+		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)) {
+				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");
+			}
+		}
+	}
+
 	/*
 	 * initialize sio_data->kind and sio_data->sioreg.
 	 *
@@ -4870,6 +5046,16 @@ static int __init sensors_nct6775_init(void)
 
 		found = true;
 
+		sio_data.access = access;
+
+		if (access == access_asuswmi) {
+			sio_data.sio_outb = superio_wmi_outb;
+			sio_data.sio_inb = superio_wmi_inb;
+			sio_data.sio_select = superio_wmi_select;
+			sio_data.sio_enter = superio_wmi_enter;
+			sio_data.sio_exit = superio_wmi_exit;
+		}
+
 		pdev[i] = platform_device_alloc(DRVNAME, address);
 		if (!pdev[i]) {
 			err = -ENOMEM;
@@ -4881,23 +5067,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


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

* Re: [PATCH v7 3/3] hwmon: (nct6775) Support access via Asus WMI
  2021-09-16 20:22 ` [PATCH v7 3/3] hwmon: (nct6775) Support access via Asus WMI Denis Pauk
@ 2021-09-16 21:10   ` Guenter Roeck
  2021-09-16 21:24     ` Denis Pauk
  0 siblings, 1 reply; 8+ messages in thread
From: Guenter Roeck @ 2021-09-16 21:10 UTC (permalink / raw)
  To: Denis Pauk
  Cc: Bernhard Seibold, Pär Ekholm, to.eivind,
	Artem S . Tashkinov, Vittorio Roberto Alfieri, Sahan Fernando,
	Andy Shevchenko, Jean Delvare, linux-hwmon, linux-kernel

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@gmail.com>
> Co-developed-by: Bernhard Seibold <mail@bernhard-seibold.de>
> Signed-off-by: Bernhard Seibold <mail@bernhard-seibold.de>
> Tested-by: Pär Ekholm <pehlm@pekholm.org>
> Tested-by: <to.eivind@gmail.com>
> Tested-by: Artem S. Tashkinov <aros@gmx.com>
> Tested-by: Vittorio Roberto Alfieri <me@rebtoor.com>
> Tested-by: Sahan Fernando <sahan.h.fernando@gmail.com>
> Cc: Andy Shevchenko <andriy.shevchenko@intel.com>
> Cc: Guenter Roeck <linux@roeck-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)
> +{
> +#if IS_ENABLED(CONFIG_ACPI_WMI)
> +	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 = ASUSWMI_UNSUPPORTED_METHOD;
> +
> +	status = wmi_evaluate_method(ASUSWMI_MONITORING_GUID, 0,
> +				     method_id, &input, &output);
> +
> +	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);
> +
> +	if (tmp == ASUSWMI_UNSUPPORTED_METHOD)
> +		return -ENODEV;
> +	return 0;
> +#else
> +	return -EOPNOTSUPP;
> +#endif
> +}
> +
> +static inline int nct6775_asuswmi_write(u8 bank, u8 reg, u8 val)
> +{
> +	return asuswmi_evaluate_method(ASUSWMI_METHODID_WHWM, bank,
> +							      reg, val, NULL);
> +}
> +
> +static inline int nct6775_asuswmi_read(u8 bank, u8 reg, u8 *val)
> +{
> +	u32 tmp = 0;
> +	int ret = asuswmi_evaluate_method(ASUSWMI_METHODID_RHWM, bank,
> +				     reg, 0, &tmp);
> +	*val = tmp;
> +	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, NULL);
> +}
> +
> +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)
>   {
>   	int ioreg = sio_data->sioreg;
> @@ -207,6 +295,7 @@ static void superio_exit(struct nct6775_sio_data *sio_data)
>   
>   #define NCT6775_REG_BANK	0x4E
>   #define NCT6775_REG_CONFIG	0x40
> +#define NCT6775_PORT_CHIPID	0x58
>   
>   /*
>    * Not currently used:
> @@ -1423,6 +1512,45 @@ static bool is_word_sized(struct nct6775_data *data, u16 reg)
>   	return false;
>   }
>   
> +static inline void nct6775_wmi_set_bank(struct nct6775_data *data, u16 reg)
> +{
> +	u8 bank = reg >> 8;
> +
> +	data->bank = bank;
> +}
> +
> +static u16 nct6775_wmi_read_value(struct nct6775_data *data, u16 reg)
> +{
> +	int res, word_sized = is_word_sized(data, reg);
> +	u8 tmp;
> +
> +	nct6775_wmi_set_bank(data, reg);
> +
> +	nct6775_asuswmi_read(data->bank, reg, &tmp);
> +	res = tmp;
> +	if (word_sized) {
> +		nct6775_asuswmi_read(data->bank, (reg & 0xff) + 1, &tmp);

I just realized that all error returns from the wmi access methods are ignored.
Why ?

> +		res = (res << 8) + tmp;
> +	}
> +	return res;
> +}
> +
> +static int nct6775_wmi_write_value(struct nct6775_data *data, u16 reg, u16 value)
> +{
> +	int word_sized = is_word_sized(data, reg);
> +
> +	nct6775_wmi_set_bank(data, reg);
> +
> +	if (word_sized) {
> +		nct6775_asuswmi_write(data->bank, reg & 0xff, value >> 8);
> +		nct6775_asuswmi_write(data->bank, (reg & 0xff) + 1, value);
> +	} else {
> +		nct6775_asuswmi_write(data->bank, reg & 0xff, value);
> +	}
> +
> +	return 0;
> +}
> +
>   /*
>    * On older chips, only registers 0x50-0x5f are banked.
>    * On more recent chips, all registers are banked.
> @@ -3818,10 +3946,12 @@ 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 (!devm_request_region(&pdev->dev, res->start, IOREGION_LENGTH,
> -				 DRVNAME))
> -		return -EBUSY;
> +	if (sio_data->access == access_direct) {
> +		res = platform_get_resource(pdev, IORESOURCE_IO, 0);
> +		if (!devm_request_region(&pdev->dev, res->start, IOREGION_LENGTH,
> +					 DRVNAME))
> +			return -EBUSY;
> +	}
>   
>   	data = devm_kzalloc(&pdev->dev, sizeof(struct nct6775_data),
>   			    GFP_KERNEL);
> @@ -3830,9 +3960,16 @@ static int nct6775_probe(struct platform_device *pdev)
>   
>   	data->kind = sio_data->kind;
>   	data->sioreg = sio_data->sioreg;
> -	data->addr = res->start;
> -	data->read_value = nct6775_read_value;
> -	data->write_value = nct6775_write_value;
> +
> +	if (sio_data->access == access_direct) {
> +		data->addr = res->start;
> +		data->read_value = nct6775_read_value;
> +		data->write_value = nct6775_write_value;
> +	} else {
> +		data->read_value = nct6775_wmi_read_value;
> +		data->write_value = nct6775_wmi_write_value;
> +	}
> +
>   	mutex_init(&data->update_lock);
>   	data->name = nct6775_device_names[data->kind];
>   	data->bank = 0xff;		/* Force initial bank selection */
> @@ -4743,6 +4880,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->sio_enter(sio_data);
> @@ -4837,6 +4975,23 @@ static int __init nct6775_find(int sioaddr, struct nct6775_sio_data *sio_data)
>    */
>   static struct platform_device *pdev[2];
>   
> +static const char * const asus_wmi_boards[] = {
> +	"PRIME B460-PLUS",
> +	"ROG CROSSHAIR VIII DARK HERO",
> +	"ROG CROSSHAIR VIII HERO",
> +	"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-PLUS (WI-FI)",
> +	"TUF GAMING X570-PRO (WI-FI)",
> +};
> +
>   static int __init sensors_nct6775_init(void)
>   {
>   	int i, err;
> @@ -4845,11 +5000,32 @@ static int __init sensors_nct6775_init(void)
>   	struct resource res;
>   	struct nct6775_sio_data sio_data;
>   	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)
>   		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.")) {
> +		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)) {
> +				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");
> +			}
> +		}
> +	}
> +
>   	/*
>   	 * initialize sio_data->kind and sio_data->sioreg.
>   	 *
> @@ -4870,6 +5046,16 @@ static int __init sensors_nct6775_init(void)
>   
>   		found = true;
>   
> +		sio_data.access = access;
> +
> +		if (access == access_asuswmi) {
> +			sio_data.sio_outb = superio_wmi_outb;
> +			sio_data.sio_inb = superio_wmi_inb;
> +			sio_data.sio_select = superio_wmi_select;
> +			sio_data.sio_enter = superio_wmi_enter;
> +			sio_data.sio_exit = superio_wmi_exit;
> +		}
> +
>   		pdev[i] = platform_device_alloc(DRVNAME, address);
>   		if (!pdev[i]) {
>   			err = -ENOMEM;
> @@ -4881,23 +5067,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)
> 


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

* Re: [PATCH v7 3/3] hwmon: (nct6775) Support access via Asus WMI
  2021-09-16 21:10   ` Guenter Roeck
@ 2021-09-16 21:24     ` Denis Pauk
  2021-09-16 22:24       ` Guenter Roeck
  0 siblings, 1 reply; 8+ messages in thread
From: Denis Pauk @ 2021-09-16 21:24 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Bernhard Seibold, Pär Ekholm, to.eivind,
	Artem S . Tashkinov, Vittorio Roberto Alfieri, Sahan Fernando,
	Andy Shevchenko, Jean Delvare, linux-hwmon, linux-kernel

On Thu, 16 Sep 2021 14:10:49 -0700
Guenter Roeck <linux@roeck-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@gmail.com>
> > Co-developed-by: Bernhard Seibold <mail@bernhard-seibold.de>
> > Signed-off-by: Bernhard Seibold <mail@bernhard-seibold.de>
> > Tested-by: Pär Ekholm <pehlm@pekholm.org>
> > Tested-by: <to.eivind@gmail.com>
> > Tested-by: Artem S. Tashkinov <aros@gmx.com>
> > Tested-by: Vittorio Roberto Alfieri <me@rebtoor.com>
> > Tested-by: Sahan Fernando <sahan.h.fernando@gmail.com>
> > Cc: Andy Shevchenko <andriy.shevchenko@intel.com>
> > Cc: Guenter Roeck <linux@roeck-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.
> > +#if IS_ENABLED(CONFIG_ACPI_WMI)
> > +	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 = ASUSWMI_UNSUPPORTED_METHOD;
> > +
> > +	status = wmi_evaluate_method(ASUSWMI_MONITORING_GUID, 0,
> > +				     method_id, &input, &output);
> > +
> > +	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);
> > +
> > +	if (tmp == ASUSWMI_UNSUPPORTED_METHOD)
> > +		return -ENODEV;
> > +	return 0;
> > +#else
> > +	return -EOPNOTSUPP;
> > +#endif
> > +}
> > +
> > +static inline int nct6775_asuswmi_write(u8 bank, u8 reg, u8 val)
> > +{
> > +	return asuswmi_evaluate_method(ASUSWMI_METHODID_WHWM, bank,
> > +							      reg,
> > val, NULL); +}
> > +
> > +static inline int nct6775_asuswmi_read(u8 bank, u8 reg, u8 *val)
> > +{
> > +	u32 tmp = 0;
> > +	int ret = asuswmi_evaluate_method(ASUSWMI_METHODID_RHWM,
> > bank,
> > +				     reg, 0, &tmp);
> > +	*val = tmp;
> > +	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, NULL);
> > +}
> > +
> > +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) {
> >   	int ioreg = sio_data->sioreg;
> > @@ -207,6 +295,7 @@ static void superio_exit(struct
> > nct6775_sio_data *sio_data) 
> >   #define NCT6775_REG_BANK	0x4E
> >   #define NCT6775_REG_CONFIG	0x40
> > +#define NCT6775_PORT_CHIPID	0x58
> >   
> >   /*
> >    * Not currently used:
> > @@ -1423,6 +1512,45 @@ static bool is_word_sized(struct
> > nct6775_data *data, u16 reg) return false;
> >   }
> >   
> > +static inline void nct6775_wmi_set_bank(struct nct6775_data *data,
> > u16 reg) +{
> > +	u8 bank = reg >> 8;
> > +
> > +	data->bank = bank;
> > +}
> > +
> > +static u16 nct6775_wmi_read_value(struct nct6775_data *data, u16
> > reg) +{
> > +	int res, word_sized = is_word_sized(data, reg);
> > +	u8 tmp;
> > +
> > +	nct6775_wmi_set_bank(data, reg);
> > +
> > +	nct6775_asuswmi_read(data->bank, reg, &tmp);
> > +	res = tmp;
> > +	if (word_sized) {
> > +		nct6775_asuswmi_read(data->bank, (reg & 0xff) + 1,
> > &tmp);  
> 
> I just realized that all error returns from the wmi access methods
> are ignored. Why ?


Good question, I will check and add checks for all cases. Thank you.

> 
> > +		res = (res << 8) + tmp;
> > +	}
> > +	return res;
> > +}
> > +
> > +static int nct6775_wmi_write_value(struct nct6775_data *data, u16
> > reg, u16 value) +{
> > +	int word_sized = is_word_sized(data, reg);
> > +
> > +	nct6775_wmi_set_bank(data, reg);
> > +
> > +	if (word_sized) {
> > +		nct6775_asuswmi_write(data->bank, reg & 0xff,
> > value >> 8);
> > +		nct6775_asuswmi_write(data->bank, (reg & 0xff) +
> > 1, value);
> > +	} else {
> > +		nct6775_asuswmi_write(data->bank, reg & 0xff,
> > value);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >   /*
> >    * On older chips, only registers 0x50-0x5f are banked.
> >    * On more recent chips, all registers are banked.
> > @@ -3818,10 +3946,12 @@ 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 (!devm_request_region(&pdev->dev, res->start,
> > IOREGION_LENGTH,
> > -				 DRVNAME))
> > -		return -EBUSY;
> > +	if (sio_data->access == access_direct) {
> > +		res = platform_get_resource(pdev, IORESOURCE_IO,
> > 0);
> > +		if (!devm_request_region(&pdev->dev, res->start,
> > IOREGION_LENGTH,
> > +					 DRVNAME))
> > +			return -EBUSY;
> > +	}
> >   
> >   	data = devm_kzalloc(&pdev->dev, sizeof(struct
> > nct6775_data), GFP_KERNEL);
> > @@ -3830,9 +3960,16 @@ static int nct6775_probe(struct
> > platform_device *pdev) 
> >   	data->kind = sio_data->kind;
> >   	data->sioreg = sio_data->sioreg;
> > -	data->addr = res->start;
> > -	data->read_value = nct6775_read_value;
> > -	data->write_value = nct6775_write_value;
> > +
> > +	if (sio_data->access == access_direct) {
> > +		data->addr = res->start;
> > +		data->read_value = nct6775_read_value;
> > +		data->write_value = nct6775_write_value;
> > +	} else {
> > +		data->read_value = nct6775_wmi_read_value;
> > +		data->write_value = nct6775_wmi_write_value;
> > +	}
> > +
> >   	mutex_init(&data->update_lock);
> >   	data->name = nct6775_device_names[data->kind];
> >   	data->bank = 0xff;		/* Force initial bank
> > selection */ @@ -4743,6 +4880,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->sio_enter(sio_data);
> > @@ -4837,6 +4975,23 @@ static int __init nct6775_find(int sioaddr,
> > struct nct6775_sio_data *sio_data) */
> >   static struct platform_device *pdev[2];
> >   
> > +static const char * const asus_wmi_boards[] = {
> > +	"PRIME B460-PLUS",
> > +	"ROG CROSSHAIR VIII DARK HERO",
> > +	"ROG CROSSHAIR VIII HERO",
> > +	"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-PLUS (WI-FI)",
> > +	"TUF GAMING X570-PRO (WI-FI)",
> > +};
> > +
> >   static int __init sensors_nct6775_init(void)
> >   {
> >   	int i, err;
> > @@ -4845,11 +5000,32 @@ static int __init sensors_nct6775_init(void)
> >   	struct resource res;
> >   	struct nct6775_sio_data sio_data;
> >   	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)
> >   		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.")) {
> > +		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)) {
> > +				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");
> > +			}
> > +		}
> > +	}
> > +
> >   	/*
> >   	 * initialize sio_data->kind and sio_data->sioreg.
> >   	 *
> > @@ -4870,6 +5046,16 @@ static int __init sensors_nct6775_init(void)
> >   
> >   		found = true;
> >   
> > +		sio_data.access = access;
> > +
> > +		if (access == access_asuswmi) {
> > +			sio_data.sio_outb = superio_wmi_outb;
> > +			sio_data.sio_inb = superio_wmi_inb;
> > +			sio_data.sio_select = superio_wmi_select;
> > +			sio_data.sio_enter = superio_wmi_enter;
> > +			sio_data.sio_exit = superio_wmi_exit;
> > +		}
> > +
> >   		pdev[i] = platform_device_alloc(DRVNAME, address);
> >   		if (!pdev[i]) {
> >   			err = -ENOMEM;
> > @@ -4881,23 +5067,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)
> >   
> 


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

* Re: [PATCH v7 3/3] hwmon: (nct6775) Support access via Asus WMI
  2021-09-16 21:24     ` Denis Pauk
@ 2021-09-16 22:24       ` Guenter Roeck
  0 siblings, 0 replies; 8+ messages in thread
From: Guenter Roeck @ 2021-09-16 22:24 UTC (permalink / raw)
  To: Denis Pauk
  Cc: Bernhard Seibold, Pär Ekholm, to.eivind,
	Artem S . Tashkinov, Vittorio Roberto Alfieri, Sahan Fernando,
	Andy Shevchenko, Jean Delvare, linux-hwmon, linux-kernel

On 9/16/21 2:24 PM, Denis Pauk wrote:
> On Thu, 16 Sep 2021 14:10:49 -0700
> Guenter Roeck <linux@roeck-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@gmail.com>
>>> Co-developed-by: Bernhard Seibold <mail@bernhard-seibold.de>
>>> Signed-off-by: Bernhard Seibold <mail@bernhard-seibold.de>
>>> Tested-by: Pär Ekholm <pehlm@pekholm.org>
>>> Tested-by: <to.eivind@gmail.com>
>>> Tested-by: Artem S. Tashkinov <aros@gmx.com>
>>> Tested-by: Vittorio Roberto Alfieri <me@rebtoor.com>
>>> Tested-by: Sahan Fernando <sahan.h.fernando@gmail.com>
>>> Cc: Andy Shevchenko <andriy.shevchenko@intel.com>
>>> Cc: Guenter Roeck <linux@roeck-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

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

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

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-16 20:22 [PATCH v6 0/3] hwmon: Support access to the NCT677x via Asus WMI Denis Pauk
2021-09-16 20:22 ` [PATCH v7 " Denis Pauk
2021-09-16 20:22 ` [PATCH v7 1/3] hwmon: (nct6775) Use superio_*() function pointers in sio_data Denis Pauk
2021-09-16 20:22 ` [PATCH v7 2/3] hwmon: (nct6775) Use nct6775_*() function pointers in nct6775_data Denis Pauk
2021-09-16 20:22 ` [PATCH v7 3/3] hwmon: (nct6775) Support access via Asus WMI Denis Pauk
2021-09-16 21:10   ` Guenter Roeck
2021-09-16 21:24     ` Denis Pauk
2021-09-16 22:24       ` Guenter Roeck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.