[PATCH v3 0/9] ARM: berlin: add nand support

[PATCH v3 0/9] ARM: berlin: add nand support

[Antoine Tenart]

Hi all,

This series introduces the support for the Marvell Berlin nand
controller. It is based on top of v4.0-rc1 and was tested on the
Marvell Berlin BG2Q DMP board.

The support is added into the existing pxa3xx nand controller. Some
additions were done in order to get this controller working:
- Support for a non mandatory ECC clock has been added.
- The Berlin nand controller needs to poll the status register.
- Sequences of commands are quite different in order to perform write
  operation.

Thanks!

Antoine

Changes since v2:
        - Reworked the timings setup
        - Added a explanation in the default chunk size commit
        - Ensured commit 4/10 was useless and removed it
        - Fixed typos, cosmetic

Changes since v1:
        - Added a fallback to NULL when looking for the 'nfc' clock
        - Added the clocks in the bindings documentation
        - Reworked the DT
        - Removed non needed RNDOUT command
        - Used the standard RESET command
        - Fixed the flash type matching order
        - Remove all non tested flash types (only one left)
        - Added a check to explicitly not support nand chips with small pages
        - Remove useless checks for clocks in the probing function
        - Various clean up

Antoine Tenart (9):
  mtd: pxa3xx_nand: add a non mandatory ECC clock
  Documentation: bindings: document the clocks for pxa3xx-nand
  mtd: pxa3xx_nand: add a default chunk size
  mtd: pxa3xx_nand: rework timings setup
  mtd: pxa3xx_nand: add support for the Marvell Berlin nand controller
  Documentation: bindings: add the Berlin nand controller compatible
  mtd: nand: let Marvell Berlin SoCs select the pxa3xx driver
  ARM: berlin: add BG2Q node for the nand
  ARM: berlin: enable flash on the BG2Q DMP

 .../devicetree/bindings/mtd/pxa3xx-nand.txt        |   5 +
 arch/arm/boot/dts/berlin2q-marvell-dmp.dts         |  25 ++
 arch/arm/boot/dts/berlin2q.dtsi                    |  13 +
 drivers/mtd/nand/Kconfig                           |   2 +-
 drivers/mtd/nand/pxa3xx_nand.c                     | 349 +++++++++++++++++----
 include/linux/platform_data/mtd-nand-pxa3xx.h      |   2 -
 6 files changed, 332 insertions(+), 64 deletions(-)

-- 
2.3.1

[PATCH v3 1/9] mtd: pxa3xx_nand: add a non mandatory ECC clock

[Antoine Tenart]

Some controllers (as the coming Berlin nand controller) need to enable
an ECC clock. Add support for this clock in the pxa3xx nand driver, and
leave it as non mandatory.

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

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index d00ac392d1c4..55fce9527c2e 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -180,7 +180,7 @@ struct pxa3xx_nand_info {
 	struct nand_hw_control	controller;
 	struct platform_device	 *pdev;
 
-	struct clk		*clk;
+	struct clk		*clk, *ecc_clk;
 	void __iomem		*mmio_base;
 	unsigned long		mmio_phys;
 	struct completion	cmd_complete, dev_ready;
@@ -1608,15 +1608,26 @@ static int alloc_nand_resource(struct platform_device *pdev)
 
 	spin_lock_init(&chip->controller->lock);
 	init_waitqueue_head(&chip->controller->wq);
-	info->clk = devm_clk_get(&pdev->dev, NULL);
+	info->clk = devm_clk_get(&pdev->dev, "nfc");
 	if (IS_ERR(info->clk)) {
-		dev_err(&pdev->dev, "failed to get nand clock\n");
-		return PTR_ERR(info->clk);
+		info->clk = devm_clk_get(&pdev->dev, NULL);
+
+		if (IS_ERR(info->clk)) {
+			dev_err(&pdev->dev, "failed to get nand clock\n");
+			return PTR_ERR(info->clk);
+		}
 	}
 	ret = clk_prepare_enable(info->clk);
 	if (ret < 0)
 		return ret;
 
+	info->ecc_clk = devm_clk_get(&pdev->dev, "ecc");
+	if (!IS_ERR(info->ecc_clk)) {
+		ret = clk_prepare_enable(info->ecc_clk);
+		if (ret < 0)
+			goto fail_disable_clk;
+	}
+
 	if (use_dma) {
 		/*
 		 * This is a dirty hack to make this driver work from
@@ -1633,7 +1644,7 @@ static int alloc_nand_resource(struct platform_device *pdev)
 				dev_err(&pdev->dev,
 					"no resource defined for data DMA\n");
 				ret = -ENXIO;
-				goto fail_disable_clk;
+				goto fail_disable_ecc_clk;
 			}
 			info->drcmr_dat = r->start;
 
@@ -1642,7 +1653,7 @@ static int alloc_nand_resource(struct platform_device *pdev)
 				dev_err(&pdev->dev,
 					"no resource defined for cmd DMA\n");
 				ret = -ENXIO;
-				goto fail_disable_clk;
+				goto fail_disable_ecc_clk;
 			}
 			info->drcmr_cmd = r->start;
 		}
@@ -1652,14 +1663,14 @@ static int alloc_nand_resource(struct platform_device *pdev)
 	if (irq < 0) {
 		dev_err(&pdev->dev, "no IRQ resource defined\n");
 		ret = -ENXIO;
-		goto fail_disable_clk;
+		goto fail_disable_ecc_clk;
 	}
 
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	info->mmio_base = devm_ioremap_resource(&pdev->dev, r);
 	if (IS_ERR(info->mmio_base)) {
 		ret = PTR_ERR(info->mmio_base);
-		goto fail_disable_clk;
+		goto fail_disable_ecc_clk;
 	}
 	info->mmio_phys = r->start;
 
@@ -1668,7 +1679,7 @@ static int alloc_nand_resource(struct platform_device *pdev)
 	info->data_buff = kmalloc(info->buf_size, GFP_KERNEL);
 	if (info->data_buff == NULL) {
 		ret = -ENOMEM;
-		goto fail_disable_clk;
+		goto fail_disable_ecc_clk;
 	}
 
 	/* initialize all interrupts to be disabled */
@@ -1687,6 +1698,8 @@ static int alloc_nand_resource(struct platform_device *pdev)
 fail_free_buf:
 	free_irq(irq, info);
 	kfree(info->data_buff);
+fail_disable_ecc_clk:
+	clk_disable_unprepare(info->ecc_clk);
 fail_disable_clk:
 	clk_disable_unprepare(info->clk);
 	return ret;
@@ -1709,6 +1722,7 @@ static int pxa3xx_nand_remove(struct platform_device *pdev)
 	pxa3xx_nand_free_buff(info);
 
 	clk_disable_unprepare(info->clk);
+	clk_disable_unprepare(info->ecc_clk);
 
 	for (cs = 0; cs < pdata->num_cs; cs++)
 		nand_release(info->host[cs]->mtd);
-- 
2.3.1

[PATCH v3 2/9] Documentation: bindings: document the clocks for pxa3xx-nand

[Antoine Tenart]

The pxa3xx nand driver requires at least one clock to probe correctly.
A second one, named 'ecc' can be specified if needed. Add the
corresponding documentation.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 Documentation/devicetree/bindings/mtd/pxa3xx-nand.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/pxa3xx-nand.txt b/Documentation/devicetree/bindings/mtd/pxa3xx-nand.txt
index de8b517a5521..ecd1fc071f81 100644
--- a/Documentation/devicetree/bindings/mtd/pxa3xx-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/pxa3xx-nand.txt
@@ -8,6 +8,8 @@ Required properties:
  - reg: 		The register base for the controller
  - interrupts:		The interrupt to map
  - #address-cells:	Set to <1> if the node includes partitions
+ - clocks:		A phandle to the clocks
+ - clock-names:		'nfc' and 'ecc'. The 'nfc' clock is mandatory.
 
 Optional properties:
 
@@ -32,6 +34,8 @@ Example:
 		compatible = "marvell,pxa3xx-nand";
 		reg = <0x43100000 90>;
 		interrupts = <45>;
+		clocks = <&gateclk 11>;
+		clock-names = "nfc";
 		#address-cells = <1>;
 
 		marvell,nand-enable-arbiter;
-- 
2.3.1

[PATCH v3 3/9] 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 55fce9527c2e..dc0edbc406bb 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -1429,6 +1429,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.3.1

[PATCH v3 4/9] mtd: pxa3xx_nand: rework timings setup

[Antoine Tenart]

Use the nand framework helpers: onfi_get_async_timing_mode() and
onfi_async_timing_mode_to_sdr_timings() to retrieve the timing
configuration. Then update the pxa3xx timing setup function to use the
timing configuration retrieved.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c                | 101 ++++++++++++++++++--------
 include/linux/platform_data/mtd-nand-pxa3xx.h |   2 -
 2 files changed, 69 insertions(+), 34 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index dc0edbc406bb..4bcfb4cf6fee 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -243,23 +243,16 @@ static bool use_dma = 1;
 module_param(use_dma, bool, 0444);
 MODULE_PARM_DESC(use_dma, "enable DMA for data transferring to/from NAND HW");
 
-static struct pxa3xx_nand_timing timing[] = {
-	{ 40, 80, 60, 100, 80, 100, 90000, 400, 40, },
-	{ 10,  0, 20,  40, 30,  40, 11123, 110, 10, },
-	{ 10, 25, 15,  25, 15,  30, 25000,  60, 10, },
-	{ 10, 35, 15,  25, 15,  25, 25000,  60, 10, },
-};
-
 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] },
+{ "DEFAULT FLASH",      0,   0, 2048,  8,  8,    0 },
+{ "64MiB 16-bit",  0x46ec,  32,  512, 16, 16, 4096 },
+{ "256MiB 8-bit",  0xdaec,  64, 2048,  8,  8, 2048 },
+{ "4GiB 8-bit",    0xd7ec, 128, 4096,  8,  8, 8192 },
+{ "128MiB 8-bit",  0xa12c,  64, 2048,  8,  8, 1024 },
+{ "128MiB 16-bit", 0xb12c,  64, 2048, 16, 16, 1024 },
+{ "512MiB 8-bit",  0xdc2c,  64, 2048,  8,  8, 4096 },
+{ "512MiB 16-bit", 0xcc2c,  64, 2048, 16, 16, 4096 },
+{ "256MiB 16-bit", 0xba20,  64, 2048, 16, 16, 2048 },
 };
 
 static u8 bbt_pattern[] = {'M', 'V', 'B', 'b', 't', '0' };
@@ -361,22 +354,31 @@ pxa3xx_nand_get_variant(struct platform_device *pdev)
 }
 
 static void pxa3xx_nand_set_timing(struct pxa3xx_nand_host *host,
-				   const struct pxa3xx_nand_timing *t)
+				   const struct nand_sdr_timings *t)
 {
 	struct pxa3xx_nand_info *info = host->info_data;
 	unsigned long nand_clk = clk_get_rate(info->clk);
 	uint32_t ndtr0, ndtr1;
-
-	ndtr0 = NDTR0_tCH(ns2cycle(t->tCH, nand_clk)) |
-		NDTR0_tCS(ns2cycle(t->tCS, nand_clk)) |
-		NDTR0_tWH(ns2cycle(t->tWH, nand_clk)) |
-		NDTR0_tWP(ns2cycle(t->tWP, nand_clk)) |
-		NDTR0_tRH(ns2cycle(t->tRH, nand_clk)) |
-		NDTR0_tRP(ns2cycle(t->tRP, nand_clk));
-
-	ndtr1 = NDTR1_tR(ns2cycle(t->tR, nand_clk)) |
-		NDTR1_tWHR(ns2cycle(t->tWHR, nand_clk)) |
-		NDTR1_tAR(ns2cycle(t->tAR, nand_clk));
+	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->tWP_min, 1000);
+	u32 tREH_min = DIV_ROUND_UP(t->tREH_min, 1000);
+	u32 tRP_min = DIV_ROUND_UP(t->tRP_min, 1000);
+	u32 tRST_max = DIV_ROUND_UP_ULL(t->tRST_max, 1000);
+	u32 tWHR_min = DIV_ROUND_UP(t->tWHR_min, 1000);
+	u32 tAR_min = DIV_ROUND_UP(t->tAR_min, 1000);
+
+	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(tRST_max, nand_clk)) |
+		NDTR1_tWHR(ns2cycle(tWHR_min, nand_clk)) |
+		NDTR1_tAR(ns2cycle(tAR_min, nand_clk));
 
 	info->ndtr0cs0 = ndtr0;
 	info->ndtr1cs0 = ndtr1;
@@ -384,6 +386,29 @@ static void pxa3xx_nand_set_timing(struct pxa3xx_nand_host *host,
 	nand_writel(info, NDTR1CS0, ndtr1);
 }
 
+static int pxa3xx_nand_init_timings(struct pxa3xx_nand_host *host)
+{
+	const struct nand_sdr_timings *timings;
+	int mode;
+
+	mode = onfi_get_async_timing_mode(&host->chip);
+	if (mode == ONFI_TIMING_MODE_UNKNOWN) {
+		mode = host->chip.onfi_timing_mode_default;
+	} 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_timing(host, timings);
+
+	return 0;
+}
+
 /*
  * Set the data and OOB size, depending on the selected
  * spare and ECC configuration.
@@ -1226,7 +1251,6 @@ static int pxa3xx_nand_config_flash(struct pxa3xx_nand_info *info,
 
 	info->reg_ndcr = ndcr;
 
-	pxa3xx_nand_set_timing(host, f->timing);
 	return 0;
 }
 
@@ -1321,20 +1345,28 @@ 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 mtd_info *mtd;
 	struct nand_chip *chip;
+	const struct nand_sdr_timings *timings;
 	int ret;
 
 	mtd = info->host[info->cs]->mtd;
 	chip = mtd->priv;
 
-	/* use the common timing to make a try */
 	ret = pxa3xx_nand_config_flash(info, &builtin_flash_types[0]);
 	if (ret)
 		return ret;
 
+	/* use the common timing to make a try */
+	timings = onfi_async_timing_mode_to_sdr_timings(0);
+	if (IS_ERR(timings))
+		return PTR_ERR(timings);
+
+	pxa3xx_nand_set_timing(host, timings);
+
 	chip->cmdfunc(mtd, NAND_CMD_RESET, 0, 0);
 	ret = chip->waitfunc(mtd, chip);
 	if (ret & NAND_STATUS_FAIL)
@@ -1420,6 +1452,7 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
 	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;
+	const struct nand_sdr_timings *timings;
 	struct nand_chip *chip = mtd->priv;
 	uint32_t id = -1;
 	uint64_t chipsize;
@@ -1432,7 +1465,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);
@@ -1498,6 +1531,10 @@ KEEP_CONFIG:
 	if (nand_scan_ident(mtd, 1, def))
 		return -ENODEV;
 
+	ret = pxa3xx_nand_init_timings(host);
+	if (ret)
+		dev_err(&info->pdev->dev, "Failed to set timings: %d\n", ret);
+
 	if (pdata->flash_bbt) {
 		/*
 		 * We'll use a bad block table stored in-flash and don't
diff --git a/include/linux/platform_data/mtd-nand-pxa3xx.h b/include/linux/platform_data/mtd-nand-pxa3xx.h
index ac4ea2e641c7..27d8f6354c2a 100644
--- a/include/linux/platform_data/mtd-nand-pxa3xx.h
+++ b/include/linux/platform_data/mtd-nand-pxa3xx.h
@@ -24,8 +24,6 @@ struct pxa3xx_nand_flash {
 	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 */
 };
 
 /*
-- 
2.3.1

[PATCH v3 5/9] mtd: pxa3xx_nand: add support for the Marvell Berlin nand controller

[Antoine Tenart]

The nand controller on Marvell Berlin SoC reuse the pxa3xx nand driver
as it quite close. The process of sending commands can be compared to
the one of the Marvell armada 370: read and write commands are done in
chunks.

But the Berlin nand controller has some other specificities which
require some modifications of the pxa3xx nand driver:
- there are no IRQ available so we need to poll the status register: we
  have to use our own cmdfunc Berlin function, and early on the probing
  function.
- PAGEPROG are very different from the one used in the pxa3xx driver,
  so we're using a specific process for this one
- the SEQIN command is equivalent to a READ0 command

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

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 4bcfb4cf6fee..983c13c80c55 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -109,6 +109,8 @@
 #define NDCB0_EXT_CMD_TYPE(x)	(((x) << 29) & NDCB0_EXT_CMD_TYPE_MASK)
 #define NDCB0_CMD_TYPE_MASK	(0x7 << 21)
 #define NDCB0_CMD_TYPE(x)	(((x) << 21) & NDCB0_CMD_TYPE_MASK)
+#define NDCB0_CMD_XTYPE_MASK	(0x7 << 29)
+#define NDCB0_CMD_XTYPE(x)	(((x) << 29) & NDCB0_CMD_XTYPE_MASK)
 #define NDCB0_NC		(0x1 << 20)
 #define NDCB0_DBC		(0x1 << 19)
 #define NDCB0_ADDR_CYC_MASK	(0x7 << 16)
@@ -117,13 +119,15 @@
 #define NDCB0_CMD1_MASK		(0xff)
 #define NDCB0_ADDR_CYC_SHIFT	(16)
 
-#define EXT_CMD_TYPE_DISPATCH	6 /* Command dispatch */
-#define EXT_CMD_TYPE_NAKED_RW	5 /* Naked read or Naked write */
-#define EXT_CMD_TYPE_READ	4 /* Read */
-#define EXT_CMD_TYPE_DISP_WR	4 /* Command dispatch with write */
-#define EXT_CMD_TYPE_FINAL	3 /* Final command */
-#define EXT_CMD_TYPE_LAST_RW	1 /* Last naked read/write */
-#define EXT_CMD_TYPE_MONO	0 /* Monolithic read/write */
+#define EXT_CMD_TYPE_LAST_PAGEPROG	8
+#define EXT_CMD_TYPE_CHUNK_PAGEPROG	7
+#define EXT_CMD_TYPE_DISPATCH		6 /* Command dispatch */
+#define EXT_CMD_TYPE_NAKED_RW		5 /* Naked read or Naked write */
+#define EXT_CMD_TYPE_READ		4 /* Read */
+#define EXT_CMD_TYPE_DISP_WR		4 /* Command dispatch with write */
+#define EXT_CMD_TYPE_FINAL		3 /* Final command */
+#define EXT_CMD_TYPE_LAST_RW		1 /* Last naked read/write */
+#define EXT_CMD_TYPE_MONO		0 /* Monolithic read/write */
 
 /* macros for registers read/write */
 #define nand_writel(info, off, val)	\
@@ -158,6 +162,7 @@ enum {
 enum pxa3xx_nand_variant {
 	PXA3XX_NAND_VARIANT_PXA,
 	PXA3XX_NAND_VARIANT_ARMADA370,
+	PXA3XX_NAND_VARIANT_BERLIN2,
 };
 
 struct pxa3xx_nand_host {
@@ -253,6 +258,12 @@ static struct pxa3xx_nand_flash builtin_flash_types[] = {
 { "512MiB 8-bit",  0xdc2c,  64, 2048,  8,  8, 4096 },
 { "512MiB 16-bit", 0xcc2c,  64, 2048, 16, 16, 4096 },
 { "256MiB 16-bit", 0xba20,  64, 2048, 16, 16, 2048 },
+{ }
+};
+
+static struct pxa3xx_nand_flash berlin_builtin_flash_types[] = {
+{ "4GiB 8-bit",    0xd7ec, 128, 8192,  8,  8, 4096 },
+{ },
 };
 
 static u8 bbt_pattern[] = {'M', 'V', 'B', 'b', 't', '0' };
@@ -313,6 +324,18 @@ static struct nand_ecclayout ecc_layout_4KB_bch8bit = {
 	.oobfree = { }
 };
 
+static struct nand_ecclayout ecc_layout_oob_128 = {
+	.eccbytes = 48,
+	.eccpos = {
+		80, 81, 82, 83, 84, 85, 86, 87,
+		88, 89, 90, 91, 92, 93, 94, 95,
+		96, 97, 98, 99, 100, 101, 102, 103,
+		104, 105, 106, 107, 108, 109, 110, 111,
+		112, 113, 114, 115, 116, 117, 118, 119,
+		120, 121, 122, 123, 124, 125, 126, 127},
+	.oobfree = { {.offset = 2, .length = 78} }
+};
+
 /* Define a default flash type setting serve as flash detecting only */
 #define DEFAULT_FLASH_TYPE (&builtin_flash_types[0])
 
@@ -339,6 +362,10 @@ static const struct of_device_id pxa3xx_nand_dt_ids[] = {
 		.compatible = "marvell,armada370-nand",
 		.data       = (void *)PXA3XX_NAND_VARIANT_ARMADA370,
 	},
+	{
+		.compatible = "marvell,berlin2-nand",
+		.data       = (void *)PXA3XX_NAND_VARIANT_BERLIN2,
+	},
 	{}
 };
 MODULE_DEVICE_TABLE(of, pxa3xx_nand_dt_ids);
@@ -670,7 +697,8 @@ static irqreturn_t pxa3xx_nand_irq(int irq, void *devid)
 		nand_writel(info, NDCB0, info->ndcb2);
 
 		/* NDCB3 register is available in NFCv2 (Armada 370/XP SoC) */
-		if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
+		if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370 ||
+				info->variant == PXA3XX_NAND_VARIANT_BERLIN2)
 			nand_writel(info, NDCB0, info->ndcb3);
 	}
 
@@ -780,6 +808,16 @@ static int prepare_set_command(struct pxa3xx_nand_info *info, int command,
 	if (command == NAND_CMD_SEQIN)
 		exec_cmd = 0;
 
+	/* Berlin specific */
+	if (info->variant == PXA3XX_NAND_VARIANT_BERLIN2) {
+		if ((command == NAND_CMD_READ0 && !ext_cmd_type) ||
+				command == NAND_CMD_READOOB)
+			exec_cmd = 0;
+
+		if (command == NAND_CMD_SEQIN)
+			command = NAND_CMD_READ0;
+	}
+
 	addr_cycle = NDCB0_ADDR_CYC(host->row_addr_cycles
 				    + host->col_addr_cycles);
 
@@ -839,6 +877,37 @@ static int prepare_set_command(struct pxa3xx_nand_info *info, int command,
 			break;
 		}
 
+		if (info->variant == PXA3XX_NAND_VARIANT_BERLIN2) {
+			if (ext_cmd_type == EXT_CMD_TYPE_LAST_PAGEPROG) {
+				info->ndcb0 |= NDCB0_CMD_TYPE(0x1)
+						| NDCB0_CMD_XTYPE(0x3)
+						| NDCB0_ST_ROW_EN
+						| NDCB0_DBC
+						| (NAND_CMD_PAGEPROG << 8);
+			} else if (ext_cmd_type == EXT_CMD_TYPE_CHUNK_PAGEPROG) {
+				info->ndcb0 |= NDCB0_CMD_TYPE(0x1)
+						| NDCB0_CMD_XTYPE(0x5)
+						| NDCB0_NC
+						| NDCB0_AUTO_RS
+						| NDCB0_LEN_OVRD
+						| (NAND_CMD_PAGEPROG << 8)
+						| NAND_CMD_SEQIN;
+				info->ndcb3 = info->chunk_size;
+			} else {
+				info->ndcb0 |= NDCB0_CMD_TYPE(0x1)
+						| NDCB0_CMD_XTYPE(0x4)
+						| NDCB0_NC
+						| NDCB0_AUTO_RS
+						| NDCB0_LEN_OVRD
+						| addr_cycle
+						| (NAND_CMD_PAGEPROG << 8)
+						| NAND_CMD_SEQIN;
+				info->ndcb3 = info->chunk_size;
+			}
+
+			break;
+		}
+
 		/* Second command setting for large pages */
 		if (mtd->writesize > PAGE_CHUNK_SIZE) {
 			/*
@@ -895,6 +964,7 @@ static int prepare_set_command(struct pxa3xx_nand_info *info, int command,
 
 		info->data_size = 8;
 		break;
+
 	case NAND_CMD_STATUS:
 		info->buf_count = 1;
 		info->ndcb0 |= NDCB0_CMD_TYPE(4)
@@ -1103,6 +1173,89 @@ static int pxa3xx_nand_write_page_hwecc(struct mtd_info *mtd,
 	return 0;
 }
 
+static void nand_cmdfunc_berlin(struct mtd_info *mtd, const unsigned command,
+		int column, int page_addr)
+{
+	struct pxa3xx_nand_host *host = mtd->priv;
+	struct pxa3xx_nand_info *info = host->info_data;
+	unsigned long timeout;
+	int exec_cmd, ext_cmd_type = 0;
+	unsigned cmd = command;
+
+	if (info->reg_ndcr & NDCR_DWIDTH_M)
+		column /= 2;
+
+	/*
+	 * There may be different NAND chip hooked to
+	 * different chip select, so check whether
+	 * chip select has been changed, if yes, reset the timing
+	 */
+	if (info->cs != host->cs) {
+		info->cs = host->cs;
+		nand_writel(info, NDTR0CS0, info->ndtr0cs0);
+		nand_writel(info, NDTR1CS0, info->ndtr1cs0);
+	}
+
+	prepare_start_command(info, cmd);
+
+	info->need_wait = 1;
+	init_completion(&info->dev_ready);
+
+	pxa3xx_nand_start(info);
+
+	do {
+		init_completion(&info->cmd_complete);
+		info->state = STATE_PREPARED;
+		exec_cmd = prepare_set_command(info, cmd, ext_cmd_type,
+				column, page_addr);
+
+		if (cmd == NAND_CMD_READ0 && !ext_cmd_type) {
+			ext_cmd_type = NDCB0_CMD_XTYPE(0x5);
+			continue;
+		}
+
+		if (!exec_cmd) {
+			info->need_wait = 0;
+			complete(&info->dev_ready);
+			break;
+		}
+
+		/* no IRQ, poll */
+		timeout = jiffies + CHIP_DELAY_TIMEOUT;
+		do {
+			pxa3xx_nand_irq(0, info);
+
+			if (cmd == NAND_CMD_PAGEPROG &&
+					ext_cmd_type != EXT_CMD_TYPE_LAST_PAGEPROG)
+				break;
+
+			if (time_after(jiffies, timeout))
+				goto berlin_timeout;
+		} while (!completion_done(&info->cmd_complete));
+
+		/* sequence completed */
+		if (info->data_size == 0)
+			break;
+
+		if (cmd == NAND_CMD_PAGEPROG &&
+				ext_cmd_type == EXT_CMD_TYPE_LAST_PAGEPROG) {
+			complete(&info->dev_ready);
+			break;
+		}
+
+		if (cmd == NAND_CMD_PAGEPROG) {
+			/* last command */
+			if (info->data_size == info->chunk_size * 2)
+				ext_cmd_type = EXT_CMD_TYPE_LAST_PAGEPROG;
+			else
+				ext_cmd_type = EXT_CMD_TYPE_CHUNK_PAGEPROG;
+		}
+	} while (1);
+
+berlin_timeout:
+	info->state = STATE_IDLE;
+}
+
 static int pxa3xx_nand_read_page_hwecc(struct mtd_info *mtd,
 		struct nand_chip *chip, uint8_t *buf, int oob_required,
 		int page)
@@ -1218,8 +1371,10 @@ static int pxa3xx_nand_config_flash(struct pxa3xx_nand_info *info,
 	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");
+	if (f->page_size != 8192 && f->page_size != 2048
+			&& f->page_size != 512) {
+		dev_err(&pdev->dev,
+			"Current only support 8192, 2048 and 512 size\n");
 		return -EINVAL;
 	}
 
@@ -1432,6 +1587,16 @@ static int pxa_ecc_init(struct pxa3xx_nand_info *info,
 		ecc->size = info->chunk_size;
 		ecc->layout = &ecc_layout_4KB_bch8bit;
 		ecc->strength = 16;
+	} else if (strength == 48 && ecc_stepsize == 1024 &&
+			page_size == 8192) {
+		info->ecc_bch = 1;
+		info->chunk_size = 2048;
+		info->spare_size = 0;
+		info->ecc_size = 32;
+		ecc->mode = NAND_ECC_HW;
+		ecc->size = info->chunk_size;
+		ecc->layout = &ecc_layout_oob_128;
+		ecc->strength = 48;
 	} else {
 		dev_err(&info->pdev->dev,
 			"ECC strength %d at page size %d is not supported\n",
@@ -1451,12 +1616,12 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
 	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;
+	const struct pxa3xx_nand_flash *flash_types = NULL, *f = NULL;
 	const struct nand_sdr_timings *timings;
 	struct nand_chip *chip = mtd->priv;
 	uint32_t id = -1;
 	uint64_t chipsize;
-	int i, ret, num;
+	int i, ret;
 	uint16_t ecc_strength, ecc_step;
 
 	if (pdata->keep_config && !pxa3xx_nand_detect_config(info))
@@ -1465,6 +1630,9 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
 	/* Set a default chunk size */
 	info->chunk_size = 512;
 
+	if (info->variant == PXA3XX_NAND_VARIANT_BERLIN2)
+		chip->cmdfunc = nand_cmdfunc_berlin;
+
 	ret = pxa3xx_nand_sensing(host);
 	if (ret) {
 		dev_info(&info->pdev->dev, "There is no chip on cs %d!\n",
@@ -1484,19 +1652,24 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
 		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];
+	if (info->variant == PXA3XX_NAND_VARIANT_BERLIN2)
+		flash_types = berlin_builtin_flash_types;
+	else
+		flash_types = builtin_flash_types;
 
-		/* find the chip in default list */
+	for (i = 0; i < pdata->num_flash; i++) {
+		f = pdata->flash + i;
 		if (f->chip_id == id)
 			break;
 	}
 
-	if (i >= (ARRAY_SIZE(builtin_flash_types) + pdata->num_flash - 1)) {
+	if (!f) {
+		for (i = 0; (f = &flash_types[i]); i++)
+			if (f->chip_id == id)
+				break;
+	}
+
+	if (!f) {
 		dev_err(&info->pdev->dev, "ERROR!! flash not defined!!!\n");
 
 		return -EINVAL;
@@ -1554,7 +1727,7 @@ KEEP_CONFIG:
 	if (mtd->writesize > PAGE_CHUNK_SIZE) {
 		if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370) {
 			chip->cmdfunc = nand_cmdfunc_extended;
-		} else {
+		} else if (info->variant != PXA3XX_NAND_VARIANT_BERLIN2) {
 			dev_err(&info->pdev->dev,
 				"unsupported page size on this variant\n");
 			return -ENODEV;
-- 
2.3.1

[PATCH v3 6/9] Documentation: bindings: add the Berlin nand controller compatible

[Antoine Tenart]

The Berlin nand controller support was introduced using the existing
pxa3xx nand driver. Add the Berlin specific compatible into the
documentation.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 Documentation/devicetree/bindings/mtd/pxa3xx-nand.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/mtd/pxa3xx-nand.txt b/Documentation/devicetree/bindings/mtd/pxa3xx-nand.txt
index ecd1fc071f81..f33ee0aa0a2e 100644
--- a/Documentation/devicetree/bindings/mtd/pxa3xx-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/pxa3xx-nand.txt
@@ -5,6 +5,7 @@ Required properties:
  - compatible:		Should be set to one of the following:
 			marvell,pxa3xx-nand
 			marvell,armada370-nand
+			marvell,berlin2-nand
  - reg: 		The register base for the controller
  - interrupts:		The interrupt to map
  - #address-cells:	Set to <1> if the node includes partitions
-- 
2.3.1

[PATCH v3 7/9] mtd: nand: let Marvell Berlin SoCs select the pxa3xx driver

[Antoine Tenart]

Marvell Berlin nand controller support has been added in the pxa3xx nand
driver. Let these SoCs select the driver.

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

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 5b76a173cd95..cfe7f223f48f 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -332,7 +332,7 @@ config MTD_NAND_ATMEL
 
 config MTD_NAND_PXA3xx
 	tristate "NAND support on PXA3xx and Armada 370/XP"
-	depends on PXA3xx || ARCH_MMP || PLAT_ORION
+	depends on PXA3xx || ARCH_MMP || PLAT_ORION || ARCH_BERLIN
 	help
 	  This enables the driver for the NAND flash device found on
 	  PXA3xx processors (NFCv1) and also on Armada 370/XP (NFCv2).
-- 
2.3.1

[PATCH v3 8/9] ARM: berlin: add BG2Q node for the nand

[Antoine Tenart]

Add a node describing the nand controller of the Marvell Berlin BG2Q
SoC. It uses the pxa3xx nand driver, with a dedicated compatible.

Also add the corresponding pinmuxing configuration.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 arch/arm/boot/dts/berlin2q.dtsi | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi
index be5397288d24..aeb3a0ea0540 100644
--- a/arch/arm/boot/dts/berlin2q.dtsi
+++ b/arch/arm/boot/dts/berlin2q.dtsi
@@ -437,6 +437,19 @@
 			status = "disabled";
 		};
 
+		nand0: nand@f00000 {
+			compatible = "marvell,berlin2-nand";
+			reg = <0xf00000 0x10000>;
+			interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&chip CLKID_NFC>,
+				 <&chip CLKID_NFC_ECC>;
+			clock-names = "nfc", "ecc";
+
+			#address-cells = <1>;
+
+			status = "disabled";
+		};
+
 		apb@fc0000 {
 			compatible = "simple-bus";
 			#address-cells = <1>;
-- 
2.3.1

[PATCH v3 9/9] ARM: berlin: enable flash on the BG2Q DMP

[Antoine Tenart]

The BG2Q DMP has a nand controller. Add the corresponding node, but do
not enable it by default because the nand is only available on some BG2Q
DMP.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 arch/arm/boot/dts/berlin2q-marvell-dmp.dts | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2q-marvell-dmp.dts b/arch/arm/boot/dts/berlin2q-marvell-dmp.dts
index a98ac1bd8f65..4157258a6fb0 100644
--- a/arch/arm/boot/dts/berlin2q-marvell-dmp.dts
+++ b/arch/arm/boot/dts/berlin2q-marvell-dmp.dts
@@ -112,3 +112,28 @@
 &sata_phy {
 	status = "okay";
 };
+
+&chip {
+	nand_pmux: nand-pmux {
+		groups = "G0", "G1";
+		function = "nand";
+	};
+};
+
+&nand0 {
+	pinctrl-0 = <&nand_pmux>;
+	pinctrl-names = "default";
+
+	marvell,nand-enable-arbiter;
+	nand-ecc-strength = <48>;
+	nand-ecc-step-size = <1024>;
+
+	num-cs = <1>;
+
+	/*
+	 * Two kinds of BG2Q DMP are available: one with emmc and
+	 * one with a nand. The latter is not widespread. Because
+	 * of this, disable the nand by default.
+	 */
+	status = "disabled";
+};
-- 
2.3.1

Re: [PATCH v3 4/9] mtd: pxa3xx_nand: rework timings setup

[Thomas Petazzoni]

Dear Antoine Tenart,

On Thu,  5 Mar 2015 12:31:20 +0100, Antoine Tenart wrote:
> Use the nand framework helpers: onfi_get_async_timing_mode() and
> onfi_async_timing_mode_to_sdr_timings() to retrieve the timing
> configuration. Then update the pxa3xx timing setup function to use the
> timing configuration retrieved.
> 
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> ---
>  drivers/mtd/nand/pxa3xx_nand.c                | 101 ++++++++++++++++++--------
>  include/linux/platform_data/mtd-nand-pxa3xx.h |   2 -
>  2 files changed, 69 insertions(+), 34 deletions(-)

After this commit, the pxa3xx_nand_timing structure type is no longer
used as far as I understand. So maybe it can be removed from
include/linux/platform_data/mtd-nand-pxa3xx.h ?

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

Re: [PATCH v3 4/9] mtd: pxa3xx_nand: rework timings setup

[Antoine Tenart]

Thomas,

On Thu, Mar 05, 2015 at 01:50:23PM +0100, Thomas Petazzoni wrote:
> On Thu,  5 Mar 2015 12:31:20 +0100, Antoine Tenart wrote:
> > Use the nand framework helpers: onfi_get_async_timing_mode() and
> > onfi_async_timing_mode_to_sdr_timings() to retrieve the timing
> > configuration. Then update the pxa3xx timing setup function to use the
> > timing configuration retrieved.
> > 
> > Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> > ---
> >  drivers/mtd/nand/pxa3xx_nand.c                | 101 ++++++++++++++++++--------
> >  include/linux/platform_data/mtd-nand-pxa3xx.h |   2 -
> >  2 files changed, 69 insertions(+), 34 deletions(-)
> 
> After this commit, the pxa3xx_nand_timing structure type is no longer
> used as far as I understand. So maybe it can be removed from
> include/linux/platform_data/mtd-nand-pxa3xx.h ?

Right, I'll remove it.

Antoine

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

Re: [PATCH v3 5/9] mtd: pxa3xx_nand: add support for the Marvell Berlin nand controller

[Thomas Petazzoni]

Dear Antoine Tenart,

On Thu,  5 Mar 2015 12:31:21 +0100, Antoine Tenart wrote:

>  struct pxa3xx_nand_host {
> @@ -253,6 +258,12 @@ static struct pxa3xx_nand_flash builtin_flash_types[] = {
>  { "512MiB 8-bit",  0xdc2c,  64, 2048,  8,  8, 4096 },
>  { "512MiB 16-bit", 0xcc2c,  64, 2048, 16, 16, 4096 },
>  { "256MiB 16-bit", 0xba20,  64, 2048, 16, 16, 2048 },
> +{ }
> +};
> +
> +static struct pxa3xx_nand_flash berlin_builtin_flash_types[] = {
> +{ "4GiB 8-bit",    0xd7ec, 128, 8192,  8,  8, 4096 },
> +{ },

This looks fishy. You know have two different definitions for the exact
same chip_id. In the builtin_flash_types[] array:

{ "4GiB 8-bit",    0xd7ec, 128, 4096,  8,  8, 8192 },

and in your new berlin_builtin_flash_types[] array:

{ "4GiB 8-bit",    0xd7ec, 128, 8192,  8,  8, 4096 },

So you have twice a big pages, and twice as less blocks. Are you sure
about your definition of the 0xd7ec NAND chip_id ?

Why cannot you use the same data for both the Berlin platform and the
platforms already supported by the driver? Are you sure your NAND isn't
using 4k pages ? Or maybe the 0xd7ec entry in builtin_flash_types[] is
incorrect?

Or maybe like
http://lists.infradead.org/pipermail/linux-mtd/2010-June/031159.html,
the NAND chip_id is the same, but the NAND ext id is different.

Is there no common NAND mechanism to handle this, rather than having
this specifically in the driver?

Best regards,

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

Re: [PATCH v3 5/9] mtd: pxa3xx_nand: add support for the Marvell Berlin nand controller

[Antoine Tenart]

Thomas,

On Thu, Mar 05, 2015 at 02:00:00PM +0100, Thomas Petazzoni wrote:
> On Thu,  5 Mar 2015 12:31:21 +0100, Antoine Tenart wrote:
> 
> >  struct pxa3xx_nand_host {
> > @@ -253,6 +258,12 @@ static struct pxa3xx_nand_flash builtin_flash_types[] = {
> >  { "512MiB 8-bit",  0xdc2c,  64, 2048,  8,  8, 4096 },
> >  { "512MiB 16-bit", 0xcc2c,  64, 2048, 16, 16, 4096 },
> >  { "256MiB 16-bit", 0xba20,  64, 2048, 16, 16, 2048 },
> > +{ }
> > +};
> > +
> > +static struct pxa3xx_nand_flash berlin_builtin_flash_types[] = {
> > +{ "4GiB 8-bit",    0xd7ec, 128, 8192,  8,  8, 4096 },
> > +{ },
> 
> This looks fishy. You know have two different definitions for the exact
> same chip_id. In the builtin_flash_types[] array:
> 
> { "4GiB 8-bit",    0xd7ec, 128, 4096,  8,  8, 8192 },
> 
> and in your new berlin_builtin_flash_types[] array:
> 
> { "4GiB 8-bit",    0xd7ec, 128, 8192,  8,  8, 4096 },
> 
> So you have twice a big pages, and twice as less blocks. Are you sure
> about your definition of the 0xd7ec NAND chip_id ?
> 
> Why cannot you use the same data for both the Berlin platform and the
> platforms already supported by the driver? Are you sure your NAND isn't
> using 4k pages ? Or maybe the 0xd7ec entry in builtin_flash_types[] is
> incorrect?
> 
> Or maybe like
> http://lists.infradead.org/pipermail/linux-mtd/2010-June/031159.html,
> the NAND chip_id is the same, but the NAND ext id is different.
> 
> Is there no common NAND mechanism to handle this, rather than having
> this specifically in the driver?

I totally agree, this is one more thing wrong with this driver. Using a
ext id would solve the issue here.

If a NULL table is given to the nand_get_flash_type() function, then the
nand_flash_ids is used. This should be handled this way.

Antoine

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

Re: [PATCH v3 5/9] mtd: pxa3xx_nand: add support for the Marvell Berlin nand controller

[Ezequiel Garcia]

Hi Antoine,

On 03/05/2015 08:31 AM, Antoine Tenart wrote:
[..]
> +
> +static struct pxa3xx_nand_flash berlin_builtin_flash_types[] = {
> +{ "4GiB 8-bit",    0xd7ec, 128, 8192,  8,  8, 4096 },
> +{ },

IMHO, supporting a specific flash shouldn't be part of this patch.

In any case, why do you need this? If you can share the details about
this device, it would be interesting for me to take a look.

This driver's open-coded, legacy-style flash detection is nasty, and the
only reason I've kept it is to avoid breaking some wacky user with some
old board. In fact, maybe we can just kill it so nobody thinks it's sane.

Flash detection is the NAND core's job, and duplicating it in the driver
is not nice. Let's try to avoid it.

BTW, nand_ids.c seems to list a similar device:

  EXTENDED_ID_NAND("NAND 4GiB 3,3V 8-bit",  0xD7, 4096, LP_OPTIONS),

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

Re: [PATCH v3 5/9] mtd: pxa3xx_nand: add support for the Marvell Berlin nand controller

[Robert Jarzmik]

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

> Hi Antoine,
>
> On 03/05/2015 08:31 AM, Antoine Tenart wrote:
> [..]
>> +
>> +static struct pxa3xx_nand_flash berlin_builtin_flash_types[] = {
>> +{ "4GiB 8-bit",    0xd7ec, 128, 8192,  8,  8, 4096 },
>> +{ },
>
> IMHO, supporting a specific flash shouldn't be part of this patch.
>
> In any case, why do you need this? If you can share the details about
> this device, it would be interesting for me to take a look.
>
> This driver's open-coded, legacy-style flash detection is nasty, and the
> only reason I've kept it is to avoid breaking some wacky user with some
> old board. In fact, maybe we can just kill it so nobody thinks it's sane.

I think you'll kill the zylonite board, and I'll nack it if that's the case. At
least that's what happened when I tried to use onfi default values last time in
barebox development.

I can test your changes, but if the specific zylonite nand (ie. nand id 0xba20,
ie. pxa310 embedded flash) gets broken, I'm against the removal of the legacy
timings removal.

And all of this because zylonite is my main vehicle as pxa3xx maintainance.

Cheers.

--
Robert

Re: [PATCH v3 5/9] mtd: pxa3xx_nand: add support for the Marvell Berlin nand controller

[Ezequiel Garcia]

On 03/08/2015 02:14 PM, Robert Jarzmik wrote:
> Ezequiel Garcia <ezequiel.garcia@free-electrons.com> writes:
> 
>> Hi Antoine,
>>
>> On 03/05/2015 08:31 AM, Antoine Tenart wrote:
>> [..]
>>> +
>>> +static struct pxa3xx_nand_flash berlin_builtin_flash_types[] = {
>>> +{ "4GiB 8-bit",    0xd7ec, 128, 8192,  8,  8, 4096 },
>>> +{ },
>>
>> IMHO, supporting a specific flash shouldn't be part of this patch.
>>
>> In any case, why do you need this? If you can share the details about
>> this device, it would be interesting for me to take a look.
>>
>> This driver's open-coded, legacy-style flash detection is nasty, and the
>> only reason I've kept it is to avoid breaking some wacky user with some
>> old board. In fact, maybe we can just kill it so nobody thinks it's sane.
> 
> I think you'll kill the zylonite board, and I'll nack it if that's the case. At
> least that's what happened when I tried to use onfi default values last time in
> barebox development.
> 
> I can test your changes, but if the specific zylonite nand (ie. nand id 0xba20,
> ie. pxa310 embedded flash) gets broken, I'm against the removal of the legacy
> timings removal.
>

I'm not speaking of any timing params here, but about the flash
identification.

Which flash do you have there?

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

Re: [PATCH v3 5/9] mtd: pxa3xx_nand: add support for the Marvell Berlin nand controller

[Robert Jarzmik]

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

>> I think you'll kill the zylonite board, and I'll nack it if that's the case. At
>> least that's what happened when I tried to use onfi default values last time in
>> barebox development.
>> 
>> I can test your changes, but if the specific zylonite nand (ie. nand id 0xba20,
>> ie. pxa310 embedded flash) gets broken, I'm against the removal of the legacy
>> timings removal.
>>
>
> I'm not speaking of any timing params here, but about the flash
> identification.
>
> Which flash do you have there?
The one with 0xba20 id as I said, which is AFAIK a Numonyx NAND02GR4B2C.

And of course a "READ_ID(ONFI)" doesn't work (tested)... sic ... as showed in :
http://www.linux-mtd.infradead.org/nand-data/nanddata.html

Cheers.

-- 
Robert

Re: [PATCH v3 5/9] mtd: pxa3xx_nand: add support for the Marvell Berlin nand controller

[Ezequiel Garcia]

On 03/08/2015 07:19 PM, Robert Jarzmik wrote:
> Ezequiel Garcia <ezequiel.garcia@free-electrons.com> writes:
> 
>>> I think you'll kill the zylonite board, and I'll nack it if that's the case. At
>>> least that's what happened when I tried to use onfi default values last time in
>>> barebox development.
>>>
>>> I can test your changes, but if the specific zylonite nand (ie. nand id 0xba20,
>>> ie. pxa310 embedded flash) gets broken, I'm against the removal of the legacy
>>> timings removal.
>>>
>>
>> I'm not speaking of any timing params here, but about the flash
>> identification.
>>
>> Which flash do you have there?
> The one with 0xba20 id as I said, which is AFAIK a Numonyx NAND02GR4B2C.
> 

$ grep "0xBA" drivers/mtd/nand/nand_ids.c
EXTENDED_ID_NAND("NAND 256MiB 1,8V 16-bit", 0xBA, 256, LP_OPTIONS16),

Seems already supported by the NAND core. The MTD way of probing
a non-ONFI device is by using the IDs in nand_ids.c. Additional
configuration (timings in this case) is applied between the nand_scan_ident()
and nand_scan_tail() calls.

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

Re: [PATCH v3 4/9] mtd: pxa3xx_nand: rework timings setup

[Ezequiel Garcia]

On 03/05/2015 08:31 AM, Antoine Tenart wrote:
> Use the nand framework helpers: onfi_get_async_timing_mode() and
> onfi_async_timing_mode_to_sdr_timings() to retrieve the timing
> configuration. Then update the pxa3xx timing setup function to use the
> timing configuration retrieved.
> 
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> ---
>  drivers/mtd/nand/pxa3xx_nand.c                | 101 ++++++++++++++++++--------
>  include/linux/platform_data/mtd-nand-pxa3xx.h |   2 -
>  2 files changed, 69 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index dc0edbc406bb..4bcfb4cf6fee 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -243,23 +243,16 @@ static bool use_dma = 1;
>  module_param(use_dma, bool, 0444);
>  MODULE_PARM_DESC(use_dma, "enable DMA for data transferring to/from NAND HW");
>  
> -static struct pxa3xx_nand_timing timing[] = {
> -	{ 40, 80, 60, 100, 80, 100, 90000, 400, 40, },
> -	{ 10,  0, 20,  40, 30,  40, 11123, 110, 10, },
> -	{ 10, 25, 15,  25, 15,  30, 25000,  60, 10, },
> -	{ 10, 35, 15,  25, 15,  25, 25000,  60, 10, },
> -};
> -
>  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] },
> +{ "DEFAULT FLASH",      0,   0, 2048,  8,  8,    0 },
> +{ "64MiB 16-bit",  0x46ec,  32,  512, 16, 16, 4096 },
> +{ "256MiB 8-bit",  0xdaec,  64, 2048,  8,  8, 2048 },
> +{ "4GiB 8-bit",    0xd7ec, 128, 4096,  8,  8, 8192 },
> +{ "128MiB 8-bit",  0xa12c,  64, 2048,  8,  8, 1024 },
> +{ "128MiB 16-bit", 0xb12c,  64, 2048, 16, 16, 1024 },
> +{ "512MiB 8-bit",  0xdc2c,  64, 2048,  8,  8, 4096 },
> +{ "512MiB 16-bit", 0xcc2c,  64, 2048, 16, 16, 4096 },
> +{ "256MiB 16-bit", 0xba20,  64, 2048, 16, 16, 2048 },
>  };
>  
>  static u8 bbt_pattern[] = {'M', 'V', 'B', 'b', 't', '0' };
> @@ -361,22 +354,31 @@ pxa3xx_nand_get_variant(struct platform_device *pdev)
>  }
>  
>  static void pxa3xx_nand_set_timing(struct pxa3xx_nand_host *host,
> -				   const struct pxa3xx_nand_timing *t)
> +				   const struct nand_sdr_timings *t)
>  {
>  	struct pxa3xx_nand_info *info = host->info_data;
>  	unsigned long nand_clk = clk_get_rate(info->clk);
>  	uint32_t ndtr0, ndtr1;
> -
> -	ndtr0 = NDTR0_tCH(ns2cycle(t->tCH, nand_clk)) |
> -		NDTR0_tCS(ns2cycle(t->tCS, nand_clk)) |
> -		NDTR0_tWH(ns2cycle(t->tWH, nand_clk)) |
> -		NDTR0_tWP(ns2cycle(t->tWP, nand_clk)) |
> -		NDTR0_tRH(ns2cycle(t->tRH, nand_clk)) |
> -		NDTR0_tRP(ns2cycle(t->tRP, nand_clk));
> -
> -	ndtr1 = NDTR1_tR(ns2cycle(t->tR, nand_clk)) |
> -		NDTR1_tWHR(ns2cycle(t->tWHR, nand_clk)) |
> -		NDTR1_tAR(ns2cycle(t->tAR, nand_clk));
> +	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->tWP_min, 1000);
> +	u32 tREH_min = DIV_ROUND_UP(t->tREH_min, 1000);
> +	u32 tRP_min = DIV_ROUND_UP(t->tRP_min, 1000);
> +	u32 tRST_max = DIV_ROUND_UP_ULL(t->tRST_max, 1000);
> +	u32 tWHR_min = DIV_ROUND_UP(t->tWHR_min, 1000);
> +	u32 tAR_min = DIV_ROUND_UP(t->tAR_min, 1000);
> +
> +	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(tRST_max, nand_clk)) |
> +		NDTR1_tWHR(ns2cycle(tWHR_min, nand_clk)) |
> +		NDTR1_tAR(ns2cycle(tAR_min, nand_clk));
>  
>  	info->ndtr0cs0 = ndtr0;
>  	info->ndtr1cs0 = ndtr1;
> @@ -384,6 +386,29 @@ static void pxa3xx_nand_set_timing(struct pxa3xx_nand_host *host,
>  	nand_writel(info, NDTR1CS0, ndtr1);
>  }
>  
> +static int pxa3xx_nand_init_timings(struct pxa3xx_nand_host *host)
> +{
> +	const struct nand_sdr_timings *timings;
> +	int mode;
> +
> +	mode = onfi_get_async_timing_mode(&host->chip);
> +	if (mode == ONFI_TIMING_MODE_UNKNOWN) {
> +		mode = host->chip.onfi_timing_mode_default;
> +	} 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_timing(host, timings);
> +
> +	return 0;
> +}
> +
>  /*
>   * Set the data and OOB size, depending on the selected
>   * spare and ECC configuration.
> @@ -1226,7 +1251,6 @@ static int pxa3xx_nand_config_flash(struct pxa3xx_nand_info *info,
>  
>  	info->reg_ndcr = ndcr;
>  
> -	pxa3xx_nand_set_timing(host, f->timing);
>  	return 0;
>  }
>  
> @@ -1321,20 +1345,28 @@ 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 mtd_info *mtd;
>  	struct nand_chip *chip;
> +	const struct nand_sdr_timings *timings;
>  	int ret;
>  
>  	mtd = info->host[info->cs]->mtd;
>  	chip = mtd->priv;
>  
> -	/* use the common timing to make a try */
>  	ret = pxa3xx_nand_config_flash(info, &builtin_flash_types[0]);
>  	if (ret)
>  		return ret;
>  
> +	/* use the common timing to make a try */
> +	timings = onfi_async_timing_mode_to_sdr_timings(0);
> +	if (IS_ERR(timings))
> +		return PTR_ERR(timings);
> +
> +	pxa3xx_nand_set_timing(host, timings);
> +
>  	chip->cmdfunc(mtd, NAND_CMD_RESET, 0, 0);
>  	ret = chip->waitfunc(mtd, chip);
>  	if (ret & NAND_STATUS_FAIL)
> @@ -1420,6 +1452,7 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
>  	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;
> +	const struct nand_sdr_timings *timings;
>  	struct nand_chip *chip = mtd->priv;
>  	uint32_t id = -1;
>  	uint64_t chipsize;
> @@ -1432,7 +1465,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);
> @@ -1498,6 +1531,10 @@ KEEP_CONFIG:
>  	if (nand_scan_ident(mtd, 1, def))
>  		return -ENODEV;
>  
> +	ret = pxa3xx_nand_init_timings(host);
> +	if (ret)
> +		dev_err(&info->pdev->dev, "Failed to set timings: %d\n", ret);
> +
>  	if (pdata->flash_bbt) {
>  		/*
>  		 * We'll use a bad block table stored in-flash and don't
> diff --git a/include/linux/platform_data/mtd-nand-pxa3xx.h b/include/linux/platform_data/mtd-nand-pxa3xx.h
> index ac4ea2e641c7..27d8f6354c2a 100644
> --- a/include/linux/platform_data/mtd-nand-pxa3xx.h
> +++ b/include/linux/platform_data/mtd-nand-pxa3xx.h
> @@ -24,8 +24,6 @@ struct pxa3xx_nand_flash {
>  	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 */
>  };
>  
>  /*
> 

This removes support for setting timing on non-ONFI devices. So, you
either need to keep the code around, or introduce some mechanism to
specify timings on these devices.

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

Re: [PATCH v3 5/9] mtd: pxa3xx_nand: add support for the Marvell Berlin nand controller

[Robert Jarzmik]

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

>>> Which flash do you have there?
>> The one with 0xba20 id as I said, which is AFAIK a Numonyx NAND02GR4B2C.
>> 
>
> $ grep "0xBA" drivers/mtd/nand/nand_ids.c
> EXTENDED_ID_NAND("NAND 256MiB 1,8V 16-bit", 0xBA, 256, LP_OPTIONS16),
>
> Seems already supported by the NAND core. The MTD way of probing
> a non-ONFI device is by using the IDs in nand_ids.c. Additional
> configuration (timings in this case) is applied between the nand_scan_ident()
> and nand_scan_tail() calls.

Ok, agreed.

I've seen your answer in the "rework timings setup" mail thread. I think you
have perfectly understood what bothers me :)

Cheers.

-- 
Robert