| Message ID | 1412081033-8136-4-git-send-email-ulf.hansson@linaro.org |
|---|---|
| State | New |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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)
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(-)