[PATCH] pmdomain: rockchip: fix domain check in rockchip_pm_add_one_domain()

[PATCH] pmdomain: rockchip: fix domain check in rockchip_pm_add_one_domain()

[Sergey Shtylyov]

In rockchip_pm_add_one_domain(), there's the check (most probably for
the case where the "reg" prop has an unexpected value?) which doesn't
make much sense as the variable pd_info will be NULL iff pmu->info->
domain_info is NULL and the variable id is 0.  What does make sense
there is checking pd_info->pwr_mask and pd_info->req_mask which (as
seems to follow from the code) can't both be 0 for a valid domain...

Found by Linux Verification Center (linuxtesting.org) with the Svace
static analysis tool.

Fixes: 7c696693a4f5 ("soc: rockchip: power-domain: Add power domain driver")
Signed-off-by: Sergey Shtylyov <s.shtylyov@auroraos.dev>

---
This patch is against the fixes branch of Ulf Hansson's linux-pm.git repo.

 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 ba66ae719428..69792e2b215b 100644
--- a/drivers/pmdomain/rockchip/pm-domains.c
+++ b/drivers/pmdomain/rockchip/pm-domains.c
@@ -809,7 +809,7 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
 		return 0;
 
 	pd_info = &pmu->info->domain_info[id];
-	if (!pd_info) {
+	if (!pd_info->pwr_mask && !pd_info->req_mask) {
 		dev_err(pmu->dev, "%pOFn: undefined domain id %d\n",
 			node, id);
 		return -EINVAL;
-- 
2.55.0

Re: [PATCH] pmdomain: rockchip: fix domain check in rockchip_pm_add_one_domain()

[Shawn Lin]

Hi Sergey,

在 2026/09/21 星期一 1:59, Sergey Shtylyov 写道:
> In rockchip_pm_add_one_domain(), there's the check (most probably for
> the case where the "reg" prop has an unexpected value?) which doesn't

Agreed that check indeed makes little sense. But "most probably for the
case where the reg prop has an unexpected value" does not hold: an 
out-of-range reg value is already rejected by the "id >= pmu->info-
 >num_domains" check above, and num_domains equals
ARRAY_SIZE(domain_info) for every SoC, so there are no holes in the
table that the new check could catch with the current data. AFAICT, this 
patch does not change any runtime behaviour; it only turns dead code
into a defensive check on the match-data tables.


> make much sense as the variable pd_info will be NULL iff pmu->info->
> domain_info is NULL and the variable id is 0.  What does make sense
> there is checking pd_info->pwr_mask and pd_info->req_mask which (as
> seems to follow from the code) can't both be 0 for a valid domain...
> 

That means there is no bug being fixed here, should we drop the fixes tag?

> Found by Linux Verification Center (linuxtesting.org) with the Svace
> static analysis tool.
> 
> Fixes: 7c696693a4f5 ("soc: rockchip: power-domain: Add power domain driver")
> Signed-off-by: Sergey Shtylyov <s.shtylyov@auroraos.dev>
> 
> ---
> This patch is against the fixes branch of Ulf Hansson's linux-pm.git repo.
> 
>   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 ba66ae719428..69792e2b215b 100644
> --- a/drivers/pmdomain/rockchip/pm-domains.c
> +++ b/drivers/pmdomain/rockchip/pm-domains.c
> @@ -809,7 +809,7 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
>   		return 0;
>   
>   	pd_info = &pmu->info->domain_info[id];
> -	if (!pd_info) {
> +	if (!pd_info->pwr_mask && !pd_info->req_mask) {
>   		dev_err(pmu->dev, "%pOFn: undefined domain id %d\n",
>   			node, id);
>   		return -EINVAL;

Re: [PATCH] pmdomain: rockchip: fix domain check in rockchip_pm_add_one_domain()

[Sergey Shtylyov]

On 9/21/26 9:36 AM, Shawn Lin wrote:

[...]

>> In rockchip_pm_add_one_domain(), there's the check (most probably for
>> the case where the "reg" prop has an unexpected value?) which doesn't
> 
> Agreed that check indeed makes little sense. But "most probably for the
> case where the reg prop has an unexpected value" does not hold: an

   I tried to guess what the check in question was actually intended for
(despite making a little sense as it is now)...

> out-of-range reg value is already rejected by the "id >= pmu->info-
>>num_domains" check above, and num_domains equals

   I noticed. :-)

> ARRAY_SIZE(domain_info) for every SoC, so there are no holes in the
> table that the new check could catch with the current data. AFAICT, this

   Oh, there are holes! :-)
   If you look at (and behind) the indexed initializers of *_pm_domains[],
you'll see that the indexes don't always start at 0, and so the arrays are
actually sparse...

> patch does not change any runtime behaviour; it only turns dead code
> into a defensive check on the match-data tables.

   I hope you see now that this defensive check makes a bit more sense...

>> make much sense as the variable pd_info will be NULL iff pmu->info->
>> domain_info is NULL and the variable id is 0.  What does make sense
>> there is checking pd_info->pwr_mask and pd_info->req_mask which (as
>> seems to follow from the code) can't both be 0 for a valid domain...
> 
> That means there is no bug being fixed here, should we drop the fixes tag?

   I'd like to keep it (but that's up to the maintainers, of course)...

>> Found by Linux Verification Center (linuxtesting.org) with the Svace
>> static analysis tool.
>>
>> Fixes: 7c696693a4f5 ("soc: rockchip: power-domain: Add power domain driver")
>> Signed-off-by: Sergey Shtylyov <s.shtylyov@auroraos.dev>

[...]

MBR, Sergey

Re: [PATCH] pmdomain: rockchip: fix domain check in rockchip_pm_add_one_domain()

[Shawn Lin]

在 2026/09/22 星期二 0:13, Sergey Shtylyov 写道:
> On 9/21/26 9:36 AM, Shawn Lin wrote:
> 
> [...]
> 
>>> In rockchip_pm_add_one_domain(), there's the check (most probably for
>>> the case where the "reg" prop has an unexpected value?) which doesn't
>>
>> Agreed that check indeed makes little sense. But "most probably for the
>> case where the reg prop has an unexpected value" does not hold: an
> 
>     I tried to guess what the check in question was actually intended for
> (despite making a little sense as it is now)...
> 
>> out-of-range reg value is already rejected by the "id >= pmu->info-
>>> num_domains" check above, and num_domains equals
> 
>     I noticed. :-)
> 
>> ARRAY_SIZE(domain_info) for every SoC, so there are no holes in the
>> table that the new check could catch with the current data. AFAICT, this
> 
>     Oh, there are holes! :-)
>     If you look at (and behind) the indexed initializers of *_pm_domains[],
> you'll see that the indexes don't always start at 0, and so the arrays are
> actually sparse...

Looked more closely by each one, you're right.

> 
>> patch does not change any runtime behaviour; it only turns dead code
>> into a defensive check on the match-data tables.
> 
>     I hope you see now that this defensive check makes a bit more sense...
> 
>>> make much sense as the variable pd_info will be NULL iff pmu->info->
>>> domain_info is NULL and the variable id is 0.  What does make sense
>>> there is checking pd_info->pwr_mask and pd_info->req_mask which (as
>>> seems to follow from the code) can't both be 0 for a valid domain...
>>
>> That means there is no bug being fixed here, should we drop the fixes tag?
> 
>     I'd like to keep it (but that's up to the maintainers, of course)...

Sure, it's  Ulf's call. Anyway, feel free to add

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

> 
>>> Found by Linux Verification Center (linuxtesting.org) with the Svace
>>> static analysis tool.
>>>
>>> Fixes: 7c696693a4f5 ("soc: rockchip: power-domain: Add power domain driver")
>>> Signed-off-by: Sergey Shtylyov <s.shtylyov@auroraos.dev>
> 
> [...]
> 
> MBR, Sergey
> 
> 

Re: [PATCH] pmdomain: rockchip: fix domain check in rockchip_pm_add_one_domain()

[Sergey Shtylyov]

On 9/22/26 3:47 AM, Shawn Lin wrote:

[...]

>>>> In rockchip_pm_add_one_domain(), there's the check (most probably for
>>>> the case where the "reg" prop has an unexpected value?) which doesn't
>>>
>>> Agreed that check indeed makes little sense. But "most probably for the
>>> case where the reg prop has an unexpected value" does not hold: an
>>
>>     I tried to guess what the check in question was actually intended for
>> (despite making a little sense as it is now)...
>>
>>> out-of-range reg value is already rejected by the "id >= pmu->info-
>>>> num_domains" check above, and num_domains equals
>>
>>     I noticed. :-)
>>
>>> ARRAY_SIZE(domain_info) for every SoC, so there are no holes in the
>>> table that the new check could catch with the current data. AFAICT, this
>>
>>     Oh, there are holes! :-)
>>     If you look at (and behind) the indexed initializers of *_pm_domains[],
>> you'll see that the indexes don't always start at 0, and so the arrays are
>> actually sparse...
> 
> Looked more closely by each one, you're right.

   Perhaps I just should've been more verbose in the patch description...

>>> patch does not change any runtime behaviour; it only turns dead code
>>> into a defensive check on the match-data tables.
>>
>>     I hope you see now that this defensive check makes a bit more sense...
>>
>>>> make much sense as the variable pd_info will be NULL iff pmu->info->
>>>> domain_info is NULL and the variable id is 0.  What does make sense
>>>> there is checking pd_info->pwr_mask and pd_info->req_mask which (as
>>>> seems to follow from the code) can't both be 0 for a valid domain...
>>>
>>> That means there is no bug being fixed here, should we drop the fixes tag?
>>
>>     I'd like to keep it (but that's up to the maintainers, of course)...
> 
> Sure, it's  Ulf's call. Anyway, feel free to add
> 
> Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>

   Thank you!

[...]

MBR, Sergey

Re: [PATCH] pmdomain: rockchip: fix domain check in rockchip_pm_add_one_domain()

[Ulf Hansson]

On Sun, Sep 20, 2026 at 8:00 PM Sergey Shtylyov <s.shtylyov@auroraos.dev> wrote:
>
> In rockchip_pm_add_one_domain(), there's the check (most probably for
> the case where the "reg" prop has an unexpected value?) which doesn't
> make much sense as the variable pd_info will be NULL iff pmu->info->
> domain_info is NULL and the variable id is 0.  What does make sense
> there is checking pd_info->pwr_mask and pd_info->req_mask which (as
> seems to follow from the code) can't both be 0 for a valid domain...
>
> Found by Linux Verification Center (linuxtesting.org) with the Svace
> static analysis tool.
>
> Fixes: 7c696693a4f5 ("soc: rockchip: power-domain: Add power domain driver")
> Signed-off-by: Sergey Shtylyov <s.shtylyov@auroraos.dev>

Applied for next, but without the fixes tag, thanks!

Note, that I am dropping the fixes tag because if there was a real
problem, we would require yet another fix on top to get the
corresponding domain declaration to be correct. Yet, the patch is
useful as is!

Kind regards
Uffe


>
> ---
> This patch is against the fixes branch of Ulf Hansson's linux-pm.git repo.
>
>  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 ba66ae719428..69792e2b215b 100644
> --- a/drivers/pmdomain/rockchip/pm-domains.c
> +++ b/drivers/pmdomain/rockchip/pm-domains.c
> @@ -809,7 +809,7 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
>                 return 0;
>
>         pd_info = &pmu->info->domain_info[id];
> -       if (!pd_info) {
> +       if (!pd_info->pwr_mask && !pd_info->req_mask) {
>                 dev_err(pmu->dev, "%pOFn: undefined domain id %d\n",
>                         node, id);
>                 return -EINVAL;
> --
> 2.55.0

Re: [PATCH] pmdomain: rockchip: fix domain check in rockchip_pm_add_one_domain()

[Sergey Shtylyov]

On 9/23/26 3:14 PM, Ulf Hansson wrote:

[...]

>> In rockchip_pm_add_one_domain(), there's the check (most probably for
>> the case where the "reg" prop has an unexpected value?) which doesn't
>> make much sense as the variable pd_info will be NULL iff pmu->info->
>> domain_info is NULL and the variable id is 0.  What does make sense
>> there is checking pd_info->pwr_mask and pd_info->req_mask which (as
>> seems to follow from the code) can't both be 0 for a valid domain...
>>
>> Found by Linux Verification Center (linuxtesting.org) with the Svace
>> static analysis tool.
>>
>> Fixes: 7c696693a4f5 ("soc: rockchip: power-domain: Add power domain driver")
>> Signed-off-by: Sergey Shtylyov <s.shtylyov@auroraos.dev>
> 
> Applied for next, but without the fixes tag, thanks!
> 
> Note, that I am dropping the fixes tag because if there was a real
> problem, we would require yet another fix on top to get the
> corresponding domain declaration to be correct. Yet, the patch is
> useful as is!

   I'm not sure I understand you. The patch should hopefully be
enough to detect the undefined entries (implicitly init'ed with
all 0s). What fix for the domain definitions do you have in mind,
turning the arrays into lookup tables by adding a domain ID as a
field to *struct* rockchip_domain_info?

> Kind regards
> Uffe
MBR, Sergey

Re: [PATCH] pmdomain: rockchip: fix domain check in rockchip_pm_add_one_domain()

[Ulf Hansson]

On Wed, Sep 23, 2026 at 10:38 PM Sergey Shtylyov
<s.shtylyov@auroraos.dev> wrote:
>
> On 9/23/26 3:14 PM, Ulf Hansson wrote:
>
> [...]
>
> >> In rockchip_pm_add_one_domain(), there's the check (most probably for
> >> the case where the "reg" prop has an unexpected value?) which doesn't
> >> make much sense as the variable pd_info will be NULL iff pmu->info->
> >> domain_info is NULL and the variable id is 0.  What does make sense
> >> there is checking pd_info->pwr_mask and pd_info->req_mask which (as
> >> seems to follow from the code) can't both be 0 for a valid domain...
> >>
> >> Found by Linux Verification Center (linuxtesting.org) with the Svace
> >> static analysis tool.
> >>
> >> Fixes: 7c696693a4f5 ("soc: rockchip: power-domain: Add power domain driver")
> >> Signed-off-by: Sergey Shtylyov <s.shtylyov@auroraos.dev>
> >
> > Applied for next, but without the fixes tag, thanks!
> >
> > Note, that I am dropping the fixes tag because if there was a real
> > problem, we would require yet another fix on top to get the
> > corresponding domain declaration to be correct. Yet, the patch is
> > useful as is!
>
>    I'm not sure I understand you. The patch should hopefully be
> enough to detect the undefined entries (implicitly init'ed with
> all 0s). What fix for the domain definitions do you have in mind,
> turning the arrays into lookup tables by adding a domain ID as a
> field to *struct* rockchip_domain_info?

Apologize if I was vague, but you kind of point out what I just said.

The patch helps us to *detect* incorrect entries. If it turns out we
find one, we need to fix that entry.

Kind regards
Uffe

Re: [PATCH] pmdomain: rockchip: fix domain check in rockchip_pm_add_one_domain()

[Sergey Shtylyov]

On 9/24/26 11:48 AM, Ulf Hansson wrote:

[...]

>>>> In rockchip_pm_add_one_domain(), there's the check (most probably for
>>>> the case where the "reg" prop has an unexpected value?) which doesn't
>>>> make much sense as the variable pd_info will be NULL iff pmu->info->
>>>> domain_info is NULL and the variable id is 0.  What does make sense
>>>> there is checking pd_info->pwr_mask and pd_info->req_mask which (as
>>>> seems to follow from the code) can't both be 0 for a valid domain...
>>>>
>>>> Found by Linux Verification Center (linuxtesting.org) with the Svace
>>>> static analysis tool.
>>>>
>>>> Fixes: 7c696693a4f5 ("soc: rockchip: power-domain: Add power domain driver")
>>>> Signed-off-by: Sergey Shtylyov <s.shtylyov@auroraos.dev>
>>>
>>> Applied for next, but without the fixes tag, thanks!

   One detail I've just remembered about: if the patch won't be backported to
the stable kernels, it would be easier to just check rockchip_domain_info::name
for NULL instead of the {pwr,req}_mask fields checked by the patch; this field
seems to always be set for all domains the driver cares about, it just was added
in 5.14-rc1 (I think), see this:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0a69452e03564c5eaf99f729de398cd94ee90851

>>> Note, that I am dropping the fixes tag because if there was a real
>>> problem, we would require yet another fix on top to get the

   The problem can (probably) happen iff a device tree has a domain # in
the "reg" property that the driver doesn't care about -- in this case the
.{pwr,req}_mask fields (and .name as well) will both be 0 (since IIRC a
C compiler should set not explicitly initialized entried to 0). My patch
was intended for checking for the domain #s that the driver doesn't care
about (and so doesn't init in the *_pm_domains[] definitions, not for the
validity of the domains it does care about; try looking at at e.g.
include/dt-bindings/power/px30-power.h and you'll see that the "valid"
domains start at 5 (and the domain #s are not even contiguous as e.g.
PX30_PD_{CRYPTO,DDR} indices remain uninitialized)...

>>> corresponding domain declaration to be correct. Yet, the patch is
>>> useful as is!  >>>>    I'm not sure I understand you. The patch should hopefully be
>> enough to detect the undefined entries (implicitly init'ed with
>> all 0s). What fix for the domain definitions do you have in mind,
>> turning the arrays into lookup tables by adding a domain ID as a
>> field to *struct* rockchip_domain_info?
> 
> Apologize if I was vague, but you kind of point out what I just said.
> 
> The patch helps us to *detect* incorrect entries. If it turns out we
> find one, we need to fix that entry.

   No, the intent was not to detect the improperly initialized entries but
to verify the supplied DT is correct; I suppose this was the intention of
the original (bogus) check in rockchip_pm_add_one_domain()...

> Kind regards
> Uffe

MBR, Sergey

Re: [PATCH] pmdomain: rockchip: fix domain check in rockchip_pm_add_one_domain()

[Sergey Shtylyov]

On 9/23/26 3:14 PM, Ulf Hansson wrote:
[...]

>> In rockchip_pm_add_one_domain(), there's the check (most probably for
>> the case where the "reg" prop has an unexpected value?) which doesn't
>> make much sense as the variable pd_info will be NULL iff pmu->info->
>> domain_info is NULL and the variable id is 0.  What does make sense
>> there is checking pd_info->pwr_mask and pd_info->req_mask which (as
>> seems to follow from the code) can't both be 0 for a valid domain...
>>
>> Found by Linux Verification Center (linuxtesting.org) with the Svace
>> static analysis tool.
>>
>> Fixes: 7c696693a4f5 ("soc: rockchip: power-domain: Add power domain driver")
>> Signed-off-by: Sergey Shtylyov <s.shtylyov@auroraos.dev>
> 
> Applied for next, but without the fixes tag, thanks!
> 
> Note, that I am dropping the fixes tag because if there was a real
> problem, we would require yet another fix on top to get the
> corresponding domain declaration to be correct. Yet, the patch is
> useful as is!

   I was thinking of doing a more elaborate description but if you
considered it good enough, OK... :-)

> Kind regards
> Uffe
[...]

MBR, Sergey

Re: [PATCH] pmdomain: rockchip: fix domain check in rockchip_pm_add_one_domain()

[Sergey Shtylyov]

On 9/23/26 11:44 PM, Sergey Shtylyov wrote:
[...]

>>> In rockchip_pm_add_one_domain(), there's the check (most probably for
>>> the case where the "reg" prop has an unexpected value?) which doesn't

   By "unexpected value" I meant a domain that isn't controlled by this
driver and so have no explicit initializer in *_pm_domains[]. Looks like
for such domain #s rockchip_pm_add_one_domain() should fail with -EINVAL.

>>> make much sense as the variable pd_info will be NULL iff pmu->info->
>>> domain_info is NULL and the variable id is 0.  What does make sense
>>> there is checking pd_info->pwr_mask and pd_info->req_mask which (as
>>> seems to follow from the code) can't both be 0 for a valid domain...

   For the domains that have no matching initializer these fields should
be both 0. I had chosen checking these 2 fields instead of pd_info->name
as I intended this patch as a fix, easily backportable to the LTS kernels.

>>> Found by Linux Verification Center (linuxtesting.org) with the Svace
>>> static analysis tool.
>>>
>>> Fixes: 7c696693a4f5 ("soc: rockchip: power-domain: Add power domain driver")
>>> Signed-off-by: Sergey Shtylyov <s.shtylyov@auroraos.dev>
>>
>> Applied for next, but without the fixes tag, thanks!
>>
>> Note, that I am dropping the fixes tag because if there was a real
>> problem, we would require yet another fix on top to get the

   If a defective DT is considered an issue then there's a problem. :-)

>> corresponding domain declaration to be correct. Yet, the patch is
>> useful as is!

   Like I've already told, my aim was not detecting the bad declarations,
just bad DTs. I probably just wasn't elaborate/assertive enough...
>    I was thinking of doing a more elaborate description but if you
> considered it good enough, OK... :-)

   So what are you going to do with this, leave things as they are: the
patch merged to the next branch (in its current form) and not merged to
fixes? Perhaps a revert or an incremental patch?

>> Kind regards
>> Uffe
MBR, Sergey