Re: [PATCH] nubus: Unconditionally register bus type

[Michael Schmitz]

Hi Greg,

Am 08.05.2018 um 19:25 schrieb Greg Kroah-Hartman:
> On Tue, May 08, 2018 at 09:07:27AM +0200, Geert Uytterhoeven wrote:
>> Hi Greg,
>>
>> On Tue, May 8, 2018 at 9:00 AM, Greg Kroah-Hartman
>> <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>>> On Mon, May 07, 2018 at 09:51:12AM +1200, Michael Schmitz wrote:
>>>> the BUG() was triggered by loading a Mac Nubus network card module on
>>>> a multiplatform kernel running on an Amiga machine. Up to Finn's Nubus
>>>> core rewrite (this February), we've seen no errors. Since then, Nubus
>>>> drivers fail to register because the Nubus bus is only registered on
>>>> Macs.
>>>>
>>>> Can't see link order involved here at all.
>>>
>>> The link order is totally involved here :)
>>>
>>> Link order determines the order in which init calls are run, so you need
>>> to ensure that your bus code comes before any drivers that use that bus
>>> code in link order.  That way, at init time, your bus is created first,
>>> preventing this type of error to happen.
>>
>> The issue here is not due to link ordering, but due to the bus not being
>> registered on a system that doesn't have that particular bus.
> 
> But how can that happen if the bus code is not present in the system at
> that point in time?  Hardware doesn't matter at all here.
> 
>> Akin to booting a kernel on an old PC without PCI, and loading a driver
>> module for a PCI network card. I guess that doesn't crash (because no one
>> has a PC without PCI anymore? ;-)
> 
> No, it should work just fine, try it!  :)
> 
> The driver will not bind to anything, but the bus code should work
> properly, as long as it is initialized before the driver tries to
> register with that specific bus type.

Before Finn's patch, the Nubus init call used to do this:

static int __init nubus_init(void)
{
        int err;

        if (!MACH_IS_MAC)
                return 0;

        nubus_proc_init();
        err = nubus_bus_register();
        if (err)
                return err;
        nubus_scan_bus();
        return 0;
}

subsys_initcall(nubus_init);

MACH_IS_MAC returns false if run on Amiga, Atari, ... anything but Mac.
The Nubus bus driver is only registered on Mac hardware (working on the
assumption it won't be useful anywhere else so should not be needed.
That was a little naive, as we've seen now.).

The initcalls for Nubus hardware drivers attempt to register the driver
with the Nubus bus driver, regardless of what hardware we're running on,
i.e. regardless of whether or not the Nubus bus ever registered with the
core.

Finn's patch sidesteps the issue by registering the Nubus bus with the
core unconditionally (PCI probably does the same). Drivers can then
register successfully but will never see their probe code called because
the Nubus driver only scans the bus for devices if running on a Mac. So
no harm done either way.

An alternative might be to only allow devices to register if the Nubus
bus has successfully registered with the core (which it wouldn't do
unless running on Mac hardware). That would solve link order and wrong
hardware issues at the same time.
(Link order is Nubus before network drivers as far as I could see from
drivers/built-in.a)

Cheers,

	Michael

> 
> thanks,
> 
> greg k-h
>