[PATCH v2 0/3] clk: samsung: fix probe-ordering races in exynos5-subcmu

[PATCH v2 0/3] clk: samsung: fix probe-ordering races in exynos5-subcmu

[Marek Szyprowski]

This series fixes a group of related issues around power-domain
availability and probe ordering on Exynos5 platforms (observed on
Exynos5800, e.g. Odroid XU3/XU4), which can leave clock muxes stuck on
the oscillator instead of their intended PLL parent.

The issue was analyzed and patches were prepared with LLM assistance.
The whole patchset was tested on the real Odroid XU3, XU4 and Chromebook
Peach-Pit and Peach-Pi boards.

Best regards
Marek Szyprowski, PhD
Samsung R&D Institute Poland


Changelog:

v2:
- added exynos-audss-clk driver fix, as the exynos5-subcmu change exposed
  an issue in it
- removed __init annotations in exynos5-subcmu to make the driver deferred
  probe safe
- grouped all patches into one patchset

v1:
https://lore.kernel.org/all/20261015115008.600082-1-m.szyprowski@samsung.com/
https://lore.kernel.org/all/20261013111840.3645724-1-m.szyprowski@samsung.com/


Patch summary:

Marek Szyprowski (3):
  clk: samsung: exynos-audss: Propagate -EPROBE_DEFER from input clocks
  clk: samsung: exynos5-subcmu: Use pm_runtime_get_sync() during probe
  clk: samsung: exynos5-subcmu: Defer probe until power domains are
    available

 drivers/clk/samsung/clk-exynos-audss.c   | 33 +++++++++++++++++-------
 drivers/clk/samsung/clk-exynos5-subcmu.c | 29 ++++++++++++++++-----
 2 files changed, 46 insertions(+), 16 deletions(-)

-- 
2.43.0

[PATCH v2 1/3] clk: samsung: exynos-audss: Propagate -EPROBE_DEFER from input clocks

[Marek Szyprowski]

exynos_audss_clk_probe() fetches the optional pll_ref/pll_in/cdclk/
sclk_audio/sclk_pcm_in input clocks with devm_clk_get() and only
checks IS_ERR() before falling back to a default parent clock name.
This means a transient -EPROBE_DEFER (e.g. because the clock provider
behind "pll_in" has not registered its real clock yet) is silently
treated the same as the clock simply not being present, so the
mout_audss/mout_i2s muxes get permanently registered with the wrong
parent name.

Fix this by checking for -EPROBE_DEFER on each of these clocks right
after fetching them, before anything is registered, and bailing out
of probe so it gets retried once the real clocks are available.

Fixes: 547f33509ccc ("clk: exynos-audss: allow input clocks to be specified in device tree")
Cc: stable@vger.kernel.org
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/clk/samsung/clk-exynos-audss.c | 33 +++++++++++++++++++-------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos-audss.c b/drivers/clk/samsung/clk-exynos-audss.c
index e11ac67819ef..950c67491dad 100644
--- a/drivers/clk/samsung/clk-exynos-audss.c
+++ b/drivers/clk/samsung/clk-exynos-audss.c
@@ -152,7 +152,25 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 	clk_table = clk_data->hws;
 
 	pll_ref = devm_clk_get(dev, "pll_ref");
+	if (PTR_ERR(pll_ref) == -EPROBE_DEFER)
+		return -EPROBE_DEFER;
+
 	pll_in = devm_clk_get(dev, "pll_in");
+	if (PTR_ERR(pll_in) == -EPROBE_DEFER)
+		return -EPROBE_DEFER;
+
+	cdclk = devm_clk_get(dev, "cdclk");
+	if (PTR_ERR(cdclk) == -EPROBE_DEFER)
+		return -EPROBE_DEFER;
+
+	sclk_audio = devm_clk_get(dev, "sclk_audio");
+	if (PTR_ERR(sclk_audio) == -EPROBE_DEFER)
+		return -EPROBE_DEFER;
+
+	sclk_pcm_in = devm_clk_get(dev, "sclk_pcm_in");
+	if (PTR_ERR(sclk_pcm_in) == -EPROBE_DEFER)
+		return -EPROBE_DEFER;
+
 	if (!IS_ERR(pll_ref))
 		mout_audss_p[0] = __clk_get_name(pll_ref);
 	if (!IS_ERR(pll_in)) {
@@ -169,6 +187,12 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 			}
 		}
 	}
+	if (!IS_ERR(cdclk))
+		mout_i2s_p[1] = __clk_get_name(cdclk);
+	if (!IS_ERR(sclk_audio))
+		mout_i2s_p[2] = __clk_get_name(sclk_audio);
+	if (!IS_ERR(sclk_pcm_in))
+		sclk_pcm_p = __clk_get_name(sclk_pcm_in);
 
 	/*
 	 * Enable runtime PM here to allow the clock core using runtime PM
@@ -185,12 +209,6 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 				CLK_SET_RATE_NO_REPARENT | CLK_SET_RATE_PARENT,
 				reg_base + ASS_CLK_SRC, 0, 1, 0, &lock);
 
-	cdclk = devm_clk_get(dev, "cdclk");
-	sclk_audio = devm_clk_get(dev, "sclk_audio");
-	if (!IS_ERR(cdclk))
-		mout_i2s_p[1] = __clk_get_name(cdclk);
-	if (!IS_ERR(sclk_audio))
-		mout_i2s_p[2] = __clk_get_name(sclk_audio);
 	clk_table[EXYNOS_MOUT_I2S] = clk_hw_register_mux(dev, "mout_i2s",
 				mout_i2s_p, ARRAY_SIZE(mout_i2s_p),
 				CLK_SET_RATE_NO_REPARENT,
@@ -224,9 +242,6 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 				 "sclk_pcm", CLK_SET_RATE_PARENT,
 				reg_base + ASS_CLK_GATE, 4, 0, &lock);
 
-	sclk_pcm_in = devm_clk_get(dev, "sclk_pcm_in");
-	if (!IS_ERR(sclk_pcm_in))
-		sclk_pcm_p = __clk_get_name(sclk_pcm_in);
 	clk_table[EXYNOS_SCLK_PCM] = clk_hw_register_gate(dev, "sclk_pcm",
 				sclk_pcm_p, CLK_SET_RATE_PARENT,
 				reg_base + ASS_CLK_GATE, 5, 0, &lock);
-- 
2.43.0

[PATCH v2 2/3] 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")
Cc: stable@vger.kernel.org
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 373129847301..1d7eb98b72cf 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);
 
 	ctx->dev = dev;
 	samsung_clk_register_div(ctx, info->div_clks, info->nr_div_clks);
-- 
2.43.0

[PATCH v2 3/3] clk: samsung: exynos5-subcmu: Defer probe until power domains are available

[Marek Szyprowski]

Sub-CMU devices are added to their power domains with
of_genpd_add_device() during exynos5_clk_probe(). Its return value is
ignored, so if the power domain provider is not registered yet, the
sub-CMU device is silently left outside of its power domain.

Since commit c09a3e6c97f0 ("soc: samsung: pm_domains: Convert to regular
platform driver"), power domain providers are registered only when their
platform devices are probed, which happens after exynos5_clk_probe() has
already been called.

When a sub-CMU device is outside of its power domain, its runtime PM
callbacks race with the power domain state changes, which might result in
losing the proper hardware state. In practice, some clock muxes are left
switched to the oscillator instead of the respective PLLs.

Fix this by deferring exynos5_clk_probe() until the platform devices for
all available power domains are instantiated.

Fixes: c09a3e6c97f0 ("soc: samsung: pm_domains: Convert to regular platform driver")
Cc: stable@vger.kernel.org
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/clk/samsung/clk-exynos5-subcmu.c | 27 ++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos5-subcmu.c b/drivers/clk/samsung/clk-exynos5-subcmu.c
index 1d7eb98b72cf..fe67c3066f2e 100644
--- a/drivers/clk/samsung/clk-exynos5-subcmu.c
+++ b/drivers/clk/samsung/clk-exynos5-subcmu.c
@@ -6,6 +6,7 @@
 
 #include <linux/io.h>
 #include <linux/of.h>
+#include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/pm_domain.h>
 #include <linux/pm_runtime.h>
@@ -96,7 +97,7 @@ static int __maybe_unused exynos5_subcmu_resume(struct device *dev)
 	return 0;
 }
 
-static int __init exynos5_subcmu_probe(struct platform_device *pdev)
+static int exynos5_subcmu_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct exynos5_subcmu_info *info = dev_get_drvdata(dev);
@@ -122,7 +123,7 @@ static const struct dev_pm_ops exynos5_subcmu_pm_ops = {
 				     pm_runtime_force_resume)
 };
 
-static struct platform_driver exynos5_subcmu_driver __refdata = {
+static struct platform_driver exynos5_subcmu_driver = {
 	.driver	= {
 		.name = "exynos5-subcmu",
 		.suppress_bind_attrs = true,
@@ -131,7 +132,7 @@ static struct platform_driver exynos5_subcmu_driver __refdata = {
 	.probe = exynos5_subcmu_probe,
 };
 
-static int __init exynos5_clk_register_subcmu(struct device *parent,
+static int exynos5_clk_register_subcmu(struct device *parent,
 					 const struct exynos5_subcmu_info *info,
 					      struct device_node *pd_node)
 {
@@ -153,13 +154,27 @@ static int __init exynos5_clk_register_subcmu(struct device *parent,
 	return ret;
 }
 
-static int __init exynos5_clk_probe(struct platform_device *pdev)
+static int exynos5_clk_probe(struct platform_device *pdev)
 {
+	static const char pd_node_name[] = "samsung,exynos4210-pd";
 	struct device_node *np;
 	const char *name;
 	int i;
 
-	for_each_compatible_node(np, NULL, "samsung,exynos4210-pd") {
+	/* Defer until all needed power domains are registered */
+	for_each_compatible_node(np, NULL, pd_node_name) {
+		if (of_device_is_available(np)) {
+			struct platform_device *pd = of_find_device_by_node(np);
+
+			if (!pd) {
+				of_node_put(np);
+				return -EPROBE_DEFER;
+			}
+			put_device(&pd->dev);
+		}
+	}
+
+	for_each_compatible_node(np, NULL, pd_node_name) {
 		if (of_property_read_string(np, "label", &name) < 0)
 			continue;
 		for (i = 0; i < nr_cmus; i++)
@@ -177,7 +192,7 @@ static const struct of_device_id exynos5_clk_of_match[] = {
 	{ },
 };
 
-static struct platform_driver exynos5_clk_driver __refdata = {
+static struct platform_driver exynos5_clk_driver = {
 	.driver	= {
 		.name = "exynos5-clock",
 		.of_match_table = exynos5_clk_of_match,
-- 
2.43.0

Re: [PATCH v2 3/3] clk: samsung: exynos5-subcmu: Defer probe until power domains are available

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] clk: samsung: exynos5-subcmu: section mismatch and init UAF in exynos5_subcmu_probe()
--

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

clk: samsung: exynos5-subcmu: Defer probe until power domains are available

This commit defers exynos5_clk_probe() until all available power domain
platform devices are instantiated so that sub-CMU devices can be properly
added to their power domains. To support deferred probing, it also drops the
__init and __refdata annotations from the probe functions and platform
drivers in clk-exynos5-subcmu.c.

> diff --git a/drivers/clk/samsung/clk-exynos5-subcmu.c b/drivers/clk/samsung/clk-exynos5-subcmu.c
> index 1d7eb98b72cf8..fe67c3066f2ef 100644
> --- a/drivers/clk/samsung/clk-exynos5-subcmu.c
> +++ b/drivers/clk/samsung/clk-exynos5-subcmu.c

[ ... ]

> @@ -96,7 +97,7 @@ static int __maybe_unused exynos5_subcmu_resume(struct device *dev)
>  	return 0;
>  }
>  
> -static int __init exynos5_subcmu_probe(struct platform_device *pdev)
> +static int exynos5_subcmu_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct exynos5_subcmu_info *info = dev_get_drvdata(dev);

[Severity: High]
Does removing __init from exynos5_subcmu_probe() introduce a section mismatch
and a potential use-after-free of init memory if probing is deferred past
free_initmem()?

In drivers/clk/samsung/clk-exynos5-subcmu.c, exynos5_subcmu_probe() calls
samsung_clk_register_div() and samsung_clk_register_gate(), passing
info->div_clks and info->gate_clks:

    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;

However, in drivers/clk/samsung/clk.c, both registration helpers are still
marked __init:

    void __init samsung_clk_register_div(struct samsung_clk_provider *ctx,
                    const struct samsung_div_clock *list,
                    unsigned int nr_clk)

    void __init samsung_clk_register_gate(struct samsung_clk_provider *ctx,
                    const struct samsung_gate_clock *list,
                    unsigned int nr_clk)

In addition, the sub-CMU clock arrays passed through info->div_clks and
info->gate_clks in drivers/clk/samsung/clk-exynos5250.c
(exynos5250_disp_gate_clks) and drivers/clk/samsung/clk-exynos5420.c
(exynos5x_disp_div_clks, exynos5x_disp_gate_clks, exynos5x_gsc_div_clks,
exynos5x_gsc_gate_clks, exynos5x_g3d_gate_clks, exynos5x_mfc_div_clks,
exynos5x_mfc_gate_clks, exynos5x_mscl_gate_clks, exynos5x_mscl_div_clks, and
exynos5800_mau_gate_clks) are still marked __initconst.

Should the __init and __initconst annotations also be removed from those
registration functions and clock tables so they remain valid when
exynos5_subcmu_probe() runs from normal .text or after init memory is freed?

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