All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/6] mtd: rawnand: add NVIDIA Tegra NAND flash support
@ 2018-06-17 20:45 Stefan Agner
  2018-06-17 20:46 ` [PATCH v5 1/6] mtd: rawnand: add Reed-Solomon error correction algorithm Stefan Agner
                   ` (6 more replies)
  0 siblings, 7 replies; 11+ messages in thread
From: Stefan Agner @ 2018-06-17 20:45 UTC (permalink / raw)
  To: boris.brezillon, dwmw2, computersforpeace, marek.vasut, robh+dt,
	mark.rutland, thierry.reding
  Cc: dev, miquel.raynal, richard, marcel, krzk, digetx,
	benjamin.lindqvist, jonathanh, pdeschrijver, pgaikwad,
	mirza.krak, gaireg, linux-mtd, linux-tegra, devicetree,
	linux-kernel, Stefan Agner

Changes definitly calm down, most noteably probably the changes
around checking whether a page is empty if the stack reports ECC
errors.. I verified the code using raw nandwrites with OOB to
simulate an empty page which has some bits flipped in the OOB area,
everthing seems to work as I would expect it.

For now I do not check extra OOB bytes since those are at variable
locations depending on algorithm.

--
Stefan

Changes since v1:
- Split controller and NAND chip structure
- Add BCH support
- Allow to select algorithm and strength using device tree
- Improve HW ECC error reporting and use DEC_STATUS_BUF only
- Use SPDX license identifier
- Use per algorithm mtd_ooblayout_ops
- Use setup_data_interface callback for NAND timing configuration

Changes since v2:
- Set clock rate using assigned-clocks
- Use BIT() macro
- Fix and improve timing calculation
- Improve ECC error handling
- Store OOB layout for tag area in Tegra chip structure
- Update/fix bindings
- Use more specific variable names (replace "value")
- Introduce nand-is-boot-medium
- Choose sensible ECC strenght automatically
- Use wait_for_completion_timeout
- Print register dump on completion timeout
- Unify tegra_nand_(read|write)_page in tegra_nand_page_xfer

Changes since v3:
- Implement tegra_nand_(read|write)_raw using DMA
- Implement tegra_nand_(read|write)_oob using DMA
- Name registers according to Tegra 2 Technical Reference Manual (v02p)
- Use wait_for_completion_io_timeout to account for IO
- Get chip select id from device tree reg property
- Clear interrupts and reinit wait queues in case command/DMA times out
- Set default MTD name after nand_set_flash_node
- Move MODULE_DEVICE_TABLE after declaration of tegra_nand_of_match
- Make (rs|bch)_strength static

Changes since v4:
- Pass OOB area to nand_check_erased_ecc_chunk
- Pass algorithm specific bits_per_step to tegra_nand_get_strength
- Store ECC layout in chip structure
- Fix pointer assignment (use NULL)
- Removed obsolete header delay.h
- Fixed newlines
- Use non-_io variant of wait_for_completion_timeout

Lucas Stach (1):
  ARM: dts: tegra: add Tegra20 NAND flash controller node

Stefan Agner (5):
  mtd: rawnand: add Reed-Solomon error correction algorithm
  mtd: rawnand: add an option to specify NAND chip as a boot device
  mtd: rawnand: tegra: add devicetree binding
  mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver
  ARM: dts: tegra: enable NAND flash on Colibri T20

 .../devicetree/bindings/mtd/nand.txt          |    6 +-
 .../bindings/mtd/nvidia-tegra20-nand.txt      |   64 +
 MAINTAINERS                                   |    7 +
 arch/arm/boot/dts/tegra20-colibri-512.dtsi    |   16 +
 arch/arm/boot/dts/tegra20.dtsi                |   15 +
 drivers/mtd/nand/raw/Kconfig                  |    6 +
 drivers/mtd/nand/raw/Makefile                 |    1 +
 drivers/mtd/nand/raw/nand_base.c              |    4 +
 drivers/mtd/nand/raw/tegra_nand.c             | 1268 +++++++++++++++++
 include/linux/mtd/rawnand.h                   |    7 +
 10 files changed, 1393 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
 create mode 100644 drivers/mtd/nand/raw/tegra_nand.c

-- 
2.17.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v5 1/6] mtd: rawnand: add Reed-Solomon error correction algorithm
  2018-06-17 20:45 [PATCH v5 0/6] mtd: rawnand: add NVIDIA Tegra NAND flash support Stefan Agner
@ 2018-06-17 20:46 ` Stefan Agner
  2018-06-17 20:46 ` [PATCH v5 2/6] mtd: rawnand: add an option to specify NAND chip as a boot device Stefan Agner
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Stefan Agner @ 2018-06-17 20:46 UTC (permalink / raw)
  To: boris.brezillon, dwmw2, computersforpeace, marek.vasut, robh+dt,
	mark.rutland, thierry.reding
  Cc: dev, miquel.raynal, richard, marcel, krzk, digetx,
	benjamin.lindqvist, jonathanh, pdeschrijver, pgaikwad,
	mirza.krak, gaireg, linux-mtd, linux-tegra, devicetree,
	linux-kernel, Stefan Agner

Add Reed-Solomon (RS) to the enumeration of ECC algorithms.

Signed-off-by: Stefan Agner <stefan@agner.ch>
Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/mtd/nand.txt | 2 +-
 drivers/mtd/nand/raw/nand_base.c               | 1 +
 include/linux/mtd/rawnand.h                    | 1 +
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
index 8bb11d809429d..eaef8c657aa5c 100644
--- a/Documentation/devicetree/bindings/mtd/nand.txt
+++ b/Documentation/devicetree/bindings/mtd/nand.txt
@@ -25,7 +25,7 @@ Optional NAND chip properties:
 		  Deprecated values:
 		  "soft_bch": use "soft" and nand-ecc-algo instead
 - nand-ecc-algo: string, algorithm of NAND ECC.
-		 Supported values are: "hamming", "bch".
+		 Valid values are: "hamming", "bch", "rs".
 - nand-bus-width : 8 or 16 bus width if not present 8
 - nand-on-flash-bbt: boolean to enable on flash bbt option if not present false
 
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 10c4f9919850c..47e9cb9063da4 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -5777,6 +5777,7 @@ static int of_get_nand_ecc_mode(struct device_node *np)
 static const char * const nand_ecc_algos[] = {
 	[NAND_ECC_HAMMING]	= "hamming",
 	[NAND_ECC_BCH]		= "bch",
+	[NAND_ECC_RS]		= "rs",
 };
 
 static int of_get_nand_ecc_algo(struct device_node *np)
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 3e8ec3b8a39c7..2d9cb7acbc3d8 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -121,6 +121,7 @@ enum nand_ecc_algo {
 	NAND_ECC_UNKNOWN,
 	NAND_ECC_HAMMING,
 	NAND_ECC_BCH,
+	NAND_ECC_RS,
 };
 
 /*
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v5 2/6] mtd: rawnand: add an option to specify NAND chip as a boot device
  2018-06-17 20:45 [PATCH v5 0/6] mtd: rawnand: add NVIDIA Tegra NAND flash support Stefan Agner
  2018-06-17 20:46 ` [PATCH v5 1/6] mtd: rawnand: add Reed-Solomon error correction algorithm Stefan Agner
@ 2018-06-17 20:46 ` Stefan Agner
  2018-06-17 20:46 ` [PATCH v5 3/6] mtd: rawnand: tegra: add devicetree binding Stefan Agner
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Stefan Agner @ 2018-06-17 20:46 UTC (permalink / raw)
  To: boris.brezillon, dwmw2, computersforpeace, marek.vasut, robh+dt,
	mark.rutland, thierry.reding
  Cc: dev, miquel.raynal, richard, marcel, krzk, digetx,
	benjamin.lindqvist, jonathanh, pdeschrijver, pgaikwad,
	mirza.krak, gaireg, linux-mtd, linux-tegra, devicetree,
	linux-kernel, Stefan Agner

Allow to define a NAND chip as a boot device. This can be helpful
for the selection of the ECC algorithm and strength in case the boot
ROM supports only a subset of controller provided options.

Signed-off-by: Stefan Agner <stefan@agner.ch>
Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
 Documentation/devicetree/bindings/mtd/nand.txt | 4 ++++
 drivers/mtd/nand/raw/nand_base.c               | 3 +++
 include/linux/mtd/rawnand.h                    | 6 ++++++
 3 files changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
index eaef8c657aa5c..e949c778e9837 100644
--- a/Documentation/devicetree/bindings/mtd/nand.txt
+++ b/Documentation/devicetree/bindings/mtd/nand.txt
@@ -43,6 +43,10 @@ Optional NAND chip properties:
 		     This is particularly useful when only the in-band area is
 		     used by the upper layers, and you want to make your NAND
 		     as reliable as possible.
+- nand-is-boot-medium: Whether the NAND chip is a boot medium. Drivers might use
+		       this information to select ECC algorithms supported by
+		       the boot ROM or similar restrictions.
+
 - nand-rb: shall contain the native Ready/Busy ids.
 
 The ECC strength and ECC step size properties define the correction capability
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 47e9cb9063da4..e70bf328c64e4 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -5859,6 +5859,9 @@ static int nand_dt_init(struct nand_chip *chip)
 	if (of_get_nand_bus_width(dn) == 16)
 		chip->options |= NAND_BUSWIDTH_16;
 
+	if (of_property_read_bool(dn, "nand-is-boot-medium"))
+		chip->options |= NAND_IS_BOOT_MEDIUM;
+
 	if (of_get_nand_on_flash_bbt(dn))
 		chip->bbt_options |= NAND_BBT_USE_FLASH;
 
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 2d9cb7acbc3d8..80aeeca03f36b 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -219,6 +219,12 @@ enum nand_ecc_algo {
  */
 #define NAND_WAIT_TCCS		0x00200000
 
+/*
+ * Whether the NAND chip is a boot medium. Drivers might use this information
+ * to select ECC algorithms supported by the boot ROM or similar restrictions.
+ */
+#define NAND_IS_BOOT_MEDIUM	0x00400000
+
 /* Options set by nand scan */
 /* Nand scan has allocated controller struct */
 #define NAND_CONTROLLER_ALLOC	0x80000000
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v5 3/6] mtd: rawnand: tegra: add devicetree binding
  2018-06-17 20:45 [PATCH v5 0/6] mtd: rawnand: add NVIDIA Tegra NAND flash support Stefan Agner
  2018-06-17 20:46 ` [PATCH v5 1/6] mtd: rawnand: add Reed-Solomon error correction algorithm Stefan Agner
  2018-06-17 20:46 ` [PATCH v5 2/6] mtd: rawnand: add an option to specify NAND chip as a boot device Stefan Agner
@ 2018-06-17 20:46 ` Stefan Agner
  2018-06-17 20:46 ` [PATCH v5 4/6] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver Stefan Agner
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Stefan Agner @ 2018-06-17 20:46 UTC (permalink / raw)
  To: boris.brezillon, dwmw2, computersforpeace, marek.vasut, robh+dt,
	mark.rutland, thierry.reding
  Cc: dev, miquel.raynal, richard, marcel, krzk, digetx,
	benjamin.lindqvist, jonathanh, pdeschrijver, pgaikwad,
	mirza.krak, gaireg, linux-mtd, linux-tegra, devicetree,
	linux-kernel, Stefan Agner

This adds the devicetree binding for the Tegra 2 NAND flash
controller.

Signed-off-by: Lucas Stach <dev@lynxeye.de>
Signed-off-by: Stefan Agner <stefan@agner.ch>
Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../bindings/mtd/nvidia-tegra20-nand.txt      | 64 +++++++++++++++++++
 1 file changed, 64 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt

diff --git a/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
new file mode 100644
index 0000000000000..1c351362f3a96
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
@@ -0,0 +1,64 @@
+NVIDIA Tegra NAND Flash controller
+
+Required properties:
+- compatible: Must be one of:
+  - "nvidia,tegra20-nand"
+- reg: MMIO address range
+- interrupts: interrupt output of the NFC controller
+- clocks: Must contain an entry for each entry in clock-names.
+  See ../clocks/clock-bindings.txt for details.
+- clock-names: Must include the following entries:
+  - nand
+- resets: Must contain an entry for each entry in reset-names.
+  See ../reset/reset.txt for details.
+- reset-names: Must include the following entries:
+  - nand
+
+Optional children nodes:
+Individual NAND chips are children of the NAND controller node. Currently
+only one NAND chip supported.
+
+Required children node properties:
+- reg: An integer ranging from 1 to 6 representing the CS line to use.
+
+Optional children node properties:
+- nand-ecc-mode: String, operation mode of the NAND ecc mode. Currently only
+		 "hw" is supported.
+- nand-ecc-algo: string, algorithm of NAND ECC.
+		 Supported values with "hw" ECC mode are: "rs", "bch".
+- nand-bus-width : See nand.txt
+- nand-on-flash-bbt: See nand.txt
+- nand-ecc-strength: integer representing the number of bits to correct
+		     per ECC step (always 512). Supported strength using HW ECC
+		     modes are:
+		     - RS: 4, 6, 8
+		     - BCH: 4, 8, 14, 16
+- nand-ecc-maximize: See nand.txt
+- nand-is-boot-medium: Makes sure only ECC strengths supported by the boot ROM
+		       are choosen.
+- wp-gpios: GPIO specifier for the write protect pin.
+
+Optional child node of NAND chip nodes:
+Partitions: see partition.txt
+
+  Example:
+	nand-controller@70008000 {
+		compatible = "nvidia,tegra20-nand";
+		reg = <0x70008000 0x100>;
+		interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&tegra_car TEGRA20_CLK_NDFLASH>;
+		clock-names = "nand";
+		resets = <&tegra_car 13>;
+		reset-names = "nand";
+
+		nand@0 {
+			reg = <0>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			nand-bus-width = <8>;
+			nand-on-flash-bbt;
+			nand-ecc-algo = "bch";
+			nand-ecc-strength = <8>;
+			wp-gpios = <&gpio TEGRA_GPIO(S, 0) GPIO_ACTIVE_LOW>;
+		};
+	};
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v5 4/6] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver
  2018-06-17 20:45 [PATCH v5 0/6] mtd: rawnand: add NVIDIA Tegra NAND flash support Stefan Agner
                   ` (2 preceding siblings ...)
  2018-06-17 20:46 ` [PATCH v5 3/6] mtd: rawnand: tegra: add devicetree binding Stefan Agner
@ 2018-06-17 20:46 ` Stefan Agner
  2018-06-17 20:46 ` [PATCH v5 5/6] ARM: dts: tegra: add Tegra20 NAND flash controller node Stefan Agner
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Stefan Agner @ 2018-06-17 20:46 UTC (permalink / raw)
  To: boris.brezillon, dwmw2, computersforpeace, marek.vasut, robh+dt,
	mark.rutland, thierry.reding
  Cc: dev, miquel.raynal, richard, marcel, krzk, digetx,
	benjamin.lindqvist, jonathanh, pdeschrijver, pgaikwad,
	mirza.krak, gaireg, linux-mtd, linux-tegra, devicetree,
	linux-kernel, Stefan Agner

Add support for the NAND flash controller found on NVIDIA
Tegra 2 SoCs. This implementation does not make use of the
command queue feature. Regular operations using ->exec_op()
use PIO mode for data transfers. Raw, ECC and OOB read/writes
make use of the DMA mode for data transfer.

Signed-off-by: Lucas Stach <dev@lynxeye.de>
Signed-off-by: Stefan Agner <stefan@agner.ch>
Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
---
 MAINTAINERS                       |    7 +
 drivers/mtd/nand/raw/Kconfig      |    6 +
 drivers/mtd/nand/raw/Makefile     |    1 +
 drivers/mtd/nand/raw/tegra_nand.c | 1268 +++++++++++++++++++++++++++++
 4 files changed, 1282 insertions(+)
 create mode 100644 drivers/mtd/nand/raw/tegra_nand.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 9d5eeff51b5fd..d62bf7dc714d2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14040,6 +14040,13 @@ M:	Laxman Dewangan <ldewangan@nvidia.com>
 S:	Supported
 F:	drivers/input/keyboard/tegra-kbc.c
 
+TEGRA NAND DRIVER
+M:	Stefan Agner <stefan@agner.ch>
+M:	Lucas Stach <dev@lynxeye.de>
+S:	Maintained
+F:	Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
+F:	drivers/mtd/nand/raw/tegra_nand.c
+
 TEGRA PWM DRIVER
 M:	Thierry Reding <thierry.reding@gmail.com>
 S:	Supported
diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
index 6871ff0fd300b..472a870cfe4b0 100644
--- a/drivers/mtd/nand/raw/Kconfig
+++ b/drivers/mtd/nand/raw/Kconfig
@@ -530,4 +530,10 @@ config MTD_NAND_MTK
 	  Enables support for NAND controller on MTK SoCs.
 	  This controller is found on mt27xx, mt81xx, mt65xx SoCs.
 
+config MTD_NAND_TEGRA
+	tristate "Support for NAND controller on NVIDIA Tegra"
+	depends on ARCH_TEGRA || COMPILE_TEST
+	help
+	  Enables support for NAND flash controller on NVIDIA Tegra SoC.
+
 endif # MTD_NAND
diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile
index 165b7ef9e9a18..d5a5f9832b887 100644
--- a/drivers/mtd/nand/raw/Makefile
+++ b/drivers/mtd/nand/raw/Makefile
@@ -56,6 +56,7 @@ obj-$(CONFIG_MTD_NAND_HISI504)	        += hisi504_nand.o
 obj-$(CONFIG_MTD_NAND_BRCMNAND)		+= brcmnand/
 obj-$(CONFIG_MTD_NAND_QCOM)		+= qcom_nandc.o
 obj-$(CONFIG_MTD_NAND_MTK)		+= mtk_ecc.o mtk_nand.o
+obj-$(CONFIG_MTD_NAND_TEGRA)		+= tegra_nand.o
 
 nand-objs := nand_base.o nand_bbt.o nand_timings.o nand_ids.o
 nand-objs += nand_amd.o
diff --git a/drivers/mtd/nand/raw/tegra_nand.c b/drivers/mtd/nand/raw/tegra_nand.c
new file mode 100644
index 0000000000000..8d8c79c52499a
--- /dev/null
+++ b/drivers/mtd/nand/raw/tegra_nand.c
@@ -0,0 +1,1268 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Stefan Agner <stefan@agner.ch>
+ * Copyright (C) 2014-2015 Lucas Stach <dev@lynxeye.de>
+ * Copyright (C) 2012 Avionic Design GmbH
+ */
+
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/dma-mapping.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/mtd/partitions.h>
+#include <linux/mtd/rawnand.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+
+#define COMMAND					0x00
+#define   COMMAND_GO				BIT(31)
+#define   COMMAND_CLE				BIT(30)
+#define   COMMAND_ALE				BIT(29)
+#define   COMMAND_PIO				BIT(28)
+#define   COMMAND_TX				BIT(27)
+#define   COMMAND_RX				BIT(26)
+#define   COMMAND_SEC_CMD			BIT(25)
+#define   COMMAND_AFT_DAT			BIT(24)
+#define   COMMAND_TRANS_SIZE(x)			(((x - 1) & 0xf) << 20)
+#define   COMMAND_A_VALID			BIT(19)
+#define   COMMAND_B_VALID			BIT(18)
+#define   COMMAND_RD_STATUS_CHK			BIT(17)
+#define   COMMAND_RBSY_CHK			BIT(16)
+#define   COMMAND_CE(x)				BIT(8 + ((x) & 0x7))
+#define   COMMAND_CLE_SIZE(x)			(((x - 1) & 0x3) << 4)
+#define   COMMAND_ALE_SIZE(x)			(((x - 1) & 0xf) << 0)
+
+#define STATUS					0x04
+
+#define ISR					0x08
+#define   ISR_CORRFAIL_ERR			BIT(24)
+#define   ISR_UND				BIT(7)
+#define   ISR_OVR				BIT(6)
+#define   ISR_CMD_DONE				BIT(5)
+#define   ISR_ECC_ERR				BIT(4)
+
+#define IER					0x0c
+#define   IER_ERR_TRIG_VAL(x)			(((x) & 0xf) << 16)
+#define   IER_UND				BIT(7)
+#define   IER_OVR				BIT(6)
+#define   IER_CMD_DONE				BIT(5)
+#define   IER_ECC_ERR				BIT(4)
+#define   IER_GIE				BIT(0)
+
+#define CONFIG					0x10
+#define   CONFIG_HW_ECC				BIT(31)
+#define   CONFIG_ECC_SEL			BIT(30)
+#define   CONFIG_ERR_COR			BIT(29)
+#define   CONFIG_PIPE_EN			BIT(28)
+#define   CONFIG_TVAL_4				(0 << 24)
+#define   CONFIG_TVAL_6				(1 << 24)
+#define   CONFIG_TVAL_8				(2 << 24)
+#define   CONFIG_SKIP_SPARE			BIT(23)
+#define   CONFIG_BUS_WIDTH_16			BIT(21)
+#define   CONFIG_COM_BSY			BIT(20)
+#define   CONFIG_PS_256				(0 << 16)
+#define   CONFIG_PS_512				(1 << 16)
+#define   CONFIG_PS_1024			(2 << 16)
+#define   CONFIG_PS_2048			(3 << 16)
+#define   CONFIG_PS_4096			(4 << 16)
+#define   CONFIG_SKIP_SPARE_SIZE_4		(0 << 14)
+#define   CONFIG_SKIP_SPARE_SIZE_8		(1 << 14)
+#define   CONFIG_SKIP_SPARE_SIZE_12		(2 << 14)
+#define   CONFIG_SKIP_SPARE_SIZE_16		(3 << 14)
+#define   CONFIG_TAG_BYTE_SIZE(x)			((x) & 0xff)
+
+#define TIMING_1				0x14
+#define   TIMING_TRP_RESP(x)			(((x) & 0xf) << 28)
+#define   TIMING_TWB(x)				(((x) & 0xf) << 24)
+#define   TIMING_TCR_TAR_TRR(x)			(((x) & 0xf) << 20)
+#define   TIMING_TWHR(x)			(((x) & 0xf) << 16)
+#define   TIMING_TCS(x)				(((x) & 0x3) << 14)
+#define   TIMING_TWH(x)				(((x) & 0x3) << 12)
+#define   TIMING_TWP(x)				(((x) & 0xf) <<  8)
+#define   TIMING_TRH(x)				(((x) & 0x3) <<  4)
+#define   TIMING_TRP(x)				(((x) & 0xf) <<  0)
+
+#define RESP					0x18
+
+#define TIMING_2				0x1c
+#define   TIMING_TADL(x)			((x) & 0xf)
+
+#define CMD_REG1				0x20
+#define CMD_REG2				0x24
+#define ADDR_REG1				0x28
+#define ADDR_REG2				0x2c
+
+#define DMA_MST_CTRL				0x30
+#define   DMA_MST_CTRL_GO			BIT(31)
+#define   DMA_MST_CTRL_IN			(0 << 30)
+#define   DMA_MST_CTRL_OUT			BIT(30)
+#define   DMA_MST_CTRL_PERF_EN			BIT(29)
+#define   DMA_MST_CTRL_IE_DONE			BIT(28)
+#define   DMA_MST_CTRL_REUSE			BIT(27)
+#define   DMA_MST_CTRL_BURST_1			(2 << 24)
+#define   DMA_MST_CTRL_BURST_4			(3 << 24)
+#define   DMA_MST_CTRL_BURST_8			(4 << 24)
+#define   DMA_MST_CTRL_BURST_16			(5 << 24)
+#define   DMA_MST_CTRL_IS_DONE			BIT(20)
+#define   DMA_MST_CTRL_EN_A			BIT(2)
+#define   DMA_MST_CTRL_EN_B			BIT(1)
+
+#define DMA_CFG_A				0x34
+#define DMA_CFG_B				0x38
+
+#define FIFO_CTRL				0x3c
+#define   FIFO_CTRL_CLR_ALL			BIT(3)
+
+#define DATA_PTR				0x40
+#define TAG_PTR					0x44
+#define ECC_PTR					0x48
+
+#define DEC_STATUS				0x4c
+#define   DEC_STATUS_A_ECC_FAIL			BIT(1)
+#define   DEC_STATUS_ERR_COUNT_MASK		0x00ff0000
+#define   DEC_STATUS_ERR_COUNT_SHIFT		16
+
+#define HWSTATUS_CMD				0x50
+#define HWSTATUS_MASK				0x54
+#define   HWSTATUS_RDSTATUS_MASK(x)		(((x) & 0xff) << 24)
+#define   HWSTATUS_RDSTATUS_VALUE(x)		(((x) & 0xff) << 16)
+#define   HWSTATUS_RBSY_MASK(x)			(((x) & 0xff) << 8)
+#define   HWSTATUS_RBSY_VALUE(x)		(((x) & 0xff) << 0)
+
+#define BCH_CONFIG				0xcc
+#define   BCH_ENABLE				BIT(0)
+#define   BCH_TVAL_4				(0 << 4)
+#define   BCH_TVAL_8				(1 << 4)
+#define   BCH_TVAL_14				(2 << 4)
+#define   BCH_TVAL_16				(3 << 4)
+
+#define DEC_STAT_RESULT				0xd0
+#define DEC_STAT_BUF				0xd4
+#define   DEC_STAT_BUF_FAIL_SEC_FLAG_MASK	0xff000000
+#define   DEC_STAT_BUF_FAIL_SEC_FLAG_SHIFT	24
+#define   DEC_STAT_BUF_CORR_SEC_FLAG_MASK	0x00ff0000
+#define   DEC_STAT_BUF_CORR_SEC_FLAG_SHIFT	16
+#define   DEC_STAT_BUF_MAX_CORR_CNT_MASK	0x00001f00
+#define   DEC_STAT_BUF_MAX_CORR_CNT_SHIFT	8
+
+#define OFFSET(val, off)	((val) < (off) ? 0 : (val) - (off))
+
+#define SKIP_SPARE_BYTES	4
+#define BITS_PER_STEP_RS	18
+#define BITS_PER_STEP_BCH	13
+
+#define INT_MASK		(IER_UND | IER_OVR | IER_CMD_DONE | IER_GIE)
+#define HWSTATUS_CMD_DEFAULT	NAND_STATUS_READY
+#define HWSTATUS_MASK_DEFAULT	(HWSTATUS_RDSTATUS_MASK(1) | \
+				HWSTATUS_RDSTATUS_VALUE(0) | \
+				HWSTATUS_RBSY_MASK(NAND_STATUS_READY) | \
+				HWSTATUS_RBSY_VALUE(NAND_STATUS_READY))
+
+struct tegra_nand_controller {
+	struct nand_hw_control controller;
+	struct device *dev;
+	void __iomem *regs;
+	int irq;
+	struct clk *clk;
+	struct completion command_complete;
+	struct completion dma_complete;
+	bool last_read_error;
+	int cur_cs;
+	struct nand_chip *chip;
+};
+
+struct tegra_nand_chip {
+	struct nand_chip chip;
+	struct gpio_desc *wp_gpio;
+	struct mtd_oob_region tag;
+	struct mtd_oob_region ecc;
+	u32 config;
+	u32 config_ecc;
+	u32 bch_config;
+	int cs[1];
+};
+
+static inline struct tegra_nand_controller *to_tegra_ctrl(
+						struct nand_hw_control *hw_ctrl)
+{
+	return container_of(hw_ctrl, struct tegra_nand_controller, controller);
+}
+
+static inline struct tegra_nand_chip *to_tegra_chip(struct nand_chip *chip)
+{
+	return container_of(chip, struct tegra_nand_chip, chip);
+}
+
+static int tegra_nand_ooblayout_rs_ecc(struct mtd_info *mtd, int section,
+				       struct mtd_oob_region *oobregion)
+{
+	struct nand_chip *chip = mtd_to_nand(mtd);
+	int bytes_per_step = DIV_ROUND_UP(BITS_PER_STEP_RS * chip->ecc.strength,
+					  BITS_PER_BYTE);
+
+	if (section > 0)
+		return -ERANGE;
+
+	oobregion->offset = SKIP_SPARE_BYTES;
+	oobregion->length = round_up(bytes_per_step * chip->ecc.steps, 4);
+
+	return 0;
+}
+
+static int tegra_nand_ooblayout_rs_free(struct mtd_info *mtd, int section,
+					struct mtd_oob_region *oobregion)
+{
+	struct nand_chip *chip = mtd_to_nand(mtd);
+	int bytes_per_step = DIV_ROUND_UP(BITS_PER_STEP_RS * chip->ecc.strength,
+					  BITS_PER_BYTE);
+
+	if (section > 0)
+		return -ERANGE;
+
+	oobregion->offset = SKIP_SPARE_BYTES +
+			    round_up(bytes_per_step * chip->ecc.steps, 4);
+	oobregion->length = mtd->oobsize - oobregion->offset;
+
+	return 0;
+}
+
+static const struct mtd_ooblayout_ops tegra_nand_oob_rs_ops = {
+	.ecc = tegra_nand_ooblayout_rs_ecc,
+	.free = tegra_nand_ooblayout_rs_free,
+};
+
+static int tegra_nand_ooblayout_bch_ecc(struct mtd_info *mtd, int section,
+				       struct mtd_oob_region *oobregion)
+{
+	struct nand_chip *chip = mtd_to_nand(mtd);
+	int bytes_per_step = DIV_ROUND_UP(BITS_PER_STEP_BCH * chip->ecc.strength,
+					  BITS_PER_BYTE);
+
+	if (section > 0)
+		return -ERANGE;
+
+	oobregion->offset = SKIP_SPARE_BYTES;
+	oobregion->length = round_up(bytes_per_step * chip->ecc.steps, 4);
+
+	return 0;
+}
+
+static int tegra_nand_ooblayout_bch_free(struct mtd_info *mtd, int section,
+					struct mtd_oob_region *oobregion)
+{
+	struct nand_chip *chip = mtd_to_nand(mtd);
+	int bytes_per_step = DIV_ROUND_UP(BITS_PER_STEP_BCH * chip->ecc.strength,
+					  BITS_PER_BYTE);
+
+	if (section > 0)
+		return -ERANGE;
+
+	oobregion->offset = SKIP_SPARE_BYTES +
+			    round_up(bytes_per_step * chip->ecc.steps, 4);
+	oobregion->length = mtd->oobsize - oobregion->offset;
+
+	return 0;
+}
+
+/*
+ * Layout with tag bytes is
+ *
+ * --------------------------------------------------------------------------
+ * | main area                       | skip bytes | tag bytes | parity | .. |
+ * --------------------------------------------------------------------------
+ *
+ * If not tag bytes are written, parity moves right after skip bytes!
+ */
+static const struct mtd_ooblayout_ops tegra_nand_oob_bch_ops = {
+	.ecc = tegra_nand_ooblayout_bch_ecc,
+	.free = tegra_nand_ooblayout_bch_free,
+};
+
+static irqreturn_t tegra_nand_irq(int irq, void *data)
+{
+	struct tegra_nand_controller *ctrl = data;
+	u32 isr, dma;
+
+	isr = readl_relaxed(ctrl->regs + ISR);
+	dma = readl_relaxed(ctrl->regs + DMA_MST_CTRL);
+	dev_dbg(ctrl->dev, "isr %08x\n", isr);
+
+	if (!isr && !(dma & DMA_MST_CTRL_IS_DONE))
+		return IRQ_NONE;
+
+	/*
+	 * The bit name is somewhat missleading: This is also set when
+	 * HW ECC was successful. The data sheet states:
+	 * Correctable OR Un-correctable errors occurred in the DMA transfer...
+	 */
+	if (isr & ISR_CORRFAIL_ERR)
+		ctrl->last_read_error = true;
+
+	if (isr & ISR_CMD_DONE)
+		complete(&ctrl->command_complete);
+
+	if (isr & ISR_UND)
+		dev_err(ctrl->dev, "FIFO underrun\n");
+
+	if (isr & ISR_OVR)
+		dev_err(ctrl->dev, "FIFO overrun\n");
+
+	/* handle DMA interrupts */
+	if (dma & DMA_MST_CTRL_IS_DONE) {
+		writel_relaxed(dma, ctrl->regs + DMA_MST_CTRL);
+		complete(&ctrl->dma_complete);
+	}
+
+	/* clear interrupts */
+	writel_relaxed(isr, ctrl->regs + ISR);
+
+	return IRQ_HANDLED;
+}
+
+static const char * const tegra_nand_reg_names[] = {
+	"COMMAND",
+	"STATUS",
+	"ISR",
+	"IER",
+	"CONFIG",
+	"TIMING",
+	NULL,
+	"TIMING2",
+	"CMD_REG1",
+	"CMD_REG2",
+	"ADDR_REG1",
+	"ADDR_REG2",
+	"DMA_MST_CTRL",
+	"DMA_CFG_A",
+	"DMA_CFG_B",
+	"FIFO_CTRL",
+};
+
+static void tegra_nand_dump_reg(struct tegra_nand_controller *ctrl)
+{
+	u32 reg;
+	int i;
+
+	dev_err(ctrl->dev, "Tegra NAND controller register dump\n");
+	for (i = 0; i < ARRAY_SIZE(tegra_nand_reg_names); i++) {
+		const char *reg_name = tegra_nand_reg_names[i];
+
+		if (!reg_name)
+			continue;
+
+		reg = readl_relaxed(ctrl->regs + (i * 4));
+		dev_err(ctrl->dev, "%s: 0x%08x\n", reg_name, reg);
+	}
+}
+
+static void tegra_nand_controller_abort(struct tegra_nand_controller *ctrl)
+{
+	u32 isr, dma;
+
+	disable_irq(ctrl->irq);
+
+	/* Abort current command/DMA operation */
+	writel_relaxed(0, ctrl->regs + DMA_MST_CTRL);
+	writel_relaxed(0, ctrl->regs + COMMAND);
+
+	/* clear interrupts */
+	isr = readl_relaxed(ctrl->regs + ISR);
+	writel_relaxed(isr, ctrl->regs + ISR);
+	dma = readl_relaxed(ctrl->regs + DMA_MST_CTRL);
+	writel_relaxed(dma, ctrl->regs + DMA_MST_CTRL);
+
+	reinit_completion(&ctrl->command_complete);
+	reinit_completion(&ctrl->dma_complete);
+
+	enable_irq(ctrl->irq);
+}
+
+static int tegra_nand_cmd(struct nand_chip *chip,
+			 const struct nand_subop *subop)
+{
+	const struct nand_op_instr *instr;
+	const struct nand_op_instr *instr_data_in = NULL;
+	struct tegra_nand_controller *ctrl = to_tegra_ctrl(chip->controller);
+	unsigned int op_id, size = 0, offset = 0;
+	bool first_cmd = true;
+	u32 reg, cmd = 0;
+	int ret;
+
+	for (op_id = 0; op_id < subop->ninstrs; op_id++) {
+		unsigned int naddrs, i;
+		const u8 *addrs;
+		u32 addr1 = 0, addr2 = 0;
+
+		instr = &subop->instrs[op_id];
+
+		switch (instr->type) {
+		case NAND_OP_CMD_INSTR:
+			if (first_cmd) {
+				cmd |= COMMAND_CLE;
+				writel_relaxed(instr->ctx.cmd.opcode,
+					       ctrl->regs + CMD_REG1);
+			} else {
+				cmd |= COMMAND_SEC_CMD;
+				writel_relaxed(instr->ctx.cmd.opcode,
+					       ctrl->regs + CMD_REG2);
+			}
+			first_cmd = false;
+			break;
+
+		case NAND_OP_ADDR_INSTR:
+			offset = nand_subop_get_addr_start_off(subop, op_id);
+			naddrs = nand_subop_get_num_addr_cyc(subop, op_id);
+			addrs = &instr->ctx.addr.addrs[offset];
+
+			cmd |= COMMAND_ALE | COMMAND_ALE_SIZE(naddrs);
+			for (i = 0; i < min_t(unsigned int, 4, naddrs); i++)
+				addr1 |= *addrs++ << (BITS_PER_BYTE * i);
+			naddrs -= i;
+			for (i = 0; i < min_t(unsigned int, 4, naddrs); i++)
+				addr2 |= *addrs++ << (BITS_PER_BYTE * i);
+
+			writel_relaxed(addr1, ctrl->regs + ADDR_REG1);
+			writel_relaxed(addr2, ctrl->regs + ADDR_REG2);
+			break;
+
+		case NAND_OP_DATA_IN_INSTR:
+			size = nand_subop_get_data_len(subop, op_id);
+			offset = nand_subop_get_data_start_off(subop, op_id);
+
+			cmd |= COMMAND_TRANS_SIZE(size) | COMMAND_PIO |
+				COMMAND_RX | COMMAND_A_VALID;
+
+			instr_data_in = instr;
+			break;
+
+		case NAND_OP_DATA_OUT_INSTR:
+			size = nand_subop_get_data_len(subop, op_id);
+			offset = nand_subop_get_data_start_off(subop, op_id);
+
+			cmd |= COMMAND_TRANS_SIZE(size) | COMMAND_PIO |
+				COMMAND_TX | COMMAND_A_VALID;
+			memcpy(&reg, instr->ctx.data.buf.out + offset, size);
+
+			writel_relaxed(reg, ctrl->regs + RESP);
+			break;
+
+		case NAND_OP_WAITRDY_INSTR:
+			cmd |= COMMAND_RBSY_CHK;
+			break;
+		}
+	}
+
+	cmd |= COMMAND_GO | COMMAND_CE(ctrl->cur_cs);
+	writel_relaxed(cmd, ctrl->regs + COMMAND);
+	ret = wait_for_completion_timeout(&ctrl->command_complete,
+					     msecs_to_jiffies(500));
+	if (!ret) {
+		dev_err(ctrl->dev, "COMMAND timeout\n");
+		tegra_nand_dump_reg(ctrl);
+		tegra_nand_controller_abort(ctrl);
+		return -ETIMEDOUT;
+	}
+
+	if (instr_data_in) {
+		reg = readl_relaxed(ctrl->regs + RESP);
+		memcpy(instr_data_in->ctx.data.buf.in + offset, &reg, size);
+	}
+
+	return 0;
+}
+
+static const struct nand_op_parser tegra_nand_op_parser = NAND_OP_PARSER(
+	NAND_OP_PARSER_PATTERN(tegra_nand_cmd,
+		NAND_OP_PARSER_PAT_CMD_ELEM(true),
+		NAND_OP_PARSER_PAT_ADDR_ELEM(true, 8),
+		NAND_OP_PARSER_PAT_CMD_ELEM(true),
+		NAND_OP_PARSER_PAT_WAITRDY_ELEM(true)),
+	NAND_OP_PARSER_PATTERN(tegra_nand_cmd,
+		NAND_OP_PARSER_PAT_DATA_OUT_ELEM(false, 4)),
+	NAND_OP_PARSER_PATTERN(tegra_nand_cmd,
+		NAND_OP_PARSER_PAT_CMD_ELEM(true),
+		NAND_OP_PARSER_PAT_ADDR_ELEM(true, 8),
+		NAND_OP_PARSER_PAT_CMD_ELEM(true),
+		NAND_OP_PARSER_PAT_WAITRDY_ELEM(true),
+		NAND_OP_PARSER_PAT_DATA_IN_ELEM(true, 4)),
+	);
+
+static int tegra_nand_exec_op(struct nand_chip *chip,
+			     const struct nand_operation *op,
+			     bool check_only)
+{
+	return nand_op_parser_exec_op(chip, &tegra_nand_op_parser, op,
+				      check_only);
+}
+
+static void tegra_nand_select_chip(struct mtd_info *mtd, int die_nr)
+{
+	struct nand_chip *chip = mtd_to_nand(mtd);
+	struct tegra_nand_chip *nand = to_tegra_chip(chip);
+	struct tegra_nand_controller *ctrl = to_tegra_ctrl(chip->controller);
+
+	if (die_nr < 0 || die_nr > 1) {
+		ctrl->cur_cs = -1;
+		return;
+	}
+
+	ctrl->cur_cs = nand->cs[die_nr];
+}
+
+static void tegra_nand_hw_ecc(struct tegra_nand_controller *ctrl,
+			      struct nand_chip *chip, bool enable)
+{
+	struct tegra_nand_chip *nand = to_tegra_chip(chip);
+
+	if (chip->ecc.algo == NAND_ECC_BCH && enable)
+		writel_relaxed(nand->bch_config, ctrl->regs + BCH_CONFIG);
+	else
+		writel_relaxed(0, ctrl->regs + BCH_CONFIG);
+
+	if (enable)
+		writel_relaxed(nand->config_ecc, ctrl->regs + CONFIG);
+	else
+		writel_relaxed(nand->config, ctrl->regs + CONFIG);
+}
+
+static int tegra_nand_page_xfer(struct mtd_info *mtd, struct nand_chip *chip,
+				void *buf, void *oob_buf, int oob_len, int page,
+				bool read)
+{
+	struct tegra_nand_controller *ctrl = to_tegra_ctrl(chip->controller);
+	enum dma_data_direction dir = read ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+	dma_addr_t dma_addr = 0, dma_addr_oob = 0;
+	u32 addr1, cmd, dma_ctrl;
+	int ret;
+
+	if (read) {
+		writel_relaxed(NAND_CMD_READ0, ctrl->regs + CMD_REG1);
+		writel_relaxed(NAND_CMD_READSTART, ctrl->regs + CMD_REG2);
+	} else {
+		writel_relaxed(NAND_CMD_SEQIN, ctrl->regs + CMD_REG1);
+		writel_relaxed(NAND_CMD_PAGEPROG, ctrl->regs + CMD_REG2);
+	}
+	cmd = COMMAND_CLE | COMMAND_SEC_CMD;
+
+	/* Lower 16-bits are column, by default 0 */
+	addr1 = page << 16;
+
+	if (!buf)
+		addr1 |= mtd->writesize;
+	writel_relaxed(addr1, ctrl->regs + ADDR_REG1);
+
+	if (chip->options & NAND_ROW_ADDR_3) {
+		writel_relaxed(page >> 16, ctrl->regs + ADDR_REG2);
+		cmd |= COMMAND_ALE | COMMAND_ALE_SIZE(5);
+	} else {
+		cmd |= COMMAND_ALE | COMMAND_ALE_SIZE(4);
+	}
+
+	if (buf) {
+		dma_addr = dma_map_single(ctrl->dev, buf, mtd->writesize, dir);
+		ret = dma_mapping_error(ctrl->dev, dma_addr);
+		if (ret) {
+			dev_err(ctrl->dev, "dma mapping error\n");
+			return -EINVAL;
+		}
+
+		writel_relaxed(mtd->writesize - 1, ctrl->regs + DMA_CFG_A);
+		writel_relaxed(dma_addr, ctrl->regs + DATA_PTR);
+	}
+
+	if (oob_buf) {
+		dma_addr_oob = dma_map_single(ctrl->dev, oob_buf, mtd->oobsize,
+					      dir);
+		ret = dma_mapping_error(ctrl->dev, dma_addr_oob);
+		if (ret) {
+			dev_err(ctrl->dev, "dma mapping error\n");
+			ret = -EINVAL;
+			goto err_unmap_dma_page;
+		}
+
+		writel_relaxed(oob_len - 1, ctrl->regs + DMA_CFG_B);
+		writel_relaxed(dma_addr_oob, ctrl->regs + TAG_PTR);
+	}
+
+	dma_ctrl = DMA_MST_CTRL_GO | DMA_MST_CTRL_PERF_EN |
+		   DMA_MST_CTRL_IE_DONE | DMA_MST_CTRL_IS_DONE |
+		   DMA_MST_CTRL_BURST_16;
+
+	if (buf)
+		dma_ctrl |= DMA_MST_CTRL_EN_A;
+	if (oob_buf)
+		dma_ctrl |= DMA_MST_CTRL_EN_B;
+
+	if (read)
+		dma_ctrl |= DMA_MST_CTRL_IN | DMA_MST_CTRL_REUSE;
+	else
+		dma_ctrl |= DMA_MST_CTRL_OUT;
+
+	writel_relaxed(dma_ctrl, ctrl->regs + DMA_MST_CTRL);
+
+	cmd |= COMMAND_GO | COMMAND_RBSY_CHK | COMMAND_TRANS_SIZE(9) |
+	       COMMAND_CE(ctrl->cur_cs);
+
+	if (buf)
+		cmd |= COMMAND_A_VALID;
+	if (oob_buf)
+		cmd |= COMMAND_B_VALID;
+
+	if (read)
+		cmd |= COMMAND_RX;
+	else
+		cmd |= COMMAND_TX | COMMAND_AFT_DAT;
+
+	writel_relaxed(cmd, ctrl->regs + COMMAND);
+
+	ret = wait_for_completion_timeout(&ctrl->command_complete,
+					     msecs_to_jiffies(500));
+	if (!ret) {
+		dev_err(ctrl->dev, "COMMAND timeout\n");
+		tegra_nand_dump_reg(ctrl);
+		tegra_nand_controller_abort(ctrl);
+		ret = -ETIMEDOUT;
+		goto err_unmap_dma;
+	}
+
+	ret = wait_for_completion_timeout(&ctrl->dma_complete,
+					     msecs_to_jiffies(500));
+	if (!ret) {
+		dev_err(ctrl->dev, "DMA timeout\n");
+		tegra_nand_dump_reg(ctrl);
+		tegra_nand_controller_abort(ctrl);
+		ret = -ETIMEDOUT;
+		goto err_unmap_dma;
+	}
+	ret = 0;
+
+err_unmap_dma:
+	if (oob_buf)
+		dma_unmap_single(ctrl->dev, dma_addr_oob, mtd->oobsize, dir);
+err_unmap_dma_page:
+	if (buf)
+		dma_unmap_single(ctrl->dev, dma_addr, mtd->writesize, dir);
+
+	return ret;
+}
+
+static int tegra_nand_read_page_raw(struct mtd_info *mtd,
+				    struct nand_chip *chip,
+				    uint8_t *buf, int oob_required, int page)
+{
+	void *oob_buf = oob_required ? chip->oob_poi : NULL;
+
+	return tegra_nand_page_xfer(mtd, chip, buf, oob_buf,
+				    mtd->oobsize, page, true);
+}
+
+static int tegra_nand_write_page_raw(struct mtd_info *mtd,
+				     struct nand_chip *chip,
+				     const uint8_t *buf, int oob_required,
+				     int page)
+{
+	void *oob_buf = oob_required ? chip->oob_poi : NULL;
+
+	return tegra_nand_page_xfer(mtd, chip, (void *)buf, oob_buf,
+				     mtd->oobsize, page, false);
+}
+
+static int tegra_nand_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
+			       int page)
+{
+	return tegra_nand_page_xfer(mtd, chip, NULL, chip->oob_poi,
+				    mtd->oobsize, page, true);
+}
+
+static int tegra_nand_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
+				int page)
+{
+	return tegra_nand_page_xfer(mtd, chip, NULL, chip->oob_poi,
+				    mtd->oobsize, page, false);
+}
+
+static int tegra_nand_read_page_hwecc(struct mtd_info *mtd,
+				      struct nand_chip *chip,
+				      uint8_t *buf, int oob_required, int page)
+{
+	struct tegra_nand_controller *ctrl = to_tegra_ctrl(chip->controller);
+	struct tegra_nand_chip *nand = to_tegra_chip(chip);
+	void *oob_buf = oob_required ? chip->oob_poi : NULL;
+	u32 dec_stat, max_corr_cnt;
+	unsigned long fail_sec_flag;
+	int ret;
+
+	tegra_nand_hw_ecc(ctrl, chip, true);
+	ret = tegra_nand_page_xfer(mtd, chip, buf, oob_buf, nand->tag.length,
+				   page, true);
+	tegra_nand_hw_ecc(ctrl, chip, false);
+	if (ret)
+		return ret;
+
+	/* No correctable or un-correctable errors, page must have 0 bitflips */
+	if (!ctrl->last_read_error)
+		return 0;
+
+	/*
+	 * Correctable or un-correctable errors occurred. Use DEC_STAT_BUF
+	 * which contains information for all ECC selections.
+	 *
+	 * Note that since we do not use Command Queues DEC_RESULT does not
+	 * state the number of pages we can read from the DEC_STAT_BUF. But
+	 * since CORRFAIL_ERR did occur during page read we do have a valid
+	 * result in DEC_STAT_BUF.
+	 */
+	ctrl->last_read_error = false;
+	dec_stat = readl_relaxed(ctrl->regs + DEC_STAT_BUF);
+
+	fail_sec_flag = (dec_stat & DEC_STAT_BUF_FAIL_SEC_FLAG_MASK) >>
+			DEC_STAT_BUF_FAIL_SEC_FLAG_SHIFT;
+
+	max_corr_cnt = (dec_stat & DEC_STAT_BUF_MAX_CORR_CNT_MASK) >>
+		       DEC_STAT_BUF_MAX_CORR_CNT_SHIFT;
+
+	if (fail_sec_flag) {
+		int bit, max_bitflips = 0;
+
+		/*
+		 * Since we do not support subpage writes, a complete page
+		 * is either written or not. We can take a shortcut here by
+		 * checking wheather any of the sector has been successful
+		 * read. If at least one sectors has been read successfully,
+		 * the page must have been a written previously. It cannot
+		 * be an erased page.
+		 *
+		 * E.g. controller might return fail_sec_flag with 0x4, which
+		 * would mean only the third sector failed to correct. The
+		 * page must have been written and the third sector is really
+		 * not correctable anymore.
+		 */
+		if (fail_sec_flag ^ GENMASK(chip->ecc.steps - 1, 0)) {
+			mtd->ecc_stats.failed += hweight8(fail_sec_flag);
+			return max_corr_cnt;
+		}
+
+		/*
+		 * All sectors failed to correct, but the ECC isn't smart
+		 * enough to figure out if a page is really just erased.
+		 * Read OOB data and check whether data/OOB is completely
+		 * erased or if error correction just failed for all sub-
+		 * pages.
+		 */
+		ret = tegra_nand_read_oob(mtd, chip, page);
+		if (ret < 0)
+			return ret;
+
+		for_each_set_bit(bit, &fail_sec_flag, chip->ecc.steps) {
+			u8 *data = buf + (chip->ecc.size * bit);
+			u8 *oob = chip->oob_poi + nand->ecc.offset +
+				  (chip->ecc.bytes * bit);
+
+			ret = nand_check_erased_ecc_chunk(data, chip->ecc.size,
+							  oob, chip->ecc.bytes,
+							  NULL, 0,
+							  chip->ecc.strength);
+			if (ret < 0) {
+				mtd->ecc_stats.failed++;
+			} else {
+				mtd->ecc_stats.corrected += ret;
+				max_bitflips = max(ret, max_bitflips);
+			}
+		}
+
+		return max_t(unsigned int, max_corr_cnt, max_bitflips);
+	} else {
+		int corr_sec_flag;
+
+		corr_sec_flag = (dec_stat & DEC_STAT_BUF_CORR_SEC_FLAG_MASK) >>
+				DEC_STAT_BUF_CORR_SEC_FLAG_SHIFT;
+
+		/*
+		 * The value returned in the register is the maximum of
+		 * bitflips encountered in any of the ECC regions. As there is
+		 * no way to get the number of bitflips in a specific regions
+		 * we are not able to deliver correct stats but instead
+		 * overestimate the number of corrected bitflips by assuming
+		 * that all regions where errors have been corrected
+		 * encountered the maximum number of bitflips.
+		 */
+		mtd->ecc_stats.corrected += max_corr_cnt * hweight8(corr_sec_flag);
+
+		return max_corr_cnt;
+	}
+}
+
+static int tegra_nand_write_page_hwecc(struct mtd_info *mtd,
+				       struct nand_chip *chip,
+				       const uint8_t *buf, int oob_required,
+				       int page)
+{
+	struct tegra_nand_controller *ctrl = to_tegra_ctrl(chip->controller);
+	struct tegra_nand_chip *nand = to_tegra_chip(chip);
+	void *oob_buf = oob_required ? chip->oob_poi : NULL;
+	int ret;
+
+	tegra_nand_hw_ecc(ctrl, chip, true);
+	ret = tegra_nand_page_xfer(mtd, chip, (void *)buf, oob_buf,
+				   nand->tag.length, page, false);
+	tegra_nand_hw_ecc(ctrl, chip, false);
+
+	return ret;
+}
+
+static void tegra_nand_setup_timing(struct tegra_nand_controller *ctrl,
+				    const struct nand_sdr_timings *timings)
+{
+	/*
+	 * The period (and all other timings in this function) is in ps,
+	 * so need to take care here to avoid integer overflows.
+	 */
+	unsigned int rate = clk_get_rate(ctrl->clk) / 1000000;
+	unsigned int period = DIV_ROUND_UP(1000000, rate);
+	u32 val, reg = 0;
+
+	val = DIV_ROUND_UP(max3(timings->tAR_min, timings->tRR_min,
+				timings->tRC_min), period);
+	reg |= TIMING_TCR_TAR_TRR(OFFSET(val, 3));
+
+	val = DIV_ROUND_UP(max(max(timings->tCS_min, timings->tCH_min),
+			       max(timings->tALS_min, timings->tALH_min)),
+			   period);
+	reg |= TIMING_TCS(OFFSET(val, 2));
+
+	val = DIV_ROUND_UP(max(timings->tRP_min, timings->tREA_max) + 6000,
+			   period);
+	reg |= TIMING_TRP(OFFSET(val, 1)) | TIMING_TRP_RESP(OFFSET(val, 1));
+
+	reg |= TIMING_TWB(OFFSET(DIV_ROUND_UP(timings->tWB_max, period), 1));
+	reg |= TIMING_TWHR(OFFSET(DIV_ROUND_UP(timings->tWHR_min, period), 1));
+	reg |= TIMING_TWH(OFFSET(DIV_ROUND_UP(timings->tWH_min, period), 1));
+	reg |= TIMING_TWP(OFFSET(DIV_ROUND_UP(timings->tWP_min, period), 1));
+	reg |= TIMING_TRH(OFFSET(DIV_ROUND_UP(timings->tREH_min, period), 1));
+
+	writel_relaxed(reg, ctrl->regs + TIMING_1);
+
+	val = DIV_ROUND_UP(timings->tADL_min, period);
+	reg = TIMING_TADL(OFFSET(val, 3));
+
+	writel_relaxed(reg, ctrl->regs + TIMING_2);
+}
+
+static int tegra_nand_setup_data_interface(struct mtd_info *mtd, int csline,
+					const struct nand_data_interface *conf)
+{
+	struct nand_chip *chip = mtd_to_nand(mtd);
+	struct tegra_nand_controller *ctrl = to_tegra_ctrl(chip->controller);
+	const struct nand_sdr_timings *timings;
+
+	timings = nand_get_sdr_timings(conf);
+	if (IS_ERR(timings))
+		return PTR_ERR(timings);
+
+	if (csline == NAND_DATA_IFACE_CHECK_ONLY)
+		return 0;
+
+	tegra_nand_setup_timing(ctrl, timings);
+
+	return 0;
+}
+
+static const int rs_strength_bootable[] = { 4 };
+static const int rs_strength[] = { 4, 6, 8 };
+static const int bch_strength_bootable[] = { 8, 16 };
+static const int bch_strength[] = { 4, 8, 14, 16 };
+
+static int tegra_nand_get_strength(struct nand_chip *chip, const int *strength,
+				   int strength_len, int bits_per_step,
+				   int oobsize)
+{
+	bool maximize = chip->ecc.options & NAND_ECC_MAXIMIZE;
+	int i;
+
+	/*
+	 * Loop through available strengths. Backwards in case we try to
+	 * maximize the BCH strength.
+	 */
+	for (i = 0; i < strength_len; i++) {
+		int strength_sel, bytes_per_step, bytes_per_page;
+
+		if (maximize) {
+			strength_sel = strength[strength_len - i - 1];
+		} else {
+			strength_sel = strength[i];
+
+			if (strength_sel < chip->ecc_strength_ds)
+				continue;
+		}
+
+		bytes_per_step = DIV_ROUND_UP(bits_per_step * strength_sel,
+					      BITS_PER_BYTE);
+		bytes_per_page = round_up(bytes_per_step * chip->ecc.steps, 4);
+
+		/* Check whether strength fits OOB */
+		if (bytes_per_page < (oobsize - SKIP_SPARE_BYTES))
+			return strength_sel;
+	}
+
+	return -EINVAL;
+}
+
+static int tegra_nand_select_strength(struct nand_chip *chip, int oobsize)
+{
+	const int *strength;
+	int strength_len, bits_per_step;
+
+	switch (chip->ecc.algo) {
+	case NAND_ECC_RS:
+		bits_per_step = BITS_PER_STEP_RS;
+		if (chip->options & NAND_IS_BOOT_MEDIUM) {
+			strength = rs_strength_bootable;
+			strength_len = ARRAY_SIZE(rs_strength_bootable);
+		} else {
+			strength = rs_strength;
+			strength_len = ARRAY_SIZE(rs_strength);
+		}
+		break;
+	case NAND_ECC_BCH:
+		bits_per_step = BITS_PER_STEP_BCH;
+		if (chip->options & NAND_IS_BOOT_MEDIUM) {
+			strength = bch_strength_bootable;
+			strength_len = ARRAY_SIZE(bch_strength_bootable);
+		} else {
+			strength = bch_strength;
+			strength_len = ARRAY_SIZE(bch_strength);
+		}
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return tegra_nand_get_strength(chip, strength, strength_len,
+				       bits_per_step, oobsize);
+}
+
+static int tegra_nand_chips_init(struct device *dev,
+				 struct tegra_nand_controller *ctrl)
+{
+	struct device_node *np = dev->of_node;
+	struct device_node *np_nand;
+	int nsels, nchips = of_get_child_count(np);
+	struct tegra_nand_chip *nand;
+	struct mtd_info *mtd;
+	struct nand_chip *chip;
+	int bits_per_step;
+	int ret;
+	u32 cs;
+
+	if (nchips != 1) {
+		dev_err(dev, "Currently only one NAND chip supported\n");
+		return -EINVAL;
+	}
+
+	np_nand = of_get_next_child(np, NULL);
+
+	nsels = of_property_count_elems_of_size(np_nand, "reg", sizeof(u32));
+	if (nsels != 1) {
+		dev_err(dev, "Missing/invalid reg property\n");
+		return -EINVAL;
+	}
+
+	/* Retrieve CS id, currently only single die NAND supported */
+	ret = of_property_read_u32(np_nand, "reg", &cs);
+	if (ret) {
+		dev_err(dev, "could not retrieve reg property: %d\n", ret);
+		return ret;
+	}
+
+	nand = devm_kzalloc(dev, sizeof(*nand), GFP_KERNEL);
+	if (!nand)
+		return -ENOMEM;
+
+	nand->cs[0] = cs;
+
+	nand->wp_gpio = devm_gpiod_get_optional(dev, "wp", GPIOD_OUT_LOW);
+
+	if (IS_ERR(nand->wp_gpio)) {
+		ret = PTR_ERR(nand->wp_gpio);
+		dev_err(dev, "Failed to request WP GPIO: %d\n", ret);
+		return ret;
+	}
+
+	chip = &nand->chip;
+	chip->controller = &ctrl->controller;
+
+	mtd = nand_to_mtd(chip);
+
+	mtd->dev.parent = dev;
+	mtd->owner = THIS_MODULE;
+
+	nand_set_flash_node(chip, np_nand);
+
+	if (!mtd->name)
+		mtd->name = "tegra_nand";
+
+	chip->options = NAND_NO_SUBPAGE_WRITE | NAND_USE_BOUNCE_BUFFER;
+	chip->exec_op = tegra_nand_exec_op;
+	chip->select_chip = tegra_nand_select_chip;
+	chip->setup_data_interface = tegra_nand_setup_data_interface;
+
+	ret = nand_scan_ident(mtd, 1, NULL);
+	if (ret)
+		return ret;
+
+	if (chip->bbt_options & NAND_BBT_USE_FLASH)
+		chip->bbt_options |= NAND_BBT_NO_OOB;
+
+	chip->ecc.mode = NAND_ECC_HW;
+	chip->ecc.size = 512;
+	chip->ecc.steps = mtd->writesize / chip->ecc.size;
+	if (chip->ecc_step_ds != 512) {
+		dev_err(dev, "Unsupported step size %d\n", chip->ecc_step_ds);
+		return -EINVAL;
+	}
+
+	chip->ecc.read_page = tegra_nand_read_page_hwecc;
+	chip->ecc.write_page = tegra_nand_write_page_hwecc;
+	chip->ecc.read_page_raw = tegra_nand_read_page_raw;
+	chip->ecc.write_page_raw = tegra_nand_write_page_raw;
+	chip->ecc.read_oob = tegra_nand_read_oob;
+	chip->ecc.write_oob = tegra_nand_write_oob;
+
+	if (chip->options & NAND_BUSWIDTH_16)
+		nand->config |= CONFIG_BUS_WIDTH_16;
+
+	if (chip->ecc.algo == NAND_ECC_UNKNOWN) {
+		if (mtd->writesize < 2048)
+			chip->ecc.algo = NAND_ECC_RS;
+		else
+			chip->ecc.algo = NAND_ECC_BCH;
+	}
+
+	if (chip->ecc.algo == NAND_ECC_BCH && mtd->writesize < 2048) {
+		dev_err(dev, "BCH supportes 2K or 4K page size only\n");
+		return -EINVAL;
+	}
+
+	if (!chip->ecc.strength) {
+		ret = tegra_nand_select_strength(chip, mtd->oobsize);
+		if (ret < 0) {
+			dev_err(dev, "No valid strenght found, minimum %d\n",
+				chip->ecc_strength_ds);
+			return ret;
+		}
+
+		chip->ecc.strength = ret;
+	}
+
+	nand->config_ecc = CONFIG_PIPE_EN | CONFIG_SKIP_SPARE |
+			   CONFIG_SKIP_SPARE_SIZE_4;
+
+	switch (chip->ecc.algo) {
+	case NAND_ECC_RS:
+		bits_per_step = BITS_PER_STEP_RS * chip->ecc.strength;
+		mtd_set_ooblayout(mtd, &tegra_nand_oob_rs_ops);
+		nand->config_ecc |= CONFIG_HW_ECC | CONFIG_ECC_SEL |
+				    CONFIG_ERR_COR;
+		switch (chip->ecc.strength) {
+		case 4:
+			nand->config_ecc |= CONFIG_TVAL_4;
+			break;
+		case 6:
+			nand->config_ecc |= CONFIG_TVAL_6;
+			break;
+		case 8:
+			nand->config_ecc |= CONFIG_TVAL_8;
+			break;
+		default:
+			dev_err(dev, "ECC strength %d not supported\n",
+				chip->ecc.strength);
+			return -EINVAL;
+		}
+		break;
+	case NAND_ECC_BCH:
+		bits_per_step = BITS_PER_STEP_BCH * chip->ecc.strength;
+		mtd_set_ooblayout(mtd, &tegra_nand_oob_bch_ops);
+		nand->bch_config = BCH_ENABLE;
+		switch (chip->ecc.strength) {
+		case 4:
+			nand->bch_config |= BCH_TVAL_4;
+			break;
+		case 8:
+			nand->bch_config |= BCH_TVAL_8;
+			break;
+		case 14:
+			nand->bch_config |= BCH_TVAL_14;
+			break;
+		case 16:
+			nand->bch_config |= BCH_TVAL_16;
+			break;
+		default:
+			dev_err(dev, "ECC strength %d not supported\n",
+				chip->ecc.strength);
+			return -EINVAL;
+		}
+		break;
+	default:
+		dev_err(dev, "ECC algorithm not supported\n");
+		return -EINVAL;
+	}
+
+	dev_info(dev, "Using %s with strength %d per 512 byte step\n",
+			chip->ecc.algo == NAND_ECC_BCH ? "BCH" : "RS",
+			chip->ecc.strength);
+
+	chip->ecc.bytes = DIV_ROUND_UP(bits_per_step, BITS_PER_BYTE);
+
+	switch (mtd->writesize) {
+	case 256:
+		nand->config |= CONFIG_PS_256;
+		break;
+	case 512:
+		nand->config |= CONFIG_PS_512;
+		break;
+	case 1024:
+		nand->config |= CONFIG_PS_1024;
+		break;
+	case 2048:
+		nand->config |= CONFIG_PS_2048;
+		break;
+	case 4096:
+		nand->config |= CONFIG_PS_4096;
+		break;
+	default:
+		dev_err(dev, "Unsupported writesize %d\n", mtd->writesize);
+		return -ENODEV;
+	}
+
+	/* Store complete configuration in config_ecc */
+	nand->config_ecc |= nand->config;
+
+	/* Non-HW ECC read/writes complete OOB */
+	nand->config |= CONFIG_TAG_BYTE_SIZE(mtd->oobsize - 1);
+	writel_relaxed(nand->config, ctrl->regs + CONFIG);
+
+	ret = nand_scan_tail(mtd);
+	if (ret)
+		return ret;
+
+	/* Store ECC/tag layout when using HW ECC */
+	mtd_ooblayout_ecc(mtd, 0, &nand->ecc);
+	mtd_ooblayout_free(mtd, 0, &nand->tag);
+	nand->config_ecc |= CONFIG_TAG_BYTE_SIZE(nand->tag.length - 1);
+
+	ret = mtd_device_register(mtd, NULL, 0);
+	if (ret) {
+		dev_err(dev, "Failed to register mtd device: %d\n", ret);
+		nand_cleanup(chip);
+		return ret;
+	}
+
+	ctrl->chip = chip;
+
+	return 0;
+}
+
+static int tegra_nand_probe(struct platform_device *pdev)
+{
+	struct reset_control *rst;
+	struct tegra_nand_controller *ctrl;
+	struct resource *res;
+	int err = 0;
+
+	ctrl = devm_kzalloc(&pdev->dev, sizeof(*ctrl), GFP_KERNEL);
+	if (!ctrl)
+		return -ENOMEM;
+
+	ctrl->dev = &pdev->dev;
+	nand_hw_control_init(&ctrl->controller);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	ctrl->regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(ctrl->regs))
+		return PTR_ERR(ctrl->regs);
+
+	rst = devm_reset_control_get(&pdev->dev, "nand");
+	if (IS_ERR(rst))
+		return PTR_ERR(rst);
+
+	ctrl->clk = devm_clk_get(&pdev->dev, "nand");
+	if (IS_ERR(ctrl->clk))
+		return PTR_ERR(ctrl->clk);
+
+	err = clk_prepare_enable(ctrl->clk);
+	if (err)
+		return err;
+
+	err = reset_control_reset(rst);
+	if (err) {
+		dev_err(ctrl->dev, "Failed to reset HW: %d\n", err);
+		goto err_disable_clk;
+	}
+
+	writel_relaxed(HWSTATUS_CMD_DEFAULT, ctrl->regs + HWSTATUS_CMD);
+	writel_relaxed(HWSTATUS_MASK_DEFAULT, ctrl->regs + HWSTATUS_MASK);
+	writel_relaxed(INT_MASK, ctrl->regs + IER);
+
+	init_completion(&ctrl->command_complete);
+	init_completion(&ctrl->dma_complete);
+
+	ctrl->irq = platform_get_irq(pdev, 0);
+	err = devm_request_irq(&pdev->dev, ctrl->irq, tegra_nand_irq, 0,
+			       dev_name(&pdev->dev), ctrl);
+	if (err) {
+		dev_err(ctrl->dev, "Failed to get IRQ: %d\n", err);
+		goto err_disable_clk;
+	}
+
+	writel_relaxed(DMA_MST_CTRL_IS_DONE, ctrl->regs + DMA_MST_CTRL);
+
+	err = tegra_nand_chips_init(ctrl->dev, ctrl);
+	if (err)
+		goto err_disable_clk;
+
+	platform_set_drvdata(pdev, ctrl);
+
+	return 0;
+
+err_disable_clk:
+	clk_disable_unprepare(ctrl->clk);
+	return err;
+}
+
+static int tegra_nand_remove(struct platform_device *pdev)
+{
+	struct tegra_nand_controller *ctrl = platform_get_drvdata(pdev);
+
+	nand_release(nand_to_mtd(ctrl->chip));
+
+	clk_disable_unprepare(ctrl->clk);
+
+	return 0;
+}
+
+static const struct of_device_id tegra_nand_of_match[] = {
+	{ .compatible = "nvidia,tegra20-nand" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, tegra_nand_of_match);
+
+static struct platform_driver tegra_nand_driver = {
+	.driver = {
+		.name = "tegra-nand",
+		.of_match_table = tegra_nand_of_match,
+	},
+	.probe = tegra_nand_probe,
+	.remove = tegra_nand_remove,
+};
+module_platform_driver(tegra_nand_driver);
+
+MODULE_DESCRIPTION("NVIDIA Tegra NAND driver");
+MODULE_AUTHOR("Thierry Reding <thierry.reding@nvidia.com>");
+MODULE_AUTHOR("Lucas Stach <dev@lynxeye.de>");
+MODULE_AUTHOR("Stefan Agner <stefan@agner.ch>");
+MODULE_LICENSE("GPL v2");
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v5 5/6] ARM: dts: tegra: add Tegra20 NAND flash controller node
  2018-06-17 20:45 [PATCH v5 0/6] mtd: rawnand: add NVIDIA Tegra NAND flash support Stefan Agner
                   ` (3 preceding siblings ...)
  2018-06-17 20:46 ` [PATCH v5 4/6] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver Stefan Agner
@ 2018-06-17 20:46 ` Stefan Agner
  2018-06-17 20:46 ` [PATCH v5 6/6] ARM: dts: tegra: enable NAND flash on Colibri T20 Stefan Agner
  2018-06-18  9:58 ` [PATCH v5 0/6] mtd: rawnand: add NVIDIA Tegra NAND flash support Boris Brezillon
  6 siblings, 0 replies; 11+ messages in thread
From: Stefan Agner @ 2018-06-17 20:46 UTC (permalink / raw)
  To: boris.brezillon, dwmw2, computersforpeace, marek.vasut, robh+dt,
	mark.rutland, thierry.reding
  Cc: dev, miquel.raynal, richard, marcel, krzk, digetx,
	benjamin.lindqvist, jonathanh, pdeschrijver, pgaikwad,
	mirza.krak, gaireg, linux-mtd, linux-tegra, devicetree,
	linux-kernel, Stefan Agner

From: Lucas Stach <dev@lynxeye.de>

Add basic controller device tree node to be extended by
individual boards. Use the assigned-clocks mechanism to set
NDFLASH clock to a sensible default rate of 150MHz.

Signed-off-by: Lucas Stach <dev@lynxeye.de>
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 arch/arm/boot/dts/tegra20.dtsi | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index 983dd5c147945..a75fe7fec7912 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -425,6 +425,21 @@
 		status = "disabled";
 	};
 
+	nand-controller@70008000 {
+		compatible = "nvidia,tegra20-nand";
+		reg = <0x70008000 0x100>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&tegra_car TEGRA20_CLK_NDFLASH>;
+		clock-names = "nand";
+		resets = <&tegra_car 13>;
+		reset-names = "nand";
+		assigned-clocks = <&tegra_car TEGRA20_CLK_NDFLASH>;
+		assigned-clock-rates = <150000000>;
+		status = "disabled";
+	};
+
 	pwm: pwm@7000a000 {
 		compatible = "nvidia,tegra20-pwm";
 		reg = <0x7000a000 0x100>;
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v5 6/6] ARM: dts: tegra: enable NAND flash on Colibri T20
  2018-06-17 20:45 [PATCH v5 0/6] mtd: rawnand: add NVIDIA Tegra NAND flash support Stefan Agner
                   ` (4 preceding siblings ...)
  2018-06-17 20:46 ` [PATCH v5 5/6] ARM: dts: tegra: add Tegra20 NAND flash controller node Stefan Agner
@ 2018-06-17 20:46 ` Stefan Agner
  2018-06-18  9:58 ` [PATCH v5 0/6] mtd: rawnand: add NVIDIA Tegra NAND flash support Boris Brezillon
  6 siblings, 0 replies; 11+ messages in thread
From: Stefan Agner @ 2018-06-17 20:46 UTC (permalink / raw)
  To: boris.brezillon, dwmw2, computersforpeace, marek.vasut, robh+dt,
	mark.rutland, thierry.reding
  Cc: dev, miquel.raynal, richard, marcel, krzk, digetx,
	benjamin.lindqvist, jonathanh, pdeschrijver, pgaikwad,
	mirza.krak, gaireg, linux-mtd, linux-tegra, devicetree,
	linux-kernel, Stefan Agner

This enables the on-module ONFI conformant NAND flash.

Signed-off-by: Lucas Stach <dev@lynxeye.de>
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 arch/arm/boot/dts/tegra20-colibri-512.dtsi | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm/boot/dts/tegra20-colibri-512.dtsi b/arch/arm/boot/dts/tegra20-colibri-512.dtsi
index 5c202b3e3bb1d..0c283fb772148 100644
--- a/arch/arm/boot/dts/tegra20-colibri-512.dtsi
+++ b/arch/arm/boot/dts/tegra20-colibri-512.dtsi
@@ -462,6 +462,22 @@
 		};
 	};
 
+	nand-controller@70008000 {
+		status = "okay";
+
+		nand@0 {
+			reg = <0>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			nand-bus-width = <8>;
+			nand-on-flash-bbt;
+			nand-ecc-algo = "bch";
+			nand-is-boot-medium;
+			nand-ecc-maximize;
+			wp-gpios = <&gpio TEGRA_GPIO(S, 0) GPIO_ACTIVE_LOW>;
+		};
+	};
+
 	usb@c5004000 {
 		status = "okay";
 		nvidia,phy-reset-gpio = <&gpio TEGRA_GPIO(V, 1)
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 0/6] mtd: rawnand: add NVIDIA Tegra NAND flash support
  2018-06-17 20:45 [PATCH v5 0/6] mtd: rawnand: add NVIDIA Tegra NAND flash support Stefan Agner
                   ` (5 preceding siblings ...)
  2018-06-17 20:46 ` [PATCH v5 6/6] ARM: dts: tegra: enable NAND flash on Colibri T20 Stefan Agner
@ 2018-06-18  9:58 ` Boris Brezillon
  2018-06-18 10:51   ` Stefan Agner
  6 siblings, 1 reply; 11+ messages in thread
From: Boris Brezillon @ 2018-06-18  9:58 UTC (permalink / raw)
  To: Stefan Agner
  Cc: dwmw2, computersforpeace, marek.vasut, robh+dt, mark.rutland,
	thierry.reding, dev, miquel.raynal, richard, marcel, krzk,
	digetx, benjamin.lindqvist, jonathanh, pdeschrijver, pgaikwad,
	mirza.krak, gaireg, linux-mtd, linux-tegra, devicetree,
	linux-kernel

On Sun, 17 Jun 2018 22:45:59 +0200
Stefan Agner <stefan@agner.ch> wrote:

> Changes definitly calm down, most noteably probably the changes
> around checking whether a page is empty if the stack reports ECC
> errors.. I verified the code using raw nandwrites with OOB to
> simulate an empty page which has some bits flipped in the OOB area,
> everthing seems to work as I would expect it.
> 
> For now I do not check extra OOB bytes since those are at variable
> locations depending on algorithm.

Hm, if you expose them as free OOB bytes, you should also check them,
otherwise you might end up with corrupted data without noticing it. Note
that, depending on whether those free OOB bytes are ECC-protected or
not, you should change the way you do the check:

- non-protected OOB bytes: all bytes should be 0xff (no bitflips
  allowed)
- data+free OOB bytes protected by the same ECC bytes: you should pass
  the free OOB bytes buffer to nand_check_erased_ecc_chunk() along with
  the data and ECC buffers
- free OOB bytes have their own ECC bytes: call
  nand_check_erased_ecc_chunk() separately and pass it the ECC + free
  OOB buffers.

> 
> --
> Stefan
> 
> Changes since v1:
> - Split controller and NAND chip structure
> - Add BCH support
> - Allow to select algorithm and strength using device tree
> - Improve HW ECC error reporting and use DEC_STATUS_BUF only
> - Use SPDX license identifier
> - Use per algorithm mtd_ooblayout_ops
> - Use setup_data_interface callback for NAND timing configuration
> 
> Changes since v2:
> - Set clock rate using assigned-clocks
> - Use BIT() macro
> - Fix and improve timing calculation
> - Improve ECC error handling
> - Store OOB layout for tag area in Tegra chip structure
> - Update/fix bindings
> - Use more specific variable names (replace "value")
> - Introduce nand-is-boot-medium
> - Choose sensible ECC strenght automatically
> - Use wait_for_completion_timeout
> - Print register dump on completion timeout
> - Unify tegra_nand_(read|write)_page in tegra_nand_page_xfer
> 
> Changes since v3:
> - Implement tegra_nand_(read|write)_raw using DMA
> - Implement tegra_nand_(read|write)_oob using DMA
> - Name registers according to Tegra 2 Technical Reference Manual (v02p)
> - Use wait_for_completion_io_timeout to account for IO
> - Get chip select id from device tree reg property
> - Clear interrupts and reinit wait queues in case command/DMA times out
> - Set default MTD name after nand_set_flash_node
> - Move MODULE_DEVICE_TABLE after declaration of tegra_nand_of_match
> - Make (rs|bch)_strength static
> 
> Changes since v4:
> - Pass OOB area to nand_check_erased_ecc_chunk
> - Pass algorithm specific bits_per_step to tegra_nand_get_strength
> - Store ECC layout in chip structure
> - Fix pointer assignment (use NULL)
> - Removed obsolete header delay.h
> - Fixed newlines
> - Use non-_io variant of wait_for_completion_timeout
> 
> Lucas Stach (1):
>   ARM: dts: tegra: add Tegra20 NAND flash controller node
> 
> Stefan Agner (5):
>   mtd: rawnand: add Reed-Solomon error correction algorithm
>   mtd: rawnand: add an option to specify NAND chip as a boot device
>   mtd: rawnand: tegra: add devicetree binding
>   mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver
>   ARM: dts: tegra: enable NAND flash on Colibri T20
> 
>  .../devicetree/bindings/mtd/nand.txt          |    6 +-
>  .../bindings/mtd/nvidia-tegra20-nand.txt      |   64 +
>  MAINTAINERS                                   |    7 +
>  arch/arm/boot/dts/tegra20-colibri-512.dtsi    |   16 +
>  arch/arm/boot/dts/tegra20.dtsi                |   15 +
>  drivers/mtd/nand/raw/Kconfig                  |    6 +
>  drivers/mtd/nand/raw/Makefile                 |    1 +
>  drivers/mtd/nand/raw/nand_base.c              |    4 +
>  drivers/mtd/nand/raw/tegra_nand.c             | 1268 +++++++++++++++++
>  include/linux/mtd/rawnand.h                   |    7 +
>  10 files changed, 1393 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
>  create mode 100644 drivers/mtd/nand/raw/tegra_nand.c
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 0/6] mtd: rawnand: add NVIDIA Tegra NAND flash support
  2018-06-18  9:58 ` [PATCH v5 0/6] mtd: rawnand: add NVIDIA Tegra NAND flash support Boris Brezillon
@ 2018-06-18 10:51   ` Stefan Agner
  2018-06-18 11:59     ` Boris Brezillon
  0 siblings, 1 reply; 11+ messages in thread
From: Stefan Agner @ 2018-06-18 10:51 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: dwmw2, computersforpeace, marek.vasut, robh+dt, mark.rutland,
	thierry.reding, dev, miquel.raynal, richard, marcel, krzk,
	digetx, benjamin.lindqvist, jonathanh, pdeschrijver, pgaikwad,
	mirza.krak, gaireg, linux-mtd, linux-tegra, devicetree,
	linux-kernel

On 18.06.2018 11:58, Boris Brezillon wrote:
> On Sun, 17 Jun 2018 22:45:59 +0200
> Stefan Agner <stefan@agner.ch> wrote:
> 
>> Changes definitly calm down, most noteably probably the changes
>> around checking whether a page is empty if the stack reports ECC
>> errors.. I verified the code using raw nandwrites with OOB to
>> simulate an empty page which has some bits flipped in the OOB area,
>> everthing seems to work as I would expect it.
>>
>> For now I do not check extra OOB bytes since those are at variable
>> locations depending on algorithm.
> 
> Hm, if you expose them as free OOB bytes, you should also check them,
> otherwise you might end up with corrupted data without noticing it. Note
> that, depending on whether those free OOB bytes are ECC-protected or
> not, you should change the way you do the check:
> 
> - non-protected OOB bytes: all bytes should be 0xff (no bitflips
>   allowed)
> - data+free OOB bytes protected by the same ECC bytes: you should pass
>   the free OOB bytes buffer to nand_check_erased_ecc_chunk() along with
>   the data and ECC buffers
> - free OOB bytes have their own ECC bytes: call
>   nand_check_erased_ecc_chunk() separately and pass it the ECC + free
>   OOB buffers.

This graphic taken from the public Tegra 2 Technical Reference Manual is
quite useful:
https://imgur.com/a/0Hqzbkc

Tegra basically has all of the above, which makes the whole business
really tricky...

I am not sure if we really could do variant 1, non-protected OOB, but
since we have the option of protected OOB, we probably anyway would do
that.

RS/Hamming implements variant 3.

BCH implements variant 2. OOB is protected with the last data buffer.

So this would require a algorithm depending implementation, which is
probably not a big deal.

But there is one more issue with BCH: Only if extra data are actually
transferred, tag space is actually allocated. If no tag bytes are
transferred, parity follows immediately skip bytes. As far as I know the
MTD stacks OOB layout assumes that is always the same layout, no matter
whether we write extra OOB data or not. For the Tegra NAND controller
this would mean that we have to always transfer tag bytes and therefor
penalize the use case we are most interested in (which is no extra OOB
bytes, since UBI does not make use of it)...

Furthermore I realized that testing is not easily possible since
nandwrite with --oob seems not to make use of "oob_required" in the main
page write but issues a separate OOB write command. I did not found a
way to issue a write from user space which sets oob_required...

Due to all this I rather prefer to not implement extra OOB support at
this point.

How do I do this properly? Set mtd_ooblayout_ops.free to NULL?

--
Stefan

> 
>>
>> --
>> Stefan
>>
>> Changes since v1:
>> - Split controller and NAND chip structure
>> - Add BCH support
>> - Allow to select algorithm and strength using device tree
>> - Improve HW ECC error reporting and use DEC_STATUS_BUF only
>> - Use SPDX license identifier
>> - Use per algorithm mtd_ooblayout_ops
>> - Use setup_data_interface callback for NAND timing configuration
>>
>> Changes since v2:
>> - Set clock rate using assigned-clocks
>> - Use BIT() macro
>> - Fix and improve timing calculation
>> - Improve ECC error handling
>> - Store OOB layout for tag area in Tegra chip structure
>> - Update/fix bindings
>> - Use more specific variable names (replace "value")
>> - Introduce nand-is-boot-medium
>> - Choose sensible ECC strenght automatically
>> - Use wait_for_completion_timeout
>> - Print register dump on completion timeout
>> - Unify tegra_nand_(read|write)_page in tegra_nand_page_xfer
>>
>> Changes since v3:
>> - Implement tegra_nand_(read|write)_raw using DMA
>> - Implement tegra_nand_(read|write)_oob using DMA
>> - Name registers according to Tegra 2 Technical Reference Manual (v02p)
>> - Use wait_for_completion_io_timeout to account for IO
>> - Get chip select id from device tree reg property
>> - Clear interrupts and reinit wait queues in case command/DMA times out
>> - Set default MTD name after nand_set_flash_node
>> - Move MODULE_DEVICE_TABLE after declaration of tegra_nand_of_match
>> - Make (rs|bch)_strength static
>>
>> Changes since v4:
>> - Pass OOB area to nand_check_erased_ecc_chunk
>> - Pass algorithm specific bits_per_step to tegra_nand_get_strength
>> - Store ECC layout in chip structure
>> - Fix pointer assignment (use NULL)
>> - Removed obsolete header delay.h
>> - Fixed newlines
>> - Use non-_io variant of wait_for_completion_timeout
>>
>> Lucas Stach (1):
>>   ARM: dts: tegra: add Tegra20 NAND flash controller node
>>
>> Stefan Agner (5):
>>   mtd: rawnand: add Reed-Solomon error correction algorithm
>>   mtd: rawnand: add an option to specify NAND chip as a boot device
>>   mtd: rawnand: tegra: add devicetree binding
>>   mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver
>>   ARM: dts: tegra: enable NAND flash on Colibri T20
>>
>>  .../devicetree/bindings/mtd/nand.txt          |    6 +-
>>  .../bindings/mtd/nvidia-tegra20-nand.txt      |   64 +
>>  MAINTAINERS                                   |    7 +
>>  arch/arm/boot/dts/tegra20-colibri-512.dtsi    |   16 +
>>  arch/arm/boot/dts/tegra20.dtsi                |   15 +
>>  drivers/mtd/nand/raw/Kconfig                  |    6 +
>>  drivers/mtd/nand/raw/Makefile                 |    1 +
>>  drivers/mtd/nand/raw/nand_base.c              |    4 +
>>  drivers/mtd/nand/raw/tegra_nand.c             | 1268 +++++++++++++++++
>>  include/linux/mtd/rawnand.h                   |    7 +
>>  10 files changed, 1393 insertions(+), 1 deletion(-)
>>  create mode 100644 Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
>>  create mode 100644 drivers/mtd/nand/raw/tegra_nand.c
>>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 0/6] mtd: rawnand: add NVIDIA Tegra NAND flash support
  2018-06-18 10:51   ` Stefan Agner
@ 2018-06-18 11:59     ` Boris Brezillon
  2018-06-18 14:11       ` Stefan Agner
  0 siblings, 1 reply; 11+ messages in thread
From: Boris Brezillon @ 2018-06-18 11:59 UTC (permalink / raw)
  To: Stefan Agner
  Cc: dwmw2, computersforpeace, marek.vasut, robh+dt, mark.rutland,
	thierry.reding, dev, miquel.raynal, richard, marcel, krzk,
	digetx, benjamin.lindqvist, jonathanh, pdeschrijver, pgaikwad,
	mirza.krak, gaireg, linux-mtd, linux-tegra, devicetree,
	linux-kernel

Hi Stefan,

On Mon, 18 Jun 2018 12:51:52 +0200
Stefan Agner <stefan@agner.ch> wrote:

> On 18.06.2018 11:58, Boris Brezillon wrote:
> > On Sun, 17 Jun 2018 22:45:59 +0200
> > Stefan Agner <stefan@agner.ch> wrote:
> >   
> >> Changes definitly calm down, most noteably probably the changes
> >> around checking whether a page is empty if the stack reports ECC
> >> errors.. I verified the code using raw nandwrites with OOB to
> >> simulate an empty page which has some bits flipped in the OOB area,
> >> everthing seems to work as I would expect it.
> >>
> >> For now I do not check extra OOB bytes since those are at variable
> >> locations depending on algorithm.  
> > 
> > Hm, if you expose them as free OOB bytes, you should also check them,
> > otherwise you might end up with corrupted data without noticing it. Note
> > that, depending on whether those free OOB bytes are ECC-protected or
> > not, you should change the way you do the check:
> > 
> > - non-protected OOB bytes: all bytes should be 0xff (no bitflips
> >   allowed)
> > - data+free OOB bytes protected by the same ECC bytes: you should pass
> >   the free OOB bytes buffer to nand_check_erased_ecc_chunk() along with
> >   the data and ECC buffers
> > - free OOB bytes have their own ECC bytes: call
> >   nand_check_erased_ecc_chunk() separately and pass it the ECC + free
> >   OOB buffers.  
> 
> This graphic taken from the public Tegra 2 Technical Reference Manual is
> quite useful:
> https://imgur.com/a/0Hqzbkc

Thanks for sharing this doc.

> 
> Tegra basically has all of the above, which makes the whole business
> really tricky...

I'm not sure. Are "Skip bytes" protected by "main data parity bytes"?

AFAICT, you have "Tag bytes" that fall in case #3 and "Remaining spare
bytes" that fall in case #1. If "Skip bytes" are protected by the "main
data parity bytes", then it falls in case #2, otherwise it probably
goes in case #1.

> 
> I am not sure if we really could do variant 1, non-protected OOB, but
> since we have the option of protected OOB, we probably anyway would do
> that.

That's up to you, but in this case, you should not declare those bytes
as free (didn't check what is currently done in the driver).

> 
> RS/Hamming implements variant 3.

It seems to be a mix of #1 and #3, but I'm not sure (see above).

> 
> BCH implements variant 2.

I'd say it's a mix of #1 (skip + remaining bytes) and #2(tag bytes).

> OOB is protected with the last data buffer.

That would be weird, but maybe you're right. HW ECC engine usually
split the OOB area in X portions, X being the number of ECC steps needed
to cover a NAND page, and then have ECC bytes cover a sub-portion of
data+OOB.

For example, for a NAND page of 2k with 64 bytes of OOB, and assuming
the ECC step is 512bytes, you usually have something like:

[512(data)+8(protected-oob)+8(ecc)] x 4

> 
> So this would require a algorithm depending implementation, which is
> probably not a big deal.

True.

> 
> But there is one more issue with BCH: Only if extra data are actually
> transferred, tag space is actually allocated. If no tag bytes are
> transferred, parity follows immediately skip bytes. As far as I know the
> MTD stacks OOB layout assumes that is always the same layout, no matter
> whether we write extra OOB data or not. For the Tegra NAND controller
> this would mean that we have to always transfer tag bytes and therefor
> penalize the use case we are most interested in (which is no extra OOB
> bytes, since UBI does not make use of it)...

Hm, given the amount of tag bytes I don't think you'll have a huge
penalty, so I'd recommend always sending those bytes. Alternatively,
you could decide that you never want to have those tag bytes and expose
none of them.

> 
> Furthermore I realized that testing is not easily possible since
> nandwrite with --oob seems not to make use of "oob_required" in the main
> page write but issues a separate OOB write command. I did not found a
> way to issue a write from user space which sets oob_required...

Maybe it's time to patch those tools. The ioctl exists, so it's just a
matter of using it in nandwrite/mtd-utils.

> 
> Due to all this I rather prefer to not implement extra OOB support at
> this point.

I'm fine with that, but that means no JFFS2 support, as I think JFFS2
wants to place some of its metadata in the OOB area. Also, I fear it
will be a mess to add support for that kind of things without breaking
existing setup afterwards, so, by taking this decision you're pretty
much saying that this controller will never expose free OOB bytes.
That's not a problem from my PoV, but I want you to be aware of that.

> 
> How do I do this properly? Set mtd_ooblayout_ops.free to NULL?

Just implement a dummy function that returns -ERANGE.

Regards,

Boris

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 0/6] mtd: rawnand: add NVIDIA Tegra NAND flash support
  2018-06-18 11:59     ` Boris Brezillon
@ 2018-06-18 14:11       ` Stefan Agner
  0 siblings, 0 replies; 11+ messages in thread
From: Stefan Agner @ 2018-06-18 14:11 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: dwmw2, computersforpeace, marek.vasut, robh+dt, mark.rutland,
	thierry.reding, dev, miquel.raynal, richard, marcel, krzk,
	digetx, benjamin.lindqvist, jonathanh, pdeschrijver, pgaikwad,
	mirza.krak, gaireg, linux-mtd, linux-tegra, devicetree,
	linux-kernel

Hi Boris,

On 18.06.2018 13:59, Boris Brezillon wrote:
> Hi Stefan,
> 
> On Mon, 18 Jun 2018 12:51:52 +0200
> Stefan Agner <stefan@agner.ch> wrote:
> 
>> On 18.06.2018 11:58, Boris Brezillon wrote:
>> > On Sun, 17 Jun 2018 22:45:59 +0200
>> > Stefan Agner <stefan@agner.ch> wrote:
>> >
>> >> Changes definitly calm down, most noteably probably the changes
>> >> around checking whether a page is empty if the stack reports ECC
>> >> errors.. I verified the code using raw nandwrites with OOB to
>> >> simulate an empty page which has some bits flipped in the OOB area,
>> >> everthing seems to work as I would expect it.
>> >>
>> >> For now I do not check extra OOB bytes since those are at variable
>> >> locations depending on algorithm.
>> >
>> > Hm, if you expose them as free OOB bytes, you should also check them,
>> > otherwise you might end up with corrupted data without noticing it. Note
>> > that, depending on whether those free OOB bytes are ECC-protected or
>> > not, you should change the way you do the check:
>> >
>> > - non-protected OOB bytes: all bytes should be 0xff (no bitflips
>> >   allowed)
>> > - data+free OOB bytes protected by the same ECC bytes: you should pass
>> >   the free OOB bytes buffer to nand_check_erased_ecc_chunk() along with
>> >   the data and ECC buffers
>> > - free OOB bytes have their own ECC bytes: call
>> >   nand_check_erased_ecc_chunk() separately and pass it the ECC + free
>> >   OOB buffers.
>>
>> This graphic taken from the public Tegra 2 Technical Reference Manual is
>> quite useful:
>> https://imgur.com/a/0Hqzbkc
> 
> Thanks for sharing this doc.
> 
>>
>> Tegra basically has all of the above, which makes the whole business
>> really tricky...
> 
> I'm not sure. Are "Skip bytes" protected by "main data parity bytes"?
> 
> AFAICT, you have "Tag bytes" that fall in case #3 and "Remaining spare
> bytes" that fall in case #1. If "Skip bytes" are protected by the "main
> data parity bytes", then it falls in case #2, otherwise it probably
> goes in case #1.
> 

Skip bytes are not protected. I think they are mainly meant to skip the
bad block marker. Only 4, 8, 12 or 16 bytes are supported.

>>
>> I am not sure if we really could do variant 1, non-protected OOB, but
>> since we have the option of protected OOB, we probably anyway would do
>> that.
> 
> That's up to you, but in this case, you should not declare those bytes
> as free (didn't check what is currently done in the driver).
> 
>>
>> RS/Hamming implements variant 3.
> 
> It seems to be a mix of #1 and #3, but I'm not sure (see above).
> 
>>
>> BCH implements variant 2.
> 
> I'd say it's a mix of #1 (skip + remaining bytes) and #2(tag bytes).
> 
>> OOB is protected with the last data buffer.
> 
> That would be weird, but maybe you're right. HW ECC engine usually
> split the OOB area in X portions, X being the number of ECC steps needed
> to cover a NAND page, and then have ECC bytes cover a sub-portion of
> data+OOB.
> 
> For example, for a NAND page of 2k with 64 bytes of OOB, and assuming
> the ECC step is 512bytes, you usually have something like:
> 
> [512(data)+8(protected-oob)+8(ecc)] x 4
> 

The TRM explicitly states so:

"BCH mode Error correction
- Error correction with involving spare only transfers is not supported.
ECC calculation of last sub-page includes tag data in spare area.
- Error correction with Main only transfers is supported.
- Maximum possible length of Tag data size is 252 bytes."


>>
>> So this would require a algorithm depending implementation, which is
>> probably not a big deal.
> 
> True.
> 
>>
>> But there is one more issue with BCH: Only if extra data are actually
>> transferred, tag space is actually allocated. If no tag bytes are
>> transferred, parity follows immediately skip bytes. As far as I know the
>> MTD stacks OOB layout assumes that is always the same layout, no matter
>> whether we write extra OOB data or not. For the Tegra NAND controller
>> this would mean that we have to always transfer tag bytes and therefor
>> penalize the use case we are most interested in (which is no extra OOB
>> bytes, since UBI does not make use of it)...
> 
> Hm, given the amount of tag bytes I don't think you'll have a huge
> penalty, so I'd recommend always sending those bytes. Alternatively,
> you could decide that you never want to have those tag bytes and expose
> none of them.
> 
>>
>> Furthermore I realized that testing is not easily possible since
>> nandwrite with --oob seems not to make use of "oob_required" in the main
>> page write but issues a separate OOB write command. I did not found a
>> way to issue a write from user space which sets oob_required...
> 
> Maybe it's time to patch those tools. The ioctl exists, so it's just a
> matter of using it in nandwrite/mtd-utils.
> 
>>
>> Due to all this I rather prefer to not implement extra OOB support at
>> this point.
> 
> I'm fine with that, but that means no JFFS2 support, as I think JFFS2
> wants to place some of its metadata in the OOB area. Also, I fear it
> will be a mess to add support for that kind of things without breaking
> existing setup afterwards, so, by taking this decision you're pretty
> much saying that this controller will never expose free OOB bytes.
> That's not a problem from my PoV, but I want you to be aware of that.
> 

We already operate without extra OOB byte support in our downstream BSP.
I'd rather have a easy upgrade path today...

Another issue I just realized: The boot ROM only supports BCH without
tag bytes... So at least the boot loader has to be written without tag
bytes.

>>
>> How do I do this properly? Set mtd_ooblayout_ops.free to NULL?
> 
> Just implement a dummy function that returns -ERANGE.
> 

Ok, I will go with this then.

Also, thanks for all your valuable feedback, really appreciated!

--
Stefan

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-06-18 14:11 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-17 20:45 [PATCH v5 0/6] mtd: rawnand: add NVIDIA Tegra NAND flash support Stefan Agner
2018-06-17 20:46 ` [PATCH v5 1/6] mtd: rawnand: add Reed-Solomon error correction algorithm Stefan Agner
2018-06-17 20:46 ` [PATCH v5 2/6] mtd: rawnand: add an option to specify NAND chip as a boot device Stefan Agner
2018-06-17 20:46 ` [PATCH v5 3/6] mtd: rawnand: tegra: add devicetree binding Stefan Agner
2018-06-17 20:46 ` [PATCH v5 4/6] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver Stefan Agner
2018-06-17 20:46 ` [PATCH v5 5/6] ARM: dts: tegra: add Tegra20 NAND flash controller node Stefan Agner
2018-06-17 20:46 ` [PATCH v5 6/6] ARM: dts: tegra: enable NAND flash on Colibri T20 Stefan Agner
2018-06-18  9:58 ` [PATCH v5 0/6] mtd: rawnand: add NVIDIA Tegra NAND flash support Boris Brezillon
2018-06-18 10:51   ` Stefan Agner
2018-06-18 11:59     ` Boris Brezillon
2018-06-18 14:11       ` Stefan Agner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.