diff mbox series

[2/2] gpiolib: acpi: Add a ignore wakeup quirk for Clevo NL5xRU

Message ID 20230116193702.31356-3-mario.limonciello@amd.com
State New
Headers
Series Fix a spurious wakeup regression |

Commit Message

Mario Limonciello Jan. 16, 2023, 7:37 p.m. UTC 
  commit 1796f808e4bb ("HID: i2c-hid: acpi: Stop setting wakeup_capable")
changed the policy such that I2C touchpads may be able to wake up the
system by default if the system is configured as such.

However on Clevo NL5xRU there is a mistake in the ACPI tables that the
TP_ATTN# signal connected to GPIO 9 is configured as ActiveLow and level
triggered but connected to a pull up. As soon as the system suspends the
touchpad loses power and then the system wakes up.

To avoid this problem, introduce a quirk for this model that will prevent
the wakeup capability for being set for GPIO 9.

Fixes: 1796f808e4bb ("HID: i2c-hid: acpi: Stop setting wakeup_capable")
Reported-by: Werner Sembach <wse@tuxedocomputers.com>
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1722#note_1720627
Co-developed-by: Werner Sembach <wse@tuxedocomputers.com>
Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/gpio/gpiolib-acpi.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Andy Shevchenko Jan. 17, 2023, 8:33 a.m. UTC | #1 
On Mon, Jan 16, 2023 at 01:37:02PM -0600, Mario Limonciello wrote:
> commit 1796f808e4bb ("HID: i2c-hid: acpi: Stop setting wakeup_capable")
> changed the policy such that I2C touchpads may be able to wake up the
> system by default if the system is configured as such.
> 
> However on Clevo NL5xRU there is a mistake in the ACPI tables that the
> TP_ATTN# signal connected to GPIO 9 is configured as ActiveLow and level
> triggered but connected to a pull up. As soon as the system suspends the
> touchpad loses power and then the system wakes up.
> 
> To avoid this problem, introduce a quirk for this model that will prevent
> the wakeup capability for being set for GPIO 9.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Fixes: 1796f808e4bb ("HID: i2c-hid: acpi: Stop setting wakeup_capable")
> Reported-by: Werner Sembach <wse@tuxedocomputers.com>
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1722#note_1720627
> Co-developed-by: Werner Sembach <wse@tuxedocomputers.com>
> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  drivers/gpio/gpiolib-acpi.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
> index 4287555a12408..9ef0f5641b521 100644
> --- a/drivers/gpio/gpiolib-acpi.c
> +++ b/drivers/gpio/gpiolib-acpi.c
> @@ -1623,6 +1623,19 @@ static const struct dmi_system_id gpiolib_acpi_quirks[] __initconst = {
>  			.ignore_interrupt = "AMDI0030:00@18",
>  		},
>  	},
> +	{
> +		/*
> +		 * Spurious wakeups from TP_ATTN# pin
> +		 * Found in BIOS 1.7.8
> +		 * https://gitlab.freedesktop.org/drm/amd/-/issues/1722#note_1720627
> +		 */
> +		.matches = {
> +			DMI_MATCH(DMI_BOARD_NAME, "NL5xRU"),
> +		},
> +		.driver_data = &(struct acpi_gpiolib_dmi_quirk) {
> +			.ignore_wake = "ELAN0415:00@9",
> +		},
> +	},
>  	{} /* Terminating entry */
>  };
>  
> -- 
> 2.34.1
>
Bartosz Golaszewski Jan. 18, 2023, 2:26 p.m. UTC | #2 
On Mon, Jan 16, 2023 at 8:37 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> commit 1796f808e4bb ("HID: i2c-hid: acpi: Stop setting wakeup_capable")
> changed the policy such that I2C touchpads may be able to wake up the
> system by default if the system is configured as such.
>
> However on Clevo NL5xRU there is a mistake in the ACPI tables that the
> TP_ATTN# signal connected to GPIO 9 is configured as ActiveLow and level
> triggered but connected to a pull up. As soon as the system suspends the
> touchpad loses power and then the system wakes up.
>
> To avoid this problem, introduce a quirk for this model that will prevent
> the wakeup capability for being set for GPIO 9.
>
> Fixes: 1796f808e4bb ("HID: i2c-hid: acpi: Stop setting wakeup_capable")
> Reported-by: Werner Sembach <wse@tuxedocomputers.com>
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1722#note_1720627
> Co-developed-by: Werner Sembach <wse@tuxedocomputers.com>
> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  drivers/gpio/gpiolib-acpi.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
> index 4287555a12408..9ef0f5641b521 100644
> --- a/drivers/gpio/gpiolib-acpi.c
> +++ b/drivers/gpio/gpiolib-acpi.c
> @@ -1623,6 +1623,19 @@ static const struct dmi_system_id gpiolib_acpi_quirks[] __initconst = {
>                         .ignore_interrupt = "AMDI0030:00@18",
>                 },
>         },
> +       {
> +               /*
> +                * Spurious wakeups from TP_ATTN# pin
> +                * Found in BIOS 1.7.8
> +                * https://gitlab.freedesktop.org/drm/amd/-/issues/1722#note_1720627
> +                */
> +               .matches = {
> +                       DMI_MATCH(DMI_BOARD_NAME, "NL5xRU"),
> +               },
> +               .driver_data = &(struct acpi_gpiolib_dmi_quirk) {
> +                       .ignore_wake = "ELAN0415:00@9",
> +               },
> +       },
>         {} /* Terminating entry */
>  };
>
> --
> 2.34.1
>

Queued for fixes, thanks!

Bart
Linus Walleij Jan. 18, 2023, 8:44 p.m. UTC | #3 
On Mon, Jan 16, 2023 at 8:37 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:

> However on Clevo NL5xRU there is a mistake in the ACPI tables that the
> TP_ATTN# signal connected to GPIO 9 is configured as ActiveLow and level
> triggered but connected to a pull up. As soon as the system suspends the
> touchpad loses power and then the system wakes up.

Now that is what I call proper root cause analysis. Hats off for this patch!
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
Mario Limonciello Jan. 18, 2023, 8:50 p.m. UTC | #4 
[Public]



> -----Original Message-----
> From: Linus Walleij <linus.walleij@linaro.org>
> Sent: Wednesday, January 18, 2023 14:45
> To: Limonciello, Mario <Mario.Limonciello@amd.com>
> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>; Andy Shevchenko
> <andriy.shevchenko@linux.intel.com>; Bartosz Golaszewski
> <brgl@bgdev.pl>; Dmitry Torokhov <dmitry.torokhov@gmail.com>; Raul E
> Rangel <rrangel@chromium.org>; Benjamin Tissoires
> <benjamin.tissoires@redhat.com>; regressions@lists.linux.dev; Werner
> Sembach <wse@tuxedocomputers.com>; linux-gpio@vger.kernel.org; linux-
> acpi@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 2/2] gpiolib: acpi: Add a ignore wakeup quirk for Clevo
> NL5xRU
> 
> On Mon, Jan 16, 2023 at 8:37 PM Mario Limonciello
> <mario.limonciello@amd.com> wrote:
> 
> > However on Clevo NL5xRU there is a mistake in the ACPI tables that the
> > TP_ATTN# signal connected to GPIO 9 is configured as ActiveLow and level
> > triggered but connected to a pull up. As soon as the system suspends the
> > touchpad loses power and then the system wakes up.
> 
> Now that is what I call proper root cause analysis. Hats off for this patch!
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Raul should get some credit too here in the confirmation on that root cause.

It certainly really helps that the schematics for this system were readily available
on the web to confirm the hypothesis.
diff mbox series

Patch

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 4287555a12408..9ef0f5641b521 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -1623,6 +1623,19 @@  static const struct dmi_system_id gpiolib_acpi_quirks[] __initconst = {
 			.ignore_interrupt = "AMDI0030:00@18",
 		},
 	},
+	{
+		/*
+		 * Spurious wakeups from TP_ATTN# pin
+		 * Found in BIOS 1.7.8
+		 * https://gitlab.freedesktop.org/drm/amd/-/issues/1722#note_1720627
+		 */
+		.matches = {
+			DMI_MATCH(DMI_BOARD_NAME, "NL5xRU"),
+		},
+		.driver_data = &(struct acpi_gpiolib_dmi_quirk) {
+			.ignore_wake = "ELAN0415:00@9",
+		},
+	},
 	{} /* Terminating entry */
 };