Daemon News Ezine BSD News BSD Mall BSD Support Forum BSD Advocacy BSD Updates

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: DRIVER_UNIDENTIFY



From: John Baldwin <jhb@xxxxxxxxxxx>
Subject: Re: DRIVER_UNIDENTIFY
Date: Thu, 15 Dec 2005 13:33:05 -0500

> On Thursday 15 December 2005 01:13 pm, Warner Losh wrote:
> > > How about creating a new driver_if.m entry point that is the complement
> > > to DRIVER_IDENTIFY.  I would call it DRIVER_UNIDENTIFY() and would change
> > > bus_generic_detach() to call it on each driver similar to how
> > > bus_generic_probe() currently calls DRIVER_IDENTIFY() for each driver. 
> > > This would allow drivers that create devices in DRIVER_IDENTIFY() have a
> > > place to remove the device when they are unloaded.
> >
> > I have mixed feelings about this.
> >
> > First, the identify routine has identified a hunk of hardware and has
> > placed it into the tree.  The driver disappearing doesn't change the
> > fact that the hardware is still there.  Adding this new function would
> > further blur the lines between attached drivers, the hardware and
> > nodes in the device tree.  Device nodes are expected to be there
> > unattached.  Once the device driver has detached from the device node,
> > there's no harm in leaving the node arround as there are no dangling
> > references.
> >
> > Second, the bus may be the one that decides what hardware is there.  I
> > have a SOC chip that has a number of different children of its nexus
> > that are always there, and will always be there.  I do not want the
> > children drivers to know anything of their location, etc, since they
> > cannot know because a different SOC will have the children at a
> > different location.  Of course, the easy thing here is to never call
> > the idenfify routine at all for this bus, but this would require
> > changes to the code.
> >
> > However, we do currently have a assymetrical arrangement.  There's a
> > way to add the device, but too many drivers are 'stupid' in how they
> > add the device.  They neglect to check to make sure that the device
> > hasn't already been added, which is what causes the grief.
> >
> > Maybe it would be better to have a better way to add instances such
> > that if the idenify routine used this better way that it could be
> > called many times (eg, make it idempotent).  Right now we recommend
> > that driver writers do the following (which is idempotent):
> >
> > static void
> > tscnmi_identify( driver_t* driver, device_t parent )
> > {
> >     devclass_t dc;
> >
> >     dc = devclass_find(DRIVERNAME);
> >     if (devclass_get_device(dc, 0) == NULL) {
> >         if (BUS_ADD_CHILD(parent, 0, DRIVERNAME, 0) == 0)
> >             panic("failed to add " DRIVERNAME);
> >     }
> > }
> >
> > I think a better solution would be:
> >
> > static void
> > tscnmi_identify( driver_t* driver, device_t parent )
> > {
> > 	device_add_child_once(parent, DRIVERNAME);
> > }
> >
> > where device_add_child_once would look like the following (run through
> > style(9)izer):
> >
> > int
> > device_add_child_once(device_t parent, char *name)
> > {
> >     devclass_t dc;
> >
> >     dc = devclass_find(name);
> >     if (devclass_get_device(dc, 0) == NULL)
> >     	return BUS_ADD_CHILD(parent, 0, name, 0);
> >     return 0;
> > }
> 
> Actually, those methods enforce one instance in the system.  I want to enforce 
> one instance per parent device (the example here is acpi_video which attaches 
> to vgapci in my agp_cvs.patch and if you have multiple video cards, each 
> might have its own acpi_video driver).  I'm about to fix acpi_video, but was 
> curious if we could come up with a better overall solution.

I thought about writing once_per_bus too that would search the parent
bus' children for one whose devclass matched...

Warner