Re: [RFC PATCH 2/3] pmdomain: core: Add a genpd config to support unknown initial status
Ulf Hansson <[email protected]>
| Newsgroups | org.kernel.vger.linux-pm,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CAPx+jO8MXQwDh2Bvws4ngO=9LeE3YvFkiv=KDgYupWxS6VePjw@mail.gmail.com> |
On Wed, Aug 19, 2026 at 7:40 PM Abel Vesa <[email protected]> wrote: > > On 26-08-19 17:14:52, Ulf Hansson wrote: > > From: Ulf Hansson <[email protected]> > > > > It's not always possible for a genpd provider to know the initial status > > for its corresponding PM domain(s). To register the PM domain in a safe > > state, the genpd provider driver may therefore have to initialize the > > genpd's status to be powered off, as a way to prevent the PM domain from > > being used when it actually could be powered off. > > > > In these cases we may end up to power on a PM domain through the genpd > > subsystem, while from the HW point of view it may already be powered on. > > Under these circumstances, it may also be required to keep the PM domain > > powered on until all the consumer devices have been probed. In other words, > > until the ->sync_state() callback for the genpd provider in question have > > been called. > > > > To support this behaviour for a genpd provider, let's introduce a new genpd > > configuration, GENPD_FLAG_OFF_STAY_ON. > > > > Reported-by: Maulik Shah <[email protected]> > > Link: https://lore.kernel.org/all/[email protected]/ > > Signed-off-by: Ulf Hansson <[email protected]> > > --- > > drivers/pmdomain/core.c | 6 +++++- > > include/linux/pm_domain.h | 9 +++++++++ > > 2 files changed, 14 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c > > index 7345c06f1d55..be4fff2f4664 100644 > > --- a/drivers/pmdomain/core.c > > +++ b/drivers/pmdomain/core.c > > @@ -189,6 +189,7 @@ static const struct genpd_lock_ops genpd_raw_spin_ops = { > > #define genpd_is_dev_name_fw(genpd) (genpd->flags & GENPD_FLAG_DEV_NAME_FW) > > #define genpd_is_no_sync_state(genpd) (genpd->flags & GENPD_FLAG_NO_SYNC_STATE) > > #define genpd_is_no_stay_on(genpd) (genpd->flags & GENPD_FLAG_NO_STAY_ON) > > +#define genpd_is_off_stay_on(genpd) (genpd->flags & GENPD_FLAG_OFF_STAY_ON) > > Maybe genpd_is_state_unknown ? > > > > > static inline bool irq_safe_dev_in_sleep_domain(struct device *dev, > > const struct generic_pm_domain *genpd) > > @@ -2445,7 +2446,10 @@ static void genpd_lock_init(struct generic_pm_domain *genpd) > > #ifdef CONFIG_PM_GENERIC_DOMAINS_OF > > static void genpd_set_stay_on(struct generic_pm_domain *genpd, bool is_off) > > { > > - genpd->stay_on = !genpd_is_no_stay_on(genpd) && !is_off; > > + if (genpd_is_off_stay_on(genpd)) > > + genpd->stay_on = is_off; > > + else > > + genpd->stay_on = !genpd_is_no_stay_on(genpd) && !is_off; > > } > > #else > > static void genpd_set_stay_on(struct generic_pm_domain *genpd, bool is_off) > > diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h > > index f925614aebdb..9929347e1d7f 100644 > > --- a/include/linux/pm_domain.h > > +++ b/include/linux/pm_domain.h > > @@ -121,6 +121,14 @@ struct dev_pm_domain_list { > > * powered-off until the ->sync_state() callback is > > * invoked. This flag informs genpd to allow a > > * power-off without waiting for ->sync_state(). > > + * > > + * GENPD_FLAG_OFF_STAY_ON: Use this flag to inform genpd that its initial > > + * status for the PM domain is set to powered off, > > + * which may not correctly reflect the state of the > > + * HW, as it's unknown. If the PM domain becomes > > + * powered on during boot, genpd will prevent it > > + * from being powered off until the ->sync_state > > + * callback is invoked for it. > > Maybe GENPD_FLAG_STATE_UNKNOWN ? Hmm, I understand your idea to clarify this and it sounds reasonable to me. However, "STATE" sounds a bit vague, what do you think of "GENPD_FLAG_POWER_UNKNOWN"? > > Or probably it would be better to just add a new state to the gpd_status > enum, like GENPD_STATE_UNKNOWN. Well, the problem with that is how it should be treated by genpd internals, when there are child/parent domains. Having an in-between status sounds unnecessarily complicated to me, at least for the purpose of $subject patch. > > > */ > > #define GENPD_FLAG_PM_CLK (1U << 0) > > #define GENPD_FLAG_IRQ_SAFE (1U << 1) > > @@ -133,6 +141,7 @@ struct dev_pm_domain_list { > > #define GENPD_FLAG_DEV_NAME_FW (1U << 8) > > #define GENPD_FLAG_NO_SYNC_STATE (1U << 9) > > #define GENPD_FLAG_NO_STAY_ON (1U << 10) > > +#define GENPD_FLAG_OFF_STAY_ON (1U << 11) > > > > enum gpd_status { > > GENPD_STATE_ON = 0, /* PM domain is on */ > > -- > > 2.43.0 > > Kind regards Uffe