Date: Thu, 13 Dec 2001 20:12:13 +0100 From: Thomas Moestl <tmoestl@gmx.net> To: Terry Lambert <tlambert2@mindspring.com> Cc: arch@FreeBSD.org Subject: Re: Please review: changes to MI bus code for sparc64 Message-ID: <20011213201213.B871@crow.dom2ip.de> In-Reply-To: <3C18F78D.C537D487@mindspring.com>; from tlambert2@mindspring.com on Thu, Dec 13, 2001 at 10:46:37AM -0800 References: <20011213192033.A871@crow.dom2ip.de> <3C18F78D.C537D487@mindspring.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20011213201213.B871>