On Sunday, August 21, 2011, Alan Stern wrote:
> On Sat, 20 Aug 2011, Rafael J. Wysocki wrote:
>
> > From: Rafael J. Wysocki <rjw@xxxxxxx>
> >
> > Add a helper function allowing drivers and subsystems to clear
> > the power.irq_safe device flag.
>
> > --- linux.orig/drivers/base/power/runtime.c
> > +++ linux/drivers/base/power/runtime.c
> > @@ -1109,22 +1109,23 @@ void pm_runtime_no_callbacks(struct devi
> > EXPORT_SYMBOL_GPL(pm_runtime_no_callbacks);
> >
> > /**
> > - * pm_runtime_irq_safe - Leave interrupts disabled during callbacks.
> > + * __pm_runtime_irq_safe - Manipulate a device's power.irq_safe flag.
> > * @dev: Device to handle
> > + * @irq_safe: Whether or not to leave interrupts disabled during callbacks.
> > *
> > - * Set the power.irq_safe flag, which tells the PM core that the
> > + * Set or unset the power.irq_safe flag, which tells the PM core that the
> > * ->runtime_suspend() and ->runtime_resume() callbacks for this device should
> > * always be invoked with the spinlock held and interrupts disabled. It also
> > * causes the parent's usage counter to be permanently incremented, preventing
> > * the parent from runtime suspending -- otherwise an irq-safe child might have
> > * to wait for a non-irq-safe parent.
> > */
> > -void pm_runtime_irq_safe(struct device *dev)
> > +void __pm_runtime_irq_safe(struct device *dev, bool irq_safe)
> > {
> > if (dev->parent)
> > pm_runtime_get_sync(dev->parent);
> > spin_lock_irq(&dev->power.lock);
> > - dev->power.irq_safe = 1;
> > + dev->power.irq_safe = irq_safe;
> > spin_unlock_irq(&dev->power.lock);
>
> It's not quite this easy. There are two important aspects that must be
> considered.
>
> Firstly, I originally envisioned pm_runtime_irq_safe() being called
> just once, before the device is enabled for runtime PM. If you allow
> the flag to be turned on and off like this, you raise the possibility
> of races with runtime PM callbacks. (That is, if a callback occurs at
> about the same time as the irq_safe flag is changed, nobody can predict
> whether the callback will be invoked with interrupts enabled.) Maybe
> that's something the driver needs to take care of, but it should at
> least be mentioned in the documentation.
Good point. Perhaps I should make it possible only if runtime PM is
disabled.
> Secondly, this doesn't manage the parent's usage counter correctly.
Right, I forgot about that.
> Do the pm_runtime_get_sync(dev->parent) at the beginning only when the
> irq_safe flag was off and is being turned on. And at the end, if the
> irq_safe flag was on and is being turned off, do
> pm_runtime_put_sync(dev->parent). See pm_runtime_remove() for why this
> matters. (Also update the documentation; the change to the parent
> isn't necessarily permanent any more.)
I'll try to figure out an alternative approach without the $subject change
first. If I can't, I'll revisit this idea.
Thanks,
Rafael
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/linux-pm
