From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S263167AbUE1M1O (ORCPT ); Fri, 28 May 2004 08:27:14 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S262963AbUE1MXG (ORCPT ); Fri, 28 May 2004 08:23:06 -0400 Received: from mail.renesas.com ([202.234.163.13]:39152 "EHLO mail01.idc.renesas.com") by vger.kernel.org with ESMTP id S262538AbUE1MWC (ORCPT ); Fri, 28 May 2004 08:22:02 -0400 Date: Fri, 28 May 2004 21:21:49 +0900 Message-ID: From: Hirokazu Takata To: linux-kernel@vger.kernel.org Subject: Re: [PATCH] m32r - Upgrade to v2.6.6 kernel In-Reply-To: <20040528072336.GD7499@pazke> References: <20040528.131611.28785624.takata.hirokazu@renesas.com> <20040528072336.GD7499@pazke> User-Agent: Wanderlust/2.10.1 (Watching The Wheels) XEmacs/21.4 (Security Through Obscurity) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org From: Andrey Panin > 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 ] > Thank you for your kind comments, again. -- Hirokazu Takata Linux M32R Project: http://www.linux-m32r.org