Wednesday 25 November 2009 10:02:52 Paul Walmsley napisaƂ(a):
> Hello Janusz,
>
> On Sat, 21 Nov 2009, Janusz Krzysztofik wrote:
> > (probably less interested persons and lists dropped)
> >
> > Tuesday 10 November 2009 11:50:52 Paul Walmsley wrote:
> > > ...  All of the LCD DMA code in plat-omap/dma.c appears to be
> > > OMAP1-only (and apparently only is available on a subset of OMAP1
> > > chips). It would be great if this code could be moved to
> > > mach-omap1/lcd_dma.c or a similar place.
> >
> > The patch just tries to implement this idea.
> >
> > Created against linux-omap for-next,
> > commit 2963c21fab52bfa8227da7f22864db393ebbc858
> >
> > Tested on OMAP1510 Amstrad Delta.
> > Compile tested using omap_generic_2420_defconfig.
> >
> > Signed-off-by: Janusz Krzysztofik <[email protected]>
> >
> > ---
> > Hi Paul,
> >
> > I hope the patch meets your suggestion closely enough.
>
> Thanks, just scanned it quickly and it looks carefully done.  Two
> suggestions below, both minor:

Paul,
Thanks for your review time.

Since I started with just duplicating files and then removing unnecessary
sections, I wondered how I could split that work into several subsequent
patches so it is more easy to verify how those chunks put into a new file
differ from those extracted from the old one, but didn't find a good solution
yet. 

> > diff -uprN git.orig/arch/arm/mach-omap1/Makefile 
> > git/arch/arm/mach-omap1/Makefile
> > --- git.orig/arch/arm/mach-omap1/Makefile   2009-11-21 00:38:45.000000000 
> > +0100
> > +++ git/arch/arm/mach-omap1/Makefile        2009-11-21 01:27:12.000000000 
> > +0100
> > @@ -3,7 +3,7 @@
> >  #
> >
> >  # Common support
> > -obj-y := io.o id.o sram.o clock.o irq.o mux.o serial.o devices.o
> > +obj-y := io.o id.o sram.o clock.o irq.o lcd_dma.o mux.o serial.o devices.o
>
> It seems that this LCD DMA controller is not present on OMAPs prior to the
> 1510, given all the references in the LCD DMA code to 1510 and 1610, etc.
> It might be good to add a Kconfig option so OMAP730/850 users can skip
> compiling it and save some memory.  The drivers that use these routines
> can then include Kconfig dependencies on that option so it is
> automatically included when those drivers are built.

OK, will prepare that for next iteration.

> > diff -uprN git.orig/arch/arm/mach-omap1/include/mach/lcd_dma.h 
> > git/arch/arm/mach-omap1/include/mach/lcd_dma.h
> > --- git.orig/arch/arm/mach-omap1/include/mach/lcd_dma.h     1970-01-01 
> > 01:00:00.000000000 +0100
> > +++ git/arch/arm/mach-omap1/include/mach/lcd_dma.h  2009-11-21 
> > 03:22:25.000000000 +0100
> > @@ -0,0 +1,78 @@ 
[snip]
> > +#ifndef __MACH_LCD_DMA_H__
>
> Consider adding _OMAP1_ in this macro name.  I doubt that name collisions
> here will be a serious issue, but adding a few extra bytes here might
> avoid some bug-hunting frustration in the long run...

OK, will do that too.

Thanks,
Janusz
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to