Messages in this thread | | | Date | Fri, 28 May 2004 18:15:40 +0400 | From | Andrey Panin <> | Subject | Re: [PATCH] m32r - Upgrade to v2.6.6 kernel |
| |
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 [unhandled content-type:application/pgp-signature] | |