[PATCH v4 0/5] mtd: pxa3xx_nand: rework the timing setup

[PATCH v4 0/5] mtd: pxa3xx_nand: rework the timing setup

[Antoine Tenart]

Hello,

Let's try one more time to get this merged.

This series was part of a bigger one[1], which was split into smaller
ones as asked by Ezequiel[2]. When we take this into account this is
v9; with a v1 sent in January, 9 months ago.

Another series, introducing the Berlin nand support, depends on this.

The aim here is to use the nand framework to setup the timings,
while keeping the old in-driver way of configuration timings for
compatibility reasons.

You can find the series at:
https://github.com/atenart/linux.git berlin/4.3/nand-timings

Antoine

Since v3:
        - Moved back a chunk into the right patch.
        - Called pxa3xx_nand_config_flash() before KEEP_CONFIG.
        - Rebased on top of l2-mtd.git master.

Since v2:
        - Added back the support for keep-config 16 bits devices
        - Fixed wrong unit in a calculation
        - Reworked the pxa3xx_nand_init_timings() logic
        - Allowed compile test the pxa3xx driver

Since v1:
        - Rebased on top of v4.2-rc1

Since the series was split up:
        - Reworked the ndcr setup
        - Removed the read_id_bytes update after device detection

Antoine Tenart (5):
  mtd: pxa3xx: prepare allowing compile test
  mtd: nand: allow compile test of MTD_NAND_PXA3xx
  mtd: pxa3xx_nand: add helpers to setup the timings
  mtd: pxa3xx_nand: rework flash detection and timing setup
  mtd: pxa3xx_nand: clean up the pxa3xx timings

 drivers/mtd/nand/Kconfig       |   2 +-
 drivers/mtd/nand/pxa3xx_nand.c | 274 +++++++++++++++++++++++++----------------
 2 files changed, 168 insertions(+), 108 deletions(-)

-- 
2.6.2

[PATCH v4 1/5] mtd: pxa3xx: prepare allowing compile test

[Antoine Tenart]

Using readsl() result in a build error on i386. Fix this
by using ioread32_rep() instead, to allow compile test the
pxa3xx nand driver later.

Suggested-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 77b3206aef01..91605c1ee169 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -532,7 +532,7 @@ static void drain_fifo(struct pxa3xx_nand_info *info, void *data, int len)
 		 * the polling on the last read.
 		 */
 		while (len > 8) {
-			readsl(info->mmio_base + NDDB, data, 8);
+			ioread32_rep(info->mmio_base + NDDB, data, 8);
 
 			ret = readl_relaxed_poll_timeout(info->mmio_base + NDSR, val,
 							 val & NDSR_RDDREQ, 1000, 5000);
@@ -547,7 +547,7 @@ static void drain_fifo(struct pxa3xx_nand_info *info, void *data, int len)
 		}
 	}
 
-	readsl(info->mmio_base + NDDB, data, len);
+	ioread32_rep(info->mmio_base + NDDB, data, len);
 }
 
 static void handle_data_pio(struct pxa3xx_nand_info *info)
-- 
2.6.2

[PATCH v4 2/5] mtd: nand: allow compile test of MTD_NAND_PXA3xx

[Antoine Tenart]

Allow compile test of the MTD_NAND_PXA3xx nand driver.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/mtd/nand/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 289664089cf3..554aff7b2f8b 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -329,7 +329,7 @@ config MTD_NAND_ATMEL
 
 config MTD_NAND_PXA3xx
 	tristate "NAND support on PXA3xx and Armada 370/XP"
-	depends on PXA3xx || ARCH_MMP || PLAT_ORION
+	depends on PXA3xx || ARCH_MMP || PLAT_ORION || COMPILE_TEST
 	help
 	  This enables the driver for the NAND flash device found on
 	  PXA3xx processors (NFCv1) and also on Armada 370/XP (NFCv2).
-- 
2.6.2

[PATCH v4 3/5] mtd: pxa3xx_nand: add helpers to setup the timings

[Antoine Tenart]

Add helpers to setup the timings in the pxa3xx driver. These helpers
allow to either make use of the nand framework nand_sdr_timings or the
pxa3xx specific pxa3xx_nand_host, for compatibility reasons.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 122 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 122 insertions(+)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 91605c1ee169..54f92837be8a 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -418,6 +418,128 @@ static void pxa3xx_nand_set_timing(struct pxa3xx_nand_host *host,
 	nand_writel(info, NDTR1CS0, ndtr1);
 }
 
+static void pxa3xx_nand_set_sdr_timing(struct pxa3xx_nand_host *host,
+				       const struct nand_sdr_timings *t)
+{
+	struct pxa3xx_nand_info *info = host->info_data;
+	struct nand_chip *chip = &host->chip;
+	unsigned long nand_clk = clk_get_rate(info->clk);
+	uint32_t ndtr0, ndtr1;
+
+	u32 tCH_min = DIV_ROUND_UP(t->tCH_min, 1000);
+	u32 tCS_min = DIV_ROUND_UP(t->tCS_min, 1000);
+	u32 tWH_min = DIV_ROUND_UP(t->tWH_min, 1000);
+	u32 tWP_min = DIV_ROUND_UP(t->tWC_min - t->tWH_min, 1000);
+	u32 tREH_min = DIV_ROUND_UP(t->tREH_min, 1000);
+	u32 tRP_min = DIV_ROUND_UP(t->tRC_min - t->tREH_min, 1000);
+	u32 tR = chip->chip_delay * 1000;
+	u32 tWHR_min = DIV_ROUND_UP(t->tWHR_min, 1000);
+	u32 tAR_min = DIV_ROUND_UP(t->tAR_min, 1000);
+
+	/* fallback to a default value if tR = 0 */
+	if (!tR)
+		tR = 20000;
+
+	ndtr0 = NDTR0_tCH(ns2cycle(tCH_min, nand_clk)) |
+		NDTR0_tCS(ns2cycle(tCS_min, nand_clk)) |
+		NDTR0_tWH(ns2cycle(tWH_min, nand_clk)) |
+		NDTR0_tWP(ns2cycle(tWP_min, nand_clk)) |
+		NDTR0_tRH(ns2cycle(tREH_min, nand_clk)) |
+		NDTR0_tRP(ns2cycle(tRP_min, nand_clk));
+
+	ndtr1 = NDTR1_tR(ns2cycle(tR, nand_clk)) |
+		NDTR1_tWHR(ns2cycle(tWHR_min, nand_clk)) |
+		NDTR1_tAR(ns2cycle(tAR_min, nand_clk));
+
+	info->ndtr0cs0 = ndtr0;
+	info->ndtr1cs0 = ndtr1;
+	nand_writel(info, NDTR0CS0, ndtr0);
+	nand_writel(info, NDTR1CS0, ndtr1);
+}
+
+static int pxa3xx_nand_init_timings_compat(struct pxa3xx_nand_host *host,
+					   unsigned int *flash_width,
+					   unsigned int *dfc_width)
+{
+	struct nand_chip *chip = &host->chip;
+	struct pxa3xx_nand_info *info = host->info_data;
+	const struct pxa3xx_nand_flash *f = NULL;
+	int i, id, ntypes;
+
+	ntypes = ARRAY_SIZE(builtin_flash_types);
+
+	chip->cmdfunc(host->mtd, NAND_CMD_READID, 0x00, -1);
+
+	id = chip->read_byte(host->mtd);
+	id |= chip->read_byte(host->mtd) << 0x8;
+
+	for (i = 0; i < ntypes; i++) {
+		f = &builtin_flash_types[i];
+
+		if (f->chip_id == id)
+			break;
+	}
+
+	if (i == ntypes) {
+		dev_err(&info->pdev->dev, "Error: timings not found\n");
+		return -EINVAL;
+	}
+
+	pxa3xx_nand_set_timing(host, f->timing);
+
+	*flash_width = f->flash_width;
+	*dfc_width = f->dfc_width;
+
+	return 0;
+}
+
+static int pxa3xx_nand_init_timings_onfi(struct pxa3xx_nand_host *host,
+					 int mode)
+{
+	const struct nand_sdr_timings *timings;
+
+	mode = fls(mode) - 1;
+	if (mode < 0)
+		mode = 0;
+
+	timings = onfi_async_timing_mode_to_sdr_timings(mode);
+	if (IS_ERR(timings))
+		return PTR_ERR(timings);
+
+	pxa3xx_nand_set_sdr_timing(host, timings);
+
+	return 0;
+}
+
+static int pxa3xx_nand_init(struct pxa3xx_nand_host *host)
+{
+	struct nand_chip *chip = &host->chip;
+	struct pxa3xx_nand_info *info = host->info_data;
+	unsigned int flash_width = 0, dfc_width = 0;
+	int mode, err;
+
+	mode = onfi_get_async_timing_mode(chip);
+	if (mode == ONFI_TIMING_MODE_UNKNOWN) {
+		err = pxa3xx_nand_init_timings_compat(host, &flash_width,
+						      &dfc_width);
+		if (err)
+			return err;
+
+		if (flash_width == 16) {
+			info->reg_ndcr |= NDCR_DWIDTH_M;
+			chip->options |= NAND_BUSWIDTH_16;
+		}
+
+		info->reg_ndcr |= (dfc_width == 16) ? NDCR_DWIDTH_C : 0;
+	} else {
+		err = pxa3xx_nand_init_timings_onfi(host, mode);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
 /*
  * Set the data and OOB size, depending on the selected
  * spare and ECC configuration.
-- 
2.6.2

[PATCH v4 4/5] mtd: pxa3xx_nand: rework flash detection and timing setup

[Antoine Tenart]

Rework the pxa3xx_nand driver to allow using functions exported by the
nand framework to detect the flash and the timings. Then setup the
timings using the helpers previously added.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 123 ++++++++++++-----------------------------
 1 file changed, 35 insertions(+), 88 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 54f92837be8a..2761282df353 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -1396,45 +1396,23 @@ static int pxa3xx_nand_waitfunc(struct mtd_info *mtd, struct nand_chip *this)
 	return NAND_STATUS_READY;
 }
 
-static int pxa3xx_nand_config_flash(struct pxa3xx_nand_info *info,
-				    const struct pxa3xx_nand_flash *f)
+static int pxa3xx_nand_config_flash(struct pxa3xx_nand_info *info)
 {
 	struct platform_device *pdev = info->pdev;
 	struct pxa3xx_nand_platform_data *pdata = dev_get_platdata(&pdev->dev);
 	struct pxa3xx_nand_host *host = info->host[info->cs];
-	uint32_t ndcr = 0x0; /* enable all interrupts */
-
-	if (f->page_size != 2048 && f->page_size != 512) {
-		dev_err(&pdev->dev, "Current only support 2048 and 512 size\n");
-		return -EINVAL;
-	}
-
-	if (f->flash_width != 16 && f->flash_width != 8) {
-		dev_err(&pdev->dev, "Only support 8bit and 16 bit!\n");
-		return -EINVAL;
-	}
-
-	/* calculate addressing information */
-	host->col_addr_cycles = (f->page_size == 2048) ? 2 : 1;
-
-	if (f->num_blocks * f->page_per_block > 65536)
-		host->row_addr_cycles = 3;
-	else
-		host->row_addr_cycles = 2;
-
-	ndcr |= (pdata->enable_arbiter) ? NDCR_ND_ARB_EN : 0;
-	ndcr |= (host->col_addr_cycles == 2) ? NDCR_RA_START : 0;
-	ndcr |= (f->page_per_block == 64) ? NDCR_PG_PER_BLK : 0;
-	ndcr |= (f->page_size == 2048) ? NDCR_PAGE_SZ : 0;
-	ndcr |= (f->flash_width == 16) ? NDCR_DWIDTH_M : 0;
-	ndcr |= (f->dfc_width == 16) ? NDCR_DWIDTH_C : 0;
-
-	ndcr |= NDCR_RD_ID_CNT(READ_ID_BYTES);
-	ndcr |= NDCR_SPARE_EN; /* enable spare by default */
+	struct mtd_info *mtd = host->mtd;
+	struct nand_chip *chip = mtd->priv;
 
-	info->reg_ndcr = ndcr;
+	/* configure default flash values */
+	info->reg_ndcr = 0x0; /* enable all interrupts */
+	info->reg_ndcr |= (pdata->enable_arbiter) ? NDCR_ND_ARB_EN : 0;
+	info->reg_ndcr |= NDCR_RD_ID_CNT(READ_ID_BYTES);
+	info->reg_ndcr |= NDCR_SPARE_EN; /* enable spare by default */
+	info->reg_ndcr |= (host->col_addr_cycles == 2) ? NDCR_RA_START : 0;
+	info->reg_ndcr |= (chip->page_shift == 6) ? NDCR_PG_PER_BLK : 0;
+	info->reg_ndcr |= (mtd->writesize == 2048) ? NDCR_PAGE_SZ : 0;
 
-	pxa3xx_nand_set_timing(host, f->timing);
 	return 0;
 }
 
@@ -1514,19 +1492,23 @@ static void pxa3xx_nand_free_buff(struct pxa3xx_nand_info *info)
 	kfree(info->data_buff);
 }
 
-static int pxa3xx_nand_sensing(struct pxa3xx_nand_info *info)
+static int pxa3xx_nand_sensing(struct pxa3xx_nand_host *host)
 {
+	struct pxa3xx_nand_info *info = host->info_data;
 	struct mtd_info *mtd;
 	struct nand_chip *chip;
+	const struct nand_sdr_timings *timings;
 	int ret;
 
 	mtd = info->host[info->cs]->mtd;
 	chip = mtd->priv;
 
 	/* use the common timing to make a try */
-	ret = pxa3xx_nand_config_flash(info, &builtin_flash_types[0]);
-	if (ret)
-		return ret;
+	timings = onfi_async_timing_mode_to_sdr_timings(0);
+	if (IS_ERR(timings))
+		return PTR_ERR(timings);
+
+	pxa3xx_nand_set_sdr_timing(host, timings);
 
 	chip->cmdfunc(mtd, NAND_CMD_RESET, 0, 0);
 	ret = chip->waitfunc(mtd, chip);
@@ -1611,12 +1593,8 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
 	struct pxa3xx_nand_info *info = host->info_data;
 	struct platform_device *pdev = info->pdev;
 	struct pxa3xx_nand_platform_data *pdata = dev_get_platdata(&pdev->dev);
-	struct nand_flash_dev pxa3xx_flash_ids[2], *def = NULL;
-	const struct pxa3xx_nand_flash *f = NULL;
 	struct nand_chip *chip = mtd->priv;
-	uint32_t id = -1;
-	uint64_t chipsize;
-	int i, ret, num;
+	int ret;
 	uint16_t ecc_strength, ecc_step;
 
 	if (pdata->keep_config && !pxa3xx_nand_detect_config(info))
@@ -1625,7 +1603,11 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
 	/* Set a default chunk size */
 	info->chunk_size = 512;
 
-	ret = pxa3xx_nand_sensing(info);
+	ret = pxa3xx_nand_config_flash(info);
+	if (ret)
+		return ret;
+
+	ret = pxa3xx_nand_sensing(host);
 	if (ret) {
 		dev_info(&info->pdev->dev, "There is no chip on cs %d!\n",
 			 info->cs);
@@ -1633,50 +1615,6 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
 		return ret;
 	}
 
-	chip->cmdfunc(mtd, NAND_CMD_READID, 0, 0);
-	id = *((uint16_t *)(info->data_buff));
-	if (id != 0)
-		dev_info(&info->pdev->dev, "Detect a flash id %x\n", id);
-	else {
-		dev_warn(&info->pdev->dev,
-			 "Read out ID 0, potential timing set wrong!!\n");
-
-		return -EINVAL;
-	}
-
-	num = ARRAY_SIZE(builtin_flash_types) - 1;
-	for (i = 0; i < num; i++) {
-		f = &builtin_flash_types[i + 1];
-
-		/* find the chip in default list */
-		if (f->chip_id == id)
-			break;
-	}
-
-	if (i >= (ARRAY_SIZE(builtin_flash_types) - 1)) {
-		dev_err(&info->pdev->dev, "ERROR!! flash not defined!!!\n");
-
-		return -EINVAL;
-	}
-
-	ret = pxa3xx_nand_config_flash(info, f);
-	if (ret) {
-		dev_err(&info->pdev->dev, "ERROR! Configure failed\n");
-		return ret;
-	}
-
-	memset(pxa3xx_flash_ids, 0, sizeof(pxa3xx_flash_ids));
-
-	pxa3xx_flash_ids[0].name = f->name;
-	pxa3xx_flash_ids[0].dev_id = (f->chip_id >> 8) & 0xffff;
-	pxa3xx_flash_ids[0].pagesize = f->page_size;
-	chipsize = (uint64_t)f->num_blocks * f->page_per_block * f->page_size;
-	pxa3xx_flash_ids[0].chipsize = chipsize >> 20;
-	pxa3xx_flash_ids[0].erasesize = f->page_size * f->page_per_block;
-	if (f->flash_width == 16)
-		pxa3xx_flash_ids[0].options = NAND_BUSWIDTH_16;
-	pxa3xx_flash_ids[1].name = NULL;
-	def = pxa3xx_flash_ids;
 KEEP_CONFIG:
 	info->reg_ndcr |= (pdata->enable_arbiter) ? NDCR_ND_ARB_EN : 0;
 	if (info->reg_ndcr & NDCR_DWIDTH_M)
@@ -1686,9 +1624,18 @@ KEEP_CONFIG:
 	if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
 		nand_writel(info, NDECCCTRL, 0x0);
 
-	if (nand_scan_ident(mtd, 1, def))
+	if (nand_scan_ident(mtd, 1, NULL))
 		return -ENODEV;
 
+	if (!pdata->keep_config) {
+		ret = pxa3xx_nand_init(host);
+		if (ret) {
+			dev_err(&info->pdev->dev, "Failed to init nand: %d\n",
+				ret);
+			return ret;
+		}
+	}
+
 	if (pdata->flash_bbt) {
 		/*
 		 * We'll use a bad block table stored in-flash and don't
-- 
2.6.2

[PATCH v4 5/5] mtd: pxa3xx_nand: clean up the pxa3xx timings

[Antoine Tenart]

With the previous modifications, lots of pxa3xx specific definitions can
be removed.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 25 ++++++++-----------------
 1 file changed, 8 insertions(+), 17 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 2761282df353..e453ae9a17fa 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -266,14 +266,9 @@ struct pxa3xx_nand_timing {
 };
 
 struct pxa3xx_nand_flash {
-	char		*name;
 	uint32_t	chip_id;
-	unsigned int	page_per_block; /* Pages per block (PG_PER_BLK) */
-	unsigned int	page_size;	/* Page size in bytes (PAGE_SZ) */
 	unsigned int	flash_width;	/* Width of Flash memory (DWIDTH_M) */
 	unsigned int	dfc_width;	/* Width of flash controller(DWIDTH_C) */
-	unsigned int	num_blocks;	/* Number of physical blocks in Flash */
-
 	struct pxa3xx_nand_timing *timing;	/* NAND Flash timing */
 };
 
@@ -285,15 +280,14 @@ static struct pxa3xx_nand_timing timing[] = {
 };
 
 static struct pxa3xx_nand_flash builtin_flash_types[] = {
-{ "DEFAULT FLASH",      0,   0, 2048,  8,  8,    0, &timing[0] },
-{ "64MiB 16-bit",  0x46ec,  32,  512, 16, 16, 4096, &timing[1] },
-{ "256MiB 8-bit",  0xdaec,  64, 2048,  8,  8, 2048, &timing[1] },
-{ "4GiB 8-bit",    0xd7ec, 128, 4096,  8,  8, 8192, &timing[1] },
-{ "128MiB 8-bit",  0xa12c,  64, 2048,  8,  8, 1024, &timing[2] },
-{ "128MiB 16-bit", 0xb12c,  64, 2048, 16, 16, 1024, &timing[2] },
-{ "512MiB 8-bit",  0xdc2c,  64, 2048,  8,  8, 4096, &timing[2] },
-{ "512MiB 16-bit", 0xcc2c,  64, 2048, 16, 16, 4096, &timing[2] },
-{ "256MiB 16-bit", 0xba20,  64, 2048, 16, 16, 2048, &timing[3] },
+	{ 0x46ec, 16, 16, &timing[1] },
+	{ 0xdaec,  8,  8, &timing[1] },
+	{ 0xd7ec,  8,  8, &timing[1] },
+	{ 0xa12c,  8,  8, &timing[2] },
+	{ 0xb12c, 16, 16, &timing[2] },
+	{ 0xdc2c,  8,  8, &timing[2] },
+	{ 0xcc2c, 16, 16, &timing[2] },
+	{ 0xba20, 16, 16, &timing[3] },
 };
 
 static u8 bbt_pattern[] = {'M', 'V', 'B', 'b', 't', '0' };
@@ -354,9 +348,6 @@ static struct nand_ecclayout ecc_layout_4KB_bch8bit = {
 	.oobfree = { }
 };
 
-/* Define a default flash type setting serve as flash detecting only */
-#define DEFAULT_FLASH_TYPE (&builtin_flash_types[0])
-
 #define NDTR0_tCH(c)	(min((c), 7) << 19)
 #define NDTR0_tCS(c)	(min((c), 7) << 16)
 #define NDTR0_tWH(c)	(min((c), 7) << 11)
-- 
2.6.2

[PATCH v4 0/5] mtd: pxa3xx_nand: rework the timing setup

[Thomas Petazzoni]

Hello Antoine,

On Wed, 21 Oct 2015 10:28:59 +0200, Antoine Tenart wrote:

> Antoine Tenart (5):
>   mtd: pxa3xx: prepare allowing compile test
>   mtd: nand: allow compile test of MTD_NAND_PXA3xx
>   mtd: pxa3xx_nand: add helpers to setup the timings
>   mtd: pxa3xx_nand: rework flash detection and timing setup
>   mtd: pxa3xx_nand: clean up the pxa3xx timings

I tested your series on Armada 375 DB, which uses the same pxa3xx
driver, but with the Armada 370 variant.

With the current Device Tree which has nand,keep-config to keep the
timing configuration from the bootloader, I don't see any problem, so
there is no regression introduced by your series, at least on this
platform.

However, when I remove nand,keep-config to use the ONFI timings from
the NAND, then things work fine (I can mount a UBIFS root filesystem),
but there is a weird:

pxa3xx-nand f10d0000.nand: Wait time out!!!

After investigating a bit, the following steps occur:

 * The timings are configured as ONFI mode 0
 * Reset command is sent to the NAND (0xff), two times in a row.
 * READID command is sent to the NAND (0x90), three times in a row.
 * PARAM command is sent to the NAND (0xec) and it times out
 * The NAND is properly identified, and the timings are reconfigured as
   ONFI mode 5
 * Everything seems to work fine

In the current implementation of the driver, the nand_cmdfunc()
function is used for all the identification phase, and only switched
later to nand_cmdfunc_extended() if we are on an Armada 370/XP variant
(which Armada 375 is) and the page size is higher than PAGE_CHUNK_SIZE.
So the timeout occurs when nand_cmdfunc() is in-use. I've tried forcing
to use nand_cmdfunc_extended() from the beginning, but it times out
similarly in this function.

Since the driver works fine, maybe the PARAM command has worked
properly, but just times out for some reason. It would be good to
understand why.

Thanks,

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

[PATCH v4 0/5] mtd: pxa3xx_nand: rework the timing setup

[Ezequiel Garcia]

On 22 October 2015 at 12:12, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Hello Antoine,
>
> On Wed, 21 Oct 2015 10:28:59 +0200, Antoine Tenart wrote:
>
>> Antoine Tenart (5):
>>   mtd: pxa3xx: prepare allowing compile test
>>   mtd: nand: allow compile test of MTD_NAND_PXA3xx
>>   mtd: pxa3xx_nand: add helpers to setup the timings
>>   mtd: pxa3xx_nand: rework flash detection and timing setup
>>   mtd: pxa3xx_nand: clean up the pxa3xx timings
>
> I tested your series on Armada 375 DB, which uses the same pxa3xx
> driver, but with the Armada 370 variant.
>
> With the current Device Tree which has nand,keep-config to keep the
> timing configuration from the bootloader, I don't see any problem, so
> there is no regression introduced by your series, at least on this
> platform.
>
> However, when I remove nand,keep-config to use the ONFI timings from
> the NAND, then things work fine (I can mount a UBIFS root filesystem),
> but there is a weird:
>

Could you compare NAND throughput using keep-config (keeping
the bootloader timings) and without keep-config (with ONFI timings) ?

-- 
Ezequiel Garc?a, VanguardiaSur
www.vanguardiasur.com.ar

[PATCH v4 0/5] mtd: pxa3xx_nand: rework the timing setup

[Thomas Petazzoni]

Dear Ezequiel Garcia,

On Thu, 22 Oct 2015 12:22:24 -0300, Ezequiel Garcia wrote:

> Could you compare NAND throughput using keep-config (keeping
> the bootloader timings) and without keep-config (with ONFI timings) ?

Seems like we are within the measurement noise. I did each test only
once, and the test lasts only a few seconds, but here are the results:

Without keep-config (i.e ONFI timings) :

# time dd if=/dev/mtd3 of=/dev/null bs=1M
32+0 records in
32+0 records out
real	0m 3.54s
user	0m 0.00s
sys	0m 0.26s

With keep-config (i.e bootloader timings)

# time dd if=/dev/mtd3 of=/dev/null bs=1M
32+0 records in
32+0 records out
real	0m 3.77s
user	0m 0.00s
sys	0m 0.21s

I.e we are apparently slightly faster with the ONFI timings. However
again, the test duration is very short, and I didn't repeat the test
several times.

Best regards,

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

[PATCH v4 0/5] mtd: pxa3xx_nand: rework the timing setup

[Robert Jarzmik]

Antoine Tenart <antoine.tenart@free-electrons.com> writes:

> Hello,
>
> Let's try one more time to get this merged.
>
> This series was part of a bigger one[1], which was split into smaller
> ones as asked by Ezequiel[2]. When we take this into account this is
> v9; with a v1 sent in January, 9 months ago.
>
> Another series, introducing the Berlin nand support, depends on this.
>
> The aim here is to use the nand framework to setup the timings,
> while keeping the old in-driver way of configuration timings for
> compatibility reasons.
>
> You can find the series at:
> https://github.com/atenart/linux.git berlin/4.3/nand-timings

Hi Antoine,

I tested this serie once on my zylonite board, with both keep_config and no
keep_config setups. Both do work fine.

Cheers.

--
Robert

[PATCH v4 0/5] mtd: pxa3xx_nand: rework the timing setup

[Ezequiel Garcia]

On 21 Oct 10:28 AM, Antoine Tenart wrote:
> Hello,
> 
> Let's try one more time to get this merged.
> 
> This series was part of a bigger one[1], which was split into smaller
> ones as asked by Ezequiel[2]. When we take this into account this is
> v9; with a v1 sent in January, 9 months ago.
> 
> Another series, introducing the Berlin nand support, depends on this.
> 
> The aim here is to use the nand framework to setup the timings,
> while keeping the old in-driver way of configuration timings for
> compatibility reasons.
> 
> You can find the series at:
> https://github.com/atenart/linux.git berlin/4.3/nand-timings
> 
> Antoine
> 
> Since v3:
>         - Moved back a chunk into the right patch.
>         - Called pxa3xx_nand_config_flash() before KEEP_CONFIG.
>         - Rebased on top of l2-mtd.git master.
> 
> Since v2:
>         - Added back the support for keep-config 16 bits devices
>         - Fixed wrong unit in a calculation
>         - Reworked the pxa3xx_nand_init_timings() logic
>         - Allowed compile test the pxa3xx driver
> 
> Since v1:
>         - Rebased on top of v4.2-rc1
> 
> Since the series was split up:
>         - Reworked the ndcr setup
>         - Removed the read_id_bytes update after device detection
> 
> Antoine Tenart (5):
>   mtd: pxa3xx: prepare allowing compile test

Nit: This commit subject sounds a bit off.

>   mtd: nand: allow compile test of MTD_NAND_PXA3xx
>   mtd: pxa3xx_nand: add helpers to setup the timings
>   mtd: pxa3xx_nand: rework flash detection and timing setup
>   mtd: pxa3xx_nand: clean up the pxa3xx timings
> 
>  drivers/mtd/nand/Kconfig       |   2 +-
>  drivers/mtd/nand/pxa3xx_nand.c | 274 +++++++++++++++++++++++++----------------
>  2 files changed, 168 insertions(+), 108 deletions(-)
> 

The series look good and it's certainly a great improvement.
Thomas and Robert tested it, so we should be good to go:

Acked-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>

We can take a look at the timeout reported by Thomas afterwards.

BTW, it would be great if you could post a patch using Boris'
timings helpers: https://patchwork.ozlabs.org/patch/534919/ ?
-- 
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar

[PATCH v4 2/5] mtd: nand: allow compile test of MTD_NAND_PXA3xx

[Brian Norris]

On Wed, Oct 21, 2015 at 10:29:01AM +0200, Antoine Tenart wrote:
> Allow compile test of the MTD_NAND_PXA3xx nand driver.
> 
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> ---
>  drivers/mtd/nand/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> index 289664089cf3..554aff7b2f8b 100644
> --- a/drivers/mtd/nand/Kconfig
> +++ b/drivers/mtd/nand/Kconfig
> @@ -329,7 +329,7 @@ config MTD_NAND_ATMEL
>  
>  config MTD_NAND_PXA3xx
>  	tristate "NAND support on PXA3xx and Armada 370/XP"
> -	depends on PXA3xx || ARCH_MMP || PLAT_ORION
> +	depends on PXA3xx || ARCH_MMP || PLAT_ORION || COMPILE_TEST

You need at least a dependency on HAS_IOMEM. I'll drop this patch for
now, as it's unrelated. Feel free to send another revision.

Brian

>  	help
>  	  This enables the driver for the NAND flash device found on
>  	  PXA3xx processors (NFCv1) and also on Armada 370/XP (NFCv2).
> -- 
> 2.6.2
> 

[PATCH v4 0/5] mtd: pxa3xx_nand: rework the timing setup

[Brian Norris]

Hi,

On Mon, Oct 26, 2015 at 10:50:33AM -0300, Ezequiel Garcia wrote:
> On 21 Oct 10:28 AM, Antoine Tenart wrote:
> > Hello,
> > 
> > Let's try one more time to get this merged.
> > 
> > This series was part of a bigger one[1], which was split into smaller
> > ones as asked by Ezequiel[2]. When we take this into account this is
> > v9; with a v1 sent in January, 9 months ago.

Thanks for the persistence.

> > Another series, introducing the Berlin nand support, depends on this.
> > 
> > The aim here is to use the nand framework to setup the timings,
> > while keeping the old in-driver way of configuration timings for
> > compatibility reasons.
> > 
> > You can find the series at:
> > https://github.com/atenart/linux.git berlin/4.3/nand-timings
> > 
> > Antoine
> > 
> > Since v3:
> >         - Moved back a chunk into the right patch.
> >         - Called pxa3xx_nand_config_flash() before KEEP_CONFIG.
> >         - Rebased on top of l2-mtd.git master.
> > 
> > Since v2:
> >         - Added back the support for keep-config 16 bits devices
> >         - Fixed wrong unit in a calculation
> >         - Reworked the pxa3xx_nand_init_timings() logic
> >         - Allowed compile test the pxa3xx driver
> > 
> > Since v1:
> >         - Rebased on top of v4.2-rc1
> > 
> > Since the series was split up:
> >         - Reworked the ndcr setup
> >         - Removed the read_id_bytes update after device detection
> > 
> > Antoine Tenart (5):
> >   mtd: pxa3xx: prepare allowing compile test
> 
> Nit: This commit subject sounds a bit off.

I fixed that up a bit to my liking.

> >   mtd: nand: allow compile test of MTD_NAND_PXA3xx
> >   mtd: pxa3xx_nand: add helpers to setup the timings
> >   mtd: pxa3xx_nand: rework flash detection and timing setup
> >   mtd: pxa3xx_nand: clean up the pxa3xx timings
> > 
> >  drivers/mtd/nand/Kconfig       |   2 +-
> >  drivers/mtd/nand/pxa3xx_nand.c | 274 +++++++++++++++++++++++++----------------
> >  2 files changed, 168 insertions(+), 108 deletions(-)
> > 
> 
> The series look good and it's certainly a great improvement.
> Thomas and Robert tested it, so we should be good to go:
> 
> Acked-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>

Thanks for the reviews.

Pushed all but patch 2 to l2-mtd.git.

> We can take a look at the timeout reported by Thomas afterwards.
> 
> BTW, it would be great if you could post a patch using Boris'
> timings helpers: https://patchwork.ozlabs.org/patch/534919/ ?

Brian

[PATCH v4 0/5] mtd: pxa3xx_nand: rework the timing setup

[Ezequiel Garcia]

On 22 October 2015 at 12:12, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Hello Antoine,
>
> On Wed, 21 Oct 2015 10:28:59 +0200, Antoine Tenart wrote:
>
>> Antoine Tenart (5):
>>   mtd: pxa3xx: prepare allowing compile test
>>   mtd: nand: allow compile test of MTD_NAND_PXA3xx
>>   mtd: pxa3xx_nand: add helpers to setup the timings
>>   mtd: pxa3xx_nand: rework flash detection and timing setup
>>   mtd: pxa3xx_nand: clean up the pxa3xx timings
>
> I tested your series on Armada 375 DB, which uses the same pxa3xx
> driver, but with the Armada 370 variant.
>
> With the current Device Tree which has nand,keep-config to keep the
> timing configuration from the bootloader, I don't see any problem, so
> there is no regression introduced by your series, at least on this
> platform.
>
> However, when I remove nand,keep-config to use the ONFI timings from
> the NAND, then things work fine (I can mount a UBIFS root filesystem),
> but there is a weird:
>
> pxa3xx-nand f10d0000.nand: Wait time out!!!
>
> After investigating a bit, the following steps occur:
>
>  * The timings are configured as ONFI mode 0
>  * Reset command is sent to the NAND (0xff), two times in a row.
>  * READID command is sent to the NAND (0x90), three times in a row.
>  * PARAM command is sent to the NAND (0xec) and it times out

This timeout happens because initial chunk_size is 512, but the PARAM
page read is now set to read  2048 (only on the NFCv2 controller variant).

I'll prepare a patchset soon.
-- 
Ezequiel Garc?a, VanguardiaSur
www.vanguardiasur.com.ar