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

[Kevin Hilman <khilman@deeprootsystems.com>]

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.

Kevin