Re: [PATCH omap-fixes] OMAP2/3: GPIO: remove recursion in IRQ wakeup path

[Kevin Hilman <khilman@deeprootsystems.com>]

Tony Lindgren wrote:
> * Kevin Hilman <khilman@deeprootsystems.com> [090210 16:42]:
>> David Brownell <david-b@pacbell.net> writes:
>>
>>> On Wednesday 28 January 2009, Kevin Hilman wrote:
>>>> Now that the generic IRQ and GPIO frameworks are used for enabling and
>>>> disabling GPIO IRQ wakeup sources, there is no longer a need to call
>>>> [enable|disable]_irq_wake() in the low-level code.  Doing so results
>>>> in recursive calls to [enable|disable]_irq_wake().
>>> Could you clarify what actually made that requirement go away?
>>>
>>> The recursion was introduced -- using the generic IRQ framework! -- as
>>> a simple way to ensure the parent IRQ was properly wake-enabled.  Is
>>> the issue that something changed, so that something else wake-enables
>>> the parent?
>>>
>> My description was not very descriptive... sorry...
>>
>> From what I can understand here, I don't see the point in
>> wake-enabling the parent IRQ since there is no wakeup glue for the
>> bank IRQs, thus these calls are actually failing and causing
>> 'unbalanced IRQ disable' noise in the generic IRQ layer.
>>
>> Here is what is happening.  Consider the gpio-keys driver.  Upon
>> suspend, it enables the IRQ wake for its GPIO.  The OMAP GPIO code
>> then calls enable_wake_irq() for the parent irq (the GPIO bank IRQ).
>> This call is actually failing because the bank IRQ has no 'set_wake'
>> method.  Because of this failure, the generic IRQ code doesn't
>> actually do anything, and sets the 'disable_depth' to zero for the
>> bank IRQ.
>>
>> Then, upon resume, the resume path disables IRQ wakeups for the GPIO.
>> The OMAP GPIO code then tries to call disable_irq_wake() for the bank
>> IRQ and you get noisy 'unbalanced IRQ disable' warnings from the
>> generic IRQ layere because of the attempt to disable the IRQ wake of
>> an IRQ that was never enabled.
>>
>> So the options that I see are:
>>
>> 1) fix the OMAP GPIO code to check the return code of the parent enable, or
>> 2) drop the parent enable/disable
>>
>> I prefer (1) since those calls will always fail AFAICT.
>>
>> Does that make any sense?
>>
>> If you're OK with (1), I will re-issue the patch with a better discription.
> 
> Ignoring this for now, please let me know if you want me to queue this
> for omap-upstream with the updates mentioned above.

OK.  I'll re-send with updated description if above is OK with Dave.

Kevin