Commit 78d3a92edbfb ("gpiolib-acpi: Register GpioInt ACPI event handlers from a late_initcall") deferred the entire acpi_gpiochip_request_interrupt call for each event resource.
This means it also delays the gpiochip_request_own_desc(..., "ACPI:Event") call. This is a problem if some AML code reads the GPIO pin before we run the deferred acpi_gpiochip_request_interrupt, because in that case acpi_gpio_adr_space_handler() will already have called gpiochip_request_own_desc(..., "ACPI:OpRegion") causing the call from acpi_gpiochip_request_interrupt to fail with -EBUSY and we will fail to register an event handler.
acpi_gpio_adr_space_handler is prepared for acpi_gpiochip_request_interrupt already having claimed the pin, but the other way around does not work.
One example of a problem this causes, is the event handler for the OTG ID pin on a Prowise PT301 tablet not registering, keeping the port stuck in whatever mode it was in during boot and e.g. only allowing charging after a reboot.
This commit fixes this by only deferring the request_irq call and the initial run of edge-triggered IRQs instead of deferring all of acpi_gpiochip_request_interrupt.
Cc: stable@vger.kernel.org Fixes: 78d3a92edbfb ("gpiolib-acpi: Register GpioInt ACPI event ...") Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/gpio/gpiolib-acpi.c | 136 +++++++++++++++++++----------------- 1 file changed, 73 insertions(+), 63 deletions(-)
diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c index 55b72fbe1631..bda19373835a 100644 --- a/drivers/gpio/gpiolib-acpi.c +++ b/drivers/gpio/gpiolib-acpi.c @@ -22,8 +22,12 @@ struct acpi_gpio_event { struct list_head node; acpi_handle handle; + irq_handler_t handler; unsigned int pin; unsigned int irq; + unsigned long irqflags; + bool irq_is_wake; + bool irq_requested; struct gpio_desc *desc; };
@@ -49,10 +53,10 @@ struct acpi_gpio_chip {
/* * For gpiochips which call acpi_gpiochip_request_interrupts() before late_init - * (so builtin drivers) we register the ACPI GpioInt event handlers from a + * (so builtin drivers) we register the ACPI GpioInt irq-handlers from a * late_initcall_sync handler, so that other builtin drivers can register their * OpRegions before the event handlers can run. This list contains gpiochips - * for which the acpi_gpiochip_request_interrupts() has been deferred. + * for which the acpi_gpiochip_request_irqs() call has been deferred. */ static DEFINE_MUTEX(acpi_gpio_deferred_req_irqs_lock); static LIST_HEAD(acpi_gpio_deferred_req_irqs_list); @@ -133,8 +137,43 @@ bool acpi_gpio_get_irq_resource(struct acpi_resource *ares, } EXPORT_SYMBOL_GPL(acpi_gpio_get_irq_resource);
-static acpi_status acpi_gpiochip_request_interrupt(struct acpi_resource *ares, - void *context) +static void acpi_gpiochip_request_irq(struct acpi_gpio_chip *acpi_gpio, + struct acpi_gpio_event *event) +{ + int ret, value; + + ret = request_threaded_irq(event->irq, NULL, event->handler, + event->irqflags, "ACPI:Event", event); + if (ret) { + dev_err(acpi_gpio->chip->parent, + "Failed to setup interrupt handler for %d\n", + event->irq); + return; + } + + if (event->irq_is_wake) + enable_irq_wake(event->irq); + + event->irq_requested = true; + + /* Make sure we trigger the initial state of edge-triggered IRQs */ + value = gpiod_get_raw_value_cansleep(event->desc); + if (((event->irqflags & IRQF_TRIGGER_RISING) && value == 1) || + ((event->irqflags & IRQF_TRIGGER_FALLING) && value == 0)) + event->handler(event->irq, event); +} + +static void acpi_gpiochip_request_irqs(struct acpi_gpio_chip *acpi_gpio) +{ + struct acpi_gpio_event *event; + + list_for_each_entry(event, &acpi_gpio->events, node) + acpi_gpiochip_request_irq(acpi_gpio, event); +} + +static acpi_status +acpi_gpiochip_request_interrupt_desc(struct acpi_resource *ares, + void *context) { struct acpi_gpio_chip *acpi_gpio = context; struct gpio_chip *chip = acpi_gpio->chip; @@ -143,8 +182,7 @@ static acpi_status acpi_gpiochip_request_interrupt(struct acpi_resource *ares, struct acpi_gpio_event *event; irq_handler_t handler = NULL; struct gpio_desc *desc; - unsigned long irqflags; - int ret, pin, irq, value; + int ret, pin, irq;
if (!acpi_gpio_get_irq_resource(ares, &agpio)) return AE_OK; @@ -175,8 +213,6 @@ static acpi_status acpi_gpiochip_request_interrupt(struct acpi_resource *ares,
gpiod_direction_input(desc);
- value = gpiod_get_value_cansleep(desc); - ret = gpiochip_lock_as_irq(chip, pin); if (ret) { dev_err(chip->parent, "Failed to lock GPIO as interrupt\n"); @@ -189,64 +225,42 @@ static acpi_status acpi_gpiochip_request_interrupt(struct acpi_resource *ares, goto fail_unlock_irq; }
- irqflags = IRQF_ONESHOT; + event = kzalloc(sizeof(*event), GFP_KERNEL); + if (!event) + goto fail_unlock_irq; + + event->irqflags = IRQF_ONESHOT; if (agpio->triggering == ACPI_LEVEL_SENSITIVE) { if (agpio->polarity == ACPI_ACTIVE_HIGH) - irqflags |= IRQF_TRIGGER_HIGH; + event->irqflags |= IRQF_TRIGGER_HIGH; else - irqflags |= IRQF_TRIGGER_LOW; + event->irqflags |= IRQF_TRIGGER_LOW; } else { switch (agpio->polarity) { case ACPI_ACTIVE_HIGH: - irqflags |= IRQF_TRIGGER_RISING; + event->irqflags |= IRQF_TRIGGER_RISING; break; case ACPI_ACTIVE_LOW: - irqflags |= IRQF_TRIGGER_FALLING; + event->irqflags |= IRQF_TRIGGER_FALLING; break; default: - irqflags |= IRQF_TRIGGER_RISING | - IRQF_TRIGGER_FALLING; + event->irqflags |= IRQF_TRIGGER_RISING | + IRQF_TRIGGER_FALLING; break; } }
- event = kzalloc(sizeof(*event), GFP_KERNEL); - if (!event) - goto fail_unlock_irq; - event->handle = evt_handle; + event->handler = handler; event->irq = irq; + event->irq_is_wake = agpio->wake_capable == ACPI_WAKE_CAPABLE; event->pin = pin; event->desc = desc;
- ret = request_threaded_irq(event->irq, NULL, handler, irqflags, - "ACPI:Event", event); - if (ret) { - dev_err(chip->parent, - "Failed to setup interrupt handler for %d\n", - event->irq); - goto fail_free_event; - } - - if (agpio->wake_capable == ACPI_WAKE_CAPABLE) - enable_irq_wake(irq); - list_add_tail(&event->node, &acpi_gpio->events);
- /* - * Make sure we trigger the initial state of the IRQ when using RISING - * or FALLING. Note we run the handlers on late_init, the AML code - * may refer to OperationRegions from other (builtin) drivers which - * may be probed after us. - */ - if (((irqflags & IRQF_TRIGGER_RISING) && value == 1) || - ((irqflags & IRQF_TRIGGER_FALLING) && value == 0)) - handler(event->irq, event); - return AE_OK;
-fail_free_event: - kfree(event); fail_unlock_irq: gpiochip_unlock_as_irq(chip, pin); fail_free_desc: @@ -283,6 +297,9 @@ void acpi_gpiochip_request_interrupts(struct gpio_chip *chip) if (ACPI_FAILURE(status)) return;
+ acpi_walk_resources(handle, "_AEI", + acpi_gpiochip_request_interrupt_desc, acpi_gpio); + mutex_lock(&acpi_gpio_deferred_req_irqs_lock); defer = !acpi_gpio_deferred_req_irqs_done; if (defer) @@ -290,11 +307,8 @@ void acpi_gpiochip_request_interrupts(struct gpio_chip *chip) &acpi_gpio_deferred_req_irqs_list); mutex_unlock(&acpi_gpio_deferred_req_irqs_lock);
- if (defer) - return; - - acpi_walk_resources(handle, "_AEI", - acpi_gpiochip_request_interrupt, acpi_gpio); + if (!defer) + acpi_gpiochip_request_irqs(acpi_gpio); } EXPORT_SYMBOL_GPL(acpi_gpiochip_request_interrupts);
@@ -331,10 +345,13 @@ void acpi_gpiochip_free_interrupts(struct gpio_chip *chip) list_for_each_entry_safe_reverse(event, ep, &acpi_gpio->events, node) { struct gpio_desc *desc;
- if (irqd_is_wakeup_set(irq_get_irq_data(event->irq))) - disable_irq_wake(event->irq); + if (event->irq_requested) { + if (event->irq_is_wake) + disable_irq_wake(event->irq); + + free_irq(event->irq, event); + }
- free_irq(event->irq, event); desc = event->desc; if (WARN_ON(IS_ERR(desc))) continue; @@ -1200,23 +1217,16 @@ bool acpi_can_fallback_to_crs(struct acpi_device *adev, const char *con_id) return con_id == NULL; }
-/* Run deferred acpi_gpiochip_request_interrupts() */ -static int acpi_gpio_handle_deferred_request_interrupts(void) +/* Run deferred acpi_gpiochip_request_irqs() */ +static int acpi_gpio_handle_deferred_request_irqs(void) { struct acpi_gpio_chip *acpi_gpio, *tmp;
mutex_lock(&acpi_gpio_deferred_req_irqs_lock); list_for_each_entry_safe(acpi_gpio, tmp, &acpi_gpio_deferred_req_irqs_list, - deferred_req_irqs_list_entry) { - acpi_handle handle; - - handle = ACPI_HANDLE(acpi_gpio->chip->parent); - acpi_walk_resources(handle, "_AEI", - acpi_gpiochip_request_interrupt, acpi_gpio); - - list_del_init(&acpi_gpio->deferred_req_irqs_list_entry); - } + deferred_req_irqs_list_entry) + acpi_gpiochip_request_irqs(acpi_gpio);
acpi_gpio_deferred_req_irqs_done = true; mutex_unlock(&acpi_gpio_deferred_req_irqs_lock); @@ -1224,4 +1234,4 @@ static int acpi_gpio_handle_deferred_request_interrupts(void) return 0; } /* We must use _sync so that this runs after the first deferred_probe run */ -late_initcall_sync(acpi_gpio_handle_deferred_request_interrupts); +late_initcall_sync(acpi_gpio_handle_deferred_request_irqs);
On Mon, Nov 26, 2018 at 11:42:31AM +0100, Hans de Goede wrote:
Commit 78d3a92edbfb ("gpiolib-acpi: Register GpioInt ACPI event handlers from a late_initcall") deferred the entire acpi_gpiochip_request_interrupt call for each event resource.
This means it also delays the gpiochip_request_own_desc(..., "ACPI:Event") call. This is a problem if some AML code reads the GPIO pin before we run the deferred acpi_gpiochip_request_interrupt, because in that case acpi_gpio_adr_space_handler() will already have called gpiochip_request_own_desc(..., "ACPI:OpRegion") causing the call from acpi_gpiochip_request_interrupt to fail with -EBUSY and we will fail to register an event handler.
acpi_gpio_adr_space_handler is prepared for acpi_gpiochip_request_interrupt already having claimed the pin, but the other way around does not work.
One example of a problem this causes, is the event handler for the OTG ID pin on a Prowise PT301 tablet not registering, keeping the port stuck in whatever mode it was in during boot and e.g. only allowing charging after a reboot.
This commit fixes this by only deferring the request_irq call and the initial run of edge-triggered IRQs instead of deferring all of acpi_gpiochip_request_interrupt.
Cc: stable@vger.kernel.org Fixes: 78d3a92edbfb ("gpiolib-acpi: Register GpioInt ACPI event ...") Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/gpio/gpiolib-acpi.c | 136 +++++++++++++++++++----------------- 1 file changed, 73 insertions(+), 63 deletions(-)
diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c index 55b72fbe1631..bda19373835a 100644 --- a/drivers/gpio/gpiolib-acpi.c +++ b/drivers/gpio/gpiolib-acpi.c @@ -22,8 +22,12 @@ struct acpi_gpio_event { struct list_head node; acpi_handle handle;
- irq_handler_t handler; unsigned int pin; unsigned int irq;
- unsigned long irqflags;
- bool irq_is_wake;
- bool irq_requested;
It might be a good idea to add kernel-doc now for the struct since it seems to be growing and it is not immediately clear what each field mean anymore ;-)
struct gpio_desc *desc; }; @@ -49,10 +53,10 @@ struct acpi_gpio_chip { /*
- For gpiochips which call acpi_gpiochip_request_interrupts() before late_init
- (so builtin drivers) we register the ACPI GpioInt event handlers from a
- (so builtin drivers) we register the ACPI GpioInt irq-handlers from a
- late_initcall_sync handler, so that other builtin drivers can register their
- OpRegions before the event handlers can run. This list contains gpiochips
- for which the acpi_gpiochip_request_interrupts() has been deferred.
*/
- for which the acpi_gpiochip_request_irqs() call has been deferred.
static DEFINE_MUTEX(acpi_gpio_deferred_req_irqs_lock); static LIST_HEAD(acpi_gpio_deferred_req_irqs_list); @@ -133,8 +137,43 @@ bool acpi_gpio_get_irq_resource(struct acpi_resource *ares, } EXPORT_SYMBOL_GPL(acpi_gpio_get_irq_resource); -static acpi_status acpi_gpiochip_request_interrupt(struct acpi_resource *ares,
void *context)
+static void acpi_gpiochip_request_irq(struct acpi_gpio_chip *acpi_gpio,
struct acpi_gpio_event *event)
+{
- int ret, value;
- ret = request_threaded_irq(event->irq, NULL, event->handler,
event->irqflags, "ACPI:Event", event);
- if (ret) {
dev_err(acpi_gpio->chip->parent,
"Failed to setup interrupt handler for %d\n",
event->irq);
return;
- }
- if (event->irq_is_wake)
enable_irq_wake(event->irq);
- event->irq_requested = true;
- /* Make sure we trigger the initial state of edge-triggered IRQs */
- value = gpiod_get_raw_value_cansleep(event->desc);
- if (((event->irqflags & IRQF_TRIGGER_RISING) && value == 1) ||
((event->irqflags & IRQF_TRIGGER_FALLING) && value == 0))
event->handler(event->irq, event);
+}
+static void acpi_gpiochip_request_irqs(struct acpi_gpio_chip *acpi_gpio) +{
- struct acpi_gpio_event *event;
- list_for_each_entry(event, &acpi_gpio->events, node)
acpi_gpiochip_request_irq(acpi_gpio, event);
+}
+static acpi_status +acpi_gpiochip_request_interrupt_desc(struct acpi_resource *ares,
void *context)
I wonder if there could be better name for this now that it does not request anything? Maybe acpi_gpiochip_event_alloc() or similar?
Hi,
On 26-11-18 13:39, Mika Westerberg wrote:
On Mon, Nov 26, 2018 at 11:42:31AM +0100, Hans de Goede wrote:
Commit 78d3a92edbfb ("gpiolib-acpi: Register GpioInt ACPI event handlers from a late_initcall") deferred the entire acpi_gpiochip_request_interrupt call for each event resource.
This means it also delays the gpiochip_request_own_desc(..., "ACPI:Event") call. This is a problem if some AML code reads the GPIO pin before we run the deferred acpi_gpiochip_request_interrupt, because in that case acpi_gpio_adr_space_handler() will already have called gpiochip_request_own_desc(..., "ACPI:OpRegion") causing the call from acpi_gpiochip_request_interrupt to fail with -EBUSY and we will fail to register an event handler.
acpi_gpio_adr_space_handler is prepared for acpi_gpiochip_request_interrupt already having claimed the pin, but the other way around does not work.
One example of a problem this causes, is the event handler for the OTG ID pin on a Prowise PT301 tablet not registering, keeping the port stuck in whatever mode it was in during boot and e.g. only allowing charging after a reboot.
This commit fixes this by only deferring the request_irq call and the initial run of edge-triggered IRQs instead of deferring all of acpi_gpiochip_request_interrupt.
Cc: stable@vger.kernel.org Fixes: 78d3a92edbfb ("gpiolib-acpi: Register GpioInt ACPI event ...") Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/gpio/gpiolib-acpi.c | 136 +++++++++++++++++++----------------- 1 file changed, 73 insertions(+), 63 deletions(-)
diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c index 55b72fbe1631..bda19373835a 100644 --- a/drivers/gpio/gpiolib-acpi.c +++ b/drivers/gpio/gpiolib-acpi.c @@ -22,8 +22,12 @@ struct acpi_gpio_event { struct list_head node; acpi_handle handle;
- irq_handler_t handler; unsigned int pin; unsigned int irq;
- unsigned long irqflags;
- bool irq_is_wake;
- bool irq_requested;
It might be a good idea to add kernel-doc now for the struct since it seems to be growing and it is not immediately clear what each field mean anymore ;-)
Ok, will do for v2.
struct gpio_desc *desc; }; @@ -49,10 +53,10 @@ struct acpi_gpio_chip { /*
- For gpiochips which call acpi_gpiochip_request_interrupts() before late_init
- (so builtin drivers) we register the ACPI GpioInt event handlers from a
- (so builtin drivers) we register the ACPI GpioInt irq-handlers from a
- late_initcall_sync handler, so that other builtin drivers can register their
- OpRegions before the event handlers can run. This list contains gpiochips
- for which the acpi_gpiochip_request_interrupts() has been deferred.
*/ static DEFINE_MUTEX(acpi_gpio_deferred_req_irqs_lock); static LIST_HEAD(acpi_gpio_deferred_req_irqs_list);
- for which the acpi_gpiochip_request_irqs() call has been deferred.
@@ -133,8 +137,43 @@ bool acpi_gpio_get_irq_resource(struct acpi_resource *ares, } EXPORT_SYMBOL_GPL(acpi_gpio_get_irq_resource); -static acpi_status acpi_gpiochip_request_interrupt(struct acpi_resource *ares,
void *context)
+static void acpi_gpiochip_request_irq(struct acpi_gpio_chip *acpi_gpio,
struct acpi_gpio_event *event)
+{
- int ret, value;
- ret = request_threaded_irq(event->irq, NULL, event->handler,
event->irqflags, "ACPI:Event", event);
- if (ret) {
dev_err(acpi_gpio->chip->parent,
"Failed to setup interrupt handler for %d\n",
event->irq);
return;
- }
- if (event->irq_is_wake)
enable_irq_wake(event->irq);
- event->irq_requested = true;
- /* Make sure we trigger the initial state of edge-triggered IRQs */
- value = gpiod_get_raw_value_cansleep(event->desc);
- if (((event->irqflags & IRQF_TRIGGER_RISING) && value == 1) ||
((event->irqflags & IRQF_TRIGGER_FALLING) && value == 0))
event->handler(event->irq, event);
+}
+static void acpi_gpiochip_request_irqs(struct acpi_gpio_chip *acpi_gpio) +{
- struct acpi_gpio_event *event;
- list_for_each_entry(event, &acpi_gpio->events, node)
acpi_gpiochip_request_irq(acpi_gpio, event);
+}
+static acpi_status +acpi_gpiochip_request_interrupt_desc(struct acpi_resource *ares,
void *context)
I wonder if there could be better name for this now that it does not request anything? Maybe acpi_gpiochip_event_alloc() or similar?
Sure I will rename this to acpi_gpiochip_alloc_event (verb then subject to follow the style of the rest of the file) for v2.
I will prepare a v2 of this patch this evening or tomorrow.
Regards,
Hans
On Mon, Nov 26, 2018 at 11:42:31AM +0100, Hans de Goede wrote:
Commit 78d3a92edbfb ("gpiolib-acpi: Register GpioInt ACPI event handlers from a late_initcall") deferred the entire acpi_gpiochip_request_interrupt call for each event resource.
This means it also delays the gpiochip_request_own_desc(..., "ACPI:Event") call. This is a problem if some AML code reads the GPIO pin before we run the deferred acpi_gpiochip_request_interrupt, because in that case acpi_gpio_adr_space_handler() will already have called gpiochip_request_own_desc(..., "ACPI:OpRegion") causing the call from acpi_gpiochip_request_interrupt to fail with -EBUSY and we will fail to register an event handler.
acpi_gpio_adr_space_handler is prepared for acpi_gpiochip_request_interrupt already having claimed the pin, but the other way around does not work.
One example of a problem this causes, is the event handler for the OTG ID pin on a Prowise PT301 tablet not registering, keeping the port stuck in whatever mode it was in during boot and e.g. only allowing charging after a reboot.
This commit fixes this by only deferring the request_irq call and the initial run of edge-triggered IRQs instead of deferring all of acpi_gpiochip_request_interrupt.
Thanks for a patch with such nice description. I have couple of nitpicks and one question below.
- For gpiochips which call acpi_gpiochip_request_interrupts() before late_init
- (so builtin drivers) we register the ACPI GpioInt event handlers from a
- (so builtin drivers) we register the ACPI GpioInt irq-handlers from a
I would rather spell "IRQ handlers"
- late_initcall_sync handler, so that other builtin drivers can register their
- OpRegions before the event handlers can run. This list contains gpiochips
- for which the acpi_gpiochip_request_interrupts() has been deferred.
- for which the acpi_gpiochip_request_irqs() call has been deferred.
mutex_unlock(&acpi_gpio_deferred_req_irqs_lock);
- if (defer)
return;
- acpi_walk_resources(handle, "_AEI",
acpi_gpiochip_request_interrupt, acpi_gpio);
- if (!defer)
acpi_gpiochip_request_irqs(acpi_gpio);
Can we leave above condition and put here just
acpi_gpiochip_request_irqs(acpi_gpio);
?
list_for_each_entry_safe_reverse(event, ep, &acpi_gpio->events, node) { struct gpio_desc *desc;
if (event->irq_requested) {
if (event->irq_is_wake)
disable_irq_wake(event->irq);
free_irq(event->irq, event);
}
desc = event->desc; if (WARN_ON(IS_ERR(desc)))
I don't remember if I asked already about possible memory leak here. Am I reading code right?
continue;
Reading more code, I'm even not sure if this condition could ever happen.
Hi,
On 26-11-18 13:53, Andy Shevchenko wrote:
On Mon, Nov 26, 2018 at 11:42:31AM +0100, Hans de Goede wrote:
Commit 78d3a92edbfb ("gpiolib-acpi: Register GpioInt ACPI event handlers from a late_initcall") deferred the entire acpi_gpiochip_request_interrupt call for each event resource.
This means it also delays the gpiochip_request_own_desc(..., "ACPI:Event") call. This is a problem if some AML code reads the GPIO pin before we run the deferred acpi_gpiochip_request_interrupt, because in that case acpi_gpio_adr_space_handler() will already have called gpiochip_request_own_desc(..., "ACPI:OpRegion") causing the call from acpi_gpiochip_request_interrupt to fail with -EBUSY and we will fail to register an event handler.
acpi_gpio_adr_space_handler is prepared for acpi_gpiochip_request_interrupt already having claimed the pin, but the other way around does not work.
One example of a problem this causes, is the event handler for the OTG ID pin on a Prowise PT301 tablet not registering, keeping the port stuck in whatever mode it was in during boot and e.g. only allowing charging after a reboot.
This commit fixes this by only deferring the request_irq call and the initial run of edge-triggered IRQs instead of deferring all of acpi_gpiochip_request_interrupt.
Thanks for a patch with such nice description. I have couple of nitpicks and one question below.
- For gpiochips which call acpi_gpiochip_request_interrupts() before late_init
- (so builtin drivers) we register the ACPI GpioInt event handlers from a
- (so builtin drivers) we register the ACPI GpioInt irq-handlers from a
I would rather spell "IRQ handlers"
Ok.
- late_initcall_sync handler, so that other builtin drivers can register their
- OpRegions before the event handlers can run. This list contains gpiochips
- for which the acpi_gpiochip_request_interrupts() has been deferred.
- for which the acpi_gpiochip_request_irqs() call has been deferred.
mutex_unlock(&acpi_gpio_deferred_req_irqs_lock);
- if (defer)
return;
- acpi_walk_resources(handle, "_AEI",
acpi_gpiochip_request_interrupt, acpi_gpio);
- if (!defer)
acpi_gpiochip_request_irqs(acpi_gpio);
Can we leave above condition and put here just
Ok.
acpi_gpiochip_request_irqs(acpi_gpio);
?
list_for_each_entry_safe_reverse(event, ep, &acpi_gpio->events, node) { struct gpio_desc *desc;
if (event->irq_requested) {
if (event->irq_is_wake)
disable_irq_wake(event->irq);
free_irq(event->irq, event);
}
desc = event->desc; if (WARN_ON(IS_ERR(desc)))
I don't remember if I asked already about possible memory leak here. Am I reading code right?
continue;
Reading more code, I'm even not sure if this condition could ever happen.
You're right an event with an IS_ERR(desc) is never added to the list of events, so this condition can never happen. I will add a follow up patch to v2 removing this unnecessary check.
I will prepare a v2 of this patch this evening or tomorrow.
Regards,
Hans
linux-stable-mirror@lists.linaro.org