[PATCH 1/4] pmdomain: rockchip: propagate subdomain add errors

[PATCH 1/4] pmdomain: rockchip: propagate subdomain add errors

[Shawn Lin]

rockchip_pm_add_subdomain() ignored the return value of its recursive
call, so a failure deeper in the hierarchy was silently dropped and
probe still succeeded with an incomplete domain tree.

Fixes: 6be05b5ec161 ("soc: rockchip: power-domain: add support for sub-power domains")
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/pmdomain/rockchip/pm-domains.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
index ba66ae7..068dd4b3 100644
--- a/drivers/pmdomain/rockchip/pm-domains.c
+++ b/drivers/pmdomain/rockchip/pm-domains.c
@@ -1018,7 +1018,9 @@ static int rockchip_pm_add_subdomain(struct rockchip_pmu *pmu,
 				parent_domain->name, child_domain->name);
 		}
 
-		rockchip_pm_add_subdomain(pmu, np);
+		error = rockchip_pm_add_subdomain(pmu, np);
+		if (error)
+			return error;
 	}
 
 	return 0;
-- 
2.7.4

Re: [PATCH 1/4] pmdomain: rockchip: propagate subdomain add errors

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1189 bytes --]

Hi,

On Fri, Sep 18, 2026 at 08:07:35PM +0800, Shawn Lin wrote:
> rockchip_pm_add_subdomain() ignored the return value of its recursive
> call, so a failure deeper in the hierarchy was silently dropped and
> probe still succeeded with an incomplete domain tree.
> 
> Fixes: 6be05b5ec161 ("soc: rockchip: power-domain: add support for sub-power domains")
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Greetings,

-- Sebastian

>  drivers/pmdomain/rockchip/pm-domains.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
> index ba66ae7..068dd4b3 100644
> --- a/drivers/pmdomain/rockchip/pm-domains.c
> +++ b/drivers/pmdomain/rockchip/pm-domains.c
> @@ -1018,7 +1018,9 @@ static int rockchip_pm_add_subdomain(struct rockchip_pmu *pmu,
>  				parent_domain->name, child_domain->name);
>  		}
>  
> -		rockchip_pm_add_subdomain(pmu, np);
> +		error = rockchip_pm_add_subdomain(pmu, np);
> +		if (error)
> +			return error;
>  	}
>  
>  	return 0;
> -- 
> 2.7.4
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 2/4] pmdomain: rockchip: fix clock leak on domain probe failure

[Shawn Lin]

When of_clk_get() failed in the middle of the loop, the references of
the clocks acquired so far were leaked instead of being released by
the error path.

Fixes: d909072d0521 ("soc: rockchip: power-domain: use clk_bulk APIs")
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 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 068dd4b3..b6b099b 100644
--- a/drivers/pmdomain/rockchip/pm-domains.c
+++ b/drivers/pmdomain/rockchip/pm-domains.c
@@ -842,7 +842,7 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
 			dev_err(pmu->dev,
 				"%pOFn: failed to get clk at index %d: %d\n",
 				node, i, error);
-			return error;
+			goto err_put_clocks;
 		}
 	}
 
-- 
2.7.4

Re: [PATCH 2/4] pmdomain: rockchip: fix clock leak on domain probe failure

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1294 bytes --]

Hi,

On Fri, Sep 18, 2026 at 08:07:36PM +0800, Shawn Lin wrote:
> When of_clk_get() failed in the middle of the loop, the references of
> the clocks acquired so far were leaked instead of being released by
> the error path.
> 
> Fixes: d909072d0521 ("soc: rockchip: power-domain: use clk_bulk APIs")
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---

Right, but this should switch

pd->clks = devm_kcalloc(...);

to

pd->clks = devm_kzalloc(...);

at the same time to ensure that clk_bulk_put() does not run over
non-initialized data. Otherwise

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Greetings,

-- Sebastian

> 
>  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 068dd4b3..b6b099b 100644
> --- a/drivers/pmdomain/rockchip/pm-domains.c
> +++ b/drivers/pmdomain/rockchip/pm-domains.c
> @@ -842,7 +842,7 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
>  			dev_err(pmu->dev,
>  				"%pOFn: failed to get clk at index %d: %d\n",
>  				node, i, error);
> -			return error;
> +			goto err_put_clocks;
>  		}
>  	}
>  
> -- 
> 2.7.4
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/4] pmdomain: rockchip: fix clock leak on domain probe failure

[Shawn Lin]

Hi Sebastian

在 2026/09/19 星期六 1:21, Sebastian Reichel 写道:
> Hi,
> 
> On Fri, Sep 18, 2026 at 08:07:36PM +0800, Shawn Lin wrote:
>> When of_clk_get() failed in the middle of the loop, the references of
>> the clocks acquired so far were leaked instead of being released by
>> the error path.
>>
>> Fixes: d909072d0521 ("soc: rockchip: power-domain: use clk_bulk APIs")
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>> ---
> 
> Right, but this should switch
> 
> pd->clks = devm_kcalloc(...);
> 
> to
> 
> pd->clks = devm_kzalloc(...);
> 
> at the same time to ensure that clk_bulk_put() does not run over
> non-initialized data. Otherwise

Good catch, will improve it in v2.

Thanks.

> 
> Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> 
> Greetings,
> 
> -- Sebastian
> 
>>
>>   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 068dd4b3..b6b099b 100644
>> --- a/drivers/pmdomain/rockchip/pm-domains.c
>> +++ b/drivers/pmdomain/rockchip/pm-domains.c
>> @@ -842,7 +842,7 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
>>   			dev_err(pmu->dev,
>>   				"%pOFn: failed to get clk at index %d: %d\n",
>>   				node, i, error);
>> -			return error;
>> +			goto err_put_clocks;
>>   		}
>>   	}
>>   
>> -- 
>> 2.7.4
>>
>>

Re: [PATCH 2/4] pmdomain: rockchip: fix clock leak on domain probe failure

[Shawn Lin]

Hi Sebastian

在 2026/09/19 星期六 8:13, Shawn Lin 写道:
> Hi Sebastian
> 
> 在 2026/09/19 星期六 1:21, Sebastian Reichel 写道:
>> Hi,
>>
>> On Fri, Sep 18, 2026 at 08:07:36PM +0800, Shawn Lin wrote:
>>> When of_clk_get() failed in the middle of the loop, the references of
>>> the clocks acquired so far were leaked instead of being released by
>>> the error path.
>>>
>>> Fixes: d909072d0521 ("soc: rockchip: power-domain: use clk_bulk APIs")
>>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>>> ---
>>
>> Right, but this should switch
>>
>> pd->clks = devm_kcalloc(...);
>>
>> to
>>
>> pd->clks = devm_kzalloc(...);
>>
>> at the same time to ensure that clk_bulk_put() does not run over
>> non-initialized data. Otherwise
> 
> Good catch, will improve it in v2.
> 

I was too quick to response, but didn't looked into devm_kcalloc, just
guess it didn't zero the data from the name. However it does zero the
data by passing in __GFP_ZERO. So I need to respin v3 and drop
devm_kzalloc change.


> Thanks.
> 
>>
>> Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
>>
>> Greetings,
>>
>> -- Sebastian
>>
>>>
>>>   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 068dd4b3..b6b099b 100644
>>> --- a/drivers/pmdomain/rockchip/pm-domains.c
>>> +++ b/drivers/pmdomain/rockchip/pm-domains.c
>>> @@ -842,7 +842,7 @@ static int rockchip_pm_add_one_domain(struct 
>>> rockchip_pmu *pmu,
>>>               dev_err(pmu->dev,
>>>                   "%pOFn: failed to get clk at index %d: %d\n",
>>>                   node, i, error);
>>> -            return error;
>>> +            goto err_put_clocks;
>>>           }
>>>       }
>>> -- 
>>> 2.7.4
>>>
>>>
> 
> 

Re: [PATCH 2/4] pmdomain: rockchip: fix clock leak on domain probe failure

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1278 bytes --]

Hi,

On Sun, Sep 20, 2026 at 09:15:52AM +0800, Shawn Lin wrote:
> Hi Sebastian
> 
> 在 2026/09/19 星期六 8:13, Shawn Lin 写道:
> > Hi Sebastian
> > 
> > 在 2026/09/19 星期六 1:21, Sebastian Reichel 写道:
> > > Hi,
> > > 
> > > On Fri, Sep 18, 2026 at 08:07:36PM +0800, Shawn Lin wrote:
> > > > When of_clk_get() failed in the middle of the loop, the references of
> > > > the clocks acquired so far were leaked instead of being released by
> > > > the error path.
> > > > 
> > > > Fixes: d909072d0521 ("soc: rockchip: power-domain: use clk_bulk APIs")
> > > > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> > > > ---
> > > 
> > > Right, but this should switch
> > > 
> > > pd->clks = devm_kcalloc(...);
> > > 
> > > to
> > > 
> > > pd->clks = devm_kzalloc(...);
> > > 
> > > at the same time to ensure that clk_bulk_put() does not run over
> > > non-initialized data. Otherwise
> > 
> > Good catch, will improve it in v2.
> > 
> 
> I was too quick to response, but didn't looked into devm_kcalloc, just
> guess it didn't zero the data from the name. However it does zero the
> data by passing in __GFP_ZERO. So I need to respin v3 and drop
> devm_kzalloc change.

Sorry about that detour.

Greetings,

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 3/4] pmdomain: rockchip: don't ignore clock lookup errors on attach

[Shawn Lin]

rockchip_pd_attach_dev() treated every of_clk_get() error as the end
of the clock list, so a clock that exists but is not available yet
(e.g. -EPROBE_DEFER) was silently skipped and the device ended up
without pm_clk management.  Only -ENOENT means there are no more
clocks.

Fixes: 7c696693a4f5 ("soc: rockchip: power-domain: Add power domain driver")
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/pmdomain/rockchip/pm-domains.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
index b6b099b..e193f97 100644
--- a/drivers/pmdomain/rockchip/pm-domains.c
+++ b/drivers/pmdomain/rockchip/pm-domains.c
@@ -759,7 +759,19 @@ static int rockchip_pd_attach_dev(struct generic_pm_domain *genpd,
 	}
 
 	i = 0;
-	while ((clk = of_clk_get(dev->of_node, i++)) && !IS_ERR(clk)) {
+	while (1) {
+		clk = of_clk_get(dev->of_node, i++);
+		if (IS_ERR(clk)) {
+			error = PTR_ERR(clk);
+			if (error == -ENOENT)
+				break;
+
+			dev_err(dev, "failed to get clock %d: %d\n", i - 1,
+				error);
+			pm_clk_destroy(dev);
+			return error;
+		}
+
 		dev_dbg(dev, "adding clock '%pC' to list of PM clocks\n", clk);
 		error = pm_clk_add_clk(dev, clk);
 		if (error) {
-- 
2.7.4

Re: [PATCH 3/4] pmdomain: rockchip: don't ignore clock lookup errors on attach

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1570 bytes --]

Hi,

On Fri, Sep 18, 2026 at 08:07:37PM +0800, Shawn Lin wrote:
> rockchip_pd_attach_dev() treated every of_clk_get() error as the end
> of the clock list, so a clock that exists but is not available yet
> (e.g. -EPROBE_DEFER) was silently skipped and the device ended up
> without pm_clk management.  Only -ENOENT means there are no more
> clocks.
> 
> Fixes: 7c696693a4f5 ("soc: rockchip: power-domain: Add power domain driver")
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>

-- Sebastian

>  drivers/pmdomain/rockchip/pm-domains.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
> index b6b099b..e193f97 100644
> --- a/drivers/pmdomain/rockchip/pm-domains.c
> +++ b/drivers/pmdomain/rockchip/pm-domains.c
> @@ -759,7 +759,19 @@ static int rockchip_pd_attach_dev(struct generic_pm_domain *genpd,
>  	}
>  
>  	i = 0;
> -	while ((clk = of_clk_get(dev->of_node, i++)) && !IS_ERR(clk)) {
> +	while (1) {
> +		clk = of_clk_get(dev->of_node, i++);
> +		if (IS_ERR(clk)) {
> +			error = PTR_ERR(clk);
> +			if (error == -ENOENT)
> +				break;
> +
> +			dev_err(dev, "failed to get clock %d: %d\n", i - 1,
> +				error);
> +			pm_clk_destroy(dev);
> +			return error;
> +		}
> +
>  		dev_dbg(dev, "adding clock '%pC' to list of PM clocks\n", clk);
>  		error = pm_clk_add_clk(dev, clk);
>  		if (error) {
> -- 
> 2.7.4
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 4/4] pmdomain: rockchip: drop duplicated errno from supply failure message

[Shawn Lin]

dev_err_probe() already appends the error code.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 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 e193f97..dd5e2987 100644
--- a/drivers/pmdomain/rockchip/pm-domains.c
+++ b/drivers/pmdomain/rockchip/pm-domains.c
@@ -721,7 +721,7 @@ static int rockchip_pd_power_on(struct generic_pm_domain *domain)
 	ret = rockchip_pd_regulator_enable(pd);
 	if (ret)
 		return dev_err_probe(pd->pmu->dev, ret,
-				     "Failed to enable supply: %d\n", ret);
+				     "Failed to enable supply\n");
 
 	ret = rockchip_pd_power(pd, true);
 	if (ret)
-- 
2.7.4

Re: [PATCH 4/4] pmdomain: rockchip: drop duplicated errno from supply failure message

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 976 bytes --]

Hi,

On Fri, Sep 18, 2026 at 08:07:38PM +0800, Shawn Lin wrote:
> dev_err_probe() already appends the error code.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>

-- Sebastian

> 
>  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 e193f97..dd5e2987 100644
> --- a/drivers/pmdomain/rockchip/pm-domains.c
> +++ b/drivers/pmdomain/rockchip/pm-domains.c
> @@ -721,7 +721,7 @@ static int rockchip_pd_power_on(struct generic_pm_domain *domain)
>  	ret = rockchip_pd_regulator_enable(pd);
>  	if (ret)
>  		return dev_err_probe(pd->pmu->dev, ret,
> -				     "Failed to enable supply: %d\n", ret);
> +				     "Failed to enable supply\n");
>  
>  	ret = rockchip_pd_power(pd, true);
>  	if (ret)
> -- 
> 2.7.4
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]