* [PATCH 1/3] platform/x86: asus-wmi: Move i8042 filter install to shared asus-wmi code
2023-11-20 15:42 [PATCH 0/3] platform/x86: asus-wmi: Filter Volume key presses if also reported via atkbd Hans de Goede
@ 2023-11-20 15:42 ` Hans de Goede
2023-11-20 15:42 ` [PATCH 2/3] platform/x86: asus-wmi: Change q500a_i8042_filter() into a generic i8042-filter Hans de Goede
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2023-11-20 15:42 UTC (permalink / raw)
To: Ilpo Järvinen, Andy Shevchenko, Corentin Chary,
Luke D . Jones
Cc: Hans de Goede, Kai Heng Feng, acpi4asus-user, platform-driver-x86,
Oleksij Rempel
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/3] platform/x86: asus-wmi: Change q500a_i8042_filter() into a generic i8042-filter
2023-11-20 15:42 [PATCH 0/3] platform/x86: asus-wmi: Filter Volume key presses if also reported via atkbd Hans de Goede
2023-11-20 15:42 ` [PATCH 1/3] platform/x86: asus-wmi: Move i8042 filter install to shared asus-wmi code Hans de Goede
@ 2023-11-20 15:42 ` Hans de Goede
2023-11-20 15:42 ` [PATCH 3/3] platform/x86: asus-wmi: Filter Volume key presses if also reported via atkbd Hans de Goede
2023-11-23 12:41 ` [PATCH 0/3] " Ilpo Järvinen
3 siblings, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2023-11-20 15:42 UTC (permalink / raw)
To: Ilpo Järvinen, Andy Shevchenko, Corentin Chary,
Luke D . Jones
Cc: Hans de Goede, Kai Heng Feng, acpi4asus-user, platform-driver-x86
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/3] platform/x86: asus-wmi: Filter Volume key presses if also reported via atkbd
2023-11-20 15:42 [PATCH 0/3] platform/x86: asus-wmi: Filter Volume key presses if also reported via atkbd Hans de Goede
2023-11-20 15:42 ` [PATCH 1/3] platform/x86: asus-wmi: Move i8042 filter install to shared asus-wmi code Hans de Goede
2023-11-20 15:42 ` [PATCH 2/3] platform/x86: asus-wmi: Change q500a_i8042_filter() into a generic i8042-filter Hans de Goede
@ 2023-11-20 15:42 ` Hans de Goede
2023-11-23 12:41 ` [PATCH 0/3] " Ilpo Järvinen
3 siblings, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2023-11-20 15:42 UTC (permalink / raw)
To: Ilpo Järvinen, Andy Shevchenko, Corentin Chary,
Luke D . Jones
Cc: Hans de Goede, Kai Heng Feng, acpi4asus-user, platform-driver-x86
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 0/3] platform/x86: asus-wmi: Filter Volume key presses if also reported via atkbd
2023-11-20 15:42 [PATCH 0/3] platform/x86: asus-wmi: Filter Volume key presses if also reported via atkbd Hans de Goede
` (2 preceding siblings ...)
2023-11-20 15:42 ` [PATCH 3/3] platform/x86: asus-wmi: Filter Volume key presses if also reported via atkbd Hans de Goede
@ 2023-11-23 12:41 ` Ilpo Järvinen
2023-11-24 13:11 ` Hans de Goede
3 siblings, 1 reply; 6+ messages in thread
From: Ilpo Järvinen @ 2023-11-23 12:41 UTC (permalink / raw)
To: Hans de Goede
Cc: Andy Shevchenko, Corentin Chary, Luke D . Jones, Kai Heng Feng,
acpi4asus-user, platform-driver-x86
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.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 0/3] platform/x86: asus-wmi: Filter Volume key presses if also reported via atkbd
2023-11-23 12:41 ` [PATCH 0/3] " Ilpo Järvinen
@ 2023-11-24 13:11 ` Hans de Goede
0 siblings, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2023-11-24 13:11 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Andy Shevchenko, Corentin Chary, Luke D . Jones, Kai Heng Feng,
acpi4asus-user, platform-driver-x86
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
^ permalink raw reply [flat|nested] 6+ messages in thread