Re: [RFT PATCH v3] ARM: omap1: enable real software node lookup of GPIOs on Nokia 770

[Dmitry Torokhov]

On Fri, Mar 27, 2026 at 11:26:02AM -0700, Bartosz Golaszewski wrote:
> On Fri, 27 Mar 2026 19:16:41 +0100, Dmitry Torokhov
> <dmitry.torokhov@xxxxxxxxx> said:
> > On Fri, Mar 27, 2026 at 10:27:07AM -0700, Bartosz Golaszewski wrote:
> >> On Fri, 27 Mar 2026 18:23:29 +0100, Bartosz Golaszewski <brgl@xxxxxxxxxx> said:
> >> > On Fri, Mar 27, 2026 at 5:59 PM Aaro Koskinen <aaro.koskinen@xxxxxx> wrote:
> >> >>
> >> >> Hi,
> >> >>
> >> >> On Fri, Mar 27, 2026 at 03:22:12PM +0100, Bartosz Golaszewski wrote:
> >> >> > Hmm, I'm wondering if there's a race with consumers already requesting
> >> >> > the GPIOs after the controller device is registered but before the
> >> >> > software node is added. I'll send a version with software nodes being
> >> >> > registered first, then passes as firmware nodes to the platform device
> >> >> > API before the device is registered.
> >> >>
> >> >> It crashes early, I was able to get an UART log from OSK (another
> >> >> 16xx board):
> >> >>
> >> >> [    1.001525] Register r12 information: 2-page vmalloc region starting at 0xc2808000 allocated at kernel_clone+0xa4/0x20c
> >> >> [    1.013092] Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
> >> >> [    1.019500] Stack: (0xc2809ed0 to 0xc280a000)
> >> >> [    1.024230] 9ec0:                                     c072d000 c0529474 c06b3aa0 c050a3cc
> >> >> [    1.032958] 9ee0: c072d000 c085c000 00000002 c052582c c050a324 c072d000 00000000 c0503160
> >> >> [    1.041687] 9f00: 00002710 00000000 c04da8f8 c0060900 c2809f64 ffffffff 00010000 946f70b5
> >> >> [    1.050384] 9f20: 00000062 c0816120 00000002 c052582c c0525848 c072d000 c04da8f8 c0060a18
> >> >> [    1.059112] 9f40: c2809f64 c2809f64 00000000 946f70b5 00000062 c0816120 00000002 c052582c
> >> >> [    1.067810] 9f60: c052584c c072d000 c04da8f8 c050352c 00000002 00000002 00000000 c0502400
> >> >> [    1.076507] 9f80: c2809f7c 00000000 c03f86f4 00000000 00000000 00000000 00000000 00000000
> >> >> [    1.085205] 9fa0: 00000000 c03f8704 00000000 c000850c 00000000 00000000 00000000 00000000
> >> >> [    1.093902] 9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> >> >> [    1.102600] 9fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
> >> >> [    1.111206] Call trace:
> >> >> [    1.111328]  software_node_to_swnode from device_add_software_node+0x20/0x80
> >> >> [    1.121704]  device_add_software_node from omap16xx_gpio_init+0xa8/0xe4
> >> >> [    1.128997]  omap16xx_gpio_init from do_one_initcall+0x68/0x1f4
> >> >> [    1.135620]  do_one_initcall from kernel_init_freeable+0x1ec/0x240
> >> >> [    1.142517]  kernel_init_freeable from kernel_init+0x10/0x108
> >> >> [    1.148864]  kernel_init from ret_from_fork+0x14/0x28
> >> >> [    1.154357] Exception stack(0xc2809fb0 to 0xc2809ff8)
> >> >> [    1.159820] 9fa0:                                     00000000 00000000 00000000 00000000
> >> >> [    1.168518] 9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> >> >> [    1.177185] 9fe0: 00000000 00000000 00000000 00000000 00000013 00000000
> >> >> [    1.184295] Code: e3500000 012fff1e e59f3034 e5932000 (e5923000)
> >> >> [    1.191040] ---[ end trace 0000000000000000 ]---
> >> >> [    1.196350] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> >> >> [    1.204559] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]---
> >> >>
> >> >
> >> > Thanks. This makes sense. Both omap16xx_gpio_init() and
> >> > software_node_init() run as postcore_initcall() so if the order is not
> >> > right, it will fail.
> >> >
> >> > Cc'ing Andy who's a reviewer for software nodes. Andy: is there any
> >> > reason to run software_node_init() as a postcore initcall? It only
> >> > allocates the kset, can we move it to core_initcall() by any chance?
> >> >
> >> > Bart
> >> >
> >>
> >> In any case, Aaro: the following should theoretically fix it:
> >>
> >> diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> >> index 51320837f3a9..5ba904f8a08a 100644
> >> --- a/drivers/base/swnode.c
> >> +++ b/drivers/base/swnode.c
> >> @@ -1134,7 +1134,7 @@ static int __init software_node_init(void)
> >>  		return -ENOMEM;
> >>  	return 0;
> >>  }
> >> -postcore_initcall(software_node_init);
> >> +core_initcall(software_node_init);
> >
> > This is wrong direction IMO. The matching by label is working now, so
> > there is no reason to rush this change into the kernel, it is simply a
> > cleanup.
> >
> > Wait until the resolution of my pact allowing using node names, or your
> > proposal that installs and attaches nodes dynamically based on bus
> > notifiers (or both) and then make the conversion. Then you will not need
> > to move init order around, add new dependencies between drivers, and so
> > on.
> >
> 
> Hi Dmitry,
> 
> I'm not rushing anything. It's fine to delay this for now. But the new solution
> you're mentioning (whatever it will end up to be) is something we should use
> when there's only a lose link between the supplier and consumer. Here both the
> GPIO controller and the consumer are instantiated from a board file and are
> built-in so there's no link-order problem and we can directly reference one
> software node from another. It's a different kind of problem.
> 
> I'd like to figure out whether software nodes are not made available too late
> into the boot process for no reason and if maybe it makes sense to initialize
> them earlier.

Actually you are right. I wonder if we should not stuff the call to
initialize software nodes into the same place we are calling
of_core_init(): drivers/base/init.c:driver_init().

Thanks.

-- 
Dmitry