Messages in this thread | | | From | "Rafael J. Wysocki" <> | Subject | Re: [PATCH] genirq: do not disable IRQ_WAKEUP marked irqs on suspend | Date | Tue, 23 Jun 2009 00:56:05 +0200 |
| |
On Monday 22 June 2009, Andrew Morton wrote: > On Fri, 12 Jun 2009 21:52:46 +0200 (CEST) > Thomas Gleixner <tglx@linutronix.de> wrote: > > > On Fri, 12 Jun 2009, Kevin Hilman wrote: > > > http://lkml.org/lkml/2009/5/4/448 > > > > > > Only difference is I did the checking outside of the lock, which is > > > probably wrong. In any case, you'll be interested in the thread that > > > follows. > > > > Hmm, darn. That means that on hardware which has trouble with the > > delayed disable and therefor uses it's own chip->disable_irq() method > > the suspend logic is wreckaged. > > Does this maen that your original patch is no longer applicable to > mainline/-stable?
I'd say so. There are good arguments for not doing this.
> > But there is always a way to get broken hardware tamed. :) > > > > suspend does: > > __disable_irq(); > > status |= IRQ_SUSPENDED; > > chip->disable_irq(); > > > > resume does: > > __enable_irq(); > > status &= ~IRQ_SUSPENDED; > > chip->enable_irq(); > > > > So > > > > - set_irq_handler(handle_level_irq); > > + set_irq_handler(my_own_handler); > > > > +my_own_handler() > > +{ > > + if (!(status & IRQ_SUSPENDED)) { > > + handle_level_irq(); > > + } else { > > + mask_at_hardware_level(); > > + status |= IRQ_PENDING; > > + save_important_information(); > > + } > > +} > > > > my_disable_irq() > > { > > + if (!(status & IRQ_SUSPENDED)) > > mask_at_hardware_level(); > > } > > > > my_enable_irq() > > { > > + if (important_information_has_been_saved) > > + replay_what_happened(); > > + > > unmask_at_hardware_level(); > > } > > > > Ugly, but that might work somehow. Not sure about the replay part, but > > that can be deferred via some more hackery as well :) > > > > Raphael, these delayed disable and the chip->irq_disable() override > > implications vs. suspend really need to be documented. The current > > comment of suspend_device_irqs() is bogus: > > > > * During system-wide suspend or hibernation device interrupts need to be > > * disabled at the chip level and this function is provided for this purpose. > > ^^^^^^^^^^^^^^^^^^^^^^^^^^
| |