Re: [PATCH/RFC] PM: domains: Call pm_runtime_barrier() before dev_pm_domain_{attach*,detach}()

[Geert Uytterhoeven]

Hi Ulf,

On Thu, 19 Mar 2026 at 11:59, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
> On Thu, 12 Mar 2026 at 10:54, Geert Uytterhoeven
> <geert+renesas@xxxxxxxxx> wrote:
> > If a device has multiple PM Domains, dev_pm_domain_detach() is called
> > multiple times on unbind or probe failure.  If the PM Domain is also a
> > Clock Domain, and thus calls pm_clk_destroy() from its .detach()
> > callback, dev_pm_put_subsys_data() will set dev->power.subsys_data to
> > NULL when psd->refcount reaches zero.
> >
> > Later/in parallel, default_suspend_ok() calls dev_gpd_data():
> >
> >     static inline struct generic_pm_domain_data *dev_gpd_data(struct device *dev)
> >     {
> >             return to_gpd_data(dev->power.subsys_data->domain_data);
> >     }
> >
> > which may trigger a NULL pointer dereference.
> >
> > All dev_pm_domain_{at,de}tach*() functions document that callers must
> > ensure proper synchronization of these functions with power management
> > callbacks.  Unfortunately no callers seem to actually do so.  This
> > includes dev_pm_domain_attach_list() and dev_pm_domain_detach_list():
> > they call dev_pm_domain_{attach*,detach}() internally, which means they
> > should take care of this synchronization themselves.
> >
> > Add synchronization to dev_pm_domain_{at,de}tach_list() by calling
> > pm_runtime_barrier() before dev_pm_domain_{attach*,detach}(), and drop
> > the now obsolete comments.
>
> My apologies for not being able to respond earlier to your
> suggestions/questions. I have started looking into this now, and I
> will follow up with more replies and perhaps a patch shortly.
>
> Anyway, the principle is that callers of dev_pm_domain_detach() must
> manage the runtime PM enabling/disabling for its device. If runtime PM
> was enabled, it must typically be disabled before calling
> dev_pm_domain_detach().
>
> What makes this a bit more complicated is that we have two different
> scenarious to consider.
>
> 1) The legacy case, attachment via dev_pm_domain_attach() for the
> single PM domain case. Runtime PM should be enabled/disabled for the
> device, from its corresponding driver/bus. I assume this isn't the
> problem you are facing, right?

No, this is not the problem I am facing.

> 2) Attachment via dev_pm_domain_attach_by_id|name() (which is called
> for the *attach_list() case too), for the single/multi PM domain
> cases. In these cases, runtime PM is enabled in
> genpd_dev_pm_attach_by_id().
>
> For 2), I am inclined to think that the proper action is to call
> pm_runtime_disable() in genpd_dev_pm_detach() before it calls
> genpd_remove_device(). Although, I need to check more closely how
> suitable that would be.

Thanks, that sounds reasonable: genpd_dev_pm_attach_by_id() calls
pm_runtime_enable(), but there is no pm_runtime_disable() call to
balance that...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

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