Re: [cdi-devel] Proposed Dynamic Driver Loading System

[Kevin Wolf <kevin@xxxxxxxxxx>]

On Sun, Jan 03, 2010 at 09:23:30AM +1000, Matthew Iselin wrote:
> On Sun, Jan 3, 2010 at 3:13 AM, Rich Edelman <redelman@xxxxxxxxx> wrote:
> > On Sat, Jan 2, 2010 at 5:46 AM, Kevin Wolf <kevin@xxxxxxxxxx> wrote:
> >> On Sat, Jan 02, 2010 at 04:58:39PM +1000, Matthew Iselin wrote:
> >>> The idea is that the OS stores a map of device identifiers (at this stage the
> >>> proposal considers the use of PCI vendor and device codes) which detail the
> >>> name of a driver which is to be loaded if the given device is detected. This
> >>> list may be OS-specific, or there may be a generic list provided as part of
> >>> CDI. Either way, the OS is open to interpret the driver name as it sees fit:
> >>> for example, appending ".o" in the case of Pedigree.
> >>>
> >>> This map might look something like this:
> >>>
> >>> /** OS-specific map of PCI identifiers -> driver names */
> >>>
> >>> struct cdi_os_pci_driver_map = {
> >>>     0x8086, 0x1230, "piix", // Vendor, Device, Driver name
> >>>     ...
> >>> };
> >>
> >> This would be an additional top-level C file, right? I don't think it's
> >> a good idea to manually create it and keep it in sync (especially
> >> considering that many OSes don't use all drivers), but generating such a
> >> file looks fine to me.
> >
> > I also agree that such a file should be generated. It'd not be too
> > hard to generate this list just from the drivers that were compiled,
> > rather than containing all the drivers.
> 
> A simple <your scripting language here> script could be added and
> called to update the list during the build of CDI.

Actually I think C would be the right tool. Link all object files in the whole
tree together and add a small main() function that enumerates all the drivers
in it. All the parsing is done by the compiler.

> >>> At a time defined by the OS implementing CDI, the list of PCI devices will be
> >>> iterated, and each device will be checked against the known PCI->driver list.
> >>> Should a driver be found in this list, the driver will be loaded (if not
> >>> already) and used to initialise the detected device.
> >>>
> >>> Now, this brings up a small tangent: it is my belief that driver loading could
> >>> be added to the CDI interface, as something like:
> >>>
> >>> /* Returns -1 if the driver could not be loaded, zero otherwise */
> >>> int cdi_load_driver(const char *);
> >
> > I think this prototype is all wrong. Whatever it is that wants to load
> > another driver needs to pass a structure that describes the device a
> > driver is needed for. Driver's shouldn't be expected to know the name
> > of the driver they need for another device. Sure, they can get this
> > from the mapping file discussed previously, but I think that's wrong
> > as well. Such a file needs to really be internal to the CDI core, and
> > not something used by the drivers (only contributed to by the
> > drivers).
> 
> Yes, this specifically is one thing I was very unsure about when I
> thought out the initial proposal. The main reason I brought it up was
> for a situation like this:
> 1. Plug in a USB device
> 2. The USB controller driver tells the  OS a new device is plugged in.
> Now the OS needs to figure out what driver to load for the device.
> The `cdi_load_driver' function is certainly incorrect here, but some
> sort of hook in CDI to inform the OS of a new device that has been
> added would help here.

Right, we're mostly complete on the device driver side, but the bus
driver side (providing/removing devices) is completely missing. However,
we seem to agree that this is the right interface for loading drivers,
so maybe the next step would be specifying the bus driver interface.

Actually, I think's quite boring. Some cdi_provide_device(struct
cdi_bus_addr*), a function for removing (probably takes the cdi_bus_addr
again?) and we're done. Finding good names might be the most difficult
thing in it. ;-)

Kevin