| Message ID | 20181003143824.13059-4-ulf.hansson@linaro.org (mailing list archive) |
|---|---|
| State | Mainlined |
| Delegated to: | Rafael Wysocki |
| Headers | show |
| Series | PM / Domains: Support hierarchical CPU arrangement (PSCI/ARM) (a subset) | expand |
Hi, * Ulf Hansson <ulf.hansson@linaro.org> [181003 14:43]: > + * GENPD_FLAG_IRQ_SAFE: This informs genpd that its backend callbacks, > + * ->power_on|off(), doesn't sleep. Hence, these > + * can be invoked from within atomic context, which > + * enables genpd to power on/off the PM domain, > + * even when pm_runtime_is_irq_safe() returns true, > + * for any of its attached devices. Note that, a > + * genpd having this flag set, requires its > + * masterdomains to also have it set. > + * Let's try to avoid adding more irq_safe stuff because of having that propagate to the masterdomains.. I think you can just flag the power_on/off in genpd, then have cpu_pm callbacks do it. Regards, Tony
On 4 October 2018 at 15:48, Tony Lindgren <tony@atomide.com> wrote: > Hi, > > * Ulf Hansson <ulf.hansson@linaro.org> [181003 14:43]: >> + * GENPD_FLAG_IRQ_SAFE: This informs genpd that its backend callbacks, >> + * ->power_on|off(), doesn't sleep. Hence, these >> + * can be invoked from within atomic context, which >> + * enables genpd to power on/off the PM domain, >> + * even when pm_runtime_is_irq_safe() returns true, >> + * for any of its attached devices. Note that, a >> + * genpd having this flag set, requires its >> + * masterdomains to also have it set. >> + * > > Let's try to avoid adding more irq_safe stuff because of having that > propagate to the masterdomains.. I am not sure I get your point. This is just documenting existing functionality in genpd, there is nothing new here. > > I think you can just flag the power_on/off in genpd, then have cpu_pm > callbacks do it. Not really sure what you propose, but feel free to send a patch. Do note, genpd has since the beginning of its time tried to cope with pm_runtime_irq_safe() devices. I admit it's quite complicated, however GENPD_FLAG_IRQ_SAFE greatly improved the support for such devices and their PM domains. Moreover, we need this functionality, in one way or the other, as long as there users of pm_runtime_irq_safe(). Kind regards Uffe
* Ulf Hansson <ulf.hansson@linaro.org> [181004 15:02]: > On 4 October 2018 at 15:48, Tony Lindgren <tony@atomide.com> wrote: > > Hi, > > > > * Ulf Hansson <ulf.hansson@linaro.org> [181003 14:43]: > >> + * GENPD_FLAG_IRQ_SAFE: This informs genpd that its backend callbacks, > >> + * ->power_on|off(), doesn't sleep. Hence, these > >> + * can be invoked from within atomic context, which > >> + * enables genpd to power on/off the PM domain, > >> + * even when pm_runtime_is_irq_safe() returns true, > >> + * for any of its attached devices. Note that, a > >> + * genpd having this flag set, requires its > >> + * masterdomains to also have it set. > >> + * > > > > Let's try to avoid adding more irq_safe stuff because of having that > > propagate to the masterdomains.. > > I am not sure I get your point. This is just documenting existing > functionality in genpd, there is nothing new here. Right, and I'm just bringing up that this FLAG_IRQ_SAFE is not a good way to in the long run :) > > I think you can just flag the power_on/off in genpd, then have cpu_pm > > callbacks do it. > > Not really sure what you propose, but feel free to send a patch. Well there is not much to really patch, just don't attempt to do irq_safe stuff from genpd and have cpu_idle callbacks to do it instead. And then no need for GENPD_FLAG_IRQ_SAFE :) > Do note, genpd has since the beginning of its time tried to cope with > pm_runtime_irq_safe() devices. I admit it's quite complicated, however > GENPD_FLAG_IRQ_SAFE greatly improved the support for such devices and > their PM domains. Moreover, we need this functionality, in one way or > the other, as long as there users of pm_runtime_irq_safe(). Right, and I'm still struggling years after with legacy device drivers that have done pm_runtime_irq_safe() and come to the conclusion that it should not be used at all. Getting rid of GENPD_FLAG_IRQ_SAFE might just safe you years of pain later on. Regards, Tony
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h index 776c546d581a..3b5d7280e52e 100644 --- a/include/linux/pm_domain.h +++ b/include/linux/pm_domain.h @@ -17,11 +17,36 @@ #include <linux/notifier.h> #include <linux/spinlock.h> -/* Defines used for the flags field in the struct generic_pm_domain */ -#define GENPD_FLAG_PM_CLK (1U << 0) /* PM domain uses PM clk */ -#define GENPD_FLAG_IRQ_SAFE (1U << 1) /* PM domain operates in atomic */ -#define GENPD_FLAG_ALWAYS_ON (1U << 2) /* PM domain is always powered on */ -#define GENPD_FLAG_ACTIVE_WAKEUP (1U << 3) /* Keep devices active if wakeup */ +/* + * Flags to control the behaviour of a genpd. + * + * These flags may be set in the struct generic_pm_domain's flags field by a + * genpd backend driver. The flags must be set before it calls pm_genpd_init(), + * which initializes a genpd. + * + * GENPD_FLAG_PM_CLK: Instructs genpd to use the PM clk framework, + * while powering on/off attached devices. + * + * GENPD_FLAG_IRQ_SAFE: This informs genpd that its backend callbacks, + * ->power_on|off(), doesn't sleep. Hence, these + * can be invoked from within atomic context, which + * enables genpd to power on/off the PM domain, + * even when pm_runtime_is_irq_safe() returns true, + * for any of its attached devices. Note that, a + * genpd having this flag set, requires its + * masterdomains to also have it set. + * + * GENPD_FLAG_ALWAYS_ON: Instructs genpd to always keep the PM domain + * powered on. + * + * GENPD_FLAG_ACTIVE_WAKEUP: Instructs genpd to keep the PM domain powered + * on, in case any of its attached devices is used + * in the wakeup path to serve system wakeups. + */ +#define GENPD_FLAG_PM_CLK (1U << 0) +#define GENPD_FLAG_IRQ_SAFE (1U << 1) +#define GENPD_FLAG_ALWAYS_ON (1U << 2) +#define GENPD_FLAG_ACTIVE_WAKEUP (1U << 3) enum gpd_status { GPD_STATE_ACTIVE = 0, /* PM domain is active */
The current documented description of the GENPD_FLAG_* flags, are too simplified, so let's extend them. Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> --- include/linux/pm_domain.h | 35 ++++++++++++++++++++++++++++++----- 1 file changed, 30 insertions(+), 5 deletions(-)