PERFORCE change 106148 for review
Hans Petter Selasky
hselasky at c2i.net
Fri Sep 15 11:39:19 PDT 2006
On Friday 15 September 2006 18:43, John Baldwin wrote:
> 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.
> > >
> > My implementation for NetBSD works like this:
>
> This is FreeBSD I'm talking about.
It is based off FreeBSD.
> > > 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.
Ok, right. But the old USB code did already do that. It shared the devclass
among multiple devices.
> >
> > 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.
...
> 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;
Maybe a missing "if (*dmd->dmd_devclass == NULL)" here. It is not a big
problem. All the USB probe / attach code is currently executed under Giant,
so there should not be any race condition. "devclass_find_internal()"
allocates memory with M_NOWAIT, and will not sleep and cause problems.
> *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:
I know. But then my NetBSD implementation is slightly different. I only
allocate a devclass when there are devices present, and not before, to save
memory. I thought that was why devclass_t was a pointer and not a structure.
> 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, ...);
>
Yes, but I cannot set the "devclass" to "NULL" ? That will panic according to
the "driver_module_handler()" function ??
> 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.
Ok.
--HPS
More information about the p4-projects
mailing list