From mboxrd@z Thu Jan  1 00:00:00 1970
From: Vladimir Zapolskiy <vz@mleia.com>
Date: Wed, 29 Jul 2015 20:22:17 +0300
Subject: [U-Boot] [PATCH v2 2/4] nand: lpc32xx: add hardware ECC support
In-Reply-To: <201507291850.16932.marex@denx.de>
References: <1438186450-4076-3-git-send-email-slemieux.tyco@gmail.com>
 <201507291850.16932.marex@denx.de>
Message-ID: <55B90BC9.50509@mleia.com>
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

Hi Marek, Sylvain,

On 29.07.2015 19:50, Marek Vasut wrote:
> On Wednesday, July 29, 2015 at 06:14:08 PM, slemieux.tyco at gmail.com wrote:
> 

[snip]

>>  static void lpc32xx_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
>>  {
>> +#if defined(CONFIG_DMA_LPC32XX) && !defined(CONFIG_SPL_BUILD)
> 
> I'm not a big fan of those ifdefs -- why not use DMA even in SPL ?

I think the main reason is that DMA in SPL has not been tested yet.

If needed (?) the dependency between DMA and SPL may be described
separately, e.g.

+#if defined(CONFIG_DMA_LPC32XX) && defined(CONFIG_SPL_BUILD)
+#warning "DMA support in SPL image is not tested"
+#endif


Regarding this particular #ifdef case I suppose just

+#if defined(CONFIG_DMA_LPC32XX)

is good enough here, either it is SPL or U-boot.

>> +	lpc32xx_nand_xfer(mtd, buf, len, 1);
>> +#else
>>  	while (len-- > 0)
>>  		*buf++ = readl(&lpc32xx_nand_slc_regs->data);
>> +#endif
>>  }

I would propose to add another pair of DMA specific
lpc32xx_read_buf()/lpc32xx_write_buf() functions chosen under
CONFIG_DMA_LPC32XX condition, this might improve readability.

--
With best wishes,
Vladimir