From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marex@denx.de>
Date: Wed, 5 Aug 2015 15:08:35 +0200
Subject: [U-Boot] [PATCH v4 1/5] dma: lpc32xx: add DMA driver
In-Reply-To: <4F172219764C784B84C2C1FF44E7DFB103009028@003FCH1MPN2-041.003f.mgd2.msft.net>
References: <1438722282-3736-2-git-send-email-slemieux.tyco@gmail.com>
 <201508042331.44924.marex@denx.de>
 <4F172219764C784B84C2C1FF44E7DFB103009028@003FCH1MPN2-041.003f.mgd2.msft.net>
Message-ID: <201508051508.35856.marex@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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@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@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