lkml.org 
[lkml]   [2022]   [Jul]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
SubjectRe: [PATCH 1/2] gpiolib: acpi: Add support to ignore programming an interrupt
From
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;
> }
>

\
 
 \ /
  Last update: 2022-07-27 15:16    [W:0.054 / U:0.096 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site