[PATCH] clk: samsung: exynos5-subcmu: Use pm_runtime_get_sync() during probe

[PATCH] clk: samsung: exynos5-subcmu: Use pm_runtime_get_sync() during probe

[Marek Szyprowski]

The clocks are registered right after pm_runtime_get(), which only
schedules a resume asynchronously and doesn't guarantee the device (and
its power domain) is active yet. Use pm_runtime_get_sync() to ensure the
power domain is really on while the clock registers are accessed during
the registration.

Fixes: b06a532bf1fa ("clk: samsung: Add Exynos5 sub-CMU clock driver")
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/clk/samsung/clk-exynos5-subcmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/samsung/clk-exynos5-subcmu.c b/drivers/clk/samsung/clk-exynos5-subcmu.c
index 587fe6e3bbed..886836494838 100644
--- a/drivers/clk/samsung/clk-exynos5-subcmu.c
+++ b/drivers/clk/samsung/clk-exynos5-subcmu.c
@@ -104,7 +104,7 @@ static int __init exynos5_subcmu_probe(struct platform_device *pdev)
 
 	pm_runtime_set_suspended(dev);
 	pm_runtime_enable(dev);
-	pm_runtime_get(dev);
+	pm_runtime_get_sync(dev);
 
 	ctx->dev = dev;
 	samsung_clk_register_div(ctx, info->div_clks, info->nr_div_clks);
-- 
2.43.0

Re: [PATCH] clk: samsung: exynos5-subcmu: Use pm_runtime_get_sync() during probe

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] pm: missing error check for pm_runtime_get_sync() allows unpowered register access
- [High] clk: samsung: exynos5-subcmu: race condition on global ctx->dev
--

commit fe90e32911f3916661d4731141c07feca831815e
Author: Marek Szyprowski <m.szyprowski@samsung.com>

clk: samsung: exynos5-subcmu: Use pm_runtime_get_sync() during probe

This commit changes the probe function to use pm_runtime_get_sync() instead
of pm_runtime_get(). This ensures that the device and its power domain are
fully active before accessing clock registers during registration.

> diff --git a/drivers/clk/samsung/clk-exynos5-subcmu.c b/drivers/clk/samsung/clk-exynos5-subcmu.c
> index 3731298473013..1d7eb98b72cf8 100644
> --- a/drivers/clk/samsung/clk-exynos5-subcmu.c
> +++ b/drivers/clk/samsung/clk-exynos5-subcmu.c
> @@ -103,7 +103,7 @@ static int __init exynos5_subcmu_probe(struct platform_device *pdev)
>  
>  	pm_runtime_set_suspended(dev);
>  	pm_runtime_enable(dev);
> -	pm_runtime_get(dev);
> +	pm_runtime_get_sync(dev);

[Severity: High]
Is it safe to ignore the return value of pm_runtime_get_sync()?

If it returns a negative error code (e.g. hardware failure, regulator
issue, or timeout), proceeding to call samsung_clk_register_div() and
samsung_clk_register_gate() will read unpowered hardware registers, which
can cause a synchronous external abort or data corruption.

Should this check the return value, abort the probe, and roll back PM
operations on failure?

>  
>  	ctx->dev = dev;
>  	samsung_clk_register_div(ctx, info->div_clks, info->nr_div_clks);
>  	samsung_clk_register_gate(ctx, info->gate_clks, info->nr_gate_clks);
>  	ctx->dev = NULL;

[Severity: High]
Does assigning and clearing the shared global ctx->dev without locking
introduce a race condition during concurrent asynchronous probes?

If asynchronous probing is enabled via driver_async_probe, multiple
exynos5-subcmu devices could probe concurrently. Since pm_runtime_get_sync()
can sleep, this widens the race window where concurrent threads could
overwrite ctx->dev or prematurely clear it to NULL while another thread is
still registering its clocks.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015115008.600082-1-m.szyprowski@samsung.com?part=1