linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 00/13] Support of ASUS TUF Gaming series laptops
@ 2019-05-14 18:47 Yurii Pavlovskyi
  2019-05-14 18:50 ` [PATCH v4 01/13] platform/x86: asus-wmi: Fix hwmon device cleanup Yurii Pavlovskyi
                   ` (13 more replies)
  0 siblings, 14 replies; 19+ messages in thread
From: Yurii Pavlovskyi @ 2019-05-14 18:47 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	Chris Chiu, acpi4asus-user, platform-driver-x86, linux-kernel

Hi,

this is the fourth version of the patch series. 

Changelog:
v4:
  * Rebase on for-next branch
  * Extract local variable in patch 01
  * Rename new method to "..._method3" and keep comma in struct declaration
    in patch 03 (NOTE: the arg2 does not fit on same line by 1 character)
  * Patch "Improve DSTS WMI method ID detection":
    - sort local variables
    - use dev_info
    - separate changes to wmi module in an own patch
    - rename method ID constants and fix comment capitalization
  * "Support WMI event queue": split into separate refactoring and new
    functionality patches, use dev_info as well
  * "Organize code into sections": split out error handling refactoring
  * "Enhance detection of thermal data": remove unreasonable refactoring
    and just change the currently used condition
  * "Control RGB keyboard backlight": removed, will be posted afterwards.
    I will follow on the status of the multicolor framework, it does look
    promising for this.
  * Mark URL references with "Link:"
  * Minor corrections to commit messages
v3: 
  * Use devm_* function in patch 01
  * Detect DSTS/DCTS using _UID in patch 04
  * Detect event queue by _UID as well in patch 05
  * Rename poll function in patch 05
  * Fix terminology in patches 09 and 10
  * Correct commit messages
v2:
  * Fix logging

INTRODUCTION
The support for this laptop series is currently non-existent, as the
asus-nb-wmi driver (which is essentially configuration for asus-wmi) fails
to load and multiple ACPI errors are logged in dmesg. This patch series
adds pretty comprehensive support for these relatively new laptops, adds
some code organization, and fixes a couple of bugs in the asus-wmi module.

Thread for V1/V2: https://lkml.org/lkml/2019/4/10/973
Thread for V3: https://lkml.org/lkml/2019/4/19/178

Yurii Pavlovskyi (13):
  platform/x86: asus-wmi: Fix hwmon device cleanup
  platform/x86: asus-wmi: Fix preserving keyboard backlight intensity on
    load
  platform/x86: asus-wmi: Increase input buffer size of WMI methods
  platform/x86: wmi: Add function to get _UID of WMI device
  platform/x86: asus-wmi: Improve DSTS WMI method ID detection
  platform/x86: asus-wmi: Refactor WMI event handling
  platform/x86: asus-wmi: Support WMI event queue
  platform/x86: asus-nb-wmi: Add microphone mute key code
  platform/x86: asus-wmi: Refactor error handling
  platform/x86: asus-wmi: Organize code into sections
  platform/x86: asus-wmi: Enhance detection of thermal data
  platform/x86: asus-wmi: Switch fan boost mode
  platform/x86: asus-wmi: Do not disable keyboard backlight on unloading

 .../ABI/testing/sysfs-platform-asus-wmi       |  10 +
 drivers/hid/hid-asus.c                        |   2 +-
 drivers/platform/x86/asus-nb-wmi.c            |   3 +-
 drivers/platform/x86/asus-wmi.c               | 427 ++++++++++++++----
 drivers/platform/x86/wmi.c                    |  19 +
 include/linux/acpi.h                          |   1 +
 include/linux/platform_data/x86/asus-wmi.h    |   5 +-
 7 files changed, 374 insertions(+), 93 deletions(-)

-- 
2.17.1


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

* [PATCH v4 01/13] platform/x86: asus-wmi: Fix hwmon device cleanup
  2019-05-14 18:47 [PATCH v4 00/13] Support of ASUS TUF Gaming series laptops Yurii Pavlovskyi
@ 2019-05-14 18:50 ` Yurii Pavlovskyi
  2019-05-24 19:54   ` Daniel Drake
  2019-05-14 18:51 ` [PATCH v4 02/13] platform/x86: asus-wmi: Fix preserving keyboard backlight intensity on load Yurii Pavlovskyi
                   ` (12 subsequent siblings)
  13 siblings, 1 reply; 19+ messages in thread
From: Yurii Pavlovskyi @ 2019-05-14 18:50 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	Chris Chiu, acpi4asus-user, platform-driver-x86, linux-kernel

The driver does not clean up the hwmon device on exit or error. To
reproduce the bug, repeat rmmod, insmod to verify that device number
/sys/devices/platform/asus-nb-wmi/hwmon/hwmon?? grows every time. Replace
call for registering device with devm_* version that unregisters it
automatically.

Signed-off-by: Yurii Pavlovskyi <yurii.pavlovskyi@gmail.com>
---
 drivers/platform/x86/asus-wmi.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index f94691615881..62567766bdfb 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -1428,11 +1428,12 @@ __ATTRIBUTE_GROUPS(hwmon_attribute);
 
 static int asus_wmi_hwmon_init(struct asus_wmi *asus)
 {
+	struct device *dev = &asus->platform_device->dev;
 	struct device *hwmon;
 
-	hwmon = hwmon_device_register_with_groups(&asus->platform_device->dev,
-						  "asus", asus,
-						  hwmon_attribute_groups);
+	hwmon = devm_hwmon_device_register_with_groups(dev, "asus", asus,
+			hwmon_attribute_groups);
+
 	if (IS_ERR(hwmon)) {
 		pr_err("Could not register asus hwmon device\n");
 		return PTR_ERR(hwmon);
-- 
2.17.1


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

* [PATCH v4 02/13] platform/x86: asus-wmi: Fix preserving keyboard backlight intensity on load
  2019-05-14 18:47 [PATCH v4 00/13] Support of ASUS TUF Gaming series laptops Yurii Pavlovskyi
  2019-05-14 18:50 ` [PATCH v4 01/13] platform/x86: asus-wmi: Fix hwmon device cleanup Yurii Pavlovskyi
@ 2019-05-14 18:51 ` Yurii Pavlovskyi
  2019-05-24 19:57   ` Daniel Drake
  2019-05-14 18:54 ` [PATCH v4 03/13] platform/x86: asus-wmi: Increase input buffer size of WMI methods Yurii Pavlovskyi
                   ` (11 subsequent siblings)
  13 siblings, 1 reply; 19+ messages in thread
From: Yurii Pavlovskyi @ 2019-05-14 18:51 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	Chris Chiu, acpi4asus-user, platform-driver-x86, linux-kernel

The error code and return value are mixed up. The intensity is always set
to 0 on load as kbd_led_read returns either 0 or negative value. To
reproduce set backlight to maximum, reload driver and try to increase it
using keyboard hotkey, the intensity will drop as a result. Correct the
implementation.

Signed-off-by: Yurii Pavlovskyi <yurii.pavlovskyi@gmail.com>
---
 drivers/platform/x86/asus-wmi.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 62567766bdfb..84d7fc6f941c 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -595,8 +595,7 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
 			goto error;
 	}
 
-	led_val = kbd_led_read(asus, NULL, NULL);
-	if (led_val >= 0) {
+	if (!kbd_led_read(asus, &led_val, NULL)) {
 		asus->kbd_led_wk = led_val;
 		asus->kbd_led.name = "asus::kbd_backlight";
 		asus->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
-- 
2.17.1


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

* [PATCH v4 03/13] platform/x86: asus-wmi: Increase input buffer size of WMI methods
  2019-05-14 18:47 [PATCH v4 00/13] Support of ASUS TUF Gaming series laptops Yurii Pavlovskyi
  2019-05-14 18:50 ` [PATCH v4 01/13] platform/x86: asus-wmi: Fix hwmon device cleanup Yurii Pavlovskyi
  2019-05-14 18:51 ` [PATCH v4 02/13] platform/x86: asus-wmi: Fix preserving keyboard backlight intensity on load Yurii Pavlovskyi
@ 2019-05-14 18:54 ` Yurii Pavlovskyi
  2019-05-24 20:01   ` Daniel Drake
  2019-05-14 18:59 ` [PATCH v4 04/13] platform/x86: wmi: Add function to get _UID of WMI device Yurii Pavlovskyi
                   ` (10 subsequent siblings)
  13 siblings, 1 reply; 19+ messages in thread
From: Yurii Pavlovskyi @ 2019-05-14 18:54 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	Chris Chiu, acpi4asus-user, platform-driver-x86, linux-kernel

The asus-nb-wmi driver is matched by WMI alias but fails to load on TUF
Gaming series laptops producing multiple ACPI errors in the kernel log.

The input buffer for WMI method invocation size is 2 dwords, whereas
3 are expected by this model.

FX505GM:
..
Method (WMNB, 3, Serialized)
{
    P8XH (Zero, 0x11)
    CreateDWordField (Arg2, Zero, IIA0)
    CreateDWordField (Arg2, 0x04, IIA1)
    CreateDWordField (Arg2, 0x08, IIA2)
    Local0 = (Arg1 & 0xFFFFFFFF)
    ...

Compare with older K54C:
...
Method (WMNB, 3, NotSerialized)
{
    CreateDWordField (Arg2, 0x00, IIA0)
    CreateDWordField (Arg2, 0x04, IIA1)
    Local0 = (Arg1 & 0xFFFFFFFF)
    ...

Increase buffer size to 3 dwords. No negative consequences of this change
are expected, as the input buffer size is not verified. The original
function is replaced by a wrapper for a new method passing value 0 for the
last parameter. The new function will be used to control RGB keyboard
backlight.

Signed-off-by: Yurii Pavlovskyi <yurii.pavlovskyi@gmail.com>
---
One of current kernel errors:
ACPI BIOS Error (bug): AE_AML_BUFFER_LIMIT, Field [IIA2] at bit offset/
	length 64/32 exceeds size of target Buffer (64 bits)
	(20190215/dsopcode-203)
[ 4528.573948] No Local Variables are initialized for Method [WMNB]
[ 4528.573949] Initialized Arguments for Method [WMNB]:  (3 arguments
	defined for method invocation)
[ 4528.573950]   Arg0:   00000000bd1bea5a <Obj>
	Integer 0000000000000000
[ 4528.573952]   Arg1:   00000000d414dc53 <Obj>
	Integer 000000004E464741
[ 4528.573954]   Arg2:   00000000fcefea4b <Obj>
	Buffer(8) F0 95 08 00 00 00 00 00
[ 4528.573959] ACPI Error: Aborting method \_SB.ATKD.WMNB due to previous
	error (AE_AML_BUFFER_LIMIT) (20190215/psparse-531)
[ 4528.686425] asus-nb-wmi: probe of asus-nb-wmi failed with error -5
---
 drivers/platform/x86/asus-wmi.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 84d7fc6f941c..6b35c1f00a3e 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -98,6 +98,7 @@ static bool ashs_present(void)
 struct bios_args {
 	u32 arg0;
 	u32 arg1;
+	u32 arg2; /* At least TUF Gaming series uses 3 dword input buffer. */
 } __packed;
 
 /*
@@ -224,11 +225,13 @@ static void asus_wmi_input_exit(struct asus_wmi *asus)
 	asus->inputdev = NULL;
 }
 
-int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval)
+static int asus_wmi_evaluate_method3(u32 method_id,
+		u32 arg0, u32 arg1, u32 arg2, u32 *retval)
 {
 	struct bios_args args = {
 		.arg0 = arg0,
 		.arg1 = arg1,
+		.arg2 = arg2,
 	};
 	struct acpi_buffer input = { (acpi_size) sizeof(args), &args };
 	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
@@ -260,6 +263,11 @@ int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval)
 
 	return 0;
 }
+
+int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval)
+{
+	return asus_wmi_evaluate_method3(method_id, arg0, arg1, 0, retval);
+}
 EXPORT_SYMBOL_GPL(asus_wmi_evaluate_method);
 
 static int asus_wmi_evaluate_method_agfn(const struct acpi_buffer args)
-- 
2.17.1


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

* [PATCH v4 04/13] platform/x86: wmi: Add function to get _UID of WMI device
  2019-05-14 18:47 [PATCH v4 00/13] Support of ASUS TUF Gaming series laptops Yurii Pavlovskyi
                   ` (2 preceding siblings ...)
  2019-05-14 18:54 ` [PATCH v4 03/13] platform/x86: asus-wmi: Increase input buffer size of WMI methods Yurii Pavlovskyi
@ 2019-05-14 18:59 ` Yurii Pavlovskyi
  2019-05-24 21:10   ` Daniel Drake
  2019-05-14 19:00 ` [PATCH v4 05/13] platform/x86: asus-wmi: Improve DSTS WMI method ID detection Yurii Pavlovskyi
                   ` (9 subsequent siblings)
  13 siblings, 1 reply; 19+ messages in thread
From: Yurii Pavlovskyi @ 2019-05-14 18:59 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	Chris Chiu, acpi4asus-user, platform-driver-x86, linux-kernel,
	Rafael J. Wysocki, Len Brown, linux-acpi

Add a new function to acpi.h / wmi.c that returns _UID of the ACPI WMI
device. For example, it returns "ATK" for the following declaration in
DSDT:
Device (ATKD)
{
    Name (_HID, "PNP0C14" /* Windows Management Instrumentation Device */)
      // _HID: Hardware ID
    Name (_UID, "ATK")  // _UID: Unique ID
    ..

Generally, it is possible that multiple PNP0C14 ACPI devices are present in
the system as mentioned in the commit message of commit bff431e49ff5
("ACPI: WMI: Add ACPI-WMI mapping driver").

Therefore the _UID is returned for a specific ACPI device that declares the
given GUID, to which it is also mapped by other methods of wmi module.

Signed-off-by: Yurii Pavlovskyi <yurii.pavlovskyi@gmail.com>
---
 drivers/platform/x86/wmi.c | 19 +++++++++++++++++++
 include/linux/acpi.h       |  1 +
 2 files changed, 20 insertions(+)

diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index 7b26b6ccf1a0..b08ffb769cbe 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -635,6 +635,25 @@ bool wmi_has_guid(const char *guid_string)
 }
 EXPORT_SYMBOL_GPL(wmi_has_guid);
 
+/**
+ * wmi_get_acpi_device_uid() - Get _UID name of ACPI device that defines GUID
+ * @guid_string: 36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba
+ *
+ * Find the _UID of ACPI device associated with this WMI GUID.
+ *
+ * Return: The ACPI _UID field value or NULL if the WMI GUID was not found
+ */
+char *wmi_get_acpi_device_uid(const char *guid_string)
+{
+	struct wmi_block *wblock = NULL;
+
+	if (!find_guid(guid_string, &wblock))
+		return NULL;
+
+	return acpi_device_uid(wblock->acpi_device);
+}
+EXPORT_SYMBOL_GPL(wmi_get_acpi_device_uid);
+
 static struct wmi_block *dev_to_wblock(struct device *dev)
 {
 	return container_of(dev, struct wmi_block, dev.dev);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index d5dcebd7aad3..d31c7fd66f97 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -376,6 +376,7 @@ extern acpi_status wmi_install_notify_handler(const char *guid,
 extern acpi_status wmi_remove_notify_handler(const char *guid);
 extern acpi_status wmi_get_event_data(u32 event, struct acpi_buffer *out);
 extern bool wmi_has_guid(const char *guid);
+extern char *wmi_get_acpi_device_uid(const char *guid);
 
 #endif	/* CONFIG_ACPI_WMI */
 
-- 
2.17.1


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

* [PATCH v4 05/13] platform/x86: asus-wmi: Improve DSTS WMI method ID detection
  2019-05-14 18:47 [PATCH v4 00/13] Support of ASUS TUF Gaming series laptops Yurii Pavlovskyi
                   ` (3 preceding siblings ...)
  2019-05-14 18:59 ` [PATCH v4 04/13] platform/x86: wmi: Add function to get _UID of WMI device Yurii Pavlovskyi
@ 2019-05-14 19:00 ` Yurii Pavlovskyi
  2019-05-14 19:01 ` [PATCH v4 06/13] platform/x86: asus-wmi: Refactor WMI event handling Yurii Pavlovskyi
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Yurii Pavlovskyi @ 2019-05-14 19:00 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	Chris Chiu, acpi4asus-user, platform-driver-x86, linux-kernel,
	Jiri Kosina, Benjamin Tissoires, linux-input

The DSTS method detection mistakenly selects DCTS instead of DSTS if
nothing is returned when the method ID is not defined in WMNB. As a result,
the control of keyboard backlight is not functional for TUF Gaming series
laptops. Implement detection based on _UID of the WMI device instead.

There is evidence that DCTS is handled by ACPI WMI devices that have _UID
ASUSWMI, whereas none of the devices without ASUSWMI respond to DCTS and
DSTS is used instead [1].

DSDT examples:

FX505GM (_UID ATK):
Method (WMNB, 3, Serialized)
{ ...
    If ((Local0 == 0x53545344))
    {
        ...
        Return (Zero)
    }
    ...
    // No return
}

K54C (_UID ATK):
Method (WMNB, 3, Serialized)
{ ...
    If ((Local0 == 0x53545344))
    {
        ...
        Return (0x02)
    }
    ...
    Return (0xFFFFFFFE)
}

[1] Link: https://lkml.org/lkml/2019/4/11/322

Signed-off-by: Yurii Pavlovskyi <yurii.pavlovskyi@gmail.com>
Suggested-by: Daniel Drake <drake@endlessm.com>

---
Depends on the previous patch in the series that adds method to
wmi module.
---
 drivers/hid/hid-asus.c                     |  2 +-
 drivers/platform/x86/asus-wmi.c            | 23 +++++++++++++++++++---
 include/linux/platform_data/x86/asus-wmi.h |  4 ++--
 3 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 336aeaed1159..1d01fe23ca0c 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -396,7 +396,7 @@ static bool asus_kbd_wmi_led_control_present(struct hid_device *hdev)
 	if (!IS_ENABLED(CONFIG_ASUS_WMI))
 		return false;
 
-	ret = asus_wmi_evaluate_method(ASUS_WMI_METHODID_DSTS2,
+	ret = asus_wmi_evaluate_method(ASUS_WMI_METHODID_DSTS,
 				       ASUS_WMI_DEVID_KBD_BACKLIGHT, 0, &value);
 	hid_dbg(hdev, "WMI backlight check: rc %d value %x", ret, value);
 	if (ret)
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 6b35c1f00a3e..5bdb4ffdbee3 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -83,6 +83,8 @@ MODULE_LICENSE("GPL");
 #define USB_INTEL_XUSB2PR		0xD0
 #define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI	0x9c31
 
+#define ASUS_ACPI_UID_ASUSWMI		"ASUSWMI"
+
 static const char * const ashs_ids[] = { "ATK4001", "ATK4002", NULL };
 
 static bool ashs_present(void)
@@ -1874,6 +1876,8 @@ static int asus_wmi_sysfs_init(struct platform_device *device)
  */
 static int asus_wmi_platform_init(struct asus_wmi *asus)
 {
+	struct device *dev = &asus->platform_device->dev;
+	char *wmi_uid;
 	int rv;
 
 	/* INIT enable hotkeys on some models */
@@ -1903,11 +1907,24 @@ static int asus_wmi_platform_init(struct asus_wmi *asus)
 	 * Note, on most Eeepc, there is no way to check if a method exist
 	 * or note, while on notebooks, they returns 0xFFFFFFFE on failure,
 	 * but once again, SPEC may probably be used for that kind of things.
+	 *
+	 * Additionally at least TUF Gaming series laptops return nothing for
+	 * unknown methods, so the detection in this way is not possible.
+	 *
+	 * There is strong indication that only ACPI WMI devices that have _UID
+	 * equal to "ASUSWMI" use DCTS whereas those with "ATK" use DSTS.
 	 */
-	if (!asus_wmi_evaluate_method(ASUS_WMI_METHODID_DSTS, 0, 0, NULL))
+	wmi_uid = wmi_get_acpi_device_uid(ASUS_WMI_MGMT_GUID);
+	if (!wmi_uid)
+		return -ENODEV;
+
+	if (!strcmp(wmi_uid, ASUS_ACPI_UID_ASUSWMI)) {
+		dev_info(dev, "Detected ASUSWMI, use DCTS\n");
+		asus->dsts_id = ASUS_WMI_METHODID_DCTS;
+	} else {
+		dev_info(dev, "Detected %s, not ASUSWMI, use DSTS\n", wmi_uid);
 		asus->dsts_id = ASUS_WMI_METHODID_DSTS;
-	else
-		asus->dsts_id = ASUS_WMI_METHODID_DSTS2;
+	}
 
 	/* CWAP allow to define the behavior of the Fn+F2 key,
 	 * this method doesn't seems to be present on Eee PCs */
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index bfba245636a7..0668f76df921 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -18,8 +18,8 @@
 #define ASUS_WMI_METHODID_GDSP		0x50534447 /* Get DiSPlay output */
 #define ASUS_WMI_METHODID_DEVP		0x50564544 /* DEVice Policy */
 #define ASUS_WMI_METHODID_OSVR		0x5256534F /* OS VeRsion */
-#define ASUS_WMI_METHODID_DSTS		0x53544344 /* Device STatuS */
-#define ASUS_WMI_METHODID_DSTS2		0x53545344 /* Device STatuS #2*/
+#define ASUS_WMI_METHODID_DCTS		0x53544344 /* Device status (DCTS) */
+#define ASUS_WMI_METHODID_DSTS		0x53545344 /* Device status (DSTS) */
 #define ASUS_WMI_METHODID_BSTS		0x53545342 /* Bios STatuS ? */
 #define ASUS_WMI_METHODID_DEVS		0x53564544 /* DEVice Set */
 #define ASUS_WMI_METHODID_CFVS		0x53564643 /* CPU Frequency Volt Set */
-- 
2.17.1


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

* [PATCH v4 06/13] platform/x86: asus-wmi: Refactor WMI event handling
  2019-05-14 18:47 [PATCH v4 00/13] Support of ASUS TUF Gaming series laptops Yurii Pavlovskyi
                   ` (4 preceding siblings ...)
  2019-05-14 19:00 ` [PATCH v4 05/13] platform/x86: asus-wmi: Improve DSTS WMI method ID detection Yurii Pavlovskyi
@ 2019-05-14 19:01 ` Yurii Pavlovskyi
  2019-05-14 19:02 ` [PATCH v4 07/13] platform/x86: asus-wmi: Support WMI event queue Yurii Pavlovskyi
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Yurii Pavlovskyi @ 2019-05-14 19:01 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	Chris Chiu, acpi4asus-user, platform-driver-x86, linux-kernel

Refactor WMI event handling into separate functions for getting the event
code and handling the retrieved event code as a preparation for
introduction of WMI event queue support.

Signed-off-by: Yurii Pavlovskyi <yurii.pavlovskyi@gmail.com>
---
 drivers/platform/x86/asus-wmi.c | 66 +++++++++++++++++++++------------
 1 file changed, 42 insertions(+), 24 deletions(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 5bdb4ffdbee3..ed7c7857012e 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -85,6 +85,8 @@ MODULE_LICENSE("GPL");
 
 #define ASUS_ACPI_UID_ASUSWMI		"ASUSWMI"
 
+#define WMI_EVENT_MASK			0xFFFF
+
 static const char * const ashs_ids[] = { "ATK4001", "ATK4002", NULL };
 
 static bool ashs_present(void)
@@ -1651,83 +1653,99 @@ static void asus_wmi_fnlock_update(struct asus_wmi *asus)
 	asus_wmi_set_devstate(ASUS_WMI_DEVID_FNLOCK, mode, NULL);
 }
 
-static void asus_wmi_notify(u32 value, void *context)
+static int asus_wmi_get_event_code(u32 value)
 {
-	struct asus_wmi *asus = context;
 	struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL };
 	union acpi_object *obj;
 	acpi_status status;
 	int code;
-	int orig_code;
-	unsigned int key_value = 1;
-	bool autorelease = 1;
 
 	status = wmi_get_event_data(value, &response);
-	if (status != AE_OK) {
-		pr_err("bad event status 0x%x\n", status);
-		return;
+	if (ACPI_FAILURE(status)) {
+		pr_warn("Failed to get WMI notify code: %s\n",
+				acpi_format_exception(status));
+		return -EIO;
 	}
 
 	obj = (union acpi_object *)response.pointer;
 
-	if (!obj || obj->type != ACPI_TYPE_INTEGER)
-		goto exit;
+	if (obj && obj->type == ACPI_TYPE_INTEGER)
+		code = (int)(obj->integer.value & WMI_EVENT_MASK);
+	else
+		code = -EIO;
+
+	kfree(obj);
+	return code;
+}
+
+static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
+{
+	int orig_code;
+	unsigned int key_value = 1;
+	bool autorelease = 1;
 
-	code = obj->integer.value;
 	orig_code = code;
 
 	if (asus->driver->key_filter) {
 		asus->driver->key_filter(asus->driver, &code, &key_value,
 					 &autorelease);
 		if (code == ASUS_WMI_KEY_IGNORE)
-			goto exit;
+			return;
 	}
 
 	if (code >= NOTIFY_BRNUP_MIN && code <= NOTIFY_BRNUP_MAX)
 		code = ASUS_WMI_BRN_UP;
-	else if (code >= NOTIFY_BRNDOWN_MIN &&
-		 code <= NOTIFY_BRNDOWN_MAX)
+	else if (code >= NOTIFY_BRNDOWN_MIN && code <= NOTIFY_BRNDOWN_MAX)
 		code = ASUS_WMI_BRN_DOWN;
 
 	if (code == ASUS_WMI_BRN_DOWN || code == ASUS_WMI_BRN_UP) {
 		if (acpi_video_get_backlight_type() == acpi_backlight_vendor) {
 			asus_wmi_backlight_notify(asus, orig_code);
-			goto exit;
+			return;
 		}
 	}
 
 	if (code == NOTIFY_KBD_BRTUP) {
 		kbd_led_set_by_kbd(asus, asus->kbd_led_wk + 1);
-		goto exit;
+		return;
 	}
 	if (code == NOTIFY_KBD_BRTDWN) {
 		kbd_led_set_by_kbd(asus, asus->kbd_led_wk - 1);
-		goto exit;
+		return;
 	}
 	if (code == NOTIFY_KBD_BRTTOGGLE) {
 		if (asus->kbd_led_wk == asus->kbd_led.max_brightness)
 			kbd_led_set_by_kbd(asus, 0);
 		else
 			kbd_led_set_by_kbd(asus, asus->kbd_led_wk + 1);
-		goto exit;
+		return;
 	}
 
 	if (code == NOTIFY_FNLOCK_TOGGLE) {
 		asus->fnlock_locked = !asus->fnlock_locked;
 		asus_wmi_fnlock_update(asus);
-		goto exit;
+		return;
 	}
 
-	if (is_display_toggle(code) &&
-	    asus->driver->quirks->no_display_toggle)
-		goto exit;
+	if (is_display_toggle(code) && asus->driver->quirks->no_display_toggle)
+		return;
 
 	if (!sparse_keymap_report_event(asus->inputdev, code,
 					key_value, autorelease))
 		pr_info("Unknown key %x pressed\n", code);
+}
 
-exit:
-	kfree(obj);
+static void asus_wmi_notify(u32 value, void *context)
+{
+	struct asus_wmi *asus = context;
+	int code = asus_wmi_get_event_code(value);
+
+	if (code < 0) {
+		pr_warn("Failed to get notify code: %d\n", code);
+		return;
+	}
+
+	asus_wmi_handle_event_code(code, asus);
 }
 
 /*
-- 
2.17.1


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

* [PATCH v4 07/13] platform/x86: asus-wmi: Support WMI event queue
  2019-05-14 18:47 [PATCH v4 00/13] Support of ASUS TUF Gaming series laptops Yurii Pavlovskyi
                   ` (5 preceding siblings ...)
  2019-05-14 19:01 ` [PATCH v4 06/13] platform/x86: asus-wmi: Refactor WMI event handling Yurii Pavlovskyi
@ 2019-05-14 19:02 ` Yurii Pavlovskyi
  2019-05-14 19:02 ` [PATCH v4 08/13] platform/x86: asus-nb-wmi: Add microphone mute key code Yurii Pavlovskyi
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Yurii Pavlovskyi @ 2019-05-14 19:02 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	Chris Chiu, acpi4asus-user, platform-driver-x86, linux-kernel

Event codes are expected to be retrieved from a queue on at least some
models. Specifically, very likely the ACPI WMI devices with _UID ATK are
queued whereas those with ASUSWMI are not [1].

The WMI event codes are pushed into a circular buffer queue. After the INIT
method is called, ACPI code is allowed to push events into this buffer.
The INIT method cannot be reverted. If the module is unloaded and an event
(such as hotkey press) gets emitted before inserting it back the events get
processed delayed by one or if the queue overflows, additionally delayed by
about 3 seconds.

It might be considered a minor issue and no normal user would likely
observe this (there is little reason unloading the driver), but it does
significantly frustrate a developer who is unlucky enough to encounter
this. Therefore, the fallback to unqueued behavior occurs whenever
something unexpected happens.

The fix flushes the old key codes out of the queue on load. After receiving
event the queue is read until either ..FFFF or 1 is encountered. Also as
noted in [1] it is checked whether notify code is equal to 0xFF before
enabling queue processing in WMI notify handler.

DSDT examples:

FX505GM
Device (ATKD)
{ ..
    Name (ATKQ, Package (0x10)
    {
        0xFFFFFFFF, ..
    }

    Method (IANQ, 1, Serialized)
    {
        If ((AQNO >= 0x10))
        {
            Local0 = 0x64
            While ((Local0 && (AQNO >= 0x10)))
            {
                Local0--
                Sleep (0x0A)
            }
            ...
        ..
        AQTI++
        AQTI &= 0x0F
        ATKQ [AQTI] = Arg0
        ...
    }

    Method (GANQ, 0, Serialized)
    {
        ..
        If (AQNO)
        {
            ...
            Local0 = DerefOf (ATKQ [AQHI])
            AQHI++
            AQHI &= 0x0F
            Return (Local0)
        }

        Return (One)
    }

This code is almost identical to K54C, which does return Ones on empty
queue.

K54C:
Method (GANQ, 0, Serialized)
{
    If (AQNO)
    {
        ...
        Return (Local0)
    }

    Return (Ones)
}

[1] Link: https://lkml.org/lkml/2019/4/12/104

Signed-off-by: Yurii Pavlovskyi <yurii.pavlovskyi@gmail.com>
Suggested-by: Daniel Drake <drake@endlessm.com>
---
 drivers/platform/x86/asus-wmi.c | 73 ++++++++++++++++++++++++++++++---
 1 file changed, 68 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index ed7c7857012e..7bfac06abae4 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -84,6 +84,13 @@ MODULE_LICENSE("GPL");
 #define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI	0x9c31
 
 #define ASUS_ACPI_UID_ASUSWMI		"ASUSWMI"
+#define ASUS_ACPI_UID_ATK		"ATK"
+
+#define WMI_EVENT_QUEUE_SIZE		0x10
+#define WMI_EVENT_QUEUE_END		0x1
+#define WMI_EVENT_MASK			0xFFFF
+/* The WMI hotkey event value is always the same. */
+#define WMI_EVENT_VALUE_ATK		0xFF
 
 #define WMI_EVENT_MASK			0xFFFF
 
@@ -150,6 +157,7 @@ struct asus_wmi {
 	int dsts_id;
 	int spec;
 	int sfun;
+	bool wmi_event_queue;
 
 	struct input_dev *inputdev;
 	struct backlight_device *backlight_device;
@@ -1738,14 +1746,52 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
 static void asus_wmi_notify(u32 value, void *context)
 {
 	struct asus_wmi *asus = context;
-	int code = asus_wmi_get_event_code(value);
+	int code;
+	int i;
 
-	if (code < 0) {
-		pr_warn("Failed to get notify code: %d\n", code);
-		return;
+	for (i = 0; i < WMI_EVENT_QUEUE_SIZE + 1; i++) {
+		code = asus_wmi_get_event_code(value);
+
+		if (code < 0) {
+			pr_warn("Failed to get notify code: %d\n", code);
+			return;
+		}
+
+		if (code == WMI_EVENT_QUEUE_END || code == WMI_EVENT_MASK)
+			return;
+
+		asus_wmi_handle_event_code(code, asus);
+
+		/*
+		 * Double check that queue is present:
+		 * ATK (with queue) uses 0xff, ASUSWMI (without) 0xd2.
+		 */
+		if (!asus->wmi_event_queue || value != WMI_EVENT_VALUE_ATK)
+			return;
 	}
 
-	asus_wmi_handle_event_code(code, asus);
+	pr_warn("Failed to process event queue, last code: 0x%x\n", code);
+}
+
+static int asus_wmi_notify_queue_flush(struct asus_wmi *asus)
+{
+	int code;
+	int i;
+
+	for (i = 0; i < WMI_EVENT_QUEUE_SIZE + 1; i++) {
+		code = asus_wmi_get_event_code(WMI_EVENT_VALUE_ATK);
+
+		if (code < 0) {
+			pr_warn("Failed to get event during flush: %d\n", code);
+			return code;
+		}
+
+		if (code == WMI_EVENT_QUEUE_END || code == WMI_EVENT_MASK)
+			return 0;
+	}
+
+	pr_warn("Failed to flush event queue\n");
+	return -EIO;
 }
 
 /*
@@ -1944,6 +1990,23 @@ static int asus_wmi_platform_init(struct asus_wmi *asus)
 		asus->dsts_id = ASUS_WMI_METHODID_DSTS;
 	}
 
+	/*
+	 * Some devices can have multiple event codes stored in a queue before
+	 * the module load if it was unloaded intermittently after calling
+	 * the INIT method (enables event handling). The WMI notify handler is
+	 * expected to retrieve all event codes until a retrieved code equals
+	 * queue end marker (One or Ones). Old codes are flushed from the queue
+	 * upon module load. Not enabling this when it should be has minimal
+	 * visible impact so fall back if anything goes wrong.
+	 */
+	wmi_uid = wmi_get_acpi_device_uid(asus->driver->event_guid);
+	if (wmi_uid && !strcmp(wmi_uid, ASUS_ACPI_UID_ATK)) {
+		dev_info(dev, "Detected ATK, enable event queue\n");
+
+		if (!asus_wmi_notify_queue_flush(asus))
+			asus->wmi_event_queue = true;
+	}
+
 	/* CWAP allow to define the behavior of the Fn+F2 key,
 	 * this method doesn't seems to be present on Eee PCs */
 	if (asus->driver->quirks->wapf >= 0)
-- 
2.17.1


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

* [PATCH v4 08/13] platform/x86: asus-nb-wmi: Add microphone mute key code
  2019-05-14 18:47 [PATCH v4 00/13] Support of ASUS TUF Gaming series laptops Yurii Pavlovskyi
                   ` (6 preceding siblings ...)
  2019-05-14 19:02 ` [PATCH v4 07/13] platform/x86: asus-wmi: Support WMI event queue Yurii Pavlovskyi
@ 2019-05-14 19:02 ` Yurii Pavlovskyi
  2019-05-14 19:03 ` [PATCH v4 09/13] platform/x86: asus-wmi: Refactor error handling Yurii Pavlovskyi
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Yurii Pavlovskyi @ 2019-05-14 19:02 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	Chris Chiu, acpi4asus-user, platform-driver-x86, linux-kernel

The microphone mute key is missing from sparse keymap. It is present on
FX505GM and possibly other laptops. Add the missing code.

Also, comment on the fan mode switch key, which has the same code as the
already used key.

Signed-off-by: Yurii Pavlovskyi <yurii.pavlovskyi@gmail.com>
---
 drivers/platform/x86/asus-nb-wmi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
index b6f2ff95c3ed..d2399ce0b3cd 100644
--- a/drivers/platform/x86/asus-nb-wmi.c
+++ b/drivers/platform/x86/asus-nb-wmi.c
@@ -468,6 +468,7 @@ static const struct key_entry asus_nb_wmi_keymap[] = {
 	{ KE_KEY, 0x6B, { KEY_TOUCHPAD_TOGGLE } },
 	{ KE_IGNORE, 0x6E, },  /* Low Battery notification */
 	{ KE_KEY, 0x7a, { KEY_ALS_TOGGLE } }, /* Ambient Light Sensor Toggle */
+	{ KE_KEY, 0x7c, { KEY_MICMUTE } },
 	{ KE_KEY, 0x7D, { KEY_BLUETOOTH } }, /* Bluetooth Enable */
 	{ KE_KEY, 0x7E, { KEY_BLUETOOTH } }, /* Bluetooth Disable */
 	{ KE_KEY, 0x82, { KEY_CAMERA } },
@@ -482,7 +483,7 @@ static const struct key_entry asus_nb_wmi_keymap[] = {
 	{ KE_KEY, 0x92, { KEY_SWITCHVIDEOMODE } }, /* SDSP CRT + TV + DVI */
 	{ KE_KEY, 0x93, { KEY_SWITCHVIDEOMODE } }, /* SDSP LCD + CRT + TV + DVI */
 	{ KE_KEY, 0x95, { KEY_MEDIA } },
-	{ KE_KEY, 0x99, { KEY_PHONE } },
+	{ KE_KEY, 0x99, { KEY_PHONE } }, /* Conflicts with fan mode switch */
 	{ KE_KEY, 0xA0, { KEY_SWITCHVIDEOMODE } }, /* SDSP HDMI only */
 	{ KE_KEY, 0xA1, { KEY_SWITCHVIDEOMODE } }, /* SDSP LCD + HDMI */
 	{ KE_KEY, 0xA2, { KEY_SWITCHVIDEOMODE } }, /* SDSP CRT + HDMI */
-- 
2.17.1


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

* [PATCH v4 09/13] platform/x86: asus-wmi: Refactor error handling
  2019-05-14 18:47 [PATCH v4 00/13] Support of ASUS TUF Gaming series laptops Yurii Pavlovskyi
                   ` (7 preceding siblings ...)
  2019-05-14 19:02 ` [PATCH v4 08/13] platform/x86: asus-nb-wmi: Add microphone mute key code Yurii Pavlovskyi
@ 2019-05-14 19:03 ` Yurii Pavlovskyi
  2019-05-14 19:04 ` [PATCH v4 10/13] platform/x86: asus-wmi: Organize code into sections Yurii Pavlovskyi
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Yurii Pavlovskyi @ 2019-05-14 19:03 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	Chris Chiu, acpi4asus-user, platform-driver-x86, linux-kernel

Remove exit label as it is only used once from the point in code where no
cleanup is required and return can be called immediately.

Signed-off-by: Yurii Pavlovskyi <yurii.pavlovskyi@gmail.com>
---
 drivers/platform/x86/asus-wmi.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 7bfac06abae4..090a00af4017 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -255,7 +255,7 @@ static int asus_wmi_evaluate_method3(u32 method_id,
 				     &input, &output);
 
 	if (ACPI_FAILURE(status))
-		goto exit;
+		return -EIO;
 
 	obj = (union acpi_object *)output.pointer;
 	if (obj && obj->type == ACPI_TYPE_INTEGER)
@@ -266,10 +266,6 @@ static int asus_wmi_evaluate_method3(u32 method_id,
 
 	kfree(obj);
 
-exit:
-	if (ACPI_FAILURE(status))
-		return -EIO;
-
 	if (tmp == ASUS_WMI_UNSUPPORTED_METHOD)
 		return -ENODEV;
 
-- 
2.17.1


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

* [PATCH v4 10/13] platform/x86: asus-wmi: Organize code into sections
  2019-05-14 18:47 [PATCH v4 00/13] Support of ASUS TUF Gaming series laptops Yurii Pavlovskyi
                   ` (8 preceding siblings ...)
  2019-05-14 19:03 ` [PATCH v4 09/13] platform/x86: asus-wmi: Refactor error handling Yurii Pavlovskyi
@ 2019-05-14 19:04 ` Yurii Pavlovskyi
  2019-05-14 19:05 ` [PATCH v4 11/13] platform/x86: asus-wmi: Enhance detection of thermal data Yurii Pavlovskyi
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Yurii Pavlovskyi @ 2019-05-14 19:04 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	Chris Chiu, acpi4asus-user, platform-driver-x86, linux-kernel

The driver has grown pretty big and will grow more, which makes it hard to
navigate and understand. Add uniform comments to the code and ensure that
it is sorted into logical sections.

Signed-off-by: Yurii Pavlovskyi <yurii.pavlovskyi@gmail.com>
---
 drivers/platform/x86/asus-wmi.c | 89 +++++++++++++++++----------------
 1 file changed, 47 insertions(+), 42 deletions(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 090a00af4017..bd9eb00f3a27 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -200,6 +200,8 @@ struct asus_wmi {
 	struct asus_wmi_driver *driver;
 };
 
+/* Input **********************************************************************/
+
 static int asus_wmi_input_init(struct asus_wmi *asus)
 {
 	int err;
@@ -237,6 +239,8 @@ static void asus_wmi_input_exit(struct asus_wmi *asus)
 	asus->inputdev = NULL;
 }
 
+/* WMI ************************************************************************/
+
 static int asus_wmi_evaluate_method3(u32 method_id,
 		u32 arg0, u32 arg1, u32 arg2, u32 *retval)
 {
@@ -349,9 +353,8 @@ static int asus_wmi_get_devstate_simple(struct asus_wmi *asus, u32 dev_id)
 					  ASUS_WMI_DSTS_STATUS_BIT);
 }
 
-/*
- * LEDs
- */
+/* LEDs ***********************************************************************/
+
 /*
  * These functions actually update the LED's, and are called from a
  * workqueue. By doing this as separate work rather than when the LED
@@ -661,6 +664,7 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
 	return rv;
 }
 
+/* RF *************************************************************************/
 
 /*
  * PCI hotplug (for wlan rfkill)
@@ -1083,6 +1087,8 @@ static int asus_wmi_rfkill_init(struct asus_wmi *asus)
 	return result;
 }
 
+/* Quirks *********************************************************************/
+
 static void asus_wmi_set_xusb2pr(struct asus_wmi *asus)
 {
 	struct pci_dev *xhci_pdev;
@@ -1115,9 +1121,8 @@ static void asus_wmi_set_als(void)
 	asus_wmi_set_devstate(ASUS_WMI_DEVID_ALS_ENABLE, 1, NULL);
 }
 
-/*
- * Hwmon device
- */
+/* Hwmon device ***************************************************************/
+
 static int asus_hwmon_agfn_fan_speed_read(struct asus_wmi *asus, int fan,
 					  int *speed)
 {
@@ -1456,9 +1461,27 @@ static int asus_wmi_hwmon_init(struct asus_wmi *asus)
 	return 0;
 }
 
-/*
- * Backlight
- */
+static int asus_wmi_fan_init(struct asus_wmi *asus)
+{
+	int status;
+
+	asus->asus_hwmon_pwm = -1;
+	asus->asus_hwmon_num_fans = -1;
+	asus->asus_hwmon_fan_manual_mode = false;
+
+	status = asus_hwmon_get_fan_number(asus, &asus->asus_hwmon_num_fans);
+	if (status) {
+		asus->asus_hwmon_num_fans = 0;
+		pr_warn("Could not determine number of fans: %d\n", status);
+		return -ENXIO;
+	}
+
+	pr_info("Number of fans: %d\n", asus->asus_hwmon_num_fans);
+	return 0;
+}
+
+/* Backlight ******************************************************************/
+
 static int read_backlight_power(struct asus_wmi *asus)
 {
 	int ret;
@@ -1640,6 +1663,8 @@ static int is_display_toggle(int code)
 	return 0;
 }
 
+/* Fn-lock ********************************************************************/
+
 static bool asus_wmi_has_fnlock_key(struct asus_wmi *asus)
 {
 	u32 result;
@@ -1657,6 +1682,8 @@ static void asus_wmi_fnlock_update(struct asus_wmi *asus)
 	asus_wmi_set_devstate(ASUS_WMI_DEVID_FNLOCK, mode, NULL);
 }
 
+/* WMI events *****************************************************************/
+
 static int asus_wmi_get_event_code(u32 value)
 {
 	struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL };
@@ -1790,9 +1817,8 @@ static int asus_wmi_notify_queue_flush(struct asus_wmi *asus)
 	return -EIO;
 }
 
-/*
- * Sys helpers
- */
+/* Sysfs **********************************************************************/
+
 static int parse_arg(const char *buf, unsigned long count, int *val)
 {
 	if (!count)
@@ -1931,9 +1957,8 @@ static int asus_wmi_sysfs_init(struct platform_device *device)
 	return sysfs_create_group(&device->dev.kobj, &platform_attribute_group);
 }
 
-/*
- * Platform device
- */
+/* Platform device ************************************************************/
+
 static int asus_wmi_platform_init(struct asus_wmi *asus)
 {
 	struct device *dev = &asus->platform_device->dev;
@@ -2017,9 +2042,8 @@ static void asus_wmi_platform_exit(struct asus_wmi *asus)
 	asus_wmi_sysfs_exit(asus->platform_device);
 }
 
-/*
- * debugfs
- */
+/* debugfs ********************************************************************/
+
 struct asus_wmi_debugfs_node {
 	struct asus_wmi *asus;
 	char *name;
@@ -2166,28 +2190,8 @@ static int asus_wmi_debugfs_init(struct asus_wmi *asus)
 	return -ENOMEM;
 }
 
-static int asus_wmi_fan_init(struct asus_wmi *asus)
-{
-	int status;
-
-	asus->asus_hwmon_pwm = -1;
-	asus->asus_hwmon_num_fans = -1;
-	asus->asus_hwmon_fan_manual_mode = false;
+/* Init / exit ****************************************************************/
 
-	status = asus_hwmon_get_fan_number(asus, &asus->asus_hwmon_num_fans);
-	if (status) {
-		asus->asus_hwmon_num_fans = 0;
-		pr_warn("Could not determine number of fans: %d\n", status);
-		return -ENXIO;
-	}
-
-	pr_info("Number of fans: %d\n", asus->asus_hwmon_num_fans);
-	return 0;
-}
-
-/*
- * WMI Driver
- */
 static int asus_wmi_add(struct platform_device *pdev)
 {
 	struct platform_driver *pdrv = to_platform_driver(pdev->dev.driver);
@@ -2319,9 +2323,8 @@ static int asus_wmi_remove(struct platform_device *device)
 	return 0;
 }
 
-/*
- * Platform driver - hibernate/resume callbacks
- */
+/* Platform driver - hibernate/resume callbacks *******************************/
+
 static int asus_hotk_thaw(struct device *device)
 {
 	struct asus_wmi *asus = dev_get_drvdata(device);
@@ -2397,6 +2400,8 @@ static const struct dev_pm_ops asus_pm_ops = {
 	.resume = asus_hotk_resume,
 };
 
+/* Registration ***************************************************************/
+
 static int asus_wmi_probe(struct platform_device *pdev)
 {
 	struct platform_driver *pdrv = to_platform_driver(pdev->dev.driver);
-- 
2.17.1


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

* [PATCH v4 11/13] platform/x86: asus-wmi: Enhance detection of thermal data
  2019-05-14 18:47 [PATCH v4 00/13] Support of ASUS TUF Gaming series laptops Yurii Pavlovskyi
                   ` (9 preceding siblings ...)
  2019-05-14 19:04 ` [PATCH v4 10/13] platform/x86: asus-wmi: Organize code into sections Yurii Pavlovskyi
@ 2019-05-14 19:05 ` Yurii Pavlovskyi
  2019-05-14 19:07 ` [PATCH v4 12/13] platform/x86: asus-wmi: Switch fan boost mode Yurii Pavlovskyi
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Yurii Pavlovskyi @ 2019-05-14 19:05 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	Chris Chiu, acpi4asus-user, platform-driver-x86, linux-kernel

The obviously wrong value 1 for temperature device ID in this driver is
returned by at least some devices, including TUF Gaming series laptops,
instead of 0 as expected previously. Observable effect is that a
temp1_input in hwmon reads temperature near absolute zero.

Consider 0.1 K an erroneous value in addition to 0 K.

Signed-off-by: Yurii Pavlovskyi <yurii.pavlovskyi@gmail.com>
---
 drivers/platform/x86/asus-wmi.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index bd9eb00f3a27..ffb4e2530ea4 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -1428,8 +1428,11 @@ static umode_t asus_hwmon_sysfs_is_visible(struct kobject *kobj,
 		else
 			ok = fan_attr <= asus->asus_hwmon_num_fans;
 	} else if (dev_id == ASUS_WMI_DEVID_THERMAL_CTRL) {
-		/* If value is zero, something is clearly wrong */
-		if (!value)
+		/*
+		 * If the temperature value in deci-Kelvin is near the absolute
+		 * zero temperature, something is clearly wrong
+		 */
+		if (value == 0 || value == 1)
 			ok = false;
 	} else if (fan_attr <= asus->asus_hwmon_num_fans && fan_attr != -1) {
 		ok = true;
-- 
2.17.1


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

* [PATCH v4 12/13] platform/x86: asus-wmi: Switch fan boost mode
  2019-05-14 18:47 [PATCH v4 00/13] Support of ASUS TUF Gaming series laptops Yurii Pavlovskyi
                   ` (10 preceding siblings ...)
  2019-05-14 19:05 ` [PATCH v4 11/13] platform/x86: asus-wmi: Enhance detection of thermal data Yurii Pavlovskyi
@ 2019-05-14 19:07 ` Yurii Pavlovskyi
  2019-05-14 19:07 ` [PATCH v4 13/13] platform/x86: asus-wmi: Do not disable keyboard backlight on unloading Yurii Pavlovskyi
  2019-06-29 13:13 ` [PATCH v4 00/13] Support of ASUS TUF Gaming series laptops Andy Shevchenko
  13 siblings, 0 replies; 19+ messages in thread
From: Yurii Pavlovskyi @ 2019-05-14 19:07 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	Chris Chiu, acpi4asus-user, platform-driver-x86, linux-kernel,
	linux-api

The WMI exposes a write-only device ID where up to three fan modes can be
switched on some laptops (TUF Gaming FX505GM). There is a hotkey
combination Fn-F5 that does have a fan icon, which is designed to toggle
between fan modes. The DSTS of the device ID returns information about the
presence of this capability and the presence of each of the two additional
fan modes as a bitmask (0x01 - overboost present, 0x02 - silent present)
[1].

Add a SysFS entry that reads the last written value and updates value in
WMI on write and a hotkey handler that toggles the modes taking into
account their availability according to DSTS.

Modes:
* 0x00 - normal or balanced,
* 0x01 - overboost, increased fan RPM,
* 0x02 - silent, decreased fan RPM

[1] Link: https://lkml.org/lkml/2019/4/12/110

Signed-off-by: Yurii Pavlovskyi <yurii.pavlovskyi@gmail.com>
Suggested-by: Daniel Drake <drake@endlessm.com>
---
 .../ABI/testing/sysfs-platform-asus-wmi       |  10 ++
 drivers/platform/x86/asus-wmi.c               | 151 +++++++++++++++++-
 include/linux/platform_data/x86/asus-wmi.h    |   1 +
 3 files changed, 154 insertions(+), 8 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-platform-asus-wmi b/Documentation/ABI/testing/sysfs-platform-asus-wmi
index 019e1e29370e..87ae5cc983bf 100644
--- a/Documentation/ABI/testing/sysfs-platform-asus-wmi
+++ b/Documentation/ABI/testing/sysfs-platform-asus-wmi
@@ -36,3 +36,13 @@ KernelVersion:	3.5
 Contact:	"AceLan Kao" <acelan.kao@canonical.com>
 Description:
 		Resume on lid open. 1 means on, 0 means off.
+
+What:		/sys/devices/platform/<platform>/fan_mode
+Date:		Apr 2019
+KernelVersion:	5.2
+Contact:	"Yurii Pavlovskyi" <yurii.pavlovskyi@gmail.com>
+Description:
+		Fan boost mode:
+			* 0 - normal,
+			* 1 - overboost,
+			* 2 - silent
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index ffb4e2530ea4..feb8d72fc3c5 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -70,6 +70,7 @@ MODULE_LICENSE("GPL");
 #define NOTIFY_KBD_BRTUP		0xc4
 #define NOTIFY_KBD_BRTDWN		0xc5
 #define NOTIFY_KBD_BRTTOGGLE		0xc7
+#define NOTIFY_KBD_FBM			0x99
 
 #define ASUS_WMI_FNLOCK_BIOS_DISABLED	BIT(0)
 
@@ -80,6 +81,13 @@ MODULE_LICENSE("GPL");
 #define ASUS_FAN_CTRL_MANUAL		1
 #define ASUS_FAN_CTRL_AUTO		2
 
+#define ASUS_FAN_MODE_NORMAL		0
+#define ASUS_FAN_MODE_OVERBOOST		1
+#define ASUS_FAN_MODE_OVERBOOST_MASK	0x01
+#define ASUS_FAN_MODE_SILENT		2
+#define ASUS_FAN_MODE_SILENT_MASK	0x02
+#define ASUS_FAN_MODES_MASK		0x03
+
 #define USB_INTEL_XUSB2PR		0xD0
 #define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI	0x9c31
 
@@ -187,6 +195,10 @@ struct asus_wmi {
 	int asus_hwmon_num_fans;
 	int asus_hwmon_pwm;
 
+	bool fan_mode_available;
+	u8 fan_mode_mask;
+	u8 fan_mode;
+
 	struct hotplug_slot hotplug_slot;
 	struct mutex hotplug_lock;
 	struct mutex wmi_lock;
@@ -1483,6 +1495,116 @@ static int asus_wmi_fan_init(struct asus_wmi *asus)
 	return 0;
 }
 
+/* Fan mode *******************************************************************/
+
+static int fan_mode_check_present(struct asus_wmi *asus)
+{
+	u32 result;
+	int err;
+
+	asus->fan_mode_available = false;
+
+	err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_FAN_MODE, &result);
+	if (err) {
+		if (err == -ENODEV)
+			return 0;
+		else
+			return err;
+	}
+
+	if ((result & ASUS_WMI_DSTS_PRESENCE_BIT) &&
+			(result & ASUS_FAN_MODES_MASK)) {
+		asus->fan_mode_available = true;
+		asus->fan_mode_mask = result & ASUS_FAN_MODES_MASK;
+	}
+
+	return 0;
+}
+
+static int fan_mode_write(struct asus_wmi *asus)
+{
+	int err;
+	u8 value;
+	u32 retval;
+
+	value = asus->fan_mode;
+
+	pr_info("Set fan mode: %u\n", value);
+	err = asus_wmi_set_devstate(ASUS_WMI_DEVID_FAN_MODE, value, &retval);
+
+	if (err) {
+		pr_warn("Failed to set fan mode: %d\n", err);
+		return err;
+	}
+
+	if (retval != 1) {
+		pr_warn("Failed to set fan mode (retval): 0x%x\n", retval);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int fan_mode_switch_next(struct asus_wmi *asus)
+{
+	if (asus->fan_mode == ASUS_FAN_MODE_NORMAL) {
+		if (asus->fan_mode_mask & ASUS_FAN_MODE_OVERBOOST_MASK)
+			asus->fan_mode = ASUS_FAN_MODE_OVERBOOST;
+		else if (asus->fan_mode_mask & ASUS_FAN_MODE_SILENT_MASK)
+			asus->fan_mode = ASUS_FAN_MODE_SILENT;
+	} else if (asus->fan_mode == ASUS_FAN_MODE_OVERBOOST) {
+		if (asus->fan_mode_mask & ASUS_FAN_MODE_SILENT_MASK)
+			asus->fan_mode = ASUS_FAN_MODE_SILENT;
+		else
+			asus->fan_mode = ASUS_FAN_MODE_NORMAL;
+	} else {
+		asus->fan_mode = ASUS_FAN_MODE_NORMAL;
+	}
+
+	return fan_mode_write(asus);
+}
+
+static ssize_t fan_mode_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	return scnprintf(buf, PAGE_SIZE, "%d\n", asus->fan_mode);
+}
+
+static ssize_t fan_mode_store(struct device *dev, struct device_attribute *attr,
+		const char *buf, size_t count)
+{
+	int result;
+	u8 new_mode;
+
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	result = kstrtou8(buf, 10, &new_mode);
+	if (result < 0) {
+		pr_warn("Trying to store invalid value\n");
+		return result;
+	}
+
+	if (new_mode == ASUS_FAN_MODE_OVERBOOST) {
+		if (!(asus->fan_mode_mask & ASUS_FAN_MODE_OVERBOOST_MASK))
+			return -EINVAL;
+	} else if (new_mode == ASUS_FAN_MODE_SILENT) {
+		if (!(asus->fan_mode_mask & ASUS_FAN_MODE_SILENT_MASK))
+			return -EINVAL;
+	} else if (new_mode != ASUS_FAN_MODE_NORMAL) {
+		return -EINVAL;
+	}
+
+	asus->fan_mode = new_mode;
+	fan_mode_write(asus);
+
+	return result;
+}
+
+// Fan mode: 0 - normal, 1 - overboost, 2 - silent
+static DEVICE_ATTR_RW(fan_mode);
+
 /* Backlight ******************************************************************/
 
 static int read_backlight_power(struct asus_wmi *asus)
@@ -1761,6 +1883,11 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
 		return;
 	}
 
+	if (asus->fan_mode_available && code == NOTIFY_KBD_FBM) {
+		fan_mode_switch_next(asus);
+		return;
+	}
+
 	if (is_display_toggle(code) && asus->driver->quirks->no_display_toggle)
 		return;
 
@@ -1917,6 +2044,7 @@ static struct attribute *platform_attributes[] = {
 	&dev_attr_touchpad.attr,
 	&dev_attr_lid_resume.attr,
 	&dev_attr_als_enable.attr,
+	&dev_attr_fan_mode.attr,
 	NULL
 };
 
@@ -1938,6 +2066,8 @@ static umode_t asus_sysfs_is_visible(struct kobject *kobj,
 		devid = ASUS_WMI_DEVID_LID_RESUME;
 	else if (attr == &dev_attr_als_enable.attr)
 		devid = ASUS_WMI_DEVID_ALS_ENABLE;
+	else if (attr == &dev_attr_fan_mode.attr)
+		ok = asus->fan_mode_available;
 
 	if (devid != -1)
 		ok = !(asus_wmi_get_devstate_simple(asus, devid) < 0);
@@ -2037,12 +2167,7 @@ static int asus_wmi_platform_init(struct asus_wmi *asus)
 		asus_wmi_set_devstate(ASUS_WMI_DEVID_CWAP,
 				      asus->driver->quirks->wapf, NULL);
 
-	return asus_wmi_sysfs_init(asus->platform_device);
-}
-
-static void asus_wmi_platform_exit(struct asus_wmi *asus)
-{
-	asus_wmi_sysfs_exit(asus->platform_device);
+	return 0;
 }
 
 /* debugfs ********************************************************************/
@@ -2221,6 +2346,14 @@ static int asus_wmi_add(struct platform_device *pdev)
 	if (err)
 		goto fail_platform;
 
+	err = fan_mode_check_present(asus);
+	if (err)
+		goto fail_fan_mode;
+
+	err = asus_wmi_sysfs_init(asus->platform_device);
+	if (err)
+		goto fail_sysfs;
+
 	err = asus_wmi_input_init(asus);
 	if (err)
 		goto fail_input;
@@ -2302,7 +2435,9 @@ static int asus_wmi_add(struct platform_device *pdev)
 fail_hwmon:
 	asus_wmi_input_exit(asus);
 fail_input:
-	asus_wmi_platform_exit(asus);
+	asus_wmi_sysfs_exit(asus->platform_device);
+fail_sysfs:
+fail_fan_mode:
 fail_platform:
 	kfree(asus);
 	return err;
@@ -2319,7 +2454,7 @@ static int asus_wmi_remove(struct platform_device *device)
 	asus_wmi_led_exit(asus);
 	asus_wmi_rfkill_exit(asus);
 	asus_wmi_debugfs_exit(asus);
-	asus_wmi_platform_exit(asus);
+	asus_wmi_sysfs_exit(asus->platform_device);
 	asus_hwmon_fan_set_auto(asus);
 
 	kfree(asus);
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index 0668f76df921..8551156b8dca 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -57,6 +57,7 @@
 #define ASUS_WMI_DEVID_KBD_BACKLIGHT	0x00050021
 #define ASUS_WMI_DEVID_LIGHT_SENSOR	0x00050022 /* ?? */
 #define ASUS_WMI_DEVID_LIGHTBAR		0x00050025
+#define ASUS_WMI_DEVID_FAN_MODE		0x00110018
 
 /* Misc */
 #define ASUS_WMI_DEVID_CAMERA		0x00060013
-- 
2.17.1


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

* [PATCH v4 13/13] platform/x86: asus-wmi: Do not disable keyboard backlight on unloading
  2019-05-14 18:47 [PATCH v4 00/13] Support of ASUS TUF Gaming series laptops Yurii Pavlovskyi
                   ` (11 preceding siblings ...)
  2019-05-14 19:07 ` [PATCH v4 12/13] platform/x86: asus-wmi: Switch fan boost mode Yurii Pavlovskyi
@ 2019-05-14 19:07 ` Yurii Pavlovskyi
  2019-06-29 13:13 ` [PATCH v4 00/13] Support of ASUS TUF Gaming series laptops Andy Shevchenko
  13 siblings, 0 replies; 19+ messages in thread
From: Yurii Pavlovskyi @ 2019-05-14 19:07 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	Chris Chiu, acpi4asus-user, platform-driver-x86, linux-kernel

The keyboard backlight is automatically disabled when the module is
unloaded as it is exposed as a ledclass device. Change this behavior to
ignore setting brightness when the device is in unloading state.

Signed-off-by: Yurii Pavlovskyi <yurii.pavlovskyi@gmail.com>
---
 drivers/platform/x86/asus-wmi.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index feb8d72fc3c5..0c330d6a5871 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -471,6 +471,10 @@ static void do_kbd_led_set(struct led_classdev *led_cdev, int value)
 static void kbd_led_set(struct led_classdev *led_cdev,
 			enum led_brightness value)
 {
+	/* Prevent disabling keyboard backlight on module unregister */
+	if (led_cdev->flags & LED_UNREGISTERING)
+		return;
+
 	do_kbd_led_set(led_cdev, value);
 }
 
-- 
2.17.1


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

* Re: [PATCH v4 01/13] platform/x86: asus-wmi: Fix hwmon device cleanup
  2019-05-14 18:50 ` [PATCH v4 01/13] platform/x86: asus-wmi: Fix hwmon device cleanup Yurii Pavlovskyi
@ 2019-05-24 19:54   ` Daniel Drake
  0 siblings, 0 replies; 19+ messages in thread
From: Daniel Drake @ 2019-05-24 19:54 UTC (permalink / raw)
  To: Yurii Pavlovskyi
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Chris Chiu,
	acpi4asus-user, Platform Driver, Linux Kernel

On Tue, May 14, 2019 at 12:50 PM Yurii Pavlovskyi
<yurii.pavlovskyi@gmail.com> wrote:
>
> The driver does not clean up the hwmon device on exit or error. To
> reproduce the bug, repeat rmmod, insmod to verify that device number
> /sys/devices/platform/asus-nb-wmi/hwmon/hwmon?? grows every time. Replace
> call for registering device with devm_* version that unregisters it
> automatically.
>
> Signed-off-by: Yurii Pavlovskyi <yurii.pavlovskyi@gmail.com>

Reviewed-by: Daniel Drake <drake@endlessm.com>

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

* Re: [PATCH v4 02/13] platform/x86: asus-wmi: Fix preserving keyboard backlight intensity on load
  2019-05-14 18:51 ` [PATCH v4 02/13] platform/x86: asus-wmi: Fix preserving keyboard backlight intensity on load Yurii Pavlovskyi
@ 2019-05-24 19:57   ` Daniel Drake
  0 siblings, 0 replies; 19+ messages in thread
From: Daniel Drake @ 2019-05-24 19:57 UTC (permalink / raw)
  To: Yurii Pavlovskyi
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Chris Chiu,
	acpi4asus-user, Platform Driver, Linux Kernel

On Tue, May 14, 2019 at 12:51 PM Yurii Pavlovskyi
<yurii.pavlovskyi@gmail.com> wrote:
>
> The error code and return value are mixed up. The intensity is always set
> to 0 on load as kbd_led_read returns either 0 or negative value. To
> reproduce set backlight to maximum, reload driver and try to increase it
> using keyboard hotkey, the intensity will drop as a result. Correct the
> implementation.
>
> Signed-off-by: Yurii Pavlovskyi <yurii.pavlovskyi@gmail.com>

Reviewed-by: Daniel Drake <drake@endlessm.com>

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

* Re: [PATCH v4 03/13] platform/x86: asus-wmi: Increase input buffer size of WMI methods
  2019-05-14 18:54 ` [PATCH v4 03/13] platform/x86: asus-wmi: Increase input buffer size of WMI methods Yurii Pavlovskyi
@ 2019-05-24 20:01   ` Daniel Drake
  0 siblings, 0 replies; 19+ messages in thread
From: Daniel Drake @ 2019-05-24 20:01 UTC (permalink / raw)
  To: Yurii Pavlovskyi
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Chris Chiu,
	acpi4asus-user, Platform Driver, Linux Kernel

On Tue, May 14, 2019 at 12:54 PM Yurii Pavlovskyi
<yurii.pavlovskyi@gmail.com> wrote:
>
> The asus-nb-wmi driver is matched by WMI alias but fails to load on TUF
> Gaming series laptops producing multiple ACPI errors in the kernel log.
>
> The input buffer for WMI method invocation size is 2 dwords, whereas
> 3 are expected by this model.
>
> FX505GM:
> ..
> Method (WMNB, 3, Serialized)
> {
>     P8XH (Zero, 0x11)
>     CreateDWordField (Arg2, Zero, IIA0)
>     CreateDWordField (Arg2, 0x04, IIA1)
>     CreateDWordField (Arg2, 0x08, IIA2)
>     Local0 = (Arg1 & 0xFFFFFFFF)
>     ...
>
> Compare with older K54C:
> ...
> Method (WMNB, 3, NotSerialized)
> {
>     CreateDWordField (Arg2, 0x00, IIA0)
>     CreateDWordField (Arg2, 0x04, IIA1)
>     Local0 = (Arg1 & 0xFFFFFFFF)
>     ...
>
> Increase buffer size to 3 dwords. No negative consequences of this change
> are expected, as the input buffer size is not verified. The original
> function is replaced by a wrapper for a new method passing value 0 for the
> last parameter. The new function will be used to control RGB keyboard
> backlight.
>
> Signed-off-by: Yurii Pavlovskyi <yurii.pavlovskyi@gmail.com>

Reviewed-by: Daniel Drake <drake@endlessm.com>

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

* Re: [PATCH v4 04/13] platform/x86: wmi: Add function to get _UID of WMI device
  2019-05-14 18:59 ` [PATCH v4 04/13] platform/x86: wmi: Add function to get _UID of WMI device Yurii Pavlovskyi
@ 2019-05-24 21:10   ` Daniel Drake
  0 siblings, 0 replies; 19+ messages in thread
From: Daniel Drake @ 2019-05-24 21:10 UTC (permalink / raw)
  To: Yurii Pavlovskyi
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Chris Chiu,
	acpi4asus-user, Platform Driver, Linux Kernel, Rafael J. Wysocki,
	Len Brown, ACPI Devel Maling List

On Tue, May 14, 2019 at 12:59 PM Yurii Pavlovskyi
<yurii.pavlovskyi@gmail.com> wrote:
>
> Add a new function to acpi.h / wmi.c that returns _UID of the ACPI WMI
> device. For example, it returns "ATK" for the following declaration in
> DSDT:
> Device (ATKD)
> {
>     Name (_HID, "PNP0C14" /* Windows Management Instrumentation Device */)
>       // _HID: Hardware ID
>     Name (_UID, "ATK")  // _UID: Unique ID
>     ..
>
> Generally, it is possible that multiple PNP0C14 ACPI devices are present in
> the system as mentioned in the commit message of commit bff431e49ff5
> ("ACPI: WMI: Add ACPI-WMI mapping driver").
>
> Therefore the _UID is returned for a specific ACPI device that declares the
> given GUID, to which it is also mapped by other methods of wmi module.
>
> Signed-off-by: Yurii Pavlovskyi <yurii.pavlovskyi@gmail.com>

Some extra background may be useful when reviewing this.

As researched in
https://marc.info/?l=linux-kernel&m=155498017207933&w=2, we are
dealing with a tricky situation.

asus-wmi currently serves two different classes of device: eeepci-wmi
and asus-nb-wmi.

The eeepci devices have:
  _WDG : includes a METHOD block with GUID ASUS_WMI_MGMT_GUID, and an
EVENT block with GUID EEEPC_WMI_EVENT_GUID
 _UID : ASUSWMI

The asus-nb-wmi devices have:
 _ WDG : includes a METHOD block with GUID ASUS_WMI_MGMT_GUID (same as
eeepc), and an EVENT block with GUID ASUS_NB_WMI_EVENT_GUID
 _UID : ATK

To support new devices we now need to start concretely identifying
which of these we are working with. But complications include:
 - The main MGMT_GUID used for matching at the moment is shared over
both device types
 - Some Asus products have both of these (via two separate two
separate PNP0C14 WMI devices).

Currently eeepci-wmi and asus-nb-wmi register themselves with
asus-wmi, which registers a platform device for each one. The platform
dev probe then succeeds the platform device probe when it finds any
_WDG entry for the main MGMT_GUID and the _WDG entry for the
corresponding event GUID (not necessarily as part of the same
underlying ACPI Device). In the case of both devices being present
with duplicate MGMT, the first one that is parsed wins, and the other
is ignored (see guid_already_parsed()).

Sticking with the current approach, which is imperfect for devices
that have both devices, adding a method to detect the _UID seems
reasonable. Although actually I just realised you could probably also
detect the difference by using wmi_has_guid() on the EVENT UUID
without having to add a new function.

I'll keep thinking about how to improve the situation around two
devices present (but don't want to needlessly conflate this patch
series with that).

Daniel

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

* Re: [PATCH v4 00/13] Support of ASUS TUF Gaming series laptops
  2019-05-14 18:47 [PATCH v4 00/13] Support of ASUS TUF Gaming series laptops Yurii Pavlovskyi
                   ` (12 preceding siblings ...)
  2019-05-14 19:07 ` [PATCH v4 13/13] platform/x86: asus-wmi: Do not disable keyboard backlight on unloading Yurii Pavlovskyi
@ 2019-06-29 13:13 ` Andy Shevchenko
  13 siblings, 0 replies; 19+ messages in thread
From: Andy Shevchenko @ 2019-06-29 13:13 UTC (permalink / raw)
  To: Yurii Pavlovskyi
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	Chris Chiu, acpi4asus-user, Platform Driver,
	Linux Kernel Mailing List

On Tue, May 14, 2019 at 9:47 PM Yurii Pavlovskyi
<yurii.pavlovskyi@gmail.com> wrote:
>
> Hi,
>
> this is the fourth version of the patch series.
>

Pushed to my review and testing queue, thanks!

> Changelog:
> v4:
>   * Rebase on for-next branch
>   * Extract local variable in patch 01
>   * Rename new method to "..._method3" and keep comma in struct declaration
>     in patch 03 (NOTE: the arg2 does not fit on same line by 1 character)
>   * Patch "Improve DSTS WMI method ID detection":
>     - sort local variables
>     - use dev_info
>     - separate changes to wmi module in an own patch
>     - rename method ID constants and fix comment capitalization
>   * "Support WMI event queue": split into separate refactoring and new
>     functionality patches, use dev_info as well
>   * "Organize code into sections": split out error handling refactoring
>   * "Enhance detection of thermal data": remove unreasonable refactoring
>     and just change the currently used condition
>   * "Control RGB keyboard backlight": removed, will be posted afterwards.
>     I will follow on the status of the multicolor framework, it does look
>     promising for this.
>   * Mark URL references with "Link:"
>   * Minor corrections to commit messages
> v3:
>   * Use devm_* function in patch 01
>   * Detect DSTS/DCTS using _UID in patch 04
>   * Detect event queue by _UID as well in patch 05
>   * Rename poll function in patch 05
>   * Fix terminology in patches 09 and 10
>   * Correct commit messages
> v2:
>   * Fix logging
>
> INTRODUCTION
> The support for this laptop series is currently non-existent, as the
> asus-nb-wmi driver (which is essentially configuration for asus-wmi) fails
> to load and multiple ACPI errors are logged in dmesg. This patch series
> adds pretty comprehensive support for these relatively new laptops, adds
> some code organization, and fixes a couple of bugs in the asus-wmi module.
>
> Thread for V1/V2: https://lkml.org/lkml/2019/4/10/973
> Thread for V3: https://lkml.org/lkml/2019/4/19/178
>
> Yurii Pavlovskyi (13):
>   platform/x86: asus-wmi: Fix hwmon device cleanup
>   platform/x86: asus-wmi: Fix preserving keyboard backlight intensity on
>     load
>   platform/x86: asus-wmi: Increase input buffer size of WMI methods
>   platform/x86: wmi: Add function to get _UID of WMI device
>   platform/x86: asus-wmi: Improve DSTS WMI method ID detection
>   platform/x86: asus-wmi: Refactor WMI event handling
>   platform/x86: asus-wmi: Support WMI event queue
>   platform/x86: asus-nb-wmi: Add microphone mute key code
>   platform/x86: asus-wmi: Refactor error handling
>   platform/x86: asus-wmi: Organize code into sections
>   platform/x86: asus-wmi: Enhance detection of thermal data
>   platform/x86: asus-wmi: Switch fan boost mode
>   platform/x86: asus-wmi: Do not disable keyboard backlight on unloading
>
>  .../ABI/testing/sysfs-platform-asus-wmi       |  10 +
>  drivers/hid/hid-asus.c                        |   2 +-
>  drivers/platform/x86/asus-nb-wmi.c            |   3 +-
>  drivers/platform/x86/asus-wmi.c               | 427 ++++++++++++++----
>  drivers/platform/x86/wmi.c                    |  19 +
>  include/linux/acpi.h                          |   1 +
>  include/linux/platform_data/x86/asus-wmi.h    |   5 +-
>  7 files changed, 374 insertions(+), 93 deletions(-)
>
> --
> 2.17.1
>


-- 
With Best Regards,
Andy Shevchenko

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

end of thread, other threads:[~2019-06-29 13:13 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-14 18:47 [PATCH v4 00/13] Support of ASUS TUF Gaming series laptops Yurii Pavlovskyi
2019-05-14 18:50 ` [PATCH v4 01/13] platform/x86: asus-wmi: Fix hwmon device cleanup Yurii Pavlovskyi
2019-05-24 19:54   ` Daniel Drake
2019-05-14 18:51 ` [PATCH v4 02/13] platform/x86: asus-wmi: Fix preserving keyboard backlight intensity on load Yurii Pavlovskyi
2019-05-24 19:57   ` Daniel Drake
2019-05-14 18:54 ` [PATCH v4 03/13] platform/x86: asus-wmi: Increase input buffer size of WMI methods Yurii Pavlovskyi
2019-05-24 20:01   ` Daniel Drake
2019-05-14 18:59 ` [PATCH v4 04/13] platform/x86: wmi: Add function to get _UID of WMI device Yurii Pavlovskyi
2019-05-24 21:10   ` Daniel Drake
2019-05-14 19:00 ` [PATCH v4 05/13] platform/x86: asus-wmi: Improve DSTS WMI method ID detection Yurii Pavlovskyi
2019-05-14 19:01 ` [PATCH v4 06/13] platform/x86: asus-wmi: Refactor WMI event handling Yurii Pavlovskyi
2019-05-14 19:02 ` [PATCH v4 07/13] platform/x86: asus-wmi: Support WMI event queue Yurii Pavlovskyi
2019-05-14 19:02 ` [PATCH v4 08/13] platform/x86: asus-nb-wmi: Add microphone mute key code Yurii Pavlovskyi
2019-05-14 19:03 ` [PATCH v4 09/13] platform/x86: asus-wmi: Refactor error handling Yurii Pavlovskyi
2019-05-14 19:04 ` [PATCH v4 10/13] platform/x86: asus-wmi: Organize code into sections Yurii Pavlovskyi
2019-05-14 19:05 ` [PATCH v4 11/13] platform/x86: asus-wmi: Enhance detection of thermal data Yurii Pavlovskyi
2019-05-14 19:07 ` [PATCH v4 12/13] platform/x86: asus-wmi: Switch fan boost mode Yurii Pavlovskyi
2019-05-14 19:07 ` [PATCH v4 13/13] platform/x86: asus-wmi: Do not disable keyboard backlight on unloading Yurii Pavlovskyi
2019-06-29 13:13 ` [PATCH v4 00/13] Support of ASUS TUF Gaming series laptops Andy Shevchenko

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