[PATCH 0/2] Error and deferred probe fixes to Rockchip PM domain driver

[PATCH 0/2] Error and deferred probe fixes to Rockchip PM domain driver

[Yao Zi]

These issues were found after I upgraded a RK3528 board to v7.3 kernel.
Here's a configuration that I used to reproduce both of these two
problems[1].

Thanks for your time and review.

[1]: https://gist.github.com/ziyao233/a5ee8862d1baf21ad756cf4e959e14c3

Yao Zi (2):
  pmdomain: rockchip: Only put clocks that have been successfully
    obtained
  pmdomain: rockchip: Don't log when of_clk_get() fails with
    -EPROBE_DEFER

 drivers/pmdomain/rockchip/pm-domains.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

-- 
2.56.0

[PATCH 1/2] pmdomain: rockchip: Only put clocks that have been successfully obtained

[Yao Zi]

Since pd->num_clks represents the number of clocks described in
devicetree, invoking clk_bulk_put() with it will put invalid pointers
when some clocks cannot be obtained, triggering a warning in CCF like,

  WARNING: drivers/clk/clk.c:4844 at __clk_put+0xf8/0x12c, CPU#0: swapper/0/1
  Call trace:
    __clk_put+0xf8/0x12c (P)
    clk_put+0x10/0x20
    clk_bulk_put+0x38/0x60
    rockchip_pm_add_one_domain+0x2b0/0x468
    rockchip_pm_domain_probe+0xd4/0x2f0
    platform_probe+0x80/0xac
    ...

Invoke clk_bulk_put() with the counter used in the loop of obtaining
clocks instead.

Fixes: d8b384777066 ("pmdomain: rockchip: fix clock leak on domain probe failure")
Signed-off-by: Yao Zi <me@ziyao.cc>
Cc: stable@vger.kernel.org
---
 drivers/pmdomain/rockchip/pm-domains.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
index db3879846db6..34c862e56aad 100644
--- a/drivers/pmdomain/rockchip/pm-domains.c
+++ b/drivers/pmdomain/rockchip/pm-domains.c
@@ -931,7 +931,7 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
 err_unprepare_clocks:
 	clk_bulk_unprepare(pd->num_clks, pd->clks);
 err_put_clocks:
-	clk_bulk_put(pd->num_clks, pd->clks);
+	clk_bulk_put(i, pd->clks);
 	return error;
 }
 
-- 
2.56.0

[PATCH 2/2] pmdomain: rockchip: Don't log when of_clk_get() fails with -EPROBE_DEFER

[Yao Zi]

Current error handling code considers -EPROBE_DEFER as hard errors,
logs misleading dmesg like,

rockchip-pm-domain ff600000.power-management:power-controller: power-domain: failed to get clk at index 0: -517
rockchip-pm-domain ff600000.power-management:power-controller: failed to handle node power-domain: -517

Use dev_err_probe() at the callsite of of_clk_get(), and check for
-EPROBE_DEFER in places where its return value could propagate to.

Fixes: 7c696693a4f5 ("soc: rockchip: power-domain: Add power domain driver")
Fixes: 6be05b5ec161 ("soc: rockchip: power-domain: add support for sub-power domains")
Signed-off-by: Yao Zi <me@ziyao.cc>
Cc: stable@vger.kernel.org
---
 drivers/pmdomain/rockchip/pm-domains.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
index 34c862e56aad..c147cc88edb1 100644
--- a/drivers/pmdomain/rockchip/pm-domains.c
+++ b/drivers/pmdomain/rockchip/pm-domains.c
@@ -851,9 +851,9 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
 		pd->clks[i].clk = of_clk_get(node, i);
 		if (IS_ERR(pd->clks[i].clk)) {
 			error = PTR_ERR(pd->clks[i].clk);
-			dev_err(pmu->dev,
-				"%pOFn: failed to get clk at index %d: %d\n",
-				node, i, error);
+			dev_err_probe(pmu->dev, error,
+				      "%pOFn: failed to get clk at index %d: %d\n",
+				      node, i, error);
 			goto err_put_clocks;
 		}
 	}
@@ -1006,8 +1006,10 @@ static int rockchip_pm_add_subdomain(struct rockchip_pmu *pmu,
 
 		error = rockchip_pm_add_one_domain(pmu, np);
 		if (error) {
-			dev_err(pmu->dev, "failed to handle node %pOFn: %d\n",
-				np, error);
+			if (error != -EPROBE_DEFER)
+				dev_err(pmu->dev,
+					"failed to handle node %pOFn: %d\n",
+					np, error);
 			return error;
 		}
 
@@ -1102,15 +1104,17 @@ static int rockchip_pm_domain_probe(struct platform_device *pdev)
 	for_each_available_child_of_node_scoped(np, node) {
 		error = rockchip_pm_add_one_domain(pmu, node);
 		if (error) {
-			dev_err(dev, "failed to handle node %pOFn: %d\n",
-				node, error);
+			if (error != -EPROBE_DEFER)
+				dev_err(dev, "failed to handle node %pOFn: %d\n",
+					node, error);
 			goto err_out;
 		}
 
 		error = rockchip_pm_add_subdomain(pmu, node);
 		if (error < 0) {
-			dev_err(dev, "failed to handle subdomain node %pOFn: %d\n",
-				node, error);
+			if (error != -EPROBE_DEFER)
+				dev_err(dev, "failed to handle subdomain node %pOFn: %d\n",
+					node, error);
 			goto err_out;
 		}
 	}
-- 
2.56.0

Re: [PATCH 2/2] pmdomain: rockchip: Don't log when of_clk_get() fails with -EPROBE_DEFER

[Sergey Shtylyov]

Hello!

   Please consider using --no-git-fallback with scripts/get_maintainer.pl --
I don't see what do I have to do with this patch (other than fixing another
bug in this driver earlier).

On 11/5/26 3:12 AM, Yao Zi wrote:

> Current error handling code considers -EPROBE_DEFER as hard errors,
> logs misleading dmesg like,
> 
> rockchip-pm-domain ff600000.power-management:power-controller: power-domain: failed to get clk at index 0: -517
> rockchip-pm-domain ff600000.power-management:power-controller: failed to handle node power-domain: -517
> 
> Use dev_err_probe() at the callsite of of_clk_get(),

> and check for -EPROBE_DEFER in places> where its return value could propagate to.

   To where?
   And I didn't understand why you couldn't just call dev_err_probe() everywhere --
the error code seems to always be propagated...

> Fixes: 7c696693a4f5 ("soc: rockchip: power-domain: Add power domain driver")
> Fixes: 6be05b5ec161 ("soc: rockchip: power-domain: add support for sub-power domains")
> Signed-off-by: Yao Zi <me@ziyao.cc>
> Cc: stable@vger.kernel.org
> ---
>  drivers/pmdomain/rockchip/pm-domains.c | 22 +++++++++++++---------
>  1 file changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
> index 34c862e56aad..c147cc88edb1 100644
> --- a/drivers/pmdomain/rockchip/pm-domains.c
> +++ b/drivers/pmdomain/rockchip/pm-domains.c
[...]> @@ -1006,8 +1006,10 @@ static int rockchip_pm_add_subdomain(struct rockchip_pmu *pmu,
> 
>                 error = rockchip_pm_add_one_domain(pmu, np);
>                 if (error) {
> -                       dev_err(pmu->dev, "failed to handle node %pOFn: %d\n",
> -                               np, error);
> +                       if (error != -EPROBE_DEFER)

   Why not just call dev_err_probe() instead?

> +                               dev_err(pmu->dev,
> +                                       "failed to handle node %pOFn: %d\n",
> +                                       np, error);
>                         return error;
>                 }
> 
> @@ -1102,15 +1104,17 @@ static int rockchip_pm_domain_probe(struct platform_device *pdev)
>         for_each_available_child_of_node_scoped(np, node) {
>                 error = rockchip_pm_add_one_domain(pmu, node);
>                 if (error) {
> -                       dev_err(dev, "failed to handle node %pOFn: %d\n",
> -                               node, error);
> +                       if (error != -EPROBE_DEFER)
> +                               dev_err(dev, "failed to handle node %pOFn: %d\n",
> +                                       node, error);
>                         goto err_out;
>                 }
> 
>                 error = rockchip_pm_add_subdomain(pmu, node);
>                 if (error < 0) {
> -                       dev_err(dev, "failed to handle subdomain node %pOFn: %d\n",
> -                               node, error);
> +                       if (error != -EPROBE_DEFER)
> +                               dev_err(dev, "failed to handle subdomain node %pOFn: %d\n",
> +                                       node, error);
>                         goto err_out;
>                 }
>         }
[...]

MBR, Sergey

Re: [PATCH 2/2] pmdomain: rockchip: Don't log when of_clk_get() fails with -EPROBE_DEFER

[Yao Zi]

On Thu, Nov 05, 2026 at 11:47:13PM +0300, Sergey Shtylyov wrote:
> Hello!
> 
>    Please consider using --no-git-fallback with scripts/get_maintainer.pl --
> I don't see what do I have to do with this patch (other than fixing another
> bug in this driver earlier).

Thanks for the reminder, I usually enable it but forgot it this time, sorry
for disturbing...

> On 11/5/26 3:12 AM, Yao Zi wrote:
> 
> > Current error handling code considers -EPROBE_DEFER as hard errors,
> > logs misleading dmesg like,
> > 
> > rockchip-pm-domain ff600000.power-management:power-controller: power-domain: failed to get clk at index 0: -517
> > rockchip-pm-domain ff600000.power-management:power-controller: failed to handle node power-domain: -517
> > 
> > Use dev_err_probe() at the callsite of of_clk_get(),
> 
> > and check for -EPROBE_DEFER in places> where its return value could propagate to.
> 
>    To where?

rockchip_pm_add_one_domain, rockchip_pm_add_subdomain, and
rockchip_pm_domain_probe. I think it's obvious in the diff and too
detailed to the commit message, so didn't add them here.

>    And I didn't understand why you couldn't just call dev_err_probe() everywhere --
> the error code seems to always be propagated...

This was what I originally wanted to do, but dev_err_probe() sets the
deferred probe reason, and if we call it several times, the earlier and
more specific messagess ("failed to get clk at index %d") get overridden
by the later and less descriptive ones ("failed to handle subdomain node
%pOFn"), which I think would make it harder to debug.

> > Fixes: 7c696693a4f5 ("soc: rockchip: power-domain: Add power domain driver")
> > Fixes: 6be05b5ec161 ("soc: rockchip: power-domain: add support for sub-power domains")
> > Signed-off-by: Yao Zi <me@ziyao.cc>
> > Cc: stable@vger.kernel.org
> > ---
> >  drivers/pmdomain/rockchip/pm-domains.c | 22 +++++++++++++---------
> >  1 file changed, 13 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
> > index 34c862e56aad..c147cc88edb1 100644
> > --- a/drivers/pmdomain/rockchip/pm-domains.c
> > +++ b/drivers/pmdomain/rockchip/pm-domains.c
> [...]> @@ -1006,8 +1006,10 @@ static int rockchip_pm_add_subdomain(struct rockchip_pmu *pmu,
> > 
> >                 error = rockchip_pm_add_one_domain(pmu, np);
> >                 if (error) {
> > -                       dev_err(pmu->dev, "failed to handle node %pOFn: %d\n",
> > -                               np, error);
> > +                       if (error != -EPROBE_DEFER)
> 
>    Why not just call dev_err_probe() instead?
> 
> > +                               dev_err(pmu->dev,
> > +                                       "failed to handle node %pOFn: %d\n",
> > +                                       np, error);
> >                         return error;
> >                 }
> > 
> > @@ -1102,15 +1104,17 @@ static int rockchip_pm_domain_probe(struct platform_device *pdev)
> >         for_each_available_child_of_node_scoped(np, node) {
> >                 error = rockchip_pm_add_one_domain(pmu, node);
> >                 if (error) {
> > -                       dev_err(dev, "failed to handle node %pOFn: %d\n",
> > -                               node, error);
> > +                       if (error != -EPROBE_DEFER)
> > +                               dev_err(dev, "failed to handle node %pOFn: %d\n",
> > +                                       node, error);
> >                         goto err_out;
> >                 }
> > 
> >                 error = rockchip_pm_add_subdomain(pmu, node);
> >                 if (error < 0) {
> > -                       dev_err(dev, "failed to handle subdomain node %pOFn: %d\n",
> > -                               node, error);
> > +                       if (error != -EPROBE_DEFER)
> > +                               dev_err(dev, "failed to handle subdomain node %pOFn: %d\n",
> > +                                       node, error);
> >                         goto err_out;
> >                 }
> >         }
> [...]
> 
> MBR, Sergey
> 

Regards,
Yao Zi