Re: [PATCH - URGENT] pmdomains: mtk-pm-domains: Fix spinlock recursion in probe

["Macpaul Lin (林智斌)" <Macpaul.Lin@mediatek.com>]

On Fri, 2025-11-21 at 14:35 +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 Angelo,
> 
> On Fri, 2025-11-21 at 13:52 +0100, AngeloGioacchino Del Regno wrote:
> > For some reason, of_find_node_with_property() is creating a
> > spinlock
> > recursion issue along with fwnode_count_parents(), and this issue
> > is making all MediaTek boards unbootable.
> > 
> > As of kernel v6.18-rc6, there are only three users of this
> > function,
> > one of which is this driver.
> > 
> > Migrate away from of_find_node_with_property() by adding a local
> > scpsys_get_legacy_regmap_node() function, which acts similarly to
> > of_find_node_with_property(), and calling the former in place of
> > the latter.
> > 
> > This resolves the following spinlock recursion issue:
> > 
> > [    1.773979] BUG: spinlock recursion on CPU#2, kworker/u24:1/60
> > [    1.790485]  lock: devtree_lock+0x0/0x40, .magic: dead4ead,
> > .owner: kworker/u24:1/60, .owner_cpu: 2
> > [    1.791644] CPU: 2 UID: 0 PID: 60 Comm: kworker/u24:1 Tainted:
> > G        W           6.18.0-rc6 #3 PREEMPT
> > [    1.791649] Tainted: [W]=WARN
> > [    1.791650] Hardware name: MediaTek Genio-510 EVK (DT)
> > [    1.791653] Workqueue: events_unbound deferred_probe_work_func
> > [    1.791658] Call trace:
> > [    1.791659]  show_stack+0x18/0x30 (C)
> > [    1.791664]  dump_stack_lvl+0x68/0x94
> > [    1.791668]  dump_stack+0x18/0x24
> > [    1.791672]  spin_dump+0x78/0x88
> > [    1.791678]  do_raw_spin_lock+0x110/0x140
> > [    1.791684]  _raw_spin_lock_irqsave+0x58/0x6c
> > [    1.791690]  of_get_parent+0x28/0x74
> > [    1.791694]  of_fwnode_get_parent+0x38/0x7c
> > [    1.791700]  fwnode_count_parents+0x34/0xf0
> > [    1.791705]  fwnode_full_name_string+0x28/0x120
> > [    1.791710]  device_node_string+0x3e4/0x50c
> > [    1.791715]  pointer+0x294/0x430
> > [    1.791718]  vsnprintf+0x21c/0x5bc
> > [    1.791722]  vprintk_store+0x108/0x47c
> > [    1.791728]  vprintk_emit+0xc4/0x350
> > [    1.791732]  vprintk_default+0x34/0x40
> > [    1.791736]  vprintk+0x24/0x30
> > [    1.791740]  _printk+0x60/0x8c
> > [    1.791744]  of_node_release+0x154/0x194
> > [    1.791749]  kobject_put+0xa0/0x120
> > [    1.791753]  of_node_put+0x18/0x28
> > [    1.791756]  of_find_node_with_property+0x74/0x100
> > [    1.791761]  scpsys_probe+0x338/0x5e0
> > [    1.791765]  platform_probe+0x5c/0xa4
> > [    1.791770]  really_probe+0xbc/0x2ac
> > [    1.791774]  __driver_probe_device+0x78/0x118
> > [    1.791779]  driver_probe_device+0x3c/0x170
> > [    1.791783]  __device_attach_driver+0xb8/0x150
> > [    1.791788]  bus_for_each_drv+0x88/0xe8
> > [    1.791792]  __device_attach+0x9c/0x1a0
> > [    1.791796]  device_initial_probe+0x14/0x20
> > [    1.791801]  bus_probe_device+0xa0/0xa4
> > [    1.791805]  deferred_probe_work_func+0x88/0xd0
> > [    1.791809]  process_one_work+0x1e8/0x448
> > [    1.791813]  worker_thread+0x1ac/0x340
> > [    1.791816]  kthread+0x138/0x220
> > [    1.791821]  ret_from_fork+0x10/0x20
> > 
> > Fixes: c29345fa5f66 ("pmdomain: mediatek: Refactor bus protection
> > regmaps retrieval")
> > Signed-off-by: AngeloGioacchino Del Regno
> > <angelogioacchino.delregno@collabora.com>
> > ---
> > 
> > This might not be the best fix to perform, and I have Cc'ed Rob and
> > Krzysztof for them to have a look at the spinlock recursion issue,
> > as I think it either has to be fixed, or the affected function has
> > to be removed (or both, actually - in any case, if not fixed, this
> > could backfire, and I'm not sure that the only function that shows
> > this issue is just only of_find_node_with_property() or if others
> > also do; at least, nothing else on MediaTek machines as of now).
> > 
> > Counting that this makes *all* MediaTek machines to be unbootable,
> > I'd prefer this fix to get merged immediately - so that it lands
> > in v6.18 (which is at rc6 at the time of writing).
> > 
> > That - because I think that resolving the source of this issue will
> > take a bit of time and research - and I really don't want the LTS
> > kernel to be broken on all MediaTek devices.
> > 
> > Note: When I initially tested the commit pointed out in the Fixes
> > tag, it was working with no spinlock recursion - I don't clearly
> > remember what kernel version was that, but I think it was some
> > linux-next around v6.16 or v6.17.
> > 
> >  drivers/pmdomain/mediatek/mtk-pm-domains.c | 18 +++++++++++++++---
> >  1 file changed, 15 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/pmdomain/mediatek/mtk-pm-domains.c
> > b/drivers/pmdomain/mediatek/mtk-pm-domains.c
> > index 407b4a7aba10..721224c89865 100644
> > --- a/drivers/pmdomain/mediatek/mtk-pm-domains.c
> > +++ b/drivers/pmdomain/mediatek/mtk-pm-domains.c
> > @@ -1006,6 +1006,18 @@ 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] = {
> > @@ -1027,7 +1039,7 @@ 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 = of_find_node_with_property(np, "mediatek,infracfg");
> > +     node = scpsys_get_legacy_regmap(np, "mediatek,infracfg");
> >       if (node) {
> >               regmap[0] = syscon_regmap_lookup_by_phandle(node,
> > "mediatek,infracfg");
> >               of_node_put(node);
> > @@ -1040,7 +1052,7 @@ static int
> > scpsys_get_bus_protection_legacy(struct device *dev, struct scpsys
> > *s
> >               regmap[0] = NULL;
> >       }
> > 
> > -     node = of_find_node_with_property(np, "mediatek,smi");
> > +     node = scpsys_get_legacy_regmap(np, "mediatek,smi");
> >       if (node) {
> >               smi_np = of_parse_phandle(node, "mediatek,smi", 0);
> >               of_node_put(node);
> > @@ -1058,7 +1070,7 @@ static int
> > scpsys_get_bus_protection_legacy(struct device *dev, struct scpsys
> > *s
> >               regmap[1] = NULL;
> >       }
> > 
> > -     node = of_find_node_with_property(np, "mediatek,infracfg-
> > nao");
> > +     node = scpsys_get_legacy_regmap(np, "mediatek,infracfg-
> > nao");
> >       if (node) {
> >               regmap[2] = syscon_regmap_lookup_by_phandle(node,
> > "mediatek,infracfg-nao");
> >               num_regmaps++;
> I've tested your patch on top of a v6.18-rc6 kernel with a default
> config. My Mediatek Genio 350-EVK (MT8365), 510-EVK (MT8370) and
> 1200-
> EVK (MT8395) boards are all now booting without issue.
> 
> Tested-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
> 
> Regards,
> Louis-Alexis
> 

I've tested it on Genio 1200-EVK (MT8395) and 700-EVK (MT8390) with
linux-next:
tag: 6.18.0-rc6-next-20251121
Hash: 92c04477931397725f0f022b5502121adf4b8abd

Thanks for digging out the where the problem is.
It's quite not easy to suspect of_* API would affect spinlock issue
here. I'll try to forward it to some related colleagues however I'm not
sure if I could find anyone had deal this issue before.

Tested-by: Macpaul Lin <macpaul.lin@mediatek.com>
Reviewed-by: Macpaul Lin <macpaul.lin@mediatek.com>

Best regards,
Macpaul Lin