* Rafael J. Wysocki <rjw@xxxxxxx> wrote:
> > > +void suspend_device_irqs(void)
> > > +{
> > > + struct irq_desc *desc;
> > > + int irq;
> > > +
> > > + for_each_irq_desc(irq, desc) {
> > > + unsigned long flags;
> > > +
> > > + spin_lock_irqsave(&desc->lock, flags);
> > > +
> > > + if (!desc->depth && desc->action
> > > + && !(desc->action->flags & IRQF_TIMER)) {
> > > + desc->depth++;
> > > + desc->status |= IRQ_DISABLED | IRQ_SUSPENDED;
> > > + desc->chip->disable(irq);
> > > + }
> > > +
> > > + spin_unlock_irqrestore(&desc->lock, flags);
> > > + }
> > > +
> > > + for_each_irq_desc(irq, desc) {
> > > + if (desc->status & IRQ_SUSPENDED)
> > > + synchronize_irq(irq);
> > > + }
> >
> > Optimization/code-flow nit: a possibility might be to do a
> > single loop, i.e. i think it's safe to couple the
> > disable+sync bits [as in 99.99% of the cases there will be
> > no in-execution irq handlers when we execute this.]
>
> Well, Linus suggested to do it in a separate loop. I'm fine
> with both ways.
Linus, do you have a strong opinion about which variant we
should use?
The two approaches are not completely equivalent, the variant
suggested by Linus is a bit more 'atomic' - in that it first
turns off everything, then looks for everything that needs to be
synchronized.
OTOH, it _shouldnt_ make much of a difference on a correctly
working system - we ought to be able to disable the irqs one by
one and wait on any pending ones on the spot. Maybe if there was
some implicit dependency between irq sources it would be more
robust to do Linus's version.
Dunno ... no strong feelings either way.
Ingo
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/linux-pm
