[PATCH] ASoC: rockchip: rockchip_sai: Hand over hclk control exclusively to Runtime PM

[PATCH] ASoC: rockchip: rockchip_sai: Hand over hclk control exclusively to Runtime PM

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

Although switching to devm_clk_get_enabled() in a previous patch was
tested successfully, it introduces overlapping ownership of the clock
lifecycle. Since the driver requires early register access to read the
device version during probe(), enabling hclk at that point is mandatory.
However, relying on devres for automatic disabling at unbind, while calling
clk_disable_unprepare() manually at the end of probe() alongside Runtime
PM's autosuspend, creates redundant and overlapping clock management.

While this mixed approach might not trigger errors under normal test
conditions, it is architecturally sub-optimal. As stated in the code
comments, Runtime PM should exclusively own the clock for register
accesses after the initial configuration phase.

Clean up the design by:

   1 Reverting back to devm_clk_get() to remove the implicit devres
     enable/disable behavior.
   2 Manually enabling and disabling hclk explicitly only around the
     early register access before Runtime PM takes over.
   3 Dropping the stray clk_disable_unprepare() at the end of probe()
     so Runtime PM solely owns hclk afterward.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---

Links: 
1 This change is based on the discussion around manual hclk handing during probe(),
  as raised by Krysztof:
  https://lore.kernel.org/all/20e4754b-ea9a-404d-b529-ec44a7263cbf@kernel.org/#t
2 Background for the earlier devm_clk_get_enbabled() conversion:
  https://lore.kernel.org/all/2818018.CQOukoFCf9@workhorse/

An alternative approach would be use devm_regmap_init_mmio_clk() and let regmap
manage clock enablement around register accesses. If preferred, I can rework the 
driver accordingly.

 sound/soc/rockchip/rockchip_sai.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/sound/soc/rockchip/rockchip_sai.c b/sound/soc/rockchip/rockchip_sai.c
index a195e96fed0a..ebb491fac77d 100644
--- a/sound/soc/rockchip/rockchip_sai.c
+++ b/sound/soc/rockchip/rockchip_sai.c
@@ -1438,20 +1438,30 @@ static int rockchip_sai_probe(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, PTR_ERR(sai->mclk),
 				     "Failed to get mclk\n");
 
-	sai->hclk = devm_clk_get_enabled(&pdev->dev, "hclk");
+	sai->hclk = devm_clk_get(&pdev->dev, "hclk");
 	if (IS_ERR(sai->hclk))
 		return dev_err_probe(&pdev->dev, PTR_ERR(sai->hclk),
 				     "Failed to get hclk\n");
 
+	ret = clk_prepare_enable(sai->hclk);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "Failed to enable hclk\n");
+
 	regmap_read(sai->regmap, SAI_VERSION, &sai->version);
 
 	ret = rockchip_sai_init_dai(sai, res, &dai);
-	if (ret)
-		return dev_err_probe(&pdev->dev, ret, "Failed to initialize DAI\n");
+	if (ret) {
+		ret = dev_err_probe(&pdev->dev, ret, "Failed to initialize DAI\n");
+		goto err_disable_hclk;
+	}
 
 	ret = rockchip_sai_parse_paths(sai, node);
-	if (ret)
-		return dev_err_probe(&pdev->dev, ret, "Failed to parse paths\n");
+	if (ret) {
+		ret = dev_err_probe(&pdev->dev, ret, "Failed to parse paths\n");
+		goto err_disable_hclk;
+	}
+
+	clk_disable_unprepare(sai->hclk);
 
 	/*
 	 * From here on, all register accesses need to be wrapped in
@@ -1482,8 +1492,6 @@ static int rockchip_sai_probe(struct platform_device *pdev)
 	pm_runtime_use_autosuspend(&pdev->dev);
 	pm_runtime_put(&pdev->dev);
 
-	clk_disable_unprepare(sai->hclk);
-
 	return 0;
 
 err_runtime_suspend:
@@ -1493,6 +1501,11 @@ static int rockchip_sai_probe(struct platform_device *pdev)
 		rockchip_sai_runtime_suspend(&pdev->dev);
 
 	return ret;
+
+err_disable_hclk:
+	clk_disable_unprepare(sai->hclk);
+
+	return ret;
 }
 
 static void rockchip_sai_remove(struct platform_device *pdev)
-- 
2.43.0