[PATCH 1/6] fdt: Add debugging to fdtdec_get_int/addr()

[PATCH 1/6] 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-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
 lib/fdtdec.c |   22 ++++++++++++++++------
 1 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index f08bfca..5d97e2a 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -77,11 +77,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;
 }
 
@@ -91,10 +96,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

[PATCH 3/6] 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>
---
 arch/arm/dts/tegra20.dtsi                     |    7 ++-
 doc/device-tree-bindings/nand/nvidia-nand.txt |   68 +++++++++++++++++++++++++
 2 files changed, 74 insertions(+), 1 deletions(-)
 create mode 100644 doc/device-tree-bindings/nand/nvidia-nand.txt

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

[PATCH 4/6] 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-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
 board/nvidia/dts/tegra2-seaboard.dts |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

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

Re: [PATCH 3/6] tegra: fdt: Add NAND controller binding and definitions

[Stephen Warren]

On 01/13/2012 04:10 PM, Simon Glass wrote:
> Add a NAND controller along with a bindings file for review.

A few questions to start with:

> diff --git a/doc/device-tree-bindings/nand/nvidia-nand.txt b/doc/device-tree-bindings/nand/nvidia-nand.txt

> +NAND Flash
> +----------
> +
> +(there isn't yet a generic binding in Linux, so this describes what is in
> +U-Boot)
> +
> +The device node for a NAND flash device is as described in the document
> +"Open Firmware Recommended Practice : Universal Serial Bus" with the
> +following modifications and additions :
> +
> +Required properties :
> + - compatible : Should be "manufacture,device", "nand-flash"
> + - page-data-bytes : Number of bytes in the data area
> + - page-spare-bytes : * Number of bytes in spare area
> +       spare area = skipped-spare-bytes + data-ecc-bytes + tag-bytes
> +			+ tag-ecc-bytes
> + - skipped-spare-bytes : Number of bytes to skip at start of spare area
> +	(these are typically used for bad block maintenance)
> + - data-ecc-bytes : Number of ECC bytes for data area
> + - tag-bytes :Number of tag bytes in spare area
> + - tag-ecc-bytes : Number ECC bytes to be generated for tag bytes

Are any of those values really needed?

I looked through all the NAND references I could find in the Linux
kernel in arch/*/boot/dts/* and none of them seem to have this kind of
information.

Looking at the drivers, they execute some form of identification command
on the NAND device which gives back a device ID, which is then looked up
in a table of known devices to give the information above.

I checked the Tegra NAND driver that's in the kernel chromeos-3.0
branch, and it does the same thing, albeit it open-codes some of the
identification routines rather than just calling into the common code.

Judging by arch/*/boot/dts/*, it is standard practice to have a node for
the NAND device itself though; it's used to house (optional) partition
definitions. In the kernel,
Documentation/devicetree/bindings/mtd/mtd-physmap.txt discusses the
format of these partition nodes, and e.g.
arch/powerpc/boot/dts/canyonlands.dts (amongst many) uses this on NAND.

> +Nvidia NAND Controller
> +----------------------
> +
> +The device node for a NAND flash controller is as described in the document
> +"Open Firmware Recommended Practice : Universal Serial Bus" with the
> +following modifications and additions :
> +
> +Optional properties:
> +
> +wp-gpio : GPIO of write-protect line, three cells in the format:
> +		phandle, parameter, flags
> +width : bus width of the NAND device in bits
> +
> +For now here is something specific to the Nvidia controller, with naming
> +based on Nvidia's original (non-fdt) NAND driver:
> +
> + - nvidia,nand-timing : Timing parameters for the NAND. Each is in ns.
> +	Order is: MAX_TRP_TREA, TWB, Max(tCS, tCH, tALS, tALH),
> +	TWHR, Max(tCS, tCH, tALS, tALH), TWH, TWP, TRH, TADL
> +
> +	MAX_TRP_TREA is:
> +		non-EDO mode: Max(tRP, tREA) + 6ns
> +		EDO mode: tRP timing

At first glance, it seems reasonable to have this in the NAND node; it's
certainly impossible to probe the timing parameters. Since NAND is so
standardized though, I wonder if there is a standard set of timing
parameters so that we could have a standard device tree binding for this?

> +Example:
> +
> +nand-controller@0x70008000 {
> +	compatible = "nvidia,tegra20-nand";
> +	wp-gpios = <&gpio 59 0>;		/* PH3 */
> +	width = <8>;
> +	nvidia,timing = <26 100 20 80 20 10 12 10 70>;
> +	nand@0 {
> +		compatible = "hynix,hy27uf4g2b", "nand-flash";
> +		page-data-bytes = <2048>;
> +		tag-ecc-bytes = <4>;
> +		tag-bytes = <20>;
> +		data-ecc-bytes = <36>;
> +		skipped-spare-bytes = <4>;
> +		page-spare-bytes = <64>;
> +	};
> +};

-- 
nvpublic

Re: [PATCH 3/6] tegra: fdt: Add NAND controller binding and definitions

[Simon Glass]

Hi Stephen,

On Thu, Jan 19, 2012 at 5:03 PM, Stephen Warren <swarren@nvidia.com> wrote:
> On 01/13/2012 04:10 PM, Simon Glass wrote:
>> Add a NAND controller along with a bindings file for review.
>
> A few questions to start with:
>
>> diff --git a/doc/device-tree-bindings/nand/nvidia-nand.txt b/doc/device-tree-bindings/nand/nvidia-nand.txt
>
>> +NAND Flash
>> +----------
>> +
>> +(there isn't yet a generic binding in Linux, so this describes what is in
>> +U-Boot)
>> +
>> +The device node for a NAND flash device is as described in the document
>> +"Open Firmware Recommended Practice : Universal Serial Bus" with the
>> +following modifications and additions :
>> +
>> +Required properties :
>> + - compatible : Should be "manufacture,device", "nand-flash"
>> + - page-data-bytes : Number of bytes in the data area
>> + - page-spare-bytes : * Number of bytes in spare area
>> +       spare area = skipped-spare-bytes + data-ecc-bytes + tag-bytes
>> +                     + tag-ecc-bytes
>> + - skipped-spare-bytes : Number of bytes to skip at start of spare area
>> +     (these are typically used for bad block maintenance)
>> + - data-ecc-bytes : Number of ECC bytes for data area
>> + - tag-bytes :Number of tag bytes in spare area
>> + - tag-ecc-bytes : Number ECC bytes to be generated for tag bytes
>
> Are any of those values really needed?
>
> I looked through all the NAND references I could find in the Linux
> kernel in arch/*/boot/dts/* and none of them seem to have this kind of
> information.
>
> Looking at the drivers, they execute some form of identification command
> on the NAND device which gives back a device ID, which is then looked up
> in a table of known devices to give the information above.
>
> I checked the Tegra NAND driver that's in the kernel chromeos-3.0
> branch, and it does the same thing, albeit it open-codes some of the
> identification routines rather than just calling into the common code.

Well that's pretty grim I think. That code should try to use the ONFi
way if it is available and supported, or provide a mechanism to
configure it (via device tree) if not / preferred.

U-Boot does have the same ID lookup feature for the mtd layer, but it
only has page size, block size and bus width.

I do wonder why this driver cannot take more of the information it
needs from the upper levels. Admittedly not all of the info is there,
but it could perhaps be inferred.

>
> Judging by arch/*/boot/dts/*, it is standard practice to have a node for
> the NAND device itself though; it's used to house (optional) partition
> definitions. In the kernel,
> Documentation/devicetree/bindings/mtd/mtd-physmap.txt discusses the
> format of these partition nodes, and e.g.
> arch/powerpc/boot/dts/canyonlands.dts (amongst many) uses this on NAND.

Yes.

>
>> +Nvidia NAND Controller
>> +----------------------
>> +
>> +The device node for a NAND flash controller is as described in the document
>> +"Open Firmware Recommended Practice : Universal Serial Bus" with the
>> +following modifications and additions :
>> +
>> +Optional properties:
>> +
>> +wp-gpio : GPIO of write-protect line, three cells in the format:
>> +             phandle, parameter, flags
>> +width : bus width of the NAND device in bits
>> +
>> +For now here is something specific to the Nvidia controller, with naming
>> +based on Nvidia's original (non-fdt) NAND driver:
>> +
>> + - nvidia,nand-timing : Timing parameters for the NAND. Each is in ns.
>> +     Order is: MAX_TRP_TREA, TWB, Max(tCS, tCH, tALS, tALH),
>> +     TWHR, Max(tCS, tCH, tALS, tALH), TWH, TWP, TRH, TADL
>> +
>> +     MAX_TRP_TREA is:
>> +             non-EDO mode: Max(tRP, tREA) + 6ns
>> +             EDO mode: tRP timing
>
> At first glance, it seems reasonable to have this in the NAND node; it's
> certainly impossible to probe the timing parameters. Since NAND is so
> standardized though, I wonder if there is a standard set of timing
> parameters so that we could have a standard device tree binding for this?

The closest datasheet I could find was this:

http://www.hynix.com/datasheet/pdf/flash/HY27(U_S)F(08_16)1G2M%20Series(Rev1.1).pdf

It has a table on p21 with 32 timing parameters.

But what we want here is the timings for the Tegra NAND controller.
Every controller will have a different take on these parameters - some
will be ignored, some added together, etc. So I think it is better to
think in terms of what the controller needs rather than getting
sidetracked on some average of all the NAND datasheets.

>
>> +Example:
>> +
>> +nand-controller@0x70008000 {
>> +     compatible = "nvidia,tegra20-nand";
>> +     wp-gpios = <&gpio 59 0>;                /* PH3 */
>> +     width = <8>;
>> +     nvidia,timing = <26 100 20 80 20 10 12 10 70>;
>> +     nand@0 {
>> +             compatible = "hynix,hy27uf4g2b", "nand-flash";
>> +             page-data-bytes = <2048>;
>> +             tag-ecc-bytes = <4>;
>> +             tag-bytes = <20>;
>> +             data-ecc-bytes = <36>;
>> +             skipped-spare-bytes = <4>;
>> +             page-spare-bytes = <64>;
>> +     };
>> +};
>
> --
> nvpublic

Regards,
Simon