[PATCH] m32r - Upgrade to v2.6.6 kernel

[PATCH] m32r - Upgrade to v2.6.6 kernel

[Hirokazu Takata]

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

In this M32R kernel,
- OProfile is newly supported.
- Architecture-dependent portion codes have been modified for gcc-3.4.0.

Best Regards,
--
Hirokazu Takata
Linux/M32R Project: http://www.linux-m32r.org/

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

[Andrey Panin]

[-- 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 --]

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

[Russell King]

On Fri, May 28, 2004 at 11:23:36AM +0400, Andrey Panin wrote:
> 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.

We're currently working with Jeff Garzik to merge a SMC91x driver which
which can drive SMC91C111 chips from Nicolas Pitre.  It would be good
if m32r could use this driver as well.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

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

[Hirokazu Takata]

From: Russell King <rmk+lkml@arm.linux.org.uk>
> On Fri, May 28, 2004 at 11:23:36AM +0400, Andrey Panin wrote:
> > 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.
> 
> We're currently working with Jeff Garzik to merge a SMC91x driver which
> which can drive SMC91C111 chips from Nicolas Pitre.  It would be good
> if m32r could use this driver as well.

It is a good news. 
We would like to try your SMC91x driver.

Where can I get it from?

Thank you.
--
Hirokazu Takata
Linux/M32R Project:  http://www.linux-m32r.org

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

[Miles Bader]

Hirokazu Takata <takata@linux-m32r.org> writes:
>> We're currently working with Jeff Garzik to merge a SMC91x driver which
>> which can drive SMC91C111 chips from Nicolas Pitre.  It would be good
>> if m32r could use this driver as well.
>
> It is a good news. 
> We would like to try your SMC91x driver.

Me too!

uClinux has an SMC9111 driver which I now use, but it would be good if
the base kernel worked (I _think_ that driver's the only reason it
doesn't on this platform).

[Hmmm, uClinux also contains patches to the SMC9194 driver; I wonder if
they should be merged?  Some of the changes seem kind of ugly though
(lots of platform-specific #ifdefs).]

-Miles
-- 
A zen-buddhist walked into a pizza shop and
said, "Make me one with everything."

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

[Hirokazu Takata]

From: Andrey Panin <pazke@donpac.ru>
> On 149, 05 28, 2004 at 01:16:11PM +0900, Hirokazu Takata wrote:
> > 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.

Yes, but the m32r port has not been merged into the mainline kernel
tree yet...  May I send many small patches to LKML?


> Now quick look at the patch itself:

Thank you for your quick reply.


> 1)
> diff -ruN linux-2.6.6.org/arch/m32r/drivers/8390.c linux-2.6.6/arch/m32r/drivers/8390.c
 ...
> diff -ruN linux-2.6.6.org/arch/m32r/drivers/8390.h linux-2.6.6/arch/m32r/drivers/8390.h
 ...
> Is this really needed ?

These files are required for mappi_ne.c.
The mappi_ne.c is based on ne.c and modified for an eva board "mappi",
I think it is highly dependent on the target platform.

There are two reasons that we placed some m32r specific drivers in 
arch/m32r/drivers/:
- We think they are highly depends on the target platform, and
- We can not merge them to drivers/ because the m32r architecture
  has not been supported by the mainline linux kernel.


> 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.)

Yes.

> 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);

Do you mean I should use drivers/net/ne.c instead of mappi_ne.c?
I'm not sure how can I merge such target-dependent code fragments to ne.c.
But I will try to merge them.


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

These are obsolete codes, which were used in m32r 2.4 kernel.
In 2.6 kernel, a framebuffer device and video console driver 
should be reimplement for m32r targets...


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

They were used for debugging. 
I will rewrite to inline them, thank you.


> 6) Lots of ugly debugging #ifdef's in arch/m32r/kernel/ptrace.c

I agree.


> 7) arch/m32r/lib/clib.c contains slightly strange abs() function which
> isn't used anywhere in the patch.

Hmm..  it is really strange.  I will remove it.


> -- 
> Andrey Panin		| Linux and UNIX system administrator
> pazke@donpac.ru		| PGP key: wwwkeys.pgp.net
> [2 Digital signature <application/pgp-signature (7bit)>]
> 

Thank you for your kind comments, again.
--
Hirokazu Takata
Linux M32R Project:  http://www.linux-m32r.org

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

[Andrey Panin]

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

On 149, 05 28, 2004 at 09:21:49PM +0900, Hirokazu Takata wrote:
> From: Andrey Panin <pazke@donpac.ru>
> > On 149, 05 28, 2004 at 01:16:11PM +0900, Hirokazu Takata wrote:
> > > 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.
> 
> Yes, but the m32r port has not been merged into the mainline kernel
> tree yet...  May I send many small patches to LKML?
> 
> 
> > Now quick look at the patch itself:
> 
> Thank you for your quick reply.
> 
> 
> > 1)
> > diff -ruN linux-2.6.6.org/arch/m32r/drivers/8390.c linux-2.6.6/arch/m32r/drivers/8390.c
>  ...
> > diff -ruN linux-2.6.6.org/arch/m32r/drivers/8390.h linux-2.6.6/arch/m32r/drivers/8390.h
>  ...
> > Is this really needed ?
> 
> These files are required for mappi_ne.c.
> The mappi_ne.c is based on ne.c and modified for an eva board "mappi",
> I think it is highly dependent on the target platform.
> 
> There are two reasons that we placed some m32r specific drivers in 
> arch/m32r/drivers/:
> - We think they are highly depends on the target platform, and

So we should merge m32r core first :)

> - We can not merge them to drivers/ because the m32r architecture
>   has not been supported by the mainline linux kernel.

We have many arch specific drivers in drivers/net already.

> > 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.)
> 
> Yes.
> 
> > 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);
> 
> Do you mean I should use drivers/net/ne.c instead of mappi_ne.c?
> I'm not sure how can I merge such target-dependent code fragments to ne.c.
> But I will try to merge them.

I didn't mean merging. My point is that general policy for linux kernel
code is to avoid #ifdef cluttered code as much as possible. It's better
to hide such code in header files.
 
> > 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 ?
> 
> These are obsolete codes, which were used in m32r 2.4 kernel.
> In 2.6 kernel, a framebuffer device and video console driver 
> should be reimplement for m32r targets...
> 
> 
> > 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 ?
> 
> They were used for debugging. 
> I will rewrite to inline them, thank you.
> 
> 
> > 6) Lots of ugly debugging #ifdef's in arch/m32r/kernel/ptrace.c
> 
> I agree.
> 
> 
> > 7) arch/m32r/lib/clib.c contains slightly strange abs() function which
> > isn't used anywhere in the patch.
> 
> Hmm..  it is really strange.  I will remove it.

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

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

[Hirokazu Takata]

From: Andrey Panin <pazke@donpac.ru>
> On 149, 05 28, 2004 at 09:21:49PM +0900, Hirokazu Takata wrote:
> > > 1)
> > > diff -ruN linux-2.6.6.org/arch/m32r/drivers/8390.c linux-2.6.6/arch/m32r/drivers/8390.c
> >  ...
> > > diff -ruN linux-2.6.6.org/arch/m32r/drivers/8390.h linux-2.6.6/arch/m32r/drivers/8390.h
> >  ...
> > > Is this really needed ?
> > 
> > These files are required for mappi_ne.c.
> > The mappi_ne.c is based on ne.c and modified for an eva board "mappi",
> > I think it is highly dependent on the target platform.
> > 
> > There are two reasons that we placed some m32r specific drivers in 
> > arch/m32r/drivers/:
> > - We think they are highly depends on the target platform, and
> 
> So we should merge m32r core first :)

Yes. I hope so. :)

> > - We can not merge them to drivers/ because the m32r architecture
> >   has not been supported by the mainline linux kernel.
> 
> We have many arch specific drivers in drivers/net already.

I see.

> > > 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.)
> > 
> > Yes.
> > 
> > > 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);
> > 
> > Do you mean I should use drivers/net/ne.c instead of mappi_ne.c?
> > I'm not sure how can I merge such target-dependent code fragments to ne.c.
> > But I will try to merge them.
> 
> I didn't mean merging. My point is that general policy for linux kernel
> code is to avoid #ifdef cluttered code as much as possible. It's better
> to hide such code in header files.

OK. I understand.

--
Hirokazu Takata
Linux/M32R Project:  http://www.linux-m32r.org