Re: [RFC PATCH 2/3] pmdomain: core: Add a genpd config to support unknown initial status

[Ulf Hansson]

On Wed, Aug 19, 2026 at 7:40 PM Abel Vesa <abel.vesa@xxxxxxxxxxxxxxxx> wrote:
>
> On 26-08-19 17:14:52, Ulf Hansson wrote:
> > From: Ulf Hansson <ulfh@xxxxxxxxxx>
> >
> > 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 <maulik.shah@xxxxxxxxxxxxxxxx>
> > Link: https://lore.kernel.org/all/20260811-domain_off_ss3-v1-0-6a0a0fc023f5@xxxxxxxxxxxxxxxx/
> > Signed-off-by: Ulf Hansson <ulfh@xxxxxxxxxx>
> > ---
> >  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