Re: [PATCH] Updating our sn code in 2.6

[Christoph Hellwig <hch@infradead.org>]

On Thu, Dec 18, 2003 at 08:59:50PM -0600, Pat Gefre wrote:
> I have created a patchset at ftp://oss.sgi.com/projects/sn2/sn2-update/
> 
> It contains all the updates for our sn I/O code. The large patch that I
> had originally sent has been broken down into 70+ smaller patches.

Thanks, this looks much better already. Comments for the patches still not
OK below. 

000-hwgfs-update.patch.inprogress

	Mostly ok, except for reintroducing hwgraph_path_lookup().

004-fakeprom-update.patch

	OK, but why is fakeprom in the kernel at all?

006-kill-big-endian-stuff.patch

	This makes merging with the IP27 I/O code much much harder
	and doesn't buy you much, so I'd prefer if it doesn't go
	in.  (Note that this isn't a really strong no), if you just
	want to get rid of the EXTRA_FLAGS using the endianess tests
	from <linux/endian.h> would be a much better idea.  (I can
	cook up a patch for that for you)

014-cleanup-pci.c.patch

	Mostly OK.  Reintroducing snia64_pci_find_bios is bogus, but
	you kill it in a later patch anyway..

016-remove-non-pic.patch

	As above I'm not too happy with this, as I'm working on a
	pcibr driver that's usable for both IP27 and SN2.  Again
	this isn't exactly a strong no, it just makes life much
	harder.  Also note that if you define IS_PIC to 1 always
	and the Bridge / XBridge test to 0 always gcc will optimize
	away the code for you.

017-remove-non-pic1.patch

	Dito.

030-pciio-cleanup.patch

	pcibr_businfo_get() with the assorted new enums looks bogus,
	else OK.

036-shub-redirect.patch

	Wrong opening placement of first opening brace in
	sn_shub_redirect_intr :)

037-shub-redirect1.patch

	What's the point point of these pcireq stuff?  And what's the
	relation to the shub redirection?

048-pci_provider.patch

	At least better than the previous variant.. :)
	set_sn_pci64() is bogus, I'm certain the qla2x00 driver
	uses the right dma interface and if qlogicfc doesn't either
	fix it or (better) don't use that broken old driver..

051-cbrick.type.patch

	Code is ok, but wrong brace placement again..

065-sn_pci_find_bios-del.patch

	Shouldn't be needed after getting 014-cleanup-pci.c.patch right.

071-xswitch.devfunc.patch

	The whole indirection was develeted on a purpose.  Do you really
	have different xswitches now?  (Don't seem to be in this patchkit).
	If yes we'd better come up with a btter abstraction.

075-rename-reorg.patch

	BRIDGE_TYPE_AND_PTR_GET is totally bogus..
	Moving pcibr_attach2 to pic_attach2 is bogus and really wrong.
	Even if _you_ don't support Bridge/Xbridge anyore it's still
	common code for all of those.  While you're at it pic.c
	really should move to the pcibr dir.
	bridge.h -> pcibr_asic move seems really pointless to me, but
	if you really want it..
	BUT: Please remove the random renamaing of the include/asm/sn/pci
	constants froms the patch, else it looks mostly okay (although
	that's hard with such a large patch doing just code reorganization),
	dito for the struct typedef names.  This really breaks anyone
	doing work on Bridge/Xbridge without any purpose.