Re: [PATCH] genirq: do not disable IRQ_WAKEUP marked irqs on suspend

[Rafael J. Wysocki]

On Monday 22 June 2009, Andrew Morton wrote:
> On Fri, 12 Jun 2009 21:52:46 +0200 (CEST)
> Thomas Gleixner <tglx@xxxxxxxxxxxxx> 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.
> >    ^^^^^^^^^^^^^^^^^^^^^^^^^^
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/