[PATCH] nubus: switch to dynamic root device

Johan Hovold posted 1 patch 1 month, 3 weeks ago
drivers/nubus/nubus.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
[PATCH] nubus: switch to dynamic root device
Posted by Johan Hovold 1 month, 3 weeks ago
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
Posted by Geert Uytterhoeven 1 month, 2 weeks ago
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
Re: [PATCH] nubus: switch to dynamic root device
Posted by Finn Thain 1 month, 3 weeks ago
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
Posted by Johan Hovold 1 month, 3 weeks ago
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
Posted by Finn Thain 1 month, 3 weeks ago
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
Posted by Johan Hovold 1 month, 2 weeks ago
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
Posted by Finn Thain 1 month, 2 weeks ago
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).