[U-Boot] [PATCH v3 0/7] tegra: Add NAND flash support

[U-Boot] [PATCH v3 0/7] tegra: Add NAND flash support

[Simon Glass]

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

Changes in v3:
- Add reg property for unit address (should be used for chip select)
- Change note in fdt binding about the need for a hardware-specific binding
- Fix up typos in fdt binding, and rename the file
- Update fdt binding to make everything Nvidia-specific

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               |   16 +
 .../nand/nvidia,tegra20-nand.txt                   |   68 ++
 drivers/mtd/nand/Makefile                          |    1 +
 drivers/mtd/nand/nand_base.c                       |    3 +-
 drivers/mtd/nand/tegra2_nand.c                     | 1095 ++++++++++++++++++++
 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, 1487 insertions(+), 10 deletions(-)
 create mode 100644 doc/device-tree-bindings/nand/nvidia,tegra20-nand.txt
 create mode 100644 drivers/mtd/nand/tegra2_nand.c
 create mode 100644 drivers/mtd/nand/tegra2_nand.h

-- 
1.7.7.3

[U-Boot] [PATCH v3 1/7] nand: Try to align the default buffers

[Simon Glass]

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

[U-Boot] [PATCH v3 2/7] fdt: Add debugging to fdtdec_get_int/addr()

[Simon Glass]

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

[U-Boot] [PATCH v3 3/7] tegra: Add NAND support to funcmux

[Simon Glass]

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

[U-Boot] [PATCH v3 4/7] tegra: fdt: Add NAND controller binding and definitions

[Simon Glass]

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

Changes in v3:
- Add reg property for unit address (should be used for chip select)
- Change note in fdt binding about the need for a hardware-specific binding
- Fix up typos in fdt binding, and rename the file
- Update fdt binding to make everything Nvidia-specific

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

diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
index bc64f42..018a3c8 100644
--- a/arch/arm/dts/tegra20.dtsi
+++ b/arch/arm/dts/tegra20.dtsi
@@ -200,4 +200,10 @@
 		reg = <0x7000f400 0x200>;
 	};
 
+	nand: nand-controller at 70008000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "nvidia,tegra20-nand";
+		reg = <0x70008000 0x100>;
+	};
 };
diff --git a/doc/device-tree-bindings/nand/nvidia,tegra20-nand.txt b/doc/device-tree-bindings/nand/nvidia,tegra20-nand.txt
new file mode 100644
index 0000000..2484556
--- /dev/null
+++ b/doc/device-tree-bindings/nand/nvidia,tegra20-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. There should not be Linux-specific or U-Boot specific binding, just
+a binding that describes this hardware. But agreeing a binding in Linux in
+the absence of a driver may be beyond my powers.)
+
+The device node for a NAND flash device is as follows:
+
+Required properties :
+ - compatible : Should be "manufacturer,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
+
+This node should sit inside its controller.
+
+
+Nvidia NAND Controller
+----------------------
+
+The device node for a NAND flash controller is as follows:
+
+Optional properties:
+
+nvidia,wp-gpios : GPIO of write-protect line, three cells in the format:
+		phandle, parameter, flags
+nvidia,nand-width : bus width of the NAND device in bits
+
+ - 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
+
+The 'reg' property should provide the chip select used by the flash chip.
+
+
+Example
+-------
+
+nand-controller at 0x70008000 {
+	compatible = "nvidia,tegra20-nand";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	nvidia,wp-gpios = <&gpio 59 0>;		/* PH3 */
+	nvidia,nand-width = <8>;
+	nvidia,timing = <26 100 20 80 20 10 12 10 70>;
+	nand at 0 {
+		reg = <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

[U-Boot] [PATCH v3 4/7] tegra: fdt: Add NAND controller binding and definitions

[Scott Wood]

On 04/17/2012 01:50 PM, Simon Glass wrote:
> diff --git a/doc/device-tree-bindings/nand/nvidia,tegra20-nand.txt b/doc/device-tree-bindings/nand/nvidia,tegra20-nand.txt
> new file mode 100644
> index 0000000..2484556
> --- /dev/null
> +++ b/doc/device-tree-bindings/nand/nvidia,tegra20-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. There should not be Linux-specific or U-Boot specific binding, just
> +a binding that describes this hardware. But agreeing a binding in Linux in
> +the absence of a driver may be beyond my powers.)
> +
> +The device node for a NAND flash device is as follows:
> +
> +Required properties :
> + - compatible : Should be "manufacturer,device", "nand-flash"

Again, "nand-flash" is not an appropriate compatible.  There is no
generic nand-flash binding.

> + - 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

Do you really need this stuff to be in the device tree?  You should be
able to determine this information from the ID table.

> + - nvidia,skipped-spare-bytes : Number of bytes to skip at start of spare area
> +	(these are typically used for bad block maintenance)

So this binding can't deal with the bad block marker being somewhere
other than the beginning of the spare area (e.g. 8-bit small page NAND)?

> + - nvidia,data-ecc-bytes : Number of ECC bytes for data area

Number of ECC bytes per page?  Number of ECC bytes per ECC block?
Number of data bytes per ECC block?

> + - nvidia,tag-bytes :Number of tag bytes in spare area

What are tag bytes?

> +Nvidia NAND Controller
> +----------------------
> +
> +The device node for a NAND flash controller is as follows:
> +
> +Optional properties:
> +
> +nvidia,wp-gpios : GPIO of write-protect line, three cells in the format:
> +		phandle, parameter, flags

Doesn't the number of cells depend on the GPIO controller binding?

> +nvidia,nand-width : bus width of the NAND device in bits
> +
> + - 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

Might want to point out that there's one cell per timing parameter.

-Scott

[U-Boot] [PATCH v3 4/7] tegra: fdt: Add NAND controller binding and definitions

[Simon Glass]

+Jim, who wrote the driver originally

Hi Scott,

On Tue, Apr 17, 2012 at 12:06 PM, Scott Wood <scottwood@freescale.com> wrote:
> On 04/17/2012 01:50 PM, Simon Glass wrote:
>> diff --git a/doc/device-tree-bindings/nand/nvidia,tegra20-nand.txt b/doc/device-tree-bindings/nand/nvidia,tegra20-nand.txt
>> new file mode 100644
>> index 0000000..2484556
>> --- /dev/null
>> +++ b/doc/device-tree-bindings/nand/nvidia,tegra20-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. There should not be Linux-specific or U-Boot specific binding, just
>> +a binding that describes this hardware. But agreeing a binding in Linux in
>> +the absence of a driver may be beyond my powers.)
>> +
>> +The device node for a NAND flash device is as follows:
>> +
>> +Required properties :
>> + - compatible : Should be "manufacturer,device", "nand-flash"
>
> Again, "nand-flash" is not an appropriate compatible. ?There is no
> generic nand-flash binding.

OK, so I should just drop this.

>
>> + - 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
>
> Do you really need this stuff to be in the device tree? ?You should be
> able to determine this information from the ID table.

I suspect so - the driver originally had a lot of CONFIGs for this.
Maybe someone who wants to take it further could do this as part of
supporting ONFI?

I will see if Jim Lin can take another look.

>
>> + - nvidia,skipped-spare-bytes : Number of bytes to skip at start of spare area
>> + ? ? (these are typically used for bad block maintenance)
>
> So this binding can't deal with the bad block marker being somewhere
> other than the beginning of the spare area (e.g. 8-bit small page NAND)?

I suppose it depends on what you want to put afterwards.

>
>> + - nvidia,data-ecc-bytes : Number of ECC bytes for data area
>
> Number of ECC bytes per page? ?Number of ECC bytes per ECC block?
> Number of data bytes per ECC block?
>
>> + - nvidia,tag-bytes :Number of tag bytes in spare area
>
> What are tag bytes?

I know that term from YAFFS, but other than that I am not sure.

>
>> +Nvidia NAND Controller
>> +----------------------
>> +
>> +The device node for a NAND flash controller is as follows:
>> +
>> +Optional properties:
>> +
>> +nvidia,wp-gpios : GPIO of write-protect line, three cells in the format:
>> + ? ? ? ? ? ? phandle, parameter, flags
>
> Doesn't the number of cells depend on the GPIO controller binding?

Yes, but this is the binding Tegra uses.

>
>> +nvidia,nand-width : bus width of the NAND device in bits
>> +
>> + - 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
>
> Might want to point out that there's one cell per timing parameter.

OK.

I'm going to wait and see if Jim can pick up this driver and make the
device-tree change you requested.

>
> -Scott
>

Regards,
Simon

[U-Boot] [PATCH v3 4/7] tegra: fdt: Add NAND controller binding and definitions

[Scott Wood]

On 04/17/2012 03:18 PM, Simon Glass wrote:
> +Jim, who wrote the driver originally
> 
> Hi Scott,
> 
> On Tue, Apr 17, 2012 at 12:06 PM, Scott Wood <scottwood@freescale.com> wrote:
>>> + - 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
>>
>> Do you really need this stuff to be in the device tree?  You should be
>> able to determine this information from the ID table.
> 
> I suspect so - the driver originally had a lot of CONFIGs for this.
> Maybe someone who wants to take it further could do this as part of
> supporting ONFI?
> 
> I will see if Jim Lin can take another look.

You don't need ONFI to get the page/spare size out of the ID table.

The generic NAND code should already be doing this for you (fills in
mtd->writesize and mtd->oobsize).  If you need it during setup, we now
have CONFIG_SYS_NAND_SELF_INIT that allows splitting up
nand_scan_ident() from nand_scan_tail().

>>> +Nvidia NAND Controller
>>> +----------------------
>>> +
>>> +The device node for a NAND flash controller is as follows:
>>> +
>>> +Optional properties:
>>> +
>>> +nvidia,wp-gpios : GPIO of write-protect line, three cells in the format:
>>> +             phandle, parameter, flags
>>
>> Doesn't the number of cells depend on the GPIO controller binding?
> 
> Yes, but this is the binding Tegra uses.

Still, it doesn't belong in the NAND binding.  Maybe a future chip wants
to use this NAND binding but a different GPIO binding.  If nothing else,
people tend to copy-and-paste such descriptions.  We've still got people
adding bindings for Freescale devices saying interrupts are encoded as a
pair of cells, even though the interrupt controller now uses four cells
per interrupt.

-Scott

[U-Boot] [PATCH v3 4/7] tegra: fdt: Add NAND controller binding and definitions

[Simon Glass]

Hi Scott,

On Tue, Apr 17, 2012 at 1:31 PM, Scott Wood <scottwood@freescale.com> wrote:
> On 04/17/2012 03:18 PM, Simon Glass wrote:
>> +Jim, who wrote the driver originally
>>
>> Hi Scott,
>>
>> On Tue, Apr 17, 2012 at 12:06 PM, Scott Wood <scottwood@freescale.com> wrote:
>>>> + - 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
>>>
>>> Do you really need this stuff to be in the device tree? ?You should be
>>> able to determine this information from the ID table.
>>
>> I suspect so - the driver originally had a lot of CONFIGs for this.
>> Maybe someone who wants to take it further could do this as part of
>> supporting ONFI?
>>
>> I will see if Jim Lin can take another look.
>
> You don't need ONFI to get the page/spare size out of the ID table.
>
> The generic NAND code should already be doing this for you (fills in
> mtd->writesize and mtd->oobsize). ?If you need it during setup, we now
> have CONFIG_SYS_NAND_SELF_INIT that allows splitting up
> nand_scan_ident() from nand_scan_tail().

OK, sounds good. It should just need to be changed over to calculate
these values from the nand layer.

>
>>>> +Nvidia NAND Controller
>>>> +----------------------
>>>> +
>>>> +The device node for a NAND flash controller is as follows:
>>>> +
>>>> +Optional properties:
>>>> +
>>>> +nvidia,wp-gpios : GPIO of write-protect line, three cells in the format:
>>>> + ? ? ? ? ? ? phandle, parameter, flags
>>>
>>> Doesn't the number of cells depend on the GPIO controller binding?
>>
>> Yes, but this is the binding Tegra uses.
>
> Still, it doesn't belong in the NAND binding. ?Maybe a future chip wants
> to use this NAND binding but a different GPIO binding. ?If nothing else,
> people tend to copy-and-paste such descriptions. ?We've still got people
> adding bindings for Freescale devices saying interrupts are encoded as a
> pair of cells, even though the interrupt controller now uses four cells
> per interrupt.

OK I see - are you are saying that we should just say something like:

"nvidia,wp-gpios : GPIO of write-protect line, as defined by gpio bindings"

>
> -Scott
>

Regards,
Simon

[U-Boot] [PATCH v3 4/7] tegra: fdt: Add NAND controller binding and definitions

[Scott Wood]

On 04/17/2012 03:36 PM, Simon Glass wrote:
> Hi Scott,
> 
> On Tue, Apr 17, 2012 at 1:31 PM, Scott Wood <scottwood@freescale.com> wrote:
>> On 04/17/2012 03:18 PM, Simon Glass wrote:
>>> On Tue, Apr 17, 2012 at 12:06 PM, Scott Wood <scottwood@freescale.com> wrote:
>>>> Doesn't the number of cells depend on the GPIO controller binding?
>>>
>>> Yes, but this is the binding Tegra uses.
>>
>> Still, it doesn't belong in the NAND binding.  Maybe a future chip wants
>> to use this NAND binding but a different GPIO binding.  If nothing else,
>> people tend to copy-and-paste such descriptions.  We've still got people
>> adding bindings for Freescale devices saying interrupts are encoded as a
>> pair of cells, even though the interrupt controller now uses four cells
>> per interrupt.
> 
> OK I see - are you are saying that we should just say something like:
> 
> "nvidia,wp-gpios : GPIO of write-protect line, as defined by gpio bindings"

Yes.  If there were more than one GPIO line, you'd specify which one is
which, similar to reg and interrupts.

-Scott

[U-Boot] [PATCH v3 5/7] tegra: fdt: Add NAND definitions to fdt

[Simon Glass]

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

Changes in v3:
- Add reg property for unit address (should be used for chip select)
- Update fdt binding to make everything Nvidia-specific

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

diff --git a/board/nvidia/dts/tegra2-seaboard.dts b/board/nvidia/dts/tegra2-seaboard.dts
index 08ada8c..72a5f5d 100644
--- a/board/nvidia/dts/tegra2-seaboard.dts
+++ b/board/nvidia/dts/tegra2-seaboard.dts
@@ -126,4 +126,20 @@
 				0x00000000 0x00000000 0x00000000 0x00000000 >;
 		};
 	};
+
+	nand-controller at 70008000 {
+		nvidia,wp-gpios = <&gpio 59 0>;		/* PH3 */
+		nvidia,width = <8>;
+		nvidia,timing = <26 100 20 80 20 10 12 10 70>;
+		nand at 0 {
+			reg = <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

[U-Boot] [PATCH v3 6/7] tegra: nand: Add Tegra NAND driver

[Simon Glass]

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

Changes in v3:
- Update fdt binding to make everything Nvidia-specific

 arch/arm/include/asm/arch-tegra2/tegra2.h |    1 +
 drivers/mtd/nand/Makefile                 |    1 +
 drivers/mtd/nand/tegra2_nand.c            | 1095 +++++++++++++++++++++++++++++
 drivers/mtd/nand/tegra2_nand.h            |  257 +++++++
 include/fdtdec.h                          |    1 +
 lib/fdtdec.c                              |    1 +
 6 files changed, 1356 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..55dd16f
--- /dev/null
+++ b/drivers/mtd/nand/tegra2_nand.c
@@ -0,0 +1,1095 @@
+/*
+ * 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,nand-width", 8);
+	err = fdtdec_decode_gpio(blob, node, "nvidia,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

[U-Boot] [PATCH v3 6/7] tegra: nand: Add Tegra NAND driver

[Scott Wood]

On 04/17/2012 01:50 PM, Simon Glass wrote:
> +#define DEBUG

Did you mean to leave this in?

> +/**
> + * [DEFAULT] Read one byte from the chip
> + *
> + * @param mtd	MTD device structure
> + * @return	data byte
> + *
> + * Default read function for 8bit bus-width
> + */

This isn't the default read function, it's the tegra read function.

> +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;
> +}

So you only read up to 4 bytes via this method?  If this is really all
that's ever needed, please add a comment to that effect.

> +/**
> + * [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];

If you're assuming the buffer is 32-bit aligned, comment it.  Ideally
these assumptions should be stated in the interface itself...

Should also comment that there's an endian dependency here.

> +		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");

You need to wait for completion every 4 bytes?  Where is the DMA?

> +	case NAND_CMD_RNDOUT:
> +		printf("%s: Unsupported RNDOUT command\n", __func__);
> +		return;
[snip]
> +	default:
> +		printf("%s: Unsupported command %d\n", __func__, command);
> +		return;
> +	}

Doesn't the print in the default case already handle RNDOUT?

> +	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;

Please don't line up the continuation line with the if-body.

What is the difference between an "A" fail and a "B" fail?  Do you
really want to do the blank_check twice?

> +	/* 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);

Should use virt_to_phys(), even if it currently makes no difference on
this platform.

> +int fdt_decode_nand(const void *blob, int node, struct fdt_nand *config)

Make this static.

> +/**
> + * 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)

Please consider using CONFIG_SYS_NAND_SELF_INIT.

> 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

Please use proper namespacing on symbols defined in headers.

-Scott

[U-Boot] [PATCH v3 6/7] tegra: nand: Add Tegra NAND driver

[Scott Wood]

On 05/21/2012 05:07 AM, Jim Lin wrote:
>> 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
> 
> Please use proper namespacing on symbols defined in headers.
> 
> [Jim] Scott, I don't know what you meant. Could you give me an example?
>  Thanks.
>  The names like AFT_DAT_MASK, TRANS_SIZE_SHIFT, and TRANS_SIZE_PAGE come from field names of register in data sheet.

Names in your data sheet are presumably unique within that chip, but not
within U-Boot as a whole.  Names like CMD_CLE, CMD_TX, etc. are
particularly likely to conflict (at least, if at least one other driver
were similarly loose with its naming).

Prefix these with TEGRA_ or TEGRA_NAND_

-Scott

[U-Boot] [PATCH v3 6/7] tegra: nand: Add Tegra NAND driver

[Simon Glass]

Hi Scott,

On Mon, May 21, 2012 at 8:47 AM, Scott Wood <scottwood@freescale.com> wrote:

> On 05/21/2012 05:07 AM, Jim Lin wrote:
> >> 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
> >
> > Please use proper namespacing on symbols defined in headers.
>
>
> > [Jim] Scott, I don't know what you meant. Could you give me an example?
> >  Thanks.
> >  The names like AFT_DAT_MASK, TRANS_SIZE_SHIFT, and TRANS_SIZE_PAGE come
> from field names of register in data sheet.
>
> Names in your data sheet are presumably unique within that chip, but not
> within U-Boot as a whole.  Names like CMD_CLE, CMD_TX, etc. are
> particularly likely to conflict (at least, if at least one other driver
> were similarly loose with its naming).
>
> Prefix these with TEGRA_ or TEGRA_NAND_
>

I'm not thrilled with this idea. It adds a lot of needless boilerplate to
the code IMO.

This header can only be included by the tegra NAND driver after all. Please
can you reconsider this?


>
> -Scott
>
>
Regards,
Simon

[U-Boot] [PATCH v3 6/7] tegra: nand: Add Tegra NAND driver

[Scott Wood]

On 05/22/2012 03:04 PM, Simon Glass wrote:
> Hi Scott,
> 
> On Mon, May 21, 2012 at 8:47 AM, Scott Wood <scottwood@freescale.com
> <mailto:scottwood@freescale.com>> wrote:
> 
>     On 05/21/2012 05:07 AM, Jim Lin wrote:
>     >> 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
>     <http://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
>     >
>     > Please use proper namespacing on symbols defined in headers.
> 
>     >
>     > [Jim] Scott, I don't know what you meant. Could you give me an
>     example?
>     >  Thanks.
>     >  The names like AFT_DAT_MASK, TRANS_SIZE_SHIFT, and
>     TRANS_SIZE_PAGE come from field names of register in data sheet.
> 
>     Names in your data sheet are presumably unique within that chip, but not
>     within U-Boot as a whole.  Names like CMD_CLE, CMD_TX, etc. are
>     particularly likely to conflict (at least, if at least one other driver
>     were similarly loose with its naming).
> 
>     Prefix these with TEGRA_ or TEGRA_NAND_
> 
> 
> I'm not thrilled with this idea. It adds a lot of needless boilerplate
> to the code IMO.
> 
> This header can only be included by the tegra NAND driver after all.
> Please can you reconsider this?

If it can only be included in the NAND driver, why does it need to be in
a header at all?

-Scott

[U-Boot] [PATCH v3 6/7] tegra: nand: Add Tegra NAND driver

[Simon Glass]

Hi Scott,

On Tue, May 22, 2012 at 1:06 PM, Scott Wood <scottwood@freescale.com> wrote:

> On 05/22/2012 03:04 PM, Simon Glass wrote:
> > Hi Scott,
> >
> > On Mon, May 21, 2012 at 8:47 AM, Scott Wood <scottwood@freescale.com
> > <mailto:scottwood@freescale.com>> wrote:
> >
> >     On 05/21/2012 05:07 AM, Jim Lin wrote:
> >     >> 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
> >     <http://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
> >     >
> >     > Please use proper namespacing on symbols defined in headers.
> >
> >     >
> >     > [Jim] Scott, I don't know what you meant. Could you give me an
> >     example?
> >     >  Thanks.
> >     >  The names like AFT_DAT_MASK, TRANS_SIZE_SHIFT, and
> >     TRANS_SIZE_PAGE come from field names of register in data sheet.
> >
> >     Names in your data sheet are presumably unique within that chip, but
> not
> >     within U-Boot as a whole.  Names like CMD_CLE, CMD_TX, etc. are
> >     particularly likely to conflict (at least, if at least one other
> driver
> >     were similarly loose with its naming).
> >
> >     Prefix these with TEGRA_ or TEGRA_NAND_
> >
> >
> > I'm not thrilled with this idea. It adds a lot of needless boilerplate
> > to the code IMO.
> >
> > This header can only be included by the tegra NAND driver after all.
> > Please can you reconsider this?
>
> If it can only be included in the NAND driver, why does it need to be in
> a header at all?
>

It doesn't - it's just a convenient way to separate out the definitions
from the rest of the code. Quite a lot of drivers in U-Boot have private .h
files.


>
> -Scott
>
> Regards,
Simon

[U-Boot] [PATCH v3 6/7] tegra: nand: Add Tegra NAND driver

[Scott Wood]

On 05/22/2012 03:06 PM, Scott Wood wrote:
> On 05/22/2012 03:04 PM, Simon Glass wrote:
>> Hi Scott,
>>
>> On Mon, May 21, 2012 at 8:47 AM, Scott Wood <scottwood@freescale.com
>> <mailto:scottwood@freescale.com>> wrote:
>>
>>     On 05/21/2012 05:07 AM, Jim Lin wrote:
>>     >> 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
>>     <http://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
>>     >
>>     > Please use proper namespacing on symbols defined in headers.
>>
>>     >
>>     > [Jim] Scott, I don't know what you meant. Could you give me an
>>     example?
>>     >  Thanks.
>>     >  The names like AFT_DAT_MASK, TRANS_SIZE_SHIFT, and
>>     TRANS_SIZE_PAGE come from field names of register in data sheet.
>>
>>     Names in your data sheet are presumably unique within that chip, but not
>>     within U-Boot as a whole.  Names like CMD_CLE, CMD_TX, etc. are
>>     particularly likely to conflict (at least, if at least one other driver
>>     were similarly loose with its naming).
>>
>>     Prefix these with TEGRA_ or TEGRA_NAND_
>>
>>
>> I'm not thrilled with this idea. It adds a lot of needless boilerplate
>> to the code IMO.
>>
>> This header can only be included by the tegra NAND driver after all.
>> Please can you reconsider this?
> 
> If it can only be included in the NAND driver, why does it need to be in
> a header at all?

That said, I see it's in drivers/mtd/nand/ and not include/, so if you
really want to structure it that way, it's OK with me.

-Scott

[U-Boot] [PATCH v3 6/7] tegra: nand: Add Tegra NAND driver

[Scott Wood]

On 07/04/2012 02:46 AM, Jim Lin wrote:
>> -----Original Message-----
>> From: Scott Wood [mailto:scottwood at freescale.com] 
>> Sent: Thursday, April 26, 2012 6:17 AM
>> To: Simon Glass
>> Cc: U-Boot Mailing List; Tom Warren; Stephen Warren; Jim Lin; Stephen Warren
>> Subject: Re: [PATCH v3 6/7] tegra: nand: Add Tegra NAND driver
>>
>> On 04/17/2012 01:50 PM, Simon Glass wrote:
>>> +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];
>>
>> If you're assuming the buffer is 32-bit aligned, comment it.  Ideally
>> these assumptions should be stated in the interface itself...
> This doesn't mean that buf needs to be 32-bit aligned.
> It only says each write is 32-bit.

OK, didn't realize modern ARM could deal with unaligned accesses.

>> Should also comment that there's an endian dependency here.
> What do you mean? Could you explain more or have an example?

You load a value using host endianness, and store it using a little
endian accessor.  This would be fine if it represented a real 32-bit
integer, but it's really a sequence of bytes that should not be swapped.

It's not a big deal if you don't see the driver ever being used with a
big endian host, but a comment would be nice.

>>> +/**
>>> + * 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)
>>
>> Please consider using CONFIG_SYS_NAND_SELF_INIT.
> So far I don't see the demand.

I'd like to see the old way go away eventually.

> -----------------------------------------------------------------------------------
> This email message is for the sole use of the intended recipient(s) and may contain
> confidential information.  Any unauthorized review, use, disclosure or distribution
> is prohibited.  If you are not the intended recipient, please contact the sender by
> reply email and destroy all copies of the original message.
> -----------------------------------------------------------------------------------

Please try to get rid of this.  This is a public mailing list.

-Scott

[U-Boot] [PATCH v3 6/7] tegra: nand: Add Tegra NAND driver

[Stephen Warren]

On 07/05/2012 07:28 PM, Scott Wood wrote:
> On 07/04/2012 02:46 AM, Jim Lin wrote:
>>> -----Original Message-----
>>> From: Scott Wood [mailto:scottwood at freescale.com] 
>>> Sent: Thursday, April 26, 2012 6:17 AM
>>> To: Simon Glass
>>> Cc: U-Boot Mailing List; Tom Warren; Stephen Warren; Jim Lin; Stephen Warren
>>> Subject: Re: [PATCH v3 6/7] tegra: nand: Add Tegra NAND driver
>>>
>>> On 04/17/2012 01:50 PM, Simon Glass wrote:
>>>> +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];
>>>
>>> If you're assuming the buffer is 32-bit aligned, comment it.  Ideally
>>> these assumptions should be stated in the interface itself...
>> This doesn't mean that buf needs to be 32-bit aligned.
>> It only says each write is 32-bit.
> 
> OK, didn't realize modern ARM could deal with unaligned accesses.

I'd like to see some more research here; I know that recent printk
changes in the ARM kernel caused unaligned accesses which in turn caused
significant problems (including on Tegra). This might only have been for
64-bit accesses, so perhaps 32-bit accesses don't have an issue, but if
you could confirm this Jim, that'd be great.

[U-Boot] [PATCH v3 7/7] tegra: Enable NAND on Seaboard

[Simon Glass]

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

[U-Boot] [PATCH v3 0/7] tegra: Add NAND flash support

[Thierry Reding]

* Simon Glass wrote:
> 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.

I've been working on getting a similar driver up and running in the Linux
kernel. While I can successfully read data from the flash (my hardware has
the same Hynix chip) I've run into a bit of a problem with the partition
tables.

When I use nvflash with a given configuration file to write the BCT, the PT
and U-Boot to the NAND, I noticed that the partitioning is not quite as it
should be. For example I use this:

	[device]
	type=nand
	instance=0

	[partition]
	name=BCT
	id=2
	type=boot_config_table
	allocation_policy=sequential
	filesystem_type=basic
	size=262144
	file_system_attribute=0
	partition_attribute=0
	allocation_attribute=8
	percent_reserved=0

	[partition]
	name=PT
	id=3
	type=partition_table
	allocation_policy=sequential
	filesystem_type=basic
	size=262144
	file_system_attribute=0
	partition_attribute=0
	allocation_attribute=8
	percent_reserved=0

	[partition]
	name=EBT
	id=4
	type=bootloader
	allocation_policy=sequential
	start_location=2097152
	filesystem_type=basic
	size=524288
	file_system_attribute=0
	partition_attribute=0
	allocation_attribute=8
	percent_reserved=0
	filename=u-boot.bin

That however doesn't produce the expected results. Looking at the UART output
produced by the bootstrap fastboot.bin it looks like it's actually allocating
more blocks than necessary.

On the Hynix chip, 256K is 2 blocks, but fastboot.bin seems to want to
allocate more:

	Erase Partition part-id=3: Start=6,End=11
	Format partition BCT
	Erase Partition part-id=2: Start=0,End=5
	Format partition EBT
	Erase Partition part-id=4: Start=12,End=19

On another board things are even worse. It has a bad block, which causes the
bootstrap to skip that when allocating the partitions:

	Erase Partition part-id=3: Start=6,End=12
	Factory Bad block: Chip0 Block=8
	Runtime Bad block: Chip0 Block=8,RTB=0xed
	Format partition BCT
	Erase Partition part-id=2: Start=0,End=5
	Format partition EBT
	Erase Partition part-id=4: Start=13,End=20

Both of these issues lead to the problem that if I encode the same partition
information in the DT, the kernel's representation will not match what the
nvflash tool did.

There seems to be more output during bootstrap that indicates that nvflash
does some remapping from logical blocks to physical blocks:

	PartId 2: LB[0 2] PB[0 6] IL1  LS[0 128]
	Factory Bad block: Chip0 Block=8
	Runtime Bad block: Chip0 Block=8,RTB=0xed
	Chip0 Block=8 bad
	PartId 3: LB[2 2] PB[6 7] IL1  LS[128 128]
	PartId 4: LB[4 4] PB[13 8] IL1  LS[256 256]

If we can get at this information (maybe it is stored within the partition
table partition?), then it should be possible to use it to automatically
build a correct partition table for both U-Boot and the kernel.

Anyway, I was wondering how you solved this problem. Or maybe you didn't have
the same problem in the first place?

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120426/af5a781d/attachment.pgp>

[U-Boot] [PATCH v3 0/7] tegra: Add NAND flash support

[Stephen Warren]

On 04/26/2012 04:50 AM, Thierry Reding wrote:
> * Simon Glass wrote:
>> 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.
> 
> I've been working on getting a similar driver up and running in the Linux
> kernel. While I can successfully read data from the flash (my hardware has
> the same Hynix chip) I've run into a bit of a problem with the partition
> tables.
...
> That however doesn't produce the expected results. Looking at the UART output
> produced by the bootstrap fastboot.bin it looks like it's actually allocating
> more blocks than necessary.
...
> Both of these issues lead to the problem that if I encode the same partition
> information in the DT, the kernel's representation will not match what the
> nvflash tool did.

Yes, I'd recommend not putting information in DT that can be easily
extracted from the partition table on the device itself.

Out of curiosity though, why do you even need the Tegra PT support; I'd
assume in a situation like this, you'd just replace the whole flash with
BCT, PT, EBT at once rather than attempting to update just parts of it
(since the BCT must be updated when writing a new EBT content anyway),
so the existing partition layout wouldn't matter, and the whole device
could just be treated raw.

[U-Boot] [PATCH v3 0/7] tegra: Add NAND flash support

[Thierry Reding]

* Stephen Warren wrote:
> Yes, I'd recommend not putting information in DT that can be easily
> extracted from the partition table on the device itself.

The problem is that neither the format of the BCT nor that of the PT is
documented anywhere. It seems like the BCT contains a reference to where in
the flash the PT starts but I wasn't able to find out where.

Ideally I think we could probably create a separate partition parser, similar
to ofpart, that takes the information directly from the PT. Or alternatively
it could be extracted manually at probe time to create the partitions in
Linux.

> Out of curiosity though, why do you even need the Tegra PT support; I'd
> assume in a situation like this, you'd just replace the whole flash with
> BCT, PT, EBT at once rather than attempting to update just parts of it
> (since the BCT must be updated when writing a new EBT content anyway),
> so the existing partition layout wouldn't matter, and the whole device
> could just be treated raw.

Actually I'd need more than those three partitions. I need at least five,
optimally six. In addition to BCT, PT and EBT I need one for the uImage and
the root filesystem. Those can probably go onto the same partition with JFFS2
or YAFFS. But a fifth will be required to store some other data. If all else
fails it could probably be done via the fourth partition as well, but that'd
be suboptimal.

As I said before, the biggest problem with updating the whole content is that
there's no documentation about either the BCT or the PT. There's cbootimage
on gitorious that has some information about the BCT, but it's incomplete.

For production use it is also impractical to bootstrap using nvflash, boot
the system to Linux and create all the partitions as well. If that turns out
to be the only option, then better documentation is certainly required. In
that case I think it may be possible to not use the PT because the partition
layout will actually be deterministic and it can be encoded with DT.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120426/0be6ba2a/attachment.pgp>

[U-Boot] [PATCH v3 0/7] tegra: Add NAND flash support

[Stephen Warren]

On 04/26/2012 12:32 PM, Thierry Reding wrote:
> * Stephen Warren wrote:
>> Yes, I'd recommend not putting information in DT that can be easily
>> extracted from the partition table on the device itself.
> 
> The problem is that neither the format of the BCT nor that of the PT is
> documented anywhere. It seems like the BCT contains a reference to where in
> the flash the PT starts but I wasn't able to find out where.

I have filed an internal bug to request this information be added to the
TRM. We will see what happens.

> Actually I'd need more than those three partitions. I need at least five,
> optimally six. In addition to BCT, PT and EBT I need one for the uImage and
> the root filesystem.

Oh I see; you're mingling the boot flash and filesystem into a single
memory device. It simpler (but I imagine more costly) not too:-)

> As I said before, the biggest problem with updating the whole content is that
> there's no documentation about either the BCT or the PT. There's cbootimage
> on gitorious that has some information about the BCT, but it's incomplete.

Out of curiosity, what's missing from cbootimage?

[U-Boot] [PATCH v3 0/7] tegra: Add NAND flash support

[Thierry Reding]

* Stephen Warren wrote:
> On 04/26/2012 12:32 PM, Thierry Reding wrote:
> > The problem is that neither the format of the BCT nor that of the PT is
> > documented anywhere. It seems like the BCT contains a reference to where in
> > the flash the PT starts but I wasn't able to find out where.
> 
> I have filed an internal bug to request this information be added to the
> TRM. We will see what happens.

I have filed a similar bug (#976261 in the NVIDIA bug tracker) and Vincent
has been informed, so that may help.

> > Actually I'd need more than those three partitions. I need at least five,
> > optimally six. In addition to BCT, PT and EBT I need one for the uImage and
> > the root filesystem.
> 
> Oh I see; you're mingling the boot flash and filesystem into a single
> memory device. It simpler (but I imagine more costly) not too:-)

Yes, it's all about the money. =) On a more serious note it makes a lot of
sense. Firstly we want to use the MMC/SD slot for user storage on Plutux and
therefore need to have the OS on internal storage (i.e. NAND). And secondly
we build a very minimalistic distribution in-house it fits without a problem.

> > As I said before, the biggest problem with updating the whole content is that
> > there's no documentation about either the BCT or the PT. There's cbootimage
> > on gitorious that has some information about the BCT, but it's incomplete.
> 
> Out of curiosity, what's missing from cbootimage?

It's missing support for PT. That may not be necessary in a setup where we
initialize the NAND from Linux user space, though, so maybe it would be
enough.

One thing I just remembered which may be a little problem is that currently
our boards need to use the L4T binaries for OpenGL ES support, which means
they need to run a Chromium kernel and that sadly doesn't properly boot from
a mainline U-Boot but requires a setup where we use quickboot as first stage
and U-Boot as a second stage bootloader. I haven't had time to investigate
this further, though. But having this kind of setup complicates the NAND
setup from user space further.

The long term plan was to incrementally move to a mainline system by first
replacing the QuickBoot/U-Boot combo that we use now with a mainline U-Boot.
Most of the pieces for that are now in place. I'm not sure if the SMSC95xx is
already supported by U-Boot (we need it to program the MAC address) during
bootstrap. Subsequently we were planning to move to a mainline kernel, which
is obviously even longer term because it requires the DRM driver to be merged
along with an updated user space to take advantage of it.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120427/aae59824/attachment.pgp>

[U-Boot] [PATCH v3 0/7] tegra: Add NAND flash support

[Stephen Warren]

On 04/26/2012 11:10 PM, Thierry Reding wrote:
> * Stephen Warren wrote:
>> On 04/26/2012 12:32 PM, Thierry Reding wrote:
>>> The problem is that neither the format of the BCT nor that of the PT is
>>> documented anywhere. It seems like the BCT contains a reference to where in
>>> the flash the PT starts but I wasn't able to find out where.
...
>>> As I said before, the biggest problem with updating the whole content is that
>>> there's no documentation about either the BCT or the PT. There's cbootimage
>>> on gitorious that has some information about the BCT, but it's incomplete.
>>
>> Out of curiosity, what's missing from cbootimage?
> 
> It's missing support for PT. That may not be necessary in a setup where we
> initialize the NAND from Linux user space, though, so maybe it would be
> enough.

I don't believe the Tegra boot process actually /requires/ the PT even
exist. IIRC, the boot ROM searches for the BCT, and the BCT contains a
pointer to the bootloader (e.g. U-Boot), so it's only at a later stage
in the game that anything would care about the PT. As such, worrying
about the PT (or even including it) may be pointless.

I believe that TrimSlice's firmware recovery SD card images are created
solely using cbootimage, and hence most likely have no PT, although
obviously no additional partitions/file-systems on the media.

Perhaps you could define some hard-coded "MTD" partitions (e.g. the
first 1MB and the rest), where the first 1MB contains BCT + U-Boot and
the rest contains a regular MBR or GPT partition table. I /think/ there
may even be a kernel command-line option to define the MTD partition layout?

Or, you could probably even get away with using a GPT for the entire
NAND by placing just the secondary GPT at the end of the NAND, putting
the BCT+U-Boot right at the start, and defining a GPT partition to
protect/cover BCT+U-Boot. I vaguely recall trying this on some Tegra
device, but I may be wrong.

Note that while most/all of the suggestions above probably work fine in
practice, you should probably run them by your support contacts at
NVIDIA in order to determine if we'd actually provide support for any of
them and/or whether they negatively interact with the boot ROM in any way.

[U-Boot] [PATCH v3 0/7] tegra: Add NAND flash support

[Thierry Reding]

* Stephen Warren wrote:
> On 04/26/2012 11:10 PM, Thierry Reding wrote:
> > * Stephen Warren wrote:
> >> On 04/26/2012 12:32 PM, Thierry Reding wrote:
> >>> The problem is that neither the format of the BCT nor that of the PT is
> >>> documented anywhere. It seems like the BCT contains a reference to where in
> >>> the flash the PT starts but I wasn't able to find out where.
> ...
> >>> As I said before, the biggest problem with updating the whole content is that
> >>> there's no documentation about either the BCT or the PT. There's cbootimage
> >>> on gitorious that has some information about the BCT, but it's incomplete.
> >>
> >> Out of curiosity, what's missing from cbootimage?
> > 
> > It's missing support for PT. That may not be necessary in a setup where we
> > initialize the NAND from Linux user space, though, so maybe it would be
> > enough.
> 
> I don't believe the Tegra boot process actually /requires/ the PT even
> exist. IIRC, the boot ROM searches for the BCT, and the BCT contains a
> pointer to the bootloader (e.g. U-Boot), so it's only at a later stage
> in the game that anything would care about the PT. As such, worrying
> about the PT (or even including it) may be pointless.

After digging into this some more, I get the same impression. PT seems
entirely optional. Information about the bootloader seems to be stored within
the BCT.

> I believe that TrimSlice's firmware recovery SD card images are created
> solely using cbootimage, and hence most likely have no PT, although
> obviously no additional partitions/file-systems on the media.

It looks like cbootimage does have support for generating the bootloader
bits, so maybe I can get this to work.

> Perhaps you could define some hard-coded "MTD" partitions (e.g. the
> first 1MB and the rest), where the first 1MB contains BCT + U-Boot and
> the rest contains a regular MBR or GPT partition table. I /think/ there
> may even be a kernel command-line option to define the MTD partition layout?
> 
> Or, you could probably even get away with using a GPT for the entire
> NAND by placing just the secondary GPT at the end of the NAND, putting
> the BCT+U-Boot right at the start, and defining a GPT partition to
> protect/cover BCT+U-Boot. I vaguely recall trying this on some Tegra
> device, but I may be wrong.

I didn't even know that you could put an MBR or GPT onto NAND. I was under
the impression that the only way to partition flash was via MTD partitions.
I'll have to see if I can make such a setup work.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120428/861049b9/attachment.pgp>