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

[Hirokazu Takata <takata@linux-m32r.org>]

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