From owner-freebsd-new-bus@FreeBSD.ORG Thu Dec 15 18:33:14 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 16FD916A41F for ; Thu, 15 Dec 2005 18:33:14 +0000 (GMT) (envelope-from jhb@freebsd.org) Received: from speedfactory.net (mail6.speedfactory.net [66.23.216.219]) by mx1.FreeBSD.org (Postfix) with ESMTP id 63E1C43D79 for ; Thu, 15 Dec 2005 18:32:56 +0000 (GMT) (envelope-from jhb@freebsd.org) Received: from server.baldwin.cx (unverified [66.23.211.162]) by speedfactory.net (SurgeMail 3.5b3) with ESMTP id 3878065 for multiple; Thu, 15 Dec 2005 13:30:42 -0500 Received: from localhost (john@localhost [127.0.0.1]) by server.baldwin.cx (8.13.1/8.13.1) with ESMTP id jBFIWb4c070712; Thu, 15 Dec 2005 13:32:39 -0500 (EST) (envelope-from jhb@freebsd.org) From: John Baldwin To: Warner Losh Date: Thu, 15 Dec 2005 13:33:05 -0500 User-Agent: KMail/1.8.2 References: <200512151247.27614.jhb@freebsd.org> <20051215.111343.78756307.imp@bsdimp.com> In-Reply-To: <20051215.111343.78756307.imp@bsdimp.com> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200512151333.06116.jhb@freebsd.org> X-Virus-Scanned: ClamAV 0.87.1/1210/Thu Dec 15 10:23:22 2005 on server.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-2.8 required=4.2 tests=ALL_TRUSTED autolearn=failed version=3.0.2 X-Spam-Checker-Version: SpamAssassin 3.0.2 (2004-11-16) on server.baldwin.cx X-Server: High Performance Mail Server - http://surgemail.com r=1653887525 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 18:33:14 -0000 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. -- John Baldwin <>< http://www.FreeBSD.org/~jhb/ "Power Users Use the Power to Serve" = http://www.FreeBSD.org