[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1d891d34-053a-368d-cf47-bcaf35284c79@redhat.com>
Date: Tue, 29 Aug 2023 21:54:25 +0200
From: Hans de Goede <hdegoede@...hat.com>
To: Mario Limonciello <mario.limonciello@....com>,
linus.walleij@...aro.org
Cc: Shyam-sundar.S-k@....com, Basavaraj.Natikar@....com,
linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
regressions@...ts.linux.dev, lucapgl2001@...il.com
Subject: Re: [PATCH 3/3] pinctrl: amd: Add a quirk for Lenovo Ideapad 5
Hi Mario,
On 8/29/23 18:56, Mario Limonciello wrote:
> Lenovo ideapad 5 doesn't use interrupts for GPIO 0, and so internally
> debouncing with WinBlue debounce behavior means that the GPIO doesn't
> clear until a separate GPIO is used (such as touchpad).
>
> Prefer to use legacy debouncing to avoid problems.
>
> Reported-by: Luca Pigliacampo <lucapgl2001@...il.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217833
> Signed-off-by: Mario Limonciello <mario.limonciello@....com>
I'm not happy to see yet another DMI quirk solution here.
and I guess you're not happy with this either...
Are we sure there is no other way? Did you check an acpidump
for the laptop and specifically for its ACPI powerbutton handling?
I would expect the ACPI powerbutton handler to somehow clear
the bit, like how patch 1/3 clears it from the GPIO chip's
own IRQ handler.
I see that drivers/acpi/button.c does:
static u32 acpi_button_event(void *data)
{
acpi_os_execute(OSL_NOTIFY_HANDLER, acpi_button_notify_run, data);
return ACPI_INTERRUPT_HANDLED;
}
So unless I'm misreading something here, there is some AML being
executed on power-button events. So maybe there is something wrong
with how Linux interprets that AML ?
Regards,
Hans
> ---
> drivers/pinctrl/pinctrl-amd.c | 34 ++++++++++++++++++++++++++++++++--
> 1 file changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-amd.c b/drivers/pinctrl/pinctrl-amd.c
> index a2468a988be3..2e1721a9249a 100644
> --- a/drivers/pinctrl/pinctrl-amd.c
> +++ b/drivers/pinctrl/pinctrl-amd.c
> @@ -8,6 +8,7 @@
> *
> */
>
> +#include <linux/dmi.h>
> #include <linux/err.h>
> #include <linux/bug.h>
> #include <linux/kernel.h>
> @@ -41,6 +42,27 @@ module_param(powerbtn, int, 0444);
> MODULE_PARM_DESC(powerbtn,
> "Power button debouncing: 0=traditional, 1=windows, -1=auto");
>
> +struct pinctrl_amd_dmi_quirk {
> + int powerbtn;
> +};
> +
> +static const struct dmi_system_id pinctrl_amd_dmi_quirks[] __initconst = {
> + {
> + /*
> + * Lenovo Ideapad 5
> + * Power button GPIO not cleared until touchpad movement
> + * https://bugzilla.kernel.org/show_bug.cgi?id=217833
> + */
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "82LM"),
> + },
> + .driver_data = &(struct pinctrl_amd_dmi_quirk) {
> + .powerbtn = 0,
> + },
> + }
> +};
> +
> static int amd_gpio_get_direction(struct gpio_chip *gc, unsigned offset)
> {
> unsigned long flags;
> @@ -1084,8 +1106,16 @@ static void handle_powerbtn(struct amd_gpio *gpio_dev)
> {
> u32 pin_reg;
>
> - if (powerbtn == -1)
> - return;
> + if (powerbtn == -1) {
> + const struct pinctrl_amd_dmi_quirk *quirk = NULL;
> + const struct dmi_system_id *id;
> +
> + id = dmi_first_match(pinctrl_amd_dmi_quirks);
> + if (!id)
> + return;
> + quirk = id->driver_data;
> + powerbtn = quirk->powerbtn;
> + }
>
> pin_reg = readl(gpio_dev->base + WAKE_INT_MASTER_REG);
> switch (powerbtn) {
Powered by blists - more mailing lists