Message ID | cover.1530252803.git.viresh.kumar@linaro.org |
---|---|
Headers | show |
Series | OPP: Support multiple power-domains per device | expand |
On Friday, June 29, 2018 8:19:30 AM CEST Viresh Kumar wrote: > Hi, > > This series improves the OPP core (and a bit of genpd core as well) to > support multiple phandles in the "required-opps" property, which are > only used for multiple power-domains per device for now. > > We still don't propagate the changes to master domains for the > sub-domains, but this patchset is an important stepping stone for that > to happen. > > Tested on Hikey960 after faking some power domains for CPUs. I'm assuming that this work will go in via OPP. Thanks, Rafael
On 29-06-18, 11:49, Viresh Kumar wrote: > Hi, > > This series improves the OPP core (and a bit of genpd core as well) to > support multiple phandles in the "required-opps" property, which are > only used for multiple power-domains per device for now. > > We still don't propagate the changes to master domains for the > sub-domains, but this patchset is an important stepping stone for that > to happen. > > Tested on Hikey960 after faking some power domains for CPUs. Ulf, can you please help review this stuff ? -- viresh
On 16 July 2018 at 06:32, Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 29-06-18, 11:49, Viresh Kumar wrote: >> Hi, >> >> This series improves the OPP core (and a bit of genpd core as well) to >> support multiple phandles in the "required-opps" property, which are >> only used for multiple power-domains per device for now. >> >> We still don't propagate the changes to master domains for the >> sub-domains, but this patchset is an important stepping stone for that >> to happen. >> >> Tested on Hikey960 after faking some power domains for CPUs. > > Ulf, can you please help review this stuff ? I am in holiday mode, so I needs some more time before I can review this. Apologize for the inconvenience. Kind regards Uffe
On 29 June 2018 at 08:19, Viresh Kumar <viresh.kumar@linaro.org> wrote: > Multiple generic power domains for a device are supported with the help > of virtual devices, which are created for each device-genpd pair. These What "device-genpd" pair are you referring to? > are the device structures which are attached to the power domain and are > required by the OPP core to set the performance state of the genpd. > > The helpers added by this commit are required to be called once for each > genpd of a device. These are required only if multiple domains are > present for a device, otherwise the actual device structure will be used > instead by the OPP core. > > This commit also updates the genpd core to automatically call the new > helper to set the required devices with the OPP layer, whenever the > virtual devices are created for multiple genpd. The prototype of > __genpd_dev_pm_attach() is slightly updated for this. > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > drivers/base/power/domain.c | 31 ++++++++++++++--- > drivers/opp/core.c | 69 +++++++++++++++++++++++++++++++++++++ > drivers/opp/of.c | 12 +++++++ > drivers/opp/opp.h | 2 ++ > include/linux/pm_opp.h | 8 +++++ > 5 files changed, 118 insertions(+), 4 deletions(-) > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > index c298de8a8308..e9c85c96580c 100644 > --- a/drivers/base/power/domain.c > +++ b/drivers/base/power/domain.c > @@ -2219,8 +2219,14 @@ static void genpd_dev_pm_detach(struct device *dev, bool power_off) > genpd_queue_power_off_work(pd); > > /* Unregister the device if it was created by genpd. */ > - if (dev->bus == &genpd_bus_type) > + if (dev->bus == &genpd_bus_type) { > + struct opp_table *opp_table = dev_get_drvdata(dev); This feels wrong, to me. Genpd is not a driver, but rather a subsystem, hence I think it's a bad idea to "abuse" the driver data pointer like this. Instead, in genpd we store device specific data, through the dev->power.subsys_data. Seems like a better option is to extend the subsys_data, to also include data needed to be shared for opp/performance states. In that way, we might not even need to call into the opp library (dev_pm_opp_set_required_device()) API, but rather the opp library can itself check what is in the subsys_data for the device that is being targeted. Would that work? [...] Kind regards Uffe