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@xxxxxxxxxx>