[PATCH] nubus: switch to dynamic root device

[PATCH] nubus: switch to dynamic root device

[Johan Hovold]

Driver core expects devices to be dynamically allocated and will, for
example, complain loudly if a device that lacks a release function is
ever freed.

Use root_device_register() to allocate and register the root device
instead of open coding using a static device.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/nubus/nubus.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/nubus/nubus.c b/drivers/nubus/nubus.c
index 559dce302d06..40ce4991c356 100644
--- a/drivers/nubus/nubus.c
+++ b/drivers/nubus/nubus.c
@@ -41,9 +41,7 @@ module_param_named(populate_procfs, nubus_populate_procfs, bool, 0);
 
 LIST_HEAD(nubus_func_rsrcs);
 
-static struct device nubus_parent = {
-	.init_name	= "nubus",
-};
+static struct device *nubus_parent;
 
 /* Meaning of "bytelanes":
 
@@ -833,7 +831,7 @@ static void __init nubus_add_board(int slot, int bytelanes)
 		list_add_tail(&fres->list, &nubus_func_rsrcs);
 	}
 
-	if (nubus_device_register(&nubus_parent, board))
+	if (nubus_device_register(nubus_parent, board))
 		put_device(&board->dev);
 }
 
@@ -880,18 +878,17 @@ static void __init nubus_scan_bus(void)
 
 static int __init nubus_init(void)
 {
-	int err;
-
 	if (!MACH_IS_MAC)
 		return 0;
 
 	nubus_proc_init();
-	err = device_register(&nubus_parent);
-	if (err) {
-		put_device(&nubus_parent);
-		return err;
-	}
+
+	nubus_parent = root_device_register("nubus");
+	if (IS_ERR(nubus_parent))
+		return PTR_ERR(nubus_parent);
+
 	nubus_scan_bus();
+
 	return 0;
 }
 
-- 
2.53.0

Re: [PATCH] nubus: switch to dynamic root device

[Finn Thain]

On Fri, 24 Apr 2026, Johan Hovold wrote:

> Driver core expects devices to be dynamically allocated and will, for
> example, complain loudly if a device that lacks a release function is
> ever freed.
> 

Yes, in drivers/base/core.c, there is a warning in device_release().

WARN(1, KERN_ERR "Device '%s' does not have a release() 
	function, it is broken and must be fixed. See 
	Documentation/core-api/kobject.rst.\n",

But there's no way for the refcount for the nubus parent device to reach 
zero that I can see. Did I miss something?

> Use root_device_register() to allocate and register the root device
> instead of open coding using a static device.
> 

Well, dynamic allocation makes sense for busses that might be instantiated 
more than once. But I don't know of any hardware like that. The nubus 
parent device is a singleton.

I suppose I could see some benefit to converting CONFIG_NUBUS into a 
tristate. Maybe the module link in root_device_register() would become 
applicable if someone wanted to do that conversion.

Re: [PATCH] nubus: switch to dynamic root device

[Johan Hovold]

On Sat, Apr 25, 2026 at 02:07:10PM +1000, Finn Thain wrote:
> On Fri, 24 Apr 2026, Johan Hovold wrote:
> 
> > Driver core expects devices to be dynamically allocated and will, for
> > example, complain loudly if a device that lacks a release function is
> > ever freed.
> > 
> 
> Yes, in drivers/base/core.c, there is a warning in device_release().
> 
> WARN(1, KERN_ERR "Device '%s' does not have a release() 
> 	function, it is broken and must be fixed. See 
> 	Documentation/core-api/kobject.rst.\n",
> 
> But there's no way for the refcount for the nubus parent device to reach 
> zero that I can see. Did I miss something?

It will if registration ever fails (e.g. due to fault injection or name
collision):

	err = device_register(&nubus_parent);
	if (err) {
		put_device(&nubus_parent);
		return err;
	}

Johan

Re: [PATCH] nubus: switch to dynamic root device

[Finn Thain]

On Mon, 27 Apr 2026, Johan Hovold wrote:

> On Sat, Apr 25, 2026 at 02:07:10PM +1000, Finn Thain wrote:
> > On Fri, 24 Apr 2026, Johan Hovold wrote:
> > 
> > > Driver core expects devices to be dynamically allocated and will, 
> > > for example, complain loudly if a device that lacks a release 
> > > function is ever freed.
> > > 
> > 
> > Yes, in drivers/base/core.c, there is a warning in device_release().
> > 
> > WARN(1, KERN_ERR "Device '%s' does not have a release()
> > 	function, it is broken and must be fixed. See 
> > 	Documentation/core-api/kobject.rst.\n",
> > 
> > But there's no way for the refcount for the nubus parent device to 
> > reach zero that I can see. Did I miss something?
> 
> It will if registration ever fails (e.g. due to fault injection or name 
> collision):
> 
> 	err = device_register(&nubus_parent);
> 	if (err) {
> 		put_device(&nubus_parent);
> 		return err;
> 	}
> 

In that situation the kernel error message would say the device "is broken 
and must be fixed" when it was deliberately broken by fault injection.
I think the commit log should state that the aim of your patch is to avoid 
that error message for that use-case.
Aside from that quibble, the patch looks fine to me.

Acked-by: Finn Thain <fthain@linux-m68k.org>

Re: [PATCH] nubus: switch to dynamic root device

[Johan Hovold]

On Tue, Apr 28, 2026 at 05:24:31PM +1000, Finn Thain wrote:
> On Mon, 27 Apr 2026, Johan Hovold wrote:
> > On Sat, Apr 25, 2026 at 02:07:10PM +1000, Finn Thain wrote:
> > > On Fri, 24 Apr 2026, Johan Hovold wrote:
> > > 
> > > > Driver core expects devices to be dynamically allocated and will, 
> > > > for example, complain loudly if a device that lacks a release 
> > > > function is ever freed.

> > > Yes, in drivers/base/core.c, there is a warning in device_release().
> > > 
> > > WARN(1, KERN_ERR "Device '%s' does not have a release()
> > > 	function, it is broken and must be fixed. See 
> > > 	Documentation/core-api/kobject.rst.\n",
> > > 
> > > But there's no way for the refcount for the nubus parent device to 
> > > reach zero that I can see. Did I miss something?
> > 
> > It will if registration ever fails (e.g. due to fault injection or name 
> > collision):
> > 
> > 	err = device_register(&nubus_parent);
> > 	if (err) {
> > 		put_device(&nubus_parent);
> > 		return err;
> > 	}
> > 
> 
> In that situation the kernel error message would say the device "is broken 
> and must be fixed" when it was deliberately broken by fault injection.
> I think the commit log should state that the aim of your patch is to avoid 
> that error message for that use-case.

No, the aim is to git rid of statically allocated device structures. The
error message is there to notify people that the driver model expects
dynamically allocated objects (e.g. as documented in kobject.rst) even
if you're unlikely to hit it with this particular module.

> Aside from that quibble, the patch looks fine to me.
> 
> Acked-by: Finn Thain <fthain@linux-m68k.org>

Thanks for taking a look.

Johan

Re: [PATCH] nubus: switch to dynamic root device

[Finn Thain]

On Mon, 4 May 2026, Johan Hovold wrote:

> 
> the aim is to git rid of statically allocated device structures
> 

Indeed, and the aim of the commit log is to provide some rationale. When I 
reviewed your patch I found that it was missing. Like I said to you 
already, the nubus parent device is a singleton (just like the ISA parent 
device).

Re: [PATCH] nubus: switch to dynamic root device

[Geert Uytterhoeven]

On Fri, 24 Apr 2026 at 12:40, Johan Hovold <johan@kernel.org> wrote:
> Driver core expects devices to be dynamically allocated and will, for
> example, complain loudly if a device that lacks a release function is
> ever freed.
>
> Use root_device_register() to allocate and register the root device
> instead of open coding using a static device.
>
> Signed-off-by: Johan Hovold <johan@kernel.org>

Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
i.e. will queue in the m68k tree for v7.2.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds