All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] gpiolib: acpi: Add support to ignore programming an interrupt
@ 2022-07-19 14:21 Mario Limonciello
  2022-07-19 14:21 ` [PATCH 2/2] gpiolib: acpi: Add a quirk for Asus UM325UAZ Mario Limonciello
  2022-07-27 13:15 ` [PATCH 1/2] gpiolib: acpi: Add support to ignore programming an interrupt Hans de Goede
  0 siblings, 2 replies; 5+ messages in thread
From: Mario Limonciello @ 2022-07-19 14:21 UTC (permalink / raw)
  To: mario.limonciello, Mika Westerberg, Andy Shevchenko,
	Linus Walleij, Bartosz Golaszewski
  Cc: Pavel Krc, linux-gpio, linux-acpi, linux-kernel

gpiolib-acpi already had support for ignoring a pin for wakeup, but
if an OEM configures a floating pin as an interrupt source then
stopping it from being a wakeup won't do much good to stop the
interrupt storm.

Add support for a module parameter and quirk infrastructure to
ignore interrupts as well.

Tested-by: Pavel Krc <reg.krn@pkrc.net>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/gpio/gpiolib-acpi.c | 39 ++++++++++++++++++++++++++++++++-----
 1 file changed, 34 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index c2523ac26fac..375942d92d6f 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -32,9 +32,21 @@ MODULE_PARM_DESC(ignore_wake,
 		 "controller@pin combos on which to ignore the ACPI wake flag "
 		 "ignore_wake=controller@pin[,controller@pin[,...]]");
 
+static char *ignore_interrupt;
+module_param(ignore_interrupt, charp, 0444);
+MODULE_PARM_DESC(ignore_interrupt,
+		 "controller@pin combos on which to ignore interrupt "
+		 "ignore_interrupt=controller@pin[,controller@pin[,...]]");
+
 struct acpi_gpiolib_dmi_quirk {
 	bool no_edge_events_on_boot;
 	char *ignore_wake;
+	char *ignore_interrupt;
+};
+
+enum ignore_type {
+	IGNORE_WAKEUP,
+	IGNORE_INTERRUPT,
 };
 
 /**
@@ -317,14 +329,18 @@ static struct gpio_desc *acpi_request_own_gpiod(struct gpio_chip *chip,
 	return desc;
 }
 
-static bool acpi_gpio_in_ignore_list(const char *controller_in, unsigned int pin_in)
+static bool acpi_gpio_in_ignore_list(const char *controller_in, unsigned int pin_in,
+				     enum ignore_type type)
 {
-	const char *controller, *pin_str;
+	const char *controller = NULL, *pin_str;
 	unsigned int pin;
 	char *endp;
 	int len;
 
-	controller = ignore_wake;
+	if (type == IGNORE_WAKEUP)
+		controller = ignore_wake;
+	else if (type == IGNORE_INTERRUPT)
+		controller = ignore_interrupt;
 	while (controller) {
 		pin_str = strchr(controller, '@');
 		if (!pin_str)
@@ -348,7 +364,12 @@ static bool acpi_gpio_in_ignore_list(const char *controller_in, unsigned int pin
 
 	return false;
 err:
-	pr_err_once("Error: Invalid value for gpiolib_acpi.ignore_wake: %s\n", ignore_wake);
+	if (type == IGNORE_WAKEUP)
+		pr_err_once("Error: Invalid value for gpiolib_acpi.ignore_wake: %s\n",
+			    ignore_wake);
+	else if (type == IGNORE_INTERRUPT)
+		pr_err_once("Error: Invalid value for gpiolib_acpi.ignore_interrupt: %s\n",
+			    ignore_interrupt);
 	return false;
 }
 
@@ -360,7 +381,7 @@ static bool acpi_gpio_irq_is_wake(struct device *parent,
 	if (agpio->wake_capable != ACPI_WAKE_CAPABLE)
 		return false;
 
-	if (acpi_gpio_in_ignore_list(dev_name(parent), pin)) {
+	if (acpi_gpio_in_ignore_list(dev_name(parent), pin, IGNORE_WAKEUP)) {
 		dev_info(parent, "Ignoring wakeup on pin %u\n", pin);
 		return false;
 	}
@@ -427,6 +448,11 @@ static acpi_status acpi_gpiochip_alloc_event(struct acpi_resource *ares,
 		goto fail_unlock_irq;
 	}
 
+	if (acpi_gpio_in_ignore_list(dev_name(chip->parent), pin, IGNORE_INTERRUPT)) {
+		dev_info(chip->parent, "Ignoring interrupt on pin %u\n", pin);
+		return AE_OK;
+	}
+
 	event = kzalloc(sizeof(*event), GFP_KERNEL);
 	if (!event)
 		goto fail_unlock_irq;
@@ -1582,6 +1608,9 @@ static int __init acpi_gpio_setup_params(void)
 	if (ignore_wake == NULL && quirk && quirk->ignore_wake)
 		ignore_wake = quirk->ignore_wake;
 
+	if (ignore_interrupt == NULL && quirk && quirk->ignore_interrupt)
+		ignore_interrupt = quirk->ignore_interrupt;
+
 	return 0;
 }
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] gpiolib: acpi: Add a quirk for Asus UM325UAZ
  2022-07-19 14:21 [PATCH 1/2] gpiolib: acpi: Add support to ignore programming an interrupt Mario Limonciello
@ 2022-07-19 14:21 ` Mario Limonciello
       [not found]   ` <CAHp75VfWwDiPecBdP0Nm-henzZVMoyiu+nWuV=eUkuxLkqeViw@mail.gmail.com>
  2022-07-27 13:15   ` Hans de Goede
  2022-07-27 13:15 ` [PATCH 1/2] gpiolib: acpi: Add support to ignore programming an interrupt Hans de Goede
  1 sibling, 2 replies; 5+ messages in thread
From: Mario Limonciello @ 2022-07-19 14:21 UTC (permalink / raw)
  To: mario.limonciello, Mika Westerberg, Andy Shevchenko,
	Linus Walleij, Bartosz Golaszewski
  Cc: Pavel Krc, linux-gpio, linux-acpi, linux-kernel

Asus UM325UAZ has GPIO 18 programmed as both an interrupt and a wake
source, but confirmed with internal team on this design this pin is
floating and shouldn't have been programmed. This causes lots of
spurious IRQs on the system and horrendous battery life.

Add a quirk to ignore attempts to program this pin on this system.

Reported-and-tested-by: Pavel Krc <reg.krn@pkrc.net>
Link: https://bugzilla.kernel.org/show_bug.cgi?id=216208
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/gpio/gpiolib-acpi.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 375942d92d6f..2149713ea8f1 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -1586,6 +1586,20 @@ static const struct dmi_system_id gpiolib_acpi_quirks[] __initconst = {
 			.ignore_wake = "INT33FF:01@0",
 		},
 	},
+	{
+		/*
+		 * Interrupt storm caused from edge triggered floating pin
+		 * Found in BIOS UX325UAZ.300
+		 * https://bugzilla.kernel.org/show_bug.cgi?id=216208
+		 */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "ZenBook UX325UAZ_UM325UAZ"),
+		},
+		.driver_data = &(struct acpi_gpiolib_dmi_quirk) {
+			.ignore_interrupt = "AMDI0030:00@18",
+		},
+	},
 	{} /* Terminating entry */
 };
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] gpiolib: acpi: Add a quirk for Asus UM325UAZ
       [not found]   ` <CAHp75VfWwDiPecBdP0Nm-henzZVMoyiu+nWuV=eUkuxLkqeViw@mail.gmail.com>
@ 2022-07-20 15:43     ` Limonciello, Mario
  0 siblings, 0 replies; 5+ messages in thread
From: Limonciello, Mario @ 2022-07-20 15:43 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Mika Westerberg, Andy Shevchenko, Linus Walleij,
	Bartosz Golaszewski, Pavel Krc, linux-gpio, linux-acpi,
	linux-kernel, Hans de Goede

On 7/20/2022 08:57, Andy Shevchenko wrote:
> I am on vacation (Mika also iirc), but I believe that Hans can review 
> it, so resend with cc’ing him and add above as a comment to the change, 
> or better cover letter.

Thanks, I redirected the original messages to Hans.  Enjoy your vacation.

> 
> On Tuesday, July 19, 2022, Mario Limonciello <mario.limonciello@amd.com 
> <mailto:mario.limonciello@amd.com>> wrote:
> 
>     Asus UM325UAZ has GPIO 18 programmed as both an interrupt and a wake
>     source, but confirmed with internal team on this design this pin is
>     floating and shouldn't have been programmed. This causes lots of
>     spurious IRQs on the system and horrendous battery life.
> 
>     Add a quirk to ignore attempts to program this pin on this system.
> 
>     Reported-and-tested-by: Pavel Krc <reg.krn@pkrc.net
>     <mailto:reg.krn@pkrc.net>>
>     Link: https://bugzilla.kernel.org/show_bug.cgi?id=216208
>     <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.kernel.org%2Fshow_bug.cgi%3Fid%3D216208&data=05%7C01%7Cmario.limonciello%40amd.com%7C2c67d6b468d44e177dbb08da6a57d2fd%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637939222681842692%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=INWMYURdKmQeTaFw1PT%2BEohTJGJSRRONYJCTw9klXu4%3D&reserved=0>
>     Signed-off-by: Mario Limonciello <mario.limonciello@amd.com
>     <mailto:mario.limonciello@amd.com>>
>     ---
>       drivers/gpio/gpiolib-acpi.c | 14 ++++++++++++++
>       1 file changed, 14 insertions(+)
> 
>     diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
>     index 375942d92d6f..2149713ea8f1 100644
>     --- a/drivers/gpio/gpiolib-acpi.c
>     +++ b/drivers/gpio/gpiolib-acpi.c
>     @@ -1586,6 +1586,20 @@ static const struct dmi_system_id
>     gpiolib_acpi_quirks[] __initconst = {
>                              .ignore_wake = "INT33FF:01@0",
>                      },
>              },
>     +       {
>     +               /*
>     +                * Interrupt storm caused from edge triggered
>     floating pin
>     +                * Found in BIOS UX325UAZ.300
>     +                *
>     https://bugzilla.kernel.org/show_bug.cgi?id=216208
>     <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.kernel.org%2Fshow_bug.cgi%3Fid%3D216208&data=05%7C01%7Cmario.limonciello%40amd.com%7C2c67d6b468d44e177dbb08da6a57d2fd%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637939222681842692%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=INWMYURdKmQeTaFw1PT%2BEohTJGJSRRONYJCTw9klXu4%3D&reserved=0>
>     +                */
>     +               .matches = {
>     +                       DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER
>     INC."),
>     +                       DMI_MATCH(DMI_PRODUCT_NAME, "ZenBook
>     UX325UAZ_UM325UAZ"),
>     +               },
>     +               .driver_data = &(struct acpi_gpiolib_dmi_quirk) {
>     +                       .ignore_interrupt = "AMDI0030:00@18",
>     +               },
>     +       },
>              {} /* Terminating entry */
>       };
> 
>     -- 
>     2.34.1
> 
> 
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] gpiolib: acpi: Add support to ignore programming an interrupt
  2022-07-19 14:21 [PATCH 1/2] gpiolib: acpi: Add support to ignore programming an interrupt Mario Limonciello
  2022-07-19 14:21 ` [PATCH 2/2] gpiolib: acpi: Add a quirk for Asus UM325UAZ Mario Limonciello
@ 2022-07-27 13:15 ` Hans de Goede
  1 sibling, 0 replies; 5+ messages in thread
From: Hans de Goede @ 2022-07-27 13:15 UTC (permalink / raw)
  To: Mario Limonciello, Mika Westerberg, Andy Shevchenko,
	Linus Walleij, Bartosz Golaszewski
  Cc: Pavel Krc, linux-gpio, linux-acpi, linux-kernel

Hi,

On 7/19/22 16:21, Mario Limonciello wrote:
> gpiolib-acpi already had support for ignoring a pin for wakeup, but
> if an OEM configures a floating pin as an interrupt source then
> stopping it from being a wakeup won't do much good to stop the
> interrupt storm.
> 
> Add support for a module parameter and quirk infrastructure to
> ignore interrupts as well.
> 
> Tested-by: Pavel Krc <reg.krn@pkrc.net>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  drivers/gpio/gpiolib-acpi.c | 39 ++++++++++++++++++++++++++++++++-----
>  1 file changed, 34 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
> index c2523ac26fac..375942d92d6f 100644
> --- a/drivers/gpio/gpiolib-acpi.c
> +++ b/drivers/gpio/gpiolib-acpi.c
> @@ -32,9 +32,21 @@ MODULE_PARM_DESC(ignore_wake,
>  		 "controller@pin combos on which to ignore the ACPI wake flag "
>  		 "ignore_wake=controller@pin[,controller@pin[,...]]");
>  
> +static char *ignore_interrupt;
> +module_param(ignore_interrupt, charp, 0444);
> +MODULE_PARM_DESC(ignore_interrupt,
> +		 "controller@pin combos on which to ignore interrupt "
> +		 "ignore_interrupt=controller@pin[,controller@pin[,...]]");
> +
>  struct acpi_gpiolib_dmi_quirk {
>  	bool no_edge_events_on_boot;
>  	char *ignore_wake;
> +	char *ignore_interrupt;
> +};
> +
> +enum ignore_type {
> +	IGNORE_WAKEUP,
> +	IGNORE_INTERRUPT,
>  };

Please drop the enum; and ...

>  
>  /**
> @@ -317,14 +329,18 @@ static struct gpio_desc *acpi_request_own_gpiod(struct gpio_chip *chip,
>  	return desc;
>  }
>  
> -static bool acpi_gpio_in_ignore_list(const char *controller_in, unsigned int pin_in)
> +static bool acpi_gpio_in_ignore_list(const char *controller_in, unsigned int pin_in,
> +				     enum ignore_type type)
>  {

make the prototype:

static bool acpi_gpio_in_ignore_list(const char *ignore_list, const char *controller_in, unsigned int pin_in)

and ...

> -	const char *controller, *pin_str;
> +	const char *controller = NULL, *pin_str;
>  	unsigned int pin;
>  	char *endp;
>  	int len;
>  
> -	controller = ignore_wake;
> +	if (type == IGNORE_WAKEUP)
> +		controller = ignore_wake;
> +	else if (type == IGNORE_INTERRUPT)
> +		controller = ignore_interrupt;

Use:

	controller = ignore_list;

here; and ...

>  	while (controller) {
>  		pin_str = strchr(controller, '@');
>  		if (!pin_str)
> @@ -348,7 +364,12 @@ static bool acpi_gpio_in_ignore_list(const char *controller_in, unsigned int pin
>  
>  	return false;
>  err:
> -	pr_err_once("Error: Invalid value for gpiolib_acpi.ignore_wake: %s\n", ignore_wake);
> +	if (type == IGNORE_WAKEUP)
> +		pr_err_once("Error: Invalid value for gpiolib_acpi.ignore_wake: %s\n",
> +			    ignore_wake);
> +	else if (type == IGNORE_INTERRUPT)
> +		pr_err_once("Error: Invalid value for gpiolib_acpi.ignore_interrupt: %s\n",
> +			    ignore_interrupt);

change this to:

	pr_err_once("Error: Invalid value for gpiolib_acpi.ignore_...: %s\n", ignore_list);

and ...

>  	return false;
>  }
>  
> @@ -360,7 +381,7 @@ static bool acpi_gpio_irq_is_wake(struct device *parent,
>  	if (agpio->wake_capable != ACPI_WAKE_CAPABLE)
>  		return false;
>  
> -	if (acpi_gpio_in_ignore_list(dev_name(parent), pin)) {
> +	if (acpi_gpio_in_ignore_list(dev_name(parent), pin, IGNORE_WAKEUP)) {

adjust this to:

	if (acpi_gpio_in_ignore_list(ignore_wakeup, dev_name(parent), pin)) {

and ...

>  		dev_info(parent, "Ignoring wakeup on pin %u\n", pin);
>  		return false;
>  	}
> @@ -427,6 +448,11 @@ static acpi_status acpi_gpiochip_alloc_event(struct acpi_resource *ares,
>  		goto fail_unlock_irq;
>  	}
>  
> +	if (acpi_gpio_in_ignore_list(dev_name(chip->parent), pin, IGNORE_INTERRUPT)) {

this line to:

	if (acpi_gpio_in_ignore_list(ignore_interrupt, dev_name(parent), pin)) {

That IMHO is a cleaner way to handle this then introducing the enum type +
enum parameter to acpi_gpio_in_ignore_list().

Regards,

Hans



> +		dev_info(chip->parent, "Ignoring interrupt on pin %u\n", pin);
> +		return AE_OK;
> +	}
> +
>  	event = kzalloc(sizeof(*event), GFP_KERNEL);
>  	if (!event)
>  		goto fail_unlock_irq;
> @@ -1582,6 +1608,9 @@ static int __init acpi_gpio_setup_params(void)
>  	if (ignore_wake == NULL && quirk && quirk->ignore_wake)
>  		ignore_wake = quirk->ignore_wake;
>  
> +	if (ignore_interrupt == NULL && quirk && quirk->ignore_interrupt)
> +		ignore_interrupt = quirk->ignore_interrupt;
> +
>  	return 0;
>  }
>  


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] gpiolib: acpi: Add a quirk for Asus UM325UAZ
  2022-07-19 14:21 ` [PATCH 2/2] gpiolib: acpi: Add a quirk for Asus UM325UAZ Mario Limonciello
       [not found]   ` <CAHp75VfWwDiPecBdP0Nm-henzZVMoyiu+nWuV=eUkuxLkqeViw@mail.gmail.com>
@ 2022-07-27 13:15   ` Hans de Goede
  1 sibling, 0 replies; 5+ messages in thread
From: Hans de Goede @ 2022-07-27 13:15 UTC (permalink / raw)
  To: Mario Limonciello, Mika Westerberg, Andy Shevchenko,
	Linus Walleij, Bartosz Golaszewski
  Cc: Pavel Krc, linux-gpio, linux-acpi, linux-kernel

Hi,

On 7/19/22 16:21, Mario Limonciello wrote:
> Asus UM325UAZ has GPIO 18 programmed as both an interrupt and a wake
> source, but confirmed with internal team on this design this pin is
> floating and shouldn't have been programmed. This causes lots of
> spurious IRQs on the system and horrendous battery life.
> 
> Add a quirk to ignore attempts to program this pin on this system.
> 
> Reported-and-tested-by: Pavel Krc <reg.krn@pkrc.net>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=216208
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans


> ---
>  drivers/gpio/gpiolib-acpi.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
> index 375942d92d6f..2149713ea8f1 100644
> --- a/drivers/gpio/gpiolib-acpi.c
> +++ b/drivers/gpio/gpiolib-acpi.c
> @@ -1586,6 +1586,20 @@ static const struct dmi_system_id gpiolib_acpi_quirks[] __initconst = {
>  			.ignore_wake = "INT33FF:01@0",
>  		},
>  	},
> +	{
> +		/*
> +		 * Interrupt storm caused from edge triggered floating pin
> +		 * Found in BIOS UX325UAZ.300
> +		 * https://bugzilla.kernel.org/show_bug.cgi?id=216208
> +		 */
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "ZenBook UX325UAZ_UM325UAZ"),
> +		},
> +		.driver_data = &(struct acpi_gpiolib_dmi_quirk) {
> +			.ignore_interrupt = "AMDI0030:00@18",
> +		},
> +	},
>  	{} /* Terminating entry */
>  };
>  


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-07-27 13:15 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-19 14:21 [PATCH 1/2] gpiolib: acpi: Add support to ignore programming an interrupt Mario Limonciello
2022-07-19 14:21 ` [PATCH 2/2] gpiolib: acpi: Add a quirk for Asus UM325UAZ Mario Limonciello
     [not found]   ` <CAHp75VfWwDiPecBdP0Nm-henzZVMoyiu+nWuV=eUkuxLkqeViw@mail.gmail.com>
2022-07-20 15:43     ` Limonciello, Mario
2022-07-27 13:15   ` Hans de Goede
2022-07-27 13:15 ` [PATCH 1/2] gpiolib: acpi: Add support to ignore programming an interrupt Hans de Goede

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.