[PATCH 0/3] platform/x86: asus-wmi: Filter Volume key presses if also reported via atkbd

[PATCH 0/3] platform/x86: asus-wmi: Filter Volume key presses if also reported via atkbd

[Hans de Goede]

Hi All,

An issue where volume keypresses get reported through both the atkbd
and the asus-wmi interface has been reported here:

https://bbs.archlinux.org/viewtopic.php?pid=2128536#p2128536

This series addresses this. Unfortunately the reporter has fallen
quiet and has not confirmed that this fixes things, but the fix
is straight forward enough that there should be little doubt
that it works.

Ilpo, the first patch in this series is a straight forward bugfix,
can you merge this into the fixes branch ?

I guess the rest of the series is a bugfix too, but since this
is untested I'm not sure what to do wrt fixes vs for-next,
what do you think ?

Regards,

Hans


Hans de Goede (3):
  platform/x86: asus-wmi: Move i8042 filter install to shared asus-wmi
    code
  platform/x86: asus-wmi: Change q500a_i8042_filter() into a generic
    i8042-filter
  platform/x86: asus-wmi: Filter Volume key presses if also reported via
    atkbd

 drivers/platform/x86/Kconfig       |  2 +-
 drivers/platform/x86/asus-nb-wmi.c | 61 +++++++++++++++++++-----------
 drivers/platform/x86/asus-wmi.c    |  8 ++++
 drivers/platform/x86/asus-wmi.h    |  7 ++--
 4 files changed, 51 insertions(+), 27 deletions(-)

-- 
2.41.0

[PATCH 1/3] platform/x86: asus-wmi: Move i8042 filter install to shared asus-wmi code

[Hans de Goede]

asus-nb-wmi calls i8042_install_filter() in some cases, but it never
calls i8042_remove_filter(). This means that a dangling pointer to
the filter function is left after rmmod leading to crashes.

Fix this by moving the i8042-filter installation to the shared
asus-wmi code and also remove it from the shared code on driver unbind.

Fixes: b5643539b825 ("platform/x86: asus-wmi: Filter buggy scan codes on ASUS Q500A")
Cc: Oleksij Rempel <linux@rempel-privat.de>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/Kconfig       |  2 +-
 drivers/platform/x86/asus-nb-wmi.c | 11 -----------
 drivers/platform/x86/asus-wmi.c    |  8 ++++++++
 3 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 7e69fdaccdd5..c94f31a5c6a3 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -263,6 +263,7 @@ config ASUS_WMI
 	depends on RFKILL || RFKILL = n
 	depends on HOTPLUG_PCI
 	depends on ACPI_VIDEO || ACPI_VIDEO = n
+	depends on SERIO_I8042 || SERIO_I8042 = n
 	select INPUT_SPARSEKMAP
 	select LEDS_CLASS
 	select NEW_LEDS
@@ -279,7 +280,6 @@ config ASUS_WMI
 config ASUS_NB_WMI
 	tristate "Asus Notebook WMI Driver"
 	depends on ASUS_WMI
-	depends on SERIO_I8042 || SERIO_I8042 = n
 	help
 	  This is a driver for newer Asus notebooks. It adds extra features
 	  like wireless radio and bluetooth control, leds, hotkeys, backlight...
diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
index 9aa1226e74e6..ff794387581d 100644
--- a/drivers/platform/x86/asus-nb-wmi.c
+++ b/drivers/platform/x86/asus-nb-wmi.c
@@ -503,8 +503,6 @@ static const struct dmi_system_id asus_quirks[] = {
 
 static void asus_nb_wmi_quirks(struct asus_wmi_driver *driver)
 {
-	int ret;
-
 	quirks = &quirk_asus_unknown;
 	dmi_check_system(asus_quirks);
 
@@ -519,15 +517,6 @@ static void asus_nb_wmi_quirks(struct asus_wmi_driver *driver)
 
 	if (tablet_mode_sw != -1)
 		quirks->tablet_switch_mode = tablet_mode_sw;
-
-	if (quirks->i8042_filter) {
-		ret = i8042_install_filter(quirks->i8042_filter);
-		if (ret) {
-			pr_warn("Unable to install key filter\n");
-			return;
-		}
-		pr_info("Using i8042 filter function for receiving events\n");
-	}
 }
 
 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 6a79f16233ab..53e25cb467d7 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -4567,6 +4567,12 @@ static int asus_wmi_add(struct platform_device *pdev)
 		goto fail_wmi_handler;
 	}
 
+	if (asus->driver->quirks->i8042_filter) {
+		err = i8042_install_filter(asus->driver->quirks->i8042_filter);
+		if (err)
+			pr_warn("Unable to install key filter - %d\n", err);
+	}
+
 	asus_wmi_battery_init(asus);
 
 	asus_wmi_debugfs_init(asus);
@@ -4603,6 +4609,8 @@ static int asus_wmi_remove(struct platform_device *device)
 	struct asus_wmi *asus;
 
 	asus = platform_get_drvdata(device);
+	if (asus->driver->quirks->i8042_filter)
+		i8042_remove_filter(asus->driver->quirks->i8042_filter);
 	wmi_remove_notify_handler(asus->driver->event_guid);
 	asus_wmi_backlight_exit(asus);
 	asus_screenpad_exit(asus);
-- 
2.41.0

[PATCH 2/3] platform/x86: asus-wmi: Change q500a_i8042_filter() into a generic i8042-filter

[Hans de Goede]

Change asus_q500a_i8042_filter() into a generic i8042-filter,
using a new filter_i8042_e1_extended_codes flag in the quirks struct
to decide if e1 extended codes should be filtered out or not.

This is a preparation patch for adding support for filtering volume key
events being reported twice through both the PS/2 keyboard and asus-wmi.

Note while modifying the code also drop the unnecessary unlikely()
annotations, this is not in a hot path so those are not necessary.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/asus-nb-wmi.c | 27 +++++++++++++++------------
 drivers/platform/x86/asus-wmi.c    |  8 ++++----
 drivers/platform/x86/asus-wmi.h    |  7 ++++---
 3 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
index ff794387581d..16241556f6fb 100644
--- a/drivers/platform/x86/asus-nb-wmi.c
+++ b/drivers/platform/x86/asus-nb-wmi.c
@@ -49,24 +49,26 @@ MODULE_PARM_DESC(tablet_mode_sw, "Tablet mode detect: -1:auto 0:disable 1:kbd-do
 
 static struct quirk_entry *quirks;
 
-static bool asus_q500a_i8042_filter(unsigned char data, unsigned char str,
-			      struct serio *port)
+static bool asus_i8042_filter(unsigned char data, unsigned char str, struct serio *port)
 {
-	static bool extended;
-	bool ret = false;
+	static bool extended_e1;
 
 	if (str & I8042_STR_AUXDATA)
 		return false;
 
-	if (unlikely(data == 0xe1)) {
-		extended = true;
-		ret = true;
-	} else if (unlikely(extended)) {
-		extended = false;
-		ret = true;
+	if (quirks->filter_i8042_e1_extended_codes) {
+		if (data == 0xe1) {
+			extended_e1 = true;
+			return true;
+		}
+
+		if (extended_e1) {
+			extended_e1 = false;
+			return true;
+		}
 	}
 
-	return ret;
+	return false;
 }
 
 static struct quirk_entry quirk_asus_unknown = {
@@ -75,7 +77,7 @@ static struct quirk_entry quirk_asus_unknown = {
 };
 
 static struct quirk_entry quirk_asus_q500a = {
-	.i8042_filter = asus_q500a_i8042_filter,
+	.filter_i8042_e1_extended_codes = true,
 	.wmi_backlight_set_devstate = true,
 };
 
@@ -619,6 +621,7 @@ static struct asus_wmi_driver asus_nb_wmi_driver = {
 	.input_phys = ASUS_NB_WMI_FILE "/input0",
 	.detect_quirks = asus_nb_wmi_quirks,
 	.key_filter = asus_nb_wmi_key_filter,
+	.i8042_filter = asus_i8042_filter,
 };
 
 
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 53e25cb467d7..ca668cf04020 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -4567,8 +4567,8 @@ static int asus_wmi_add(struct platform_device *pdev)
 		goto fail_wmi_handler;
 	}
 
-	if (asus->driver->quirks->i8042_filter) {
-		err = i8042_install_filter(asus->driver->quirks->i8042_filter);
+	if (asus->driver->i8042_filter) {
+		err = i8042_install_filter(asus->driver->i8042_filter);
 		if (err)
 			pr_warn("Unable to install key filter - %d\n", err);
 	}
@@ -4609,8 +4609,8 @@ static int asus_wmi_remove(struct platform_device *device)
 	struct asus_wmi *asus;
 
 	asus = platform_get_drvdata(device);
-	if (asus->driver->quirks->i8042_filter)
-		i8042_remove_filter(asus->driver->quirks->i8042_filter);
+	if (asus->driver->i8042_filter)
+		i8042_remove_filter(asus->driver->i8042_filter);
 	wmi_remove_notify_handler(asus->driver->event_guid);
 	asus_wmi_backlight_exit(asus);
 	asus_screenpad_exit(asus);
diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
index adb67c925724..cc30f1853847 100644
--- a/drivers/platform/x86/asus-wmi.h
+++ b/drivers/platform/x86/asus-wmi.h
@@ -39,6 +39,7 @@ struct quirk_entry {
 	bool wmi_backlight_set_devstate;
 	bool wmi_force_als_set;
 	bool wmi_ignore_fan;
+	bool filter_i8042_e1_extended_codes;
 	enum asus_wmi_tablet_switch_mode tablet_switch_mode;
 	int wapf;
 	/*
@@ -49,9 +50,6 @@ struct quirk_entry {
 	 */
 	int no_display_toggle;
 	u32 xusb2pr;
-
-	bool (*i8042_filter)(unsigned char data, unsigned char str,
-			     struct serio *serio);
 };
 
 struct asus_wmi_driver {
@@ -73,6 +71,9 @@ struct asus_wmi_driver {
 	 * Return ASUS_WMI_KEY_IGNORE in code if event should be ignored. */
 	void (*key_filter) (struct asus_wmi_driver *driver, int *code,
 			    unsigned int *value, bool *autorelease);
+	/* Optional standard i8042 filter */
+	bool (*i8042_filter)(unsigned char data, unsigned char str,
+			     struct serio *serio);
 
 	int (*probe) (struct platform_device *device);
 	void (*detect_quirks) (struct asus_wmi_driver *driver);
-- 
2.41.0

[PATCH 3/3] platform/x86: asus-wmi: Filter Volume key presses if also reported via atkbd

[Hans de Goede]

Use the i8042-filter to check if Volume key presses are also reported
via atkbd and if yes then filter out the WMI events to avoid reporting
each key-press twice.

Note depending on in which order the PS/2 data vs the WMI event are
handled the first volume key press may still be reported twice. This is
a compromise versus DMI quirks (unmaintainable) or other more complex
solutions.

Closes: https://bbs.archlinux.org/viewtopic.php?pid=2128536#p2128536
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/asus-nb-wmi.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
index 16241556f6fb..fceffe2082ec 100644
--- a/drivers/platform/x86/asus-nb-wmi.c
+++ b/drivers/platform/x86/asus-nb-wmi.c
@@ -48,9 +48,11 @@ module_param(tablet_mode_sw, uint, 0444);
 MODULE_PARM_DESC(tablet_mode_sw, "Tablet mode detect: -1:auto 0:disable 1:kbd-dock 2:lid-flip 3:lid-flip-rog");
 
 static struct quirk_entry *quirks;
+static bool atkbd_reports_vol_keys;
 
 static bool asus_i8042_filter(unsigned char data, unsigned char str, struct serio *port)
 {
+	static bool extended_e0;
 	static bool extended_e1;
 
 	if (str & I8042_STR_AUXDATA)
@@ -68,6 +70,20 @@ static bool asus_i8042_filter(unsigned char data, unsigned char str, struct seri
 		}
 	}
 
+	if (data == 0xe0) {
+		extended_e0 = true;
+	} else if (extended_e0) {
+		extended_e0 = false;
+
+		switch (data & 0x7f) {
+		case 0x20: /* e0 20 / e0 a0, Volume Mute press / release */
+		case 0x2e: /* e0 2e / e0 ae, Volume Down press / release */
+		case 0x30: /* e0 30 / e0 b0, Volume Up press / release */
+			atkbd_reports_vol_keys = true;
+			break;
+		}
+	}
+
 	return false;
 }
 
@@ -608,6 +624,13 @@ static void asus_nb_wmi_key_filter(struct asus_wmi_driver *asus_wmi, int *code,
 		if (acpi_video_handles_brightness_key_presses())
 			*code = ASUS_WMI_KEY_IGNORE;
 
+		break;
+	case 0x30: /* Volume Up */
+	case 0x31: /* Volume Down */
+	case 0x32: /* Volume Mute */
+		if (atkbd_reports_vol_keys)
+			*code = ASUS_WMI_KEY_IGNORE;
+
 		break;
 	}
 }
-- 
2.41.0

Re: [PATCH 0/3] platform/x86: asus-wmi: Filter Volume key presses if also reported via atkbd

[Ilpo Järvinen]

On Mon, 20 Nov 2023, Hans de Goede wrote:

> An issue where volume keypresses get reported through both the atkbd
> and the asus-wmi interface has been reported here:
> 
> https://bbs.archlinux.org/viewtopic.php?pid=2128536#p2128536
> 
> This series addresses this. Unfortunately the reporter has fallen
> quiet and has not confirmed that this fixes things, but the fix
> is straight forward enough that there should be little doubt
> that it works.
> 
> Ilpo, the first patch in this series is a straight forward bugfix,
> can you merge this into the fixes branch ?
> 
> I guess the rest of the series is a bugfix too, but since this
> is untested I'm not sure what to do wrt fixes vs for-next,
> what do you think ?

I just took them all into review-ilpo and will propagate them to fixes.
I don't think putting them into for-next wins that much here.

-- 
 i.

Re: [PATCH 0/3] platform/x86: asus-wmi: Filter Volume key presses if also reported via atkbd

[Hans de Goede]

Hi,

On 11/23/23 13:41, Ilpo Järvinen wrote:
> On Mon, 20 Nov 2023, Hans de Goede wrote:
> 
>> An issue where volume keypresses get reported through both the atkbd
>> and the asus-wmi interface has been reported here:
>>
>> https://bbs.archlinux.org/viewtopic.php?pid=2128536#p2128536
>>
>> This series addresses this. Unfortunately the reporter has fallen
>> quiet and has not confirmed that this fixes things, but the fix
>> is straight forward enough that there should be little doubt
>> that it works.
>>
>> Ilpo, the first patch in this series is a straight forward bugfix,
>> can you merge this into the fixes branch ?
>>
>> I guess the rest of the series is a bugfix too, but since this
>> is untested I'm not sure what to do wrt fixes vs for-next,
>> what do you think ?
> 
> I just took them all into review-ilpo and will propagate them to fixes.
> I don't think putting them into for-next wins that much here.

Ok, sounds good. Thank you.

Regards,

Hans