PERFORCE change 106148 for review

[John Baldwin]

On Friday 15 September 2006 11:10, Hans Petter Selasky wrote:
> On Friday 15 September 2006 16:39, you wrote:
> > On Friday 15 September 2006 10:17, Hans Petter Selasky wrote:
> > > http://perforce.freebsd.org/chv.cgi?CH=106148
> > >
> > > Change 106148 by hselasky at hselasky_mini_itx on 2006/09/15 14:17:12
> > >
> > >  Initialize all driver_t structures by record. Bugfix: Some of the
> > >  modem drivers did not use the "ucom_devclass". Make sure that all
> > >  modem drivers use this devclass.
> >
> > That doesn't actually matter.  Also, no other drivers use this style to
> > initialize their driver_t.  If you really wanted to do a change, you 
should
> > change them to use DEFINE_CLASS macros to define a KOBJ class instead.
> 
> Ok. Will consider that next time.
> 
> > BTW, why the devclass doesn't matter:  the devclass_t is just a pointer to
> > a device class.  The device classes are based on the driver name, so if 
you
> > add a new driver with the name "foo", it will look up the device class by
> > name ("foo"), creating one if it doesn't exist.  
> 
> My implementation for NetBSD works like this:

This is FreeBSD I'm talking about.

> > It then saves a pointer to 
> > that device class object in the devclass_t pointer specified in
> > DRIVER_MODULE(). So, by making them all share the same devclass_t, they 
are
> > all just going to overwrite the same pointer when the driver module loads. 
> 
> No, wrong. The devclass_t pointer is not overwritten if it is already 
> initialized!

Umm.  It's overwritten, but to the same value.  Go look at the actual code.  
This is the function that is called via SYSINIT via DRIVER_MODULE():

/**
 * @brief Module handler for registering device drivers
 *
 * This module handler is used to automatically register device
 * drivers when modules are loaded. If @p what is MOD_LOAD, it calls
 * devclass_add_driver() for the driver described by the
 * driver_module_data structure pointed to by @p arg
 */
int
driver_module_handler(module_t mod, int what, void *arg)
{
	...
	dmd = (struct driver_module_data *)arg;
	...
	switch (what) {
	case MOD_LOAD:
		...
		if (driver->baseclasses) {
			const char *parentname;
			parentname = driver->baseclasses[0]->name;
			*dmd->dmd_devclass =
				devclass_find_internal(driver->name,
				    parentname, TRUE);
		} else {
			*dmd->dmd_devclass =
				devclass_find_internal(driver->name, 0, TRUE);
		}
		break;
	...
}

The writes to dmd_devclass are writing to the 'static devclass_t' you
specify in your DRIVER_MODULE() line.  As I mentioned earlier, devclass_t
isn't a struct, it's a pointer to a struct:

typedef struct devclass		*devclass_t;

Basically, passing a devclass_t into DRIVER_MODULE() just gives you a 
pre-intialized pointer to your devclass.  The devclass's aren't bound to that 
devclass_t though, they are bound to the name.  Thus if you have:

static devclass_t foo_class, bar_class;

static driver_t foo_driver {
	"foo", ...
};

static driver_t bar_driver {
	"foo", ...
};

DRIVER_MODULE(..., foo_driver, foo_devclass, ...);
DRIVER_MODULE(..., bar_driver, bar_devclass, ...);

foo_devclass and bar_devclass will both point to the same device class object.

> > To be honest, most drivers don't even use the devclass pointer, and I'd
> > actually like to axe it and make the few drivers that do care use
> > 'devclass_find("foo")' when they need the devclass pointer instead.
> 
> I need the devclass to get unique unit numbers.

Again, go look at the actual code, it can still call devclass_find_internal(), 
but it is not required to save the pointer to do so.

-- 
John Baldwin