Re: [PATCH 4/9] pmdomain: core: Add initial fine grained sync_state support

[Ulf Hansson]

On Fri, 20 Mar 2026 at 10:29, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
>
> Hi Ulf,
>
> Thanks for your patch!
>
> On Tue, 3 Mar 2026 at 14:23, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
> > A onecell (#power-domain-cells = <1 or 2>; in DT) power domain provider
> > typically provides multiple independent power domains, each with their own
> > corresponding consumers. In these cases we have to wait for all consumers
> > for all the provided power domains before the ->sync_state() callback gets
> > called for the supplier.
> >
> > In a first step to improve this, let's implement support for fine grained
> > sync_state support a per genpd basis by using the ->queue_sync_state()
>
> ... support on a ...
>
> > callback. To take step by step, let's initially limit the improvement to
> > the internal genpd provider driver and to its corresponding genpd devices
> > for onecell providers.
> >
> > Signed-off-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
>
> > --- a/drivers/pmdomain/core.c
> > +++ b/drivers/pmdomain/core.c
>
> > +static void genpd_queue_sync_state(struct device *dev)
> > +{
> > +       struct device_node *np = dev->of_node;
> > +       struct device_link *link;
> > +
> > +       if (!genpd_should_wait_for_consumer(np))
> > +               return;
> > +
> > +       list_for_each_entry(link, &dev->links.consumers, s_node) {
> > +               struct device *consumer = link->consumer;
> > +
> > +               pr_info("%s:%s con=%s\n", __func__, dev_name(dev),
> > +                       dev_name(consumer));
>
> pr_debug? Or better, dev_dbg(), so you don't have to add dev_name(dev)
> explicitly.
>
> However, the printed provider name is again the name of the first
> domain.  This is incorrect for all but the first domain of a onecell
> genpd provider, and thus confusing.

I agree, I think we should drop the print. In fact, this is a
left-over from my debugging, I didn't intend for it to be part of the
submission.

Although, perhaps there is a need for pr_debug/dev_dbg somewhere, but
I think we can better add those in separate patches on top instead.

>
> > +
> > +               if (!device_link_test(link, DL_FLAG_MANAGED))
> > +                       continue;
> > +
> > +               if (link->status == DL_STATE_ACTIVE)
> > +                       continue;
> > +
> > +               if (!consumer->of_node)
> > +                       continue;
> > +
> > +               /*
> > +                * A consumer device has not been probed yet. Let's parse its
> > +                * device node for the power-domains property, to find out the
> > +                * genpds it may belong to and then prevent sync state for them.
> > +                */
> > +               genpd_parse_for_consumer(np, consumer->of_node);
> > +       }
> > +
> > +       _genpd_queue_sync_state(np);
> > +}
> > +
> >  static void genpd_sync_state(struct device *dev)
> >  {
> >         return of_genpd_sync_state(dev->of_node);
>

Thanks for reviewing!

Kind regards
Uffe