[PATCH 0/4] PM / Domains: Fix race conditions during boot

[PATCH 0/4] PM / Domains: Fix race conditions during boot

[Ulf Hansson]

When there are more than one device in a PM domain these will obviously
be probed at different times. Depending on timing and the implemented
support for runtime PM in a driver/subsystem, genpd may be advised to
power off a PM domain after a successful probe sequence.
    
Ideally we should have relied on the driver/subsystem, through runtime
PM, to bring their device's PM domain into powered state prior doing
probing if such requirement exist.
    
Since this is not a common practice by drivers/subsystems, enforcing
such a change doesn't seem viable.
    
Instead, let's improve the situation, by preventing genpd from powering
off any of the PM domains until late_init. At that point genpd already
tries to power off unused PM domains, so it seems like a decent match.
    
Cases which can't be covered within the window of until late_init needs
to be adressed separately and likely through a more common long term
solution.


Ulf Hansson (4):
  PM / Domains: Remove pm_genpd_dev_need_restore() API
  ARM: exynos: Ensure PM domains are powered at initialization
  PM / Domains: Expect PM domains being powered at initialization
  PM / Domains: Enforce PM domains to stay powered during boot

 arch/arm/mach-exynos/pm_domains.c   |  7 ++++---
 arch/arm/mach-s3c64xx/pm.c          |  4 ++--
 arch/arm/mach-shmobile/pm-r8a7779.c |  2 +-
 arch/arm/mach-shmobile/pm-rmobile.c |  2 +-
 drivers/base/power/domain.c         | 42 ++++++++++++++++---------------------
 include/linux/pm_domain.h           |  7 +++----
 6 files changed, 29 insertions(+), 35 deletions(-)

-- 
1.9.1

[PATCH 1/4] PM / Domains: Remove pm_genpd_dev_need_restore() API

[Ulf Hansson]

There are currently no users of this API, let's remove it.

Additionally, if such feature would be needed future wise, a better
option is likely use pm_runtime_set_active|suspended() in some form.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/domain.c | 20 --------------------
 include/linux/pm_domain.h   |  2 --
 2 files changed, 22 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 18cc68d..36871b3 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1533,26 +1533,6 @@ int pm_genpd_remove_device(struct generic_pm_domain *genpd,
 }
 
 /**
- * pm_genpd_dev_need_restore - Set/unset the device's "need restore" flag.
- * @dev: Device to set/unset the flag for.
- * @val: The new value of the device's "need restore" flag.
- */
-void pm_genpd_dev_need_restore(struct device *dev, bool val)
-{
-	struct pm_subsys_data *psd;
-	unsigned long flags;
-
-	spin_lock_irqsave(&dev->power.lock, flags);
-
-	psd = dev_to_psd(dev);
-	if (psd && psd->domain_data)
-		to_gpd_data(psd->domain_data)->need_restore = val;
-
-	spin_unlock_irqrestore(&dev->power.lock, flags);
-}
-EXPORT_SYMBOL_GPL(pm_genpd_dev_need_restore);
-
-/**
  * pm_genpd_add_subdomain - Add a subdomain to an I/O PM domain.
  * @genpd: Master PM domain to add the subdomain to.
  * @subdomain: Subdomain to be added.
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 9004743..a21dfa9 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -129,7 +129,6 @@ extern int __pm_genpd_name_add_device(const char *domain_name,
 
 extern int pm_genpd_remove_device(struct generic_pm_domain *genpd,
 				  struct device *dev);
-extern void pm_genpd_dev_need_restore(struct device *dev, bool val);
 extern int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
 				  struct generic_pm_domain *new_subdomain);
 extern int pm_genpd_add_subdomain_names(const char *master_name,
@@ -175,7 +174,6 @@ static inline int pm_genpd_remove_device(struct generic_pm_domain *genpd,
 {
 	return -ENOSYS;
 }
-static inline void pm_genpd_dev_need_restore(struct device *dev, bool val) {}
 static inline int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
 					 struct generic_pm_domain *new_sd)
 {
-- 
1.9.1

Re: [PATCH 1/4] PM / Domains: Remove pm_genpd_dev_need_restore() API

[Geert Uytterhoeven]

On Tue, Sep 30, 2014 at 2:43 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> There are currently no users of this API, let's remove it.
>
> Additionally, if such feature would be needed future wise, a better
> option is likely use pm_runtime_set_active|suspended() in some form.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/4] PM / Domains: Remove pm_genpd_dev_need_restore() API

[Kevin Hilman]

Ulf Hansson <ulf.hansson@linaro.org> writes:

> There are currently no users of this API, let's remove it.
>
> Additionally, if such feature would be needed future wise, a better
> option is likely use pm_runtime_set_active|suspended() in some form.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Acked-by: Kevin Hilman <khilman@linaro.org>

[PATCH 2/4] ARM: exynos: Ensure PM domains are powered at initialization

[Ulf Hansson]

At ->probe() it's common practice for drivers/subsystems to bring their
devices to full power and without depending on CONFIG_PM_RUNTIME.

We could also expect that drivers/subsystems requires their device's
corresponding PM domains to be powered, to successfully complete a
->probe() sequence.

Align to the behavior above, by ensuring all PM domains are powered
prior initialization of a generic PM domain.

Do note, since the generic PM domain will try to power off unused PM
domains at late_init, there are no increased power consumption over
time.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 arch/arm/mach-exynos/pm_domains.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-exynos/pm_domains.c b/arch/arm/mach-exynos/pm_domains.c
index 20f2671..58e18e9 100644
--- a/arch/arm/mach-exynos/pm_domains.c
+++ b/arch/arm/mach-exynos/pm_domains.c
@@ -112,7 +112,7 @@ static __init int exynos4_pm_init_power_domain(void)
 
 	for_each_compatible_node(np, NULL, "samsung,exynos4210-pd") {
 		struct exynos_pm_domain *pd;
-		int on, i;
+		int i;
 		struct device *dev;
 
 		pdev = of_find_device_by_node(np);
@@ -155,9 +155,10 @@ static __init int exynos4_pm_init_power_domain(void)
 			clk_put(pd->oscclk);
 
 no_clk:
-		on = __raw_readl(pd->base + 0x4) & INT_LOCAL_PWR_EN;
+		if (!(__raw_readl(pd->base + 0x4) & INT_LOCAL_PWR_EN))
+			exynos_pd_power_on(&pd->pd);
 
-		pm_genpd_init(&pd->pd, NULL, !on);
+		pm_genpd_init(&pd->pd, NULL, false);
 		of_genpd_add_provider_simple(np, &pd->pd);
 	}
 
-- 
1.9.1

Re: [PATCH 2/4] ARM: exynos: Ensure PM domains are powered at initialization

[Kevin Hilman]

Ulf Hansson <ulf.hansson@linaro.org> writes:

> At ->probe() it's common practice for drivers/subsystems to bring their
> devices to full power and without depending on CONFIG_PM_RUNTIME.
>
> We could also expect that drivers/subsystems requires their device's
> corresponding PM domains to be powered, to successfully complete a
> ->probe() sequence.
>
> Align to the behavior above, by ensuring all PM domains are powered
> prior initialization of a generic PM domain.
>
> Do note, since the generic PM domain will try to power off unused PM
> domains at late_init, there are no increased power consumption over
> time.

IMO "no increased power consumption" is a bit misleading because
boot-time power consumption may have a major increase when all power
domains are powered on.  Sure, they will eventually (hopefully) be
turned off in after late_initcall, but there will still be an impact.

I guess the Samsung folks should comment here on whether that is
significant.

Kevin

Re: [PATCH 2/4] ARM: exynos: Ensure PM domains are powered at initialization

[Tomasz Figa]

On 30.09.2014 20:33, Kevin Hilman wrote:
> Ulf Hansson <ulf.hansson@linaro.org> writes:
> 
>> At ->probe() it's common practice for drivers/subsystems to bring their
>> devices to full power and without depending on CONFIG_PM_RUNTIME.
>>
>> We could also expect that drivers/subsystems requires their device's
>> corresponding PM domains to be powered, to successfully complete a
>> ->probe() sequence.
>>
>> Align to the behavior above, by ensuring all PM domains are powered
>> prior initialization of a generic PM domain.
>>
>> Do note, since the generic PM domain will try to power off unused PM
>> domains at late_init, there are no increased power consumption over
>> time.
> 
> IMO "no increased power consumption" is a bit misleading because
> boot-time power consumption may have a major increase when all power
> domains are powered on.  Sure, they will eventually (hopefully) be
> turned off in after late_initcall, but there will still be an impact.
> 
> I guess the Samsung folks should comment here on whether that is
> significant.

Unfortunately this series has not been posted to linux-samsung-soc
mailing list and I'm a formerly-Samsung folk now, so it hasn't really
reached there.

Best regards,
Tomasz

Re: [PATCH 2/4] ARM: exynos: Ensure PM domains are powered at initialization

[Ulf Hansson]

On 1 October 2014 13:23, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> On 30.09.2014 20:33, Kevin Hilman wrote:
>> Ulf Hansson <ulf.hansson@linaro.org> writes:
>>
>>> At ->probe() it's common practice for drivers/subsystems to bring their
>>> devices to full power and without depending on CONFIG_PM_RUNTIME.
>>>
>>> We could also expect that drivers/subsystems requires their device's
>>> corresponding PM domains to be powered, to successfully complete a
>>> ->probe() sequence.
>>>
>>> Align to the behavior above, by ensuring all PM domains are powered
>>> prior initialization of a generic PM domain.
>>>
>>> Do note, since the generic PM domain will try to power off unused PM
>>> domains at late_init, there are no increased power consumption over
>>> time.
>>
>> IMO "no increased power consumption" is a bit misleading because
>> boot-time power consumption may have a major increase when all power
>> domains are powered on.  Sure, they will eventually (hopefully) be
>> turned off in after late_initcall, but there will still be an impact.
>>
>> I guess the Samsung folks should comment here on whether that is
>> significant.
>
> Unfortunately this series has not been posted to linux-samsung-soc
> mailing list and I'm a formerly-Samsung folk now, so it hasn't really
> reached there.
>

Ohh, thanks for pointing this out. I will resend it.

Kind regards
Uffe

[PATCH 3/4] PM / Domains: Expect PM domains being powered at initialization

[Ulf Hansson]

At ->probe() it's common practice for drivers/subsystems to bring their
devices to full power and without depending on CONFIG_PM_RUNTIME.

We could also expect that drivers/subsystems requires their device's
corresponding PM domains to be powered, to successfully complete a
->probe() sequence.

Align the generic PM domain to the behavior above, by enforcing a PM
domain to be powered at initialization. Since all callers of
pm_genpd_init() already provides "false" as the value for the "is_off"
parameter, let's make it clear by removing this option.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 arch/arm/mach-exynos/pm_domains.c   | 2 +-
 arch/arm/mach-s3c64xx/pm.c          | 4 ++--
 arch/arm/mach-shmobile/pm-r8a7779.c | 2 +-
 arch/arm/mach-shmobile/pm-rmobile.c | 2 +-
 drivers/base/power/domain.c         | 5 ++---
 include/linux/pm_domain.h           | 4 ++--
 6 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-exynos/pm_domains.c b/arch/arm/mach-exynos/pm_domains.c
index 58e18e9..f2e5096 100644
--- a/arch/arm/mach-exynos/pm_domains.c
+++ b/arch/arm/mach-exynos/pm_domains.c
@@ -158,7 +158,7 @@ no_clk:
 		if (!(__raw_readl(pd->base + 0x4) & INT_LOCAL_PWR_EN))
 			exynos_pd_power_on(&pd->pd);
 
-		pm_genpd_init(&pd->pd, NULL, false);
+		pm_genpd_init(&pd->pd, NULL);
 		of_genpd_add_provider_simple(np, &pd->pd);
 	}
 
diff --git a/arch/arm/mach-s3c64xx/pm.c b/arch/arm/mach-s3c64xx/pm.c
index aaf7bea..42dead0 100644
--- a/arch/arm/mach-s3c64xx/pm.c
+++ b/arch/arm/mach-s3c64xx/pm.c
@@ -315,10 +315,10 @@ int __init s3c64xx_pm_init(void)
 
 	for (i = 0; i < ARRAY_SIZE(s3c64xx_always_on_pm_domains); i++)
 		pm_genpd_init(&s3c64xx_always_on_pm_domains[i]->pd,
-			      &pm_domain_always_on_gov, false);
+			      &pm_domain_always_on_gov);
 
 	for (i = 0; i < ARRAY_SIZE(s3c64xx_pm_domains); i++)
-		pm_genpd_init(&s3c64xx_pm_domains[i]->pd, NULL, false);
+		pm_genpd_init(&s3c64xx_pm_domains[i]->pd, NULL);
 
 #ifdef CONFIG_S3C_DEV_FB
 	if (dev_get_platdata(&s3c_device_fb.dev))
diff --git a/arch/arm/mach-shmobile/pm-r8a7779.c b/arch/arm/mach-shmobile/pm-r8a7779.c
index 82fe3d7..c20ef44 100644
--- a/arch/arm/mach-shmobile/pm-r8a7779.c
+++ b/arch/arm/mach-shmobile/pm-r8a7779.c
@@ -83,7 +83,7 @@ static void r8a7779_init_pm_domain(struct r8a7779_pm_domain *r8a7779_pd)
 {
 	struct generic_pm_domain *genpd = &r8a7779_pd->genpd;
 
-	pm_genpd_init(genpd, NULL, false);
+	pm_genpd_init(genpd, NULL);
 	genpd->dev_ops.stop = pm_clk_suspend;
 	genpd->dev_ops.start = pm_clk_resume;
 	genpd->dev_ops.active_wakeup = pd_active_wakeup;
diff --git a/arch/arm/mach-shmobile/pm-rmobile.c b/arch/arm/mach-shmobile/pm-rmobile.c
index 818de2f..e6a0490 100644
--- a/arch/arm/mach-shmobile/pm-rmobile.c
+++ b/arch/arm/mach-shmobile/pm-rmobile.c
@@ -107,7 +107,7 @@ static void rmobile_init_pm_domain(struct rmobile_pm_domain *rmobile_pd)
 	struct generic_pm_domain *genpd = &rmobile_pd->genpd;
 	struct dev_power_governor *gov = rmobile_pd->gov;
 
-	pm_genpd_init(genpd, gov ? : &simple_qos_governor, false);
+	pm_genpd_init(genpd, gov ? : &simple_qos_governor);
 	genpd->dev_ops.stop		= pm_clk_suspend;
 	genpd->dev_ops.start		= pm_clk_resume;
 	genpd->dev_ops.active_wakeup	= rmobile_pd_active_wakeup;
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 36871b3..cfb76e8 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1836,10 +1836,9 @@ static int pm_genpd_default_restore_state(struct device *dev)
  * pm_genpd_init - Initialize a generic I/O PM domain object.
  * @genpd: PM domain object to initialize.
  * @gov: PM domain governor to associate with the domain (may be NULL).
- * @is_off: Initial value of the domain's power_is_off field.
  */
 void pm_genpd_init(struct generic_pm_domain *genpd,
-		   struct dev_power_governor *gov, bool is_off)
+		   struct dev_power_governor *gov)
 {
 	if (IS_ERR_OR_NULL(genpd))
 		return;
@@ -1852,7 +1851,7 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
 	INIT_WORK(&genpd->power_off_work, genpd_power_off_work_fn);
 	genpd->in_progress = 0;
 	atomic_set(&genpd->sd_count, 0);
-	genpd->status = is_off ? GPD_STATE_POWER_OFF : GPD_STATE_ACTIVE;
+	genpd->status = GPD_STATE_ACTIVE;
 	init_waitqueue_head(&genpd->status_wait_queue);
 	genpd->poweroff_task = NULL;
 	genpd->resume_count = 0;
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index a21dfa9..ad4aa87 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -140,7 +140,7 @@ extern int pm_genpd_name_attach_cpuidle(const char *name, int state);
 extern int pm_genpd_detach_cpuidle(struct generic_pm_domain *genpd);
 extern int pm_genpd_name_detach_cpuidle(const char *name);
 extern void pm_genpd_init(struct generic_pm_domain *genpd,
-			  struct dev_power_governor *gov, bool is_off);
+			  struct dev_power_governor *gov);
 
 extern int pm_genpd_poweron(struct generic_pm_domain *genpd);
 extern int pm_genpd_name_poweron(const char *domain_name);
@@ -206,7 +206,7 @@ static inline int pm_genpd_name_detach_cpuidle(const char *name)
 	return -ENOSYS;
 }
 static inline void pm_genpd_init(struct generic_pm_domain *genpd,
-				 struct dev_power_governor *gov, bool is_off)
+				 struct dev_power_governor *gov)
 {
 }
 static inline int pm_genpd_poweron(struct generic_pm_domain *genpd)
-- 
1.9.1

Re: [PATCH 3/4] PM / Domains: Expect PM domains being powered at initialization

[Geert Uytterhoeven]

On Tue, Sep 30, 2014 at 2:43 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> At ->probe() it's common practice for drivers/subsystems to bring their
> devices to full power and without depending on CONFIG_PM_RUNTIME.
>
> We could also expect that drivers/subsystems requires their device's
> corresponding PM domains to be powered, to successfully complete a
> ->probe() sequence.
>
> Align the generic PM domain to the behavior above, by enforcing a PM
> domain to be powered at initialization. Since all callers of
> pm_genpd_init() already provides "false" as the value for the "is_off"
> parameter, let's make it clear by removing this option.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 3/4] PM / Domains: Expect PM domains being powered at initialization

[Kevin Hilman]

Ulf Hansson <ulf.hansson@linaro.org> writes:

> At ->probe() it's common practice for drivers/subsystems to bring their
> devices to full power and without depending on CONFIG_PM_RUNTIME.

If they're not using pm_runtime to bring the device to full power,
shouldn't these drivers be using pm_runtime_set_active()?  (or maybe
pm_runtime_force_resume()?)

Wouldn't using those force the genpd to stay powered on?

Kevin

Re: [PATCH 3/4] PM / Domains: Expect PM domains being powered at initialization

[Ulf Hansson]

On 30 September 2014 20:21, Kevin Hilman <khilman@kernel.org> wrote:
> Ulf Hansson <ulf.hansson@linaro.org> writes:
>
>> At ->probe() it's common practice for drivers/subsystems to bring their
>> devices to full power and without depending on CONFIG_PM_RUNTIME.
>
> If they're not using pm_runtime to bring the device to full power,
> shouldn't these drivers be using pm_runtime_set_active()?  (or maybe
> pm_runtime_force_resume()?)

Since drivers needs to cope with combinations of
CONFIG_PM_|RUNTIME|SLEEP a common practice it to follow below
sequence:

1. Fetch resources. (Clocks, regulators, pinctrls, irqs, etc)
2. Enable resources etc.
...
3. pm_runtime_get_noresume()
4. pm_runtime_set_active()
5. pm_runtime_enable()
....
6. pm_runtime_put()

Obviously there are variations, but the idea is the same; Use
pm_runtime_set_active() to reflect that the current state is active,
then schedule a runtime PM idle request before leaving ->probe().

>
> Wouldn't using those force the genpd to stay powered on?

pm_runtime_set_active() - no.
pm_runtime_force_resume() - yes, but with some side effects.

Kind regards
Uffe

Re: [PATCH 3/4] PM / Domains: Expect PM domains being powered at initialization

[Kevin Hilman]

Ulf Hansson <ulf.hansson@linaro.org> writes:

> At ->probe() it's common practice for drivers/subsystems to bring their
> devices to full power and without depending on CONFIG_PM_RUNTIME.
>
> We could also expect that drivers/subsystems requires their device's
> corresponding PM domains to be powered, to successfully complete a
> ->probe() sequence.
>
> Align the generic PM domain to the behavior above, by enforcing a PM
> domain to be powered at initialization. Since all callers of
> pm_genpd_init() already provides "false" as the value for the "is_off"
> parameter

This is only true after PATCH 2/4 is applied.

The exynos code currently checks the hw state of its power domains will
call pm_genpd_init() with is_off = true any hw domains that are off.

Kevin

Re: [PATCH 3/4] PM / Domains: Expect PM domains being powered at initialization

[Ulf Hansson]

On 30 September 2014 20:30, Kevin Hilman <khilman@kernel.org> wrote:
> Ulf Hansson <ulf.hansson@linaro.org> writes:
>
>> At ->probe() it's common practice for drivers/subsystems to bring their
>> devices to full power and without depending on CONFIG_PM_RUNTIME.
>>
>> We could also expect that drivers/subsystems requires their device's
>> corresponding PM domains to be powered, to successfully complete a
>> ->probe() sequence.
>>
>> Align the generic PM domain to the behavior above, by enforcing a PM
>> domain to be powered at initialization. Since all callers of
>> pm_genpd_init() already provides "false" as the value for the "is_off"
>> parameter
>
> This is only true after PATCH 2/4 is applied.

That's right. I could have been more clear about that in the commit message.

Do you want me to send a v2 to fix it?

Kind regards
Uffe

>
> The exynos code currently checks the hw state of its power domains will
> call pm_genpd_init() with is_off = true any hw domains that are off.

>
> Kevin

[PATCH 4/4] PM / Domains: Enforce PM domains to stay powered during boot

[Ulf Hansson]

When there are more than one device in a PM domain these will obviously
be probed at different times. Depending on timing and the implemented
support for runtime PM in a driver/subsystem, genpd may be advised to
power off a PM domain after a successful probe sequence.

Ideally we should have relied on the driver/subsystem, through runtime
PM, to bring their device's PM domain into powered state prior doing
probing if such requirement exist.

Since this is not a common practice by drivers/subsystems, enforcing
such a change doesn't seem viable.

Instead, let's improve the situation, by preventing genpd from powering
off any of the PM domains until late_init. At that point genpd already
tries to power off unused PM domains, so it seems like a decent match.

Cases which can't be covered within the window of until late_init needs
to be adressed separately and likely through a more common long term
solution.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/domain.c | 17 ++++++++++++++++-
 include/linux/pm_domain.h   |  1 +
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index cfb76e8..3dbadfd 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -451,10 +451,12 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd)
 	 * (2) The domain is waiting for its master to power up.
 	 * (3) One of the domain's devices is being resumed right now.
 	 * (4) System suspend is in progress.
+	 * (5) late_init hasn't completed to allow it.
 	 */
 	if (genpd->status == GPD_STATE_POWER_OFF
 	    || genpd->status == GPD_STATE_WAIT_MASTER
-	    || genpd->resume_count > 0 || genpd->prepared_count > 0)
+	    || genpd->resume_count > 0 || genpd->prepared_count > 0
+	    || genpd->keep_power)
 		return 0;
 
 	if (atomic_read(&genpd->sd_count) > 0)
@@ -724,6 +726,18 @@ void pm_genpd_poweroff_unused(void)
 
 static int __init genpd_poweroff_unused(void)
 {
+	struct generic_pm_domain *genpd;
+
+	mutex_lock(&gpd_list_lock);
+
+	list_for_each_entry(genpd, &gpd_list, gpd_list_node) {
+		genpd_acquire_lock(genpd);
+		genpd->keep_power = false;
+		genpd_release_lock(genpd);
+	}
+
+	mutex_unlock(&gpd_list_lock);
+
 	pm_genpd_poweroff_unused();
 	return 0;
 }
@@ -1854,6 +1868,7 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
 	genpd->status = GPD_STATE_ACTIVE;
 	init_waitqueue_head(&genpd->status_wait_queue);
 	genpd->poweroff_task = NULL;
+	genpd->keep_power = true;
 	genpd->resume_count = 0;
 	genpd->device_count = 0;
 	genpd->max_off_time_ns = -1;
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index ad4aa87..d87ef6a 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -58,6 +58,7 @@ struct generic_pm_domain {
 	enum gpd_status status;	/* Current state of the domain */
 	wait_queue_head_t status_wait_queue;
 	struct task_struct *poweroff_task;	/* Powering off task */
+	bool keep_power;		/* Flag to keep power until late_init */
 	unsigned int resume_count;	/* Number of devices being resumed */
 	unsigned int device_count;	/* Number of devices */
 	unsigned int suspended_count;	/* System suspend device counter */
-- 
1.9.1

Re: [PATCH 0/4] PM / Domains: Fix race conditions during boot

[Rafael J. Wysocki]

On Tuesday, September 30, 2014 02:43:49 PM Ulf Hansson wrote:
> When there are more than one device in a PM domain these will obviously
> be probed at different times. Depending on timing and the implemented
> support for runtime PM in a driver/subsystem, genpd may be advised to
> power off a PM domain after a successful probe sequence.
>     
> Ideally we should have relied on the driver/subsystem, through runtime
> PM, to bring their device's PM domain into powered state prior doing
> probing if such requirement exist.
>     
> Since this is not a common practice by drivers/subsystems, enforcing
> such a change doesn't seem viable.
>     
> Instead, let's improve the situation, by preventing genpd from powering
> off any of the PM domains until late_init. At that point genpd already
> tries to power off unused PM domains, so it seems like a decent match.
>     
> Cases which can't be covered within the window of until late_init needs
> to be adressed separately and likely through a more common long term
> solution.

The history here is that we started with something like what the patches
do and then realized that on some systems we don't want certain power domains
to be powered on even for a while (for reasons I can't recall today,
unfortunately).  So this may not be the case any more, but I'm not actually
sure and it would be good to check.

Also I'm pretty sure we'd like to do the opposite (that is, leave things
in low-power states until we need to access them) going forward, so I'm
seeing this as a step back.

What's the real motivation here?

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 0/4] PM / Domains: Fix race conditions during boot

[Ulf Hansson]

On 30 September 2014 22:08, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Tuesday, September 30, 2014 02:43:49 PM Ulf Hansson wrote:
>> When there are more than one device in a PM domain these will obviously
>> be probed at different times. Depending on timing and the implemented
>> support for runtime PM in a driver/subsystem, genpd may be advised to
>> power off a PM domain after a successful probe sequence.
>>
>> Ideally we should have relied on the driver/subsystem, through runtime
>> PM, to bring their device's PM domain into powered state prior doing
>> probing if such requirement exist.
>>
>> Since this is not a common practice by drivers/subsystems, enforcing
>> such a change doesn't seem viable.
>>
>> Instead, let's improve the situation, by preventing genpd from powering
>> off any of the PM domains until late_init. At that point genpd already
>> tries to power off unused PM domains, so it seems like a decent match.
>>
>> Cases which can't be covered within the window of until late_init needs
>> to be adressed separately and likely through a more common long term
>> solution.
>
> The history here is that we started with something like what the patches
> do and then realized that on some systems we don't want certain power domains
> to be powered on even for a while (for reasons I can't recall today,
> unfortunately).  So this may not be the case any more, but I'm not actually
> sure and it would be good to check.
>
> Also I'm pretty sure we'd like to do the opposite (that is, leave things
> in low-power states until we need to access them) going forward, so I'm
> seeing this as a step back.

I agree that this would be the best set up and this is my intent to
have genpd support in the long run. But, today, genpd together with
how drivers/subsystems handles ->probe() just doesn't work. We need to
fix that first.

>
> What's the real motivation here?

Let me try to summarize the arguments.

1) Even if CONFIG_PM_RUNTIME is enabled, we have no mechanism to use
for drivers/subsystems to bring device's PM domains into powered state
before doing ->probe(). Well, of course, they could use
pm_runtime_get_sync() in that path, but that's just not the common
practice and there are good reasons to why.

So, even if we figured out a solution for drivers/subsystems to
support the above, it's not reasonable to think we can adopt each of
them shortly. We need a fix in genpd.

2) For !CONFIG_PM_RUNTIME, drivers/subsystem are completely unable to
bring a PM domain into powered state. The only solution is to make
sure the PM domain gets initialized in powered state.

Kind regards
Uffe

Re: [PATCH 0/4] PM / Domains: Fix race conditions during boot

[Rafael J. Wysocki]

On Wednesday, October 01, 2014 09:35:45 AM Ulf Hansson wrote:
> On 30 September 2014 22:08, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> > On Tuesday, September 30, 2014 02:43:49 PM Ulf Hansson wrote:
> >> When there are more than one device in a PM domain these will obviously
> >> be probed at different times. Depending on timing and the implemented
> >> support for runtime PM in a driver/subsystem, genpd may be advised to
> >> power off a PM domain after a successful probe sequence.
> >>
> >> Ideally we should have relied on the driver/subsystem, through runtime
> >> PM, to bring their device's PM domain into powered state prior doing
> >> probing if such requirement exist.
> >>
> >> Since this is not a common practice by drivers/subsystems, enforcing
> >> such a change doesn't seem viable.
> >>
> >> Instead, let's improve the situation, by preventing genpd from powering
> >> off any of the PM domains until late_init. At that point genpd already
> >> tries to power off unused PM domains, so it seems like a decent match.
> >>
> >> Cases which can't be covered within the window of until late_init needs
> >> to be adressed separately and likely through a more common long term
> >> solution.
> >
> > The history here is that we started with something like what the patches
> > do and then realized that on some systems we don't want certain power domains
> > to be powered on even for a while (for reasons I can't recall today,
> > unfortunately).  So this may not be the case any more, but I'm not actually
> > sure and it would be good to check.
> >
> > Also I'm pretty sure we'd like to do the opposite (that is, leave things
> > in low-power states until we need to access them) going forward, so I'm
> > seeing this as a step back.
> 
> I agree that this would be the best set up and this is my intent to
> have genpd support in the long run. But, today, genpd together with
> how drivers/subsystems handles ->probe() just doesn't work. We need to
> fix that first.
> 
> >
> > What's the real motivation here?
> 
> Let me try to summarize the arguments.
> 
> 1) Even if CONFIG_PM_RUNTIME is enabled, we have no mechanism to use
> for drivers/subsystems to bring device's PM domains into powered state
> before doing ->probe(). Well, of course, they could use
> pm_runtime_get_sync() in that path, but that's just not the common
> practice and there are good reasons to why.
> 
> So, even if we figured out a solution for drivers/subsystems to
> support the above, it's not reasonable to think we can adopt each of
> them shortly. We need a fix in genpd.

My concern is that if we change the default behavior today, people will see
power regressions on some platforms.  Surely, power regressions are not as
bad as functional regressions, but we can't say we don't care.

Did you consider adding some handling of that to driver_probe_device()?
We do a barrier in there, but conceivably we could do more if a PM domain
is attached.

> 2) For !CONFIG_PM_RUNTIME, drivers/subsystem are completely unable to
> bring a PM domain into powered state. The only solution is to make
> sure the PM domain gets initialized in powered state.

That is a special case and it should be addressed as such in my view.

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 0/4] PM / Domains: Fix race conditions during boot

[Ulf Hansson]

On 1 October 2014 22:31, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Wednesday, October 01, 2014 09:35:45 AM Ulf Hansson wrote:
>> On 30 September 2014 22:08, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>> > On Tuesday, September 30, 2014 02:43:49 PM Ulf Hansson wrote:
>> >> When there are more than one device in a PM domain these will obviously
>> >> be probed at different times. Depending on timing and the implemented
>> >> support for runtime PM in a driver/subsystem, genpd may be advised to
>> >> power off a PM domain after a successful probe sequence.
>> >>
>> >> Ideally we should have relied on the driver/subsystem, through runtime
>> >> PM, to bring their device's PM domain into powered state prior doing
>> >> probing if such requirement exist.
>> >>
>> >> Since this is not a common practice by drivers/subsystems, enforcing
>> >> such a change doesn't seem viable.
>> >>
>> >> Instead, let's improve the situation, by preventing genpd from powering
>> >> off any of the PM domains until late_init. At that point genpd already
>> >> tries to power off unused PM domains, so it seems like a decent match.
>> >>
>> >> Cases which can't be covered within the window of until late_init needs
>> >> to be adressed separately and likely through a more common long term
>> >> solution.
>> >
>> > The history here is that we started with something like what the patches
>> > do and then realized that on some systems we don't want certain power domains
>> > to be powered on even for a while (for reasons I can't recall today,
>> > unfortunately).  So this may not be the case any more, but I'm not actually
>> > sure and it would be good to check.
>> >
>> > Also I'm pretty sure we'd like to do the opposite (that is, leave things
>> > in low-power states until we need to access them) going forward, so I'm
>> > seeing this as a step back.
>>
>> I agree that this would be the best set up and this is my intent to
>> have genpd support in the long run. But, today, genpd together with
>> how drivers/subsystems handles ->probe() just doesn't work. We need to
>> fix that first.
>>
>> >
>> > What's the real motivation here?
>>
>> Let me try to summarize the arguments.
>>
>> 1) Even if CONFIG_PM_RUNTIME is enabled, we have no mechanism to use
>> for drivers/subsystems to bring device's PM domains into powered state
>> before doing ->probe(). Well, of course, they could use
>> pm_runtime_get_sync() in that path, but that's just not the common
>> practice and there are good reasons to why.
>>
>> So, even if we figured out a solution for drivers/subsystems to
>> support the above, it's not reasonable to think we can adopt each of
>> them shortly. We need a fix in genpd.
>
> My concern is that if we change the default behavior today, people will see
> power regressions on some platforms.  Surely, power regressions are not as
> bad as functional regressions, but we can't say we don't care.

The power regressions are only present during boot, until late_init.
Is that something we should worry about in this phase?

I am not saying that we shouldn't care, but I believe we need to fix
the functional problems first.

Currently we have drivers that succeeds to probe their devices by pure
luck, and some that don't. Those that don't seems to rely on using
pm_genpd_dev_need_restore(). We need to stop continue supporting this
crazy behaviour.

>
> Did you consider adding some handling of that to driver_probe_device()?
> We do a barrier in there, but conceivably we could do more if a PM domain
> is attached.
>
>> 2) For !CONFIG_PM_RUNTIME, drivers/subsystem are completely unable to
>> bring a PM domain into powered state. The only solution is to make
>> sure the PM domain gets initialized in powered state.
>
> That is a special case and it should be addressed as such in my view.

Even if it is a somewhat special case, it's perfectly fine to build
genpd with CONFIG_PM_SLEEP and !CONFIG_PM_RUNTIME. The same applies
for many different SoCs - which then will be broken, unless the make
sure to have powered PM domains at pm_genpd_init().

Kind regards
Uffe

Re: [PATCH 0/4] PM / Domains: Fix race conditions during boot

[Ulf Hansson]

[...]

>> 1) Even if CONFIG_PM_RUNTIME is enabled, we have no mechanism to use
>> for drivers/subsystems to bring device's PM domains into powered state
>> before doing ->probe(). Well, of course, they could use
>> pm_runtime_get_sync() in that path, but that's just not the common
>> practice and there are good reasons to why.
>>
>> So, even if we figured out a solution for drivers/subsystems to
>> support the above, it's not reasonable to think we can adopt each of
>> them shortly. We need a fix in genpd.
>
> My concern is that if we change the default behavior today, people will see
> power regressions on some platforms.  Surely, power regressions are not as
> bad as functional regressions, but we can't say we don't care.
>
> Did you consider adding some handling of that to driver_probe_device()?
> We do a barrier in there, but conceivably we could do more if a PM domain
> is attached.

Forgot to comment on this.

I think the problem with such approach would be that the PM domain
hasn't yet been attached to the device, since that happens at
bus->probe().

Maybe we could find a solution which only affect the buses though and
not the drivers. But I need to think a bit more on this, before I can
post a proposal.

Kind regards
Uffe