Re: [RFC PATCH 1/1] PM / Domains: Add multi PM domains support for attach_dev

From: Ulf Hansson
Date: Tue Feb 05 2019 - 04:40:27 EST


[...]

> > > Then attach_dev() can parse the correct "resource id" (e.g.
> > > IMX_SC_R_SDHC_1) from device tree And store it in per-device struct
> > generic_pm_domain_data for later start()/stop() using.
> >
> > I see, thanks for clarifying.
> >
> > Seem like, we have two options to make this work.
> >
> > 1. Let genpd pre-store the index in a the per device generic_pm_domain_data
> > while doing the attach and before calling the
> > ->attach_dev() callback. This would make sense, if we consider this to
> > be a common thing.
> >
> > 2. Provide the index as an additional new in-parameter to the
> > ->attach_dev() callback. This requires a tree wide change as it means
> > we need to update existing code using the ->attach_dev() callback.
> >
> > I would probably try 1) first to see how the code would look like and then fall
> > back to 2). What do you think?
> >
>
> Yes, I agree with you.
> This patch is exactly doing 1.
>
> For your former suggestion:
> " Just assign virt_dev->of_node = of_node_get(dev->of_node), somewhere in
> genpd_dev_pm_attach_by_id() and before calling __genpd_dev_pm_attach().
> Doing that, would mean the genpd provider's ->attach_dev() callback,
> don't have to distinguish between virtual and non-virtual devices"
>
> As this can only solve passing the real device node issue and we still need
> pre-store the index in a the per device generic_pm_domain_data and then
> distinguish if it's multi power domain virtual devices in attach_dev(), so
> I'm not sure if it's still quite necessary to do that way by assigning
> "virt_dev->of_node = of_node_get(dev->of_node) before attach".
> Probably after fully switch to option 2, then we can do it to fully drop the using
> of per device generic_pm_domain_data to make it transparent to attach_dev()
> users.
>
> So now the options may be:
> 1. Pre-store both device node and domain index in generic_pm_domain_data before
> Calling attach_dev(). This is exactly this patch does.
> 2. Pre-store only domain index in generic_pm_domain_data. For device node, assigning
> "virt_dev->of_node = of_node_get(dev->of_node) before attach.
> 3. Change attach_dev() to passing domain index and assigning
> "virt_dev->of_node = of_node_get(dev->of_node) before attach.
>
> Can you please tell what's your prefer?

Apologize for the delay, just realized that you were expecting a reply from me.

To me is sounds like option 2) is the most feasible. Perhaps it's even
a good idea to store the domain index in the struct
generic_pm_domain_data, as it allows it then to be used at any point
later in time, this not only from the ->attach_dev() callback as
option 3) would mean.

So according to my understandings, then your genpd backend driver's
->attach_dev() callback, would use dev_gpd_data() to find out the
domain index for the device in question. Is that sufficient, or is
there anything else that is needed?

[...]

Kind regards
Uffe