From owner-freebsd-new-bus@FreeBSD.ORG Thu Dec 15 18:39:32 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 7756E16A41F; Thu, 15 Dec 2005 18:39:32 +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 1097643D49; Thu, 15 Dec 2005 18:39:31 +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 jBFIc7nG051959; Thu, 15 Dec 2005 11:38:07 -0700 (MST) (envelope-from imp@bsdimp.com) Date: Thu, 15 Dec 2005 11:38:06 -0700 (MST) Message-Id: <20051215.113806.112572560.imp@bsdimp.com> To: jhb@freebsd.org From: Warner Losh In-Reply-To: <20051215.111343.78756307.imp@bsdimp.com> References: <200512151247.27614.jhb@freebsd.org> <20051215.111343.78756307.imp@bsdimp.com> 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:38:07 -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:39:32 -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. Thinking about it, however, it would allow for a two different drivers to be loaded and fight over the hardware... but you can do that already today and adding this won't change that. > 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; > }