Re: [PATCH] PM: domains: Don't attach a device to genpd that corresponds to a provider

[Bjorn Andersson]

On Fri 09 Jul 08:22 CDT 2021, Ulf Hansson wrote:

> On Fri, 9 Jul 2021 at 15:07, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
> >
> > Hi Ulf,
> >
> > Thanks for your patch!
> >
> > On Fri, Jul 9, 2021 at 2:56 PM Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
> > > According to the common power domain DT bindings, a power domain provider
> > > must have a "#power-domain-cells" property in its OF node. Additionally, if
> > > a provider has a "power-domains" property, it means that it has a parent
> > > domain.
> >
> > OK.
> >
> > > It has turned out that some OF nodes that represents a genpd provider may
> > > also be compatible with a regular platform device. This leads to, during
> > > probe, genpd_dev_pm_attach(), genpd_dev_pm_attach_by_name() and
> > > genpd_dev_pm_attach_by_id() tries to attach the corresponding struct device
> > > to the genpd provider's parent domain, which is wrong. Instead the genpd
> >
> > Why is that wrong?
> 
> It may lead to that the struct device that corresponds to a genpd
> provider may be attached to the parent domain. In other words, the
> parent domain will not only be controlled by a child domain
> (corresponding to the provider), but also through the provider's
> struct device. As far as I can tell, this has never been the intent
> for how things should work in genpd.
> 
> So wrong or not, I guess it depends on what you expect to happen.
> 
> Do you see an issue with changing this?
> 

But this exactly what we have in the case of the "dispcc" in the
Qualcomm platform that Dmitry is working on.

The provider driver needs the parent power-domain to be powered in order
to poke the registers and then it is the parent of the power-domains
exposed.

If I understand your proposed patch we'll have to manually attach the
parent domain to the struct device of the controller with this patch?

Is the Qualcomm case unique or will this change cut power do other genpd
providers assuming the same?



Worth mentioning as we discuss this is that we have another genpd
provider, where I think the exposed genpds are parented by a few
different (each one with a specific) parent domains. In this case we'd
be forced to manually attach the genpd provider to the parent domain
that it actually is powered by (as no automatic attachment happens when
multiple domains are specified).

Regards,
Bjorn

> >
> > > provider should only assign a parent domain, through
> > > pm_genpd_add_subdomain() or of_genpd_add_subdomain().
> 
> Kind regards
> Uffe