[PATCH v2 00/27] Armada 370/XP NAND support

[PATCH v2 00/27] Armada 370/XP NAND support

[Ezequiel Garcia]

Hi guys,

This is the v2 of the work implementing NAND support in Armada 370/XP SoCs.
This series is probably not yet complete (see below), but I feel like
we're really closer now :-)

As in the previous version and given I don't have any public datasheet
to show, I'll try to write some of the most relevant parts of the controller.

* NFCv2 controller background

The controller has a 2176 bytes FIFO buffer. Therefore, in order to support
larger pages, I/O operations on 4 KiB and 8 KiB pages is done with a set of
chunked transfers.

For instance, if we choose a 2048 data chunk and set "BCH" ECC (see below)
we'll have this layout in the pages:

  ------------------------------------------------------------------------------
  | 2048B data | 32B spare | 30B ECC || 2048B data | 32B spare | 30B ECC | ... |
  ------------------------------------------------------------------------------

The driver reads the data and spare portions independently and builds an internal
buffer with this layout (in the 4 KiB page case):

  ------------------------------------------
  |     4096B data     |     64B spare     |
  ------------------------------------------

Also, for the READOOB command the driver disables the ECC and reads a 'spare + ECC'
OOB, one per chunk read.

  -------------------------------------------------------------------
  |     4096B data     |  32B spare | 30B ECC | 32B spare | 30B ECC |
  -------------------------------------------------------------------

So, in order to achieve reading (for instance), we issue several READ0 commands
(with some additional controller-specific magic) and read two chunks of 2080B
(2048 data + 32 spare) each.
The driver accomodates this data to expose the NAND core a contiguous buffer
(4096 data + spare) or (4096 + spare + ECC + spare + ECC).

* ECC

The controller has built-in hardware ECC capabilities. In addition it is
configurable between two modes: 1) Hamming, 2) BCH.

Note that the actual BCH mode: BCH-4 or BCH-8 will depend on the way
the controller is configured to transfer the data.

In the BCH mode the ECC code will be calculated for each transfered chunk
and expected to be located (when reading/programming) right after the spare
bytes as the figure above shows.

So, repeating the above scheme, a 2048B data chunk will be followed by 32B
spare, and then the ECC controller will read/write the ECC code (30B in
this case):

  ------------------------------------
  | 2048B data | 32B spare | 30B ECC |
  ------------------------------------

If the ECC mode is 'BCH' then the ECC is *always* 30 bytes long.
If the ECC mode is 'Hamming' the ECC is 6 bytes long, for each 512B block.
So in Hamming mode, a 2048B page will have a 24B ECC.

Despite all of the above, the controller requires the driver to only read or
write in multiples of 8-bytes, because the data buffer is 64-bits.

* Changes from v1

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

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

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

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

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

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

* Pending stuff

  1. Factory bad blocks handling

  Currently, there's no factory bad block initial scan (when the bad block
  table is missing). The ECC BCH requires to layout the device's pages in
  a splitted "data + OOB + data + OOB" way. This layout being different from
  the factory layout. In other words,

  Factory view:

  -----------------------------------------------
  |                    Data           |x  OOB   |
  -----------------------------------------------

  Driver's view:

  -----------------------------------------------
  |      Data      | OOB |      Data   x  | OOB |
  -----------------------------------------------

  It can be seen from the above, that the factory bad block marker must be
  searched within the 'data' region, and not in the usual OOB region.

  This seems to be similar to the gpmi-nand driver, yet I really find its
  'bad block swapping' solution odd, so I'm trying to find something
  cleaner.

  2. ECC modeling

  Although I've exposed some ECC information in the ECC layout, I'm still
  not sure why is this needed, or if it's needed at all.

* About this patchset

This is based in l2-mtd's master branch and I've pushed a branch to our
github in case anyone wants to test it:

  https://github.com/MISL-EBU-System-SW/mainline-public/tree/l2-mtd/upstream-nand-v2

Just as the previous, this series is complex and lengthy and any feedback will
be highly appreciated as well as questions, suggestions or flames. Also, I hope
our brave PXA regression tester Daniel Mack finds some time to give it a ride
and report any issues. Daniel: I think owe you several beers already.

* About Mirabox testing

As this work is not considered completely stable, be extra careful when trying
on the Mirabox. The Mirabox has the bootloader at NAND, and there's some risk
to brick the board.

That said: I've been using the driver on my Mirabox for several days doing
simple long-run NAND activity and everything worked fine.

If despite this you happen to brick the board, Willy Tarreau has provided
excellent instructions to un-brick the Mirabox:

  http://1wt.eu/articles/mirabox-vs-guruplug/

Thanks!

Ezequiel Garcia (27):
  clk: mvebu: Add Core Divider clock
  ARM: mvebu: Add Core Divider clock device-tree binding
  ARM: mvebu: Add a 2 GHz fixed-clock Armada 370/XP
  ARM: mvebu: Add the core-divider clock to Armada 370/XP
  mtd: nand: pxa3xx: Make config menu show supported platforms
  mtd: nand: pxa3xx: Prevent sub-page writes
  mtd: nand: pxa3xx: Early variant detection
  mtd: nand: pxa3xx: Use chip->cmdfunc instead of the internal
  mtd: nand: pxa3xx: Split FIFO size from to-be-read FIFO count
  mtd: nand: pxa3xx: Replace host->page_size by mtd->writesize
  mtd: nand: pxa3xx: Disable OOB on arbitrary length commands
  mtd: nand: pxa3xx: Use a completion to signal device ready
  mtd: nand: pxa3xx: Add bad block handling
  mtd: nand: pxa3xx: Add driver-specific ECC BCH support
  mtd: nand: pxa3xx: Clear cmd buffer #3 (NDCB3) on command start
  mtd: nand: pxa3xx: Add helper function to set page address
  mtd: nand: pxa3xx: Remove READ0 switch/case falltrough
  mtd: nand: pxa3xx: Split prepare_command_pool() in two stages
  mtd: nand: pxa3xx: Move the data buffer clean to
    prepare_start_command()
  mtd: nand: pxa3xx: Fix SEQIN column address set
  mtd: nand: pxa3xx: Add a read/write buffers markers
  mtd: nand: pxa3xx: Introduce multiple page I/O support
  mtd: nand: pxa3xx: Add multiple chunk write support
  mtd: nand: pxa3xx: Add ECC BCH correctable errors detection
  ARM: mvebu: Add support for NAND controller in Armada 370/XP
  ARM: mvebu: Enable NAND controller in Armada XP GP board
  ARM: mvebu: Enable NAND controller in Armada 370 Mirabox

 .../bindings/clock/mvebu-corediv-clock.txt         |  19 +
 arch/arm/boot/dts/armada-370-mirabox.dts           |  21 +
 arch/arm/boot/dts/armada-370-xp.dtsi               |  26 +
 arch/arm/boot/dts/armada-xp-gp.dts                 |   8 +
 drivers/clk/mvebu/Kconfig                          |   5 +
 drivers/clk/mvebu/Makefile                         |   1 +
 drivers/clk/mvebu/clk-corediv.c                    | 223 +++++++
 drivers/mtd/nand/Kconfig                           |   4 +-
 drivers/mtd/nand/pxa3xx_nand.c                     | 686 ++++++++++++++++-----
 include/linux/platform_data/mtd-nand-pxa3xx.h      |   3 +
 10 files changed, 856 insertions(+), 140 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/mvebu-corediv-clock.txt
 create mode 100644 drivers/clk/mvebu/clk-corediv.c

-- 
1.8.1.5

[PATCH v2 01/27] clk: mvebu: Add Core Divider clock

[Ezequiel Garcia]

This commit introduces a new group of clocks present in Armada 370/XP
SoCs (called "Core Divider" clocks) and add a provider for them.
The only clock supported for now is the NAND clock (ndclk), but the
infrastructure to add the rest is already set.

Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Acked-by: Jason Cooper <jason@lakedaemon.net>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/clk/mvebu/Kconfig       |   5 +
 drivers/clk/mvebu/Makefile      |   1 +
 drivers/clk/mvebu/clk-corediv.c | 223 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 229 insertions(+)
 create mode 100644 drivers/clk/mvebu/clk-corediv.c

diff --git a/drivers/clk/mvebu/Kconfig b/drivers/clk/mvebu/Kconfig
index 0b0f3e7..c339b82 100644
--- a/drivers/clk/mvebu/Kconfig
+++ b/drivers/clk/mvebu/Kconfig
@@ -4,15 +4,20 @@ config MVEBU_CLK_COMMON
 config MVEBU_CLK_CPU
 	bool
 
+config MVEBU_CLK_COREDIV
+	bool
+
 config ARMADA_370_CLK
 	bool
 	select MVEBU_CLK_COMMON
 	select MVEBU_CLK_CPU
+	select MVEBU_CLK_COREDIV
 
 config ARMADA_XP_CLK
 	bool
 	select MVEBU_CLK_COMMON
 	select MVEBU_CLK_CPU
+	select MVEBU_CLK_COREDIV
 
 config DOVE_CLK
 	bool
diff --git a/drivers/clk/mvebu/Makefile b/drivers/clk/mvebu/Makefile
index 1c7e70c..21bbfb4 100644
--- a/drivers/clk/mvebu/Makefile
+++ b/drivers/clk/mvebu/Makefile
@@ -1,5 +1,6 @@
 obj-$(CONFIG_MVEBU_CLK_COMMON)	+= common.o
 obj-$(CONFIG_MVEBU_CLK_CPU) 	+= clk-cpu.o
+obj-$(CONFIG_MVEBU_CLK_COREDIV)	+= clk-corediv.o
 
 obj-$(CONFIG_ARMADA_370_CLK)	+= armada-370.o
 obj-$(CONFIG_ARMADA_XP_CLK)	+= armada-xp.o
diff --git a/drivers/clk/mvebu/clk-corediv.c b/drivers/clk/mvebu/clk-corediv.c
new file mode 100644
index 0000000..7162615
--- /dev/null
+++ b/drivers/clk/mvebu/clk-corediv.c
@@ -0,0 +1,223 @@
+/*
+ * MVEBU Core divider clock
+ *
+ * Copyright (C) 2013 Marvell
+ *
+ * Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/kernel.h>
+#include <linux/clk-provider.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include "common.h"
+
+#define CORE_CLK_DIV_RATIO_MASK		0xff
+#define CORE_CLK_DIV_RATIO_RELOAD	BIT(8)
+#define CORE_CLK_DIV_ENABLE_OFFSET	24
+#define CORE_CLK_DIV_RATIO_OFFSET	0x8
+
+struct clk_corediv_desc {
+	unsigned int mask;
+	unsigned int offset;
+	unsigned int fieldbit;
+};
+
+struct clk_corediv {
+	struct clk_hw hw;
+	void __iomem *reg;
+	struct clk_corediv_desc desc;
+	spinlock_t lock;
+};
+
+static struct clk_onecell_data clk_data;
+
+static const struct clk_corediv_desc mvebu_corediv_desc[] __initconst = {
+	{ .mask = 0x3f, .offset = 8, .fieldbit = 1 }, /* NAND clock */
+};
+
+#define to_corediv_clk(p) container_of(p, struct clk_corediv, hw)
+
+static int clk_corediv_is_enabled(struct clk_hw *hwclk)
+{
+	struct clk_corediv *corediv = to_corediv_clk(hwclk);
+	struct clk_corediv_desc *desc = &corediv->desc;
+	u32 enable_mask = BIT(desc->fieldbit) << CORE_CLK_DIV_ENABLE_OFFSET;
+
+	return !!(readl(corediv->reg) & enable_mask);
+}
+
+static int clk_corediv_enable(struct clk_hw *hwclk)
+{
+	struct clk_corediv *corediv = to_corediv_clk(hwclk);
+	struct clk_corediv_desc *desc = &corediv->desc;
+	unsigned long flags = 0;
+	u32 reg;
+
+	spin_lock_irqsave(&corediv->lock, flags);
+
+	reg = readl(corediv->reg);
+	reg |= (BIT(desc->fieldbit) << CORE_CLK_DIV_ENABLE_OFFSET);
+	writel(reg, corediv->reg);
+
+	spin_unlock_irqrestore(&corediv->lock, flags);
+
+	return 0;
+}
+
+static void clk_corediv_disable(struct clk_hw *hwclk)
+{
+	struct clk_corediv *corediv = to_corediv_clk(hwclk);
+	struct clk_corediv_desc *desc = &corediv->desc;
+	unsigned long flags = 0;
+	u32 reg;
+
+	spin_lock_irqsave(&corediv->lock, flags);
+
+	reg = readl(corediv->reg);
+	reg &= ~(BIT(desc->fieldbit) << CORE_CLK_DIV_ENABLE_OFFSET);
+	writel(reg, corediv->reg);
+
+	spin_unlock_irqrestore(&corediv->lock, flags);
+}
+
+static unsigned long clk_corediv_recalc_rate(struct clk_hw *hwclk,
+					 unsigned long parent_rate)
+{
+	struct clk_corediv *corediv = to_corediv_clk(hwclk);
+	struct clk_corediv_desc *desc = &corediv->desc;
+	u32 reg, div;
+
+	reg = readl(corediv->reg + CORE_CLK_DIV_RATIO_OFFSET);
+	div = (reg >> desc->offset) & desc->mask;
+	return parent_rate / div;
+}
+
+static long clk_corediv_round_rate(struct clk_hw *hwclk, unsigned long rate,
+			       unsigned long *parent_rate)
+{
+	/* Valid ratio are 1:4, 1:5, 1:6 and 1:8 */
+	u32 div;
+
+	div = *parent_rate / rate;
+	if (div < 4)
+		div = 4;
+	else if (div > 6)
+		div = 8;
+
+	return *parent_rate / div;
+}
+
+static int clk_corediv_set_rate(struct clk_hw *hwclk, unsigned long rate,
+			    unsigned long parent_rate)
+{
+	struct clk_corediv *corediv = to_corediv_clk(hwclk);
+	struct clk_corediv_desc *desc = &corediv->desc;
+	unsigned long flags = 0;
+	u32 reg, div;
+
+	div = parent_rate / rate;
+
+	spin_lock_irqsave(&corediv->lock, flags);
+
+	/* Write new divider to the divider ratio register */
+	reg = readl(corediv->reg + CORE_CLK_DIV_RATIO_OFFSET);
+	reg &= ~(desc->mask << desc->offset);
+	reg |= (div & desc->mask) << desc->offset;
+	writel(reg, corediv->reg + CORE_CLK_DIV_RATIO_OFFSET);
+
+	/* Set reload-force for this clock */
+	reg = readl(corediv->reg) | BIT(desc->fieldbit);
+	writel(reg, corediv->reg);
+
+	/* Now trigger the clock update */
+	reg = readl(corediv->reg) | CORE_CLK_DIV_RATIO_RELOAD;
+	writel(reg, corediv->reg);
+
+	/*
+	 * Wait for clocks to settle down, and then clear all the
+	 * ratios request and the reload request.
+	 */
+	udelay(1000);
+	reg &= ~(CORE_CLK_DIV_RATIO_MASK | CORE_CLK_DIV_RATIO_RELOAD);
+	writel(reg, corediv->reg);
+	udelay(1000);
+
+	spin_unlock_irqrestore(&corediv->lock, flags);
+
+	return 0;
+}
+
+static const struct clk_ops corediv_ops = {
+	.enable = clk_corediv_enable,
+	.disable = clk_corediv_disable,
+	.is_enabled = clk_corediv_is_enabled,
+	.recalc_rate = clk_corediv_recalc_rate,
+	.round_rate = clk_corediv_round_rate,
+	.set_rate = clk_corediv_set_rate,
+};
+
+static void __init mvebu_corediv_clk_init(struct device_node *node)
+{
+	struct clk_init_data init;
+	struct clk_corediv *corediv;
+	struct clk **clks;
+	void __iomem *base;
+	const char *parent_name;
+	const char *clk_name;
+	int i;
+
+	base = of_iomap(node, 0);
+	if (WARN_ON(!base))
+		return;
+
+	parent_name = of_clk_get_parent_name(node, 0);
+
+	clk_data.clk_num = ARRAY_SIZE(mvebu_corediv_desc);
+
+	/* clks holds the clock array */
+	clks = kcalloc(clk_data.clk_num, sizeof(struct clk *),
+				GFP_KERNEL);
+	if (WARN_ON(!clks))
+		goto err_unmap;
+	/* corediv holds the clock specific array */
+	corediv = kcalloc(clk_data.clk_num, sizeof(struct clk_corediv),
+				GFP_KERNEL);
+	if (WARN_ON(!corediv))
+		goto err_free_clks;
+
+	spin_lock_init(&corediv->lock);
+
+	for (i = 0; i < clk_data.clk_num; i++) {
+		of_property_read_string_index(node, "clock-output-names",
+					      i, &clk_name);
+		init.num_parents = 1;
+		init.parent_names = &parent_name;
+		init.name = clk_name;
+		init.ops = &corediv_ops;
+		init.flags = 0;
+
+		corediv[i].desc = mvebu_corediv_desc[i];
+		corediv[i].reg = base;
+		corediv[i].hw.init = &init;
+
+		clks[i] = clk_register(NULL, &corediv[i].hw);
+		WARN_ON(IS_ERR(clks[i]));
+	}
+
+	clk_data.clks = clks;
+	of_clk_add_provider(node, of_clk_src_onecell_get, &clk_data);
+	return;
+
+err_free_clks:
+	kfree(clks);
+err_unmap:
+	iounmap(base);
+}
+CLK_OF_DECLARE(mvebu_corediv_clk, "marvell,armada-370-corediv-clock",
+	       mvebu_corediv_clk_init);
-- 
1.8.1.5

[PATCH v2 02/27] ARM: mvebu: Add Core Divider clock device-tree binding

[Ezequiel Garcia]

The Armada 370/XP SoCs have a Core Divider clock providing
several clocks. For now, only the NAND clock is supported.

Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 .../devicetree/bindings/clock/mvebu-corediv-clock.txt | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/mvebu-corediv-clock.txt

diff --git a/Documentation/devicetree/bindings/clock/mvebu-corediv-clock.txt b/Documentation/devicetree/bindings/clock/mvebu-corediv-clock.txt
new file mode 100644
index 0000000..c62391f
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/mvebu-corediv-clock.txt
@@ -0,0 +1,19 @@
+* Core Divider Clock bindings for Marvell MVEBU SoCs
+
+The following is a list of provided IDs and clock names on Armada 370/XP:
+ 0 = nand (NAND clock)
+
+Required properties:
+- compatible : must be "marvell,armada-370-corediv-clock"
+- reg : must be the register address of Core Divider control register
+- #clock-cells : from common clock binding; shall be set to 1
+- clocks : must be set to the parent's phandle
+
+Example:
+
+corediv_clk: corediv-clocks at 18740 {
+	compatible = "marvell,armada-370-corediv-clock";
+	reg = <0x18740 0xc>;
+	#clock-cells = <1>;
+	clocks = <&pll>;
+};
-- 
1.8.1.5

[PATCH v2 03/27] ARM: mvebu: Add a 2 GHz fixed-clock Armada 370/XP

[Ezequiel Garcia]

Armada 370/XP SoCs have a 2 GHz fixed PLL that is used to feed
other clocks. This commit adds a DT representation of this clock
through a fixed-clock compatible node.

Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/boot/dts/armada-370-xp.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/armada-370-xp.dtsi b/arch/arm/boot/dts/armada-370-xp.dtsi
index 1de2dae..924c721 100644
--- a/arch/arm/boot/dts/armada-370-xp.dtsi
+++ b/arch/arm/boot/dts/armada-370-xp.dtsi
@@ -252,4 +252,13 @@
 
 		};
 	};
+
+	clocks {
+		/* 2 GHz fixed main PLL */
+		mainpll: mainpll {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <2000000000>;
+		};
+	};
  };
-- 
1.8.1.5

[PATCH v2 04/27] ARM: mvebu: Add the core-divider clock to Armada 370/XP

[Ezequiel Garcia]

The Armada 370/XP SoC has a clock provider called "Core Divider",
that is derived from a fixed 2 GHz PLL clock.

Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/boot/dts/armada-370-xp.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/armada-370-xp.dtsi b/arch/arm/boot/dts/armada-370-xp.dtsi
index 924c721..01e69fc 100644
--- a/arch/arm/boot/dts/armada-370-xp.dtsi
+++ b/arch/arm/boot/dts/armada-370-xp.dtsi
@@ -137,6 +137,14 @@
 				status = "disabled";
 			};
 
+			coredivclk: corediv-clock at 18740 {
+				compatible = "marvell,armada-370-corediv-clock";
+				reg = <0x18740 0xc>;
+				#clock-cells = <1>;
+				clocks = <&mainpll>;
+				clock-output-names = "nand";
+			};
+
 			timer at 20300 {
 				reg = <0x20300 0x30>, <0x21040 0x30>;
 				interrupts = <37>, <38>, <39>, <40>, <5>, <6>;
-- 
1.8.1.5

[PATCH v2 05/27] mtd: nand: pxa3xx: Make config menu show supported platforms

[Ezequiel Garcia]

Since we have now support for the NFCv2 controller found on
Armada 370/XP platforms.

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

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index d885298..791d219 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -354,11 +354,11 @@ config MTD_NAND_ATMEL
 	  on Atmel AT91 and AVR32 processors.
 
 config MTD_NAND_PXA3xx
-	tristate "Support for NAND flash devices on PXA3xx"
+	tristate "NAND support on PXA3xx and Armada 370/XP"
 	depends on PXA3xx || ARCH_MMP || PLAT_ORION
 	help
 	  This enables the driver for the NAND flash device found on
-	  PXA3xx processors
+	  PXA3xx processors (NFCv1) and also on Armada 370/XP (NFCv2).
 
 config MTD_NAND_SLC_LPC32XX
 	tristate "NXP LPC32xx SLC Controller"
-- 
1.8.1.5

[PATCH v2 06/27] mtd: nand: pxa3xx: Prevent sub-page writes

[Ezequiel Garcia]

The current driver doesn't support sub-page writing, so report
that to the NAND core.

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

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index aa75adf..5ca3154 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -1143,6 +1143,7 @@ static int alloc_nand_resource(struct platform_device *pdev)
 		chip->read_byte		= pxa3xx_nand_read_byte;
 		chip->read_buf		= pxa3xx_nand_read_buf;
 		chip->write_buf		= pxa3xx_nand_write_buf;
+		chip->options		|= NAND_NO_SUBPAGE_WRITE;
 	}
 
 	spin_lock_init(&chip->controller->lock);
-- 
1.8.1.5

[PATCH v2 07/27] mtd: nand: pxa3xx: Early variant detection

[Ezequiel Garcia]

In order to customize early settings depending on the detected SoC variant,
move the detection to be before the nand_chip struct filling.

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

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 5ca3154..83ea806 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -256,6 +256,29 @@ static struct pxa3xx_nand_flash builtin_flash_types[] = {
 /* convert nano-seconds to nand flash controller clock cycles */
 #define ns2cycle(ns, clk)	(int)((ns) * (clk / 1000000) / 1000)
 
+static struct of_device_id pxa3xx_nand_dt_ids[] = {
+	{
+		.compatible = "marvell,pxa3xx-nand",
+		.data       = (void *)PXA3XX_NAND_VARIANT_PXA,
+	},
+	{
+		.compatible = "marvell,armada370-nand",
+		.data       = (void *)PXA3XX_NAND_VARIANT_ARMADA370,
+	},
+	{}
+};
+MODULE_DEVICE_TABLE(of, pxa3xx_nand_dt_ids);
+
+static enum pxa3xx_nand_variant
+pxa3xx_nand_get_variant(struct platform_device *pdev)
+{
+	const struct of_device_id *of_id =
+			of_match_device(pxa3xx_nand_dt_ids, &pdev->dev);
+	if (!of_id)
+		return PXA3XX_NAND_VARIANT_PXA;
+	return (enum pxa3xx_nand_variant)of_id->data;
+}
+
 static void pxa3xx_nand_set_timing(struct pxa3xx_nand_host *host,
 				   const struct pxa3xx_nand_timing *t)
 {
@@ -1121,6 +1144,7 @@ static int alloc_nand_resource(struct platform_device *pdev)
 		return -ENOMEM;
 
 	info->pdev = pdev;
+	info->variant = pxa3xx_nand_get_variant(pdev);
 	for (cs = 0; cs < pdata->num_cs; cs++) {
 		mtd = (struct mtd_info *)((unsigned int)&info[1] +
 		      (sizeof(*mtd) + sizeof(*host)) * cs);
@@ -1256,29 +1280,6 @@ static int pxa3xx_nand_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static struct of_device_id pxa3xx_nand_dt_ids[] = {
-	{
-		.compatible = "marvell,pxa3xx-nand",
-		.data       = (void *)PXA3XX_NAND_VARIANT_PXA,
-	},
-	{
-		.compatible = "marvell,armada370-nand",
-		.data       = (void *)PXA3XX_NAND_VARIANT_ARMADA370,
-	},
-	{}
-};
-MODULE_DEVICE_TABLE(of, pxa3xx_nand_dt_ids);
-
-static enum pxa3xx_nand_variant
-pxa3xx_nand_get_variant(struct platform_device *pdev)
-{
-	const struct of_device_id *of_id =
-			of_match_device(pxa3xx_nand_dt_ids, &pdev->dev);
-	if (!of_id)
-		return PXA3XX_NAND_VARIANT_PXA;
-	return (enum pxa3xx_nand_variant)of_id->data;
-}
-
 static int pxa3xx_nand_probe_dt(struct platform_device *pdev)
 {
 	struct pxa3xx_nand_platform_data *pdata;
@@ -1335,7 +1336,6 @@ static int pxa3xx_nand_probe(struct platform_device *pdev)
 	}
 
 	info = platform_get_drvdata(pdev);
-	info->variant = pxa3xx_nand_get_variant(pdev);
 	probe_success = 0;
 	for (cs = 0; cs < pdata->num_cs; cs++) {
 		struct mtd_info *mtd = info->host[cs]->mtd;
-- 
1.8.1.5

[PATCH v2 08/27] mtd: nand: pxa3xx: Use chip->cmdfunc instead of the internal

[Ezequiel Garcia]

Whenever possible, it's always better to use the generic chip->cmdfunc
instead of the internal pxa3xx_nand_cmdfunc().
In this particular case, this will allow to have multiple cmdfunc()
implementations for different SoC variants.

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

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 83ea806..1e2f4ae 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -1011,14 +1011,16 @@ static void pxa3xx_nand_free_buff(struct pxa3xx_nand_info *info)
 static int pxa3xx_nand_sensing(struct pxa3xx_nand_info *info)
 {
 	struct mtd_info *mtd;
+	struct nand_chip *chip;
 	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;
 
-	pxa3xx_nand_cmdfunc(mtd, NAND_CMD_RESET, 0, 0);
+	chip->cmdfunc(mtd, NAND_CMD_RESET, 0, 0);
 	if (info->is_ready)
 		return 0;
 
-- 
1.8.1.5

[PATCH v2 09/27] mtd: nand: pxa3xx: Split FIFO size from to-be-read FIFO count

[Ezequiel Garcia]

Introduce a fifo_size field to represent the size of the controller's
FIFO buffer, and use it to distinguish that size from the amount
of data bytes to be read from the FIFO.

This is important to support devices with pages larger than the
controller's internal FIFO, that need to read the pages in FIFO-sized
chunks.

In particular, the current code is at least confusing, for it mixes
all the different sizes involved: FIFO size, page size and data size.

This commit starts the cleaning by removing the info->page_size field
that is not currently used. The host->page_size field should also
be removed and use always mtd->writesize instead. Follow up commits
will clean this up.

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

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 1e2f4ae..fe2e27c 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -199,8 +199,8 @@ struct pxa3xx_nand_info {
 	int			use_spare;	/* use spare ? */
 	int			is_ready;
 
-	unsigned int		page_size;	/* page size of attached chip */
-	unsigned int		data_size;	/* data size in FIFO */
+	unsigned int		fifo_size;	/* max. data size in the FIFO */
+	unsigned int		data_size;	/* data to be read from FIFO */
 	unsigned int		oob_size;
 	int 			retcode;
 
@@ -305,16 +305,15 @@ static void pxa3xx_nand_set_timing(struct pxa3xx_nand_host *host,
 
 static void pxa3xx_set_datasize(struct pxa3xx_nand_info *info)
 {
-	struct pxa3xx_nand_host *host = info->host[info->cs];
 	int oob_enable = info->reg_ndcr & NDCR_SPARE_EN;
 
-	info->data_size = host->page_size;
+	info->data_size = info->fifo_size;
 	if (!oob_enable) {
 		info->oob_size = 0;
 		return;
 	}
 
-	switch (host->page_size) {
+	switch (info->fifo_size) {
 	case 2048:
 		info->oob_size = (info->use_ecc) ? 40 : 64;
 		break;
@@ -929,9 +928,12 @@ static int pxa3xx_nand_detect_config(struct pxa3xx_nand_info *info)
 	uint32_t ndcr = nand_readl(info, NDCR);
 
 	if (ndcr & NDCR_PAGE_SZ) {
+		/* Controller's FIFO size */
+		info->fifo_size = 2048;
 		host->page_size = 2048;
 		host->read_id_bytes = 4;
 	} else {
+		info->fifo_size = 512;
 		host->page_size = 512;
 		host->read_id_bytes = 2;
 	}
-- 
1.8.1.5

[PATCH v2 10/27] mtd: nand: pxa3xx: Replace host->page_size by mtd->writesize

[Ezequiel Garcia]

There's no need to privately store the device page size as it's
available in mtd structure field mtd->writesize.
Also, this removes the hardcoded page size value, leaving the
auto-detected value only.

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

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index fe2e27c..83ba036 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -149,7 +149,6 @@ struct pxa3xx_nand_host {
 	void			*info_data;
 
 	/* page size of attached chip */
-	unsigned int		page_size;
 	int			use_ecc;
 	int			cs;
 
@@ -612,12 +611,12 @@ static int prepare_command_pool(struct pxa3xx_nand_info *info, int command,
 			info->buf_start += mtd->writesize;
 
 		/* Second command setting for large pages */
-		if (host->page_size >= PAGE_CHUNK_SIZE)
+		if (mtd->writesize >= PAGE_CHUNK_SIZE)
 			info->ndcb0 |= NDCB0_DBC | (NAND_CMD_READSTART << 8);
 
 	case NAND_CMD_SEQIN:
 		/* small page addr setting */
-		if (unlikely(host->page_size < PAGE_CHUNK_SIZE)) {
+		if (unlikely(mtd->writesize < PAGE_CHUNK_SIZE)) {
 			info->ndcb1 = ((page_addr & 0xFFFFFF) << 8)
 					| (column & 0xFF);
 
@@ -891,7 +890,6 @@ static int pxa3xx_nand_config_flash(struct pxa3xx_nand_info *info,
 	}
 
 	/* calculate flash information */
-	host->page_size = f->page_size;
 	host->read_id_bytes = (f->page_size == 2048) ? 4 : 2;
 
 	/* calculate addressing information */
@@ -930,11 +928,9 @@ static int pxa3xx_nand_detect_config(struct pxa3xx_nand_info *info)
 	if (ndcr & NDCR_PAGE_SZ) {
 		/* Controller's FIFO size */
 		info->fifo_size = 2048;
-		host->page_size = 2048;
 		host->read_id_bytes = 4;
 	} else {
 		info->fifo_size = 512;
-		host->page_size = 512;
 		host->read_id_bytes = 2;
 	}
 
-- 
1.8.1.5

[PATCH v2 11/27] mtd: nand: pxa3xx: Disable OOB on arbitrary length commands

[Ezequiel Garcia]

READID, STATUS and PARAM (aka ONFI read paramater page) don't read
the OOB area. Set the oob_size to zero and prevent it.
Also, add a comment clarifying the use of pxa3xx_set_datasize()
which is only applicable on data read/write commands.

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

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 83ba036..95e2ce3 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -302,6 +302,11 @@ static void pxa3xx_nand_set_timing(struct pxa3xx_nand_host *host,
 	nand_writel(info, NDTR1CS0, ndtr1);
 }
 
+/*
+ * Set the data and OOB size, depending on the selected
+ * spare and ECC configuration.
+ * Only applicable to READ0, READOOB and PAGEPROG commands.
+ */
 static void pxa3xx_set_datasize(struct pxa3xx_nand_info *info)
 {
 	int oob_enable = info->reg_ndcr & NDCR_SPARE_EN;
@@ -661,6 +666,7 @@ static int prepare_command_pool(struct pxa3xx_nand_info *info, int command,
 		info->ndcb1 = (column & 0xFF);
 		info->ndcb3 = 256;
 		info->data_size = 256;
+		info->oob_size = 0;
 		break;
 
 	case NAND_CMD_READID:
@@ -671,6 +677,7 @@ static int prepare_command_pool(struct pxa3xx_nand_info *info, int command,
 		info->ndcb1 = (column & 0xFF);
 
 		info->data_size = 8;
+		info->oob_size = 0;
 		break;
 	case NAND_CMD_STATUS:
 		info->buf_count = 1;
@@ -679,6 +686,7 @@ static int prepare_command_pool(struct pxa3xx_nand_info *info, int command,
 				| command;
 
 		info->data_size = 8;
+		info->oob_size = 0;
 		break;
 
 	case NAND_CMD_ERASE1:
-- 
1.8.1.5

[PATCH v2 12/27] mtd: nand: pxa3xx: Use a completion to signal device ready

[Ezequiel Garcia]

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

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

Because the "is ready" flag is cleared from the controller status
register, it's needed to store that state in the driver, and because the
field is accesed from an interruption, the field needs to be of an
atomic type.

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

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 95e2ce3..1ceccb6 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -35,6 +35,7 @@
 
 #include <linux/platform_data/mtd-nand-pxa3xx.h>
 
+#define NAND_DEV_READY_TIMEOUT  50
 #define	CHIP_DELAY_TIMEOUT	(2 * HZ/10)
 #define NAND_STOP_DELAY		(2 * HZ/50)
 #define PAGE_CHUNK_SIZE		(2048)
@@ -166,7 +167,7 @@ struct pxa3xx_nand_info {
 	struct clk		*clk;
 	void __iomem		*mmio_base;
 	unsigned long		mmio_phys;
-	struct completion	cmd_complete;
+	struct completion	cmd_complete, dev_ready;
 
 	unsigned int 		buf_start;
 	unsigned int		buf_count;
@@ -196,7 +197,13 @@ struct pxa3xx_nand_info {
 	int			use_ecc;	/* use HW ECC ? */
 	int			use_dma;	/* use DMA ? */
 	int			use_spare;	/* use spare ? */
-	int			is_ready;
+
+	/*
+	 * The is_ready flag is accesed from several places,
+	 * including an interruption hander. We need an atomic
+	 * type to avoid races.
+	 */
+	atomic_t		is_ready;
 
 	unsigned int		fifo_size;	/* max. data size in the FIFO */
 	unsigned int		data_size;	/* data to be read from FIFO */
@@ -478,7 +485,7 @@ static void start_data_dma(struct pxa3xx_nand_info *info)
 static irqreturn_t pxa3xx_nand_irq(int irq, void *devid)
 {
 	struct pxa3xx_nand_info *info = devid;
-	unsigned int status, is_completed = 0;
+	unsigned int status, is_completed = 0, is_ready = 0;
 	unsigned int ready, cmd_done;
 
 	if (info->cs == 0) {
@@ -514,8 +521,9 @@ static irqreturn_t pxa3xx_nand_irq(int irq, void *devid)
 		is_completed = 1;
 	}
 	if (status & ready) {
-		info->is_ready = 1;
+		atomic_set(&info->is_ready, 1);
 		info->state = STATE_READY;
+		is_ready = 1;
 	}
 
 	if (status & NDSR_WRCMDREQ) {
@@ -544,6 +552,8 @@ static irqreturn_t pxa3xx_nand_irq(int irq, void *devid)
 	nand_writel(info, NDSR, status);
 	if (is_completed)
 		complete(&info->cmd_complete);
+	if (is_ready)
+		complete(&info->dev_ready);
 NORMAL_IRQ_EXIT:
 	return IRQ_HANDLED;
 }
@@ -574,7 +584,6 @@ static int prepare_command_pool(struct pxa3xx_nand_info *info, int command,
 	info->oob_size		= 0;
 	info->use_ecc		= 0;
 	info->use_spare		= 1;
-	info->is_ready		= 0;
 	info->retcode		= ERR_NONE;
 	if (info->cs != 0)
 		info->ndcb0 = NDCB0_CSEL;
@@ -750,6 +759,8 @@ static void pxa3xx_nand_cmdfunc(struct mtd_info *mtd, unsigned command,
 	exec_cmd = prepare_command_pool(info, command, column, page_addr);
 	if (exec_cmd) {
 		init_completion(&info->cmd_complete);
+		init_completion(&info->dev_ready);
+		atomic_set(&info->is_ready, 0);
 		pxa3xx_nand_start(info);
 
 		ret = wait_for_completion_timeout(&info->cmd_complete,
@@ -862,21 +873,27 @@ static int pxa3xx_nand_waitfunc(struct mtd_info *mtd, struct nand_chip *this)
 {
 	struct pxa3xx_nand_host *host = mtd->priv;
 	struct pxa3xx_nand_info *info = host->info_data;
+	int ret;
+
+	/* Need to wait? */
+	if (!atomic_read(&info->is_ready)) {
+		ret = wait_for_completion_timeout(&info->dev_ready,
+				CHIP_DELAY_TIMEOUT);
+		if (!ret) {
+			dev_err(&info->pdev->dev, "Ready time out!!!\n");
+			return NAND_STATUS_FAIL;
+		}
+	}
 
 	/* pxa3xx_nand_send_command has waited for command complete */
 	if (this->state == FL_WRITING || this->state == FL_ERASING) {
 		if (info->retcode == ERR_NONE)
 			return 0;
-		else {
-			/*
-			 * any error make it return 0x01 which will tell
-			 * the caller the erase and write fail
-			 */
-			return 0x01;
-		}
+		else
+			return NAND_STATUS_FAIL;
 	}
 
-	return 0;
+	return NAND_STATUS_READY;
 }
 
 static int pxa3xx_nand_config_flash(struct pxa3xx_nand_info *info,
@@ -1027,7 +1044,7 @@ static int pxa3xx_nand_sensing(struct pxa3xx_nand_info *info)
 		return ret;
 
 	chip->cmdfunc(mtd, NAND_CMD_RESET, 0, 0);
-	if (info->is_ready)
+	if (atomic_read(&info->is_ready))
 		return 0;
 
 	return -ENODEV;
-- 
1.8.1.5

[PATCH v2 13/27] mtd: nand: pxa3xx: Add bad block handling

[Ezequiel Garcia]

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

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

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 1ceccb6..451ebb6 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -24,6 +24,7 @@
 #include <linux/slab.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/of_mtd.h>
 
 #if defined(CONFIG_ARCH_PXA) || defined(CONFIG_ARCH_MMP)
 #define ARCH_HAS_DMA
@@ -245,6 +246,29 @@ static struct pxa3xx_nand_flash builtin_flash_types[] = {
 { "256MiB 16-bit", 0xba20,  64, 2048, 16, 16, 2048, &timing[3] },
 };
 
+static u8 bbt_pattern[] = {'M', 'V', 'B', 'b', 't', '0' };
+static u8 bbt_mirror_pattern[] = {'1', 't', 'b', 'B', 'V', 'M' };
+
+static struct nand_bbt_descr bbt_main_descr = {
+	.options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE
+		| NAND_BBT_2BIT | NAND_BBT_VERSION,
+	.offs =	8,
+	.len = 6,
+	.veroffs = 14,
+	.maxblocks = 8,		/* Last 8 blocks in each chip */
+	.pattern = bbt_pattern
+};
+
+static struct nand_bbt_descr bbt_mirror_descr = {
+	.options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE
+		| NAND_BBT_2BIT | NAND_BBT_VERSION,
+	.offs =	8,
+	.len = 6,
+	.veroffs = 14,
+	.maxblocks = 8,		/* Last 8 blocks in each chip */
+	.pattern = bbt_mirror_pattern
+};
+
 /* Define a default flash type setting serve as flash detecting only */
 #define DEFAULT_FLASH_TYPE (&builtin_flash_types[0])
 
@@ -1129,6 +1153,14 @@ KEEP_CONFIG:
 
 	if (nand_scan_ident(mtd, 1, def))
 		return -ENODEV;
+
+	if (pdata->flash_bbt) {
+		chip->bbt_options |= NAND_BBT_USE_FLASH |
+				     NAND_BBT_NO_OOB_BBM;
+		chip->bbt_td = &bbt_main_descr;
+		chip->bbt_md = &bbt_mirror_descr;
+	}
+
 	/* calculate addressing information */
 	if (mtd->writesize >= 2048)
 		host->col_addr_cycles = 2;
@@ -1324,6 +1356,7 @@ static int pxa3xx_nand_probe_dt(struct platform_device *pdev)
 	if (of_get_property(np, "marvell,nand-keep-config", NULL))
 		pdata->keep_config = 1;
 	of_property_read_u32(np, "num-cs", &pdata->num_cs);
+	pdata->flash_bbt = of_get_nand_on_flash_bbt(np);
 
 	pdev->dev.platform_data = pdata;
 
diff --git a/include/linux/platform_data/mtd-nand-pxa3xx.h b/include/linux/platform_data/mtd-nand-pxa3xx.h
index ffb8019..a941471 100644
--- a/include/linux/platform_data/mtd-nand-pxa3xx.h
+++ b/include/linux/platform_data/mtd-nand-pxa3xx.h
@@ -55,6 +55,9 @@ struct pxa3xx_nand_platform_data {
 	/* indicate how many chip selects will be used */
 	int	num_cs;
 
+	/* use an flash-based bad block table */
+	bool	flash_bbt;
+
 	const struct mtd_partition		*parts[NUM_CHIP_SELECT];
 	unsigned int				nr_parts[NUM_CHIP_SELECT];
 
-- 
1.8.1.5

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

[Ezequiel Garcia]

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

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

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

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

[PATCH v2 15/27] mtd: nand: pxa3xx: Clear cmd buffer #3 (NDCB3) on command start

[Ezequiel Garcia]

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

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

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

[PATCH v2 16/27] mtd: nand: pxa3xx: Add helper function to set page address

[Ezequiel Garcia]

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

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

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

[PATCH v2 17/27] mtd: nand: pxa3xx: Remove READ0 switch/case falltrough

[Ezequiel Garcia]

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

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

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

[PATCH v2 18/27] mtd: nand: pxa3xx: Split prepare_command_pool() in two stages

[Ezequiel Garcia]

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

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

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

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

[PATCH v2 19/27] mtd: nand: pxa3xx: Move the data buffer clean to prepare_start_command()

[Ezequiel Garcia]

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

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

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

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

[PATCH v2 20/27] mtd: nand: pxa3xx: Fix SEQIN column address set

[Ezequiel Garcia]

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

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

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

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

[PATCH v2 21/27] mtd: nand: pxa3xx: Add a read/write buffers markers

[Ezequiel Garcia]

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

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

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

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

[PATCH v2 22/27] mtd: nand: pxa3xx: Introduce multiple page I/O support

[Ezequiel Garcia]

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

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

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

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

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

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 84ae555..b297b22 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -7,6 +7,20 @@
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
+ *
+ * Large pages implementation
+ * --------------------------
+ *
+ * NAND stack expects data in the following format:
+ * ---------------------------
+ * | Data (4K) | Spare | ECC |
+ * ---------------------------
+ *
+ * While NAND controller expects data to be in the following format:
+ * -----------------------------------------------------
+ * | Data (2K) | Spare | ECC | Data (2K) | Spare | ECC |
+ * -----------------------------------------------------
+ *
  */
 
 #include <linux/kernel.h>
@@ -101,6 +115,8 @@
 #define NDCB0_ST_ROW_EN         (0x1 << 26)
 #define NDCB0_AUTO_RS		(0x1 << 25)
 #define NDCB0_CSEL		(0x1 << 24)
+#define NDCB0_EXT_CMD_TYPE_MASK	(0x7 << 29)
+#define NDCB0_EXT_CMD_TYPE(x)	(((x) << 29) & NDCB0_EXT_CMD_TYPE_MASK)
 #define NDCB0_CMD_TYPE_MASK	(0x7 << 21)
 #define NDCB0_CMD_TYPE(x)	(((x) << 21) & NDCB0_CMD_TYPE_MASK)
 #define NDCB0_NC		(0x1 << 20)
@@ -111,6 +127,14 @@
 #define NDCB0_CMD1_MASK		(0xff)
 #define NDCB0_ADDR_CYC_SHIFT	(16)
 
+#define EXT_CMD_TYPE_DISPATCH	6 /* Command dispatch */
+#define EXT_CMD_TYPE_NAKED_RW	5 /* Naked read or Naked write */
+#define EXT_CMD_TYPE_READ	4 /* Read */
+#define EXT_CMD_TYPE_DISP_WR	4 /* Command dispatch with write */
+#define EXT_CMD_TYPE_FINAL	3 /* Final command */
+#define EXT_CMD_TYPE_LAST_RW	1 /* Last naked read/write */
+#define EXT_CMD_TYPE_MONO	0 /* Monolithic read/write */
+
 /* macros for registers read/write */
 #define nand_writel(info, off, val)	\
 	__raw_writel((val), (info)->mmio_base + (off))
@@ -212,6 +236,7 @@ struct pxa3xx_nand_info {
 
 	unsigned int		fifo_size;	/* max. data size in the FIFO */
 	unsigned int		data_size;	/* data to be read from FIFO */
+	unsigned int		chunk_size;	/* split commands chunk size */
 	unsigned int		oob_size;
 	unsigned int		spare_size;
 	unsigned int		ecc_size;
@@ -275,6 +300,31 @@ static struct nand_bbt_descr bbt_mirror_descr = {
 	.pattern = bbt_mirror_pattern
 };
 
+static struct nand_ecclayout ecc_layout_4KB_bch4bit = {
+	.eccbytes = 64,
+	.eccpos = {
+		32,  33,  34,  35,  36,  37,  38,  39,
+		40,  41,  42,  43,  44,  45,  46,  47,
+		48,  49,  50,  51,  52,  53,  54,  55,
+		56,  57,  58,  59,  60,  61,  62,  63,
+		96,  97,  98,  99,  100, 101, 102, 103,
+		104, 105, 106, 107, 108, 109, 110, 111,
+		112, 113, 114, 115, 116, 117, 118, 119,
+		120, 121, 122, 123, 124, 125, 126, 127},
+	/* Bootrom looks in bytes 0 & 5 for bad blocks */
+	.oobfree = { {6, 26}, { 64, 32} }
+};
+
+static struct nand_ecclayout ecc_layout_4KB_bch8bit = {
+	.eccbytes = 128,
+	.eccpos = {
+		32,  33,  34,  35,  36,  37,  38,  39,
+		40,  41,  42,  43,  44,  45,  46,  47,
+		48,  49,  50,  51,  52,  53,  54,  55,
+		56,  57,  58,  59,  60,  61,  62,  63},
+	.oobfree = { }
+};
+
 /* Define a default flash type setting serve as flash detecting only */
 #define DEFAULT_FLASH_TYPE (&builtin_flash_types[0])
 
@@ -437,7 +487,7 @@ static void disable_int(struct pxa3xx_nand_info *info, uint32_t int_mask)
 
 static void handle_data_pio(struct pxa3xx_nand_info *info)
 {
-	unsigned int do_bytes = min(info->data_size, info->fifo_size);
+	unsigned int do_bytes = min(info->data_size, info->chunk_size);
 
 	switch (info->state) {
 	case STATE_PIO_WRITING:
@@ -675,7 +725,7 @@ static void prepare_start_command(struct pxa3xx_nand_info *info, int command)
 }
 
 static int prepare_set_command(struct pxa3xx_nand_info *info, int command,
-		uint16_t column, int page_addr)
+		int ext_cmd_type, uint16_t column, int page_addr)
 {
 	int addr_cycle, exec_cmd;
 	struct pxa3xx_nand_host *host;
@@ -708,9 +758,20 @@ static int prepare_set_command(struct pxa3xx_nand_info *info, int command,
 		if (command == NAND_CMD_READOOB)
 			info->buf_start += mtd->writesize;
 
-		/* Second command setting for large pages */
-		if (mtd->writesize >= PAGE_CHUNK_SIZE)
+		/*
+		 * Multiple page read needs an 'extended command type' field,
+		 * which is either naked-read or last-read according to the
+		 * state.
+		 */
+		if (mtd->writesize == PAGE_CHUNK_SIZE) {
 			info->ndcb0 |= NDCB0_DBC | (NAND_CMD_READSTART << 8);
+		} else if (mtd->writesize > PAGE_CHUNK_SIZE) {
+			info->ndcb0 |= NDCB0_DBC | (NAND_CMD_READSTART << 8)
+					| NDCB0_LEN_OVRD
+					| NDCB0_EXT_CMD_TYPE(ext_cmd_type);
+			info->ndcb3 = info->chunk_size +
+				      info->oob_size;
+		}
 
 		set_command_address(info, mtd->writesize, column, page_addr);
 		break;
@@ -829,7 +890,8 @@ static void pxa3xx_nand_cmdfunc(struct mtd_info *mtd, unsigned command,
 	prepare_start_command(info, command);
 
 	info->state = STATE_PREPARED;
-	exec_cmd = prepare_set_command(info, command, column, page_addr);
+	exec_cmd = prepare_set_command(info, command, -1, column, page_addr);
+
 	if (exec_cmd) {
 		init_completion(&info->cmd_complete);
 		init_completion(&info->dev_ready);
@@ -847,6 +909,91 @@ static void pxa3xx_nand_cmdfunc(struct mtd_info *mtd, unsigned command,
 	info->state = STATE_IDLE;
 }
 
+static void armada370_nand_cmdfunc(struct mtd_info *mtd,
+				   const unsigned command,
+				   int column, int page_addr)
+{
+	struct pxa3xx_nand_host *host = mtd->priv;
+	struct pxa3xx_nand_info *info = host->info_data;
+	int ret, exec_cmd, ext_cmd_type;
+
+	/*
+	 * if this is a x16 device ,then convert the input
+	 * "byte" address into a "word" address appropriate
+	 * for indexing a word-oriented device
+	 */
+	if (info->reg_ndcr & NDCR_DWIDTH_M)
+		column /= 2;
+
+	/*
+	 * There may be different NAND chip hooked to
+	 * different chip select, so check whether
+	 * chip select has been changed, if yes, reset the timing
+	 */
+	if (info->cs != host->cs) {
+		info->cs = host->cs;
+		nand_writel(info, NDTR0CS0, info->ndtr0cs0);
+		nand_writel(info, NDTR1CS0, info->ndtr1cs0);
+	}
+
+	/* Select the extended command for the first command */
+	switch (command) {
+	case NAND_CMD_READ0:
+	case NAND_CMD_READOOB:
+		ext_cmd_type = EXT_CMD_TYPE_MONO;
+		break;
+	}
+
+	prepare_start_command(info, command);
+
+	/*
+	 * Prepare the "is ready" completion before starting a command
+	 * transaction sequence. If the command is not executed the
+	 * completion will be completed, see below.
+	 *
+	 * We can do that inside the loop because the command variable
+	 * is invariant and thus so is the exec_cmd.
+	 */
+	atomic_set(&info->is_ready, 0);
+	init_completion(&info->dev_ready);
+	do {
+		info->state = STATE_PREPARED;
+		exec_cmd = prepare_set_command(info, command, ext_cmd_type,
+					       column, page_addr);
+		if (!exec_cmd) {
+			atomic_set(&info->is_ready, 1);
+			complete(&info->dev_ready);
+			break;
+		}
+
+		init_completion(&info->cmd_complete);
+		pxa3xx_nand_start(info);
+
+		ret = wait_for_completion_timeout(&info->cmd_complete,
+				CHIP_DELAY_TIMEOUT);
+		if (!ret) {
+			dev_err(&info->pdev->dev, "Wait time out!!!\n");
+			/* Stop State Machine for next command cycle */
+			pxa3xx_nand_stop(info);
+			break;
+		}
+
+		/* Check if the sequence is complete */
+		if (info->data_size == 0)
+			break;
+
+		if (command == NAND_CMD_READ0 || command == NAND_CMD_READOOB) {
+			/* Last read: issue a 'last naked read' */
+			if (info->data_size == info->chunk_size)
+				ext_cmd_type = EXT_CMD_TYPE_LAST_RW;
+			else
+				ext_cmd_type = EXT_CMD_TYPE_NAKED_RW;
+		}
+	} while (1);
+
+	info->state = STATE_IDLE;
+}
+
 static int pxa3xx_nand_write_page_hwecc(struct mtd_info *mtd,
 		struct nand_chip *chip, const uint8_t *buf, int oob_required)
 {
@@ -1032,6 +1179,8 @@ static int pxa3xx_nand_detect_config(struct pxa3xx_nand_info *info)
 		host->read_id_bytes = 2;
 	}
 
+	/* Set an initial chunk size */
+	info->chunk_size = info->fifo_size;
 	info->reg_ndcr = ndcr & ~NDCR_INT_MASK;
 	info->ndtr0cs0 = nand_readl(info, NDTR0CS0);
 	info->ndtr1cs0 = nand_readl(info, NDTR1CS0);
@@ -1136,6 +1285,7 @@ static int pxa_ecc_init(struct pxa3xx_nand_info *info,
 		ecc->size = 512;
 		ecc->strength = 1;
 
+		info->chunk_size = 2048;
 		info->spare_size = 40;
 		info->ecc_size = 24;
 		return 1;
@@ -1145,6 +1295,7 @@ static int pxa_ecc_init(struct pxa3xx_nand_info *info,
 		ecc->size = 512;
 		ecc->strength = 1;
 
+		info->chunk_size = 512;
 		info->spare_size = 8;
 		info->ecc_size = 8;
 		return 1;
@@ -1156,7 +1307,30 @@ static int armada370_ecc_init(struct pxa3xx_nand_info *info,
 			      struct nand_ecc_ctrl *ecc,
 			      int strength, int page_size)
 {
-	/* Unimplemented yet */
+	if (strength == 4 && page_size == 4096) {
+		ecc->mode = NAND_ECC_HW;
+		ecc->size = 512;
+		ecc->layout = &ecc_layout_4KB_bch4bit;
+		ecc->strength = 4;
+
+		info->ecc_bch = 1;
+		info->chunk_size = 2048;
+		info->spare_size = 32;
+		info->ecc_size = 32;
+		return 1;
+
+	} else if (strength == 8 && page_size == 4096) {
+		ecc->mode = NAND_ECC_HW;
+		ecc->size = 512;
+		ecc->layout = &ecc_layout_4KB_bch8bit;
+		ecc->strength = 8;
+
+		info->ecc_bch = 1;
+		info->chunk_size = 1024;
+		info->spare_size = 0;
+		info->ecc_size = 32;
+		return 1;
+	}
 	return 0;
 }
 
@@ -1320,12 +1494,16 @@ static int alloc_nand_resource(struct platform_device *pdev)
 		chip->controller        = &info->controller;
 		chip->waitfunc		= pxa3xx_nand_waitfunc;
 		chip->select_chip	= pxa3xx_nand_select_chip;
-		chip->cmdfunc		= pxa3xx_nand_cmdfunc;
 		chip->read_word		= pxa3xx_nand_read_word;
 		chip->read_byte		= pxa3xx_nand_read_byte;
 		chip->read_buf		= pxa3xx_nand_read_buf;
 		chip->write_buf		= pxa3xx_nand_write_buf;
 		chip->options		|= NAND_NO_SUBPAGE_WRITE;
+
+		if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
+			chip->cmdfunc = armada370_nand_cmdfunc;
+		else
+			chip->cmdfunc = pxa3xx_nand_cmdfunc;
 	}
 
 	spin_lock_init(&chip->controller->lock);
-- 
1.8.1.5

[PATCH v2 23/27] mtd: nand: pxa3xx: Add multiple chunk write support

[Ezequiel Garcia]

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

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

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

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

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

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

[PATCH v2 24/27] mtd: nand: pxa3xx: Add ECC BCH correctable errors detection

[Ezequiel Garcia]

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

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

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

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 5e92ef8..cbc7240 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -97,6 +97,9 @@
 #define NDCR_INT_MASK           (0xFFF)
 
 #define NDSR_MASK		(0xfff)
+#define NDSR_ERR_CNT_OFF	(16)
+#define NDSR_ERR_CNT_MASK       (0x1f)
+#define NDSR_ERR_CNT(sr)	((sr >> NDSR_ERR_CNT_OFF) & NDSR_ERR_CNT_MASK)
 #define NDSR_RDY                (0x1 << 12)
 #define NDSR_FLASH_RDY          (0x1 << 11)
 #define NDSR_CS0_PAGED		(0x1 << 10)
@@ -105,8 +108,8 @@
 #define NDSR_CS1_CMDD		(0x1 << 7)
 #define NDSR_CS0_BBD		(0x1 << 6)
 #define NDSR_CS1_BBD		(0x1 << 5)
-#define NDSR_DBERR		(0x1 << 4)
-#define NDSR_SBERR		(0x1 << 3)
+#define NDSR_UNCORERR		(0x1 << 4)
+#define NDSR_CORERR		(0x1 << 3)
 #define NDSR_WRDREQ		(0x1 << 2)
 #define NDSR_RDDREQ		(0x1 << 1)
 #define NDSR_WRCMDREQ		(0x1)
@@ -147,9 +150,9 @@ enum {
 	ERR_NONE	= 0,
 	ERR_DMABUSERR	= -1,
 	ERR_SENDCMD	= -2,
-	ERR_DBERR	= -3,
+	ERR_UNCORERR	= -3,
 	ERR_BBERR	= -4,
-	ERR_SBERR	= -5,
+	ERR_CORERR	= -5,
 };
 
 enum {
@@ -240,6 +243,7 @@ struct pxa3xx_nand_info {
 	unsigned int		oob_size;
 	unsigned int		spare_size;
 	unsigned int		ecc_size;
+	unsigned int		ecc_err_cnt;
 	int 			retcode;
 
 	/* cached register value */
@@ -590,10 +594,16 @@ static irqreturn_t pxa3xx_nand_irq(int irq, void *devid)
 
 	status = nand_readl(info, NDSR);
 
-	if (status & NDSR_DBERR)
-		info->retcode = ERR_DBERR;
-	if (status & NDSR_SBERR)
-		info->retcode = ERR_SBERR;
+	if (status & NDSR_UNCORERR)
+		info->retcode = ERR_UNCORERR;
+	if (status & NDSR_CORERR) {
+		info->retcode = ERR_CORERR;
+		if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370 &&
+		    info->ecc_bch)
+			info->ecc_err_cnt = NDSR_ERR_CNT(status);
+		else
+			info->ecc_err_cnt = 1;
+	}
 	if (status & (NDSR_RDDREQ | NDSR_WRDREQ)) {
 		/* whether use dma to transfer data */
 		if (info->use_dma) {
@@ -692,6 +702,7 @@ static void prepare_start_command(struct pxa3xx_nand_info *info, int command)
 	info->use_ecc		= 0;
 	info->use_spare		= 1;
 	info->retcode		= ERR_NONE;
+	info->ecc_err_cnt	= 0;
 	info->ndcb3		= 0;
 
 	switch (command) {
@@ -1080,20 +1091,14 @@ static int pxa3xx_nand_read_page_hwecc(struct mtd_info *mtd,
 	chip->read_buf(mtd, buf, mtd->writesize);
 	chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);
 
-	if (info->retcode == ERR_SBERR) {
-		switch (info->use_ecc) {
-		case 1:
-			mtd->ecc_stats.corrected++;
-			break;
-		case 0:
-		default:
-			break;
-		}
-	} else if (info->retcode == ERR_DBERR) {
+	if (info->retcode == ERR_CORERR && info->use_ecc) {
+		mtd->ecc_stats.corrected += info->ecc_err_cnt;
+
+	} else if (info->retcode == ERR_UNCORERR) {
 		/*
 		 * for blank page (all 0xff), HW will calculate its ECC as
 		 * 0, which is different from the ECC information within
-		 * OOB, ignore such double bit errors
+		 * OOB, ignore such ucorrectable errors
 		 */
 		if (is_buf_blank(buf, mtd->writesize))
 			info->retcode = ERR_NONE;
-- 
1.8.1.5

[PATCH v2 25/27] ARM: mvebu: Add support for NAND controller in Armada 370/XP

[Ezequiel Garcia]

The Armada 370 and Armada XP SoC have a NAND controller (aka NFCv2).
This commit adds support for it in Armada 370 and Armada XP SoC
common devicetree.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/boot/dts/armada-370-xp.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/armada-370-xp.dtsi b/arch/arm/boot/dts/armada-370-xp.dtsi
index 01e69fc..b4e6898 100644
--- a/arch/arm/boot/dts/armada-370-xp.dtsi
+++ b/arch/arm/boot/dts/armada-370-xp.dtsi
@@ -258,6 +258,15 @@
 				status = "disabled";
 			};
 
+			nand at d0000 {
+				compatible = "marvell,armada370-nand";
+				reg = <0xd0000 0x54>;
+				#address-cells = <1>;
+				#size-cells = <1>;
+				interrupts = <113>;
+				clocks = <&coredivclk 0>;
+				status = "disabled";
+			};
 		};
 	};
 
-- 
1.8.1.5

[PATCH v2 26/27] ARM: mvebu: Enable NAND controller in Armada XP GP board

[Ezequiel Garcia]

The Armada XP GP board has a NAND flash, so enable it in the devicetree.

In order to skip the driver's custom device detection and use only ONFI
detection, the "marvell,keep-config" parameter is used.
This is needed because we haven't support for setting the timings
parameters yet and must rely in bootloader's.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/boot/dts/armada-xp-gp.dts | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/armada-xp-gp.dts b/arch/arm/boot/dts/armada-xp-gp.dts
index 2298e4a..274e2ad 100644
--- a/arch/arm/boot/dts/armada-xp-gp.dts
+++ b/arch/arm/boot/dts/armada-xp-gp.dts
@@ -175,6 +175,14 @@
 					spi-max-frequency = <108000000>;
 				};
 			};
+
+			nand at d0000 {
+				status = "okay";
+				num-cs = <1>;
+				marvell,nand-keep-config;
+				marvell,nand-enable-arbiter;
+				nand-on-flash-bbt;
+			};
 		};
 	};
 };
-- 
1.8.1.5

[PATCH v2 27/27] ARM: mvebu: Enable NAND controller in Armada 370 Mirabox

[Ezequiel Garcia]

The Armada 370 Mirabox has a NAND flash, so enable it in the devicetree
and add the partitions as prepared in the factory images.

In order to skip the driver's custom device detection and use only
ONFI detection, the "marvell,keep-config" parameter is used.
This is needed because we have no support for setting the timings parameters yet.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/boot/dts/armada-370-mirabox.dts | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm/boot/dts/armada-370-mirabox.dts b/arch/arm/boot/dts/armada-370-mirabox.dts
index 2471d9d..1e73017 100644
--- a/arch/arm/boot/dts/armada-370-mirabox.dts
+++ b/arch/arm/boot/dts/armada-370-mirabox.dts
@@ -139,6 +139,27 @@
 					reg = <0x25>;
 				};
 			};
+
+			nand at d0000 {
+				status = "okay";
+				num-cs = <1>;
+				marvell,nand-keep-config;
+				marvell,nand-enable-arbiter;
+				nand-on-flash-bbt;
+
+				partition at 0 {
+					label = "U-Boot";
+					reg = <0 0x400000>;
+				};
+				partition at 400000 {
+					label = "Linux";
+					reg = <0x400000 0x400000>;
+				};
+				partition at 800000 {
+					label = "Filesystem";
+					reg = <0x800000 0x3f800000>;
+				};
+			};
 		};
 	};
 };
-- 
1.8.1.5

[PATCH v2 00/27] Armada 370/XP NAND support

[Willy Tarreau]

Hi Ezequiel,

On Fri, Oct 18, 2013 at 08:02:27PM -0300, Ezequiel Garcia wrote:
> Hi guys,
> 
> This is the v2 of the work implementing NAND support in Armada 370/XP SoCs.
> This series is probably not yet complete (see below), but I feel like
> we're really closer now :-)
> 
> As in the previous version and given I don't have any public datasheet
> to show, I'll try to write some of the most relevant parts of the controller.
> 
> * NFCv2 controller background
> 
> The controller has a 2176 bytes FIFO buffer. Therefore, in order to support
> larger pages, I/O operations on 4 KiB and 8 KiB pages is done with a set of
> chunked transfers.
> 
> For instance, if we choose a 2048 data chunk and set "BCH" ECC (see below)
> we'll have this layout in the pages:
> 
>   ------------------------------------------------------------------------------
>   | 2048B data | 32B spare | 30B ECC || 2048B data | 32B spare | 30B ECC | ... |
>   ------------------------------------------------------------------------------
> 
> The driver reads the data and spare portions independently and builds an internal
> buffer with this layout (in the 4 KiB page case):
> 
>   ------------------------------------------
>   |     4096B data     |     64B spare     |
>   ------------------------------------------
> 
> Also, for the READOOB command the driver disables the ECC and reads a 'spare + ECC'
> OOB, one per chunk read.
> 
>   -------------------------------------------------------------------
>   |     4096B data     |  32B spare | 30B ECC | 32B spare | 30B ECC |
>   -------------------------------------------------------------------
> 
> So, in order to achieve reading (for instance), we issue several READ0 commands
> (with some additional controller-specific magic) and read two chunks of 2080B
> (2048 data + 32 spare) each.
> The driver accomodates this data to expose the NAND core a contiguous buffer
> (4096 data + spare) or (4096 + spare + ECC + spare + ECC).
> 
> * ECC
> 
> The controller has built-in hardware ECC capabilities. In addition it is
> configurable between two modes: 1) Hamming, 2) BCH.
> 
> Note that the actual BCH mode: BCH-4 or BCH-8 will depend on the way
> the controller is configured to transfer the data.
> 
> In the BCH mode the ECC code will be calculated for each transfered chunk
> and expected to be located (when reading/programming) right after the spare
> bytes as the figure above shows.
> 
> So, repeating the above scheme, a 2048B data chunk will be followed by 32B
> spare, and then the ECC controller will read/write the ECC code (30B in
> this case):
> 
>   ------------------------------------
>   | 2048B data | 32B spare | 30B ECC |
>   ------------------------------------
> 
> If the ECC mode is 'BCH' then the ECC is *always* 30 bytes long.
> If the ECC mode is 'Hamming' the ECC is 6 bytes long, for each 512B block.
> So in Hamming mode, a 2048B page will have a 24B ECC.
> 
> Despite all of the above, the controller requires the driver to only read or
> write in multiples of 8-bytes, because the data buffer is 64-bits.
> 
> * Changes from v1
> 
> Aside from several changes based in Brian's feedback, the main changes
> from v1 are:
> 
>   * The controller's clock source is now fully modeled, see patche 1 to 4.
>     Of course, none of those patches should be taken through the mtd
>     subsystem, but I'm adding them here for completeness.
> 
>   * The chip's cmdfunc() is now independently implemented in each SoC variant.
>     The rationale behind this decision is that 'chunked' I/O is the only tested
>     mode on the Armada370 variant, while the old 'vanilla' I/O is the only
>     tested mode on the PXA variant.
> 
>     So it's safer to have an implementation for each variant.
> 
>   * Added support for BCH-8, in other words: 8-bits of correction in a 512-byte
>     region. This is obtained by using a data chunk size of 1024B, thus doubling
>     the ECC BCH strength, as per this ECC engine mechanism.
> 
>   * The ECC layout in use, which must be set according to the page size and
>     desired ECC strength is now strictly chosen to match only the tested
>     combinations.
> 
> * Pending stuff
> 
>   1. Factory bad blocks handling
> 
>   Currently, there's no factory bad block initial scan (when the bad block
>   table is missing). The ECC BCH requires to layout the device's pages in
>   a splitted "data + OOB + data + OOB" way. This layout being different from
>   the factory layout. In other words,
> 
>   Factory view:
> 
>   -----------------------------------------------
>   |                    Data           |x  OOB   |
>   -----------------------------------------------
> 
>   Driver's view:
> 
>   -----------------------------------------------
>   |      Data      | OOB |      Data   x  | OOB |
>   -----------------------------------------------
> 
>   It can be seen from the above, that the factory bad block marker must be
>   searched within the 'data' region, and not in the usual OOB region.
> 
>   This seems to be similar to the gpmi-nand driver, yet I really find its
>   'bad block swapping' solution odd, so I'm trying to find something
>   cleaner.
> 
>   2. ECC modeling
> 
>   Although I've exposed some ECC information in the ECC layout, I'm still
>   not sure why is this needed, or if it's needed at all.
> 

I strongly believe that all the information above is very valuable and
that it's a shame to lose it in a cover message. It would definitely
make sense to have this in a doc file.

Best regards,
Willy

[PATCH v2 00/27] Armada 370/XP NAND support

[Jason Cooper]

On Sat, Oct 19, 2013 at 08:35:35AM +0200, Willy Tarreau wrote:
> Hi Ezequiel,
> 
> On Fri, Oct 18, 2013 at 08:02:27PM -0300, Ezequiel Garcia wrote:
> > Hi guys,
> > 
> > This is the v2 of the work implementing NAND support in Armada 370/XP SoCs.
> > This series is probably not yet complete (see below), but I feel like
> > we're really closer now :-)
> > 
> > As in the previous version and given I don't have any public datasheet
> > to show, I'll try to write some of the most relevant parts of the controller.
> > 
> > * NFCv2 controller background
> > 
> > The controller has a 2176 bytes FIFO buffer. Therefore, in order to support
> > larger pages, I/O operations on 4 KiB and 8 KiB pages is done with a set of
> > chunked transfers.
> > 
> > For instance, if we choose a 2048 data chunk and set "BCH" ECC (see below)
> > we'll have this layout in the pages:
> > 
> >   ------------------------------------------------------------------------------
> >   | 2048B data | 32B spare | 30B ECC || 2048B data | 32B spare | 30B ECC | ... |
> >   ------------------------------------------------------------------------------
> > 
> > The driver reads the data and spare portions independently and builds an internal
> > buffer with this layout (in the 4 KiB page case):
> > 
> >   ------------------------------------------
> >   |     4096B data     |     64B spare     |
> >   ------------------------------------------
> > 
> > Also, for the READOOB command the driver disables the ECC and reads a 'spare + ECC'
> > OOB, one per chunk read.
> > 
> >   -------------------------------------------------------------------
> >   |     4096B data     |  32B spare | 30B ECC | 32B spare | 30B ECC |
> >   -------------------------------------------------------------------
> > 
> > So, in order to achieve reading (for instance), we issue several READ0 commands
> > (with some additional controller-specific magic) and read two chunks of 2080B
> > (2048 data + 32 spare) each.
> > The driver accomodates this data to expose the NAND core a contiguous buffer
> > (4096 data + spare) or (4096 + spare + ECC + spare + ECC).
> > 
> > * ECC
> > 
> > The controller has built-in hardware ECC capabilities. In addition it is
> > configurable between two modes: 1) Hamming, 2) BCH.
> > 
> > Note that the actual BCH mode: BCH-4 or BCH-8 will depend on the way
> > the controller is configured to transfer the data.
> > 
> > In the BCH mode the ECC code will be calculated for each transfered chunk
> > and expected to be located (when reading/programming) right after the spare
> > bytes as the figure above shows.
> > 
> > So, repeating the above scheme, a 2048B data chunk will be followed by 32B
> > spare, and then the ECC controller will read/write the ECC code (30B in
> > this case):
> > 
> >   ------------------------------------
> >   | 2048B data | 32B spare | 30B ECC |
> >   ------------------------------------
> > 
> > If the ECC mode is 'BCH' then the ECC is *always* 30 bytes long.
> > If the ECC mode is 'Hamming' the ECC is 6 bytes long, for each 512B block.
> > So in Hamming mode, a 2048B page will have a 24B ECC.
> > 
> > Despite all of the above, the controller requires the driver to only read or
> > write in multiples of 8-bytes, because the data buffer is 64-bits.
> > 
> > * Changes from v1
> > 
> > Aside from several changes based in Brian's feedback, the main changes
> > from v1 are:
> > 
> >   * The controller's clock source is now fully modeled, see patche 1 to 4.
> >     Of course, none of those patches should be taken through the mtd
> >     subsystem, but I'm adding them here for completeness.
> > 
> >   * The chip's cmdfunc() is now independently implemented in each SoC variant.
> >     The rationale behind this decision is that 'chunked' I/O is the only tested
> >     mode on the Armada370 variant, while the old 'vanilla' I/O is the only
> >     tested mode on the PXA variant.
> > 
> >     So it's safer to have an implementation for each variant.
> > 
> >   * Added support for BCH-8, in other words: 8-bits of correction in a 512-byte
> >     region. This is obtained by using a data chunk size of 1024B, thus doubling
> >     the ECC BCH strength, as per this ECC engine mechanism.
> > 
> >   * The ECC layout in use, which must be set according to the page size and
> >     desired ECC strength is now strictly chosen to match only the tested
> >     combinations.
> > 
> > * Pending stuff
> > 
> >   1. Factory bad blocks handling
> > 
> >   Currently, there's no factory bad block initial scan (when the bad block
> >   table is missing). The ECC BCH requires to layout the device's pages in
> >   a splitted "data + OOB + data + OOB" way. This layout being different from
> >   the factory layout. In other words,
> > 
> >   Factory view:
> > 
> >   -----------------------------------------------
> >   |                    Data           |x  OOB   |
> >   -----------------------------------------------
> > 
> >   Driver's view:
> > 
> >   -----------------------------------------------
> >   |      Data      | OOB |      Data   x  | OOB |
> >   -----------------------------------------------
> > 
> >   It can be seen from the above, that the factory bad block marker must be
> >   searched within the 'data' region, and not in the usual OOB region.
> > 
> >   This seems to be similar to the gpmi-nand driver, yet I really find its
> >   'bad block swapping' solution odd, so I'm trying to find something
> >   cleaner.
> > 
> >   2. ECC modeling
> > 
> >   Although I've exposed some ECC information in the ECC layout, I'm still
> >   not sure why is this needed, or if it's needed at all.
> > 
> 
> I strongly believe that all the information above is very valuable and
> that it's a shame to lose it in a cover message. It would definitely
> make sense to have this in a doc file.

Agreed.  Great work Ezequiel!  Let's capture this in Documentation/.

thx,

Jason.

[PATCH v2 00/27] Armada 370/XP NAND support

[Ezequiel Garcia]

On Sat, Oct 19, 2013 at 10:25:49AM -0400, Jason Cooper wrote:
> On Sat, Oct 19, 2013 at 08:35:35AM +0200, Willy Tarreau wrote:
> > Hi Ezequiel,
> > 
> > On Fri, Oct 18, 2013 at 08:02:27PM -0300, Ezequiel Garcia wrote:
> > > Hi guys,
> > > 
> > > This is the v2 of the work implementing NAND support in Armada 370/XP SoCs.
> > > This series is probably not yet complete (see below), but I feel like
> > > we're really closer now :-)
> > > 
> > > As in the previous version and given I don't have any public datasheet
> > > to show, I'll try to write some of the most relevant parts of the controller.
> > > 
> > > * NFCv2 controller background
> > > 
> > > The controller has a 2176 bytes FIFO buffer. Therefore, in order to support
> > > larger pages, I/O operations on 4 KiB and 8 KiB pages is done with a set of
> > > chunked transfers.
> > > 
> > > For instance, if we choose a 2048 data chunk and set "BCH" ECC (see below)
> > > we'll have this layout in the pages:
> > > 
> > >   ------------------------------------------------------------------------------
> > >   | 2048B data | 32B spare | 30B ECC || 2048B data | 32B spare | 30B ECC | ... |
> > >   ------------------------------------------------------------------------------
> > > 
> > > The driver reads the data and spare portions independently and builds an internal
> > > buffer with this layout (in the 4 KiB page case):
> > > 
> > >   ------------------------------------------
> > >   |     4096B data     |     64B spare     |
> > >   ------------------------------------------
> > > 
> > > Also, for the READOOB command the driver disables the ECC and reads a 'spare + ECC'
> > > OOB, one per chunk read.
> > > 
> > >   -------------------------------------------------------------------
> > >   |     4096B data     |  32B spare | 30B ECC | 32B spare | 30B ECC |
> > >   -------------------------------------------------------------------
> > > 
> > > So, in order to achieve reading (for instance), we issue several READ0 commands
> > > (with some additional controller-specific magic) and read two chunks of 2080B
> > > (2048 data + 32 spare) each.
> > > The driver accomodates this data to expose the NAND core a contiguous buffer
> > > (4096 data + spare) or (4096 + spare + ECC + spare + ECC).
> > > 
> > > * ECC
> > > 
> > > The controller has built-in hardware ECC capabilities. In addition it is
> > > configurable between two modes: 1) Hamming, 2) BCH.
> > > 
> > > Note that the actual BCH mode: BCH-4 or BCH-8 will depend on the way
> > > the controller is configured to transfer the data.
> > > 
> > > In the BCH mode the ECC code will be calculated for each transfered chunk
> > > and expected to be located (when reading/programming) right after the spare
> > > bytes as the figure above shows.
> > > 
> > > So, repeating the above scheme, a 2048B data chunk will be followed by 32B
> > > spare, and then the ECC controller will read/write the ECC code (30B in
> > > this case):
> > > 
> > >   ------------------------------------
> > >   | 2048B data | 32B spare | 30B ECC |
> > >   ------------------------------------
> > > 
> > > If the ECC mode is 'BCH' then the ECC is *always* 30 bytes long.
> > > If the ECC mode is 'Hamming' the ECC is 6 bytes long, for each 512B block.
> > > So in Hamming mode, a 2048B page will have a 24B ECC.
> > > 
> > > Despite all of the above, the controller requires the driver to only read or
> > > write in multiples of 8-bytes, because the data buffer is 64-bits.
> > > 
> > > * Changes from v1
> > > 
> > > Aside from several changes based in Brian's feedback, the main changes
> > > from v1 are:
> > > 
> > >   * The controller's clock source is now fully modeled, see patche 1 to 4.
> > >     Of course, none of those patches should be taken through the mtd
> > >     subsystem, but I'm adding them here for completeness.
> > > 
> > >   * The chip's cmdfunc() is now independently implemented in each SoC variant.
> > >     The rationale behind this decision is that 'chunked' I/O is the only tested
> > >     mode on the Armada370 variant, while the old 'vanilla' I/O is the only
> > >     tested mode on the PXA variant.
> > > 
> > >     So it's safer to have an implementation for each variant.
> > > 
> > >   * Added support for BCH-8, in other words: 8-bits of correction in a 512-byte
> > >     region. This is obtained by using a data chunk size of 1024B, thus doubling
> > >     the ECC BCH strength, as per this ECC engine mechanism.
> > > 
> > >   * The ECC layout in use, which must be set according to the page size and
> > >     desired ECC strength is now strictly chosen to match only the tested
> > >     combinations.
> > > 
> > > * Pending stuff
> > > 
> > >   1. Factory bad blocks handling
> > > 
> > >   Currently, there's no factory bad block initial scan (when the bad block
> > >   table is missing). The ECC BCH requires to layout the device's pages in
> > >   a splitted "data + OOB + data + OOB" way. This layout being different from
> > >   the factory layout. In other words,
> > > 
> > >   Factory view:
> > > 
> > >   -----------------------------------------------
> > >   |                    Data           |x  OOB   |
> > >   -----------------------------------------------
> > > 
> > >   Driver's view:
> > > 
> > >   -----------------------------------------------
> > >   |      Data      | OOB |      Data   x  | OOB |
> > >   -----------------------------------------------
> > > 
> > >   It can be seen from the above, that the factory bad block marker must be
> > >   searched within the 'data' region, and not in the usual OOB region.
> > > 
> > >   This seems to be similar to the gpmi-nand driver, yet I really find its
> > >   'bad block swapping' solution odd, so I'm trying to find something
> > >   cleaner.
> > > 
> > >   2. ECC modeling
> > > 
> > >   Although I've exposed some ECC information in the ECC layout, I'm still
> > >   not sure why is this needed, or if it's needed at all.
> > > 
> > 
> > I strongly believe that all the information above is very valuable and
> > that it's a shame to lose it in a cover message. It would definitely
> > make sense to have this in a doc file.
> 
> Agreed.  Great work Ezequiel!  Let's capture this in Documentation/.
> 

Wait a second!

  $ ls Documentation/mtd/ | wc -l
  1

Isn't documenting deprecated? ...

Just kiddin'! I'll add this and the other cover to Documentation/
so typos and english fixes are welcome!
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[PATCH v2 00/27] Armada 370/XP NAND support

[Thomas Petazzoni]

Dear Ezequiel Garcia,

On Sat, 19 Oct 2013 13:21:59 -0300, Ezequiel Garcia wrote:

> > > I strongly believe that all the information above is very valuable and
> > > that it's a shame to lose it in a cover message. It would definitely
> > > make sense to have this in a doc file.
> > 
> > Agreed.  Great work Ezequiel!  Let's capture this in Documentation/.
> > 
> 
> Wait a second!
> 
>   $ ls Documentation/mtd/ | wc -l
>   1
> 
> Isn't documenting deprecated? ...
> 
> Just kiddin'! I'll add this and the other cover to Documentation/
> so typos and english fixes are welcome!

I certainly agree this documentation should be kept, but I'm wondering
if keeping it as a comment inside the driver doesn't make more sense.
This document is really more about specific implementation details, and
the next person looking at the driver is not very likely to search such
details in Documentation/, in my opinion.

Best regards,

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

[PATCH v2 00/27] Armada 370/XP NAND support

[Jason Cooper]

On Sun, Oct 20, 2013 at 07:50:49PM +0200, Thomas Petazzoni wrote:
> Dear Ezequiel Garcia,
> 
> On Sat, 19 Oct 2013 13:21:59 -0300, Ezequiel Garcia wrote:
> 
> > > > I strongly believe that all the information above is very valuable and
> > > > that it's a shame to lose it in a cover message. It would definitely
> > > > make sense to have this in a doc file.
> > > 
> > > Agreed.  Great work Ezequiel!  Let's capture this in Documentation/.
> > > 
> > 
> > Wait a second!
> > 
> >   $ ls Documentation/mtd/ | wc -l
> >   1
> > 
> > Isn't documenting deprecated? ...
> > 
> > Just kiddin'! I'll add this and the other cover to Documentation/
> > so typos and english fixes are welcome!
> 
> I certainly agree this documentation should be kept, but I'm wondering
> if keeping it as a comment inside the driver doesn't make more sense.

That's a _long_ comment...

> This document is really more about specific implementation details, and
> the next person looking at the driver is not very likely to search such
> details in Documentation/, in my opinion.

A succinct comment in the code pointing to the Documentation/ should be
sufficient.

thx,

Jason.

[PATCH v2 01/27] clk: mvebu: Add Core Divider clock

[Jason Cooper]

Ezequiel,

MikeT took this the other day.

As such, I've taken in patches 2-4 inclusive into mvebu/dt.  That should
be the last items for this window.

Please drop them from your series next time around.

thx,

Jason.

On Fri, Oct 18, 2013 at 08:02:28PM -0300, Ezequiel Garcia wrote:
> This commit introduces a new group of clocks present in Armada 370/XP
> SoCs (called "Core Divider" clocks) and add a provider for them.
> The only clock supported for now is the NAND clock (ndclk), but the
> infrastructure to add the rest is already set.
> 
> Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Acked-by: Jason Cooper <jason@lakedaemon.net>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
>  drivers/clk/mvebu/Kconfig       |   5 +
>  drivers/clk/mvebu/Makefile      |   1 +
>  drivers/clk/mvebu/clk-corediv.c | 223 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 229 insertions(+)
>  create mode 100644 drivers/clk/mvebu/clk-corediv.c
> 
> diff --git a/drivers/clk/mvebu/Kconfig b/drivers/clk/mvebu/Kconfig
> index 0b0f3e7..c339b82 100644
> --- a/drivers/clk/mvebu/Kconfig
> +++ b/drivers/clk/mvebu/Kconfig
> @@ -4,15 +4,20 @@ config MVEBU_CLK_COMMON
>  config MVEBU_CLK_CPU
>  	bool
>  
> +config MVEBU_CLK_COREDIV
> +	bool
> +
>  config ARMADA_370_CLK
>  	bool
>  	select MVEBU_CLK_COMMON
>  	select MVEBU_CLK_CPU
> +	select MVEBU_CLK_COREDIV
>  
>  config ARMADA_XP_CLK
>  	bool
>  	select MVEBU_CLK_COMMON
>  	select MVEBU_CLK_CPU
> +	select MVEBU_CLK_COREDIV
>  
>  config DOVE_CLK
>  	bool
> diff --git a/drivers/clk/mvebu/Makefile b/drivers/clk/mvebu/Makefile
> index 1c7e70c..21bbfb4 100644
> --- a/drivers/clk/mvebu/Makefile
> +++ b/drivers/clk/mvebu/Makefile
> @@ -1,5 +1,6 @@
>  obj-$(CONFIG_MVEBU_CLK_COMMON)	+= common.o
>  obj-$(CONFIG_MVEBU_CLK_CPU) 	+= clk-cpu.o
> +obj-$(CONFIG_MVEBU_CLK_COREDIV)	+= clk-corediv.o
>  
>  obj-$(CONFIG_ARMADA_370_CLK)	+= armada-370.o
>  obj-$(CONFIG_ARMADA_XP_CLK)	+= armada-xp.o
> diff --git a/drivers/clk/mvebu/clk-corediv.c b/drivers/clk/mvebu/clk-corediv.c
> new file mode 100644
> index 0000000..7162615
> --- /dev/null
> +++ b/drivers/clk/mvebu/clk-corediv.c
> @@ -0,0 +1,223 @@
> +/*
> + * MVEBU Core divider clock
> + *
> + * Copyright (C) 2013 Marvell
> + *
> + * Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2.  This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/clk-provider.h>
> +#include <linux/of_address.h>
> +#include <linux/slab.h>
> +#include <linux/delay.h>
> +#include "common.h"
> +
> +#define CORE_CLK_DIV_RATIO_MASK		0xff
> +#define CORE_CLK_DIV_RATIO_RELOAD	BIT(8)
> +#define CORE_CLK_DIV_ENABLE_OFFSET	24
> +#define CORE_CLK_DIV_RATIO_OFFSET	0x8
> +
> +struct clk_corediv_desc {
> +	unsigned int mask;
> +	unsigned int offset;
> +	unsigned int fieldbit;
> +};
> +
> +struct clk_corediv {
> +	struct clk_hw hw;
> +	void __iomem *reg;
> +	struct clk_corediv_desc desc;
> +	spinlock_t lock;
> +};
> +
> +static struct clk_onecell_data clk_data;
> +
> +static const struct clk_corediv_desc mvebu_corediv_desc[] __initconst = {
> +	{ .mask = 0x3f, .offset = 8, .fieldbit = 1 }, /* NAND clock */
> +};
> +
> +#define to_corediv_clk(p) container_of(p, struct clk_corediv, hw)
> +
> +static int clk_corediv_is_enabled(struct clk_hw *hwclk)
> +{
> +	struct clk_corediv *corediv = to_corediv_clk(hwclk);
> +	struct clk_corediv_desc *desc = &corediv->desc;
> +	u32 enable_mask = BIT(desc->fieldbit) << CORE_CLK_DIV_ENABLE_OFFSET;
> +
> +	return !!(readl(corediv->reg) & enable_mask);
> +}
> +
> +static int clk_corediv_enable(struct clk_hw *hwclk)
> +{
> +	struct clk_corediv *corediv = to_corediv_clk(hwclk);
> +	struct clk_corediv_desc *desc = &corediv->desc;
> +	unsigned long flags = 0;
> +	u32 reg;
> +
> +	spin_lock_irqsave(&corediv->lock, flags);
> +
> +	reg = readl(corediv->reg);
> +	reg |= (BIT(desc->fieldbit) << CORE_CLK_DIV_ENABLE_OFFSET);
> +	writel(reg, corediv->reg);
> +
> +	spin_unlock_irqrestore(&corediv->lock, flags);
> +
> +	return 0;
> +}
> +
> +static void clk_corediv_disable(struct clk_hw *hwclk)
> +{
> +	struct clk_corediv *corediv = to_corediv_clk(hwclk);
> +	struct clk_corediv_desc *desc = &corediv->desc;
> +	unsigned long flags = 0;
> +	u32 reg;
> +
> +	spin_lock_irqsave(&corediv->lock, flags);
> +
> +	reg = readl(corediv->reg);
> +	reg &= ~(BIT(desc->fieldbit) << CORE_CLK_DIV_ENABLE_OFFSET);
> +	writel(reg, corediv->reg);
> +
> +	spin_unlock_irqrestore(&corediv->lock, flags);
> +}
> +
> +static unsigned long clk_corediv_recalc_rate(struct clk_hw *hwclk,
> +					 unsigned long parent_rate)
> +{
> +	struct clk_corediv *corediv = to_corediv_clk(hwclk);
> +	struct clk_corediv_desc *desc = &corediv->desc;
> +	u32 reg, div;
> +
> +	reg = readl(corediv->reg + CORE_CLK_DIV_RATIO_OFFSET);
> +	div = (reg >> desc->offset) & desc->mask;
> +	return parent_rate / div;
> +}
> +
> +static long clk_corediv_round_rate(struct clk_hw *hwclk, unsigned long rate,
> +			       unsigned long *parent_rate)
> +{
> +	/* Valid ratio are 1:4, 1:5, 1:6 and 1:8 */
> +	u32 div;
> +
> +	div = *parent_rate / rate;
> +	if (div < 4)
> +		div = 4;
> +	else if (div > 6)
> +		div = 8;
> +
> +	return *parent_rate / div;
> +}
> +
> +static int clk_corediv_set_rate(struct clk_hw *hwclk, unsigned long rate,
> +			    unsigned long parent_rate)
> +{
> +	struct clk_corediv *corediv = to_corediv_clk(hwclk);
> +	struct clk_corediv_desc *desc = &corediv->desc;
> +	unsigned long flags = 0;
> +	u32 reg, div;
> +
> +	div = parent_rate / rate;
> +
> +	spin_lock_irqsave(&corediv->lock, flags);
> +
> +	/* Write new divider to the divider ratio register */
> +	reg = readl(corediv->reg + CORE_CLK_DIV_RATIO_OFFSET);
> +	reg &= ~(desc->mask << desc->offset);
> +	reg |= (div & desc->mask) << desc->offset;
> +	writel(reg, corediv->reg + CORE_CLK_DIV_RATIO_OFFSET);
> +
> +	/* Set reload-force for this clock */
> +	reg = readl(corediv->reg) | BIT(desc->fieldbit);
> +	writel(reg, corediv->reg);
> +
> +	/* Now trigger the clock update */
> +	reg = readl(corediv->reg) | CORE_CLK_DIV_RATIO_RELOAD;
> +	writel(reg, corediv->reg);
> +
> +	/*
> +	 * Wait for clocks to settle down, and then clear all the
> +	 * ratios request and the reload request.
> +	 */
> +	udelay(1000);
> +	reg &= ~(CORE_CLK_DIV_RATIO_MASK | CORE_CLK_DIV_RATIO_RELOAD);
> +	writel(reg, corediv->reg);
> +	udelay(1000);
> +
> +	spin_unlock_irqrestore(&corediv->lock, flags);
> +
> +	return 0;
> +}
> +
> +static const struct clk_ops corediv_ops = {
> +	.enable = clk_corediv_enable,
> +	.disable = clk_corediv_disable,
> +	.is_enabled = clk_corediv_is_enabled,
> +	.recalc_rate = clk_corediv_recalc_rate,
> +	.round_rate = clk_corediv_round_rate,
> +	.set_rate = clk_corediv_set_rate,
> +};
> +
> +static void __init mvebu_corediv_clk_init(struct device_node *node)
> +{
> +	struct clk_init_data init;
> +	struct clk_corediv *corediv;
> +	struct clk **clks;
> +	void __iomem *base;
> +	const char *parent_name;
> +	const char *clk_name;
> +	int i;
> +
> +	base = of_iomap(node, 0);
> +	if (WARN_ON(!base))
> +		return;
> +
> +	parent_name = of_clk_get_parent_name(node, 0);
> +
> +	clk_data.clk_num = ARRAY_SIZE(mvebu_corediv_desc);
> +
> +	/* clks holds the clock array */
> +	clks = kcalloc(clk_data.clk_num, sizeof(struct clk *),
> +				GFP_KERNEL);
> +	if (WARN_ON(!clks))
> +		goto err_unmap;
> +	/* corediv holds the clock specific array */
> +	corediv = kcalloc(clk_data.clk_num, sizeof(struct clk_corediv),
> +				GFP_KERNEL);
> +	if (WARN_ON(!corediv))
> +		goto err_free_clks;
> +
> +	spin_lock_init(&corediv->lock);
> +
> +	for (i = 0; i < clk_data.clk_num; i++) {
> +		of_property_read_string_index(node, "clock-output-names",
> +					      i, &clk_name);
> +		init.num_parents = 1;
> +		init.parent_names = &parent_name;
> +		init.name = clk_name;
> +		init.ops = &corediv_ops;
> +		init.flags = 0;
> +
> +		corediv[i].desc = mvebu_corediv_desc[i];
> +		corediv[i].reg = base;
> +		corediv[i].hw.init = &init;
> +
> +		clks[i] = clk_register(NULL, &corediv[i].hw);
> +		WARN_ON(IS_ERR(clks[i]));
> +	}
> +
> +	clk_data.clks = clks;
> +	of_clk_add_provider(node, of_clk_src_onecell_get, &clk_data);
> +	return;
> +
> +err_free_clks:
> +	kfree(clks);
> +err_unmap:
> +	iounmap(base);
> +}
> +CLK_OF_DECLARE(mvebu_corediv_clk, "marvell,armada-370-corediv-clock",
> +	       mvebu_corediv_clk_init);
> -- 
> 1.8.1.5
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 01/27] clk: mvebu: Add Core Divider clock

[Ezequiel Garcia]

Jason,

On Wed, Oct 23, 2013 at 07:28:23AM -0400, Jason Cooper wrote:
> Ezequiel,
> 
> MikeT took this the other day.
> 
> As such, I've taken in patches 2-4 inclusive into mvebu/dt.  That should
> be the last items for this window.
> 

Just saw the pull... thanks a lot!
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[PATCH v2 00/27] Armada 370/XP NAND support

[Ezequiel Garcia]

Brian,

Can we start discussing this patchset?
I realize is a big series, but better starting sooner than later! :)

In particular... (see below)

On Fri, Oct 18, 2013 at 08:02:27PM -0300, Ezequiel Garcia wrote:
> Hi guys,
> 
> This is the v2 of the work implementing NAND support in Armada 370/XP SoCs.
> This series is probably not yet complete (see below), but I feel like
> we're really closer now :-)
> 
> As in the previous version and given I don't have any public datasheet
> to show, I'll try to write some of the most relevant parts of the controller.
> 
> * NFCv2 controller background
> 
> The controller has a 2176 bytes FIFO buffer. Therefore, in order to support
> larger pages, I/O operations on 4 KiB and 8 KiB pages is done with a set of
> chunked transfers.
> 
> For instance, if we choose a 2048 data chunk and set "BCH" ECC (see below)
> we'll have this layout in the pages:
> 
>   ------------------------------------------------------------------------------
>   | 2048B data | 32B spare | 30B ECC || 2048B data | 32B spare | 30B ECC | ... |
>   ------------------------------------------------------------------------------
> 
> The driver reads the data and spare portions independently and builds an internal
> buffer with this layout (in the 4 KiB page case):
> 
>   ------------------------------------------
>   |     4096B data     |     64B spare     |
>   ------------------------------------------
> 
> Also, for the READOOB command the driver disables the ECC and reads a 'spare + ECC'
> OOB, one per chunk read.
> 
>   -------------------------------------------------------------------
>   |     4096B data     |  32B spare | 30B ECC | 32B spare | 30B ECC |
>   -------------------------------------------------------------------
> 
> So, in order to achieve reading (for instance), we issue several READ0 commands
> (with some additional controller-specific magic) and read two chunks of 2080B
> (2048 data + 32 spare) each.
> The driver accomodates this data to expose the NAND core a contiguous buffer
> (4096 data + spare) or (4096 + spare + ECC + spare + ECC).
> 
> * ECC
> 
> The controller has built-in hardware ECC capabilities. In addition it is
> configurable between two modes: 1) Hamming, 2) BCH.
> 
> Note that the actual BCH mode: BCH-4 or BCH-8 will depend on the way
> the controller is configured to transfer the data.
> 
> In the BCH mode the ECC code will be calculated for each transfered chunk
> and expected to be located (when reading/programming) right after the spare
> bytes as the figure above shows.
> 
> So, repeating the above scheme, a 2048B data chunk will be followed by 32B
> spare, and then the ECC controller will read/write the ECC code (30B in
> this case):
> 
>   ------------------------------------
>   | 2048B data | 32B spare | 30B ECC |
>   ------------------------------------
> 
> If the ECC mode is 'BCH' then the ECC is *always* 30 bytes long.
> If the ECC mode is 'Hamming' the ECC is 6 bytes long, for each 512B block.
> So in Hamming mode, a 2048B page will have a 24B ECC.
> 
> Despite all of the above, the controller requires the driver to only read or
> write in multiples of 8-bytes, because the data buffer is 64-bits.
> 
> * Changes from v1
> 
> Aside from several changes based in Brian's feedback, the main changes
> from v1 are:
> 
>   * The controller's clock source is now fully modeled, see patche 1 to 4.
>     Of course, none of those patches should be taken through the mtd
>     subsystem, but I'm adding them here for completeness.
> 
>   * The chip's cmdfunc() is now independently implemented in each SoC variant.
>     The rationale behind this decision is that 'chunked' I/O is the only tested
>     mode on the Armada370 variant, while the old 'vanilla' I/O is the only
>     tested mode on the PXA variant.
> 
>     So it's safer to have an implementation for each variant.
> 
>   * Added support for BCH-8, in other words: 8-bits of correction in a 512-byte
>     region. This is obtained by using a data chunk size of 1024B, thus doubling
>     the ECC BCH strength, as per this ECC engine mechanism.
> 
>   * The ECC layout in use, which must be set according to the page size and
>     desired ECC strength is now strictly chosen to match only the tested
>     combinations.
> 
> * Pending stuff
> 
>   1. Factory bad blocks handling
> 
>   Currently, there's no factory bad block initial scan (when the bad block
>   table is missing). The ECC BCH requires to layout the device's pages in
>   a splitted "data + OOB + data + OOB" way. This layout being different from
>   the factory layout. In other words,
> 
>   Factory view:
> 
>   -----------------------------------------------
>   |                    Data           |x  OOB   |
>   -----------------------------------------------
> 
>   Driver's view:
> 
>   -----------------------------------------------
>   |      Data      | OOB |      Data   x  | OOB |
>   -----------------------------------------------
> 
>   It can be seen from the above, that the factory bad block marker must be
>   searched within the 'data' region, and not in the usual OOB region.
> 
>   This seems to be similar to the gpmi-nand driver, yet I really find its
>   'bad block swapping' solution odd, so I'm trying to find something
>   cleaner.
> 
>   2. ECC modeling
> 
>   Although I've exposed some ECC information in the ECC layout, I'm still
>   not sure why is this needed, or if it's needed at all.
> 
> * About this patchset
> 
> This is based in l2-mtd's master branch and I've pushed a branch to our
> github in case anyone wants to test it:
> 
>   https://github.com/MISL-EBU-System-SW/mainline-public/tree/l2-mtd/upstream-nand-v2
> 
> Just as the previous, this series is complex and lengthy and any feedback will
> be highly appreciated as well as questions, suggestions or flames. Also, I hope
> our brave PXA regression tester Daniel Mack finds some time to give it a ride
> and report any issues. Daniel: I think owe you several beers already.
> 
> * About Mirabox testing
> 
> As this work is not considered completely stable, be extra careful when trying
> on the Mirabox. The Mirabox has the bootloader at NAND, and there's some risk
> to brick the board.
> 
> That said: I've been using the driver on my Mirabox for several days doing
> simple long-run NAND activity and everything worked fine.
> 
> If despite this you happen to brick the board, Willy Tarreau has provided
> excellent instructions to un-brick the Mirabox:
> 
>   http://1wt.eu/articles/mirabox-vs-guruplug/
> 
> Thanks!
> 
> Ezequiel Garcia (27):
>   clk: mvebu: Add Core Divider clock
>   ARM: mvebu: Add Core Divider clock device-tree binding
>   ARM: mvebu: Add a 2 GHz fixed-clock Armada 370/XP
>   ARM: mvebu: Add the core-divider clock to Armada 370/XP
>   mtd: nand: pxa3xx: Make config menu show supported platforms
>   mtd: nand: pxa3xx: Prevent sub-page writes
>   mtd: nand: pxa3xx: Early variant detection
>   mtd: nand: pxa3xx: Use chip->cmdfunc instead of the internal
>   mtd: nand: pxa3xx: Split FIFO size from to-be-read FIFO count
>   mtd: nand: pxa3xx: Replace host->page_size by mtd->writesize
>   mtd: nand: pxa3xx: Disable OOB on arbitrary length commands
>   mtd: nand: pxa3xx: Use a completion to signal device ready
>   mtd: nand: pxa3xx: Add bad block handling
>   mtd: nand: pxa3xx: Add driver-specific ECC BCH support
^^
This one and ...

>   mtd: nand: pxa3xx: Clear cmd buffer #3 (NDCB3) on command start
>   mtd: nand: pxa3xx: Add helper function to set page address
>   mtd: nand: pxa3xx: Remove READ0 switch/case falltrough
>   mtd: nand: pxa3xx: Split prepare_command_pool() in two stages
>   mtd: nand: pxa3xx: Move the data buffer clean to
>     prepare_start_command()
>   mtd: nand: pxa3xx: Fix SEQIN column address set
>   mtd: nand: pxa3xx: Add a read/write buffers markers
>   mtd: nand: pxa3xx: Introduce multiple page I/O support
^^^
.. this one seems to me like the most conflicting patches as
they extend the ECC, adding BCH. Could you review them and
provide any comments?

>   mtd: nand: pxa3xx: Add multiple chunk write support
>   mtd: nand: pxa3xx: Add ECC BCH correctable errors detection
>   ARM: mvebu: Add support for NAND controller in Armada 370/XP
>   ARM: mvebu: Enable NAND controller in Armada XP GP board
>   ARM: mvebu: Enable NAND controller in Armada 370 Mirabox
> 
>  .../bindings/clock/mvebu-corediv-clock.txt         |  19 +
>  arch/arm/boot/dts/armada-370-mirabox.dts           |  21 +
>  arch/arm/boot/dts/armada-370-xp.dtsi               |  26 +
>  arch/arm/boot/dts/armada-xp-gp.dts                 |   8 +
>  drivers/clk/mvebu/Kconfig                          |   5 +
>  drivers/clk/mvebu/Makefile                         |   1 +
>  drivers/clk/mvebu/clk-corediv.c                    | 223 +++++++
>  drivers/mtd/nand/Kconfig                           |   4 +-
>  drivers/mtd/nand/pxa3xx_nand.c                     | 686 ++++++++++++++++-----
>  include/linux/platform_data/mtd-nand-pxa3xx.h      |   3 +
>  10 files changed, 856 insertions(+), 140 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/clock/mvebu-corediv-clock.txt
>  create mode 100644 drivers/clk/mvebu/clk-corediv.c
> 
> -- 
> 1.8.1.5
> 

Huang: It seems gpmi-nand driver has some similar quirks,
so I'd like if you could take some time to review my patches.

I realise that you might be busy with your own patches, but the
best way of having them cross-reviewed is by doing some review
on others work.

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

[PATCH v2 00/27] Armada 370/XP NAND support

[Brian Norris]

On Thu, Oct 31, 2013 at 12:58:09PM -0300, Ezequiel Garcia wrote:
> Can we start discussing this patchset?
> I realize is a big series, but better starting sooner than later! :)

Yeah, I'm still figuring out prioritization here. I got a little
side-tracked on a few other patch series, some of which had
dependencies on each other. I will take a look at yours next.

...

> >   mtd: nand: pxa3xx: Introduce multiple page I/O support
> ^^^
> .. this one seems to me like the most conflicting patches as
> they extend the ECC, adding BCH. Could you review them and
> provide any comments?

Sure thing.

Brian

[PATCH v2 00/27] Armada 370/XP NAND support

[Huang Shijie]

? 2013?10?31? 23:58, Ezequiel Garcia ??:
> Huang: It seems gpmi-nand driver has some similar quirks,
> so I'd like if you could take some time to review my patches.
i am busy recently. I will review your patch set tomorrow.

thanks
Huang Shijie

[PATCH v2 07/27] mtd: nand: pxa3xx: Early variant detection

[Huang Shijie]

On Fri, Oct 18, 2013 at 08:02:34PM -0300, Ezequiel Garcia wrote:
> In order to customize early settings depending on the detected SoC variant,
what's the early settings depending on the variant?
could you add them in the commit?

thanks
Huang Shijie

[PATCH v2 08/27] mtd: nand: pxa3xx: Use chip->cmdfunc instead of the internal

[Huang Shijie]

On Fri, Oct 18, 2013 at 08:02:35PM -0300, Ezequiel Garcia wrote:
> @@ -1011,14 +1011,16 @@ static void pxa3xx_nand_free_buff(struct pxa3xx_nand_info *info)
>  static int pxa3xx_nand_sensing(struct pxa3xx_nand_info *info)
>  {
>  	struct mtd_info *mtd;
> +	struct nand_chip *chip;
>  	int ret;
Please add a empty line here.

>  	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;
>  
> -	pxa3xx_nand_cmdfunc(mtd, NAND_CMD_RESET, 0, 0);
> +	chip->cmdfunc(mtd, NAND_CMD_RESET, 0, 0);
>  	if (info->is_ready)
>  		return 0;
Reviewed-by: Huang Shijie <shijie8@gmail.com>

[PATCH v2 09/27] mtd: nand: pxa3xx: Split FIFO size from to-be-read FIFO count

[Huang Shijie]

On Fri, Oct 18, 2013 at 08:02:36PM -0300, Ezequiel Garcia wrote:
>  
> -	switch (host->page_size) {
> +	switch (info->fifo_size) {
>  	case 2048:
	
there is a item in the builtin_flash_types:
{ "4GiB 8-bit",    0xd7ec, 128, 4096,  8,  8, 8192, &timing[1] },

The original host->page_size could be 4096, how can the old code support
this nand with page-size is 4096?

thanks
Huang Shijie

[PATCH v2 10/27] mtd: nand: pxa3xx: Replace host->page_size by mtd->writesize

[Huang Shijie]

On Fri, Oct 18, 2013 at 08:02:37PM -0300, Ezequiel Garcia wrote:
> There's no need to privately store the device page size as it's
> available in mtd structure field mtd->writesize.
> Also, this removes the hardcoded page size value, leaving the
> auto-detected value only.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
>  drivers/mtd/nand/pxa3xx_nand.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index fe2e27c..83ba036 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -149,7 +149,6 @@ struct pxa3xx_nand_host {
>  	void			*info_data;
>  
>  	/* page size of attached chip */
> -	unsigned int		page_size;
>  	int			use_ecc;
>  	int			cs;
>  
> @@ -612,12 +611,12 @@ static int prepare_command_pool(struct pxa3xx_nand_info *info, int command,
>  			info->buf_start += mtd->writesize;
>  
>  		/* Second command setting for large pages */
> -		if (host->page_size >= PAGE_CHUNK_SIZE)
> +		if (mtd->writesize >= PAGE_CHUNK_SIZE)
>  			info->ndcb0 |= NDCB0_DBC | (NAND_CMD_READSTART << 8);
>  
>  	case NAND_CMD_SEQIN:
>  		/* small page addr setting */
> -		if (unlikely(host->page_size < PAGE_CHUNK_SIZE)) {
> +		if (unlikely(mtd->writesize < PAGE_CHUNK_SIZE)) {
>  			info->ndcb1 = ((page_addr & 0xFFFFFF) << 8)
>  					| (column & 0xFF);
>  
> @@ -891,7 +890,6 @@ static int pxa3xx_nand_config_flash(struct pxa3xx_nand_info *info,
>  	}
>  
>  	/* calculate flash information */
> -	host->page_size = f->page_size;
you remove this line, but how the following code get the right value:?

	chip->ecc.size = host->page_size;
this line in the pxa3xx_nand_scan.

thanks
Huang Shijie

[PATCH v2 11/27] mtd: nand: pxa3xx: Disable OOB on arbitrary length commands

[Huang Shijie]

On Fri, Oct 18, 2013 at 08:02:38PM -0300, Ezequiel Garcia wrote:
> READID, STATUS and PARAM (aka ONFI read paramater page) don't read
> the OOB area. Set the oob_size to zero and prevent it.
The ERASE, RESET also donot need to read the OOB,
   why not zero the oob_size for these commands?

  thanks
 Huang Shiji 

[PATCH v2 12/27] mtd: nand: pxa3xx: Use a completion to signal device ready

[Huang Shijie]

On Fri, Oct 18, 2013 at 08:02:39PM -0300, Ezequiel Garcia wrote:
> Apparently, the expected behavior of the waitfunc() NAND chip call
> is to wait for the device to be READY (this is a standard chip line).
> However, the current implementation does almost nothing, which opens
> a possibility to issue a command to a non-ready device.
> 
> Fix this by adding a new completion to wait for the ready event to arrive.
> 
> Because the "is ready" flag is cleared from the controller status
> register, it's needed to store that state in the driver, and because the
> field is accesed from an interruption, the field needs to be of an
> atomic type.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
>  drivers/mtd/nand/pxa3xx_nand.c | 45 +++++++++++++++++++++++++++++-------------
>  1 file changed, 31 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index 95e2ce3..1ceccb6 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -35,6 +35,7 @@
>  
>  #include <linux/platform_data/mtd-nand-pxa3xx.h>
>  
> +#define NAND_DEV_READY_TIMEOUT  50
>  #define	CHIP_DELAY_TIMEOUT	(2 * HZ/10)
>  #define NAND_STOP_DELAY		(2 * HZ/50)
>  #define PAGE_CHUNK_SIZE		(2048)
> @@ -166,7 +167,7 @@ struct pxa3xx_nand_info {
>  	struct clk		*clk;
>  	void __iomem		*mmio_base;
>  	unsigned long		mmio_phys;
> -	struct completion	cmd_complete;
> +	struct completion	cmd_complete, dev_ready;
>  
>  	unsigned int 		buf_start;
>  	unsigned int		buf_count;
> @@ -196,7 +197,13 @@ struct pxa3xx_nand_info {
>  	int			use_ecc;	/* use HW ECC ? */
>  	int			use_dma;	/* use DMA ? */
>  	int			use_spare;	/* use spare ? */
> -	int			is_ready;
> +
> +	/*
> +	 * The is_ready flag is accesed from several places,
> +	 * including an interruption hander. We need an atomic
> +	 * type to avoid races.
> +	 */
> +	atomic_t		is_ready;
Do we really need to change it to atomic_t?

IMHO, the write is also a atomic operation.

thanks
Huang Shijie

[PATCH v2 08/27] mtd: nand: pxa3xx: Use chip->cmdfunc instead of the internal

[Ezequiel Garcia]

On Sun, Nov 03, 2013 at 04:32:40PM -0500, Huang Shijie wrote:
> On Fri, Oct 18, 2013 at 08:02:35PM -0300, Ezequiel Garcia wrote:
> > @@ -1011,14 +1011,16 @@ static void pxa3xx_nand_free_buff(struct pxa3xx_nand_info *info)
> >  static int pxa3xx_nand_sensing(struct pxa3xx_nand_info *info)
> >  {
> >  	struct mtd_info *mtd;
> > +	struct nand_chip *chip;
> >  	int ret;
> Please add a empty line here.
> 

Sure, I will.

> >  	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;
> >  
> > -	pxa3xx_nand_cmdfunc(mtd, NAND_CMD_RESET, 0, 0);
> > +	chip->cmdfunc(mtd, NAND_CMD_RESET, 0, 0);
> >  	if (info->is_ready)
> >  		return 0;
> Reviewed-by: Huang Shijie <shijie8@gmail.com>

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

[PATCH v2 07/27] mtd: nand: pxa3xx: Early variant detection

[Ezequiel Garcia]

On Sun, Nov 03, 2013 at 04:18:58PM -0500, Huang Shijie wrote:
> On Fri, Oct 18, 2013 at 08:02:34PM -0300, Ezequiel Garcia wrote:
> > In order to customize early settings depending on the detected SoC variant,
> what's the early settings depending on the variant?

We need to detect the variant *before* alloc_nand_resource(),
in order to set a different cmdfunc() for each variant.

Having a separate cmdfunc() is, IMO, the best solution for two
reasons:

1. We keep the legacy pxa platform away from our intrusive changes,
which such platform won't use.

2. The 'splitted' command semantic is weird and complex enough
to live in its own function.

> could you add them in the commit?
> 

Yes, that sound better. Thanks for the feedback.
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[PATCH v2 09/27] mtd: nand: pxa3xx: Split FIFO size from to-be-read FIFO count

[Ezequiel Garcia]

On Sun, Nov 03, 2013 at 05:36:50PM -0500, Huang Shijie wrote:
> On Fri, Oct 18, 2013 at 08:02:36PM -0300, Ezequiel Garcia wrote:
> >  
> > -	switch (host->page_size) {
> > +	switch (info->fifo_size) {
> >  	case 2048:
> 	
> there is a item in the builtin_flash_types:
> { "4GiB 8-bit",    0xd7ec, 128, 4096,  8,  8, 8192, &timing[1] },
> 
> The original host->page_size could be 4096, how can the old code support
> this nand with page-size is 4096?
> 

Ah, nice catch. I completely missed that line!

I've rechecked the PXA3xx spec. but it makes no mention to supporting
4096 (and it doesn't mention the FIFO buffer's size).

On the other side, and AFAIK, the 'splitted' command semantics in the
NFCv2 (the one I'm working on) was added specifically to support
page-size of 4096 and 8192.

In other words, I'm not sure how could the old code support 4096!
The support was adding long ago, in this commit:

commit d3490dfdbc453a16bc7f3cff731c9f7851735ab3
Author: Haojian Zhuang <haojian.zhuang@marvell.com>
Date:   Thu Sep 10 14:33:30 2009 +0800

pxa3xx_nand: add new nand chip support

Support samsung 2GbX8 and 32GbX8 nand flash.
Support micron 4GbX8 and 4GbX16 nand flash.

Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
Signed-off-by: Eric Miao <eric.y.miao@gmail.com>

So, maybe they can provide some answers. Haojian? Eric?
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[PATCH v2 10/27] mtd: nand: pxa3xx: Replace host->page_size by mtd->writesize

[Ezequiel Garcia]

On Sun, Nov 03, 2013 at 05:52:13PM -0500, Huang Shijie wrote:
> On Fri, Oct 18, 2013 at 08:02:37PM -0300, Ezequiel Garcia wrote:
> > There's no need to privately store the device page size as it's
> > available in mtd structure field mtd->writesize.
> > Also, this removes the hardcoded page size value, leaving the
> > auto-detected value only.
> > 
> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> > ---
> >  drivers/mtd/nand/pxa3xx_nand.c | 8 ++------
> >  1 file changed, 2 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> > index fe2e27c..83ba036 100644
> > --- a/drivers/mtd/nand/pxa3xx_nand.c
> > +++ b/drivers/mtd/nand/pxa3xx_nand.c
> > @@ -149,7 +149,6 @@ struct pxa3xx_nand_host {
> >  	void			*info_data;
> >  
> >  	/* page size of attached chip */
> > -	unsigned int		page_size;
> >  	int			use_ecc;
> >  	int			cs;
> >  
> > @@ -612,12 +611,12 @@ static int prepare_command_pool(struct pxa3xx_nand_info *info, int command,
> >  			info->buf_start += mtd->writesize;
> >  
> >  		/* Second command setting for large pages */
> > -		if (host->page_size >= PAGE_CHUNK_SIZE)
> > +		if (mtd->writesize >= PAGE_CHUNK_SIZE)
> >  			info->ndcb0 |= NDCB0_DBC | (NAND_CMD_READSTART << 8);
> >  
> >  	case NAND_CMD_SEQIN:
> >  		/* small page addr setting */
> > -		if (unlikely(host->page_size < PAGE_CHUNK_SIZE)) {
> > +		if (unlikely(mtd->writesize < PAGE_CHUNK_SIZE)) {
> >  			info->ndcb1 = ((page_addr & 0xFFFFFF) << 8)
> >  					| (column & 0xFF);
> >  
> > @@ -891,7 +890,6 @@ static int pxa3xx_nand_config_flash(struct pxa3xx_nand_info *info,
> >  	}
> >  
> >  	/* calculate flash information */
> > -	host->page_size = f->page_size;
> you remove this line, but how the following code get the right value:?
> 
> 	chip->ecc.size = host->page_size;
> this line in the pxa3xx_nand_scan.
> 

Ah, nice catch. I guess it splipped through the million rebases I did...

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

[PATCH v2 11/27] mtd: nand: pxa3xx: Disable OOB on arbitrary length commands

[Ezequiel Garcia]

On Sun, Nov 03, 2013 at 05:59:05PM -0500, Huang Shijie wrote:
> On Fri, Oct 18, 2013 at 08:02:38PM -0300, Ezequiel Garcia wrote:
> > READID, STATUS and PARAM (aka ONFI read paramater page) don't read
> > the OOB area. Set the oob_size to zero and prevent it.
> The ERASE, RESET also donot need to read the OOB,
>    why not zero the oob_size for these commands?
> 

Because this only matters for commands that will use the oob_size field
when doing some I/O operation. Both ERASE and RESET commands have no data
cycle, so it makes no sense to haev a non-zero oob_size field.

On the other side, this patch is rather stupid, as oob_size is already
cleared at the beggining of a command sequence. So, I'll either rework
this or drop it entirely.

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

[PATCH v2 12/27] mtd: nand: pxa3xx: Use a completion to signal device ready

[Ezequiel Garcia]

On Sun, Nov 03, 2013 at 06:03:39PM -0500, Huang Shijie wrote:
> On Fri, Oct 18, 2013 at 08:02:39PM -0300, Ezequiel Garcia wrote:
> > Apparently, the expected behavior of the waitfunc() NAND chip call
> > is to wait for the device to be READY (this is a standard chip line).
> > However, the current implementation does almost nothing, which opens
> > a possibility to issue a command to a non-ready device.
> > 
> > Fix this by adding a new completion to wait for the ready event to arrive.
> > 
> > Because the "is ready" flag is cleared from the controller status
> > register, it's needed to store that state in the driver, and because the
> > field is accesed from an interruption, the field needs to be of an
> > atomic type.
> > 
> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> > ---
> >  drivers/mtd/nand/pxa3xx_nand.c | 45 +++++++++++++++++++++++++++++-------------
> >  1 file changed, 31 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> > index 95e2ce3..1ceccb6 100644
> > --- a/drivers/mtd/nand/pxa3xx_nand.c
> > +++ b/drivers/mtd/nand/pxa3xx_nand.c
> > @@ -35,6 +35,7 @@
> >  
> >  #include <linux/platform_data/mtd-nand-pxa3xx.h>
> >  
> > +#define NAND_DEV_READY_TIMEOUT  50
> >  #define	CHIP_DELAY_TIMEOUT	(2 * HZ/10)
> >  #define NAND_STOP_DELAY		(2 * HZ/50)
> >  #define PAGE_CHUNK_SIZE		(2048)
> > @@ -166,7 +167,7 @@ struct pxa3xx_nand_info {
> >  	struct clk		*clk;
> >  	void __iomem		*mmio_base;
> >  	unsigned long		mmio_phys;
> > -	struct completion	cmd_complete;
> > +	struct completion	cmd_complete, dev_ready;
> >  
> >  	unsigned int 		buf_start;
> >  	unsigned int		buf_count;
> > @@ -196,7 +197,13 @@ struct pxa3xx_nand_info {
> >  	int			use_ecc;	/* use HW ECC ? */
> >  	int			use_dma;	/* use DMA ? */
> >  	int			use_spare;	/* use spare ? */
> > -	int			is_ready;
> > +
> > +	/*
> > +	 * The is_ready flag is accesed from several places,
> > +	 * including an interruption hander. We need an atomic
> > +	 * type to avoid races.
> > +	 */
> > +	atomic_t		is_ready;
> Do we really need to change it to atomic_t?
> 
> IMHO, the write is also a atomic operation.
> 

I see. Well, if reading and setting an int type is atomic
(at least on ARM), then of course we don't need the atomic_t.

On the other side, it really shouldn't hurt too much and
it emphasize the fact that it's accesed from the IRQ handler
and the other callbacks (such as waitfunc).

So, I'm still inclined to keeping it atomic.

Does it sound stupid?
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[PATCH v2 12/27] mtd: nand: pxa3xx: Use a completion to signal device ready

[Brian Norris]

On Mon, Nov 4, 2013 at 5:51 AM, Ezequiel Garcia
<ezequiel.garcia@free-electrons.com> wrote:
> On Sun, Nov 03, 2013 at 06:03:39PM -0500, Huang Shijie wrote:
>> On Fri, Oct 18, 2013 at 08:02:39PM -0300, Ezequiel Garcia wrote:
>> > --- a/drivers/mtd/nand/pxa3xx_nand.c
>> > +++ b/drivers/mtd/nand/pxa3xx_nand.c
>> > @@ -196,7 +197,13 @@ struct pxa3xx_nand_info {
>> >     int                     use_ecc;        /* use HW ECC ? */
>> >     int                     use_dma;        /* use DMA ? */
>> >     int                     use_spare;      /* use spare ? */
>> > -   int                     is_ready;
>> > +
>> > +   /*
>> > +    * The is_ready flag is accesed from several places,
>> > +    * including an interruption hander. We need an atomic
>> > +    * type to avoid races.
>> > +    */
>> > +   atomic_t                is_ready;
>> Do we really need to change it to atomic_t?
>>
>> IMHO, the write is also a atomic operation.

Atomicity is not an opinion :)

> I see. Well, if reading and setting an int type is atomic
> (at least on ARM), then of course we don't need the atomic_t.
>
> On the other side, it really shouldn't hurt too much and
> it emphasize the fact that it's accesed from the IRQ handler
> and the other callbacks (such as waitfunc).
>
> So, I'm still inclined to keeping it atomic.
>
> Does it sound stupid?

No, it is not stupid. If nothing else, it is self-documenting, telling
the world that this variable is accessed in racy situations, forcing
developers to pay closer attention to it. And atomicity means more
than simply "indivisible" (i.e., a single load instruction) in this
context; I believe it helps prevent other non-thread-safe
optimizations from being made by the compiler.

That being said, the jury is still out on the need for the atomic flag
+ 2 completions in the first place (I brought this up a while ago but
failed to follow up), but I'll hold my comments to the v3 patch.

Brian

[PATCH v2 09/27] mtd: nand: pxa3xx: Split FIFO size from to-be-read FIFO count

[Haojian Zhuang]

On 11/04/2013 08:13 PM, Ezequiel Garcia wrote:
> On Sun, Nov 03, 2013 at 05:36:50PM -0500, Huang Shijie wrote:
>> On Fri, Oct 18, 2013 at 08:02:36PM -0300, Ezequiel Garcia wrote:
>>>
>>> -	switch (host->page_size) {
>>> +	switch (info->fifo_size) {
>>>   	case 2048:
>> 	
>> there is a item in the builtin_flash_types:
>> { "4GiB 8-bit",    0xd7ec, 128, 4096,  8,  8, 8192, &timing[1] },
>>
>> The original host->page_size could be 4096, how can the old code support
>> this nand with page-size is 4096?
>>
>
> Ah, nice catch. I completely missed that line!
>
> I've rechecked the PXA3xx spec. but it makes no mention to supporting
> 4096 (and it doesn't mention the FIFO buffer's size).
>
> On the other side, and AFAIK, the 'splitted' command semantics in the
> NFCv2 (the one I'm working on) was added specifically to support
> page-size of 4096 and 8192.
>
> In other words, I'm not sure how could the old code support 4096!
> The support was adding long ago, in this commit:
>
> commit d3490dfdbc453a16bc7f3cff731c9f7851735ab3
> Author: Haojian Zhuang <haojian.zhuang@marvell.com>
> Date:   Thu Sep 10 14:33:30 2009 +0800
>
> pxa3xx_nand: add new nand chip support
>
> Support samsung 2GbX8 and 32GbX8 nand flash.
> Support micron 4GbX8 and 4GbX16 nand flash.
>
> Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
> Signed-off-by: Eric Miao <eric.y.miao@gmail.com>
>
> So, maybe they can provide some answers. Haojian? Eric?
>

Hi Ezequiel,

I can't remember all details. The main problem is that I failed to find 
the flash datasheet.

Regards
Haojian