* [PATCH] ASoC: rockchip: i2s: Use managed hclk and runtime PM cleanup
@ 2026-05-22 2:30 Cássio Gabriel
2026-06-01 17:34 ` Sebastian Reichel
0 siblings, 1 reply; 2+ messages in thread
From: Cássio Gabriel @ 2026-05-22 2:30 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Takashi Iwai, Jaroslav Kysela,
Heiko Stuebner
Cc: linux-sound, linux-arm-kernel, linux-rockchip, linux-kernel,
Cássio Gabriel
The Rockchip I2S driver mixes devm-managed probe resources with manual
runtime PM and hclk cleanup. This leaves the remove path doing runtime PM
shutdown and clock disable before devm-managed ASoC and PCM resources are
released.
Keep the bus clock enabled for the device lifetime with
devm_clk_get_enabled(), and move the runtime PM teardown into devres so the
unwind order matches the managed registrations. This also removes the
remove callback, which only existed for cleanup.
Use a devm action for the final runtime suspend and register it before the
managed runtime PM action, so teardown disables runtime PM before forcing
the device into the suspended state.
Signed-off-by: Cássio Gabriel <cassiogabrielcontato@gmail.com>
---
sound/soc/rockchip/rockchip_i2s.c | 68 +++++++++++++++------------------------
1 file changed, 26 insertions(+), 42 deletions(-)
diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
index 49ff86b35ef1..4e3af0f37941 100644
--- a/sound/soc/rockchip/rockchip_i2s.c
+++ b/sound/soc/rockchip/rockchip_i2s.c
@@ -664,6 +664,14 @@ static const struct of_device_id rockchip_i2s_match[] __maybe_unused = {
};
MODULE_DEVICE_TABLE(of, rockchip_i2s_match);
+static void rockchip_i2s_suspend(void *data)
+{
+ struct device *dev = data;
+
+ if (!pm_runtime_status_suspended(dev))
+ i2s_runtime_suspend(dev);
+}
+
static int rockchip_i2s_init_dai(struct rk_i2s_dev *i2s, struct resource *res,
struct snd_soc_dai_driver **dp)
{
@@ -758,37 +766,28 @@ static int rockchip_i2s_probe(struct platform_device *pdev)
}
/* try to prepare related clocks */
- i2s->hclk = devm_clk_get(&pdev->dev, "i2s_hclk");
+ i2s->hclk = devm_clk_get_enabled(&pdev->dev, "i2s_hclk");
if (IS_ERR(i2s->hclk)) {
dev_err(&pdev->dev, "Can't retrieve i2s bus clock\n");
return PTR_ERR(i2s->hclk);
}
- ret = clk_prepare_enable(i2s->hclk);
- if (ret) {
- dev_err(i2s->dev, "hclock enable failed %d\n", ret);
- return ret;
- }
i2s->mclk = devm_clk_get(&pdev->dev, "i2s_clk");
if (IS_ERR(i2s->mclk)) {
dev_err(&pdev->dev, "Can't retrieve i2s master clock\n");
- ret = PTR_ERR(i2s->mclk);
- goto err_clk;
+ return PTR_ERR(i2s->mclk);
}
regs = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
- if (IS_ERR(regs)) {
- ret = PTR_ERR(regs);
- goto err_clk;
- }
+ if (IS_ERR(regs))
+ return PTR_ERR(regs);
i2s->regmap = devm_regmap_init_mmio(&pdev->dev, regs,
&rockchip_i2s_regmap_config);
if (IS_ERR(i2s->regmap)) {
dev_err(&pdev->dev,
"Failed to initialise managed register map\n");
- ret = PTR_ERR(i2s->regmap);
- goto err_clk;
+ return PTR_ERR(i2s->regmap);
}
i2s->bclk_ratio = 64;
@@ -799,8 +798,7 @@ static int rockchip_i2s_probe(struct platform_device *pdev)
i2s->bclk_off = pinctrl_lookup_state(i2s->pinctrl, "bclk_off");
if (IS_ERR_OR_NULL(i2s->bclk_off)) {
dev_err(&pdev->dev, "failed to find i2s bclk_off\n");
- ret = -EINVAL;
- goto err_clk;
+ return -EINVAL;
}
}
} else {
@@ -811,16 +809,23 @@ static int rockchip_i2s_probe(struct platform_device *pdev)
dev_set_drvdata(&pdev->dev, i2s);
- pm_runtime_enable(&pdev->dev);
+ ret = devm_add_action(&pdev->dev, rockchip_i2s_suspend, &pdev->dev);
+ if (ret)
+ return ret;
+
+ ret = devm_pm_runtime_enable(&pdev->dev);
+ if (ret)
+ return ret;
+
if (!pm_runtime_enabled(&pdev->dev)) {
ret = i2s_runtime_resume(&pdev->dev);
if (ret)
- goto err_pm_disable;
+ return ret;
}
ret = rockchip_i2s_init_dai(i2s, res, &dai);
if (ret)
- goto err_pm_disable;
+ return ret;
ret = devm_snd_soc_register_component(&pdev->dev,
&rockchip_i2s_component,
@@ -828,36 +833,16 @@ static int rockchip_i2s_probe(struct platform_device *pdev)
if (ret) {
dev_err(&pdev->dev, "Could not register DAI\n");
- goto err_suspend;
+ return ret;
}
ret = devm_snd_dmaengine_pcm_register(&pdev->dev, NULL, 0);
if (ret) {
dev_err(&pdev->dev, "Could not register PCM\n");
- goto err_suspend;
+ return ret;
}
return 0;
-
-err_suspend:
- if (!pm_runtime_status_suspended(&pdev->dev))
- i2s_runtime_suspend(&pdev->dev);
-err_pm_disable:
- pm_runtime_disable(&pdev->dev);
-err_clk:
- clk_disable_unprepare(i2s->hclk);
- return ret;
-}
-
-static void rockchip_i2s_remove(struct platform_device *pdev)
-{
- struct rk_i2s_dev *i2s = dev_get_drvdata(&pdev->dev);
-
- pm_runtime_disable(&pdev->dev);
- if (!pm_runtime_status_suspended(&pdev->dev))
- i2s_runtime_suspend(&pdev->dev);
-
- clk_disable_unprepare(i2s->hclk);
}
static const struct dev_pm_ops rockchip_i2s_pm_ops = {
@@ -866,7 +851,6 @@ static const struct dev_pm_ops rockchip_i2s_pm_ops = {
static struct platform_driver rockchip_i2s_driver = {
.probe = rockchip_i2s_probe,
- .remove = rockchip_i2s_remove,
.driver = {
.name = DRV_NAME,
.of_match_table = of_match_ptr(rockchip_i2s_match),
---
base-commit: 40cc9602caf2539369bd3dd7d66ee67e204e75ef
change-id: 20260521-asoc-rockchip-i2s-devm-cleanup-6cade5f495e5
Best regards,
--
Cássio Gabriel <cassiogabrielcontato@gmail.com>
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] ASoC: rockchip: i2s: Use managed hclk and runtime PM cleanup
2026-05-22 2:30 [PATCH] ASoC: rockchip: i2s: Use managed hclk and runtime PM cleanup Cássio Gabriel
@ 2026-06-01 17:34 ` Sebastian Reichel
0 siblings, 0 replies; 2+ messages in thread
From: Sebastian Reichel @ 2026-06-01 17:34 UTC (permalink / raw)
To: Cássio Gabriel
Cc: Liam Girdwood, Mark Brown, Takashi Iwai, Jaroslav Kysela,
Heiko Stuebner, linux-sound, linux-arm-kernel, linux-rockchip,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 5780 bytes --]
Hi,
On Thu, May 21, 2026 at 11:30:07PM -0300, Cássio Gabriel wrote:
> The Rockchip I2S driver mixes devm-managed probe resources with manual
> runtime PM and hclk cleanup. This leaves the remove path doing runtime PM
> shutdown and clock disable before devm-managed ASoC and PCM resources are
> released.
>
> Keep the bus clock enabled for the device lifetime with
> devm_clk_get_enabled(), and move the runtime PM teardown into devres so the
> unwind order matches the managed registrations. This also removes the
> remove callback, which only existed for cleanup.
>
> Use a devm action for the final runtime suspend and register it before the
> managed runtime PM action, so teardown disables runtime PM before forcing
> the device into the suspended state.
>
> Signed-off-by: Cássio Gabriel <cassiogabrielcontato@gmail.com>
> ---
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Greetings,
-- Sebastian
> sound/soc/rockchip/rockchip_i2s.c | 68 +++++++++++++++------------------------
> 1 file changed, 26 insertions(+), 42 deletions(-)
>
> diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
> index 49ff86b35ef1..4e3af0f37941 100644
> --- a/sound/soc/rockchip/rockchip_i2s.c
> +++ b/sound/soc/rockchip/rockchip_i2s.c
> @@ -664,6 +664,14 @@ static const struct of_device_id rockchip_i2s_match[] __maybe_unused = {
> };
> MODULE_DEVICE_TABLE(of, rockchip_i2s_match);
>
> +static void rockchip_i2s_suspend(void *data)
> +{
> + struct device *dev = data;
> +
> + if (!pm_runtime_status_suspended(dev))
> + i2s_runtime_suspend(dev);
> +}
> +
> static int rockchip_i2s_init_dai(struct rk_i2s_dev *i2s, struct resource *res,
> struct snd_soc_dai_driver **dp)
> {
> @@ -758,37 +766,28 @@ static int rockchip_i2s_probe(struct platform_device *pdev)
> }
>
> /* try to prepare related clocks */
> - i2s->hclk = devm_clk_get(&pdev->dev, "i2s_hclk");
> + i2s->hclk = devm_clk_get_enabled(&pdev->dev, "i2s_hclk");
> if (IS_ERR(i2s->hclk)) {
> dev_err(&pdev->dev, "Can't retrieve i2s bus clock\n");
> return PTR_ERR(i2s->hclk);
> }
> - ret = clk_prepare_enable(i2s->hclk);
> - if (ret) {
> - dev_err(i2s->dev, "hclock enable failed %d\n", ret);
> - return ret;
> - }
>
> i2s->mclk = devm_clk_get(&pdev->dev, "i2s_clk");
> if (IS_ERR(i2s->mclk)) {
> dev_err(&pdev->dev, "Can't retrieve i2s master clock\n");
> - ret = PTR_ERR(i2s->mclk);
> - goto err_clk;
> + return PTR_ERR(i2s->mclk);
> }
>
> regs = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
> - if (IS_ERR(regs)) {
> - ret = PTR_ERR(regs);
> - goto err_clk;
> - }
> + if (IS_ERR(regs))
> + return PTR_ERR(regs);
>
> i2s->regmap = devm_regmap_init_mmio(&pdev->dev, regs,
> &rockchip_i2s_regmap_config);
> if (IS_ERR(i2s->regmap)) {
> dev_err(&pdev->dev,
> "Failed to initialise managed register map\n");
> - ret = PTR_ERR(i2s->regmap);
> - goto err_clk;
> + return PTR_ERR(i2s->regmap);
> }
>
> i2s->bclk_ratio = 64;
> @@ -799,8 +798,7 @@ static int rockchip_i2s_probe(struct platform_device *pdev)
> i2s->bclk_off = pinctrl_lookup_state(i2s->pinctrl, "bclk_off");
> if (IS_ERR_OR_NULL(i2s->bclk_off)) {
> dev_err(&pdev->dev, "failed to find i2s bclk_off\n");
> - ret = -EINVAL;
> - goto err_clk;
> + return -EINVAL;
> }
> }
> } else {
> @@ -811,16 +809,23 @@ static int rockchip_i2s_probe(struct platform_device *pdev)
>
> dev_set_drvdata(&pdev->dev, i2s);
>
> - pm_runtime_enable(&pdev->dev);
> + ret = devm_add_action(&pdev->dev, rockchip_i2s_suspend, &pdev->dev);
> + if (ret)
> + return ret;
> +
> + ret = devm_pm_runtime_enable(&pdev->dev);
> + if (ret)
> + return ret;
> +
> if (!pm_runtime_enabled(&pdev->dev)) {
> ret = i2s_runtime_resume(&pdev->dev);
> if (ret)
> - goto err_pm_disable;
> + return ret;
> }
>
> ret = rockchip_i2s_init_dai(i2s, res, &dai);
> if (ret)
> - goto err_pm_disable;
> + return ret;
>
> ret = devm_snd_soc_register_component(&pdev->dev,
> &rockchip_i2s_component,
> @@ -828,36 +833,16 @@ static int rockchip_i2s_probe(struct platform_device *pdev)
>
> if (ret) {
> dev_err(&pdev->dev, "Could not register DAI\n");
> - goto err_suspend;
> + return ret;
> }
>
> ret = devm_snd_dmaengine_pcm_register(&pdev->dev, NULL, 0);
> if (ret) {
> dev_err(&pdev->dev, "Could not register PCM\n");
> - goto err_suspend;
> + return ret;
> }
>
> return 0;
> -
> -err_suspend:
> - if (!pm_runtime_status_suspended(&pdev->dev))
> - i2s_runtime_suspend(&pdev->dev);
> -err_pm_disable:
> - pm_runtime_disable(&pdev->dev);
> -err_clk:
> - clk_disable_unprepare(i2s->hclk);
> - return ret;
> -}
> -
> -static void rockchip_i2s_remove(struct platform_device *pdev)
> -{
> - struct rk_i2s_dev *i2s = dev_get_drvdata(&pdev->dev);
> -
> - pm_runtime_disable(&pdev->dev);
> - if (!pm_runtime_status_suspended(&pdev->dev))
> - i2s_runtime_suspend(&pdev->dev);
> -
> - clk_disable_unprepare(i2s->hclk);
> }
>
> static const struct dev_pm_ops rockchip_i2s_pm_ops = {
> @@ -866,7 +851,6 @@ static const struct dev_pm_ops rockchip_i2s_pm_ops = {
>
> static struct platform_driver rockchip_i2s_driver = {
> .probe = rockchip_i2s_probe,
> - .remove = rockchip_i2s_remove,
> .driver = {
> .name = DRV_NAME,
> .of_match_table = of_match_ptr(rockchip_i2s_match),
>
> ---
> base-commit: 40cc9602caf2539369bd3dd7d66ee67e204e75ef
> change-id: 20260521-asoc-rockchip-i2s-devm-cleanup-6cade5f495e5
>
> Best regards,
> --
> Cássio Gabriel <cassiogabrielcontato@gmail.com>
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-06-01 17:34 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-22 2:30 [PATCH] ASoC: rockchip: i2s: Use managed hclk and runtime PM cleanup Cássio Gabriel
2026-06-01 17:34 ` Sebastian Reichel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox