* [PATCH v3 0/2] PM: direct_complete_default and pm_runtime_enable_recursive @ 2015-05-19 14:11 Tomeu Vizoso 2015-05-19 14:11 ` [PATCH v3 1/2] PM / sleep: Add power.direct_complete_default flag Tomeu Vizoso 2015-05-19 14:11 ` [PATCH v3 2/2] PM / Runtime: Add pm_runtime_enable_recursive Tomeu Vizoso 0 siblings, 2 replies; 26+ messages in thread From: Tomeu Vizoso @ 2015-05-19 14:11 UTC (permalink / raw) To: linux-pm Cc: Laurent Pinchart, Dmitry Torokhov, Alan Stern, Tomeu Vizoso, Greg Kroah-Hartman, Kevin Hilman, Krzysztof Kozlowski, Len Brown, linux-kernel, Pavel Machek, Rafael J. Wysocki, Russell King, Ulf Hansson Hi, this is v3 of a series that adds API to make it more convenient for drivers to opt into remaining runtime suspended when the system goes to sleep. in this version I have replaced force_direct_complete with direct_complete_default as suggested by Rafael and thus have had to add a pm_runtime_enable_recursive() function call to make sure that all descendants have runtime PM enabled. Inheritance of the direct_complete_default flag has to be done in device_pm_add instead of in pm_runtime_init because sometimes the parent won't have been set yet by then. Thanks, Tomeu Tomeu Vizoso (2): PM / sleep: Add power.direct_complete_default flag PM / Runtime: Add pm_runtime_enable_recursive Documentation/power/runtime_pm.txt | 8 +++++++- drivers/base/power/main.c | 5 +++++ drivers/base/power/runtime.c | 15 +++++++++++++++ include/linux/pm.h | 1 + include/linux/pm_runtime.h | 1 + 5 files changed, 29 insertions(+), 1 deletion(-) -- 2.4.1 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v3 1/2] PM / sleep: Add power.direct_complete_default flag 2015-05-19 14:11 [PATCH v3 0/2] PM: direct_complete_default and pm_runtime_enable_recursive Tomeu Vizoso @ 2015-05-19 14:11 ` Tomeu Vizoso 2015-05-19 17:50 ` Alan Stern ` (2 more replies) 2015-05-19 14:11 ` [PATCH v3 2/2] PM / Runtime: Add pm_runtime_enable_recursive Tomeu Vizoso 1 sibling, 3 replies; 26+ messages in thread From: Tomeu Vizoso @ 2015-05-19 14:11 UTC (permalink / raw) To: linux-pm Cc: Laurent Pinchart, Dmitry Torokhov, Alan Stern, Tomeu Vizoso, Rafael J. Wysocki, Len Brown, Pavel Machek, Greg Kroah-Hartman, linux-kernel 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(-) 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; -- 2.4.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH v3 1/2] PM / sleep: Add power.direct_complete_default flag 2015-05-19 14:11 ` [PATCH v3 1/2] PM / sleep: Add power.direct_complete_default flag Tomeu Vizoso @ 2015-05-19 17:50 ` Alan Stern 2015-05-19 20:47 ` Ulf Hansson 2015-05-19 23:38 ` Rafael J. Wysocki 2 siblings, 0 replies; 26+ messages in thread From: Alan Stern @ 2015-05-19 17:50 UTC (permalink / raw) To: Tomeu Vizoso Cc: linux-pm, Laurent Pinchart, Dmitry Torokhov, Rafael J. Wysocki, Len Brown, Pavel Machek, Greg Kroah-Hartman, linux-kernel 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 1/2] PM / sleep: Add power.direct_complete_default flag 2015-05-19 14:11 ` [PATCH v3 1/2] PM / sleep: Add power.direct_complete_default flag Tomeu Vizoso 2015-05-19 17:50 ` Alan Stern @ 2015-05-19 20:47 ` Ulf Hansson 2015-05-19 23:38 ` Rafael J. Wysocki 2 siblings, 0 replies; 26+ messages in thread From: Ulf Hansson @ 2015-05-19 20:47 UTC (permalink / raw) To: Tomeu Vizoso Cc: linux-pm, Laurent Pinchart, Dmitry Torokhov, Alan Stern, Rafael J. Wysocki, Len Brown, Pavel Machek, Greg Kroah-Hartman, linux-kernel 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 1/2] PM / sleep: Add power.direct_complete_default flag 2015-05-19 14:11 ` [PATCH v3 1/2] PM / sleep: Add power.direct_complete_default flag Tomeu Vizoso 2015-05-19 17:50 ` Alan Stern 2015-05-19 20:47 ` Ulf Hansson @ 2015-05-19 23:38 ` Rafael J. Wysocki 2 siblings, 0 replies; 26+ messages in thread From: Rafael J. Wysocki @ 2015-05-19 23:38 UTC (permalink / raw) To: Tomeu Vizoso Cc: linux-pm, Laurent Pinchart, Dmitry Torokhov, Alan Stern, Len Brown, Pavel Machek, Greg Kroah-Hartman, linux-kernel, Ulf Hansson 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. -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v3 2/2] PM / Runtime: Add pm_runtime_enable_recursive 2015-05-19 14:11 [PATCH v3 0/2] PM: direct_complete_default and pm_runtime_enable_recursive Tomeu Vizoso 2015-05-19 14:11 ` [PATCH v3 1/2] PM / sleep: Add power.direct_complete_default flag Tomeu Vizoso @ 2015-05-19 14:11 ` Tomeu Vizoso 2015-05-19 17:49 ` Alan Stern 1 sibling, 1 reply; 26+ messages in thread From: Tomeu Vizoso @ 2015-05-19 14:11 UTC (permalink / raw) To: linux-pm Cc: Laurent Pinchart, Dmitry Torokhov, Alan Stern, Tomeu Vizoso, Rafael J. Wysocki, Len Brown, Pavel Machek, Greg Kroah-Hartman, Ulf Hansson, Kevin Hilman, Russell King, Krzysztof Kozlowski, linux-kernel This function makes less cumbersome to enable runtime PM in a device and its descendants. Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> --- drivers/base/power/runtime.c | 15 +++++++++++++++ include/linux/pm_runtime.h | 1 + 2 files changed, 16 insertions(+) diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index 5070c4f..2c96f3f 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -1189,6 +1189,21 @@ void pm_runtime_enable(struct device *dev) } EXPORT_SYMBOL_GPL(pm_runtime_enable); +static int __pm_runtime_enable_recursive(struct device *dev, void *data) +{ + pm_runtime_enable(dev); + + device_for_each_child(dev, NULL, __pm_runtime_enable_recursive); + + return 0; +} + +void pm_runtime_enable_recursive(struct device *dev) +{ + __pm_runtime_enable_recursive(dev, NULL); +} +EXPORT_SYMBOL_GPL(pm_runtime_enable_recursive); + /** * pm_runtime_forbid - Block runtime PM of a device. * @dev: Device to handle. diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h index 30e84d4..d393ac3 100644 --- a/include/linux/pm_runtime.h +++ b/include/linux/pm_runtime.h @@ -43,6 +43,7 @@ extern int pm_schedule_suspend(struct device *dev, unsigned int delay); extern int __pm_runtime_set_status(struct device *dev, unsigned int status); extern int pm_runtime_barrier(struct device *dev); extern void pm_runtime_enable(struct device *dev); +extern void pm_runtime_enable_recursive(struct device *dev); extern void __pm_runtime_disable(struct device *dev, bool check_resume); extern void pm_runtime_allow(struct device *dev); extern void pm_runtime_forbid(struct device *dev); -- 2.4.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] PM / Runtime: Add pm_runtime_enable_recursive 2015-05-19 14:11 ` [PATCH v3 2/2] PM / Runtime: Add pm_runtime_enable_recursive Tomeu Vizoso @ 2015-05-19 17:49 ` Alan Stern 2015-05-19 23:39 ` Rafael J. Wysocki 2015-05-20 9:03 ` Tomeu Vizoso 0 siblings, 2 replies; 26+ messages in thread From: Alan Stern @ 2015-05-19 17:49 UTC (permalink / raw) To: Tomeu Vizoso Cc: linux-pm, Laurent Pinchart, Dmitry Torokhov, Rafael J. Wysocki, Len Brown, Pavel Machek, Greg Kroah-Hartman, Ulf Hansson, Kevin Hilman, Russell King, Krzysztof Kozlowski, linux-kernel On Tue, 19 May 2015, Tomeu Vizoso wrote: > This function makes less cumbersome to enable runtime PM in a device and > its descendants. > > Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> I don't see the point of this. In the scenario you have in mind, are the device and all its descendants registered by the same subsystem/driver? If they are, can't the subsystem/driver code enable runtime PM for each of them when they are registered, by adding a single call in the right spot? If they don't all belong to the same subsystem/driver, who is going to call your pm_runtime_enable_recursive routine? No single caller will have the right to enable runtime PM for all these devices. Alan Stern ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] PM / Runtime: Add pm_runtime_enable_recursive 2015-05-19 17:49 ` Alan Stern @ 2015-05-19 23:39 ` Rafael J. Wysocki 2015-05-20 9:03 ` Tomeu Vizoso 1 sibling, 0 replies; 26+ messages in thread From: Rafael J. Wysocki @ 2015-05-19 23:39 UTC (permalink / raw) To: Alan Stern Cc: Tomeu Vizoso, linux-pm, Laurent Pinchart, Dmitry Torokhov, Len Brown, Pavel Machek, Greg Kroah-Hartman, Ulf Hansson, Kevin Hilman, Russell King, Krzysztof Kozlowski, linux-kernel On Tuesday, May 19, 2015 01:49:15 PM Alan Stern wrote: > On Tue, 19 May 2015, Tomeu Vizoso wrote: > > > This function makes less cumbersome to enable runtime PM in a device and > > its descendants. > > > > Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> > > I don't see the point of this. In the scenario you have in mind, are > the device and all its descendants registered by the same > subsystem/driver? If they are, can't the subsystem/driver code enable > runtime PM for each of them when they are registered, by adding a > single call in the right spot? > > If they don't all belong to the same subsystem/driver, who is going to > call your pm_runtime_enable_recursive routine? No single caller will > have the right to enable runtime PM for all these devices. Agreed. -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] PM / Runtime: Add pm_runtime_enable_recursive 2015-05-19 17:49 ` Alan Stern 2015-05-19 23:39 ` Rafael J. Wysocki @ 2015-05-20 9:03 ` Tomeu Vizoso 2015-05-20 14:24 ` Alan Stern 1 sibling, 1 reply; 26+ messages in thread From: Tomeu Vizoso @ 2015-05-20 9:03 UTC (permalink / raw) To: Alan Stern Cc: linux-pm, Laurent Pinchart, Dmitry Torokhov, Rafael J. Wysocki, Len Brown, Pavel Machek, Greg Kroah-Hartman, Ulf Hansson, Kevin Hilman, Russell King, Krzysztof Kozlowski, linux-kernel On 19 May 2015 at 19:49, Alan Stern <stern@rowland.harvard.edu> wrote: > On Tue, 19 May 2015, Tomeu Vizoso wrote: > >> This function makes less cumbersome to enable runtime PM in a device and >> its descendants. >> >> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> > > I don't see the point of this. In the scenario you have in mind, are > the device and all its descendants registered by the same > subsystem/driver? Not quite, the scenario here is a driver (uvcvideo) that deals with a specific piece of hardware and knows that all the descendants of the device it's bound to are virtual. The subtree is: 1-1:1.0 (bound to uvcvideo) ep_87 input4 event4 media0 video0 I liked how the force_direct_complete flag played out here, but I agree with Rafael that it can be abused as the PM domain or the bus type weren't able to prevent going directly to complete. This is my testing branch, btw: https://git.collabora.com/cgit/user/tomeu/linux.git/log/?h=fast-resume-v5 > If they are, can't the subsystem/driver code enable > runtime PM for each of them when they are registered, by adding a > single call in the right spot? > > If they don't all belong to the same subsystem/driver, who is going to > call your pm_runtime_enable_recursive routine? No single caller will > have the right to enable runtime PM for all these devices. Yeah, I was thinking that uvcvideo might be able to decide that, but I'm not really sure about it. Thanks, Tomeu > Alan Stern > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] PM / Runtime: Add pm_runtime_enable_recursive 2015-05-20 9:03 ` Tomeu Vizoso @ 2015-05-20 14:24 ` Alan Stern 2015-07-02 13:59 ` Tomeu Vizoso 0 siblings, 1 reply; 26+ messages in thread From: Alan Stern @ 2015-05-20 14:24 UTC (permalink / raw) To: Tomeu Vizoso Cc: linux-pm, Laurent Pinchart, Dmitry Torokhov, Rafael J. Wysocki, Len Brown, Pavel Machek, Greg Kroah-Hartman, Ulf Hansson, Kevin Hilman, Russell King, Krzysztof Kozlowski, linux-kernel On Wed, 20 May 2015, Tomeu Vizoso wrote: > On 19 May 2015 at 19:49, Alan Stern <stern@rowland.harvard.edu> wrote: > > On Tue, 19 May 2015, Tomeu Vizoso wrote: > > > >> This function makes less cumbersome to enable runtime PM in a device and > >> its descendants. > >> > >> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> > > > > I don't see the point of this. In the scenario you have in mind, are > > the device and all its descendants registered by the same > > subsystem/driver? > > Not quite, the scenario here is a driver (uvcvideo) that deals with a > specific piece of hardware and knows that all the descendants of the > device it's bound to are virtual. > > The subtree is: > > 1-1:1.0 (bound to uvcvideo) > ep_87 > input4 > event4 > media0 > video0 Just because these sub-devices are virtual, it doesn't mean you can ignore the way they interact with runtime PM. In the case of ep_87 this doesn't matter. Endpoint devices (like all devices) are in the SUSPENDED state by default when they are created, and they never leave that state. Does the uvcvideo interface go into runtime suspend at the right times? If it does then you shouldn't have anything more to worry about. > I liked how the force_direct_complete flag played out here, but I > agree with Rafael that it can be abused as the PM domain or the bus > type weren't able to prevent going directly to complete. > > This is my testing branch, btw: > > https://git.collabora.com/cgit/user/tomeu/linux.git/log/?h=fast-resume-v5 > > > If they are, can't the subsystem/driver code enable > > runtime PM for each of them when they are registered, by adding a > > single call in the right spot? > > > > If they don't all belong to the same subsystem/driver, who is going to > > call your pm_runtime_enable_recursive routine? No single caller will > > have the right to enable runtime PM for all these devices. > > Yeah, I was thinking that uvcvideo might be able to decide that, but > I'm not really sure about it. A possible way around the problem is to use pm_suspend_ignore_children on the uvcvideo interface. But I'm not sure that would be the right thing to do. Alan Stern ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] PM / Runtime: Add pm_runtime_enable_recursive 2015-05-20 14:24 ` Alan Stern @ 2015-07-02 13:59 ` Tomeu Vizoso 2015-07-02 15:21 ` Alan Stern 0 siblings, 1 reply; 26+ messages in thread From: Tomeu Vizoso @ 2015-07-02 13:59 UTC (permalink / raw) To: Alan Stern Cc: linux-pm, Laurent Pinchart, Dmitry Torokhov, Rafael J. Wysocki, Len Brown, Pavel Machek, Greg Kroah-Hartman, Ulf Hansson, Kevin Hilman, Russell King, Krzysztof Kozlowski, linux-kernel On 20 May 2015 at 16:24, Alan Stern <stern@rowland.harvard.edu> wrote: > On Wed, 20 May 2015, Tomeu Vizoso wrote: > >> On 19 May 2015 at 19:49, Alan Stern <stern@rowland.harvard.edu> wrote: >> > On Tue, 19 May 2015, Tomeu Vizoso wrote: >> > >> >> This function makes less cumbersome to enable runtime PM in a device and >> >> its descendants. >> >> >> >> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> >> > >> > I don't see the point of this. In the scenario you have in mind, are >> > the device and all its descendants registered by the same >> > subsystem/driver? >> >> Not quite, the scenario here is a driver (uvcvideo) that deals with a >> specific piece of hardware and knows that all the descendants of the >> device it's bound to are virtual. >> >> The subtree is: >> >> 1-1:1.0 (bound to uvcvideo) >> ep_87 >> input4 >> event4 >> media0 >> video0 > > Just because these sub-devices are virtual, it doesn't mean you can > ignore the way they interact with runtime PM. Fair enough, but then, how are we expected to be able to use the direct_complete facility if the core bails out if a descendant doesn't have runtime PM enabled? > In the case of ep_87 this doesn't matter. Endpoint devices (like all > devices) are in the SUSPENDED state by default when they are created, > and they never leave that state. I don't see why it doesn't matter for endpoints or the others. They don't have runtime PM enabled, so no ancestor will be able to do direct_complete. > Does the uvcvideo interface go into runtime suspend at the right times? > If it does then you shouldn't have anything more to worry about. It goes in and out of runtime suspend just fine, but when the system goes to sleep, it's runtime resumed and then suspended. And a full resume takes typically very long for USB cameras. >> I liked how the force_direct_complete flag played out here, but I >> agree with Rafael that it can be abused as the PM domain or the bus >> type weren't able to prevent going directly to complete. >> >> This is my testing branch, btw: >> >> https://git.collabora.com/cgit/user/tomeu/linux.git/log/?h=fast-resume-v5 >> >> > If they are, can't the subsystem/driver code enable >> > runtime PM for each of them when they are registered, by adding a >> > single call in the right spot? >> > >> > If they don't all belong to the same subsystem/driver, who is going to >> > call your pm_runtime_enable_recursive routine? No single caller will >> > have the right to enable runtime PM for all these devices. >> >> Yeah, I was thinking that uvcvideo might be able to decide that, but >> I'm not really sure about it. > > A possible way around the problem is to use pm_suspend_ignore_children > on the uvcvideo interface. But I'm not sure that would be the right > thing to do. Would that mean that if a device has ignore_children then it could still do direct_complete even if its descendants weren't able to? Thanks, Tomeu > Alan Stern > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] PM / Runtime: Add pm_runtime_enable_recursive 2015-07-02 13:59 ` Tomeu Vizoso @ 2015-07-02 15:21 ` Alan Stern 2015-07-03 8:11 ` Tomeu Vizoso 0 siblings, 1 reply; 26+ messages in thread From: Alan Stern @ 2015-07-02 15:21 UTC (permalink / raw) To: Tomeu Vizoso Cc: linux-pm, Laurent Pinchart, Dmitry Torokhov, Rafael J. Wysocki, Len Brown, Pavel Machek, Greg Kroah-Hartman, Ulf Hansson, Kevin Hilman, Russell King, Krzysztof Kozlowski, linux-kernel On Thu, 2 Jul 2015, Tomeu Vizoso wrote: > > Just because these sub-devices are virtual, it doesn't mean you can > > ignore the way they interact with runtime PM. > > Fair enough, but then, how are we expected to be able to use the > direct_complete facility if the core bails out if a descendant doesn't > have runtime PM enabled? > > > In the case of ep_87 this doesn't matter. Endpoint devices (like all > > devices) are in the SUSPENDED state by default when they are created, > > and they never leave that state. > > I don't see why it doesn't matter for endpoints or the others. They > don't have runtime PM enabled, so no ancestor will be able to do > direct_complete. Ah, you're concerned about these lines near the start of __device_suspend(): if (dev->power.direct_complete) { if (pm_runtime_status_suspended(dev)) { pm_runtime_disable(dev); if (pm_runtime_suspended_if_enabled(dev)) goto Complete; pm_runtime_enable(dev); } dev->power.direct_complete = false; } Perhaps the pm_runtime_suspended_if_enabled() test should be changed to pm_runtime_status_suspended(). Then it won't matter whether the descendant devices are enabled for runtime PM. > > A possible way around the problem is to use pm_suspend_ignore_children > > on the uvcvideo interface. But I'm not sure that would be the right > > thing to do. > > Would that mean that if a device has ignore_children then it could > still do direct_complete even if its descendants weren't able to? I think we could justify that. The ignore_children flag means we can communicate with the children even when the device is in runtime suspend, so there's no reason to force the device to leave runtime suspend during a system sleep. Alan Stern ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] PM / Runtime: Add pm_runtime_enable_recursive 2015-07-02 15:21 ` Alan Stern @ 2015-07-03 8:11 ` Tomeu Vizoso 2015-07-03 14:16 ` Alan Stern 0 siblings, 1 reply; 26+ messages in thread From: Tomeu Vizoso @ 2015-07-03 8:11 UTC (permalink / raw) To: Alan Stern Cc: linux-pm, Laurent Pinchart, Dmitry Torokhov, Rafael J. Wysocki, Len Brown, Pavel Machek, Greg Kroah-Hartman, Ulf Hansson, Kevin Hilman, Russell King, Krzysztof Kozlowski, linux-kernel On 2 July 2015 at 17:21, Alan Stern <stern@rowland.harvard.edu> wrote: > On Thu, 2 Jul 2015, Tomeu Vizoso wrote: > >> > Just because these sub-devices are virtual, it doesn't mean you can >> > ignore the way they interact with runtime PM. >> >> Fair enough, but then, how are we expected to be able to use the >> direct_complete facility if the core bails out if a descendant doesn't >> have runtime PM enabled? >> >> > In the case of ep_87 this doesn't matter. Endpoint devices (like all >> > devices) are in the SUSPENDED state by default when they are created, >> > and they never leave that state. >> >> I don't see why it doesn't matter for endpoints or the others. They >> don't have runtime PM enabled, so no ancestor will be able to do >> direct_complete. > > Ah, you're concerned about these lines near the start of > __device_suspend(): > > if (dev->power.direct_complete) { > if (pm_runtime_status_suspended(dev)) { > pm_runtime_disable(dev); > if (pm_runtime_suspended_if_enabled(dev)) > goto Complete; > > pm_runtime_enable(dev); > } > dev->power.direct_complete = false; > } > > Perhaps the pm_runtime_suspended_if_enabled() test should be changed to > pm_runtime_status_suspended(). Then it won't matter whether the > descendant devices are enabled for runtime PM. Yeah, that would remove the need for messing with the runtime PM enable status of descendant devices, but I wonder why Rafael went that way initially. >> > A possible way around the problem is to use pm_suspend_ignore_children >> > on the uvcvideo interface. But I'm not sure that would be the right >> > thing to do. >> >> Would that mean that if a device has ignore_children then it could >> still do direct_complete even if its descendants weren't able to? > > I think we could justify that. The ignore_children flag means we can > communicate with the children even when the device is in runtime > suspend, so there's no reason to force the device to leave runtime > suspend during a system sleep. IIUIC, what you are proposing is to use ignore_children in a way similar to how force_direct_complete was used in this patch? http://thread.gmane.org/gmane.linux.power-management.general/60198/focus=60292 That should work as well, but Rafael raised some objections and thus I went with the present direct_complete_default, which should work if we can relax the check as discussed above. Thanks, Tomeu > Alan Stern > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] PM / Runtime: Add pm_runtime_enable_recursive 2015-07-03 8:11 ` Tomeu Vizoso @ 2015-07-03 14:16 ` Alan Stern 2015-07-03 14:22 ` Tomeu Vizoso 0 siblings, 1 reply; 26+ messages in thread From: Alan Stern @ 2015-07-03 14:16 UTC (permalink / raw) To: Tomeu Vizoso Cc: linux-pm, Laurent Pinchart, Dmitry Torokhov, Rafael J. Wysocki, Len Brown, Pavel Machek, Greg Kroah-Hartman, Ulf Hansson, Kevin Hilman, Russell King, Krzysztof Kozlowski, linux-kernel On Fri, 3 Jul 2015, Tomeu Vizoso wrote: > On 2 July 2015 at 17:21, Alan Stern <stern@rowland.harvard.edu> wrote: > > On Thu, 2 Jul 2015, Tomeu Vizoso wrote: > > > >> > Just because these sub-devices are virtual, it doesn't mean you can > >> > ignore the way they interact with runtime PM. > >> > >> Fair enough, but then, how are we expected to be able to use the > >> direct_complete facility if the core bails out if a descendant doesn't > >> have runtime PM enabled? > >> > >> > In the case of ep_87 this doesn't matter. Endpoint devices (like all > >> > devices) are in the SUSPENDED state by default when they are created, > >> > and they never leave that state. > >> > >> I don't see why it doesn't matter for endpoints or the others. They > >> don't have runtime PM enabled, so no ancestor will be able to do > >> direct_complete. > > > > Ah, you're concerned about these lines near the start of > > __device_suspend(): > > > > if (dev->power.direct_complete) { > > if (pm_runtime_status_suspended(dev)) { > > pm_runtime_disable(dev); > > if (pm_runtime_suspended_if_enabled(dev)) > > goto Complete; > > > > pm_runtime_enable(dev); > > } > > dev->power.direct_complete = false; > > } > > > > Perhaps the pm_runtime_suspended_if_enabled() test should be changed to > > pm_runtime_status_suspended(). Then it won't matter whether the > > descendant devices are enabled for runtime PM. > > Yeah, that would remove the need for messing with the runtime PM > enable status of descendant devices, but I wonder why Rafael went that > way initially. I forget the details. Probably it was just to be safe. We probably thought that if a device was disabled for runtime PM then its runtime PM status might not be accurate. But if direct_complete is set then it may be reasonable to assume that the runtime PM status _is_ accurate. > >> > A possible way around the problem is to use pm_suspend_ignore_children > >> > on the uvcvideo interface. But I'm not sure that would be the right > >> > thing to do. > >> > >> Would that mean that if a device has ignore_children then it could > >> still do direct_complete even if its descendants weren't able to? > > > > I think we could justify that. The ignore_children flag means we can > > communicate with the children even when the device is in runtime > > suspend, so there's no reason to force the device to leave runtime > > suspend during a system sleep. > > IIUIC, what you are proposing is to use ignore_children in a way > similar to how force_direct_complete was used in this patch? > > http://thread.gmane.org/gmane.linux.power-management.general/60198/focus=60292 That message doesn't contain a patch. > That should work as well, but Rafael raised some objections and thus I > went with the present direct_complete_default, which should work if we > can relax the check as discussed above. Rafael and I briefly discussed ignore_children while the original direct_complete patch was being designed. We didn't come to any definite conclusion and decided to forget about it for the time being. Maybe now would be a good time to reconsider it. Alan Stern ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] PM / Runtime: Add pm_runtime_enable_recursive 2015-07-03 14:16 ` Alan Stern @ 2015-07-03 14:22 ` Tomeu Vizoso 2015-07-03 15:11 ` Alan Stern 2015-07-04 0:31 ` Rafael J. Wysocki 0 siblings, 2 replies; 26+ messages in thread From: Tomeu Vizoso @ 2015-07-03 14:22 UTC (permalink / raw) To: Alan Stern Cc: linux-pm, Laurent Pinchart, Dmitry Torokhov, Rafael J. Wysocki, Len Brown, Pavel Machek, Greg Kroah-Hartman, Ulf Hansson, Kevin Hilman, Russell King, Krzysztof Kozlowski, linux-kernel On 3 July 2015 at 16:16, Alan Stern <stern@rowland.harvard.edu> wrote: > On Fri, 3 Jul 2015, Tomeu Vizoso wrote: > >> On 2 July 2015 at 17:21, Alan Stern <stern@rowland.harvard.edu> wrote: >> > On Thu, 2 Jul 2015, Tomeu Vizoso wrote: >> > >> >> > Just because these sub-devices are virtual, it doesn't mean you can >> >> > ignore the way they interact with runtime PM. >> >> >> >> Fair enough, but then, how are we expected to be able to use the >> >> direct_complete facility if the core bails out if a descendant doesn't >> >> have runtime PM enabled? >> >> >> >> > In the case of ep_87 this doesn't matter. Endpoint devices (like all >> >> > devices) are in the SUSPENDED state by default when they are created, >> >> > and they never leave that state. >> >> >> >> I don't see why it doesn't matter for endpoints or the others. They >> >> don't have runtime PM enabled, so no ancestor will be able to do >> >> direct_complete. >> > >> > Ah, you're concerned about these lines near the start of >> > __device_suspend(): >> > >> > if (dev->power.direct_complete) { >> > if (pm_runtime_status_suspended(dev)) { >> > pm_runtime_disable(dev); >> > if (pm_runtime_suspended_if_enabled(dev)) >> > goto Complete; >> > >> > pm_runtime_enable(dev); >> > } >> > dev->power.direct_complete = false; >> > } >> > >> > Perhaps the pm_runtime_suspended_if_enabled() test should be changed to >> > pm_runtime_status_suspended(). Then it won't matter whether the >> > descendant devices are enabled for runtime PM. >> >> Yeah, that would remove the need for messing with the runtime PM >> enable status of descendant devices, but I wonder why Rafael went that >> way initially. > > I forget the details. Probably it was just to be safe. We probably > thought that if a device was disabled for runtime PM then its runtime > PM status might not be accurate. But if direct_complete is set then it > may be reasonable to assume that the runtime PM status _is_ accurate. Cool. >> >> > A possible way around the problem is to use pm_suspend_ignore_children >> >> > on the uvcvideo interface. But I'm not sure that would be the right >> >> > thing to do. >> >> >> >> Would that mean that if a device has ignore_children then it could >> >> still do direct_complete even if its descendants weren't able to? >> > >> > I think we could justify that. The ignore_children flag means we can >> > communicate with the children even when the device is in runtime >> > suspend, so there's no reason to force the device to leave runtime >> > suspend during a system sleep. >> >> IIUIC, what you are proposing is to use ignore_children in a way >> similar to how force_direct_complete was used in this patch? >> >> http://thread.gmane.org/gmane.linux.power-management.general/60198/focus=60292 > > That message doesn't contain a patch. The patch is at the top of the thread: http://thread.gmane.org/gmane.linux.power-management.general/60198/focus=60292 >> That should work as well, but Rafael raised some objections and thus I >> went with the present direct_complete_default, which should work if we >> can relax the check as discussed above. > > Rafael and I briefly discussed ignore_children while the original > direct_complete patch was being designed. We didn't come to any > definite conclusion and decided to forget about it for the time being. > Maybe now would be a good time to reconsider it. I would prefer to have ignore_children ignore whether the children of a device were able to do direct_complete, rather than having a direct_complete_default flag (plus not requiring that all its descendants have runtime PM enabled). Thanks, Tomeu > Alan Stern > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] PM / Runtime: Add pm_runtime_enable_recursive 2015-07-03 14:22 ` Tomeu Vizoso @ 2015-07-03 15:11 ` Alan Stern 2015-07-04 0:32 ` Rafael J. Wysocki 2015-07-04 0:31 ` Rafael J. Wysocki 1 sibling, 1 reply; 26+ messages in thread From: Alan Stern @ 2015-07-03 15:11 UTC (permalink / raw) To: Tomeu Vizoso Cc: linux-pm, Laurent Pinchart, Dmitry Torokhov, Rafael J. Wysocki, Len Brown, Pavel Machek, Greg Kroah-Hartman, Ulf Hansson, Kevin Hilman, Russell King, Krzysztof Kozlowski, linux-kernel On Fri, 3 Jul 2015, Tomeu Vizoso wrote: > >> Yeah, that would remove the need for messing with the runtime PM > >> enable status of descendant devices, but I wonder why Rafael went that > >> way initially. > > > > I forget the details. Probably it was just to be safe. We probably > > thought that if a device was disabled for runtime PM then its runtime > > PM status might not be accurate. But if direct_complete is set then it > > may be reasonable to assume that the runtime PM status _is_ accurate. > > Cool. > > Rafael and I briefly discussed ignore_children while the original > > direct_complete patch was being designed. We didn't come to any > > definite conclusion and decided to forget about it for the time being. > > Maybe now would be a good time to reconsider it. > > I would prefer to have ignore_children ignore whether the children of > a device were able to do direct_complete, rather than having a > direct_complete_default flag (plus not requiring that all its > descendants have runtime PM enabled). Okay, but remember that sometimes these "virtual" devices will exist beneath a device that needs to have ignore_children off. So this won't be a complete solution to your problem. Let's see what Rafael thinks about these two issues. It seems to me that the hardest part is dealing with drivers/subsystems that have no runtime PM support. In such cases, we have to be very careful not to use direct_complete unless we know that the device does no power management at all. Alan Stern ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] PM / Runtime: Add pm_runtime_enable_recursive 2015-07-03 15:11 ` Alan Stern @ 2015-07-04 0:32 ` Rafael J. Wysocki 0 siblings, 0 replies; 26+ messages in thread From: Rafael J. Wysocki @ 2015-07-04 0:32 UTC (permalink / raw) To: Alan Stern Cc: Tomeu Vizoso, linux-pm, Laurent Pinchart, Dmitry Torokhov, Len Brown, Pavel Machek, Greg Kroah-Hartman, Ulf Hansson, Kevin Hilman, Russell King, Krzysztof Kozlowski, linux-kernel On Friday, July 03, 2015 11:11:19 AM Alan Stern wrote: > On Fri, 3 Jul 2015, Tomeu Vizoso wrote: > > > >> Yeah, that would remove the need for messing with the runtime PM > > >> enable status of descendant devices, but I wonder why Rafael went that > > >> way initially. > > > > > > I forget the details. Probably it was just to be safe. We probably > > > thought that if a device was disabled for runtime PM then its runtime > > > PM status might not be accurate. But if direct_complete is set then it > > > may be reasonable to assume that the runtime PM status _is_ accurate. > > > > Cool. > > > > Rafael and I briefly discussed ignore_children while the original > > > direct_complete patch was being designed. We didn't come to any > > > definite conclusion and decided to forget about it for the time being. > > > Maybe now would be a good time to reconsider it. > > > > I would prefer to have ignore_children ignore whether the children of > > a device were able to do direct_complete, rather than having a > > direct_complete_default flag (plus not requiring that all its > > descendants have runtime PM enabled). > > Okay, but remember that sometimes these "virtual" devices will exist > beneath a device that needs to have ignore_children off. So this won't > be a complete solution to your problem. > > Let's see what Rafael thinks about these two issues. It seems to me > that the hardest part is dealing with drivers/subsystems that have no > runtime PM support. In such cases, we have to be very careful not to > use direct_complete unless we know that the device does no power > management at all. Precisely. Thanks, Rafael ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] PM / Runtime: Add pm_runtime_enable_recursive 2015-07-03 14:22 ` Tomeu Vizoso 2015-07-03 15:11 ` Alan Stern @ 2015-07-04 0:31 ` Rafael J. Wysocki 2015-07-04 14:37 ` Alan Stern 1 sibling, 1 reply; 26+ messages in thread From: Rafael J. Wysocki @ 2015-07-04 0:31 UTC (permalink / raw) To: Tomeu Vizoso Cc: Alan Stern, linux-pm, Laurent Pinchart, Dmitry Torokhov, Len Brown, Pavel Machek, Greg Kroah-Hartman, Ulf Hansson, Kevin Hilman, Russell King, Krzysztof Kozlowski, linux-kernel On Friday, July 03, 2015 04:22:02 PM Tomeu Vizoso wrote: > On 3 July 2015 at 16:16, Alan Stern <stern@rowland.harvard.edu> wrote: > > On Fri, 3 Jul 2015, Tomeu Vizoso wrote: > > > >> On 2 July 2015 at 17:21, Alan Stern <stern@rowland.harvard.edu> wrote: > >> > On Thu, 2 Jul 2015, Tomeu Vizoso wrote: > >> > > >> >> > Just because these sub-devices are virtual, it doesn't mean you can > >> >> > ignore the way they interact with runtime PM. > >> >> > >> >> Fair enough, but then, how are we expected to be able to use the > >> >> direct_complete facility if the core bails out if a descendant doesn't > >> >> have runtime PM enabled? > >> >> > >> >> > In the case of ep_87 this doesn't matter. Endpoint devices (like all > >> >> > devices) are in the SUSPENDED state by default when they are created, > >> >> > and they never leave that state. > >> >> > >> >> I don't see why it doesn't matter for endpoints or the others. They > >> >> don't have runtime PM enabled, so no ancestor will be able to do > >> >> direct_complete. > >> > > >> > Ah, you're concerned about these lines near the start of > >> > __device_suspend(): > >> > > >> > if (dev->power.direct_complete) { > >> > if (pm_runtime_status_suspended(dev)) { > >> > pm_runtime_disable(dev); > >> > if (pm_runtime_suspended_if_enabled(dev)) > >> > goto Complete; > >> > > >> > pm_runtime_enable(dev); > >> > } > >> > dev->power.direct_complete = false; > >> > } > >> > > >> > Perhaps the pm_runtime_suspended_if_enabled() test should be changed to > >> > pm_runtime_status_suspended(). Then it won't matter whether the > >> > descendant devices are enabled for runtime PM. > >> > >> Yeah, that would remove the need for messing with the runtime PM > >> enable status of descendant devices, but I wonder why Rafael went that > >> way initially. > > > > I forget the details. Probably it was just to be safe. We probably > > thought that if a device was disabled for runtime PM then its runtime > > PM status might not be accurate. But if direct_complete is set then it > > may be reasonable to assume that the runtime PM status _is_ accurate. > > Cool. We're walking a grey area here. What exactly does power.direct_complete mean for devices whose runtime PM is disabled? > >> >> > A possible way around the problem is to use pm_suspend_ignore_children > >> >> > on the uvcvideo interface. But I'm not sure that would be the right > >> >> > thing to do. > >> >> > >> >> Would that mean that if a device has ignore_children then it could > >> >> still do direct_complete even if its descendants weren't able to? > >> > > >> > I think we could justify that. The ignore_children flag means we can > >> > communicate with the children even when the device is in runtime > >> > suspend, so there's no reason to force the device to leave runtime > >> > suspend during a system sleep. > >> > >> IIUIC, what you are proposing is to use ignore_children in a way > >> similar to how force_direct_complete was used in this patch? > >> > >> http://thread.gmane.org/gmane.linux.power-management.general/60198/focus=60292 > > > > That message doesn't contain a patch. > > The patch is at the top of the thread: > > http://thread.gmane.org/gmane.linux.power-management.general/60198/focus=60292 > > >> That should work as well, but Rafael raised some objections and thus I > >> went with the present direct_complete_default, which should work if we > >> can relax the check as discussed above. > > > > Rafael and I briefly discussed ignore_children while the original > > direct_complete patch was being designed. We didn't come to any > > definite conclusion and decided to forget about it for the time being. > > Maybe now would be a good time to reconsider it. > > I would prefer to have ignore_children ignore whether the children of > a device were able to do direct_complete, rather than having a > direct_complete_default flag (plus not requiring that all its > descendants have runtime PM enabled). Why? Rafael ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] PM / Runtime: Add pm_runtime_enable_recursive 2015-07-04 0:31 ` Rafael J. Wysocki @ 2015-07-04 14:37 ` Alan Stern 2015-07-05 23:36 ` Rafael J. Wysocki 0 siblings, 1 reply; 26+ messages in thread From: Alan Stern @ 2015-07-04 14:37 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Tomeu Vizoso, linux-pm, Laurent Pinchart, Dmitry Torokhov, Len Brown, Pavel Machek, Greg Kroah-Hartman, Ulf Hansson, Kevin Hilman, Russell King, Krzysztof Kozlowski, linux-kernel On Sat, 4 Jul 2015, Rafael J. Wysocki wrote: > > >> > Perhaps the pm_runtime_suspended_if_enabled() test should be changed to > > >> > pm_runtime_status_suspended(). Then it won't matter whether the > > >> > descendant devices are enabled for runtime PM. > > >> > > >> Yeah, that would remove the need for messing with the runtime PM > > >> enable status of descendant devices, but I wonder why Rafael went that > > >> way initially. > > > > > > I forget the details. Probably it was just to be safe. We probably > > > thought that if a device was disabled for runtime PM then its runtime > > > PM status might not be accurate. But if direct_complete is set then it > > > may be reasonable to assume that the runtime PM status _is_ accurate. > > > > Cool. > > We're walking a grey area here. What exactly does power.direct_complete mean > for devices whose runtime PM is disabled? > > Let's see what Rafael thinks about these two issues. It seems to me > > that the hardest part is dealing with drivers/subsystems that have no > > runtime PM support. In such cases, we have to be very careful not to > > use direct_complete unless we know that the device does no power > > management at all. > > Precisely. All right, we can make a decision and document it. The following seems reasonable to me: If dev->power.direct_complete is set then the PM core will assume that dev->power.rpm_status is accurate even when dev->power.disable_depth > 0. The core will obey the .direct_complete setting regardless of .disable_depth. As a consequence, devices that support system sleep but don't support runtime PM must _never_ have .direct_complete set. On the other hand, if a device (such as a "virtual" device) requires no callbacks for either system sleep or runtime PM, then there is no harm in setting .direct_complete. Indeed, doing so may help speed up an ancestor device's sleep transition. How does that sound? Alan Stern ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] PM / Runtime: Add pm_runtime_enable_recursive 2015-07-04 14:37 ` Alan Stern @ 2015-07-05 23:36 ` Rafael J. Wysocki 2015-07-07 0:07 ` Rafael J. Wysocki 0 siblings, 1 reply; 26+ messages in thread From: Rafael J. Wysocki @ 2015-07-05 23:36 UTC (permalink / raw) To: Alan Stern Cc: Tomeu Vizoso, linux-pm, Laurent Pinchart, Dmitry Torokhov, Len Brown, Pavel Machek, Greg Kroah-Hartman, Ulf Hansson, Kevin Hilman, Russell King, Krzysztof Kozlowski, linux-kernel On Saturday, July 04, 2015 10:37:55 AM Alan Stern wrote: > On Sat, 4 Jul 2015, Rafael J. Wysocki wrote: > > > > >> > Perhaps the pm_runtime_suspended_if_enabled() test should be changed to > > > >> > pm_runtime_status_suspended(). Then it won't matter whether the > > > >> > descendant devices are enabled for runtime PM. > > > >> > > > >> Yeah, that would remove the need for messing with the runtime PM > > > >> enable status of descendant devices, but I wonder why Rafael went that > > > >> way initially. > > > > > > > > I forget the details. Probably it was just to be safe. We probably > > > > thought that if a device was disabled for runtime PM then its runtime > > > > PM status might not be accurate. But if direct_complete is set then it > > > > may be reasonable to assume that the runtime PM status _is_ accurate. > > > > > > Cool. > > > > We're walking a grey area here. What exactly does power.direct_complete mean > > for devices whose runtime PM is disabled? > > > > Let's see what Rafael thinks about these two issues. It seems to me > > > that the hardest part is dealing with drivers/subsystems that have no > > > runtime PM support. In such cases, we have to be very careful not to > > > use direct_complete unless we know that the device does no power > > > management at all. > > > > Precisely. > > All right, we can make a decision and document it. The following seems > reasonable to me: > > If dev->power.direct_complete is set then the PM core will > assume that dev->power.rpm_status is accurate even when > dev->power.disable_depth > 0. The core will obey the > .direct_complete setting regardless of .disable_depth. > > As a consequence, devices that support system sleep but don't > support runtime PM must _never_ have .direct_complete set. > > On the other hand, if a device (such as a "virtual" device) > requires no callbacks for either system sleep or runtime PM, > then there is no harm in setting .direct_complete. Indeed, > doing so may help speed up an ancestor device's sleep > transition. > > How does that sound? It would be workable I think, but I'd prefer the core to be told directly about devices whose runtime PM status doesn't matter (because nothing changes between "suspended" and "active"), so they may be treated in a special way safely. If we had that information, no special rules other than "that is a device whose runtime PM status doesn't matter, so treat it accordingly" would be necessary. Thanks, Rafael ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] PM / Runtime: Add pm_runtime_enable_recursive 2015-07-05 23:36 ` Rafael J. Wysocki @ 2015-07-07 0:07 ` Rafael J. Wysocki 2015-07-07 14:55 ` Alan Stern 0 siblings, 1 reply; 26+ messages in thread From: Rafael J. Wysocki @ 2015-07-07 0:07 UTC (permalink / raw) To: Alan Stern Cc: Tomeu Vizoso, linux-pm, Laurent Pinchart, Dmitry Torokhov, Len Brown, Pavel Machek, Greg Kroah-Hartman, Ulf Hansson, Kevin Hilman, Russell King, Krzysztof Kozlowski, linux-kernel On Monday, July 06, 2015 01:36:46 AM Rafael J. Wysocki wrote: > On Saturday, July 04, 2015 10:37:55 AM Alan Stern wrote: > > On Sat, 4 Jul 2015, Rafael J. Wysocki wrote: > > > > > > >> > Perhaps the pm_runtime_suspended_if_enabled() test should be changed to > > > > >> > pm_runtime_status_suspended(). Then it won't matter whether the > > > > >> > descendant devices are enabled for runtime PM. > > > > >> > > > > >> Yeah, that would remove the need for messing with the runtime PM > > > > >> enable status of descendant devices, but I wonder why Rafael went that > > > > >> way initially. > > > > > > > > > > I forget the details. Probably it was just to be safe. We probably > > > > > thought that if a device was disabled for runtime PM then its runtime > > > > > PM status might not be accurate. But if direct_complete is set then it > > > > > may be reasonable to assume that the runtime PM status _is_ accurate. > > > > > > > > Cool. > > > > > > We're walking a grey area here. What exactly does power.direct_complete mean > > > for devices whose runtime PM is disabled? > > > > > > Let's see what Rafael thinks about these two issues. It seems to me > > > > that the hardest part is dealing with drivers/subsystems that have no > > > > runtime PM support. In such cases, we have to be very careful not to > > > > use direct_complete unless we know that the device does no power > > > > management at all. > > > > > > Precisely. > > > > All right, we can make a decision and document it. The following seems > > reasonable to me: > > > > If dev->power.direct_complete is set then the PM core will > > assume that dev->power.rpm_status is accurate even when > > dev->power.disable_depth > 0. The core will obey the > > .direct_complete setting regardless of .disable_depth. > > > > As a consequence, devices that support system sleep but don't > > support runtime PM must _never_ have .direct_complete set. > > > > On the other hand, if a device (such as a "virtual" device) > > requires no callbacks for either system sleep or runtime PM, > > then there is no harm in setting .direct_complete. Indeed, > > doing so may help speed up an ancestor device's sleep > > transition. > > > > How does that sound? > > It would be workable I think, but I'd prefer the core to be told directly > about devices whose runtime PM status doesn't matter (because nothing changes > between "suspended" and "active"), so they may be treated in a special way > safely. > > If we had that information, no special rules other than "that is a device > whose runtime PM status doesn't matter, so treat it accordingly" would be > necessary. That said, a situation to consider is when device X is just a software device, but it has children that correspond to physical hardware. If that is the case, the usual parent-children rules should apply to X and its children (ie. X should only be marked as "suspended" if all of its children are suspended) and I see no reason why the parent-children rules for direct_resume should not apply here. Thanks, Rafael ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] PM / Runtime: Add pm_runtime_enable_recursive 2015-07-07 0:07 ` Rafael J. Wysocki @ 2015-07-07 14:55 ` Alan Stern 2015-07-07 22:06 ` Rafael J. Wysocki 0 siblings, 1 reply; 26+ messages in thread From: Alan Stern @ 2015-07-07 14:55 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Tomeu Vizoso, linux-pm, Laurent Pinchart, Dmitry Torokhov, Len Brown, Pavel Machek, Greg Kroah-Hartman, Ulf Hansson, Kevin Hilman, Russell King, Krzysztof Kozlowski, linux-kernel On Tue, 7 Jul 2015, Rafael J. Wysocki wrote: > > > All right, we can make a decision and document it. The following seems > > > reasonable to me: > > > > > > If dev->power.direct_complete is set then the PM core will > > > assume that dev->power.rpm_status is accurate even when > > > dev->power.disable_depth > 0. The core will obey the > > > .direct_complete setting regardless of .disable_depth. > > > > > > As a consequence, devices that support system sleep but don't > > > support runtime PM must _never_ have .direct_complete set. > > > > > > On the other hand, if a device (such as a "virtual" device) > > > requires no callbacks for either system sleep or runtime PM, > > > then there is no harm in setting .direct_complete. Indeed, > > > doing so may help speed up an ancestor device's sleep > > > transition. > > > > > > How does that sound? > > > > It would be workable I think, but I'd prefer the core to be told directly > > about devices whose runtime PM status doesn't matter (because nothing changes > > between "suspended" and "active"), so they may be treated in a special way > > safely. > > > > If we had that information, no special rules other than "that is a device > > whose runtime PM status doesn't matter, so treat it accordingly" would be > > necessary. > > That said, a situation to consider is when device X is just a software device, > but it has children that correspond to physical hardware. If that is the case, > the usual parent-children rules should apply to X and its children (ie. X should > only be marked as "suspended" if all of its children are suspended) and I see > no reason why the parent-children rules for direct_resume should not apply here. Yes, this illustrates that in some ways we must not treat "virtual" or "software" devices specially. Being "virtual" is not the same as having the ignore_children flag set. The change I'm proposing is not related to whether a device is "virtual". I'm just suggesting that the normal direct_complete rules should apply even when devices are runtime-PM-disabled. This doesn't mean that their runtime PM status doesn't matter. Just the opposite, in fact -- it means that the PM core should pay attention to the runtime PM status during a sleep transition even though disabled_depth > 0. Alan Stern ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] PM / Runtime: Add pm_runtime_enable_recursive 2015-07-07 14:55 ` Alan Stern @ 2015-07-07 22:06 ` Rafael J. Wysocki 2015-07-08 20:31 ` Alan Stern 0 siblings, 1 reply; 26+ messages in thread From: Rafael J. Wysocki @ 2015-07-07 22:06 UTC (permalink / raw) To: Alan Stern Cc: Tomeu Vizoso, linux-pm, Laurent Pinchart, Dmitry Torokhov, Len Brown, Pavel Machek, Greg Kroah-Hartman, Ulf Hansson, Kevin Hilman, Russell King, Krzysztof Kozlowski, linux-kernel On Tuesday, July 07, 2015 10:55:59 AM Alan Stern wrote: > On Tue, 7 Jul 2015, Rafael J. Wysocki wrote: > > > > > All right, we can make a decision and document it. The following seems > > > > reasonable to me: > > > > > > > > If dev->power.direct_complete is set then the PM core will > > > > assume that dev->power.rpm_status is accurate even when > > > > dev->power.disable_depth > 0. The core will obey the > > > > .direct_complete setting regardless of .disable_depth. > > > > > > > > As a consequence, devices that support system sleep but don't > > > > support runtime PM must _never_ have .direct_complete set. > > > > > > > > On the other hand, if a device (such as a "virtual" device) > > > > requires no callbacks for either system sleep or runtime PM, > > > > then there is no harm in setting .direct_complete. Indeed, > > > > doing so may help speed up an ancestor device's sleep > > > > transition. > > > > > > > > How does that sound? > > > > > > It would be workable I think, but I'd prefer the core to be told directly > > > about devices whose runtime PM status doesn't matter (because nothing changes > > > between "suspended" and "active"), so they may be treated in a special way > > > safely. > > > > > > If we had that information, no special rules other than "that is a device > > > whose runtime PM status doesn't matter, so treat it accordingly" would be > > > necessary. > > > > That said, a situation to consider is when device X is just a software device, > > but it has children that correspond to physical hardware. If that is the case, > > the usual parent-children rules should apply to X and its children (ie. X should > > only be marked as "suspended" if all of its children are suspended) and I see > > no reason why the parent-children rules for direct_resume should not apply here. > > Yes, this illustrates that in some ways we must not treat "virtual" or > "software" devices specially. Being "virtual" is not the same as > having the ignore_children flag set. > > The change I'm proposing is not related to whether a device is > "virtual". I'm just suggesting that the normal direct_complete rules > should apply even when devices are runtime-PM-disabled. > > This doesn't mean that their runtime PM status doesn't matter. Just > the opposite, in fact -- it means that the PM core should pay attention > to the runtime PM status during a sleep transition even though > disabled_depth > 0. I seem to have lost the context here, sorry about that. The idea seems to be to rely on the fact that the RPM status for all devices is initially RPM_SUSPENDED and that never changes if runtime PM is never enabled for the device, so in that particular case it would be OK to treat the "power.direct_complete set + RPM status == RPM_SUSPENDED" combination as valid even though runtime PM has never been enabled for the device in question (provided that power.direct_complete will never be set for "real" devices that don't support runtime PM). Is that correct? That seems to be fragile, but I have no strong opinion. Let's do that change if it allows us to make forward progress here. Please feel free to submit a documentation patch along the lines you've suggested. Thanks, Rafael ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] PM / Runtime: Add pm_runtime_enable_recursive 2015-07-07 22:06 ` Rafael J. Wysocki @ 2015-07-08 20:31 ` Alan Stern 2015-07-14 13:19 ` Tomeu Vizoso 0 siblings, 1 reply; 26+ messages in thread From: Alan Stern @ 2015-07-08 20:31 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Tomeu Vizoso, linux-pm, Laurent Pinchart, Dmitry Torokhov, Len Brown, Pavel Machek, Greg Kroah-Hartman, Ulf Hansson, Kevin Hilman, Russell King, Krzysztof Kozlowski, linux-kernel On Wed, 8 Jul 2015, Rafael J. Wysocki wrote: > I seem to have lost the context here, sorry about that. > > The idea seems to be to rely on the fact that the RPM status for all devices > is initially RPM_SUSPENDED and that never changes if runtime PM is never > enabled for the device, so in that particular case it would be OK to treat > the "power.direct_complete set + RPM status == RPM_SUSPENDED" combination > as valid even though runtime PM has never been enabled for the device in > question (provided that power.direct_complete will never be set for "real" > devices that don't support runtime PM). Is that correct? I would have expressed it slightly differently, but yes, that's correct. > That seems to be fragile, but I have no strong opinion. In itself it's not all that bad, I think. In the presence of Tomeu's new direct_complete_default flag, however, it does seem quite fragile. We may want to do the direct_complete_default thing in a different way. For example, the PM core could automatically set the direct_complete flag if a device has _none_ of the system suspend callbacks (i.e., no prepare, suspend, suspend_late, suspend_noirq, resume_noirq, resume_early, resume, or complete). Although it would be a little awkward to check this, it would be safer than inheriting direct_complete_default from the parent and it ought to solve Tomeu's problem just as well. > Let's do that change if it allows us to make forward progress here. Please > feel free to submit a documentation patch along the lines you've suggested. Here's a proposed patch to illustrate what I have in mind. Since it removes the only usage of pm_runtime_suspended_if_enabled(), it also removes the definition of that function. Alan Stern Index: usb-4.1/drivers/base/power/main.c =================================================================== --- usb-4.1.orig/drivers/base/power/main.c +++ usb-4.1/drivers/base/power/main.c @@ -1374,7 +1374,7 @@ static int __device_suspend(struct devic if (dev->power.direct_complete) { if (pm_runtime_status_suspended(dev)) { pm_runtime_disable(dev); - if (pm_runtime_suspended_if_enabled(dev)) + if (pm_runtime_status_suspended(dev)) goto Complete; pm_runtime_enable(dev); Index: usb-4.1/Documentation/power/devices.txt =================================================================== --- usb-4.1.orig/Documentation/power/devices.txt +++ usb-4.1/Documentation/power/devices.txt @@ -341,6 +341,13 @@ the phases are: and is entirely responsible for bringing the device back to the functional state as appropriate. + Note that this direct-complete procedure applies even if the device is + disabled for runtime PM; only the runtime-PM status matters. It follows + that if a device has system-sleep callbacks but does not support runtime + PM, then its prepare callback must never return a positive value. This + is because all devices are initially set to runtime-suspended with + runtime PM disabled. + 2. The suspend methods should quiesce the device to stop it from performing I/O. They also may save the device registers and put it into the appropriate low-power state, depending on the bus type the device is on, Index: usb-4.1/Documentation/power/runtime_pm.txt =================================================================== --- usb-4.1.orig/Documentation/power/runtime_pm.txt +++ usb-4.1/Documentation/power/runtime_pm.txt @@ -445,10 +445,6 @@ drivers/base/power/runtime.c and include bool pm_runtime_status_suspended(struct device *dev); - return true if the device's runtime PM status is 'suspended' - bool pm_runtime_suspended_if_enabled(struct device *dev); - - return true if the device's runtime PM status is 'suspended' and its - 'power.disable_depth' field is equal to 1 - void pm_runtime_allow(struct device *dev); - set the power.runtime_auto flag for the device and decrease its usage counter (used by the /sys/devices/.../power/control interface to Index: usb-4.1/include/linux/pm_runtime.h =================================================================== --- usb-4.1.orig/include/linux/pm_runtime.h +++ usb-4.1/include/linux/pm_runtime.h @@ -98,11 +98,6 @@ static inline bool pm_runtime_status_sus return dev->power.runtime_status == RPM_SUSPENDED; } -static inline bool pm_runtime_suspended_if_enabled(struct device *dev) -{ - return pm_runtime_status_suspended(dev) && dev->power.disable_depth == 1; -} - static inline bool pm_runtime_enabled(struct device *dev) { return !dev->power.disable_depth; @@ -164,7 +159,6 @@ static inline void device_set_run_wake(s static inline bool pm_runtime_suspended(struct device *dev) { return false; } static inline bool pm_runtime_active(struct device *dev) { return true; } static inline bool pm_runtime_status_suspended(struct device *dev) { return false; } -static inline bool pm_runtime_suspended_if_enabled(struct device *dev) { return false; } static inline bool pm_runtime_enabled(struct device *dev) { return false; } static inline void pm_runtime_no_callbacks(struct device *dev) {} ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] PM / Runtime: Add pm_runtime_enable_recursive 2015-07-08 20:31 ` Alan Stern @ 2015-07-14 13:19 ` Tomeu Vizoso 2015-07-14 21:57 ` Alan Stern 0 siblings, 1 reply; 26+ messages in thread From: Tomeu Vizoso @ 2015-07-14 13:19 UTC (permalink / raw) To: Alan Stern Cc: Rafael J. Wysocki, linux-pm, Laurent Pinchart, Dmitry Torokhov, Len Brown, Pavel Machek, Greg Kroah-Hartman, Ulf Hansson, Kevin Hilman, Russell King, Krzysztof Kozlowski, linux-kernel On 8 July 2015 at 22:31, Alan Stern <stern@rowland.harvard.edu> wrote: > On Wed, 8 Jul 2015, Rafael J. Wysocki wrote: > >> I seem to have lost the context here, sorry about that. >> >> The idea seems to be to rely on the fact that the RPM status for all devices >> is initially RPM_SUSPENDED and that never changes if runtime PM is never >> enabled for the device, so in that particular case it would be OK to treat >> the "power.direct_complete set + RPM status == RPM_SUSPENDED" combination >> as valid even though runtime PM has never been enabled for the device in >> question (provided that power.direct_complete will never be set for "real" >> devices that don't support runtime PM). Is that correct? > > I would have expressed it slightly differently, but yes, that's correct. > >> That seems to be fragile, but I have no strong opinion. > > In itself it's not all that bad, I think. In the presence of Tomeu's > new direct_complete_default flag, however, it does seem quite fragile. > > We may want to do the direct_complete_default thing in a different way. > For example, the PM core could automatically set the direct_complete > flag if a device has _none_ of the system suspend callbacks (i.e., no > prepare, suspend, suspend_late, suspend_noirq, resume_noirq, > resume_early, resume, or complete). Although it would be a little > awkward to check this, it would be safer than inheriting > direct_complete_default from the parent and it ought to solve Tomeu's > problem just as well. Yeah, I think this is an improvement. Will give it a try. >> Let's do that change if it allows us to make forward progress here. Please >> feel free to submit a documentation patch along the lines you've suggested. > > Here's a proposed patch to illustrate what I have in mind. Since it > removes the only usage of pm_runtime_suspended_if_enabled(), it also > removes the definition of that function. Will this patch be picked up as-is or should I add it to my series with a proper changelog? Thanks, Tomeu > Alan Stern > > > > Index: usb-4.1/drivers/base/power/main.c > =================================================================== > --- usb-4.1.orig/drivers/base/power/main.c > +++ usb-4.1/drivers/base/power/main.c > @@ -1374,7 +1374,7 @@ static int __device_suspend(struct devic > if (dev->power.direct_complete) { > if (pm_runtime_status_suspended(dev)) { > pm_runtime_disable(dev); > - if (pm_runtime_suspended_if_enabled(dev)) > + if (pm_runtime_status_suspended(dev)) > goto Complete; > > pm_runtime_enable(dev); > Index: usb-4.1/Documentation/power/devices.txt > =================================================================== > --- usb-4.1.orig/Documentation/power/devices.txt > +++ usb-4.1/Documentation/power/devices.txt > @@ -341,6 +341,13 @@ the phases are: > and is entirely responsible for bringing the device back to the > functional state as appropriate. > > + Note that this direct-complete procedure applies even if the device is > + disabled for runtime PM; only the runtime-PM status matters. It follows > + that if a device has system-sleep callbacks but does not support runtime > + PM, then its prepare callback must never return a positive value. This > + is because all devices are initially set to runtime-suspended with > + runtime PM disabled. > + > 2. The suspend methods should quiesce the device to stop it from performing > I/O. They also may save the device registers and put it into the > appropriate low-power state, depending on the bus type the device is on, > Index: usb-4.1/Documentation/power/runtime_pm.txt > =================================================================== > --- usb-4.1.orig/Documentation/power/runtime_pm.txt > +++ usb-4.1/Documentation/power/runtime_pm.txt > @@ -445,10 +445,6 @@ drivers/base/power/runtime.c and include > bool pm_runtime_status_suspended(struct device *dev); > - return true if the device's runtime PM status is 'suspended' > > - bool pm_runtime_suspended_if_enabled(struct device *dev); > - - return true if the device's runtime PM status is 'suspended' and its > - 'power.disable_depth' field is equal to 1 > - > void pm_runtime_allow(struct device *dev); > - set the power.runtime_auto flag for the device and decrease its usage > counter (used by the /sys/devices/.../power/control interface to > Index: usb-4.1/include/linux/pm_runtime.h > =================================================================== > --- usb-4.1.orig/include/linux/pm_runtime.h > +++ usb-4.1/include/linux/pm_runtime.h > @@ -98,11 +98,6 @@ static inline bool pm_runtime_status_sus > return dev->power.runtime_status == RPM_SUSPENDED; > } > > -static inline bool pm_runtime_suspended_if_enabled(struct device *dev) > -{ > - return pm_runtime_status_suspended(dev) && dev->power.disable_depth == 1; > -} > - > static inline bool pm_runtime_enabled(struct device *dev) > { > return !dev->power.disable_depth; > @@ -164,7 +159,6 @@ static inline void device_set_run_wake(s > static inline bool pm_runtime_suspended(struct device *dev) { return false; } > static inline bool pm_runtime_active(struct device *dev) { return true; } > static inline bool pm_runtime_status_suspended(struct device *dev) { return false; } > -static inline bool pm_runtime_suspended_if_enabled(struct device *dev) { return false; } > static inline bool pm_runtime_enabled(struct device *dev) { return false; } > > static inline void pm_runtime_no_callbacks(struct device *dev) {} > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] PM / Runtime: Add pm_runtime_enable_recursive 2015-07-14 13:19 ` Tomeu Vizoso @ 2015-07-14 21:57 ` Alan Stern 0 siblings, 0 replies; 26+ messages in thread From: Alan Stern @ 2015-07-14 21:57 UTC (permalink / raw) To: Tomeu Vizoso Cc: Rafael J. Wysocki, linux-pm, Laurent Pinchart, Dmitry Torokhov, Len Brown, Pavel Machek, Greg Kroah-Hartman, Ulf Hansson, Kevin Hilman, Russell King, Krzysztof Kozlowski, linux-kernel On Tue, 14 Jul 2015, Tomeu Vizoso wrote: > > We may want to do the direct_complete_default thing in a different way. > > For example, the PM core could automatically set the direct_complete > > flag if a device has _none_ of the system suspend callbacks (i.e., no > > prepare, suspend, suspend_late, suspend_noirq, resume_noirq, > > resume_early, resume, or complete). Although it would be a little > > awkward to check this, it would be safer than inheriting > > direct_complete_default from the parent and it ought to solve Tomeu's > > problem just as well. > > Yeah, I think this is an improvement. Will give it a try. Sounds good. > > Here's a proposed patch to illustrate what I have in mind. Since it > > removes the only usage of pm_runtime_suspended_if_enabled(), it also > > removes the definition of that function. > > Will this patch be picked up as-is or should I add it to my series > with a proper changelog? You can add it to your series with my S-O-B: Signed-off-by: Alan Stern <stern@rowland.harvard.edu> Alan Stern ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2015-07-14 21:57 UTC | newest] Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-05-19 14:11 [PATCH v3 0/2] PM: direct_complete_default and pm_runtime_enable_recursive Tomeu Vizoso 2015-05-19 14:11 ` [PATCH v3 1/2] PM / sleep: Add power.direct_complete_default flag Tomeu Vizoso 2015-05-19 17:50 ` Alan Stern 2015-05-19 20:47 ` Ulf Hansson 2015-05-19 23:38 ` Rafael J. Wysocki 2015-05-19 14:11 ` [PATCH v3 2/2] PM / Runtime: Add pm_runtime_enable_recursive Tomeu Vizoso 2015-05-19 17:49 ` Alan Stern 2015-05-19 23:39 ` Rafael J. Wysocki 2015-05-20 9:03 ` Tomeu Vizoso 2015-05-20 14:24 ` Alan Stern 2015-07-02 13:59 ` Tomeu Vizoso 2015-07-02 15:21 ` Alan Stern 2015-07-03 8:11 ` Tomeu Vizoso 2015-07-03 14:16 ` Alan Stern 2015-07-03 14:22 ` Tomeu Vizoso 2015-07-03 15:11 ` Alan Stern 2015-07-04 0:32 ` Rafael J. Wysocki 2015-07-04 0:31 ` Rafael J. Wysocki 2015-07-04 14:37 ` Alan Stern 2015-07-05 23:36 ` Rafael J. Wysocki 2015-07-07 0:07 ` Rafael J. Wysocki 2015-07-07 14:55 ` Alan Stern 2015-07-07 22:06 ` Rafael J. Wysocki 2015-07-08 20:31 ` Alan Stern 2015-07-14 13:19 ` Tomeu Vizoso 2015-07-14 21:57 ` Alan Stern
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).