From owner-freebsd-arch Thu Dec 13 11:12:13 2001 Delivered-To: freebsd-arch@freebsd.org Received: from mail.gmx.net (mail.gmx.net [213.165.64.20]) by hub.freebsd.org (Postfix) with SMTP id 4BDA137B417 for ; Thu, 13 Dec 2001 11:12:09 -0800 (PST) Received: (qmail 3923 invoked by uid 0); 13 Dec 2001 19:12:08 -0000 Received: from p5086f9a0.dip.t-dialin.net (HELO forge.local) (80.134.249.160) by mail.gmx.net (mp008-rz3) with SMTP; 13 Dec 2001 19:12:08 -0000 Received: from tmm by forge.local with local (Exim 3.30 #1) id 16EbHB-00013b-00; Thu, 13 Dec 2001 20:12:13 +0100 Date: Thu, 13 Dec 2001 20:12:13 +0100 From: Thomas Moestl To: Terry Lambert Cc: arch@FreeBSD.org Subject: Re: Please review: changes to MI bus code for sparc64 Message-ID: <20011213201213.B871@crow.dom2ip.de> Mail-Followup-To: Terry Lambert , arch@FreeBSD.org References: <20011213192033.A871@crow.dom2ip.de> <3C18F78D.C537D487@mindspring.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5i In-Reply-To: <3C18F78D.C537D487@mindspring.com>; from tlambert2@mindspring.com on Thu, Dec 13, 2001 at 10:46:37AM -0800 Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Thu, 2001/12/13 at 10:46:37 -0800, Terry Lambert wrote: > Thomas Moestl wrote: > [ ... ] > > Comments? > [ ... ] > > I don't understand the, isa_init() parameter change, as the post-patch > code does not appear to use "dev"? The sparc64 one does. > The PCI_BROKEN_INTPIN/PCI_INTLINE_0_BAD seem to be the same thing; > they should be protected by a single name (probably PCI_BROKEN_INTPIN) > in the #ifdef in pci.c; it should be "all or nothing" on a single > value. As it is, you must define one if you define the other, but > not vice versa, and the effect seems to be linked, anyway, so you > might as well use a single protection mechanism. It is not uncommon that i386 BIOSes to set the intline register to 0 when it should really be 0xff (to indicate an unrouted interrupt). So, I figured that it might be useful to make this an extra option. 0 as an intline value (which on sparc64 corresponds to an interrupt number offset (INO), which is added to an interrupt group number (IGN) to get the actual interrupt) can also be valid on sparc64, so there might be machines which need PCI_BROKEN_INTPIN activated, but PCI_INTLINE_0_BAD turned off to work correctly. > I don't think I fully understand why the "non-standard PCI-PCI bridge > drivers" are non-standard, and won't fit into the general framework; > if you are going to do this, doesn't it make more sense to have a > registration function that gets back a struct containing entrypoint > vectors, rather than making the vectors global? As "non-standard", > the additional overhead from the function pointer dereferences should > be an acceptable loss, to keep the code compartmentalized. I would > be tempted to protect the registration function with an #ifdef, as > well. I think that doing this would discourage casual use, since then > the symbols would be effectively protected by an option. I did this change to support the Sun APB bridge, which does not have base and limit registers, but uses a different mechanism to set the decoded ranges. So, to check resource allocations correctly, the apb driver needs to implement it's own alloc_resource method. The pcib functions which need not be 'overridden' can be directly used in the method table. > In subr_rman.c, I think it would be better to name "amask"/"bmask" as > "align_mask"/"bound_mask", so that it was more obvious that you were > not just using alphabetically inspired scratch variables. > > Also in subr_rman.c, in int_rman_activate_resource(), the change from > 32 to 31 is not obvious... should you also change "> 0" to ">= 0"? It's > not clear to me what this change fixes. I'm probably just missing > something... 1 << 32 is out of u_int32_t range, so it just would make no sense. In addition, it will overflow i (which is an int) on most architectures. - thomas To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message