From owner-freebsd-new-bus@FreeBSD.ORG Thu Dec 15 18:15:36 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 BBDF516A41F; Thu, 15 Dec 2005 18:15:36 +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 6B21E43D80; Thu, 15 Dec 2005 18:15:16 +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 jBFIDhTG051778; Thu, 15 Dec 2005 11:13:43 -0700 (MST) (envelope-from imp@bsdimp.com) Date: Thu, 15 Dec 2005 11:13:43 -0700 (MST) Message-Id: <20051215.111343.78756307.imp@bsdimp.com> To: jhb@freebsd.org From: Warner Losh In-Reply-To: <200512151247.27614.jhb@freebsd.org> References: <200512151247.27614.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 11:13:43 -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 18:15:36 -0000 > 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; }