Date: Thu, 13 Dec 2001 10:46:37 -0800 From: Terry Lambert <tlambert2@mindspring.com> To: Thomas Moestl <tmoestl@gmx.net> Cc: arch@FreeBSD.org Subject: Re: Please review: changes to MI bus code for sparc64 Message-ID: <3C18F78D.C537D487@mindspring.com> References: <20011213192033.A871@crow.dom2ip.de>
next in thread | previous in thread | raw e-mail | index | archive | help
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 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. 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. 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... Other than that, the other changes look useful, even if one does change the ISA print order, since they don't seem to impair functionality in any way by doing that. -- Terry 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?3C18F78D.C537D487>