Re: [PATCH] m32r - Upgrade to v2.6.6 kernel

[Andrey Panin <pazke@donpac.ru>]

[-- Attachment #1: Type: text/plain, Size: 2607 bytes --]

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]