[PATCH v2 0/4] mtd: pxa3xx_nand: rework the timing setup

[PATCH v2 0/4] mtd: pxa3xx_nand: rework the timing setup

[Antoine Tenart]

Hi,

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
v7. In addition, there was absolutely no comment for more than 1 month
(since June 2nd). Given this, I really expect the series to be merge
soon as other series are based 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.

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

[1] https://lkml.org/lkml/2015/5/11/495
[2] https://lkml.org/lkml/2015/5/16/309

Antoine Tenart (4):
  mtd: pxa3xx_nand: add a default chunk size
  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/pxa3xx_nand.c                | 251 ++++++++++++++------------
 include/linux/platform_data/mtd-nand-pxa3xx.h |  11 +-
 2 files changed, 143 insertions(+), 119 deletions(-)

-- 
2.4.5

[PATCH v2 0/4] mtd: pxa3xx_nand: rework the timing setup

[Antoine Tenart]

Hi,

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
v7. In addition, there was absolutely no comment for more than 1 month
(since June 2nd). Given this, I really expect the series to be merge
soon as other series are based 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.

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

[1] https://lkml.org/lkml/2015/5/11/495
[2] https://lkml.org/lkml/2015/5/16/309

Antoine Tenart (4):
  mtd: pxa3xx_nand: add a default chunk size
  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/pxa3xx_nand.c                | 251 ++++++++++++++------------
 include/linux/platform_data/mtd-nand-pxa3xx.h |  11 +-
 2 files changed, 143 insertions(+), 119 deletions(-)

-- 
2.4.5

[PATCH v2 1/4] mtd: pxa3xx_nand: add a default chunk size

[Antoine Tenart]

When keeping the configuration set by the bootloader (by using
the marvell,nand-keep-config property), the pxa3xx_nand_detect_config()
function is called and set the chunk size to 512 as a default value if
NDCR_PAGE_SZ is not set.

In the other case, when not keeping the bootloader configuration, no
chunk size is set. Fix this by adding a default chunk size of 512.

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

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 1259cc558ce9..5465fa439c9e 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -1473,6 +1473,9 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
 	if (pdata->keep_config && !pxa3xx_nand_detect_config(info))
 		goto KEEP_CONFIG;
 
+	/* Set a default chunk size */
+	info->chunk_size = 512;
+
 	ret = pxa3xx_nand_sensing(info);
 	if (ret) {
 		dev_info(&info->pdev->dev, "There is no chip on cs %d!\n",
-- 
2.4.5

[PATCH v2 1/4] mtd: pxa3xx_nand: add a default chunk size

[Antoine Tenart]

When keeping the configuration set by the bootloader (by using
the marvell,nand-keep-config property), the pxa3xx_nand_detect_config()
function is called and set the chunk size to 512 as a default value if
NDCR_PAGE_SZ is not set.

In the other case, when not keeping the bootloader configuration, no
chunk size is set. Fix this by adding a default chunk size of 512.

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

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 1259cc558ce9..5465fa439c9e 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -1473,6 +1473,9 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
 	if (pdata->keep_config && !pxa3xx_nand_detect_config(info))
 		goto KEEP_CONFIG;
 
+	/* Set a default chunk size */
+	info->chunk_size = 512;
+
 	ret = pxa3xx_nand_sensing(info);
 	if (ret) {
 		dev_info(&info->pdev->dev, "There is no chip on cs %d!\n",
-- 
2.4.5

[PATCH v2 2/4] 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 | 91 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 91 insertions(+)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 5465fa439c9e..9a95c24ab2ce 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -385,6 +385,97 @@ 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 - tWH_min, 1000);
+	u32 tREH_min = DIV_ROUND_UP(t->tREH_min, 1000);
+	u32 tRP_min = DIV_ROUND_UP(t->tRC_min - 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(struct pxa3xx_nand_host *host)
+{
+	const struct nand_sdr_timings *timings;
+	struct nand_chip *chip = &host->chip;
+	struct pxa3xx_nand_info *info = host->info_data;
+	const struct pxa3xx_nand_flash *f = NULL;
+	int mode, id, ntypes, i;
+
+	mode = onfi_get_async_timing_mode(chip);
+	if (mode == ONFI_TIMING_MODE_UNKNOWN) {
+		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);
+
+		if (f->flash_width == 16) {
+			info->reg_ndcr |= NDCR_DWIDTH_M;
+			chip->options |= NAND_BUSWIDTH_16;
+		}
+
+		info->reg_ndcr |= (f->dfc_width == 16) ? NDCR_DWIDTH_C : 0;
+	} else {
+		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;
+}
+
 /*
  * Set the data and OOB size, depending on the selected
  * spare and ECC configuration.
-- 
2.4.5

[PATCH v2 2/4] 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 | 91 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 91 insertions(+)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 5465fa439c9e..9a95c24ab2ce 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -385,6 +385,97 @@ 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 - tWH_min, 1000);
+	u32 tREH_min = DIV_ROUND_UP(t->tREH_min, 1000);
+	u32 tRP_min = DIV_ROUND_UP(t->tRC_min - 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(struct pxa3xx_nand_host *host)
+{
+	const struct nand_sdr_timings *timings;
+	struct nand_chip *chip = &host->chip;
+	struct pxa3xx_nand_info *info = host->info_data;
+	const struct pxa3xx_nand_flash *f = NULL;
+	int mode, id, ntypes, i;
+
+	mode = onfi_get_async_timing_mode(chip);
+	if (mode == ONFI_TIMING_MODE_UNKNOWN) {
+		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);
+
+		if (f->flash_width == 16) {
+			info->reg_ndcr |= NDCR_DWIDTH_M;
+			chip->options |= NAND_BUSWIDTH_16;
+		}
+
+		info->reg_ndcr |= (f->dfc_width == 16) ? NDCR_DWIDTH_C : 0;
+	} else {
+		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;
+}
+
 /*
  * Set the data and OOB size, depending on the selected
  * spare and ECC configuration.
-- 
2.4.5

[PATCH v2 3/4] 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 | 137 ++++++++++++-----------------------------
 1 file changed, 38 insertions(+), 99 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 9a95c24ab2ce..513f8f6069f0 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -1320,48 +1320,16 @@ 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 flash information */
-	host->read_id_bytes = (f->page_size == 2048) ? 4 : 2;
-
-	/* 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(host->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;
+	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;
 }
 
@@ -1456,19 +1424,31 @@ static void pxa3xx_nand_free_buff(struct pxa3xx_nand_info *info)
 }
 #endif
 
-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 platform_device *pdev = info->pdev;
+	struct pxa3xx_nand_platform_data *pdata = dev_get_platdata(&pdev->dev);
 	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;
 
+	/* 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(host->read_id_bytes);
+	info->reg_ndcr |= NDCR_SPARE_EN; /* enable spare by default */
+
 	/* 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);
@@ -1553,12 +1533,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))
@@ -1567,7 +1543,7 @@ 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_sensing(host);
 	if (ret) {
 		dev_info(&info->pdev->dev, "There is no chip on cs %d!\n",
 			 info->cs);
@@ -1575,64 +1551,26 @@ 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) + pdata->num_flash - 1;
-	for (i = 0; i < num; i++) {
-		if (i < pdata->num_flash)
-			f = pdata->flash + i;
-		else
-			f = &builtin_flash_types[i - pdata->num_flash + 1];
-
-		/* find the chip in default list */
-		if (f->chip_id == id)
-			break;
-	}
-
-	if (i >= (ARRAY_SIZE(builtin_flash_types) + pdata->num_flash - 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:
-	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))
+	if (nand_scan_ident(mtd, 1, NULL))
 		return -ENODEV;
 
+	if (!pdata->keep_config) {
+		ret = pxa3xx_nand_init_timings(host);
+		if (ret) {
+			dev_err(&info->pdev->dev, "Failed to set timings: %d\n", ret);
+			return ret;
+		}
+	}
+
+	ret = pxa3xx_nand_config_flash(info);
+	if (ret)
+		return ret;
+
 	if (pdata->flash_bbt) {
 		/*
 		 * We'll use a bad block table stored in-flash and don't
@@ -1729,6 +1667,7 @@ static int alloc_nand_resource(struct platform_device *pdev)
 		host->mtd = mtd;
 		host->cs = cs;
 		host->info_data = info;
+		host->read_id_bytes = 4;
 		mtd->priv = host;
 		mtd->owner = THIS_MODULE;
 
-- 
2.4.5

[PATCH v2 3/4] 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 | 137 ++++++++++++-----------------------------
 1 file changed, 38 insertions(+), 99 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 9a95c24ab2ce..513f8f6069f0 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -1320,48 +1320,16 @@ 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 flash information */
-	host->read_id_bytes = (f->page_size == 2048) ? 4 : 2;
-
-	/* 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(host->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;
+	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;
 }
 
@@ -1456,19 +1424,31 @@ static void pxa3xx_nand_free_buff(struct pxa3xx_nand_info *info)
 }
 #endif
 
-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 platform_device *pdev = info->pdev;
+	struct pxa3xx_nand_platform_data *pdata = dev_get_platdata(&pdev->dev);
 	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;
 
+	/* 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(host->read_id_bytes);
+	info->reg_ndcr |= NDCR_SPARE_EN; /* enable spare by default */
+
 	/* 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);
@@ -1553,12 +1533,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))
@@ -1567,7 +1543,7 @@ 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_sensing(host);
 	if (ret) {
 		dev_info(&info->pdev->dev, "There is no chip on cs %d!\n",
 			 info->cs);
@@ -1575,64 +1551,26 @@ 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) + pdata->num_flash - 1;
-	for (i = 0; i < num; i++) {
-		if (i < pdata->num_flash)
-			f = pdata->flash + i;
-		else
-			f = &builtin_flash_types[i - pdata->num_flash + 1];
-
-		/* find the chip in default list */
-		if (f->chip_id == id)
-			break;
-	}
-
-	if (i >= (ARRAY_SIZE(builtin_flash_types) + pdata->num_flash - 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:
-	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))
+	if (nand_scan_ident(mtd, 1, NULL))
 		return -ENODEV;
 
+	if (!pdata->keep_config) {
+		ret = pxa3xx_nand_init_timings(host);
+		if (ret) {
+			dev_err(&info->pdev->dev, "Failed to set timings: %d\n", ret);
+			return ret;
+		}
+	}
+
+	ret = pxa3xx_nand_config_flash(info);
+	if (ret)
+		return ret;
+
 	if (pdata->flash_bbt) {
 		/*
 		 * We'll use a bad block table stored in-flash and don't
@@ -1729,6 +1667,7 @@ static int alloc_nand_resource(struct platform_device *pdev)
 		host->mtd = mtd;
 		host->cs = cs;
 		host->info_data = info;
+		host->read_id_bytes = 4;
 		mtd->priv = host;
 		mtd->owner = THIS_MODULE;
 
-- 
2.4.5

[PATCH v2 4/4] 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                | 20 ++++++++------------
 include/linux/platform_data/mtd-nand-pxa3xx.h | 11 +++--------
 2 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 513f8f6069f0..0e1db91b386c 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -252,15 +252,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' };
@@ -321,9 +320,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)
diff --git a/include/linux/platform_data/mtd-nand-pxa3xx.h b/include/linux/platform_data/mtd-nand-pxa3xx.h
index ac4ea2e641c7..25538cfeee26 100644
--- a/include/linux/platform_data/mtd-nand-pxa3xx.h
+++ b/include/linux/platform_data/mtd-nand-pxa3xx.h
@@ -17,15 +17,10 @@ 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 */
+	unsigned int	flash_width;    /* Width of Flash memory (DWIDTH_M) */
+	unsigned int	dfc_width;      /* Width of flash controller(DWIDTH_C) */
+	struct pxa3xx_nand_timing *timing; /* NAND Flash timing */
 };
 
 /*
-- 
2.4.5

[PATCH v2 4/4] 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                | 20 ++++++++------------
 include/linux/platform_data/mtd-nand-pxa3xx.h | 11 +++--------
 2 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 513f8f6069f0..0e1db91b386c 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -252,15 +252,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' };
@@ -321,9 +320,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)
diff --git a/include/linux/platform_data/mtd-nand-pxa3xx.h b/include/linux/platform_data/mtd-nand-pxa3xx.h
index ac4ea2e641c7..25538cfeee26 100644
--- a/include/linux/platform_data/mtd-nand-pxa3xx.h
+++ b/include/linux/platform_data/mtd-nand-pxa3xx.h
@@ -17,15 +17,10 @@ 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 */
+	unsigned int	flash_width;    /* Width of Flash memory (DWIDTH_M) */
+	unsigned int	dfc_width;      /* Width of flash controller(DWIDTH_C) */
+	struct pxa3xx_nand_timing *timing; /* NAND Flash timing */
 };
 
 /*
-- 
2.4.5

Re: [PATCH v2 0/4] mtd: pxa3xx_nand: rework the timing setup

[Antoine Tenart]

Hi guys,

On Tue, Jul 07, 2015 at 05:08:23PM +0200, Antoine Tenart wrote:
> 
> 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
> v7. In addition, there was absolutely no comment for more than 1 month
> (since June 2nd). Given this, I really expect the series to be merge
> soon as other series are based on this.

Any news on this series?

Thanks,

Antoine

> 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.
> 
> 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
> 
> [1] https://lkml.org/lkml/2015/5/11/495
> [2] https://lkml.org/lkml/2015/5/16/309
> 
> Antoine Tenart (4):
>   mtd: pxa3xx_nand: add a default chunk size
>   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/pxa3xx_nand.c                | 251 ++++++++++++++------------
>  include/linux/platform_data/mtd-nand-pxa3xx.h |  11 +-
>  2 files changed, 143 insertions(+), 119 deletions(-)
> 
> -- 
> 2.4.5
> 

-- 
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH v2 0/4] mtd: pxa3xx_nand: rework the timing setup

[Antoine Tenart]

Hi guys,

On Tue, Jul 07, 2015 at 05:08:23PM +0200, Antoine Tenart wrote:
> 
> 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
> v7. In addition, there was absolutely no comment for more than 1 month
> (since June 2nd). Given this, I really expect the series to be merge
> soon as other series are based on this.

Any news on this series?

Thanks,

Antoine

> 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.
> 
> 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
> 
> [1] https://lkml.org/lkml/2015/5/11/495
> [2] https://lkml.org/lkml/2015/5/16/309
> 
> Antoine Tenart (4):
>   mtd: pxa3xx_nand: add a default chunk size
>   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/pxa3xx_nand.c                | 251 ++++++++++++++------------
>  include/linux/platform_data/mtd-nand-pxa3xx.h |  11 +-
>  2 files changed, 143 insertions(+), 119 deletions(-)
> 
> -- 
> 2.4.5
> 

-- 
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH v2 0/4] mtd: pxa3xx_nand: rework the timing setup

[Ezequiel Garcia]

Hi Antoine,

On 07/17/2015 10:41 AM, Antoine Tenart wrote:
> Hi guys,
>
> On Tue, Jul 07, 2015 at 05:08:23PM +0200, Antoine Tenart wrote:
>>
>> 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
>> v7. In addition, there was absolutely no comment for more than 1 month
>> (since June 2nd). Given this, I really expect the series to be merge
>> soon as other series are based on this.
>
> Any news on this series?
>

I'll try to take a look and do some testing over the weekend.

Robert: think you can test this on a pxa board? I have one here,
but it's non trivial for me to set it up.

Thanks,
-- 
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar

[PATCH v2 0/4] mtd: pxa3xx_nand: rework the timing setup

[Ezequiel Garcia]

Hi Antoine,

On 07/17/2015 10:41 AM, Antoine Tenart wrote:
> Hi guys,
>
> On Tue, Jul 07, 2015 at 05:08:23PM +0200, Antoine Tenart wrote:
>>
>> 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
>> v7. In addition, there was absolutely no comment for more than 1 month
>> (since June 2nd). Given this, I really expect the series to be merge
>> soon as other series are based on this.
>
> Any news on this series?
>

I'll try to take a look and do some testing over the weekend.

Robert: think you can test this on a pxa board? I have one here,
but it's non trivial for me to set it up.

Thanks,
-- 
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar

Re: [PATCH v2 0/4] mtd: pxa3xx_nand: rework the timing setup

[Robert Jarzmik]

Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> writes:

> Hi Antoine,
>
> On 07/17/2015 10:41 AM, Antoine Tenart wrote:
>> Hi guys,
>>
>> On Tue, Jul 07, 2015 at 05:08:23PM +0200, Antoine Tenart wrote:
>>>
>>> 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
>>> v7. In addition, there was absolutely no comment for more than 1 month
>>> (since June 2nd). Given this, I really expect the series to be merge
>>> soon as other series are based on this.
>>
>> Any news on this series?
>>
>
> I'll try to take a look and do some testing over the weekend.
>
> Robert: think you can test this on a pxa board? I have one here,
> but it's non trivial for me to set it up.

Sure, if somebody sends patches to me, it's very easy and automated, so please
send the whole serie to robert.jarzmik@free.fr, and I'll reply to this mail with
the test result on both cm-x300 and zylonite board.

Cheers.

-- 
Robert

[PATCH v2 0/4] mtd: pxa3xx_nand: rework the timing setup

[Robert Jarzmik]

Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> writes:

> Hi Antoine,
>
> On 07/17/2015 10:41 AM, Antoine Tenart wrote:
>> Hi guys,
>>
>> On Tue, Jul 07, 2015 at 05:08:23PM +0200, Antoine Tenart wrote:
>>>
>>> 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
>>> v7. In addition, there was absolutely no comment for more than 1 month
>>> (since June 2nd). Given this, I really expect the series to be merge
>>> soon as other series are based on this.
>>
>> Any news on this series?
>>
>
> I'll try to take a look and do some testing over the weekend.
>
> Robert: think you can test this on a pxa board? I have one here,
> but it's non trivial for me to set it up.

Sure, if somebody sends patches to me, it's very easy and automated, so please
send the whole serie to robert.jarzmik at free.fr, and I'll reply to this mail with
the test result on both cm-x300 and zylonite board.

Cheers.

-- 
Robert

Re: [PATCH v2 0/4] mtd: pxa3xx_nand: rework the timing setup

[Ezequiel Garcia]

On 17 July 2015 at 14:55, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> writes:
>
>> Hi Antoine,
>>
>> On 07/17/2015 10:41 AM, Antoine Tenart wrote:
>>> Hi guys,
>>>
>>> On Tue, Jul 07, 2015 at 05:08:23PM +0200, Antoine Tenart wrote:
>>>>
>>>> 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
>>>> v7. In addition, there was absolutely no comment for more than 1 month
>>>> (since June 2nd). Given this, I really expect the series to be merge
>>>> soon as other series are based on this.
>>>
>>> Any news on this series?
>>>
>>
>> I'll try to take a look and do some testing over the weekend.
>>
>> Robert: think you can test this on a pxa board? I have one here,
>> but it's non trivial for me to set it up.
>
> Sure, if somebody sends patches to me, it's very easy and automated, so please
> send the whole serie to robert.jarzmik@free.fr, and I'll reply to this mail with
> the test result on both cm-x300 and zylonite board.
>

Here you go:

http://git.infradead.org/users/ezequielg/linux/shortlog/refs/heads/pxa3xx-nand-timing-rework-v2

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

[PATCH v2 0/4] mtd: pxa3xx_nand: rework the timing setup

[Ezequiel Garcia]

On 17 July 2015 at 14:55, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> writes:
>
>> Hi Antoine,
>>
>> On 07/17/2015 10:41 AM, Antoine Tenart wrote:
>>> Hi guys,
>>>
>>> On Tue, Jul 07, 2015 at 05:08:23PM +0200, Antoine Tenart wrote:
>>>>
>>>> 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
>>>> v7. In addition, there was absolutely no comment for more than 1 month
>>>> (since June 2nd). Given this, I really expect the series to be merge
>>>> soon as other series are based on this.
>>>
>>> Any news on this series?
>>>
>>
>> I'll try to take a look and do some testing over the weekend.
>>
>> Robert: think you can test this on a pxa board? I have one here,
>> but it's non trivial for me to set it up.
>
> Sure, if somebody sends patches to me, it's very easy and automated, so please
> send the whole serie to robert.jarzmik at free.fr, and I'll reply to this mail with
> the test result on both cm-x300 and zylonite board.
>

Here you go:

http://git.infradead.org/users/ezequielg/linux/shortlog/refs/heads/pxa3xx-nand-timing-rework-v2

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

Re: [PATCH v2 2/4] mtd: pxa3xx_nand: add helpers to setup the timings

[Ezequiel Garcia]

On 07/07/2015 12:08 PM, Antoine Tenart wrote:
> 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 | 91 ++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 91 insertions(+)
>
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index 5465fa439c9e..9a95c24ab2ce 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -385,6 +385,97 @@ 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 - tWH_min, 1000);

Is the substraction above correct? You seem to be substracting
picoseconds to nanoseconds.

> +	u32 tREH_min = DIV_ROUND_UP(t->tREH_min, 1000);
> +	u32 tRP_min = DIV_ROUND_UP(t->tRC_min - tREH_min, 1000);

Ditto.

> +	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(struct pxa3xx_nand_host *host)
> +{
> +	const struct nand_sdr_timings *timings;
> +	struct nand_chip *chip = &host->chip;
> +	struct pxa3xx_nand_info *info = host->info_data;
> +	const struct pxa3xx_nand_flash *f = NULL;
> +	int mode, id, ntypes, i;
> +
> +	mode = onfi_get_async_timing_mode(chip);
> +	if (mode == ONFI_TIMING_MODE_UNKNOWN) {
> +		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);
> +
> +		if (f->flash_width == 16) {
> +			info->reg_ndcr |= NDCR_DWIDTH_M;
> +			chip->options |= NAND_BUSWIDTH_16;
> +		}
> +
> +		info->reg_ndcr |= (f->dfc_width == 16) ? NDCR_DWIDTH_C : 0;

Nitpick: the 16-bit flash configuration doesn't belong in a function
called "xxx_init_timings".

> +	} else {
> +		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;
> +}
> +
>   /*
>    * Set the data and OOB size, depending on the selected
>    * spare and ECC configuration.
>

Thanks,
-- 
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar

[PATCH v2 2/4] mtd: pxa3xx_nand: add helpers to setup the timings

[Ezequiel Garcia]

On 07/07/2015 12:08 PM, Antoine Tenart wrote:
> 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 | 91 ++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 91 insertions(+)
>
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index 5465fa439c9e..9a95c24ab2ce 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -385,6 +385,97 @@ 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 - tWH_min, 1000);

Is the substraction above correct? You seem to be substracting
picoseconds to nanoseconds.

> +	u32 tREH_min = DIV_ROUND_UP(t->tREH_min, 1000);
> +	u32 tRP_min = DIV_ROUND_UP(t->tRC_min - tREH_min, 1000);

Ditto.

> +	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(struct pxa3xx_nand_host *host)
> +{
> +	const struct nand_sdr_timings *timings;
> +	struct nand_chip *chip = &host->chip;
> +	struct pxa3xx_nand_info *info = host->info_data;
> +	const struct pxa3xx_nand_flash *f = NULL;
> +	int mode, id, ntypes, i;
> +
> +	mode = onfi_get_async_timing_mode(chip);
> +	if (mode == ONFI_TIMING_MODE_UNKNOWN) {
> +		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);
> +
> +		if (f->flash_width == 16) {
> +			info->reg_ndcr |= NDCR_DWIDTH_M;
> +			chip->options |= NAND_BUSWIDTH_16;
> +		}
> +
> +		info->reg_ndcr |= (f->dfc_width == 16) ? NDCR_DWIDTH_C : 0;

Nitpick: the 16-bit flash configuration doesn't belong in a function
called "xxx_init_timings".

> +	} else {
> +		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;
> +}
> +
>   /*
>    * Set the data and OOB size, depending on the selected
>    * spare and ECC configuration.
>

Thanks,
-- 
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar

Re: [PATCH v2 3/4] mtd: pxa3xx_nand: rework flash detection and timing setup

[Ezequiel Garcia]

On 07/07/2015 12:08 PM, Antoine Tenart wrote:
> 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 | 137 ++++++++++++-----------------------------
>   1 file changed, 38 insertions(+), 99 deletions(-)
>
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index 9a95c24ab2ce..513f8f6069f0 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -1320,48 +1320,16 @@ 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 flash information */
> -	host->read_id_bytes = (f->page_size == 2048) ? 4 : 2;
> -
> -	/* 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(host->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;
> +	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;
>   }
>
> @@ -1456,19 +1424,31 @@ static void pxa3xx_nand_free_buff(struct pxa3xx_nand_info *info)
>   }
>   #endif
>
> -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 platform_device *pdev = info->pdev;
> +	struct pxa3xx_nand_platform_data *pdata = dev_get_platdata(&pdev->dev);
>   	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;
>
> +	/* 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(host->read_id_bytes);
> +	info->reg_ndcr |= NDCR_SPARE_EN; /* enable spare by default */
> +
>   	/* 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);
> @@ -1553,12 +1533,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))
> @@ -1567,7 +1543,7 @@ 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_sensing(host);
>   	if (ret) {
>   		dev_info(&info->pdev->dev, "There is no chip on cs %d!\n",
>   			 info->cs);
> @@ -1575,64 +1551,26 @@ 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) + pdata->num_flash - 1;
> -	for (i = 0; i < num; i++) {
> -		if (i < pdata->num_flash)
> -			f = pdata->flash + i;
> -		else
> -			f = &builtin_flash_types[i - pdata->num_flash + 1];
> -
> -		/* find the chip in default list */
> -		if (f->chip_id == id)
> -			break;
> -	}
> -
> -	if (i >= (ARRAY_SIZE(builtin_flash_types) + pdata->num_flash - 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:

Hm...

> -	if (info->reg_ndcr & NDCR_DWIDTH_M)
> -		chip->options |= NAND_BUSWIDTH_16;
> -

...can't see why this removal is needed. AFAICS, it removes
support for 16-bit devices (only supported when using keep-config).

>   	/* 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))
> +	if (nand_scan_ident(mtd, 1, NULL))
>   		return -ENODEV;
>
> +	if (!pdata->keep_config) {
> +		ret = pxa3xx_nand_init_timings(host);
> +		if (ret) {
> +			dev_err(&info->pdev->dev, "Failed to set timings: %d\n", ret);
> +			return ret;
> +		}
> +	}
> +
> +	ret = pxa3xx_nand_config_flash(info);
> +	if (ret)
> +		return ret;
> +
>   	if (pdata->flash_bbt) {
>   		/*
>   		 * We'll use a bad block table stored in-flash and don't
> @@ -1729,6 +1667,7 @@ static int alloc_nand_resource(struct platform_device *pdev)
>   		host->mtd = mtd;
>   		host->cs = cs;
>   		host->info_data = info;
> +		host->read_id_bytes = 4;
>   		mtd->priv = host;
>   		mtd->owner = THIS_MODULE;
>
>


-- 
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar

[PATCH v2 3/4] mtd: pxa3xx_nand: rework flash detection and timing setup

[Ezequiel Garcia]

On 07/07/2015 12:08 PM, Antoine Tenart wrote:
> 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 | 137 ++++++++++++-----------------------------
>   1 file changed, 38 insertions(+), 99 deletions(-)
>
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index 9a95c24ab2ce..513f8f6069f0 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -1320,48 +1320,16 @@ 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 flash information */
> -	host->read_id_bytes = (f->page_size == 2048) ? 4 : 2;
> -
> -	/* 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(host->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;
> +	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;
>   }
>
> @@ -1456,19 +1424,31 @@ static void pxa3xx_nand_free_buff(struct pxa3xx_nand_info *info)
>   }
>   #endif
>
> -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 platform_device *pdev = info->pdev;
> +	struct pxa3xx_nand_platform_data *pdata = dev_get_platdata(&pdev->dev);
>   	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;
>
> +	/* 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(host->read_id_bytes);
> +	info->reg_ndcr |= NDCR_SPARE_EN; /* enable spare by default */
> +
>   	/* 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);
> @@ -1553,12 +1533,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))
> @@ -1567,7 +1543,7 @@ 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_sensing(host);
>   	if (ret) {
>   		dev_info(&info->pdev->dev, "There is no chip on cs %d!\n",
>   			 info->cs);
> @@ -1575,64 +1551,26 @@ 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) + pdata->num_flash - 1;
> -	for (i = 0; i < num; i++) {
> -		if (i < pdata->num_flash)
> -			f = pdata->flash + i;
> -		else
> -			f = &builtin_flash_types[i - pdata->num_flash + 1];
> -
> -		/* find the chip in default list */
> -		if (f->chip_id == id)
> -			break;
> -	}
> -
> -	if (i >= (ARRAY_SIZE(builtin_flash_types) + pdata->num_flash - 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:

Hm...

> -	if (info->reg_ndcr & NDCR_DWIDTH_M)
> -		chip->options |= NAND_BUSWIDTH_16;
> -

...can't see why this removal is needed. AFAICS, it removes
support for 16-bit devices (only supported when using keep-config).

>   	/* 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))
> +	if (nand_scan_ident(mtd, 1, NULL))
>   		return -ENODEV;
>
> +	if (!pdata->keep_config) {
> +		ret = pxa3xx_nand_init_timings(host);
> +		if (ret) {
> +			dev_err(&info->pdev->dev, "Failed to set timings: %d\n", ret);
> +			return ret;
> +		}
> +	}
> +
> +	ret = pxa3xx_nand_config_flash(info);
> +	if (ret)
> +		return ret;
> +
>   	if (pdata->flash_bbt) {
>   		/*
>   		 * We'll use a bad block table stored in-flash and don't
> @@ -1729,6 +1667,7 @@ static int alloc_nand_resource(struct platform_device *pdev)
>   		host->mtd = mtd;
>   		host->cs = cs;
>   		host->info_data = info;
> +		host->read_id_bytes = 4;
>   		mtd->priv = host;
>   		mtd->owner = THIS_MODULE;
>
>


-- 
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar

Re: [PATCH v2 0/4] mtd: pxa3xx_nand: rework the timing setup

[Robert Jarzmik]

Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> writes:

> Here you go:
>
> http://git.infradead.org/users/ezequielg/linux/shortlog/refs/heads/pxa3xx-nand-timing-rework-v2

Okay, I've tested this on cm-x300. The result is not bright :
	nand: device found, Manufacturer ID: 0xec, Chip ID: 0xdc
	nand: Samsung NAND 512MiB 3,3V 8-bit
	nand: 512 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
	pxa3xx-nand pxa3xx-nand: ECC strength 1, ECC step size 512
	Scanning device for bad blocks
	random: nonblocking pool is initialized
	Bad eraseblock 1037 at 0x0000081a0000
	Creating 6 MTD partitions on "pxa3xx_nand-0":
	0x000000000000-0x000000040000 : "OBM"
	0x000000040000-0x000000080000 : "U-Boot"
	0x000000080000-0x0000000c0000 : "Environment"
	0x0000000c0000-0x000000200000 : "reserved"
	0x000000200000-0x000000600000 : "kernel"
	0x000000600000-0x000020000000 : "fs"

The bad black is not bad normally.
And then :
	# dd if=/dev/mtd0 of=toto count=1
	pxa3xx-nand pxa3xx-nand: Wait time out!!!
	1+0 records in
	1+0 records out
	# ls -l toto
	-rw-r--r--    1 root     root           512 Jan  1 00:02 toto
	# hexdump-C toto
	00000000  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
	*
	00000200

That's not possible, as it should have dumped the OBMI.

And yet, I hesitate to incriminate this patchset, I will try without it. It wont
be very fast, as my USB to serial converter just broke, and my order will take a
couple of days to complete.

And of course without this converter I cannot test anymore, especially the
zylonite board which will be more interesting.

So I'll report back as soon as the UK mail has completed the shipment.

Cheers.

-- 
Robert

[PATCH v2 0/4] mtd: pxa3xx_nand: rework the timing setup

[Robert Jarzmik]

Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> writes:

> Here you go:
>
> http://git.infradead.org/users/ezequielg/linux/shortlog/refs/heads/pxa3xx-nand-timing-rework-v2

Okay, I've tested this on cm-x300. The result is not bright :
	nand: device found, Manufacturer ID: 0xec, Chip ID: 0xdc
	nand: Samsung NAND 512MiB 3,3V 8-bit
	nand: 512 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
	pxa3xx-nand pxa3xx-nand: ECC strength 1, ECC step size 512
	Scanning device for bad blocks
	random: nonblocking pool is initialized
	Bad eraseblock 1037 at 0x0000081a0000
	Creating 6 MTD partitions on "pxa3xx_nand-0":
	0x000000000000-0x000000040000 : "OBM"
	0x000000040000-0x000000080000 : "U-Boot"
	0x000000080000-0x0000000c0000 : "Environment"
	0x0000000c0000-0x000000200000 : "reserved"
	0x000000200000-0x000000600000 : "kernel"
	0x000000600000-0x000020000000 : "fs"

The bad black is not bad normally.
And then :
	# dd if=/dev/mtd0 of=toto count=1
	pxa3xx-nand pxa3xx-nand: Wait time out!!!
	1+0 records in
	1+0 records out
	# ls -l toto
	-rw-r--r--    1 root     root           512 Jan  1 00:02 toto
	# hexdump-C toto
	00000000  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
	*
	00000200

That's not possible, as it should have dumped the OBMI.

And yet, I hesitate to incriminate this patchset, I will try without it. It wont
be very fast, as my USB to serial converter just broke, and my order will take a
couple of days to complete.

And of course without this converter I cannot test anymore, especially the
zylonite board which will be more interesting.

So I'll report back as soon as the UK mail has completed the shipment.

Cheers.

-- 
Robert

Re: [PATCH v2 0/4] mtd: pxa3xx_nand: rework the timing setup

[Ezequiel Garcia]

On 20 July 2015 at 16:49, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> writes:
>
>> Here you go:
>>
>> http://git.infradead.org/users/ezequielg/linux/shortlog/refs/heads/pxa3xx-nand-timing-rework-v2
>
> Okay, I've tested this on cm-x300. The result is not bright :
>         nand: device found, Manufacturer ID: 0xec, Chip ID: 0xdc
>         nand: Samsung NAND 512MiB 3,3V 8-bit
>         nand: 512 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
>         pxa3xx-nand pxa3xx-nand: ECC strength 1, ECC step size 512
>         Scanning device for bad blocks
>         random: nonblocking pool is initialized
>         Bad eraseblock 1037 at 0x0000081a0000
>         Creating 6 MTD partitions on "pxa3xx_nand-0":
>         0x000000000000-0x000000040000 : "OBM"
>         0x000000040000-0x000000080000 : "U-Boot"
>         0x000000080000-0x0000000c0000 : "Environment"
>         0x0000000c0000-0x000000200000 : "reserved"
>         0x000000200000-0x000000600000 : "kernel"
>         0x000000600000-0x000020000000 : "fs"
>
> The bad black is not bad normally.
> And then :
>         # dd if=/dev/mtd0 of=toto count=1
>         pxa3xx-nand pxa3xx-nand: Wait time out!!!
>         1+0 records in
>         1+0 records out
>         # ls -l toto
>         -rw-r--r--    1 root     root           512 Jan  1 00:02 toto
>         # hexdump-C toto
>         00000000  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
>         *
>         00000200
>
> That's not possible, as it should have dumped the OBMI.
>
> And yet, I hesitate to incriminate this patchset, I will try without it. It wont
> be very fast, as my USB to serial converter just broke, and my order will take a
> couple of days to complete.
>

Just tested linux-next (hence *without* the patchset) and I see
the same "Wait time out". In other words, pxa3xx-nand is broken
on PXA :/

Interestingly, the culprit doesn't seem to be in pxa3xx-nand itself.
Reverting the recent commits on pxa3xx-nand doesn't help.

ce914e6 mtd: nand: pxa3xx: fix build on ARM64
afca11e mtd: nand: pxa3xx: Switch FIFO draining to jiffies-based timeout
e5860c1 mtd: pxa3xx_nand: cleanup wait_for_completion handling
7c2f717 mtd: pxa3xx_nand: initialiaze pxa3xx_flash_ids to 0
ed446cc Merge MTD updates into -next
e423c90 mtd: pxa3xx_nand: fix driver when num_cs is 0
2454225 mtd: pxa3xx-nand: handle PIO in threaded interrupt
8dad038 mtd: nand: pxa3xx: Fix PIO FIFO draining
b7e46062 mtd: pxa3xx_nand: make the driver work on big-endian systems
5b3e507 mtd: nand: pxa3xx: Use ECC strength and step size devicetree binding
eee0166 mtd: nand: pxa3xx: Clean pxa_ecc_init() error handling
17754ad mtd: nand: pxa3xx: Make of_device_id array const
e634ce5 mtd: nand: pxa3xx: Print actual ECC strength in error message

Yet v3.18 succeeds to pass a few NAND blocks on nandtest.

Robert: any ideas?
-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar

[PATCH v2 0/4] mtd: pxa3xx_nand: rework the timing setup

[Ezequiel Garcia]

On 20 July 2015 at 16:49, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> writes:
>
>> Here you go:
>>
>> http://git.infradead.org/users/ezequielg/linux/shortlog/refs/heads/pxa3xx-nand-timing-rework-v2
>
> Okay, I've tested this on cm-x300. The result is not bright :
>         nand: device found, Manufacturer ID: 0xec, Chip ID: 0xdc
>         nand: Samsung NAND 512MiB 3,3V 8-bit
>         nand: 512 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
>         pxa3xx-nand pxa3xx-nand: ECC strength 1, ECC step size 512
>         Scanning device for bad blocks
>         random: nonblocking pool is initialized
>         Bad eraseblock 1037 at 0x0000081a0000
>         Creating 6 MTD partitions on "pxa3xx_nand-0":
>         0x000000000000-0x000000040000 : "OBM"
>         0x000000040000-0x000000080000 : "U-Boot"
>         0x000000080000-0x0000000c0000 : "Environment"
>         0x0000000c0000-0x000000200000 : "reserved"
>         0x000000200000-0x000000600000 : "kernel"
>         0x000000600000-0x000020000000 : "fs"
>
> The bad black is not bad normally.
> And then :
>         # dd if=/dev/mtd0 of=toto count=1
>         pxa3xx-nand pxa3xx-nand: Wait time out!!!
>         1+0 records in
>         1+0 records out
>         # ls -l toto
>         -rw-r--r--    1 root     root           512 Jan  1 00:02 toto
>         # hexdump-C toto
>         00000000  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
>         *
>         00000200
>
> That's not possible, as it should have dumped the OBMI.
>
> And yet, I hesitate to incriminate this patchset, I will try without it. It wont
> be very fast, as my USB to serial converter just broke, and my order will take a
> couple of days to complete.
>

Just tested linux-next (hence *without* the patchset) and I see
the same "Wait time out". In other words, pxa3xx-nand is broken
on PXA :/

Interestingly, the culprit doesn't seem to be in pxa3xx-nand itself.
Reverting the recent commits on pxa3xx-nand doesn't help.

ce914e6 mtd: nand: pxa3xx: fix build on ARM64
afca11e mtd: nand: pxa3xx: Switch FIFO draining to jiffies-based timeout
e5860c1 mtd: pxa3xx_nand: cleanup wait_for_completion handling
7c2f717 mtd: pxa3xx_nand: initialiaze pxa3xx_flash_ids to 0
ed446cc Merge MTD updates into -next
e423c90 mtd: pxa3xx_nand: fix driver when num_cs is 0
2454225 mtd: pxa3xx-nand: handle PIO in threaded interrupt
8dad038 mtd: nand: pxa3xx: Fix PIO FIFO draining
b7e46062 mtd: pxa3xx_nand: make the driver work on big-endian systems
5b3e507 mtd: nand: pxa3xx: Use ECC strength and step size devicetree binding
eee0166 mtd: nand: pxa3xx: Clean pxa_ecc_init() error handling
17754ad mtd: nand: pxa3xx: Make of_device_id array const
e634ce5 mtd: nand: pxa3xx: Print actual ECC strength in error message

Yet v3.18 succeeds to pass a few NAND blocks on nandtest.

Robert: any ideas?
-- 
Ezequiel Garc?a, VanguardiaSur
www.vanguardiasur.com.ar

Re: [PATCH v2 0/4] mtd: pxa3xx_nand: rework the timing setup

[Robert Jarzmik]

Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> writes:

> Just tested linux-next (hence *without* the patchset) and I see
> the same "Wait time out". In other words, pxa3xx-nand is broken
> on PXA :/
>
> Interestingly, the culprit doesn't seem to be in pxa3xx-nand itself.
> Reverting the recent commits on pxa3xx-nand doesn't help.
>
> ce914e6 mtd: nand: pxa3xx: fix build on ARM64
> afca11e mtd: nand: pxa3xx: Switch FIFO draining to jiffies-based timeout
> e5860c1 mtd: pxa3xx_nand: cleanup wait_for_completion handling
> 7c2f717 mtd: pxa3xx_nand: initialiaze pxa3xx_flash_ids to 0
> ed446cc Merge MTD updates into -next
> e423c90 mtd: pxa3xx_nand: fix driver when num_cs is 0
> 2454225 mtd: pxa3xx-nand: handle PIO in threaded interrupt
> 8dad038 mtd: nand: pxa3xx: Fix PIO FIFO draining
> b7e46062 mtd: pxa3xx_nand: make the driver work on big-endian systems
> 5b3e507 mtd: nand: pxa3xx: Use ECC strength and step size devicetree binding
> eee0166 mtd: nand: pxa3xx: Clean pxa_ecc_init() error handling
> 17754ad mtd: nand: pxa3xx: Make of_device_id array const
> e634ce5 mtd: nand: pxa3xx: Print actual ECC strength in error message
>
> Yet v3.18 succeeds to pass a few NAND blocks on nandtest.
>
> Robert: any ideas?
Actually yes, I worked on that this weekend.
Would you try the following patch [1] to see if it works for you ?

The issue I see is that :
 - there is a bug in the clk driver for pxa3xx I introduced (CKEN_AB)
 - and shutting down the GCU clock prevents the NAND from working (I can't
 explain that one yet)

Cheers.

-- 
Robert

[1] Clocks patch
---8>---
diff --git a/drivers/clk/pxa/clk-pxa3xx.c b/drivers/clk/pxa/clk-pxa3xx.c
index c677b9ab5367..a47a0c40f937 100644
--- a/drivers/clk/pxa/clk-pxa3xx.c
+++ b/drivers/clk/pxa/clk-pxa3xx.c
@@ -126,7 +126,7 @@ PARENTS(pxa3xx_ac97_bus) = { "ring_osc_60mhz", "ac97" };
 PARENTS(pxa3xx_sbus) = { "ring_osc_60mhz", "system_bus" };
 PARENTS(pxa3xx_smemcbus) = { "ring_osc_60mhz", "smemc" };
 
-#define CKEN_AB(bit) ((CKEN_ ## bit > 31) ? &CKENA : &CKENB)
+#define CKEN_AB(bit) ((CKEN_ ## bit > 31) ? &CKENB : &CKENA)
 #define PXA3XX_CKEN(dev_id, con_id, parents, mult_lp, div_lp, mult_hp,	\
 		    div_hp, bit, is_lp, flags)				\
 	PXA_CKEN(dev_id, con_id, bit, parents, mult_lp, div_lp,		\
@@ -136,6 +136,10 @@ PARENTS(pxa3xx_smemcbus) = { "ring_osc_60mhz", "smemc" };
 			 mult_hp, div_hp, delay)			\
 	PXA3XX_CKEN(dev_id, con_id, pxa3xx_pbus_parents, mult_lp,	\
 		    div_lp, mult_hp, div_hp, bit, pxa3xx_is_ring_osc_forced, 0)
+#define PXA3XX_PBUS_CKENF(dev_id, con_id, bit, mult_lp, div_lp,		\
+			  mult_hp, div_hp, delay, flag)			\
+	PXA3XX_CKEN(dev_id, con_id, pxa3xx_pbus_parents, mult_lp,	\
+		    div_lp, mult_hp, div_hp, bit, pxa3xx_is_ring_osc_forced, flag)
 #define PXA3XX_CKEN_1RATE(dev_id, con_id, bit, parents)			\
 	PXA_CKEN_1RATE(dev_id, con_id, bit, parents,			\
 		       CKEN_AB(bit), (CKEN_ ## bit % 32), 0)
@@ -173,13 +177,13 @@ static struct desc_clk_cken pxa3xx_clocks[] __initdata = {
 
 static struct desc_clk_cken pxa300_310_clocks[] __initdata = {
 
-	PXA3XX_PBUS_CKEN("pxa3xx-gcu", NULL, PXA300_GCU, 1, 1, 1, 1, 0),
-	PXA3XX_PBUS_CKEN("pxa3xx-nand", NULL, NAND, 1, 2, 1, 4, 0),
+	PXA3XX_PBUS_CKENF("pxa3xx-gcu", NULL, PXA300_GCU, 1, 1, 1, 1, 0, CLK_IGNORE_UNUSED),
+	PXA3XX_PBUS_CKENF("pxa3xx-nand", NULL, NAND, 1, 2, 1, 4, 0, CLK_IGNORE_UNUSED),
 	PXA3XX_CKEN_1RATE("pxa3xx-gpio", NULL, GPIO, pxa3xx_13MHz_bus_parents),
 };
 
 static struct desc_clk_cken pxa320_clocks[] __initdata = {
-	PXA3XX_PBUS_CKEN("pxa3xx-nand", NULL, NAND, 1, 2, 1, 6, 0),
+	PXA3XX_PBUS_CKENF("pxa3xx-nand", NULL, NAND, 1, 2, 1, 4, 0, CLK_IGNORE_UNUSED),
 	PXA3XX_PBUS_CKEN("pxa3xx-gcu", NULL, PXA320_GCU, 1, 1, 1, 1, 0),
 	PXA3XX_CKEN_1RATE("pxa3xx-gpio", NULL, GPIO, pxa3xx_13MHz_bus_parents),
 };
@@ -187,7 +191,7 @@ static struct desc_clk_cken pxa320_clocks[] __initdata = {
 static struct desc_clk_cken pxa93x_clocks[] __initdata = {
 
 	PXA3XX_PBUS_CKEN("pxa3xx-gcu", NULL, PXA300_GCU, 1, 1, 1, 1, 0),
-	PXA3XX_PBUS_CKEN("pxa3xx-nand", NULL, NAND, 1, 2, 1, 4, 0),
+	PXA3XX_PBUS_CKENF("pxa3xx-nand", NULL, NAND, 1, 2, 1, 4, 0, CLK_IGNORE_UNUSED),
 	PXA3XX_CKEN_1RATE("pxa93x-gpio", NULL, GPIO, pxa3xx_13MHz_bus_parents),
 };
 

[PATCH v2 0/4] mtd: pxa3xx_nand: rework the timing setup

[Robert Jarzmik]

Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> writes:

> Just tested linux-next (hence *without* the patchset) and I see
> the same "Wait time out". In other words, pxa3xx-nand is broken
> on PXA :/
>
> Interestingly, the culprit doesn't seem to be in pxa3xx-nand itself.
> Reverting the recent commits on pxa3xx-nand doesn't help.
>
> ce914e6 mtd: nand: pxa3xx: fix build on ARM64
> afca11e mtd: nand: pxa3xx: Switch FIFO draining to jiffies-based timeout
> e5860c1 mtd: pxa3xx_nand: cleanup wait_for_completion handling
> 7c2f717 mtd: pxa3xx_nand: initialiaze pxa3xx_flash_ids to 0
> ed446cc Merge MTD updates into -next
> e423c90 mtd: pxa3xx_nand: fix driver when num_cs is 0
> 2454225 mtd: pxa3xx-nand: handle PIO in threaded interrupt
> 8dad038 mtd: nand: pxa3xx: Fix PIO FIFO draining
> b7e46062 mtd: pxa3xx_nand: make the driver work on big-endian systems
> 5b3e507 mtd: nand: pxa3xx: Use ECC strength and step size devicetree binding
> eee0166 mtd: nand: pxa3xx: Clean pxa_ecc_init() error handling
> 17754ad mtd: nand: pxa3xx: Make of_device_id array const
> e634ce5 mtd: nand: pxa3xx: Print actual ECC strength in error message
>
> Yet v3.18 succeeds to pass a few NAND blocks on nandtest.
>
> Robert: any ideas?
Actually yes, I worked on that this weekend.
Would you try the following patch [1] to see if it works for you ?

The issue I see is that :
 - there is a bug in the clk driver for pxa3xx I introduced (CKEN_AB)
 - and shutting down the GCU clock prevents the NAND from working (I can't
 explain that one yet)

Cheers.

-- 
Robert

[1] Clocks patch
---8>---
diff --git a/drivers/clk/pxa/clk-pxa3xx.c b/drivers/clk/pxa/clk-pxa3xx.c
index c677b9ab5367..a47a0c40f937 100644
--- a/drivers/clk/pxa/clk-pxa3xx.c
+++ b/drivers/clk/pxa/clk-pxa3xx.c
@@ -126,7 +126,7 @@ PARENTS(pxa3xx_ac97_bus) = { "ring_osc_60mhz", "ac97" };
 PARENTS(pxa3xx_sbus) = { "ring_osc_60mhz", "system_bus" };
 PARENTS(pxa3xx_smemcbus) = { "ring_osc_60mhz", "smemc" };
 
-#define CKEN_AB(bit) ((CKEN_ ## bit > 31) ? &CKENA : &CKENB)
+#define CKEN_AB(bit) ((CKEN_ ## bit > 31) ? &CKENB : &CKENA)
 #define PXA3XX_CKEN(dev_id, con_id, parents, mult_lp, div_lp, mult_hp,	\
 		    div_hp, bit, is_lp, flags)				\
 	PXA_CKEN(dev_id, con_id, bit, parents, mult_lp, div_lp,		\
@@ -136,6 +136,10 @@ PARENTS(pxa3xx_smemcbus) = { "ring_osc_60mhz", "smemc" };
 			 mult_hp, div_hp, delay)			\
 	PXA3XX_CKEN(dev_id, con_id, pxa3xx_pbus_parents, mult_lp,	\
 		    div_lp, mult_hp, div_hp, bit, pxa3xx_is_ring_osc_forced, 0)
+#define PXA3XX_PBUS_CKENF(dev_id, con_id, bit, mult_lp, div_lp,		\
+			  mult_hp, div_hp, delay, flag)			\
+	PXA3XX_CKEN(dev_id, con_id, pxa3xx_pbus_parents, mult_lp,	\
+		    div_lp, mult_hp, div_hp, bit, pxa3xx_is_ring_osc_forced, flag)
 #define PXA3XX_CKEN_1RATE(dev_id, con_id, bit, parents)			\
 	PXA_CKEN_1RATE(dev_id, con_id, bit, parents,			\
 		       CKEN_AB(bit), (CKEN_ ## bit % 32), 0)
@@ -173,13 +177,13 @@ static struct desc_clk_cken pxa3xx_clocks[] __initdata = {
 
 static struct desc_clk_cken pxa300_310_clocks[] __initdata = {
 
-	PXA3XX_PBUS_CKEN("pxa3xx-gcu", NULL, PXA300_GCU, 1, 1, 1, 1, 0),
-	PXA3XX_PBUS_CKEN("pxa3xx-nand", NULL, NAND, 1, 2, 1, 4, 0),
+	PXA3XX_PBUS_CKENF("pxa3xx-gcu", NULL, PXA300_GCU, 1, 1, 1, 1, 0, CLK_IGNORE_UNUSED),
+	PXA3XX_PBUS_CKENF("pxa3xx-nand", NULL, NAND, 1, 2, 1, 4, 0, CLK_IGNORE_UNUSED),
 	PXA3XX_CKEN_1RATE("pxa3xx-gpio", NULL, GPIO, pxa3xx_13MHz_bus_parents),
 };
 
 static struct desc_clk_cken pxa320_clocks[] __initdata = {
-	PXA3XX_PBUS_CKEN("pxa3xx-nand", NULL, NAND, 1, 2, 1, 6, 0),
+	PXA3XX_PBUS_CKENF("pxa3xx-nand", NULL, NAND, 1, 2, 1, 4, 0, CLK_IGNORE_UNUSED),
 	PXA3XX_PBUS_CKEN("pxa3xx-gcu", NULL, PXA320_GCU, 1, 1, 1, 1, 0),
 	PXA3XX_CKEN_1RATE("pxa3xx-gpio", NULL, GPIO, pxa3xx_13MHz_bus_parents),
 };
@@ -187,7 +191,7 @@ static struct desc_clk_cken pxa320_clocks[] __initdata = {
 static struct desc_clk_cken pxa93x_clocks[] __initdata = {
 
 	PXA3XX_PBUS_CKEN("pxa3xx-gcu", NULL, PXA300_GCU, 1, 1, 1, 1, 0),
-	PXA3XX_PBUS_CKEN("pxa3xx-nand", NULL, NAND, 1, 2, 1, 4, 0),
+	PXA3XX_PBUS_CKENF("pxa3xx-nand", NULL, NAND, 1, 2, 1, 4, 0, CLK_IGNORE_UNUSED),
 	PXA3XX_CKEN_1RATE("pxa93x-gpio", NULL, GPIO, pxa3xx_13MHz_bus_parents),
 };
 

Re: [PATCH v2 0/4] mtd: pxa3xx_nand: rework the timing setup

[Ezequiel Garcia]

On 3 August 2015 at 01:44, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> writes:
>
>> Just tested linux-next (hence *without* the patchset) and I see
>> the same "Wait time out". In other words, pxa3xx-nand is broken
>> on PXA :/
>>
>> Interestingly, the culprit doesn't seem to be in pxa3xx-nand itself.
>> Reverting the recent commits on pxa3xx-nand doesn't help.
>>
>> ce914e6 mtd: nand: pxa3xx: fix build on ARM64
>> afca11e mtd: nand: pxa3xx: Switch FIFO draining to jiffies-based timeout
>> e5860c1 mtd: pxa3xx_nand: cleanup wait_for_completion handling
>> 7c2f717 mtd: pxa3xx_nand: initialiaze pxa3xx_flash_ids to 0
>> ed446cc Merge MTD updates into -next
>> e423c90 mtd: pxa3xx_nand: fix driver when num_cs is 0
>> 2454225 mtd: pxa3xx-nand: handle PIO in threaded interrupt
>> 8dad038 mtd: nand: pxa3xx: Fix PIO FIFO draining
>> b7e46062 mtd: pxa3xx_nand: make the driver work on big-endian systems
>> 5b3e507 mtd: nand: pxa3xx: Use ECC strength and step size devicetree binding
>> eee0166 mtd: nand: pxa3xx: Clean pxa_ecc_init() error handling
>> 17754ad mtd: nand: pxa3xx: Make of_device_id array const
>> e634ce5 mtd: nand: pxa3xx: Print actual ECC strength in error message
>>
>> Yet v3.18 succeeds to pass a few NAND blocks on nandtest.
>>
>> Robert: any ideas?
> Actually yes, I worked on that this weekend.
> Would you try the following patch [1] to see if it works for you ?
>
> The issue I see is that :
>  - there is a bug in the clk driver for pxa3xx I introduced (CKEN_AB)
>  - and shutting down the GCU clock prevents the NAND from working (I can't
>  explain that one yet)
>

IIRC, on Armada 370/XP SoCs we have two clocks feeding the NAND
controller: one for the NAND logic (not really sure?), and one for the
NAND ECC engine.

Maybe the GCU feeds the ECC?

> Cheers.
>
> --
> Robert
>
> [1] Clocks patch

Ah, good. I was almost sure it was the clock (after trying a bunch of
other stuff it was the only big change left).

I'll try this and will let you know.

> ---8>---
> diff --git a/drivers/clk/pxa/clk-pxa3xx.c b/drivers/clk/pxa/clk-pxa3xx.c
> index c677b9ab5367..a47a0c40f937 100644
> --- a/drivers/clk/pxa/clk-pxa3xx.c
> +++ b/drivers/clk/pxa/clk-pxa3xx.c
> @@ -126,7 +126,7 @@ PARENTS(pxa3xx_ac97_bus) = { "ring_osc_60mhz", "ac97" };
>  PARENTS(pxa3xx_sbus) = { "ring_osc_60mhz", "system_bus" };
>  PARENTS(pxa3xx_smemcbus) = { "ring_osc_60mhz", "smemc" };
>
> -#define CKEN_AB(bit) ((CKEN_ ## bit > 31) ? &CKENA : &CKENB)
> +#define CKEN_AB(bit) ((CKEN_ ## bit > 31) ? &CKENB : &CKENA)
>  #define PXA3XX_CKEN(dev_id, con_id, parents, mult_lp, div_lp, mult_hp, \
>                     div_hp, bit, is_lp, flags)                          \
>         PXA_CKEN(dev_id, con_id, bit, parents, mult_lp, div_lp,         \
> @@ -136,6 +136,10 @@ PARENTS(pxa3xx_smemcbus) = { "ring_osc_60mhz", "smemc" };
>                          mult_hp, div_hp, delay)                        \
>         PXA3XX_CKEN(dev_id, con_id, pxa3xx_pbus_parents, mult_lp,       \
>                     div_lp, mult_hp, div_hp, bit, pxa3xx_is_ring_osc_forced, 0)
> +#define PXA3XX_PBUS_CKENF(dev_id, con_id, bit, mult_lp, div_lp,                \
> +                         mult_hp, div_hp, delay, flag)                 \
> +       PXA3XX_CKEN(dev_id, con_id, pxa3xx_pbus_parents, mult_lp,       \
> +                   div_lp, mult_hp, div_hp, bit, pxa3xx_is_ring_osc_forced, flag)
>  #define PXA3XX_CKEN_1RATE(dev_id, con_id, bit, parents)                        \
>         PXA_CKEN_1RATE(dev_id, con_id, bit, parents,                    \
>                        CKEN_AB(bit), (CKEN_ ## bit % 32), 0)
> @@ -173,13 +177,13 @@ static struct desc_clk_cken pxa3xx_clocks[] __initdata = {
>
>  static struct desc_clk_cken pxa300_310_clocks[] __initdata = {
>
> -       PXA3XX_PBUS_CKEN("pxa3xx-gcu", NULL, PXA300_GCU, 1, 1, 1, 1, 0),
> -       PXA3XX_PBUS_CKEN("pxa3xx-nand", NULL, NAND, 1, 2, 1, 4, 0),
> +       PXA3XX_PBUS_CKENF("pxa3xx-gcu", NULL, PXA300_GCU, 1, 1, 1, 1, 0, CLK_IGNORE_UNUSED),
> +       PXA3XX_PBUS_CKENF("pxa3xx-nand", NULL, NAND, 1, 2, 1, 4, 0, CLK_IGNORE_UNUSED),
>         PXA3XX_CKEN_1RATE("pxa3xx-gpio", NULL, GPIO, pxa3xx_13MHz_bus_parents),
>  };
>
>  static struct desc_clk_cken pxa320_clocks[] __initdata = {
> -       PXA3XX_PBUS_CKEN("pxa3xx-nand", NULL, NAND, 1, 2, 1, 6, 0),
> +       PXA3XX_PBUS_CKENF("pxa3xx-nand", NULL, NAND, 1, 2, 1, 4, 0, CLK_IGNORE_UNUSED),
>         PXA3XX_PBUS_CKEN("pxa3xx-gcu", NULL, PXA320_GCU, 1, 1, 1, 1, 0),
>         PXA3XX_CKEN_1RATE("pxa3xx-gpio", NULL, GPIO, pxa3xx_13MHz_bus_parents),
>  };
> @@ -187,7 +191,7 @@ static struct desc_clk_cken pxa320_clocks[] __initdata = {
>  static struct desc_clk_cken pxa93x_clocks[] __initdata = {
>
>         PXA3XX_PBUS_CKEN("pxa3xx-gcu", NULL, PXA300_GCU, 1, 1, 1, 1, 0),
> -       PXA3XX_PBUS_CKEN("pxa3xx-nand", NULL, NAND, 1, 2, 1, 4, 0),
> +       PXA3XX_PBUS_CKENF("pxa3xx-nand", NULL, NAND, 1, 2, 1, 4, 0, CLK_IGNORE_UNUSED),
>         PXA3XX_CKEN_1RATE("pxa93x-gpio", NULL, GPIO, pxa3xx_13MHz_bus_parents),
>  };
>



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

[PATCH v2 0/4] mtd: pxa3xx_nand: rework the timing setup

[Ezequiel Garcia]

On 3 August 2015 at 01:44, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> writes:
>
>> Just tested linux-next (hence *without* the patchset) and I see
>> the same "Wait time out". In other words, pxa3xx-nand is broken
>> on PXA :/
>>
>> Interestingly, the culprit doesn't seem to be in pxa3xx-nand itself.
>> Reverting the recent commits on pxa3xx-nand doesn't help.
>>
>> ce914e6 mtd: nand: pxa3xx: fix build on ARM64
>> afca11e mtd: nand: pxa3xx: Switch FIFO draining to jiffies-based timeout
>> e5860c1 mtd: pxa3xx_nand: cleanup wait_for_completion handling
>> 7c2f717 mtd: pxa3xx_nand: initialiaze pxa3xx_flash_ids to 0
>> ed446cc Merge MTD updates into -next
>> e423c90 mtd: pxa3xx_nand: fix driver when num_cs is 0
>> 2454225 mtd: pxa3xx-nand: handle PIO in threaded interrupt
>> 8dad038 mtd: nand: pxa3xx: Fix PIO FIFO draining
>> b7e46062 mtd: pxa3xx_nand: make the driver work on big-endian systems
>> 5b3e507 mtd: nand: pxa3xx: Use ECC strength and step size devicetree binding
>> eee0166 mtd: nand: pxa3xx: Clean pxa_ecc_init() error handling
>> 17754ad mtd: nand: pxa3xx: Make of_device_id array const
>> e634ce5 mtd: nand: pxa3xx: Print actual ECC strength in error message
>>
>> Yet v3.18 succeeds to pass a few NAND blocks on nandtest.
>>
>> Robert: any ideas?
> Actually yes, I worked on that this weekend.
> Would you try the following patch [1] to see if it works for you ?
>
> The issue I see is that :
>  - there is a bug in the clk driver for pxa3xx I introduced (CKEN_AB)
>  - and shutting down the GCU clock prevents the NAND from working (I can't
>  explain that one yet)
>

IIRC, on Armada 370/XP SoCs we have two clocks feeding the NAND
controller: one for the NAND logic (not really sure?), and one for the
NAND ECC engine.

Maybe the GCU feeds the ECC?

> Cheers.
>
> --
> Robert
>
> [1] Clocks patch

Ah, good. I was almost sure it was the clock (after trying a bunch of
other stuff it was the only big change left).

I'll try this and will let you know.

> ---8>---
> diff --git a/drivers/clk/pxa/clk-pxa3xx.c b/drivers/clk/pxa/clk-pxa3xx.c
> index c677b9ab5367..a47a0c40f937 100644
> --- a/drivers/clk/pxa/clk-pxa3xx.c
> +++ b/drivers/clk/pxa/clk-pxa3xx.c
> @@ -126,7 +126,7 @@ PARENTS(pxa3xx_ac97_bus) = { "ring_osc_60mhz", "ac97" };
>  PARENTS(pxa3xx_sbus) = { "ring_osc_60mhz", "system_bus" };
>  PARENTS(pxa3xx_smemcbus) = { "ring_osc_60mhz", "smemc" };
>
> -#define CKEN_AB(bit) ((CKEN_ ## bit > 31) ? &CKENA : &CKENB)
> +#define CKEN_AB(bit) ((CKEN_ ## bit > 31) ? &CKENB : &CKENA)
>  #define PXA3XX_CKEN(dev_id, con_id, parents, mult_lp, div_lp, mult_hp, \
>                     div_hp, bit, is_lp, flags)                          \
>         PXA_CKEN(dev_id, con_id, bit, parents, mult_lp, div_lp,         \
> @@ -136,6 +136,10 @@ PARENTS(pxa3xx_smemcbus) = { "ring_osc_60mhz", "smemc" };
>                          mult_hp, div_hp, delay)                        \
>         PXA3XX_CKEN(dev_id, con_id, pxa3xx_pbus_parents, mult_lp,       \
>                     div_lp, mult_hp, div_hp, bit, pxa3xx_is_ring_osc_forced, 0)
> +#define PXA3XX_PBUS_CKENF(dev_id, con_id, bit, mult_lp, div_lp,                \
> +                         mult_hp, div_hp, delay, flag)                 \
> +       PXA3XX_CKEN(dev_id, con_id, pxa3xx_pbus_parents, mult_lp,       \
> +                   div_lp, mult_hp, div_hp, bit, pxa3xx_is_ring_osc_forced, flag)
>  #define PXA3XX_CKEN_1RATE(dev_id, con_id, bit, parents)                        \
>         PXA_CKEN_1RATE(dev_id, con_id, bit, parents,                    \
>                        CKEN_AB(bit), (CKEN_ ## bit % 32), 0)
> @@ -173,13 +177,13 @@ static struct desc_clk_cken pxa3xx_clocks[] __initdata = {
>
>  static struct desc_clk_cken pxa300_310_clocks[] __initdata = {
>
> -       PXA3XX_PBUS_CKEN("pxa3xx-gcu", NULL, PXA300_GCU, 1, 1, 1, 1, 0),
> -       PXA3XX_PBUS_CKEN("pxa3xx-nand", NULL, NAND, 1, 2, 1, 4, 0),
> +       PXA3XX_PBUS_CKENF("pxa3xx-gcu", NULL, PXA300_GCU, 1, 1, 1, 1, 0, CLK_IGNORE_UNUSED),
> +       PXA3XX_PBUS_CKENF("pxa3xx-nand", NULL, NAND, 1, 2, 1, 4, 0, CLK_IGNORE_UNUSED),
>         PXA3XX_CKEN_1RATE("pxa3xx-gpio", NULL, GPIO, pxa3xx_13MHz_bus_parents),
>  };
>
>  static struct desc_clk_cken pxa320_clocks[] __initdata = {
> -       PXA3XX_PBUS_CKEN("pxa3xx-nand", NULL, NAND, 1, 2, 1, 6, 0),
> +       PXA3XX_PBUS_CKENF("pxa3xx-nand", NULL, NAND, 1, 2, 1, 4, 0, CLK_IGNORE_UNUSED),
>         PXA3XX_PBUS_CKEN("pxa3xx-gcu", NULL, PXA320_GCU, 1, 1, 1, 1, 0),
>         PXA3XX_CKEN_1RATE("pxa3xx-gpio", NULL, GPIO, pxa3xx_13MHz_bus_parents),
>  };
> @@ -187,7 +191,7 @@ static struct desc_clk_cken pxa320_clocks[] __initdata = {
>  static struct desc_clk_cken pxa93x_clocks[] __initdata = {
>
>         PXA3XX_PBUS_CKEN("pxa3xx-gcu", NULL, PXA300_GCU, 1, 1, 1, 1, 0),
> -       PXA3XX_PBUS_CKEN("pxa3xx-nand", NULL, NAND, 1, 2, 1, 4, 0),
> +       PXA3XX_PBUS_CKENF("pxa3xx-nand", NULL, NAND, 1, 2, 1, 4, 0, CLK_IGNORE_UNUSED),
>         PXA3XX_CKEN_1RATE("pxa93x-gpio", NULL, GPIO, pxa3xx_13MHz_bus_parents),
>  };
>



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

Re: [PATCH v2 0/4] mtd: pxa3xx_nand: rework the timing setup

[Robert Jarzmik]

Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> writes:

>>
>> The issue I see is that :
>>  - there is a bug in the clk driver for pxa3xx I introduced (CKEN_AB)

>>  - and shutting down the GCU clock prevents the NAND from working (I can't
>>  explain that one yet)

Ah actually it's even trickier, but it has nothing to do with the GCU clock,
that was a wrong interpretation of the test sequence.

What actually happens is that on the platform I have, the NAND is sharing the
DFI bus with the Static Memory Controller. Now let's see what happens on
boot-up, knowing that my pxa3xx-nand is a module, not builtin :
 - the kernel boots
 - the core clock initializes
 - the ethernet card initializes (it is on the static memory controller)
 - the kernel finishes the boot sequence
   => the NAND clock is unused
   => as such, the core clock framework disables it

And here is the catch : on the next ethernet access, the system bus will be
stalled forever. The reason behind is that because the bootloader left the
"NDCR_ND_ARB_EN" bit enabled, the DFI bus sees the ethernet register access, and
asks for arbitration; as the NAND clock is down, the arbitration never happens,
and the pxa3xx is stalled forever.

The very same thing happens when you load and unload pxa3xx-nand with a platform
where it was loaded with "enable-arbiter" platform-data, and if that platform
has any driver mapped in the SMC address space (0x0 .. 0x14000000).

If you have an opportunity to load/unload a pxa3xx-nand, I'd be glad to have
someone verify this theory.

The first fix comming to my mind would be to :
 - disable the NDCR_ND_ARB_EN in the pxa3xx core bring up
 - keep enablement in pxa3xx-nand
 - ensure it is disabled on the probe error path or remove of pxa3xx-nand

Cheers.

--
Robert

PS: That also means that the fix I posted for CKENA/CKENB inversion should fix
the issues you see.

[PATCH v2 0/4] mtd: pxa3xx_nand: rework the timing setup

[Robert Jarzmik]

Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> writes:

>>
>> The issue I see is that :
>>  - there is a bug in the clk driver for pxa3xx I introduced (CKEN_AB)

>>  - and shutting down the GCU clock prevents the NAND from working (I can't
>>  explain that one yet)

Ah actually it's even trickier, but it has nothing to do with the GCU clock,
that was a wrong interpretation of the test sequence.

What actually happens is that on the platform I have, the NAND is sharing the
DFI bus with the Static Memory Controller. Now let's see what happens on
boot-up, knowing that my pxa3xx-nand is a module, not builtin :
 - the kernel boots
 - the core clock initializes
 - the ethernet card initializes (it is on the static memory controller)
 - the kernel finishes the boot sequence
   => the NAND clock is unused
   => as such, the core clock framework disables it

And here is the catch : on the next ethernet access, the system bus will be
stalled forever. The reason behind is that because the bootloader left the
"NDCR_ND_ARB_EN" bit enabled, the DFI bus sees the ethernet register access, and
asks for arbitration; as the NAND clock is down, the arbitration never happens,
and the pxa3xx is stalled forever.

The very same thing happens when you load and unload pxa3xx-nand with a platform
where it was loaded with "enable-arbiter" platform-data, and if that platform
has any driver mapped in the SMC address space (0x0 .. 0x14000000).

If you have an opportunity to load/unload a pxa3xx-nand, I'd be glad to have
someone verify this theory.

The first fix comming to my mind would be to :
 - disable the NDCR_ND_ARB_EN in the pxa3xx core bring up
 - keep enablement in pxa3xx-nand
 - ensure it is disabled on the probe error path or remove of pxa3xx-nand

Cheers.

--
Robert

PS: That also means that the fix I posted for CKENA/CKENB inversion should fix
the issues you see.

Re: [PATCH v2 0/4] mtd: pxa3xx_nand: rework the timing setup

[Ezequiel Garcia]

On 17 July 2015 at 11:36, Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> wrote:
> Hi Antoine,
>
> On 07/17/2015 10:41 AM, Antoine Tenart wrote:
>>
>> Hi guys,
>>
>> On Tue, Jul 07, 2015 at 05:08:23PM +0200, Antoine Tenart wrote:
>>>
>>>
>>> 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
>>> v7. In addition, there was absolutely no comment for more than 1 month
>>> (since June 2nd). Given this, I really expect the series to be merge
>>> soon as other series are based on this.
>>
>>
>> Any news on this series?
>>

Antoine,

Do you think you can respin this patchset and send a new one so we
can move forward? There might be still some issues with it (16-bit support),
but I think it'll be better to discuss them on a fresh patchset.

Don't forget to Cc Robert, since he's able to provide valuable input
and testing on PXA boards.

Thanks a lot,
-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar

[PATCH v2 0/4] mtd: pxa3xx_nand: rework the timing setup

[Ezequiel Garcia]

On 17 July 2015 at 11:36, Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> wrote:
> Hi Antoine,
>
> On 07/17/2015 10:41 AM, Antoine Tenart wrote:
>>
>> Hi guys,
>>
>> On Tue, Jul 07, 2015 at 05:08:23PM +0200, Antoine Tenart wrote:
>>>
>>>
>>> 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
>>> v7. In addition, there was absolutely no comment for more than 1 month
>>> (since June 2nd). Given this, I really expect the series to be merge
>>> soon as other series are based on this.
>>
>>
>> Any news on this series?
>>

Antoine,

Do you think you can respin this patchset and send a new one so we
can move forward? There might be still some issues with it (16-bit support),
but I think it'll be better to discuss them on a fresh patchset.

Don't forget to Cc Robert, since he's able to provide valuable input
and testing on PXA boards.

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