From: Sergey Shtylyov <s.shtylyov@auroraos.dev>
To: Shawn Lin <shawn.lin@rock-chips.com>,
Ulf Hansson <ulfh@kernel.org>, Heiko Stuebner <heiko@sntech.de>,
<linux-pm@vger.kernel.org>, <linux-rockchip@lists.infradead.org>
Cc: Caesar Wang <wxt@rock-chips.com>,
Kevin Hilman <khilman@linaro.org>,
<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] pmdomain: rockchip: fix domain check in rockchip_pm_add_one_domain()
Date: Tue, 22 Sep 2026 19:04:08 +0300 [thread overview]
Message-ID: <f38e4d45-5a4b-4526-b630-7189bd187b77@auroraos.dev> (raw)
In-Reply-To: <1d8950b4-315f-44a4-9fc8-fcb35ac33043@rock-chips.com>
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
next prev parent reply other threads:[~2026-09-22 16:04 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-20 17:59 [PATCH] pmdomain: rockchip: fix domain check in rockchip_pm_add_one_domain() Sergey Shtylyov
2026-09-21 6:36 ` Shawn Lin
2026-09-21 16:13 ` Sergey Shtylyov
2026-09-22 0:47 ` Shawn Lin
2026-09-22 16:04 ` Sergey Shtylyov [this message]
2026-09-23 12:14 ` Ulf Hansson
2026-09-23 20:38 ` Sergey Shtylyov
2026-09-24 8:48 ` Ulf Hansson
2026-09-24 20:20 ` Sergey Shtylyov
2026-09-23 20:44 ` Sergey Shtylyov
2026-09-25 19:38 ` Sergey Shtylyov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f38e4d45-5a4b-4526-b630-7189bd187b77@auroraos.dev \
--to=s.shtylyov@auroraos.dev \
--cc=heiko@sntech.de \
--cc=khilman@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-pm@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=shawn.lin@rock-chips.com \
--cc=ulfh@kernel.org \
--cc=wxt@rock-chips.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox