[PATCH] isa: switch to dynamic root device

[PATCH] isa: 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.

Note that this also fixes a reference leak in case device_register()
fails which may be flagged by static checkers.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/base/isa.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/base/isa.c b/drivers/base/isa.c
index fd076cc63cb6..5887e4211f80 100644
--- a/drivers/base/isa.c
+++ b/drivers/base/isa.c
@@ -11,9 +11,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/isa.h>
 
-static struct device isa_bus = {
-	.init_name	= "isa"
-};
+static struct device *isa_bus;
 
 struct isa_dev {
 	struct device dev;
@@ -131,7 +129,7 @@ int isa_register_driver(struct isa_driver *isa_driver, unsigned int ndev)
 			break;
 		}
 
-		isa_dev->dev.parent	= &isa_bus;
+		isa_dev->dev.parent	= isa_bus;
 		isa_dev->dev.bus	= &isa_bus_type;
 
 		dev_set_name(&isa_dev->dev, "%s.%u",
@@ -169,9 +167,11 @@ static int __init isa_bus_init(void)
 
 	error = bus_register(&isa_bus_type);
 	if (!error) {
-		error = device_register(&isa_bus);
-		if (error)
+		isa_bus = root_device_register("isa");
+		if (IS_ERR(isa_bus)) {
+			error = PTR_ERR(isa_bus);
 			bus_unregister(&isa_bus_type);
+		}
 	}
 	return error;
 }
-- 
2.53.0

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

[William Breathitt Gray]

On Fri, Apr 24, 2026 at 12:24:00PM +0200, 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.
> 
> Use root_device_register() to allocate and register the root device
> instead of open coding using a static device.
> 
> Note that this also fixes a reference leak in case device_register()
> fails which may be flagged by static checkers.
> 
> Signed-off-by: Johan Hovold <johan@kernel.org>

Hi Johan,

Would you resend with a Fixes tag and CC stable@vger.kernel.org so we
can get this patch picked up in the stable trees as well?

Thanks,

William Breathitt Gray

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

[Danilo Krummrich]

On Sun May 3, 2026 at 7:08 AM CEST, William Breathitt Gray wrote:
> Would you resend with a Fixes tag

Devices should generally be allocated dynamically for various reasons (e.g. I
recently had a case where adding a dynamic lock class key to struct device for
debugging purposes caused warnings for all the static devices).

While it clearly should be improved, I don't think this causes a bug in the ISA
code -- there is no "real" leak as the device is static anyway and there's no
spurious WARN() as release() is never hit, since ISA is always built-in.

I'd assume this is why Johan did not add it in the first place.

That said, if you mean to add a Fixes: tag anyway to indicate the technically
wrong usage pattern of struct device, that seems reasonable to me.

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

[William Breathitt Gray]

On Sun, May 03, 2026 at 05:50:17PM +0200, Danilo Krummrich wrote:
> On Sun May 3, 2026 at 7:08 AM CEST, William Breathitt Gray wrote:
> > Would you resend with a Fixes tag
> 
> Devices should generally be allocated dynamically for various reasons (e.g. I
> recently had a case where adding a dynamic lock class key to struct device for
> debugging purposes caused warnings for all the static devices).
> 
> While it clearly should be improved, I don't think this causes a bug in the ISA
> code -- there is no "real" leak as the device is static anyway and there's no
> spurious WARN() as release() is never hit, since ISA is always built-in.
> 
> I'd assume this is why Johan did not add it in the first place.
> 
> That said, if you mean to add a Fixes: tag anyway to indicate the technically
> wrong usage pattern of struct device, that seems reasonable to me.

Okay that makes sense, this is improvement of the code rather than a
true bug fix, so we don't need a Fixes tag.

Johan, I do have another request. Would you refactor the changes in
isa_bus_init() to avoid the nested blocks? For example:

	error = bus_register(&isa_bus_type);
	if (error)
		return error;

	isa_bus = root_device_register("isa");
	if (IS_ERR(isa_bus)) {
		bus_unregister(&isa_bus_type);
		return PTR_ERR(isa_bus);
	}

	return 0;

I believe that makes the logic easier to understand when reading the
code here.

William Breathitt Gray

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

[Greg Kroah-Hartman]

On Mon, May 04, 2026 at 03:35:17PM +0900, William Breathitt Gray wrote:
> On Sun, May 03, 2026 at 05:50:17PM +0200, Danilo Krummrich wrote:
> > On Sun May 3, 2026 at 7:08 AM CEST, William Breathitt Gray wrote:
> > > Would you resend with a Fixes tag
> > 
> > Devices should generally be allocated dynamically for various reasons (e.g. I
> > recently had a case where adding a dynamic lock class key to struct device for
> > debugging purposes caused warnings for all the static devices).
> > 
> > While it clearly should be improved, I don't think this causes a bug in the ISA
> > code -- there is no "real" leak as the device is static anyway and there's no
> > spurious WARN() as release() is never hit, since ISA is always built-in.
> > 
> > I'd assume this is why Johan did not add it in the first place.
> > 
> > That said, if you mean to add a Fixes: tag anyway to indicate the technically
> > wrong usage pattern of struct device, that seems reasonable to me.
> 
> Okay that makes sense, this is improvement of the code rather than a
> true bug fix, so we don't need a Fixes tag.
> 
> Johan, I do have another request. Would you refactor the changes in
> isa_bus_init() to avoid the nested blocks? For example:
> 
> 	error = bus_register(&isa_bus_type);
> 	if (error)
> 		return error;
> 
> 	isa_bus = root_device_register("isa");
> 	if (IS_ERR(isa_bus)) {
> 		bus_unregister(&isa_bus_type);
> 		return PTR_ERR(isa_bus);
> 	}
> 
> 	return 0;
> 
> I believe that makes the logic easier to understand when reading the
> code here.

Nah, for now, it's fine, if you wish to touch this isa code in the
future to "clean it up", that's great, but it should be done in a
separate change.

thanks,

greg k-h

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

[William Breathitt Gray]

On Mon, May 04, 2026 at 08:52:05AM +0200, Greg Kroah-Hartman wrote:
> On Mon, May 04, 2026 at 03:35:17PM +0900, William Breathitt Gray wrote:
> > On Sun, May 03, 2026 at 05:50:17PM +0200, Danilo Krummrich wrote:
> > > On Sun May 3, 2026 at 7:08 AM CEST, William Breathitt Gray wrote:
> > > > Would you resend with a Fixes tag
> > >
> > > Devices should generally be allocated dynamically for various reasons (e.g. I
> > > recently had a case where adding a dynamic lock class key to struct device for
> > > debugging purposes caused warnings for all the static devices).
> > >
> > > While it clearly should be improved, I don't think this causes a bug in the ISA
> > > code -- there is no "real" leak as the device is static anyway and there's no
> > > spurious WARN() as release() is never hit, since ISA is always built-in.
> > >
> > > I'd assume this is why Johan did not add it in the first place.
> > >
> > > That said, if you mean to add a Fixes: tag anyway to indicate the technically
> > > wrong usage pattern of struct device, that seems reasonable to me.
> >
> > Okay that makes sense, this is improvement of the code rather than a
> > true bug fix, so we don't need a Fixes tag.
> >
> > Johan, I do have another request. Would you refactor the changes in
> > isa_bus_init() to avoid the nested blocks? For example:
> >
> > 	error = bus_register(&isa_bus_type);
> > 	if (error)
> > 		return error;
> >
> > 	isa_bus = root_device_register("isa");
> > 	if (IS_ERR(isa_bus)) {
> > 		bus_unregister(&isa_bus_type);
> > 		return PTR_ERR(isa_bus);
> > 	}
> >
> > 	return 0;
> >
> > I believe that makes the logic easier to understand when reading the
> > code here.
> 
> Nah, for now, it's fine, if you wish to touch this isa code in the
> future to "clean it up", that's great, but it should be done in a
> separate change.
> 
> thanks,
> 
> greg k-h

Fair enough, this version is all right with me after all in the end.

Acked-by: William Breathitt Gray <wbg@kernel.org>

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

[Danilo Krummrich]

On Fri, 24 Apr 2026 12:24:00 +0200, Johan Hovold wrote:
> [PATCH] isa: switch to dynamic root device

Applied, thanks!

  Branch: driver-core-testing
  Tree:   git://git.kernel.org/pub/scm/linux/kernel/git/driver-core/driver-core.git

[1/1] isa: switch to dynamic root device
      commit: 30c878ed1699

The patch will appear in the next linux-next integration (typically within 24
hours on weekdays).

The patch is in the driver-core-testing branch and will be promoted to
driver-core-next after validation.