| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <87pqqtrwxt.fsf@ti.com>
Date: Tue, 15 Feb 2011 10:10:06 -0800
From: Kevin Hilman <khilman@...com>
To: "Rafael J. Wysocki" <rjw@...k.pl>
Cc: "Linux-pm mailing list" <linux-pm@...ts.linux-foundation.org>,
Grant Likely <grant.likely@...retlab.ca>,
Greg KH <greg@...ah.com>, LKML <linux-kernel@...r.kernel.org>,
Magnus Damm <magnus.damm@...il.com>,
Alan Stern <stern@...land.harvard.edu>,
Len Brown <lenb@...nel.org>,
Mark Brown <broonie@...nsource.wolfsonmicro.com>
Subject: Re: [RFC][PATCH 2/2] PM: Make system-wide PM and runtime PM handle subsystems consistently
"Rafael J. Wysocki" <rjw@...k.pl> writes:
> From: Rafael J. Wysocki <rjw@...k.pl>
>
> The code handling system-wide power transitions (eg. suspend-to-RAM)
> can in theory execute callbacks provided by the device's bus type,
> device type and class in each phase of the power transition. In
> turn, the runtime PM core code only calls one of those callbacks at
> a time, preferring bus type callbacks to device type or class
> callbacks and device type callbacks to class callbacks.
>
> It seems reasonable to make them both behave in the same way in that
> respect. Moreover, even though a device may belong to two subsystems
> (eg. bus type and device class) simultaneously, in practice power
> management callbacks for system-wide power transitions are always
> provided by only one of them (ie. if the bus type callbacks are
> defined, the device class ones are not and vice versa). Thus it is
> possible to modify the code handling system-wide power transitions
> so that it follows the core runtime PM code (ie. treats the
> subsystem callbacks as mutually exclusive).
>
> On the other hand, the core runtime PM code will choose to execute,
> for example, a runtime suspend callback provided by the device type
> even if the bus type's struct dev_pm_ops object exists, but the
> runtime_suspend pointer in it happens to be NULL. This is confusing,
> because it may lead to the execution of callbacks from different
> subsystems during different operations (eg. the bus type suspend
> callback may be executed during runtime suspend, while the device
> type callback will be executed during runtime resume).
>
> Make all of the power management code treat subsystem callbacks in
> a consistent way, such that:
> (1) If the device's bus type is defined (eg. dev->bus is not NULL)
> and its pm pointer is not NULL, the callbacks from dev->bus->pm
> will be used.
> (2) If dev->bus is NULL or dev->bus->pm is NULL, but the device's
> device type is defined (eg. dev->type is not NULL) and its pm
> pointer is not NULL, the callbacks from dev->type->pm will be
> used.
> (3) If dev->bus is NULL or dev->bus->pm is NULL and dev->type is
> NULL or dev->type->pm is NULL, the callbacks from dev->class->pm
> will be used provided that both dev->class and dev->class->pm
> are not NULL.
>
> Signed-off-by: Rafael J. Wysocki <rjw@...k.pl>
This looks good, consistency between system and runtime PM is a great
help to readability.
Acked-by: Kevin Hilman <khilman@...com>
> ---
> Documentation/power/devices.txt | 28 +++----
> drivers/base/power/main.c | 141 +++++++++++++++++-----------------------
> drivers/base/power/runtime.c | 12 +--
> 3 files changed, 79 insertions(+), 102 deletions(-)
>
> Index: linux-2.6/drivers/base/power/main.c
> ===================================================================
> --- linux-2.6.orig/drivers/base/power/main.c
> +++ linux-2.6/drivers/base/power/main.c
> @@ -433,21 +433,17 @@ static int device_resume_noirq(struct de
> error = pm_noirq_op(dev, dev->bus->pm, state);
> if (error)
> goto End;
> - }
> -
> - if (dev->type && dev->type->pm) {
> + } else if (dev->type && dev->type->pm) {
> pm_dev_dbg(dev, state, "EARLY type ");
> error = pm_noirq_op(dev, dev->type->pm, state);
> if (error)
> goto End;
> - }
> -
> - if (dev->class && dev->class->pm) {
> + } else if (dev->class && dev->class->pm) {
> pm_dev_dbg(dev, state, "EARLY class ");
> error = pm_noirq_op(dev, dev->class->pm, state);
> }
>
> -End:
> + End:
> TRACE_RESUME(error);
> return error;
> }
> @@ -532,21 +528,18 @@ static int device_resume(struct device *
> if (dev->bus->pm) {
> pm_dev_dbg(dev, state, "");
> error = pm_op(dev, dev->bus->pm, state);
> + goto End;
> } else if (dev->bus->resume) {
> pm_dev_dbg(dev, state, "legacy ");
> error = legacy_resume(dev, dev->bus->resume);
> - }
> - if (error)
> goto End;
> + }
> }
>
> - if (dev->type) {
> - if (dev->type->pm) {
> - pm_dev_dbg(dev, state, "type ");
> - error = pm_op(dev, dev->type->pm, state);
> - }
> - if (error)
> - goto End;
> + if (dev->type && dev->type->pm) {
> + pm_dev_dbg(dev, state, "type ");
> + error = pm_op(dev, dev->type->pm, state);
> + goto End;
> }
>
> if (dev->class) {
> @@ -558,6 +551,7 @@ static int device_resume(struct device *
> error = legacy_resume(dev, dev->class->resume);
> }
> }
> +
> End:
> device_unlock(dev);
> complete_all(&dev->power.completion);
> @@ -644,19 +638,18 @@ static void device_complete(struct devic
> dev->pwr_domain->ops.complete(dev);
> }
>
> - if (dev->class && dev->class->pm && dev->class->pm->complete) {
> - pm_dev_dbg(dev, state, "completing class ");
> - dev->class->pm->complete(dev);
> - }
> -
> - if (dev->type && dev->type->pm && dev->type->pm->complete) {
> - pm_dev_dbg(dev, state, "completing type ");
> - dev->type->pm->complete(dev);
> - }
> -
> - if (dev->bus && dev->bus->pm && dev->bus->pm->complete) {
> + if (dev->bus && dev->bus->pm) {
> pm_dev_dbg(dev, state, "completing ");
> - dev->bus->pm->complete(dev);
> + if (dev->bus->pm->complete)
> + dev->bus->pm->complete(dev);
> + } else if (dev->type && dev->type->pm) {
> + pm_dev_dbg(dev, state, "completing type ");
> + if (dev->type->pm->complete)
> + dev->type->pm->complete(dev);
> + } else if (dev->class && dev->class->pm) {
> + pm_dev_dbg(dev, state, "completing class ");
> + if (dev->class->pm->complete)
> + dev->class->pm->complete(dev);
> }
>
> device_unlock(dev);
> @@ -741,27 +734,23 @@ static pm_message_t resume_event(pm_mess
> */
> static int device_suspend_noirq(struct device *dev, pm_message_t state)
> {
> - int error = 0;
> + int error;
>
> - if (dev->class && dev->class->pm) {
> - pm_dev_dbg(dev, state, "LATE class ");
> - error = pm_noirq_op(dev, dev->class->pm, state);
> + if (dev->bus && dev->bus->pm) {
> + pm_dev_dbg(dev, state, "LATE ");
> + error = pm_noirq_op(dev, dev->bus->pm, state);
> if (error)
> - goto End;
> - }
> -
> - if (dev->type && dev->type->pm) {
> + return error;
> + } else if (dev->type && dev->type->pm) {
> pm_dev_dbg(dev, state, "LATE type ");
> error = pm_noirq_op(dev, dev->type->pm, state);
> if (error)
> - goto End;
> - }
> -
> - if (dev->bus && dev->bus->pm) {
> - pm_dev_dbg(dev, state, "LATE ");
> - error = pm_noirq_op(dev, dev->bus->pm, state);
> + return error;
> + } else if (dev->class && dev->class->pm) {
> + pm_dev_dbg(dev, state, "LATE class ");
> + error = pm_noirq_op(dev, dev->class->pm, state);
> if (error)
> - goto End;
> + return error;
> }
>
> if (dev->pwr_domain) {
> @@ -769,8 +758,7 @@ static int device_suspend_noirq(struct d
> pm_noirq_op(dev, &dev->pwr_domain->ops, state);
> }
>
> -End:
> - return error;
> + return 0;
> }
>
> /**
> @@ -857,40 +845,36 @@ static int __device_suspend(struct devic
> goto End;
> }
>
> - if (dev->class) {
> - if (dev->class->pm) {
> - pm_dev_dbg(dev, state, "class ");
> - error = pm_op(dev, dev->class->pm, state);
> - } else if (dev->class->suspend) {
> - pm_dev_dbg(dev, state, "legacy class ");
> - error = legacy_suspend(dev, state, dev->class->suspend);
> - }
> - if (error)
> - goto End;
> - }
> -
> - if (dev->type) {
> - if (dev->type->pm) {
> - pm_dev_dbg(dev, state, "type ");
> - error = pm_op(dev, dev->type->pm, state);
> - }
> - if (error)
> - goto End;
> - }
> -
> if (dev->bus) {
> if (dev->bus->pm) {
> pm_dev_dbg(dev, state, "");
> error = pm_op(dev, dev->bus->pm, state);
> + goto Domain;
> } else if (dev->bus->suspend) {
> pm_dev_dbg(dev, state, "legacy ");
> error = legacy_suspend(dev, state, dev->bus->suspend);
> + goto Domain;
> }
> - if (error)
> - goto End;
> }
>
> - if (dev->pwr_domain) {
> + if (dev->type && dev->type->pm) {
> + pm_dev_dbg(dev, state, "type ");
> + error = pm_op(dev, dev->type->pm, state);
> + goto Domain;
> + }
> +
> + if (dev->class) {
> + if (dev->class->pm) {
> + pm_dev_dbg(dev, state, "class ");
> + error = pm_op(dev, dev->class->pm, state);
> + } else if (dev->class->suspend) {
> + pm_dev_dbg(dev, state, "legacy class ");
> + error = legacy_suspend(dev, state, dev->class->suspend);
> + }
> + }
> +
> + Domain:
> + if (!error && dev->pwr_domain) {
> pm_dev_dbg(dev, state, "power domain ");
> pm_op(dev, &dev->pwr_domain->ops, state);
> }
> @@ -985,25 +969,24 @@ static int device_prepare(struct device
>
> device_lock(dev);
>
> - if (dev->bus && dev->bus->pm && dev->bus->pm->prepare) {
> + if (dev->bus && dev->bus->pm) {
> pm_dev_dbg(dev, state, "preparing ");
> - error = dev->bus->pm->prepare(dev);
> + if (dev->bus->pm->prepare)
> + error = dev->bus->pm->prepare(dev);
> suspend_report_result(dev->bus->pm->prepare, error);
> if (error)
> goto End;
> - }
> -
> - if (dev->type && dev->type->pm && dev->type->pm->prepare) {
> + } else if (dev->type && dev->type->pm) {
> pm_dev_dbg(dev, state, "preparing type ");
> - error = dev->type->pm->prepare(dev);
> + if (dev->type->pm->prepare)
> + error = dev->type->pm->prepare(dev);
> suspend_report_result(dev->type->pm->prepare, error);
> if (error)
> goto End;
> - }
> -
> - if (dev->class && dev->class->pm && dev->class->pm->prepare) {
> + } else if (dev->class && dev->class->pm) {
> pm_dev_dbg(dev, state, "preparing class ");
> - error = dev->class->pm->prepare(dev);
> + if (dev->class->pm->prepare)
> + error = dev->class->pm->prepare(dev);
> suspend_report_result(dev->class->pm->prepare, error);
> if (error)
> goto End;
> 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
> @@ -214,9 +214,9 @@ static int rpm_idle(struct device *dev,
>
> dev->power.idle_notification = true;
>
> - if (dev->bus && dev->bus->pm && dev->bus->pm->runtime_idle)
> + if (dev->bus && dev->bus->pm)
> callback = dev->bus->pm->runtime_idle;
> - else if (dev->type && dev->type->pm && dev->type->pm->runtime_idle)
> + else if (dev->type && dev->type->pm)
> callback = dev->type->pm->runtime_idle;
> else if (dev->class && dev->class->pm)
> callback = dev->class->pm->runtime_idle;
> @@ -382,9 +382,9 @@ static int rpm_suspend(struct device *de
>
> __update_runtime_status(dev, RPM_SUSPENDING);
>
> - if (dev->bus && dev->bus->pm && dev->bus->pm->runtime_suspend)
> + if (dev->bus && dev->bus->pm)
> callback = dev->bus->pm->runtime_suspend;
> - else if (dev->type && dev->type->pm && dev->type->pm->runtime_suspend)
> + else if (dev->type && dev->type->pm)
> callback = dev->type->pm->runtime_suspend;
> else if (dev->class && dev->class->pm)
> callback = dev->class->pm->runtime_suspend;
> @@ -584,9 +584,9 @@ static int rpm_resume(struct device *dev
> if (dev->pwr_domain)
> rpm_callback(dev->pwr_domain->ops.runtime_resume, dev);
>
> - if (dev->bus && dev->bus->pm && dev->bus->pm->runtime_resume)
> + if (dev->bus && dev->bus->pm)
> callback = dev->bus->pm->runtime_resume;
> - else if (dev->type && dev->type->pm && dev->type->pm->runtime_resume)
> + else if (dev->type && dev->type->pm)
> callback = dev->type->pm->runtime_resume;
> else if (dev->class && dev->class->pm)
> callback = dev->class->pm->runtime_resume;
> Index: linux-2.6/Documentation/power/devices.txt
> ===================================================================
> --- linux-2.6.orig/Documentation/power/devices.txt
> +++ linux-2.6/Documentation/power/devices.txt
> @@ -249,23 +249,17 @@ various phases always run after tasks ha
> unfrozen. Furthermore, the *_noirq phases run at a time when IRQ handlers have
> been disabled (except for those marked with the IRQ_WAKEUP flag).
>
> -Most phases use bus, type, and class callbacks (that is, methods defined in
> -dev->bus->pm, dev->type->pm, and dev->class->pm). The prepare and complete
> -phases are exceptions; they use only bus callbacks. When multiple callbacks
> -are used in a phase, they are invoked in the order: <class, type, bus> during
> -power-down transitions and in the opposite order during power-up transitions.
> -For example, during the suspend phase the PM core invokes
> -
> - dev->class->pm.suspend(dev);
> - dev->type->pm.suspend(dev);
> - dev->bus->pm.suspend(dev);
> -
> -before moving on to the next device, whereas during the resume phase the core
> -invokes
> -
> - dev->bus->pm.resume(dev);
> - dev->type->pm.resume(dev);
> - dev->class->pm.resume(dev);
> +All phases use bus, type, or class callbacks (that is, methods defined in
> +dev->bus->pm, dev->type->pm, or dev->class->pm). These callbacks are mutually
> +exclusive, so if the bus provides a struct dev_pm_ops object pointed to by its
> +pm field (i.e. both dev->bus and dev->bus->pm are defined), the callbacks
> +included in that object (i.e. dev->bus->pm) will be used. In turn, if the
> +device type provides a struct dev_pm_ops object pointed to by its pm field
> +(i.e. both dev->type and dev->type->pm are defined), the PM core will used the
> +callbacks from that object (i.e. dev->type->pm). Finally, if the pm fields of
> +both the bus and device type objects are NULL (or those objects do not exist),
> +the callbacks provided by the class (that is, the callbacks from dev->class->pm)
> +will be used.
>
> These callbacks may in turn invoke device- or driver-specific methods stored in
> dev->driver->pm, but they don't have to.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists