| Message ID | 1432044679-10256-2-git-send-email-tomeu.vizoso@collabora.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On Tue, 19 May 2015, Tomeu Vizoso wrote: > Introduce a new per-device flag power.direct_complete_default that will > instruct the PM core to let that device remain in runtime suspend when > the system goes into a sleep power state, without it having to implement > the prepare() callback. > > This is useful because otherwise it would be needed to get dozens of > drivers to implement the prepare() callback even if they don't have a > 1-to-1 relationship with a piece of HW. > > Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> This seems to be a very reasonable approach, and much like what I envisioned from Rafael's original suggestion. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 19 May 2015 at 16:11, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote: > > Introduce a new per-device flag power.direct_complete_default that will > instruct the PM core to let that device remain in runtime suspend when > the system goes into a sleep power state, without it having to implement > the prepare() callback. The above wording make it sounds like the normal behavior is that the PM core will runtime resume the device during the system PM suspend phase. That's not the case. As I understand it for the mentioned cases above, it will skip invoking the system PM callbacks, if the device is already runtime suspended. > > > This is useful because otherwise it would be needed to get dozens of > drivers to implement the prepare() callback even if they don't have a > 1-to-1 relationship with a piece of HW. > > Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> > > --- > > v3: * Have the flag be a more direct substitute for prepare() as > suggested by Rafael > * Inherit this flag from the parent device > > v2: * Fix wording as suggested by Kevin Hilman > --- > Documentation/power/runtime_pm.txt | 8 +++++++- > drivers/base/power/main.c | 5 +++++ > include/linux/pm.h | 1 + > 3 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/Documentation/power/runtime_pm.txt b/Documentation/power/runtime_pm.txt > index e76dc0a..caf7b53 100644 > --- a/Documentation/power/runtime_pm.txt > +++ b/Documentation/power/runtime_pm.txt > @@ -228,6 +228,12 @@ defined in include/linux/pm.h: > unsigned int ignore_children; > - if set, the value of child_count is ignored (but still updated) > > + bool direct_complete_default; > + - if set, the device will be left runtime-suspended when the system > + transitions to a sleep state, unless there's a .prepare() callback that > + returns a non-positive value. This flag is inherited from the parent > + device when a device is added to the PM core's list of active devices. Similar comment as to the change log. > + > unsigned int disable_depth; > - used for disabling the helper functions (they work normally if this is > equal to zero); the initial value of it is 1 (i.e. runtime PM is > @@ -669,7 +675,7 @@ system suspend and resume callbacks for all of those devices, except for the > complete callback, which is then entirely responsible for handling the device > as appropriate. This only applies to system suspend transitions that are not > related to hibernation (see Documentation/power/devices.txt for more > -information). > +information). See also power.direct_complete_default. > > The PM core does its best to reduce the probability of race conditions between > the runtime PM and system suspend/resume (and hibernation) callbacks by carrying > diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c > index 3d874ec..175ddec 100644 > --- a/drivers/base/power/main.c > +++ b/drivers/base/power/main.c > @@ -128,6 +128,9 @@ void device_pm_add(struct device *dev) > if (dev->parent && dev->parent->power.is_prepared) > dev_warn(dev, "parent %s should not be sleeping\n", > dev_name(dev->parent)); > + if (dev->parent) > + dev->power.direct_complete_default = > + dev->parent->power.direct_complete_default; > list_add_tail(&dev->power.entry, &dpm_list); > mutex_unlock(&dpm_list_mtx); > } > @@ -1589,6 +1592,8 @@ static int device_prepare(struct device *dev, pm_message_t state) > trace_device_pm_callback_start(dev, info, state.event); > ret = callback(dev); > trace_device_pm_callback_end(dev, ret); > + } else if (dev->power.direct_complete_default) { > + ret = 1; > } > > device_unlock(dev); > diff --git a/include/linux/pm.h b/include/linux/pm.h > index 4890743..fcb3451 100644 > --- a/include/linux/pm.h > +++ b/include/linux/pm.h > @@ -565,6 +565,7 @@ struct dev_pm_info { > bool ignore_children:1; > bool early_init:1; /* Owned by the PM core */ > bool direct_complete:1; /* Owned by the PM core */ > + bool direct_complete_default:1; /* Ditto */ > spinlock_t lock; > #ifdef CONFIG_PM_SLEEP > struct list_head entry; > -- Besides the above minor things, this looks good to me. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday, May 19, 2015 04:11:15 PM Tomeu Vizoso wrote: > Introduce a new per-device flag power.direct_complete_default that will > instruct the PM core to let that device remain in runtime suspend when > the system goes into a sleep power state, without it having to implement > the prepare() callback. > > This is useful because otherwise it would be needed to get dozens of > drivers to implement the prepare() callback even if they don't have a > 1-to-1 relationship with a piece of HW. > > Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> The code changes look good, but please change the doc/changelog as already suggested by Ulf.
diff --git a/Documentation/power/runtime_pm.txt b/Documentation/power/runtime_pm.txt index e76dc0a..caf7b53 100644 --- a/Documentation/power/runtime_pm.txt +++ b/Documentation/power/runtime_pm.txt @@ -228,6 +228,12 @@ defined in include/linux/pm.h: unsigned int ignore_children; - if set, the value of child_count is ignored (but still updated) + bool direct_complete_default; + - if set, the device will be left runtime-suspended when the system + transitions to a sleep state, unless there's a .prepare() callback that + returns a non-positive value. This flag is inherited from the parent + device when a device is added to the PM core's list of active devices. + unsigned int disable_depth; - used for disabling the helper functions (they work normally if this is equal to zero); the initial value of it is 1 (i.e. runtime PM is @@ -669,7 +675,7 @@ system suspend and resume callbacks for all of those devices, except for the complete callback, which is then entirely responsible for handling the device as appropriate. This only applies to system suspend transitions that are not related to hibernation (see Documentation/power/devices.txt for more -information). +information). See also power.direct_complete_default. The PM core does its best to reduce the probability of race conditions between the runtime PM and system suspend/resume (and hibernation) callbacks by carrying diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c index 3d874ec..175ddec 100644 --- a/drivers/base/power/main.c +++ b/drivers/base/power/main.c @@ -128,6 +128,9 @@ void device_pm_add(struct device *dev) if (dev->parent && dev->parent->power.is_prepared) dev_warn(dev, "parent %s should not be sleeping\n", dev_name(dev->parent)); + if (dev->parent) + dev->power.direct_complete_default = + dev->parent->power.direct_complete_default; list_add_tail(&dev->power.entry, &dpm_list); mutex_unlock(&dpm_list_mtx); } @@ -1589,6 +1592,8 @@ static int device_prepare(struct device *dev, pm_message_t state) trace_device_pm_callback_start(dev, info, state.event); ret = callback(dev); trace_device_pm_callback_end(dev, ret); + } else if (dev->power.direct_complete_default) { + ret = 1; } device_unlock(dev); diff --git a/include/linux/pm.h b/include/linux/pm.h index 4890743..fcb3451 100644 --- a/include/linux/pm.h +++ b/include/linux/pm.h @@ -565,6 +565,7 @@ struct dev_pm_info { bool ignore_children:1; bool early_init:1; /* Owned by the PM core */ bool direct_complete:1; /* Owned by the PM core */ + bool direct_complete_default:1; /* Ditto */ spinlock_t lock; #ifdef CONFIG_PM_SLEEP struct list_head entry;
Introduce a new per-device flag power.direct_complete_default that will instruct the PM core to let that device remain in runtime suspend when the system goes into a sleep power state, without it having to implement the prepare() callback. This is useful because otherwise it would be needed to get dozens of drivers to implement the prepare() callback even if they don't have a 1-to-1 relationship with a piece of HW. Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> --- v3: * Have the flag be a more direct substitute for prepare() as suggested by Rafael * Inherit this flag from the parent device v2: * Fix wording as suggested by Kevin Hilman --- Documentation/power/runtime_pm.txt | 8 +++++++- drivers/base/power/main.c | 5 +++++ include/linux/pm.h | 1 + 3 files changed, 13 insertions(+), 1 deletion(-)