From owner-freebsd-current@FreeBSD.ORG  Thu Mar 27 22:53:27 2003
Return-Path: <owner-freebsd-current@FreeBSD.ORG>
Delivered-To: freebsd-current@freebsd.org
Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125])
	by hub.freebsd.org (Postfix) with ESMTP id 7AF8D37B401
	for <current@freebsd.org>; Thu, 27 Mar 2003 22:53:27 -0800 (PST)
Received: from harmony.village.org (rover.bsdimp.com [204.144.255.66])
	by mx1.FreeBSD.org (Postfix) with ESMTP id 80F3E43F75
	for <current@freebsd.org>; Thu, 27 Mar 2003 22:53:26 -0800 (PST)
	(envelope-from imp@bsdimp.com)
Received: from localhost (warner@rover2.village.org [10.0.0.1])
	by harmony.village.org (8.12.8/8.12.3) with ESMTP id h2S6rPA7099906;
	Thu, 27 Mar 2003 23:53:25 -0700 (MST)
	(envelope-from imp@bsdimp.com)
Date: Thu, 27 Mar 2003 23:53:06 -0700 (MST)
Message-Id: <20030327.235306.06521148.imp@bsdimp.com>
To: nate@root.org
From: "M. Warner Losh" <imp@bsdimp.com>
In-Reply-To: <Pine.BSF.4.21.0303271519390.30498-100000@root.org>
References: <Pine.BSF.4.21.0303271519390.30498-100000@root.org>
X-Mailer: Mew version 2.1 on Emacs 21.2 / Mule 5.0 (SAKAKI)
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
X-Spam-Status: No, hits=-10.2 required=5.0
	tests=AWL,IN_REP_TO,REFERENCES
	autolearn=ham	version=2.50
X-Spam-Level: 
X-Spam-Checker-Version: SpamAssassin 2.50 (1.173-2003-02-20-exp)
cc: current@freebsd.org
Subject: Re: Updated pci/if_* attach patches
X-BeenThere: freebsd-current@freebsd.org
X-Mailman-Version: 2.1.1
Precedence: list
List-Id: Discussions about the use of FreeBSD-current
	<freebsd-current.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-current>,
	<mailto:freebsd-current-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-current>
List-Post: <mailto:freebsd-current@freebsd.org>
List-Help: <mailto:freebsd-current-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-current>,
	<mailto:freebsd-current-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Fri, 28 Mar 2003 06:53:28 -0000

In message: <Pine.BSF.4.21.0303271519390.30498-100000@root.org>
            Nate Lawson <nate@root.org> writes:
: * Add bus_child_present check to calls to *_stop in the detach method for
: devices that have children (i.e. miibus).

bus_child_present isn't quite right for this.  bus_child_present means
"this child is still there in hardware" not "this node has children"

+	if (device_is_alive(dev)) {
+		if (bus_child_present(dev)) {
+			dc_stop(sc);
+			device_delete_child(dev, sc->dc_miibus);
+		}
+		ether_ifdetach(ifp);
+		bus_generic_detach(dev);
+	}

should be just:

+	if (device_is_alive(dev)) {
+		if (bus_child_present(dev))
+			dc_stop(sc);
+		device_delete_child(dev, sc->dc_miibus);
+		ether_ifdetach(ifp);
+		bus_generic_detach(dev);
+	}

since it says 'if the card is still there, stop it' which is what you
want in the cardbus case (and one day hotplug pci) since the card
might not be there, in which case trying to do anything with it is
doomed to failure.  Please fix this issue before commit.

# guess this is my fault for never writing a bus_child_present man
# page :-(

Also, on a stylistic note:

-	free(sc->dc_srom, M_DEVBUF);
+	if (sc->dc_srom)
+		free(sc->dc_srom, M_DEVBUF);

The kernel free, just like its ANSI-89 userland counterpart, can
tolerate free(NULL) now.  I know some of the other frees check, some
don't, but none should.  I'd not hold up the commit on this issue,
however.

from kern_malloc.c:
void
free(addr, type)
	void *addr;
	struct malloc_type *type;
{
...
	/* free(NULL, ...) does nothing */
	if (addr == NULL)
		return;
...

Warner