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

Hi,

I'm new to kernel development, so first I would like to apologize in
advance for any mistakes.

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 comprehenisve support for these relatively new laptops, adds
some code organization, and fixes couple of bugs in the asus-wmi module.

I have FX565GM (FX505GM) model, but I would guess that the same will
likely apply to the complete FX505 / FX705 series. I've also tested 
this on an older K54C model to ensure that it doesn't break support.
Unfortunately I don't have capacity to test this on more devices.

The patches 1 and 2 are pure bug fixes, but I can not measure relevance
for stable.

OVERALL DESIGN DECISIONS
Besides this patch, I've written experimental separate driver [1] for this
series to make it usable on my system as a DKMS module for 4.18 kernel for
the time being. One might wonder if it is more reasonable to make a new
independent module. The things to consider are that: asus-nb-wmi is
currently loaded by the WMI GUID alias, whereas the original ASUS driver
does check for the ASUS7000 device in ACPI. One should then choose
appropriate base driver instead of asus-wmi when asus-nb-wmi is loaded,
about third of the code gets duplicated in this case and the whole ends up
ugly.

Another question, does it make sense to embed RGB keyboard backlight
support in kernel code? There was discussion [2] about exposing WMI to the
userspace. The same would apply for the fan boost mode support. As I
understand as of yet it is still preferrable to support hardware features
in kernel.

NOTE ON HWMON
One open issue with the result is that hwmon device gets loaded anyway,
but it does not do anything noticeable. The heavily reduced code for the
MFUN is present in DSDT, but it either really does nothing or possibly
call something unnoticeable via DMA.

I've managed to detect that thermal sensor is not present, but the MFUN
for read fan speed does return 0 and not an error. Taking this as 
condition for disabling hwmon might intermittently break some existing
devices if the RPM is really 0 (no idea if that is really possible). One
might ponder on the better way to detect the presence of manual fan
control.

NOTE ON QUIRKS
I would speculate that the queue might be really present in many more
devices, it just didn't get noticed. Anyway after this is merged one might
consider if it is reasonable to enable it always and fallback if flush
fails. The patch does enable the new quirks only for very new models.

Regarding the DSTS force quirk, as I understand the underlying issue is a
workaround for EEEPC devices and use of DSTS is more conventional. It
might be reasonable to find a way to detect specific DSTS device ID that
is present on EEEPC instead, apply same ACPI device detection approach or
just duplicate the relevant method calls. I don't have access to such
device or it's DSDT and can't evaluate any of these options myself.

NOTE ON KEYBOARD BACKLIGHT
When the keyboard backlight is set via 0x50021 DEVID the brightness drops
slightly compared to brightness after boot. I did not found any way to
revert this. The method does set some bit in EC RAM, but this address is
not used anywhere else.  Unfortunately I wiped original OS after two hours
after unpacking.If someone can verify whether it is identical to behavior
of the vendor driver it would be appreciated.

NOTE ON UPOWER DAEMON
If you're testing with GNOME, notice that UPower does hang pretty badly if
the module is removed at runtime at least on my device. Stop it with
'systemctl stop upower' before removing the module and then restart it
again.

[1] https://github.com/hackbnw/faustus
[2] https://lwn.net/Articles/725725/

Yurii Pavlovskyi (11):
  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: asus-wmi: Add quirk to force DSTS WMI method detection
  platform/x86: asus-wmi: Support queued WMI event codes
  platform/x86: asus-nb-wmi: Add microphone mute key code
  platform/x86: asus-wmi: Organize code into sections
  platform/x86: asus-wmi: Enhance detection of thermal data
  platform/x86: asus-wmi: Control RGB keyboard backlight
  platform/x86: asus-wmi: Switch fan boost mode
  platform/x86: asus-wmi: Do not disable keyboard backlight on unload

 .../ABI/testing/sysfs-platform-asus-wmi       |  71 ++
 drivers/platform/x86/asus-nb-wmi.c            |   9 +-
 drivers/platform/x86/asus-wmi.c               | 755 +++++++++++++++---
 drivers/platform/x86/asus-wmi.h               |   7 +
 include/linux/platform_data/x86/asus-wmi.h    |   3 +
 5 files changed, 742 insertions(+), 103 deletions(-)

-- 
2.17.1


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

* [PATCH 01/11] platform/x86: asus-wmi: Fix hwmon device cleanup
  2019-04-10 20:11 [PATCH 00/11] asus-wmi: Support of ASUS TUF Gaming series laptops Yurii Pavlovskyi
@ 2019-04-10 20:20 ` Yurii Pavlovskyi
  2019-04-11  8:21   ` Daniel Drake
  2019-04-10 20:26 ` [PATCH 02/11] platform/x86: asus-wmi: Fix preserving keyboard, backlight intensity on load Yurii Pavlovskyi
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 29+ messages in thread
From: Yurii Pavlovskyi @ 2019-04-10 20:20 UTC (permalink / raw)
  To: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake
  Cc: acpi4asus-user, platform-driver-x86, linux-kernel

The asus-wmi 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. Add
pointer to the device in module state and call cleanup on error.

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

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index ee1fa93708ec..6b736a9375ef 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -145,6 +145,7 @@ struct asus_wmi {
 
 	struct input_dev *inputdev;
 	struct backlight_device *backlight_device;
+	struct device *hwmon_device;
 	struct platform_device *platform_device;
 
 	struct led_classdev wlan_led;
@@ -1432,9 +1433,19 @@ static int asus_wmi_hwmon_init(struct asus_wmi *asus)
 		pr_err("Could not register asus hwmon device\n");
 		return PTR_ERR(hwmon);
 	}
+
+	asus->hwmon_device = hwmon;
 	return 0;
 }
 
+static void asus_wmi_hwmon_exit(struct asus_wmi *asus)
+{
+	if (asus->hwmon_device) {
+		asus_hwmon_fan_set_auto(asus);
+		hwmon_device_unregister(asus->hwmon_device);
+	}
+}
+
 /*
  * Backlight
  */
@@ -2157,6 +2168,7 @@ static int asus_wmi_add(struct platform_device *pdev)
 fail_rfkill:
 	asus_wmi_led_exit(asus);
 fail_leds:
+	asus_wmi_hwmon_exit(asus);
 fail_hwmon:
 	asus_wmi_input_exit(asus);
 fail_input:
@@ -2178,7 +2190,7 @@ static int asus_wmi_remove(struct platform_device *device)
 	asus_wmi_rfkill_exit(asus);
 	asus_wmi_debugfs_exit(asus);
 	asus_wmi_platform_exit(asus);
-	asus_hwmon_fan_set_auto(asus);
+	asus_wmi_hwmon_exit(asus);
 
 	kfree(asus);
 	return 0;
-- 
2.17.1


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

* [PATCH 02/11] platform/x86: asus-wmi: Fix preserving keyboard, backlight intensity on load
  2019-04-10 20:11 [PATCH 00/11] asus-wmi: Support of ASUS TUF Gaming series laptops Yurii Pavlovskyi
  2019-04-10 20:20 ` [PATCH 01/11] platform/x86: asus-wmi: Fix hwmon device cleanup Yurii Pavlovskyi
@ 2019-04-10 20:26 ` Yurii Pavlovskyi
  2019-04-10 20:27 ` [PATCH 03/11] platform/x86: asus-wmi: Increase input buffer size of WMI methods Yurii Pavlovskyi
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 29+ messages in thread
From: Yurii Pavlovskyi @ 2019-04-10 20:26 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	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 6b736a9375ef..0fbb947b07c4 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -591,8 +591,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] 29+ messages in thread

* [PATCH 03/11] platform/x86: asus-wmi: Increase input buffer size of WMI methods
  2019-04-10 20:11 [PATCH 00/11] asus-wmi: Support of ASUS TUF Gaming series laptops Yurii Pavlovskyi
  2019-04-10 20:20 ` [PATCH 01/11] platform/x86: asus-wmi: Fix hwmon device cleanup Yurii Pavlovskyi
  2019-04-10 20:26 ` [PATCH 02/11] platform/x86: asus-wmi: Fix preserving keyboard, backlight intensity on load Yurii Pavlovskyi
@ 2019-04-10 20:27 ` Yurii Pavlovskyi
  2019-04-10 20:28 ` [PATCH 04/11] platform/x86: asus-wmi: Add quirk to force DSTS WMI method detection Yurii Pavlovskyi
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 29+ messages in thread
From: Yurii Pavlovskyi @ 2019-04-10 20:27 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	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 kernel log. Patch
was tested on TUF Gaming FX505GM and older K54C model.

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 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 0fbb947b07c4..cfccfc0b8c2f 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -95,6 +95,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;
 
 /*
@@ -220,11 +221,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_method_3dw(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 };
@@ -256,6 +259,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_method_3dw(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] 29+ messages in thread

* [PATCH 04/11] platform/x86: asus-wmi: Add quirk to force DSTS WMI method detection
  2019-04-10 20:11 [PATCH 00/11] asus-wmi: Support of ASUS TUF Gaming series laptops Yurii Pavlovskyi
                   ` (2 preceding siblings ...)
  2019-04-10 20:27 ` [PATCH 03/11] platform/x86: asus-wmi: Increase input buffer size of WMI methods Yurii Pavlovskyi
@ 2019-04-10 20:28 ` Yurii Pavlovskyi
  2019-04-11  5:42   ` [PATCH v2 " Yurii Pavlovskyi
  2019-04-11 10:55   ` [PATCH " Daniel Drake
  2019-04-10 20:29 ` [PATCH 05/11] platform/x86: asus-wmi: Support queued WMI event codes Yurii Pavlovskyi
                   ` (7 subsequent siblings)
  11 siblings, 2 replies; 29+ messages in thread
From: Yurii Pavlovskyi @ 2019-04-10 20:28 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	acpi4asus-user, platform-driver-x86, linux-kernel

The DSTS method detection fails, as nothing is returned if method is not
defined in WMNB. As a result the control of keyboard backlight is not
functional for TUF Gaming series laptops (at the time the only
functionality of the driver on this model implemented with WMI methods).

Patch was tested on a newer TUF Gaming FX505GM and older K54C model.

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

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

The non-existing method ASUS_WMI_METHODID_DSTS=0x53544344 (actually it is
DCTS in little endian ASCII) is selected in asus->dsts.

One way to fix this would be to call both for every known device ID until
some answers - this would increase module load time.

Another option is to check some device that is known to exist on every
model - none known at the time.

Last option, which is implemented, is to check for presence of the
ASUS7000 device in ACPI tree (it is a dummy device), which is the
condition used for loading the vendor driver for this model. This might
not fix every affected model ever produced, but it likely does not
introduce any regressions. The patch introduces a quirk that is enabled
when ASUS7000 is found.

Scope (_SB)
{
    Device (ATK)
    {
        Name (_HID, "ASUS7000")  // _HID: Hardware ID
    }
}

Signed-off-by: Yurii Pavlovskyi <yurii.pavlovskyi@gmail.com>
---
 drivers/platform/x86/asus-nb-wmi.c |  5 +++++
 drivers/platform/x86/asus-wmi.c    | 16 +++++++++++++---
 drivers/platform/x86/asus-wmi.h    |  5 +++++
 3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
index b6f2ff95c3ed..cc5f0765a8d9 100644
--- a/drivers/platform/x86/asus-nb-wmi.c
+++ b/drivers/platform/x86/asus-nb-wmi.c
@@ -28,6 +28,7 @@
 #include <linux/fb.h>
 #include <linux/dmi.h>
 #include <linux/i8042.h>
+#include <linux/acpi.h>
 
 #include "asus-wmi.h"
 
@@ -434,6 +435,10 @@ static void asus_nb_wmi_quirks(struct asus_wmi_driver *driver)
 		}
 		pr_info("Using i8042 filter function for receiving events\n");
 	}
+
+	if (acpi_dev_found("ASUS7000")) {
+		driver->quirks->force_dsts = true;
+	}
 }
 
 static const struct key_entry asus_nb_wmi_keymap[] = {
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index cfccfc0b8c2f..58890d87d50c 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -24,7 +24,7 @@
  *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#define PR KBUILD_MODNAME ": "
 
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -1885,11 +1885,21 @@ 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 0 for unknown
+	 * methods, so the detection in this way is not possible and method must
+	 * be forced. Likely the presence of ACPI device ASUS7000 indicates
+	 * this.
 	 */
-	if (!asus_wmi_evaluate_method(ASUS_WMI_METHODID_DSTS, 0, 0, NULL))
+	if (asus->driver->quirks->force_dsts) {
+		pr_info(PR "DSTS method forced\n");
+		asus->dsts_id = ASUS_WMI_METHODID_DSTS2;
+	} else if (!asus_wmi_evaluate_method(ASUS_WMI_METHODID_DSTS,
+			0, 0, NULL)) {
 		asus->dsts_id = ASUS_WMI_METHODID_DSTS;
-	else
+	} 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/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
index 6c1311f4b04d..94056da02fde 100644
--- a/drivers/platform/x86/asus-wmi.h
+++ b/drivers/platform/x86/asus-wmi.h
@@ -54,6 +54,11 @@ struct quirk_entry {
 	 */
 	int no_display_toggle;
 	u32 xusb2pr;
+	/**
+	 * Force DSTS instead of DSCS and skip detection. Useful if WMNB
+	 * returns nothing on unknown method call.
+	 */
+	bool force_dsts;
 
 	bool (*i8042_filter)(unsigned char data, unsigned char str,
 			     struct serio *serio);
-- 
2.17.1


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

* [PATCH 05/11] platform/x86: asus-wmi: Support queued WMI event codes
  2019-04-10 20:11 [PATCH 00/11] asus-wmi: Support of ASUS TUF Gaming series laptops Yurii Pavlovskyi
                   ` (3 preceding siblings ...)
  2019-04-10 20:28 ` [PATCH 04/11] platform/x86: asus-wmi: Add quirk to force DSTS WMI method detection Yurii Pavlovskyi
@ 2019-04-10 20:29 ` Yurii Pavlovskyi
  2019-04-11  5:44   ` [PATCH v2 " Yurii Pavlovskyi
  2019-04-10 20:30 ` [PATCH 06/11] platform/x86: asus-nb-wmi: Add microphone mute key code Yurii Pavlovskyi
                   ` (6 subsequent siblings)
  11 siblings, 1 reply; 29+ messages in thread
From: Yurii Pavlovskyi @ 2019-04-10 20:29 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	acpi4asus-user, platform-driver-x86, linux-kernel

Event codes are expected to be polled from a queue on at least some
models.

The WMI event codes are pushed into queue based on circular buffer. After
INIT method is called ACPI code is allowed to push events into this buffer
the INIT method can not 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.

Patch was tested on a newer TUF Gaming FX505GM and older K54C model.

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)
}

The fix flushes the old key codes out of the queue on load and after
receiving event the queue is read until either ..FFFF or 1 is encountered.

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

Introduce functionality for flushing and processing queued codes, which is
enabled via quirk flag for ASUS7000. It might be considered if it is
reasonable to enable it everywhere (might introduce regressions) or always
try to flush the queue on module load and try to detect if this quirk is
present in the future.

This patch limits the effect to the specific hardware defined by ASUS7000
device that is used for driver detection by vendor driver of Fx505. The
fallback is also implemented in case initial flush fails.

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

diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
index cc5f0765a8d9..357d273ed336 100644
--- a/drivers/platform/x86/asus-nb-wmi.c
+++ b/drivers/platform/x86/asus-nb-wmi.c
@@ -438,6 +438,7 @@ static void asus_nb_wmi_quirks(struct asus_wmi_driver *driver)
 
 	if (acpi_dev_found("ASUS7000")) {
 		driver->quirks->force_dsts = true;
+		driver->quirks->wmi_event_queue = true;
 	}
 }
 
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 58890d87d50c..e0a710c64dea 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -80,6 +80,12 @@ MODULE_LICENSE("GPL");
 #define USB_INTEL_XUSB2PR		0xD0
 #define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI	0x9c31
 
+#define WMI_EVENT_QUEUE_SIZE		0x10
+#define WMI_EVENT_QUEUE_END		0x1
+#define WMI_EVENT_MASK			0xFFFF
+/* The event value is always the same. */
+#define WMI_EVENT_VALUE			0xFF
+
 static const char * const ashs_ids[] = { "ATK4001", "ATK4002", NULL };
 
 static bool ashs_present(void)
@@ -143,6 +149,7 @@ struct asus_wmi {
 	int dsts_id;
 	int spec;
 	int sfun;
+	bool wmi_event_queue;
 
 	struct input_dev *inputdev;
 	struct backlight_device *backlight_device;
@@ -1637,77 +1644,126 @@ static int is_display_toggle(int code)
 	return 0;
 }
 
-static void asus_wmi_notify(u32 value, void *context)
+static int asus_poll_wmi_event(u32 value)
 {
-	struct asus_wmi *asus = context;
-	struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL };
+	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
 	union acpi_object *obj;
 	acpi_status status;
-	int code;
-	int orig_code;
-	unsigned int key_value = 1;
-	bool autorelease = 1;
+	int code = -EIO;
 
-	status = wmi_get_event_data(value, &response);
-	if (status != AE_OK) {
-		pr_err("bad event status 0x%x\n", status);
-		return;
+	status = wmi_get_event_data(value, &output);
+	if (ACPI_FAILURE(status)) {
+		pr_warn(PR "Failed to get WMI event code: %s\n",
+				acpi_format_exception(status));
+		return code;
 	}
 
-	obj = (union acpi_object *)response.pointer;
+	obj = (union acpi_object *)output.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);
+
+	kfree(obj);
+	return code;
+}
+
+static void asus_wmi_handle_notify(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 (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;
+	int i;
+
+	for (i = 0; i < WMI_EVENT_QUEUE_SIZE + 1; i++) {
+		code = asus_poll_wmi_event(value);
+
+		if (code < 0) {
+			pr_warn(PR "Failed to get event code: 0x%x\n", code);
+			return;
+		}
+
+		if (code == WMI_EVENT_QUEUE_END || code == WMI_EVENT_MASK)
+			return;
+
+		asus_wmi_handle_notify(code, asus);
+
+		if (!asus->wmi_event_queue)
+			return;
+	}
+
+	pr_warn(PR "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_poll_wmi_event(WMI_EVENT_VALUE);
+
+		if (code < 0) {
+			pr_warn(PR "Failed to poll event during flush: %d\n",
+					code);
+			return code;
+		}
+
+		if (code == WMI_EVENT_QUEUE_END || code == WMI_EVENT_MASK)
+			return 0;
+	}
+
+	pr_warn(PR "Failed to flush event queue\n");
+	return -EIO;
 }
 
 /*
@@ -2159,8 +2215,18 @@ static int asus_wmi_add(struct platform_device *pdev)
 		err = asus_wmi_backlight_init(asus);
 		if (err && err != -ENODEV)
 			goto fail_backlight;
-	} else
+	} else {
 		err = asus_wmi_set_devstate(ASUS_WMI_DEVID_BACKLIGHT, 2, NULL);
+	}
+
+	/** Try to initialize queue and fallback if it fails. */
+	if (asus->driver->quirks->wmi_event_queue) {
+		err = asus_wmi_notify_queue_flush(asus);
+		if (err)
+			asus->driver->quirks->wmi_event_queue = false;
+		else
+			pr_info(PR "WMI event queue enabled\n");
+	}
 
 	status = wmi_install_notify_handler(asus->driver->event_guid,
 					    asus_wmi_notify, asus);
diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
index 94056da02fde..1248658d6442 100644
--- a/drivers/platform/x86/asus-wmi.h
+++ b/drivers/platform/x86/asus-wmi.h
@@ -54,6 +54,8 @@ struct quirk_entry {
 	 */
 	int no_display_toggle;
 	u32 xusb2pr;
+	/* Multiple event codes can be queued in buffer. */
+	bool wmi_event_queue;
 	/**
 	 * Force DSTS instead of DSCS and skip detection. Useful if WMNB
 	 * returns nothing on unknown method call.
-- 
2.17.1


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

* [PATCH 06/11] platform/x86: asus-nb-wmi: Add microphone mute key code
  2019-04-10 20:11 [PATCH 00/11] asus-wmi: Support of ASUS TUF Gaming series laptops Yurii Pavlovskyi
                   ` (4 preceding siblings ...)
  2019-04-10 20:29 ` [PATCH 05/11] platform/x86: asus-wmi: Support queued WMI event codes Yurii Pavlovskyi
@ 2019-04-10 20:30 ` Yurii Pavlovskyi
  2019-04-11  5:44   ` [PATCH v2 " Yurii Pavlovskyi
  2019-04-10 20:31 ` [PATCH 07/11] platform/x86: asus-wmi: Organize code into sections Yurii Pavlovskyi
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 29+ messages in thread
From: Yurii Pavlovskyi @ 2019-04-10 20:30 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	acpi4asus-user, platform-driver-x86, linux-kernel

The microphone mute key that is present on FX505GM laptop and possibly
others is missing from sparse keymap. Add the missing code.

Also comment on the fan mode switch key that 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 357d273ed336..39cf447198a9 100644
--- a/drivers/platform/x86/asus-nb-wmi.c
+++ b/drivers/platform/x86/asus-nb-wmi.c
@@ -474,6 +474,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 } },
@@ -488,7 +489,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] 29+ messages in thread

* [PATCH 07/11] platform/x86: asus-wmi: Organize code into sections
  2019-04-10 20:11 [PATCH 00/11] asus-wmi: Support of ASUS TUF Gaming series laptops Yurii Pavlovskyi
                   ` (5 preceding siblings ...)
  2019-04-10 20:30 ` [PATCH 06/11] platform/x86: asus-nb-wmi: Add microphone mute key code Yurii Pavlovskyi
@ 2019-04-10 20:31 ` Yurii Pavlovskyi
  2019-04-11  5:45   ` [PATCH v2 " Yurii Pavlovskyi
  2019-04-10 20:32 ` [PATCH 08/11] platform/x86: asus-wmi: Enhance detection of thermal data Yurii Pavlovskyi
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 29+ messages in thread
From: Yurii Pavlovskyi @ 2019-04-10 20:31 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	acpi4asus-user, platform-driver-x86, linux-kernel

The driver has grown (and will more) pretty big 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 | 94 ++++++++++++++++-----------------
 1 file changed, 46 insertions(+), 48 deletions(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index e0a710c64dea..b9a6dc224e08 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -191,6 +191,8 @@ struct asus_wmi {
 	struct asus_wmi_driver *driver;
 };
 
+/* Input **********************************************************************/
+
 static int asus_wmi_input_init(struct asus_wmi *asus)
 {
 	int err;
@@ -228,6 +230,8 @@ static void asus_wmi_input_exit(struct asus_wmi *asus)
 	asus->inputdev = NULL;
 }
 
+/* WMI ************************************************************************/
+
 static int asus_wmi_evaluate_method_3dw(u32 method_id, u32 arg0, u32 arg1,
 		u32 arg2, u32 *retval)
 {
@@ -246,7 +250,7 @@ static int asus_wmi_evaluate_method_3dw(u32 method_id, u32 arg0, u32 arg1,
 				     &input, &output);
 
 	if (ACPI_FAILURE(status))
-		goto exit;
+		return -EIO;
 
 	obj = (union acpi_object *)output.pointer;
 	if (obj && obj->type == ACPI_TYPE_INTEGER)
@@ -257,10 +261,6 @@ static int asus_wmi_evaluate_method_3dw(u32 method_id, u32 arg0, u32 arg1,
 
 	kfree(obj);
 
-exit:
-	if (ACPI_FAILURE(status))
-		return -EIO;
-
 	if (tmp == ASUS_WMI_UNSUPPORTED_METHOD)
 		return -ENODEV;
 
@@ -344,9 +344,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
@@ -656,6 +655,7 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
 	return rv;
 }
 
+/* RF *************************************************************************/
 
 /*
  * PCI hotplug (for wlan rfkill)
@@ -1078,6 +1078,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;
@@ -1110,9 +1112,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)
 {
@@ -1388,7 +1389,6 @@ static umode_t asus_hwmon_sysfs_is_visible(struct kobject *kobj,
 	else if (attr == &dev_attr_temp1_input.attr)
 		dev_id = ASUS_WMI_DEVID_THERMAL_CTRL;
 
-
 	if (attr == &dev_attr_fan1_input.attr
 	    || attr == &dev_attr_fan1_label.attr
 	    || attr == &dev_attr_pwm1.attr
@@ -1460,9 +1460,27 @@ static void asus_wmi_hwmon_exit(struct asus_wmi *asus)
 	}
 }
 
-/*
- * 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;
@@ -1644,6 +1662,8 @@ static int is_display_toggle(int code)
 	return 0;
 }
 
+/* WMI events *****************************************************************/
+
 static int asus_poll_wmi_event(u32 value)
 {
 	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
@@ -1766,9 +1786,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)
@@ -1907,9 +1926,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)
 {
 	int rv;
@@ -1971,9 +1989,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;
@@ -2120,28 +2137,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;
-
-	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;
-}
+/* Init / exit ****************************************************************/
 
-/*
- * WMI Driver
- */
 static int asus_wmi_add(struct platform_device *pdev)
 {
 	struct platform_driver *pdrv = to_platform_driver(pdev->dev.driver);
@@ -2279,9 +2276,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);
@@ -2353,6 +2349,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] 29+ messages in thread

* [PATCH 08/11] platform/x86: asus-wmi: Enhance detection of thermal data
  2019-04-10 20:11 [PATCH 00/11] asus-wmi: Support of ASUS TUF Gaming series laptops Yurii Pavlovskyi
                   ` (6 preceding siblings ...)
  2019-04-10 20:31 ` [PATCH 07/11] platform/x86: asus-wmi: Organize code into sections Yurii Pavlovskyi
@ 2019-04-10 20:32 ` Yurii Pavlovskyi
  2019-04-11  5:45   ` [PATCH v2 " Yurii Pavlovskyi
  2019-04-10 20:33 ` [PATCH 09/11] platform/x86: asus-wmi: Control RGB keyboard backlight Yurii Pavlovskyi
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 29+ messages in thread
From: Yurii Pavlovskyi @ 2019-04-10 20:32 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	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 as erroneous value in addition to 0 K.
* Refactor detection of thermal input availability to a separate function.

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

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index b9a6dc224e08..175ecd5b7c51 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -176,6 +176,7 @@ struct asus_wmi {
 	struct asus_rfkill gps;
 	struct asus_rfkill uwb;
 
+	bool asus_hwmon_thermal_available;
 	bool asus_hwmon_fan_manual_mode;
 	int asus_hwmon_num_fans;
 	int asus_hwmon_pwm;
@@ -1373,6 +1374,32 @@ static struct attribute *hwmon_attributes[] = {
 	NULL
 };
 
+static int asus_hwmon_check_thermal_available(struct asus_wmi *asus)
+{
+	u32 value = ASUS_WMI_UNSUPPORTED_METHOD;
+	int err;
+
+	asus->asus_hwmon_thermal_available = false;
+	err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_THERMAL_CTRL, &value);
+
+	if (err < 0) {
+		if (err == -ENODEV)
+			return 0;
+
+		return err;
+	}
+
+	/*
+	 * If the temperature value in deci-Kelvin is near the absolute
+	 * zero temperature, something is clearly wrong.
+	 */
+	if (!value || value == 1)
+		return 0;
+
+	asus->asus_hwmon_thermal_available = true;
+	return 0;
+}
+
 static umode_t asus_hwmon_sysfs_is_visible(struct kobject *kobj,
 					  struct attribute *attr, int idx)
 {
@@ -1386,8 +1413,6 @@ static umode_t asus_hwmon_sysfs_is_visible(struct kobject *kobj,
 
 	if (attr == &dev_attr_pwm1.attr)
 		dev_id = ASUS_WMI_DEVID_FAN_CTRL;
-	else if (attr == &dev_attr_temp1_input.attr)
-		dev_id = ASUS_WMI_DEVID_THERMAL_CTRL;
 
 	if (attr == &dev_attr_fan1_input.attr
 	    || attr == &dev_attr_fan1_label.attr
@@ -1412,15 +1437,13 @@ static umode_t asus_hwmon_sysfs_is_visible(struct kobject *kobj,
 		 * - reverved bits are non-zero
 		 * - sfun and presence bit are not set
 		 */
-		if (value == ASUS_WMI_UNSUPPORTED_METHOD || value & 0xFFF80000
+		if (value == ASUS_WMI_UNSUPPORTED_METHOD || (value & 0xFFF80000)
 		    || (!asus->sfun && !(value & ASUS_WMI_DSTS_PRESENCE_BIT)))
 			ok = false;
 		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)
-			ok = false;
+	} else if (attr == &dev_attr_temp1_input.attr) {
+		ok = asus->asus_hwmon_thermal_available;
 	} else if (fan_attr <= asus->asus_hwmon_num_fans && fan_attr != -1) {
 		ok = true;
 	} else {
@@ -1476,6 +1499,15 @@ static int asus_wmi_fan_init(struct asus_wmi *asus)
 	}
 
 	pr_info("Number of fans: %d\n", asus->asus_hwmon_num_fans);
+
+	status = asus_hwmon_check_thermal_available(asus);
+	if (status) {
+		pr_warn("Could not check if thermal available: %d\n", status);
+		return -ENXIO;
+	}
+
+	pr_info(PR "Thermal available: %d\n",
+			asus->asus_hwmon_thermal_available);
 	return 0;
 }
 
-- 
2.17.1


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

* [PATCH 09/11] platform/x86: asus-wmi: Control RGB keyboard backlight
  2019-04-10 20:11 [PATCH 00/11] asus-wmi: Support of ASUS TUF Gaming series laptops Yurii Pavlovskyi
                   ` (7 preceding siblings ...)
  2019-04-10 20:32 ` [PATCH 08/11] platform/x86: asus-wmi: Enhance detection of thermal data Yurii Pavlovskyi
@ 2019-04-10 20:33 ` Yurii Pavlovskyi
  2019-04-11  5:46   ` [PATCH v2 " Yurii Pavlovskyi
  2019-04-10 20:34 ` [PATCH 10/11] platform/x86: asus-wmi: Switch fan boost mode Yurii Pavlovskyi
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 29+ messages in thread
From: Yurii Pavlovskyi @ 2019-04-10 20:33 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	acpi4asus-user, platform-driver-x86, linux-kernel, linux-api

The WMI exposes two methods for controlling RGB keyboard backlight which
allow to control:
* RGB components in range 00 - ff,
* Switch between 4 effects,
* Switch between 3 effect speed modes,
* Separately enable the backlight on boot, in awake state (after driver
  load), in sleep mode, and probably in something called shutdown mode
  (no observable effects of enabling it are known so far).

The configuration should be written to several sysfs parameter buffers
which are then written via WMI by writing either 1 or 2 to the "kbbl_set"
parameter. When reading the buffers the last written value is returned.

If the 2 is written to "kbbl_set", the parameters will be reset on reboot
(temporary mode), 1 is permanent mode, parameters are retained.

The calls use new 3-dword input buffer method call.

The functionality is only enabled if corresponding DSTS methods return
exact valid values.

The following script demonstrates usage:

echo Red [00 - ff]
echo 33 > /sys/devices/platform/asus-nb-wmi/kbbl/kbbl_red
echo Green [00 - ff]
echo ff > /sys/devices/platform/asus-nb-wmi/kbbl/kbbl_green
echo Blue [00 - ff]
echo 0 > /sys/devices/platform/asus-nb-wmi/kbbl/kbbl_blue
echo Mode: 0 - static color, 1 - blink, 2 - rainbow, 3 - strobe
echo 0 > /sys/devices/platform/asus-nb-wmi/kbbl/kbbl_mode
echo Speed for modes 1 and 2: 0 - slow, 1 - medium, 2 - fast
echo 0 > /sys/devices/platform/asus-nb-wmi/kbbl/kbbl_speed
echo Enable: 02 - on boot, before module load, 08 - awake, 20 - sleep,
echo 2a or ff to set all
echo 2a > /sys/devices/platform/asus-nb-wmi/kbbl/kbbl_flags
echo Save: 1 - permanently, 2 - temporarily, reset after reboot
echo 1 > /sys/devices/platform/asus-nb-wmi/kbbl/kbbl_set

Signed-off-by: Yurii Pavlovskyi <yurii.pavlovskyi@gmail.com>
---
 .../ABI/testing/sysfs-platform-asus-wmi       |  61 ++++
 drivers/platform/x86/asus-wmi.c               | 329 ++++++++++++++++++
 include/linux/platform_data/x86/asus-wmi.h    |   2 +
 3 files changed, 392 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-platform-asus-wmi b/Documentation/ABI/testing/sysfs-platform-asus-wmi
index 019e1e29370e..300a40519695 100644
--- a/Documentation/ABI/testing/sysfs-platform-asus-wmi
+++ b/Documentation/ABI/testing/sysfs-platform-asus-wmi
@@ -36,3 +36,64 @@ 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>/kbbl/kbbl_red
+Date:		Apr 2019
+KernelVersion:	5.1
+Contact:	"Yurii Pavlovskyi" <yurii.pavlovskyi@gmail.com>
+Description:
+		RGB keyboard backlight red component: 00 .. ff.
+
+What:		/sys/devices/platform/<platform>/kbbl/kbbl_green
+Date:		Apr 2019
+KernelVersion:	5.1
+Contact:	"Yurii Pavlovskyi" <yurii.pavlovskyi@gmail.com>
+Description:
+		RGB keyboard backlight green component: 00 .. ff.
+
+What:		/sys/devices/platform/<platform>/kbbl/kbbl_blue
+Date:		Apr 2019
+KernelVersion:	5.1
+Contact:	"Yurii Pavlovskyi" <yurii.pavlovskyi@gmail.com>
+Description:
+		RGB keyboard backlight blue component: 00 .. ff.
+
+What:		/sys/devices/platform/<platform>/kbbl/kbbl_mode
+Date:		Apr 2019
+KernelVersion:	5.1
+Contact:	"Yurii Pavlovskyi" <yurii.pavlovskyi@gmail.com>
+Description:
+		RGB keyboard backlight mode:
+			* 0 - static color,
+			* 1 - blink,
+			* 2 - rainbow,
+			* 3 - strobe.
+
+What:		/sys/devices/platform/<platform>/kbbl/kbbl_speed
+Date:		Apr 2019
+KernelVersion:	5.1
+Contact:	"Yurii Pavlovskyi" <yurii.pavlovskyi@gmail.com>
+Description:
+		RGB keyboard backlight speed for modes 1 and 2:
+			* 0 - slow,
+			* 1 - medium,
+			* 2 - fast.
+
+What:		/sys/devices/platform/<platform>/kbbl/kbbl_flags
+Date:		Apr 2019
+KernelVersion:	5.1
+Contact:	"Yurii Pavlovskyi" <yurii.pavlovskyi@gmail.com>
+Description:
+		RGB keyboard backlight enable flags (2a to enable everything), OR of:
+			* 02 - on boot (until module load),
+			* 08 - awake,
+			* 20 - sleep.
+
+What:		/sys/devices/platform/<platform>/kbbl/kbbl_set
+Date:		Apr 2019
+KernelVersion:	5.1
+Contact:	"Yurii Pavlovskyi" <yurii.pavlovskyi@gmail.com>
+Description:
+		Write changed RGB keyboard backlight parameters:
+			* 1 - permanently,
+			* 2 - temporarily.
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 175ecd5b7c51..f4323a57f22f 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -145,6 +145,21 @@ struct asus_rfkill {
 	u32 dev_id;
 };
 
+struct asus_kbbl_rgb {
+	u8 kbbl_red;
+	u8 kbbl_green;
+	u8 kbbl_blue;
+	u8 kbbl_mode;
+	u8 kbbl_speed;
+
+	u8 kbbl_set_red;
+	u8 kbbl_set_green;
+	u8 kbbl_set_blue;
+	u8 kbbl_set_mode;
+	u8 kbbl_set_speed;
+	u8 kbbl_set_flags;
+};
+
 struct asus_wmi {
 	int dsts_id;
 	int spec;
@@ -181,6 +196,9 @@ struct asus_wmi {
 	int asus_hwmon_num_fans;
 	int asus_hwmon_pwm;
 
+	bool kbbl_rgb_available;
+	struct asus_kbbl_rgb kbbl_rgb;
+
 	struct hotplug_slot hotplug_slot;
 	struct mutex hotplug_lock;
 	struct mutex wmi_lock;
@@ -656,6 +674,310 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
 	return rv;
 }
 
+/* RGB keyboard backlight *****************************************************/
+
+static ssize_t show_u8(u8 value, char *buf)
+{
+	return scnprintf(buf, PAGE_SIZE, "%02x\n", value);
+}
+
+static ssize_t store_u8(u8 *value, const char *buf, int count)
+{
+	int err;
+	u8 result;
+
+	err = kstrtou8(buf, 16, &result);
+	if (err < 0) {
+		pr_warn(PR "Trying to store invalid value\n");
+		return err;
+	}
+
+	*value = result;
+
+	return count;
+}
+
+static ssize_t kbbl_red_show(struct device *dev, struct device_attribute *attr,
+		char *buf)
+{
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	return show_u8(asus->kbbl_rgb.kbbl_red, buf);
+}
+
+static ssize_t kbbl_red_store(struct device *dev, struct device_attribute *attr,
+		const char *buf, size_t count)
+{
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	return store_u8(&asus->kbbl_rgb.kbbl_set_red, buf, count);
+}
+
+static ssize_t kbbl_green_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	return show_u8(asus->kbbl_rgb.kbbl_green, buf);
+}
+
+static ssize_t kbbl_green_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	return store_u8(&asus->kbbl_rgb.kbbl_set_green, buf, count);
+}
+
+static ssize_t kbbl_blue_show(struct device *dev, struct device_attribute *attr,
+		char *buf)
+{
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	return show_u8(asus->kbbl_rgb.kbbl_blue, buf);
+}
+
+static ssize_t kbbl_blue_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	return store_u8(&asus->kbbl_rgb.kbbl_set_blue, buf, count);
+}
+
+static ssize_t kbbl_mode_show(struct device *dev, struct device_attribute *attr,
+		char *buf)
+{
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	return show_u8(asus->kbbl_rgb.kbbl_mode, buf);
+}
+
+static ssize_t kbbl_mode_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	return store_u8(&asus->kbbl_rgb.kbbl_set_mode, buf, count);
+}
+
+static ssize_t kbbl_speed_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	return show_u8(asus->kbbl_rgb.kbbl_speed, buf);
+}
+
+static ssize_t kbbl_speed_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	return store_u8(&asus->kbbl_rgb.kbbl_set_speed, buf, count);
+}
+
+static ssize_t kbbl_flags_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	return show_u8(asus->kbbl_rgb.kbbl_set_flags, buf);
+}
+
+static ssize_t kbbl_flags_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	return store_u8(&asus->kbbl_rgb.kbbl_set_flags, buf, count);
+}
+
+static ssize_t kbbl_set_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	return scnprintf(buf, PAGE_SIZE,
+			"Write to configure RGB keyboard backlight\n");
+}
+
+static int kbbl_rgb_write(struct asus_wmi *asus, int persistent)
+{
+	int err;
+	u32 retval;
+	u8 speed_byte;
+	u8 mode_byte;
+	u8 speed;
+	u8 mode;
+
+	speed = asus->kbbl_rgb.kbbl_set_speed;
+	switch (speed) {
+	case 0:
+	default:
+		speed_byte = 0xe1; // slow
+		speed = 0;
+		break;
+	case 1:
+		speed_byte = 0xeb; // medium
+		break;
+	case 2:
+		speed_byte = 0xf5; // fast
+		break;
+	}
+
+	mode = asus->kbbl_rgb.kbbl_set_mode;
+	switch (mode) {
+	case 0:
+	default:
+		mode_byte = 0x00; // static color
+		mode = 0;
+		break;
+	case 1:
+		mode_byte = 0x01; // blink
+		break;
+	case 2:
+		mode_byte = 0x02; // rainbow
+		break;
+	case 3:
+		mode_byte = 0x0a; // strobe
+		break;
+	}
+
+	err = asus_wmi_evaluate_method_3dw(ASUS_WMI_METHODID_DEVS,
+		ASUS_WMI_DEVID_KBD_RGB,
+		(persistent ? 0xb4 : 0xb3) |
+		(mode_byte << 8) |
+		(asus->kbbl_rgb.kbbl_set_red << 16) |
+		(asus->kbbl_rgb.kbbl_set_green << 24),
+		(asus->kbbl_rgb.kbbl_set_blue) |
+		(speed_byte << 8), &retval);
+	if (err) {
+		pr_warn(PR "RGB keyboard device 1, write error: %d\n", err);
+		return err;
+	}
+
+	if (retval != 1) {
+		pr_warn(PR "RGB keyboard device 1, write error (retval): %x\n",
+				retval);
+		return -EIO;
+	}
+
+	err = asus_wmi_evaluate_method_3dw(ASUS_WMI_METHODID_DEVS,
+		ASUS_WMI_DEVID_KBD_RGB2,
+		(0xbd) |
+		(asus->kbbl_rgb.kbbl_set_flags << 16) |
+		(persistent ? 0x0100 : 0x0000), 0, &retval);
+	if (err) {
+		pr_warn(PR "RGB keyboard device 2, write error: %d\n", err);
+		return err;
+	}
+
+	if (retval != 1) {
+		pr_warn(PR "RGB keyboard device 2, write error (retval): %x\n",
+				retval);
+		return -EIO;
+	}
+
+	asus->kbbl_rgb.kbbl_red = asus->kbbl_rgb.kbbl_set_red;
+	asus->kbbl_rgb.kbbl_green = asus->kbbl_rgb.kbbl_set_green;
+	asus->kbbl_rgb.kbbl_blue = asus->kbbl_rgb.kbbl_set_blue;
+	asus->kbbl_rgb.kbbl_mode = mode;
+	asus->kbbl_rgb.kbbl_speed = speed;
+
+	return 0;
+}
+
+static ssize_t kbbl_set_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	u8 value;
+	struct asus_wmi *asus;
+	int result;
+
+	asus = dev_get_drvdata(dev);
+	result = store_u8(&value, buf, count);
+	if (result < 0)
+		return result;
+
+	if (value == 1)
+		kbbl_rgb_write(asus, 1);
+	else if (value == 2)
+		kbbl_rgb_write(asus, 0);
+
+	return count;
+}
+
+/* RGB values: 00 .. ff */
+static DEVICE_ATTR_RW(kbbl_red);
+static DEVICE_ATTR_RW(kbbl_green);
+static DEVICE_ATTR_RW(kbbl_blue);
+
+/* Color modes: 0 - static color, 1 - blink, 2 - rainbow, 3 - strobe */
+static DEVICE_ATTR_RW(kbbl_mode);
+
+/* Speed for modes 1 and 2: 0 - slow, 1 - medium, 2 - fast */
+static DEVICE_ATTR_RW(kbbl_speed);
+
+/*
+ * Enable: 02 - on boot (until module load) | 08 - awake | 20 - sleep
+ * (2a or ff to enable everything)
+ *
+ * Logically 80 would be shutdown, but no visible effects of this option
+ * were observed so far
+ */
+static DEVICE_ATTR_RW(kbbl_flags);
+
+/* Write data: 1 - permanently, 2 - temporarily (reset after reboot) */
+static DEVICE_ATTR_RW(kbbl_set);
+
+static struct attribute *rgbkb_sysfs_attributes[] = {
+	&dev_attr_kbbl_red.attr,
+	&dev_attr_kbbl_green.attr,
+	&dev_attr_kbbl_blue.attr,
+	&dev_attr_kbbl_mode.attr,
+	&dev_attr_kbbl_speed.attr,
+	&dev_attr_kbbl_flags.attr,
+	&dev_attr_kbbl_set.attr,
+	NULL,
+};
+
+static const struct attribute_group kbbl_attribute_group = {
+	.name = "kbbl",
+	.attrs = rgbkb_sysfs_attributes
+};
+
+static int kbbl_rgb_init(struct asus_wmi *asus)
+{
+	int err;
+
+	err = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_KBD_RGB);
+	if (err) {
+		if (err == -ENODEV)
+			return 0;
+		else
+			return err;
+	}
+
+	err = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_KBD_RGB2);
+	if (err) {
+		if (err == -ENODEV)
+			return 0;
+		else
+			return err;
+	}
+
+	asus->kbbl_rgb_available = true;
+	return sysfs_create_group(&asus->platform_device->dev.kobj,
+			&kbbl_attribute_group);
+}
+
+static void kbbl_rgb_exit(struct asus_wmi *asus)
+{
+	if (asus->kbbl_rgb_available) {
+		sysfs_remove_group(&asus->platform_device->dev.kobj,
+				&kbbl_attribute_group);
+	}
+}
+
 /* RF *************************************************************************/
 
 /*
@@ -2212,6 +2534,10 @@ static int asus_wmi_add(struct platform_device *pdev)
 	if (err)
 		goto fail_leds;
 
+	err = kbbl_rgb_init(asus);
+	if (err)
+		goto fail_rgbkb;
+
 	asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_WLAN, &result);
 	if (result & (ASUS_WMI_DSTS_PRESENCE_BIT | ASUS_WMI_DSTS_USER_BIT))
 		asus->driver->wlan_ctrl_by_user = 1;
@@ -2278,6 +2604,8 @@ static int asus_wmi_add(struct platform_device *pdev)
 fail_backlight:
 	asus_wmi_rfkill_exit(asus);
 fail_rfkill:
+	kbbl_rgb_exit(asus);
+fail_rgbkb:
 	asus_wmi_led_exit(asus);
 fail_leds:
 	asus_wmi_hwmon_exit(asus);
@@ -2299,6 +2627,7 @@ static int asus_wmi_remove(struct platform_device *device)
 	asus_wmi_backlight_exit(asus);
 	asus_wmi_input_exit(asus);
 	asus_wmi_led_exit(asus);
+	kbbl_rgb_exit(asus);
 	asus_wmi_rfkill_exit(asus);
 	asus_wmi_debugfs_exit(asus);
 	asus_wmi_platform_exit(asus);
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index 53dfc2541960..25b7b653e6d2 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -57,6 +57,8 @@
 #define ASUS_WMI_DEVID_KBD_BACKLIGHT	0x00050021
 #define ASUS_WMI_DEVID_LIGHT_SENSOR	0x00050022 /* ?? */
 #define ASUS_WMI_DEVID_LIGHTBAR		0x00050025
+#define ASUS_WMI_DEVID_KBD_RGB		0x00100056
+#define ASUS_WMI_DEVID_KBD_RGB2		0x00100057
 
 /* Misc */
 #define ASUS_WMI_DEVID_CAMERA		0x00060013
-- 
2.17.1


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

* [PATCH 10/11] platform/x86: asus-wmi: Switch fan boost mode
  2019-04-10 20:11 [PATCH 00/11] asus-wmi: Support of ASUS TUF Gaming series laptops Yurii Pavlovskyi
                   ` (8 preceding siblings ...)
  2019-04-10 20:33 ` [PATCH 09/11] platform/x86: asus-wmi: Control RGB keyboard backlight Yurii Pavlovskyi
@ 2019-04-10 20:34 ` Yurii Pavlovskyi
  2019-04-11  5:47   ` [PATCH v2 " Yurii Pavlovskyi
  2019-04-10 20:36 ` [PATCH 11/11] platform/x86: asus-wmi: Do not disable keyboard backlight on unload Yurii Pavlovskyi
  2019-04-11  5:38 ` [PATCH 00/11] asus-wmi: Support of ASUS TUF Gaming series laptops Yurii Pavlovskyi
  11 siblings, 1 reply; 29+ messages in thread
From: Yurii Pavlovskyi @ 2019-04-10 20:34 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	acpi4asus-user, platform-driver-x86, linux-kernel, linux-api

The WMI exposes a write-only device ID where three 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 these 3
modes.

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. The
corresponding DEVS device handler does obviously take 3 possible
argument values.

Method (SFBM, 1, NotSerialized)
{
    If ((Arg0 == Zero) { .. }
    If ((Arg0 == One)) { .. }
    If ((Arg0 == 0x02)) { .. }
}

... // DEVS
If ((IIA0 == 0x00110018))
{
   SFBM (IIA1)
   Return (One)
}

* 0x00 - is normal,
* 0x01 - is obviously turbo by the amount of noise, might be useful to
avoid CPU frequency throttling on high load,
* 0x02 - the meaning is unknown at the time as modes are not named
in the vendor documentation, but it does look like a quiet mode as CPU
temperature does increase about 10 degrees on maximum load.

Signed-off-by: Yurii Pavlovskyi <yurii.pavlovskyi@gmail.com>
---
 .../ABI/testing/sysfs-platform-asus-wmi       |  10 ++
 drivers/platform/x86/asus-wmi.c               | 119 ++++++++++++++++--
 include/linux/platform_data/x86/asus-wmi.h    |   1 +
 3 files changed, 117 insertions(+), 13 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-platform-asus-wmi b/Documentation/ABI/testing/sysfs-platform-asus-wmi
index 300a40519695..2b3184e297a7 100644
--- a/Documentation/ABI/testing/sysfs-platform-asus-wmi
+++ b/Documentation/ABI/testing/sysfs-platform-asus-wmi
@@ -97,3 +97,13 @@ Description:
 		Write changed RGB keyboard backlight parameters:
 			* 1 - permanently,
 			* 2 - temporarily.
+
+What:		/sys/devices/platform/<platform>/fan_mode
+Date:		Apr 2019
+KernelVersion:	5.1
+Contact:	"Yurii Pavlovskyi" <yurii.pavlovskyi@gmail.com>
+Description:
+		Fan boost mode:
+			* 0 - normal,
+			* 1 - turbo,
+			* 2 - quiet?
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index f4323a57f22f..941c628945ac 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -69,6 +69,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_FAN_DESC			"cpu_fan"
 #define ASUS_FAN_MFUN			0x13
@@ -77,6 +78,8 @@ MODULE_LICENSE("GPL");
 #define ASUS_FAN_CTRL_MANUAL		1
 #define ASUS_FAN_CTRL_AUTO		2
 
+#define ASUS_FAN_MODE_COUNT		3
+
 #define USB_INTEL_XUSB2PR		0xD0
 #define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI	0x9c31
 
@@ -196,6 +199,9 @@ struct asus_wmi {
 	int asus_hwmon_num_fans;
 	int asus_hwmon_pwm;
 
+	bool fan_mode_available;
+	u8 fan_mode;
+
 	bool kbbl_rgb_available;
 	struct asus_kbbl_rgb kbbl_rgb;
 
@@ -1833,6 +1839,87 @@ 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)
+		asus->fan_mode_available = true;
+
+	return 0;
+}
+
+static int fan_mode_write(struct asus_wmi *asus)
+{
+	int err;
+	u8 value;
+	u32 retval;
+
+	value = asus->fan_mode % ASUS_FAN_MODE_COUNT;
+	pr_info(PR "Set fan mode: %u\n", value);
+	err = asus_wmi_set_devstate(ASUS_WMI_DEVID_FAN_MODE, value, &retval);
+
+	if (err) {
+		pr_warn(PR "Failed to set fan mode: %d\n", err);
+		return err;
+	}
+
+	if (retval != 1) {
+		pr_warn(PR "Failed to set fan mode (retval): 0x%x\n", retval);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int fan_mode_switch_next(struct asus_wmi *asus)
+{
+	asus->fan_mode = (asus->fan_mode + 1) % ASUS_FAN_MODE_COUNT;
+	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 show_u8(asus->fan_mode, buf);
+}
+
+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 = store_u8(&new_mode, buf, count);
+	if (result < 0)
+		return result;
+
+	asus->fan_mode = new_mode % ASUS_FAN_MODE_COUNT;
+	fan_mode_write(asus);
+
+	return result;
+}
+
+// Fan mode: 0 - normal, 1 - turbo, 2 - quiet?
+static DEVICE_ATTR_RW(fan_mode);
+
 /* Backlight ******************************************************************/
 
 static int read_backlight_power(struct asus_wmi *asus)
@@ -2084,6 +2171,9 @@ static void asus_wmi_handle_notify(int code, struct asus_wmi *asus)
 		return;
 	}
 
+	if (asus->fan_mode_available && code == NOTIFY_KBD_FBM)
+		fan_mode_switch_next(asus);
+
 	if (is_display_toggle(code) && asus->driver->quirks->no_display_toggle)
 		return;
 
@@ -2237,6 +2327,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
 };
 
@@ -2258,6 +2349,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);
@@ -2282,7 +2375,7 @@ static int asus_wmi_sysfs_init(struct platform_device *device)
 
 /* Platform device ************************************************************/
 
-static int asus_wmi_platform_init(struct asus_wmi *asus)
+static void asus_wmi_platform_init(struct asus_wmi *asus)
 {
 	int rv;
 
@@ -2334,13 +2427,6 @@ static int asus_wmi_platform_init(struct asus_wmi *asus)
 	if (asus->driver->quirks->wapf >= 0)
 		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);
 }
 
 /* debugfs ********************************************************************/
@@ -2515,9 +2601,15 @@ static int asus_wmi_add(struct platform_device *pdev)
 	if (wdrv->detect_quirks)
 		wdrv->detect_quirks(asus->driver);
 
-	err = asus_wmi_platform_init(asus);
+	asus_wmi_platform_init(asus);
+
+	err = fan_mode_check_present(asus);
 	if (err)
-		goto fail_platform;
+		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)
@@ -2612,8 +2704,9 @@ static int asus_wmi_add(struct platform_device *pdev)
 fail_hwmon:
 	asus_wmi_input_exit(asus);
 fail_input:
-	asus_wmi_platform_exit(asus);
-fail_platform:
+	asus_wmi_sysfs_exit(asus->platform_device);
+fail_sysfs:
+fail_fan_mode:
 	kfree(asus);
 	return err;
 }
@@ -2630,7 +2723,7 @@ static int asus_wmi_remove(struct platform_device *device)
 	kbbl_rgb_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_wmi_hwmon_exit(asus);
 
 	kfree(asus);
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index 25b7b653e6d2..0f3654b7b8a8 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -59,6 +59,7 @@
 #define ASUS_WMI_DEVID_LIGHTBAR		0x00050025
 #define ASUS_WMI_DEVID_KBD_RGB		0x00100056
 #define ASUS_WMI_DEVID_KBD_RGB2		0x00100057
+#define ASUS_WMI_DEVID_FAN_MODE		0x00110018
 
 /* Misc */
 #define ASUS_WMI_DEVID_CAMERA		0x00060013
-- 
2.17.1


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

* [PATCH 11/11] platform/x86: asus-wmi: Do not disable keyboard backlight on unload
  2019-04-10 20:11 [PATCH 00/11] asus-wmi: Support of ASUS TUF Gaming series laptops Yurii Pavlovskyi
                   ` (9 preceding siblings ...)
  2019-04-10 20:34 ` [PATCH 10/11] platform/x86: asus-wmi: Switch fan boost mode Yurii Pavlovskyi
@ 2019-04-10 20:36 ` Yurii Pavlovskyi
  2019-04-11  5:48   ` [PATCH v2 " Yurii Pavlovskyi
  2019-04-11  5:38 ` [PATCH 00/11] asus-wmi: Support of ASUS TUF Gaming series laptops Yurii Pavlovskyi
  11 siblings, 1 reply; 29+ messages in thread
From: Yurii Pavlovskyi @ 2019-04-10 20:36 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	acpi4asus-user, platform-driver-x86, linux-kernel

The keyboard backlight is disabled when module is unloaded as it is
exposed as LED device. Change this behavior to ignore setting 0 brightness
when the ledclass device is unloading.

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 941c628945ac..a0ffdd99eae2 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -475,6 +475,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] 29+ messages in thread

* Re: [PATCH 00/11] asus-wmi: Support of ASUS TUF Gaming series laptops
  2019-04-10 20:11 [PATCH 00/11] asus-wmi: Support of ASUS TUF Gaming series laptops Yurii Pavlovskyi
                   ` (10 preceding siblings ...)
  2019-04-10 20:36 ` [PATCH 11/11] platform/x86: asus-wmi: Do not disable keyboard backlight on unload Yurii Pavlovskyi
@ 2019-04-11  5:38 ` Yurii Pavlovskyi
  11 siblings, 0 replies; 29+ messages in thread
From: Yurii Pavlovskyi @ 2019-04-11  5:38 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	acpi4asus-user, platform-driver-x86, linux-kernel

Hi,

sorry, just realized, I've broken the logging. I will re-post patches 04 to 11 as replies to original ones, 1 to 3 were ok.

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

* [PATCH v2 04/11] platform/x86: asus-wmi: Add quirk to force DSTS WMI method detection
  2019-04-10 20:28 ` [PATCH 04/11] platform/x86: asus-wmi: Add quirk to force DSTS WMI method detection Yurii Pavlovskyi
@ 2019-04-11  5:42   ` Yurii Pavlovskyi
  2019-04-11 10:55   ` [PATCH " Daniel Drake
  1 sibling, 0 replies; 29+ messages in thread
From: Yurii Pavlovskyi @ 2019-04-11  5:42 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	acpi4asus-user, platform-driver-x86, linux-kernel

The DSTS method detection fails, as nothing is returned if method is not
defined in WMNB. As a result the control of keyboard backlight is not
functional for TUF Gaming series laptops (at the time the only
functionality of the driver on this model implemented with WMI methods).

Patch was tested on a newer TUF Gaming FX505GM and older K54C model.

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

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

The non-existing method ASUS_WMI_METHODID_DSTS=0x53544344 (actually it is
DCTS in little endian ASCII) is selected in asus->dsts.

One way to fix this would be to call both for every known device ID until
some answers - this would increase module load time.

Another option is to check some device that is known to exist on every
model - none known at the time.

Last option, which is implemented, is to check for presence of the
ASUS7000 device in ACPI tree (it is a dummy device), which is the
condition used for loading the vendor driver for this model. This might
not fix every affected model ever produced, but it likely does not
introduce any regressions. The patch introduces a quirk that is enabled
when ASUS7000 is found.

Scope (_SB)
{
    Device (ATK)
    {
        Name (_HID, "ASUS7000")  // _HID: Hardware ID
    }
}

Signed-off-by: Yurii Pavlovskyi <yurii.pavlovskyi@gmail.com>
---
 drivers/platform/x86/asus-nb-wmi.c |  5 +++++
 drivers/platform/x86/asus-wmi.c    | 14 ++++++++++++--
 drivers/platform/x86/asus-wmi.h    |  5 +++++
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
index b6f2ff95c3ed..cc5f0765a8d9 100644
--- a/drivers/platform/x86/asus-nb-wmi.c
+++ b/drivers/platform/x86/asus-nb-wmi.c
@@ -28,6 +28,7 @@
 #include <linux/fb.h>
 #include <linux/dmi.h>
 #include <linux/i8042.h>
+#include <linux/acpi.h>
 
 #include "asus-wmi.h"
 
@@ -434,6 +435,10 @@ static void asus_nb_wmi_quirks(struct asus_wmi_driver *driver)
 		}
 		pr_info("Using i8042 filter function for receiving events\n");
 	}
+
+	if (acpi_dev_found("ASUS7000")) {
+		driver->quirks->force_dsts = true;
+	}
 }
 
 static const struct key_entry asus_nb_wmi_keymap[] = {
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index cfccfc0b8c2f..80f3447734fc 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -1885,11 +1885,21 @@ 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 0 for unknown
+	 * methods, so the detection in this way is not possible and method must
+	 * be forced. Likely the presence of ACPI device ASUS7000 indicates
+	 * this.
 	 */
-	if (!asus_wmi_evaluate_method(ASUS_WMI_METHODID_DSTS, 0, 0, NULL))
+	if (asus->driver->quirks->force_dsts) {
+		pr_info("DSTS method forced\n");
+		asus->dsts_id = ASUS_WMI_METHODID_DSTS2;
+	} else if (!asus_wmi_evaluate_method(ASUS_WMI_METHODID_DSTS,
+			0, 0, NULL)) {
 		asus->dsts_id = ASUS_WMI_METHODID_DSTS;
-	else
+	} 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/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
index 6c1311f4b04d..94056da02fde 100644
--- a/drivers/platform/x86/asus-wmi.h
+++ b/drivers/platform/x86/asus-wmi.h
@@ -54,6 +54,11 @@ struct quirk_entry {
 	 */
 	int no_display_toggle;
 	u32 xusb2pr;
+	/**
+	 * Force DSTS instead of DSCS and skip detection. Useful if WMNB
+	 * returns nothing on unknown method call.
+	 */
+	bool force_dsts;
 
 	bool (*i8042_filter)(unsigned char data, unsigned char str,
 			     struct serio *serio);
-- 
2.17.1


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

* [PATCH v2 05/11] platform/x86: asus-wmi: Support queued WMI event codes
  2019-04-10 20:29 ` [PATCH 05/11] platform/x86: asus-wmi: Support queued WMI event codes Yurii Pavlovskyi
@ 2019-04-11  5:44   ` Yurii Pavlovskyi
  2019-04-12  7:48     ` Daniel Drake
  0 siblings, 1 reply; 29+ messages in thread
From: Yurii Pavlovskyi @ 2019-04-11  5:44 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	acpi4asus-user, platform-driver-x86, linux-kernel

Event codes are expected to be polled from a queue on at least some
models.

The WMI event codes are pushed into queue based on circular buffer. After
INIT method is called ACPI code is allowed to push events into this buffer
the INIT method can not 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.

Patch was tested on a newer TUF Gaming FX505GM and older K54C model.

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)
}

The fix flushes the old key codes out of the queue on load and after
receiving event the queue is read until either ..FFFF or 1 is encountered.

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

Introduce functionality for flushing and processing queued codes, which is
enabled via quirk flag for ASUS7000. It might be considered if it is
reasonable to enable it everywhere (might introduce regressions) or always
try to flush the queue on module load and try to detect if this quirk is
present in the future.

This patch limits the effect to the specific hardware defined by ASUS7000
device that is used for driver detection by vendor driver of Fx505. The
fallback is also implemented in case initial flush fails.

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

diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
index cc5f0765a8d9..357d273ed336 100644
--- a/drivers/platform/x86/asus-nb-wmi.c
+++ b/drivers/platform/x86/asus-nb-wmi.c
@@ -438,6 +438,7 @@ static void asus_nb_wmi_quirks(struct asus_wmi_driver *driver)
 
 	if (acpi_dev_found("ASUS7000")) {
 		driver->quirks->force_dsts = true;
+		driver->quirks->wmi_event_queue = true;
 	}
 }
 
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 80f3447734fc..5aa30f8a2a38 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -80,6 +80,12 @@ MODULE_LICENSE("GPL");
 #define USB_INTEL_XUSB2PR		0xD0
 #define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI	0x9c31
 
+#define WMI_EVENT_QUEUE_SIZE		0x10
+#define WMI_EVENT_QUEUE_END		0x1
+#define WMI_EVENT_MASK			0xFFFF
+/* The event value is always the same. */
+#define WMI_EVENT_VALUE			0xFF
+
 static const char * const ashs_ids[] = { "ATK4001", "ATK4002", NULL };
 
 static bool ashs_present(void)
@@ -143,6 +149,7 @@ struct asus_wmi {
 	int dsts_id;
 	int spec;
 	int sfun;
+	bool wmi_event_queue;
 
 	struct input_dev *inputdev;
 	struct backlight_device *backlight_device;
@@ -1637,77 +1644,126 @@ static int is_display_toggle(int code)
 	return 0;
 }
 
-static void asus_wmi_notify(u32 value, void *context)
+static int asus_poll_wmi_event(u32 value)
 {
-	struct asus_wmi *asus = context;
-	struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL };
+	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
 	union acpi_object *obj;
 	acpi_status status;
-	int code;
-	int orig_code;
-	unsigned int key_value = 1;
-	bool autorelease = 1;
+	int code = -EIO;
 
-	status = wmi_get_event_data(value, &response);
-	if (status != AE_OK) {
-		pr_err("bad event status 0x%x\n", status);
-		return;
+	status = wmi_get_event_data(value, &output);
+	if (ACPI_FAILURE(status)) {
+		pr_warn("Failed to get WMI event code: %s\n",
+				acpi_format_exception(status));
+		return code;
 	}
 
-	obj = (union acpi_object *)response.pointer;
+	obj = (union acpi_object *)output.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);
+
+	kfree(obj);
+	return code;
+}
+
+static void asus_wmi_handle_notify(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 (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;
+	int i;
+
+	for (i = 0; i < WMI_EVENT_QUEUE_SIZE + 1; i++) {
+		code = asus_poll_wmi_event(value);
+
+		if (code < 0) {
+			pr_warn("Failed to get event code: 0x%x\n", code);
+			return;
+		}
+
+		if (code == WMI_EVENT_QUEUE_END || code == WMI_EVENT_MASK)
+			return;
+
+		asus_wmi_handle_notify(code, asus);
+
+		if (!asus->wmi_event_queue)
+			return;
+	}
+
+	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_poll_wmi_event(WMI_EVENT_VALUE);
+
+		if (code < 0) {
+			pr_warn("Failed to poll 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;
 }
 
 /*
@@ -2159,8 +2215,18 @@ static int asus_wmi_add(struct platform_device *pdev)
 		err = asus_wmi_backlight_init(asus);
 		if (err && err != -ENODEV)
 			goto fail_backlight;
-	} else
+	} else {
 		err = asus_wmi_set_devstate(ASUS_WMI_DEVID_BACKLIGHT, 2, NULL);
+	}
+
+	/** Try to initialize queue and fallback if it fails. */
+	if (asus->driver->quirks->wmi_event_queue) {
+		err = asus_wmi_notify_queue_flush(asus);
+		if (err)
+			asus->driver->quirks->wmi_event_queue = false;
+		else
+			pr_info("WMI event queue enabled\n");
+	}
 
 	status = wmi_install_notify_handler(asus->driver->event_guid,
 					    asus_wmi_notify, asus);
diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
index 94056da02fde..1248658d6442 100644
--- a/drivers/platform/x86/asus-wmi.h
+++ b/drivers/platform/x86/asus-wmi.h
@@ -54,6 +54,8 @@ struct quirk_entry {
 	 */
 	int no_display_toggle;
 	u32 xusb2pr;
+	/* Multiple event codes can be queued in buffer. */
+	bool wmi_event_queue;
 	/**
 	 * Force DSTS instead of DSCS and skip detection. Useful if WMNB
 	 * returns nothing on unknown method call.
-- 
2.17.1


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

* [PATCH v2 06/11] platform/x86: asus-nb-wmi: Add microphone mute key code
  2019-04-10 20:30 ` [PATCH 06/11] platform/x86: asus-nb-wmi: Add microphone mute key code Yurii Pavlovskyi
@ 2019-04-11  5:44   ` Yurii Pavlovskyi
  0 siblings, 0 replies; 29+ messages in thread
From: Yurii Pavlovskyi @ 2019-04-11  5:44 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	acpi4asus-user, platform-driver-x86, linux-kernel

The microphone mute key that is present on FX505GM laptop and possibly
others is missing from sparse keymap. Add the missing code.

Also comment on the fan mode switch key that 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 357d273ed336..39cf447198a9 100644
--- a/drivers/platform/x86/asus-nb-wmi.c
+++ b/drivers/platform/x86/asus-nb-wmi.c
@@ -474,6 +474,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 } },
@@ -488,7 +489,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] 29+ messages in thread

* [PATCH v2 07/11] platform/x86: asus-wmi: Organize code into sections
  2019-04-10 20:31 ` [PATCH 07/11] platform/x86: asus-wmi: Organize code into sections Yurii Pavlovskyi
@ 2019-04-11  5:45   ` Yurii Pavlovskyi
  0 siblings, 0 replies; 29+ messages in thread
From: Yurii Pavlovskyi @ 2019-04-11  5:45 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	acpi4asus-user, platform-driver-x86, linux-kernel

The driver has grown (and will more) pretty big 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 | 94 ++++++++++++++++-----------------
 1 file changed, 46 insertions(+), 48 deletions(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 5aa30f8a2a38..a98df005d6cb 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -191,6 +191,8 @@ struct asus_wmi {
 	struct asus_wmi_driver *driver;
 };
 
+/* Input **********************************************************************/
+
 static int asus_wmi_input_init(struct asus_wmi *asus)
 {
 	int err;
@@ -228,6 +230,8 @@ static void asus_wmi_input_exit(struct asus_wmi *asus)
 	asus->inputdev = NULL;
 }
 
+/* WMI ************************************************************************/
+
 static int asus_wmi_evaluate_method_3dw(u32 method_id, u32 arg0, u32 arg1,
 		u32 arg2, u32 *retval)
 {
@@ -246,7 +250,7 @@ static int asus_wmi_evaluate_method_3dw(u32 method_id, u32 arg0, u32 arg1,
 				     &input, &output);
 
 	if (ACPI_FAILURE(status))
-		goto exit;
+		return -EIO;
 
 	obj = (union acpi_object *)output.pointer;
 	if (obj && obj->type == ACPI_TYPE_INTEGER)
@@ -257,10 +261,6 @@ static int asus_wmi_evaluate_method_3dw(u32 method_id, u32 arg0, u32 arg1,
 
 	kfree(obj);
 
-exit:
-	if (ACPI_FAILURE(status))
-		return -EIO;
-
 	if (tmp == ASUS_WMI_UNSUPPORTED_METHOD)
 		return -ENODEV;
 
@@ -344,9 +344,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
@@ -656,6 +655,7 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
 	return rv;
 }
 
+/* RF *************************************************************************/
 
 /*
  * PCI hotplug (for wlan rfkill)
@@ -1078,6 +1078,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;
@@ -1110,9 +1112,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)
 {
@@ -1388,7 +1389,6 @@ static umode_t asus_hwmon_sysfs_is_visible(struct kobject *kobj,
 	else if (attr == &dev_attr_temp1_input.attr)
 		dev_id = ASUS_WMI_DEVID_THERMAL_CTRL;
 
-
 	if (attr == &dev_attr_fan1_input.attr
 	    || attr == &dev_attr_fan1_label.attr
 	    || attr == &dev_attr_pwm1.attr
@@ -1460,9 +1460,27 @@ static void asus_wmi_hwmon_exit(struct asus_wmi *asus)
 	}
 }
 
-/*
- * 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;
@@ -1644,6 +1662,8 @@ static int is_display_toggle(int code)
 	return 0;
 }
 
+/* WMI events *****************************************************************/
+
 static int asus_poll_wmi_event(u32 value)
 {
 	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
@@ -1766,9 +1786,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)
@@ -1907,9 +1926,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)
 {
 	int rv;
@@ -1971,9 +1989,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;
@@ -2120,28 +2137,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;
-
-	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;
-}
+/* Init / exit ****************************************************************/
 
-/*
- * WMI Driver
- */
 static int asus_wmi_add(struct platform_device *pdev)
 {
 	struct platform_driver *pdrv = to_platform_driver(pdev->dev.driver);
@@ -2279,9 +2276,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);
@@ -2353,6 +2349,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] 29+ messages in thread

* [PATCH v2 08/11] platform/x86: asus-wmi: Enhance detection of thermal data
  2019-04-10 20:32 ` [PATCH 08/11] platform/x86: asus-wmi: Enhance detection of thermal data Yurii Pavlovskyi
@ 2019-04-11  5:45   ` Yurii Pavlovskyi
  0 siblings, 0 replies; 29+ messages in thread
From: Yurii Pavlovskyi @ 2019-04-11  5:45 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	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 as erroneous value in addition to 0 K.
* Refactor detection of thermal input availability to a separate function.

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

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index a98df005d6cb..de0a8f61d4a1 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -176,6 +176,7 @@ struct asus_wmi {
 	struct asus_rfkill gps;
 	struct asus_rfkill uwb;
 
+	bool asus_hwmon_thermal_available;
 	bool asus_hwmon_fan_manual_mode;
 	int asus_hwmon_num_fans;
 	int asus_hwmon_pwm;
@@ -1373,6 +1374,32 @@ static struct attribute *hwmon_attributes[] = {
 	NULL
 };
 
+static int asus_hwmon_check_thermal_available(struct asus_wmi *asus)
+{
+	u32 value = ASUS_WMI_UNSUPPORTED_METHOD;
+	int err;
+
+	asus->asus_hwmon_thermal_available = false;
+	err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_THERMAL_CTRL, &value);
+
+	if (err < 0) {
+		if (err == -ENODEV)
+			return 0;
+
+		return err;
+	}
+
+	/*
+	 * If the temperature value in deci-Kelvin is near the absolute
+	 * zero temperature, something is clearly wrong.
+	 */
+	if (!value || value == 1)
+		return 0;
+
+	asus->asus_hwmon_thermal_available = true;
+	return 0;
+}
+
 static umode_t asus_hwmon_sysfs_is_visible(struct kobject *kobj,
 					  struct attribute *attr, int idx)
 {
@@ -1386,8 +1413,6 @@ static umode_t asus_hwmon_sysfs_is_visible(struct kobject *kobj,
 
 	if (attr == &dev_attr_pwm1.attr)
 		dev_id = ASUS_WMI_DEVID_FAN_CTRL;
-	else if (attr == &dev_attr_temp1_input.attr)
-		dev_id = ASUS_WMI_DEVID_THERMAL_CTRL;
 
 	if (attr == &dev_attr_fan1_input.attr
 	    || attr == &dev_attr_fan1_label.attr
@@ -1412,15 +1437,13 @@ static umode_t asus_hwmon_sysfs_is_visible(struct kobject *kobj,
 		 * - reverved bits are non-zero
 		 * - sfun and presence bit are not set
 		 */
-		if (value == ASUS_WMI_UNSUPPORTED_METHOD || value & 0xFFF80000
+		if (value == ASUS_WMI_UNSUPPORTED_METHOD || (value & 0xFFF80000)
 		    || (!asus->sfun && !(value & ASUS_WMI_DSTS_PRESENCE_BIT)))
 			ok = false;
 		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)
-			ok = false;
+	} else if (attr == &dev_attr_temp1_input.attr) {
+		ok = asus->asus_hwmon_thermal_available;
 	} else if (fan_attr <= asus->asus_hwmon_num_fans && fan_attr != -1) {
 		ok = true;
 	} else {
@@ -1476,6 +1499,14 @@ static int asus_wmi_fan_init(struct asus_wmi *asus)
 	}
 
 	pr_info("Number of fans: %d\n", asus->asus_hwmon_num_fans);
+
+	status = asus_hwmon_check_thermal_available(asus);
+	if (status) {
+		pr_warn("Could not check if thermal available: %d\n", status);
+		return -ENXIO;
+	}
+
+	pr_info("Thermal available: %d\n", asus->asus_hwmon_thermal_available);
 	return 0;
 }
 
-- 
2.17.1


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

* [PATCH v2 09/11] platform/x86: asus-wmi: Control RGB keyboard backlight
  2019-04-10 20:33 ` [PATCH 09/11] platform/x86: asus-wmi: Control RGB keyboard backlight Yurii Pavlovskyi
@ 2019-04-11  5:46   ` Yurii Pavlovskyi
  0 siblings, 0 replies; 29+ messages in thread
From: Yurii Pavlovskyi @ 2019-04-11  5:46 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	acpi4asus-user, platform-driver-x86, linux-kernel, linux-api

The WMI exposes two methods for controlling RGB keyboard backlight which
allow to control:
* RGB components in range 00 - ff,
* Switch between 4 effects,
* Switch between 3 effect speed modes,
* Separately enable the backlight on boot, in awake state (after driver
  load), in sleep mode, and probably in something called shutdown mode
  (no observable effects of enabling it are known so far).

The configuration should be written to several sysfs parameter buffers
which are then written via WMI by writing either 1 or 2 to the "kbbl_set"
parameter. When reading the buffers the last written value is returned.

If the 2 is written to "kbbl_set", the parameters will be reset on reboot
(temporary mode), 1 is permanent mode, parameters are retained.

The calls use new 3-dword input buffer method call.

The functionality is only enabled if corresponding DSTS methods return
exact valid values.

The following script demonstrates usage:

echo Red [00 - ff]
echo 33 > /sys/devices/platform/asus-nb-wmi/kbbl/kbbl_red
echo Green [00 - ff]
echo ff > /sys/devices/platform/asus-nb-wmi/kbbl/kbbl_green
echo Blue [00 - ff]
echo 0 > /sys/devices/platform/asus-nb-wmi/kbbl/kbbl_blue
echo Mode: 0 - static color, 1 - blink, 2 - rainbow, 3 - strobe
echo 0 > /sys/devices/platform/asus-nb-wmi/kbbl/kbbl_mode
echo Speed for modes 1 and 2: 0 - slow, 1 - medium, 2 - fast
echo 0 > /sys/devices/platform/asus-nb-wmi/kbbl/kbbl_speed
echo Enable: 02 - on boot, before module load, 08 - awake, 20 - sleep,
echo 2a or ff to set all
echo 2a > /sys/devices/platform/asus-nb-wmi/kbbl/kbbl_flags
echo Save: 1 - permanently, 2 - temporarily, reset after reboot
echo 1 > /sys/devices/platform/asus-nb-wmi/kbbl/kbbl_set

Signed-off-by: Yurii Pavlovskyi <yurii.pavlovskyi@gmail.com>
---
 .../ABI/testing/sysfs-platform-asus-wmi       |  61 ++++
 drivers/platform/x86/asus-wmi.c               | 329 ++++++++++++++++++
 include/linux/platform_data/x86/asus-wmi.h    |   2 +
 3 files changed, 392 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-platform-asus-wmi b/Documentation/ABI/testing/sysfs-platform-asus-wmi
index 019e1e29370e..300a40519695 100644
--- a/Documentation/ABI/testing/sysfs-platform-asus-wmi
+++ b/Documentation/ABI/testing/sysfs-platform-asus-wmi
@@ -36,3 +36,64 @@ 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>/kbbl/kbbl_red
+Date:		Apr 2019
+KernelVersion:	5.1
+Contact:	"Yurii Pavlovskyi" <yurii.pavlovskyi@gmail.com>
+Description:
+		RGB keyboard backlight red component: 00 .. ff.
+
+What:		/sys/devices/platform/<platform>/kbbl/kbbl_green
+Date:		Apr 2019
+KernelVersion:	5.1
+Contact:	"Yurii Pavlovskyi" <yurii.pavlovskyi@gmail.com>
+Description:
+		RGB keyboard backlight green component: 00 .. ff.
+
+What:		/sys/devices/platform/<platform>/kbbl/kbbl_blue
+Date:		Apr 2019
+KernelVersion:	5.1
+Contact:	"Yurii Pavlovskyi" <yurii.pavlovskyi@gmail.com>
+Description:
+		RGB keyboard backlight blue component: 00 .. ff.
+
+What:		/sys/devices/platform/<platform>/kbbl/kbbl_mode
+Date:		Apr 2019
+KernelVersion:	5.1
+Contact:	"Yurii Pavlovskyi" <yurii.pavlovskyi@gmail.com>
+Description:
+		RGB keyboard backlight mode:
+			* 0 - static color,
+			* 1 - blink,
+			* 2 - rainbow,
+			* 3 - strobe.
+
+What:		/sys/devices/platform/<platform>/kbbl/kbbl_speed
+Date:		Apr 2019
+KernelVersion:	5.1
+Contact:	"Yurii Pavlovskyi" <yurii.pavlovskyi@gmail.com>
+Description:
+		RGB keyboard backlight speed for modes 1 and 2:
+			* 0 - slow,
+			* 1 - medium,
+			* 2 - fast.
+
+What:		/sys/devices/platform/<platform>/kbbl/kbbl_flags
+Date:		Apr 2019
+KernelVersion:	5.1
+Contact:	"Yurii Pavlovskyi" <yurii.pavlovskyi@gmail.com>
+Description:
+		RGB keyboard backlight enable flags (2a to enable everything), OR of:
+			* 02 - on boot (until module load),
+			* 08 - awake,
+			* 20 - sleep.
+
+What:		/sys/devices/platform/<platform>/kbbl/kbbl_set
+Date:		Apr 2019
+KernelVersion:	5.1
+Contact:	"Yurii Pavlovskyi" <yurii.pavlovskyi@gmail.com>
+Description:
+		Write changed RGB keyboard backlight parameters:
+			* 1 - permanently,
+			* 2 - temporarily.
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index de0a8f61d4a1..b4fd200e8335 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -145,6 +145,21 @@ struct asus_rfkill {
 	u32 dev_id;
 };
 
+struct asus_kbbl_rgb {
+	u8 kbbl_red;
+	u8 kbbl_green;
+	u8 kbbl_blue;
+	u8 kbbl_mode;
+	u8 kbbl_speed;
+
+	u8 kbbl_set_red;
+	u8 kbbl_set_green;
+	u8 kbbl_set_blue;
+	u8 kbbl_set_mode;
+	u8 kbbl_set_speed;
+	u8 kbbl_set_flags;
+};
+
 struct asus_wmi {
 	int dsts_id;
 	int spec;
@@ -181,6 +196,9 @@ struct asus_wmi {
 	int asus_hwmon_num_fans;
 	int asus_hwmon_pwm;
 
+	bool kbbl_rgb_available;
+	struct asus_kbbl_rgb kbbl_rgb;
+
 	struct hotplug_slot hotplug_slot;
 	struct mutex hotplug_lock;
 	struct mutex wmi_lock;
@@ -656,6 +674,310 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
 	return rv;
 }
 
+/* RGB keyboard backlight *****************************************************/
+
+static ssize_t show_u8(u8 value, char *buf)
+{
+	return scnprintf(buf, PAGE_SIZE, "%02x\n", value);
+}
+
+static ssize_t store_u8(u8 *value, const char *buf, int count)
+{
+	int err;
+	u8 result;
+
+	err = kstrtou8(buf, 16, &result);
+	if (err < 0) {
+		pr_warn("Trying to store invalid value\n");
+		return err;
+	}
+
+	*value = result;
+
+	return count;
+}
+
+static ssize_t kbbl_red_show(struct device *dev, struct device_attribute *attr,
+		char *buf)
+{
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	return show_u8(asus->kbbl_rgb.kbbl_red, buf);
+}
+
+static ssize_t kbbl_red_store(struct device *dev, struct device_attribute *attr,
+		const char *buf, size_t count)
+{
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	return store_u8(&asus->kbbl_rgb.kbbl_set_red, buf, count);
+}
+
+static ssize_t kbbl_green_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	return show_u8(asus->kbbl_rgb.kbbl_green, buf);
+}
+
+static ssize_t kbbl_green_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	return store_u8(&asus->kbbl_rgb.kbbl_set_green, buf, count);
+}
+
+static ssize_t kbbl_blue_show(struct device *dev, struct device_attribute *attr,
+		char *buf)
+{
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	return show_u8(asus->kbbl_rgb.kbbl_blue, buf);
+}
+
+static ssize_t kbbl_blue_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	return store_u8(&asus->kbbl_rgb.kbbl_set_blue, buf, count);
+}
+
+static ssize_t kbbl_mode_show(struct device *dev, struct device_attribute *attr,
+		char *buf)
+{
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	return show_u8(asus->kbbl_rgb.kbbl_mode, buf);
+}
+
+static ssize_t kbbl_mode_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	return store_u8(&asus->kbbl_rgb.kbbl_set_mode, buf, count);
+}
+
+static ssize_t kbbl_speed_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	return show_u8(asus->kbbl_rgb.kbbl_speed, buf);
+}
+
+static ssize_t kbbl_speed_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	return store_u8(&asus->kbbl_rgb.kbbl_set_speed, buf, count);
+}
+
+static ssize_t kbbl_flags_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	return show_u8(asus->kbbl_rgb.kbbl_set_flags, buf);
+}
+
+static ssize_t kbbl_flags_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct asus_wmi *asus = dev_get_drvdata(dev);
+
+	return store_u8(&asus->kbbl_rgb.kbbl_set_flags, buf, count);
+}
+
+static ssize_t kbbl_set_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	return scnprintf(buf, PAGE_SIZE,
+			"Write to configure RGB keyboard backlight\n");
+}
+
+static int kbbl_rgb_write(struct asus_wmi *asus, int persistent)
+{
+	int err;
+	u32 retval;
+	u8 speed_byte;
+	u8 mode_byte;
+	u8 speed;
+	u8 mode;
+
+	speed = asus->kbbl_rgb.kbbl_set_speed;
+	switch (speed) {
+	case 0:
+	default:
+		speed_byte = 0xe1; // slow
+		speed = 0;
+		break;
+	case 1:
+		speed_byte = 0xeb; // medium
+		break;
+	case 2:
+		speed_byte = 0xf5; // fast
+		break;
+	}
+
+	mode = asus->kbbl_rgb.kbbl_set_mode;
+	switch (mode) {
+	case 0:
+	default:
+		mode_byte = 0x00; // static color
+		mode = 0;
+		break;
+	case 1:
+		mode_byte = 0x01; // blink
+		break;
+	case 2:
+		mode_byte = 0x02; // rainbow
+		break;
+	case 3:
+		mode_byte = 0x0a; // strobe
+		break;
+	}
+
+	err = asus_wmi_evaluate_method_3dw(ASUS_WMI_METHODID_DEVS,
+		ASUS_WMI_DEVID_KBD_RGB,
+		(persistent ? 0xb4 : 0xb3) |
+		(mode_byte << 8) |
+		(asus->kbbl_rgb.kbbl_set_red << 16) |
+		(asus->kbbl_rgb.kbbl_set_green << 24),
+		(asus->kbbl_rgb.kbbl_set_blue) |
+		(speed_byte << 8), &retval);
+	if (err) {
+		pr_warn("RGB keyboard device 1, write error: %d\n", err);
+		return err;
+	}
+
+	if (retval != 1) {
+		pr_warn("RGB keyboard device 1, write error (retval): %x\n",
+				retval);
+		return -EIO;
+	}
+
+	err = asus_wmi_evaluate_method_3dw(ASUS_WMI_METHODID_DEVS,
+		ASUS_WMI_DEVID_KBD_RGB2,
+		(0xbd) |
+		(asus->kbbl_rgb.kbbl_set_flags << 16) |
+		(persistent ? 0x0100 : 0x0000), 0, &retval);
+	if (err) {
+		pr_warn("RGB keyboard device 2, write error: %d\n", err);
+		return err;
+	}
+
+	if (retval != 1) {
+		pr_warn("RGB keyboard device 2, write error (retval): %x\n",
+				retval);
+		return -EIO;
+	}
+
+	asus->kbbl_rgb.kbbl_red = asus->kbbl_rgb.kbbl_set_red;
+	asus->kbbl_rgb.kbbl_green = asus->kbbl_rgb.kbbl_set_green;
+	asus->kbbl_rgb.kbbl_blue = asus->kbbl_rgb.kbbl_set_blue;
+	asus->kbbl_rgb.kbbl_mode = mode;
+	asus->kbbl_rgb.kbbl_speed = speed;
+
+	return 0;
+}
+
+static ssize_t kbbl_set_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	u8 value;
+	struct asus_wmi *asus;
+	int result;
+
+	asus = dev_get_drvdata(dev);
+	result = store_u8(&value, buf, count);
+	if (result < 0)
+		return result;
+
+	if (value == 1)
+		kbbl_rgb_write(asus, 1);
+	else if (value == 2)
+		kbbl_rgb_write(asus, 0);
+
+	return count;
+}
+
+/* RGB values: 00 .. ff */
+static DEVICE_ATTR_RW(kbbl_red);
+static DEVICE_ATTR_RW(kbbl_green);
+static DEVICE_ATTR_RW(kbbl_blue);
+
+/* Color modes: 0 - static color, 1 - blink, 2 - rainbow, 3 - strobe */
+static DEVICE_ATTR_RW(kbbl_mode);
+
+/* Speed for modes 1 and 2: 0 - slow, 1 - medium, 2 - fast */
+static DEVICE_ATTR_RW(kbbl_speed);
+
+/*
+ * Enable: 02 - on boot (until module load) | 08 - awake | 20 - sleep
+ * (2a or ff to enable everything)
+ *
+ * Logically 80 would be shutdown, but no visible effects of this option
+ * were observed so far
+ */
+static DEVICE_ATTR_RW(kbbl_flags);
+
+/* Write data: 1 - permanently, 2 - temporarily (reset after reboot) */
+static DEVICE_ATTR_RW(kbbl_set);
+
+static struct attribute *rgbkb_sysfs_attributes[] = {
+	&dev_attr_kbbl_red.attr,
+	&dev_attr_kbbl_green.attr,
+	&dev_attr_kbbl_blue.attr,
+	&dev_attr_kbbl_mode.attr,
+	&dev_attr_kbbl_speed.attr,
+	&dev_attr_kbbl_flags.attr,
+	&dev_attr_kbbl_set.attr,
+	NULL,
+};
+
+static const struct attribute_group kbbl_attribute_group = {
+	.name = "kbbl",
+	.attrs = rgbkb_sysfs_attributes
+};
+
+static int kbbl_rgb_init(struct asus_wmi *asus)
+{
+	int err;
+
+	err = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_KBD_RGB);
+	if (err) {
+		if (err == -ENODEV)
+			return 0;
+		else
+			return err;
+	}
+
+	err = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_KBD_RGB2);
+	if (err) {
+		if (err == -ENODEV)
+			return 0;
+		else
+			return err;
+	}
+
+	asus->kbbl_rgb_available = true;
+	return sysfs_create_group(&asus->platform_device->dev.kobj,
+			&kbbl_attribute_group);
+}
+
+static void kbbl_rgb_exit(struct asus_wmi *asus)
+{
+	if (asus->kbbl_rgb_available) {
+		sysfs_remove_group(&asus->platform_device->dev.kobj,
+				&kbbl_attribute_group);
+	}
+}
+
 /* RF *************************************************************************/
 
 /*
@@ -2211,6 +2533,10 @@ static int asus_wmi_add(struct platform_device *pdev)
 	if (err)
 		goto fail_leds;
 
+	err = kbbl_rgb_init(asus);
+	if (err)
+		goto fail_rgbkb;
+
 	asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_WLAN, &result);
 	if (result & (ASUS_WMI_DSTS_PRESENCE_BIT | ASUS_WMI_DSTS_USER_BIT))
 		asus->driver->wlan_ctrl_by_user = 1;
@@ -2277,6 +2603,8 @@ static int asus_wmi_add(struct platform_device *pdev)
 fail_backlight:
 	asus_wmi_rfkill_exit(asus);
 fail_rfkill:
+	kbbl_rgb_exit(asus);
+fail_rgbkb:
 	asus_wmi_led_exit(asus);
 fail_leds:
 	asus_wmi_hwmon_exit(asus);
@@ -2298,6 +2626,7 @@ static int asus_wmi_remove(struct platform_device *device)
 	asus_wmi_backlight_exit(asus);
 	asus_wmi_input_exit(asus);
 	asus_wmi_led_exit(asus);
+	kbbl_rgb_exit(asus);
 	asus_wmi_rfkill_exit(asus);
 	asus_wmi_debugfs_exit(asus);
 	asus_wmi_platform_exit(asus);
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index 53dfc2541960..25b7b653e6d2 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -57,6 +57,8 @@
 #define ASUS_WMI_DEVID_KBD_BACKLIGHT	0x00050021
 #define ASUS_WMI_DEVID_LIGHT_SENSOR	0x00050022 /* ?? */
 #define ASUS_WMI_DEVID_LIGHTBAR		0x00050025
+#define ASUS_WMI_DEVID_KBD_RGB		0x00100056
+#define ASUS_WMI_DEVID_KBD_RGB2		0x00100057
 
 /* Misc */
 #define ASUS_WMI_DEVID_CAMERA		0x00060013
-- 
2.17.1


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

* [PATCH v2 10/11] platform/x86: asus-wmi: Switch fan boost mode
  2019-04-10 20:34 ` [PATCH 10/11] platform/x86: asus-wmi: Switch fan boost mode Yurii Pavlovskyi
@ 2019-04-11  5:47   ` Yurii Pavlovskyi
  2019-04-12  8:03     ` Daniel Drake
  0 siblings, 1 reply; 29+ messages in thread
From: Yurii Pavlovskyi @ 2019-04-11  5:47 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	acpi4asus-user, platform-driver-x86, linux-kernel, linux-api

The WMI exposes a write-only device ID where three 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 these 3
modes.

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. The
corresponding DEVS device handler does obviously take 3 possible
argument values.

Method (SFBM, 1, NotSerialized)
{
    If ((Arg0 == Zero) { .. }
    If ((Arg0 == One)) { .. }
    If ((Arg0 == 0x02)) { .. }
}

... // DEVS
If ((IIA0 == 0x00110018))
{
   SFBM (IIA1)
   Return (One)
}

* 0x00 - is normal,
* 0x01 - is obviously turbo by the amount of noise, might be useful to
avoid CPU frequency throttling on high load,
* 0x02 - the meaning is unknown at the time as modes are not named
in the vendor documentation, but it does look like a quiet mode as CPU
temperature does increase about 10 degrees on maximum load.

Signed-off-by: Yurii Pavlovskyi <yurii.pavlovskyi@gmail.com>
---
 .../ABI/testing/sysfs-platform-asus-wmi       |  10 ++
 drivers/platform/x86/asus-wmi.c               | 119 ++++++++++++++++--
 include/linux/platform_data/x86/asus-wmi.h    |   1 +
 3 files changed, 117 insertions(+), 13 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-platform-asus-wmi b/Documentation/ABI/testing/sysfs-platform-asus-wmi
index 300a40519695..2b3184e297a7 100644
--- a/Documentation/ABI/testing/sysfs-platform-asus-wmi
+++ b/Documentation/ABI/testing/sysfs-platform-asus-wmi
@@ -97,3 +97,13 @@ Description:
 		Write changed RGB keyboard backlight parameters:
 			* 1 - permanently,
 			* 2 - temporarily.
+
+What:		/sys/devices/platform/<platform>/fan_mode
+Date:		Apr 2019
+KernelVersion:	5.1
+Contact:	"Yurii Pavlovskyi" <yurii.pavlovskyi@gmail.com>
+Description:
+		Fan boost mode:
+			* 0 - normal,
+			* 1 - turbo,
+			* 2 - quiet?
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index b4fd200e8335..f0e506feb924 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -69,6 +69,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_FAN_DESC			"cpu_fan"
 #define ASUS_FAN_MFUN			0x13
@@ -77,6 +78,8 @@ MODULE_LICENSE("GPL");
 #define ASUS_FAN_CTRL_MANUAL		1
 #define ASUS_FAN_CTRL_AUTO		2
 
+#define ASUS_FAN_MODE_COUNT		3
+
 #define USB_INTEL_XUSB2PR		0xD0
 #define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI	0x9c31
 
@@ -196,6 +199,9 @@ struct asus_wmi {
 	int asus_hwmon_num_fans;
 	int asus_hwmon_pwm;
 
+	bool fan_mode_available;
+	u8 fan_mode;
+
 	bool kbbl_rgb_available;
 	struct asus_kbbl_rgb kbbl_rgb;
 
@@ -1832,6 +1838,87 @@ 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)
+		asus->fan_mode_available = true;
+
+	return 0;
+}
+
+static int fan_mode_write(struct asus_wmi *asus)
+{
+	int err;
+	u8 value;
+	u32 retval;
+
+	value = asus->fan_mode % ASUS_FAN_MODE_COUNT;
+	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)
+{
+	asus->fan_mode = (asus->fan_mode + 1) % ASUS_FAN_MODE_COUNT;
+	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 show_u8(asus->fan_mode, buf);
+}
+
+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 = store_u8(&new_mode, buf, count);
+	if (result < 0)
+		return result;
+
+	asus->fan_mode = new_mode % ASUS_FAN_MODE_COUNT;
+	fan_mode_write(asus);
+
+	return result;
+}
+
+// Fan mode: 0 - normal, 1 - turbo, 2 - quiet?
+static DEVICE_ATTR_RW(fan_mode);
+
 /* Backlight ******************************************************************/
 
 static int read_backlight_power(struct asus_wmi *asus)
@@ -2083,6 +2170,9 @@ static void asus_wmi_handle_notify(int code, struct asus_wmi *asus)
 		return;
 	}
 
+	if (asus->fan_mode_available && code == NOTIFY_KBD_FBM)
+		fan_mode_switch_next(asus);
+
 	if (is_display_toggle(code) && asus->driver->quirks->no_display_toggle)
 		return;
 
@@ -2236,6 +2326,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
 };
 
@@ -2257,6 +2348,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);
@@ -2281,7 +2374,7 @@ static int asus_wmi_sysfs_init(struct platform_device *device)
 
 /* Platform device ************************************************************/
 
-static int asus_wmi_platform_init(struct asus_wmi *asus)
+static void asus_wmi_platform_init(struct asus_wmi *asus)
 {
 	int rv;
 
@@ -2333,13 +2426,6 @@ static int asus_wmi_platform_init(struct asus_wmi *asus)
 	if (asus->driver->quirks->wapf >= 0)
 		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);
 }
 
 /* debugfs ********************************************************************/
@@ -2514,9 +2600,15 @@ static int asus_wmi_add(struct platform_device *pdev)
 	if (wdrv->detect_quirks)
 		wdrv->detect_quirks(asus->driver);
 
-	err = asus_wmi_platform_init(asus);
+	asus_wmi_platform_init(asus);
+
+	err = fan_mode_check_present(asus);
 	if (err)
-		goto fail_platform;
+		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)
@@ -2611,8 +2703,9 @@ static int asus_wmi_add(struct platform_device *pdev)
 fail_hwmon:
 	asus_wmi_input_exit(asus);
 fail_input:
-	asus_wmi_platform_exit(asus);
-fail_platform:
+	asus_wmi_sysfs_exit(asus->platform_device);
+fail_sysfs:
+fail_fan_mode:
 	kfree(asus);
 	return err;
 }
@@ -2629,7 +2722,7 @@ static int asus_wmi_remove(struct platform_device *device)
 	kbbl_rgb_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_wmi_hwmon_exit(asus);
 
 	kfree(asus);
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index 25b7b653e6d2..0f3654b7b8a8 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -59,6 +59,7 @@
 #define ASUS_WMI_DEVID_LIGHTBAR		0x00050025
 #define ASUS_WMI_DEVID_KBD_RGB		0x00100056
 #define ASUS_WMI_DEVID_KBD_RGB2		0x00100057
+#define ASUS_WMI_DEVID_FAN_MODE		0x00110018
 
 /* Misc */
 #define ASUS_WMI_DEVID_CAMERA		0x00060013
-- 
2.17.1


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

* [PATCH v2 11/11] platform/x86: asus-wmi: Do not disable keyboard backlight on unload
  2019-04-10 20:36 ` [PATCH 11/11] platform/x86: asus-wmi: Do not disable keyboard backlight on unload Yurii Pavlovskyi
@ 2019-04-11  5:48   ` Yurii Pavlovskyi
  0 siblings, 0 replies; 29+ messages in thread
From: Yurii Pavlovskyi @ 2019-04-11  5:48 UTC (permalink / raw)
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
	acpi4asus-user, platform-driver-x86, linux-kernel

The keyboard backlight is disabled when module is unloaded as it is
exposed as LED device. Change this behavior to ignore setting 0 brightness
when the ledclass device is unloading.

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 f0e506feb924..f49992fa87b3 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -475,6 +475,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] 29+ messages in thread

* Re: [PATCH 01/11] platform/x86: asus-wmi: Fix hwmon device cleanup
  2019-04-10 20:20 ` [PATCH 01/11] platform/x86: asus-wmi: Fix hwmon device cleanup Yurii Pavlovskyi
@ 2019-04-11  8:21   ` Daniel Drake
  2019-04-12 17:49     ` Yurii Pavlovskyi
  0 siblings, 1 reply; 29+ messages in thread
From: Daniel Drake @ 2019-04-11  8:21 UTC (permalink / raw)
  To: Yurii Pavlovskyi
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, acpi4asus-user,
	Platform Driver, Linux Kernel

On Thu, Apr 11, 2019 at 4:21 AM Yurii Pavlovskyi
<yurii.pavlovskyi@gmail.com> wrote:
>
> The asus-wmi 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. Add
> pointer to the device in module state and call cleanup on error.

I wonder if this can be fixed more cleanly by using
devm_hwmon_device_register_with_groups() ?

Thanks
Daniel

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

* Re: [PATCH 04/11] platform/x86: asus-wmi: Add quirk to force DSTS WMI method detection
  2019-04-10 20:28 ` [PATCH 04/11] platform/x86: asus-wmi: Add quirk to force DSTS WMI method detection Yurii Pavlovskyi
  2019-04-11  5:42   ` [PATCH v2 " Yurii Pavlovskyi
@ 2019-04-11 10:55   ` Daniel Drake
  2019-04-12 20:04     ` Yurii Pavlovskyi
  1 sibling, 1 reply; 29+ messages in thread
From: Daniel Drake @ 2019-04-11 10:55 UTC (permalink / raw)
  To: Yurii Pavlovskyi
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, acpi4asus-user,
	Platform Driver, Linux Kernel, Endless Linux Upstreaming Team

On Thu, Apr 11, 2019 at 4:28 AM Yurii Pavlovskyi
<yurii.pavlovskyi@gmail.com> wrote:
> The DSTS method detection fails, as nothing is returned if method is not
> defined in WMNB. As a result the control of keyboard backlight is not
> functional for TUF Gaming series laptops (at the time the only
> functionality of the driver on this model implemented with WMI methods).
>
> Patch was tested on a newer TUF Gaming FX505GM and older K54C model.
>
> FX505GM:
> Method (WMNB, 3, Serialized)
> { ...
>     If ((Local0 == 0x53545344))
>     {
>         ...
>         Return (Zero)
>     }
>     ...
>     // No return
> }
>
> K54C:
> Method (WMNB, 3, Serialized)
> { ...
>     If ((Local0 == 0x53545344))
>     {
>         ...
>         Return (0x02)
>     }
>     ...
>     Return (0xFFFFFFFE)
> }
>
> The non-existing method ASUS_WMI_METHODID_DSTS=0x53544344 (actually it is
> DCTS in little endian ASCII) is selected in asus->dsts.
>
> One way to fix this would be to call both for every known device ID until
> some answers - this would increase module load time.
>
> Another option is to check some device that is known to exist on every
> model - none known at the time.
>
> Last option, which is implemented, is to check for presence of the
> ASUS7000 device in ACPI tree (it is a dummy device), which is the
> condition used for loading the vendor driver for this model. This might
> not fix every affected model ever produced, but it likely does not
> introduce any regressions. The patch introduces a quirk that is enabled
> when ASUS7000 is found.
>
> Scope (_SB)
> {
>     Device (ATK)
>     {
>         Name (_HID, "ASUS7000")  // _HID: Hardware ID
>     }
> }

Hmm, tricky! But about time we did something about the DSTS vs DCTS guessing.

While we don't have definitive knowledge of the right thing to do
here, I think I have enough info available to solidify some
assumptions we'd otherwise be afraid to make, and then we can
implement a better approach here.

In our database of 144 Asus DSDTs, 14 of them respond to DCTS:

AS_D520MT
D425MC
D640SA
D320SF-K
D415MT
D830MT
G11DF
M32CD4-K
V221ID
V272UN_SKU3
Z240IE
ZN220IC-K
ZN241IC
ZN270IE

Of those 14, they all additionally respond to DSTS, except for D415MT
and AS_D520MT.

In all 14 cases, the DCTS handling is done inside a device with _UID
ASUSWMI. None of the other 130 products have a device with that _UID.

Furthermore, we recently received access to the ASUS spec, which
confirms that the Instance Name for EeePC is "ACPI\PNP0C14\ASUSWMI_0"
whereas the Instance Name for other notebooks is "ACPI\PNP0C14\ATK_0".

The 12 devices that respond to both DCTS and DSTS, do it on separate
different devices, albeit with the same _WDG UUID. The one with _UID
ASUSWMI responds to DCTS, and the one with _UID ATK responds to DSTS.

So that seems fairly convincing. My thinking is that we can check the
_UID of the underlying device, and use DCTS for ASUSWMI, DSTS
otherwise. To do that, I think you'd have to rework the driver probing
so that it happens through wmi_driver_register(). Then from the probe
function onwards you will get a wmi_device, and hopefully you can get
the _UID through that instance somehow.

There's a separate issue of what happens on those 12 machines where
there are two WMI devs, with the same _WDG GUID.
drivers/platform/x86/wmi.c drops duplicates, so one of them is being
ignored in that case. We'd ideally find a way to ignore the ASUSWMI
node and go with ATK in that situation. But I think this can be
separated from your work here.

Thanks for these patches and welcome to the world of kernel development!

Daniel

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

* Re: [PATCH v2 05/11] platform/x86: asus-wmi: Support queued WMI event codes
  2019-04-11  5:44   ` [PATCH v2 " Yurii Pavlovskyi
@ 2019-04-12  7:48     ` Daniel Drake
  2019-04-12 20:32       ` Yurii Pavlovskyi
  0 siblings, 1 reply; 29+ messages in thread
From: Daniel Drake @ 2019-04-12  7:48 UTC (permalink / raw)
  To: Yurii Pavlovskyi
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, acpi4asus-user,
	Platform Driver, Linux Kernel, Endless Linux Upstreaming Team

On Thu, Apr 11, 2019 at 1:44 PM Yurii Pavlovskyi
<yurii.pavlovskyi@gmail.com> wrote:
> Event codes are expected to be polled from a queue on at least some
> models.

Maybe avoid the word "poll" since it suggests something else (at least to me).

> The fix flushes the old key codes out of the queue on load and after
> receiving event the queue is read until either ..FFFF or 1 is encountered.
>
> It might be considered a minor issue and no normal user would likely to
> observe this (there is little reason unloading the driver), but it does
> significantly frustrate a developer who is unlucky enough to encounter
> this.
>
> Introduce functionality for flushing and processing queued codes, which is
> enabled via quirk flag for ASUS7000. It might be considered if it is
> reasonable to enable it everywhere (might introduce regressions) or always
> try to flush the queue on module load and try to detect if this quirk is
> present in the future.
>
> This patch limits the effect to the specific hardware defined by ASUS7000
> device that is used for driver detection by vendor driver of Fx505. The
> fallback is also implemented in case initial flush fails.

Which vendor driver are you referring to here?

Researching more:
In our database of 144 Asus models, 142 of them have GANQ.

Of those 142, 9 of them return One in the empty-queue case. The other
133 match your FX505GM device exactly. So it seems valid to interpret
both 0xffff and 0x1 as queue-end markers.

The 2 devices that do not have GANQ are not laptops. They also do not
have the _UID "ATK" WMI device, they only have "ASUSWMI" and their WMI
_WED method does not use a queue.
There are a few more units that have both ASUSWMI and ATK devices, and
the ASUSWMI device appears to never be queued.
Another observation is that the ASUSWMI device works with notify code
0xD2, and the ATK device works with 0xFF.

Nailing this down a bit further, I found a DSDT for EEEPC X101H: that
one only has ASUSWMI and it is also not queued.

So I think you should make this queue behaviour applied more
generically, but either avoid it when the WMI device _UID is not "ATK"
(as discussed in the DCTS/DSTS thread), or avoid it when the notify
code is not 0xFF.

Thanks
Daniel

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

* Re: [PATCH v2 10/11] platform/x86: asus-wmi: Switch fan boost mode
  2019-04-11  5:47   ` [PATCH v2 " Yurii Pavlovskyi
@ 2019-04-12  8:03     ` Daniel Drake
  2019-04-12 20:50       ` Yurii Pavlovskyi
  0 siblings, 1 reply; 29+ messages in thread
From: Daniel Drake @ 2019-04-12  8:03 UTC (permalink / raw)
  To: Yurii Pavlovskyi
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, acpi4asus-user,
	Platform Driver, Linux Kernel, linux-api,
	Endless Linux Upstreaming Team

On Thu, Apr 11, 2019 at 1:47 PM Yurii Pavlovskyi
<yurii.pavlovskyi@gmail.com> wrote:
> * 0x00 - is normal,
> * 0x01 - is obviously turbo by the amount of noise, might be useful to
> avoid CPU frequency throttling on high load,
> * 0x02 - the meaning is unknown at the time as modes are not named
> in the vendor documentation, but it does look like a quiet mode as CPU
> temperature does increase about 10 degrees on maximum load.

I'm curious which vendor documentation you're working with here.

From the spec,
0 = normal
1 = overboost
2 = silent

Also you can use DSTS on the 0x00110018 device to check the exact
capabilities supported, which you should use to refine which modes can
be cycled through.
Bit 0 = overboost supported
Bit 1 = silent supported

Thanks
Daniel

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

* Re: [PATCH 01/11] platform/x86: asus-wmi: Fix hwmon device cleanup
  2019-04-11  8:21   ` Daniel Drake
@ 2019-04-12 17:49     ` Yurii Pavlovskyi
  0 siblings, 0 replies; 29+ messages in thread
From: Yurii Pavlovskyi @ 2019-04-12 17:49 UTC (permalink / raw)
  To: Daniel Drake
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, acpi4asus-user,
	Platform Driver, Linux Kernel

Hello Daniel,

thank you for the tip. I did not know about this function. It does indeed
seem to make this complete patch redundant looking at
6e5f62b9e3651e61 hwmon: (lm90) Use devm_hwmon_device_register_with_groups

I will surely implement it this way in the next version.

Best regards,
Yurii

On 11.04.19 10:21, Daniel Drake wrote:
> On Thu, Apr 11, 2019 at 4:21 AM Yurii Pavlovskyi
> <yurii.pavlovskyi@gmail.com> wrote:
>>
>> The asus-wmi 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. Add
>> pointer to the device in module state and call cleanup on error.
> 
> I wonder if this can be fixed more cleanly by using
> devm_hwmon_device_register_with_groups() ?
> 
> Thanks
> Daniel
> 

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

* Re: [PATCH 04/11] platform/x86: asus-wmi: Add quirk to force DSTS WMI method detection
  2019-04-11 10:55   ` [PATCH " Daniel Drake
@ 2019-04-12 20:04     ` Yurii Pavlovskyi
  0 siblings, 0 replies; 29+ messages in thread
From: Yurii Pavlovskyi @ 2019-04-12 20:04 UTC (permalink / raw)
  To: Daniel Drake
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, acpi4asus-user,
	Platform Driver, Linux Kernel, Endless Linux Upstreaming Team

Wow that is a great thing you done, thanks a lot for your time and your
kind words! Your suggestion indeed sounds good judging by your results.

Both of my devices have UID "ATK" (actually FX505 also has two other
PNP0C14 devices with HIDs SampleDev and TestDev, but I think they are
disabled by default).

It looks like the driver is loaded already only if ASUS_WMI_MGMT_GUID is
present, so I guess that could be used for wmi_driver_register. A little
obstacle is that ACPI device pointer is stored in wmi_block structure,
which is internal to the wmi.c. This means we would have to add a new
method to wmi.h / wmi.c for getting the UID of WMI ACPI device.

I guess it might be possible to somehow navigate the device tree back to
the platform driver of WMI module, but it would definitely be more obscure,
and searching for the device by HID again is not only slower but generally
would require a guarantee that it's the same device. So adding a new
function looks reasonable to me. It might also be useful to someone
implementing similar things for other vendors in the future.

I'm going to implement this in updated patch.

Thanks,
Yurii

On 11.04.19 12:55, Daniel Drake wrote:
> On Thu, Apr 11, 2019 at 4:28 AM Yurii Pavlovskyi
> <yurii.pavlovskyi@gmail.com> wrote:
>> The DSTS method detection fails, as nothing is returned if method is not
>> defined in WMNB. As a result the control of keyboard backlight is not
>> functional for TUF Gaming series laptops (at the time the only
>> functionality of the driver on this model implemented with WMI methods).
>>
>> Patch was tested on a newer TUF Gaming FX505GM and older K54C model.
>>
>> FX505GM:
>> Method (WMNB, 3, Serialized)
>> { ...
>>     If ((Local0 == 0x53545344))
>>     {
>>         ...
>>         Return (Zero)
>>     }
>>     ...
>>     // No return
>> }
>>
>> K54C:
>> Method (WMNB, 3, Serialized)
>> { ...
>>     If ((Local0 == 0x53545344))
>>     {
>>         ...
>>         Return (0x02)
>>     }
>>     ...
>>     Return (0xFFFFFFFE)
>> }
>>
>> The non-existing method ASUS_WMI_METHODID_DSTS=0x53544344 (actually it is
>> DCTS in little endian ASCII) is selected in asus->dsts.
>>
>> One way to fix this would be to call both for every known device ID until
>> some answers - this would increase module load time.
>>
>> Another option is to check some device that is known to exist on every
>> model - none known at the time.
>>
>> Last option, which is implemented, is to check for presence of the
>> ASUS7000 device in ACPI tree (it is a dummy device), which is the
>> condition used for loading the vendor driver for this model. This might
>> not fix every affected model ever produced, but it likely does not
>> introduce any regressions. The patch introduces a quirk that is enabled
>> when ASUS7000 is found.
>>
>> Scope (_SB)
>> {
>>     Device (ATK)
>>     {
>>         Name (_HID, "ASUS7000")  // _HID: Hardware ID
>>     }
>> }
> 
> Hmm, tricky! But about time we did something about the DSTS vs DCTS guessing.
> 
> While we don't have definitive knowledge of the right thing to do
> here, I think I have enough info available to solidify some
> assumptions we'd otherwise be afraid to make, and then we can
> implement a better approach here.
> 
> In our database of 144 Asus DSDTs, 14 of them respond to DCTS:
> 
> AS_D520MT
> D425MC
> D640SA
> D320SF-K
> D415MT
> D830MT
> G11DF
> M32CD4-K
> V221ID
> V272UN_SKU3
> Z240IE
> ZN220IC-K
> ZN241IC
> ZN270IE
> 
> Of those 14, they all additionally respond to DSTS, except for D415MT
> and AS_D520MT.
> 
> In all 14 cases, the DCTS handling is done inside a device with _UID
> ASUSWMI. None of the other 130 products have a device with that _UID.
> 
> Furthermore, we recently received access to the ASUS spec, which
> confirms that the Instance Name for EeePC is "ACPI\PNP0C14\ASUSWMI_0"
> whereas the Instance Name for other notebooks is "ACPI\PNP0C14\ATK_0".
> 
> The 12 devices that respond to both DCTS and DSTS, do it on separate
> different devices, albeit with the same _WDG UUID. The one with _UID
> ASUSWMI responds to DCTS, and the one with _UID ATK responds to DSTS.
> 
> So that seems fairly convincing. My thinking is that we can check the
> _UID of the underlying device, and use DCTS for ASUSWMI, DSTS
> otherwise. To do that, I think you'd have to rework the driver probing
> so that it happens through wmi_driver_register(). Then from the probe
> function onwards you will get a wmi_device, and hopefully you can get
> the _UID through that instance somehow.
> 
> There's a separate issue of what happens on those 12 machines where
> there are two WMI devs, with the same _WDG GUID.
> drivers/platform/x86/wmi.c drops duplicates, so one of them is being
> ignored in that case. We'd ideally find a way to ignore the ASUSWMI
> node and go with ATK in that situation. But I think this can be
> separated from your work here.
> 
> Thanks for these patches and welcome to the world of kernel development!
> 
> Daniel
> 

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

* Re: [PATCH v2 05/11] platform/x86: asus-wmi: Support queued WMI event codes
  2019-04-12  7:48     ` Daniel Drake
@ 2019-04-12 20:32       ` Yurii Pavlovskyi
  0 siblings, 0 replies; 29+ messages in thread
From: Yurii Pavlovskyi @ 2019-04-12 20:32 UTC (permalink / raw)
  To: Daniel Drake
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, acpi4asus-user,
	Platform Driver, Linux Kernel, Endless Linux Upstreaming Team



On 12.04.19 09:48, Daniel Drake wrote:
> On Thu, Apr 11, 2019 at 1:44 PM Yurii Pavlovskyi
> <yurii.pavlovskyi@gmail.com> wrote:
>> Event codes are expected to be polled from a queue on at least some
>> models.
> 
> Maybe avoid the word "poll" since it suggests something else (at least to me).

Ok, will change this in the code as well.

>>> The fix flushes the old key codes out of the queue on load and after
>> receiving event the queue is read until either ..FFFF or 1 is encountered.
>>
>> It might be considered a minor issue and no normal user would likely to
>> observe this (there is little reason unloading the driver), but it does
>> significantly frustrate a developer who is unlucky enough to encounter
>> this.
>>
>> Introduce functionality for flushing and processing queued codes, which is
>> enabled via quirk flag for ASUS7000. It might be considered if it is
>> reasonable to enable it everywhere (might introduce regressions) or always
>> try to flush the queue on module load and try to detect if this quirk is
>> present in the future.
>>
>> This patch limits the effect to the specific hardware defined by ASUS7000
>> device that is used for driver detection by vendor driver of Fx505. The
>> fallback is also implemented in case initial flush fails.
> 
> Which vendor driver are you referring to here?

The ASUS Keyboard hotkeys Driver V2.0.5 which is available to download for
FX505 has this in his INF file:
[ATKP.ntamd64]

%DeviceDesc1% = NO_DRV64, ACPI\ASUS7000

But this driver is not generic. It is limited to very specific hardware,
I'd guess gaming series (for instance K54C does not have this device). It
was rather a way to avoid breaking anything. I think your suggestion below
is much better.

> 
> Researching more:
> In our database of 144 Asus models, 142 of them have GANQ.
> 
> Of those 142, 9 of them return One in the empty-queue case. The other
> 133 match your FX505GM device exactly. So it seems valid to interpret
> both 0xffff and 0x1 as queue-end markers.
> 
> The 2 devices that do not have GANQ are not laptops. They also do not
> have the _UID "ATK" WMI device, they only have "ASUSWMI" and their WMI
> _WED method does not use a queue.
> There are a few more units that have both ASUSWMI and ATK devices, and
> the ASUSWMI device appears to never be queued.
> Another observation is that the ASUSWMI device works with notify code
> 0xD2, and the ATK device works with 0xFF.
> 
> Nailing this down a bit further, I found a DSDT for EEEPC X101H: that
> one only has ASUSWMI and it is also not queued.
> 
> So I think you should make this queue behaviour applied more
> generically, but either avoid it when the WMI device _UID is not "ATK"
> (as discussed in the DCTS/DSTS thread), or avoid it when the notify
> code is not 0x>
> Thanks
> Daniel

Thanks a lot for your research, it's much appreciated! That seems to
confirm that these two quirks are actually connected with ATK device. I
guess it makes sense to combine the detection for both of them. Also to
flush the queue we need to know the notify code beforehand, because it is
checked in _WED so checking for ATK seems reasonable to me.

Best regards,
Yurii

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

* Re: [PATCH v2 10/11] platform/x86: asus-wmi: Switch fan boost mode
  2019-04-12  8:03     ` Daniel Drake
@ 2019-04-12 20:50       ` Yurii Pavlovskyi
  0 siblings, 0 replies; 29+ messages in thread
From: Yurii Pavlovskyi @ 2019-04-12 20:50 UTC (permalink / raw)
  To: Daniel Drake
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, acpi4asus-user,
	Platform Driver, Linux Kernel, linux-api,
	Endless Linux Upstreaming Team

On 12.04.19 10:03, Daniel Drake wrote:
> On Thu, Apr 11, 2019 at 1:47 PM Yurii Pavlovskyi
> <yurii.pavlovskyi@gmail.com> wrote:
>> * 0x00 - is normal,
>> * 0x01 - is obviously turbo by the amount of noise, might be useful to
>> avoid CPU frequency throttling on high load,
>> * 0x02 - the meaning is unknown at the time as modes are not named
>> in the vendor documentation, but it does look like a quiet mode as CPU
>> temperature does increase about 10 degrees on maximum load.
> 
> I'm curious which vendor documentation you're working with here.

That would be user manual for FX505 series, which is pretty minimalistic.
It says it has a fan mode switch key and that's it. Following up on your
comment, I've searched more and actually did found their names hidden in
marketing web page on the website. You're right, they call them balanced,
overboost and silent there.

> From the spec,
> 0 = normal
> 1 = overboost
> 2 = silent

> Also you can use DSTS on the 0x00110018 device to check the exact
> capabilities supported, which you should use to refine which modes can
> be cycled through.
> Bit 0 = overboost supported
> Bit 1 = silent supported
> 
> Thanks
> Daniel
> 

Thanks! I was guessing that the 3 in DSTS must've meant something.

Appreciate your comments! Will definitely implement them. I'm going to post
the v3 patch series approximately middle of next week.

Best regards,
Yurii

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

end of thread, other threads:[~2019-04-12 20:50 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-10 20:11 [PATCH 00/11] asus-wmi: Support of ASUS TUF Gaming series laptops Yurii Pavlovskyi
2019-04-10 20:20 ` [PATCH 01/11] platform/x86: asus-wmi: Fix hwmon device cleanup Yurii Pavlovskyi
2019-04-11  8:21   ` Daniel Drake
2019-04-12 17:49     ` Yurii Pavlovskyi
2019-04-10 20:26 ` [PATCH 02/11] platform/x86: asus-wmi: Fix preserving keyboard, backlight intensity on load Yurii Pavlovskyi
2019-04-10 20:27 ` [PATCH 03/11] platform/x86: asus-wmi: Increase input buffer size of WMI methods Yurii Pavlovskyi
2019-04-10 20:28 ` [PATCH 04/11] platform/x86: asus-wmi: Add quirk to force DSTS WMI method detection Yurii Pavlovskyi
2019-04-11  5:42   ` [PATCH v2 " Yurii Pavlovskyi
2019-04-11 10:55   ` [PATCH " Daniel Drake
2019-04-12 20:04     ` Yurii Pavlovskyi
2019-04-10 20:29 ` [PATCH 05/11] platform/x86: asus-wmi: Support queued WMI event codes Yurii Pavlovskyi
2019-04-11  5:44   ` [PATCH v2 " Yurii Pavlovskyi
2019-04-12  7:48     ` Daniel Drake
2019-04-12 20:32       ` Yurii Pavlovskyi
2019-04-10 20:30 ` [PATCH 06/11] platform/x86: asus-nb-wmi: Add microphone mute key code Yurii Pavlovskyi
2019-04-11  5:44   ` [PATCH v2 " Yurii Pavlovskyi
2019-04-10 20:31 ` [PATCH 07/11] platform/x86: asus-wmi: Organize code into sections Yurii Pavlovskyi
2019-04-11  5:45   ` [PATCH v2 " Yurii Pavlovskyi
2019-04-10 20:32 ` [PATCH 08/11] platform/x86: asus-wmi: Enhance detection of thermal data Yurii Pavlovskyi
2019-04-11  5:45   ` [PATCH v2 " Yurii Pavlovskyi
2019-04-10 20:33 ` [PATCH 09/11] platform/x86: asus-wmi: Control RGB keyboard backlight Yurii Pavlovskyi
2019-04-11  5:46   ` [PATCH v2 " Yurii Pavlovskyi
2019-04-10 20:34 ` [PATCH 10/11] platform/x86: asus-wmi: Switch fan boost mode Yurii Pavlovskyi
2019-04-11  5:47   ` [PATCH v2 " Yurii Pavlovskyi
2019-04-12  8:03     ` Daniel Drake
2019-04-12 20:50       ` Yurii Pavlovskyi
2019-04-10 20:36 ` [PATCH 11/11] platform/x86: asus-wmi: Do not disable keyboard backlight on unload Yurii Pavlovskyi
2019-04-11  5:48   ` [PATCH v2 " Yurii Pavlovskyi
2019-04-11  5:38 ` [PATCH 00/11] asus-wmi: Support of ASUS TUF Gaming series laptops Yurii Pavlovskyi

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).