[PATCH v5 00/14] Armada 370/XP NAND support

[PATCH v5 00/14] Armada 370/XP NAND support

[Ezequiel Garcia]

* Changes from v4

  * Rebased on top of l2-mtd.git origin/next branch

  * Fixed is_ready flag reset, renamed by a more descriptive
    and readble "need_wait".

* Changes from v3 (feedback from Brian Norris)

  * Add binding documentation for the nand-flash-bbt DT property.

  * Expand in the documentation and in a comment the reason for
    setting the NAND_BBT_NO_OOB_BBM option.

  * Reworked the 'is_ready' completion handler. We still have
    two completions, but we've dropped the atomic_t type as
    now the variable is no longer accesed from interruption context.

  * Fixed the ecc.read_page() which lacked the max_bitflip return.

  * Reworked the ECC strength and size setting. This is important
    to allow the MTD layer to properly report on bitflip threshold
    situation.

  * Dropped an unused fifo_size state variable

  * Use '0' instead of the wrong '-1' when the extended command type
    doesn't matter or extended semantics are not supposed to be used.

* Changes from v2 (some minor fixes as per Huang's good feedback)

  * Add some more details to the commit log in patch
    "mtd: nand: pxa3xx: Early variant detection"

  * Add an empty line between variable declaration and function body
    in patch "mtd: nand: pxa3xx: Use chip->cmdfunc instead of the internal".

  * Fix a build break caused by incomplete variable replacement:
    "mtd: nand: pxa3xx: Replace host->page_size by mtd->writesize"

* Changes from v1

Aside from several changes based in Brian's feedback, the main changes
from v1 are:

  * The controller's clock source is now fully modeled, see patche 1 to 4.
    Of course, none of those patches should be taken through the mtd
    subsystem, but I'm adding them here for completeness.

  * The chip's cmdfunc() is now independently implemented in each SoC variant.
    The rationale behind this decision is that 'chunked' I/O is the only tested
    mode on the Armada370 variant, while the old 'vanilla' I/O is the only
    tested mode on the PXA variant.

    So it's safer to have an implementation for each variant.

  * Added support for BCH-8, in other words: 8-bits of correction in a 512-byte
    region. This is obtained by using a data chunk size of 1024B, thus doubling
    the ECC BCH strength, as per this ECC engine mechanism.

  * The ECC layout in use, which must be set according to the page size and
    desired ECC strength is now strictly chosen to match only the tested
    combinations.


Ezequiel Garcia (14):
  mtd: nand: pxa3xx: Use a completion to signal device ready
  mtd: nand: pxa3xx: Use waitfunc() to wait for the device to be ready
  mtd: nand: pxa3xx: Add bad block handling
  mtd: nand: pxa3xx: Add driver-specific ECC BCH support
  mtd: nand: pxa3xx: Clear cmd buffer #3 (NDCB3) on command start
  mtd: nand: pxa3xx: Add helper function to set page address
  mtd: nand: pxa3xx: Remove READ0 switch/case falltrough
  mtd: nand: pxa3xx: Split prepare_command_pool() in two stages
  mtd: nand: pxa3xx: Move the data buffer clean to
    prepare_start_command()
  mtd: nand: pxa3xx: Fix SEQIN column address set
  mtd: nand: pxa3xx: Add a read/write buffers markers
  mtd: nand: pxa3xx: Introduce multiple page I/O support
  mtd: nand: pxa3xx: Add multiple chunk write support
  mtd: nand: pxa3xx: Add ECC BCH correctable errors detection

 .../devicetree/bindings/mtd/pxa3xx-nand.txt        |   2 +
 drivers/mtd/nand/pxa3xx_nand.c                     | 613 +++++++++++++++++----
 include/linux/platform_data/mtd-nand-pxa3xx.h      |   3 +
 3 files changed, 505 insertions(+), 113 deletions(-)

-- 
1.8.1.5

[PATCH v5 01/14] mtd: nand: pxa3xx: Use a completion to signal device ready

[Ezequiel Garcia]

The expected behavior of the waitfunc() NAND chip call is to wait
for the device to be READY (this is a standard chip line).
However, the current implementation does almost nothing, which opens
the possibility of issuing a command to a non-ready device.

Fix this by adding a new completion to wait for the ready event to arrive.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 38 ++++++++++++++++++++++++--------------
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index f9156f3..ea2f907 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -37,6 +37,7 @@
 
 #include <linux/platform_data/mtd-nand-pxa3xx.h>
 
+#define NAND_DEV_READY_TIMEOUT  50
 #define	CHIP_DELAY_TIMEOUT	(2 * HZ/10)
 #define NAND_STOP_DELAY		(2 * HZ/50)
 #define PAGE_CHUNK_SIZE		(2048)
@@ -168,7 +169,7 @@ struct pxa3xx_nand_info {
 	struct clk		*clk;
 	void __iomem		*mmio_base;
 	unsigned long		mmio_phys;
-	struct completion	cmd_complete;
+	struct completion	cmd_complete, dev_ready;
 
 	unsigned int 		buf_start;
 	unsigned int		buf_count;
@@ -198,7 +199,7 @@ struct pxa3xx_nand_info {
 	int			use_ecc;	/* use HW ECC ? */
 	int			use_dma;	/* use DMA ? */
 	int			use_spare;	/* use spare ? */
-	int			is_ready;
+	int			need_wait;
 
 	unsigned int		fifo_size;	/* max. data size in the FIFO */
 	unsigned int		data_size;	/* data to be read from FIFO */
@@ -480,7 +481,7 @@ static void start_data_dma(struct pxa3xx_nand_info *info)
 static irqreturn_t pxa3xx_nand_irq(int irq, void *devid)
 {
 	struct pxa3xx_nand_info *info = devid;
-	unsigned int status, is_completed = 0;
+	unsigned int status, is_completed = 0, is_ready = 0;
 	unsigned int ready, cmd_done;
 
 	if (info->cs == 0) {
@@ -516,8 +517,8 @@ static irqreturn_t pxa3xx_nand_irq(int irq, void *devid)
 		is_completed = 1;
 	}
 	if (status & ready) {
-		info->is_ready = 1;
 		info->state = STATE_READY;
+		is_ready = 1;
 	}
 
 	if (status & NDSR_WRCMDREQ) {
@@ -546,6 +547,8 @@ static irqreturn_t pxa3xx_nand_irq(int irq, void *devid)
 	nand_writel(info, NDSR, status);
 	if (is_completed)
 		complete(&info->cmd_complete);
+	if (is_ready)
+		complete(&info->dev_ready);
 NORMAL_IRQ_EXIT:
 	return IRQ_HANDLED;
 }
@@ -576,7 +579,6 @@ static int prepare_command_pool(struct pxa3xx_nand_info *info, int command,
 	info->oob_size		= 0;
 	info->use_ecc		= 0;
 	info->use_spare		= 1;
-	info->is_ready		= 0;
 	info->retcode		= ERR_NONE;
 	if (info->cs != 0)
 		info->ndcb0 = NDCB0_CSEL;
@@ -749,6 +751,8 @@ static void pxa3xx_nand_cmdfunc(struct mtd_info *mtd, unsigned command,
 	exec_cmd = prepare_command_pool(info, command, column, page_addr);
 	if (exec_cmd) {
 		init_completion(&info->cmd_complete);
+		init_completion(&info->dev_ready);
+		info->need_wait = 1;
 		pxa3xx_nand_start(info);
 
 		ret = wait_for_completion_timeout(&info->cmd_complete,
@@ -863,21 +867,27 @@ static int pxa3xx_nand_waitfunc(struct mtd_info *mtd, struct nand_chip *this)
 {
 	struct pxa3xx_nand_host *host = mtd->priv;
 	struct pxa3xx_nand_info *info = host->info_data;
+	int ret;
+
+	if (info->need_wait) {
+		ret = wait_for_completion_timeout(&info->dev_ready,
+				CHIP_DELAY_TIMEOUT);
+		info->need_wait = 0;
+		if (!ret) {
+			dev_err(&info->pdev->dev, "Ready time out!!!\n");
+			return NAND_STATUS_FAIL;
+		}
+	}
 
 	/* pxa3xx_nand_send_command has waited for command complete */
 	if (this->state == FL_WRITING || this->state == FL_ERASING) {
 		if (info->retcode == ERR_NONE)
 			return 0;
-		else {
-			/*
-			 * any error make it return 0x01 which will tell
-			 * the caller the erase and write fail
-			 */
-			return 0x01;
-		}
+		else
+			return NAND_STATUS_FAIL;
 	}
 
-	return 0;
+	return NAND_STATUS_READY;
 }
 
 static int pxa3xx_nand_config_flash(struct pxa3xx_nand_info *info,
@@ -1030,7 +1040,7 @@ static int pxa3xx_nand_sensing(struct pxa3xx_nand_info *info)
 		return ret;
 
 	chip->cmdfunc(mtd, NAND_CMD_RESET, 0, 0);
-	if (info->is_ready)
+	if (!info->need_wait)
 		return 0;
 
 	return -ENODEV;
-- 
1.8.1.5

[PATCH v5 02/14] mtd: nand: pxa3xx: Use waitfunc() to wait for the device to be ready

[Ezequiel Garcia]

In pxa3xx_nand_sensing() instead of simply using info->is_ready
after issuing a command, the correct way of checking is to wait
for the device to be ready through the chip's waitfunc().

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index ea2f907..8af4746 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -1040,10 +1040,11 @@ static int pxa3xx_nand_sensing(struct pxa3xx_nand_info *info)
 		return ret;
 
 	chip->cmdfunc(mtd, NAND_CMD_RESET, 0, 0);
-	if (!info->need_wait)
-		return 0;
+	ret = chip->waitfunc(mtd, chip);
+	if (ret & NAND_STATUS_FAIL)
+		return -ENODEV;
 
-	return -ENODEV;
+	return 0;
 }
 
 static int pxa3xx_nand_scan(struct mtd_info *mtd)
-- 
1.8.1.5

[PATCH v5 03/14] mtd: nand: pxa3xx: Add bad block handling

[Ezequiel Garcia]

Add support for flash-based bad block table using Marvell's
custom in-flash bad block table layout. The support is enabled
a 'flash_bbt' platform data or device tree parameter.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 .../devicetree/bindings/mtd/pxa3xx-nand.txt        |  2 ++
 drivers/mtd/nand/pxa3xx_nand.c                     | 37 ++++++++++++++++++++++
 include/linux/platform_data/mtd-nand-pxa3xx.h      |  3 ++
 3 files changed, 42 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/pxa3xx-nand.txt b/Documentation/devicetree/bindings/mtd/pxa3xx-nand.txt
index bed8390..86e0a56 100644
--- a/Documentation/devicetree/bindings/mtd/pxa3xx-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/pxa3xx-nand.txt
@@ -15,6 +15,8 @@ Optional properties:
  - marvell,nand-keep-config:	Set to keep the NAND controller config as set
 				by the bootloader
  - num-cs:			Number of chipselect lines to usw
+ - nand-on-flash-bbt: 		boolean to enable on flash bbt option if
+				not present false
 
 Example:
 
diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 8af4746..6293ff4 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -26,6 +26,7 @@
 #include <linux/slab.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/of_mtd.h>
 
 #if defined(CONFIG_ARCH_PXA) || defined(CONFIG_ARCH_MMP)
 #define ARCH_HAS_DMA
@@ -241,6 +242,29 @@ static struct pxa3xx_nand_flash builtin_flash_types[] = {
 { "256MiB 16-bit", 0xba20,  64, 2048, 16, 16, 2048, &timing[3] },
 };
 
+static u8 bbt_pattern[] = {'M', 'V', 'B', 'b', 't', '0' };
+static u8 bbt_mirror_pattern[] = {'1', 't', 'b', 'B', 'V', 'M' };
+
+static struct nand_bbt_descr bbt_main_descr = {
+	.options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE
+		| NAND_BBT_2BIT | NAND_BBT_VERSION,
+	.offs =	8,
+	.len = 6,
+	.veroffs = 14,
+	.maxblocks = 8,		/* Last 8 blocks in each chip */
+	.pattern = bbt_pattern
+};
+
+static struct nand_bbt_descr bbt_mirror_descr = {
+	.options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE
+		| NAND_BBT_2BIT | NAND_BBT_VERSION,
+	.offs =	8,
+	.len = 6,
+	.veroffs = 14,
+	.maxblocks = 8,		/* Last 8 blocks in each chip */
+	.pattern = bbt_mirror_pattern
+};
+
 /* Define a default flash type setting serve as flash detecting only */
 #define DEFAULT_FLASH_TYPE (&builtin_flash_types[0])
 
@@ -1126,6 +1150,18 @@ KEEP_CONFIG:
 
 	if (nand_scan_ident(mtd, 1, def))
 		return -ENODEV;
+
+	if (pdata->flash_bbt) {
+		/*
+		 * We'll use a bad block table stored in-flash and don't
+		 * allow writing the bad block marker to the flash.
+		 */
+		chip->bbt_options |= NAND_BBT_USE_FLASH |
+				     NAND_BBT_NO_OOB_BBM;
+		chip->bbt_td = &bbt_main_descr;
+		chip->bbt_md = &bbt_mirror_descr;
+	}
+
 	/* calculate addressing information */
 	if (mtd->writesize >= 2048)
 		host->col_addr_cycles = 2;
@@ -1320,6 +1356,7 @@ static int pxa3xx_nand_probe_dt(struct platform_device *pdev)
 	if (of_get_property(np, "marvell,nand-keep-config", NULL))
 		pdata->keep_config = 1;
 	of_property_read_u32(np, "num-cs", &pdata->num_cs);
+	pdata->flash_bbt = of_get_nand_on_flash_bbt(np);
 
 	pdev->dev.platform_data = pdata;
 
diff --git a/include/linux/platform_data/mtd-nand-pxa3xx.h b/include/linux/platform_data/mtd-nand-pxa3xx.h
index ffb8019..a941471 100644
--- a/include/linux/platform_data/mtd-nand-pxa3xx.h
+++ b/include/linux/platform_data/mtd-nand-pxa3xx.h
@@ -55,6 +55,9 @@ struct pxa3xx_nand_platform_data {
 	/* indicate how many chip selects will be used */
 	int	num_cs;
 
+	/* use an flash-based bad block table */
+	bool	flash_bbt;
+
 	const struct mtd_partition		*parts[NUM_CHIP_SELECT];
 	unsigned int				nr_parts[NUM_CHIP_SELECT];
 
-- 
1.8.1.5

Re: [PATCH v5 03/14] mtd: nand: pxa3xx: Add bad block handling

[Brian Norris]

On Thu, Nov 14, 2013 at 06:25:28PM -0300, Ezequiel Garcia wrote:
> --- a/Documentation/devicetree/bindings/mtd/pxa3xx-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/pxa3xx-nand.txt
> @@ -15,6 +15,8 @@ Optional properties:
>   - marvell,nand-keep-config:	Set to keep the NAND controller config as set
>  				by the bootloader
>   - num-cs:			Number of chipselect lines to usw
> + - nand-on-flash-bbt: 		boolean to enable on flash bbt option if
> +				not present false

It's not really your problem, but I just noticed that this binding
description is particularly badly worded! No need to change it in this
patch, since it's a copy-and-paste from the generic nand.txt, but it's
probably worth rewriting all of them sometime.

Brian

Re: [PATCH v5 03/14] mtd: nand: pxa3xx: Add bad block handling

[Ezequiel Garcia]

On Thu, Nov 14, 2013 at 02:12:12PM -0800, Brian Norris wrote:
> On Thu, Nov 14, 2013 at 06:25:28PM -0300, Ezequiel Garcia wrote:
> > --- a/Documentation/devicetree/bindings/mtd/pxa3xx-nand.txt
> > +++ b/Documentation/devicetree/bindings/mtd/pxa3xx-nand.txt
> > @@ -15,6 +15,8 @@ Optional properties:
> >   - marvell,nand-keep-config:	Set to keep the NAND controller config as set
> >  				by the bootloader
> >   - num-cs:			Number of chipselect lines to usw
> > + - nand-on-flash-bbt: 		boolean to enable on flash bbt option if
> > +				not present false
> 
> It's not really your problem, but I just noticed that this binding
> description is particularly badly worded! No need to change it in this
> patch, since it's a copy-and-paste from the generic nand.txt, but it's
> probably worth rewriting all of them sometime.
> 

Indeed. And it's a bit embarrasing that I did such obviously mediocre
copy-paste.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[PATCH v5 04/14] mtd: nand: pxa3xx: Add driver-specific ECC BCH support

[Ezequiel Garcia]

This commit adds the BCH ECC support available in NFCv2 controller.
Depending on the detected required strength the respective ECC layout
is selected.

This commit adds an empty ECC layout, since support to access large
pages is first required. Once that support is added, a proper ECC
layout will be added as well.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 86 +++++++++++++++++++++++++++++++++---------
 1 file changed, 69 insertions(+), 17 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 6293ff4..40eff6d 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -58,6 +58,7 @@
 #define NDPCR		(0x18) /* Page Count Register */
 #define NDBDR0		(0x1C) /* Bad Block Register 0 */
 #define NDBDR1		(0x20) /* Bad Block Register 1 */
+#define NDECCCTRL	(0x28) /* ECC control */
 #define NDDB		(0x40) /* Data Buffer */
 #define NDCB0		(0x48) /* Command Buffer0 */
 #define NDCB1		(0x4C) /* Command Buffer1 */
@@ -198,6 +199,7 @@ struct pxa3xx_nand_info {
 
 	int			cs;
 	int			use_ecc;	/* use HW ECC ? */
+	int			ecc_bch;	/* using BCH ECC? */
 	int			use_dma;	/* use DMA ? */
 	int			use_spare;	/* use spare ? */
 	int			need_wait;
@@ -205,6 +207,8 @@ struct pxa3xx_nand_info {
 	unsigned int		fifo_size;	/* max. data size in the FIFO */
 	unsigned int		data_size;	/* data to be read from FIFO */
 	unsigned int		oob_size;
+	unsigned int		spare_size;
+	unsigned int		ecc_size;
 	int 			retcode;
 
 	/* cached register value */
@@ -339,19 +343,12 @@ static void pxa3xx_set_datasize(struct pxa3xx_nand_info *info)
 	int oob_enable = info->reg_ndcr & NDCR_SPARE_EN;
 
 	info->data_size = info->fifo_size;
-	if (!oob_enable) {
-		info->oob_size = 0;
+	if (!oob_enable)
 		return;
-	}
 
-	switch (info->fifo_size) {
-	case 2048:
-		info->oob_size = (info->use_ecc) ? 40 : 64;
-		break;
-	case 512:
-		info->oob_size = (info->use_ecc) ? 8 : 16;
-		break;
-	}
+	info->oob_size = info->spare_size;
+	if (!info->use_ecc)
+		info->oob_size += info->ecc_size;
 }
 
 /**
@@ -366,10 +363,15 @@ static void pxa3xx_nand_start(struct pxa3xx_nand_info *info)
 
 	ndcr = info->reg_ndcr;
 
-	if (info->use_ecc)
+	if (info->use_ecc) {
 		ndcr |= NDCR_ECC_EN;
-	else
+		if (info->ecc_bch)
+			nand_writel(info, NDECCCTRL, 0x1);
+	} else {
 		ndcr &= ~NDCR_ECC_EN;
+		if (info->ecc_bch)
+			nand_writel(info, NDECCCTRL, 0x0);
+	}
 
 	if (info->use_dma)
 		ndcr |= NDCR_DMA_EN;
@@ -1071,6 +1073,41 @@ static int pxa3xx_nand_sensing(struct pxa3xx_nand_info *info)
 	return 0;
 }
 
+static int pxa_ecc_init(struct pxa3xx_nand_info *info,
+			struct nand_ecc_ctrl *ecc,
+			int strength, int page_size)
+{
+	/*
+	 * We don't use strength here as the PXA variant
+	 * is used with non-ONFI compliant devices.
+	 */
+	if (page_size == 2048) {
+		info->spare_size = 40;
+		info->ecc_size = 24;
+		ecc->mode = NAND_ECC_HW;
+		ecc->size = 512;
+		ecc->strength = 1;
+		return 1;
+
+	} else if (page_size == 512) {
+		info->spare_size = 8;
+		info->ecc_size = 8;
+		ecc->mode = NAND_ECC_HW;
+		ecc->size = 512;
+		ecc->strength = 1;
+		return 1;
+	}
+	return 0;
+}
+
+static int armada370_ecc_init(struct pxa3xx_nand_info *info,
+			      struct nand_ecc_ctrl *ecc,
+			      int strength, int page_size)
+{
+	/* Unimplemented yet */
+	return 0;
+}
+
 static int pxa3xx_nand_scan(struct mtd_info *mtd)
 {
 	struct pxa3xx_nand_host *host = mtd->priv;
@@ -1141,13 +1178,13 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
 	pxa3xx_flash_ids[1].name = NULL;
 	def = pxa3xx_flash_ids;
 KEEP_CONFIG:
-	chip->ecc.mode = NAND_ECC_HW;
-	chip->ecc.size = info->fifo_size;
-	chip->ecc.strength = 1;
-
 	if (info->reg_ndcr & NDCR_DWIDTH_M)
 		chip->options |= NAND_BUSWIDTH_16;
 
+	/* Device detection must be done with ECC disabled */
+	if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
+		nand_writel(info, NDECCCTRL, 0x0);
+
 	if (nand_scan_ident(mtd, 1, def))
 		return -ENODEV;
 
@@ -1162,6 +1199,21 @@ KEEP_CONFIG:
 		chip->bbt_md = &bbt_mirror_descr;
 	}
 
+	if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
+		ret = armada370_ecc_init(info, &chip->ecc,
+				   chip->ecc_strength_ds,
+				   mtd->writesize);
+	else
+		ret = pxa_ecc_init(info, &chip->ecc,
+				   chip->ecc_strength_ds,
+				   mtd->writesize);
+	if (!ret) {
+		dev_err(&info->pdev->dev,
+			"ECC strength %d at page size %d is not supported\n",
+			chip->ecc_strength_ds, mtd->writesize);
+		return -ENODEV;
+	}
+
 	/* calculate addressing information */
 	if (mtd->writesize >= 2048)
 		host->col_addr_cycles = 2;
-- 
1.8.1.5

[PATCH v5 05/14] mtd: nand: pxa3xx: Clear cmd buffer #3 (NDCB3) on command start

[Ezequiel Garcia]

Command buffer #3 is not properly cleared and it keeps the last
set value. Fix this by clearing when a command is setup.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 40eff6d..5b5243f 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -606,6 +606,7 @@ static int prepare_command_pool(struct pxa3xx_nand_info *info, int command,
 	info->use_ecc		= 0;
 	info->use_spare		= 1;
 	info->retcode		= ERR_NONE;
+	info->ndcb3		= 0;
 	if (info->cs != 0)
 		info->ndcb0 = NDCB0_CSEL;
 	else
@@ -627,7 +628,6 @@ static int prepare_command_pool(struct pxa3xx_nand_info *info, int command,
 	default:
 		info->ndcb1 = 0;
 		info->ndcb2 = 0;
-		info->ndcb3 = 0;
 		break;
 	}
 
-- 
1.8.1.5

Re: [PATCH v5 05/14] mtd: nand: pxa3xx: Clear cmd buffer #3 (NDCB3) on command start

[Brian Norris]

On Thu, Nov 14, 2013 at 06:25:30PM -0300, Ezequiel Garcia wrote:
> Command buffer #3 is not properly cleared and it keeps the last
> set value. Fix this by clearing when a command is setup.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>

Does this have any bugfix ramifications for -stable? You started using
ndcb3 in:

commit 41a634303f8dbf97c8087773b9d66914d2a9c0fd
Author: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
Date:   Mon Aug 12 14:14:51 2013 -0300

    mtd: nand: pxa3xx: Use 'length override' in ONFI paramater page read

This was merged for 3.12.

Brian

[PATCH v5 06/14] mtd: nand: pxa3xx: Add helper function to set page address

[Ezequiel Garcia]

Let's simplify the code by first introducing a helper function
to set the page address, as done by the READ0, READOOB and SEQIN
commands.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 5b5243f..5e0d201 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -587,6 +587,26 @@ static inline int is_buf_blank(uint8_t *buf, size_t len)
 	return 1;
 }
 
+static void set_command_address(struct pxa3xx_nand_info *info,
+		unsigned int page_size, uint16_t column, int page_addr)
+{
+	/* small page addr setting */
+	if (page_size < PAGE_CHUNK_SIZE) {
+		info->ndcb1 = ((page_addr & 0xFFFFFF) << 8)
+				| (column & 0xFF);
+
+		info->ndcb2 = 0;
+	} else {
+		info->ndcb1 = ((page_addr & 0xFFFF) << 16)
+				| (column & 0xFFFF);
+
+		if (page_addr & 0xFF0000)
+			info->ndcb2 = (page_addr & 0xFF0000) >> 16;
+		else
+			info->ndcb2 = 0;
+	}
+}
+
 static int prepare_command_pool(struct pxa3xx_nand_info *info, int command,
 		uint16_t column, int page_addr)
 {
@@ -650,22 +670,8 @@ static int prepare_command_pool(struct pxa3xx_nand_info *info, int command,
 			info->ndcb0 |= NDCB0_DBC | (NAND_CMD_READSTART << 8);
 
 	case NAND_CMD_SEQIN:
-		/* small page addr setting */
-		if (unlikely(mtd->writesize < PAGE_CHUNK_SIZE)) {
-			info->ndcb1 = ((page_addr & 0xFFFFFF) << 8)
-					| (column & 0xFF);
-
-			info->ndcb2 = 0;
-		} else {
-			info->ndcb1 = ((page_addr & 0xFFFF) << 16)
-					| (column & 0xFFFF);
-
-			if (page_addr & 0xFF0000)
-				info->ndcb2 = (page_addr & 0xFF0000) >> 16;
-			else
-				info->ndcb2 = 0;
-		}
 
+		set_command_address(info, mtd->writesize, column, page_addr);
 		info->buf_count = mtd->writesize + mtd->oobsize;
 		memset(info->data_buff, 0xFF, info->buf_count);
 
-- 
1.8.1.5

[PATCH v5 07/14] mtd: nand: pxa3xx: Remove READ0 switch/case falltrough

[Ezequiel Garcia]

READ0 and READOOB command preparation has a falltrough to SEQIN
case, where the command address is specified.
This is certainly confusing and makes the code less readable with
no added value. Let's remove it.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 5e0d201..eea1ff8 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -669,6 +669,11 @@ static int prepare_command_pool(struct pxa3xx_nand_info *info, int command,
 		if (mtd->writesize >= PAGE_CHUNK_SIZE)
 			info->ndcb0 |= NDCB0_DBC | (NAND_CMD_READSTART << 8);
 
+		set_command_address(info, mtd->writesize, column, page_addr);
+		info->buf_count = mtd->writesize + mtd->oobsize;
+		memset(info->data_buff, 0xFF, info->buf_count);
+		break;
+
 	case NAND_CMD_SEQIN:
 
 		set_command_address(info, mtd->writesize, column, page_addr);
-- 
1.8.1.5

[PATCH v5 08/14] mtd: nand: pxa3xx: Split prepare_command_pool() in two stages

[Ezequiel Garcia]

This commit splits the prepare_command_pool() function into two
stages: prepare_start_command() / prepare_set_command().

This is a preparation patch without any functionality changes,
and is meant to allow support for multiple page reading/writing
operations.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 44 ++++++++++++++++++++++++------------------
 1 file changed, 25 insertions(+), 19 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index eea1ff8..b66f32b 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -607,18 +607,8 @@ static void set_command_address(struct pxa3xx_nand_info *info,
 	}
 }
 
-static int prepare_command_pool(struct pxa3xx_nand_info *info, int command,
-		uint16_t column, int page_addr)
+static void prepare_start_command(struct pxa3xx_nand_info *info, int command)
 {
-	int addr_cycle, exec_cmd;
-	struct pxa3xx_nand_host *host;
-	struct mtd_info *mtd;
-
-	host = info->host[info->cs];
-	mtd = host->mtd;
-	addr_cycle = 0;
-	exec_cmd = 1;
-
 	/* reset data and oob column point to handle data */
 	info->buf_start		= 0;
 	info->buf_count		= 0;
@@ -627,10 +617,6 @@ static int prepare_command_pool(struct pxa3xx_nand_info *info, int command,
 	info->use_spare		= 1;
 	info->retcode		= ERR_NONE;
 	info->ndcb3		= 0;
-	if (info->cs != 0)
-		info->ndcb0 = NDCB0_CSEL;
-	else
-		info->ndcb0 = 0;
 
 	switch (command) {
 	case NAND_CMD_READ0:
@@ -642,14 +628,32 @@ static int prepare_command_pool(struct pxa3xx_nand_info *info, int command,
 	case NAND_CMD_PARAM:
 		info->use_spare = 0;
 		break;
-	case NAND_CMD_SEQIN:
-		exec_cmd = 0;
-		break;
 	default:
 		info->ndcb1 = 0;
 		info->ndcb2 = 0;
 		break;
 	}
+}
+
+static int prepare_set_command(struct pxa3xx_nand_info *info, int command,
+		uint16_t column, int page_addr)
+{
+	int addr_cycle, exec_cmd;
+	struct pxa3xx_nand_host *host;
+	struct mtd_info *mtd;
+
+	host = info->host[info->cs];
+	mtd = host->mtd;
+	addr_cycle = 0;
+	exec_cmd = 1;
+
+	if (info->cs != 0)
+		info->ndcb0 = NDCB0_CSEL;
+	else
+		info->ndcb0 = 0;
+
+	if (command == NAND_CMD_SEQIN)
+		exec_cmd = 0;
 
 	addr_cycle = NDCB0_ADDR_CYC(host->row_addr_cycles
 				    + host->col_addr_cycles);
@@ -784,8 +788,10 @@ static void pxa3xx_nand_cmdfunc(struct mtd_info *mtd, unsigned command,
 		nand_writel(info, NDTR1CS0, info->ndtr1cs0);
 	}
 
+	prepare_start_command(info, command);
+
 	info->state = STATE_PREPARED;
-	exec_cmd = prepare_command_pool(info, command, column, page_addr);
+	exec_cmd = prepare_set_command(info, command, column, page_addr);
 	if (exec_cmd) {
 		init_completion(&info->cmd_complete);
 		init_completion(&info->dev_ready);
-- 
1.8.1.5

[PATCH v5 09/14] mtd: nand: pxa3xx: Move the data buffer clean to prepare_start_command()

[Ezequiel Garcia]

To allow future support of multiple page reading/writing, move the data
buffer clean out of prepare_set_command().

This is done to prevent the data buffer from being cleaned on every command
preparation, when a multiple command sequence is implemented to read/write
pages larger than the FIFO size (2 KiB).

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index b66f32b..1a9849f 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -609,6 +609,9 @@ static void set_command_address(struct pxa3xx_nand_info *info,
 
 static void prepare_start_command(struct pxa3xx_nand_info *info, int command)
 {
+	struct pxa3xx_nand_host *host = info->host[info->cs];
+	struct mtd_info *mtd = host->mtd;
+
 	/* reset data and oob column point to handle data */
 	info->buf_start		= 0;
 	info->buf_count		= 0;
@@ -633,6 +636,19 @@ static void prepare_start_command(struct pxa3xx_nand_info *info, int command)
 		info->ndcb2 = 0;
 		break;
 	}
+
+	/*
+	 * If we are about to isse a read command, or about to set
+	 * the write address, then clean the data buffer.
+	 */
+	if (command == NAND_CMD_READ0 ||
+	    command == NAND_CMD_READOOB ||
+	    command == NAND_CMD_SEQIN) {
+
+		info->buf_count = mtd->writesize + mtd->oobsize;
+		memset(info->data_buff, 0xFF, info->buf_count);
+	}
+
 }
 
 static int prepare_set_command(struct pxa3xx_nand_info *info, int command,
@@ -674,16 +690,11 @@ static int prepare_set_command(struct pxa3xx_nand_info *info, int command,
 			info->ndcb0 |= NDCB0_DBC | (NAND_CMD_READSTART << 8);
 
 		set_command_address(info, mtd->writesize, column, page_addr);
-		info->buf_count = mtd->writesize + mtd->oobsize;
-		memset(info->data_buff, 0xFF, info->buf_count);
 		break;
 
 	case NAND_CMD_SEQIN:
 
 		set_command_address(info, mtd->writesize, column, page_addr);
-		info->buf_count = mtd->writesize + mtd->oobsize;
-		memset(info->data_buff, 0xFF, info->buf_count);
-
 		break;
 
 	case NAND_CMD_PAGEPROG:
-- 
1.8.1.5

Re: [PATCH v5 09/14] mtd: nand: pxa3xx: Move the data buffer clean to prepare_start_command()

[Brian Norris]

On Thu, Nov 14, 2013 at 06:25:34PM -0300, Ezequiel Garcia wrote:
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -633,6 +636,19 @@ static void prepare_start_command(struct pxa3xx_nand_info *info, int command)
>  		info->ndcb2 = 0;
>  		break;
>  	}
> +
> +	/*
> +	 * If we are about to isse a read command, or about to set

s/isse/issue/

> +	 * the write address, then clean the data buffer.
> +	 */
> +	if (command == NAND_CMD_READ0 ||
> +	    command == NAND_CMD_READOOB ||
> +	    command == NAND_CMD_SEQIN) {
> +

Superfluous blank line?

> +		info->buf_count = mtd->writesize + mtd->oobsize;
> +		memset(info->data_buff, 0xFF, info->buf_count);
> +	}
> +
>  }
>  
>  static int prepare_set_command(struct pxa3xx_nand_info *info, int command,

I'll make these changes myself, if I take the series. The rest of the
series is looking good so far (up to this point), so it's not worth a
respin.

Brian

Re: [PATCH v5 09/14] mtd: nand: pxa3xx: Move the data buffer clean to prepare_start_command()

[Ezequiel Garcia]

On Thu, Nov 14, 2013 at 02:25:54PM -0800, Brian Norris wrote:
> On Thu, Nov 14, 2013 at 06:25:34PM -0300, Ezequiel Garcia wrote:
> > --- a/drivers/mtd/nand/pxa3xx_nand.c
> > +++ b/drivers/mtd/nand/pxa3xx_nand.c
> > @@ -633,6 +636,19 @@ static void prepare_start_command(struct pxa3xx_nand_info *info, int command)
> >  		info->ndcb2 = 0;
> >  		break;
> >  	}
> > +
> > +	/*
> > +	 * If we are about to isse a read command, or about to set
> 
> s/isse/issue/
> 

Ack.

> > +	 * the write address, then clean the data buffer.
> > +	 */
> > +	if (command == NAND_CMD_READ0 ||
> > +	    command == NAND_CMD_READOOB ||
> > +	    command == NAND_CMD_SEQIN) {
> > +
> 
> Superfluous blank line?
> 

No, I find blank lines increase readability. I know others might
disagree. Feel free to chop it.

> > +		info->buf_count = mtd->writesize + mtd->oobsize;
> > +		memset(info->data_buff, 0xFF, info->buf_count);
> > +	}
> > +
> >  }
> >  
> >  static int prepare_set_command(struct pxa3xx_nand_info *info, int command,
> 
> I'll make these changes myself, if I take the series. The rest of the
> series is looking good so far (up to this point), so it's not worth a
> respin.
> 

OK.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[PATCH v5 10/14] mtd: nand: pxa3xx: Fix SEQIN column address set

[Ezequiel Garcia]

This commit adds support page programming with a non-zero "column"
address setting. This is important to support OOB writing, through
command sequences such as:

  cmdfunc(mtd, NAND_CMD_SEQIN, mtd->writesize, ofs);
  write_buf(mtd, oob_buf, 6);
  cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 1a9849f..d15aee6 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -694,7 +694,8 @@ static int prepare_set_command(struct pxa3xx_nand_info *info, int command,
 
 	case NAND_CMD_SEQIN:
 
-		set_command_address(info, mtd->writesize, column, page_addr);
+		info->buf_start = column;
+		set_command_address(info, mtd->writesize, 0, page_addr);
 		break;
 
 	case NAND_CMD_PAGEPROG:
-- 
1.8.1.5

[PATCH v5 11/14] mtd: nand: pxa3xx: Add a read/write buffers markers

[Ezequiel Garcia]

In preparation to support multiple (aka chunked, aka splitted)
page I/O, this commit adds 'data_buff_pos' and 'oob_buff_pos' fields
to keep track of where the next read (or write) should be done.

This will allow multiple calls to handle_data_pio() to continue
the read (or write) operation.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 40 +++++++++++++++++++++++++++++-----------
 1 file changed, 29 insertions(+), 11 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index d15aee6..8e16eeb 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -176,6 +176,8 @@ struct pxa3xx_nand_info {
 	unsigned int 		buf_start;
 	unsigned int		buf_count;
 	unsigned int		buf_size;
+	unsigned int		data_buff_pos;
+	unsigned int		oob_buff_pos;
 
 	/* DMA information */
 	int			drcmr_dat;
@@ -338,11 +340,12 @@ static void pxa3xx_nand_set_timing(struct pxa3xx_nand_host *host,
  * spare and ECC configuration.
  * Only applicable to READ0, READOOB and PAGEPROG commands.
  */
-static void pxa3xx_set_datasize(struct pxa3xx_nand_info *info)
+static void pxa3xx_set_datasize(struct pxa3xx_nand_info *info,
+				struct mtd_info *mtd)
 {
 	int oob_enable = info->reg_ndcr & NDCR_SPARE_EN;
 
-	info->data_size = info->fifo_size;
+	info->data_size = mtd->writesize;
 	if (!oob_enable)
 		return;
 
@@ -430,26 +433,39 @@ static void disable_int(struct pxa3xx_nand_info *info, uint32_t int_mask)
 
 static void handle_data_pio(struct pxa3xx_nand_info *info)
 {
+	unsigned int do_bytes = min(info->data_size, info->fifo_size);
+
 	switch (info->state) {
 	case STATE_PIO_WRITING:
-		__raw_writesl(info->mmio_base + NDDB, info->data_buff,
-				DIV_ROUND_UP(info->data_size, 4));
+		__raw_writesl(info->mmio_base + NDDB,
+			      info->data_buff + info->data_buff_pos,
+			      DIV_ROUND_UP(do_bytes, 4));
+
 		if (info->oob_size > 0)
-			__raw_writesl(info->mmio_base + NDDB, info->oob_buff,
-					DIV_ROUND_UP(info->oob_size, 4));
+			__raw_writesl(info->mmio_base + NDDB,
+				      info->oob_buff + info->oob_buff_pos,
+				      DIV_ROUND_UP(info->oob_size, 4));
 		break;
 	case STATE_PIO_READING:
-		__raw_readsl(info->mmio_base + NDDB, info->data_buff,
-				DIV_ROUND_UP(info->data_size, 4));
+		__raw_readsl(info->mmio_base + NDDB,
+			     info->data_buff + info->data_buff_pos,
+			     DIV_ROUND_UP(do_bytes, 4));
+
 		if (info->oob_size > 0)
-			__raw_readsl(info->mmio_base + NDDB, info->oob_buff,
-					DIV_ROUND_UP(info->oob_size, 4));
+			__raw_readsl(info->mmio_base + NDDB,
+				     info->oob_buff + info->oob_buff_pos,
+				     DIV_ROUND_UP(info->oob_size, 4));
 		break;
 	default:
 		dev_err(&info->pdev->dev, "%s: invalid state %d\n", __func__,
 				info->state);
 		BUG();
 	}
+
+	/* Update buffer pointers for multi-page read/write */
+	info->data_buff_pos += do_bytes;
+	info->oob_buff_pos += info->oob_size;
+	info->data_size -= do_bytes;
 }
 
 #ifdef ARCH_HAS_DMA
@@ -616,6 +632,8 @@ static void prepare_start_command(struct pxa3xx_nand_info *info, int command)
 	info->buf_start		= 0;
 	info->buf_count		= 0;
 	info->oob_size		= 0;
+	info->data_buff_pos	= 0;
+	info->oob_buff_pos	= 0;
 	info->use_ecc		= 0;
 	info->use_spare		= 1;
 	info->retcode		= ERR_NONE;
@@ -626,7 +644,7 @@ static void prepare_start_command(struct pxa3xx_nand_info *info, int command)
 	case NAND_CMD_PAGEPROG:
 		info->use_ecc = 1;
 	case NAND_CMD_READOOB:
-		pxa3xx_set_datasize(info);
+		pxa3xx_set_datasize(info, mtd);
 		break;
 	case NAND_CMD_PARAM:
 		info->use_spare = 0;
-- 
1.8.1.5

[PATCH v5 12/14] mtd: nand: pxa3xx: Introduce multiple page I/O support

[Ezequiel Garcia]

As preparation work to fully support large pages, this commit adds
the initial infrastructure to support splitted (aka chunked) I/O
operation. This commit adds support for read, and follow-up patches
will add write support.

When a read (aka READ0) command is issued, the driver loops issuing
the same command until all the requested data is transfered, changing
the 'extended' command field as needed.

For instance, if the driver is required to read a 4 KiB page, using a
chunk size of 2 KiB, the transaction is splitted in:
1. Monolithic read, first 2 KiB page chunk is read
2. Last naked read, second and last 2KiB page chunk is read

If ECC is enabled it is calculated on each chunk transfered and added
at a controller-fixed location after the data chunk that must be
spare area.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 182 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 172 insertions(+), 10 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 8e16eeb..838068f 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -103,6 +103,8 @@
 #define NDCB0_ST_ROW_EN         (0x1 << 26)
 #define NDCB0_AUTO_RS		(0x1 << 25)
 #define NDCB0_CSEL		(0x1 << 24)
+#define NDCB0_EXT_CMD_TYPE_MASK	(0x7 << 29)
+#define NDCB0_EXT_CMD_TYPE(x)	(((x) << 29) & NDCB0_EXT_CMD_TYPE_MASK)
 #define NDCB0_CMD_TYPE_MASK	(0x7 << 21)
 #define NDCB0_CMD_TYPE(x)	(((x) << 21) & NDCB0_CMD_TYPE_MASK)
 #define NDCB0_NC		(0x1 << 20)
@@ -113,6 +115,14 @@
 #define NDCB0_CMD1_MASK		(0xff)
 #define NDCB0_ADDR_CYC_SHIFT	(16)
 
+#define EXT_CMD_TYPE_DISPATCH	6 /* Command dispatch */
+#define EXT_CMD_TYPE_NAKED_RW	5 /* Naked read or Naked write */
+#define EXT_CMD_TYPE_READ	4 /* Read */
+#define EXT_CMD_TYPE_DISP_WR	4 /* Command dispatch with write */
+#define EXT_CMD_TYPE_FINAL	3 /* Final command */
+#define EXT_CMD_TYPE_LAST_RW	1 /* Last naked read/write */
+#define EXT_CMD_TYPE_MONO	0 /* Monolithic read/write */
+
 /* macros for registers read/write */
 #define nand_writel(info, off, val)	\
 	__raw_writel((val), (info)->mmio_base + (off))
@@ -206,8 +216,8 @@ struct pxa3xx_nand_info {
 	int			use_spare;	/* use spare ? */
 	int			need_wait;
 
-	unsigned int		fifo_size;	/* max. data size in the FIFO */
 	unsigned int		data_size;	/* data to be read from FIFO */
+	unsigned int		chunk_size;	/* split commands chunk size */
 	unsigned int		oob_size;
 	unsigned int		spare_size;
 	unsigned int		ecc_size;
@@ -271,6 +281,31 @@ static struct nand_bbt_descr bbt_mirror_descr = {
 	.pattern = bbt_mirror_pattern
 };
 
+static struct nand_ecclayout ecc_layout_4KB_bch4bit = {
+	.eccbytes = 64,
+	.eccpos = {
+		32,  33,  34,  35,  36,  37,  38,  39,
+		40,  41,  42,  43,  44,  45,  46,  47,
+		48,  49,  50,  51,  52,  53,  54,  55,
+		56,  57,  58,  59,  60,  61,  62,  63,
+		96,  97,  98,  99,  100, 101, 102, 103,
+		104, 105, 106, 107, 108, 109, 110, 111,
+		112, 113, 114, 115, 116, 117, 118, 119,
+		120, 121, 122, 123, 124, 125, 126, 127},
+	/* Bootrom looks in bytes 0 & 5 for bad blocks */
+	.oobfree = { {6, 26}, { 64, 32} }
+};
+
+static struct nand_ecclayout ecc_layout_4KB_bch8bit = {
+	.eccbytes = 128,
+	.eccpos = {
+		32,  33,  34,  35,  36,  37,  38,  39,
+		40,  41,  42,  43,  44,  45,  46,  47,
+		48,  49,  50,  51,  52,  53,  54,  55,
+		56,  57,  58,  59,  60,  61,  62,  63},
+	.oobfree = { }
+};
+
 /* Define a default flash type setting serve as flash detecting only */
 #define DEFAULT_FLASH_TYPE (&builtin_flash_types[0])
 
@@ -433,7 +468,7 @@ static void disable_int(struct pxa3xx_nand_info *info, uint32_t int_mask)
 
 static void handle_data_pio(struct pxa3xx_nand_info *info)
 {
-	unsigned int do_bytes = min(info->data_size, info->fifo_size);
+	unsigned int do_bytes = min(info->data_size, info->chunk_size);
 
 	switch (info->state) {
 	case STATE_PIO_WRITING:
@@ -670,7 +705,7 @@ static void prepare_start_command(struct pxa3xx_nand_info *info, int command)
 }
 
 static int prepare_set_command(struct pxa3xx_nand_info *info, int command,
-		uint16_t column, int page_addr)
+		int ext_cmd_type, uint16_t column, int page_addr)
 {
 	int addr_cycle, exec_cmd;
 	struct pxa3xx_nand_host *host;
@@ -703,9 +738,20 @@ static int prepare_set_command(struct pxa3xx_nand_info *info, int command,
 		if (command == NAND_CMD_READOOB)
 			info->buf_start += mtd->writesize;
 
-		/* Second command setting for large pages */
-		if (mtd->writesize >= PAGE_CHUNK_SIZE)
+		/*
+		 * Multiple page read needs an 'extended command type' field,
+		 * which is either naked-read or last-read according to the
+		 * state.
+		 */
+		if (mtd->writesize == PAGE_CHUNK_SIZE) {
 			info->ndcb0 |= NDCB0_DBC | (NAND_CMD_READSTART << 8);
+		} else if (mtd->writesize > PAGE_CHUNK_SIZE) {
+			info->ndcb0 |= NDCB0_DBC | (NAND_CMD_READSTART << 8)
+					| NDCB0_LEN_OVRD
+					| NDCB0_EXT_CMD_TYPE(ext_cmd_type);
+			info->ndcb3 = info->chunk_size +
+				      info->oob_size;
+		}
 
 		set_command_address(info, mtd->writesize, column, page_addr);
 		break;
@@ -821,7 +867,8 @@ static void pxa3xx_nand_cmdfunc(struct mtd_info *mtd, unsigned command,
 	prepare_start_command(info, command);
 
 	info->state = STATE_PREPARED;
-	exec_cmd = prepare_set_command(info, command, column, page_addr);
+	exec_cmd = prepare_set_command(info, command, 0, column, page_addr);
+
 	if (exec_cmd) {
 		init_completion(&info->cmd_complete);
 		init_completion(&info->dev_ready);
@@ -839,6 +886,93 @@ static void pxa3xx_nand_cmdfunc(struct mtd_info *mtd, unsigned command,
 	info->state = STATE_IDLE;
 }
 
+static void armada370_nand_cmdfunc(struct mtd_info *mtd,
+				   const unsigned command,
+				   int column, int page_addr)
+{
+	struct pxa3xx_nand_host *host = mtd->priv;
+	struct pxa3xx_nand_info *info = host->info_data;
+	int ret, exec_cmd, ext_cmd_type;
+
+	/*
+	 * if this is a x16 device then convert the input
+	 * "byte" address into a "word" address appropriate
+	 * for indexing a word-oriented device
+	 */
+	if (info->reg_ndcr & NDCR_DWIDTH_M)
+		column /= 2;
+
+	/*
+	 * There may be different NAND chip hooked to
+	 * different chip select, so check whether
+	 * chip select has been changed, if yes, reset the timing
+	 */
+	if (info->cs != host->cs) {
+		info->cs = host->cs;
+		nand_writel(info, NDTR0CS0, info->ndtr0cs0);
+		nand_writel(info, NDTR1CS0, info->ndtr1cs0);
+	}
+
+	/* Select the extended command for the first command */
+	switch (command) {
+	case NAND_CMD_READ0:
+	case NAND_CMD_READOOB:
+		ext_cmd_type = EXT_CMD_TYPE_MONO;
+		break;
+	default:
+		ext_cmd_type = 0;
+	}
+
+	prepare_start_command(info, command);
+
+	/*
+	 * Prepare the "is ready" completion before starting a command
+	 * transaction sequence. If the command is not executed the
+	 * completion will be completed, see below.
+	 *
+	 * We can do that inside the loop because the command variable
+	 * is invariant and thus so is the exec_cmd.
+	 */
+	info->need_wait = 1;
+	init_completion(&info->dev_ready);
+	do {
+		info->state = STATE_PREPARED;
+		exec_cmd = prepare_set_command(info, command, ext_cmd_type,
+					       column, page_addr);
+		if (!exec_cmd) {
+			info->need_wait = 0;
+			complete(&info->dev_ready);
+			break;
+		}
+
+		init_completion(&info->cmd_complete);
+		pxa3xx_nand_start(info);
+
+		ret = wait_for_completion_timeout(&info->cmd_complete,
+				CHIP_DELAY_TIMEOUT);
+		if (!ret) {
+			dev_err(&info->pdev->dev, "Wait time out!!!\n");
+			/* Stop State Machine for next command cycle */
+			pxa3xx_nand_stop(info);
+			break;
+		}
+
+		/* Check if the sequence is complete */
+		if (info->data_size == 0)
+			break;
+
+		if (command == NAND_CMD_READ0 || command == NAND_CMD_READOOB) {
+			/* Last read: issue a 'last naked read' */
+			if (info->data_size == info->chunk_size)
+				ext_cmd_type = EXT_CMD_TYPE_LAST_RW;
+			else
+				ext_cmd_type = EXT_CMD_TYPE_NAKED_RW;
+		}
+	} while (1);
+
+	info->state = STATE_IDLE;
+}
+
 static int pxa3xx_nand_write_page_hwecc(struct mtd_info *mtd,
 		struct nand_chip *chip, const uint8_t *buf, int oob_required)
 {
@@ -1019,13 +1153,14 @@ static int pxa3xx_nand_detect_config(struct pxa3xx_nand_info *info)
 
 	if (ndcr & NDCR_PAGE_SZ) {
 		/* Controller's FIFO size */
-		info->fifo_size = 2048;
+		info->chunk_size = 2048;
 		host->read_id_bytes = 4;
 	} else {
-		info->fifo_size = 512;
+		info->chunk_size = 512;
 		host->read_id_bytes = 2;
 	}
 
+	/* Set an initial chunk size */
 	info->reg_ndcr = ndcr & ~NDCR_INT_MASK;
 	info->ndtr0cs0 = nand_readl(info, NDTR0CS0);
 	info->ndtr1cs0 = nand_readl(info, NDTR1CS0);
@@ -1129,6 +1264,7 @@ static int pxa_ecc_init(struct pxa3xx_nand_info *info,
 	 * is used with non-ONFI compliant devices.
 	 */
 	if (page_size == 2048) {
+		info->chunk_size = 2048;
 		info->spare_size = 40;
 		info->ecc_size = 24;
 		ecc->mode = NAND_ECC_HW;
@@ -1137,6 +1273,7 @@ static int pxa_ecc_init(struct pxa3xx_nand_info *info,
 		return 1;
 
 	} else if (page_size == 512) {
+		info->chunk_size = 512;
 		info->spare_size = 8;
 		info->ecc_size = 8;
 		ecc->mode = NAND_ECC_HW;
@@ -1151,7 +1288,28 @@ static int armada370_ecc_init(struct pxa3xx_nand_info *info,
 			      struct nand_ecc_ctrl *ecc,
 			      int strength, int page_size)
 {
-	/* Unimplemented yet */
+	if (strength == 4 && page_size == 4096) {
+		info->ecc_bch = 1;
+		info->chunk_size = 2048;
+		info->spare_size = 32;
+		info->ecc_size = 32;
+		ecc->mode = NAND_ECC_HW;
+		ecc->size = info->chunk_size;
+		ecc->layout = &ecc_layout_4KB_bch4bit;
+		ecc->strength = 16;
+		return 1;
+
+	} else if (strength == 8 && page_size == 4096) {
+		info->ecc_bch = 1;
+		info->chunk_size = 1024;
+		info->spare_size = 0;
+		info->ecc_size = 32;
+		ecc->mode = NAND_ECC_HW;
+		ecc->size = info->chunk_size;
+		ecc->layout = &ecc_layout_4KB_bch8bit;
+		ecc->strength = 16;
+		return 1;
+	}
 	return 0;
 }
 
@@ -1319,12 +1477,16 @@ static int alloc_nand_resource(struct platform_device *pdev)
 		chip->controller        = &info->controller;
 		chip->waitfunc		= pxa3xx_nand_waitfunc;
 		chip->select_chip	= pxa3xx_nand_select_chip;
-		chip->cmdfunc		= pxa3xx_nand_cmdfunc;
 		chip->read_word		= pxa3xx_nand_read_word;
 		chip->read_byte		= pxa3xx_nand_read_byte;
 		chip->read_buf		= pxa3xx_nand_read_buf;
 		chip->write_buf		= pxa3xx_nand_write_buf;
 		chip->options		|= NAND_NO_SUBPAGE_WRITE;
+
+		if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
+			chip->cmdfunc = armada370_nand_cmdfunc;
+		else
+			chip->cmdfunc = pxa3xx_nand_cmdfunc;
 	}
 
 	spin_lock_init(&chip->controller->lock);
-- 
1.8.1.5

Re: [PATCH v5 12/14] mtd: nand: pxa3xx: Introduce multiple page I/O support

[Brian Norris]

On Thu, Nov 14, 2013 at 06:25:37PM -0300, Ezequiel Garcia wrote:
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -1151,7 +1288,28 @@ static int armada370_ecc_init(struct pxa3xx_nand_info *info,
>  			      struct nand_ecc_ctrl *ecc,
>  			      int strength, int page_size)
>  {
> -	/* Unimplemented yet */
> +	if (strength == 4 && page_size == 4096) {

I still think this sort of check can be improved just a bit. Can you
comment on my additional diff, pasted below? (Not even compile-tested)

> +		info->ecc_bch = 1;
> +		info->chunk_size = 2048;
> +		info->spare_size = 32;
> +		info->ecc_size = 32;
> +		ecc->mode = NAND_ECC_HW;
> +		ecc->size = info->chunk_size;
> +		ecc->layout = &ecc_layout_4KB_bch4bit;
> +		ecc->strength = 16;
> +		return 1;
> +
> +	} else if (strength == 8 && page_size == 4096) {
> +		info->ecc_bch = 1;
> +		info->chunk_size = 1024;
> +		info->spare_size = 0;
> +		info->ecc_size = 32;
> +		ecc->mode = NAND_ECC_HW;
> +		ecc->size = info->chunk_size;
> +		ecc->layout = &ecc_layout_4KB_bch8bit;
> +		ecc->strength = 16;
> +		return 1;
> +	}
>  	return 0;
>  }
>  

Brian

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 8cb6640..e219d75 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -1363,9 +1363,13 @@ static int pxa_ecc_init(struct pxa3xx_nand_info *info,
 
 static int armada370_ecc_init(struct pxa3xx_nand_info *info,
 			      struct nand_ecc_ctrl *ecc,
-			      int strength, int page_size)
+			      int strength, int ecc_stepsize, int page_size)
 {
-	if (strength == 4 && page_size == 4096) {
+	/*
+	 * Required ECC: 4-bit correction per 512 bytes
+	 * Select: 16-bit correction per 2048 bytes
+	 */
+	if (strength == 4 && ecc_stepsize == 512 && page_size == 4096) {
 		info->ecc_bch = 1;
 		info->chunk_size = 2048;
 		info->spare_size = 32;
@@ -1376,7 +1380,11 @@ static int armada370_ecc_init(struct pxa3xx_nand_info *info,
 		ecc->strength = 16;
 		return 1;
 
-	} else if (strength == 8 && page_size == 4096) {
+	/*
+	 * Required ECC: 8-bit correction per 512 bytes
+	 * Select: 16-bit correction per 1024 bytes
+	 */
+	} else if (strength == 8 && ecc_stepsize == 512 && page_size == 4096) {
 		info->ecc_bch = 1;
 		info->chunk_size = 1024;
 		info->spare_size = 0;
@@ -1484,6 +1492,7 @@ KEEP_CONFIG:
 	if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
 		ret = armada370_ecc_init(info, &chip->ecc,
 				   chip->ecc_strength_ds,
+				   chip->ecc_step_ds,
 				   mtd->writesize);
 	else
 		ret = pxa_ecc_init(info, &chip->ecc,

Re: [PATCH v5 12/14] mtd: nand: pxa3xx: Introduce multiple page I/O support

[Ezequiel Garcia]

On Thu, Nov 14, 2013 at 02:40:56PM -0800, Brian Norris wrote:
> On Thu, Nov 14, 2013 at 06:25:37PM -0300, Ezequiel Garcia wrote:
> > --- a/drivers/mtd/nand/pxa3xx_nand.c
> > +++ b/drivers/mtd/nand/pxa3xx_nand.c
> > @@ -1151,7 +1288,28 @@ static int armada370_ecc_init(struct pxa3xx_nand_info *info,
> >  			      struct nand_ecc_ctrl *ecc,
> >  			      int strength, int page_size)
> >  {
> > -	/* Unimplemented yet */
> > +	if (strength == 4 && page_size == 4096) {
> 
> I still think this sort of check can be improved just a bit. Can you
> comment on my additional diff, pasted below? (Not even compile-tested)
> 
> > +		info->ecc_bch = 1;
> > +		info->chunk_size = 2048;
> > +		info->spare_size = 32;
> > +		info->ecc_size = 32;
> > +		ecc->mode = NAND_ECC_HW;
> > +		ecc->size = info->chunk_size;
> > +		ecc->layout = &ecc_layout_4KB_bch4bit;
> > +		ecc->strength = 16;
> > +		return 1;
> > +
> > +	} else if (strength == 8 && page_size == 4096) {
> > +		info->ecc_bch = 1;
> > +		info->chunk_size = 1024;
> > +		info->spare_size = 0;
> > +		info->ecc_size = 32;
> > +		ecc->mode = NAND_ECC_HW;
> > +		ecc->size = info->chunk_size;
> > +		ecc->layout = &ecc_layout_4KB_bch8bit;
> > +		ecc->strength = 16;
> > +		return 1;
> > +	}
> >  	return 0;
> >  }
> >  
> 
> Brian
> 
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index 8cb6640..e219d75 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -1363,9 +1363,13 @@ static int pxa_ecc_init(struct pxa3xx_nand_info *info,
>  
>  static int armada370_ecc_init(struct pxa3xx_nand_info *info,
>  			      struct nand_ecc_ctrl *ecc,
> -			      int strength, int page_size)
> +			      int strength, int ecc_stepsize, int page_size)
>  {
> -	if (strength == 4 && page_size == 4096) {
> +	/*
> +	 * Required ECC: 4-bit correction per 512 bytes
> +	 * Select: 16-bit correction per 2048 bytes
> +	 */
> +	if (strength == 4 && ecc_stepsize == 512 && page_size == 4096) {
>  		info->ecc_bch = 1;
>  		info->chunk_size = 2048;
>  		info->spare_size = 32;
> @@ -1376,7 +1380,11 @@ static int armada370_ecc_init(struct pxa3xx_nand_info *info,
>  		ecc->strength = 16;
>  		return 1;
>  
> -	} else if (strength == 8 && page_size == 4096) {
> +	/*
> +	 * Required ECC: 8-bit correction per 512 bytes
> +	 * Select: 16-bit correction per 1024 bytes
> +	 */
> +	} else if (strength == 8 && ecc_stepsize == 512 && page_size == 4096) {
>  		info->ecc_bch = 1;
>  		info->chunk_size = 1024;
>  		info->spare_size = 0;
> @@ -1484,6 +1492,7 @@ KEEP_CONFIG:
>  	if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
>  		ret = armada370_ecc_init(info, &chip->ecc,
>  				   chip->ecc_strength_ds,
> +				   chip->ecc_step_ds,
>  				   mtd->writesize);
>  	else
>  		ret = pxa_ecc_init(info, &chip->ecc,

Looks good, but let me see if I'm getting this straight.

Your point is that an ECC "strength" is meaningless without
the _step_ definition.

So, I was assuming the step to be 512, and your diff is making such
assumption an explicit check.

Correct me if I didn't understand.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH v5 12/14] mtd: nand: pxa3xx: Introduce multiple page I/O support

[Brian Norris]

On Thu, Nov 14, 2013 at 08:02:50PM -0300, Ezequiel Garcia wrote:
> On Thu, Nov 14, 2013 at 02:40:56PM -0800, Brian Norris wrote:
> > On Thu, Nov 14, 2013 at 06:25:37PM -0300, Ezequiel Garcia wrote:
> > > --- a/drivers/mtd/nand/pxa3xx_nand.c
> > > +++ b/drivers/mtd/nand/pxa3xx_nand.c
> > > @@ -1151,7 +1288,28 @@ static int armada370_ecc_init(struct pxa3xx_nand_info *info,
> > >  			      struct nand_ecc_ctrl *ecc,
> > >  			      int strength, int page_size)
> > >  {
> > > -	/* Unimplemented yet */
> > > +	if (strength == 4 && page_size == 4096) {
> > 
> > I still think this sort of check can be improved just a bit. Can you
> > comment on my additional diff, pasted below? (Not even compile-tested)
> > 
> > > +		info->ecc_bch = 1;
> > > +		info->chunk_size = 2048;
> > > +		info->spare_size = 32;
> > > +		info->ecc_size = 32;
> > > +		ecc->mode = NAND_ECC_HW;
> > > +		ecc->size = info->chunk_size;
> > > +		ecc->layout = &ecc_layout_4KB_bch4bit;
> > > +		ecc->strength = 16;
> > > +		return 1;
> > > +
> > > +	} else if (strength == 8 && page_size == 4096) {
> > > +		info->ecc_bch = 1;
> > > +		info->chunk_size = 1024;
> > > +		info->spare_size = 0;
> > > +		info->ecc_size = 32;
> > > +		ecc->mode = NAND_ECC_HW;
> > > +		ecc->size = info->chunk_size;
> > > +		ecc->layout = &ecc_layout_4KB_bch8bit;
> > > +		ecc->strength = 16;
> > > +		return 1;
> > > +	}
> > >  	return 0;
> > >  }
> > >  
> > 
> > Brian
> > 
> > diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> > index 8cb6640..e219d75 100644
> > --- a/drivers/mtd/nand/pxa3xx_nand.c
> > +++ b/drivers/mtd/nand/pxa3xx_nand.c
> > @@ -1363,9 +1363,13 @@ static int pxa_ecc_init(struct pxa3xx_nand_info *info,
> >  
> >  static int armada370_ecc_init(struct pxa3xx_nand_info *info,
> >  			      struct nand_ecc_ctrl *ecc,
> > -			      int strength, int page_size)
> > +			      int strength, int ecc_stepsize, int page_size)
> >  {
> > -	if (strength == 4 && page_size == 4096) {
> > +	/*
> > +	 * Required ECC: 4-bit correction per 512 bytes
> > +	 * Select: 16-bit correction per 2048 bytes
> > +	 */
> > +	if (strength == 4 && ecc_stepsize == 512 && page_size == 4096) {
> >  		info->ecc_bch = 1;
> >  		info->chunk_size = 2048;
> >  		info->spare_size = 32;
> > @@ -1376,7 +1380,11 @@ static int armada370_ecc_init(struct pxa3xx_nand_info *info,
> >  		ecc->strength = 16;
> >  		return 1;
> >  
> > -	} else if (strength == 8 && page_size == 4096) {
> > +	/*
> > +	 * Required ECC: 8-bit correction per 512 bytes
> > +	 * Select: 16-bit correction per 1024 bytes
> > +	 */
> > +	} else if (strength == 8 && ecc_stepsize == 512 && page_size == 4096) {
> >  		info->ecc_bch = 1;
> >  		info->chunk_size = 1024;
> >  		info->spare_size = 0;
> > @@ -1484,6 +1492,7 @@ KEEP_CONFIG:
> >  	if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
> >  		ret = armada370_ecc_init(info, &chip->ecc,
> >  				   chip->ecc_strength_ds,
> > +				   chip->ecc_step_ds,
> >  				   mtd->writesize);
> >  	else
> >  		ret = pxa_ecc_init(info, &chip->ecc,
> 
> Looks good, but let me see if I'm getting this straight.
> 
> Your point is that an ECC "strength" is meaningless without
> the _step_ definition.
> 
> So, I was assuming the step to be 512, and your diff is making such
> assumption an explicit check.
> 
> Correct me if I didn't understand.

Exactly correct. I'll put a description on this and send it separately.

Brian

[PATCH v5 13/14] mtd: nand: pxa3xx: Add multiple chunk write support

[Ezequiel Garcia]

This commit adds write support for large pages (4 KiB, 8 KiB).
Such support is implemented by issuing a multiple command sequence,
transfering a set of 2 KiB chunks per transaction.

The splitted command sequence requires to send the SEQIN command
independently of the PAGEPROG command and therefore it's set as
an execution command.

Since PAGEPROG enables ECC, each 2 KiB chunk of data is written
together with ECC code at a controller-fixed location within
the flash page.

Currently, only devices with a 4 KiB page size has been tested.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 81 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 73 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 838068f..be28b81 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -760,6 +760,20 @@ static int prepare_set_command(struct pxa3xx_nand_info *info, int command,
 
 		info->buf_start = column;
 		set_command_address(info, mtd->writesize, 0, page_addr);
+
+		/*
+		 * Multiple page programming needs to execute the initial
+		 * SEQIN command that sets the page address.
+		 */
+		if (mtd->writesize > PAGE_CHUNK_SIZE) {
+			info->ndcb0 |= NDCB0_CMD_TYPE(0x1)
+				| NDCB0_EXT_CMD_TYPE(ext_cmd_type)
+				| addr_cycle
+				| command;
+			/* No data transfer in this case */
+			info->data_size = 0;
+			exec_cmd = 1;
+		}
 		break;
 
 	case NAND_CMD_PAGEPROG:
@@ -769,13 +783,40 @@ static int prepare_set_command(struct pxa3xx_nand_info *info, int command,
 			break;
 		}
 
-		info->ndcb0 |= NDCB0_CMD_TYPE(0x1)
-				| NDCB0_AUTO_RS
-				| NDCB0_ST_ROW_EN
-				| NDCB0_DBC
-				| (NAND_CMD_PAGEPROG << 8)
-				| NAND_CMD_SEQIN
-				| addr_cycle;
+		/* Second command setting for large pages */
+		if (mtd->writesize > PAGE_CHUNK_SIZE) {
+			/*
+			 * Multiple page write uses the 'extended command'
+			 * field. This can be used to issue a command dispatch
+			 * or a naked-write depending on the current stage.
+			 */
+			info->ndcb0 |= NDCB0_CMD_TYPE(0x1)
+					| NDCB0_LEN_OVRD
+					| NDCB0_EXT_CMD_TYPE(ext_cmd_type);
+			info->ndcb3 = info->chunk_size +
+				      info->oob_size;
+
+			/*
+			 * This is the command dispatch that completes a chunked
+			 * page program operation.
+			 */
+			if (info->data_size == 0) {
+				info->ndcb0 = NDCB0_CMD_TYPE(0x1)
+					| NDCB0_EXT_CMD_TYPE(ext_cmd_type)
+					| command;
+				info->ndcb1 = 0;
+				info->ndcb2 = 0;
+				info->ndcb3 = 0;
+			}
+		} else {
+			info->ndcb0 |= NDCB0_CMD_TYPE(0x1)
+					| NDCB0_AUTO_RS
+					| NDCB0_ST_ROW_EN
+					| NDCB0_DBC
+					| (NAND_CMD_PAGEPROG << 8)
+					| NAND_CMD_SEQIN
+					| addr_cycle;
+		}
 		break;
 
 	case NAND_CMD_PARAM:
@@ -919,8 +960,15 @@ static void armada370_nand_cmdfunc(struct mtd_info *mtd,
 	case NAND_CMD_READOOB:
 		ext_cmd_type = EXT_CMD_TYPE_MONO;
 		break;
+	case NAND_CMD_SEQIN:
+		ext_cmd_type = EXT_CMD_TYPE_DISPATCH;
+		break;
+	case NAND_CMD_PAGEPROG:
+		ext_cmd_type = EXT_CMD_TYPE_NAKED_RW;
+		break;
 	default:
 		ext_cmd_type = 0;
+		break;
 	}
 
 	prepare_start_command(info, command);
@@ -958,7 +1006,16 @@ static void armada370_nand_cmdfunc(struct mtd_info *mtd,
 		}
 
 		/* Check if the sequence is complete */
-		if (info->data_size == 0)
+		if (info->data_size == 0 && command != NAND_CMD_PAGEPROG)
+			break;
+
+		/*
+		 * After a splitted program command sequence has issued
+		 * the command dispatch, the command sequence is complete.
+		 */
+		if (info->data_size == 0 &&
+		    command == NAND_CMD_PAGEPROG &&
+		    ext_cmd_type == EXT_CMD_TYPE_DISPATCH)
 			break;
 
 		if (command == NAND_CMD_READ0 || command == NAND_CMD_READOOB) {
@@ -967,6 +1024,14 @@ static void armada370_nand_cmdfunc(struct mtd_info *mtd,
 				ext_cmd_type = EXT_CMD_TYPE_LAST_RW;
 			else
 				ext_cmd_type = EXT_CMD_TYPE_NAKED_RW;
+
+		/*
+		 * If a splitted program command has no more data to transfer,
+		 * the command dispatch must be issued to complete.
+		 */
+		} else if (command == NAND_CMD_PAGEPROG &&
+			   info->data_size == 0) {
+				ext_cmd_type = EXT_CMD_TYPE_DISPATCH;
 		}
 	} while (1);
 
-- 
1.8.1.5

[PATCH v5 14/14] mtd: nand: pxa3xx: Add ECC BCH correctable errors detection

[Ezequiel Garcia]

This commit extends the ECC correctable error detection to include
ECC BCH errors. The number of BCH correctable errors can be any up to 16,
and the actual value is exposed in the NDSR register.

Therefore, we change some symbol names to refer to correctable or
uncorrectable (instead of single-bit or double-bit as it was in the
Hamming case) and while at it, cleanup the detection code slightly.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 57 ++++++++++++++++++++++++++----------------
 1 file changed, 35 insertions(+), 22 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index be28b81..16a8f93 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -85,6 +85,9 @@
 #define NDCR_INT_MASK           (0xFFF)
 
 #define NDSR_MASK		(0xfff)
+#define NDSR_ERR_CNT_OFF	(16)
+#define NDSR_ERR_CNT_MASK       (0x1f)
+#define NDSR_ERR_CNT(sr)	((sr >> NDSR_ERR_CNT_OFF) & NDSR_ERR_CNT_MASK)
 #define NDSR_RDY                (0x1 << 12)
 #define NDSR_FLASH_RDY          (0x1 << 11)
 #define NDSR_CS0_PAGED		(0x1 << 10)
@@ -93,8 +96,8 @@
 #define NDSR_CS1_CMDD		(0x1 << 7)
 #define NDSR_CS0_BBD		(0x1 << 6)
 #define NDSR_CS1_BBD		(0x1 << 5)
-#define NDSR_DBERR		(0x1 << 4)
-#define NDSR_SBERR		(0x1 << 3)
+#define NDSR_UNCORERR		(0x1 << 4)
+#define NDSR_CORERR		(0x1 << 3)
 #define NDSR_WRDREQ		(0x1 << 2)
 #define NDSR_RDDREQ		(0x1 << 1)
 #define NDSR_WRCMDREQ		(0x1)
@@ -135,9 +138,9 @@ enum {
 	ERR_NONE	= 0,
 	ERR_DMABUSERR	= -1,
 	ERR_SENDCMD	= -2,
-	ERR_DBERR	= -3,
+	ERR_UNCORERR	= -3,
 	ERR_BBERR	= -4,
-	ERR_SBERR	= -5,
+	ERR_CORERR	= -5,
 };
 
 enum {
@@ -221,6 +224,8 @@ struct pxa3xx_nand_info {
 	unsigned int		oob_size;
 	unsigned int		spare_size;
 	unsigned int		ecc_size;
+	unsigned int		ecc_err_cnt;
+	unsigned int		max_bitflips;
 	int 			retcode;
 
 	/* cached register value */
@@ -571,10 +576,25 @@ static irqreturn_t pxa3xx_nand_irq(int irq, void *devid)
 
 	status = nand_readl(info, NDSR);
 
-	if (status & NDSR_DBERR)
-		info->retcode = ERR_DBERR;
-	if (status & NDSR_SBERR)
-		info->retcode = ERR_SBERR;
+	if (status & NDSR_UNCORERR)
+		info->retcode = ERR_UNCORERR;
+	if (status & NDSR_CORERR) {
+		info->retcode = ERR_CORERR;
+		if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370 &&
+		    info->ecc_bch)
+			info->ecc_err_cnt = NDSR_ERR_CNT(status);
+		else
+			info->ecc_err_cnt = 1;
+
+		/*
+		 * Each chunk composing a page is corrected independently,
+		 * and we need to store maximum number of corrected bitflips
+		 * to return it to the MTD layer in ecc.read_page().
+		 */
+		info->max_bitflips = max_t(unsigned int,
+					   info->max_bitflips,
+					   info->ecc_err_cnt);
+	}
 	if (status & (NDSR_RDDREQ | NDSR_WRDREQ)) {
 		/* whether use dma to transfer data */
 		if (info->use_dma) {
@@ -672,6 +692,7 @@ static void prepare_start_command(struct pxa3xx_nand_info *info, int command)
 	info->use_ecc		= 0;
 	info->use_spare		= 1;
 	info->retcode		= ERR_NONE;
+	info->ecc_err_cnt	= 0;
 	info->ndcb3		= 0;
 
 	switch (command) {
@@ -1053,26 +1074,18 @@ static int pxa3xx_nand_read_page_hwecc(struct mtd_info *mtd,
 {
 	struct pxa3xx_nand_host *host = mtd->priv;
 	struct pxa3xx_nand_info *info = host->info_data;
-	int max_bitflips = 0;
 
 	chip->read_buf(mtd, buf, mtd->writesize);
 	chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);
 
-	if (info->retcode == ERR_SBERR) {
-		switch (info->use_ecc) {
-		case 1:
-			max_bitflips = 1;
-			mtd->ecc_stats.corrected++;
-			break;
-		case 0:
-		default:
-			break;
-		}
-	} else if (info->retcode == ERR_DBERR) {
+	if (info->retcode == ERR_CORERR && info->use_ecc) {
+		mtd->ecc_stats.corrected += info->ecc_err_cnt;
+
+	} else if (info->retcode == ERR_UNCORERR) {
 		/*
 		 * for blank page (all 0xff), HW will calculate its ECC as
 		 * 0, which is different from the ECC information within
-		 * OOB, ignore such double bit errors
+		 * OOB, ignore such ucorrectable errors
 		 */
 		if (is_buf_blank(buf, mtd->writesize))
 			info->retcode = ERR_NONE;
@@ -1080,7 +1093,7 @@ static int pxa3xx_nand_read_page_hwecc(struct mtd_info *mtd,
 			mtd->ecc_stats.failed++;
 	}
 
-	return max_bitflips;
+	return info->max_bitflips;
 }
 
 static uint8_t pxa3xx_nand_read_byte(struct mtd_info *mtd)
-- 
1.8.1.5

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Brian Norris]

On Thu, Nov 14, 2013 at 06:25:25PM -0300, Ezequiel Garcia wrote:
> * Changes from v4
> 
>   * Rebased on top of l2-mtd.git origin/next branch
> 
>   * Fixed is_ready flag reset, renamed by a more descriptive
>     and readble "need_wait".
> 

...

I think this series is now good as-is. Pushing to l2-mtd.git/next.
Thanks for the efforts! I can submit my extra diff from patch 12 as a
separate patch for review.

Do you have any available regression tests? Or was there somebody else
we could run this by for some Tested-by's? Daniel Mack, perhaps?

Thanks,
Brian

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Brian Norris]

+ Daniel (missed the CC)

On Thu, Nov 14, 2013 at 03:04:36PM -0800, Brian Norris wrote:
> On Thu, Nov 14, 2013 at 06:25:25PM -0300, Ezequiel Garcia wrote:
> > * Changes from v4
> > 
> >   * Rebased on top of l2-mtd.git origin/next branch
> > 
> >   * Fixed is_ready flag reset, renamed by a more descriptive
> >     and readble "need_wait".
> > 
> 
> ...
> 
> I think this series is now good as-is. Pushing to l2-mtd.git/next.
> Thanks for the efforts! I can submit my extra diff from patch 12 as a
> separate patch for review.
> 
> Do you have any available regression tests? Or was there somebody else
> we could run this by for some Tested-by's? Daniel Mack, perhaps?

Brian

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Ezequiel Garcia]

On Thu, Nov 14, 2013 at 03:05:40PM -0800, Brian Norris wrote:
> + Daniel (missed the CC)
> 
> On Thu, Nov 14, 2013 at 03:04:36PM -0800, Brian Norris wrote:
> > On Thu, Nov 14, 2013 at 06:25:25PM -0300, Ezequiel Garcia wrote:
> > > * Changes from v4
> > > 
> > >   * Rebased on top of l2-mtd.git origin/next branch
> > > 
> > >   * Fixed is_ready flag reset, renamed by a more descriptive
> > >     and readble "need_wait".
> > > 
> > 
> > ...
> > 
> > I think this series is now good as-is. Pushing to l2-mtd.git/next.
> > Thanks for the efforts! I can submit my extra diff from patch 12 as a
> > separate patch for review.
> > 
> > Do you have any available regression tests? Or was there somebody else
> > we could run this by for some Tested-by's? Daniel Mack, perhaps?
> 
> Brian

Yes, if Daniel (or anybody else) can push a Tested-by, that's better because
he's a "third-party" tester.

Other than that, since I could solve my PXA board issues I can now test PXA
non-DMA and DMA. I must thank the Compulab guys for that.

And of course, I'll test my Armada 370 and Armada XP boxes.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Brian Norris]

On Thu, Nov 14, 2013 at 3:15 PM, Ezequiel Garcia
<ezequiel.garcia@free-electrons.com> wrote:
> Other than that, since I could solve my PXA board issues I can now test PXA
> non-DMA and DMA. I must thank the Compulab guys for that.

Ah, I thought you mentioned having a working PXA platform, but I
couldn't find the email quickly. That's good enough for now, then. Of
course, the more testers the better!

Brian

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Daniel Mack]

On 11/15/2013 12:15 AM, Ezequiel Garcia wrote:
> On Thu, Nov 14, 2013 at 03:05:40PM -0800, Brian Norris wrote:
>> + Daniel (missed the CC)
>>
>> On Thu, Nov 14, 2013 at 03:04:36PM -0800, Brian Norris wrote:
>>> On Thu, Nov 14, 2013 at 06:25:25PM -0300, Ezequiel Garcia wrote:
>>>> * Changes from v4
>>>>
>>>>   * Rebased on top of l2-mtd.git origin/next branch
>>>>
>>>>   * Fixed is_ready flag reset, renamed by a more descriptive
>>>>     and readble "need_wait".
>>>>
>>>
>>> ...
>>>
>>> I think this series is now good as-is. Pushing to l2-mtd.git/next.
>>> Thanks for the efforts! I can submit my extra diff from patch 12 as a
>>> separate patch for review.
>>>
>>> Do you have any available regression tests? Or was there somebody else
>>> we could run this by for some Tested-by's? Daniel Mack, perhaps?
>>
>> Brian
> 
> Yes, if Daniel (or anybody else) can push a Tested-by, that's better because
> he's a "third-party" tester.

Yes. Sorry, I could only partially follow all the discussions around
these patch sets, but I can certainly do a quick regression test on my
pxa3xx platform.

Ezequiel, could you maybe point me to a repository where I can pull
everything from you want me to have a look at?


Thanks,
Daniel

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Ezequiel Garcia]

On Fri, Nov 15, 2013 at 08:59:08AM +0100, Daniel Mack wrote:
> On 11/15/2013 12:15 AM, Ezequiel Garcia wrote:
> > On Thu, Nov 14, 2013 at 03:05:40PM -0800, Brian Norris wrote:
> >> + Daniel (missed the CC)
> >>
> >> On Thu, Nov 14, 2013 at 03:04:36PM -0800, Brian Norris wrote:
> >>> On Thu, Nov 14, 2013 at 06:25:25PM -0300, Ezequiel Garcia wrote:
> >>>> * Changes from v4
> >>>>
> >>>>   * Rebased on top of l2-mtd.git origin/next branch
> >>>>
> >>>>   * Fixed is_ready flag reset, renamed by a more descriptive
> >>>>     and readble "need_wait".
> >>>>
> >>>
> >>> ...
> >>>
> >>> I think this series is now good as-is. Pushing to l2-mtd.git/next.
> >>> Thanks for the efforts! I can submit my extra diff from patch 12 as a
> >>> separate patch for review.
> >>>
> >>> Do you have any available regression tests? Or was there somebody else
> >>> we could run this by for some Tested-by's? Daniel Mack, perhaps?
> >>
> >> Brian
> > 
> > Yes, if Daniel (or anybody else) can push a Tested-by, that's better because
> > he's a "third-party" tester.
> 
> Yes. Sorry, I could only partially follow all the discussions around
> these patch sets, but I can certainly do a quick regression test on my
> pxa3xx platform.
> 
> Ezequiel, could you maybe point me to a repository where I can pull
> everything from you want me to have a look at?
> 

Latest work is at the next branch in l2-mtd.git:

http://git.infradead.org/l2-mtd.git/shortlog/refs/heads/next

You could try that, with Brian's latest patch:

http://patchwork.ozlabs.org/patch/291408/

Thanks!
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Daniel Mack]

On 11/15/2013 02:07 PM, Ezequiel Garcia wrote:
> On Fri, Nov 15, 2013 at 08:59:08AM +0100, Daniel Mack wrote:
>> On 11/15/2013 12:15 AM, Ezequiel Garcia wrote:
>>> On Thu, Nov 14, 2013 at 03:05:40PM -0800, Brian Norris wrote:
>>>> + Daniel (missed the CC)
>>>>
>>>> On Thu, Nov 14, 2013 at 03:04:36PM -0800, Brian Norris wrote:
>>>>> On Thu, Nov 14, 2013 at 06:25:25PM -0300, Ezequiel Garcia wrote:
>>>>>> * Changes from v4
>>>>>>
>>>>>>   * Rebased on top of l2-mtd.git origin/next branch
>>>>>>
>>>>>>   * Fixed is_ready flag reset, renamed by a more descriptive
>>>>>>     and readble "need_wait".
>>>>>>
>>>>>
>>>>> ...
>>>>>
>>>>> I think this series is now good as-is. Pushing to l2-mtd.git/next.
>>>>> Thanks for the efforts! I can submit my extra diff from patch 12 as a
>>>>> separate patch for review.
>>>>>
>>>>> Do you have any available regression tests? Or was there somebody else
>>>>> we could run this by for some Tested-by's? Daniel Mack, perhaps?
>>>>
>>>> Brian
>>>
>>> Yes, if Daniel (or anybody else) can push a Tested-by, that's better because
>>> he's a "third-party" tester.
>>
>> Yes. Sorry, I could only partially follow all the discussions around
>> these patch sets, but I can certainly do a quick regression test on my
>> pxa3xx platform.
>>
>> Ezequiel, could you maybe point me to a repository where I can pull
>> everything from you want me to have a look at?
>>
> 
> Latest work is at the next branch in l2-mtd.git:
> 
> http://git.infradead.org/l2-mtd.git/shortlog/refs/heads/next
> 
> You could try that, with Brian's latest patch:
> 
> http://patchwork.ozlabs.org/patch/291408/

Thanks Ezequiel! Tested with and without DMA on my PXA3xx platform and
it seems to work just well. Awesome work :)

[    2.675715] NAND device: Manufacturer ID: 0x20, Chip ID: 0xa1 (ST
Micro NAND01GR3B2CZA6)
[    2.683965] NAND device: 128MiB, SLC, page size: 2048, OOB size: 64
[    2.690867] Scanning device for bad blocks
[    2.977987] Bad eraseblock 528 at 0x000004200000
[    2.983394] Bad eraseblock 529 at 0x000004220000
[    3.251150] 4 ofpart partitions found on MTD device pxa3xx_nand-0
[    3.257313] Creating 4 MTD partitions on "pxa3xx_nand-0":
[    3.263051] 0x000000000000-0x0000000a0000 : "Bootloader"
[    3.278845] 0x0000000a0000-0x0000000c0000 : "BootloaderEnvironment"
[    3.296388] 0x0000000c0000-0x000000120000 : "BootloaderSplashScreen"
[    3.314704] 0x000000120000-0x000008000000 : "UBI"



Best regards,
Daniel

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Ezequiel Garcia]

On Fri, Nov 15, 2013 at 02:47:22PM +0100, Daniel Mack wrote:
> On 11/15/2013 02:07 PM, Ezequiel Garcia wrote:
> > On Fri, Nov 15, 2013 at 08:59:08AM +0100, Daniel Mack wrote:
> >> On 11/15/2013 12:15 AM, Ezequiel Garcia wrote:
> >>> On Thu, Nov 14, 2013 at 03:05:40PM -0800, Brian Norris wrote:
> >>>> + Daniel (missed the CC)
> >>>>
> >>>> On Thu, Nov 14, 2013 at 03:04:36PM -0800, Brian Norris wrote:
> >>>>> On Thu, Nov 14, 2013 at 06:25:25PM -0300, Ezequiel Garcia wrote:
> >>>>>> * Changes from v4
> >>>>>>
> >>>>>>   * Rebased on top of l2-mtd.git origin/next branch
> >>>>>>
> >>>>>>   * Fixed is_ready flag reset, renamed by a more descriptive
> >>>>>>     and readble "need_wait".
> >>>>>>
> >>>>>
> >>>>> ...
> >>>>>
> >>>>> I think this series is now good as-is. Pushing to l2-mtd.git/next.
> >>>>> Thanks for the efforts! I can submit my extra diff from patch 12 as a
> >>>>> separate patch for review.
> >>>>>
> >>>>> Do you have any available regression tests? Or was there somebody else
> >>>>> we could run this by for some Tested-by's? Daniel Mack, perhaps?
> >>>>
> >>>> Brian
> >>>
> >>> Yes, if Daniel (or anybody else) can push a Tested-by, that's better because
> >>> he's a "third-party" tester.
> >>
> >> Yes. Sorry, I could only partially follow all the discussions around
> >> these patch sets, but I can certainly do a quick regression test on my
> >> pxa3xx platform.
> >>
> >> Ezequiel, could you maybe point me to a repository where I can pull
> >> everything from you want me to have a look at?
> >>
> > 
> > Latest work is at the next branch in l2-mtd.git:
> > 
> > http://git.infradead.org/l2-mtd.git/shortlog/refs/heads/next
> > 
> > You could try that, with Brian's latest patch:
> > 
> > http://patchwork.ozlabs.org/patch/291408/
> 
> Thanks Ezequiel! Tested with and without DMA on my PXA3xx platform and
> it seems to work just well. Awesome work :)
> 
> [    2.675715] NAND device: Manufacturer ID: 0x20, Chip ID: 0xa1 (ST
> Micro NAND01GR3B2CZA6)
> [    2.683965] NAND device: 128MiB, SLC, page size: 2048, OOB size: 64
> [    2.690867] Scanning device for bad blocks
> [    2.977987] Bad eraseblock 528 at 0x000004200000
> [    2.983394] Bad eraseblock 529 at 0x000004220000
> [    3.251150] 4 ofpart partitions found on MTD device pxa3xx_nand-0
> [    3.257313] Creating 4 MTD partitions on "pxa3xx_nand-0":
> [    3.263051] 0x000000000000-0x0000000a0000 : "Bootloader"
> [    3.278845] 0x0000000a0000-0x0000000c0000 : "BootloaderEnvironment"
> [    3.296388] 0x0000000c0000-0x000000120000 : "BootloaderSplashScreen"
> [    3.314704] 0x000000120000-0x000008000000 : "UBI"
> 

Good news! Have you tried with Brian's patch [1]?
In that case, is this a Tested-by?

[1] http://patchwork.ozlabs.org/patch/291408/

Thanks for all your tests!
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Daniel Mack]

On 11/15/2013 03:27 PM, Ezequiel Garcia wrote:
> On Fri, Nov 15, 2013 at 02:47:22PM +0100, Daniel Mack wrote:
>> On 11/15/2013 02:07 PM, Ezequiel Garcia wrote:
>>> On Fri, Nov 15, 2013 at 08:59:08AM +0100, Daniel Mack wrote:
>>>> On 11/15/2013 12:15 AM, Ezequiel Garcia wrote:
>>>>> On Thu, Nov 14, 2013 at 03:05:40PM -0800, Brian Norris wrote:
>>>>>> + Daniel (missed the CC)
>>>>>>
>>>>>> On Thu, Nov 14, 2013 at 03:04:36PM -0800, Brian Norris wrote:
>>>>>>> On Thu, Nov 14, 2013 at 06:25:25PM -0300, Ezequiel Garcia wrote:
>>>>>>>> * Changes from v4
>>>>>>>>
>>>>>>>>   * Rebased on top of l2-mtd.git origin/next branch
>>>>>>>>
>>>>>>>>   * Fixed is_ready flag reset, renamed by a more descriptive
>>>>>>>>     and readble "need_wait".
>>>>>>>>
>>>>>>>
>>>>>>> ...
>>>>>>>
>>>>>>> I think this series is now good as-is. Pushing to l2-mtd.git/next.
>>>>>>> Thanks for the efforts! I can submit my extra diff from patch 12 as a
>>>>>>> separate patch for review.
>>>>>>>
>>>>>>> Do you have any available regression tests? Or was there somebody else
>>>>>>> we could run this by for some Tested-by's? Daniel Mack, perhaps?
>>>>>>
>>>>>> Brian
>>>>>
>>>>> Yes, if Daniel (or anybody else) can push a Tested-by, that's better because
>>>>> he's a "third-party" tester.
>>>>
>>>> Yes. Sorry, I could only partially follow all the discussions around
>>>> these patch sets, but I can certainly do a quick regression test on my
>>>> pxa3xx platform.
>>>>
>>>> Ezequiel, could you maybe point me to a repository where I can pull
>>>> everything from you want me to have a look at?
>>>>
>>>
>>> Latest work is at the next branch in l2-mtd.git:
>>>
>>> http://git.infradead.org/l2-mtd.git/shortlog/refs/heads/next
>>>
>>> You could try that, with Brian's latest patch:
>>>
>>> http://patchwork.ozlabs.org/patch/291408/
>>
>> Thanks Ezequiel! Tested with and without DMA on my PXA3xx platform and
>> it seems to work just well. Awesome work :)
>>
>> [    2.675715] NAND device: Manufacturer ID: 0x20, Chip ID: 0xa1 (ST
>> Micro NAND01GR3B2CZA6)
>> [    2.683965] NAND device: 128MiB, SLC, page size: 2048, OOB size: 64
>> [    2.690867] Scanning device for bad blocks
>> [    2.977987] Bad eraseblock 528 at 0x000004200000
>> [    2.983394] Bad eraseblock 529 at 0x000004220000
>> [    3.251150] 4 ofpart partitions found on MTD device pxa3xx_nand-0
>> [    3.257313] Creating 4 MTD partitions on "pxa3xx_nand-0":
>> [    3.263051] 0x000000000000-0x0000000a0000 : "Bootloader"
>> [    3.278845] 0x0000000a0000-0x0000000c0000 : "BootloaderEnvironment"
>> [    3.296388] 0x0000000c0000-0x000000120000 : "BootloaderSplashScreen"
>> [    3.314704] 0x000000120000-0x000008000000 : "UBI"
>>
> 
> Good news! Have you tried with Brian's patch [1]?

Yes. l2-mtd 'next' branch + Brian's patch.

> In that case, is this a Tested-by?
> 
> [1] http://patchwork.ozlabs.org/patch/291408/

  Tested-by: Daniel Mack <zonque@gmail.com>


Daniel

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Brian Norris]

On Fri, Nov 15, 2013 at 6:30 AM, Daniel Mack <zonque@gmail.com> wrote:
> On 11/15/2013 03:27 PM, Ezequiel Garcia wrote:
>> On Fri, Nov 15, 2013 at 02:47:22PM +0100, Daniel Mack wrote:
>>> On 11/15/2013 02:07 PM, Ezequiel Garcia wrote:
>>>> On Fri, Nov 15, 2013 at 08:59:08AM +0100, Daniel Mack wrote:
>>>>> On 11/15/2013 12:15 AM, Ezequiel Garcia wrote:
>>>>>> On Thu, Nov 14, 2013 at 03:05:40PM -0800, Brian Norris wrote:
>>>>>>> + Daniel (missed the CC)
>>>>>>>
>>>>>>> On Thu, Nov 14, 2013 at 03:04:36PM -0800, Brian Norris wrote:
>>>>>>>> On Thu, Nov 14, 2013 at 06:25:25PM -0300, Ezequiel Garcia wrote:
>>>>>>>>> * Changes from v4
>>>>>>>>>
>>>>>>>>>   * Rebased on top of l2-mtd.git origin/next branch
>>>>>>>>>
>>>>>>>>>   * Fixed is_ready flag reset, renamed by a more descriptive
>>>>>>>>>     and readble "need_wait".
>>>>>>>>>
>>>>>>>>
>>>>>>>> ...
>>>>>>>>
>>>>>>>> I think this series is now good as-is. Pushing to l2-mtd.git/next.
>>>>>>>> Thanks for the efforts! I can submit my extra diff from patch 12 as a
>>>>>>>> separate patch for review.
>>>>>>>>
>>>>>>>> Do you have any available regression tests? Or was there somebody else
>>>>>>>> we could run this by for some Tested-by's? Daniel Mack, perhaps?
>>>>>>>
>>>>>>> Brian
>>>>>>
>>>>>> Yes, if Daniel (or anybody else) can push a Tested-by, that's better because
>>>>>> he's a "third-party" tester.
>>>>>
>>>>> Yes. Sorry, I could only partially follow all the discussions around
>>>>> these patch sets, but I can certainly do a quick regression test on my
>>>>> pxa3xx platform.
>>>>>
>>>>> Ezequiel, could you maybe point me to a repository where I can pull
>>>>> everything from you want me to have a look at?
>>>>>
>>>>
>>>> Latest work is at the next branch in l2-mtd.git:
>>>>
>>>> http://git.infradead.org/l2-mtd.git/shortlog/refs/heads/next
>>>>
>>>> You could try that, with Brian's latest patch:
>>>>
>>>> http://patchwork.ozlabs.org/patch/291408/
>>>
>>> Thanks Ezequiel! Tested with and without DMA on my PXA3xx platform and
>>> it seems to work just well. Awesome work :)
>>>
>>> [    2.675715] NAND device: Manufacturer ID: 0x20, Chip ID: 0xa1 (ST
>>> Micro NAND01GR3B2CZA6)
>>> [    2.683965] NAND device: 128MiB, SLC, page size: 2048, OOB size: 64
>>> [    2.690867] Scanning device for bad blocks
>>> [    2.977987] Bad eraseblock 528 at 0x000004200000
>>> [    2.983394] Bad eraseblock 529 at 0x000004220000
>>> [    3.251150] 4 ofpart partitions found on MTD device pxa3xx_nand-0
>>> [    3.257313] Creating 4 MTD partitions on "pxa3xx_nand-0":
>>> [    3.263051] 0x000000000000-0x0000000a0000 : "Bootloader"
>>> [    3.278845] 0x0000000a0000-0x0000000c0000 : "BootloaderEnvironment"
>>> [    3.296388] 0x0000000c0000-0x000000120000 : "BootloaderSplashScreen"
>>> [    3.314704] 0x000000120000-0x000008000000 : "UBI"
>>>
>>
>> Good news! Have you tried with Brian's patch [1]?
>
> Yes. l2-mtd 'next' branch + Brian's patch.
>
>> In that case, is this a Tested-by?
>>
>> [1] http://patchwork.ozlabs.org/patch/291408/
>
>   Tested-by: Daniel Mack <zonque@gmail.com>

Well, that last patch wouldn't be relevant to the PXA platform, but
I'll add your Tested-by to the whole series. Thanks!

Ezequiel, is that an "acked-by" for my last patch?

Brian

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Ezequiel Garcia]

On Fri, Nov 15, 2013 at 10:05:35AM -0800, Brian Norris wrote:
> On Fri, Nov 15, 2013 at 6:30 AM, Daniel Mack <zonque@gmail.com> wrote:
> > On 11/15/2013 03:27 PM, Ezequiel Garcia wrote:
> >> On Fri, Nov 15, 2013 at 02:47:22PM +0100, Daniel Mack wrote:
> >>> On 11/15/2013 02:07 PM, Ezequiel Garcia wrote:
> >>>> On Fri, Nov 15, 2013 at 08:59:08AM +0100, Daniel Mack wrote:
> >>>>> On 11/15/2013 12:15 AM, Ezequiel Garcia wrote:
> >>>>>> On Thu, Nov 14, 2013 at 03:05:40PM -0800, Brian Norris wrote:
> >>>>>>> + Daniel (missed the CC)
> >>>>>>>
> >>>>>>> On Thu, Nov 14, 2013 at 03:04:36PM -0800, Brian Norris wrote:
> >>>>>>>> On Thu, Nov 14, 2013 at 06:25:25PM -0300, Ezequiel Garcia wrote:
> >>>>>>>>> * Changes from v4
> >>>>>>>>>
> >>>>>>>>>   * Rebased on top of l2-mtd.git origin/next branch
> >>>>>>>>>
> >>>>>>>>>   * Fixed is_ready flag reset, renamed by a more descriptive
> >>>>>>>>>     and readble "need_wait".
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>> ...
> >>>>>>>>
> >>>>>>>> I think this series is now good as-is. Pushing to l2-mtd.git/next.
> >>>>>>>> Thanks for the efforts! I can submit my extra diff from patch 12 as a
> >>>>>>>> separate patch for review.
> >>>>>>>>
> >>>>>>>> Do you have any available regression tests? Or was there somebody else
> >>>>>>>> we could run this by for some Tested-by's? Daniel Mack, perhaps?
> >>>>>>>
> >>>>>>> Brian
> >>>>>>
> >>>>>> Yes, if Daniel (or anybody else) can push a Tested-by, that's better because
> >>>>>> he's a "third-party" tester.
> >>>>>
> >>>>> Yes. Sorry, I could only partially follow all the discussions around
> >>>>> these patch sets, but I can certainly do a quick regression test on my
> >>>>> pxa3xx platform.
> >>>>>
> >>>>> Ezequiel, could you maybe point me to a repository where I can pull
> >>>>> everything from you want me to have a look at?
> >>>>>
> >>>>
> >>>> Latest work is at the next branch in l2-mtd.git:
> >>>>
> >>>> http://git.infradead.org/l2-mtd.git/shortlog/refs/heads/next
> >>>>
> >>>> You could try that, with Brian's latest patch:
> >>>>
> >>>> http://patchwork.ozlabs.org/patch/291408/
> >>>
> >>> Thanks Ezequiel! Tested with and without DMA on my PXA3xx platform and
> >>> it seems to work just well. Awesome work :)
> >>>
> >>> [    2.675715] NAND device: Manufacturer ID: 0x20, Chip ID: 0xa1 (ST
> >>> Micro NAND01GR3B2CZA6)
> >>> [    2.683965] NAND device: 128MiB, SLC, page size: 2048, OOB size: 64
> >>> [    2.690867] Scanning device for bad blocks
> >>> [    2.977987] Bad eraseblock 528 at 0x000004200000
> >>> [    2.983394] Bad eraseblock 529 at 0x000004220000
> >>> [    3.251150] 4 ofpart partitions found on MTD device pxa3xx_nand-0
> >>> [    3.257313] Creating 4 MTD partitions on "pxa3xx_nand-0":
> >>> [    3.263051] 0x000000000000-0x0000000a0000 : "Bootloader"
> >>> [    3.278845] 0x0000000a0000-0x0000000c0000 : "BootloaderEnvironment"
> >>> [    3.296388] 0x0000000c0000-0x000000120000 : "BootloaderSplashScreen"
> >>> [    3.314704] 0x000000120000-0x000008000000 : "UBI"
> >>>
> >>
> >> Good news! Have you tried with Brian's patch [1]?
> >
> > Yes. l2-mtd 'next' branch + Brian's patch.
> >
> >> In that case, is this a Tested-by?
> >>
> >> [1] http://patchwork.ozlabs.org/patch/291408/
> >
> >   Tested-by: Daniel Mack <zonque@gmail.com>
> 
> Well, that last patch wouldn't be relevant to the PXA platform,

Ah, right. Forgot about that.

> but I'll add your Tested-by to the whole series. Thanks!
> 
> Ezequiel, is that an "acked-by" for my last patch?
> 

No, I want to give it a try on the boards. I just haven't found
time to work on it.

-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Arnaud Ebalard]

Hi Ezequiel,

Ezequiel Garcia <ezequiel.garcia@free-electrons.com> writes:

> Ezequiel Garcia (14):
>   mtd: nand: pxa3xx: Use a completion to signal device ready
>   mtd: nand: pxa3xx: Use waitfunc() to wait for the device to be ready
>   mtd: nand: pxa3xx: Add bad block handling
>   mtd: nand: pxa3xx: Add driver-specific ECC BCH support
>   mtd: nand: pxa3xx: Clear cmd buffer #3 (NDCB3) on command start
>   mtd: nand: pxa3xx: Add helper function to set page address
>   mtd: nand: pxa3xx: Remove READ0 switch/case falltrough
>   mtd: nand: pxa3xx: Split prepare_command_pool() in two stages
>   mtd: nand: pxa3xx: Move the data buffer clean to
>     prepare_start_command()
>   mtd: nand: pxa3xx: Fix SEQIN column address set
>   mtd: nand: pxa3xx: Add a read/write buffers markers
>   mtd: nand: pxa3xx: Introduce multiple page I/O support
>   mtd: nand: pxa3xx: Add multiple chunk write support
>   mtd: nand: pxa3xx: Add ECC BCH correctable errors detection
>
>  .../devicetree/bindings/mtd/pxa3xx-nand.txt        |   2 +
>  drivers/mtd/nand/pxa3xx_nand.c                     | 613 +++++++++++++++++----
>  include/linux/platform_data/mtd-nand-pxa3xx.h      |   3 +
>  3 files changed, 505 insertions(+), 113 deletions(-)

As ReadyNAS 102, 104 and 2120 all depend on your driver, I decided to
give v4 a change on a 102. As a side note, all those device have the
same NAND chip, i.e. a 128 MB hynix H27U1G8F2BTR. Additionally, this is
also the chip found on ReadyNAS Duo v2, which is perfectly handled by
orion-nand driver.

With your 31 patches in my quilt set against current linus tree (w/
2 to 4 of 31 disabled as they are already in Linus tree), I modified
my .dts in the following way:

 nand@d0000 {
         status = "okay";
         num-cs = <1>;
         marvell,nand-keep-config;
         marvell,nand-enable-arbiter;
         nand-on-flash-bbt;


  [followed by partitions]

 }

At boot, I get the following:

[   80.189852] pxa3xx-nand d00d0000.nand: This platform can't do DMA on this device
[   80.197454] NAND device: Manufacturer ID: 0xad, Chip ID: 0xf1 (Hynix H27U1G8F2BTR-BC)
[   80.205308] NAND device: 128MiB, SLC, page size: 2048, OOB size: 64
[   80.211594] pxa3xx-nand d00d0000.nand: ECC strength 1 at page size 2048 is not supported
[   80.219699] pxa3xx-nand d00d0000.nand: failed to scan nand at cs 0

I took a look and I guess this is due to armada370_ecc_init() (your
Armada version of the pxa_ecc_init() you also introduced) which contains
the following:

static int armada370_ecc_init(struct pxa3xx_nand_info *info,
			      struct nand_ecc_ctrl *ecc,
			      int strength, int page_size)
{
	if (strength == 4 && page_size == 4096) {
		info->ecc_bch = 1;
		info->chunk_size = 2048;
		info->spare_size = 32;
		info->ecc_size = 32;
		ecc->mode = NAND_ECC_HW;
		ecc->size = info->chunk_size;
		ecc->layout = &ecc_layout_4KB_bch4bit;
		ecc->strength = 16;
		return 1;

	} else if (strength == 8 && page_size == 4096) {
		info->ecc_bch = 1;
		info->chunk_size = 1024;
		info->spare_size = 0;
		info->ecc_size = 32;
		ecc->mode = NAND_ECC_HW;
		ecc->size = info->chunk_size;
		ecc->layout = &ecc_layout_4KB_bch8bit;
		ecc->strength = 16;
		return 1;
	}
	return 0;
}

i.e. return 0 because it does not support page size and strength of the
chip. I also guess this works fine on the Armada mirabox and XP GP
boards you have which both include a 1GB NAND flash w/ higher page size
and ECC strength.

I wonder if you could extend your support support to lower page size and
strength. I'd be happy to test it on a 370 and XP based platfom.

If it can help, here is what Netgear 2.6.31.8 kernel reports for the
chip on the Duo v2: 

Using Hamming 1-bit ECC for NAND device
NAND device: Manufacturer ID: 0xad, Chip ID: 0xf1 (Hynix NAND 128MiB 3,3V 8-bit)
Scanning device for bad blocks
Using static partition definition
Creating 5 MTD partitions on "nand_mtd":
0x000000000000-0x000000180000 : "u-boot"
0x000000180000-0x0000001a0000 : "u-boot-env"
0x000000200000-0x000000800000 : "uImage"
0x000000800000-0x000001800000 : "minirootfs"
0x000001800000-0x000008000000 : "jffs2"

Here is what 3.11 kernel reports for the chip on the duo v2:

ONFI param page 0 valid
ONFI flash detected
NAND device: Manufacturer ID: 0xad, Chip ID: 0xf1 (Hynix H27U1G8F2BTR-BC), 128MiB, page size: 2048, OOB size: 64
Scanning device for bad blocks
mtd: no mtd-id
5 ofpart partitions found on MTD device orion_nand
Creating 5 MTD partitions on "orion_nand":
0x000000000000-0x000000180000 : "u-boot"
0x000000180000-0x0000001a0000 : "u-boot-env"
0x000000200000-0x000000800000 : "uImage"
0x000000800000-0x000001800000 : "minirootfs"
0x000001800000-0x000008000000 : "jffs2"

Anyway, thanks for that work!

Cheers,

a+

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Thomas Petazzoni]

Dear Arnaud Ebalard,

On Sun, 24 Nov 2013 15:08:46 +0100, Arnaud Ebalard wrote:

> [   80.189852] pxa3xx-nand d00d0000.nand: This platform can't do DMA on this device
> [   80.197454] NAND device: Manufacturer ID: 0xad, Chip ID: 0xf1 (Hynix H27U1G8F2BTR-BC)
> [   80.205308] NAND device: 128MiB, SLC, page size: 2048, OOB size: 64
> [   80.211594] pxa3xx-nand d00d0000.nand: ECC strength 1 at page size 2048 is not supported
> [   80.219699] pxa3xx-nand d00d0000.nand: failed to scan nand at cs 0

[...]

> i.e. return 0 because it does not support page size and strength of the
> chip. I also guess this works fine on the Armada mirabox and XP GP
> boards you have which both include a 1GB NAND flash w/ higher page size
> and ECC strength.

Right, that's correct. Ezequiel will expand on this, but we
intentionally excluded the cases we couldn't test, such as 2048 bytes
page size.

> I wonder if you could extend your support support to lower page size and
> strength. I'd be happy to test it on a 370 and XP based platfom.

We were just waiting for people to have practical cases that we can
test, so I'm sure Ezequiel will provide patches to support 2048 bytes
page size.

Thanks for your testing, very useful!

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Ezequiel Garcia]

Hi Arnaud,

On Sun, Nov 24, 2013 at 03:08:46PM +0100, Arnaud Ebalard wrote:
> 
> As ReadyNAS 102, 104 and 2120 all depend on your driver, I decided to
> give v4 a change on a 102. As a side note, all those device have the
> same NAND chip, i.e. a 128 MB hynix H27U1G8F2BTR. Additionally, this is
> also the chip found on ReadyNAS Duo v2, which is perfectly handled by
> orion-nand driver.
> 
> With your 31 patches in my quilt set against current linus tree (w/
> 2 to 4 of 31 disabled as they are already in Linus tree), I modified
> my .dts in the following way:
> 
>  nand@d0000 {
>          status = "okay";
>          num-cs = <1>;
>          marvell,nand-keep-config;
>          marvell,nand-enable-arbiter;
>          nand-on-flash-bbt;
> 

Great! Thanks for giving NAND a chance :-)

Could you try using the devicetree snippet below?

nand@d0000 {
	/* HACK: Use legacy compatible to handle smaller pages */
	compatible = "marvell,pxa3xx-nand";
	status = "okay";
	num-cs = <1>;
	marvell,nand-keep-config;
	marvell,nand-enable-arbiter;
	nand-on-flash-bbt;

	/* partitions */
};

Please check if your device is detected and try to do some reading
first. If that works, you can try to mount the filesystem, or write
a new kernel.

If the above works, I'll prepare a proper patch.

Thanks!
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Arnaud Ebalard]

Hi,

Ezequiel Garcia <ezequiel.garcia@free-electrons.com> writes:

> On Sun, Nov 24, 2013 at 03:08:46PM +0100, Arnaud Ebalard wrote:
>> 
>> As ReadyNAS 102, 104 and 2120 all depend on your driver, I decided to
>> give v4 a change on a 102. As a side note, all those device have the
>> same NAND chip, i.e. a 128 MB hynix H27U1G8F2BTR. Additionally, this is
>> also the chip found on ReadyNAS Duo v2, which is perfectly handled by
>> orion-nand driver.
>> 
>> With your 31 patches in my quilt set against current linus tree (w/
>> 2 to 4 of 31 disabled as they are already in Linus tree), I modified
>> my .dts in the following way:
>> 
>>  nand@d0000 {
>>          status = "okay";
>>          num-cs = <1>;
>>          marvell,nand-keep-config;
>>          marvell,nand-enable-arbiter;
>>          nand-on-flash-bbt;
>> 
>
> Great! Thanks for giving NAND a chance :-)
>
> Could you try using the devicetree snippet below?
>
> nand@d0000 {
> 	/* HACK: Use legacy compatible to handle smaller pages */
> 	compatible = "marvell,pxa3xx-nand";
> 	status = "okay";
> 	num-cs = <1>;
> 	marvell,nand-keep-config;
> 	marvell,nand-enable-arbiter;
> 	nand-on-flash-bbt;
>
> 	/* partitions */
> };


The snippet above gave me the following:

pxa3xx-nand d00d0000.nand: This platform can't do DMA on this device
pxa3xx-nand d00d0000.nand: Wait time out!!!
pxa3xx-nand d00d0000.nand: Wait time out!!!
pxa3xx-nand d00d0000.nand: Wait time out!!!
pxa3xx-nand d00d0000.nand: failed to scan nand at cs 0

So, I then tried and match the chip->chip_delay with the one in
kirkwood.dtsi (by setting it to 25) but this provided the same
result.


Then, I tried a different approach: use the armada370 variant but w/ a
small extension of your armada370_ecc_init():

@@ -1388,6 +1388,14 @@
                ecc->layout = &ecc_layout_4KB_bch8bit;
                ecc->strength = 16;
                return 1;
+       } else if (page_size == 2048) {
+               info->chunk_size = 2048;
+               info->spare_size = 40;
+               info->ecc_size = 24;
+               ecc->mode = NAND_ECC_HW;
+               ecc->size = 2048;
+               ecc->strength = 1;
+               return 1;
        }
        return 0;
 }

For the record, my .dts had the following at that point:

	nand@d0000 {
		status = "okay";
		num-cs = <1>;
		marvell,nand-keep-config;
		marvell,nand-enable-arbiter;
		nand-on-flash-bbt;

                /* partitions */

        };

And \o/ i.e. here is what I get:

root@mood:~# dmesg
...
pxa3xx-nand d00d0000.nand: This platform can't do DMA on this device
NAND device: Manufacturer ID: 0xad, Chip ID: 0xf1 (Hynix H27U1G8F2BTR-BC)
NAND device: 128MiB, SLC, page size: 2048, OOB size: 64
Bad block table found at page 65472, version 0x01
Bad block table found at page 65408, version 0x01
5 ofpart partitions found on MTD device pxa3xx_nand-0
Creating 5 MTD partitions on "pxa3xx_nand-0":
0x000000000000-0x000000180000 : "u-boot"
0x000000180000-0x0000001a0000 : "u-boot-env"
0x000000200000-0x000000800000 : "uImage"
0x000000800000-0x000001800000 : "minirootfs"
0x000001800000-0x000008000000 : "jffs2"
...

root@mood:~# ls /dev/mtd
mtd0       mtd1ro     mtd3       mtd4ro     mtdblock2  
mtd0ro     mtd2       mtd3ro     mtdblock0  mtdblock3  
mtd1       mtd2ro     mtd4       mtdblock1  mtdblock4  

root@mood:~# dd if=/dev/mtd2ro of=/tmp/foo
12288+0 records in
12288+0 records out
6291456 bytes (6.3 MB) copied, 1.98731 s, 3.2 MB/s

root@mood:~# file /tmp/foo 
/tmp/foo: u-boot legacy uImage, Linux-3.12.0.rn102-00048-gbe408c, Linux/ARM, OS Kernel Image (Not compressed), 3740317 bytes, Tue Nov  5 22:24:01 2013, Load Address: 0x00008000, Entry Point: 0x00008000, Header CRC: 0xD84586E1, Data CRC: 0xC4357CED

But then /o\ i.e. write does not seem to work out of the box ;-)

root@mood:~# flash_erase /dev/mtd2 0 0 
Erasing 128 Kibyte @ 5e0000 -- 100 % complete 
root@mood:~# nand
nanddump   nandtest   nandwrite  
root@mood:~# nandwrite -p /dev/mtd2 /tmp/uImage
Writing data to block 0 at offset 0x0
[ 1456.154142] pxa3xx-nand d00d0000.nand: Wait time out!!!
[ 1456.354143] pxa3xx-nand d00d0000.nand: Wait time out!!!
[ 1456.554144] pxa3xx-nand d00d0000.nand: Wait time out!!!
[ 1456.754141] pxa3xx-nand d00d0000.nand: Wait time out!!!
[ 1456.954140] pxa3xx-nand d00d0000.nand: Wait time out!!!
[ 1457.154140] pxa3xx-nand d00d0000.nand: Wait time out!!!
[ 1457.354140] pxa3xx-nand d00d0000.nand: Wait time out!!!
[ 1457.554140] pxa3xx-nand d00d0000.nand: Wait time out!!!
[ 1457.754140] pxa3xx-nand d00d0000.nand: Wait time out!!!
[ 1457.954197] pxa3xx-nand d00d0000.nand: Wait time out!!!
[ 1458.154140] pxa3xx-nand d00d0000.nand: Wait time out!!!

But I guess this gives you some hints on possible directions.

Cheers,

a+

ps: I will not be available tomorrow but can test whatever you
    come with the day after tomorrow.

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Ezequiel Garcia]

Arnaud,

On Tue, Nov 26, 2013 at 12:04:52AM +0100, Arnaud Ebalard wrote:
> 
> And \o/ i.e. here is what I get:
> 
> root@mood:~# dmesg
> ...
> pxa3xx-nand d00d0000.nand: This platform can't do DMA on this device
> NAND device: Manufacturer ID: 0xad, Chip ID: 0xf1 (Hynix H27U1G8F2BTR-BC)
> NAND device: 128MiB, SLC, page size: 2048, OOB size: 64
> Bad block table found at page 65472, version 0x01
> Bad block table found at page 65408, version 0x01
> 5 ofpart partitions found on MTD device pxa3xx_nand-0
> Creating 5 MTD partitions on "pxa3xx_nand-0":
> 0x000000000000-0x000000180000 : "u-boot"
> 0x000000180000-0x0000001a0000 : "u-boot-env"
> 0x000000200000-0x000000800000 : "uImage"
> 0x000000800000-0x000001800000 : "minirootfs"
> 0x000001800000-0x000008000000 : "jffs2"
> ...
> 
> root@mood:~# ls /dev/mtd
> mtd0       mtd1ro     mtd3       mtd4ro     mtdblock2  
> mtd0ro     mtd2       mtd3ro     mtdblock0  mtdblock3  
> mtd1       mtd2ro     mtd4       mtdblock1  mtdblock4  
> 
> root@mood:~# dd if=/dev/mtd2ro of=/tmp/foo
> 12288+0 records in
> 12288+0 records out
> 6291456 bytes (6.3 MB) copied, 1.98731 s, 3.2 MB/s
> 
> root@mood:~# file /tmp/foo 
> /tmp/foo: u-boot legacy uImage, Linux-3.12.0.rn102-00048-gbe408c, Linux/ARM, OS Kernel Image (Not compressed), 3740317 bytes, Tue Nov  5 22:24:01 2013, Load Address: 0x00008000, Entry Point: 0x00008000, Header CRC: 0xD84586E1, Data CRC: 0xC4357CED

Good!

> 
> But then /o\ i.e. write does not seem to work out of the box ;-)
> 
> root@mood:~# flash_erase /dev/mtd2 0 0 
> Erasing 128 Kibyte @ 5e0000 -- 100 % complete 
> root@mood:~# nand
> nanddump   nandtest   nandwrite  
> root@mood:~# nandwrite -p /dev/mtd2 /tmp/uImage
> Writing data to block 0 at offset 0x0
> [ 1456.154142] pxa3xx-nand d00d0000.nand: Wait time out!!!
> [ 1456.354143] pxa3xx-nand d00d0000.nand: Wait time out!!!
> [ 1456.554144] pxa3xx-nand d00d0000.nand: Wait time out!!!
> [ 1456.754141] pxa3xx-nand d00d0000.nand: Wait time out!!!
> [ 1456.954140] pxa3xx-nand d00d0000.nand: Wait time out!!!
> [ 1457.154140] pxa3xx-nand d00d0000.nand: Wait time out!!!
> [ 1457.354140] pxa3xx-nand d00d0000.nand: Wait time out!!!
> [ 1457.554140] pxa3xx-nand d00d0000.nand: Wait time out!!!
> [ 1457.754140] pxa3xx-nand d00d0000.nand: Wait time out!!!
> [ 1457.954197] pxa3xx-nand d00d0000.nand: Wait time out!!!
> [ 1458.154140] pxa3xx-nand d00d0000.nand: Wait time out!!!
> 

Yes, this behavior was expected, and it is the reason I wanted to try with
the 'pxa3xx' compatible.

> But I guess this gives you some hints on possible directions.
> 

Indeed! Thanks a lot for such detailed testings. Can you test this
branch instead (you will have to add the DTS for your board)?

  https://github.com/MISL-EBU-System-SW/mainline-public/tree/pxa3xx-armada-nand-new-ecc

It's basically v3.13-rc1, plus latest NAND patches (cherry-picked
from l2-mtd.git), plus a couple more patches to support 2048 pages.

Using this branch you should be able to have detect the device
and test read/write without any issues. You previous DT node should work:

nand@d0000 {
	status = "okay";
	num-cs = <1>;
	marvell,nand-keep-config;
	marvell,nand-enable-arbiter;
	nand-on-flash-bbt;
};


Thanks a lot!
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Arnaud Ebalard]

Hi Ezequiel,

Ezequiel Garcia <ezequiel.garcia@free-electrons.com> writes:

>> But I guess this gives you some hints on possible directions.
>
> Indeed! Thanks a lot for such detailed testings. Can you test this
> branch instead (you will have to add the DTS for your board)?
>
>   https://github.com/MISL-EBU-System-SW/mainline-public/tree/pxa3xx-armada-nand-new-ecc
>
> It's basically v3.13-rc1, plus latest NAND patches (cherry-picked
> from l2-mtd.git), plus a couple more patches to support 2048 pages.
>
> Using this branch you should be able to have detect the device
> and test read/write without any issues. You previous DT node should work:
>
> nand@d0000 {
> 	status = "okay";
> 	num-cs = <1>;
> 	marvell,nand-keep-config;
> 	marvell,nand-enable-arbiter;
> 	nand-on-flash-bbt;
> };

I first did simple raw read and write of a kernel image, which worked
fine:

root@mood:~# dmesg | grep -A 10 -B 10 nand
...
pxa3xx-nand d00d0000.nand: This platform can't do DMA on this device
NAND device: Manufacturer ID: 0xad, Chip ID: 0xf1 (Hynix H27U1G8F2BTR-BC)
NAND device: 128MiB, SLC, page size: 2048, OOB size: 64
Bad block table found at page 65472, version 0x01
Bad block table found at page 65408, version 0x01
5 ofpart partitions found on MTD device pxa3xx_nand-0
Creating 5 MTD partitions on "pxa3xx_nand-0":
0x000000000000-0x000000180000 : "u-boot"
0x000000180000-0x0000001a0000 : "u-boot-env"
0x000000200000-0x000000800000 : "uImage"
0x000000800000-0x000001800000 : "minirootfs"
0x000001800000-0x000008000000 : "jffs2"
...

root@mood:~# ls /dev/mtd*
/dev/mtd0    /dev/mtd1ro  /dev/mtd3    /dev/mtd4ro     /dev/mtdblock2
/dev/mtd0ro  /dev/mtd2    /dev/mtd3ro  /dev/mtdblock0  /dev/mtdblock3
/dev/mtd1    /dev/mtd2ro  /dev/mtd4    /dev/mtdblock1  /dev/mtdblock4

root@mood:~# dd if=/dev/mtd2ro of=/tmp/foo
12288+0 records in
12288+0 records out
6291456 bytes (6.3 MB) copied, 1.98142 s, 3.2 MB/s

root@mood:~# file /tmp/foo 
/tmp/foo: u-boot legacy uImage, Linux-3.12.0.rn102-10710-gb4789b, Linux/ARM, OS Kernel Image (Not compressed), 4268974 bytes, Mon Nov 25 23:34:01 2013, Load Address: 0x00008000, Entry Point: 0x00008000, Header CRC: 0xB390DB2D, Data CRC: 0x985233AE

root@mood:~# flash_erase /dev/mtd2 0 0 
Erasing 128 Kibyte @ 5e0000 -- 100 % complete 

root@mood:~# nandwrite -p /dev/mtd2 /tmp/test-rn102 
Writing data to block 0 at offset 0x0
Writing data to block 1 at offset 0x20000
Writing data to block 2 at offset 0x40000
Writing data to block 3 at offset 0x60000
Writing data to block 4 at offset 0x80000
Writing data to block 5 at offset 0xa0000
Writing data to block 6 at offset 0xc0000
Writing data to block 7 at offset 0xe0000
Writing data to block 8 at offset 0x100000
Writing data to block 9 at offset 0x120000
Writing data to block 10 at offset 0x140000
Writing data to block 11 at offset 0x160000
Writing data to block 12 at offset 0x180000
Writing data to block 13 at offset 0x1a0000
Writing data to block 14 at offset 0x1c0000
Writing data to block 15 at offset 0x1e0000
Writing data to block 16 at offset 0x200000
Writing data to block 17 at offset 0x220000
Writing data to block 18 at offset 0x240000
Writing data to block 19 at offset 0x260000
Writing data to block 20 at offset 0x280000
Writing data to block 21 at offset 0x2a0000
Writing data to block 22 at offset 0x2c0000
Writing data to block 23 at offset 0x2e0000
Writing data to block 24 at offset 0x300000
Writing data to block 25 at offset 0x320000
Writing data to block 26 at offset 0x340000
Writing data to block 27 at offset 0x360000
Writing data to block 28 at offset 0x380000
Writing data to block 29 at offset 0x3a0000
Writing data to block 30 at offset 0x3c0000
Writing data to block 31 at offset 0x3e0000
Writing data to block 32 at offset 0x400000

root@mood:~# dd if=/dev/mtd2 of=/tmp/foo bs=`wc -c /tmp/test-rn102 | cut -d' ' -f1` count=1
1+0 records in
1+0 records out
4274950 bytes (4.3 MB) copied, 0.345413 s, 12.4 MB/s

root@mood:~# sha256sum /tmp/foo /tmp/test-rn102 
4a923e8ccf0358d9cc70fd82c339eff344986faf3ba37e396b5e635ab8c8d928  /tmp/foo
4a923e8ccf0358d9cc70fd82c339eff344986faf3ba37e396b5e635ab8c8d928  /tmp/test-rn102





Then, I tried and mount the jffs2 partition. This failed with some
errors:

Erasing failed write from 0x6680000 to 0x669ffff
Writing data to block 821 at offset 0x66a0000
[ 1938.054752] pxa3xx-nand d00d0000.nand: Ready time out!!!
libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 821, offset 0)
        error 5 (Input/output error)
Erasing failed write from 0x66a0000 to 0x66bffff
Writing data to block 822 at offset 0x66c0000
[ 1938.254753] pxa3xx-nand d00d0000.nand: Ready time out!!!
libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 822, offset 0)
        error 5 (Input/output error)
Erasing failed write from 0x66c0000 to 0x66dffff
Writing data to block 823 at offset 0x66e0000
[ 1938.454752] pxa3xx-nand d00d0000.nand: Ready time out!!!
libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 823, offset 0)
        error 5 (Input/output error)
Erasing failed write from 0x66e0000 to 0x66fffff
Writing data to block 824 at offset 0x6700000
Bad block at 6700000, 1 block(s) from 6700000 will be skipped
Writing data to block 825 at offset 0x6720000
Bad block at 6720000, 1 block(s) from 6720000 will be skipped
Writing data to block 826 at offset 0x6740000
Bad block at 6740000, 1 block(s) from 6740000 will be skipped
Writing data to block 827 at offset 0x6760000
Bad block at 6760000, 1 block(s) from 6760000 will be skipped
Writing data to block 828 at offset 0x6780000
Bad block at 6780000, 1 block(s) from 6780000 will be skipped
Writing data to block 829 at offset 0x67a0000
Bad block at 67a0000, 1 block(s) from 67a0000 will be skipped
Writing data to block 830 at offset 0x67c0000
Bad block at 67c0000, 1 block(s) from 67c0000 will be skipped
Writing data to block 831 at offset 0x67e0000
Bad block at 67e0000, 1 block(s) from 67e0000 will be skipped
Writing data to block 832 at offset 0x6800000
libmtd: error!: bad eraseblock number 832, mtd4 has 832 eraseblocks
nandwrite: error!: /dev/mtd4: MTD get bad block failed
           error 22 (Invalid argument)
nandwrite: error!: Data was only partially written due to error
           error 22 (Invalid argument)



At that point, I though the jffs2 partition was somewhat broken
on my RN102 from previous tests so I decided and try to flash on
it a recovery jffs2 dump I had:


root@mood:~# nandwrite -p /dev/mtd4 /tmp/mtd4ro 

jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found at 0x034e0028: 0x55da instead
jffs2: Further such events for this erase block will not be printed
jffs2: mtd->read(0x100 bytes from 0x3500000) returned ECC error
jffs2: jffs2_scan_inode_node(): CRC failed on node at 0x03620000: Read 0xc04c8a8f, calculated 0x8fa1cb40
jffs2: jffs2_scan_inode_node(): CRC failed on node at 0x03840000: Read 0xa5af0f35, calculated 0x6059ed2e
jffs2: notice: (4553) read_dnode: wrong data CRC in data node at 0x03820000: read 0x56e035c2, calculated 0x6895725b.
jffs2: notice: (4553) read_dnode: wrong data CRC in data node at 0x037e0000: read 0x3f7674d6, calculated 0xf5df9a6e.
jffs2: notice: (4553) read_dnode: wrong data CRC in data node at 0x03780000: read 0xaefa5c80, calculated 0xea10dc2a.
jffs2: notice: (4553) read_dnode: wrong data CRC in data node at 0x03740000: read 0x47c3b89b, calculated 0xafaeb853.
jffs2: notice: (4553) read_dnode: wrong data CRC in data node at 0x03700000: read 0x4583c6cb, calculated 0x13c702d0.
jffs2: notice: (4553) read_dnode: wrong data CRC in data node at 0x036c0000: read 0x38df270b, calculated 0xc64fadf.
jffs2: notice: (4553) read_dnode: wrong data CRC in data node at 0x03680000: read 0x6d69a120, calculated 0xabbd2598.
jffs2: notice: (4553) read_dnode: wrong data CRC in data node at 0x03640000: read 0xaa02c940, calculated 0x28f6188b.
jffs2: notice: (4553) read_dnode: wrong data CRC in data node at 0x03520000: read 0x7eab8c3b, calculated 0xd5457f42.


And then trying to mount the fs (I guess I should have mounted it RO,
but I don't even know if it would have prevent the bad block table to
be filled)

jffs2: marking eraseblock at 046c0000 as bad
Bad block table written to 0x000007fe0000, version 0xA2
Bad block table written to 0x000007fc0000, version 0xA2
jffs2: Read of newly-erased block at 0x046a0000 failed: -117. Putting on bad_list
jffs2: Read of newly-erased block at 0x046a0000 failed: -117. Putting on bad_list
jffs2: marking eraseblock at 046a0000 as bad
Bad block table written to 0x000007fe0000, version 0xA3
Bad block table written to 0x000007fc0000, version 0xA3
jffs2: Read of newly-erased block at 0x04680000 failed: -117. Putting on bad_list
jffs2: Read of newly-erased block at 0x04680000 failed: -117. Putting on bad_list
jffs2: marking eraseblock at 04680000 as bad
Bad block table written to 0x000007fe0000, version 0xA4


And then during boot, here is what I now get when u-boot loads :-)

NAND:  (ID 0xf1ad)	128 MiB
Bad block table found at page 65472, version 0xA4
Bad block table found at page 65408, version 0xA4
nand_read_bbt: Bad block at 0x000005e80000
nand_read_bbt: Bad block at 0x000005ea0000
nand_read_bbt: Bad block at 0x000005ec0000
nand_read_bbt: Bad block at 0x000005ee0000
nand_read_bbt: Bad block at 0x000005f00000
nand_read_bbt: Bad block at 0x000005f20000
nand_read_bbt: Bad block at 0x000005f40000
nand_read_bbt: Bad block at 0x000005f60000
nand_read_bbt: Bad block at 0x000005f80000
nand_read_bbt: Bad block at 0x000005fa0000
nand_read_bbt: Bad block at 0x000005fc0000
nand_read_bbt: Bad block at 0x000005fe0000
nand_read_bbt: Bad block at 0x000006000000
nand_read_bbt: Bad block at 0x000006020000
nand_read_bbt: Bad block at 0x000006040000
nand_read_bbt: Bad block at 0x000006060000
nand_read_bbt: Bad block at 0x000006080000
nand_read_bbt: Bad block at 0x0000060a0000
nand_read_bbt: Bad block at 0x0000060c0000
nand_read_bbt: Bad block at 0x0000060e0000
nand_read_bbt: Bad block at 0x000006100000
nand_read_bbt: Bad block at 0x000006120000
nand_read_bbt: Bad block at 0x000006140000
nand_read_bbt: Bad block at 0x000006160000
nand_read_bbt: Bad block at 0x000006180000
nand_read_bbt: Bad block at 0x0000061a0000
nand_read_bbt: Bad block at 0x0000061c0000
nand_read_bbt: Bad block at 0x0000061e0000
nand_read_bbt: Bad block at 0x000006200000
nand_read_bbt: Bad block at 0x000006220000
nand_read_bbt: Bad block at 0x000006240000
nand_read_bbt: Bad block at 0x000006260000
nand_read_bbt: Bad block at 0x000006280000
nand_read_bbt: Bad block at 0x0000062a0000
nand_read_bbt: Bad block at 0x0000062c0000
nand_read_bbt: Bad block at 0x0000062e0000
nand_read_bbt: Bad block at 0x000006300000
nand_read_bbt: Bad block at 0x000006320000
nand_read_bbt: Bad block at 0x000006340000
nand_read_bbt: Bad block at 0x000006360000
nand_read_bbt: Bad block at 0x000006380000
nand_read_bbt: Bad block at 0x0000063a0000
nand_read_bbt: Bad block at 0x0000063c0000
nand_read_bbt: Bad block at 0x0000063e0000
nand_read_bbt: Bad block at 0x000006400000
nand_read_bbt: Bad block at 0x000006420000
nand_read_bbt: Bad block at 0x000006440000
nand_read_bbt: Bad block at 0x000006460000
nand_read_bbt: Bad block at 0x000006480000
nand_read_bbt: Bad block at 0x0000064a0000
nand_read_bbt: Bad block at 0x0000064c0000
nand_read_bbt: Bad block at 0x0000064e0000
nand_read_bbt: Bad block at 0x000006500000
nand_read_bbt: Bad block at 0x000006520000
nand_read_bbt: Bad block at 0x000006540000
nand_read_bbt: Bad block at 0x000006560000
nand_read_bbt: Bad block at 0x000006580000
nand_read_bbt: Bad block at 0x0000065a0000
nand_read_bbt: Bad block at 0x0000065c0000
nand_read_bbt: Bad block at 0x0000065e0000
nand_read_bbt: Bad block at 0x000006600000
nand_read_bbt: Bad block at 0x000006620000
nand_read_bbt: Bad block at 0x000006640000
nand_read_bbt: Bad block at 0x000006660000
nand_read_bbt: Bad block at 0x000006680000
nand_read_bbt: Bad block at 0x0000066a0000
nand_read_bbt: Bad block at 0x0000066c0000
nand_read_bbt: Bad block at 0x0000066e0000
nand_read_bbt: Bad block at 0x000006700000
nand_read_bbt: Bad block at 0x000006720000
nand_read_bbt: Bad block at 0x000006740000
nand_read_bbt: Bad block at 0x000006760000
nand_read_bbt: Bad block at 0x000006780000
nand_read_bbt: Bad block at 0x0000067a0000
nand_read_bbt: Bad block at 0x0000067c0000
nand_read_bbt: Bad block at 0x0000067e0000
nand_read_bbt: Bad block at 0x000006800000
nand_read_bbt: Bad block at 0x000006820000
nand_read_bbt: Bad block at 0x000006840000
nand_read_bbt: Bad block at 0x000006860000
nand_read_bbt: Bad block at 0x000006880000
nand_read_bbt: Bad block at 0x0000068a0000
nand_read_bbt: Bad block at 0x0000068c0000
nand_read_bbt: Bad block at 0x0000068e0000
nand_read_bbt: Bad block at 0x000006900000
nand_read_bbt: Bad block at 0x000006920000
nand_read_bbt: Bad block at 0x000006940000
nand_read_bbt: Bad block at 0x000006960000
nand_read_bbt: Bad block at 0x000006980000
nand_read_bbt: Bad block at 0x0000069a0000
nand_read_bbt: Bad block at 0x0000069c0000
nand_read_bbt: Bad block at 0x0000069e0000
nand_read_bbt: Bad block at 0x000006a00000
nand_read_bbt: Bad block at 0x000006a20000
nand_read_bbt: Bad block at 0x000006a40000
nand_read_bbt: Bad block at 0x000006a60000
nand_read_bbt: Bad block at 0x000006a80000
nand_read_bbt: Bad block at 0x000006aa0000
nand_read_bbt: Bad block at 0x000006ac0000
nand_read_bbt: Bad block at 0x000006ae0000
nand_read_bbt: Bad block at 0x000006b00000
nand_read_bbt: Bad block at 0x000006b20000
nand_read_bbt: Bad block at 0x000006b40000
nand_read_bbt: Bad block at 0x000006b60000
nand_read_bbt: Bad block at 0x000006b80000
nand_read_bbt: Bad block at 0x000006ba0000
nand_read_bbt: Bad block at 0x000006bc0000
nand_read_bbt: Bad block at 0x000006be0000
nand_read_bbt: Bad block at 0x000006c00000
nand_read_bbt: Bad block at 0x000006c20000
nand_read_bbt: Bad block at 0x000006c40000
nand_read_bbt: Bad block at 0x000006c60000
nand_read_bbt: Bad block at 0x000006c80000
nand_read_bbt: Bad block at 0x000006ca0000
nand_read_bbt: Bad block at 0x000006cc0000
nand_read_bbt: Bad block at 0x000006ce0000
nand_read_bbt: Bad block at 0x000006d00000
nand_read_bbt: Bad block at 0x000006d20000
nand_read_bbt: Bad block at 0x000006d40000
nand_read_bbt: Bad block at 0x000006d60000
nand_read_bbt: Bad block at 0x000006d80000
nand_read_bbt: Bad block at 0x000006da0000
nand_read_bbt: Bad block at 0x000006dc0000
nand_read_bbt: Bad block at 0x000006de0000
nand_read_bbt: Bad block at 0x000006e00000
nand_read_bbt: Bad block at 0x000006e20000
nand_read_bbt: Bad block at 0x000006e40000
nand_read_bbt: Bad block at 0x000006e60000
nand_read_bbt: Bad block at 0x000006e80000
nand_read_bbt: Bad block at 0x000006ea0000
nand_read_bbt: Bad block at 0x000006ec0000
nand_read_bbt: Bad block at 0x000006ee0000
nand_read_bbt: Bad block at 0x000006f00000
nand_read_bbt: Bad block at 0x000006f20000
nand_read_bbt: Bad block at 0x000006f40000
nand_read_bbt: Bad block at 0x000006f60000
nand_read_bbt: Bad block at 0x000006f80000
nand_read_bbt: Bad block at 0x000006fa0000
nand_read_bbt: Bad block at 0x000006fc0000
nand_read_bbt: Bad block at 0x000006fe0000
nand_read_bbt: Bad block at 0x000007000000
nand_read_bbt: Bad block at 0x000007020000
nand_read_bbt: Bad block at 0x000007040000
nand_read_bbt: Bad block at 0x000007060000
nand_read_bbt: Bad block at 0x000007080000
nand_read_bbt: Bad block at 0x0000070a0000
nand_read_bbt: Bad block at 0x0000070c0000
nand_read_bbt: Bad block at 0x0000070e0000
nand_read_bbt: Bad block at 0x000007100000
nand_read_bbt: Bad block at 0x000007120000
nand_read_bbt: Bad block at 0x000007140000
nand_read_bbt: Bad block at 0x000007160000
nand_read_bbt: Bad block at 0x000007180000
nand_read_bbt: Bad block at 0x0000071a0000
nand_read_bbt: Bad block at 0x0000071c0000
nand_read_bbt: Bad block at 0x0000071e0000
nand_read_bbt: Bad block at 0x000007200000
nand_read_bbt: Bad block at 0x000007220000
nand_read_bbt: Bad block at 0x000007240000
nand_read_bbt: Bad block at 0x000007260000
nand_read_bbt: Bad block at 0x000007280000
nand_read_bbt: Bad block at 0x0000072a0000
nand_read_bbt: Bad block at 0x0000072c0000
FPU not initialized
USB 0: Host Mode
USB 1: Host Mode
Shutting down unused interfaces:
       GBE0
       SDIO
       AUDIO
       TDM
Modules/Interfaces Detected:
       RGMII1 Phy
       PEX0 (Lane 0)
       PEX1 (Lane 1)
       SATA0 (Lane 2)
       SATA1 (Lane 3)
Power On!

MMC:   MRVL_MMC: 0
Net:   egiga1 [PRIME]
Hit any key to stop autoboot:  0
Marvell>> 


I hope the error messages produced during the session will somehow help
find were the issue comes from. I somewhat suspects some chip delay or
CRC issue (those errors appear above).

Well, I guess the bad block the driver put in the bbt are not that bad
in practice, i.e. they were mistakenly marked that way. I also
guess^Whope there is a some low level command I could use to simply
clear the bbt (the NAND had no bad block prior to the test). Any help
would be appreciated on that point.

Anyway, I have somehow decided to delay the test on my RN2120 (dual-core)
for now ;-)

Cheers,

a+

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Arnaud Ebalard]

Hi,

arno@natisbad.org (Arnaud Ebalard) writes:

> Well, I guess the bad block the driver put in the bbt are not that bad
> in practice, i.e. they were mistakenly marked that way. I also
> guess^Whope there is a some low level command I could use to simply
> clear the bbt (the NAND had no bad block prior to the test). Any help
> would be appreciated on that point.

Replying to myself w/ the solution: just in case it happens to someone
else, it's as simple as using flash_erase w/ -N option on a kernel
modified (in drivers/mtd/nand/nand_base.c) to allow write to a bad
block w/ the following patch:

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index bd39f7b..e70ef60 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2591,6 +2591,7 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr,
        instr->state = MTD_ERASING;
 
        while (len) {
+#if 0
                /* Check if we have a bad block, we do not erase bad blocks! */
                if (nand_block_checkbad(mtd, ((loff_t) page) <<
                                        chip->page_shift, 0, allowbbt)) {
@@ -2599,6 +2600,7 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr,
                        instr->state = MTD_ERASE_FAILED;
                        goto erase_exit;
                }
+#endif
 
                /*
                 * Invalidate the page cache, if we erase the block which


After a reboot on that modified kernel, a simple call to flash_erase
using -N option will erase the badblocks:

 # flash_erase -N /dev/mtd4 0 0 

Then, at next reboot an empty bad block table is recreated.

NAND:  (ID 0xf1ad)      128 MiB
Bad block table not found for chip 0
Bad block table not found for chip 0
Bad block table written to 0x000007fe0000, version 0x01
Bad block table written to 0x000007fc0000, version 0x01
FPU not initialized
USB 0: Host Mode
USB 1: Host Mode

Ezequiel, I am back in business to test a v2 ;-)

Cheers,

a+

ps: primary source: http://lists.infradead.org/pipermail/linux-mtd/2012-June/041969.html

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Ricard Wanderlof]

On Wed, 27 Nov 2013, Arnaud Ebalard wrote:

>> Well, I guess the bad block the driver put in the bbt are not that bad
>> in practice, i.e. they were mistakenly marked that way. I also
>> guess^Whope there is a some low level command I could use to simply
>> clear the bbt (the NAND had no bad block prior to the test). Any help
>> would be appreciated on that point.
>
> Replying to myself w/ the solution: just in case it happens to someone
> else, it's as simple as using flash_erase w/ -N option on a kernel
> modified (in drivers/mtd/nand/nand_base.c) to allow write to a bad
> block w/ the following patch:
>
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index bd39f7b..e70ef60 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -2591,6 +2591,7 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr,
>        instr->state = MTD_ERASING;
>
>        while (len) {
> +#if 0
>                /* Check if we have a bad block, we do not erase bad blocks! */
>                if (nand_block_checkbad(mtd, ((loff_t) page) <<
>                                        chip->page_shift, 0, allowbbt)) {
> @@ -2599,6 +2600,7 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr,
>                        instr->state = MTD_ERASE_FAILED;
>                        goto erase_exit;
>                }
> +#endif
>
>                /*
>                 * Invalidate the page cache, if we erase the block which
>
>
> After a reboot on that modified kernel, a simple call to flash_erase
> using -N option will erase the badblocks:
>
> # flash_erase -N /dev/mtd4 0 0
> ...

I agree, I usually do something along these lines too (especially the 
kernel bit) when I run into a flash with mistakingly marked bad blocks.

I've always argued against having it switchable in any way in the kernel, 
I've figured it needs to be hard to do (i.e. require access to kernel code 
and target build system) so that average users don't start erasing bad 
blocks at the sign of bad block trouble by just flicking a switch in /sys 
or whatever.

/Ricard
-- 
Ricard Wolf Wanderlöf                           ricardw(at)axis.com
Axis Communications AB, Lund, Sweden            www.axis.com
Phone +46 46 272 2016                           Fax +46 46 13 61 30

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Ezequiel Garcia]

Arnaud,

On Wed, Nov 27, 2013 at 09:52:52PM +0100, Arnaud Ebalard wrote:
> 
> arno@natisbad.org (Arnaud Ebalard) writes:
> 
> > Well, I guess the bad block the driver put in the bbt are not that bad
> > in practice, i.e. they were mistakenly marked that way. I also
> > guess^Whope there is a some low level command I could use to simply
> > clear the bbt (the NAND had no bad block prior to the test). Any help
> > would be appreciated on that point.
> 
> Replying to myself w/ the solution: just in case it happens to someone
> else, it's as simple as using flash_erase w/ -N option on a kernel
> modified (in drivers/mtd/nand/nand_base.c) to allow write to a bad
> block w/ the following patch:
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index bd39f7b..e70ef60 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -2591,6 +2591,7 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr,
>         instr->state = MTD_ERASING;
>  
>         while (len) {
> +#if 0
>                 /* Check if we have a bad block, we do not erase bad blocks! */
>                 if (nand_block_checkbad(mtd, ((loff_t) page) <<
>                                         chip->page_shift, 0, allowbbt)) {
> @@ -2599,6 +2600,7 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr,
>                         instr->state = MTD_ERASE_FAILED;
>                         goto erase_exit;
>                 }
> +#endif
>  
>                 /*
>                  * Invalidate the page cache, if we erase the block which
> 
> 
> After a reboot on that modified kernel, a simple call to flash_erase
> using -N option will erase the badblocks:
> 
>  # flash_erase -N /dev/mtd4 0 0 
> 
> Then, at next reboot an empty bad block table is recreated.
> 
> NAND:  (ID 0xf1ad)      128 MiB
> Bad block table not found for chip 0
> Bad block table not found for chip 0
> Bad block table written to 0x000007fe0000, version 0x01
> Bad block table written to 0x000007fc0000, version 0x01
> FPU not initialized
> USB 0: Host Mode
> USB 1: Host Mode
> 

Yup, I'm using the same hack to clean the (not really) bad blocks from
the table. FWIW, the MTD people has been playing with some idea to
kernel support to unmark bad blocks from the bad block table, but
nothing was merged and (AFAIK) nobody is working on it. So patches are
welcome!

> Ezequiel, I am back in business to test a v2 ;-)

Well, I'm not sure yet what's going on. Do you have any spare NAND partition
to run some destructive testings?

In that case, please run:

$ nandtest /dev/mtd{X}

Be careful as you _will_ destroy all your data on that partition!

Thanks for such brave testings!
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Arnaud Ebalard]

Hi,

Ezequiel Garcia <ezequiel.garcia@free-electrons.com> writes:

>> Ezequiel, I am back in business to test a v2 ;-)
>
> Well, I'm not sure yet what's going on. Do you have any spare NAND partition
> to run some destructive testings?
>
> In that case, please run:
>
> $ nandtest /dev/mtd{X}

Here is the nandtest run:

  nandtest /dev/mtd4
  ECC corrections: 0
  ECC failures   : 0
  Bad blocks     : 8
  BBT blocks     : 0
  Bad block at 0x06700000
  Bad block at 0x06720000
  Bad block at 0x06740000
  Bad block at 0x06760000
  Bad block at 0x06780000
  Bad block at 0x067a0000
  Bad block at 0x067c0000
  Bad block at 0x067e0000
  
  Finished pass 1 successfully


Then, doing a simple read and copying back the data:

  root@mood:~# dd if=/dev/mtd4 of=/tmp/toto 
  212992+0 records in
  212992+0 records out
  109051904 bytes (109 MB) copied, 10.8671 s, 10.0 MB/s
  
  root@mood:~# flash_erase /dev/mtd4 0 0
  flash_erase: Skipping bad block at 06700000
  flash_erase: Skipping bad block at 06720000
  flash_erase: Skipping bad block at 06740000
  flash_erase: Skipping bad block at 06760000
  flash_erase: Skipping bad block at 06780000
  flash_erase: Skipping bad block at 067a0000
  flash_erase: Skipping bad block at 067c0000
  flash_erase: Skipping bad block at 067e0000
  
  root@mood:~# nandwrite -p /dev/mtd4 /tmp/toto
  Writing data to block 0 at offset 0x0
  Writing data to block 1 at offset 0x20000
  Writing data to block 2 at offset 0x40000
  Writing data to block 3 at offset 0x60000
  Writing data to block 4 at offset 0x80000
  Writing data to block 5 at offset 0xa0000
  Writing data to block 6 at offset 0xc0000
  Writing data to block 7 at offset 0xe0000
  ...
  Writing data to block 795 at offset 0x6360000
  Writing data to block 796 at offset 0x6380000
  Writing data to block 797 at offset 0x63a0000
  Writing data to block 798 at offset 0x63c0000
  Writing data to block 799 at offset 0x63e0000
  Writing data to block 800 at offset 0x6400000
  [ 1509.210395] pxa3xx-nand d00d0000.nand: Ready time out!!!
  libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 800, offset 0)
          error 5 (Input/output error)
  Erasing failed write from 0x6400000 to 0x641ffff
  Writing data to block 801 at offset 0x6420000
  [ 1509.410388] pxa3xx-nand d00d0000.nand: Ready time out!!!
  libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 801, offset 0)
          error 5 (Input/output error)
  Erasing failed write from 0x6420000 to 0x643ffff
  Writing data to block 802 at offset 0x6440000
  [ 1509.610386] pxa3xx-nand d00d0000.nand: Ready time out!!!
  libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 802, offset 0)
          error 5 (Input/output error)
  Erasing failed write from 0x6440000 to 0x645ffff
  Writing data to block 803 at offset 0x6460000
  [ 1509.810386] pxa3xx-nand d00d0000.nand: Ready time out!!!
  libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 803, offset 0)
          error 5 (Input/output error)
  Erasing failed write from 0x6460000 to 0x647ffff
  Writing data to block 804 at offset 0x6480000
  [ 1510.010387] pxa3xx-nand d00d0000.nand: Ready time out!!!
  libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 804, offset 0)
          error 5 (Input/output error)
  Erasing failed write from 0x6480000 to 0x649ffff
  Writing data to block 805 at offset 0x64a0000
  [ 1510.210386] pxa3xx-nand d00d0000.nand: Ready time out!!!
  libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 805, offset 0)
          error 5 (Input/output error)
  Erasing failed write from 0x64a0000 to 0x64bffff
  Writing data to block 806 at offset 0x64c0000
  [ 1510.410386] pxa3xx-nand d00d0000.nand: Ready time out!!!
  libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 806, offset 0)
          error 5 (Input/output error)
  Erasing failed write from 0x64c0000 to 0x64dffff
  Writing data to block 807 at offset 0x64e0000
  [ 1510.610387] pxa3xx-nand d00d0000.nand: Ready time out!!!
  libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 807, offset 0)
          error 5 (Input/output error)
  Erasing failed write from 0x64e0000 to 0x64fffff
  Writing data to block 808 at offset 0x6500000
  [ 1510.810386] pxa3xx-nand d00d0000.nand: Ready time out!!!
  libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 808, offset 0)
          error 5 (Input/output error)
  Erasing failed write from 0x6500000 to 0x651ffff
  Writing data to block 809 at offset 0x6520000
  [ 1511.010385] pxa3xx-nand d00d0000.nand: Ready time out!!!
  libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 809, offset 0)
          error 5 (Input/output error)
  Erasing failed write from 0x6520000 to 0x653ffff
  Writing data to block 810 at offset 0x6540000
  [ 1511.210387] pxa3xx-nand d00d0000.nand: Ready time out!!!
  libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 810, offset 0)
          error 5 (Input/output error)
  Erasing failed write from 0x6540000 to 0x655ffff
  Writing data to block 811 at offset 0x6560000
  [ 1511.410387] pxa3xx-nand d00d0000.nand: Ready time out!!!
  libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 811, offset 0)
          error 5 (Input/output error)
  Erasing failed write from 0x6560000 to 0x657ffff
  Writing data to block 812 at offset 0x6580000
  [ 1511.610388] pxa3xx-nand d00d0000.nand: Ready time out!!!
  libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 812, offset 0)
          error 5 (Input/output error)
  Erasing failed write from 0x6580000 to 0x659ffff
  Writing data to block 813 at offset 0x65a0000
  [ 1511.810444] pxa3xx-nand d00d0000.nand: Ready time out!!!
  libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 813, offset 0)
          error 5 (Input/output error)
  Erasing failed write from 0x65a0000 to 0x65bffff
  Writing data to block 814 at offset 0x65c0000
  [ 1512.010387] pxa3xx-nand d00d0000.nand: Ready time out!!!
  libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 814, offset 0)
          error 5 (Input/output error)
  Erasing failed write from 0x65c0000 to 0x65dffff
  Writing data to block 815 at offset 0x65e0000
  [ 1512.210386] pxa3xx-nand d00d0000.nand: Ready time out!!!
  libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 815, offset 0)
          error 5 (Input/output error)
  Erasing failed write from 0x65e0000 to 0x65fffff
  Writing data to block 816 at offset 0x6600000
  [ 1512.410386] pxa3xx-nand d00d0000.nand: Ready time out!!!
  libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 816, offset 0)
          error 5 (Input/output error)
  Erasing failed write from 0x6600000 to 0x661ffff
  Writing data to block 817 at offset 0x6620000
  [ 1512.610386] pxa3xx-nand d00d0000.nand: Ready time out!!!
  libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 817, offset 0)
          error 5 (Input/output error)
  Erasing failed write from 0x6620000 to 0x663ffff
  Writing data to block 818 at offset 0x6640000
  [ 1512.810386] pxa3xx-nand d00d0000.nand: Ready time out!!!
  libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 818, offset 0)
          error 5 (Input/output error)
  Erasing failed write from 0x6640000 to 0x665ffff
  Writing data to block 819 at offset 0x6660000
  [ 1513.010386] pxa3xx-nand d00d0000.nand: Ready time out!!!
  libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 819, offset 0)
          error 5 (Input/output error)
  Erasing failed write from 0x6660000 to 0x667ffff
  Writing data to block 820 at offset 0x6680000
  [ 1513.210386] pxa3xx-nand d00d0000.nand: Ready time out!!!
  libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 820, offset 0)
          error 5 (Input/output error)
  Erasing failed write from 0x6680000 to 0x669ffff
  Writing data to block 821 at offset 0x66a0000
  [ 1513.410386] pxa3xx-nand d00d0000.nand: Ready time out!!!
  libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 821, offset 0)
          error 5 (Input/output error)
  Erasing failed write from 0x66a0000 to 0x66bffff
  Writing data to block 822 at offset 0x66c0000
  [ 1513.610388] pxa3xx-nand d00d0000.nand: Ready time out!!!
  libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 822, offset 0)
          error 5 (Input/output error)
  Erasing failed write from 0x66c0000 to 0x66dffff
  Writing data to block 823 at offset 0x66e0000
  [ 1513.810387] pxa3xx-nand d00d0000.nand: Ready time out!!!
  libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 823, offset 0)
          error 5 (Input/output error)
  Erasing failed write from 0x66e0000 to 0x66fffff
  Writing data to block 824 at offset 0x6700000
  Bad block at 6700000, 1 block(s) from 6700000 will be skipped
  Writing data to block 825 at offset 0x6720000
  Bad block at 6720000, 1 block(s) from 6720000 will be skipped
  Writing data to block 826 at offset 0x6740000
  Bad block at 6740000, 1 block(s) from 6740000 will be skipped
  Writing data to block 827 at offset 0x6760000
  Bad block at 6760000, 1 block(s) from 6760000 will be skipped
  Writing data to block 828 at offset 0x6780000
  Bad block at 6780000, 1 block(s) from 6780000 will be skipped
  Writing data to block 829 at offset 0x67a0000
  Bad block at 67a0000, 1 block(s) from 67a0000 will be skipped
  Writing data to block 830 at offset 0x67c0000
  Bad block at 67c0000, 1 block(s) from 67c0000 will be skipped
  Writing data to block 831 at offset 0x67e0000
  Bad block at 67e0000, 1 block(s) from 67e0000 will be skipped
  Writing data to block 832 at offset 0x6800000
  libmtd: error!: bad eraseblock number 832, mtd4 has 832 eraseblocks
  nandwrite: error!: /dev/mtd4: MTD get bad block failed
             error 22 (Invalid argument)
  nandwrite: error!: Data was only partially written due to error
             error 22 (Invalid argument)
  
This is the kind of errors I got last time but I think am starting to
understand the root cause now. Tell me if I get it right: what is
understood as bad blocks above (and in nandtest) is in fact the two bad
block tables reported during boot:

 NAND device: Manufacturer ID: 0xad, Chip ID: 0xf1 (Hynix H27U1G8F2BTR-BC)
 NAND device: 128MiB, SLC, page size: 2048, OOB size: 64
 Bad block table found at page 65472, version 0x01
 Bad block table found at page 65408, version 0x01

*If* that is the case, 

 1) I think your driver works as expected

 2) I also think I should reduce partition definition from 

         partition@1800000 {
                 label = "jffs2";
                 reg = <0x1800000 0x6800000>;
         };
   to
         partition@1800000 { /* Followed by 256KB of BBT */
                 label = "jffs2";
                 reg = <0x1800000 0x67c0000>;
         };

In that case, I wonder if there some automatic way to have the
*effective* size of that last partition computed from start position,
chip size and bad block tables positions and size? Or hardcoding the
value is ok?

I took a look at devicetree bindings for MTD partitions (partitions.txt)
but found nothing. Looking at existing .dts files, I wonder if 0 as a
specific meaning:

$ grep -A3 partition@ *.dts | grep -A 1 root 
...
--
spear1310-evb.dts-                              label = "rootfs";
spear1310-evb.dts-                              reg = <0xE40000 0x0>;
--
spear1310-evb.dts-                                      label = "rootfs";
spear1310-evb.dts-                                      reg = <0x390000 0x0>;
--
spear1340-evb.dts-                              label = "rootfs";
spear1340-evb.dts-                              reg = <0x1200000 0x0>;
--
spear1340-evb.dts-                                      label = "rootfs";
spear1340-evb.dts-                                      reg = <0x390000 0x0>;
--
spear300-evb.dts-                                       label = "rootfs";
spear300-evb.dts-                                       reg = <0x390000 0x0>;
--
spear310-evb.dts-                                       label = "rootfs";
spear310-evb.dts-                                       reg = <0x390000 0x0>;
--
spear320-evb.dts-                                       label = "rootfs";
spear320-evb.dts-                                       reg = <0x390000 0x0>;
--
spear320-hmi.dts-                               label = "rootfs";
spear320-hmi.dts-                               reg = <0xE40000 0x0>;
--
spear320-hmi.dts-                                       label = "rootfs";
spear320-hmi.dts-                                       reg = <0x390000 0x0>;
--
spear600-evb.dts-                                       label = "rootfs";
spear600-evb.dts-                                       reg = <0x390000 0x0>;


As a side note, if I am somewhat right, I wonder how the defintion of
the partitions in for instance armada-370-mirabox.dts (introduced by
016b74660d14) would allow to effectively use the end of that partition:

         nand@d0000 {
                 status = "okay";
                 num-cs = <1>;
                 marvell,nand-keep-config;
                 marvell,nand-enable-arbiter;
                 nand-on-flash-bbt;
 
                 partition@0 {
                         label = "U-Boot";
                         reg = <0 0x400000>;
                 };
                 partition@400000 {
                         label = "Linux";
                         reg = <0x400000 0x400000>;
                 };
                 partition@800000 {
                         label = "Filesystem";
                         reg = <0x800000 0x3f800000>;
                 };
        };

Comments welcome.

Cheers,

a+

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Ezequiel Garcia]

Hi Arnaud,

First of all: thanks for such great testings!

On Sat, Nov 30, 2013 at 12:25:14AM +0100, Arnaud Ebalard wrote:
> Ezequiel Garcia <ezequiel.garcia@free-electrons.com> writes:
> 
> >> Ezequiel, I am back in business to test a v2 ;-)
> >
> > Well, I'm not sure yet what's going on. Do you have any spare NAND partition
> > to run some destructive testings?
> >
> > In that case, please run:
> >
> > $ nandtest /dev/mtd{X}
> 
> Here is the nandtest run:
> 
>   nandtest /dev/mtd4
>   ECC corrections: 0
>   ECC failures   : 0
>   Bad blocks     : 8
>   BBT blocks     : 0
>   Bad block at 0x06700000
>   Bad block at 0x06720000
>   Bad block at 0x06740000
>   Bad block at 0x06760000
>   Bad block at 0x06780000
>   Bad block at 0x067a0000
>   Bad block at 0x067c0000
>   Bad block at 0x067e0000
>   
>   Finished pass 1 successfully
> 

Hm, so something *is* working properly. Notice that mtd4 has its 8 last blocks
marked 'bad' becuase it holds the bad block table.

If you don't mind running a few more rounds, then it would be nice to do:

  $ nandtest --passes {N}

So we run the test a few more times, just to be sure.

> 
> Then, doing a simple read and copying back the data:
> 
>   root@mood:~# dd if=/dev/mtd4 of=/tmp/toto 
>   212992+0 records in
>   212992+0 records out
>   109051904 bytes (109 MB) copied, 10.8671 s, 10.0 MB/s
>   
>   root@mood:~# flash_erase /dev/mtd4 0 0
>   
>   root@mood:~# nandwrite -p /dev/mtd4 /tmp/toto
>   ...
>   Writing data to block 795 at offset 0x6360000
>   Writing data to block 796 at offset 0x6380000
>   Writing data to block 797 at offset 0x63a0000
>   Writing data to block 798 at offset 0x63c0000
>   Writing data to block 799 at offset 0x63e0000
>   Writing data to block 800 at offset 0x6400000
>   [ 1509.210395] pxa3xx-nand d00d0000.nand: Ready time out!!!
>   libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 800, offset 0)
>           error 5 (Input/output error)
[..]

>   [ 1513.810387] pxa3xx-nand d00d0000.nand: Ready time out!!!
>   libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 823, offset 0)
>           error 5 (Input/output error)
>   Erasing failed write from 0x66e0000 to 0x66fffff

Hm.. so you get errors when writing to mtd4 blocks from 800 to 823.

Is that completely reproducible, IOW do you get always the error
on those blocks?

And what happens if you write directly to those blocks only?
(you can use nandwrite --offset)

>   Writing data to block 824 at offset 0x6700000
>   Bad block at 6700000, 1 block(s) from 6700000 will be skipped
>   Writing data to block 825 at offset 0x6720000
>   Bad block at 6720000, 1 block(s) from 6720000 will be skipped
>   Writing data to block 826 at offset 0x6740000
>   Bad block at 6740000, 1 block(s) from 6740000 will be skipped
>   Writing data to block 827 at offset 0x6760000
>   Bad block at 6760000, 1 block(s) from 6760000 will be skipped
>   Writing data to block 828 at offset 0x6780000
>   Bad block at 6780000, 1 block(s) from 6780000 will be skipped
>   Writing data to block 829 at offset 0x67a0000
>   Bad block at 67a0000, 1 block(s) from 67a0000 will be skipped
>   Writing data to block 830 at offset 0x67c0000
>   Bad block at 67c0000, 1 block(s) from 67c0000 will be skipped
>   Writing data to block 831 at offset 0x67e0000
>   Bad block at 67e0000, 1 block(s) from 67e0000 will be skipped
>   Writing data to block 832 at offset 0x6800000
>   libmtd: error!: bad eraseblock number 832, mtd4 has 832 eraseblocks
>   nandwrite: error!: /dev/mtd4: MTD get bad block failed
>              error 22 (Invalid argument)
>   nandwrite: error!: Data was only partially written due to error
>              error 22 (Invalid argument)
>   

These 8 blocks (824-832) that has been skipped are the ones marked
as 'bad' because they hold the bad block table.

> This is the kind of errors I got last time but I think am starting to
> understand the root cause now. Tell me if I get it right: what is
> understood as bad blocks above (and in nandtest) is in fact the two bad
> block tables reported during boot:
> 
>  NAND device: Manufacturer ID: 0xad, Chip ID: 0xf1 (Hynix H27U1G8F2BTR-BC)
>  NAND device: 128MiB, SLC, page size: 2048, OOB size: 64
>  Bad block table found at page 65472, version 0x01
>  Bad block table found at page 65408, version 0x01
> 

Yes and no :-) The bad block table consists of 8 blocks at the end
of the flash device. These blocks are marked as 'reserved' and nandtest
or any other userspace writing/erasing tool will skip them.

Hence, the bad block table is what explains the skipping of the group
of blocks [824..832]. However, you're getting errors when writing
data to [800..823], and it's a "Ready timeout" condition. I'm not sure
exactly what's going on, but we can say that:

  * Either the waiting time is not enough, or ...

  * The commands (maybe some race) were badly issued so there's nothing
    to wait at all.

To check the former, you only need to hack the driver like this, and
then re-try the nandwrite:

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 3d143fe..9bb7d35 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -39,7 +39,7 @@
 #include <linux/platform_data/mtd-nand-pxa3xx.h>
 
 #define NAND_DEV_READY_TIMEOUT  50
-#define	CHIP_DELAY_TIMEOUT	(2 * HZ/10)
+#define	CHIP_DELAY_TIMEOUT	(10 * HZ/10)
 #define NAND_STOP_DELAY		(2 * HZ/50)
 #define PAGE_CHUNK_SIZE		(2048)
 
It's just a test.

To check about the latter, I cannot think of anything but adding printk
all over the place and inspect the command sequence.

Brian: Do you have any better idea?

Thanks again for these tests and for your patience!
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Arnaud Ebalard]

Hi Ezequiel,

Ezequiel Garcia <ezequiel.garcia@free-electrons.com> writes:

>> Here is the nandtest run:
>> 
>>   nandtest /dev/mtd4
>>   ECC corrections: 0
>>   ECC failures   : 0
>>   Bad blocks     : 8
>>   BBT blocks     : 0
>>   Bad block at 0x06700000
>>   Bad block at 0x06720000
>>   Bad block at 0x06740000
>>   Bad block at 0x06760000
>>   Bad block at 0x06780000
>>   Bad block at 0x067a0000
>>   Bad block at 0x067c0000
>>   Bad block at 0x067e0000
>>   
>>   Finished pass 1 successfully
>> 
>
> Hm, so something *is* working properly. Notice that mtd4 has its 8 last blocks
> marked 'bad' becuase it holds the bad block table.

I think I am missing something here: u-boot reports that the BBT are
located here:

  >>  Bad block table found at page 65472, version 0x01
  >>  Bad block table found at page 65408, version 0x01

which seems to indicate that they are each 64 pages (i.e. 128KB)
long. AFAICT, the log output seems to indicate that blocks are
128KB in size:

  >>   Bad block at 0x06700000
  >>   Bad block at 0x06720000
  >>   Bad block at 0x06740000
  >>   Bad block at 0x06760000
  >>   Bad block at 0x06780000
  >>   Bad block at 0x067a0000
  >>   Bad block at 0x067c0000
  >>   Bad block at 0x067e0000

which is also what mtdinfo reports:

  root@mood:~# mtdinfo /dev/mtd4
  mtd4
  Name:                           jffs2
  Type:                           nand
  Eraseblock size:                131072 bytes, 128.0 KiB
  Amount of eraseblocks:          832 (109051904 bytes, 104.0 MiB)
  Minimum input/output unit size: 2048 bytes
  Sub-page size:                  2048 bytes
  OOB size:                       64 bytes
  Character device major/minor:   90:8
  Bad blocks are allowed:         true
  Device is writable:             true
  
So unless I am missing something, *according* to u-boot there are 2 BBT
and each BBT is 1 block long. 

Additionally - and this is a point I would like to understand - the
reported location of this BBT is not after the 128MB of the chip but
before the end of those 128MB. This also means before the end of the
last partition. So the definition of the partition itsef gives the
impression that all the blocks in the partition are available when in
fact, the last two blocks cannot be used at all (or 8 if you are
right). Put in a different manner, if I nandwrite a 104.0 MiB image to
the last partition (/dev/mtd4), I am guaranteed to try and overwrite the
BBT. 

Do not hesitate to tell me what I am missing here or if it is
expected that the partition definition (start address and length)
*includes* the bbt pages/blocks.

> If you don't mind running a few more rounds, then it would be nice to do:
>
>   $ nandtest --passes {N}
>
> So we run the test a few more times, just to be sure.

root@mood:~# nandtest --passes 2 /dev/mtd4
ECC corrections: 0
ECC failures   : 0
Bad blocks     : 8
BBT blocks     : 0
Bad block at 0x06700000
Bad block at 0x06720000
Bad block at 0x06740000
Bad block at 0x06760000
Bad block at 0x06780000
Bad block at 0x067a0000
Bad block at 0x067c0000
Bad block at 0x067e0000

Finished pass 1 successfully
Bad block at 0x06700000
Bad block at 0x06720000
Bad block at 0x06740000
Bad block at 0x06760000
Bad block at 0x06780000
Bad block at 0x067a0000
Bad block at 0x067c0000
Bad block at 0x067e0000

Finished pass 2 successfully

>>   root@mood:~# nandwrite -p /dev/mtd4 /tmp/toto
>>   ...
>>   Writing data to block 795 at offset 0x6360000
>>   Writing data to block 796 at offset 0x6380000
>>   Writing data to block 797 at offset 0x63a0000
>>   Writing data to block 798 at offset 0x63c0000
>>   Writing data to block 799 at offset 0x63e0000
>>   Writing data to block 800 at offset 0x6400000
>>   [ 1509.210395] pxa3xx-nand d00d0000.nand: Ready time out!!!
>>   libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 800, offset 0)
>>           error 5 (Input/output error)
> [..]
>
>>   [ 1513.810387] pxa3xx-nand d00d0000.nand: Ready time out!!!
>>   libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 823, offset 0)
>>           error 5 (Input/output error)
>>   Erasing failed write from 0x66e0000 to 0x66fffff
>
> Hm.. so you get errors when writing to mtd4 blocks from 800 to 823.
>
> Is that completely reproducible, IOW do you get always the error
> on those blocks?

Well, I did the same test again, and I think the answer is 'no':

root@mood:~# dd if=/dev/mtd4 of=/tmp/toto
212992+0 records in
212992+0 records out
109051904 bytes (109 MB) copied, 11.136 s, 9.8 MB/s

root@mood:~# flash_erase /dev/mtd4 0 0
Erasing 128 Kibyte @ 66e0000 -- 98 % complete flash_erase: Skipping bad block at 06700000
flash_erase: Skipping bad block at 06720000
flash_erase: Skipping bad block at 06740000
flash_erase: Skipping bad block at 06760000
flash_erase: Skipping bad block at 06780000
flash_erase: Skipping bad block at 067a0000
flash_erase: Skipping bad block at 067c0000
flash_erase: Skipping bad block at 067e0000
Erasing 128 Kibyte @ 67e0000 -- 100 % complete 

root@mood:~# nandwrite -p /dev/mtd4 /tmp/toto
Writing data to block 0 at offset 0x0
Writing data to block 1 at offset 0x20000
Writing data to block 2 at offset 0x40000
Writing data to block 3 at offset 0x60000
Writing data to block 4 at offset 0x80000
Writing data to block 5 at offset 0xa0000
Writing data to block 6 at offset 0xc0000
Writing data to block 7 at offset 0xe0000
...
Writing data to block 818 at offset 0x6640000
Writing data to block 819 at offset 0x6660000
Writing data to block 820 at offset 0x6680000
Writing data to block 821 at offset 0x66a0000
Writing data to block 822 at offset 0x66c0000
Writing data to block 823 at offset 0x66e0000
Writing data to block 824 at offset 0x6700000
Bad block at 6700000, 1 block(s) from 6700000 will be skipped
Writing data to block 825 at offset 0x6720000
Bad block at 6720000, 1 block(s) from 6720000 will be skipped
Writing data to block 826 at offset 0x6740000
Bad block at 6740000, 1 block(s) from 6740000 will be skipped
Writing data to block 827 at offset 0x6760000
Bad block at 6760000, 1 block(s) from 6760000 will be skipped
Writing data to block 828 at offset 0x6780000
Bad block at 6780000, 1 block(s) from 6780000 will be skipped
Writing data to block 829 at offset 0x67a0000
Bad block at 67a0000, 1 block(s) from 67a0000 will be skipped
Writing data to block 830 at offset 0x67c0000
Bad block at 67c0000, 1 block(s) from 67c0000 will be skipped
Writing data to block 831 at offset 0x67e0000
Bad block at 67e0000, 1 block(s) from 67e0000 will be skipped
Writing data to block 832 at offset 0x6800000
libmtd: error!: bad eraseblock number 832, mtd4 has 832 eraseblocks
nandwrite: error!: /dev/mtd4: MTD get bad block failed
           error 22 (Invalid argument)
nandwrite: error!: Data was only partially written due to error
           error 22 (Invalid argument)
root@mood:~# 

Well, this time I don't get the "ready timeout" error I had last
time around block 800. I did the test a second time (flash_erase and
then nandwrite) and got the same result, i.e. no error before 824.


>>   Writing data to block 824 at offset 0x6700000
>>   Bad block at 6700000, 1 block(s) from 6700000 will be skipped
>>   Writing data to block 825 at offset 0x6720000
>>   Bad block at 6720000, 1 block(s) from 6720000 will be skipped
>>   Writing data to block 826 at offset 0x6740000
>>   Bad block at 6740000, 1 block(s) from 6740000 will be skipped
>>   Writing data to block 827 at offset 0x6760000
>>   Bad block at 6760000, 1 block(s) from 6760000 will be skipped
>>   Writing data to block 828 at offset 0x6780000
>>   Bad block at 6780000, 1 block(s) from 6780000 will be skipped
>>   Writing data to block 829 at offset 0x67a0000
>>   Bad block at 67a0000, 1 block(s) from 67a0000 will be skipped
>>   Writing data to block 830 at offset 0x67c0000
>>   Bad block at 67c0000, 1 block(s) from 67c0000 will be skipped
>>   Writing data to block 831 at offset 0x67e0000
>>   Bad block at 67e0000, 1 block(s) from 67e0000 will be skipped
>>   Writing data to block 832 at offset 0x6800000
>>   libmtd: error!: bad eraseblock number 832, mtd4 has 832 eraseblocks
>>   nandwrite: error!: /dev/mtd4: MTD get bad block failed
>>              error 22 (Invalid argument)
>>   nandwrite: error!: Data was only partially written due to error
>>              error 22 (Invalid argument)
>>   
>
> These 8 blocks (824-832) that has been skipped are the ones marked
> as 'bad' because they hold the bad block table.
>
>> This is the kind of errors I got last time but I think am starting to
>> understand the root cause now. Tell me if I get it right: what is
>> understood as bad blocks above (and in nandtest) is in fact the two bad
>> block tables reported during boot:
>> 
>>  NAND device: Manufacturer ID: 0xad, Chip ID: 0xf1 (Hynix H27U1G8F2BTR-BC)
>>  NAND device: 128MiB, SLC, page size: 2048, OOB size: 64
>>  Bad block table found at page 65472, version 0x01
>>  Bad block table found at page 65408, version 0x01
>> 
>
> Yes and no :-) The bad block table consists of 8 blocks at the end
> of the flash device. These blocks are marked as 'reserved' and nandtest
> or any other userspace writing/erasing tool will skip them.
>
> Hence, the bad block table is what explains the skipping of the group
> of blocks [824..832]. However, you're getting errors when writing
> data to [800..823], and it's a "Ready timeout" condition. I'm not sure
> exactly what's going on, but we can say that:
>
>   * Either the waiting time is not enough, or ...
>
>   * The commands (maybe some race) were badly issued so there's nothing
>     to wait at all.

As I can not reproduce previous behavior (I am on the exact same
kernel), I guess it's difficult to go any further yet.

Cheers,

a+

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Ezequiel Garcia]

Hi Arnaud,

I'll try to explain the numbers you're observing. If I say something stupid
along the way, feel free to correct me.

When I wrote this driver I got puzzled by the same questions you're asking
so rest assured I completely understand your confusion ;-)

Let me being by writing your NAND layout, just for reference.

0x000000000000-0x000000180000 : "u-boot"
0x000000180000-0x0000001a0000 : "u-boot-env"
0x000000200000-0x000000800000 : "uImage"
0x000000800000-0x000001800000 : "minirootfs"
0x000001800000-0x000008000000 : "jffs2"

Your NAND device has page size 2048 byte. Therefore, the following message:

> 
>   >>  Bad block table found at page 65472, version 0x01
>   >>  Bad block table found at page 65408, version 0x01
> 

Is telling us that there are two bad block tables (the table and its so-called
mirror): one at page 65472 and another at page 65408.
These page numbers correspond to offset 0x7fe0000 and 0x7fc0000 respectively.

Then, considering the mtd4 (aka "jffs2") partition starts at 0x1800000, we can
say the bad block tables start at offsets 0x67e0000 and 0x67c0000, from the
start of mtd4.

And since you run 'nandtest' on a partition and not on the entire
device you get the bad block locations at mtd4 offset:

>   >>   Bad block at 0x06700000
>   >>   Bad block at 0x06720000
>   >>   Bad block at 0x06740000
>   >>   Bad block at 0x06760000
>   >>   Bad block at 0x06780000
>   >>   Bad block at 0x067a0000
>   >>   Bad block at 0x067c0000 <-- Mirror bad block table
>   >>   Bad block at 0x067e0000 <-- Bad block table
> 

The reason the numbers appear as "upside-down" is because the pxa3xx-nand
driver uses this BBT options:

.options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE
	 | NAND_BBT_2BIT | NAND_BBT_VERSION,

So, the NAND core code writes the table to the last block (as per
NAND_BBT_LASTBLOCK option). Of course, it must write the bad block
table *inside* the flash (before the end of its last partition) and
to prevent any user I/O operation from losing the BBT the NAND core
marks as bad using a special 'reserved' mark.

Now, why does NAND reserve eight blocks, if there are only two tables?
Well, you'll be able to find this in the driver:

static struct nand_bbt_descr bbt_main_descr = {
	/* stuff */
	.maxblocks = 8,         /* Last 8 blocks in each chip */
};

The snippet above asks the NAND core to scan the last 8 blocks when searching
for the in-flash bad block table. The NAND core will also reserve these
8 blocks as the maximum amount of blocks that can be used to store a bad
block table (I guess that's in case one block gets 'really' bad).

In conclusion, answering your question, yes, this is correct:

> it's expected that the partition definition (start address and length)
> *includes* the bbt pages/blocks.
> 

The flash stores 8 x 128 KiB = 1 MiB for the bad block table and so
whenever you prepare your filesystem, you must now that the last
partition is actually 1 MiB smaller.

On the other issue, about the timeout...

> root@mood:~# nandwrite -p /dev/mtd4 /tmp/toto
> Writing data to block 0 at offset 0x0
> Writing data to block 1 at offset 0x20000
> Writing data to block 2 at offset 0x40000
> Writing data to block 3 at offset 0x60000
> Writing data to block 4 at offset 0x80000
> Writing data to block 5 at offset 0xa0000
> Writing data to block 6 at offset 0xc0000
> Writing data to block 7 at offset 0xe0000
> ...
> Writing data to block 818 at offset 0x6640000
> Writing data to block 819 at offset 0x6660000
> Writing data to block 820 at offset 0x6680000
> Writing data to block 821 at offset 0x66a0000
> Writing data to block 822 at offset 0x66c0000
> Writing data to block 823 at offset 0x66e0000
> Writing data to block 824 at offset 0x6700000
> Bad block at 6700000, 1 block(s) from 6700000 will be skipped
> Writing data to block 825 at offset 0x6720000
> Bad block at 6720000, 1 block(s) from 6720000 will be skipped
> Writing data to block 826 at offset 0x6740000
> Bad block at 6740000, 1 block(s) from 6740000 will be skipped
> Writing data to block 827 at offset 0x6760000
> Bad block at 6760000, 1 block(s) from 6760000 will be skipped
> Writing data to block 828 at offset 0x6780000
> Bad block at 6780000, 1 block(s) from 6780000 will be skipped
> Writing data to block 829 at offset 0x67a0000
> Bad block at 67a0000, 1 block(s) from 67a0000 will be skipped
> Writing data to block 830 at offset 0x67c0000
> Bad block at 67c0000, 1 block(s) from 67c0000 will be skipped
> Writing data to block 831 at offset 0x67e0000
> Bad block at 67e0000, 1 block(s) from 67e0000 will be skipped
> Writing data to block 832 at offset 0x6800000
> libmtd: error!: bad eraseblock number 832, mtd4 has 832 eraseblocks
> nandwrite: error!: /dev/mtd4: MTD get bad block failed
>            error 22 (Invalid argument)
> nandwrite: error!: Data was only partially written due to error
>            error 22 (Invalid argument)
> root@mood:~# 
> 

OK... Then I guess the driver works fine but you're trying to write
an image bigger than possible. Where did you get such filesystem?

Maybe reserving 8 blocks is *a lot* (I personally think it is a lot,
but maybe it's more robust that way).

> > Hence, the bad block table is what explains the skipping of the group
> > of blocks [824..832]. However, you're getting errors when writing
> > data to [800..823], and it's a "Ready timeout" condition. I'm not sure
> > exactly what's going on, but we can say that:
> >
> >   * Either the waiting time is not enough, or ...
> >
> >   * The commands (maybe some race) were badly issued so there's nothing
> >     to wait at all.
> 
> As I can not reproduce previous behavior (I am on the exact same
> kernel), I guess it's difficult to go any further yet.
> 

Hm.. that's not cool. The fact that you got those timeouts, but now
you don't have them anymore only worries me even more.

Feel free to ask any more questions about the NAND stuff, although I've
shared with you pretty much all I could collect from my readings to the code.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Arnaud Ebalard]

Hi Ezequiel,

Ezequiel Garcia <ezequiel.garcia@free-electrons.com> writes:

> I'll try to explain the numbers you're observing. If I say something stupid
> along the way, feel free to correct me.
>
> When I wrote this driver I got puzzled by the same questions you're asking
> so rest assured I completely understand your confusion ;-)

Thanks. Unless ones spends the time being taught that or reading the code,
the logic of BBT mapping and reservation is not exactly straightforward.
Thanks for the explanation.

More news below.


> The reason the numbers appear as "upside-down" is because the pxa3xx-nand
> driver uses this BBT options:
>
> .options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE
> 	 | NAND_BBT_2BIT | NAND_BBT_VERSION,
>
> So, the NAND core code writes the table to the last block (as per
> NAND_BBT_LASTBLOCK option). Of course, it must write the bad block
> table *inside* the flash (before the end of its last partition) and
> to prevent any user I/O operation from losing the BBT the NAND core
> marks as bad using a special 'reserved' mark.

ok.


> Now, why does NAND reserve eight blocks, if there are only two tables?
> Well, you'll be able to find this in the driver:
>
> static struct nand_bbt_descr bbt_main_descr = {
> 	/* stuff */
> 	.maxblocks = 8,         /* Last 8 blocks in each chip */
> };
>
> The snippet above asks the NAND core to scan the last 8 blocks when searching
> for the in-flash bad block table. The NAND core will also reserve these
> 8 blocks as the maximum amount of blocks that can be used to store a bad
> block table (I guess that's in case one block gets 'really' bad).
>
> In conclusion, answering your question, yes, this is correct:
>
>> it's expected that the partition definition (start address and length)
>> *includes* the bbt pages/blocks.
>> 
>
> The flash stores 8 x 128 KiB = 1 MiB for the bad block table and so
> whenever you prepare your filesystem, you must now that the last
> partition is actually 1 MiB smaller.

ok. Will put a comment in the .dts about that.


> On the other issue, about the timeout...
>
>> root@mood:~# nandwrite -p /dev/mtd4 /tmp/toto
>> Writing data to block 0 at offset 0x0
>> Writing data to block 1 at offset 0x20000
>> Writing data to block 2 at offset 0x40000
>> Writing data to block 3 at offset 0x60000
>> Writing data to block 4 at offset 0x80000
>> Writing data to block 5 at offset 0xa0000
>> Writing data to block 6 at offset 0xc0000
>> Writing data to block 7 at offset 0xe0000
>> ...
>> Writing data to block 818 at offset 0x6640000
>> Writing data to block 819 at offset 0x6660000
>> Writing data to block 820 at offset 0x6680000
>> Writing data to block 821 at offset 0x66a0000
>> Writing data to block 822 at offset 0x66c0000
>> Writing data to block 823 at offset 0x66e0000
>> Writing data to block 824 at offset 0x6700000
>> Bad block at 6700000, 1 block(s) from 6700000 will be skipped
>> Writing data to block 825 at offset 0x6720000
>> Bad block at 6720000, 1 block(s) from 6720000 will be skipped
>> Writing data to block 826 at offset 0x6740000
>> Bad block at 6740000, 1 block(s) from 6740000 will be skipped
>> Writing data to block 827 at offset 0x6760000
>> Bad block at 6760000, 1 block(s) from 6760000 will be skipped
>> Writing data to block 828 at offset 0x6780000
>> Bad block at 6780000, 1 block(s) from 6780000 will be skipped
>> Writing data to block 829 at offset 0x67a0000
>> Bad block at 67a0000, 1 block(s) from 67a0000 will be skipped
>> Writing data to block 830 at offset 0x67c0000
>> Bad block at 67c0000, 1 block(s) from 67c0000 will be skipped
>> Writing data to block 831 at offset 0x67e0000
>> Bad block at 67e0000, 1 block(s) from 67e0000 will be skipped
>> Writing data to block 832 at offset 0x6800000
>> libmtd: error!: bad eraseblock number 832, mtd4 has 832 eraseblocks
>> nandwrite: error!: /dev/mtd4: MTD get bad block failed
>>            error 22 (Invalid argument)
>> nandwrite: error!: Data was only partially written due to error
>>            error 22 (Invalid argument)
>> root@mood:~# 
>> 
>
> OK... Then I guess the driver works fine but you're trying to write
> an image bigger than possible. Where did you get such filesystem?

The image is a dd from the partition itself. Trying to put it back will
obviously not work for the reason you provide above, i.e. because dd
reads to the end of the partition (including the BBT) but then during
the write operation the reserved block cannot be written back.


> Maybe reserving 8 blocks is *a lot* (I personally think it is a lot,
> but maybe it's more robust that way).
>
>> > Hence, the bad block table is what explains the skipping of the group
>> > of blocks [824..832]. However, you're getting errors when writing
>> > data to [800..823], and it's a "Ready timeout" condition. I'm not sure
>> > exactly what's going on, but we can say that:
>> >
>> >   * Either the waiting time is not enough, or ...
>> >
>> >   * The commands (maybe some race) were badly issued so there's nothing
>> >     to wait at all.
>> 
>> As I can not reproduce previous behavior (I am on the exact same
>> kernel), I guess it's difficult to go any further yet.
>> 
>
> Hm.. that's not cool. The fact that you got those timeouts, but now
> you don't have them anymore only worries me even more.

Well, before submitting the patches for armada-based ReadyNAS .dts
files, I decided to give each of them a try on their respective hardware
and I was somewhat lucky (if I can say so) on my RN2120 and got the
following. Note that the NAND chip is the same but the SoC on the NAS
is different (dual-core armada Xp mv78230 on RN2120 instead of Armada
370 on RN102). The same tree is used in each case with the same MTD
related config options:


root@thin:~# flash_erase /dev/mtd4 0 0
...

root@thin:~# nandwrite -p /dev/mtd4 /tmp/mtd4ro 
Writing data to block 0 at offset 0x0
[  782.766892] pxa3xx-nand d00d0000.nand: Ready time out!!!
libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 0, offset 2048)
        error 5 (Input/output error)
Erasing failed write from 00000000 to 0x01ffff
Writing data to block 1 at offset 0x20000
[  782.966891] pxa3xx-nand d00d0000.nand: Ready time out!!!
libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 1, offset 2048)
        error 5 (Input/output error)
Erasing failed write from 0x020000 to 0x03ffff
Writing data to block 2 at offset 0x40000
[  783.166891] pxa3xx-nand d00d0000.nand: Ready time out!!!
libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 2, offset 2048)
        error 5 (Input/output error)
Erasing failed write from 0x040000 to 0x05ffff
Writing data to block 3 at offset 0x60000
[  783.366889] pxa3xx-nand d00d0000.nand: Ready time out!!!
libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 3, offset 2048)
        error 5 (Input/output error)
Erasing failed write from 0x060000 to 0x07ffff
Writing data to block 4 at offset 0x80000
[  783.566890] pxa3xx-nand d00d0000.nand: Ready time out!!!
libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 4, offset 2048)
        error 5 (Input/output error)
Erasing failed write from 0x080000 to 0x09ffff
Writing data to block 5 at offset 0xa0000
[  783.766890] pxa3xx-nand d00d0000.nand: Ready time out!!!
libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 5, offset 2048)
        error 5 (Input/output error)
Erasing failed write from 0x0a0000 to 0x0bffff
Writing data to block 6 at offset 0xc0000
[  783.966890] pxa3xx-nand d00d0000.nand: Ready time out!!!
libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 6, offset 2048)
        error 5 (Input/output error)
Erasing failed write from 0x0c0000 to 0x0dffff
Writing data to block 7 at offset 0xe0000
[  784.166890] pxa3xx-nand d00d0000.nand: Ready time out!!!
libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 7, offset 2048)
        error 5 (Input/output error)
Erasing failed write from 0x0e0000 to 0x0fffff
Writing data to block 8 at offset 0x100000
[  784.366889] pxa3xx-nand d00d0000.nand: Ready time out!!!
libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 8, offset 2048)
        error 5 (Input/output error)
Erasing failed write from 0x100000 to 0x11ffff
Writing data to block 9 at offset 0x120000
^C[  784.566889] pxa3xx-nand d00d0000.nand: Ready time out!!!


So I decided to increase CHIP_DELAY_TIMEOUT in the driver as you
suggested in a previous email (HZ instead of 2*HZ/10):

root@thin:~# nandwrite -p /dev/mtd4 mtd4ro 
Writing data to block 0 at offset 0x0
[  273.917098] pxa3xx-nand d00d0000.nand: Ready time out!!!
libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 0, offset 2048)
        error 5 (Input/output error)
Erasing failed write from 00000000 to 0x01ffff
Writing data to block 1 at offset 0x20000
[  274.917094] pxa3xx-nand d00d0000.nand: Ready time out!!!
libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 1, offset 2048)
        error 5 (Input/output error)
Erasing failed write from 0x020000 to 0x03ffff
Writing data to block 2 at offset 0x40000
[  275.917094] pxa3xx-nand d00d0000.nand: Ready time out!!!
libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 2, offset 2048)
        error 5 (Input/output error)
Erasing failed write from 0x040000 to 0x05ffff
Writing data to block 3 at offset 0x60000
^C[  276.917094] pxa3xx-nand d00d0000.nand: Ready time out!!!


And then to 25*HZ/10 (I know it's a lot):

root@thin:~# nandwrite /dev/mtd4 mtd4ro 
Writing data to block 0 at offset 0x0
[  192.118574] pxa3xx-nand d00d0000.nand: Ready time out!!!
libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 0, offset 2048)
        error 5 (Input/output error)
Erasing failed write from 00000000 to 0x01ffff
Writing data to block 1 at offset 0x20000
[  194.618569] pxa3xx-nand d00d0000.nand: Ready time out!!!
libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 1, offset 2048)
        error 5 (Input/output error)
Erasing failed write from 0x020000 to 0x03ffff
Writing data to block 2 at offset 0x40000
^C[  197.118570] pxa3xx-nand d00d0000.nand: Ready time out!!!


So I guess, if you have any (non hardware destructive ;-) ) idea, I now
have a platform on which the problem is reproducible. Meanwhile, I will
try and do the same on my RN104 (should be the same as the RN102).

Cheers,

a+

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Arnaud Ebalard]

Hi Ezequiel,

arno@natisbad.org (Arnaud Ebalard) writes:

> So I guess, if you have any (non hardware destructive ;-) ) idea, I now
> have a platform on which the problem is reproducible. Meanwhile, I will
> try and do the same on my RN104 (should be the same as the RN102).

Well, on the RN104 (same chip, same kernel and same SoC as the RN102), I
have the following (I get the same with a patched driver to increase the
timeout):

root@humble:~# flash_erase /dev/mtd4 0 0 
Erasing 128 Kibyte @ 72e0000 -- 99 % complete flash_erase: Skipping bad block at 07300000
flash_erase: Skipping bad block at 07320000
flash_erase: Skipping bad block at 07340000
flash_erase: Skipping bad block at 07360000
flash_erase: Skipping bad block at 07380000
flash_erase: Skipping bad block at 073a0000
flash_erase: Skipping bad block at 073c0000
flash_erase: Skipping bad block at 073e0000
Erasing 128 Kibyte @ 73e0000 -- 100 % complete 

root@humble:~# nandwrite -p /dev/mtd4 mtd4ro 
Writing data to block 0 at offset 0x0
[  449.915173] pxa3xx-nand d00d0000.nand: Ready time out!!!
libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 0, offset 2048)
        error 5 (Input/output error)
Erasing failed write from 00000000 to 0x01ffff
Writing data to block 1 at offset 0x20000
[  450.115172] pxa3xx-nand d00d0000.nand: Ready time out!!!
libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 1, offset 2048)
        error 5 (Input/output error)
Erasing failed write from 0x020000 to 0x03ffff
Writing data to block 2 at offset 0x40000
[  450.315171] pxa3xx-nand d00d0000.nand: Ready time out!!!
libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 2, offset 2048)
        error 5 (Input/output error)
Erasing failed write from 0x040000 to 0x05ffff
Writing data to block 3 at offset 0x60000
[  450.515171] pxa3xx-nand d00d0000.nand: Ready time out!!!
libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 3, offset 2048)
        error 5 (Input/output error)
Erasing failed write from 0x060000 to 0x07ffff
Writing data to block 4 at offset 0x80000
[  450.715169] pxa3xx-nand d00d0000.nand: Ready time out!!!
libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 4, offset 2048)
        error 5 (Input/output error)
Erasing failed write from 0x080000 to 0x09ffff
Writing data to block 5 at offset 0xa0000
[  450.915171] pxa3xx-nand d00d0000.nand: Ready time out!!!
libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 5, offset 2048)
        error 5 (Input/output error)
Erasing failed write from 0x0a0000 to 0x0bffff
Writing data to block 6 at offset 0xc0000
[  451.115171] pxa3xx-nand d00d0000.nand: Ready time out!!!
libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 6, offset 2048)
        error 5 (Input/output error)
Erasing failed write from 0x0c0000 to 0x0dffff
Writing data to block 7 at offset 0xe0000


So I then did the same test again on the RN102 by doing a flash_erase on
/dev/mtd4 and then a nandwrite: it worked as expected (errors only at
the end when trying to overwrite the reserved block for the BBT).

The only difference I can see between RN104/RN2120 and RN102 is the
debian: the three are jessie but the one on the RN102 I used is armel
when the ones on the RN104 and RN2120 are armhf (all kernels are
armel).

Cheers,

a+

ps: I can provide the config files for each device's kernel if you think
a specific option (debug or something else) may have an impact on how the
driver behave.

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Ezequiel Garcia]

Arnaud,

Thanks a lot for the patience!


On Tue, Dec 03, 2013 at 10:25:14PM +0100, Arnaud Ebalard wrote:
> Hi Ezequiel,
> 
> arno@natisbad.org (Arnaud Ebalard) writes:
> 
> > So I guess, if you have any (non hardware destructive ;-) ) idea, I now
> > have a platform on which the problem is reproducible. Meanwhile, I will
> > try and do the same on my RN104 (should be the same as the RN102).
> 
> Well, on the RN104 (same chip, same kernel and same SoC as the RN102), I
> have the following (I get the same with a patched driver to increase the
> timeout):
> 
> root@humble:~# flash_erase /dev/mtd4 0 0 
> Erasing 128 Kibyte @ 72e0000 -- 99 % complete flash_erase: Skipping bad block at 07300000
> flash_erase: Skipping bad block at 07320000
> flash_erase: Skipping bad block at 07340000
> flash_erase: Skipping bad block at 07360000
> flash_erase: Skipping bad block at 07380000
> flash_erase: Skipping bad block at 073a0000
> flash_erase: Skipping bad block at 073c0000
> flash_erase: Skipping bad block at 073e0000
> Erasing 128 Kibyte @ 73e0000 -- 100 % complete 
> 
> root@humble:~# nandwrite -p /dev/mtd4 mtd4ro 
> Writing data to block 0 at offset 0x0
> [  449.915173] pxa3xx-nand d00d0000.nand: Ready time out!!!
> libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 0, offset 2048)
>         error 5 (Input/output error)
> Erasing failed write from 00000000 to 0x01ffff
> Writing data to block 1 at offset 0x20000
> [  450.115172] pxa3xx-nand d00d0000.nand: Ready time out!!!
> libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 1, offset 2048)
>         error 5 (Input/output error)
> Erasing failed write from 0x020000 to 0x03ffff
> Writing data to block 2 at offset 0x40000
> [  450.315171] pxa3xx-nand d00d0000.nand: Ready time out!!!
> libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 2, offset 2048)
>         error 5 (Input/output error)
> Erasing failed write from 0x040000 to 0x05ffff
> Writing data to block 3 at offset 0x60000
> [  450.515171] pxa3xx-nand d00d0000.nand: Ready time out!!!
> libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 3, offset 2048)
>         error 5 (Input/output error)
> Erasing failed write from 0x060000 to 0x07ffff
> Writing data to block 4 at offset 0x80000
> [  450.715169] pxa3xx-nand d00d0000.nand: Ready time out!!!
> libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 4, offset 2048)
>         error 5 (Input/output error)
> Erasing failed write from 0x080000 to 0x09ffff
> Writing data to block 5 at offset 0xa0000
> [  450.915171] pxa3xx-nand d00d0000.nand: Ready time out!!!
> libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 5, offset 2048)
>         error 5 (Input/output error)
> Erasing failed write from 0x0a0000 to 0x0bffff
> Writing data to block 6 at offset 0xc0000
> [  451.115171] pxa3xx-nand d00d0000.nand: Ready time out!!!
> libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 6, offset 2048)
>         error 5 (Input/output error)
> Erasing failed write from 0x0c0000 to 0x0dffff
> Writing data to block 7 at offset 0xe0000
> 
> 

So, let me confirm this: you have systematically obtained a "Ready
timeout" when writing to the device, on every single write to a page,
correct?

I'll prepare a patch against the branch we're working that adds lots of
pr_info(). It'll be very annoying for you, but it's the only way I can
think of, to get the driver's dirty inner sequence and to see *where*
is failing.

However, before that patch, please do a quick test for me. It's just a
shot in the dark. The below diff removes the NDCB0_AUTO_RS flag from
the PAGE_PROG command when the page size is 2048 byte (or less).

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index fd0ef93..038cf5d 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -831,7 +831,6 @@ static int prepare_set_command(struct pxa3xx_nand_info *info, int command,
 			}
 		} else {
 			info->ndcb0 |= NDCB0_CMD_TYPE(0x1)
-					| NDCB0_AUTO_RS
 					| NDCB0_ST_ROW_EN
 					| NDCB0_DBC
 					| (NAND_CMD_PAGEPROG << 8)

Please apply this diff, and then try to write at least one page. If at
all possible you might try different approaches:

  $ flash_erase /dev/mtd2 0 1
  $ dd if=page.raw of=/dev/mtd2 bs=2048 count=1

  $ flash_erase /dev/mtd2 0 1
  $ nandwrite page.raw /dev/mtd2

Where page.raw is some 2048B file, which you can create with random junk in it.

Regards,
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Ezequiel Garcia]

On Wed, Dec 04, 2013 at 11:20:09AM -0300, Ezequiel Garcia wrote:
> On Tue, Dec 03, 2013 at 10:25:14PM +0100, Arnaud Ebalard wrote:
[..]
> > Erasing failed write from 0x0a0000 to 0x0bffff
> > Writing data to block 6 at offset 0xc0000
> > [  451.115171] pxa3xx-nand d00d0000.nand: Ready time out!!!
> > libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 6, offset 2048)
> >         error 5 (Input/output error)
> > Erasing failed write from 0x0c0000 to 0x0dffff
> > Writing data to block 7 at offset 0xe0000
> > 
> > 
> 
> So, let me confirm this: you have systematically obtained a "Ready
> timeout" when writing to the device, on every single write to a page,
> correct?
> 
> I'll prepare a patch against the branch we're working that adds lots of
> pr_info(). It'll be very annoying for you, but it's the only way I can
> think of, to get the driver's dirty inner sequence and to see *where*
> is failing.
> 
[..]

And here's the patch, along with instructions...

commit cc7999a922bf3c09ddc65effb11de6fc5ea88b1c
Author: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
Date:   Wed Dec 4 11:27:04 2013 -0300

    mtd: nand: pxa3xx: Add debug messages
    
    Need CONFIG_DYNAMIC_DEBUG=y and then:
    
    $ mount -t debugfs debugfs /sys/kernel/debug/
    $ cd /sys/kernel/debug/dynamic_debug/
    $ echo "file drivers/mtd/nand/pxa3xx_nand.c +p" > control
    
    Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 038cf5d..1744599 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -616,6 +616,7 @@ static irqreturn_t pxa3xx_nand_irq(int irq, void *devid)
 	if (status & ready) {
 		info->state = STATE_READY;
 		is_ready = 1;
+		pr_debug("Status: ready\n");
 	}
 
 	if (status & NDSR_WRCMDREQ) {
@@ -631,6 +632,11 @@ static irqreturn_t pxa3xx_nand_irq(int irq, void *devid)
 		 * Direct write access to NDCB1, NDCB2 and NDCB3 is ignored
 		 * but each NDCBx register can be read.
 		 */
+		pr_debug("Command0 0x%x\n", info->ndcb0);
+		pr_debug("Command1 0x%x\n", info->ndcb1);
+		pr_debug("Command2 0x%x\n", info->ndcb2);
+		pr_debug("Command3 0x%x\n", info->ndcb3);
+
 		nand_writel(info, NDCB0, info->ndcb0);
 		nand_writel(info, NDCB0, info->ndcb1);
 		nand_writel(info, NDCB0, info->ndcb2);
@@ -1154,6 +1160,7 @@ static int pxa3xx_nand_waitfunc(struct mtd_info *mtd, struct nand_chip *this)
 	int ret;
 
 	if (info->need_wait) {
+		pr_debug("Waiting for device to be ready\n");
 		ret = wait_for_completion_timeout(&info->dev_ready,
 				CHIP_DELAY_TIMEOUT);
 		info->need_wait = 0;
@@ -1161,6 +1168,7 @@ static int pxa3xx_nand_waitfunc(struct mtd_info *mtd, struct nand_chip *this)
 			dev_err(&info->pdev->dev, "Ready time out!!!\n");
 			return NAND_STATUS_FAIL;
 		}
+		pr_debug("Device is ready\n");
 	}
 
 	/* pxa3xx_nand_send_command has waited for command complete */
@@ -1468,6 +1476,10 @@ KEEP_CONFIG:
 	if (nand_scan_ident(mtd, 1, def))
 		return -ENODEV;
 
+	pr_info("Control  register dump 0x%x\n", info->reg_ndcr);
+	pr_info("Timing 0 register dump 0x%x\n", info->ndtr0cs0);
+	pr_info("Timing 1 register dump 0x%x\n", info->ndtr1cs0);
+
 	if (pdata->flash_bbt) {
 		/*
 		 * We'll use a bad block table stored in-flash and don't

-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Arnaud Ebalard]

Hi Ezequiel,

Ezequiel Garcia <ezequiel.garcia@free-electrons.com> writes:

> Thanks a lot for the patience!

Well, if we let the machines win, it's over.


>> root@humble:~# nandwrite -p /dev/mtd4 mtd4ro 
>> Writing data to block 0 at offset 0x0
>> [  449.915173] pxa3xx-nand d00d0000.nand: Ready time out!!!
>> libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 0, offset 2048)
>>         error 5 (Input/output error)
>> Erasing failed write from 00000000 to 0x01ffff
>> Writing data to block 1 at offset 0x20000
>> [  450.115172] pxa3xx-nand d00d0000.nand: Ready time out!!!
>> libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 1, offset 2048)
>>         error 5 (Input/output error)
>> Erasing failed write from 0x020000 to 0x03ffff
>> Writing data to block 2 at offset 0x40000
>> [  450.315171] pxa3xx-nand d00d0000.nand: Ready time out!!!
>> libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 2, offset 2048)
>>         error 5 (Input/output error)
>> Erasing failed write from 0x040000 to 0x05ffff
>> Writing data to block 3 at offset 0x60000
>> [  450.515171] pxa3xx-nand d00d0000.nand: Ready time out!!!
>> libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 3, offset 2048)
>>         error 5 (Input/output error)
>> Erasing failed write from 0x060000 to 0x07ffff
>> Writing data to block 4 at offset 0x80000
>> [  450.715169] pxa3xx-nand d00d0000.nand: Ready time out!!!
>> libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 4, offset 2048)
>>         error 5 (Input/output error)
>> Erasing failed write from 0x080000 to 0x09ffff
>> Writing data to block 5 at offset 0xa0000
>> [  450.915171] pxa3xx-nand d00d0000.nand: Ready time out!!!
>> libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 5, offset 2048)
>>         error 5 (Input/output error)
>> Erasing failed write from 0x0a0000 to 0x0bffff
>> Writing data to block 6 at offset 0xc0000
>> [  451.115171] pxa3xx-nand d00d0000.nand: Ready time out!!!
>> libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 6, offset 2048)
>>         error 5 (Input/output error)
>> Erasing failed write from 0x0c0000 to 0x0dffff
>> Writing data to block 7 at offset 0xe0000
>> 
>> 
>
> So, let me confirm this: you have systematically obtained a "Ready
> timeout" when writing to the device, on every single write to a page,
> correct?

yep, this is what I have on the RN2120.


> I'll prepare a patch against the branch we're working that adds lots of
> pr_info(). It'll be very annoying for you, but it's the only way I can
> think of, to get the driver's dirty inner sequence and to see *where*
> is failing.
>
> However, before that patch, please do a quick test for me. It's just a
> shot in the dark. The below diff removes the NDCB0_AUTO_RS flag from
> the PAGE_PROG command when the page size is 2048 byte (or less).
>
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index fd0ef93..038cf5d 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -831,7 +831,6 @@ static int prepare_set_command(struct pxa3xx_nand_info *info, int command,
>  			}
>  		} else {
>  			info->ndcb0 |= NDCB0_CMD_TYPE(0x1)
> -					| NDCB0_AUTO_RS
>  					| NDCB0_ST_ROW_EN
>  					| NDCB0_DBC
>  					| (NAND_CMD_PAGEPROG << 8)
>
> Please apply this diff, and then try to write at least one page. If at
> all possible you might try different approaches:
>
>   $ flash_erase /dev/mtd2 0 1
>   $ dd if=page.raw of=/dev/mtd2 bs=2048 count=1
>
>   $ flash_erase /dev/mtd2 0 1
>   $ nandwrite page.raw /dev/mtd2

That's very weird. With the patch applied (I did not redo the whole set
of tests below w/o the patch):

Let's erase the first block of /dev/mtd4

  root@thin:~# flash_erase /dev/mtd4 0 1
  Erasing 128 Kibyte @ 0 -- 100 % complete 
  
Create a 2048 byte file:

  root@thin:~# dd if=/dev/urandom of=/tmp/page.raw bs=2048 count=1
  1+0 records in
  1+0 records out
  2048 bytes (2.0 kB) copied, 0.00104352 s, 2.0 MB/s

dd that file to the first block

  root@thin:~# dd if=/tmp/page.raw of=/dev/mtd4 bs=2048 count=1
  1+0 records in
  1+0 records out
  2048 bytes (2.0 kB) copied, 0.0008344 s, 2.5 MB/s

Do the same with nandwrite:

  root@thin:~# flash_erase /dev/mtd4 0 1
  Erasing 128 Kibyte @ 0 -- 100 % complete 

  root@thin:~# nandwrite /dev/mtd4 /tmp/page.raw 
  Writing data to block 0 at offset 0x0

As it seems to work, let's now try with a complete image:
    
  root@thin:~# flash_erase /dev/mtd4 0 0
  Erasing 128 Kibyte @ 72e0000 -- 99 % complete flash_erase: Skipping bad block at 07300000
  flash_erase: Skipping bad block at 07320000
  flash_erase: Skipping bad block at 07340000
  flash_erase: Skipping bad block at 07360000
  flash_erase: Skipping bad block at 07380000
  flash_erase: Skipping bad block at 073a0000
  flash_erase: Skipping bad block at 073c0000
  flash_erase: Skipping bad block at 073e0000
  Erasing 128 Kibyte @ 73e0000 -- 100 % complete 

  root@thin:~# nandwrite /dev/mtd4 mtd4ro 
  Writing data to block 0 at offset 0x0
  [  421.406929] pxa3xx-nand d00d0000.nand: Ready time out!!!
  libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 0, offset 2048)
          error 5 (Input/output error)
  Erasing failed write from 00000000 to 0x01ffff
  Writing data to block 1 at offset 0x20000
  [  421.606928] pxa3xx-nand d00d0000.nand: Ready time out!!!
  libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 1, offset 2048)
          error 5 (Input/output error)
  Erasing failed write from 0x020000 to 0x03ffff
  Writing data to block 2 at offset 0x40000
  [  421.806928] pxa3xx-nand d00d0000.nand: Ready time out!!!
  libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 2, offset 2048)
          error 5 (Input/output error)
  Erasing failed write from 0x040000 to 0x05ffff
  Writing data to block 3 at offset 0x60000
  [  422.006928] pxa3xx-nand d00d0000.nand: Ready time out!!!
  libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 3, offset 2048)
          error 5 (Input/output error)
  Erasing failed write from 0x060000 to 0x07ffff
  Writing data to block 4 at offset 0x80000
  [  422.206928] pxa3xx-nand d00d0000.nand: Ready time out!!!
  libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 4, offset 2048)
          error 5 (Input/output error)
  Erasing failed write from 0x080000 to 0x09ffff
  Writing data to block 5 at offset 0xa0000
  ^C[  422.406926] pxa3xx-nand d00d0000.nand: Ready time out!!!

So let's try again with a single page:
  
  root@thin:~# flash_erase /dev/mtd4 0 1
  Erasing 128 Kibyte @ 0 -- 100 % complete 

  root@thin:~# nandwrite /dev/mtd4 /tmp/page.raw 
  Writing data to block 0 at offset 0x0
  
FWIW, the version of nandwrite I have on my armhf debian is the
following (no specific bug report related to that version of
mtd-utils):

  root@thin:~# nandwrite --version
  nandwrite 1.5.0
  
  Copyright (C) 2003 Thomas Gleixner 
  
  nandwrite comes with NO WARRANTY
  to the extent permitted by law.
  
  You may redistribute copies of nandwrite
  under the terms of the GNU General Public Licence.
  See the file `COPYING' for more information.

Ok, so let's see if the problem appears if we write more than 1 block:
  
  root@thin:/tmp# flash_erase /dev/mtd4 0 3
  Erasing 128 Kibyte @ 40000 -- 100 % complete 
  
  root@thin:/tmp# dd if=/dev/urandom of=/tmp/3-blocks.raw bs=128K count=3
  3+0 records in
  3+0 records out
  393216 bytes (393 kB) copied, 0.120255 s, 3.3 MB/s
  
  root@thin:/tmp# nandwrite /dev/mtd4 /tmp/3-blocks.raw 
  Writing data to block 0 at offset 0x0
  Writing data to block 1 at offset 0x20000
  Writing data to block 2 at offset 0x40000
  
Now, I am starting to wonder if nandwrite does not try and do something
smart depending on the content of the file. So let's try and replace
our "mtd4ro" file by a file of same size filled with random:
  
  root@thin:~# ls -l mtd4ro 
  -rw-r--r-- 1 arno root 121634816 Dec  3 20:30 mtd4ro
  
  root@thin:~# dd if=/dev/urandom of=/tmp/whole.raw bs=128K count=928
  928+0 records in
  928+0 records out
  121634816 bytes (122 MB) copied, 37.1333 s, 3.3 MB/s
  
  root@thin:~# flash_erase /dev/mtd4 0 1
  Erasing 128 Kibyte @ 0 -- 100 % complete 
  
  
  root@thin:~# nandwrite /dev/mtd4 /tmp/whole.raw 
  Writing data to block 0 at offset 0x0
  ...
  Writing data to block 916 at offset 0x7280000
  Writing data to block 917 at offset 0x72a0000
  Writing data to block 918 at offset 0x72c0000
  Writing data to block 919 at offset 0x72e0000
  Writing data to block 920 at offset 0x7300000
  Bad block at 7300000, 1 block(s) from 7300000 will be skipped
  Writing data to block 921 at offset 0x7320000
  Bad block at 7320000, 1 block(s) from 7320000 will be skipped
  Writing data to block 922 at offset 0x7340000
  Bad block at 7340000, 1 block(s) from 7340000 will be skipped
  Writing data to block 923 at offset 0x7360000
  Bad block at 7360000, 1 block(s) from 7360000 will be skipped
  Writing data to block 924 at offset 0x7380000
  Bad block at 7380000, 1 block(s) from 7380000 will be skipped
  Writing data to block 925 at offset 0x73a0000
  Bad block at 73a0000, 1 block(s) from 73a0000 will be skipped
  Writing data to block 926 at offset 0x73c0000
  Bad block at 73c0000, 1 block(s) from 73c0000 will be skipped
  Writing data to block 927 at offset 0x73e0000
  Bad block at 73e0000, 1 block(s) from 73e0000 will be skipped
  Writing data to block 928 at offset 0x7400000
  libmtd: error!: bad eraseblock number 928, mtd4 has 928 eraseblocks
  nandwrite: error!: /dev/mtd4: MTD get bad block failed
             error 22 (Invalid argument)
  nandwrite: error!: Data was only partially written due to error
             error 22 (Invalid argument)
  
  I don't believe it! It works as expected. As said previously, the
content of my "mtd4ro" is a simple a backup of the partition via
dd (a ubi fs):

  root@thin:~# hexdump -C mtd4ro | less
  00000000  55 42 49 23 01 00 00 00  00 00 00 00 00 00 00 02  |UBI#............|
  00000010  00 00 08 00 00 00 10 00  16 9a 76 a5 00 00 00 00  |..........v.....|
  00000020  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
  00000030  00 00 00 00 00 00 00 00  00 00 00 00 65 9c 26 3a  |............e.&:|
  00000040  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
  *
  000007f0  00 00 00 00 00 00 00 00  00 00 00 02 00 00 00 00  |................|
  00000800  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
  *
  00020000  55 42 49 23 01 00 00 00  00 00 00 00 00 00 00 02  |UBI#............|
  00020010  00 00 08 00 00 00 10 00  16 9a 76 a5 00 00 00 00  |..........v.....|
  00020020  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
  00020030  00 00 00 00 00 00 00 00  00 00 00 00 65 9c 26 3a  |............e.&:|
  00020040  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
  *
  000207f0  00 00 00 00 00 00 00 00  00 00 00 02 00 00 00 00  |................|
  00020800  55 42 49 21 01 01 00 00  00 00 00 00 00 00 00 00  |UBI!............|

Just to be sure, I did the test three times for each input file: mtd4ro,
whole.raw, mtd4ro, whole.raw, mtd4ro, whole.raw:

 - whole.raw: no error the three times
 - mtd4ro: errors each time

Let's now try with the first block only:

  root@thin:~# dd if=mtd4ro of=/tmp/oneblock bs=128K count=1
  1+0 records in
  1+0 records out
  131072 bytes (131 kB) copied, 0.00126152 s, 104 MB/s

  root@thin:~# flash_erase /dev/mtd4 0 1
  Erasing 128 Kibyte @ 0 -- 100 % complete 

  root@thin:~# nandwrite /dev/mtd4 /tmp/oneblock 
  Writing data to block 0 at offset 0x0
  [ 3860.346928] pxa3xx-nand d00d0000.nand: Ready time out!!!
  libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 0, offset 2048)
          error 5 (Input/output error)
  Erasing failed write from 00000000 to 0x01ffff
  Writing data to block 1 at offset 0x20000
  [ 3860.546928] pxa3xx-nand d00d0000.nand: Ready time out!!!
  libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 1, offset 2048)
          error 5 (Input/output error)
  
AFAICT, it seems someone in the process does not like the UBI fs dump I
provide: nandwrite, your driver, mtd base code or the chip.

I took a very quick look at nandwrite code and it has some options to do
smart stuff but they seem to be all disabled by default when no cmd line
option is passed.

Could you try and dump the block to another flash. As I don't think I can
attach a binary blob to this email, can you drop the following in a
python shell:

l = ["55 42 49 23 01 00 00 00  00 00 00 00 00 00 00 02",
     "00 00 08 00 00 00 10 00  16 9a 76 a5 00 00 00 00",
     "00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00",
     "00 00 00 00 00 00 00 00  00 00 00 00 65 9c 26 3a",
     "00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00"*0x7b,
     "00 00 00 00 00 00 00 00  00 00 00 02 00 00 00 00",
     "ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff",
     "ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff"*0x1f7f]

open("/tmp/block.raw", "w").write("".join(l).replace(' ', '').decode("hex"))

FWIW, you should have the following sha256 for the file:
d3e5534e054f2ab1220194c434d90fbfe9580459de887016db8e308739aa3a7b

If you have any additional idea, I am interested. I have no more time
this evening to test the other debug patch you sent but I will
definitely give it a try tomorrow.

If I did something stupid in the process, do not hesitate!

Cheers,

a+

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Arnaud Ebalard]

Hi Ezequiel,

arno@natisbad.org (Arnaud Ebalard) writes:

> Could you try and dump the block to another flash. As I don't think I can
> attach a binary blob to this email, can you drop the following in a
> python shell:
>
> l = ["55 42 49 23 01 00 00 00  00 00 00 00 00 00 00 02",
>      "00 00 08 00 00 00 10 00  16 9a 76 a5 00 00 00 00",
>      "00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00",
>      "00 00 00 00 00 00 00 00  00 00 00 00 65 9c 26 3a",
>      "00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00"*0x7b,
>      "00 00 00 00 00 00 00 00  00 00 00 02 00 00 00 00",
>      "ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff",
>      "ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff"*0x1f7f]
>
> open("/tmp/block.raw", "w").write("".join(l).replace(' ', '').decode("hex"))
>
> FWIW, you should have the following sha256 for the file:
> d3e5534e054f2ab1220194c434d90fbfe9580459de887016db8e308739aa3a7b
>
> If you have any additional idea, I am interested. I have no more time
> this evening to test the other debug patch you sent but I will
> definitely give it a try tomorrow.

I reused the block above to test on the kernel w/ your debug patch
applied:

root@thin:~# python
Python 2.7.6 (default, Nov 19 2013, 18:53:23) 
[GCC 4.8.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
l = ["55 42 49 23 01 00 00 00  00 00 00 00 00 00 00 02",
     "00 00 08 00 00 00 10 00  16 9a 76 a5 00 00 00 00",
     "00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00",
     "00 00 00 00 00 00 00 00  00 00 00 00 65 9c 26 3a",
     "00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00"*0x7b,
     "00 00 00 00 00 00 00 00  00 00 00 02 00 00 00 00",
     "ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff",
     "ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff"*0x1f7f]

>>> l = ["55 42 49 23 01 00 00 00  00 00 00 00 00 00 00 02",
...      "00 00 08 00 00 00 10 00  16 9a 76 a5 00 00 00 00",
...      "00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00",
...      "00 00 00 00 00 00 00 00  00 00 00 00 65 9c 26 3a",
...      "00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00"*0x7b,
...      "00 00 00 00 00 00 00 00  00 00 00 02 00 00 00 00",
...      "ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff",
...      "ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff"*0x1f7f]
>>>
>>> open("/tmp/block.raw", "w").write("".join(l).replace(' ', '').decode("hex")) root@thin:~#

root@thin:/sys/kernel/debug/dynamic_debug# flash_erase /dev/mtd4 0 1
Erasing 128 Kibyte @ 0 -- 100 % complete 

root@thin:/sys/kernel/debug/dynamic_debug# nandwrite /dev/mtd4 /tmp/block.raw 
Writing data to block 0 at offset 0x0
[  784.543390] pxa3xx-nand d00d0000.nand: Ready time out!!!
libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 0, offset 2048)
        error 5 (Input/output error)
Erasing failed write from 00000000 to 0x01ffff
Writing data to block 1 at offset 0x20000
[  784.743391] pxa3xx-nand d00d0000.nand: Ready time out!!!
libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 1, offset 2048)
        error 5 (Input/output error)
Erasing failed write from 0x020000 to 0x03ffff
Writing data to block 2 at offset 0x40000
[  784.943390] pxa3xx-nand d00d0000.nand: Ready time out!!!
libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 2, offset 2048)
        error 5 (Input/output error)
Erasing failed write from 0x040000 to 0x05ffff
Writing data to block 3 at offset 0x60000
[  785.143391] pxa3xx-nand d00d0000.nand: Ready time out!!!
libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 3, offset 2048)
        error 5 (Input/output error)
Erasing failed write from 0x060000 to 0x07ffff
Writing data to block 4 at offset 0x80000
[  785.343388] pxa3xx-nand d00d0000.nand: Ready time out!!!
libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 4, offset 2048)
        error 5 (Input/output error)
Erasing failed write from 0x080000 to 0x09ffff
Writing data to block 5 at offset 0xa0000
[  785.543390] pxa3xx-nand d00d0000.nand: Ready time out!!!
libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 5, offset 2048)
        error 5 (Input/output error)
Erasing failed write from 0x0a0000 to 0x0bffff
Writing data to block 6 at offset 0xc0000
[  785.743390] pxa3xx-nand d00d0000.nand: Ready time out!!!
libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 6, offset 2048)
        error 5 (Input/output error)
Erasing failed write from 0x0c0000 to 0x0dffff
Writing data to block 7 at offset 0xe0000

And here is what I get in kernel logs:

[  766.262816] Command0 0x810070
[  766.262827] Command1 0x0
[  766.262833] Command2 0x0
[  766.262838] Command3 0x0
[  766.262853] Command0 0x24bd060
[  766.262858] Command1 0x1800
[  766.262863] Command2 0x0
[  766.262868] Command3 0x0
[  766.263617] Status: ready
[  766.263937] Waiting for device to be ready
[  766.263945] Device is ready
[  784.345577] Command0 0x810070
[  784.345588] Command1 0x0
[  784.345594] Command2 0x0
[  784.345599] Command3 0x0
[  784.345626] Command0 0x62c1080
[  784.345632] Command1 0x18000000
[  784.345637] Command2 0x0
[  784.345642] Command3 0x0
[  784.345960] Status: ready
[  784.345976] Waiting for device to be ready
[  784.345981] Device is ready
[  784.346006] Command0 0x810070
[  784.346012] Command1 0x0
[  784.346017] Command2 0x0
[  784.346022] Command3 0x0
[  784.346044] Waiting for device to be ready
[  784.543390] pxa3xx-nand d00d0000.nand: Ready time out!!!
[  784.548850] Command0 0x810070
[  784.548857] Command1 0x0
[  784.548863] Command2 0x0
[  784.548868] Command3 0x0
[  784.548888] Command0 0x24bd060
[  784.548893] Command1 0x1800
[  784.548898] Command2 0x0
[  784.548903] Command3 0x0
[  784.549648] Status: ready
[  784.549661] Waiting for device to be ready
[  784.549667] Device is ready
[  784.549701] Command0 0x810070
[  784.549707] Command1 0x0
[  784.549712] Command2 0x0
[  784.549716] Command3 0x0
[  784.549736] Command0 0x62c1080
[  784.549742] Command1 0x18400000
[  784.549747] Command2 0x0
[  784.549751] Command3 0x0
[  784.550074] Status: ready
[  784.550087] Waiting for device to be ready
[  784.550093] Device is ready
[  784.550109] Command0 0x810070
[  784.550115] Command1 0x0
[  784.550119] Command2 0x0
[  784.550124] Command3 0x0
[  784.550146] Waiting for device to be ready
[  784.743391] pxa3xx-nand d00d0000.nand: Ready time out!!!
[  784.748780] Command0 0x810070
[  784.748786] Command1 0x0
[  784.748792] Command2 0x0
[  784.748797] Command3 0x0
[  784.748815] Command0 0x24bd060
[  784.748821] Command1 0x1840
[  784.748826] Command2 0x0
[  784.748831] Command3 0x0
[  784.749575] Status: ready
[  784.749589] Waiting for device to be ready
[  784.749595] Device is ready
[  784.749628] Command0 0x810070
[  784.749634] Command1 0x0
[  784.749639] Command2 0x0
[  784.749644] Command3 0x0
[  784.749663] Command0 0x62c1080
[  784.749669] Command1 0x18800000
[  784.749674] Command2 0x0
[  784.749679] Command3 0x0
[  784.749995] Status: ready
[  784.750007] Waiting for device to be ready
[  784.750013] Device is ready
[  784.750028] Command0 0x810070


So let's now do the same with a fully random block:

root@thin:/# dd if=/dev/urandom of=/tmp/block.random bs=131072 count=1
1+0 records in
1+0 records out
131072 bytes (131 kB) copied, 0.0406636 s, 3.2 MB/s

root@thin:~# flash_erase /dev/mtd4 0 1
Erasing 128 Kibyte @ 0 -- 100 % complete 

root@thin:~# nandwrite /dev/mtd4 /tmp/block.random 
Writing data to block 0 at offset 0x0

In the log, I get the following:

[ 1422.296953] Command0 0x810070
[ 1422.296965] Command1 0x0
[ 1422.296970] Command2 0x0
[ 1422.296975] Command3 0x0
[ 1422.296990] Command0 0x24bd060
[ 1422.296996] Command1 0x1800
[ 1422.297001] Command2 0x0
[ 1422.297006] Command3 0x0
[ 1422.297752] Status: ready
[ 1422.297767] Waiting for device to be ready
[ 1422.297773] Device is ready
[ 1488.147298] Command0 0x810070
[ 1488.147310] Command1 0x0
[ 1488.147316] Command2 0x0
[ 1488.147321] Command3 0x0
[ 1488.147340] Command0 0x62c1080
[ 1488.147345] Command1 0x18000000
[ 1488.147350] Command2 0x0
[ 1488.147355] Command3 0x0
[ 1488.147673] Status: ready
[ 1488.147689] Waiting for device to be ready
[ 1488.147695] Device is ready
[ 1488.147720] Command0 0x810070
[ 1488.147725] Command1 0x0
[ 1488.147730] Command2 0x0
[ 1488.147735] Command3 0x0
[ 1488.147754] Command0 0x62c1080
[ 1488.147759] Command1 0x18010000
[ 1488.147764] Command2 0x0
[ 1488.147769] Command3 0x0
[ 1488.148085] Status: ready
[ 1488.148098] Waiting for device to be ready
[ 1488.148103] Device is ready
[ 1488.148125] Command0 0x810070
[ 1488.148130] Command1 0x0
[ 1488.148135] Command2 0x0
[ 1488.148140] Command3 0x0
[ 1488.148158] Command0 0x62c1080
[ 1488.148164] Command1 0x18020000
[ 1488.148169] Command2 0x0
[ 1488.148174] Command3 0x0
[ 1488.148490] Status: ready
[ 1488.148503] Waiting for device to be ready
[ 1488.148508] Device is ready
[ 1488.148537] Command0 0x810070
[ 1488.148543] Command1 0x0
[ 1488.148548] Command2 0x0
[ 1488.148553] Command3 0x0
[ 1488.148567] Command0 0x62c1080
[ 1488.148572] Command1 0x18030000
[ 1488.148577] Command2 0x0
[ 1488.148582] Command3 0x0
[ 1488.148955] Status: ready
[ 1488.148968] Waiting for device to be ready
[ 1488.148973] Device is ready
[ 1488.148995] Command0 0x810070
[ 1488.149001] Command1 0x0
[ 1488.149006] Command2 0x0
[ 1488.149011] Command3 0x0
[ 1488.149029] Command0 0x62c1080
[ 1488.149035] Command1 0x18040000
[ 1488.149039] Command2 0x0
[ 1488.149044] Command3 0x0
[ 1488.149417] Status: ready
[ 1488.149429] Waiting for device to be ready
[ 1488.149434] Device is ready
[ 1488.149456] Command0 0x810070
[ 1488.149462] Command1 0x0
[ 1488.149466] Command2 0x0
[ 1488.149471] Command3 0x0
[ 1488.149486] Command0 0x62c1080
[ 1488.149491] Command1 0x18050000
[ 1488.149496] Command2 0x0
[ 1488.149501] Command3 0x0
[ 1488.149817] Status: ready
[ 1488.149830] Waiting for device to be ready
[ 1488.149835] Device is ready
[ 1488.149857] Command0 0x810070
[ 1488.149863] Command1 0x0
[ 1488.149867] Command2 0x0
[ 1488.149872] Command3 0x0
[ 1488.149891] Command0 0x62c1080
[ 1488.149896] Command1 0x18060000
[ 1488.149901] Command2 0x0
[ 1488.149906] Command3 0x0
[ 1488.150279] Status: ready
[ 1488.150292] Waiting for device to be ready
[ 1488.150297] Device is ready
[ 1488.150319] Command0 0x810070
[ 1488.150325] Command1 0x0
[ 1488.150330] Command2 0x0
[ 1488.150334] Command3 0x0
[ 1488.150353] Command0 0x62c1080
[ 1488.150359] Command1 0x18070000
[ 1488.150363] Command2 0x0
[ 1488.150368] Command3 0x0
[ 1488.150685] Status: ready
[ 1488.150697] Waiting for device to be ready
[ 1488.150702] Device is ready

If you had the time to test the offending block on another device, did
you manage to reproduce what I get?

Cheers,

a+

Re: [PATCH v5 00/14] Armada 370/XP NAND supportg

[Ezequiel Garcia]

On Thu, Dec 05, 2013 at 09:42:25PM +0100, Arnaud Ebalard wrote:
> Hi Ezequiel,
> 
> arno@natisbad.org (Arnaud Ebalard) writes:
> 
> > Could you try and dump the block to another flash. As I don't think I can
> > attach a binary blob to this email, can you drop the following in a
> > python shell:
> >
> > l = ["55 42 49 23 01 00 00 00  00 00 00 00 00 00 00 02",
> >      "00 00 08 00 00 00 10 00  16 9a 76 a5 00 00 00 00",
> >      "00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00",
> >      "00 00 00 00 00 00 00 00  00 00 00 00 65 9c 26 3a",
> >      "00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00"*0x7b,
> >      "00 00 00 00 00 00 00 00  00 00 00 02 00 00 00 00",
> >      "ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff",
> >      "ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff"*0x1f7f]
> >
> > open("/tmp/block.raw", "w").write("".join(l).replace(' ', '').decode("hex"))
> >
> > FWIW, you should have the following sha256 for the file:
> > d3e5534e054f2ab1220194c434d90fbfe9580459de887016db8e308739aa3a7b
> >
> > If you have any additional idea, I am interested. I have no more time
> > this evening to test the other debug patch you sent but I will
> > definitely give it a try tomorrow.
> 
> I reused the block above to test on the kernel w/ your debug patch
> applied:
> 
> root@thin:~# python
> Python 2.7.6 (default, Nov 19 2013, 18:53:23) 
> [GCC 4.8.2] on linux2
> Type "help", "copyright", "credits" or "license" for more information.
> l = ["55 42 49 23 01 00 00 00  00 00 00 00 00 00 00 02",
>      "00 00 08 00 00 00 10 00  16 9a 76 a5 00 00 00 00",
>      "00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00",
>      "00 00 00 00 00 00 00 00  00 00 00 00 65 9c 26 3a",
>      "00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00"*0x7b,
>      "00 00 00 00 00 00 00 00  00 00 00 02 00 00 00 00",
>      "ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff",
>      "ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff"*0x1f7f]
> 
> >>> l = ["55 42 49 23 01 00 00 00  00 00 00 00 00 00 00 02",
> ...      "00 00 08 00 00 00 10 00  16 9a 76 a5 00 00 00 00",
> ...      "00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00",
> ...      "00 00 00 00 00 00 00 00  00 00 00 00 65 9c 26 3a",
> ...      "00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00"*0x7b,
> ...      "00 00 00 00 00 00 00 00  00 00 00 02 00 00 00 00",
> ...      "ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff",
> ...      "ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff"*0x1f7f]
> >>>
> >>> open("/tmp/block.raw", "w").write("".join(l).replace(' ', '').decode("hex")) root@thin:~#
> 
> root@thin:/sys/kernel/debug/dynamic_debug# flash_erase /dev/mtd4 0 1
> Erasing 128 Kibyte @ 0 -- 100 % complete 
> 
> root@thin:/sys/kernel/debug/dynamic_debug# nandwrite /dev/mtd4 /tmp/block.raw 
> Writing data to block 0 at offset 0x0
> [  784.543390] pxa3xx-nand d00d0000.nand: Ready time out!!!
> libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 0, offset 2048)
>         error 5 (Input/output error)
> Erasing failed write from 00000000 to 0x01ffff
> Writing data to block 1 at offset 0x20000
> [  784.743391] pxa3xx-nand d00d0000.nand: Ready time out!!!
> libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 1, offset 2048)
>         error 5 (Input/output error)
> Erasing failed write from 0x020000 to 0x03ffff
> Writing data to block 2 at offset 0x40000
> [  784.943390] pxa3xx-nand d00d0000.nand: Ready time out!!!
> libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 2, offset 2048)
>         error 5 (Input/output error)
> Erasing failed write from 0x040000 to 0x05ffff
> Writing data to block 3 at offset 0x60000
> [  785.143391] pxa3xx-nand d00d0000.nand: Ready time out!!!
> libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 3, offset 2048)
>         error 5 (Input/output error)
> Erasing failed write from 0x060000 to 0x07ffff
> Writing data to block 4 at offset 0x80000
> [  785.343388] pxa3xx-nand d00d0000.nand: Ready time out!!!
> libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 4, offset 2048)
>         error 5 (Input/output error)
> Erasing failed write from 0x080000 to 0x09ffff
> Writing data to block 5 at offset 0xa0000
> [  785.543390] pxa3xx-nand d00d0000.nand: Ready time out!!!
> libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 5, offset 2048)
>         error 5 (Input/output error)
> Erasing failed write from 0x0a0000 to 0x0bffff
> Writing data to block 6 at offset 0xc0000
> [  785.743390] pxa3xx-nand d00d0000.nand: Ready time out!!!
> libmtd: error!: cannot write 2048 bytes to mtd4 (eraseblock 6, offset 2048)
>         error 5 (Input/output error)
> Erasing failed write from 0x0c0000 to 0x0dffff
> Writing data to block 7 at offset 0xe0000
> 
> And here is what I get in kernel logs:
> 
> [  766.262816] Command0 0x810070
> [  766.262827] Command1 0x0
> [  766.262833] Command2 0x0
> [  766.262838] Command3 0x0
> [  766.262853] Command0 0x24bd060
> [  766.262858] Command1 0x1800
> [  766.262863] Command2 0x0
> [  766.262868] Command3 0x0
> [  766.263617] Status: ready
> [  766.263937] Waiting for device to be ready
> [  766.263945] Device is ready
> [  784.345577] Command0 0x810070
> [  784.345588] Command1 0x0
> [  784.345594] Command2 0x0
> [  784.345599] Command3 0x0
> [  784.345626] Command0 0x62c1080
> [  784.345632] Command1 0x18000000
> [  784.345637] Command2 0x0
> [  784.345642] Command3 0x0
> [  784.345960] Status: ready
> [  784.345976] Waiting for device to be ready
> [  784.345981] Device is ready
> [  784.346006] Command0 0x810070
> [  784.346012] Command1 0x0
> [  784.346017] Command2 0x0
> [  784.346022] Command3 0x0
> [  784.346044] Waiting for device to be ready
> [  784.543390] pxa3xx-nand d00d0000.nand: Ready time out!!!
> [  784.548850] Command0 0x810070
> [  784.548857] Command1 0x0
> [  784.548863] Command2 0x0
> [  784.548868] Command3 0x0
> [  784.548888] Command0 0x24bd060
> [  784.548893] Command1 0x1800
> [  784.548898] Command2 0x0
> [  784.548903] Command3 0x0
> [  784.549648] Status: ready
> [  784.549661] Waiting for device to be ready
> [  784.549667] Device is ready
> [  784.549701] Command0 0x810070
> [  784.549707] Command1 0x0
> [  784.549712] Command2 0x0
> [  784.549716] Command3 0x0
> [  784.549736] Command0 0x62c1080
> [  784.549742] Command1 0x18400000
> [  784.549747] Command2 0x0
> [  784.549751] Command3 0x0
> [  784.550074] Status: ready
> [  784.550087] Waiting for device to be ready
> [  784.550093] Device is ready
> [  784.550109] Command0 0x810070
> [  784.550115] Command1 0x0
> [  784.550119] Command2 0x0
> [  784.550124] Command3 0x0
> [  784.550146] Waiting for device to be ready
> [  784.743391] pxa3xx-nand d00d0000.nand: Ready time out!!!
> [  784.748780] Command0 0x810070
> [  784.748786] Command1 0x0
> [  784.748792] Command2 0x0
> [  784.748797] Command3 0x0
> [  784.748815] Command0 0x24bd060
> [  784.748821] Command1 0x1840
> [  784.748826] Command2 0x0
> [  784.748831] Command3 0x0
> [  784.749575] Status: ready
> [  784.749589] Waiting for device to be ready
> [  784.749595] Device is ready
> [  784.749628] Command0 0x810070
> [  784.749634] Command1 0x0
> [  784.749639] Command2 0x0
> [  784.749644] Command3 0x0
> [  784.749663] Command0 0x62c1080
> [  784.749669] Command1 0x18800000
> [  784.749674] Command2 0x0
> [  784.749679] Command3 0x0
> [  784.749995] Status: ready
> [  784.750007] Waiting for device to be ready
> [  784.750013] Device is ready
> [  784.750028] Command0 0x810070
> 
> 
> So let's now do the same with a fully random block:
> 
> root@thin:/# dd if=/dev/urandom of=/tmp/block.random bs=131072 count=1
> 1+0 records in
> 1+0 records out
> 131072 bytes (131 kB) copied, 0.0406636 s, 3.2 MB/s
> 
> root@thin:~# flash_erase /dev/mtd4 0 1
> Erasing 128 Kibyte @ 0 -- 100 % complete 
> 
> root@thin:~# nandwrite /dev/mtd4 /tmp/block.random 
> Writing data to block 0 at offset 0x0
> 
> In the log, I get the following:
> 
> [ 1422.296953] Command0 0x810070
> [ 1422.296965] Command1 0x0
> [ 1422.296970] Command2 0x0
> [ 1422.296975] Command3 0x0
> [ 1422.296990] Command0 0x24bd060
> [ 1422.296996] Command1 0x1800
> [ 1422.297001] Command2 0x0
> [ 1422.297006] Command3 0x0
> [ 1422.297752] Status: ready
> [ 1422.297767] Waiting for device to be ready
> [ 1422.297773] Device is ready
> [ 1488.147298] Command0 0x810070
> [ 1488.147310] Command1 0x0
> [ 1488.147316] Command2 0x0
> [ 1488.147321] Command3 0x0
> [ 1488.147340] Command0 0x62c1080
> [ 1488.147345] Command1 0x18000000
> [ 1488.147350] Command2 0x0
> [ 1488.147355] Command3 0x0
> [ 1488.147673] Status: ready
> [ 1488.147689] Waiting for device to be ready
> [ 1488.147695] Device is ready
> [ 1488.147720] Command0 0x810070
> [ 1488.147725] Command1 0x0
> [ 1488.147730] Command2 0x0
> [ 1488.147735] Command3 0x0
> [ 1488.147754] Command0 0x62c1080
> [ 1488.147759] Command1 0x18010000
> [ 1488.147764] Command2 0x0
> [ 1488.147769] Command3 0x0
> [ 1488.148085] Status: ready
> [ 1488.148098] Waiting for device to be ready
> [ 1488.148103] Device is ready
> [ 1488.148125] Command0 0x810070
> [ 1488.148130] Command1 0x0
> [ 1488.148135] Command2 0x0
> [ 1488.148140] Command3 0x0
> [ 1488.148158] Command0 0x62c1080
> [ 1488.148164] Command1 0x18020000
> [ 1488.148169] Command2 0x0
> [ 1488.148174] Command3 0x0
> [ 1488.148490] Status: ready
> [ 1488.148503] Waiting for device to be ready
> [ 1488.148508] Device is ready
> [ 1488.148537] Command0 0x810070
> [ 1488.148543] Command1 0x0
> [ 1488.148548] Command2 0x0
> [ 1488.148553] Command3 0x0
> [ 1488.148567] Command0 0x62c1080
> [ 1488.148572] Command1 0x18030000
> [ 1488.148577] Command2 0x0
> [ 1488.148582] Command3 0x0
> [ 1488.148955] Status: ready
> [ 1488.148968] Waiting for device to be ready
> [ 1488.148973] Device is ready
> [ 1488.148995] Command0 0x810070
> [ 1488.149001] Command1 0x0
> [ 1488.149006] Command2 0x0
> [ 1488.149011] Command3 0x0
> [ 1488.149029] Command0 0x62c1080
> [ 1488.149035] Command1 0x18040000
> [ 1488.149039] Command2 0x0
> [ 1488.149044] Command3 0x0
> [ 1488.149417] Status: ready
> [ 1488.149429] Waiting for device to be ready
> [ 1488.149434] Device is ready
> [ 1488.149456] Command0 0x810070
> [ 1488.149462] Command1 0x0
> [ 1488.149466] Command2 0x0
> [ 1488.149471] Command3 0x0
> [ 1488.149486] Command0 0x62c1080
> [ 1488.149491] Command1 0x18050000
> [ 1488.149496] Command2 0x0
> [ 1488.149501] Command3 0x0
> [ 1488.149817] Status: ready
> [ 1488.149830] Waiting for device to be ready
> [ 1488.149835] Device is ready
> [ 1488.149857] Command0 0x810070
> [ 1488.149863] Command1 0x0
> [ 1488.149867] Command2 0x0
> [ 1488.149872] Command3 0x0
> [ 1488.149891] Command0 0x62c1080
> [ 1488.149896] Command1 0x18060000
> [ 1488.149901] Command2 0x0
> [ 1488.149906] Command3 0x0
> [ 1488.150279] Status: ready
> [ 1488.150292] Waiting for device to be ready
> [ 1488.150297] Device is ready
> [ 1488.150319] Command0 0x810070
> [ 1488.150325] Command1 0x0
> [ 1488.150330] Command2 0x0
> [ 1488.150334] Command3 0x0
> [ 1488.150353] Command0 0x62c1080
> [ 1488.150359] Command1 0x18070000
> [ 1488.150363] Command2 0x0
> [ 1488.150368] Command3 0x0
> [ 1488.150685] Status: ready
> [ 1488.150697] Waiting for device to be ready
> [ 1488.150702] Device is ready
> 
> If you had the time to test the offending block on another device, did
> you manage to reproduce what I get?
> 

Thanks for the command trace! I'll look at this in detail tomorrow
and let you know.

-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Ezequiel Garcia]

Hi Arnaud,

On Thu, Dec 05, 2013 at 09:42:25PM +0100, Arnaud Ebalard wrote:
[..]
> [ 1488.150697] Waiting for device to be ready
> [ 1488.150702] Device is ready
> 
> If you had the time to test the offending block on another device, did
> you manage to reproduce what I get?
> 

Yes, I managed to reproduced your timeout. You found a bug on my
dev_ready completion handler scheme, and now I officially hate this
evil completion handler ;-)

I've pushed a new branch for you to try:

https://github.com/MISL-EBU-System-SW/mainline-public/tree/pxa3xx-armada-nand-new-ecc-v2

Please give it a try, and test as heavily and creatively as you can think off.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Arnaud Ebalard]

Hi Ezequiel,

Ezequiel Garcia <ezequiel.garcia@free-electrons.com> writes:

> Hi Arnaud,
>
> On Thu, Dec 05, 2013 at 09:42:25PM +0100, Arnaud Ebalard wrote:
> [..]
>> [ 1488.150697] Waiting for device to be ready
>> [ 1488.150702] Device is ready
>> 
>> If you had the time to test the offending block on another device, did
>> you manage to reproduce what I get?
>> 
>
> Yes, I managed to reproduced your timeout. You found a bug on my
> dev_ready completion handler scheme, and now I officially hate this
> evil completion handler ;-)
>
> I've pushed a new branch for you to try:
>
> https://github.com/MISL-EBU-System-SW/mainline-public/tree/pxa3xx-armada-nand-new-ecc-v2
>
> Please give it a try, and test as heavily and creatively as you can
> think off.

Sorry, but I was unable to find any more bug, i.e. AFAICT it does work
as expected now :-) FWIW, thanks for your time, you can put my:

Tested-by: Arnaud Ebalard <arno@natisbad.org>

Cheers,

a+

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Ezequiel Garcia]

On Fri, Dec 06, 2013 at 10:41:32PM +0100, Arnaud Ebalard wrote:
> Hi Ezequiel,
> 
> Ezequiel Garcia <ezequiel.garcia@free-electrons.com> writes:
> 
> > Hi Arnaud,
> >
> > On Thu, Dec 05, 2013 at 09:42:25PM +0100, Arnaud Ebalard wrote:
> > [..]
> >> [ 1488.150697] Waiting for device to be ready
> >> [ 1488.150702] Device is ready
> >> 
> >> If you had the time to test the offending block on another device, did
> >> you manage to reproduce what I get?
> >> 
> >
> > Yes, I managed to reproduced your timeout. You found a bug on my
> > dev_ready completion handler scheme, and now I officially hate this
> > evil completion handler ;-)
> >
> > I've pushed a new branch for you to try:
> >
> > https://github.com/MISL-EBU-System-SW/mainline-public/tree/pxa3xx-armada-nand-new-ecc-v2
> >
> > Please give it a try, and test as heavily and creatively as you can
> > think off.
> 
> Sorry, but I was unable to find any more bug, i.e. AFAICT it does work
> as expected now :-) FWIW, thanks for your time, you can put my:
> 
> Tested-by: Arnaud Ebalard <arno@natisbad.org>
> 

Great! I'll push the patches then with your tested-by.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Brian Norris]

On Wed, Dec 04, 2013 at 09:48:04PM +0100, Arnaud Ebalard wrote:
> Now, I am starting to wonder if nandwrite does not try and do something
> smart depending on the content of the file.

nandwrite does not do any data-dependent operations. It only checks the
size of the file to verify against MTD alignment restrictions, but
otherwise, it is simply a dump page-at-a-time programmer.

However, there are other characteristics which do differentiate it from
dd, so it's not surprising if nandwrite can uncover different code
behaviors than dd.

(BTW, I wouldn't recommend dd for NAND programming. It may be useful for
certain debugging purposes, but it ignore bad blocks and is generally
dangerous to use for any regular purposes.)

Brian

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Brian Norris]

On Mon, Dec 02, 2013 at 09:22:26PM -0300, Ezequiel Garcia wrote:
> Now, why does NAND reserve eight blocks, if there are only two tables?
> Well, you'll be able to find this in the driver:
> 
> static struct nand_bbt_descr bbt_main_descr = {
> 	/* stuff */
> 	.maxblocks = 8,         /* Last 8 blocks in each chip */
> };
> 
> The snippet above asks the NAND core to scan the last 8 blocks when searching
> for the in-flash bad block table. The NAND core will also reserve these
> 8 blocks as the maximum amount of blocks that can be used to store a bad
> block table (I guess that's in case one block gets 'really' bad).

That doesn't reflect mainline, where you'll see:

static struct nand_bbt_descr bbt_main_descr = {
...
        .maxblocks = NAND_BBT_SCAN_MAXBLOCKS,
...
};

Where NAND_BBT_SCAN_MAXBLOCKS == 4.

Do you have local modifications that make .maxblocks = 8?

Brian

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Ezequiel Garcia]

On Thu, Dec 05, 2013 at 01:23:33PM -0800, Brian Norris wrote:
> On Mon, Dec 02, 2013 at 09:22:26PM -0300, Ezequiel Garcia wrote:
> > Now, why does NAND reserve eight blocks, if there are only two tables?
> > Well, you'll be able to find this in the driver:
> > 
> > static struct nand_bbt_descr bbt_main_descr = {
> > 	/* stuff */
> > 	.maxblocks = 8,         /* Last 8 blocks in each chip */
> > };
> > 
> > The snippet above asks the NAND core to scan the last 8 blocks when searching
> > for the in-flash bad block table. The NAND core will also reserve these
> > 8 blocks as the maximum amount of blocks that can be used to store a bad
> > block table (I guess that's in case one block gets 'really' bad).
> 
> That doesn't reflect mainline, where you'll see:
> 

I wasn't mentioning nand_bbt.c but rather the pxa3xx-nand custom
nand_bbt_descr.

$ grep "maxblocks =" drivers/mtd/nand/pxa3xx_nand.c 
 	.maxblocks = 8,		/* Last 8 blocks in each chip */
	.maxblocks = 8,		/* Last 8 blocks in each chip */

> static struct nand_bbt_descr bbt_main_descr = {
> ...
>         .maxblocks = NAND_BBT_SCAN_MAXBLOCKS,
> ...
> };
> 
> Where NAND_BBT_SCAN_MAXBLOCKS == 4.
> 

Do you think using 8 is too much? (I'd agree at changing it)
Does it break anything to lower it to 4?
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH v5 00/14] Armada 370/XP NAND support

[Brian Norris]

On Thu, Dec 05, 2013 at 07:23:53PM -0300, Ezequiel Garcia wrote:
> On Thu, Dec 05, 2013 at 01:23:33PM -0800, Brian Norris wrote:
> > On Mon, Dec 02, 2013 at 09:22:26PM -0300, Ezequiel Garcia wrote:
> > > Now, why does NAND reserve eight blocks, if there are only two tables?
> > > Well, you'll be able to find this in the driver:
> > > 
> > > static struct nand_bbt_descr bbt_main_descr = {
> > > 	/* stuff */
> > > 	.maxblocks = 8,         /* Last 8 blocks in each chip */
> > > };
> > > 
> > > The snippet above asks the NAND core to scan the last 8 blocks when searching
> > > for the in-flash bad block table. The NAND core will also reserve these
> > > 8 blocks as the maximum amount of blocks that can be used to store a bad
> > > block table (I guess that's in case one block gets 'really' bad).
> > 
> > That doesn't reflect mainline, where you'll see:
> > 
> 
> I wasn't mentioning nand_bbt.c but rather the pxa3xx-nand custom
> nand_bbt_descr.
> 
> $ grep "maxblocks =" drivers/mtd/nand/pxa3xx_nand.c 
>  	.maxblocks = 8,		/* Last 8 blocks in each chip */
> 	.maxblocks = 8,		/* Last 8 blocks in each chip */

Ah, I forgot about that.

> > static struct nand_bbt_descr bbt_main_descr = {
> > ...
> >         .maxblocks = NAND_BBT_SCAN_MAXBLOCKS,
> > ...
> > };
> > 
> > Where NAND_BBT_SCAN_MAXBLOCKS == 4.
> > 
> 
> Do you think using 8 is too much? (I'd agree at changing it)
> Does it break anything to lower it to 4?

I think it's primarily a concern if you have too many consecutive bad
blocks at the end of your flash (factory, or from wear). On a pristine
flash, the table will only ever be in the last 2 blocks, and so 4 blocks
is a reasonable guess at the maximum needed, leaving room for two blocks
to go bad.

So, you'd only have a problem if 3 or more of the last 4 blocks go bad.

Brian