On Friday, October 01, 2010, Rafael J. Wysocki wrote: > On Friday, October 01, 2010, Alan Stern wrote: > > On Fri, 1 Oct 2010, Rafael J. Wysocki wrote: > > > > > On Thursday, September 30, 2010, Alan Stern wrote: > > > > This patch (as1431) adds a synchronous runtime-PM interface suitable > > > > for use in interrupt handlers. Four new helper functions are defined: > > > > > > > > pm_runtime_suspend_irq(), pm_runtime_resume_irq(), > > > > pm_runtime_get_sync_irq(), pm_runtime_put_sync_irq(), > > > > > > > > together with pm_runtime_callbacks_in_irq(), which subsystems use to > > > > tell the PM core that the runtime callbacks should be invoked with > > > > interrupts disabled. > > > > > > BTW, I like some changes made by your patch that aren't really related to > > > the issue at hand, so I think the patch below can be applied regardless of > > > the other changes, unless I made a mistake I can see now. > > > > It looks like a good change, but you forgot to preserve the assignments > > to dev->power.runtime_error. > > That's correct and there's a difference betwee _idle and the other cases > because of that. I'll send updated patch with a changelog shortly. Appended as promised. Thanks, Rafael --- From: Rafael J. Wysocki <rjw@xxxxxxx> Subject: PM / Runtime: Reduce code duplication in core helper functions Reduce code duplication in rpm_idle(), rpm_suspend() and rpm_resume() by using local pointers to store callback addresses and moving some duplicated code into a separate function. Signed-off-by: Rafael J. Wysocki <rjw@xxxxxxx> --- drivers/base/power/runtime.c | 122 +++++++++++++++++++------------------------ 1 file changed, 54 insertions(+), 68 deletions(-) Index: linux-2.6/drivers/base/power/runtime.c =================================================================== --- linux-2.6.orig/drivers/base/power/runtime.c +++ linux-2.6/drivers/base/power/runtime.c @@ -153,7 +153,6 @@ static int rpm_check_suspend_allowed(str return retval; } - /** * rpm_idle - Notify device bus type if the device can be suspended. * @dev: Device to notify the bus type about. @@ -167,8 +166,8 @@ static int rpm_check_suspend_allowed(str * This function must be called under dev->power.lock with interrupts disabled. */ static int rpm_idle(struct device *dev, int rpmflags) - __releases(&dev->power.lock) __acquires(&dev->power.lock) { + int (*callback)(struct device *); int retval; retval = rpm_check_suspend_allowed(dev); @@ -214,23 +213,19 @@ static int rpm_idle(struct device *dev, dev->power.idle_notification = true; - if (dev->bus && dev->bus->pm && dev->bus->pm->runtime_idle) { - spin_unlock_irq(&dev->power.lock); - - dev->bus->pm->runtime_idle(dev); - - spin_lock_irq(&dev->power.lock); - } else if (dev->type && dev->type->pm && dev->type->pm->runtime_idle) { - spin_unlock_irq(&dev->power.lock); - - dev->type->pm->runtime_idle(dev); + if (dev->bus && dev->bus->pm && dev->bus->pm->runtime_idle) + callback = dev->bus->pm->runtime_idle; + else if (dev->type && dev->type->pm && dev->type->pm->runtime_idle) + callback = dev->type->pm->runtime_idle; + else if (dev->class && dev->class->pm) + callback = dev->class->pm->runtime_idle; + else + callback = NULL; - spin_lock_irq(&dev->power.lock); - } else if (dev->class && dev->class->pm - && dev->class->pm->runtime_idle) { + if (callback) { spin_unlock_irq(&dev->power.lock); - dev->class->pm->runtime_idle(dev); + callback(dev); spin_lock_irq(&dev->power.lock); } @@ -243,6 +238,29 @@ static int rpm_idle(struct device *dev, } /** + * rpm_callback - Run a given runtime PM callback for a given device. + * @cb: Runtime PM callback to run. + * @dev: Device to run the callback for. + */ +static int rpm_callback(int (*cb)(struct device *), struct device *dev) + __releases(&dev->power.lock) __acquires(&dev->power.lock) +{ + int retval; + + if (!cb) + return -ENOSYS; + + spin_unlock_irq(&dev->power.lock); + + retval = cb(dev); + + spin_lock_irq(&dev->power.lock); + dev->power.runtime_error = retval; + + return retval; +} + +/** * rpm_suspend - Carry out run-time suspend of given device. * @dev: Device to suspend. * @rpmflags: Flag bits. @@ -261,6 +279,7 @@ static int rpm_idle(struct device *dev, static int rpm_suspend(struct device *dev, int rpmflags) __releases(&dev->power.lock) __acquires(&dev->power.lock) { + int (*callback)(struct device *); struct device *parent = NULL; bool notify = false; int retval; @@ -351,33 +370,16 @@ static int rpm_suspend(struct device *de __update_runtime_status(dev, RPM_SUSPENDING); - if (dev->bus && dev->bus->pm && dev->bus->pm->runtime_suspend) { - spin_unlock_irq(&dev->power.lock); - - retval = dev->bus->pm->runtime_suspend(dev); - - spin_lock_irq(&dev->power.lock); - dev->power.runtime_error = retval; - } else if (dev->type && dev->type->pm - && dev->type->pm->runtime_suspend) { - spin_unlock_irq(&dev->power.lock); - - retval = dev->type->pm->runtime_suspend(dev); - - spin_lock_irq(&dev->power.lock); - dev->power.runtime_error = retval; - } else if (dev->class && dev->class->pm - && dev->class->pm->runtime_suspend) { - spin_unlock_irq(&dev->power.lock); - - retval = dev->class->pm->runtime_suspend(dev); - - spin_lock_irq(&dev->power.lock); - dev->power.runtime_error = retval; - } else { - retval = -ENOSYS; - } + if (dev->bus && dev->bus->pm && dev->bus->pm->runtime_suspend) + callback = dev->bus->pm->runtime_suspend; + else if (dev->type && dev->type->pm && dev->type->pm->runtime_suspend) + callback = dev->type->pm->runtime_suspend; + else if (dev->class && dev->class->pm) + callback = dev->class->pm->runtime_suspend; + else + callback = NULL; + retval = rpm_callback(callback, dev); if (retval) { __update_runtime_status(dev, RPM_ACTIVE); dev->power.deferred_resume = 0; @@ -443,6 +445,7 @@ static int rpm_suspend(struct device *de static int rpm_resume(struct device *dev, int rpmflags) __releases(&dev->power.lock) __acquires(&dev->power.lock) { + int (*callback)(struct device *); struct device *parent = NULL; int retval = 0; @@ -563,33 +566,16 @@ static int rpm_resume(struct device *dev __update_runtime_status(dev, RPM_RESUMING); - if (dev->bus && dev->bus->pm && dev->bus->pm->runtime_resume) { - spin_unlock_irq(&dev->power.lock); - - retval = dev->bus->pm->runtime_resume(dev); - - spin_lock_irq(&dev->power.lock); - dev->power.runtime_error = retval; - } else if (dev->type && dev->type->pm - && dev->type->pm->runtime_resume) { - spin_unlock_irq(&dev->power.lock); - - retval = dev->type->pm->runtime_resume(dev); - - spin_lock_irq(&dev->power.lock); - dev->power.runtime_error = retval; - } else if (dev->class && dev->class->pm - && dev->class->pm->runtime_resume) { - spin_unlock_irq(&dev->power.lock); - - retval = dev->class->pm->runtime_resume(dev); - - spin_lock_irq(&dev->power.lock); - dev->power.runtime_error = retval; - } else { - retval = -ENOSYS; - } + if (dev->bus && dev->bus->pm && dev->bus->pm->runtime_resume) + callback = dev->bus->pm->runtime_resume; + else if (dev->type && dev->type->pm && dev->type->pm->runtime_resume) + callback = dev->type->pm->runtime_resume; + else if (dev->class && dev->class->pm) + callback = dev->class->pm->runtime_resume; + else + callback = NULL; + retval = rpm_callback(callback, dev); if (retval) { __update_runtime_status(dev, RPM_SUSPENDED); pm_runtime_cancel_pending(dev); _______________________________________________ linux-pm mailing list linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/linux-pm