Re: [PATCH v2] gpio: acpi: Call enable_irq_wake for _IAE GpioInts with Wake set

["Rafael J. Wysocki" <rjw@xxxxxxxxxxxxx>]

On Monday, March 20, 2017 06:32:21 PM Hans de Goede wrote:
> On Bay Trail / Cherry Trail systems with a LID switch, the LID switch is
> often connect to a gpioint handled by an _IAE event handler.
> Before this commit such systems would not wake up when opening the lid,
> requiring the powerbutton to be pressed after opening the lid to wakeup.
> 
> Note that Bay Trail / Cherry Trail systems use suspend-to-idle, so
> the interrupts are generated anyway on those lines on lid switch changes,
> but they are treated by the IRQ subsystem as spurious while suspended if
> not marked as wakeup IRQs.
> 
> This commit calls enable_irq_wake() for _IAE GpioInts with a valid
> event handler which have their Wake flag set. This fixes such systems
> not waking up when opening the lid.
> 
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> Acked-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
> ---
> Changes in v2:
> -Improve commit msg
> -Add Mika's Acked-by
> ---
>  drivers/gpio/gpiolib-acpi.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
> index 8cd3f66..18207b2 100644
> --- a/drivers/gpio/gpiolib-acpi.c
> +++ b/drivers/gpio/gpiolib-acpi.c
> @@ -28,6 +28,7 @@ struct acpi_gpio_event {
>  	acpi_handle handle;
>  	unsigned int pin;
>  	unsigned int irq;
> +	bool irq_wake_enabled;
>  	struct gpio_desc *desc;
>  };
>  
> @@ -266,6 +267,11 @@ static acpi_status acpi_gpiochip_request_interrupt(struct acpi_resource *ares,
>  		goto fail_free_event;
>  	}
>  
> +	if (agpio->wake_capable == ACPI_WAKE_CAPABLE) {
> +		enable_irq_wake(irq);
> +		event->irq_wake_enabled = true;
> +	}
> +
>  	list_add_tail(&event->node, &acpi_gpio->events);
>  	return AE_OK;
>  
> @@ -339,6 +345,9 @@ 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 (event->irq_wake_enabled)

It has just occurred to me that if the event is in the list, the IRQ will be
enabled to wake up as long as agpio->wake_capable == ACPI_WAKE_CAPABLE, so it
looks like it should be sufficient to check

	if (agpio->wake_capable == ACPI_WAKE_CAPABLE)

here and the new flag is not necessary.  Or is it?

> +			disable_irq_wake(event->irq);
> +
>  		free_irq(event->irq, event);
>  		desc = event->desc;
>  		if (WARN_ON(IS_ERR(desc)))
> 

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html