Messages in this thread | | | From | Enric Balletbo i Serra <> | Subject | Re: [PATCH v2 08/12] soc: mediatek: pm-domains: Add subsystem clocks | Date | Mon, 26 Oct 2020 16:17:43 +0100 |
| |
Hi Matthias,
Thank you for your review.
On 2/10/20 11:04, Matthias Brugger wrote: > > > On 01/10/2020 18:01, Enric Balletbo i Serra wrote: >> From: Matthias Brugger <mbrugger@suse.com> >> >> For the bus protection operations, some subsystem clocks need to be enabled >> before releasing the protection. This patch identifies the subsystem clocks >> by it's name. >> >> Suggested-by: Weiyi Lu <weiyi.lu@mediatek.com> >> [Adapted the patch to the mtk-pm-domains driver] >> Signed-off-by: Matthias Brugger <mbrugger@suse.com> >> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> >> --- >> >> Changes in v2: >> - Use dev_err_probe if getting clocks fails, so an error is not printed >> if is deferred. >> >> drivers/soc/mediatek/mtk-pm-domains.c | 89 ++++++++++++++++++++++----- >> 1 file changed, 75 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c >> b/drivers/soc/mediatek/mtk-pm-domains.c >> index e0a52d489fea..2075072f16da 100644 >> --- a/drivers/soc/mediatek/mtk-pm-domains.c >> +++ b/drivers/soc/mediatek/mtk-pm-domains.c > [...] >> + for (i = 0; i < pd->num_subsys_clks; i++) { >> + struct clk *clk = of_clk_get(node, i + clk_ind); >> + >> + if (IS_ERR(clk)) { >> + ret = PTR_ERR(clk); >> + dev_err_probe(scpsys->dev, PTR_ERR(clk), >> + "%pOFn: failed to get clk at index %d: %d\n", >> + node, i + clk_ind, ret); >> + goto err_put_subsys_clocks; >> + } >> + >> + pd->subsys_clks[i].clk = clk; >> + } >> + >> + ret = clk_bulk_prepare(pd->num_subsys_clks, pd->subsys_clks); >> + if (ret) >> + goto err_put_subsys_clocks; >> + > > Not sure if it really matters, but logically we should prepare the basic clocks > before we prepare the subsystem clocks. And fix the error handling accordingly. >
Changed in next version.
> Regards, > Matthias > >> ret = clk_bulk_prepare(pd->num_clks, pd->clks); >> if (ret) >> - goto err_put_clocks; >> + goto err_unprepare_subsys_clocks; >> /* >> * Initially turn on all domains to make the domains usable >> @@ -462,6 +513,12 @@ static int scpsys_add_one_domain(struct scpsys *scpsys, >> struct device_node *node >> err_unprepare_clocks: >> clk_bulk_unprepare(pd->num_clks, pd->clks); >> +err_unprepare_subsys_clocks: >> + clk_bulk_unprepare(pd->num_subsys_clks, pd->subsys_clks); >> +err_put_subsys_clocks: >> + clk_bulk_put(pd->num_subsys_clks, pd->subsys_clks); >> + devm_kfree(scpsys->dev, pd->subsys_clks); >> + pd->num_subsys_clks = 0; >> err_put_clocks: >> clk_bulk_put(pd->num_clks, pd->clks); >> devm_kfree(scpsys->dev, pd->clks); >> @@ -541,6 +598,10 @@ static void scpsys_remove_one_domain(struct scpsys_domain >> *pd) >> clk_bulk_unprepare(pd->num_clks, pd->clks); >> clk_bulk_put(pd->num_clks, pd->clks); >> pd->num_clks = 0; >> + >> + clk_bulk_unprepare(pd->num_subsys_clks, pd->subsys_clks); >> + clk_bulk_put(pd->num_subsys_clks, pd->subsys_clks); >> + pd->num_subsys_clks = 0; >> } >> static void scpsys_domain_cleanup(struct scpsys *scpsys) >>
| |