[U-Boot] [PATCH v2 2/4] nand: lpc32xx: add hardware ECC support

[Vladimir Zapolskiy]

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