Re: [PATCH] Updating our sn code in 2.6

From: Christoph Hellwig
Date: Fri Dec 19 2003 - 06:45:39 EST


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.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/