Date: Fri, 15 Sep 2006 12:43:50 -0400 From: John Baldwin <jhb@freebsd.org> To: Hans Petter Selasky <hselasky@c2i.net> Cc: Perforce Change Reviews <perforce@freebsd.org> Subject: Re: PERFORCE change 106148 for review Message-ID: <200609151243.50388.jhb@freebsd.org> In-Reply-To: <200609151710.17572.hselasky@c2i.net> References: <200609151417.k8FEHSFx098273@repoman.freebsd.org> <200609151039.16523.jhb@freebsd.org> <200609151710.17572.hselasky@c2i.net>
next in thread | previous in thread | raw e-mail | index | archive | help
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@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200609151243.50388.jhb>