Subject: Modules autoloading for bus 'of' and 'macio' : The situation

From: Sylvain Munaut <[email-masked]>
Date: Sat, 09 Dec 2006 10:05:05 +0100

Hi everyone,

Recently I've been looking at the uevent code in macio and of_platform
to see how it works and
I think there may be a problem with it. If it works today it may be
partly due to luck.
The email is a little long, but I'm trying to be clear ;)

First, a quick refresh on how it supposed to work (It's my understanding
of the process so I may be mistaken) :

 * On the device driver side : You have a MODULE_DEVICE_TABLE(....),
this will result in the table being put in a section in the module
object, to be picked up my modpost during compile and finally results in
line like this in the 'modinfos' of the .ko :

alias:          of:NusbT*Cohci-bigendian*
alias:          of:NusbT*Cohci-be*

The match table that generated theses is :

static struct of_device_id ohci_hcd_ppc_of_match[] = {
        {
                .name = "usb",
                .compatible = "ohci-bigendian",
        },
        {
                .name = "usb",
                .compatible = "ohci-be",
        },
};

As you can see, since no .type is specified, it has been replaced by a
wildcard *, and there is also a wildcard at the end, added for future
compatibility.

 * On the device node side :

When a device is attached to a bus, the uevent handler of the bus is
called and this one generates the event to be sent to user space. The
handler is free to send whatever he wants, but the most critical entry
is the MODALIAS variable. It's meant to be matched against the alias of
the modules to find what to load.

On PCI for example, the modalias is the vendord id, product id, and some
others id as well. On the macio and of bus, we include the name, the
type and the compatible list. For the latter though, since there can be
multiple value we must put several values ... That could be for example :

of:NusbTohciCmpc5200-ohciCmpc52xx-ohciCohci-be

That's what would be generated for this device node :

                usb@1000 {
                        device_type = "usb-ohci-be";
                        compatible = "mpc5200-ohci\0mpc52xx-ohci\0ohci-be";
                        reg = <1000 ff>;
                        interrupts = <2 6 0>;
                        interrupt-parent = <500>;
                };

 * On the kernel side :

When doing the depmod, a modules.alias file is created containing
entries like this :

alias of:NusbT*Cohci-bigendian* ohci_hcd
alias of:NusbT*Cohci-be* ohci_hcd

 * On the user space side : When it receives an event that a new device
'appeared', udev will try to load a module for it. As far as I can tell
the only thing it does, is call modprobe $DEVALIAS, then modprobe tries
to load the module with that name. As there is no module whose file name
is "of:...", it will load the modules.alias file and try to match it
(with wildcards).

Now the problem :

It lies with the compatible property as in this case, the device as a
list of compatible and the device as a list of compatible as well and we
just want 1 common entry.
In the usual case (other bus), that's not the case, the driver has a
list of supported ids but the device has only a unique one.

The solution found here is to put several C...C... in the MODALIAS sent
in the userspace event.
But the whole alias stuff was never really meant for this and the
matching in modprobe really doesn't 'know' about this trick.

So why does it work ? Well because of the wildcards.
There is always one at the end in the alias generated by modpost,
meaning if the uevent handler sent some more 'C...' after the 'good'
one, they'll be matched by the wildcard. And if there is 'C...' before
the 'good' one they _may_ be matched using the wildcard of the field
just before : type. And I didn't find any driver using both compatible
and type for matching, that's why it works.

The solution I see now would be to change modpost to always add a *
before the first C (if there is not already one due to type).

Any thought on this ? Am I correct or completly wrong ?

    Sylvain