All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v2 0/7] tegra: Add NAND flash support
@ 2012-04-13 18:29 Simon Glass
  2012-04-13 18:29 ` [U-Boot] [PATCH v2 1/7] nand: Try to align the default buffers Simon Glass
                   ` (6 more replies)
  0 siblings, 7 replies; 45+ messages in thread
From: Simon Glass @ 2012-04-13 18:29 UTC (permalink / raw)
  To: u-boot

This series adds NAND flash support to Tegra and enables it on Seaboard.

Included here is a proposed device tree binding with most of the properties
private to "nvidia,". The binding includes information about the NAND
controller as well as the connected NAND device. The Seaboard has a
Hynix HY27UF4G2B.

The driver supports ECC-based access and uses DMA and NAND acceleration
features of the Tegra SOC to provide access at reasonable speed.

Changes in v2:
- Add new patch to align default buffers in nand_base
- Added comment about the behaviour of the 'resp' register
- Call set_bus_width_page_size() at init to report errors earlier
- Change set_bus_width_page_size() to return an error when needed
- Change timing structure member to u32 to match device tree
- Check for supported bus width in board_nand_init()
- Fix tegra nand header file to remove BIT defines
- Implement a dummy nand_select_chip() instead of nand_hwcontro()
- Make nand_command() display an error on an unknown command
- Minor code tidy-ups in driver for style
- Move cache logic into a separate dma_prepare() function
- Remove CMD_TRANS_SIZE_BYTESx enum
- Remove space after casts
- Remove use of 'register' variables
- Rename struct nand_info to struct nand_drv to avoid nand_info_t confusion
- Support 4096 byte page devices, drop 1024 and 2048
- Tidy up nand_waitfor_cmd_completion() logic
- Update NAND binding to add "nvidia," prefix
- Use s32 for device tree integer values

Jim Lin (1):
  tegra: nand: Add Tegra NAND driver

Simon Glass (6):
  nand: Try to align the default buffers
  fdt: Add debugging to fdtdec_get_int/addr()
  tegra: Add NAND support to funcmux
  tegra: fdt: Add NAND controller binding and definitions
  tegra: fdt: Add NAND definitions to fdt
  tegra: Enable NAND on Seaboard

 arch/arm/cpu/armv7/tegra2/funcmux.c           |    7 +
 arch/arm/dts/tegra20.dtsi                     |    6 +
 arch/arm/include/asm/arch-tegra2/funcmux.h    |    3 +
 arch/arm/include/asm/arch-tegra2/tegra2.h     |    1 +
 board/nvidia/dts/tegra2-seaboard.dts          |   15 +
 doc/device-tree-bindings/nand/nvidia-nand.txt |   68 ++
 drivers/mtd/nand/Makefile                     |    1 +
 drivers/mtd/nand/nand_base.c                  |    3 +-
 drivers/mtd/nand/tegra2_nand.c                | 1094 +++++++++++++++++++++++++
 drivers/mtd/nand/tegra2_nand.h                |  257 ++++++
 include/configs/seaboard.h                    |    9 +
 include/fdtdec.h                              |    1 +
 include/linux/mtd/nand.h                      |    7 +-
 lib/fdtdec.c                                  |   23 +-
 14 files changed, 1485 insertions(+), 10 deletions(-)
 create mode 100644 doc/device-tree-bindings/nand/nvidia-nand.txt
 create mode 100644 drivers/mtd/nand/tegra2_nand.c
 create mode 100644 drivers/mtd/nand/tegra2_nand.h

-- 
1.7.7.3

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

* [U-Boot] [PATCH v2 1/7] nand: Try to align the default buffers
  2012-04-13 18:29 [U-Boot] [PATCH v2 0/7] tegra: Add NAND flash support Simon Glass
@ 2012-04-13 18:29 ` Simon Glass
  2012-04-13 18:37   ` Scott Wood
  2012-04-13 18:29   ` [U-Boot] " Simon Glass
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 45+ messages in thread
From: Simon Glass @ 2012-04-13 18:29 UTC (permalink / raw)
  To: u-boot

The NAND layer needs to use cache-aligned buffers by default. Towards this
goal. align the default buffers and their members according to the minimum
DMA alignment defined for the architecture.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Add new patch to align default buffers in nand_base

 drivers/mtd/nand/nand_base.c |    3 ++-
 include/linux/mtd/nand.h     |    7 ++++---
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 44f7b91..7bfc29e 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2935,7 +2935,8 @@ int nand_scan_tail(struct mtd_info *mtd)
 	struct nand_chip *chip = mtd->priv;
 
 	if (!(chip->options & NAND_OWN_BUFFERS))
-		chip->buffers = kmalloc(sizeof(*chip->buffers), GFP_KERNEL);
+		chip->buffers = memalign(ARCH_DMA_MINALIGN,
+					 sizeof(*chip->buffers));
 	if (!chip->buffers)
 		return -ENOMEM;
 
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index da6fa18..ae0bdf6 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -391,9 +391,10 @@ struct nand_ecc_ctrl {
  * consecutive order.
  */
 struct nand_buffers {
-	uint8_t	ecccalc[NAND_MAX_OOBSIZE];
-	uint8_t	ecccode[NAND_MAX_OOBSIZE];
-	uint8_t databuf[NAND_MAX_PAGESIZE + NAND_MAX_OOBSIZE];
+	uint8_t	ecccalc[ALIGN(NAND_MAX_OOBSIZE, ARCH_DMA_MINALIGN)];
+	uint8_t	ecccode[ALIGN(NAND_MAX_OOBSIZE, ARCH_DMA_MINALIGN)];
+	uint8_t databuf[ALIGN(NAND_MAX_PAGESIZE + NAND_MAX_OOBSIZE,
+			      ARCH_DMA_MINALIGN)];
 };
 
 /**
-- 
1.7.7.3

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

* [PATCH v2 2/7] fdt: Add debugging to fdtdec_get_int/addr()
  2012-04-13 18:29 [U-Boot] [PATCH v2 0/7] tegra: Add NAND flash support Simon Glass
@ 2012-04-13 18:29   ` Simon Glass
  2012-04-13 18:29   ` [U-Boot] " Simon Glass
                     ` (5 subsequent siblings)
  6 siblings, 0 replies; 45+ messages in thread
From: Simon Glass @ 2012-04-13 18:29 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Devicetree Discuss, Jerry Van Baren, Tom Warren, Scott Wood

The new debugging shows the value of integers and addresses read
from the device tree.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 lib/fdtdec.c |   22 ++++++++++++++++------
 1 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 42c3e89..3885634 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -79,11 +79,16 @@ fdt_addr_t fdtdec_get_addr(const void *blob, int node,
 	const fdt_addr_t *cell;
 	int len;
 
-	debug("get_addr: %s\n", prop_name);
+	debug("%s: %s\n", __func__, prop_name);
 	cell = fdt_getprop(blob, node, prop_name, &len);
 	if (cell && (len == sizeof(fdt_addr_t) ||
-			len == sizeof(fdt_addr_t) * 2))
-		return fdt_addr_to_cpu(*cell);
+			len == sizeof(fdt_addr_t) * 2)) {
+		fdt_addr_t addr = fdt_addr_to_cpu(*cell);
+
+		debug("%p\n", (void *)addr);
+		return addr;
+	}
+	debug("(not found)\n");
 	return FDT_ADDR_T_NONE;
 }
 
@@ -93,10 +98,15 @@ s32 fdtdec_get_int(const void *blob, int node, const char *prop_name,
 	const s32 *cell;
 	int len;
 
-	debug("get_size: %s\n", prop_name);
+	debug("%s: %s: ", __func__, prop_name);
 	cell = fdt_getprop(blob, node, prop_name, &len);
-	if (cell && len >= sizeof(s32))
-		return fdt32_to_cpu(cell[0]);
+	if (cell && len >= sizeof(s32)) {
+		s32 val = fdt32_to_cpu(cell[0]);
+
+		debug("%#x (%d)\n", val, val);
+		return val;
+	}
+	debug("(not found)\n");
 	return default_val;
 }
 
-- 
1.7.7.3

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

* [U-Boot] [PATCH v2 2/7] fdt: Add debugging to fdtdec_get_int/addr()
@ 2012-04-13 18:29   ` Simon Glass
  0 siblings, 0 replies; 45+ messages in thread
From: Simon Glass @ 2012-04-13 18:29 UTC (permalink / raw)
  To: u-boot

The new debugging shows the value of integers and addresses read
from the device tree.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 lib/fdtdec.c |   22 ++++++++++++++++------
 1 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 42c3e89..3885634 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -79,11 +79,16 @@ fdt_addr_t fdtdec_get_addr(const void *blob, int node,
 	const fdt_addr_t *cell;
 	int len;
 
-	debug("get_addr: %s\n", prop_name);
+	debug("%s: %s\n", __func__, prop_name);
 	cell = fdt_getprop(blob, node, prop_name, &len);
 	if (cell && (len == sizeof(fdt_addr_t) ||
-			len == sizeof(fdt_addr_t) * 2))
-		return fdt_addr_to_cpu(*cell);
+			len == sizeof(fdt_addr_t) * 2)) {
+		fdt_addr_t addr = fdt_addr_to_cpu(*cell);
+
+		debug("%p\n", (void *)addr);
+		return addr;
+	}
+	debug("(not found)\n");
 	return FDT_ADDR_T_NONE;
 }
 
@@ -93,10 +98,15 @@ s32 fdtdec_get_int(const void *blob, int node, const char *prop_name,
 	const s32 *cell;
 	int len;
 
-	debug("get_size: %s\n", prop_name);
+	debug("%s: %s: ", __func__, prop_name);
 	cell = fdt_getprop(blob, node, prop_name, &len);
-	if (cell && len >= sizeof(s32))
-		return fdt32_to_cpu(cell[0]);
+	if (cell && len >= sizeof(s32)) {
+		s32 val = fdt32_to_cpu(cell[0]);
+
+		debug("%#x (%d)\n", val, val);
+		return val;
+	}
+	debug("(not found)\n");
 	return default_val;
 }
 
-- 
1.7.7.3

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

* [U-Boot] [PATCH v2 3/7] tegra: Add NAND support to funcmux
  2012-04-13 18:29 [U-Boot] [PATCH v2 0/7] tegra: Add NAND flash support Simon Glass
  2012-04-13 18:29 ` [U-Boot] [PATCH v2 1/7] nand: Try to align the default buffers Simon Glass
  2012-04-13 18:29   ` [U-Boot] " Simon Glass
@ 2012-04-13 18:29 ` Simon Glass
  2012-04-13 18:29   ` [U-Boot] " Simon Glass
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 45+ messages in thread
From: Simon Glass @ 2012-04-13 18:29 UTC (permalink / raw)
  To: u-boot

Add selection of NAND flash pins to the funcmux.

Signed-off-by: Simon Glass <sjg@chromium.org>
Acked-by: Stephen Warren <swarren@nvidia.com>
---

 arch/arm/cpu/armv7/tegra2/funcmux.c        |    7 +++++++
 arch/arm/include/asm/arch-tegra2/funcmux.h |    3 +++
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/arm/cpu/armv7/tegra2/funcmux.c b/arch/arm/cpu/armv7/tegra2/funcmux.c
index c1d2dfe..8931d5e 100644
--- a/arch/arm/cpu/armv7/tegra2/funcmux.c
+++ b/arch/arm/cpu/armv7/tegra2/funcmux.c
@@ -169,6 +169,13 @@ int funcmux_select(enum periph_id id, int config)
 		}
 		break;
 
+	case PERIPH_ID_NDFLASH:
+		if (config == FUNCMUX_NDFLASH_ATC) {
+			pinmux_set_func(PINGRP_ATC, PMUX_FUNC_NAND);
+			pinmux_tristate_disable(PINGRP_ATC);
+		}
+		break;
+
 	default:
 		debug("%s: invalid periph_id %d", __func__, id);
 		return -1;
diff --git a/arch/arm/include/asm/arch-tegra2/funcmux.h b/arch/arm/include/asm/arch-tegra2/funcmux.h
index ae73c72..9419522 100644
--- a/arch/arm/include/asm/arch-tegra2/funcmux.h
+++ b/arch/arm/include/asm/arch-tegra2/funcmux.h
@@ -47,6 +47,9 @@ enum {
 	FUNCMUX_SDMMC4_ATC_ATD_8BIT = 0,
 	FUNCMUX_SDMMC4_ATB_GMA_4_BIT,
 	FUNCMUX_SDMMC4_ATB_GMA_GME_8_BIT,
+
+	/* NAND flags */
+	FUNCMUX_NDFLASH_ATC = 0,
 };
 
 /**
-- 
1.7.7.3

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

* [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
  2012-04-13 18:29 [U-Boot] [PATCH v2 0/7] tegra: Add NAND flash support Simon Glass
@ 2012-04-13 18:29   ` Simon Glass
  2012-04-13 18:29   ` [U-Boot] " Simon Glass
                     ` (5 subsequent siblings)
  6 siblings, 0 replies; 45+ messages in thread
From: Simon Glass @ 2012-04-13 18:29 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Jerry Van Baren, Tom Warren, Scott Wood, Devicetree Discuss

Add a NAND controller along with a bindings file for review.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Update NAND binding to add "nvidia," prefix

 arch/arm/dts/tegra20.dtsi                     |    6 ++
 doc/device-tree-bindings/nand/nvidia-nand.txt |   68 +++++++++++++++++++++++++
 2 files changed, 74 insertions(+), 0 deletions(-)
 create mode 100644 doc/device-tree-bindings/nand/nvidia-nand.txt

diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
index bc64f42..7be0462 100644
--- a/arch/arm/dts/tegra20.dtsi
+++ b/arch/arm/dts/tegra20.dtsi
@@ -200,4 +200,10 @@
 		reg = <0x7000f400 0x200>;
 	};
 
+	nand: nand-controller@0x70008000 {
+		#address-cells = <0>;
+		#size-cells = <0>;
+		compatible = "nvidia,tegra20-nand";
+		reg = <0x70008000 0x100>;
+	};
 };
diff --git a/doc/device-tree-bindings/nand/nvidia-nand.txt b/doc/device-tree-bindings/nand/nvidia-nand.txt
new file mode 100644
index 0000000..b19ce8e
--- /dev/null
+++ b/doc/device-tree-bindings/nand/nvidia-nand.txt
@@ -0,0 +1,68 @@
+NAND Flash
+----------
+
+(there isn't yet a generic binding in Linux, so this describes what is in
+U-Boot)
+
+The device node for a NAND flash device is as described in the document
+"Open Firmware Recommended Practice : Universal Serial Bus" with the
+following modifications and additions :
+
+Required properties :
+ - compatible : Should be "manufacture,device", "nand-flash"
+ - nvidia,page-data-bytes : Number of bytes in the data area
+ - nvidia,page-spare-bytes : * Number of bytes in spare area
+       spare area = skipped-spare-bytes + data-ecc-bytes + tag-bytes
+			+ tag-ecc-bytes
+ - nvidia,skipped-spare-bytes : Number of bytes to skip at start of spare area
+	(these are typically used for bad block maintenance)
+ - nvidia,data-ecc-bytes : Number of ECC bytes for data area
+ - nvidia,tag-bytes :Number of tag bytes in spare area
+ - nvidia,tag-ecc-bytes : Number ECC bytes to be generated for tag bytes
+
+(replace -bytes with -size or -length?)
+
+This node should sit inside its controller.
+
+
+Nvidia NAND Controller
+----------------------
+
+The device node for a NAND flash controller is as described in the document
+"Open Firmware Recommended Practice : Universal Serial Bus" with the
+following modifications and additions :
+
+Optional properties:
+
+wp-gpio : GPIO of write-protect line, three cells in the format:
+		phandle, parameter, flags
+nvidia,,width : bus width of the NAND device in bits
+
+For now here is something specific to the Nvidia controller, with naming
+based on Nvidia's original (non-fdt) NAND driver:
+
+ - nvidia,nand-timing : Timing parameters for the NAND. Each is in ns.
+	Order is: MAX_TRP_TREA, TWB, Max(tCS, tCH, tALS, tALH),
+	TWHR, Max(tCS, tCH, tALS, tALH), TWH, TWP, TRH, TADL
+
+	MAX_TRP_TREA is:
+		non-EDO mode: Max(tRP, tREA) + 6ns
+		EDO mode: tRP timing
+
+Example:
+
+nand-controller@0x70008000 {
+	compatible = "nvidia,tegra20-nand";
+	wp-gpios = <&gpio 59 0>;		/* PH3 */
+	nvidia,width = <8>;
+	nvidia,timing = <26 100 20 80 20 10 12 10 70>;
+	nand@0 {
+		compatible = "hynix,hy27uf4g2b", "nand-flash";
+		nvidia,page-data-bytes = <2048>;
+		nvidia,tag-ecc-bytes = <4>;
+		nvidia,tag-bytes = <20>;
+		nvidia,data-ecc-bytes = <36>;
+		nvidia,skipped-spare-bytes = <4>;
+		nvidia,page-spare-bytes = <64>;
+	};
+};
-- 
1.7.7.3

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

* [U-Boot] [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
@ 2012-04-13 18:29   ` Simon Glass
  0 siblings, 0 replies; 45+ messages in thread
From: Simon Glass @ 2012-04-13 18:29 UTC (permalink / raw)
  To: u-boot

Add a NAND controller along with a bindings file for review.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Update NAND binding to add "nvidia," prefix

 arch/arm/dts/tegra20.dtsi                     |    6 ++
 doc/device-tree-bindings/nand/nvidia-nand.txt |   68 +++++++++++++++++++++++++
 2 files changed, 74 insertions(+), 0 deletions(-)
 create mode 100644 doc/device-tree-bindings/nand/nvidia-nand.txt

diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
index bc64f42..7be0462 100644
--- a/arch/arm/dts/tegra20.dtsi
+++ b/arch/arm/dts/tegra20.dtsi
@@ -200,4 +200,10 @@
 		reg = <0x7000f400 0x200>;
 	};
 
+	nand: nand-controller at 0x70008000 {
+		#address-cells = <0>;
+		#size-cells = <0>;
+		compatible = "nvidia,tegra20-nand";
+		reg = <0x70008000 0x100>;
+	};
 };
diff --git a/doc/device-tree-bindings/nand/nvidia-nand.txt b/doc/device-tree-bindings/nand/nvidia-nand.txt
new file mode 100644
index 0000000..b19ce8e
--- /dev/null
+++ b/doc/device-tree-bindings/nand/nvidia-nand.txt
@@ -0,0 +1,68 @@
+NAND Flash
+----------
+
+(there isn't yet a generic binding in Linux, so this describes what is in
+U-Boot)
+
+The device node for a NAND flash device is as described in the document
+"Open Firmware Recommended Practice : Universal Serial Bus" with the
+following modifications and additions :
+
+Required properties :
+ - compatible : Should be "manufacture,device", "nand-flash"
+ - nvidia,page-data-bytes : Number of bytes in the data area
+ - nvidia,page-spare-bytes : * Number of bytes in spare area
+       spare area = skipped-spare-bytes + data-ecc-bytes + tag-bytes
+			+ tag-ecc-bytes
+ - nvidia,skipped-spare-bytes : Number of bytes to skip at start of spare area
+	(these are typically used for bad block maintenance)
+ - nvidia,data-ecc-bytes : Number of ECC bytes for data area
+ - nvidia,tag-bytes :Number of tag bytes in spare area
+ - nvidia,tag-ecc-bytes : Number ECC bytes to be generated for tag bytes
+
+(replace -bytes with -size or -length?)
+
+This node should sit inside its controller.
+
+
+Nvidia NAND Controller
+----------------------
+
+The device node for a NAND flash controller is as described in the document
+"Open Firmware Recommended Practice : Universal Serial Bus" with the
+following modifications and additions :
+
+Optional properties:
+
+wp-gpio : GPIO of write-protect line, three cells in the format:
+		phandle, parameter, flags
+nvidia,,width : bus width of the NAND device in bits
+
+For now here is something specific to the Nvidia controller, with naming
+based on Nvidia's original (non-fdt) NAND driver:
+
+ - nvidia,nand-timing : Timing parameters for the NAND. Each is in ns.
+	Order is: MAX_TRP_TREA, TWB, Max(tCS, tCH, tALS, tALH),
+	TWHR, Max(tCS, tCH, tALS, tALH), TWH, TWP, TRH, TADL
+
+	MAX_TRP_TREA is:
+		non-EDO mode: Max(tRP, tREA) + 6ns
+		EDO mode: tRP timing
+
+Example:
+
+nand-controller at 0x70008000 {
+	compatible = "nvidia,tegra20-nand";
+	wp-gpios = <&gpio 59 0>;		/* PH3 */
+	nvidia,width = <8>;
+	nvidia,timing = <26 100 20 80 20 10 12 10 70>;
+	nand at 0 {
+		compatible = "hynix,hy27uf4g2b", "nand-flash";
+		nvidia,page-data-bytes = <2048>;
+		nvidia,tag-ecc-bytes = <4>;
+		nvidia,tag-bytes = <20>;
+		nvidia,data-ecc-bytes = <36>;
+		nvidia,skipped-spare-bytes = <4>;
+		nvidia,page-spare-bytes = <64>;
+	};
+};
-- 
1.7.7.3

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

* [PATCH v2 5/7] tegra: fdt: Add NAND definitions to fdt
  2012-04-13 18:29 [U-Boot] [PATCH v2 0/7] tegra: Add NAND flash support Simon Glass
@ 2012-04-13 18:29     ` Simon Glass
  2012-04-13 18:29   ` [U-Boot] " Simon Glass
                       ` (5 subsequent siblings)
  6 siblings, 0 replies; 45+ messages in thread
From: Simon Glass @ 2012-04-13 18:29 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Jerry Van Baren, Tom Warren, Scott Wood, Devicetree Discuss

Add a flash node to handle the NAND, including memory timings and
page / block size information.

Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
Changes in v2:
- Update NAND binding to add "nvidia," prefix

 board/nvidia/dts/tegra2-seaboard.dts |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/board/nvidia/dts/tegra2-seaboard.dts b/board/nvidia/dts/tegra2-seaboard.dts
index 08ada8c..a11bceb 100644
--- a/board/nvidia/dts/tegra2-seaboard.dts
+++ b/board/nvidia/dts/tegra2-seaboard.dts
@@ -126,4 +126,19 @@
 				0x00000000 0x00000000 0x00000000 0x00000000 >;
 		};
 	};
+
+	nand-controller@0x70008000 {
+		wp-gpios = <&gpio 59 0>;		/* PH3 */
+		nvidia,width = <8>;
+		nvidia,timing = <26 100 20 80 20 10 12 10 70>;
+		nand@0 {
+			compatible = "hynix,hy27uf4g2b", "nand-flash";
+			nvidia,page-data-bytes = <2048>;
+			nvidia,tag-ecc-bytes = <4>;
+			nvidia,tag-bytes = <20>;
+			nvidia,data-ecc-bytes = <36>;
+			nvidia,skipped-spare-bytes = <4>;
+			nvidia,page-spare-bytes = <64>;
+		};
+	};
 };
-- 
1.7.7.3

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

* [U-Boot] [PATCH v2 5/7] tegra: fdt: Add NAND definitions to fdt
@ 2012-04-13 18:29     ` Simon Glass
  0 siblings, 0 replies; 45+ messages in thread
From: Simon Glass @ 2012-04-13 18:29 UTC (permalink / raw)
  To: u-boot

Add a flash node to handle the NAND, including memory timings and
page / block size information.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Update NAND binding to add "nvidia," prefix

 board/nvidia/dts/tegra2-seaboard.dts |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/board/nvidia/dts/tegra2-seaboard.dts b/board/nvidia/dts/tegra2-seaboard.dts
index 08ada8c..a11bceb 100644
--- a/board/nvidia/dts/tegra2-seaboard.dts
+++ b/board/nvidia/dts/tegra2-seaboard.dts
@@ -126,4 +126,19 @@
 				0x00000000 0x00000000 0x00000000 0x00000000 >;
 		};
 	};
+
+	nand-controller at 0x70008000 {
+		wp-gpios = <&gpio 59 0>;		/* PH3 */
+		nvidia,width = <8>;
+		nvidia,timing = <26 100 20 80 20 10 12 10 70>;
+		nand at 0 {
+			compatible = "hynix,hy27uf4g2b", "nand-flash";
+			nvidia,page-data-bytes = <2048>;
+			nvidia,tag-ecc-bytes = <4>;
+			nvidia,tag-bytes = <20>;
+			nvidia,data-ecc-bytes = <36>;
+			nvidia,skipped-spare-bytes = <4>;
+			nvidia,page-spare-bytes = <64>;
+		};
+	};
 };
-- 
1.7.7.3

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

* [U-Boot] [PATCH v2 6/7] tegra: nand: Add Tegra NAND driver
  2012-04-13 18:29 [U-Boot] [PATCH v2 0/7] tegra: Add NAND flash support Simon Glass
                   ` (4 preceding siblings ...)
       [not found] ` <1334341777-2681-1-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
@ 2012-04-13 18:29 ` Simon Glass
  2012-04-13 18:29 ` [U-Boot] [PATCH v2 7/7] tegra: Enable NAND on Seaboard Simon Glass
  6 siblings, 0 replies; 45+ messages in thread
From: Simon Glass @ 2012-04-13 18:29 UTC (permalink / raw)
  To: u-boot

From: Jim Lin <jilin@nvidia.com>

A device tree is used to configure the NAND, including memory
timings and block/pages sizes.

If this node is not present or is disabled, then NAND will not
be initialized.

Signed-off-by: Jim Lin <jilin@nvidia.com>
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Added comment about the behaviour of the 'resp' register
- Call set_bus_width_page_size() at init to report errors earlier
- Change set_bus_width_page_size() to return an error when needed
- Change timing structure member to u32 to match device tree
- Check for supported bus width in board_nand_init()
- Fix tegra nand header file to remove BIT defines
- Implement a dummy nand_select_chip() instead of nand_hwcontro()
- Make nand_command() display an error on an unknown command
- Minor code tidy-ups in driver for style
- Move cache logic into a separate dma_prepare() function
- Remove CMD_TRANS_SIZE_BYTESx enum
- Remove space after casts
- Remove use of 'register' variables
- Rename struct nand_info to struct nand_drv to avoid nand_info_t confusion
- Support 4096 byte page devices, drop 1024 and 2048
- Tidy up nand_waitfor_cmd_completion() logic
- Update NAND binding to add "nvidia," prefix
- Use s32 for device tree integer values

 arch/arm/include/asm/arch-tegra2/tegra2.h |    1 +
 drivers/mtd/nand/Makefile                 |    1 +
 drivers/mtd/nand/tegra2_nand.c            | 1094 +++++++++++++++++++++++++++++
 drivers/mtd/nand/tegra2_nand.h            |  257 +++++++
 include/fdtdec.h                          |    1 +
 lib/fdtdec.c                              |    1 +
 6 files changed, 1355 insertions(+), 0 deletions(-)
 create mode 100644 drivers/mtd/nand/tegra2_nand.c
 create mode 100644 drivers/mtd/nand/tegra2_nand.h

diff --git a/arch/arm/include/asm/arch-tegra2/tegra2.h b/arch/arm/include/asm/arch-tegra2/tegra2.h
index d4ada10..a080b63 100644
--- a/arch/arm/include/asm/arch-tegra2/tegra2.h
+++ b/arch/arm/include/asm/arch-tegra2/tegra2.h
@@ -39,6 +39,7 @@
 #define NV_PA_APB_UARTC_BASE	(NV_PA_APB_MISC_BASE + 0x6200)
 #define NV_PA_APB_UARTD_BASE	(NV_PA_APB_MISC_BASE + 0x6300)
 #define NV_PA_APB_UARTE_BASE	(NV_PA_APB_MISC_BASE + 0x6400)
+#define TEGRA2_NAND_BASE	(NV_PA_APB_MISC_BASE + 0x8000)
 #define TEGRA2_SPI_BASE		(NV_PA_APB_MISC_BASE + 0xC380)
 #define TEGRA2_PMC_BASE		(NV_PA_APB_MISC_BASE + 0xE400)
 #define TEGRA2_FUSE_BASE	(NV_PA_APB_MISC_BASE + 0xF800)
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 1d1b628..7efccf8 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -61,6 +61,7 @@ COBJS-$(CONFIG_NAND_NOMADIK) += nomadik.o
 COBJS-$(CONFIG_NAND_S3C2410) += s3c2410_nand.o
 COBJS-$(CONFIG_NAND_S3C64XX) += s3c64xx.o
 COBJS-$(CONFIG_NAND_SPEAR) += spr_nand.o
+COBJS-$(CONFIG_TEGRA2_NAND) += tegra2_nand.o
 COBJS-$(CONFIG_NAND_OMAP_GPMC) += omap_gpmc.o
 COBJS-$(CONFIG_NAND_PLAT) += nand_plat.o
 endif
diff --git a/drivers/mtd/nand/tegra2_nand.c b/drivers/mtd/nand/tegra2_nand.c
new file mode 100644
index 0000000..d73e61b
--- /dev/null
+++ b/drivers/mtd/nand/tegra2_nand.c
@@ -0,0 +1,1094 @@
+/*
+ * Copyright (c) 2011 The Chromium OS Authors.
+ * (C) Copyright 2011 NVIDIA Corporation <www.nvidia.com>
+ * (C) Copyright 2006 Detlev Zundel, dzu at denx.de
+ * (C) Copyright 2006 DENX Software Engineering
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+#define DEBUG
+
+#include <common.h>
+#include <asm/io.h>
+#include <nand.h>
+#include <asm/arch/clk_rst.h>
+#include <asm/arch/clock.h>
+#include <asm/arch/funcmux.h>
+#include <asm/arch/gpio.h>
+#include <asm/errno.h>
+#include <asm-generic/gpio.h>
+#include <fdtdec.h>
+#include "tegra2_nand.h"
+
+DECLARE_GLOBAL_DATA_PTR;
+
+#define NAND_CMD_TIMEOUT_MS		10
+
+static struct nand_ecclayout eccoob = {
+	.eccpos = {
+		4,  5,  6,  7,  8,  9,  10, 11, 12,
+		13, 14, 15, 16, 17, 18, 19, 20, 21,
+		22, 23, 24, 25, 26, 27, 28, 29, 30,
+		31, 32, 33, 34, 35, 36, 37, 38, 39,
+		60, 61, 62, 63,
+	},
+};
+
+enum {
+	ECC_OK,
+	ECC_TAG_ERROR = 1 << 0,
+	ECC_DATA_ERROR = 1 << 1
+};
+
+/* Timing parameters */
+enum {
+	FDT_NAND_MAX_TRP_TREA,
+	FDT_NAND_TWB,
+	FDT_NAND_MAX_TCR_TAR_TRR,
+	FDT_NAND_TWHR,
+	FDT_NAND_MAX_TCS_TCH_TALS_TALH,
+	FDT_NAND_TWH,
+	FDT_NAND_TWP,
+	FDT_NAND_TRH,
+	FDT_NAND_TADL,
+
+	FDT_NAND_TIMING_COUNT
+};
+
+/* Information about an attached NAND chip */
+struct fdt_nand {
+	struct nand_ctlr *reg;
+	int enabled;		/* 1 to enable, 0 to disable */
+	struct fdt_gpio_state wp_gpio;	/* write-protect GPIO */
+	s32 width;		/* bit width, normally 8 */
+	s32 tag_ecc_bytes;	/* ECC bytes to be generated for tag bytes */
+	s32 tag_bytes;		/* Tag bytes in spare area */
+	s32 data_ecc_bytes;	/* ECC bytes for data area */
+	s32 skipped_spare_bytes;
+	/*
+	 * How many bytes in spare area:
+	 * spare area = skipped bytes + ECC bytes of data area
+	 * + tag bytes + ECC bytes of tag bytes
+	 */
+	s32 page_spare_bytes;
+	s32 page_data_bytes;	/* Bytes in data area */
+	u32 timing[FDT_NAND_TIMING_COUNT];
+};
+
+struct nand_drv {
+	struct nand_ctlr *reg;
+
+	/*
+	* When running in PIO mode to get READ ID bytes from register
+	* RESP_0, we need this variable as an index to know which byte in
+	* register RESP_0 should be read.
+	* Because common code in nand_base.c invokes read_byte function two
+	* times for NAND_CMD_READID.
+	* And our controller returns 4 bytes@once in register RESP_0.
+	*/
+	int pio_byte_index;
+	struct fdt_nand config;
+};
+
+static struct nand_drv nand_ctrl;
+
+#ifdef CONFIG_SYS_DCACHE_OFF
+static inline void dma_prepare(void *start, unsigned long length,
+			       int is_writing)
+{
+}
+#else
+/**
+ * Prepare for a DMA transaction
+ *
+ * For a write we flush out our data. For a read we invalidate, since we
+ * need to do this before we read from the buffer after the DMA has
+ * completed, so may as well do it now.
+ *
+ * @param start		Start address for DMA buffer (should be cache-aligned)
+ * @param length	Length of DMA buffer in bytes
+ * @param is_writing	0 if reading, non-zero if writing
+ */
+static void dma_prepare(void *start, unsigned long length, int is_writing)
+{
+	unsigned long addr = (unsigned long)start;
+
+	if (is_writing)
+		flush_dcache_range(addr, addr + length);
+	else
+		invalidate_dcache_range(addr, addr + length);
+}
+#endif
+
+/**
+ * Wait for command completion
+ *
+ * @param reg	nand_ctlr structure
+ * @return
+ *	1 - Command completed
+ *	0 - Timeout
+ */
+static int nand_waitfor_cmd_completion(struct nand_ctlr *reg)
+{
+	u32 reg_val;
+	int running;
+	int i;
+
+	for (i = 0; i < NAND_CMD_TIMEOUT_MS * 1000; i++) {
+		if ((readl(&reg->command) & CMD_GO) ||
+				!(readl(&reg->status) & STATUS_RBSY0) ||
+				!(readl(&reg->isr) & ISR_IS_CMD_DONE)) {
+			udelay(1);
+			continue;
+		}
+		reg_val = readl(&reg->dma_mst_ctrl);
+		/*
+		 * If DMA_MST_CTRL_EN_A_ENABLE or DMA_MST_CTRL_EN_B_ENABLE
+		 * is set, that means DMA engine is running.
+		 *
+		 * Then we have to wait until DMA_MST_CTRL_IS_DMA_DONE
+		 * is cleared, indicating DMA transfer completion.
+		 */
+		running = reg_val & (DMA_MST_CTRL_EN_A_ENABLE |
+				DMA_MST_CTRL_EN_B_ENABLE);
+		if (!running || (reg_val & DMA_MST_CTRL_IS_DMA_DONE))
+			return 1;
+		udelay(1);
+	}
+	return 0;
+}
+
+/**
+ * [DEFAULT] Read one byte from the chip
+ *
+ * @param mtd	MTD device structure
+ * @return	data byte
+ *
+ * Default read function for 8bit bus-width
+ */
+static uint8_t read_byte(struct mtd_info *mtd)
+{
+	struct nand_chip *chip = mtd->priv;
+	u32 dword_read;
+	struct nand_drv *info;
+
+	info = (struct nand_drv *)chip->priv;
+	if (info->pio_byte_index > 3)
+		return 0;
+
+	/* We can read this data multiple times and get the same word */
+	dword_read = readl(&info->reg->resp);
+	dword_read = dword_read >> (8 * info->pio_byte_index);
+	info->pio_byte_index++;
+	return (uint8_t)dword_read;
+}
+
+/**
+ * [DEFAULT] Write buffer to chip
+ *
+ * @param mtd	MTD device structure
+ * @param buf	data buffer
+ * @param len	number of bytes to write
+ *
+ * Default write function for 8bit bus-width
+ */
+static void write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
+{
+	int i, j, l;
+	struct nand_chip *chip = mtd->priv;
+	struct nand_drv *info;
+
+	info = (struct nand_drv *)chip->priv;
+
+	for (i = 0; i < len / 4; i++) {
+		l = ((int *)buf)[i];
+		writel(l, &info->reg->resp);
+		writel(CMD_GO | CMD_PIO | CMD_TX |
+			((4 - 1) << CMD_TRANS_SIZE_SHIFT)
+			| CMD_A_VALID | CMD_CE0,
+			&info->reg->command);
+
+		if (!nand_waitfor_cmd_completion(info->reg))
+			printf("Command timeout during write_buf\n");
+	}
+	if (len & 3) {
+		l = 0;
+		for (j = 0; j < (len & 3); j++)
+			l |= (((int)buf[i * 4 + j]) << (8 * j));
+
+		writel(l, &info->reg->resp);
+		writel(CMD_GO | CMD_PIO | CMD_TX |
+			(((len & 3) - 1) << CMD_TRANS_SIZE_SHIFT) |
+			CMD_A_VALID | CMD_CE0,
+			&info->reg->command);
+		if (!nand_waitfor_cmd_completion(info->reg))
+			printf("Command timeout during write_buf\n");
+	}
+}
+
+/**
+ * [DEFAULT] Read chip data into buffer
+ *
+ * @param mtd	MTD device structure
+ * @param buf	buffer to store date
+ * @param len	number of bytes to read
+ *
+ * Default read function for 8bit bus-width
+ */
+static void read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
+{
+	int i, j, l;
+	struct nand_chip *chip = mtd->priv;
+	int *buf_dword;
+	struct nand_drv *info;
+
+	info = (struct nand_drv *)chip->priv;
+
+	buf_dword = (int *)buf;
+	for (i = 0; i < len / 4; i++) {
+		writel(CMD_GO | CMD_PIO | CMD_RX |
+			((4 - 1) << CMD_TRANS_SIZE_SHIFT)
+			| CMD_A_VALID | CMD_CE0,
+			&info->reg->command);
+		if (!nand_waitfor_cmd_completion(info->reg))
+			printf("Command timeout during read_buf\n");
+		l = readl(&info->reg->resp);
+		buf_dword[i] = l;
+	}
+	if (len & 3) {
+		writel(CMD_GO | CMD_PIO | CMD_RX |
+			(((len & 3) - 1) << CMD_TRANS_SIZE_SHIFT) |
+			CMD_A_VALID | CMD_CE0,
+			&info->reg->command);
+		if (!nand_waitfor_cmd_completion(info->reg))
+			printf("Command timeout during read_buf\n");
+		l = readl(&info->reg->resp);
+		for (j = 0; j < (len & 3); j++)
+			buf[i * 4 + j] = (char) (l >> (8 * j));
+	}
+}
+
+/**
+ * Check NAND status to see if it is ready or not
+ *
+ * @param mtd	MTD device structure
+ * @return
+ *	1 - ready
+ *	0 - not ready
+ */
+static int nand_dev_ready(struct mtd_info *mtd)
+{
+	struct nand_chip *chip = mtd->priv;
+	int reg_val;
+	struct nand_drv *info;
+
+	info = (struct nand_drv *)chip->priv;
+
+	reg_val = readl(&info->reg->status);
+	if (reg_val & STATUS_RBSY0)
+		return 1;
+	else
+		return 0;
+}
+
+/* Dummy implementation: we don't support multiple chips */
+static void nand_select_chip(struct mtd_info *mtd, int chipnr)
+{
+	switch (chipnr) {
+	case -1:
+	case 0:
+		break;
+
+	default:
+		BUG();
+	}
+}
+
+/**
+ * Clear all interrupt status bits
+ *
+ * @param reg	nand_ctlr structure
+ */
+static void nand_clear_interrupt_status(struct nand_ctlr *reg)
+{
+	u32 reg_val;
+
+	/* Clear interrupt status */
+	reg_val = readl(&reg->isr);
+	writel(reg_val, &reg->isr);
+}
+
+/**
+ * [DEFAULT] Send command to NAND device
+ *
+ * @param mtd		MTD device structure
+ * @param command	the command to be sent
+ * @param column	the column address for this command, -1 if none
+ * @param page_addr	the page address for this command, -1 if none
+ */
+static void nand_command(struct mtd_info *mtd, unsigned int command,
+	int column, int page_addr)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct nand_drv *info;
+
+	info = (struct nand_drv *)chip->priv;
+
+	/*
+	 * Write out the command to the device.
+	 *
+	 * Only command NAND_CMD_RESET or NAND_CMD_READID will come
+	 * here before mtd->writesize is initialized.
+	 */
+
+	/* Emulate NAND_CMD_READOOB */
+	if (command == NAND_CMD_READOOB) {
+		assert(mtd->writesize != 0);
+		column += mtd->writesize;
+		command = NAND_CMD_READ0;
+	}
+
+	/* Adjust columns for 16 bit bus-width */
+	if (column != -1 && (chip->options & NAND_BUSWIDTH_16))
+		column >>= 1;
+
+	nand_clear_interrupt_status(info->reg);
+
+	/* Stop DMA engine, clear DMA completion status */
+	writel(DMA_MST_CTRL_EN_A_DISABLE
+		| DMA_MST_CTRL_EN_B_DISABLE
+		| DMA_MST_CTRL_IS_DMA_DONE,
+		&info->reg->dma_mst_ctrl);
+
+	/*
+	 * Program and erase have their own busy handlers
+	 * status and sequential in needs no delay
+	 */
+	switch (command) {
+	case NAND_CMD_READID:
+		writel(NAND_CMD_READID, &info->reg->cmd_reg1);
+		writel(CMD_GO | CMD_CLE | CMD_ALE | CMD_PIO
+			| CMD_RX |
+			((4 - 1) << CMD_TRANS_SIZE_SHIFT)
+			| CMD_CE0,
+			&info->reg->command);
+		info->pio_byte_index = 0;
+		break;
+	case NAND_CMD_READ0:
+		writel(NAND_CMD_READ0, &info->reg->cmd_reg1);
+		writel(NAND_CMD_READSTART, &info->reg->cmd_reg2);
+		writel((page_addr << 16) | (column & 0xFFFF),
+			&info->reg->addr_reg1);
+		writel(page_addr >> 16, &info->reg->addr_reg2);
+		return;
+	case NAND_CMD_SEQIN:
+		writel(NAND_CMD_SEQIN, &info->reg->cmd_reg1);
+		writel(NAND_CMD_PAGEPROG, &info->reg->cmd_reg2);
+		writel((page_addr << 16) | (column & 0xFFFF),
+			&info->reg->addr_reg1);
+		writel(page_addr >> 16,
+			&info->reg->addr_reg2);
+		return;
+	case NAND_CMD_PAGEPROG:
+		return;
+	case NAND_CMD_ERASE1:
+		writel(NAND_CMD_ERASE1, &info->reg->cmd_reg1);
+		writel(NAND_CMD_ERASE2, &info->reg->cmd_reg2);
+		writel(page_addr, &info->reg->addr_reg1);
+		writel(CMD_GO | CMD_CLE | CMD_ALE |
+			CMD_SEC_CMD | CMD_CE0 | CMD_ALE_BYTES3,
+			&info->reg->command);
+		break;
+	case NAND_CMD_RNDOUT:
+		printf("%s: Unsupported RNDOUT command\n", __func__);
+		return;
+	case NAND_CMD_ERASE2:
+		return;
+	case NAND_CMD_STATUS:
+		writel(NAND_CMD_STATUS, &info->reg->cmd_reg1);
+		writel(CMD_GO | CMD_CLE | CMD_PIO | CMD_RX
+			| ((1 - 0) << CMD_TRANS_SIZE_SHIFT)
+			| CMD_CE0,
+			&info->reg->command);
+		info->pio_byte_index = 0;
+		break;
+	case NAND_CMD_RESET:
+		writel(NAND_CMD_RESET, &info->reg->cmd_reg1);
+		writel(CMD_GO | CMD_CLE | CMD_CE0,
+			&info->reg->command);
+		break;
+	default:
+		printf("%s: Unsupported command %d\n", __func__, command);
+		return;
+	}
+	if (!nand_waitfor_cmd_completion(info->reg))
+		printf("Command 0x%02X timeout\n", command);
+}
+
+/**
+ * Check whether the pointed buffer are all 0xff (blank).
+ *
+ * @param buf	data buffer for blank check
+ * @param len	length of the buffer in byte
+ * @return
+ *	1 - blank
+ *	0 - non-blank
+ */
+static int blank_check(u8 *buf, int len)
+{
+	int i;
+
+	for (i = 0; i < len; i++)
+		if (buf[i] != 0xFF)
+			return 0;
+	return 1;
+}
+
+/**
+ * After a DMA transfer for read, we call this function to see whether there
+ * is any uncorrectable error on the pointed data buffer or oob buffer.
+ *
+ * @param reg		nand_ctlr structure
+ * @param databuf	data buffer
+ * @param a_len		data buffer length
+ * @param oobbuf	oob buffer
+ * @param b_len		oob buffer length
+ * @return
+ *	ECC_OK - no ECC error or correctable ECC error
+ *	ECC_TAG_ERROR - uncorrectable tag ECC error
+ *	ECC_DATA_ERROR - uncorrectable data ECC error
+ *	ECC_DATA_ERROR + ECC_TAG_ERROR - uncorrectable data+tag ECC error
+ */
+static int check_ecc_error(struct nand_ctlr *reg, u8 *databuf,
+	int a_len, u8 *oobbuf, int b_len)
+{
+	int return_val = ECC_OK;
+	u32 reg_val;
+
+	if (!(readl(&reg->isr) & ISR_IS_ECC_ERR))
+		return ECC_OK;
+
+	reg_val = readl(&reg->dec_status);
+	if ((reg_val & DEC_STATUS_A_ECC_FAIL) && databuf) {
+		reg_val = readl(&reg->bch_dec_status_buf);
+		/*
+		 * If uncorrectable error occurs on data area, then see whether
+		 * they are all FF. If all are FF, it's a blank page.
+		 * Not error.
+		 */
+		if ((reg_val & BCH_DEC_STATUS_FAIL_SEC_FLAG_MASK) &&
+			!blank_check(databuf, a_len))
+			return_val |= ECC_DATA_ERROR;
+	}
+
+	if ((reg_val & DEC_STATUS_B_ECC_FAIL) && oobbuf) {
+		reg_val = readl(&reg->bch_dec_status_buf);
+		/*
+		 * If uncorrectable error occurs on tag area, then see whether
+		 * they are all FF. If all are FF, it's a blank page.
+		 * Not error.
+		 */
+		if ((reg_val & BCH_DEC_STATUS_FAIL_TAG_MASK) &&
+			!blank_check(oobbuf, b_len))
+			return_val |= ECC_TAG_ERROR;
+	}
+
+	return return_val;
+}
+
+/**
+ * Set GO bit to send command to device
+ *
+ * @param reg	nand_ctlr structure
+ */
+static void start_command(struct nand_ctlr *reg)
+{
+	u32 reg_val;
+
+	reg_val = readl(&reg->command);
+	reg_val |= CMD_GO;
+	writel(reg_val, &reg->command);
+}
+
+/**
+ * Clear command GO bit, DMA GO bit, and DMA completion status
+ *
+ * @param reg	nand_ctlr structure
+ */
+static void stop_command(struct nand_ctlr *reg)
+{
+	/* Stop command */
+	writel(0, &reg->command);
+
+	/* Stop DMA engine and clear DMA completion status */
+	writel(DMA_MST_CTRL_GO_DISABLE
+		| DMA_MST_CTRL_IS_DMA_DONE,
+		&reg->dma_mst_ctrl);
+}
+
+/**
+ * Set up NAND bus width and page size
+ *
+ * @param info		nand_info structure
+ * @param *reg_val	address of reg_val
+ * @return 0 if ok, -1 on error
+ */
+static int set_bus_width_page_size(struct fdt_nand *config,
+	u32 *reg_val)
+{
+	if (config->width == 8)
+		*reg_val = CFG_BUS_WIDTH_8BIT;
+	else if (config->width == 16)
+		*reg_val = CFG_BUS_WIDTH_16BIT;
+	else {
+		debug("%s: Unsupported bus width %d\n", __func__,
+		      config->width);
+		return -1;
+	}
+
+	if (config->page_data_bytes == 512)
+		*reg_val |= CFG_PAGE_SIZE_512;
+	else if (config->page_data_bytes == 2048)
+		*reg_val |= CFG_PAGE_SIZE_2048;
+	else if (config->page_data_bytes == 4096)
+		*reg_val |= CFG_PAGE_SIZE_4096;
+	else {
+		debug("%s: Unsupported page size %d\n", __func__,
+		      config->page_data_bytes);
+		return -1;
+	}
+
+	return 0;
+}
+
+/**
+ * Page read/write function
+ *
+ * @param mtd		mtd info structure
+ * @param chip		nand chip info structure
+ * @param buf		data buffer
+ * @param page		page number
+ * @param with_ecc	1 to enable ECC, 0 to disable ECC
+ * @param is_writing	0 for read, 1 for write
+ * @return	0 when successfully completed
+ *		-EIO when command timeout
+ */
+static int nand_rw_page(struct mtd_info *mtd, struct nand_chip *chip,
+	uint8_t *buf, int page, int with_ecc, int is_writing)
+{
+	u32 reg_val;
+	int tag_size;
+	struct nand_oobfree *free = chip->ecc.layout->oobfree;
+	/* 128 is larger than the value that our HW can support. */
+	ALLOC_CACHE_ALIGN_BUFFER(u32, tag_buf, 128);
+	char *tag_ptr;
+	struct nand_drv *info;
+	struct fdt_nand *config;
+
+	if ((uintptr_t)buf & 0x03) {
+		printf("buf %p has to be 4-byte aligned\n", buf);
+		return -EINVAL;
+	}
+
+	info = (struct nand_drv *)chip->priv;
+	config = &info->config;
+	if (set_bus_width_page_size(config, &reg_val))
+		return -EINVAL;
+
+	/* Need to be 4-byte aligned */
+	tag_ptr = (char *)&tag_buf;
+
+	stop_command(info->reg);
+
+	writel((1 << chip->page_shift) - 1, &info->reg->dma_cfg_a);
+	writel((u32)buf, &info->reg->data_block_ptr);
+
+	if (with_ecc) {
+		writel((u32)tag_ptr, &info->reg->tag_ptr);
+		if (is_writing)
+			memcpy(tag_ptr, chip->oob_poi + free->offset,
+				config->tag_bytes +
+				config->tag_ecc_bytes);
+	} else
+		writel((u32)chip->oob_poi, &info->reg->tag_ptr);
+
+	/* Set ECC selection, configure ECC settings */
+	if (with_ecc) {
+		tag_size = config->tag_bytes + config->tag_ecc_bytes;
+		reg_val |= (CFG_SKIP_SPARE_SEL_4
+			| CFG_SKIP_SPARE_ENABLE
+			| CFG_HW_ECC_CORRECTION_ENABLE
+			| CFG_ECC_EN_TAG_DISABLE
+			| CFG_HW_ECC_SEL_RS
+			| CFG_HW_ECC_ENABLE
+			| CFG_TVAL4
+			| (tag_size - 1));
+
+		if (!is_writing)
+			tag_size += config->skipped_spare_bytes;
+		dma_prepare(tag_ptr, tag_size, is_writing);
+	} else {
+		tag_size = mtd->oobsize;
+		reg_val |= (CFG_SKIP_SPARE_DISABLE
+			| CFG_HW_ECC_CORRECTION_DISABLE
+			| CFG_ECC_EN_TAG_DISABLE
+			| CFG_HW_ECC_DISABLE
+			| (tag_size - 1));
+		dma_prepare(chip->oob_poi, tag_size, is_writing);
+	}
+	writel(reg_val, &info->reg->config);
+
+	dma_prepare(buf, 1 << chip->page_shift, is_writing);
+
+	writel(BCH_CONFIG_BCH_ECC_DISABLE, &info->reg->bch_config);
+
+	writel(tag_size - 1, &info->reg->dma_cfg_b);
+
+	nand_clear_interrupt_status(info->reg);
+
+	reg_val = CMD_CLE | CMD_ALE
+		| CMD_SEC_CMD
+		| (CMD_ALE_BYTES5 << CMD_ALE_BYTE_SIZE_SHIFT)
+		| CMD_A_VALID
+		| CMD_B_VALID
+		| (CMD_TRANS_SIZE_PAGE << CMD_TRANS_SIZE_SHIFT)
+		| CMD_CE0;
+	if (!is_writing)
+		reg_val |= (CMD_AFT_DAT_DISABLE | CMD_RX);
+	else
+		reg_val |= (CMD_AFT_DAT_ENABLE | CMD_TX);
+	writel(reg_val, &info->reg->command);
+
+	/* Setup DMA engine */
+	reg_val = DMA_MST_CTRL_GO_ENABLE
+		| DMA_MST_CTRL_BURST_8WORDS
+		| DMA_MST_CTRL_EN_A_ENABLE
+		| DMA_MST_CTRL_EN_B_ENABLE;
+
+	if (!is_writing)
+		reg_val |= DMA_MST_CTRL_DIR_READ;
+	else
+		reg_val |= DMA_MST_CTRL_DIR_WRITE;
+
+	writel(reg_val, &info->reg->dma_mst_ctrl);
+
+	start_command(info->reg);
+
+	if (!nand_waitfor_cmd_completion(info->reg)) {
+		if (!is_writing)
+			printf("Read Page 0x%X timeout ", page);
+		else
+			printf("Write Page 0x%X timeout ", page);
+		if (with_ecc)
+			printf("with ECC");
+		else
+			printf("without ECC");
+		printf("\n");
+		return -EIO;
+	}
+
+	if (with_ecc && !is_writing) {
+		memcpy(chip->oob_poi, tag_ptr,
+			config->skipped_spare_bytes);
+		memcpy(chip->oob_poi + free->offset,
+			tag_ptr + config->skipped_spare_bytes,
+			config->tag_bytes);
+		reg_val = (u32)check_ecc_error(info->reg, (u8 *)buf,
+			1 << chip->page_shift,
+			(u8 *)(tag_ptr + config->skipped_spare_bytes),
+			config->tag_bytes);
+		if (reg_val & ECC_TAG_ERROR)
+			printf("Read Page 0x%X tag ECC error\n", page);
+		if (reg_val & ECC_DATA_ERROR)
+			printf("Read Page 0x%X data ECC error\n",
+				page);
+		if (reg_val & (ECC_DATA_ERROR | ECC_TAG_ERROR))
+			return -EIO;
+	}
+	return 0;
+}
+
+/**
+ * Hardware ecc based page read function
+ *
+ * @param mtd	mtd info structure
+ * @param chip	nand chip info structure
+ * @param buf	buffer to store read data
+ * @param page	page number to read
+ * @return	0 when successfully completed
+ *		-EIO when command timeout
+ */
+static int nand_read_page_hwecc(struct mtd_info *mtd,
+	struct nand_chip *chip, uint8_t *buf, int page)
+{
+	return nand_rw_page(mtd, chip, buf, page, 1, 0);
+}
+
+/**
+ * Hardware ecc based page write function
+ *
+ * @param mtd	mtd info structure
+ * @param chip	nand chip info structure
+ * @param buf	data buffer
+ */
+static void nand_write_page_hwecc(struct mtd_info *mtd,
+	struct nand_chip *chip, const uint8_t *buf)
+{
+	int page;
+	struct nand_drv *info;
+
+	info = (struct nand_drv *)chip->priv;
+
+	page = (readl(&info->reg->addr_reg1) >> 16) |
+		(readl(&info->reg->addr_reg2) << 16);
+
+	nand_rw_page(mtd, chip, (uint8_t *)buf, page, 1, 1);
+}
+
+
+/**
+ * Read raw page data without ecc
+ *
+ * @param mtd	mtd info structure
+ * @param chip	nand chip info structure
+ * @param buf	buffer to store read data
+ * @param page	page number to read
+ * @return	0 when successfully completed
+ *		-EINVAL when chip->oob_poi is not double-word aligned
+ *		-EIO when command timeout
+ */
+static int nand_read_page_raw(struct mtd_info *mtd,
+	struct nand_chip *chip, uint8_t *buf, int page)
+{
+	return nand_rw_page(mtd, chip, buf, page, 0, 0);
+}
+
+/**
+ * Raw page write function
+ *
+ * @param mtd	mtd info structure
+ * @param chip	nand chip info structure
+ * @param buf	data buffer
+ */
+static void nand_write_page_raw(struct mtd_info *mtd,
+		struct nand_chip *chip,	const uint8_t *buf)
+{
+	int page;
+	struct nand_drv *info;
+
+	info = (struct nand_drv *)chip->priv;
+	page = (readl(&info->reg->addr_reg1) >> 16) |
+		(readl(&info->reg->addr_reg2) << 16);
+
+	nand_rw_page(mtd, chip, (uint8_t *)buf, page, 0, 1);
+}
+
+/**
+ * OOB data read/write function
+ *
+ * @param mtd		mtd info structure
+ * @param chip		nand chip info structure
+ * @param page		page number to read
+ * @param with_ecc	1 to enable ECC, 0 to disable ECC
+ * @param is_writing	0 for read, 1 for write
+ * @return	0 when successfully completed
+ *		-EINVAL when chip->oob_poi is not double-word aligned
+ *		-EIO when command timeout
+ */
+static int nand_rw_oob(struct mtd_info *mtd, struct nand_chip *chip,
+	int page, int with_ecc, int is_writing)
+{
+	u32 reg_val;
+	int tag_size;
+	struct nand_oobfree *free = chip->ecc.layout->oobfree;
+	struct nand_drv *info;
+
+	if (((int)chip->oob_poi) & 0x03)
+		return -EINVAL;
+	info = (struct nand_drv *)chip->priv;
+	if (set_bus_width_page_size(&info->config, &reg_val))
+		return -EINVAL;
+
+	stop_command(info->reg);
+
+	writel((u32)chip->oob_poi, &info->reg->tag_ptr);
+
+	/* Set ECC selection */
+	tag_size = mtd->oobsize;
+	if (with_ecc)
+		reg_val |= CFG_ECC_EN_TAG_ENABLE;
+	else
+		reg_val |= (CFG_ECC_EN_TAG_DISABLE);
+
+	reg_val |= ((tag_size - 1) |
+		CFG_SKIP_SPARE_DISABLE |
+		CFG_HW_ECC_CORRECTION_DISABLE |
+		CFG_HW_ECC_DISABLE);
+	writel(reg_val, &info->reg->config);
+
+	dma_prepare(chip->oob_poi, tag_size, is_writing);
+
+	writel(BCH_CONFIG_BCH_ECC_DISABLE, &info->reg->bch_config);
+
+	if (is_writing && with_ecc)
+		tag_size -= info->config.tag_ecc_bytes;
+
+	writel(tag_size - 1, &info->reg->dma_cfg_b);
+
+	nand_clear_interrupt_status(info->reg);
+
+	reg_val = CMD_CLE | CMD_ALE
+		| CMD_SEC_CMD
+		| (CMD_ALE_BYTES5 << CMD_ALE_BYTE_SIZE_SHIFT)
+		| CMD_B_VALID
+		| CMD_CE0;
+	if (!is_writing)
+		reg_val |= (CMD_AFT_DAT_DISABLE | CMD_RX);
+	else
+		reg_val |= (CMD_AFT_DAT_ENABLE | CMD_TX);
+	writel(reg_val, &info->reg->command);
+
+	/* Setup DMA engine */
+	reg_val = DMA_MST_CTRL_GO_ENABLE
+		| DMA_MST_CTRL_BURST_8WORDS
+		| DMA_MST_CTRL_EN_B_ENABLE;
+	if (!is_writing)
+		reg_val |= DMA_MST_CTRL_DIR_READ;
+	else
+		reg_val |= DMA_MST_CTRL_DIR_WRITE;
+
+	writel(reg_val, &info->reg->dma_mst_ctrl);
+
+	start_command(info->reg);
+
+	if (!nand_waitfor_cmd_completion(info->reg)) {
+		if (!is_writing)
+			printf("Read OOB of Page 0x%X timeout\n", page);
+		else
+			printf("Write OOB of Page 0x%X timeout\n", page);
+		return -EIO;
+	}
+
+	if (with_ecc && !is_writing) {
+		reg_val = (u32)check_ecc_error(info->reg, 0, 0,
+			(u8 *)(chip->oob_poi + free->offset),
+			info->config.tag_bytes);
+		if (reg_val & ECC_TAG_ERROR)
+			printf("Read OOB of Page 0x%X tag ECC error\n", page);
+	}
+	return 0;
+}
+
+/**
+ * OOB data read function
+ *
+ * @param mtd		mtd info structure
+ * @param chip		nand chip info structure
+ * @param page		page number to read
+ * @param sndcmd	flag whether to issue read command or not
+ * @return	1 - issue read command next time
+ *		0 - not to issue
+ */
+static int nand_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
+	int page, int sndcmd)
+{
+	if (sndcmd) {
+		chip->cmdfunc(mtd, NAND_CMD_READOOB, 0, page);
+		sndcmd = 0;
+	}
+	nand_rw_oob(mtd, chip, page, 0, 0);
+	return sndcmd;
+}
+
+/**
+ * OOB data write function
+ *
+ * @param mtd	mtd info structure
+ * @param chip	nand chip info structure
+ * @param page	page number to write
+ * @return	0 when successfully completed
+ *		-EINVAL when chip->oob_poi is not double-word aligned
+ *		-EIO when command timeout
+ */
+static int nand_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
+	int page)
+{
+	chip->cmdfunc(mtd, NAND_CMD_SEQIN, mtd->writesize, page);
+
+	return nand_rw_oob(mtd, chip, page, 0, 1);
+}
+
+/**
+ * Set up NAND memory timings according to the provided parameters
+ *
+ * @param timing	Timing parameters
+ * @param reg		NAND controller register address
+ */
+static void setup_timing(unsigned timing[FDT_NAND_TIMING_COUNT],
+			 struct nand_ctlr *reg)
+{
+	u32 reg_val, clk_rate, clk_period, time_val;
+
+	clk_rate = (u32)clock_get_periph_rate(PERIPH_ID_NDFLASH,
+		CLOCK_ID_PERIPH) / 1000000;
+	clk_period = 1000 / clk_rate;
+	reg_val = ((timing[FDT_NAND_MAX_TRP_TREA] / clk_period) <<
+		TIMING_TRP_RESP_CNT_SHIFT) & TIMING_TRP_RESP_CNT_MASK;
+	reg_val |= ((timing[FDT_NAND_TWB] / clk_period) <<
+		TIMING_TWB_CNT_SHIFT) & TIMING_TWB_CNT_MASK;
+	time_val = timing[FDT_NAND_MAX_TCR_TAR_TRR] / clk_period;
+	if (time_val > 2)
+		reg_val |= ((time_val - 2) << TIMING_TCR_TAR_TRR_CNT_SHIFT) &
+			TIMING_TCR_TAR_TRR_CNT_MASK;
+	reg_val |= ((timing[FDT_NAND_TWHR] / clk_period) <<
+		TIMING_TWHR_CNT_SHIFT) & TIMING_TWHR_CNT_MASK;
+	time_val = timing[FDT_NAND_MAX_TCS_TCH_TALS_TALH] / clk_period;
+	if (time_val > 1)
+		reg_val |= ((time_val - 1) << TIMING_TCS_CNT_SHIFT) &
+			TIMING_TCS_CNT_MASK;
+	reg_val |= ((timing[FDT_NAND_TWH] / clk_period) <<
+		TIMING_TWH_CNT_SHIFT) & TIMING_TWH_CNT_MASK;
+	reg_val |= ((timing[FDT_NAND_TWP] / clk_period) <<
+		TIMING_TWP_CNT_SHIFT) & TIMING_TWP_CNT_MASK;
+	reg_val |= ((timing[FDT_NAND_TRH] / clk_period) <<
+		TIMING_TRH_CNT_SHIFT) & TIMING_TRH_CNT_MASK;
+	reg_val |= ((timing[FDT_NAND_MAX_TRP_TREA] / clk_period) <<
+		TIMING_TRP_CNT_SHIFT) & TIMING_TRP_CNT_MASK;
+	writel(reg_val, &reg->timing);
+
+	reg_val = 0;
+	time_val = timing[FDT_NAND_TADL] / clk_period;
+	if (time_val > 2)
+		reg_val = (time_val - 2) & TIMING2_TADL_CNT_MASK;
+	writel(reg_val, &reg->timing2);
+}
+
+/**
+ * Decode NAND parameters from the device tree
+ *
+ * @param blob	Device tree blob
+ * @param node	Node containing "nand-flash" compatble node
+ * @return 0 if ok, -ve on error (FDT_ERR_...)
+ */
+int fdt_decode_nand(const void *blob, int node, struct fdt_nand *config)
+{
+	int err;
+
+	config->reg = (struct nand_ctlr *)fdtdec_get_addr(blob, node, "reg");
+	config->enabled = fdtdec_get_is_enabled(blob, node);
+	config->width = fdtdec_get_int(blob, node, "nvidia,width", 8);
+	err = fdtdec_decode_gpio(blob, node, "wp-gpios", &config->wp_gpio);
+	if (err)
+		return err;
+	err = fdtdec_get_int_array(blob, node, "nvidia,timing",
+			config->timing, FDT_NAND_TIMING_COUNT);
+	if (err < 0)
+		return err;
+
+	/* Now look up the controller and decode that */
+	node = fdt_next_node(blob, node, NULL);
+	if (node < 0)
+		return node;
+
+	config->page_data_bytes = fdtdec_get_int(blob, node,
+					"nvidia,page-data-bytes", -1);
+	config->tag_ecc_bytes = fdtdec_get_int(blob, node,
+					"nvidia,tag-ecc-bytes", -1);
+	config->tag_bytes = fdtdec_get_int(blob, node, "nvidia,tag-bytes", -1);
+	config->data_ecc_bytes = fdtdec_get_int(blob, node,
+					"nvidia,data-ecc-bytes", -1);
+	config->skipped_spare_bytes = fdtdec_get_int(blob, node,
+					"nvidia,skipped-spare-bytes", -1);
+	config->page_spare_bytes = fdtdec_get_int(blob, node,
+					"nvidia,page-spare-bytes", -1);
+	if (config->page_data_bytes == -1 || config->tag_ecc_bytes == -1 ||
+		config->tag_bytes == -1 || config->data_ecc_bytes == -1 ||
+		config->skipped_spare_bytes == -1 ||
+		config->page_spare_bytes == -1)
+		return -FDT_ERR_NOTFOUND;
+
+	return 0;
+}
+
+/**
+ * Board-specific NAND initialization
+ *
+ * @param nand	nand chip info structure
+ * @return 0, after initialized, -1 on error
+ */
+int board_nand_init(struct nand_chip *nand)
+{
+	struct nand_drv *info = &nand_ctrl;
+	struct fdt_nand *config = &info->config;
+	u32 reg_val;
+	int node;
+
+	node = fdtdec_next_compatible(gd->fdt_blob, 0,
+				      COMPAT_NVIDIA_TEGRA20_NAND);
+	if (node < 0)
+		return -1;
+	if (fdt_decode_nand(gd->fdt_blob, node, config)) {
+		printf("Could not decode nand-flash in device tree\n");
+		return -1;
+	}
+	if (!config->enabled)
+		return -1;
+	if (set_bus_width_page_size(&info->config, &reg_val)) {
+		printf("NAND bus width / page size not supported\n");
+		return -EINVAL;
+	}
+	info->reg = config->reg;
+	eccoob.eccbytes = config->data_ecc_bytes + config->tag_ecc_bytes;
+	eccoob.oobavail = config->tag_bytes;
+	eccoob.oobfree[0].offset = config->skipped_spare_bytes +
+				config->data_ecc_bytes;
+	eccoob.oobfree[0].length = config->tag_bytes;
+
+	nand->ecc.mode = NAND_ECC_HW;
+	nand->ecc.layout = &eccoob;
+	nand->ecc.size = config->page_data_bytes;
+	nand->ecc.bytes = config->page_spare_bytes;
+
+	nand->options = LP_OPTIONS;
+	nand->cmdfunc = nand_command;
+	nand->read_byte = read_byte;
+	nand->read_buf = read_buf;
+	nand->write_buf = write_buf;
+	nand->ecc.read_page = nand_read_page_hwecc;
+	nand->ecc.write_page = nand_write_page_hwecc;
+	nand->ecc.read_page_raw = nand_read_page_raw;
+	nand->ecc.write_page_raw = nand_write_page_raw;
+	nand->ecc.read_oob = nand_read_oob;
+	nand->ecc.write_oob = nand_write_oob;
+	nand->select_chip = nand_select_chip;
+	nand->dev_ready  = nand_dev_ready;
+	nand->priv = &nand_ctrl;
+
+	/* Adjust controller clock rate */
+	clock_start_periph_pll(PERIPH_ID_NDFLASH, CLOCK_ID_PERIPH, 52000000);
+
+	/* Adjust timing for NAND device */
+	setup_timing(config->timing, info->reg);
+
+	funcmux_select(PERIPH_ID_NDFLASH, FUNCMUX_DEFAULT);
+	fdtdec_setup_gpio(&config->wp_gpio);
+	gpio_direction_output(config->wp_gpio.gpio, 1);
+
+	return 0;
+}
diff --git a/drivers/mtd/nand/tegra2_nand.h b/drivers/mtd/nand/tegra2_nand.h
new file mode 100644
index 0000000..7e74be7
--- /dev/null
+++ b/drivers/mtd/nand/tegra2_nand.h
@@ -0,0 +1,257 @@
+/*
+ * (C) Copyright 2011 NVIDIA Corporation <www.nvidia.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+/* register offset */
+#define COMMAND_0		0x00
+#define CMD_GO			(1 << 31)
+#define CMD_CLE			(1 << 30)
+#define CMD_ALE			(1 << 29)
+#define CMD_PIO			(1 << 28)
+#define CMD_TX			(1 << 27)
+#define CMD_RX			(1 << 26)
+#define CMD_SEC_CMD		(1 << 25)
+#define CMD_AFT_DAT_MASK	(1 << 24)
+#define CMD_AFT_DAT_DISABLE	0
+#define CMD_AFT_DAT_ENABLE	(1 << 24)
+#define CMD_TRANS_SIZE_SHIFT	20
+#define CMD_TRANS_SIZE_PAGE	8
+#define CMD_A_VALID		(1 << 19)
+#define CMD_B_VALID		(1 << 18)
+#define CMD_RD_STATUS_CHK	(1 << 17)
+#define CMD_R_BSY_CHK		(1 << 16)
+#define CMD_CE7			(1 << 15)
+#define CMD_CE6			(1 << 14)
+#define CMD_CE5			(1 << 13)
+#define CMD_CE4			(1 << 12)
+#define CMD_CE3			(1 << 11)
+#define CMD_CE2			(1 << 10)
+#define CMD_CE1			(1 << 9)
+#define CMD_CE0			(1 << 8)
+#define CMD_CLE_BYTE_SIZE_SHIFT	4
+enum {
+	CMD_CLE_BYTES1 = 0,
+	CMD_CLE_BYTES2,
+	CMD_CLE_BYTES3,
+	CMD_CLE_BYTES4,
+};
+#define CMD_ALE_BYTE_SIZE_SHIFT	0
+enum {
+	CMD_ALE_BYTES1 = 0,
+	CMD_ALE_BYTES2,
+	CMD_ALE_BYTES3,
+	CMD_ALE_BYTES4,
+	CMD_ALE_BYTES5,
+	CMD_ALE_BYTES6,
+	CMD_ALE_BYTES7,
+	CMD_ALE_BYTES8
+};
+
+#define STATUS_0			0x04
+#define STATUS_RBSY0			(1 << 8)
+
+#define ISR_0				0x08
+#define ISR_IS_CMD_DONE			(1 << 5)
+#define ISR_IS_ECC_ERR			(1 << 4)
+
+#define IER_0				0x0C
+
+#define CFG_0				0x10
+#define CFG_HW_ECC_MASK			(1 << 31)
+#define CFG_HW_ECC_DISABLE		0
+#define CFG_HW_ECC_ENABLE		(1 << 31)
+#define CFG_HW_ECC_SEL_MASK		(1 << 30)
+#define CFG_HW_ECC_SEL_HAMMING		0
+#define CFG_HW_ECC_SEL_RS		(1 << 30)
+#define CFG_HW_ECC_CORRECTION_MASK	(1 << 29)
+#define CFG_HW_ECC_CORRECTION_DISABLE	0
+#define CFG_HW_ECC_CORRECTION_ENABLE	(1 << 29)
+#define CFG_PIPELINE_EN_MASK		(1 << 28)
+#define CFG_PIPELINE_EN_DISABLE		0
+#define CFG_PIPELINE_EN_ENABLE		(1 << 28)
+#define CFG_ECC_EN_TAG_MASK		(1 << 27)
+#define CFG_ECC_EN_TAG_DISABLE		0
+#define CFG_ECC_EN_TAG_ENABLE		(1 << 27)
+#define CFG_TVALUE_MASK			(3 << 24)
+enum {
+	CFG_TVAL4 = 0 << 24,
+	CFG_TVAL6 = 1 << 24,
+	CFG_TVAL8 = 2 << 24
+};
+#define CFG_SKIP_SPARE_MASK		(1 << 23)
+#define CFG_SKIP_SPARE_DISABLE		0
+#define CFG_SKIP_SPARE_ENABLE		(1 << 23)
+#define CFG_COM_BSY_MASK		(1 << 22)
+#define CFG_COM_BSY_DISABLE		0
+#define CFG_COM_BSY_ENABLE		(1 << 22)
+#define CFG_BUS_WIDTH_MASK		(1 << 21)
+#define CFG_BUS_WIDTH_8BIT		0
+#define CFG_BUS_WIDTH_16BIT		(1 << 21)
+#define CFG_LPDDR1_MODE_MASK		(1 << 20)
+#define CFG_LPDDR1_MODE_DISABLE		0
+#define CFG_LPDDR1_MODE_ENABLE		(1 << 20)
+#define CFG_EDO_MODE_MASK		(1 << 19)
+#define CFG_EDO_MODE_DISABLE		0
+#define CFG_EDO_MODE_ENABLE		(1 << 19)
+#define CFG_PAGE_SIZE_SEL_MASK		(7 << 16)
+enum {
+	CFG_PAGE_SIZE_256	= 0 << 16,
+	CFG_PAGE_SIZE_512	= 1 << 16,
+	CFG_PAGE_SIZE_1024	= 2 << 16,
+	CFG_PAGE_SIZE_2048	= 3 << 16,
+	CFG_PAGE_SIZE_4096	= 4 << 16
+};
+#define CFG_SKIP_SPARE_SEL_MASK		(3 << 14)
+enum {
+	CFG_SKIP_SPARE_SEL_4	= 0 << 14,
+	CFG_SKIP_SPARE_SEL_8	= 1 << 14,
+	CFG_SKIP_SPARE_SEL_12	= 2 << 14,
+	CFG_SKIP_SPARE_SEL_16	= 3 << 14
+};
+#define CFG_TAG_BYTE_SIZE_MASK	0x1FF
+
+#define TIMING_0			0x14
+#define TIMING_TRP_RESP_CNT_SHIFT	28
+#define TIMING_TRP_RESP_CNT_MASK	(0xf << TIMING_TRP_RESP_CNT_SHIFT)
+#define TIMING_TWB_CNT_SHIFT		24
+#define TIMING_TWB_CNT_MASK		(0xf << TIMING_TWB_CNT_SHIFT)
+#define TIMING_TCR_TAR_TRR_CNT_SHIFT	20
+#define TIMING_TCR_TAR_TRR_CNT_MASK	(0xf << TIMING_TCR_TAR_TRR_CNT_SHIFT)
+#define TIMING_TWHR_CNT_SHIFT		16
+#define TIMING_TWHR_CNT_MASK		(0xf << TIMING_TWHR_CNT_SHIFT)
+#define TIMING_TCS_CNT_SHIFT		14
+#define TIMING_TCS_CNT_MASK		(3 << TIMING_TCS_CNT_SHIFT)
+#define TIMING_TWH_CNT_SHIFT		12
+#define TIMING_TWH_CNT_MASK		(3 << TIMING_TWH_CNT_SHIFT)
+#define TIMING_TWP_CNT_SHIFT		8
+#define TIMING_TWP_CNT_MASK		(0xf << TIMING_TWP_CNT_SHIFT)
+#define TIMING_TRH_CNT_SHIFT		4
+#define TIMING_TRH_CNT_MASK		(3 << TIMING_TRH_CNT_SHIFT)
+#define TIMING_TRP_CNT_SHIFT		0
+#define TIMING_TRP_CNT_MASK		(0xf << TIMING_TRP_CNT_SHIFT)
+
+#define RESP_0				0x18
+
+#define TIMING2_0			0x1C
+#define TIMING2_TADL_CNT_SHIFT		0
+#define TIMING2_TADL_CNT_MASK		(0xf << TIMING2_TADL_CNT_SHIFT)
+
+#define CMD_REG1_0			0x20
+#define CMD_REG2_0			0x24
+#define ADDR_REG1_0			0x28
+#define ADDR_REG2_0			0x2C
+
+#define DMA_MST_CTRL_0			0x30
+#define DMA_MST_CTRL_GO_MASK		(1 << 31)
+#define DMA_MST_CTRL_GO_DISABLE		0
+#define DMA_MST_CTRL_GO_ENABLE		(1 << 31)
+#define DMA_MST_CTRL_DIR_MASK		(1 << 30)
+#define DMA_MST_CTRL_DIR_READ		0
+#define DMA_MST_CTRL_DIR_WRITE		(1 << 30)
+#define DMA_MST_CTRL_PERF_EN_MASK	(1 << 29)
+#define DMA_MST_CTRL_PERF_EN_DISABLE	0
+#define DMA_MST_CTRL_PERF_EN_ENABLE	(1 << 29)
+#define DMA_MST_CTRL_REUSE_BUFFER_MASK	(1 << 27)
+#define DMA_MST_CTRL_REUSE_BUFFER_DISABLE	0
+#define DMA_MST_CTRL_REUSE_BUFFER_ENABLE	(1 << 27)
+#define DMA_MST_CTRL_BURST_SIZE_SHIFT	24
+#define DMA_MST_CTRL_BURST_SIZE_MASK	(7 << DMA_MST_CTRL_BURST_SIZE_SHIFT)
+enum {
+	DMA_MST_CTRL_BURST_1WORDS	= 2 << DMA_MST_CTRL_BURST_SIZE_SHIFT,
+	DMA_MST_CTRL_BURST_4WORDS	= 3 << DMA_MST_CTRL_BURST_SIZE_SHIFT,
+	DMA_MST_CTRL_BURST_8WORDS	= 4 << DMA_MST_CTRL_BURST_SIZE_SHIFT,
+	DMA_MST_CTRL_BURST_16WORDS	= 5 << DMA_MST_CTRL_BURST_SIZE_SHIFT
+};
+#define DMA_MST_CTRL_IS_DMA_DONE	(1 << 20)
+#define DMA_MST_CTRL_EN_A_MASK		(1 << 2)
+#define DMA_MST_CTRL_EN_A_DISABLE	0
+#define DMA_MST_CTRL_EN_A_ENABLE	(1 << 2)
+#define DMA_MST_CTRL_EN_B_MASK		(1 << 1)
+#define DMA_MST_CTRL_EN_B_DISABLE	0
+#define DMA_MST_CTRL_EN_B_ENABLE	(1 << 1)
+
+#define DMA_CFG_A_0			0x34
+#define DMA_CFG_B_0			0x38
+#define FIFO_CTRL_0			0x3C
+#define DATA_BLOCK_PTR_0		0x40
+#define TAG_PTR_0			0x44
+#define ECC_PTR_0			0x48
+
+#define DEC_STATUS_0			0x4C
+#define DEC_STATUS_A_ECC_FAIL		(1 << 1)
+#define DEC_STATUS_B_ECC_FAIL		(1 << 0)
+
+#define BCH_CONFIG_0			0xCC
+#define BCH_CONFIG_BCH_TVALUE_SHIFT	4
+#define BCH_CONFIG_BCH_TVALUE_MASK	(3 << BCH_CONFIG_BCH_TVALUE_SHIFT)
+enum {
+	BCH_CONFIG_BCH_TVAL4	= 0 << BCH_CONFIG_BCH_TVALUE_SHIFT,
+	BCH_CONFIG_BCH_TVAL8	= 1 << BCH_CONFIG_BCH_TVALUE_SHIFT,
+	BCH_CONFIG_BCH_TVAL14	= 2 << BCH_CONFIG_BCH_TVALUE_SHIFT,
+	BCH_CONFIG_BCH_TVAL16	= 3 << BCH_CONFIG_BCH_TVALUE_SHIFT
+};
+#define BCH_CONFIG_BCH_ECC_MASK		(1 << 0)
+#define BCH_CONFIG_BCH_ECC_DISABLE	0
+#define BCH_CONFIG_BCH_ECC_ENABLE	(1 << 0)
+
+#define BCH_DEC_RESULT_0			0xD0
+#define BCH_DEC_RESULT_CORRFAIL_ERR_MASK	(1 << 8)
+#define BCH_DEC_RESULT_PAGE_COUNT_MASK		0xFF
+
+#define BCH_DEC_STATUS_BUF_0			0xD4
+#define BCH_DEC_STATUS_FAIL_SEC_FLAG_MASK	0xFF000000
+#define BCH_DEC_STATUS_CORR_SEC_FLAG_MASK	0x00FF0000
+#define BCH_DEC_STATUS_FAIL_TAG_MASK		(1 << 14)
+#define BCH_DEC_STATUS_CORR_TAG_MASK		(1 << 13)
+#define BCH_DEC_STATUS_MAX_CORR_CNT_MASK	(0x1f << 8)
+#define BCH_DEC_STATUS_PAGE_NUMBER_MASK		0xFF
+
+#define LP_OPTIONS (NAND_NO_READRDY | NAND_NO_AUTOINCR)
+
+struct nand_ctlr {
+	u32	command;	/* offset 00h */
+	u32	status;		/* offset 04h */
+	u32	isr;		/* offset 08h */
+	u32	ier;		/* offset 0Ch */
+	u32	config;		/* offset 10h */
+	u32	timing;		/* offset 14h */
+	u32	resp;		/* offset 18h */
+	u32	timing2;	/* offset 1Ch */
+	u32	cmd_reg1;	/* offset 20h */
+	u32	cmd_reg2;	/* offset 24h */
+	u32	addr_reg1;	/* offset 28h */
+	u32	addr_reg2;	/* offset 2Ch */
+	u32	dma_mst_ctrl;	/* offset 30h */
+	u32	dma_cfg_a;	/* offset 34h */
+	u32	dma_cfg_b;	/* offset 38h */
+	u32	fifo_ctrl;	/* offset 3Ch */
+	u32	data_block_ptr;	/* offset 40h */
+	u32	tag_ptr;	/* offset 44h */
+	u32	resv1;		/* offset 48h */
+	u32	dec_status;	/* offset 4Ch */
+	u32	hwstatus_cmd;	/* offset 50h */
+	u32	hwstatus_mask;	/* offset 54h */
+	u32	resv2[29];
+	u32	bch_config;	/* offset CCh */
+	u32	bch_dec_result;	/* offset D0h */
+	u32	bch_dec_status_buf;
+				/* offset D4h */
+};
diff --git a/include/fdtdec.h b/include/fdtdec.h
index 49251d5..7bae2f8 100644
--- a/include/fdtdec.h
+++ b/include/fdtdec.h
@@ -62,6 +62,7 @@ enum fdt_compat_id {
 	COMPAT_NVIDIA_TEGRA20_DVC,	/* Tegra2 dvc (really just i2c) */
 	COMPAT_NVIDIA_TEGRA20_EMC,	/* Tegra2 memory controller */
 	COMPAT_NVIDIA_TEGRA20_EMC_TABLE, /* Tegra2 memory timing table */
+	COMPAT_NVIDIA_TEGRA20_NAND,	/* Tegra2 NAND controller */
 
 	COMPAT_COUNT,
 };
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 3885634..2eb5b41 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -42,6 +42,7 @@ static const char * const compat_names[COMPAT_COUNT] = {
 	COMPAT(NVIDIA_TEGRA20_DVC, "nvidia,tegra20-i2c-dvc"),
 	COMPAT(NVIDIA_TEGRA20_EMC, "nvidia,tegra20-emc"),
 	COMPAT(NVIDIA_TEGRA20_EMC_TABLE, "nvidia,tegra20-emc-table"),
+	COMPAT(NVIDIA_TEGRA20_NAND, "nvidia,tegra20-nand"),
 };
 
 const char *fdtdec_get_compatible(enum fdt_compat_id id)
-- 
1.7.7.3

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

* [U-Boot] [PATCH v2 7/7] tegra: Enable NAND on Seaboard
  2012-04-13 18:29 [U-Boot] [PATCH v2 0/7] tegra: Add NAND flash support Simon Glass
                   ` (5 preceding siblings ...)
  2012-04-13 18:29 ` [U-Boot] [PATCH v2 6/7] tegra: nand: Add Tegra NAND driver Simon Glass
@ 2012-04-13 18:29 ` Simon Glass
  6 siblings, 0 replies; 45+ messages in thread
From: Simon Glass @ 2012-04-13 18:29 UTC (permalink / raw)
  To: u-boot

This enables NAND support for the Seaboard.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/configs/seaboard.h |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/include/configs/seaboard.h b/include/configs/seaboard.h
index 5d33dc5..3306622 100644
--- a/include/configs/seaboard.h
+++ b/include/configs/seaboard.h
@@ -105,4 +105,13 @@
 #define CONFIG_USB_STORAGE
 #define CONFIG_CMD_USB
 
+/* NAND support */
+#define CONFIG_CMD_NAND
+#define CONFIG_TEGRA2_NAND
+
+/* Max number of NAND devices */
+#define CONFIG_SYS_MAX_NAND_DEVICE	1
+
+/* Somewhat oddly, the NAND base address must be a config option */
+#define CONFIG_SYS_NAND_BASE	TEGRA2_NAND_BASE
 #endif /* __CONFIG_H */
-- 
1.7.7.3

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

* [U-Boot] [PATCH v2 1/7] nand: Try to align the default buffers
  2012-04-13 18:29 ` [U-Boot] [PATCH v2 1/7] nand: Try to align the default buffers Simon Glass
@ 2012-04-13 18:37   ` Scott Wood
  2012-04-13 18:52     ` Simon Glass
  0 siblings, 1 reply; 45+ messages in thread
From: Scott Wood @ 2012-04-13 18:37 UTC (permalink / raw)
  To: u-boot

On 04/13/2012 01:29 PM, Simon Glass wrote:
> The NAND layer needs to use cache-aligned buffers by default. Towards this
> goal. align the default buffers and their members according to the minimum
> DMA alignment defined for the architecture.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> Changes in v2:
> - Add new patch to align default buffers in nand_base
> 
>  drivers/mtd/nand/nand_base.c |    3 ++-
>  include/linux/mtd/nand.h     |    7 ++++---
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 44f7b91..7bfc29e 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -2935,7 +2935,8 @@ int nand_scan_tail(struct mtd_info *mtd)
>  	struct nand_chip *chip = mtd->priv;
>  
>  	if (!(chip->options & NAND_OWN_BUFFERS))
> -		chip->buffers = kmalloc(sizeof(*chip->buffers), GFP_KERNEL);
> +		chip->buffers = memalign(ARCH_DMA_MINALIGN,
> +					 sizeof(*chip->buffers));

This sort of requirement seems to be what NAND_OWN_BUFFERS was made for.

> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index da6fa18..ae0bdf6 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -391,9 +391,10 @@ struct nand_ecc_ctrl {
>   * consecutive order.
>   */
>  struct nand_buffers {
> -	uint8_t	ecccalc[NAND_MAX_OOBSIZE];
> -	uint8_t	ecccode[NAND_MAX_OOBSIZE];
> -	uint8_t databuf[NAND_MAX_PAGESIZE + NAND_MAX_OOBSIZE];
> +	uint8_t	ecccalc[ALIGN(NAND_MAX_OOBSIZE, ARCH_DMA_MINALIGN)];
> +	uint8_t	ecccode[ALIGN(NAND_MAX_OOBSIZE, ARCH_DMA_MINALIGN)];
> +	uint8_t databuf[ALIGN(NAND_MAX_PAGESIZE + NAND_MAX_OOBSIZE,
> +			      ARCH_DMA_MINALIGN)];
>  };

I remember a while back someone wanting to change this to be pointers
intsead of arrays, so that the driver can manage alignment -- I don't
recall what happened to that.

-Scott

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

* Re: [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
  2012-04-13 18:29   ` [U-Boot] " Simon Glass
@ 2012-04-13 18:43     ` Scott Wood
  -1 siblings, 0 replies; 45+ messages in thread
From: Scott Wood @ 2012-04-13 18:43 UTC (permalink / raw)
  To: Simon Glass
  Cc: Devicetree, Discuss, U-Boot Mailing List, Jerry Van Baren, Tom Warren

On 04/13/2012 01:29 PM, Simon Glass wrote:
> Add a NAND controller along with a bindings file for review.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> Changes in v2:
> - Update NAND binding to add "nvidia," prefix
> 
>  arch/arm/dts/tegra20.dtsi                     |    6 ++
>  doc/device-tree-bindings/nand/nvidia-nand.txt |   68 +++++++++++++++++++++++++
>  2 files changed, 74 insertions(+), 0 deletions(-)
>  create mode 100644 doc/device-tree-bindings/nand/nvidia-nand.txt
> 
> diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
> index bc64f42..7be0462 100644
> --- a/arch/arm/dts/tegra20.dtsi
> +++ b/arch/arm/dts/tegra20.dtsi
> @@ -200,4 +200,10 @@
>  		reg = <0x7000f400 0x200>;
>  	};
>  
> +	nand: nand-controller@0x70008000 {

s/nand-controller@/flash@/ (or "nand@" if you really want -- there's
enough of that in use already)

> +		#address-cells = <0>;
> +		#size-cells = <0>;
> +		compatible = "nvidia,tegra20-nand";
> +		reg = <0x70008000 0x100>;
> +	};
>  };
> diff --git a/doc/device-tree-bindings/nand/nvidia-nand.txt b/doc/device-tree-bindings/nand/nvidia-nand.txt
> new file mode 100644
> index 0000000..b19ce8e
> --- /dev/null
> +++ b/doc/device-tree-bindings/nand/nvidia-nand.txt
> @@ -0,0 +1,68 @@
> +NAND Flash
> +----------
> +
> +(there isn't yet a generic binding in Linux, so this describes what is in
> +U-Boot)

Ideally the binding should not be Linux-specific or U-Boot specific --
it's just the binding that describes this hardware.

> +The device node for a NAND flash device is as described in the document
> +"Open Firmware Recommended Practice : Universal Serial Bus" with the
> +following modifications and additions :
> +
> +Required properties :
> + - compatible : Should be "manufacture,device", "nand-flash"

s/manufacture/manufacturer/

No "nand-flash" compatible, as there's no standard "nand-flash" binding.
 You might want something like "nvidia,tegra20-nand-chip".

> + - nvidia,page-data-bytes : Number of bytes in the data area
> + - nvidia,page-spare-bytes : * Number of bytes in spare area
> +       spare area = skipped-spare-bytes + data-ecc-bytes + tag-bytes
> +			+ tag-ecc-bytes
> + - nvidia,skipped-spare-bytes : Number of bytes to skip at start of spare area
> +	(these are typically used for bad block maintenance)
> + - nvidia,data-ecc-bytes : Number of ECC bytes for data area
> + - nvidia,tag-bytes :Number of tag bytes in spare area
> + - nvidia,tag-ecc-bytes : Number ECC bytes to be generated for tag bytes
> +
> +(replace -bytes with -size or -length?)

I like bytes -- makes the unit clear.

> +Nvidia NAND Controller
> +----------------------
> +
> +The device node for a NAND flash controller is as described in the document
> +"Open Firmware Recommended Practice : Universal Serial Bus" with the
> +following modifications and additions :
> +
> +Optional properties:
> +
> +wp-gpio : GPIO of write-protect line, three cells in the format:
> +		phandle, parameter, flags

nvidia,nand-wp-gpio

> +nvidia,,width : bus width of the NAND device in bits

s/,,width/,nand-width/

> +For now here is something specific to the Nvidia controller, 

Isn't this whole file specific to the nvidia controller?

-Scott

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

* [U-Boot] [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
@ 2012-04-13 18:43     ` Scott Wood
  0 siblings, 0 replies; 45+ messages in thread
From: Scott Wood @ 2012-04-13 18:43 UTC (permalink / raw)
  To: u-boot

On 04/13/2012 01:29 PM, Simon Glass wrote:
> Add a NAND controller along with a bindings file for review.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> Changes in v2:
> - Update NAND binding to add "nvidia," prefix
> 
>  arch/arm/dts/tegra20.dtsi                     |    6 ++
>  doc/device-tree-bindings/nand/nvidia-nand.txt |   68 +++++++++++++++++++++++++
>  2 files changed, 74 insertions(+), 0 deletions(-)
>  create mode 100644 doc/device-tree-bindings/nand/nvidia-nand.txt
> 
> diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
> index bc64f42..7be0462 100644
> --- a/arch/arm/dts/tegra20.dtsi
> +++ b/arch/arm/dts/tegra20.dtsi
> @@ -200,4 +200,10 @@
>  		reg = <0x7000f400 0x200>;
>  	};
>  
> +	nand: nand-controller at 0x70008000 {

s/nand-controller@/flash@/ (or "nand@" if you really want -- there's
enough of that in use already)

> +		#address-cells = <0>;
> +		#size-cells = <0>;
> +		compatible = "nvidia,tegra20-nand";
> +		reg = <0x70008000 0x100>;
> +	};
>  };
> diff --git a/doc/device-tree-bindings/nand/nvidia-nand.txt b/doc/device-tree-bindings/nand/nvidia-nand.txt
> new file mode 100644
> index 0000000..b19ce8e
> --- /dev/null
> +++ b/doc/device-tree-bindings/nand/nvidia-nand.txt
> @@ -0,0 +1,68 @@
> +NAND Flash
> +----------
> +
> +(there isn't yet a generic binding in Linux, so this describes what is in
> +U-Boot)

Ideally the binding should not be Linux-specific or U-Boot specific --
it's just the binding that describes this hardware.

> +The device node for a NAND flash device is as described in the document
> +"Open Firmware Recommended Practice : Universal Serial Bus" with the
> +following modifications and additions :
> +
> +Required properties :
> + - compatible : Should be "manufacture,device", "nand-flash"

s/manufacture/manufacturer/

No "nand-flash" compatible, as there's no standard "nand-flash" binding.
 You might want something like "nvidia,tegra20-nand-chip".

> + - nvidia,page-data-bytes : Number of bytes in the data area
> + - nvidia,page-spare-bytes : * Number of bytes in spare area
> +       spare area = skipped-spare-bytes + data-ecc-bytes + tag-bytes
> +			+ tag-ecc-bytes
> + - nvidia,skipped-spare-bytes : Number of bytes to skip at start of spare area
> +	(these are typically used for bad block maintenance)
> + - nvidia,data-ecc-bytes : Number of ECC bytes for data area
> + - nvidia,tag-bytes :Number of tag bytes in spare area
> + - nvidia,tag-ecc-bytes : Number ECC bytes to be generated for tag bytes
> +
> +(replace -bytes with -size or -length?)

I like bytes -- makes the unit clear.

> +Nvidia NAND Controller
> +----------------------
> +
> +The device node for a NAND flash controller is as described in the document
> +"Open Firmware Recommended Practice : Universal Serial Bus" with the
> +following modifications and additions :
> +
> +Optional properties:
> +
> +wp-gpio : GPIO of write-protect line, three cells in the format:
> +		phandle, parameter, flags

nvidia,nand-wp-gpio

> +nvidia,,width : bus width of the NAND device in bits

s/,,width/,nand-width/

> +For now here is something specific to the Nvidia controller, 

Isn't this whole file specific to the nvidia controller?

-Scott

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

* [U-Boot] [PATCH v2 1/7] nand: Try to align the default buffers
  2012-04-13 18:37   ` Scott Wood
@ 2012-04-13 18:52     ` Simon Glass
  2012-04-13 19:17       ` Scott Wood
  0 siblings, 1 reply; 45+ messages in thread
From: Simon Glass @ 2012-04-13 18:52 UTC (permalink / raw)
  To: u-boot

Hi Scott,

On Fri, Apr 13, 2012 at 11:37 AM, Scott Wood <scottwood@freescale.com> wrote:
> On 04/13/2012 01:29 PM, Simon Glass wrote:
>> The NAND layer needs to use cache-aligned buffers by default. Towards this
>> goal. align the default buffers and their members according to the minimum
>> DMA alignment defined for the architecture.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>> Changes in v2:
>> - Add new patch to align default buffers in nand_base
>>
>> ?drivers/mtd/nand/nand_base.c | ? ?3 ++-
>> ?include/linux/mtd/nand.h ? ? | ? ?7 ++++---
>> ?2 files changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
>> index 44f7b91..7bfc29e 100644
>> --- a/drivers/mtd/nand/nand_base.c
>> +++ b/drivers/mtd/nand/nand_base.c
>> @@ -2935,7 +2935,8 @@ int nand_scan_tail(struct mtd_info *mtd)
>> ? ? ? struct nand_chip *chip = mtd->priv;
>>
>> ? ? ? if (!(chip->options & NAND_OWN_BUFFERS))
>> - ? ? ? ? ? ? chip->buffers = kmalloc(sizeof(*chip->buffers), GFP_KERNEL);
>> + ? ? ? ? ? ? chip->buffers = memalign(ARCH_DMA_MINALIGN,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?sizeof(*chip->buffers));
>
> This sort of requirement seems to be what NAND_OWN_BUFFERS was made for.

That's a bit of a cop-out I think. Arguably the current NAND code is
deliberately ignoring DMA alignment and requiring bounce buffers in
the drivers to deal with its ignorance. Other subsystems are changing
over, so what not NAND?

>
>> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
>> index da6fa18..ae0bdf6 100644
>> --- a/include/linux/mtd/nand.h
>> +++ b/include/linux/mtd/nand.h
>> @@ -391,9 +391,10 @@ struct nand_ecc_ctrl {
>> ? * consecutive order.
>> ? */
>> ?struct nand_buffers {
>> - ? ? uint8_t ecccalc[NAND_MAX_OOBSIZE];
>> - ? ? uint8_t ecccode[NAND_MAX_OOBSIZE];
>> - ? ? uint8_t databuf[NAND_MAX_PAGESIZE + NAND_MAX_OOBSIZE];
>> + ? ? uint8_t ecccalc[ALIGN(NAND_MAX_OOBSIZE, ARCH_DMA_MINALIGN)];
>> + ? ? uint8_t ecccode[ALIGN(NAND_MAX_OOBSIZE, ARCH_DMA_MINALIGN)];
>> + ? ? uint8_t databuf[ALIGN(NAND_MAX_PAGESIZE + NAND_MAX_OOBSIZE,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ARCH_DMA_MINALIGN)];
>> ?};
>
> I remember a while back someone wanting to change this to be pointers
> intsead of arrays, so that the driver can manage alignment -- I don't
> recall what happened to that.

I was concerned about the comment "Do not change the order of buffers.
databuf and oobrbuf must be in consecutive order." but then I couldn't
find oobrbuf so perhaps it is not true.

Anyway, alignment seems like a small price to pay - if the NAND layer
is going to allocate buffers they may as well be generally useful.

>
> -Scott
>

Regards,
Simon

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

* Re: [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
  2012-04-13 18:43     ` [U-Boot] " Scott Wood
@ 2012-04-13 19:01       ` Simon Glass
  -1 siblings, 0 replies; 45+ messages in thread
From: Simon Glass @ 2012-04-13 19:01 UTC (permalink / raw)
  To: Scott Wood
  Cc: Devicetree Discuss, U-Boot Mailing List, Jerry Van Baren, Tom Warren

Hi Scott,

On Fri, Apr 13, 2012 at 11:43 AM, Scott Wood <scottwood@freescale.com> wrote:
> On 04/13/2012 01:29 PM, Simon Glass wrote:
>> Add a NAND controller along with a bindings file for review.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>> Changes in v2:
>> - Update NAND binding to add "nvidia," prefix
>>
>>  arch/arm/dts/tegra20.dtsi                     |    6 ++
>>  doc/device-tree-bindings/nand/nvidia-nand.txt |   68 +++++++++++++++++++++++++
>>  2 files changed, 74 insertions(+), 0 deletions(-)
>>  create mode 100644 doc/device-tree-bindings/nand/nvidia-nand.txt
>>
>> diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
>> index bc64f42..7be0462 100644
>> --- a/arch/arm/dts/tegra20.dtsi
>> +++ b/arch/arm/dts/tegra20.dtsi
>> @@ -200,4 +200,10 @@
>>               reg = <0x7000f400 0x200>;
>>       };
>>
>> +     nand: nand-controller@0x70008000 {
>
> s/nand-controller@/flash@/ (or "nand@" if you really want -- there's
> enough of that in use already)

Changed to flash@.

I am a little concerned that we are co-mingling the controller with
the device, but I think this is ok.

>
>> +             #address-cells = <0>;
>> +             #size-cells = <0>;
>> +             compatible = "nvidia,tegra20-nand";
>> +             reg = <0x70008000 0x100>;
>> +     };
>>  };
>> diff --git a/doc/device-tree-bindings/nand/nvidia-nand.txt b/doc/device-tree-bindings/nand/nvidia-nand.txt
>> new file mode 100644
>> index 0000000..b19ce8e
>> --- /dev/null
>> +++ b/doc/device-tree-bindings/nand/nvidia-nand.txt
>> @@ -0,0 +1,68 @@
>> +NAND Flash
>> +----------
>> +
>> +(there isn't yet a generic binding in Linux, so this describes what is in
>> +U-Boot)
>
> Ideally the binding should not be Linux-specific or U-Boot specific --
> it's just the binding that describes this hardware.

Agreed, but trying to agree a binding in Linux in the absence of a
driver may be beyond my powers. Let's see if anyone picks up on this,
I could be wrong. I will add a comment in the meantime.

>
>> +The device node for a NAND flash device is as described in the document
>> +"Open Firmware Recommended Practice : Universal Serial Bus" with the
>> +following modifications and additions :
>> +
>> +Required properties :
>> + - compatible : Should be "manufacture,device", "nand-flash"
>
> s/manufacture/manufacturer/

Done

>
> No "nand-flash" compatible, as there's no standard "nand-flash" binding.
>  You might want something like "nvidia,tegra20-nand-chip".
>
>> + - nvidia,page-data-bytes : Number of bytes in the data area
>> + - nvidia,page-spare-bytes : * Number of bytes in spare area
>> +       spare area = skipped-spare-bytes + data-ecc-bytes + tag-bytes
>> +                     + tag-ecc-bytes
>> + - nvidia,skipped-spare-bytes : Number of bytes to skip at start of spare area
>> +     (these are typically used for bad block maintenance)
>> + - nvidia,data-ecc-bytes : Number of ECC bytes for data area
>> + - nvidia,tag-bytes :Number of tag bytes in spare area
>> + - nvidia,tag-ecc-bytes : Number ECC bytes to be generated for tag bytes
>> +
>> +(replace -bytes with -size or -length?)
>
> I like bytes -- makes the unit clear.

OK, will remove comment.

>
>> +Nvidia NAND Controller
>> +----------------------
>> +
>> +The device node for a NAND flash controller is as described in the document
>> +"Open Firmware Recommended Practice : Universal Serial Bus" with the
>> +following modifications and additions :
>> +
>> +Optional properties:
>> +
>> +wp-gpio : GPIO of write-protect line, three cells in the format:
>> +             phandle, parameter, flags
>
> nvidia,nand-wp-gpio

Done, nvidia,nand-wp-gpios which seems to the the standard in Linux.

>
>> +nvidia,,width : bus width of the NAND device in bits
>
> s/,,width/,nand-width/

Done

>
>> +For now here is something specific to the Nvidia controller,
>
> Isn't this whole file specific to the nvidia controller?

Yes, removed.

>
> -Scott
>

Regards,
Simon

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

* [U-Boot] [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
@ 2012-04-13 19:01       ` Simon Glass
  0 siblings, 0 replies; 45+ messages in thread
From: Simon Glass @ 2012-04-13 19:01 UTC (permalink / raw)
  To: u-boot

Hi Scott,

On Fri, Apr 13, 2012 at 11:43 AM, Scott Wood <scottwood@freescale.com> wrote:
> On 04/13/2012 01:29 PM, Simon Glass wrote:
>> Add a NAND controller along with a bindings file for review.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>> Changes in v2:
>> - Update NAND binding to add "nvidia," prefix
>>
>> ?arch/arm/dts/tegra20.dtsi ? ? ? ? ? ? ? ? ? ? | ? ?6 ++
>> ?doc/device-tree-bindings/nand/nvidia-nand.txt | ? 68 +++++++++++++++++++++++++
>> ?2 files changed, 74 insertions(+), 0 deletions(-)
>> ?create mode 100644 doc/device-tree-bindings/nand/nvidia-nand.txt
>>
>> diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
>> index bc64f42..7be0462 100644
>> --- a/arch/arm/dts/tegra20.dtsi
>> +++ b/arch/arm/dts/tegra20.dtsi
>> @@ -200,4 +200,10 @@
>> ? ? ? ? ? ? ? reg = <0x7000f400 0x200>;
>> ? ? ? };
>>
>> + ? ? nand: nand-controller at 0x70008000 {
>
> s/nand-controller@/flash@/ (or "nand@" if you really want -- there's
> enough of that in use already)

Changed to flash at .

I am a little concerned that we are co-mingling the controller with
the device, but I think this is ok.

>
>> + ? ? ? ? ? ? #address-cells = <0>;
>> + ? ? ? ? ? ? #size-cells = <0>;
>> + ? ? ? ? ? ? compatible = "nvidia,tegra20-nand";
>> + ? ? ? ? ? ? reg = <0x70008000 0x100>;
>> + ? ? };
>> ?};
>> diff --git a/doc/device-tree-bindings/nand/nvidia-nand.txt b/doc/device-tree-bindings/nand/nvidia-nand.txt
>> new file mode 100644
>> index 0000000..b19ce8e
>> --- /dev/null
>> +++ b/doc/device-tree-bindings/nand/nvidia-nand.txt
>> @@ -0,0 +1,68 @@
>> +NAND Flash
>> +----------
>> +
>> +(there isn't yet a generic binding in Linux, so this describes what is in
>> +U-Boot)
>
> Ideally the binding should not be Linux-specific or U-Boot specific --
> it's just the binding that describes this hardware.

Agreed, but trying to agree a binding in Linux in the absence of a
driver may be beyond my powers. Let's see if anyone picks up on this,
I could be wrong. I will add a comment in the meantime.

>
>> +The device node for a NAND flash device is as described in the document
>> +"Open Firmware Recommended Practice : Universal Serial Bus" with the
>> +following modifications and additions :
>> +
>> +Required properties :
>> + - compatible : Should be "manufacture,device", "nand-flash"
>
> s/manufacture/manufacturer/

Done

>
> No "nand-flash" compatible, as there's no standard "nand-flash" binding.
> ?You might want something like "nvidia,tegra20-nand-chip".
>
>> + - nvidia,page-data-bytes : Number of bytes in the data area
>> + - nvidia,page-spare-bytes : * Number of bytes in spare area
>> + ? ? ? spare area = skipped-spare-bytes + data-ecc-bytes + tag-bytes
>> + ? ? ? ? ? ? ? ? ? ? + tag-ecc-bytes
>> + - nvidia,skipped-spare-bytes : Number of bytes to skip at start of spare area
>> + ? ? (these are typically used for bad block maintenance)
>> + - nvidia,data-ecc-bytes : Number of ECC bytes for data area
>> + - nvidia,tag-bytes :Number of tag bytes in spare area
>> + - nvidia,tag-ecc-bytes : Number ECC bytes to be generated for tag bytes
>> +
>> +(replace -bytes with -size or -length?)
>
> I like bytes -- makes the unit clear.

OK, will remove comment.

>
>> +Nvidia NAND Controller
>> +----------------------
>> +
>> +The device node for a NAND flash controller is as described in the document
>> +"Open Firmware Recommended Practice : Universal Serial Bus" with the
>> +following modifications and additions :
>> +
>> +Optional properties:
>> +
>> +wp-gpio : GPIO of write-protect line, three cells in the format:
>> + ? ? ? ? ? ? phandle, parameter, flags
>
> nvidia,nand-wp-gpio

Done, nvidia,nand-wp-gpios which seems to the the standard in Linux.

>
>> +nvidia,,width : bus width of the NAND device in bits
>
> s/,,width/,nand-width/

Done

>
>> +For now here is something specific to the Nvidia controller,
>
> Isn't this whole file specific to the nvidia controller?

Yes, removed.

>
> -Scott
>

Regards,
Simon

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

* Re: [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
  2012-04-13 19:01       ` [U-Boot] " Simon Glass
@ 2012-04-13 19:07         ` Scott Wood
  -1 siblings, 0 replies; 45+ messages in thread
From: Scott Wood @ 2012-04-13 19:07 UTC (permalink / raw)
  To: Simon Glass
  Cc: Devicetree, Discuss, U-Boot Mailing List, Jerry Van Baren, Tom Warren

On 04/13/2012 02:01 PM, Simon Glass wrote:
> Hi Scott,
> 
> On Fri, Apr 13, 2012 at 11:43 AM, Scott Wood <scottwood@freescale.com> wrote:
>> On 04/13/2012 01:29 PM, Simon Glass wrote:
>>> Add a NAND controller along with a bindings file for review.
>>>
>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>> ---
>>> Changes in v2:
>>> - Update NAND binding to add "nvidia," prefix
>>>
>>>  arch/arm/dts/tegra20.dtsi                     |    6 ++
>>>  doc/device-tree-bindings/nand/nvidia-nand.txt |   68 +++++++++++++++++++++++++
>>>  2 files changed, 74 insertions(+), 0 deletions(-)
>>>  create mode 100644 doc/device-tree-bindings/nand/nvidia-nand.txt
>>>
>>> diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
>>> index bc64f42..7be0462 100644
>>> --- a/arch/arm/dts/tegra20.dtsi
>>> +++ b/arch/arm/dts/tegra20.dtsi
>>> @@ -200,4 +200,10 @@
>>>               reg = <0x7000f400 0x200>;
>>>       };
>>>
>>> +     nand: nand-controller@0x70008000 {
>>
>> s/nand-controller@/flash@/ (or "nand@" if you really want -- there's
>> enough of that in use already)
> 
> Changed to flash@.
> 
> I am a little concerned that we are co-mingling the controller with
> the device, but I think this is ok.

No, you're right -- it should be something like nand-controller@.  For
some reason I didn't notice the node split when I wrote that.

>>> +             #address-cells = <0>;
>>> +             #size-cells = <0>;
>>> +             compatible = "nvidia,tegra20-nand";
>>> +             reg = <0x70008000 0x100>;
>>> +     };
>>>  };
>>> diff --git a/doc/device-tree-bindings/nand/nvidia-nand.txt b/doc/device-tree-bindings/nand/nvidia-nand.txt
>>> new file mode 100644
>>> index 0000000..b19ce8e
>>> --- /dev/null
>>> +++ b/doc/device-tree-bindings/nand/nvidia-nand.txt
>>> @@ -0,0 +1,68 @@
>>> +NAND Flash
>>> +----------
>>> +
>>> +(there isn't yet a generic binding in Linux, so this describes what is in
>>> +U-Boot)
>>
>> Ideally the binding should not be Linux-specific or U-Boot specific --
>> it's just the binding that describes this hardware.
> 
> Agreed, but trying to agree a binding in Linux in the absence of a
> driver may be beyond my powers.

It shouldn't be, and if it is then we should move on to a better binding
repository (Grant set up devicetree.org for this a while back, but I'm
not sure what the process is for considering a binding there to be final).

-Scott

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

* [U-Boot] [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
@ 2012-04-13 19:07         ` Scott Wood
  0 siblings, 0 replies; 45+ messages in thread
From: Scott Wood @ 2012-04-13 19:07 UTC (permalink / raw)
  To: u-boot

On 04/13/2012 02:01 PM, Simon Glass wrote:
> Hi Scott,
> 
> On Fri, Apr 13, 2012 at 11:43 AM, Scott Wood <scottwood@freescale.com> wrote:
>> On 04/13/2012 01:29 PM, Simon Glass wrote:
>>> Add a NAND controller along with a bindings file for review.
>>>
>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>> ---
>>> Changes in v2:
>>> - Update NAND binding to add "nvidia," prefix
>>>
>>>  arch/arm/dts/tegra20.dtsi                     |    6 ++
>>>  doc/device-tree-bindings/nand/nvidia-nand.txt |   68 +++++++++++++++++++++++++
>>>  2 files changed, 74 insertions(+), 0 deletions(-)
>>>  create mode 100644 doc/device-tree-bindings/nand/nvidia-nand.txt
>>>
>>> diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
>>> index bc64f42..7be0462 100644
>>> --- a/arch/arm/dts/tegra20.dtsi
>>> +++ b/arch/arm/dts/tegra20.dtsi
>>> @@ -200,4 +200,10 @@
>>>               reg = <0x7000f400 0x200>;
>>>       };
>>>
>>> +     nand: nand-controller at 0x70008000 {
>>
>> s/nand-controller@/flash@/ (or "nand@" if you really want -- there's
>> enough of that in use already)
> 
> Changed to flash at .
> 
> I am a little concerned that we are co-mingling the controller with
> the device, but I think this is ok.

No, you're right -- it should be something like nand-controller at .  For
some reason I didn't notice the node split when I wrote that.

>>> +             #address-cells = <0>;
>>> +             #size-cells = <0>;
>>> +             compatible = "nvidia,tegra20-nand";
>>> +             reg = <0x70008000 0x100>;
>>> +     };
>>>  };
>>> diff --git a/doc/device-tree-bindings/nand/nvidia-nand.txt b/doc/device-tree-bindings/nand/nvidia-nand.txt
>>> new file mode 100644
>>> index 0000000..b19ce8e
>>> --- /dev/null
>>> +++ b/doc/device-tree-bindings/nand/nvidia-nand.txt
>>> @@ -0,0 +1,68 @@
>>> +NAND Flash
>>> +----------
>>> +
>>> +(there isn't yet a generic binding in Linux, so this describes what is in
>>> +U-Boot)
>>
>> Ideally the binding should not be Linux-specific or U-Boot specific --
>> it's just the binding that describes this hardware.
> 
> Agreed, but trying to agree a binding in Linux in the absence of a
> driver may be beyond my powers.

It shouldn't be, and if it is then we should move on to a better binding
repository (Grant set up devicetree.org for this a while back, but I'm
not sure what the process is for considering a binding there to be final).

-Scott

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

* Re: [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
  2012-04-13 19:07         ` [U-Boot] " Scott Wood
@ 2012-04-13 19:16           ` Simon Glass
  -1 siblings, 0 replies; 45+ messages in thread
From: Simon Glass @ 2012-04-13 19:16 UTC (permalink / raw)
  To: Scott Wood
  Cc: Devicetree Discuss, U-Boot Mailing List, Jerry Van Baren, Tom Warren

Hi Scott,

On Fri, Apr 13, 2012 at 12:07 PM, Scott Wood <scottwood@freescale.com> wrote:
> On 04/13/2012 02:01 PM, Simon Glass wrote:
>> Hi Scott,
>>
>> On Fri, Apr 13, 2012 at 11:43 AM, Scott Wood <scottwood@freescale.com> wrote:
>>> On 04/13/2012 01:29 PM, Simon Glass wrote:
>>>> Add a NAND controller along with a bindings file for review.
>>>>
>>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>>> ---
>>>> Changes in v2:
>>>> - Update NAND binding to add "nvidia," prefix
>>>>
>>>>  arch/arm/dts/tegra20.dtsi                     |    6 ++
>>>>  doc/device-tree-bindings/nand/nvidia-nand.txt |   68 +++++++++++++++++++++++++
>>>>  2 files changed, 74 insertions(+), 0 deletions(-)
>>>>  create mode 100644 doc/device-tree-bindings/nand/nvidia-nand.txt
>>>>
>>>> diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
>>>> index bc64f42..7be0462 100644
>>>> --- a/arch/arm/dts/tegra20.dtsi
>>>> +++ b/arch/arm/dts/tegra20.dtsi
>>>> @@ -200,4 +200,10 @@
>>>>               reg = <0x7000f400 0x200>;
>>>>       };
>>>>
>>>> +     nand: nand-controller@0x70008000 {
>>>
>>> s/nand-controller@/flash@/ (or "nand@" if you really want -- there's
>>> enough of that in use already)
>>
>> Changed to flash@.
>>
>> I am a little concerned that we are co-mingling the controller with
>> the device, but I think this is ok.
>
> No, you're right -- it should be something like nand-controller@.  For
> some reason I didn't notice the node split when I wrote that.

OK, changed it back.

>
>>>> +             #address-cells = <0>;
>>>> +             #size-cells = <0>;
>>>> +             compatible = "nvidia,tegra20-nand";
>>>> +             reg = <0x70008000 0x100>;
>>>> +     };
>>>>  };
>>>> diff --git a/doc/device-tree-bindings/nand/nvidia-nand.txt b/doc/device-tree-bindings/nand/nvidia-nand.txt
>>>> new file mode 100644
>>>> index 0000000..b19ce8e
>>>> --- /dev/null
>>>> +++ b/doc/device-tree-bindings/nand/nvidia-nand.txt
>>>> @@ -0,0 +1,68 @@
>>>> +NAND Flash
>>>> +----------
>>>> +
>>>> +(there isn't yet a generic binding in Linux, so this describes what is in
>>>> +U-Boot)
>>>
>>> Ideally the binding should not be Linux-specific or U-Boot specific --
>>> it's just the binding that describes this hardware.
>>
>> Agreed, but trying to agree a binding in Linux in the absence of a
>> driver may be beyond my powers.
>
> It shouldn't be, and if it is then we should move on to a better binding
> repository (Grant set up devicetree.org for this a while back, but I'm
> not sure what the process is for considering a binding there to be final).

Well we probably agree there should be a new repo for this. This is
going to the right mailing list (Devicetree Discuss), so people can
chime in as needed.

>
> -Scott
>

Regards,
Simon

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

* [U-Boot] [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
@ 2012-04-13 19:16           ` Simon Glass
  0 siblings, 0 replies; 45+ messages in thread
From: Simon Glass @ 2012-04-13 19:16 UTC (permalink / raw)
  To: u-boot

Hi Scott,

On Fri, Apr 13, 2012 at 12:07 PM, Scott Wood <scottwood@freescale.com> wrote:
> On 04/13/2012 02:01 PM, Simon Glass wrote:
>> Hi Scott,
>>
>> On Fri, Apr 13, 2012 at 11:43 AM, Scott Wood <scottwood@freescale.com> wrote:
>>> On 04/13/2012 01:29 PM, Simon Glass wrote:
>>>> Add a NAND controller along with a bindings file for review.
>>>>
>>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>>> ---
>>>> Changes in v2:
>>>> - Update NAND binding to add "nvidia," prefix
>>>>
>>>> ?arch/arm/dts/tegra20.dtsi ? ? ? ? ? ? ? ? ? ? | ? ?6 ++
>>>> ?doc/device-tree-bindings/nand/nvidia-nand.txt | ? 68 +++++++++++++++++++++++++
>>>> ?2 files changed, 74 insertions(+), 0 deletions(-)
>>>> ?create mode 100644 doc/device-tree-bindings/nand/nvidia-nand.txt
>>>>
>>>> diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
>>>> index bc64f42..7be0462 100644
>>>> --- a/arch/arm/dts/tegra20.dtsi
>>>> +++ b/arch/arm/dts/tegra20.dtsi
>>>> @@ -200,4 +200,10 @@
>>>> ? ? ? ? ? ? ? reg = <0x7000f400 0x200>;
>>>> ? ? ? };
>>>>
>>>> + ? ? nand: nand-controller at 0x70008000 {
>>>
>>> s/nand-controller@/flash@/ (or "nand@" if you really want -- there's
>>> enough of that in use already)
>>
>> Changed to flash at .
>>
>> I am a little concerned that we are co-mingling the controller with
>> the device, but I think this is ok.
>
> No, you're right -- it should be something like nand-controller at . ?For
> some reason I didn't notice the node split when I wrote that.

OK, changed it back.

>
>>>> + ? ? ? ? ? ? #address-cells = <0>;
>>>> + ? ? ? ? ? ? #size-cells = <0>;
>>>> + ? ? ? ? ? ? compatible = "nvidia,tegra20-nand";
>>>> + ? ? ? ? ? ? reg = <0x70008000 0x100>;
>>>> + ? ? };
>>>> ?};
>>>> diff --git a/doc/device-tree-bindings/nand/nvidia-nand.txt b/doc/device-tree-bindings/nand/nvidia-nand.txt
>>>> new file mode 100644
>>>> index 0000000..b19ce8e
>>>> --- /dev/null
>>>> +++ b/doc/device-tree-bindings/nand/nvidia-nand.txt
>>>> @@ -0,0 +1,68 @@
>>>> +NAND Flash
>>>> +----------
>>>> +
>>>> +(there isn't yet a generic binding in Linux, so this describes what is in
>>>> +U-Boot)
>>>
>>> Ideally the binding should not be Linux-specific or U-Boot specific --
>>> it's just the binding that describes this hardware.
>>
>> Agreed, but trying to agree a binding in Linux in the absence of a
>> driver may be beyond my powers.
>
> It shouldn't be, and if it is then we should move on to a better binding
> repository (Grant set up devicetree.org for this a while back, but I'm
> not sure what the process is for considering a binding there to be final).

Well we probably agree there should be a new repo for this. This is
going to the right mailing list (Devicetree Discuss), so people can
chime in as needed.

>
> -Scott
>

Regards,
Simon

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

* [U-Boot] [PATCH v2 1/7] nand: Try to align the default buffers
  2012-04-13 18:52     ` Simon Glass
@ 2012-04-13 19:17       ` Scott Wood
  2012-04-13 19:24         ` Simon Glass
  0 siblings, 1 reply; 45+ messages in thread
From: Scott Wood @ 2012-04-13 19:17 UTC (permalink / raw)
  To: u-boot

On 04/13/2012 01:52 PM, Simon Glass wrote:
> Hi Scott,
> 
> On Fri, Apr 13, 2012 at 11:37 AM, Scott Wood <scottwood@freescale.com> wrote:
>> On 04/13/2012 01:29 PM, Simon Glass wrote:
>>> The NAND layer needs to use cache-aligned buffers by default. Towards this
>>> goal. align the default buffers and their members according to the minimum
>>> DMA alignment defined for the architecture.
>>>
>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>> ---
>>> Changes in v2:
>>> - Add new patch to align default buffers in nand_base
>>>
>>>  drivers/mtd/nand/nand_base.c |    3 ++-
>>>  include/linux/mtd/nand.h     |    7 ++++---
>>>  2 files changed, 6 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
>>> index 44f7b91..7bfc29e 100644
>>> --- a/drivers/mtd/nand/nand_base.c
>>> +++ b/drivers/mtd/nand/nand_base.c
>>> @@ -2935,7 +2935,8 @@ int nand_scan_tail(struct mtd_info *mtd)
>>>       struct nand_chip *chip = mtd->priv;
>>>
>>>       if (!(chip->options & NAND_OWN_BUFFERS))
>>> -             chip->buffers = kmalloc(sizeof(*chip->buffers), GFP_KERNEL);
>>> +             chip->buffers = memalign(ARCH_DMA_MINALIGN,
>>> +                                      sizeof(*chip->buffers));
>>
>> This sort of requirement seems to be what NAND_OWN_BUFFERS was made for.
> 
> That's a bit of a cop-out I think. Arguably the current NAND code is
> deliberately ignoring DMA alignment and requiring bounce buffers in
> the drivers to deal with its ignorance. Other subsystems are changing
> over, so what not NAND?

Most NAND drivers do not do DMA, and ARCH_DMA_MINALIGN seems a little
oversimple -- what if I have one device that needs more alignment than
another on the same system?  I guess it's better to just apply the max
alignment of any device if the cost is low, but what if some driver
starts using ARCH_DMA_MINALIGN to determine whether it needs bounce buffers?

>>> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
>>> index da6fa18..ae0bdf6 100644
>>> --- a/include/linux/mtd/nand.h
>>> +++ b/include/linux/mtd/nand.h
>>> @@ -391,9 +391,10 @@ struct nand_ecc_ctrl {
>>>   * consecutive order.
>>>   */
>>>  struct nand_buffers {
>>> -     uint8_t ecccalc[NAND_MAX_OOBSIZE];
>>> -     uint8_t ecccode[NAND_MAX_OOBSIZE];
>>> -     uint8_t databuf[NAND_MAX_PAGESIZE + NAND_MAX_OOBSIZE];
>>> +     uint8_t ecccalc[ALIGN(NAND_MAX_OOBSIZE, ARCH_DMA_MINALIGN)];
>>> +     uint8_t ecccode[ALIGN(NAND_MAX_OOBSIZE, ARCH_DMA_MINALIGN)];
>>> +     uint8_t databuf[ALIGN(NAND_MAX_PAGESIZE + NAND_MAX_OOBSIZE,
>>> +                           ARCH_DMA_MINALIGN)];
>>>  };
>>
>> I remember a while back someone wanting to change this to be pointers
>> intsead of arrays, so that the driver can manage alignment -- I don't
>> recall what happened to that.
> 
> I was concerned about the comment "Do not change the order of buffers.
> databuf and oobrbuf must be in consecutive order." but then I couldn't
> find oobrbuf so perhaps it is not true.

Maybe databuf[] used to be split into two arrays?

> Anyway, alignment seems like a small price to pay - if the NAND layer
> is going to allocate buffers they may as well be generally useful.

They have been useful to the most drivers so far, but I guess this is
OK... we'd have to make one change or another anyway (either add
alignment or convert to pointers).

-Scott

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

* [U-Boot] [PATCH v2 1/7] nand: Try to align the default buffers
  2012-04-13 19:17       ` Scott Wood
@ 2012-04-13 19:24         ` Simon Glass
  0 siblings, 0 replies; 45+ messages in thread
From: Simon Glass @ 2012-04-13 19:24 UTC (permalink / raw)
  To: u-boot

Hi Scott,

On Fri, Apr 13, 2012 at 12:17 PM, Scott Wood <scottwood@freescale.com> wrote:
> On 04/13/2012 01:52 PM, Simon Glass wrote:
>> Hi Scott,
>>
>> On Fri, Apr 13, 2012 at 11:37 AM, Scott Wood <scottwood@freescale.com> wrote:
>>> On 04/13/2012 01:29 PM, Simon Glass wrote:
>>>> The NAND layer needs to use cache-aligned buffers by default. Towards this
>>>> goal. align the default buffers and their members according to the minimum
>>>> DMA alignment defined for the architecture.
>>>>
>>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>>> ---
>>>> Changes in v2:
>>>> - Add new patch to align default buffers in nand_base
>>>>
>>>> ?drivers/mtd/nand/nand_base.c | ? ?3 ++-
>>>> ?include/linux/mtd/nand.h ? ? | ? ?7 ++++---
>>>> ?2 files changed, 6 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
>>>> index 44f7b91..7bfc29e 100644
>>>> --- a/drivers/mtd/nand/nand_base.c
>>>> +++ b/drivers/mtd/nand/nand_base.c
>>>> @@ -2935,7 +2935,8 @@ int nand_scan_tail(struct mtd_info *mtd)
>>>> ? ? ? struct nand_chip *chip = mtd->priv;
>>>>
>>>> ? ? ? if (!(chip->options & NAND_OWN_BUFFERS))
>>>> - ? ? ? ? ? ? chip->buffers = kmalloc(sizeof(*chip->buffers), GFP_KERNEL);
>>>> + ? ? ? ? ? ? chip->buffers = memalign(ARCH_DMA_MINALIGN,
>>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?sizeof(*chip->buffers));
>>>
>>> This sort of requirement seems to be what NAND_OWN_BUFFERS was made for.
>>
>> That's a bit of a cop-out I think. Arguably the current NAND code is
>> deliberately ignoring DMA alignment and requiring bounce buffers in
>> the drivers to deal with its ignorance. Other subsystems are changing
>> over, so what not NAND?
>
> Most NAND drivers do not do DMA, and ARCH_DMA_MINALIGN seems a little
> oversimple -- what if I have one device that needs more alignment than
> another on the same system? ?I guess it's better to just apply the max
> alignment of any device if the cost is low, but what if some driver
> starts using ARCH_DMA_MINALIGN to determine whether it needs bounce buffers?

I have not seen that problem, but ARCH_DMA_MINALIGN is a system option
- it generally comes from CONFIG_SYS_CACHELINE_SIZE which can be
adjusted on a per-board basis if needed.

I am rather hoping that we could avoid bounce buffers. If you think
about it, the implications of every driver in every subsystem having
to add this logic are not good. While it might feel a bit like a
fiddle, it is more efficient on code size and complexity to do this
stuff higher up if possible.

>
>>>> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
>>>> index da6fa18..ae0bdf6 100644
>>>> --- a/include/linux/mtd/nand.h
>>>> +++ b/include/linux/mtd/nand.h
>>>> @@ -391,9 +391,10 @@ struct nand_ecc_ctrl {
>>>> ? * consecutive order.
>>>> ? */
>>>> ?struct nand_buffers {
>>>> - ? ? uint8_t ecccalc[NAND_MAX_OOBSIZE];
>>>> - ? ? uint8_t ecccode[NAND_MAX_OOBSIZE];
>>>> - ? ? uint8_t databuf[NAND_MAX_PAGESIZE + NAND_MAX_OOBSIZE];
>>>> + ? ? uint8_t ecccalc[ALIGN(NAND_MAX_OOBSIZE, ARCH_DMA_MINALIGN)];
>>>> + ? ? uint8_t ecccode[ALIGN(NAND_MAX_OOBSIZE, ARCH_DMA_MINALIGN)];
>>>> + ? ? uint8_t databuf[ALIGN(NAND_MAX_PAGESIZE + NAND_MAX_OOBSIZE,
>>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ARCH_DMA_MINALIGN)];
>>>> ?};
>>>
>>> I remember a while back someone wanting to change this to be pointers
>>> intsead of arrays, so that the driver can manage alignment -- I don't
>>> recall what happened to that.
>>
>> I was concerned about the comment "Do not change the order of buffers.
>> databuf and oobrbuf must be in consecutive order." but then I couldn't
>> find oobrbuf so perhaps it is not true.
>
> Maybe databuf[] used to be split into two arrays?

Yes that's probably it.

>
>> Anyway, alignment seems like a small price to pay - if the NAND layer
>> is going to allocate buffers they may as well be generally useful.
>
> They have been useful to the most drivers so far, but I guess this is
> OK... we'd have to make one change or another anyway (either add
> alignment or convert to pointers).

Yes.

>
> -Scott
>

Regards,
Simon

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

* Re: [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
  2012-04-13 18:43     ` [U-Boot] " Scott Wood
@ 2012-04-13 20:58       ` Stephen Warren
  -1 siblings, 0 replies; 45+ messages in thread
From: Stephen Warren @ 2012-04-13 20:58 UTC (permalink / raw)
  To: Scott Wood
  Cc: Devicetree Discuss, U-Boot Mailing List, Jerry Van Baren, Tom Warren

On 04/13/2012 12:43 PM, Scott Wood wrote:
> On 04/13/2012 01:29 PM, Simon Glass wrote:
>> Add a NAND controller along with a bindings file for review.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>

>> +++ b/doc/device-tree-bindings/nand/nvidia-nand.txt

>> +wp-gpio : GPIO of write-protect line, three cells in the format:
>> +		phandle, parameter, flags
> 
> nvidia,nand-wp-gpio

I'm not convinced about this. For example many SDHCI bindings use just
"wp-gpios" not "shdci-wp-gpios". Is there really a need to keep the
property names unique across all bindings, even though a given node only
relies on one binding?

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

* [U-Boot] [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
@ 2012-04-13 20:58       ` Stephen Warren
  0 siblings, 0 replies; 45+ messages in thread
From: Stephen Warren @ 2012-04-13 20:58 UTC (permalink / raw)
  To: u-boot

On 04/13/2012 12:43 PM, Scott Wood wrote:
> On 04/13/2012 01:29 PM, Simon Glass wrote:
>> Add a NAND controller along with a bindings file for review.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>

>> +++ b/doc/device-tree-bindings/nand/nvidia-nand.txt

>> +wp-gpio : GPIO of write-protect line, three cells in the format:
>> +		phandle, parameter, flags
> 
> nvidia,nand-wp-gpio

I'm not convinced about this. For example many SDHCI bindings use just
"wp-gpios" not "shdci-wp-gpios". Is there really a need to keep the
property names unique across all bindings, even though a given node only
relies on one binding?

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

* Re: [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
  2012-04-13 18:29   ` [U-Boot] " Simon Glass
@ 2012-04-13 21:05     ` Stephen Warren
  -1 siblings, 0 replies; 45+ messages in thread
From: Stephen Warren @ 2012-04-13 21:05 UTC (permalink / raw)
  To: Simon Glass
  Cc: Devicetree Discuss, U-Boot Mailing List, Jerry Van Baren,
	Tom Warren, Scott Wood

On 04/13/2012 12:29 PM, Simon Glass wrote:
> Add a NAND controller along with a bindings file for review.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

> +++ b/doc/device-tree-bindings/nand/nvidia-nand.txt

I'd prefer this be called nvidia,tegra20-nand.txt so filenames are named
according to compatible value. This makes it easier to look things up.

> +The device node for a NAND flash device is as described in the document
> +"Open Firmware Recommended Practice : Universal Serial Bus" with the

This is really based on USB?

> +Required properties :
> + - compatible : Should be "manufacture,device", "nand-flash"
> + - nvidia,page-data-bytes : Number of bytes in the data area
> + - nvidia,page-spare-bytes : * Number of bytes in spare area

Not sure what that "*" is?

> +Nvidia NAND Controller
> +----------------------
> +
> +The device node for a NAND flash controller is as described in the document
> +"Open Firmware Recommended Practice : Universal Serial Bus" with the

USB again?

> +nand-controller@0x70008000 {
> +	compatible = "nvidia,tegra20-nand";
> +	wp-gpios = <&gpio 59 0>;		/* PH3 */
> +	nvidia,width = <8>;
> +	nvidia,timing = <26 100 20 80 20 10 12 10 70>;
> +	nand@0 {
> +		compatible = "hynix,hy27uf4g2b", "nand-flash";

The TRM says there can be up to 8 chip selects. Don't the NAND device
sub-nodes need a reg property to indicate which chip-select they're on?

Also, the TRM mentions async vs. ONFI devices. Don't we need properties
somewhere to configure that kind of thing?

> +		nvidia,page-data-bytes = <2048>;
> +		nvidia,tag-ecc-bytes = <4>;
> +		nvidia,tag-bytes = <20>;
> +		nvidia,data-ecc-bytes = <36>;
> +		nvidia,skipped-spare-bytes = <4>;
> +		nvidia,page-spare-bytes = <64>;
> +	};
> +};

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

* [U-Boot] [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
@ 2012-04-13 21:05     ` Stephen Warren
  0 siblings, 0 replies; 45+ messages in thread
From: Stephen Warren @ 2012-04-13 21:05 UTC (permalink / raw)
  To: u-boot

On 04/13/2012 12:29 PM, Simon Glass wrote:
> Add a NAND controller along with a bindings file for review.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

> +++ b/doc/device-tree-bindings/nand/nvidia-nand.txt

I'd prefer this be called nvidia,tegra20-nand.txt so filenames are named
according to compatible value. This makes it easier to look things up.

> +The device node for a NAND flash device is as described in the document
> +"Open Firmware Recommended Practice : Universal Serial Bus" with the

This is really based on USB?

> +Required properties :
> + - compatible : Should be "manufacture,device", "nand-flash"
> + - nvidia,page-data-bytes : Number of bytes in the data area
> + - nvidia,page-spare-bytes : * Number of bytes in spare area

Not sure what that "*" is?

> +Nvidia NAND Controller
> +----------------------
> +
> +The device node for a NAND flash controller is as described in the document
> +"Open Firmware Recommended Practice : Universal Serial Bus" with the

USB again?

> +nand-controller at 0x70008000 {
> +	compatible = "nvidia,tegra20-nand";
> +	wp-gpios = <&gpio 59 0>;		/* PH3 */
> +	nvidia,width = <8>;
> +	nvidia,timing = <26 100 20 80 20 10 12 10 70>;
> +	nand at 0 {
> +		compatible = "hynix,hy27uf4g2b", "nand-flash";

The TRM says there can be up to 8 chip selects. Don't the NAND device
sub-nodes need a reg property to indicate which chip-select they're on?

Also, the TRM mentions async vs. ONFI devices. Don't we need properties
somewhere to configure that kind of thing?

> +		nvidia,page-data-bytes = <2048>;
> +		nvidia,tag-ecc-bytes = <4>;
> +		nvidia,tag-bytes = <20>;
> +		nvidia,data-ecc-bytes = <36>;
> +		nvidia,skipped-spare-bytes = <4>;
> +		nvidia,page-spare-bytes = <64>;
> +	};
> +};

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

* Re: [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
  2012-04-13 21:05     ` [U-Boot] " Stephen Warren
@ 2012-04-13 21:12       ` Scott Wood
  -1 siblings, 0 replies; 45+ messages in thread
From: Scott Wood @ 2012-04-13 21:12 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Devicetree, U-Boot Mailing List, Jerry Van Baren, Tom Warren, Discuss

On 04/13/2012 04:05 PM, Stephen Warren wrote:
> On 04/13/2012 12:29 PM, Simon Glass wrote:
>> Add a NAND controller along with a bindings file for review.
>>
>> Signed-off-by: Simon Glass<sjg@chromium.org>
>
>> +++ b/doc/device-tree-bindings/nand/nvidia-nand.txt
>
> I'd prefer this be called nvidia,tegra20-nand.txt so filenames are named
> according to compatible value. This makes it easier to look things up.

Could be awkward if additional compatibles are added, though.  Grep can 
find compatibles within the text.

-Scott

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

* [U-Boot] [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
@ 2012-04-13 21:12       ` Scott Wood
  0 siblings, 0 replies; 45+ messages in thread
From: Scott Wood @ 2012-04-13 21:12 UTC (permalink / raw)
  To: u-boot

On 04/13/2012 04:05 PM, Stephen Warren wrote:
> On 04/13/2012 12:29 PM, Simon Glass wrote:
>> Add a NAND controller along with a bindings file for review.
>>
>> Signed-off-by: Simon Glass<sjg@chromium.org>
>
>> +++ b/doc/device-tree-bindings/nand/nvidia-nand.txt
>
> I'd prefer this be called nvidia,tegra20-nand.txt so filenames are named
> according to compatible value. This makes it easier to look things up.

Could be awkward if additional compatibles are added, though.  Grep can 
find compatibles within the text.

-Scott

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

* Re: [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
  2012-04-13 20:58       ` [U-Boot] " Stephen Warren
@ 2012-04-13 21:21         ` Scott Wood
  -1 siblings, 0 replies; 45+ messages in thread
From: Scott Wood @ 2012-04-13 21:21 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Devicetree, U-Boot Mailing List, Jerry Van Baren, Tom Warren, Discuss

On 04/13/2012 03:58 PM, Stephen Warren wrote:
> On 04/13/2012 12:43 PM, Scott Wood wrote:
>> On 04/13/2012 01:29 PM, Simon Glass wrote:
>>> Add a NAND controller along with a bindings file for review.
>>>
>>> Signed-off-by: Simon Glass<sjg@chromium.org>
>
>>> +++ b/doc/device-tree-bindings/nand/nvidia-nand.txt
>
>>> +wp-gpio : GPIO of write-protect line, three cells in the format:
>>> +		phandle, parameter, flags
>>
>> nvidia,nand-wp-gpio
>
> I'm not convinced about this. For example many SDHCI bindings use just
> "wp-gpios" not "shdci-wp-gpios". Is there really a need to keep the
> property names unique across all bindings, even though a given node only
> relies on one binding?
>

Yeah, there's a lot of bad practice in the existing trees.  But the 
general recommendation for a while now has been to namespace properties 
that aren't defined in standardized, device-indpendent way.  That way we 
don't get conflicts if we want to use that name for a standard property 
in the future, and there's less confusion if multiple people use the 
same name in different devices with different semantics.

-Scott

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

* [U-Boot] [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
@ 2012-04-13 21:21         ` Scott Wood
  0 siblings, 0 replies; 45+ messages in thread
From: Scott Wood @ 2012-04-13 21:21 UTC (permalink / raw)
  To: u-boot

On 04/13/2012 03:58 PM, Stephen Warren wrote:
> On 04/13/2012 12:43 PM, Scott Wood wrote:
>> On 04/13/2012 01:29 PM, Simon Glass wrote:
>>> Add a NAND controller along with a bindings file for review.
>>>
>>> Signed-off-by: Simon Glass<sjg@chromium.org>
>
>>> +++ b/doc/device-tree-bindings/nand/nvidia-nand.txt
>
>>> +wp-gpio : GPIO of write-protect line, three cells in the format:
>>> +		phandle, parameter, flags
>>
>> nvidia,nand-wp-gpio
>
> I'm not convinced about this. For example many SDHCI bindings use just
> "wp-gpios" not "shdci-wp-gpios". Is there really a need to keep the
> property names unique across all bindings, even though a given node only
> relies on one binding?
>

Yeah, there's a lot of bad practice in the existing trees.  But the 
general recommendation for a while now has been to namespace properties 
that aren't defined in standardized, device-indpendent way.  That way we 
don't get conflicts if we want to use that name for a standard property 
in the future, and there's less confusion if multiple people use the 
same name in different devices with different semantics.

-Scott

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

* Re: [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
  2012-04-13 21:21         ` [U-Boot] " Scott Wood
@ 2012-04-13 21:22           ` Stephen Warren
  -1 siblings, 0 replies; 45+ messages in thread
From: Stephen Warren @ 2012-04-13 21:22 UTC (permalink / raw)
  To: Scott Wood
  Cc: Devicetree Discuss, U-Boot Mailing List, Jerry Van Baren, Tom Warren

On 04/13/2012 03:21 PM, Scott Wood wrote:
> On 04/13/2012 03:58 PM, Stephen Warren wrote:
>> On 04/13/2012 12:43 PM, Scott Wood wrote:
>>> On 04/13/2012 01:29 PM, Simon Glass wrote:
>>>> Add a NAND controller along with a bindings file for review.
>>>>
>>>> Signed-off-by: Simon Glass<sjg@chromium.org>
>>
>>>> +++ b/doc/device-tree-bindings/nand/nvidia-nand.txt
>>
>>>> +wp-gpio : GPIO of write-protect line, three cells in the format:
>>>> +        phandle, parameter, flags
>>>
>>> nvidia,nand-wp-gpio
>>
>> I'm not convinced about this. For example many SDHCI bindings use just
>> "wp-gpios" not "shdci-wp-gpios". Is there really a need to keep the
>> property names unique across all bindings, even though a given node only
>> relies on one binding?
>>
> 
> Yeah, there's a lot of bad practice in the existing trees.  But the
> general recommendation for a while now has been to namespace properties
> that aren't defined in standardized, device-indpendent way.  That way we
> don't get conflicts if we want to use that name for a standard property
> in the future, and there's less confusion if multiple people use the
> same name in different devices with different semantics.

I thought that's what the "nvidia," vendor prefix was for. Presumably
standardized properties wouldn't have that?

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

* [U-Boot] [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
@ 2012-04-13 21:22           ` Stephen Warren
  0 siblings, 0 replies; 45+ messages in thread
From: Stephen Warren @ 2012-04-13 21:22 UTC (permalink / raw)
  To: u-boot

On 04/13/2012 03:21 PM, Scott Wood wrote:
> On 04/13/2012 03:58 PM, Stephen Warren wrote:
>> On 04/13/2012 12:43 PM, Scott Wood wrote:
>>> On 04/13/2012 01:29 PM, Simon Glass wrote:
>>>> Add a NAND controller along with a bindings file for review.
>>>>
>>>> Signed-off-by: Simon Glass<sjg@chromium.org>
>>
>>>> +++ b/doc/device-tree-bindings/nand/nvidia-nand.txt
>>
>>>> +wp-gpio : GPIO of write-protect line, three cells in the format:
>>>> +        phandle, parameter, flags
>>>
>>> nvidia,nand-wp-gpio
>>
>> I'm not convinced about this. For example many SDHCI bindings use just
>> "wp-gpios" not "shdci-wp-gpios". Is there really a need to keep the
>> property names unique across all bindings, even though a given node only
>> relies on one binding?
>>
> 
> Yeah, there's a lot of bad practice in the existing trees.  But the
> general recommendation for a while now has been to namespace properties
> that aren't defined in standardized, device-indpendent way.  That way we
> don't get conflicts if we want to use that name for a standard property
> in the future, and there's less confusion if multiple people use the
> same name in different devices with different semantics.

I thought that's what the "nvidia," vendor prefix was for. Presumably
standardized properties wouldn't have that?

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

* Re: [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
  2012-04-13 21:22           ` [U-Boot] " Stephen Warren
@ 2012-04-13 21:56             ` Scott Wood
  -1 siblings, 0 replies; 45+ messages in thread
From: Scott Wood @ 2012-04-13 21:56 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Devicetree, U-Boot Mailing List, Jerry Van Baren, Tom Warren, Discuss

On 04/13/2012 04:22 PM, Stephen Warren wrote:
> On 04/13/2012 03:21 PM, Scott Wood wrote:
>> On 04/13/2012 03:58 PM, Stephen Warren wrote:
>>> On 04/13/2012 12:43 PM, Scott Wood wrote:
>>>> On 04/13/2012 01:29 PM, Simon Glass wrote:
>>>>> Add a NAND controller along with a bindings file for review.
>>>>>
>>>>> Signed-off-by: Simon Glass<sjg@chromium.org>
>>>
>>>>> +++ b/doc/device-tree-bindings/nand/nvidia-nand.txt
>>>
>>>>> +wp-gpio : GPIO of write-protect line, three cells in the format:
>>>>> +        phandle, parameter, flags
>>>>
>>>> nvidia,nand-wp-gpio
>>>
>>> I'm not convinced about this. For example many SDHCI bindings use just
>>> "wp-gpios" not "shdci-wp-gpios". Is there really a need to keep the
>>> property names unique across all bindings, even though a given node only
>>> relies on one binding?
>>>
>>
>> Yeah, there's a lot of bad practice in the existing trees.  But the
>> general recommendation for a while now has been to namespace properties
>> that aren't defined in standardized, device-indpendent way.  That way we
>> don't get conflicts if we want to use that name for a standard property
>> in the future, and there's less confusion if multiple people use the
>> same name in different devices with different semantics.
>
> I thought that's what the "nvidia," vendor prefix was for.

Yes, and it applies to non-standard properties too.

> Presumably standardized properties wouldn't have that?

Right.

-Scott

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

* [U-Boot] [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
@ 2012-04-13 21:56             ` Scott Wood
  0 siblings, 0 replies; 45+ messages in thread
From: Scott Wood @ 2012-04-13 21:56 UTC (permalink / raw)
  To: u-boot

On 04/13/2012 04:22 PM, Stephen Warren wrote:
> On 04/13/2012 03:21 PM, Scott Wood wrote:
>> On 04/13/2012 03:58 PM, Stephen Warren wrote:
>>> On 04/13/2012 12:43 PM, Scott Wood wrote:
>>>> On 04/13/2012 01:29 PM, Simon Glass wrote:
>>>>> Add a NAND controller along with a bindings file for review.
>>>>>
>>>>> Signed-off-by: Simon Glass<sjg@chromium.org>
>>>
>>>>> +++ b/doc/device-tree-bindings/nand/nvidia-nand.txt
>>>
>>>>> +wp-gpio : GPIO of write-protect line, three cells in the format:
>>>>> +        phandle, parameter, flags
>>>>
>>>> nvidia,nand-wp-gpio
>>>
>>> I'm not convinced about this. For example many SDHCI bindings use just
>>> "wp-gpios" not "shdci-wp-gpios". Is there really a need to keep the
>>> property names unique across all bindings, even though a given node only
>>> relies on one binding?
>>>
>>
>> Yeah, there's a lot of bad practice in the existing trees.  But the
>> general recommendation for a while now has been to namespace properties
>> that aren't defined in standardized, device-indpendent way.  That way we
>> don't get conflicts if we want to use that name for a standard property
>> in the future, and there's less confusion if multiple people use the
>> same name in different devices with different semantics.
>
> I thought that's what the "nvidia," vendor prefix was for.

Yes, and it applies to non-standard properties too.

> Presumably standardized properties wouldn't have that?

Right.

-Scott

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

* Re: [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
  2012-04-13 21:05     ` [U-Boot] " Stephen Warren
@ 2012-04-17 18:33       ` Simon Glass
  -1 siblings, 0 replies; 45+ messages in thread
From: Simon Glass @ 2012-04-17 18:33 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Devicetree Discuss, U-Boot Mailing List, Jerry Van Baren,
	Tom Warren, Scott Wood

Hi Stephen,

On Fri, Apr 13, 2012 at 2:05 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 04/13/2012 12:29 PM, Simon Glass wrote:
>> Add a NAND controller along with a bindings file for review.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>
>> +++ b/doc/device-tree-bindings/nand/nvidia-nand.txt
>
> I'd prefer this be called nvidia,tegra20-nand.txt so filenames are named
> according to compatible value. This makes it easier to look things up.

OK, changed

>
>> +The device node for a NAND flash device is as described in the document
>> +"Open Firmware Recommended Practice : Universal Serial Bus" with the
>
> This is really based on USB?

Well just in that it follows the conventions. I will remove this since
I'm not sure we use anything that isn't in the ePAPR.

>
>> +Required properties :
>> + - compatible : Should be "manufacture,device", "nand-flash"
>> + - nvidia,page-data-bytes : Number of bytes in the data area
>> + - nvidia,page-spare-bytes : * Number of bytes in spare area
>
> Not sure what that "*" is?

Removed

>
>> +Nvidia NAND Controller
>> +----------------------
>> +
>> +The device node for a NAND flash controller is as described in the document
>> +"Open Firmware Recommended Practice : Universal Serial Bus" with the
>
> USB again?

Removed

>
>> +nand-controller@0x70008000 {
>> +     compatible = "nvidia,tegra20-nand";
>> +     wp-gpios = <&gpio 59 0>;                /* PH3 */
>> +     nvidia,width = <8>;
>> +     nvidia,timing = <26 100 20 80 20 10 12 10 70>;
>> +     nand@0 {
>> +             compatible = "hynix,hy27uf4g2b", "nand-flash";
>
> The TRM says there can be up to 8 chip selects. Don't the NAND device
> sub-nodes need a reg property to indicate which chip-select they're on?

We don't have driver support for this at present.

>
> Also, the TRM mentions async vs. ONFI devices. Don't we need properties
> somewhere to configure that kind of thing?

We don't have driver support for this at present, either :-(

>
>> +             nvidia,page-data-bytes = <2048>;
>> +             nvidia,tag-ecc-bytes = <4>;
>> +             nvidia,tag-bytes = <20>;
>> +             nvidia,data-ecc-bytes = <36>;
>> +             nvidia,skipped-spare-bytes = <4>;
>> +             nvidia,page-spare-bytes = <64>;
>> +     };
>> +};

Regards,
Simon

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

* [U-Boot] [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
@ 2012-04-17 18:33       ` Simon Glass
  0 siblings, 0 replies; 45+ messages in thread
From: Simon Glass @ 2012-04-17 18:33 UTC (permalink / raw)
  To: u-boot

Hi Stephen,

On Fri, Apr 13, 2012 at 2:05 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 04/13/2012 12:29 PM, Simon Glass wrote:
>> Add a NAND controller along with a bindings file for review.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>
>> +++ b/doc/device-tree-bindings/nand/nvidia-nand.txt
>
> I'd prefer this be called nvidia,tegra20-nand.txt so filenames are named
> according to compatible value. This makes it easier to look things up.

OK, changed

>
>> +The device node for a NAND flash device is as described in the document
>> +"Open Firmware Recommended Practice : Universal Serial Bus" with the
>
> This is really based on USB?

Well just in that it follows the conventions. I will remove this since
I'm not sure we use anything that isn't in the ePAPR.

>
>> +Required properties :
>> + - compatible : Should be "manufacture,device", "nand-flash"
>> + - nvidia,page-data-bytes : Number of bytes in the data area
>> + - nvidia,page-spare-bytes : * Number of bytes in spare area
>
> Not sure what that "*" is?

Removed

>
>> +Nvidia NAND Controller
>> +----------------------
>> +
>> +The device node for a NAND flash controller is as described in the document
>> +"Open Firmware Recommended Practice : Universal Serial Bus" with the
>
> USB again?

Removed

>
>> +nand-controller at 0x70008000 {
>> + ? ? compatible = "nvidia,tegra20-nand";
>> + ? ? wp-gpios = <&gpio 59 0>; ? ? ? ? ? ? ? ?/* PH3 */
>> + ? ? nvidia,width = <8>;
>> + ? ? nvidia,timing = <26 100 20 80 20 10 12 10 70>;
>> + ? ? nand at 0 {
>> + ? ? ? ? ? ? compatible = "hynix,hy27uf4g2b", "nand-flash";
>
> The TRM says there can be up to 8 chip selects. Don't the NAND device
> sub-nodes need a reg property to indicate which chip-select they're on?

We don't have driver support for this at present.

>
> Also, the TRM mentions async vs. ONFI devices. Don't we need properties
> somewhere to configure that kind of thing?

We don't have driver support for this at present, either :-(

>
>> + ? ? ? ? ? ? nvidia,page-data-bytes = <2048>;
>> + ? ? ? ? ? ? nvidia,tag-ecc-bytes = <4>;
>> + ? ? ? ? ? ? nvidia,tag-bytes = <20>;
>> + ? ? ? ? ? ? nvidia,data-ecc-bytes = <36>;
>> + ? ? ? ? ? ? nvidia,skipped-spare-bytes = <4>;
>> + ? ? ? ? ? ? nvidia,page-spare-bytes = <64>;
>> + ? ? };
>> +};

Regards,
Simon

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

* Re: [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
  2012-04-17 18:33       ` [U-Boot] " Simon Glass
@ 2012-04-17 18:38         ` Scott Wood
  -1 siblings, 0 replies; 45+ messages in thread
From: Scott Wood @ 2012-04-17 18:38 UTC (permalink / raw)
  To: Simon Glass
  Cc: Devicetree, Discuss, U-Boot Mailing List, Jerry Van Baren, Tom Warren

On 04/17/2012 01:33 PM, Simon Glass wrote:
> Hi Stephen,
> 
> On Fri, Apr 13, 2012 at 2:05 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 04/13/2012 12:29 PM, Simon Glass wrote:
>>> +nand-controller@0x70008000 {
>>> +     compatible = "nvidia,tegra20-nand";
>>> +     wp-gpios = <&gpio 59 0>;                /* PH3 */
>>> +     nvidia,width = <8>;
>>> +     nvidia,timing = <26 100 20 80 20 10 12 10 70>;
>>> +     nand@0 {
>>> +             compatible = "hynix,hy27uf4g2b", "nand-flash";
>>
>> The TRM says there can be up to 8 chip selects. Don't the NAND device
>> sub-nodes need a reg property to indicate which chip-select they're on?
> 
> We don't have driver support for this at present.

That shouldn't matter.  The device tree is about describing the
hardware.  Ideally the device tree shouldn't have to change if in the
future you do get driver support for it.

Also, unit addresses should only be present if reg is present, and they
should match.

-Scott

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

* [U-Boot] [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
@ 2012-04-17 18:38         ` Scott Wood
  0 siblings, 0 replies; 45+ messages in thread
From: Scott Wood @ 2012-04-17 18:38 UTC (permalink / raw)
  To: u-boot

On 04/17/2012 01:33 PM, Simon Glass wrote:
> Hi Stephen,
> 
> On Fri, Apr 13, 2012 at 2:05 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 04/13/2012 12:29 PM, Simon Glass wrote:
>>> +nand-controller at 0x70008000 {
>>> +     compatible = "nvidia,tegra20-nand";
>>> +     wp-gpios = <&gpio 59 0>;                /* PH3 */
>>> +     nvidia,width = <8>;
>>> +     nvidia,timing = <26 100 20 80 20 10 12 10 70>;
>>> +     nand at 0 {
>>> +             compatible = "hynix,hy27uf4g2b", "nand-flash";
>>
>> The TRM says there can be up to 8 chip selects. Don't the NAND device
>> sub-nodes need a reg property to indicate which chip-select they're on?
> 
> We don't have driver support for this at present.

That shouldn't matter.  The device tree is about describing the
hardware.  Ideally the device tree shouldn't have to change if in the
future you do get driver support for it.

Also, unit addresses should only be present if reg is present, and they
should match.

-Scott

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

* Re: [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
  2012-04-17 18:38         ` [U-Boot] " Scott Wood
@ 2012-04-17 18:44           ` Simon Glass
  -1 siblings, 0 replies; 45+ messages in thread
From: Simon Glass @ 2012-04-17 18:44 UTC (permalink / raw)
  To: Scott Wood
  Cc: Devicetree Discuss, U-Boot Mailing List, Jerry Van Baren, Tom Warren

Hi,

On Tue, Apr 17, 2012 at 11:38 AM, Scott Wood <scottwood@freescale.com> wrote:
> On 04/17/2012 01:33 PM, Simon Glass wrote:
>> Hi Stephen,
>>
>> On Fri, Apr 13, 2012 at 2:05 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>> On 04/13/2012 12:29 PM, Simon Glass wrote:
>>>> +nand-controller@0x70008000 {
>>>> +     compatible = "nvidia,tegra20-nand";
>>>> +     wp-gpios = <&gpio 59 0>;                /* PH3 */
>>>> +     nvidia,width = <8>;
>>>> +     nvidia,timing = <26 100 20 80 20 10 12 10 70>;
>>>> +     nand@0 {
>>>> +             compatible = "hynix,hy27uf4g2b", "nand-flash";
>>>
>>> The TRM says there can be up to 8 chip selects. Don't the NAND device
>>> sub-nodes need a reg property to indicate which chip-select they're on?
>>
>> We don't have driver support for this at present.
>
> That shouldn't matter.  The device tree is about describing the
> hardware.  Ideally the device tree shouldn't have to change if in the
> future you do get driver support for it.
>
> Also, unit addresses should only be present if reg is present, and they
> should match.

OK I will leave @0 in there, and add a reg property to the node.

>
> -Scott
>

Regards,
Simon

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

* [U-Boot] [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
@ 2012-04-17 18:44           ` Simon Glass
  0 siblings, 0 replies; 45+ messages in thread
From: Simon Glass @ 2012-04-17 18:44 UTC (permalink / raw)
  To: u-boot

Hi,

On Tue, Apr 17, 2012 at 11:38 AM, Scott Wood <scottwood@freescale.com> wrote:
> On 04/17/2012 01:33 PM, Simon Glass wrote:
>> Hi Stephen,
>>
>> On Fri, Apr 13, 2012 at 2:05 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>> On 04/13/2012 12:29 PM, Simon Glass wrote:
>>>> +nand-controller at 0x70008000 {
>>>> + ? ? compatible = "nvidia,tegra20-nand";
>>>> + ? ? wp-gpios = <&gpio 59 0>; ? ? ? ? ? ? ? ?/* PH3 */
>>>> + ? ? nvidia,width = <8>;
>>>> + ? ? nvidia,timing = <26 100 20 80 20 10 12 10 70>;
>>>> + ? ? nand at 0 {
>>>> + ? ? ? ? ? ? compatible = "hynix,hy27uf4g2b", "nand-flash";
>>>
>>> The TRM says there can be up to 8 chip selects. Don't the NAND device
>>> sub-nodes need a reg property to indicate which chip-select they're on?
>>
>> We don't have driver support for this at present.
>
> That shouldn't matter. ?The device tree is about describing the
> hardware. ?Ideally the device tree shouldn't have to change if in the
> future you do get driver support for it.
>
> Also, unit addresses should only be present if reg is present, and they
> should match.

OK I will leave @0 in there, and add a reg property to the node.

>
> -Scott
>

Regards,
Simon

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

* Re: [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
  2012-04-17 18:44           ` [U-Boot] " Simon Glass
@ 2012-04-17 18:45             ` Scott Wood
  -1 siblings, 0 replies; 45+ messages in thread
From: Scott Wood @ 2012-04-17 18:45 UTC (permalink / raw)
  To: Simon Glass
  Cc: Devicetree, Discuss, U-Boot Mailing List, Jerry Van Baren, Tom Warren

On 04/17/2012 01:44 PM, Simon Glass wrote:
> Hi,
> 
> On Tue, Apr 17, 2012 at 11:38 AM, Scott Wood <scottwood@freescale.com> wrote:
>> On 04/17/2012 01:33 PM, Simon Glass wrote:
>>> Hi Stephen,
>>>
>>> On Fri, Apr 13, 2012 at 2:05 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>> On 04/13/2012 12:29 PM, Simon Glass wrote:
>>>>> +nand-controller@0x70008000 {
>>>>> +     compatible = "nvidia,tegra20-nand";
>>>>> +     wp-gpios = <&gpio 59 0>;                /* PH3 */
>>>>> +     nvidia,width = <8>;
>>>>> +     nvidia,timing = <26 100 20 80 20 10 12 10 70>;
>>>>> +     nand@0 {
>>>>> +             compatible = "hynix,hy27uf4g2b", "nand-flash";
>>>>
>>>> The TRM says there can be up to 8 chip selects. Don't the NAND device
>>>> sub-nodes need a reg property to indicate which chip-select they're on?
>>>
>>> We don't have driver support for this at present.
>>
>> That shouldn't matter.  The device tree is about describing the
>> hardware.  Ideally the device tree shouldn't have to change if in the
>> future you do get driver support for it.
>>
>> Also, unit addresses should only be present if reg is present, and they
>> should match.
> 
> OK I will leave @0 in there, and add a reg property to the node.

Also set #address-cells = <1> and #size-cells = <0> in the controller node.

-Scott

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

* [U-Boot] [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
@ 2012-04-17 18:45             ` Scott Wood
  0 siblings, 0 replies; 45+ messages in thread
From: Scott Wood @ 2012-04-17 18:45 UTC (permalink / raw)
  To: u-boot

On 04/17/2012 01:44 PM, Simon Glass wrote:
> Hi,
> 
> On Tue, Apr 17, 2012 at 11:38 AM, Scott Wood <scottwood@freescale.com> wrote:
>> On 04/17/2012 01:33 PM, Simon Glass wrote:
>>> Hi Stephen,
>>>
>>> On Fri, Apr 13, 2012 at 2:05 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>> On 04/13/2012 12:29 PM, Simon Glass wrote:
>>>>> +nand-controller at 0x70008000 {
>>>>> +     compatible = "nvidia,tegra20-nand";
>>>>> +     wp-gpios = <&gpio 59 0>;                /* PH3 */
>>>>> +     nvidia,width = <8>;
>>>>> +     nvidia,timing = <26 100 20 80 20 10 12 10 70>;
>>>>> +     nand at 0 {
>>>>> +             compatible = "hynix,hy27uf4g2b", "nand-flash";
>>>>
>>>> The TRM says there can be up to 8 chip selects. Don't the NAND device
>>>> sub-nodes need a reg property to indicate which chip-select they're on?
>>>
>>> We don't have driver support for this at present.
>>
>> That shouldn't matter.  The device tree is about describing the
>> hardware.  Ideally the device tree shouldn't have to change if in the
>> future you do get driver support for it.
>>
>> Also, unit addresses should only be present if reg is present, and they
>> should match.
> 
> OK I will leave @0 in there, and add a reg property to the node.

Also set #address-cells = <1> and #size-cells = <0> in the controller node.

-Scott

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

* Re: [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
  2012-04-17 18:45             ` [U-Boot] " Scott Wood
@ 2012-04-17 18:47               ` Simon Glass
  -1 siblings, 0 replies; 45+ messages in thread
From: Simon Glass @ 2012-04-17 18:47 UTC (permalink / raw)
  To: Scott Wood
  Cc: Devicetree Discuss, U-Boot Mailing List, Jerry Van Baren, Tom Warren

Hi Scott,

On Tue, Apr 17, 2012 at 11:45 AM, Scott Wood <scottwood@freescale.com> wrote:
> On 04/17/2012 01:44 PM, Simon Glass wrote:
>> Hi,
>>
>> On Tue, Apr 17, 2012 at 11:38 AM, Scott Wood <scottwood@freescale.com> wrote:
>>> On 04/17/2012 01:33 PM, Simon Glass wrote:
>>>> Hi Stephen,
>>>>
>>>> On Fri, Apr 13, 2012 at 2:05 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>>> On 04/13/2012 12:29 PM, Simon Glass wrote:
>>>>>> +nand-controller@0x70008000 {
>>>>>> +     compatible = "nvidia,tegra20-nand";
>>>>>> +     wp-gpios = <&gpio 59 0>;                /* PH3 */
>>>>>> +     nvidia,width = <8>;
>>>>>> +     nvidia,timing = <26 100 20 80 20 10 12 10 70>;
>>>>>> +     nand@0 {
>>>>>> +             compatible = "hynix,hy27uf4g2b", "nand-flash";
>>>>>
>>>>> The TRM says there can be up to 8 chip selects. Don't the NAND device
>>>>> sub-nodes need a reg property to indicate which chip-select they're on?
>>>>
>>>> We don't have driver support for this at present.
>>>
>>> That shouldn't matter.  The device tree is about describing the
>>> hardware.  Ideally the device tree shouldn't have to change if in the
>>> future you do get driver support for it.
>>>
>>> Also, unit addresses should only be present if reg is present, and they
>>> should match.
>>
>> OK I will leave @0 in there, and add a reg property to the node.
>
> Also set #address-cells = <1> and #size-cells = <0> in the controller node.

Thanks, yes spotted that - dtc crashes if you have address cells as zero.

Will send a new series out in a minute.

>
> -Scott
>

Regards,
Simon

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

* [U-Boot] [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
@ 2012-04-17 18:47               ` Simon Glass
  0 siblings, 0 replies; 45+ messages in thread
From: Simon Glass @ 2012-04-17 18:47 UTC (permalink / raw)
  To: u-boot

Hi Scott,

On Tue, Apr 17, 2012 at 11:45 AM, Scott Wood <scottwood@freescale.com> wrote:
> On 04/17/2012 01:44 PM, Simon Glass wrote:
>> Hi,
>>
>> On Tue, Apr 17, 2012 at 11:38 AM, Scott Wood <scottwood@freescale.com> wrote:
>>> On 04/17/2012 01:33 PM, Simon Glass wrote:
>>>> Hi Stephen,
>>>>
>>>> On Fri, Apr 13, 2012 at 2:05 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>>> On 04/13/2012 12:29 PM, Simon Glass wrote:
>>>>>> +nand-controller at 0x70008000 {
>>>>>> + ? ? compatible = "nvidia,tegra20-nand";
>>>>>> + ? ? wp-gpios = <&gpio 59 0>; ? ? ? ? ? ? ? ?/* PH3 */
>>>>>> + ? ? nvidia,width = <8>;
>>>>>> + ? ? nvidia,timing = <26 100 20 80 20 10 12 10 70>;
>>>>>> + ? ? nand at 0 {
>>>>>> + ? ? ? ? ? ? compatible = "hynix,hy27uf4g2b", "nand-flash";
>>>>>
>>>>> The TRM says there can be up to 8 chip selects. Don't the NAND device
>>>>> sub-nodes need a reg property to indicate which chip-select they're on?
>>>>
>>>> We don't have driver support for this at present.
>>>
>>> That shouldn't matter. ?The device tree is about describing the
>>> hardware. ?Ideally the device tree shouldn't have to change if in the
>>> future you do get driver support for it.
>>>
>>> Also, unit addresses should only be present if reg is present, and they
>>> should match.
>>
>> OK I will leave @0 in there, and add a reg property to the node.
>
> Also set #address-cells = <1> and #size-cells = <0> in the controller node.

Thanks, yes spotted that - dtc crashes if you have address cells as zero.

Will send a new series out in a minute.

>
> -Scott
>

Regards,
Simon

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

end of thread, other threads:[~2012-04-17 18:47 UTC | newest]

Thread overview: 45+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-13 18:29 [U-Boot] [PATCH v2 0/7] tegra: Add NAND flash support Simon Glass
2012-04-13 18:29 ` [U-Boot] [PATCH v2 1/7] nand: Try to align the default buffers Simon Glass
2012-04-13 18:37   ` Scott Wood
2012-04-13 18:52     ` Simon Glass
2012-04-13 19:17       ` Scott Wood
2012-04-13 19:24         ` Simon Glass
2012-04-13 18:29 ` [PATCH v2 2/7] fdt: Add debugging to fdtdec_get_int/addr() Simon Glass
2012-04-13 18:29   ` [U-Boot] " Simon Glass
2012-04-13 18:29 ` [U-Boot] [PATCH v2 3/7] tegra: Add NAND support to funcmux Simon Glass
2012-04-13 18:29 ` [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions Simon Glass
2012-04-13 18:29   ` [U-Boot] " Simon Glass
2012-04-13 18:43   ` Scott Wood
2012-04-13 18:43     ` [U-Boot] " Scott Wood
2012-04-13 19:01     ` Simon Glass
2012-04-13 19:01       ` [U-Boot] " Simon Glass
2012-04-13 19:07       ` Scott Wood
2012-04-13 19:07         ` [U-Boot] " Scott Wood
2012-04-13 19:16         ` Simon Glass
2012-04-13 19:16           ` [U-Boot] " Simon Glass
2012-04-13 20:58     ` Stephen Warren
2012-04-13 20:58       ` [U-Boot] " Stephen Warren
2012-04-13 21:21       ` Scott Wood
2012-04-13 21:21         ` [U-Boot] " Scott Wood
2012-04-13 21:22         ` Stephen Warren
2012-04-13 21:22           ` [U-Boot] " Stephen Warren
2012-04-13 21:56           ` Scott Wood
2012-04-13 21:56             ` [U-Boot] " Scott Wood
2012-04-13 21:05   ` Stephen Warren
2012-04-13 21:05     ` [U-Boot] " Stephen Warren
2012-04-13 21:12     ` Scott Wood
2012-04-13 21:12       ` [U-Boot] " Scott Wood
2012-04-17 18:33     ` Simon Glass
2012-04-17 18:33       ` [U-Boot] " Simon Glass
2012-04-17 18:38       ` Scott Wood
2012-04-17 18:38         ` [U-Boot] " Scott Wood
2012-04-17 18:44         ` Simon Glass
2012-04-17 18:44           ` [U-Boot] " Simon Glass
2012-04-17 18:45           ` Scott Wood
2012-04-17 18:45             ` [U-Boot] " Scott Wood
2012-04-17 18:47             ` Simon Glass
2012-04-17 18:47               ` [U-Boot] " Simon Glass
     [not found] ` <1334341777-2681-1-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2012-04-13 18:29   ` [PATCH v2 5/7] tegra: fdt: Add NAND definitions to fdt Simon Glass
2012-04-13 18:29     ` [U-Boot] " Simon Glass
2012-04-13 18:29 ` [U-Boot] [PATCH v2 6/7] tegra: nand: Add Tegra NAND driver Simon Glass
2012-04-13 18:29 ` [U-Boot] [PATCH v2 7/7] tegra: Enable NAND on Seaboard Simon Glass

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.