On Saturday, November 20, 2010, Alan Stern wrote:
> On Sat, 20 Nov 2010, Rafael J. Wysocki wrote:
>
> > On Friday, November 19, 2010, Alan Stern wrote:
> > > This patch (as1431b) makes the synchronous runtime-PM interface
> > > suitable for use in interrupt handlers. Subsystems can call the new
> > > pm_runtime_irq_safe() function to tell the PM core that a device's
> > > runtime-PM callbacks should be invoked with interrupts disabled
> > > (runtime_suspend and runtime_resume callbacks will be invoked with the
> > > spinlock held as well). This permits the pm_runtime_get_sync() and
> > > pm_runtime_put_sync() routines to be called from within interrupt
> > > handlers.
> > >
> > > When a device is declared irq-safe in this way, the PM core increments
> > > the parent's usage count, so the parent will never be runtime
> > > suspended. This prevents difficult situations in which an irq-safe
> > > device can't resume because it is forced to wait for its non-irq-safe
> > > parent.
> > >
> > > Signed-off-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
>
> > > --- usb-2.6.orig/drivers/base/power/runtime.c
> > > +++ usb-2.6/drivers/base/power/runtime.c
> > > @@ -223,11 +223,19 @@ static int rpm_idle(struct device *dev,
> > > callback = NULL;
> > >
> > > if (callback) {
> > > - spin_unlock_irq(&dev->power.lock);
> > > + if (dev->power.irq_safe) {
> > > + spin_unlock(&dev->power.lock);
> > >
> > > - callback(dev);
> > > + callback(dev);
> > >
> > > - spin_lock_irq(&dev->power.lock);
> > > + spin_lock(&dev->power.lock);
> > > + } else {
> > > + spin_unlock_irq(&dev->power.lock);
> > > +
> > > + callback(dev);
> > > +
> > > + spin_lock_irq(&dev->power.lock);
> > > + }
> > > }
> >
> > I didn't like this change before and I still don't like it. Quite frankly, I'm
> > not sure I can convince Linus to pull it. :-)
> >
> > Why don't we simply execute the callback under the spinlock in the
> > IRQ safe case?
>
> Because it wouldn't work. The job of the runtime_idle callback is to
> call pm_runtime_suspend when needed. But if the callback runs under
> the spinlock then pm_runtime_suspend would hang when it tries to grab
> the lock.
Yes, in the _idle case. I actually should have put my comment under
the change in rpm_callback(), which is what I really meant.
Moreover, I'm not sure if we need an "IRQ safe" version of _idle. Why do
we need it, exactly?
> I don't think Linus will object to this.
Well, I guess we'll see. :-)
Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
