Re: [PATCH v2 08/12] soc: mediatek: pm-domains: Add subsystem clocks

[Matthias Brugger]

On 01/10/2020 18:01, Enric Balletbo i Serra wrote:
From: Matthias Brugger <mbrug...@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...@mediatek.com>
[Adapted the patch to the mtk-pm-domains driver]
Signed-off-by: Matthias Brugger <mbrug...@suse.com>
Signed-off-by: Enric Balletbo i Serra <enric.balle...@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.

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)