![]() | |||||||||||
Messages in this thread |
On 149, 05 28, 2004 at 01:16:11PM +0900, Hirokazu Takata wrote: > Hello, > > I would like to send the latest 2.6.6 kernel patch for > the Renesas M32R processor. > > Patch information to the stock 2.6.6 kernel is placed as follows: > - m32r architecture dependent portions (arch/m32r, include/asm-m32r) > http://www.linux-m32r.org/public/linux-2.6.6_m32r_20040528.arch-m32r.patch Single megapatch (1.5 Mb in size) is not the best way to merge something into Linux kernel. Now quick look at the patch itself: 1) diff -ruN linux-2.6.6.org/arch/m32r/drivers/8390.c linux-2.6.6/arch/m32r/drivers/8390.c --- linux-2.6.6.org/arch/m32r/drivers/8390.c 1970-01-01 09:00:00.000000000 +0900 +++ linux-2.6.6/arch/m32r/drivers/8390.c 2003-09-09 10:15:02.000000000 +0900 @@ -0,0 +1 @@ +#include "../../../drivers/net/8390.c" diff -ruN linux-2.6.6.org/arch/m32r/drivers/8390.h linux-2.6.6/arch/m32r/drivers/8390.h --- linux-2.6.6.org/arch/m32r/drivers/8390.h 1970-01-01 09:00:00.000000000 +0900 +++ linux-2.6.6/arch/m32r/drivers/8390.h 2003-09-09 10:15:02.000000000 +0900 @@ -0,0 +1 @@ +#include "../../../drivers/net/8390.h" Is this really needed ? 2) File arch/m32r/drivers/mappi_ne.c contains almost complete copy of drivers/net/ne.c with lots of code probably useless for your systems (old style ISA probing, ISAPnP support etc.) Also code like this is definetely unacceptable: +#ifdef CONFIG_PLAT_MAPPI + outb_p(0x4b, ioaddr + EN0_DCFG); +#elif CONFIG_PLAT_OAKS32R + outb_p(0x48, ioaddr + EN0_DCFG); +#else + outb_p(0x49, ioaddr + EN0_DCFG); +#endif This fragment can be rewritten this way, with all #ifdef mess hidden in the some header file: + outb_p(MY_MAGIC_OFFSET, ioaddr + EN0_DCFG); 3) arch/m32r/drivers/smc91111.copying contains GPL copy. Do you really need it ? arch/m32r/drivers/smc91111.readme.txt can happily live in Documentation/networking. 4) Do you really need to reimplement Linux console subsystem in arch/m32r/drivers/video/console.c, arch/m32r/drivers/video/fbmem.c, arch/m32r/drivers/video/fbcon.h ? 5) Any specific reason to implement read[bwl]/write[bwl] this way: +unsigned char _readb(unsigned long addr) +{ + return *(volatile unsigned char *)addr; +} Why not to inline them ? 6) Lots of ugly debugging #ifdef's in arch/m32r/kernel/ptrace.c 7) arch/m32r/lib/clib.c contains slightly strange abs() function which isn't used anywhere in the patch. Best reards. -- Andrey Panin | Linux and UNIX system administrator pazke@donpac.ru | PGP key: wwwkeys.pgp.net[unhandled content-type:application/pgp-signature] | ||||||||||
| Last update: 2005-03-22 14:03 [from the cache] ©2003-2008 | |||||||||||