From: Rafael J. Wysocki <[email protected]>
Prevent rpm_get_suppliers() from returning an error code if runtime
PM is disabled for one or more of the supplier devices it wants to
runtime-resume, so as to make runtime PM work for devices with links
to suppliers that don't use runtime PM (such links may be created
during device enumeration even before it is known whether or not
runtime PM will be enabled for the devices in question, for example).
Reported-by: Adrian Hunter <[email protected]>
Fixes: 21d5c57b3726 (PM / runtime: Use device links)
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/base/power/runtime.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Index: linux-pm/drivers/base/power/runtime.c
===================================================================
--- linux-pm.orig/drivers/base/power/runtime.c
+++ linux-pm/drivers/base/power/runtime.c
@@ -276,7 +276,8 @@ static int rpm_get_suppliers(struct devi
continue;
retval = pm_runtime_get_sync(link->supplier);
- if (retval < 0) {
+ /* Ignore suppliers with disabled runtime PM. */
+ if (retval < 0 && retval != -EACCES) {
pm_runtime_put_noidle(link->supplier);
return retval;
}
On Fri, Dec 01, 2017 at 02:58:34PM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> Prevent rpm_get_suppliers() from returning an error code if runtime
> PM is disabled for one or more of the supplier devices it wants to
> runtime-resume, so as to make runtime PM work for devices with links
> to suppliers that don't use runtime PM (such links may be created
> during device enumeration even before it is known whether or not
> runtime PM will be enabled for the devices in question, for example).
>
> Reported-by: Adrian Hunter <[email protected]>
> Fixes: 21d5c57b3726 (PM / runtime: Use device links)
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
> drivers/base/power/runtime.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> Index: linux-pm/drivers/base/power/runtime.c
> ===================================================================
> --- linux-pm.orig/drivers/base/power/runtime.c
> +++ linux-pm/drivers/base/power/runtime.c
> @@ -276,7 +276,8 @@ static int rpm_get_suppliers(struct devi
> continue;
>
> retval = pm_runtime_get_sync(link->supplier);
> - if (retval < 0) {
> + /* Ignore suppliers with disabled runtime PM. */
> + if (retval < 0 && retval != -EACCES) {
> pm_runtime_put_noidle(link->supplier);
> return retval;
> }
>
You could alternatively call pm_runtime_get_sync() under the condition
link->supplier->power.disable_depth > 0 but then the usage_count wouldn't
be incremented and I guess we want that in case runtime PM is only
temporarily disabled and later enabled, right?
I'm wondering if checking for that condition in lieu of retval != -EACCES
would be more explicit and less fragile here (there's a theoretical
possibility that the supplier's ->runtime_resume callback returns -EACCESS,
leading to a false positive).
Apart from that,
Reviewed-by: Lukas Wunner <[email protected]>
Thanks,
Lukas
On Friday, December 1, 2017 3:18:01 PM CET Lukas Wunner wrote:
> On Fri, Dec 01, 2017 at 02:58:34PM +0100, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <[email protected]>
> >
> > Prevent rpm_get_suppliers() from returning an error code if runtime
> > PM is disabled for one or more of the supplier devices it wants to
> > runtime-resume, so as to make runtime PM work for devices with links
> > to suppliers that don't use runtime PM (such links may be created
> > during device enumeration even before it is known whether or not
> > runtime PM will be enabled for the devices in question, for example).
> >
> > Reported-by: Adrian Hunter <[email protected]>
> > Fixes: 21d5c57b3726 (PM / runtime: Use device links)
> > Signed-off-by: Rafael J. Wysocki <[email protected]>
> > ---
> > drivers/base/power/runtime.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > Index: linux-pm/drivers/base/power/runtime.c
> > ===================================================================
> > --- linux-pm.orig/drivers/base/power/runtime.c
> > +++ linux-pm/drivers/base/power/runtime.c
> > @@ -276,7 +276,8 @@ static int rpm_get_suppliers(struct devi
> > continue;
> >
> > retval = pm_runtime_get_sync(link->supplier);
> > - if (retval < 0) {
> > + /* Ignore suppliers with disabled runtime PM. */
> > + if (retval < 0 && retval != -EACCES) {
> > pm_runtime_put_noidle(link->supplier);
> > return retval;
> > }
> >
>
> You could alternatively call pm_runtime_get_sync() under the condition
> link->supplier->power.disable_depth > 0 but then the usage_count wouldn't
> be incremented and I guess we want that in case runtime PM is only
> temporarily disabled and later enabled, right?
>
> I'm wondering if checking for that condition in lieu of retval != -EACCES
> would be more explicit and less fragile here (there's a theoretical
> possibility that the supplier's ->runtime_resume callback returns -EACCESS,
> leading to a false positive).
That will be filtered out by rpm_callback() which converts -EACCES to -EIO.
>
> Apart from that,
>
> Reviewed-by: Lukas Wunner <[email protected]>
Thanks,
Rafael
On Fri, Dec 1, 2017 at 3:18 PM, Lukas Wunner <[email protected]> wrote:
> On Fri, Dec 01, 2017 at 02:58:34PM +0100, Rafael J. Wysocki wrote:
>> From: Rafael J. Wysocki <[email protected]>
>>
>> Prevent rpm_get_suppliers() from returning an error code if runtime
>> PM is disabled for one or more of the supplier devices it wants to
>> runtime-resume, so as to make runtime PM work for devices with links
>> to suppliers that don't use runtime PM (such links may be created
>> during device enumeration even before it is known whether or not
>> runtime PM will be enabled for the devices in question, for example).
>>
>> Reported-by: Adrian Hunter <[email protected]>
>> Fixes: 21d5c57b3726 (PM / runtime: Use device links)
>> Signed-off-by: Rafael J. Wysocki <[email protected]>
>> ---
>> drivers/base/power/runtime.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> Index: linux-pm/drivers/base/power/runtime.c
>> ===================================================================
>> --- linux-pm.orig/drivers/base/power/runtime.c
>> +++ linux-pm/drivers/base/power/runtime.c
>> @@ -276,7 +276,8 @@ static int rpm_get_suppliers(struct devi
>> continue;
>>
>> retval = pm_runtime_get_sync(link->supplier);
>> - if (retval < 0) {
>> + /* Ignore suppliers with disabled runtime PM. */
>> + if (retval < 0 && retval != -EACCES) {
>> pm_runtime_put_noidle(link->supplier);
>> return retval;
>> }
>>
>
> You could alternatively call pm_runtime_get_sync() under the condition
> link->supplier->power.disable_depth > 0 but then the usage_count wouldn't
> be incremented and I guess we want that in case runtime PM is only
> temporarily disabled and later enabled, right?
Right.
Thanks,
Rafael
On 01/12/17 15:58, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> Prevent rpm_get_suppliers() from returning an error code if runtime
> PM is disabled for one or more of the supplier devices it wants to
> runtime-resume, so as to make runtime PM work for devices with links
> to suppliers that don't use runtime PM (such links may be created
> during device enumeration even before it is known whether or not
> runtime PM will be enabled for the devices in question, for example).
>
> Reported-by: Adrian Hunter <[email protected]>
> Fixes: 21d5c57b3726 (PM / runtime: Use device links)
> Signed-off-by: Rafael J. Wysocki <[email protected]>
Tested-by: Adrian Hunter <[email protected]>
> ---
> drivers/base/power/runtime.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> Index: linux-pm/drivers/base/power/runtime.c
> ===================================================================
> --- linux-pm.orig/drivers/base/power/runtime.c
> +++ linux-pm/drivers/base/power/runtime.c
> @@ -276,7 +276,8 @@ static int rpm_get_suppliers(struct devi
> continue;
>
> retval = pm_runtime_get_sync(link->supplier);
> - if (retval < 0) {
> + /* Ignore suppliers with disabled runtime PM. */
> + if (retval < 0 && retval != -EACCES) {
> pm_runtime_put_noidle(link->supplier);
> return retval;
> }
>
>
On 1 December 2017 at 14:58, Rafael J. Wysocki <[email protected]> wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> Prevent rpm_get_suppliers() from returning an error code if runtime
> PM is disabled for one or more of the supplier devices it wants to
> runtime-resume, so as to make runtime PM work for devices with links
> to suppliers that don't use runtime PM (such links may be created
> during device enumeration even before it is known whether or not
> runtime PM will be enabled for the devices in question, for example).
>
> Reported-by: Adrian Hunter <[email protected]>
> Fixes: 21d5c57b3726 (PM / runtime: Use device links)
> Signed-off-by: Rafael J. Wysocki <[email protected]>
Reviewed-by: Ulf Hansson <[email protected]>
Kind regards
Uffe
> ---
> drivers/base/power/runtime.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> Index: linux-pm/drivers/base/power/runtime.c
> ===================================================================
> --- linux-pm.orig/drivers/base/power/runtime.c
> +++ linux-pm/drivers/base/power/runtime.c
> @@ -276,7 +276,8 @@ static int rpm_get_suppliers(struct devi
> continue;
>
> retval = pm_runtime_get_sync(link->supplier);
> - if (retval < 0) {
> + /* Ignore suppliers with disabled runtime PM. */
> + if (retval < 0 && retval != -EACCES) {
> pm_runtime_put_noidle(link->supplier);
> return retval;
> }
>