Marvell NAND controller rework with ->exec_op()

Document the legacy and the new bindings for Marvell NAND controller.

The pxa3xx_nand.c driver does only support legacy bindings, which are incomplete and inaccurate. A rework of this controller (called marvell_nand.c) does support both.

Signed-off-by Miquel Raynal <[email protected]>

---
 .../devicetree/bindings/mtd/marvell-nand.txt       | 123 +++++++++++++++++++++
 1 file changed, 123 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/marvell-nand.txt

Documentation/devicetree/bindings/mtd/marvell-nand.txt new file +26 -0

new file mode 100644
index 000000000000..aa6a1ed045b2
@@ -0,0 +1,123 @@
+Marvell NAND Flash Controller (NFC)
+
+Required properties:
+- compatible: can be one of the following:
+    * "marvell,armada-8k-nand-controller"
+    * "marvell,armada370-nand-controller"
+    * "marvell,pxa3xx-nand-controller"
+    * "marvell,armada-8k-nand" (deprecated)
+    * "marvell,armada370-nand" (deprecated)
+    * "marvell,pxa3xx-nand" (deprecated)
+  Compatibles marked deprecated support only the old bindings described
+  at the bottom.
+- reg: NAND flash controller memory area.
+- #address-cells: shall be set to 1. Encode the NAND CS.
+- #size-cells: shall be set to 0.
+- interrupts: shall define the NAND controller interrupt.
+- clocks: shall reference the NAND controller clock.
+- marvell,system-controller: Set to retrieve the syscon node that handles
+  NAND controller related registers (only required with the
+  "marvell,armada-8k-nand[-controller]" compatibles).
+
+Optional properties:
+- label: see partition.txt. New platforms shall omit this property.
+- dmas: shall reference DMA channel associated to the NAND controller.
+  This property is only used with "marvell,pxa3xx-nand[-controller]"
+  compatible strings.

Documentation/devicetree/bindings/mtd/marvell-nand.txt new file +30 -0

+
+See Documentation/devicetree/bindings/mtd/nand.txt for more details on
+generic bindings.
+
+
+Example:
+nand_controller: nand-controller at d0000 {
+	compatible = "marvell,armada370-nand-controller";
+	reg = <0xd0000 0x54>;
+	#address-cells = <1>;
+	#size-cells = <0>;
+	interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>;
+	clocks = <&coredivclk 0>;
+
+	nand at 0 {
+		reg = <0>;
+		label = "main-storage";
+		marvell,rb = <0>;
+		nand-ecc-mode = "hw";
+		marvell,nand-keep-config;
+		nand-on-flash-bbt;
+		nand-ecc-strength = <4>;
+		nand-ecc-step-size = <512>;
+
+		partitions {
+			compatible = "fixed-partitions";
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			partition at 0 {

Documentation/devicetree/bindings/mtd/marvell-nand.txt new file +27 -0

+described below as well as the partitions node at the root of a so
+called "nand" node (without clear controller/chip separation).
+
+Legacy properties:
+- marvell,nand-enable-arbiter: To enable the arbiter, all boards blindly
+  used it, this bit was set by the bootloader for many boards and even if
+  it is marked reserved in several datasheets, it might be needed to set
+  it (otherwise it is harmless) so whether or not this property is set,
+  the bit is selected by the driver.
+- num-cs: Number of chip-select lines to use, all boards blindly set 1
+  to this and for a reason, other values would have failed. The value of
+  this property is ignored.
+
+Example:
+
+	nand0: nand at 43100000 {
+		compatible = "marvell,pxa3xx-nand";
+		reg = <0x43100000 90>;
+		interrupts = <45>;
+		dmas = <&pdma 97 0>;
+		dma-names = "rxtx";
+		#address-cells = <1>;
+		marvell,nand-keep-config;
+		marvell,nand-enable-arbiter;
+		num-cs = <1>;
+		/* Partitions (optional) */
+       };

Signature · 1 line

2.11.0

Quoted 56 lines · Miquel Raynal

Document the legacy and the new bindings for Marvell NAND controller.

The pxa3xx_nand.c driver does only support legacy bindings, which are incomplete and inaccurate. A rework of this controller (called marvell_nand.c) does support both.

Signed-off-by Miquel Raynal <[email protected]>

---
 .../devicetree/bindings/mtd/marvell-nand.txt       | 123
+++++++++++++++++++++ 1 file changed, 123 insertions(+)
 create mode 100644

Documentation/devicetree/bindings/mtd/marvell-nand.txt

(quoted diff) +0 -0

b/Documentation/devicetree/bindings/mtd/marvell-nand.txt new file mode 100644 index 000000000000..aa6a1ed045b2

Documentation/devicetree/bindings/mtd/marvell-nand.txt new file +11 -0

@@ -0,0 +1,123 @@
+Marvell NAND Flash Controller (NFC)
+
+Required properties:
+- compatible: can be one of the following:
+    * "marvell,armada-8k-nand-controller"
+    * "marvell,armada370-nand-controller"
+    * "marvell,pxa3xx-nand-controller"
+    * "marvell,armada-8k-nand" (deprecated)
+    * "marvell,armada370-nand" (deprecated)
+    * "marvell,pxa3xx-nand" (deprecated)
+  Compatibles marked deprecated support only the old bindings

described
+ at the bottom.

+- reg: NAND flash controller memory area.
+- #address-cells: shall be set to 1. Encode the NAND CS.
+- #size-cells: shall be set to 0.
+- interrupts: shall define the NAND controller interrupt.
+- clocks: shall reference the NAND controller clock.
+- marvell,system-controller: Set to retrieve the syscon node that

handles

Documentation/devicetree/bindings/mtd/marvell-nand.txt new file +6 -0

+  NAND controller related registers (only required with the
+  "marvell,armada-8k-nand[-controller]" compatibles).
+
+Optional properties:
+- label: see partition.txt. New platforms shall omit this property.
+- dmas: shall reference DMA channel associated to the NAND

controller.
+ This property is only used with
"marvell,pxa3xx-nand[-controller]"

Documentation/devicetree/bindings/mtd/marvell-nand.txt new file +3 -0

+  compatible strings.
+- dma-names: shall be "rxtx".
+  This property is only used with

"marvell,pxa3xx-nand[-controller]"

Documentation/devicetree/bindings/mtd/marvell-nand.txt new file +3 -0

+  compatible strings.
+
+Optional children nodes:

Documentation/devicetree/bindings/mtd/marvell-nand.txt line 36

Quoted 5 lines

Documentation/devicetree/bindings/mtd/marvell-nand.txt new file +5 -0

+Children nodes represent the available NAND chips.
+
+Required properties:
+- reg: shall contain the native Chip Select ids (0-3)
+- marvell,rb: shall contain the native Ready/Busy ids (0-1)  

We already have at least 2 other <vendor>,rb properties. Let's not add a 3rd and make a common one instead.

Quoted 11 lines

Documentation/devicetree/bindings/mtd/marvell-nand.txt new file +3 -0

+
+Optional properties:
+- marvell,nand-keep-config: orders the driver not to take the

timings

Documentation/devicetree/bindings/mtd/marvell-nand.txt new file +5 -0

+  from the core and leaving them completely untouched. Bootloader
+  timings will then be used.
+- label: MTD name.
+- nand-on-flash-bbt: see nand.txt.
+- nand-ecc-mode: see nand.txt. Will use hardware ECC if not

specified. +- nand-ecc-algo: see nand.txt. This property may be

Documentation/devicetree/bindings/mtd/marvell-nand.txt line 49

Quoted 6 lines

added when using

Documentation/devicetree/bindings/mtd/marvell-nand.txt new file +2 -0

+  hardware ECC for clarification but will be ignored by the driver
+  because ECC mode is chosen depending on the page size and the

strength

Documentation/devicetree/bindings/mtd/marvell-nand.txt new file +2 -0

+  required by the NAND chip. This value may be overwritten with
+  nand-ecc-strength property.  

If not used, then drop it.

Quoted 4 lines

+- nand-ecc-strength: see nand.txt.
+- nand-ecc-step-size: see nand.txt. This has no effect and will be
+  ignored by the driver when using hardware ECC because Marvell's

Quoted 9 lines

NAND
+ flash controller does use fixed strength (1-bit for Hamming, 16-bit
+ for BCH), so the step size will shrink or grow in order to fit the
+ required strength. Step sizes are not completely random for all and
+ follow certain patterns described in AN-379, "Marvell SoC NFC ECC".

Same here.

Add marvell_nand driver which aims at replacing the existing pxa3xx_nand driver.

The new driver intends to be easier to understand and follows the brand new NAND framework rules by implementing hooks for every pattern the controller might support and referencing them inside a parser object that will be given to the core at each ->exec_op() call.

Raw accessors are implemented, useful to test/debug memory/filesystem corruptions. Userspace binaries contained in the mtd-utils package may now be used and their output trusted.

Timings may not be kept from the bootloader anymore, the timings used for instance in U-Boot were not optimal and it supposed to have NAND support (and initialized) in the bootloader.

Thanks to the improved timings, implementation of ONFI mode 5 support (with EDO managed by adding a delay on data sampling), merging the commands together and optimizing writes in the command registers, the new driver may achieve faster throughputs in both directions. Measurements show an improvement of about +23% read throughput and +24% write throughput. These measurements have been done with an Armada-385-DB-AP (4kiB NAND pages forced in 4-bit strength BCH ECC correction) using the userspace tool 'flash_speed' from the MTD test suite.

Besides these important topics, the new driver addresses several unsolved known issues in the old driver which:
- did not work with ECC soft neither with ECC none ;
- relied on naked read/write (which is unchanged) while the NFCv1 embedded in the pxa3xx platforms do not implement it, so several NAND commands did not actually ever work without any notice (like reading the ONFI PARAM_PAGE or SET/GET_FEATURES) ;
- wrote the OOB data correctly, but was not able to read it correctly past the first OOB data chunk ;
- did not displayed ECC bytes ;

- used device tree bindings that did not allow more than one NAND chip, and did not allow to choose the correct chip select if not incrementing from 0. Plus, the Ready/Busy line used had to be 0.

Old device tree bindings are still supported but deprecated. A more hierarchical view has to be used to keep the controller and the NAND chip structures clearly separated both inside the device tree and also in the driver code.

Signed-off-by Miquel Raynal <[email protected]>

Tested-by Sean Nyekjaer <[email protected]>

Tested-by Willy Tarreau <[email protected]>

---
 drivers/mtd/nand/Kconfig        |   12 +
 drivers/mtd/nand/Makefile       |    1 +
 drivers/mtd/nand/marvell_nand.c | 2942 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 2955 insertions(+)
 create mode 100644 drivers/mtd/nand/marvell_nand.c

drivers/mtd/nand/Kconfig +12 -0

index 859eb7790c46..9e141e03f5c2 100644
@@ -323,6 +323,18 @@ config MTD_NAND_PXA3xx
 	  platforms (XP, 370, 375, 38x, 39x) and 64-bit Armada
 	  platforms (7K, 8K) (NFCv2).
 
+config MTD_NAND_MARVELL
+	tristate "NAND controller support on Marvell boards"
+	depends on PXA3xx || ARCH_MMP || PLAT_ORION || ARCH_MVEBU || \
+		   COMPILE_TEST
+	depends on HAS_IOMEM
+	help
+	  This enables the NAND flash controller driver for Marvell boards,
+	  including:
+	  - PXA3xx processors (NFCv1)
+	  - 32-bit Armada platforms (XP, 37x, 38x, 39x) (NFCv2)
+	  - 64-bit Aramda platforms (7k, 8k) (NFCv2)
+
 config MTD_NAND_SLC_LPC32XX
 	tristate "NXP LPC32xx SLC Controller"
 	depends on ARCH_LPC32XX

drivers/mtd/nand/Makefile +1 -0

index 118a1349aad3..921634ba400c 100644
@@ -32,6 +32,7 @@ obj-$(CONFIG_MTD_NAND_OMAP2) 		+= omap2_nand.o
 obj-$(CONFIG_MTD_NAND_OMAP_BCH_BUILD)	+= omap_elm.o
 obj-$(CONFIG_MTD_NAND_CM_X270)		+= cmx270_nand.o
 obj-$(CONFIG_MTD_NAND_PXA3xx)		+= pxa3xx_nand.o
+obj-$(CONFIG_MTD_NAND_MARVELL)		+= marvell_nand.o
 obj-$(CONFIG_MTD_NAND_TMIO)		+= tmio_nand.o
 obj-$(CONFIG_MTD_NAND_PLATFORM)		+= plat_nand.o
 obj-$(CONFIG_MTD_NAND_PASEMI)		+= pasemi_nand.o

drivers/mtd/nand/marvell_nand.c new file +0 -0

new file mode 100644
index 000000000000..0a5432bcda6a

drivers/mtd/nand/marvell_nand.c new file +57 -0

+ */
+
+#include <linux/module.h>
+#include <linux/clk.h>
+#include <linux/mtd/rawnand.h>
+#include <linux/of_platform.h>
+#include <linux/iopoll.h>
+#include <linux/interrupt.h>
+#include <linux/slab.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
+#include <asm/unaligned.h>
+
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
+#include <linux/dma/pxa-dma.h>
+#include <linux/platform_data/mtd-nand-pxa3xx.h>
+
+/* Data FIFO granularity, FIFO reads/writes must be a multiple of this length */
+#define FIFO_DEPTH		8
+#define FIFO_REP(x)		(x / sizeof(u32))
+#define BCH_SEQ_READS		(32 / FIFO_DEPTH)
+/* NFC does not support transfers of larger chunks at a time */
+#define MAX_CHUNK_SIZE		2112
+/* NFCv1 cannot read more that 7 bytes of ID */
+#define NFCV1_READID_LEN	7
+/* Polling is done at a pace of POLL_PERIOD us until POLL_TIMEOUT is reached */
+#define POLL_PERIOD		0
+#define POLL_TIMEOUT		100000
+/* Interrupt maximum wait period in ms */
+#define IRQ_TIMEOUT		1000
+/* Latency in clock cycles between SoC pins and NFC logic */
+#define MIN_RD_DEL_CNT		3
+/* Maximum number of contiguous address cycles */
+#define MAX_ADDRESS_CYC_NFCV1	5
+#define MAX_ADDRESS_CYC_NFCV2	7
+/* System control registers/bits to enable the NAND controller on some SoCs */
+#define GENCONF_SOC_DEVICE_MUX	0x208
+#define GENCONF_SOC_DEVICE_MUX_NFC_EN BIT(0)
+#define GENCONF_SOC_DEVICE_MUX_ECC_CLK_RST BIT(20)
+#define GENCONF_SOC_DEVICE_MUX_ECC_CORE_RST BIT(21)
+#define GENCONF_SOC_DEVICE_MUX_NFC_INT_EN BIT(25)
+#define GENCONF_CLK_GATING_CTRL	0x220
+#define GENCONF_CLK_GATING_CTRL_ND_GATE BIT(2)
+#define GENCONF_ND_CLK_CTRL	0x700
+#define GENCONF_ND_CLK_CTRL_EN	BIT(0)
+
+/* NAND controller data flash control register */
+#define NDCR			0x00
+/* NAND interface timing parameter 0 register */
+#define NDTR0			0x04
+/* NAND interface timing parameter 1 register */
+#define NDTR1			0x0C
+/* NAND controller status register */
+#define NDSR			0x14
+/* NAND ECC control register */
+#define NDECCCTRL		0x28

drivers/mtd/nand/marvell_nand.c new file +17 -0

+
+/* Data flash control register bitfields */
+#define NDCR_ALL_INT		GENMASK(11, 0)
+#define NDCR_CS1_CMDDM		BIT(7)
+#define NDCR_CS0_CMDDM		BIT(8)
+#define NDCR_RDYM		BIT(11)
+#define NDCR_ND_ARB_EN		BIT(12)
+#define NDCR_RA_START		BIT(15)
+#define NDCR_RD_ID_CNT(x)	(min_t(unsigned int, x, 0x7) << 16)
+#define NDCR_PAGE_SZ(x)		(x >= 2048 ? BIT(24) : 0)
+#define NDCR_DWIDTH_M		BIT(26)
+#define NDCR_DWIDTH_C		BIT(27)
+#define NDCR_ND_RUN		BIT(28)
+#define NDCR_DMA_EN		BIT(29)
+#define NDCR_ECC_EN		BIT(30)
+#define NDCR_SPARE_EN		BIT(31)
+

+ * Marvell ECC engine works differently than the others, in order to limit the
+ * size of the IP, hardware engineers choose to set a fixed strength at 16 bits

+ * per subpage, and depending on a the desired strength needed by the NAND chip,
+ * a particular layout mixing data/spare/ecc is defined, with a possible last
+ * chunk smaller that the others.
+ *
+ * @writesize: Full page size on which the layout applies
+ * @chunk: Desired ECC chunk size on which the layout applies
+ * @strength: Desired ECC strength (per chunk size bytes) on which the
+ * layout applies

+ * @full_chunk_cnt: Number of full-sized chunks, which is the number of
+ * repetitions of the pattern:
+ * (data_bytes + spare_bytes + ecc_bytes).
+ * @data_bytes: Number of data bytes per chunk
+ * @spare_bytes: Number of spare bytes per chunk
+ * @ecc_bytes: Number of ecc bytes per chunk
+ * @last_chunk_cnt: If there is a last chunk with a different size than
+ * the first ones, the next fields may not be empty

+ * @last_data_bytes: Number of data bytes in the last chunk
+ * @last_spare_bytes: Number of spare bytes in the last chunk
+ * @last_ecc_bytes: Number of ecc bytes in the last chunk
+ */

drivers/mtd/nand/marvell_nand.c new file +16 -0

+struct marvell_hw_ecc_layout {
+	/* Constraints */
+	int writesize;
+	int chunk;
+	int strength;
+	/* Corresponding layout */
+	int full_chunk_cnt;
+	int data_bytes;
+	int spare_bytes;
+	int ecc_bytes;
+	int last_chunk_cnt;
+	int last_data_bytes;
+	int last_spare_bytes;
+	int last_ecc_bytes;
+};
+

drivers/mtd/nand/marvell_nand.c new file +27 -0

+/*
+ * NAND controller structure: stores Marvell NAND controller information
+ *
+ * @controller:		Base controller structure
+ * @dev:		Parent device (used to print error messages)
+ * @regs:		NAND controller registers
+ * @ecc_clk:		ECC block clock, two times the NAND controller clock
+ * @complete:		Completion object to wait for NAND controller events
+ * @assigned_cs:	Bitmask describing already assigned CS lines
+ * @chips:		List containing all the NAND chips attached to
+ *			this NAND controller
+ * @caps:		NAND controller capabilities for each compatible string
+ * @buf:		Controller local buffer to store a part of the read
+ *			buffer when the read operation was not 8 bytes aligned
+ *			as is the FIFO.
+ * @buf_pos:		Position in the 'buf' buffer
+ * @dma_chan:		DMA channel (NFCv1 only)
+ * @dma_buf:		32-bit aligned buffer for DMA transfers (NFCv1 only)
+ */
+struct marvell_nfc {
+	struct nand_hw_control controller;
+	struct device *dev;
+	void __iomem *regs;
+	struct clk *ecc_clk;
+	struct completion complete;
+	unsigned long assigned_cs;
+	struct list_head chips;

drivers/mtd/nand/marvell_nand.c new file +7 -0

+
+	/* DMA (NFCv1 only) */
+	bool use_dma;
+	struct dma_chan *dma_chan;
+	u8 *dma_buf;
+};
+

drivers/mtd/nand/marvell_nand.c new file +6 -0

+/*
+ * Derives a duration in numbers of clock cycles.
+ *
+ * @ps: Duration in pico-seconds
+ * @period_ns:  Clock period in nano-seconds
+ *

+ * @cle_ale_delay_ns: Optional delay after the last CMD or ADDR cycle
+ * @rdy_timeout_ms: Timeout for waits on Ready/Busy pin
+ * @rdy_delay_ns: Optional delay after waiting for the RB pin
+ * @data_delay_ns: Optional delay after the data xfer
+ * @data_instr_idx: Index of the data instruction in the subop
+ * @data_instr: Pointer to the data instruction in the subop
+ */

drivers/mtd/nand/marvell_nand.c new file +9 -0

+struct marvell_nfc_op {
+	u32 ndcb[4];
+	unsigned int cle_ale_delay_ns;
+	unsigned int rdy_timeout_ms;
+	unsigned int rdy_delay_ns;
+	unsigned int data_delay_ns;
+	unsigned int data_instr_idx;
+	const struct nand_op_instr *data_instr;
+};

+ * boolean to indicate if 8-bit accesses must be enabled of disabled without

+ * knowing if 16-bit accesses are actually in use.
+ */

drivers/mtd/nand/marvell_nand.c new file +12 -0

+	if (!(chip->options & NAND_BUSWIDTH_16))
+		return;
+
+	ndcr = readl_relaxed(nfc->regs + NDCR);
+
+	if (force_8bit)
+		ndcr &= ~(NDCR_DWIDTH_M | NDCR_DWIDTH_C);
+	else
+		ndcr |= NDCR_DWIDTH_M | NDCR_DWIDTH_C;
+
+	writel_relaxed(ndcr, nfc->regs + NDCR);
+}

drivers/mtd/nand/marvell_nand.c new file +4 -0

+
+/*
+ * Any time a command has to be sent to the controller, the following sequence
+ * has to be followed:

+ */

drivers/mtd/nand/marvell_nand.c new file +31 -0

+static int marvell_nfc_prepare_cmd(struct nand_chip *chip)
+{
+	struct marvell_nfc *nfc = to_marvell_nfc(chip->controller);
+	u32 ndcr, val;
+	int ret;
+
+	/* Poll ND_RUN and clear NDSR before issuing any command */
+	ret = marvell_nfc_wait_ndrun(chip);
+	if (ret) {
+		dev_err(nfc->dev, "Last operation did not suceed\n");
+		return ret;
+	}
+
+	ndcr = readl_relaxed(nfc->regs + NDCR);
+	writel_relaxed(readl_relaxed(nfc->regs + NDSR), nfc->regs + NDSR);
+
+	/* Assert ND_RUN bit and wait the NFC to be ready */
+	writel_relaxed(ndcr | NDCR_ND_RUN, nfc->regs + NDCR);
+	ret = readl_relaxed_poll_timeout(nfc->regs + NDSR, val,
+					 val & NDSR_WRCMDREQ,
+					 POLL_PERIOD, POLL_TIMEOUT);
+	if (ret) {
+		dev_err(nfc->dev, "Timeout on WRCMDRE\n");
+		return -ETIMEDOUT;
+	}
+
+	/* Command may be written, clear WRCMDREQ status bit */
+	writel_relaxed(NDSR_WRCMDREQ, nfc->regs + NDSR);
+
+	return 0;
+}

drivers/mtd/nand/marvell_nand.c new file +29 -0

+
+static void marvell_nfc_select_chip(struct mtd_info *mtd, int die_nr)
+{
+	struct nand_chip *chip = mtd_to_nand(mtd);
+	struct marvell_nand_chip *marvell_nand = to_marvell_nand(chip);
+	struct marvell_nfc *nfc = to_marvell_nfc(chip->controller);
+	u32 ndcr;
+
+	if (chip == nfc->selected_chip && die_nr == marvell_nand->selected_die)
+		return;
+
+	if (die_nr < 0 || die_nr >= marvell_nand->nsels) {
+		nfc->selected_chip = NULL;
+		marvell_nand->selected_die = -1;
+		return;
+	}
+
+	/*
+	 * Do not change the timing registers when using the DT property
+	 * marvell,nand-keep-config; in that case ->ndtr0 and ->ndtr1 from the
+	 * marvell_nand structure are supposedly empty.
+	 */
+	if (marvell_nand->ndtr0 && marvell_nand->ndtr1) {
+		writel_relaxed(marvell_nand->ndtr0, nfc->regs + NDTR0);
+		writel_relaxed(marvell_nand->ndtr1, nfc->regs + NDTR1);
+	}
+
+	ndcr = readl_relaxed(nfc->regs + NDCR);
+

drivers/mtd/nand/marvell_nand.c new file +11 -0

+
+	/* Update the control register */
+	writel_relaxed(ndcr,  nfc->regs + NDCR);
+
+	/* Also reset the interrupt status register */
+	marvell_nfc_clear_int(nfc, NDCR_ALL_INT);
+
+	nfc->selected_chip = chip;
+	marvell_nand->selected_die = die_nr;
+}
+

drivers/mtd/nand/marvell_nand.c new file +48 -0

+
+/* Read/write PIO/DMA accessors */
+static int marvell_nfc_xfer_data_dma(struct marvell_nfc *nfc,
+				     enum dma_data_direction direction,
+				     unsigned int len)
+{
+	unsigned int dma_len = min_t(int, ALIGN(len, 32), MAX_CHUNK_SIZE);
+	struct dma_async_tx_descriptor *tx;
+	struct scatterlist sg;
+	dma_cookie_t cookie;
+	int ret;
+
+	marvell_nfc_enable_dma(nfc);
+	/* Prepare the DMA transfer */
+	sg_init_one(&sg, nfc->dma_buf, dma_len);
+	dma_map_sg(nfc->dma_chan->device->dev, &sg, 1, direction);
+	tx = dmaengine_prep_slave_sg(nfc->dma_chan, &sg, 1,
+				     direction == DMA_FROM_DEVICE ?
+				     DMA_DEV_TO_MEM : DMA_MEM_TO_DEV,
+				     DMA_PREP_INTERRUPT);
+	if (!tx) {
+		dev_err(nfc->dev, "Could not prepare DMA S/G list\n");
+		return -ENXIO;
+	}
+
+	/* Do the task and wait for it to finish */
+	cookie = dmaengine_submit(tx);
+	ret = dma_submit_error(cookie);
+	if (ret)
+		return -EIO;
+
+	dma_async_issue_pending(nfc->dma_chan);
+	ret = marvell_nfc_wait_cmdd(nfc->selected_chip);
+	dma_unmap_sg(nfc->dma_chan->device->dev, &sg, 1, direction);
+	marvell_nfc_disable_dma(nfc);
+	if (ret) {
+		dev_err(nfc->dev, "Timeout waiting for DMA (status: %d)\n",
+			dmaengine_tx_status(nfc->dma_chan, cookie, NULL));
+		dmaengine_terminate_all(nfc->dma_chan);
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+static int marvell_nfc_xfer_data_in_pio(struct marvell_nfc *nfc, u8 *in,
+					unsigned int len)
+{

drivers/mtd/nand/marvell_nand.c new file +5 -0

+	}
+
+	return 0;
+}
+

drivers/mtd/nand/marvell_nand.c new file +20 -0

+
+/*
+ * Check a chunk is correct or not according to hardware ECC engine.
+ * mtd->ecc_stats.corrected is updated, as well as max_bitflips, however
+ * mtd->ecc_stats.failure is not, the function will instead return a non-zero
+ * value indicating that a check on the emptyness of the subpage must be
+ * performed before declaring the subpage corrupted.
+ */
+static int marvell_nfc_hw_ecc_correct(struct nand_chip *chip,
+				      unsigned int *max_bitflips)
+{
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	struct marvell_nfc *nfc = to_marvell_nfc(chip->controller);
+	int bf = 0;
+	u32 ndsr;
+
+	ndsr = readl_relaxed(nfc->regs + NDSR);
+
+	/* Check uncorrectable error flag */
+	if (ndsr & NDSR_UNCERR) {

drivers/mtd/nand/marvell_nand.c new file +39 -0

+	}
+
+	/* Check correctable error flag */
+	if (ndsr & NDSR_CORERR) {
+		writel_relaxed(ndsr, nfc->regs + NDSR);
+
+		if (chip->ecc.algo == NAND_ECC_BCH)
+			bf = NDSR_ERRCNT(ndsr);
+		else
+			bf = 1;
+	}
+
+	/* Update the stats and max_bitflips */
+	mtd->ecc_stats.corrected += bf;
+	*max_bitflips = max_t(unsigned int, *max_bitflips, bf);
+
+	return 0;
+}
+
+/* Hamming read helpers */
+static int marvell_nfc_hw_ecc_hmg_do_read_page(struct nand_chip *chip, u8 *buf,
+					       bool oob_required, bool raw,
+					       int page)
+{
+	struct marvell_nand_chip *marvell_nand = to_marvell_nand(chip);
+	struct marvell_nfc *nfc = to_marvell_nfc(chip->controller);
+	const struct marvell_hw_ecc_layout *lt = to_marvell_nand(chip)->layout;
+	struct marvell_nfc_op nfc_op = {
+		.ndcb[0] = NDCB0_CMD_TYPE(TYPE_READ) |
+			   NDCB0_ADDR_CYC(marvell_nand->addr_cyc) |
+			   NDCB0_DBC |
+			   NDCB0_CMD1(NAND_CMD_READ0) |
+			   NDCB0_CMD2(NAND_CMD_READSTART),
+		.ndcb[1] = NDCB1_ADDRS_PAGE(page),
+		.ndcb[2] = NDCB2_ADDR5_PAGE(page),
+	};
+	unsigned int oob_bytes = 0;
+	int ret;
+

drivers/mtd/nand/marvell_nand.c new file +29 -0

+		if (raw)
+			oob_bytes += lt->ecc_bytes;
+	}
+
+	marvell_nfc_send_cmd(chip, &nfc_op);
+	ret = marvell_nfc_end_cmd(chip, NDSR_RDDREQ,
+				  "RDDREQ while draining FIFO (data/oob)");
+	if (ret)
+		return ret;
+
+	/* Read the page then the OOB area */
+	if (nfc->use_dma) {
+		marvell_nfc_xfer_data_dma(nfc, DMA_FROM_DEVICE,
+					  lt->data_bytes + oob_bytes);
+		memcpy(buf, nfc->dma_buf, lt->data_bytes);
+		memcpy(chip->oob_poi + (raw ? 0 : lt->ecc_bytes),
+		       nfc->dma_buf + lt->data_bytes, oob_bytes);
+	} else {
+		marvell_nfc_xfer_data_in_pio(nfc, buf, lt->data_bytes);
+		marvell_nfc_xfer_data_in_pio(nfc, chip->oob_poi, oob_bytes);
+	}
+
+	ret = marvell_nfc_wait_cmdd(chip);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+

drivers/mtd/nand/marvell_nand.c new file +8 -0

+
+static int marvell_nfc_hw_ecc_hmg_read_page(struct mtd_info *mtd,
+					    struct nand_chip *chip,
+					    u8 *buf, int oob_required,
+					    int page)
+{
+	const struct marvell_hw_ecc_layout *lt = to_marvell_nand(chip)->layout;
+	int max_bitflips = 0, ret;

drivers/mtd/nand/marvell_nand.c new file +23 -0

+	marvell_nfc_enable_hw_ecc(chip);
+	marvell_nfc_hw_ecc_hmg_do_read_page(chip, buf, true, false, page);
+	ret = marvell_nfc_hw_ecc_correct(chip, &max_bitflips);
+	marvell_nfc_disable_hw_ecc(chip);
+
+	if (!ret)
+		return max_bitflips;
+
+	/*
+	 * Re-read the OOB area in raw mode if ECC failures have been detected
+	 * to check for empty pages.
+	 */
+	nand_change_read_column_op(chip, lt->data_bytes + lt->spare_bytes,
+				   chip->oob_poi + lt->spare_bytes,
+				   lt->ecc_bytes, false);
+	marvell_nfc_check_empty_chunk(chip, buf, lt->data_bytes, chip->oob_poi,
+				      lt->spare_bytes, chip->oob_poi +
+				      lt->spare_bytes, lt->ecc_bytes,
+				      &max_bitflips);
+
+	return max_bitflips;
+}
+

drivers/mtd/nand/marvell_nand.c new file +21 -0

+
+/* Hamming write helpers */
+static int marvell_nfc_hw_ecc_hmg_do_write_page(struct nand_chip *chip,
+						const u8 *buf,
+						bool oob_required, bool raw,
+						int page)
+{
+	struct marvell_nand_chip *marvell_nand = to_marvell_nand(chip);
+	struct marvell_nfc *nfc = to_marvell_nfc(chip->controller);
+	const struct marvell_hw_ecc_layout *lt = to_marvell_nand(chip)->layout;
+	struct marvell_nfc_op nfc_op = {
+		.ndcb[0] = NDCB0_CMD_TYPE(TYPE_WRITE) |
+			   NDCB0_ADDR_CYC(marvell_nand->addr_cyc) |
+			   NDCB0_CMD1(NAND_CMD_SEQIN) |
+			   NDCB0_CMD2(NAND_CMD_PAGEPROG) |
+			   NDCB0_DBC,
+		.ndcb[1] = NDCB1_ADDRS_PAGE(page),
+		.ndcb[2] = NDCB2_ADDR5_PAGE(page),
+	};
+	int oob_bytes = 0;
+	int ret;

drivers/mtd/nand/marvell_nand.c new file +35 -0

+		oob_bytes = lt->spare_bytes;
+		if (raw)
+			oob_bytes += lt->ecc_bytes;
+	}
+
+	marvell_nfc_send_cmd(chip, &nfc_op);
+	ret = marvell_nfc_end_cmd(chip, NDSR_WRDREQ,
+				  "WRDREQ while loading FIFO (data)");
+	if (ret)
+		return ret;
+
+	/* Write the page then the OOB area */
+	if (nfc->use_dma) {
+		memcpy(nfc->dma_buf, buf, lt->data_bytes);
+		if (oob_required)
+			memcpy(nfc->dma_buf + lt->data_bytes, chip->oob_poi,
+			       oob_bytes);
+		marvell_nfc_xfer_data_dma(nfc, DMA_TO_DEVICE, lt->data_bytes +
+					  lt->ecc_bytes + lt->spare_bytes);
+	} else {
+		marvell_nfc_xfer_data_out_pio(nfc, buf, lt->data_bytes);
+		if (oob_required)
+			marvell_nfc_xfer_data_out_pio(nfc, chip->oob_poi,
+						      oob_bytes);
+	}
+
+	ret = marvell_nfc_wait_cmdd(chip);
+	if (ret)
+		return ret;
+
+	ret = marvell_nfc_wait_op(chip,
+				  chip->data_interface.timings.sdr.tPROG_max);
+	return ret;
+}
+

drivers/mtd/nand/marvell_nand.c new file +7 -0

+static int marvell_nfc_hw_ecc_hmg_write_page(struct mtd_info *mtd,
+					     struct nand_chip *chip,
+					     const u8 *buf,
+					     int oob_required, int page)
+{
+	int ret;
+

drivers/mtd/nand/marvell_nand.c new file +9 -0

+
+	marvell_nfc_enable_hw_ecc(chip);
+	ret = marvell_nfc_hw_ecc_hmg_do_write_page(chip, buf, true, false,
+						   page);
+	marvell_nfc_disable_hw_ecc(chip);
+
+	return ret;
+}
+

drivers/mtd/nand/marvell_nand.c new file +41 -0

+
+static void marvell_nfc_hw_ecc_bch_read_chunk(struct nand_chip *chip, int chunk,
+					      u8 *data, unsigned int data_len,
+					      u8 *spare, unsigned int spare_len,
+					      int page)
+{
+	struct marvell_nand_chip *marvell_nand = to_marvell_nand(chip);
+	struct marvell_nfc *nfc = to_marvell_nfc(chip->controller);
+	const struct marvell_hw_ecc_layout *lt = to_marvell_nand(chip)->layout;
+	int nchunks = lt->full_chunk_cnt + lt->last_chunk_cnt;
+	int i, ret;
+	struct marvell_nfc_op nfc_op = {
+		.ndcb[0] = NDCB0_CMD_TYPE(TYPE_READ) |
+			   NDCB0_ADDR_CYC(marvell_nand->addr_cyc) |
+			   NDCB0_LEN_OVRD,
+		.ndcb[1] = NDCB1_ADDRS_PAGE(page),
+		.ndcb[2] = NDCB2_ADDR5_PAGE(page),
+		.ndcb[3] = data_len + spare_len,
+	};
+
+	ret = marvell_nfc_prepare_cmd(chip);
+	if (ret)
+		return;
+
+	if (chunk == 0)
+		nfc_op.ndcb[0] |= NDCB0_DBC |
+				  NDCB0_CMD1(NAND_CMD_READ0) |
+				  NDCB0_CMD2(NAND_CMD_READSTART);
+
+	/*
+	 * Trigger the naked read operation only on the last chunk.
+	 * Otherwise, use monolithic read.
+	 */
+	if (nchunks == 1 || (chunk < nchunks - 1))
+		nfc_op.ndcb[0] |= NDCB0_CMD_XTYPE(XTYPE_MONOLITHIC_RW);
+	else
+		nfc_op.ndcb[0] |= NDCB0_CMD_XTYPE(XTYPE_LAST_NAKED_RW);
+
+	marvell_nfc_send_cmd(chip, &nfc_op);
+
+	/*

drivers/mtd/nand/marvell_nand.c new file +63 -0

+	for (i = 0; i < data_len; i += FIFO_DEPTH * BCH_SEQ_READS) {
+		marvell_nfc_end_cmd(chip, NDSR_RDDREQ,
+				    "RDDREQ while draining FIFO (data)");
+		marvell_nfc_xfer_data_in_pio(nfc, data,
+					     FIFO_DEPTH * BCH_SEQ_READS);
+		data += FIFO_DEPTH * BCH_SEQ_READS;
+	}
+
+	for (i = 0; i < spare_len; i += FIFO_DEPTH * BCH_SEQ_READS) {
+		marvell_nfc_end_cmd(chip, NDSR_RDDREQ,
+				    "RDDREQ while draining FIFO (OOB)");
+		marvell_nfc_xfer_data_in_pio(nfc, spare,
+					     FIFO_DEPTH * BCH_SEQ_READS);
+		spare += FIFO_DEPTH * BCH_SEQ_READS;
+	}
+}
+
+static int marvell_nfc_hw_ecc_bch_read_page(struct mtd_info *mtd,
+					    struct nand_chip *chip,
+					    u8 *buf, int oob_required,
+					    int page)
+{
+	const struct marvell_hw_ecc_layout *lt = to_marvell_nand(chip)->layout;
+	int nchunks = lt->full_chunk_cnt + lt->last_chunk_cnt;
+	int data_len = lt->data_bytes, spare_len = lt->spare_bytes, ecc_len;
+	u8 *data = buf, *spare = chip->oob_poi, *ecc;
+	int max_bitflips = 0;
+	u32 failure_mask = 0;
+	int chunk, ecc_offset_in_page, ret;
+
+	/*
+	 * With BCH, OOB is not fully used (and thus not read entirely), not
+	 * expected bytes could show up at the end of the OOB buffer if not
+	 * explicitly erased.
+	 */
+	if (oob_required)
+		memset(chip->oob_poi, 0xFF, mtd->oobsize);
+
+	marvell_nfc_enable_hw_ecc(chip);
+
+	for (chunk = 0; chunk < nchunks; chunk++) {
+		/* Update length for the last chunk */
+		if (chunk >= lt->full_chunk_cnt) {
+			data_len = lt->last_data_bytes;
+			spare_len = lt->last_spare_bytes;
+		}
+
+		/* Read the chunk and detect number of bitflips */
+		marvell_nfc_hw_ecc_bch_read_chunk(chip, chunk, data, data_len,
+						  spare, spare_len, page);
+		ret = marvell_nfc_hw_ecc_correct(chip, &max_bitflips);
+		if (ret)
+			failure_mask |= BIT(chunk);
+
+		data += data_len;
+		spare += spare_len;
+	}
+
+	marvell_nfc_disable_hw_ecc(chip);
+
+	if (!failure_mask)
+		return max_bitflips;
+

drivers/mtd/nand/marvell_nand.c new file +41 -0

+	for (chunk = 0; chunk < nchunks; chunk++) {
+		/* No failure reported for this chunk, move to the next one */
+		if (!(failure_mask & BIT(chunk)))
+			continue;
+
+		/* Derive ECC bytes positions (in page/buffer) and length */
+		ecc = chip->oob_poi +
+			(lt->full_chunk_cnt * lt->spare_bytes) +
+			(lt->last_chunk_cnt * lt->last_spare_bytes) +
+			(chunk * ALIGN(lt->ecc_bytes, 32));
+		ecc_offset_in_page =
+			(chunk * (lt->data_bytes + lt->spare_bytes +
+				  lt->ecc_bytes)) +
+			(chunk < lt->full_chunk_cnt ?
+			 lt->data_bytes + lt->spare_bytes :
+			 lt->last_data_bytes + lt->last_spare_bytes);
+		ecc_len = chunk < lt->full_chunk_cnt ?
+			lt->ecc_bytes : lt->last_ecc_bytes;
+
+		/* Do the actual raw read of the ECC bytes */
+		nand_change_read_column_op(chip, ecc_offset_in_page,
+					   ecc, ecc_len, false);
+
+		/* Derive data/spare bytes positions (in buffer) and length */
+		data = buf + (chunk * lt->data_bytes);
+		data_len = chunk < lt->full_chunk_cnt ?
+			lt->data_bytes : lt->last_data_bytes;
+		spare = chip->oob_poi + (chunk * (lt->spare_bytes +
+						  lt->ecc_bytes));
+		spare_len = chunk < lt->full_chunk_cnt ?
+			lt->spare_bytes : lt->last_spare_bytes;
+
+		/* Check the entire chunk (data + spare + ecc) for emptyness */
+		marvell_nfc_check_empty_chunk(chip, data, data_len, spare,
+					      spare_len, ecc, ecc_len,
+					      &max_bitflips);
+	}
+
+	return max_bitflips;
+}
+

drivers/mtd/nand/marvell_nand.c new file +8 -0

+
+/*
+ * HW ECC layouts, identical to old pxa3xx_nand driver,
+ * to be fully backward compatible.
+ */
+static int marvell_nand_ooblayout_ecc(struct mtd_info *mtd, int section,
+				      struct mtd_oob_region *oobregion)
+{

drivers/mtd/nand/marvell_nand.c new file +41 -0

+
+	return 0;
+}
+
+static int marvell_nand_ooblayout_free(struct mtd_info *mtd, int section,
+				       struct mtd_oob_region *oobregion)
+{
+	struct nand_chip *chip = mtd_to_nand(mtd);
+	const struct marvell_hw_ecc_layout *lt = to_marvell_nand(chip)->layout;
+	int nchunks = lt->full_chunk_cnt;
+
+	if (section >= nchunks)
+		return -ERANGE;
+
+	if (!lt->spare_bytes)
+		return 0;
+
+	oobregion->offset = section * (lt->spare_bytes + lt->ecc_bytes);
+	oobregion->length = lt->spare_bytes;
+	if (!section) {
+		/*
+		 * Bootrom looks in bytes 0 & 5 for bad blocks for the
+		 * 4KB page / 4bit BCH combination.
+		 */
+		if (mtd->writesize == SZ_4K && lt->data_bytes == SZ_2K) {
+			oobregion->offset += 6;
+			oobregion->length -= 6;
+		} else {
+			oobregion->offset += 2;
+			oobregion->length -= 2;
+		}
+	}
+
+	return 0;
+}
+
+static const struct mtd_ooblayout_ops marvell_nand_ooblayout_ops = {
+	.ecc = marvell_nand_ooblayout_ecc,
+	.free = marvell_nand_ooblayout_free,
+};
+

drivers/mtd/nand/marvell_nand.c new file +18 -0

+
+static int marvell_nand_ecc_init(struct mtd_info *mtd,
+				 struct nand_ecc_ctrl *ecc)
+{
+	struct nand_chip *chip = mtd_to_nand(mtd);
+	struct marvell_nfc *nfc = to_marvell_nfc(chip->controller);
+	int ret;
+
+	if ((ecc->mode != NAND_ECC_NONE) && (!ecc->size || !ecc->strength)) {
+		if (chip->ecc_step_ds && chip->ecc_strength_ds) {
+			ecc->size = chip->ecc_step_ds;
+			ecc->strength = chip->ecc_strength_ds;
+		} else {
+			dev_info(nfc->dev,
+				 "No minimum ECC strength, using 1b/512B\n");
+			ecc->size = 512;
+			ecc->strength = 1;
+		}

drivers/mtd/nand/marvell_nand.c new file +8 -0

+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+

drivers/mtd/nand/marvell_nand.c new file +183 -0

+static int marvell_nand_chip_init(struct device *dev, struct marvell_nfc *nfc,
+				  struct device_node *np)
+{
+	struct pxa3xx_nand_platform_data *pdata = dev_get_platdata(dev);
+	struct marvell_nand_chip *marvell_nand;
+	struct mtd_info *mtd;
+	struct nand_chip *chip;
+	int nsels, ret, i;
+	u32 cs, rb;
+
+	/*
+	 * The legacy "num-cs" property indicates the number of CS on the only
+	 * chip connected to the controller (legacy bindings does not support
+	 * more than one chip). CS are only incremented one by one while the RB
+	 * pin is always the #0.
+	 *
+	 * When not using legacy bindings, a couple of "reg" and "marvell,rb"
+	 * properties must be filled. For each chip, expressed as a subnode,
+	 * "reg" points to the CS lines and "marvell,rb" to the RB line.
+	 */
+	if (pdata) {
+		nsels = 1;
+	} else if (nfc->caps->legacy_of_bindings) {
+		if (!of_get_property(np, "num-cs", &nsels)) {
+			dev_err(dev, "missing num-cs property\n");
+			return -EINVAL;
+		}
+	} else {
+		if (!of_get_property(np, "reg", &nsels)) {
+			dev_err(dev, "missing reg property\n");
+			return -EINVAL;
+		}
+	}
+
+	if (!pdata)
+		nsels /= sizeof(u32);
+	if (!nsels) {
+		dev_err(dev, "invalid reg property size\n");
+		return -EINVAL;
+	}
+
+	/* Alloc the nand chip structure */
+	marvell_nand = devm_kzalloc(dev, sizeof(*marvell_nand) +
+				    (nsels *
+				     sizeof(struct marvell_nand_chip_sel)),
+				    GFP_KERNEL);
+	if (!marvell_nand) {
+		dev_err(dev, "could not allocate chip structure\n");
+		return -ENOMEM;
+	}
+
+	marvell_nand->nsels = nsels;
+	marvell_nand->selected_die = -1;
+
+	for (i = 0; i < nsels; i++) {
+		if (pdata || nfc->caps->legacy_of_bindings) {
+			/*
+			 * Legacy bindings use the CS lines in natural
+			 * order (0, 1, ...)
+			 */
+			cs = i;
+		} else {
+			/* Retrieve CS id */
+			ret = of_property_read_u32_index(np, "reg", i, &cs);
+			if (ret) {
+				dev_err(dev, "could not retrieve reg property: %d\n",
+					ret);
+				return ret;
+			}
+		}
+
+		if (cs >= nfc->caps->max_cs_nb) {
+			dev_err(dev, "invalid reg value: %u (max CS = %d)\n",
+				cs, nfc->caps->max_cs_nb);
+			return -EINVAL;
+		}
+
+		if (test_and_set_bit(cs, &nfc->assigned_cs)) {
+			dev_err(dev, "CS %d already assigned\n", cs);
+			return -EINVAL;
+		}
+
+		/*
+		 * The cs variable represents the chip select id, which must be
+		 * converted in bit fields for NDCB0 and NDCB2 to select the
+		 * right chip. Unfortunately, due to a lack of information on
+		 * the subject and incoherent documentation, the user should not
+		 * use CS1 and CS3 at all as asserting them is not supported in
+		 * a reliable way (due to multiplexing inside ADDR5 field).
+		 */
+		marvell_nand->sels[i].cs = cs;
+		switch (cs) {
+		case 0:
+		case 2:
+			marvell_nand->sels[i].ndcb0_csel = 0;
+			break;
+		case 1:
+		case 3:
+			marvell_nand->sels[i].ndcb0_csel = NDCB0_CSEL;
+			break;
+		default:
+			return -EINVAL;
+		}
+
+		/* Retrieve RB id */
+		if (pdata || nfc->caps->legacy_of_bindings) {
+			/* Legacy bindings always use RB #0 */
+			rb = 0;
+		} else {
+			ret = of_property_read_u32_index(np, "marvell,rb", i,
+							 &rb);
+			if (ret) {
+				dev_err(dev,
+					"could not retrieve RB property: %d\n",
+					ret);
+				return ret;
+			}
+		}
+
+		if (rb >= nfc->caps->max_rb_nb) {
+			dev_err(dev, "invalid reg value: %u (max RB = %d)\n",
+				rb, nfc->caps->max_rb_nb);
+			return -EINVAL;
+		}
+
+		marvell_nand->sels[i].rb = rb;
+	}
+
+	chip = &marvell_nand->chip;
+	chip->controller = &nfc->controller;
+	nand_set_flash_node(chip, np);
+
+	chip->exec_op = marvell_nfc_exec_op;
+	chip->select_chip = marvell_nfc_select_chip;
+	if (nfc->caps->is_nfcv2 &&
+	    !of_property_read_bool(np, "marvell,nand-keep-config"))
+		chip->setup_data_interface = marvell_nfc_setup_data_interface;
+
+	mtd = nand_to_mtd(chip);
+	mtd->dev.parent = dev;
+
+	/*
+	 * Default to HW ECC engine mode. If the nand-ecc-mode property is given
+	 * in the DT node, this entry will be overwritten in nand_scan_ident().
+	 */
+	chip->ecc.mode = NAND_ECC_HW;
+
+	ret = nand_scan_ident(mtd, marvell_nand->nsels, NULL);
+	if (ret) {
+		dev_err(dev, "could not identify the nand chip\n");
+		return ret;
+	}
+
+	if (pdata && pdata->flash_bbt)
+		chip->bbt_options |= NAND_BBT_USE_FLASH;
+
+	if (chip->bbt_options & NAND_BBT_USE_FLASH) {
+		/*
+		 * We'll use a bad block table stored in-flash and don't
+		 * allow writing the bad block marker to the flash.
+		 */
+		chip->bbt_options |= NAND_BBT_NO_OOB_BBM;
+		chip->bbt_td = &bbt_main_descr;
+		chip->bbt_md = &bbt_mirror_descr;
+	}
+
+	/*
+	 * With RA_START bit set in NDCR, columns takes two address cycles. This
+	 * means addressing a chip with more than 256 pages needs a fifth
+	 * address cycle. Addressing a chip using CS 2 or 3 should also needs
+	 * this additional cycle but due to insistance in the documentation and
+	 * lack of hardware to test this situation, this case has been dropped
+	 * and is not supported by this driver.
+	 */
+	marvell_nand->addr_cyc = 4;
+	if (chip->options & NAND_ROW_ADDR_3)
+		marvell_nand->addr_cyc = 5;
+
+	if (pdata) {
+		chip->ecc.size = pdata->ecc_step_size;
+		chip->ecc.strength = pdata->ecc_strength;
+	}
+

drivers/mtd/nand/marvell_nand.c new file +69 -0

+		 * allowed and subpage write, if used, would lead to numerous
+		 * uncorrectable ECC errors.
+		 */
+		chip->options |= NAND_NO_SUBPAGE_WRITE;
+	}
+
+	if (pdata || nfc->caps->legacy_of_bindings) {
+		/*
+		 * We keep the MTD name unchanged to avoid breaking platforms
+		 * where the MTD cmdline parser is used and the bootloader
+		 * has not been updated to use the new naming scheme.
+		 */
+		mtd->name = "pxa3xx_nand-0";
+	} else if (!mtd->name) {
+		/*
+		 * If the new bindings are used and the bootloader has not been
+		 * updated to pass a new mtdparts parameter on the cmdline, you
+		 * should define the following property in your NAND node, ie:
+		 *
+		 *	label = "main-storage";
+		 *
+		 * This way, mtd->name will be set by the core when
+		 * nand_set_flash_node() is called.
+		 */
+		mtd->name = devm_kasprintf(nfc->dev, GFP_KERNEL,
+					   "%s:nand.%d", dev_name(nfc->dev),
+					   marvell_nand->sels[0].cs);
+		if (!mtd->name) {
+			dev_err(nfc->dev, "Failed to allocate mtd->name\n");
+			return -ENOMEM;
+		}
+	}
+
+	ret = nand_scan_tail(mtd);
+	if (ret) {
+		dev_err(dev, "nand_scan_tail failed: %d\n", ret);
+		return ret;
+	}
+
+	if (pdata)
+		/* Legacy bindings support only one chip */
+		ret = mtd_device_register(mtd, pdata->parts[0],
+					  pdata->nr_parts[0]);
+	else
+		ret = mtd_device_register(mtd, NULL, 0);
+	if (ret) {
+		dev_err(dev, "failed to register mtd device: %d\n", ret);
+		nand_release(mtd);
+		return ret;
+	}
+
+	list_add_tail(&marvell_nand->node, &nfc->chips);
+
+	return 0;
+}
+
+static int marvell_nand_chips_init(struct device *dev, struct marvell_nfc *nfc)
+{
+	struct device_node *np = dev->of_node;
+	struct device_node *nand_np;
+	int max_cs = nfc->caps->max_cs_nb;
+	int nchips;
+	int ret;
+
+	if (!np)
+		nchips = 1;
+	else
+		nchips = of_get_child_count(np);
+

drivers/mtd/nand/marvell_nand.c new file +7 -0

+	 */
+	if (nfc->caps->legacy_of_bindings) {
+		ret = marvell_nand_chip_init(dev, nfc, np);
+		return ret;
+	}
+
+	for_each_child_of_node(np, nand_np) {

Hi Miquel,

Quoted 9 lines · On Tue, 19 Dec 2017 14:29:39 +0100 Miquel Raynal…

Add marvell_nand driver which aims at replacing the existing pxa3xx_nand driver.

The new driver intends to be easier to understand and follows the brand new NAND framework rules by implementing hooks for every pattern the controller might support and referencing them inside a parser object that will be given to the core at each ->exec_op() call.

Quoted 7 lines

Raw accessors are implemented, useful to test/debug memory/filesystem corruptions. Userspace binaries contained in the mtd-utils package may now be used and their output trusted.

Timings may not be kept from the bootloader anymore, the timings used for instance in U-Boot were not optimal and it supposed to have NAND support (and initialized) in the bootloader.

Hm, AFAIR the old driver was able to dynamically adjust the timings when the NAND was ONFI compliant.

Quoted 11 lines

Thanks to the improved timings, implementation of ONFI mode 5 support (with EDO managed by adding a delay on data sampling), merging the commands together and optimizing writes in the command registers, the new driver may achieve faster throughputs in both directions. Measurements show an improvement of about +23% read throughput and +24% write throughput. These measurements have been done with an Armada-385-DB-AP (4kiB NAND pages forced in 4-bit strength BCH ECC correction) using the userspace tool 'flash_speed' from the MTD test suite.

Quoted 10 lines

Besides these important topics, the new driver addresses several unsolved known issues in the old driver which:
- did not work with ECC soft neither with ECC none ;
- relied on naked read/write (which is unchanged) while the NFCv1 embedded in the pxa3xx platforms do not implement it, so several NAND commands did not actually ever work without any notice (like reading the ONFI PARAM_PAGE or SET/GET_FEATURES) ;
- wrote the OOB data correctly, but was not able to read it correctly past the first OOB data chunk ;
- did not displayed ECC bytes ;

^display

and I'm not even sure display is the right term here. How about 'retrieve'.

Quoted 13 lines

- used device tree bindings that did not allow more than one NAND chip, and did not allow to choose the correct chip select if not incrementing from 0. Plus, the Ready/Busy line used had to be 0.

Old device tree bindings are still supported but deprecated. A more hierarchical view has to be used to keep the controller and the NAND chip structures clearly separated both inside the device tree and also in the driver code.

Signed-off-by Miquel Raynal <[email protected]>

Tested-by Sean Nyekjaer <[email protected]>

Tested-by Willy Tarreau <[email protected]>

---

Actually remove pxa3xx_nand.c to let marvell_nand.c be compiled instead. Also change the defconfig files using it as well as some board files depending on CONFIG_MTD_NAND_PXA3xx.

Signed-off-by Miquel Raynal <[email protected]>

Quoted 5 lines · Miquel Raynal

Actually remove pxa3xx_nand.c to let marvell_nand.c be compiled instead. Also change the defconfig files using it as well as some board files depending on CONFIG_MTD_NAND_PXA3xx.

Signed-off-by Miquel Raynal <[email protected]>

Actually that approach doesn't quite suit me.
I'm not convinced at all a new rewrite of the driver will be stable right out of the box as the old one is.

What I would propose instead is :
- keep both the new marvell nand driver and the old pxa3xx_nand driver
- switch pxa_defconfig to compile them both
- continue testing (with my help)

Once I see the marvell_nand is working properly, we can remove pca3xx_nand in a second stage, keeping the pxa platform functional all along.

Would you agree to that path ?

As a side not, this patch should be split into 2 parts :
- one for arch/arm -> switch to the new driver
- one for drivers/mtd -> removal of the old driver

Cheers.

--
Robert

Quoted 9 lines · On Fri, 22 Dec 2017 21:47:18 +0100 Robert Jarzmik <[email protected]>…

What I would propose instead is :
- keep both the new marvell nand driver and the old pxa3xx_nand driver
- switch pxa_defconfig to compile them both
- continue testing (with my help)

Once I see the marvell_nand is working properly, we can remove pca3xx_nand in a second stage, keeping the pxa platform functional all along.

Would you agree to that path ?

Hm, I tried this approach already (with Marvell's crypto engine) and it doesn't quite solve the problem, just delay it a bit until someone decides to remove the old driver. In my experience, people only start testing the new driver when they're forced to do it (IOW, when the old driver is removed and defconfigs are patched to use the new one). So let's find a way to fix the remaining issues you have instead of delaying the inevitable.

Quoted 5 lines · Miquel Raynal

Actually remove pxa3xx_nand.c to let marvell_nand.c be compiled instead. Also change the defconfig files using it as well as some board files depending on CONFIG_MTD_NAND_PXA3xx.

Signed-off-by Miquel Raynal <[email protected]>

Actually that approach doesn't quite suit me.
I'm not convinced at all a new rewrite of the driver will be stable right out of the box as the old one is.

What I would propose instead is :
- keep both the new marvell nand driver and the old pxa3xx_nand driver
- switch pxa_defconfig to compile them both

- continue testing (with my help)

Once I see the marvell_nand is working properly, we can remove pca3xx_nand in a second stage, keeping the pxa platform functional all along.

Would you agree to that path ?

As a side not, this patch should be split into 2 parts :
- one for arch/arm -> switch to the new driver
- one for drivers/mtd -> removal of the old driver

Cheers.

--
Robert

Quoted 3 lines

What I would propose instead is :
- keep both the new marvell nand driver and the old pxa3xx_nand driver
- switch pxa_defconfig to compile them both

Didn't notice you were suggesting to compile both, which doesn't work because both drivers match the same devices, and only one of them can actually claim the device (likely the first one to register to the device model). So, to make it safe you need to have a

depends on !MTD_NAND_PXA3xx

in your MTD_NAND_MARVELL entry, which means only one driver can be compiled.

So let's find a way to fix the remaining issues you have instead of delaying the inevitable.

Quoted 9 lines · Boris Brezillon

Quoted 3 lines

What I would propose instead is :
- keep both the new marvell nand driver and the old pxa3xx_nand driver
- switch pxa_defconfig to compile them both

Didn't notice you were suggesting to compile both, which doesn't work because both drivers match the same devices, and only one of them can actually claim the device (likely the first one to register to the device model). So, to make it safe you need to have a

Quoted 4 lines

depends on !MTD_NAND_PXA3xx

in your MTD_NAND_MARVELL entry, which means only one driver can be compiled.

Mmm... that is I didn't explain to you what pxa_defconfig is designed for. This defconfig is not for any board actually, it rather is a build coverage tool.

Quoted 2 lines

So let's find a way to fix the remaining issues you have instead of delaying the inevitable.

It's up to you of course, as long as my boards don't break, and nothing breaking them is merged, I'm fine with it. It's just not the approach I usually choose, I'm rather a 2-step guy, ie. merge the new one, then merge the switch (which can be reverted easilly).

The deprecated pxa3xx_nand.c driver does not exist anymore, it has been replaced by marvell_nand.c which has its own up-to-date documentation.

Signed-off-by Miquel Raynal <[email protected]>

---
 .../devicetree/bindings/mtd/pxa3xx-nand.txt        | 50 ----------------------
 1 file changed, 50 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/mtd/pxa3xx-nand.txt