Messages in this thread | | | Subject | Re: [Patch v5 4/6] soc: qcom: Extend RPMh power controller driver to register warming devices. | From | Thara Gopinath <> | Date | Mon, 30 Mar 2020 10:53:58 -0400 |
| |
On 3/27/20 6:53 PM, Bjorn Andersson wrote: > On Thu 19 Mar 18:41 PDT 2020, Thara Gopinath wrote: > >> RPMh power control hosts power domains that can be used as >> thermal warming devices. Register these power domains >> with the generic power domain warming device thermal framework. >> >> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> >> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org> >> --- >> >> v3->v4: >> - Introduce a boolean value is_warming_dev in rpmhpd structure to >> indicate if a generic power domain can be used as a warming >> device or not.With this change, device tree no longer has to >> specify which power domain inside the rpmh power domain provider >> is a warming device. >> - Move registering of warming devices into a late initcall to >> ensure that warming devices are registered after thermal >> framework is initialized. > > This information is lost when we merge patches, as such I would like > such design decisions to be described in the commit message itself. > But... > >> >> drivers/soc/qcom/rpmhpd.c | 37 ++++++++++++++++++++++++++++++++++++- >> 1 file changed, 36 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c >> index 7142409a3b77..4e9c0bbb8826 100644 >> --- a/drivers/soc/qcom/rpmhpd.c >> +++ b/drivers/soc/qcom/rpmhpd.c >> @@ -11,6 +11,7 @@ >> #include <linux/of_device.h> >> #include <linux/platform_device.h> >> #include <linux/pm_opp.h> >> +#include <linux/pd_warming.h> >> #include <soc/qcom/cmd-db.h> >> #include <soc/qcom/rpmh.h> >> #include <dt-bindings/power/qcom-rpmpd.h> >> @@ -48,6 +49,7 @@ struct rpmhpd { >> bool enabled; >> const char *res_name; >> u32 addr; >> + bool is_warming_dev; >> }; >> >> struct rpmhpd_desc { >> @@ -55,6 +57,8 @@ struct rpmhpd_desc { >> size_t num_pds; >> }; >> >> +const struct rpmhpd_desc *global_desc; >> + >> static DEFINE_MUTEX(rpmhpd_lock); >> >> /* SDM845 RPMH powerdomains */ >> @@ -89,6 +93,7 @@ static struct rpmhpd sdm845_mx = { >> .pd = { .name = "mx", }, >> .peer = &sdm845_mx_ao, >> .res_name = "mx.lvl", >> + .is_warming_dev = true, >> }; >> >> static struct rpmhpd sdm845_mx_ao = { >> @@ -452,7 +457,14 @@ static int rpmhpd_probe(struct platform_device *pdev) >> &rpmhpds[i]->pd); >> } >> >> - return of_genpd_add_provider_onecell(pdev->dev.of_node, data); >> + ret = of_genpd_add_provider_onecell(pdev->dev.of_node, data); >> + >> + if (ret) >> + return ret; >> + >> + global_desc = desc; >> + >> + return 0; >> } >> >> static struct platform_driver rpmhpd_driver = { >> @@ -469,3 +481,26 @@ static int __init rpmhpd_init(void) >> return platform_driver_register(&rpmhpd_driver); >> } >> core_initcall(rpmhpd_init); >> + >> +static int __init rpmhpd_init_warming_device(void) >> +{ >> + size_t num_pds; >> + struct rpmhpd **rpmhpds; >> + int i; >> + >> + if (!global_desc) >> + return -EINVAL; >> + >> + rpmhpds = global_desc->rpmhpds; >> + num_pds = global_desc->num_pds; >> + >> + if (!of_find_property(rpmhpds[0]->dev->of_node, "#cooling-cells", NULL)) >> + return 0; >> + >> + for (i = 0; i < num_pds; i++) >> + if (rpmhpds[i]->is_warming_dev) >> + of_pd_warming_register(rpmhpds[i]->dev, i); >> + >> + return 0; >> +} >> +late_initcall(rpmhpd_init_warming_device); > > ...why can't this be done in rpmhpd_probe()? > > In particular with the recent patches from John Stultz to allow rpmhpd > to be built as a module I don't think there's any guarantees that > rpmh_probe() will have succeeded before rpmhpd_init_warming_device() > executes.
It is to take care of boot order. So this has to happen after the thermal framework is initialized. Thermal framework is initialized with core_initcall. Can I move the rpmhpd init as a postcore_initcall ? Then I can get rid of this separate function and keep it as part of probe.
> > Regards, > Bjorn >
-- Warm Regards Thara
| |