[U-Boot] [PATCH] mtd: nand: lpc32xx slc: disable DMA support in SPL builds

[U-Boot] [PATCH] mtd: nand: lpc32xx slc: disable DMA support in SPL builds

[Vladimir Zapolskiy]

Testing and analysis shows that at the moment LPC32xx NAND SLC driver
can not get PL080 DMA backbone support in SPL build, because SPL NAND
loaders operate with subpage (ECC step to be precisely) reads, and
this is not supported in the NAND SLC + DMA + hardware ECC calculation
bundle.

The change removes a cautious build time warning and explicitly
disables DMA flavour of the driver for SPL builds, to reduce the
amound of #ifdef sections the code blocks are minimally reorganized.

Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
---
 drivers/mtd/nand/raw/lpc32xx_nand_slc.c | 78 ++++++++++---------------
 1 file changed, 32 insertions(+), 46 deletions(-)

diff --git a/drivers/mtd/nand/raw/lpc32xx_nand_slc.c b/drivers/mtd/nand/raw/lpc32xx_nand_slc.c
index 99f6e15f4e07..8615b112a21f 100644
--- a/drivers/mtd/nand/raw/lpc32xx_nand_slc.c
+++ b/drivers/mtd/nand/raw/lpc32xx_nand_slc.c
@@ -2,13 +2,12 @@
 /*
  * LPC32xx SLC NAND flash controller driver
  *
- * (C) Copyright 2015 Vladimir Zapolskiy <vz@mleia.com>
+ * (C) Copyright 2015-2018 Vladimir Zapolskiy <vz@mleia.com>
+ * Copyright (c) 2015 Tyco Fire Protection Products.
  *
  * Hardware ECC support original source code
  * Copyright (C) 2008 by NXP Semiconductors
  * Author: Kevin Wells
- *
- * Copyright (c) 2015 Tyco Fire Protection Products.
  */
 
 #include <common.h>
@@ -22,10 +21,6 @@
 #include <asm/arch/dma.h>
 #include <asm/arch/cpu.h>
 
-#if defined(CONFIG_DMA_LPC32XX) && defined(CONFIG_SPL_BUILD)
-#warning "DMA support in SPL image is not tested"
-#endif
-
 struct lpc32xx_nand_slc_regs {
 	u32 data;
 	u32 addr;
@@ -78,16 +73,14 @@ struct lpc32xx_nand_slc_regs {
  * Note: For large page devices, the default layouts are used. */
 static struct nand_ecclayout lpc32xx_nand_oob_16 = {
 	.eccbytes = 6,
-	.eccpos = {10, 11, 12, 13, 14, 15},
+	.eccpos = { 10, 11, 12, 13, 14, 15, },
 	.oobfree = {
-		{.offset = 0,
-		 . length = 4},
-		{.offset = 6,
-		 . length = 4}
-		}
+		{ .offset = 0, .length = 4, },
+		{ .offset = 6, .length = 4, },
+	}
 };
 
-#if defined(CONFIG_DMA_LPC32XX)
+#if defined(CONFIG_DMA_LPC32XX) && !defined(CONFIG_SPL_BUILD)
 #define ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / CONFIG_SYS_NAND_ECCSIZE)
 
 /*
@@ -165,7 +158,7 @@ static int lpc32xx_nand_dev_ready(struct mtd_info *mtd)
 	return readl(&lpc32xx_nand_slc_regs->stat) & STAT_NAND_READY;
 }
 
-#if defined(CONFIG_DMA_LPC32XX)
+#if defined(CONFIG_DMA_LPC32XX) && !defined(CONFIG_SPL_BUILD)
 /*
  * Prepares DMA descriptors for NAND RD/WR operations
  * If the size is < 256 Bytes then it is assumed to be
@@ -324,7 +317,6 @@ static void lpc32xx_nand_xfer(struct mtd_info *mtd, const u8 *buf,
 	if (unlikely(ret < 0))
 		BUG();
 
-
 	/* Wait for NAND to be ready */
 	while (!lpc32xx_nand_dev_ready(mtd))
 		;
@@ -404,46 +396,18 @@ int lpc32xx_correct_data(struct mtd_info *mtd, u_char *dat,
 
 	return ret2;
 }
-#endif
 
-#if defined(CONFIG_DMA_LPC32XX)
 static void lpc32xx_dma_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
 {
 	lpc32xx_nand_xfer(mtd, buf, len, 1);
 }
-#else
-static void lpc32xx_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
-{
-	while (len-- > 0)
-		*buf++ = readl(&lpc32xx_nand_slc_regs->data);
-}
-#endif
 
-static uint8_t lpc32xx_read_byte(struct mtd_info *mtd)
-{
-	return readl(&lpc32xx_nand_slc_regs->data);
-}
-
-#if defined(CONFIG_DMA_LPC32XX)
 static void lpc32xx_dma_write_buf(struct mtd_info *mtd, const uint8_t *buf,
 				  int len)
 {
 	lpc32xx_nand_xfer(mtd, buf, len, 0);
 }
-#else
-static void lpc32xx_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
-{
-	while (len-- > 0)
-		writel(*buf++, &lpc32xx_nand_slc_regs->data);
-}
-#endif
-
-static void lpc32xx_write_byte(struct mtd_info *mtd, uint8_t byte)
-{
-	writel(byte, &lpc32xx_nand_slc_regs->data);
-}
 
-#if defined(CONFIG_DMA_LPC32XX)
 /* Reuse the logic from "nand_read_page_hwecc()" */
 static int lpc32xx_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
 				uint8_t *buf, int oob_required, int page)
@@ -511,8 +475,30 @@ static int lpc32xx_write_page_hwecc(struct mtd_info *mtd,
 
 	return 0;
 }
+#else
+static void lpc32xx_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
+{
+	while (len-- > 0)
+		*buf++ = readl(&lpc32xx_nand_slc_regs->data);
+}
+
+static void lpc32xx_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
+{
+	while (len-- > 0)
+		writel(*buf++, &lpc32xx_nand_slc_regs->data);
+}
 #endif
 
+static uint8_t lpc32xx_read_byte(struct mtd_info *mtd)
+{
+	return readl(&lpc32xx_nand_slc_regs->data);
+}
+
+static void lpc32xx_write_byte(struct mtd_info *mtd, uint8_t byte)
+{
+	writel(byte, &lpc32xx_nand_slc_regs->data);
+}
+
 /*
  * LPC32xx has only one SLC NAND controller, don't utilize
  * CONFIG_SYS_NAND_SELF_INIT to be able to reuse this function
@@ -520,7 +506,7 @@ static int lpc32xx_write_page_hwecc(struct mtd_info *mtd,
  */
 int board_nand_init(struct nand_chip *lpc32xx_chip)
 {
-#if defined(CONFIG_DMA_LPC32XX)
+#if defined(CONFIG_DMA_LPC32XX) && !defined(CONFIG_SPL_BUILD)
 	int ret;
 
 	/* Acquire a channel for our use */
@@ -543,7 +529,7 @@ int board_nand_init(struct nand_chip *lpc32xx_chip)
 	lpc32xx_chip->read_byte  = lpc32xx_read_byte;
 	lpc32xx_chip->write_byte = lpc32xx_write_byte;
 
-#if defined(CONFIG_DMA_LPC32XX)
+#if defined(CONFIG_DMA_LPC32XX) && !defined(CONFIG_SPL_BUILD)
 	/* Hardware ECC calculation is supported when DMA driver is selected */
 	lpc32xx_chip->ecc.mode		= NAND_ECC_HW;
 
-- 
2.17.1

[U-Boot] [PATCH] mtd: nand: lpc32xx slc: disable DMA support in SPL builds

[Miquel Raynal]

Hi Vladimir,

Vladimir Zapolskiy <vz@mleia.com> wrote on Fri, 19 Oct 2018 03:21:18
+0300:

> Testing and analysis shows that at the moment LPC32xx NAND SLC driver
> can not get PL080 DMA backbone support in SPL build, because SPL NAND
> loaders operate with subpage (ECC step to be precisely) reads, and
> this is not supported in the NAND SLC + DMA + hardware ECC calculation
> bundle.
> 
> The change removes a cautious build time warning and explicitly
> disables DMA flavour of the driver for SPL builds, to reduce the
> amound of #ifdef sections the code blocks are minimally reorganized.
> 
> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>

Could you please split this patch?

The copyright change should be a patch on its own.
The changes in the coding style without functional changes also.
Then you can do your changes about removing DMA support for SPL.

However, on this topic, I'm not sure this is a wise idea. Maybe the SPL
should be fixed so that it can work with DMA?

Thanks,
Miquèl

[U-Boot] [PATCH] mtd: nand: lpc32xx slc: disable DMA support in SPL builds

[Vladimir Zapolskiy]

Hi Miquel,

thank you for review.

On 10/29/2018 11:34 AM, Miquel Raynal wrote:
> Hi Vladimir,
> 
> Vladimir Zapolskiy <vz@mleia.com> wrote on Fri, 19 Oct 2018 03:21:18
> +0300:
> 
>> Testing and analysis shows that at the moment LPC32xx NAND SLC driver
>> can not get PL080 DMA backbone support in SPL build, because SPL NAND
>> loaders operate with subpage (ECC step to be precisely) reads, and
>> this is not supported in the NAND SLC + DMA + hardware ECC calculation
>> bundle.
>>
>> The change removes a cautious build time warning and explicitly
>> disables DMA flavour of the driver for SPL builds, to reduce the
>> amound of #ifdef sections the code blocks are minimally reorganized.
>>
>> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
> 
> Could you please split this patch?
> 
> The copyright change should be a patch on its own.

Why? The copyright update is associated with the change. Just a copyright
change is meaningless in my humble opinion...

> The changes in the coding style without functional changes also.

This is accepted, my position was to lessen the burden on reviewers and
maintainers side. If you don't care about it, I'll drop the cleanup
change and send v2, but my comprehension that the meaning is pretty low.

> Then you can do your changes about removing DMA support for SPL.
> 
> However, on this topic, I'm not sure this is a wise idea. Maybe the SPL
> should be fixed so that it can work with DMA?

Not at the moment. Generally the problem is related to SPL NAND base
code, which assumes that subpage reads (ECC block size) are always
possible, but that's not the case for this particular NAND controller
working in DMA mode.

So, if/when this is fixed in drivers/mtd/nand/raw/nand_spl_simple.c,
only then I can consider to remove the guard in my driver.

--
Best wishes,
Vladimir

[U-Boot] mtd: nand: lpc32xx slc: disable DMA support in SPL builds

[Tom Rini]

On Fri, Oct 19, 2018 at 03:21:18AM +0300, Vladimir Zapolskiy wrote:

> Testing and analysis shows that at the moment LPC32xx NAND SLC driver
> can not get PL080 DMA backbone support in SPL build, because SPL NAND
> loaders operate with subpage (ECC step to be precisely) reads, and
> this is not supported in the NAND SLC + DMA + hardware ECC calculation
> bundle.
> 
> The change removes a cautious build time warning and explicitly
> disables DMA flavour of the driver for SPL builds, to reduce the
> amound of #ifdef sections the code blocks are minimally reorganized.
> 
> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181117/669935f8/attachment.sig>