| Message ID | 1363351143-28490-1-git-send-email-rickard.andersson@stericsson.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 15 March 2013 13:39, Rickard Andersson <rickard.andersson@stericsson.com> wrote: > Some drivers needs to restore their context directly > when a power domain is activated. For example a driver > handling system bus settings must be able to restore > context before the bus is being used for the first time. > Other examples could be clock settings hardware blocks and > special debugging hardware blocks which should be restored > as early as possible in order to be able to debug direcly > from waking up. Typically these notifers are needed in > systems with CPU coupled power domains. The notifiers > are intended to be trigged by cpuidle driver or the > suspend ops hooks. > > The drivers that needs to use these notifiers are > some special cases. Most drivers should not use this > method and instead control their context via the > pm_runtime interface. > > Signed-off-by: Rickard Andersson <rickard.andersson@stericsson.com> > --- > drivers/base/power/domain.c | 64 +++++++++++++++++++++++++++++++++++++++++++++ > include/linux/pm_domain.h | 22 ++++++++++++++++ > 2 files changed, 86 insertions(+) > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > index 9a6b05a..16fbc52 100644 > --- a/drivers/base/power/domain.c > +++ b/drivers/base/power/domain.c > @@ -1955,6 +1955,69 @@ int pm_genpd_name_detach_cpuidle(const char *name) > return pm_genpd_detach_cpuidle(pm_genpd_lookup_name(name)); > } > > +/** > + * pm_genpd_register_on_off_notifier - Register to early power on / > + * late power off notifications. > + * Only use this function if absolutly needed. It is only intended to be > + * used for power domains that are coupled with the CPU, that is, power > + * domains being controlled from cpuidle and the platform suspend ops hooks. > + * Also only devices that needs their context to be restored directly when > + * leaving a sleep state should use this method. Most devices should be > + * fine handling their context and power domains via pm_runtime. > + * @dev: Device to register. > + * @nb: Notifier block to be registered. > + */ > +int pm_genpd_register_on_off_notifier(struct device *dev, > + struct notifier_block *nb) > +{ > + struct generic_pm_domain *genpd; > + > + dev_dbg(dev, "%s()\n", __func__); > + > + genpd = dev_to_genpd(dev); > + > + return atomic_notifier_chain_register(&genpd->on_off_notifier, nb); > +} > + > +/** > + * pm_genpd_unregister_on_off_notifier - Unregister to early power on / > + * late power off notification. > + * @dev: Device to unregister. > + * @nb: Notifier block to be unregistered. > + */ > +int pm_genpd_unregister_on_off_notifier(struct device *dev, > + struct notifier_block *nb) > +{ > + struct generic_pm_domain *genpd; > + > + dev_dbg(dev, "%s()\n", __func__); > + > + genpd = dev_to_genpd(dev); > + > + return atomic_notifier_chain_unregister(&genpd->on_off_notifier, nb); > +} > + > +/** > + * pm_genpd_notify_power_on_off - Notify that the CPU coupled power > + * domain is going to be powered off or has been powered on. > + * Intended users of this function are cpuidle drivers and the platform > + * suspend operations implementation. > + * @genpd: pm domain that will change state. > + * @nb: true = has been powered on, false = will power off. > + */ > +int pm_genpd_notify_power_on_off(struct generic_pm_domain *genpd, > + bool power_on_off) > +{ > + if (IS_ERR_OR_NULL(genpd)) > + return -EINVAL; > + > + atomic_notifier_call_chain((&genpd->on_off_notifier), > + power_on_off, NULL); > + > + return 0; > +} > + > + > /* Default device callbacks for generic PM domains. */ > > /** > @@ -2136,6 +2199,7 @@ void pm_genpd_init(struct generic_pm_domain *genpd, > atomic_set(&genpd->sd_count, 0); > genpd->status = is_off ? GPD_STATE_POWER_OFF : GPD_STATE_ACTIVE; > init_waitqueue_head(&genpd->status_wait_queue); > + ATOMIC_INIT_NOTIFIER_HEAD(&genpd->on_off_notifier); > genpd->poweroff_task = NULL; > genpd->resume_count = 0; > genpd->device_count = 0; > diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h > index 7c1d252..a0a9cf7 100644 > --- a/include/linux/pm_domain.h > +++ b/include/linux/pm_domain.h > @@ -82,6 +82,7 @@ struct generic_pm_domain { > bool cached_power_down_ok; > struct device_node *of_node; /* Node in device tree */ > struct gpd_cpu_data *cpu_data; > + struct atomic_notifier_head on_off_notifier; > }; > > static inline struct generic_pm_domain *pd_to_genpd(struct dev_pm_domain *pd) > @@ -159,6 +160,12 @@ extern int pm_genpd_attach_cpuidle(struct generic_pm_domain *genpd, int state); > 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 int pm_genpd_register_on_off_notifier(struct device *dev, > + struct notifier_block *nb); > +int pm_genpd_unregister_on_off_notifier(struct device *dev, > + struct notifier_block *nb); > +int pm_genpd_notify_power_on_off(struct generic_pm_domain *genpd, > + bool power_on_off); > extern void pm_genpd_init(struct generic_pm_domain *genpd, > struct dev_power_governor *gov, bool is_off); > > @@ -243,6 +250,21 @@ static inline int pm_genpd_name_detach_cpuidle(const char *name) > { > return -ENOSYS; > } > +static inline int pm_genpd_register_on_off_notifier(struct device *dev, > + struct notifier_block *nb) > +{ > + return -ENOSYS; > +} > +int pm_genpd_unregister_on_off_notifier(struct device *dev, > + struct notifier_block *nb) > +{ > + return -ENOSYS; > +} > +int pm_genpd_notify_power_on_off(struct generic_pm_domain *genpd, > + bool power_on_off) > +{ > + return -ENOSYS; > +} > static inline void pm_genpd_init(struct generic_pm_domain *genpd, > struct dev_power_governor *gov, bool is_off) > { > -- > 1.8.0 > > -- > 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 This makes sense to me. Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> -- 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
Hi Rickard, Rickard Andersson <rickard.andersson@stericsson.com> writes: > Some drivers needs to restore their context directly > when a power domain is activated. For example a driver > handling system bus settings must be able to restore > context before the bus is being used for the first time. > Other examples could be clock settings hardware blocks and > special debugging hardware blocks which should be restored > as early as possible in order to be able to debug direcly > from waking up. Typically these notifers are needed in > systems with CPU coupled power domains. The notifiers > are intended to be trigged by cpuidle driver or the > suspend ops hooks. > > The drivers that needs to use these notifiers are > some special cases. Most drivers should not use this > method and instead control their context via the > pm_runtime interface. > > Signed-off-by: Rickard Andersson <rickard.andersson@stericsson.com> Some general comments. First, I don't see where the notifiers are actually called in this patch, so their intended use is not entirely clear. Second, I'm a bit reluctant to see another layer of callbacks added to the drivers. I think using notifiers for this adds another level of complexity to drivers that already have a pile of PM related callbacks to manage. IMO, it would be helpful to see an example driver using this feature, and how it would interact with the drivers runtime PM callbacks. For example, I suspect that the the notifier callback would bascially be the same as the runtime_resume callback, right? Stated differently, at first glance, it seems the feature needed is "runtime resume immidately after genpd power on". Is that correct? Also, the changelog implies that ordering may be crucial here (e.g bus settings restored before bus is accessed), and using notifiers is not going to give you that kind of ordering guarantee. 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
Hi Kevin, On 03/18/2013 08:14 PM, Kevin Hilman wrote: > Hi Rickard, > > Rickard Andersson<rickard.andersson@stericsson.com> writes: > >> Some drivers needs to restore their context directly >> when a power domain is activated. For example a driver >> handling system bus settings must be able to restore >> context before the bus is being used for the first time. >> Other examples could be clock settings hardware blocks and >> special debugging hardware blocks which should be restored >> as early as possible in order to be able to debug direcly >> from waking up. Typically these notifers are needed in >> systems with CPU coupled power domains. The notifiers >> are intended to be trigged by cpuidle driver or the >> suspend ops hooks. >> >> The drivers that needs to use these notifiers are >> some special cases. Most drivers should not use this >> method and instead control their context via the >> pm_runtime interface. >> >> Signed-off-by: Rickard Andersson<rickard.andersson@stericsson.com> > Some general comments. > > First, I don't see where the notifiers are actually called in this > patch, so their intended use is not entirely clear. I can send a patch of how the notifiers are trigged and used. > Second, I'm a bit reluctant to see another layer of callbacks added to > the drivers. I think using notifiers for this adds another level of > complexity to drivers that already have a pile of PM related callbacks > to manage. > I understand your point but these callback should only be used when really needed. For example one can compare with that drivers today can register to cpufreq transition callbacks but most drivers does not need to do that. > IMO, it would be helpful to see an example driver using this feature, > and how it would interact with the drivers runtime PM callbacks. For > example, I suspect that the the notifier callback would bascially be the > same as the runtime_resume callback, right? Yes, they will do similar things. > Stated differently, at first glance, it seems the feature needed is > "runtime resume immidately after genpd power on". Is that correct? Yes, and "runtime suspend just before power off", but only trigged from cpuidle or platform suspend ops. But there are differences, for example the devices should be able to get the notifier call in the platform suspend ops. Normally pm_runtime is disabled during system suspend if I understand correct. > Also, the changelog implies that ordering may be crucial here (e.g bus > settings restored before bus is accessed), and using notifiers is not > going to give you that kind of ordering guarantee. If the drivers are probed in the right order and the drivers registers to the notifiers in the probe function then it will work fine. One can think of my notifiers patch as something similar as /kernel/cpu_pm.c but for a genpd CPU coupled domain instead of the CPU power domains. It would probably be possible to call the drivers runtime_resume callbacks for specially registered "runtime resume immediately" devices. (It is comparable to try to use runtime_resume for restoring context of for example the GIC instead of using /kernel/cpu_pm.c). I am unsure which solution is best. BR Rickard -- 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/drivers/base/power/domain.c b/drivers/base/power/domain.c index 9a6b05a..16fbc52 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -1955,6 +1955,69 @@ int pm_genpd_name_detach_cpuidle(const char *name) return pm_genpd_detach_cpuidle(pm_genpd_lookup_name(name)); } +/** + * pm_genpd_register_on_off_notifier - Register to early power on / + * late power off notifications. + * Only use this function if absolutly needed. It is only intended to be + * used for power domains that are coupled with the CPU, that is, power + * domains being controlled from cpuidle and the platform suspend ops hooks. + * Also only devices that needs their context to be restored directly when + * leaving a sleep state should use this method. Most devices should be + * fine handling their context and power domains via pm_runtime. + * @dev: Device to register. + * @nb: Notifier block to be registered. + */ +int pm_genpd_register_on_off_notifier(struct device *dev, + struct notifier_block *nb) +{ + struct generic_pm_domain *genpd; + + dev_dbg(dev, "%s()\n", __func__); + + genpd = dev_to_genpd(dev); + + return atomic_notifier_chain_register(&genpd->on_off_notifier, nb); +} + +/** + * pm_genpd_unregister_on_off_notifier - Unregister to early power on / + * late power off notification. + * @dev: Device to unregister. + * @nb: Notifier block to be unregistered. + */ +int pm_genpd_unregister_on_off_notifier(struct device *dev, + struct notifier_block *nb) +{ + struct generic_pm_domain *genpd; + + dev_dbg(dev, "%s()\n", __func__); + + genpd = dev_to_genpd(dev); + + return atomic_notifier_chain_unregister(&genpd->on_off_notifier, nb); +} + +/** + * pm_genpd_notify_power_on_off - Notify that the CPU coupled power + * domain is going to be powered off or has been powered on. + * Intended users of this function are cpuidle drivers and the platform + * suspend operations implementation. + * @genpd: pm domain that will change state. + * @nb: true = has been powered on, false = will power off. + */ +int pm_genpd_notify_power_on_off(struct generic_pm_domain *genpd, + bool power_on_off) +{ + if (IS_ERR_OR_NULL(genpd)) + return -EINVAL; + + atomic_notifier_call_chain((&genpd->on_off_notifier), + power_on_off, NULL); + + return 0; +} + + /* Default device callbacks for generic PM domains. */ /** @@ -2136,6 +2199,7 @@ void pm_genpd_init(struct generic_pm_domain *genpd, atomic_set(&genpd->sd_count, 0); genpd->status = is_off ? GPD_STATE_POWER_OFF : GPD_STATE_ACTIVE; init_waitqueue_head(&genpd->status_wait_queue); + ATOMIC_INIT_NOTIFIER_HEAD(&genpd->on_off_notifier); genpd->poweroff_task = NULL; genpd->resume_count = 0; genpd->device_count = 0; diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h index 7c1d252..a0a9cf7 100644 --- a/include/linux/pm_domain.h +++ b/include/linux/pm_domain.h @@ -82,6 +82,7 @@ struct generic_pm_domain { bool cached_power_down_ok; struct device_node *of_node; /* Node in device tree */ struct gpd_cpu_data *cpu_data; + struct atomic_notifier_head on_off_notifier; }; static inline struct generic_pm_domain *pd_to_genpd(struct dev_pm_domain *pd) @@ -159,6 +160,12 @@ extern int pm_genpd_attach_cpuidle(struct generic_pm_domain *genpd, int state); 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 int pm_genpd_register_on_off_notifier(struct device *dev, + struct notifier_block *nb); +int pm_genpd_unregister_on_off_notifier(struct device *dev, + struct notifier_block *nb); +int pm_genpd_notify_power_on_off(struct generic_pm_domain *genpd, + bool power_on_off); extern void pm_genpd_init(struct generic_pm_domain *genpd, struct dev_power_governor *gov, bool is_off); @@ -243,6 +250,21 @@ static inline int pm_genpd_name_detach_cpuidle(const char *name) { return -ENOSYS; } +static inline int pm_genpd_register_on_off_notifier(struct device *dev, + struct notifier_block *nb) +{ + return -ENOSYS; +} +int pm_genpd_unregister_on_off_notifier(struct device *dev, + struct notifier_block *nb) +{ + return -ENOSYS; +} +int pm_genpd_notify_power_on_off(struct generic_pm_domain *genpd, + bool power_on_off) +{ + return -ENOSYS; +} static inline void pm_genpd_init(struct generic_pm_domain *genpd, struct dev_power_governor *gov, bool is_off) {
Some drivers needs to restore their context directly when a power domain is activated. For example a driver handling system bus settings must be able to restore context before the bus is being used for the first time. Other examples could be clock settings hardware blocks and special debugging hardware blocks which should be restored as early as possible in order to be able to debug direcly from waking up. Typically these notifers are needed in systems with CPU coupled power domains. The notifiers are intended to be trigged by cpuidle driver or the suspend ops hooks. The drivers that needs to use these notifiers are some special cases. Most drivers should not use this method and instead control their context via the pm_runtime interface. Signed-off-by: Rickard Andersson <rickard.andersson@stericsson.com> --- drivers/base/power/domain.c | 64 +++++++++++++++++++++++++++++++++++++++++++++ include/linux/pm_domain.h | 22 ++++++++++++++++ 2 files changed, 86 insertions(+)