[PATCH] PM / Domains: defer dev_pm_domain_set() until genpd->attach_dev succeeds if present

[PATCH] PM / Domains: defer dev_pm_domain_set() until genpd->attach_dev succeeds if present

[Sudeep Holla]

If the genpd->attach_dev or genpd->power_on fails, genpd_dev_pm_attach
may return -EPROBE_DEFER initially. However genpd_alloc_dev_data sets
the PM domain for the device unconditionally.

When subsequent attempts are made to call genpd_dev_pm_attach, it may
return -EEXISTS checking dev->pm_domain without re-attempting to call
attach_dev or power_on.

platform_drv_probe then attempts to call drv->probe as the return value
-EEXIST != -EPROBE_DEFER, which may end up in a situation where the
device is accessed without it's power domain switched on.

Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Kevin Hilman <khilman@kernel.org>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/base/power/domain.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index da49a8383dc3..b195d34de888 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1168,8 +1168,6 @@ static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev,
 
 	spin_unlock_irq(&dev->power.lock);
 
-	dev_pm_domain_set(dev, &genpd->domain);
-
 	return gpd_data;
 
  err_free:
@@ -1221,6 +1219,8 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
 	if (ret)
 		goto out;
 
+	dev_pm_domain_set(dev, &genpd->domain);
+
 	genpd->device_count++;
 	genpd->max_off_time_changed = true;
 
-- 
2.7.4

Re: [PATCH] PM / Domains: defer dev_pm_domain_set() until genpd->attach_dev succeeds if present

[Rafael J. Wysocki]

On Thu, Jun 29, 2017 at 8:00 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
> If the genpd->attach_dev or genpd->power_on fails, genpd_dev_pm_attach
> may return -EPROBE_DEFER initially. However genpd_alloc_dev_data sets
> the PM domain for the device unconditionally.
>
> When subsequent attempts are made to call genpd_dev_pm_attach, it may
> return -EEXISTS checking dev->pm_domain without re-attempting to call
> attach_dev or power_on.
>
> platform_drv_probe then attempts to call drv->probe as the return value
> -EEXIST != -EPROBE_DEFER, which may end up in a situation where the
> device is accessed without it's power domain switched on.
>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Kevin Hilman <khilman@kernel.org>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

Ulf, this looks like a genuine fix to me, any comments?

> ---
>  drivers/base/power/domain.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index da49a8383dc3..b195d34de888 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1168,8 +1168,6 @@ static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev,
>
>         spin_unlock_irq(&dev->power.lock);
>
> -       dev_pm_domain_set(dev, &genpd->domain);
> -
>         return gpd_data;
>
>   err_free:
> @@ -1221,6 +1219,8 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
>         if (ret)
>                 goto out;
>
> +       dev_pm_domain_set(dev, &genpd->domain);
> +
>         genpd->device_count++;
>         genpd->max_off_time_changed = true;
>
> --
> 2.7.4
>

Re: [PATCH] PM / Domains: defer dev_pm_domain_set() until genpd->attach_dev succeeds if present

[Kevin Hilman]

"Rafael J. Wysocki" <rafael@kernel.org> writes:

> On Thu, Jun 29, 2017 at 8:00 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>> If the genpd->attach_dev or genpd->power_on fails, genpd_dev_pm_attach
>> may return -EPROBE_DEFER initially. However genpd_alloc_dev_data sets
>> the PM domain for the device unconditionally.
>>
>> When subsequent attempts are made to call genpd_dev_pm_attach, it may
>> return -EEXISTS checking dev->pm_domain without re-attempting to call
>> attach_dev or power_on.
>>
>> platform_drv_probe then attempts to call drv->probe as the return value
>> -EEXIST != -EPROBE_DEFER, which may end up in a situation where the
>> device is accessed without it's power domain switched on.
>>
>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>> Cc: Kevin Hilman <khilman@kernel.org>
>> Cc: Ulf Hansson <ulf.hansson@linaro.org>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>
> Ulf, this looks like a genuine fix to me, any comments?
>

This looks like the right fix to me.

Acked-by: Kevin Hilman <khilman@baylibre.com>

Re: [PATCH] PM / Domains: defer dev_pm_domain_set() until genpd->attach_dev succeeds if present

[Ulf Hansson]

On 29 June 2017 at 20:00, Sudeep Holla <sudeep.holla@arm.com> wrote:
> If the genpd->attach_dev or genpd->power_on fails, genpd_dev_pm_attach
> may return -EPROBE_DEFER initially. However genpd_alloc_dev_data sets
> the PM domain for the device unconditionally.
>
> When subsequent attempts are made to call genpd_dev_pm_attach, it may
> return -EEXISTS checking dev->pm_domain without re-attempting to call
> attach_dev or power_on.
>
> platform_drv_probe then attempts to call drv->probe as the return value
> -EEXIST != -EPROBE_DEFER, which may end up in a situation where the
> device is accessed without it's power domain switched on.

Right, this makes sense.

>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Kevin Hilman <khilman@kernel.org>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

Could we perhaps work out which commit it fixes, or perhaps the
problem been there long time ago and we should just add a stable tag?

> ---
>  drivers/base/power/domain.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index da49a8383dc3..b195d34de888 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1168,8 +1168,6 @@ static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev,
>
>         spin_unlock_irq(&dev->power.lock);
>
> -       dev_pm_domain_set(dev, &genpd->domain);
> -
>         return gpd_data;
>
>   err_free:
> @@ -1221,6 +1219,8 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
>         if (ret)
>                 goto out;
>
> +       dev_pm_domain_set(dev, &genpd->domain);
> +
>         genpd->device_count++;
>         genpd->max_off_time_changed = true;
>
> --
> 2.7.4
>

One piece is missing to make this fix complete.

More precisely, you must also move the call to dev_pm_domain_set(dev, NULL).

Currently that is done from genpd_free_dev_data(), as it corresponds
to genpd_alloc_dev_data(), but clearly the proper place to call it
should be from genpd_remove_device().

Kind regards
Uffe

Re: [PATCH] PM / Domains: defer dev_pm_domain_set() until genpd->attach_dev succeeds if present

[Sudeep Holla]

On 12/07/17 14:59, Ulf Hansson wrote:
> On 29 June 2017 at 20:00, Sudeep Holla <sudeep.holla@arm.com> wrote:
>> If the genpd->attach_dev or genpd->power_on fails, genpd_dev_pm_attach
>> may return -EPROBE_DEFER initially. However genpd_alloc_dev_data sets
>> the PM domain for the device unconditionally.
>>
>> When subsequent attempts are made to call genpd_dev_pm_attach, it may
>> return -EEXISTS checking dev->pm_domain without re-attempting to call
>> attach_dev or power_on.
>>
>> platform_drv_probe then attempts to call drv->probe as the return value
>> -EEXIST != -EPROBE_DEFER, which may end up in a situation where the
>> device is accessed without it's power domain switched on.
> 
> Right, this makes sense.
> 

Thanks

>>
>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>> Cc: Kevin Hilman <khilman@kernel.org>
>> Cc: Ulf Hansson <ulf.hansson@linaro.org>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> 
> Could we perhaps work out which commit it fixes, or perhaps the
> problem been there long time ago and we should just add a stable tag?
>

OK I will dig that out.

>> ---
>>  drivers/base/power/domain.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> index da49a8383dc3..b195d34de888 100644
>> --- a/drivers/base/power/domain.c
>> +++ b/drivers/base/power/domain.c
>> @@ -1168,8 +1168,6 @@ static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev,
>>
>>         spin_unlock_irq(&dev->power.lock);
>>
>> -       dev_pm_domain_set(dev, &genpd->domain);
>> -
>>         return gpd_data;
>>
>>   err_free:
>> @@ -1221,6 +1219,8 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
>>         if (ret)
>>                 goto out;
>>
>> +       dev_pm_domain_set(dev, &genpd->domain);
>> +
>>         genpd->device_count++;
>>         genpd->max_off_time_changed = true;
>>
>> --
>> 2.7.4
>>
> 
> One piece is missing to make this fix complete.
> 
> More precisely, you must also move the call to dev_pm_domain_set(dev, NULL).
> 

Sure will add.

> Currently that is done from genpd_free_dev_data(), as it corresponds
> to genpd_alloc_dev_data(), but clearly the proper place to call it
> should be from genpd_remove_device().
> 

OK, will also look into that.

-- 
Regards,
Sudeep

Re: [PATCH] PM / Domains: defer dev_pm_domain_set() until genpd->attach_dev succeeds if present

[Sudeep Holla]

On 12/07/17 17:36, Sudeep Holla wrote:
> 
> 
> On 12/07/17 14:59, Ulf Hansson wrote:
>> On 29 June 2017 at 20:00, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>> If the genpd->attach_dev or genpd->power_on fails, genpd_dev_pm_attach
>>> may return -EPROBE_DEFER initially. However genpd_alloc_dev_data sets
>>> the PM domain for the device unconditionally.
>>>
>>> When subsequent attempts are made to call genpd_dev_pm_attach, it may
>>> return -EEXISTS checking dev->pm_domain without re-attempting to call
>>> attach_dev or power_on.
>>>
>>> platform_drv_probe then attempts to call drv->probe as the return value
>>> -EEXIST != -EPROBE_DEFER, which may end up in a situation where the
>>> device is accessed without it's power domain switched on.
>>
>> Right, this makes sense.
>>
> 
> Thanks
> 
>>>
>>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>>> Cc: Kevin Hilman <khilman@kernel.org>
>>> Cc: Ulf Hansson <ulf.hansson@linaro.org>
>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>>
>> Could we perhaps work out which commit it fixes, or perhaps the
>> problem been there long time ago and we should just add a stable tag?
>>
> 
> OK I will dig that out.
> 

It looks like commit 989561de9b51 ("PM / Domains: add setter for
dev.pm_domain") added the helper. I then follow it to commit
f104e1e5ef57 ("PM / Domains: Re-order initialization of
generic_pm_domain_data"). The code in the original commit 6ff7bb0d02f8
("PM / Domains: Cache device stop and domain power off governor results,
v3") looks OK to me.

So What should I do ? Add
Fixes: f104e1e5ef57 ("PM / Domains: Re-order initialization of
generic_pm_domain_data")

tag in the mainline. And when Greg or others fix up before v4.4, I need
to work out the fix for that stable version ?

-- 
Regards,
Sudeep

[PATCH v2] PM / Domains: defer dev_pm_domain_set() until genpd->attach_dev succeeds if present

[Sudeep Holla]

If the genpd->attach_dev or genpd->power_on fails, genpd_dev_pm_attach
may return -EPROBE_DEFER initially. However genpd_alloc_dev_data sets
the PM domain for the device unconditionally.

When subsequent attempts are made to call genpd_dev_pm_attach, it may
return -EEXISTS checking dev->pm_domain without re-attempting to call
attach_dev or power_on.

platform_drv_probe then attempts to call drv->probe as the return value
-EEXIST != -EPROBE_DEFER, which may end up in a situation where the
device is accessed without it's power domain switched on.

Fixes: f104e1e5ef57 ("PM / Domains: Re-order initialization of generic_pm_domain_data")
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Kevin Hilman <khilman@kernel.org>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: <stable@vger.kernel.org> # v4.0+
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/base/power/domain.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Hi,

I have added Cc/Fixes tags based on the original commit that caused the issue.
I need to workout a patch that can cleanly apply before commit 989561de9b51
("PM / Domains: add setter for dev.pm_domain"), i.e. for v4.0.x to v4.4.x
This patch should apply from v4.5.x onwards.

Let me know if I got anything wrong.

v1->v2: Moved dev_pm_domain_set(dev, NULL) from genpd_free_dev_data

Regards,
Sudeep

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index da49a8383dc3..2d9178660061 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1168,8 +1168,6 @@ static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev,
 
 	spin_unlock_irq(&dev->power.lock);
 
-	dev_pm_domain_set(dev, &genpd->domain);
-
 	return gpd_data;
 
  err_free:
@@ -1183,8 +1181,6 @@ static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev,
 static void genpd_free_dev_data(struct device *dev,
 				struct generic_pm_domain_data *gpd_data)
 {
-	dev_pm_domain_set(dev, NULL);
-
 	spin_lock_irq(&dev->power.lock);
 
 	dev->power.subsys_data->domain_data = NULL;
@@ -1221,6 +1217,8 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
 	if (ret)
 		goto out;
 
+	dev_pm_domain_set(dev, &genpd->domain);
+
 	genpd->device_count++;
 	genpd->max_off_time_changed = true;
 
@@ -1282,6 +1280,8 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
 	if (genpd->detach_dev)
 		genpd->detach_dev(genpd, dev);
 
+	dev_pm_domain_set(dev, NULL);
+
 	list_del_init(&pdd->list_node);
 
 	genpd_unlock(genpd);
-- 
2.7.4

Re: [PATCH v2] PM / Domains: defer dev_pm_domain_set() until genpd->attach_dev succeeds if present

[Rafael J. Wysocki]

On Friday, July 14, 2017 11:51:48 AM Sudeep Holla wrote:
> If the genpd->attach_dev or genpd->power_on fails, genpd_dev_pm_attach
> may return -EPROBE_DEFER initially. However genpd_alloc_dev_data sets
> the PM domain for the device unconditionally.
> 
> When subsequent attempts are made to call genpd_dev_pm_attach, it may
> return -EEXISTS checking dev->pm_domain without re-attempting to call
> attach_dev or power_on.
> 
> platform_drv_probe then attempts to call drv->probe as the return value
> -EEXIST != -EPROBE_DEFER, which may end up in a situation where the
> device is accessed without it's power domain switched on.
> 
> Fixes: f104e1e5ef57 ("PM / Domains: Re-order initialization of generic_pm_domain_data")
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Kevin Hilman <khilman@kernel.org>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: <stable@vger.kernel.org> # v4.0+
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

Ulf, Kevin?

> ---
>  drivers/base/power/domain.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> Hi,
> 
> I have added Cc/Fixes tags based on the original commit that caused the issue.
> I need to workout a patch that can cleanly apply before commit 989561de9b51
> ("PM / Domains: add setter for dev.pm_domain"), i.e. for v4.0.x to v4.4.x
> This patch should apply from v4.5.x onwards.
> 
> Let me know if I got anything wrong.
> 
> v1->v2: Moved dev_pm_domain_set(dev, NULL) from genpd_free_dev_data
> 
> Regards,
> Sudeep
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index da49a8383dc3..2d9178660061 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1168,8 +1168,6 @@ static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev,
>  
>  	spin_unlock_irq(&dev->power.lock);
>  
> -	dev_pm_domain_set(dev, &genpd->domain);
> -
>  	return gpd_data;
>  
>   err_free:
> @@ -1183,8 +1181,6 @@ static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev,
>  static void genpd_free_dev_data(struct device *dev,
>  				struct generic_pm_domain_data *gpd_data)
>  {
> -	dev_pm_domain_set(dev, NULL);
> -
>  	spin_lock_irq(&dev->power.lock);
>  
>  	dev->power.subsys_data->domain_data = NULL;
> @@ -1221,6 +1217,8 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
>  	if (ret)
>  		goto out;
>  
> +	dev_pm_domain_set(dev, &genpd->domain);
> +
>  	genpd->device_count++;
>  	genpd->max_off_time_changed = true;
>  
> @@ -1282,6 +1280,8 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
>  	if (genpd->detach_dev)
>  		genpd->detach_dev(genpd, dev);
>  
> +	dev_pm_domain_set(dev, NULL);
> +
>  	list_del_init(&pdd->list_node);
>  
>  	genpd_unlock(genpd);
> 

Re: [PATCH v2] PM / Domains: defer dev_pm_domain_set() until genpd->attach_dev succeeds if present

[Ulf Hansson]

On 14 July 2017 at 12:51, Sudeep Holla <sudeep.holla@arm.com> wrote:
> If the genpd->attach_dev or genpd->power_on fails, genpd_dev_pm_attach
> may return -EPROBE_DEFER initially. However genpd_alloc_dev_data sets
> the PM domain for the device unconditionally.
>
> When subsequent attempts are made to call genpd_dev_pm_attach, it may
> return -EEXISTS checking dev->pm_domain without re-attempting to call
> attach_dev or power_on.
>
> platform_drv_probe then attempts to call drv->probe as the return value
> -EEXIST != -EPROBE_DEFER, which may end up in a situation where the
> device is accessed without it's power domain switched on.
>
> Fixes: f104e1e5ef57 ("PM / Domains: Re-order initialization of generic_pm_domain_data")
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Kevin Hilman <khilman@kernel.org>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: <stable@vger.kernel.org> # v4.0+
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

Acked-by: Ulf Hansson <ulf.hansson@linaro.org>

> ---
>  drivers/base/power/domain.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> Hi,
>
> I have added Cc/Fixes tags based on the original commit that caused the issue.
> I need to workout a patch that can cleanly apply before commit 989561de9b51
> ("PM / Domains: add setter for dev.pm_domain"), i.e. for v4.0.x to v4.4.x
> This patch should apply from v4.5.x onwards.

Thanks for checking this.

Perhaps we should change the stable tag to # v4.5+ instead then?
Otherwise the stable people will just report problems when trying to
apply it.

>
> Let me know if I got anything wrong.
>
> v1->v2: Moved dev_pm_domain_set(dev, NULL) from genpd_free_dev_data
>
> Regards,
> Sudeep

[...]

Kind regards
Uffe

Re: [PATCH v2] PM / Domains: defer dev_pm_domain_set() until genpd->attach_dev succeeds if present

[Sudeep Holla]

On 17/07/17 08:01, Ulf Hansson wrote:
> On 14 July 2017 at 12:51, Sudeep Holla <sudeep.holla@arm.com> wrote:
>> If the genpd->attach_dev or genpd->power_on fails, genpd_dev_pm_attach
>> may return -EPROBE_DEFER initially. However genpd_alloc_dev_data sets
>> the PM domain for the device unconditionally.
>>
>> When subsequent attempts are made to call genpd_dev_pm_attach, it may
>> return -EEXISTS checking dev->pm_domain without re-attempting to call
>> attach_dev or power_on.
>>
>> platform_drv_probe then attempts to call drv->probe as the return value
>> -EEXIST != -EPROBE_DEFER, which may end up in a situation where the
>> device is accessed without it's power domain switched on.
>>
>> Fixes: f104e1e5ef57 ("PM / Domains: Re-order initialization of generic_pm_domain_data")
>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>> Cc: Kevin Hilman <khilman@kernel.org>
>> Cc: Ulf Hansson <ulf.hansson@linaro.org>
>> Cc: <stable@vger.kernel.org> # v4.0+
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> 
> Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
> 
>> ---
>>  drivers/base/power/domain.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> Hi,
>>
>> I have added Cc/Fixes tags based on the original commit that caused the issue.
>> I need to workout a patch that can cleanly apply before commit 989561de9b51
>> ("PM / Domains: add setter for dev.pm_domain"), i.e. for v4.0.x to v4.4.x
>> This patch should apply from v4.5.x onwards.
> 
> Thanks for checking this.
> 
> Perhaps we should change the stable tag to # v4.5+ instead then?
> Otherwise the stable people will just report problems when trying to
> apply it.
> 

Yes, I know. I thought then I can work out the solution for whatever
version they need. Instead of trying to post patch for all versions from
v4.0 to v4.4
If that's not acceptable, then yes it should be #4.5+
Rafael, can you just update that if required ?

I am fine to work a patch for v4.[0-4].x when requested.

-- 
Regards,
Sudeep

Re: [PATCH v2] PM / Domains: defer dev_pm_domain_set() until genpd->attach_dev succeeds if present

[Rafael J. Wysocki]

On Monday, July 17, 2017 11:34:57 AM Sudeep Holla wrote:
> 
> On 17/07/17 08:01, Ulf Hansson wrote:
> > On 14 July 2017 at 12:51, Sudeep Holla <sudeep.holla@arm.com> wrote:
> >> If the genpd->attach_dev or genpd->power_on fails, genpd_dev_pm_attach
> >> may return -EPROBE_DEFER initially. However genpd_alloc_dev_data sets
> >> the PM domain for the device unconditionally.
> >>
> >> When subsequent attempts are made to call genpd_dev_pm_attach, it may
> >> return -EEXISTS checking dev->pm_domain without re-attempting to call
> >> attach_dev or power_on.
> >>
> >> platform_drv_probe then attempts to call drv->probe as the return value
> >> -EEXIST != -EPROBE_DEFER, which may end up in a situation where the
> >> device is accessed without it's power domain switched on.
> >>
> >> Fixes: f104e1e5ef57 ("PM / Domains: Re-order initialization of generic_pm_domain_data")
> >> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> >> Cc: Kevin Hilman <khilman@kernel.org>
> >> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> >> Cc: <stable@vger.kernel.org> # v4.0+
> >> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > 
> > Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
> > 
> >> ---
> >>  drivers/base/power/domain.c | 8 ++++----
> >>  1 file changed, 4 insertions(+), 4 deletions(-)
> >>
> >> Hi,
> >>
> >> I have added Cc/Fixes tags based on the original commit that caused the issue.
> >> I need to workout a patch that can cleanly apply before commit 989561de9b51
> >> ("PM / Domains: add setter for dev.pm_domain"), i.e. for v4.0.x to v4.4.x
> >> This patch should apply from v4.5.x onwards.
> > 
> > Thanks for checking this.
> > 
> > Perhaps we should change the stable tag to # v4.5+ instead then?
> > Otherwise the stable people will just report problems when trying to
> > apply it.
> > 
> 
> Yes, I know. I thought then I can work out the solution for whatever
> version they need. Instead of trying to post patch for all versions from
> v4.0 to v4.4
> If that's not acceptable, then yes it should be #4.5+
> Rafael, can you just update that if required ?

I can.

> I am fine to work a patch for v4.[0-4].x when requested.

That depends on how much of a problem this can be for the users of those
kernels, but I guess 4.4 is used by Android, so maybe it's worth the effort.

Thanks,
Rafael