Linux Power Management development
 help / color / mirror / Atom feed
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

  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