Messages in this thread | | | Date | Fri, 28 May 2004 11:23:36 +0400 | From | Andrey Panin <> | Subject | Re: [PATCH] m32r - Upgrade to v2.6.6 kernel |
| |
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] | |