Re: [PATCH] CRIS architecture update

[Jeff Garzik]

Linux Kernel Mailing List wrote:
> ChangeSet 1.1726.1.144, 2004/06/01 08:52:29-07:00, akpm@xxxxxxxx
>
> 	[PATCH] CRIS architecture update
> 	
> 	From: "Mikael Starvik" <mikael.starvik@xxxxxxxx>
> 	
> 	- Lots of fixes from 2.4.
> 	
> 	- Updated for 2.6.6.
> 	
> 	- Added IDE driver
> 	
> 	Signed-off-by: Andrew Morton <akpm@xxxxxxxx>
> 	Signed-off-by: Linus Torvalds <torvalds@xxxxxxxx>

Who reviewed the ethernet driver?
Who reviewed the IDE driver?
Has Bart seen this new driver?
Why was this committed without first being run by the subsystem maintainers?

In ethernet, the MII phy probe is wrong (don't need at id 0 first), it 
should be using linux/mii.h bits rather than inventing its own, 
del_timer versus del_timer_sync is questionable, the newly-added full 
duplex handling seems to be incorrect (ref drivers/net/mii.c and drivers 
that use mii_if->full_duplex), and cosmetic issues I won't bother to 
mention.

In IDE, it uses virt_to_page() when building the scatter/gather list -- 
something that IMO should not have been allowed in -mm much less 
mainline -- and other yuckiness.  In the same function, it's 
questionable whether or not it breaks with lba48.  etrax_dma_intr 
doesn't appear to check for some DMA engine conditions that code 
comments elsewhere in the driver indicate _do_ occur.  The 
ATAPI-specific e100_start_dma code doesn't look like it will work with 
all current ATAPI drivers (ide-{cdrom,floppy,tape,scsi,...}.c).

I'm happy that the CRIS people resurfaced, too, but that's no reason to 
just shove an unreviewed patch into mainline.

	Jeff


-
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/