From: phucduc.bui@gmail.com
To: Heiko Stuebner <heiko@sntech.de>, Mark Brown <broonie@kernel.org>,
Liam Girdwood <lgirdwood@gmail.com>
Cc: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>,
Krzysztof Kozlowski <krzk@kernel.org>,
Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>,
linux-sound@vger.kernel.org, linux-rockchip@lists.infradead.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org,
bui duc phuc <phucduc.bui@gmail.com>
Subject: [PATCH] ASoC: rockchip: rockchip_sai: Hand over hclk control exclusively to Runtime PM
Date: Mon, 22 Jun 2026 07:56:13 +0700 [thread overview]
Message-ID: <20260622005613.21870-1-phucduc.bui@gmail.com> (raw)
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
next reply other threads:[~2026-06-22 0:56 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-22 0:56 phucduc.bui [this message]
2026-06-22 13:04 ` [PATCH] ASoC: rockchip: rockchip_sai: Hand over hclk control exclusively to Runtime PM Mark Brown
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260622005613.21870-1-phucduc.bui@gmail.com \
--to=phucduc.bui@gmail.com \
--cc=broonie@kernel.org \
--cc=heiko@sntech.de \
--cc=krzk@kernel.org \
--cc=lgirdwood@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=linux-sound@vger.kernel.org \
--cc=nicolas.frattaroli@collabora.com \
--cc=perex@perex.cz \
--cc=tiwai@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox