[U-Boot] [PATCH v4 1/5] dma: lpc32xx: add DMA driver

Marek Vasut marex at denx.de
Wed Aug 5 15:08:35 CEST 2015


On Wednesday, August 05, 2015 at 01:00:03 PM, LEMIEUX, SYLVAIN wrote:
> Hi Marek,

Hi!

> See comment and question below.
> 
> > -----Original Message-----
> > From: Marek Vasut [mailto:marex at denx.de]
> > Sent: 4-Aug-15 5:32 PM
> > 
> > On Tuesday, August 04, 2015 at 11:04:38 PM, slemieux.tyco at gmail.com wrote:
> > > From: Sylvain Lemieux <slemieux at tycoint.com>
> > > 
> > > Incorporate DMA driver from legacy LPCLinux NXP BSP.
> > > The files taken from the legacy patch are:
> > > - lpc32xx DMA driver
> > > - lpc3250 header file DMA registers definition.
> > > 
> > > The legacy driver was updated and clean-up as part of the integration
> > > with the latest u-boot.
> > > 
> > > Signed-off-by: Sylvain Lemieux <slemieux at tycoint.com>
> > 
> > [...]
> > 
> > > +
> > > +/*
> > > + * The "dmac_ll" structure is mapping the DMA channel register,
> > > + * as per UM10326, "LPC32x0 and LPC32x0/01 User manual" - Rev. 3
> > > + * tables 84, 85, 86 & 87.
> > > + *
> > > + * The DMA client (ex. NAND SLC) can use this macro to write the
> > > + * next DMA linked list item address.
> > > + */
> > > +#define lpc32xx_dmac_next_lli(x) (uint32_t)x
> > 
> > Ew, and it's not even used :-)
> 
> This is used inside "drivers/mtd/nand/lpc32xx_nand_slc.c";
> Refer to this patch for details: https://patchwork.ozlabs.org/patch/503815/
> 
> Are we OK to keep this define there?

Sorry, I didn't realize that. But in that case, just use (x) in 
lpc32xx_nand_slc.c ? The macro is only obfuscating the code.

Also, the macro is wrong, not only does it do explicit cast (not a very nice
thing to do), but it also doens't put parenthesis around the x, which can
have ugly impacts.

Best regards,
Marek Vasut


More information about the U-Boot mailing list