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

[Tony Lindgren <tony@atomide.com>]

* 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.

Regards,

Tony