Messages in this thread |  | | Subject | Re: [Patch v4 5/7] soc: qcom: Extend RPMh power controller driver to register warming devices. | From | Thara Gopinath <> | Date | Sun, 1 Mar 2020 18:36:02 -0500 |
| |
On 2/4/20 12:40 PM, Ulf Hansson wrote: > On Wed, 20 Nov 2019 at 13:56, Thara Gopinath <thara.gopinath@linaro.org> 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. >> >> 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 registerd after thermal >> framework is initialized. >> >> drivers/soc/qcom/rpmhpd.c | 38 +++++++++++++++++++++++++++++++++++++- >> 1 file changed, 37 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c >> index 9d37534..5666d1f 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/pwr_domain_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 = { >> @@ -396,7 +401,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; > > I assume this works fine, for now. > > Although, nothing prevents this driver from being probed for two > different compatibles for the same platform. Thus the global_desc > could be overwritten with the last one being probed, so then how do > you know which one to use?
Yes. It works fine for now. There are multiple ways to fix this in future. One is to make global_desc an array. Other would be to move the code in rpmhpd_init_warming_device to this init and make this a post_core init considering thermal subsytem uses core init. Like you said I will leave this at this for now and we can fix this if a need arises. I don't think there is a need for multiple compatibles for the same platform now. Thanks for the reviewed by! I will add it in the next version.
> >> + >> + return 0; >> } >> >> static struct platform_driver rpmhpd_driver = { >> @@ -413,3 +425,27 @@ 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) >> + pwr_domain_warming_register(rpmhpds[i]->dev, >> + rpmhpds[i]->res_name, i); >> + >> + return 0; >> +} >> +late_initcall(rpmhpd_init_warming_device); > > For the record, there are limitations with this approach, for example > you can't deal with -EPROBE_DEFER. > > On the other hand, I don't have anything better to suggest, from the > top of my head. So, feel free to add: > > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> > > Kind regards > Uffe >
-- Warm Regards Thara
|  |