* Re: [PATCH] asus-wmi: filter buggy scan codes on ASUS Q500A
2016-09-12 15:48 ` [PATCH] asus-wmi: filter buggy scan codes on ASUS Q500A Oleksij Rempel
@ 2016-09-24 0:09 ` Darren Hart
0 siblings, 0 replies; 2+ messages in thread
From: Darren Hart @ 2016-09-24 0:09 UTC (permalink / raw)
To: Oleksij Rempel
Cc: fixed-term.Oleksij.Rempel, Alex Henrie, Dmitry Torokhov,
Corentin Chary, acpi4asus-user, platform-driver-x86,
linux-kernel
On Mon, Sep 12, 2016 at 05:48:17PM +0200, Oleksij Rempel wrote:
> Some revision of ASUS Q500A series have keyboard related
> issue which is reproducible only if Windows with installed ASUS
> tools was ever started.
> In this case the Linux side will have blocked keyboard or
> report wrong or incomplete hotkey events.
> To make Linux work properly again complete power down
> (unplug power supply and remove battery) should be made.
>
> Linux/atkbd after clean start will get fallowing code on VOLUME_UP
> key: {0xe0, 0x30, 0xe0, 0xb0}. After Windows, same key will generate this
> codes: {0xe1, 0x23, 0xe0, 0x30, 0xe0, 0xb0}. As result atkdb will
> be confused by buggy codes.
>
> This patch is filtering this buggy code out.
>
> https://bugzilla.kernel.org/show_bug.cgi?id=119391
First of all - wow - what a bunch of work. Nice job tracking this all down.
A couple of minor nits below, which I've taken care of.
Queued to testing.
>
> Signed-off-by: Oleksij Rempel <linux@rempel-privat.de>
> Cc: Alex Henrie <alexhenrie24@gmail.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Corentin Chary <corentin.chary@gmail.com>
> Cc: Darren Hart <dvhart@infradead.org>
> Cc: acpi4asus-user@lists.sourceforge.net
> Cc: platform-driver-x86@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
> drivers/platform/x86/asus-nb-wmi.c | 45 ++++++++++++++++++++++++++++++++++++++
> drivers/platform/x86/asus-wmi.h | 4 ++++
> 2 files changed, 49 insertions(+)
>
> diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
> index adecc1c..787da83 100644
> --- a/drivers/platform/x86/asus-nb-wmi.c
> +++ b/drivers/platform/x86/asus-nb-wmi.c
> @@ -27,6 +27,7 @@
> #include <linux/input/sparse-keymap.h>
> #include <linux/fb.h>
> #include <linux/dmi.h>
> +#include <linux/i8042.h>
>
> #include "asus-wmi.h"
>
> @@ -55,10 +56,35 @@ MODULE_PARM_DESC(wapf, "WAPF value");
>
> static struct quirk_entry *quirks;
>
> +static bool asus_q500a_i8042_filter(unsigned char data, unsigned char str,
> + struct serio *port)
> +{
> + static bool extended;
> + bool ret = false;
> +
> +
Just one empty line is plenty
> + if (str & I8042_STR_AUXDATA)
> + return false;
> +
> + if (unlikely(data == 0xe1)) {
> + extended = true;
> + ret = true;
> + } else if (unlikely(extended)) {
> + extended = false;
> + ret = true;
> + }
> +
> + return ret;
> +}
> +
> static struct quirk_entry quirk_asus_unknown = {
> .wapf = 0,
> };
>
> +static struct quirk_entry quirk_asus_q500a = {
> + .i8042_filter = asus_q500a_i8042_filter,
> +};
> +
> /*
> * For those machines that need software to control bt/wifi status
> * and can't adjust brightness through ACPI interface
> @@ -96,6 +122,15 @@ static int dmi_matched(const struct dmi_system_id *dmi)
> static const struct dmi_system_id asus_quirks[] = {
> {
> .callback = dmi_matched,
> + .ident = "ASUSTeK COMPUTER INC. Q500A",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "Q500A"),
> + },
> + .driver_data = &quirk_asus_q500a,
> + },
> + {
> + .callback = dmi_matched,
> .ident = "ASUSTeK COMPUTER INC. U32U",
> .matches = {
> DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK Computer Inc."),
> @@ -356,6 +391,8 @@ 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);
>
> @@ -367,6 +404,14 @@ static void asus_nb_wmi_quirks(struct asus_wmi_driver *driver)
> quirks->wapf = wapf;
> else
> wapf = quirks->wapf;
> +
> + if (quirks->i8042_filter) {
> + ret = i8042_install_filter(quirks->i8042_filter);
> + if (ret) {
> + pr_warn("Unable to install key filter\n");
> + }
No braces for a single line block
> + pr_info("Using i8042 filter function for receiving events\n");
Well, then again, a return in the above block would make more sense.
pr_warn
return
}
pr_info
I've taken the liberty of making this change.... mostly to speed things a long
since I've neglected this patch for so long! Apologies. This is queued to
testing, let me know if you have any concerns. It'll hit linux-next this
weekend.
> + }
> }
>
> static const struct key_entry asus_nb_wmi_keymap[] = {
> diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
> index 5de1df5..dd2e6cc 100644
> --- a/drivers/platform/x86/asus-wmi.h
> +++ b/drivers/platform/x86/asus-wmi.h
> @@ -28,6 +28,7 @@
> #define _ASUS_WMI_H_
>
> #include <linux/platform_device.h>
> +#include <linux/i8042.h>
>
> #define ASUS_WMI_KEY_IGNORE (-1)
> #define ASUS_WMI_BRN_DOWN 0x20
> @@ -51,6 +52,9 @@ struct quirk_entry {
> * and let the ACPI interrupt to send out the key event.
> */
> int no_display_toggle;
> +
> + bool (*i8042_filter)(unsigned char data, unsigned char str,
> + struct serio *serio);
> };
>
> struct asus_wmi_driver {
> --
> 2.7.4
>
>
--
Darren Hart
Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 2+ messages in thread