Re: Reworking suspend-resume sequence (was: Re: PCI PM: Restore standard config registers of all devices early)

[Jesse Barnes <jesse.barnes@intel.com>]

On Tuesday, February 3, 2009 9:59 am Linus Torvalds wrote:
> On Tue, 3 Feb 2009, Rafael J. Wysocki wrote:
> > Having reconsidered it, I think that the "loop of disable_irq()" may be
> > problematic due to MSI/MSI-X and devices that are put into D3 during the
> > "normal" suspend.  That is, we shouldn't try to mask MSI/MSI-X for
> > devices in D3
>
> Rafael, you seem to be confused about what "disable_irq()" does.
>
> It does not touch the driver hardware AT ALL. It literally just touches
> the interrupt controller, and even that only indirectly.
>
> What disable_irq() does it literally:
>
> 	void disable_irq_nosync(unsigned int irq)
> 	{
> 	        struct irq_desc *desc = irq_to_desc(irq);
> 	        unsigned long flags;
>
> 	        if (!desc)
> 	                return;
>
> 	        spin_lock_irqsave(&desc->lock, flags);
> 	        if (!desc->depth++) {
> 	                desc->status |= IRQ_DISABLED;
> 	                desc->chip->disable(irq);
> 	        }
> 	        spin_unlock_irqrestore(&desc->lock, flags);
> 	}
>
> and then it does a "synchronize_irq()" to wait to make sure that there are
> no pending ones.
>
> And in many cases, even the
>
> 	desc->chip->disable(irq);
>
> doesn't actually _do_ anything - we'll quite possibly continue to take the
> interrupt, and only when the interrupt happens will it see the "oh,
> IRQ_DISABLED is set" thing, and do something about it.

But won't ->disable point at ->mask in the MSI case (msi_chip doesn't have 
a ->disable, so the IRQ core will make ->disable point at ->mask)?

And in mask we do go poke at PCI regs (msi_set_mask_bits) to mask interrupts 
if possible (though if there's no mask bit in the legacy MSI case we don't do 
anything).

But again, these interrupts won't be shared, so maybe it doesn't matter, and 
maybe the IRQ_DISABLED flag will keep any drivers from seeing post suspend 
interrupts anyway...

-- 
Jesse Barnes, Intel Open Source Technology Center