From owner-svn-src-all@FreeBSD.ORG Tue Jul 30 19:19:03 2013 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id 05EFCB3B; Tue, 30 Jul 2013 19:19:03 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (bigwig.baldwin.cx [IPv6:2001:470:1f11:75::1]) (using TLSv1 with cipher ADH-CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id C5A182645; Tue, 30 Jul 2013 19:19:02 +0000 (UTC) Received: from jhbbsd.localnet (unknown [209.249.190.124]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 7855FB963; Tue, 30 Jul 2013 15:19:01 -0400 (EDT) From: John Baldwin To: sbruno@freebsd.org Subject: Re: svn commit: r253708 - head/sys/dev/ipmi Date: Tue, 30 Jul 2013 15:18:51 -0400 User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110714-p28; KDE/4.5.5; amd64; ; ) References: <201307271632.r6RGWYF8046749@svn.freebsd.org> <201307301202.19954.jhb@freebsd.org> <1375210620.1496.30.camel@localhost> In-Reply-To: <1375210620.1496.30.camel@localhost> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Message-Id: <201307301518.51746.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Tue, 30 Jul 2013 15:19:01 -0400 (EDT) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 30 Jul 2013 19:19:03 -0000 On Tuesday, July 30, 2013 2:57:00 pm Sean Bruno wrote: > > > > > > > Ok then what is this ^^^^^^^^^ ? Doesn't this mean that if > > > device_find_child() returns a child node that we should abort? Is that > > > not the same as what I'm going on about? > > > > This makes it only add at most one child device. It is a common idiom in > > identify routines so that if you kldunload and re-kldload you don't end > > up with two "ipmi" devices added by the identify routine. > > > Ok, understood. Why, in this edge case, is ipmi attaching twice then? > Shouldn't device_find_child() return non-NULL because the ACPI interface > is attached and /dev/ipmi0 has been started? Or is it too early in the > probe/attach device enumeration process for this check to succeed? Two _different_ buses. This prevents multiple ipmiX devices on the parent "isa0" device. The other ipmiX device is one on acpi0, not isa0. > Because of this, doesn't the BUS_ADD_CHILD later on "contaminate" our > device tree with a device that doesn't exist? I'm being kind of a PITA > about this, I know. I just want to understand what I'm missing here. Well, except it does exist (in a manner of speaking). :) The ACPI ipmiX device represents a device in the ACPI namespace that has a known _HID for IPMI BMCs. The ISA ipmiX device represents a device discovered via the SMBIOS tables. In practice the same physical device is being enumerated two different ways. Most physical devices don't have aliases in this fashion, but x86 has a lot of cruft, and so in this particular instance you can find out about the device from two very different sources, and there are systems that support ACPI, but only enumerate the IPMI BMC via SMBIOS, so we can't just ignore the SMBIOS table if ACPI is being used. Instead, we have to wait until the SMBIOS-enumerated device probes to decide that we have a fully functional ACPI-enumerated device that already "owns" the device. > I'm "arguing" that the attempted creation (even though it fails) > of /dev/ipmi1 in any way is really a bug. To be clear, we never attempt to create /dev/ipmi1 (a character device in /dev). We might have a new-bus node named ipmi1 that represents the IPMI BMC described by the SMBIOS table, but that is different from /dev/ipmi1. > Sounds good. Done. Tests look good. commited r253813. Looks good to me, thanks. -- John Baldwin