Re: [RFC PATCH] gpiolib-acpi: Add ACPI5 event model support to gpio.

[Mathias Nyman <mathias.nyman@linux.intel.com>]

On 01/25/2013 02:54 PM, Linus Walleij wrote:
> On Fri, Jan 25, 2013 at 12:48 PM, Mathias Nyman
> <mathias.nyman@linux.intel.com>  wrote:
>
>> Add ability to handle ACPI events signalled by GPIO interrupts.
>>
>> ACPI5 platforms can use GPIO signaled ACPI events. These GPIO interrupts are
>> handled by ACPI event methods which need to be called from the GPIO
>> controller's interrupt handler. acpi_gpio_request_interrupt() finds out which
>> gpio pins have acpi event methods and assigns interrupt handlers that calls
>> the acpi event methods for those pins.
>>
>> Partially based on work by Rui Zhang<rui.zhang@intel.com>
>>
>> Signed-off-by: Mathias Nyman<mathias.nyman@linux.intel.com>
> (...)
>> +/**
>> + * acpi_gpio_request_interrupt() - Register isr for gpio controller ACPI events
>> + * @chip:      gpio chip representation of the gpio controller
>
>
> Hm chip, controller, controller, chip chip, controller controller...
> Are we using two different names for the same thing?

They get mixed up a bit here yes.
I'll change them all to chip.

>
>> + *
>> + * ACPI5 platforms can use GPIO signaled ACPI events. These GPIO interrupts are
>> + * handled by ACPI event methods which need to be called from the GPIO
>> + * controller's interrupt handler. acpi_gpio_request_interrupt finds out which
>> + * gpio pins have acpi event methods and assigns interrupt handlers that calls
>> + * the acpi event methods for those pins.
>> + */
>> +
>> +void acpi_gpio_request_interrupt(struct gpio_chip *chip)
>
> So I was like "um, what acpi requests an interrupt for a GPIO (maybe a pin)...
>
> ... read read ...
>
> Aha the function should probably be named:
> acpi_gpiochip_request_interrupts()
>

True, will rename it.

> Because it just grabs all IRQs coming from that chip right?

Not all, only the ones that have an ACPI handler method defined. 
Basically ACPI5 supports using some of the gpio interrupts for the same 
purpose as SCI interrupts.

>
> Second: why is there no mirror function *releasing* all the IRQs again?
> One-way interface?

My thinking was that using devm_request_threaded_irq(chip->dev, ...) 
will automatically free the interrupts on driver detach, making the 
release function unnecessary.

-Mathias