Date:   Tue, 5 Feb 2019 10:39:47 +0100
From:   Ulf Hansson <ulf.hansson@...aro.org>
To:     Aisheng Dong <aisheng.dong@....com>
Cc:     "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "dongas86@...il.com" <dongas86@...il.com>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>,
        "shawnguo@...nel.org" <shawnguo@...nel.org>,
        Fabio Estevam <fabio.estevam@....com>,
        dl-linux-imx <linux-imx@....com>,
        "rjw@...ysocki.net" <rjw@...ysocki.net>,
        "khilman@...nel.org" <khilman@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [RFC PATCH 1/1] PM / Domains: Add multi PM domains support for attach_dev

[...]

> > > 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

Powered by blists - more mailing lists