linux-kernel - Re: [PATCH] pmdomains: mtk-pm-domains: improve spinlock recursion fix in probe

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <34bdaffe2d3179accfc39ffd92d0083ba31bb124.camel@mediatek.com>
Date: Fri, 28 Nov 2025 02:51:31 +0000
From: Macpaul Lin (林智斌) <Macpaul.Lin@...iatek.com>
To: "linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
	"ulf.hansson@...aro.org" <ulf.hansson@...aro.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "wenst@...omium.org"
	<wenst@...omium.org>, "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	Nicolas Prado <nfraprado@...labora.com>, Louis-Alexis Eyraud
	<louisalexis.eyraud@...labora.com>, "matthias.bgg@...il.com"
	<matthias.bgg@...il.com>
CC: Bear Wang (萩原惟德) <bear.wang@...iatek.com>,
	"krzk@...nel.org" <krzk@...nel.org>,
	Irving-CH Lin (林建弘)
	<Irving-CH.Lin@...iatek.com>, Project_Global_Chrome_Upstream_Group
	<Project_Global_Chrome_Upstream_Group@...iatek.com>,
	Ramax Lo (羅明遠) <Ramax.Lo@...iatek.com>,
	Weiyi Lu (呂威儀) <Weiyi.Lu@...iatek.com>, "Jian Hui
 Lee" <jianhui.lee@...onical.com>, Pablo Sun (孫毓翔)
	<pablo.sun@...iatek.com>, "conor@...nel.org" <conor@...nel.org>,
	"macpaul@...il.com" <macpaul@...il.com>
Subject: Re: [PATCH] pmdomains: mtk-pm-domains: improve spinlock recursion fix
 in probe

On Wed, 2025-11-26 at 13:51 +0100, Louis-Alexis Eyraud wrote:
> 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Hi Macpaul,
> 
> On Tue, 2025-11-25 at 18:56 +0800, Macpaul Lin wrote:
> > Remove scpsys_get_legacy_regmap() and update usages with
> > of_find_node_with_property(). Use an explicit of_node_get(np) to
> > ensure
> > correct node referencing against of_node_put() and ensuring it is
> > called
> > in a safe context (i.e., not while holding devtree_lock).
> > 
> > If fwnode_count_parents() obtains parent nodes via
> > fwnode_for_each_parent_node() and this process requires device tree
> > operations, it may result in repeated acquisition of devtree_lock
> > in
> > the same thread/context, leading to spinlock recursion errors.
> > 
> > Signed-off-by: Macpaul Lin <macpaul.lin@...iatek.com>
> > ---
> >  drivers/pmdomain/mediatek/mtk-pm-domains.c | 21 ++++++------------
> > --
> > -
> >  1 file changed, 6 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/pmdomain/mediatek/mtk-pm-domains.c
> > b/drivers/pmdomain/mediatek/mtk-pm-domains.c
> > index 80561d27f2b2..f64f24d520dd 100644
> > --- a/drivers/pmdomain/mediatek/mtk-pm-domains.c
> > +++ b/drivers/pmdomain/mediatek/mtk-pm-domains.c
> > @@ -984,18 +984,6 @@ static void scpsys_domain_cleanup(struct
> > scpsys
> > *scpsys)
> >       }
> >  }
> > 
> > -static struct device_node *scpsys_get_legacy_regmap(struct
> > device_node *np, const char *pn)
> > -{
> > -     struct device_node *local_node;
> > -
> > -     for_each_child_of_node(np, local_node) {
> > -             if (of_property_present(local_node, pn))
> > -                     return local_node;
> > -     }
> > -
> > -     return NULL;
> > -}
> > -
> >  static int scpsys_get_bus_protection_legacy(struct device *dev,
> > struct scpsys *scpsys)
> >  {
> >       const u8 bp_blocks[3] = {
> > @@ -1017,7 +1005,8 @@ static int
> > scpsys_get_bus_protection_legacy(struct device *dev, struct scpsys
> > *s
> >        * this makes it then possible to allocate the array of
> > bus_prot
> >        * regmaps and convert all to the new style handling.
> >        */
> > -     node = scpsys_get_legacy_regmap(np, "mediatek,infracfg");
> > +     of_node_get(np);
> > +     node = of_find_node_with_property(np, "mediatek,infracfg");
> 
> with a kernel based on next-20251125 plus this patch, my Mediatek
> Genio
> 350, 510 and 1200 EVK boards booted OK.
> 

Thanks for helping the verification work on all platforms.
Well, should I add a Tested-by: tag from you for the next patch?

> About the patch itself, it looks that you try to balance the
> of_node_put() done by of_find_node_with_property() on its `from`
> parameter (np in this case):
> https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v6.18-rc7/source/drivers/of/base.c*L1066__;Iw!!CTRNKA9wMg0ARbw!nIEKBNc582OkFUWke5JW66FKC78AzItgTwlefoTjKiyAR8kzJOCcF-hTBwWVw2loY4mG4lY5AKvvBpCNTbXDoHbl8liqB8sS$
> 
> Your patch description is a bit unclear about that and could be
> reworded.

I'll try to refine and see if I could express it more strait forward.

> 
> I'm not sure if it is the proper way to use the
> of_find_node_with_property API, seems a bit hacky. Though, I found
> similar sequences for other of_find_* calls (like
> of_find_compatible_node or of_find_node_by_name) in the `from is not
> null` case but not all the times. Hope someone else can confirm if
> using of_node_get before calling these API is OK.
> 

Yeah. I'm totally agree with you. That's why I didn't add a 'Fixes:'
tag since we already have Angelo's fix in tree and we might be able to
have more discussion on this 'hacky' implementation. To ensure if we
want to migrate to native of_* APIs or keep some legacy functions on
this driver.

> Finally the patch also misses the Fixes tag since it is a follow up
> one.
> 

Okay, I'll add Fixes: tag in the v2. Thanks for review.

> Regards,
> Louis-Alexis
> 
> 

[snip]

Best regards,
Macpaul Lin