From owner-freebsd-new-bus@FreeBSD.ORG Thu Dec 15 20:06:17 2005 Return-Path: X-Original-To: new-bus@freebsd.org Delivered-To: freebsd-new-bus@FreeBSD.ORG Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 5EBFA16A41F; Thu, 15 Dec 2005 20:06:17 +0000 (GMT) (envelope-from imp@bsdimp.com) Received: from harmony.bsdimp.com (vc4-2-0-87.dsl.netrack.net [199.45.160.85]) by mx1.FreeBSD.org (Postfix) with ESMTP id CB66E43D64; Thu, 15 Dec 2005 20:06:15 +0000 (GMT) (envelope-from imp@bsdimp.com) Received: from localhost (localhost.village.org [127.0.0.1] (may be forged)) by harmony.bsdimp.com (8.13.3/8.13.3) with ESMTP id jBFK5APM052649; Thu, 15 Dec 2005 13:05:10 -0700 (MST) (envelope-from imp@bsdimp.com) Date: Thu, 15 Dec 2005 13:05:10 -0700 (MST) Message-Id: <20051215.130510.59706461.imp@bsdimp.com> To: jhb@freebsd.org From: Warner Losh In-Reply-To: <200512151333.06116.jhb@freebsd.org> References: <200512151247.27614.jhb@freebsd.org> <20051215.111343.78756307.imp@bsdimp.com> <200512151333.06116.jhb@freebsd.org> X-Mailer: Mew version 3.3 on Emacs 21.3 / Mule 5.0 (SAKAKI) Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-2.0 (harmony.bsdimp.com [127.0.0.1]); Thu, 15 Dec 2005 13:05:10 -0700 (MST) Cc: new-bus@freebsd.org Subject: Re: DRIVER_UNIDENTIFY X-BeenThere: freebsd-new-bus@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: FreeBSD's new-bus architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 15 Dec 2005 20:06:17 -0000 From: John Baldwin 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