Defining new section for bus driver init

Defining new section for bus driver init

[Patrick Mochel]

Currently, in init/main.c, static calls are made to some device
subsystems, like this:

#ifdef CONFIG_PCI
        pci_init();
#endif

By careful examination and a bit of mediatating, I have inferred a few
things about those calls.

They initialize the "root" buses in the system - the buses that hang right
off of the equivalent of the host controller (i.e. north bridge) on $arch.

They are statically called in do_basic_setup() because if they used
__initcall(), it would hard, if not impossible, to guarantee that they are
called before anything else.

The collection of them in init/main.c is ugly, and the model doesn't scale
well.

Attached is a patch that creates a new section for device subsystem init
calls. With it, the root bus init calls are handled just like init calls
- the section consists of a table of function pointers.
device_driver_init() iterates over that table and calls each one.
(device_driver_init() currently happens just before that pci_init() call
above).

What do people think about the concept? I am more than willing to port the
other calls to it, as well add the vmlinux.lds entries (though both are
trivial). I don't have any other hardware to test on, though, besides an
x86 PCI-based system.

I will warn that the name is kinda clumsy, but it's the best that I could
come up with (I wasted my creativity for the day on thinking about
Penelope). I used "subsystem" because I have alterior motives.

In order for a driver to be loaded, there needs to be drivers for the bus
on which it resides loaded, and drivers for the device class to which it
belongs (disk, network, etc) loaded.

Buses that are not currently "root" buses have initcall init calls. All
device classes do, too. And the device-specific drivers are in there, too.
Bus and class drivers need to be loaded before device drivers, and they
do not depend on each other, so it seems that they can be grouped
together in a section and called (like below). Right?

	-pat

diff -Nur linux-2.5.1.orig/arch/i386/vmlinux.lds linux-2.5.1/arch/i386/vmlinux.lds
--- linux-2.5.1.orig/arch/i386/vmlinux.lds	Mon Jul  2 14:40:14 2001
+++ linux-2.5.1/arch/i386/vmlinux.lds	Mon Jan 14 16:50:12 2002
@@ -50,6 +50,11 @@
   __initcall_start = .;
   .initcall.init : { *(.initcall.init) }
   __initcall_end = .;
+
+  __devsubsys_start = .;
+  .devsubsys.init : { *(.devsubsys.init) }
+  __devsubsys_end = .;
+
   . = ALIGN(4096);
   __init_end = .;

diff -Nur linux-2.5.1.orig/drivers/pci/pci.c linux-2.5.1/drivers/pci/pci.c
--- linux-2.5.1.orig/drivers/pci/pci.c	Tue Nov 20 21:53:29 2001
+++ linux-2.5.1/drivers/pci/pci.c	Mon Jan 14 16:57:44 2002
@@ -1929,7 +1929,7 @@
 }


-void __devinit  pci_init(void)
+int __devinit  pci_init(void)
 {
 	struct pci_dev *dev;

@@ -1942,6 +1942,7 @@
 #ifdef CONFIG_PM
 	pm_register(PM_PCI_DEV, 0, pci_pm_callback);
 #endif
+	return 0;
 }

 static int __devinit  pci_setup(char *str)
@@ -1958,6 +1959,8 @@
 	}
 	return 1;
 }
+
+__devsubsys_init(pci_init);

 __setup("pci=", pci_setup);

diff -Nur linux-2.5.1.orig/include/linux/init.h linux-2.5.1/include/linux/init.h
--- linux-2.5.1.orig/include/linux/init.h	Fri Dec 21 16:05:57 2001
+++ linux-2.5.1/include/linux/init.h	Mon Jan 14 16:54:08 2002
@@ -55,6 +55,12 @@
 #define __exitcall(fn)								\
 	static exitcall_t __exitcall_##fn __exit_call = fn

+/* device subsystem initialization */
+extern initcall_t __devsubsys_start, __devsubsys_end;
+
+#define __devsubsys_init(fn) \
+static initcall_t __devsubsys_##fn __devsubsys_call = fn;
+
 /*
  * Used for kernel command line parameter setup
  */
@@ -82,6 +88,7 @@
 #define __initsetup	__attribute__ ((unused,__section__ (".setup.init")))
 #define __init_call	__attribute__ ((unused,__section__ (".initcall.init")))
 #define __exit_call	__attribute__ ((unused,__section__ (".exitcall.exit")))
+#define __devsubsys_call __attribute__ ((unused,__section__ (".devsubsys.init")))

 /* For assembly routines */
 #define __INIT		.section	".text.init","ax"
diff -Nur linux-2.5.1.orig/include/linux/pci.h linux-2.5.1/include/linux/pci.h
--- linux-2.5.1.orig/include/linux/pci.h	Fri Dec 21 16:06:13 2001
+++ linux-2.5.1/include/linux/pci.h	Mon Jan 14 17:32:42 2002
@@ -525,7 +525,7 @@

 /* Generic PCI functions used internally */

-void pci_init(void);
+int pci_init(void);
 int pci_bus_exists(const struct list_head *list, int nr);
 struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops, void *sysdata);
 struct pci_bus *pci_alloc_primary_bus(int bus);
diff -Nur linux-2.5.1.orig/init/main.c linux-2.5.1/init/main.c
--- linux-2.5.1.orig/init/main.c	Wed Jan  9 11:03:06 2002
+++ linux-2.5.1/init/main.c	Mon Jan 14 17:35:25 2002
@@ -38,10 +38,6 @@
 #include <asm/ccwcache.h>
 #endif

-#ifdef CONFIG_PCI
-#include <linux/pci.h>
-#endif
-
 #ifdef CONFIG_DIO
 #include <linux/dio.h>
 #endif
@@ -481,9 +477,6 @@
 	/* bring up the device tree */
 	device_driver_init();

-#ifdef CONFIG_PCI
-	pci_init();
-#endif
 #ifdef CONFIG_SBUS
 	sbus_init();
 #endif
diff -Nur linux-2.5.1.orig/kernel/device.c linux-2.5.1/kernel/device.c
--- linux-2.5.1.orig/kernel/device.c	Wed Jan  9 11:03:00 2002
+++ linux-2.5.1/kernel/device.c	Mon Jan 14 17:03:46 2002
@@ -890,6 +890,17 @@
 	return (device_root.dir ? 0 : -EFAULT);
 }

+initcall_t __devsubsys_start, __devsubsys_end;
+
+static void device_subsys_init(void)
+{
+	initcall_t * fn = &__devsubsys_start;
+
+	do {
+		(*fn)();
+	} while (++fn < &__devsubsys_end);
+}
+
 int __init device_driver_init(void)
 {
 	int error = 0;
@@ -910,6 +921,8 @@
 		printk(KERN_ERR "%s: device root init failed!\n", __FUNCTION__);
 		return error;
 	}
+
+	device_subsys_init();

 	DBG("DEV: Done Initialising\n");
 	return error;

Re: Defining new section for bus driver init

[Russell King]

On Mon, Jan 14, 2002 at 05:47:15PM -0800, Patrick Mochel wrote:
> Attached is a patch that creates a new section for device subsystem init
> calls. With it, the root bus init calls are handled just like init calls
> - the section consists of a table of function pointers.
> device_driver_init() iterates over that table and calls each one.
> (device_driver_init() currently happens just before that pci_init() call
> above).

I've been thinking about this, and would like to suggest something that'd
reduce the code size, but is dependent on the ordering of stuff in
do_basic_setup.  So first some questions.  Currently, we do:

	device_driver_init()
	random bus driver init...
	sock_init()
	start_context_thread()
	do_initcalls()

Is there any ordering dependency between sock_init(), start_context_thread()
and the bus driver init calls?  From a brief look at the code, it would
appear that start_context_thread() is rather safe, but sock_init() is
questionable.  If they are both safe, then we could move these two calls
before, or even just after device_driver_init():

	device_driver_init()
	sock_init()
	start_context_thread()
	random bus driver init...
	do_initcalls()

Now we have the bus driver initialisation and the initcall initialisation
next to each other.  We can then pull this trick with the linker file:

	__initcall_start = .;
	.initcall : {
		*(.devsubsys.init)
		*(.initcall.init)
	}
	__initcall_end = .;

All the magic then happens within do_initcalls() without any extra code
needing to be added.  The really funky thing about this approach is
that you can add other sections to handle network protocol modules
and such like with virtually zero code.

-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Re: Defining new section for bus driver init

[Patrick Mochel]

On Tue, 15 Jan 2002, Russell King wrote:

> On Mon, Jan 14, 2002 at 05:47:15PM -0800, Patrick Mochel wrote:
> > Attached is a patch that creates a new section for device subsystem init
> > calls. With it, the root bus init calls are handled just like init calls
> > - the section consists of a table of function pointers.
> > device_driver_init() iterates over that table and calls each one.
> > (device_driver_init() currently happens just before that pci_init() call
> > above).
>
> I've been thinking about this, and would like to suggest something that'd
> reduce the code size, but is dependent on the ordering of stuff in
> do_basic_setup.  So first some questions.  Currently, we do:
>
> 	device_driver_init()
> 	random bus driver init...
> 	sock_init()
> 	start_context_thread()
> 	do_initcalls()
>
> Is there any ordering dependency between sock_init(), start_context_thread()
> and the bus driver init calls?  From a brief look at the code, it would
> appear that start_context_thread() is rather safe, but sock_init() is
> questionable.  If they are both safe, then we could move these two calls
> before, or even just after device_driver_init():
>
> 	device_driver_init()
> 	sock_init()
> 	start_context_thread()
> 	random bus driver init...
> 	do_initcalls()

device_driver_init() is not that special. It only needs to be called
before any of the bus or class subsystems. So, in theory, you could
declare it __devsubsys_init, and use the link order to guarantee its
ordering.

I don't know about sock_init(). That looks like the core network init,
which is the other type of thing that I had intended for this section. It
shouldn't be dependent on any other device classes or device buses,
right? So, it could be moved as well.

Although, device_driver_init() should be called first. So maybe we need a
__dev_bus_init and __dev_class_init. Or, is that much?

> Now we have the bus driver initialisation and the initcall initialisation
> next to each other.  We can then pull this trick with the linker file:
>
> 	__initcall_start = .;
> 	.initcall : {
> 		*(.devsubsys.init)
> 		*(.initcall.init)
> 	}
> 	__initcall_end = .;
>
> All the magic then happens within do_initcalls() without any extra code
> needing to be added.  The really funky thing about this approach is
> that you can add other sections to handle network protocol modules
> and such like with virtually zero code.

Hey, cool. I'd much prefer this.

I had been thinking about creating a new one for device driver init
routines, and grouping them together. This, however, is only for the sake
of grouping, and doesn't bear any obvious technical merit. And, it would
require touching all the drivers, so maybe not...

	-pat

Re: Defining new section for bus driver init

[Dave Jones]

> Attached is a patch that creates a new section for device subsystem init
> calls. With it, the root bus init calls are handled just like init calls
> - the section consists of a table of function pointers.
> device_driver_init() iterates over that table and calls each one.
> (device_driver_init() currently happens just before that pci_init() call
> above).
> What do people think about the concept? 

Well, it chops out a load of ugly ifdefs, and makes adding support
for a new bus less intrusive than it currently is. I quite like it.

> I will warn that the name is kinda clumsy, but it's the best that I could
> come up with (I wasted my creativity for the day on thinking about
> Penelope). I used "subsystem" because I have alterior motives.

I think you hit the nail on the head with the subject line.
struct BusDriver also conjures up amusing[*] imagery.

One thing I'm wondering about though. Is it possible for a new
bus to be added after boot ? Docking stations etc show up as
children on the root PCI bus, so that shouldn't be an issue.

Ah! hotplug PCI USB controller ?

Dave.

[*] I'm easily amused.

-- 
Dave Jones.                    http://www.codemonkey.org.uk
SuSE Labs.

Re: Defining new section for bus driver init

[Patrick Mochel]

> > What do people think about the concept?
>
> Well, it chops out a load of ugly ifdefs, and makes adding support
> for a new bus less intrusive than it currently is. I quite like it.

Cool. I'll change the other buses in init/main.c and post it.

> > I will warn that the name is kinda clumsy, but it's the best that I could
> > come up with (I wasted my creativity for the day on thinking about
> > Penelope). I used "subsystem" because I have alterior motives.
>
> I think you hit the nail on the head with the subject line.
> struct BusDriver also conjures up amusing[*] imagery.

Yes, next I'll be adding a common set of routines for BusDrivers,
including drive(), stop(), give_people_dirty_looks(), and swerve()...

> One thing I'm wondering about though. Is it possible for a new
> bus to be added after boot ? Docking stations etc show up as
> children on the root PCI bus, so that shouldn't be an issue.

A new bus? Sure, the bus driver just has to probe behind it.

USB can do it. The PCI probe routines are all declared __devinit, so if
you have hotplug support, you should be able to do it in theory. Greg?

> Ah! hotplug PCI USB controller ?

Cardbus, Hotplug PCI, docking stations..

	-pat

Re: Defining new section for bus driver init

[Patrick Mochel]

> I like it.  Are you going to want to move the other busses to this
> (sbus, mca, ecard, zorro, etc.)?

Yes, though I don't have a way to test them...

> Don't add USB to the list of busses that should be
> moved to this scheme, it works just fine today in the initcall section
> (after pci starts up.)

Ok, fine. My main concern was to remedy the root buses of the system. I
don't believe that USB is such a thing, except maybe on some ARM systems?

	-pat

Re: Defining new section for bus driver init

[Dave Jones]

On Tue, Jan 15, 2002 at 09:53:27AM -0800, Patrick Mochel wrote:
 > > I like it.  Are you going to want to move the other busses to this
 > > (sbus, mca, ecard, zorro, etc.)?
 > Yes, though I don't have a way to test them...

 Not a problem, I'm sure you'll find an army of testers on l-k.
 
-- 
| Dave Jones.        http://www.codemonkey.org.uk
| SuSE Labs

Re: Defining new section for bus driver init

[Patrick Mochel]

On Tue, 15 Jan 2002, Dave Jones wrote:

> On Tue, Jan 15, 2002 at 09:53:27AM -0800, Patrick Mochel wrote:
>  > > I like it.  Are you going to want to move the other busses to this
>  > > (sbus, mca, ecard, zorro, etc.)?
>  > Yes, though I don't have a way to test them...
>
>  Not a problem, I'm sure you'll find an army of testers on l-k.

Ok, as I'm converting the drivers, I find that some have the option of
modular compilation, like isapnp and irda. In theory, it seems that each
of the bus drivers could be compiled modular. But, how do you define it in
the section?

I was thinking something like this:

#ifdef MODULE
#define __devsubsys_init(fn) \
	module_init(fn);
#else
#define __devsubsys_init(fn) \
	static initcall_t __devsubsys_##fn __devsubsys_call = fn;
#endif

Thoughts?

	-pat