[PATCH v2 0/4] Add DT support for NAND to LCDK

[PATCH v2 0/4] Add DT support for NAND to LCDK

[Karl Beldan]

Changes from v1:
 - s/cs2/cs3
 - kept "ti,.." only nand properties (the adjustments made by
   nand_davinci_probe are broken)
 - replaced v1_1/4:
     "memory: ti-aemif: Get a named clock rather than an unnamed one"
   with v2_1/4:
     "davinci: da8xx-dt: Add ti-aemif lookup for clock matching"
 - Make the same improvements for the EVM (and retire nand_cs3)

Karl Beldan (4):
  ARM: davinci: da8xx-dt: Add ti-aemif lookup for clock matching
  ARM: davinci_all_defconfig: Enable AEMIF as a module
  ARM: dts: da850,da850-evm: Add an aemif node and use it for the NAND
  ARM: dts: da850-lcdk: Add NAND to DT

 arch/arm/boot/dts/da850-evm.dts        |  49 +++++++++++++--
 arch/arm/boot/dts/da850-lcdk.dts       | 107 +++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/da850.dtsi           |  19 +++---
 arch/arm/configs/davinci_all_defconfig |   2 +
 arch/arm/mach-davinci/da850.c          |   1 +
 arch/arm/mach-davinci/da8xx-dt.c       |   1 +
 6 files changed, 163 insertions(+), 16 deletions(-)

-- 
2.9.2

[PATCH v2 1/4] ARM: davinci: da8xx-dt: Add ti-aemif lookup for clock matching

[Karl Beldan]

Many davinci boards (da830 and da850 families) don't have their clocks
in DT yet and won't be successful in getting an unnamed aemif clock
without explicitly registering them via clk_lookups, failing the
ti-aemif memory driver probe.

The current aemif lookup entry resolving to the same clock:
    'CLK(NULL,               "aemif",        &aemif_clk)'
remains, as it is currently used (davinci_nand is getting a named clock
"aemif").

This change will allow to switch from the mach-davinci aemif code to
the ti-aemif memory driver.

Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
---
 arch/arm/mach-davinci/da850.c    | 1 +
 arch/arm/mach-davinci/da8xx-dt.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index 2398862..3477d30 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -485,6 +485,7 @@ static struct clk_lookup da850_clks[] = {
 	CLK("da8xx_lcdc.0",	"fck",		&lcdc_clk),
 	CLK("da830-mmc.0",	NULL,		&mmcsd0_clk),
 	CLK("da830-mmc.1",	NULL,		&mmcsd1_clk),
+	CLK("ti-aemif",		NULL,		&aemif_clk),
 	CLK(NULL,		"aemif",	&aemif_clk),
 	CLK(NULL,		"usb11",	&usb11_clk),
 	CLK(NULL,		"usb20",	&usb20_clk),
diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
index ca99711..c9f7e92 100644
--- a/arch/arm/mach-davinci/da8xx-dt.c
+++ b/arch/arm/mach-davinci/da8xx-dt.c
@@ -37,6 +37,7 @@ static struct of_dev_auxdata da850_auxdata_lookup[] __initdata = {
 	OF_DEV_AUXDATA("ti,davinci-dm6467-emac", 0x01e20000, "davinci_emac.1",
 		       NULL),
 	OF_DEV_AUXDATA("ti,da830-mcasp-audio", 0x01d00000, "davinci-mcasp.0", NULL),
+	OF_DEV_AUXDATA("ti,da850-aemif", 0x68000000, "ti-aemif", NULL),
 	{}
 };
 
-- 
2.9.2

[v2, 1/4] ARM: davinci: da8xx-dt: Add ti-aemif lookup for clock matching

[David Lechner]

On 08/10/2016 06:00 AM, Karl Beldan wrote:
> Many davinci boards (da830 and da850 families) don't have their clocks
> in DT yet and won't be successful in getting an unnamed aemif clock
> without explicitly registering them via clk_lookups, failing the
> ti-aemif memory driver probe.
>
> The current aemif lookup entry resolving to the same clock:
>     'CLK(NULL,               "aemif",        &aemif_clk)'
> remains, as it is currently used (davinci_nand is getting a named clock
> "aemif").
>
> This change will allow to switch from the mach-davinci aemif code to
> the ti-aemif memory driver.
>
> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> ---

FYI, I can't boot LEGO MINDSTORMS EV3 (AM1908) with a v4.9 mainline 
kernel. I did a git bisect and traced it down to this patch. I'm 
guessing that simply reverting it will break other things.

The problem is that &aemif_clk is a node in a linked list and points to 
itself, which creates an infinite loop when looking up the usb clocks 
that are later in the list.

I thought there was a patch to fix this properly from one of the Bay 
Libre guys to fix this already, but I can't seem to find it at the 
moment. When it is found, it would be good to have it applied to the 4.9 
stable and 4.10 mainline trees.


>  arch/arm/mach-davinci/da850.c    | 1 +
>  arch/arm/mach-davinci/da8xx-dt.c | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
> index 2398862..3477d30 100644
> --- a/arch/arm/mach-davinci/da850.c
> +++ b/arch/arm/mach-davinci/da850.c
> @@ -485,6 +485,7 @@ static struct clk_lookup da850_clks[] = {
>  	CLK("da8xx_lcdc.0",	"fck",		&lcdc_clk),
>  	CLK("da830-mmc.0",	NULL,		&mmcsd0_clk),
>  	CLK("da830-mmc.1",	NULL,		&mmcsd1_clk),
> +	CLK("ti-aemif",		NULL,		&aemif_clk),
>  	CLK(NULL,		"aemif",	&aemif_clk),
>  	CLK(NULL,		"usb11",	&usb11_clk),
>  	CLK(NULL,		"usb20",	&usb20_clk),
> diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
> index ca99711..c9f7e92 100644
> --- a/arch/arm/mach-davinci/da8xx-dt.c
> +++ b/arch/arm/mach-davinci/da8xx-dt.c
> @@ -37,6 +37,7 @@ static struct of_dev_auxdata da850_auxdata_lookup[] __initdata = {
>  	OF_DEV_AUXDATA("ti,davinci-dm6467-emac", 0x01e20000, "davinci_emac.1",
>  		       NULL),
>  	OF_DEV_AUXDATA("ti,da830-mcasp-audio", 0x01d00000, "davinci-mcasp.0", NULL),
> +	OF_DEV_AUXDATA("ti,da850-aemif", 0x68000000, "ti-aemif", NULL),
>  	{}
>  };
>
>

[v2, 1/4] ARM: davinci: da8xx-dt: Add ti-aemif lookup for clock matching

[Sekhar Nori]

On Saturday 31 December 2016 06:22 AM, David Lechner wrote:
> On 08/10/2016 06:00 AM, Karl Beldan wrote:
>> Many davinci boards (da830 and da850 families) don't have their clocks
>> in DT yet and won't be successful in getting an unnamed aemif clock
>> without explicitly registering them via clk_lookups, failing the
>> ti-aemif memory driver probe.
>>
>> The current aemif lookup entry resolving to the same clock:
>>     'CLK(NULL,               "aemif",        &aemif_clk)'
>> remains, as it is currently used (davinci_nand is getting a named clock
>> "aemif").
>>
>> This change will allow to switch from the mach-davinci aemif code to
>> the ti-aemif memory driver.
>>
>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
>> ---
> 
> FYI, I can't boot LEGO MINDSTORMS EV3 (AM1908) with a v4.9 mainline
> kernel. I did a git bisect and traced it down to this patch. I'm
> guessing that simply reverting it will break other things.
> 
> The problem is that &aemif_clk is a node in a linked list and points to
> itself, which creates an infinite loop when looking up the usb clocks
> that are later in the list.
> 
> I thought there was a patch to fix this properly from one of the Bay
> Libre guys to fix this already, but I can't seem to find it at the
> moment. When it is found, it would be good to have it applied to the 4.9
> stable and 4.10 mainline trees.

Yes, a patch was submitted. It is pending in my queue since I need to do
a few experiments myself to determine if its the best solution possible.
Thanks for the heads-up on need to mark it for v4.9 stable. Will do.

Thanks,
Sekhar

[v2, 1/4] ARM: davinci: da8xx-dt: Add ti-aemif lookup for clock matching

[Sekhar Nori]

On Monday 02 January 2017 01:34 PM, Sekhar Nori wrote:
> On Saturday 31 December 2016 06:22 AM, David Lechner wrote:
>> On 08/10/2016 06:00 AM, Karl Beldan wrote:
>>> Many davinci boards (da830 and da850 families) don't have their clocks
>>> in DT yet and won't be successful in getting an unnamed aemif clock
>>> without explicitly registering them via clk_lookups, failing the
>>> ti-aemif memory driver probe.
>>>
>>> The current aemif lookup entry resolving to the same clock:
>>>     'CLK(NULL,               "aemif",        &aemif_clk)'
>>> remains, as it is currently used (davinci_nand is getting a named clock
>>> "aemif").
>>>
>>> This change will allow to switch from the mach-davinci aemif code to
>>> the ti-aemif memory driver.
>>>
>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
>>> ---
>>
>> FYI, I can't boot LEGO MINDSTORMS EV3 (AM1908) with a v4.9 mainline
>> kernel. I did a git bisect and traced it down to this patch. I'm
>> guessing that simply reverting it will break other things.
>>
>> The problem is that &aemif_clk is a node in a linked list and points to
>> itself, which creates an infinite loop when looking up the usb clocks
>> that are later in the list.
>>
>> I thought there was a patch to fix this properly from one of the Bay
>> Libre guys to fix this already, but I can't seem to find it at the
>> moment. When it is found, it would be good to have it applied to the 4.9
>> stable and 4.10 mainline trees.
> 
> Yes, a patch was submitted. It is pending in my queue since I need to do
> a few experiments myself to determine if its the best solution possible.
> Thanks for the heads-up on need to mark it for v4.9 stable. Will do.

The master branch of my tree should have the patch included. If you can
test and confirm it works fine on the lego platform, it will be great.

Thanks,
Sekhar

[v2, 1/4] ARM: davinci: da8xx-dt: Add ti-aemif lookup for clock matching

[David Lechner]

On 01/03/2017 02:58 AM, Sekhar Nori wrote:
> On Monday 02 January 2017 01:34 PM, Sekhar Nori wrote:
>> On Saturday 31 December 2016 06:22 AM, David Lechner wrote:
>>> On 08/10/2016 06:00 AM, Karl Beldan wrote:
>>>> Many davinci boards (da830 and da850 families) don't have their clocks
>>>> in DT yet and won't be successful in getting an unnamed aemif clock
>>>> without explicitly registering them via clk_lookups, failing the
>>>> ti-aemif memory driver probe.
>>>>
>>>> The current aemif lookup entry resolving to the same clock:
>>>>     'CLK(NULL,               "aemif",        &aemif_clk)'
>>>> remains, as it is currently used (davinci_nand is getting a named clock
>>>> "aemif").
>>>>
>>>> This change will allow to switch from the mach-davinci aemif code to
>>>> the ti-aemif memory driver.
>>>>
>>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
>>>> ---
>>>
>>> FYI, I can't boot LEGO MINDSTORMS EV3 (AM1908) with a v4.9 mainline
>>> kernel. I did a git bisect and traced it down to this patch. I'm
>>> guessing that simply reverting it will break other things.
>>>
>>> The problem is that &aemif_clk is a node in a linked list and points to
>>> itself, which creates an infinite loop when looking up the usb clocks
>>> that are later in the list.
>>>
>>> I thought there was a patch to fix this properly from one of the Bay
>>> Libre guys to fix this already, but I can't seem to find it at the
>>> moment. When it is found, it would be good to have it applied to the 4.9
>>> stable and 4.10 mainline trees.
>>
>> Yes, a patch was submitted. It is pending in my queue since I need to do
>> a few experiments myself to determine if its the best solution possible.
>> Thanks for the heads-up on need to mark it for v4.9 stable. Will do.
>
> The master branch of my tree should have the patch included. If you can
> test and confirm it works fine on the lego platform, it will be great.
>

Tested "ARM: davinci: da850: fix infinite loop in clk_set_rate()" 
working on LEGO Mindstorms with a 4.9.0 kernel.

[PATCH v2 2/4] ARM: davinci_all_defconfig: Enable AEMIF as a module

[Karl Beldan]

This enables the use of the memory/ti-aemif.c driver.
ATM most davinci boards use the mach-davinci aemif code which gets in
the way of genericity and proper DT boot.

Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
---
 arch/arm/configs/davinci_all_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/configs/davinci_all_defconfig b/arch/arm/configs/davinci_all_defconfig
index d037d3d..2802897 100644
--- a/arch/arm/configs/davinci_all_defconfig
+++ b/arch/arm/configs/davinci_all_defconfig
@@ -192,6 +192,8 @@ CONFIG_RTC_CLASS=y
 CONFIG_RTC_DRV_OMAP=m
 CONFIG_DMADEVICES=y
 CONFIG_TI_EDMA=y
+CONFIG_MEMORY=y
+CONFIG_TI_AEMIF=m
 CONFIG_EXT2_FS=y
 CONFIG_EXT3_FS=y
 CONFIG_XFS_FS=m
-- 
2.9.2

[PATCH v2 3/4] ARM: dts: da850, da850-evm: Add an aemif node and use it for the NAND

[Karl Beldan]

Currently the davinci da8xx boards use the mach-davinci aemif code.
Instantiating an aemif node into the DT allows to use the ti-aemif
memory driver and is another step to better DT support.
This change adds an aemif node in the dtsi while retiring the nand_cs3
node. The NAND is now instantiated in the dts as a subnode of the aemif
one along with its pins.

Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
---
 arch/arm/boot/dts/da850-evm.dts | 49 ++++++++++++++++++++++++++++++++++++-----
 arch/arm/boot/dts/da850.dtsi    | 19 +++++++---------
 2 files changed, 52 insertions(+), 16 deletions(-)

diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
index 1a15db8..eedcc59 100644
--- a/arch/arm/boot/dts/da850-evm.dts
+++ b/arch/arm/boot/dts/da850-evm.dts
@@ -29,6 +29,20 @@
 					0x04 0x00011000 0x000ff000
 				>;
 			};
+			nand_pins: nand_pins {
+				pinctrl-single,bits = <
+					/* EMA_WAIT[0], EMA_OE, EMA_WE, EMA_CS[4], EMA_CS[3] */
+					0x1c 0x10110110  0xf0ff0ff0
+					/*
+					 * EMA_D[0], EMA_D[1], EMA_D[2],
+					 * EMA_D[3], EMA_D[4], EMA_D[5],
+					 * EMA_D[6], EMA_D[7]
+					 */
+					0x24 0x11111111  0xffffffff
+					/* EMA_A[1], EMA_A[2] */
+					0x30 0x01100000  0x0ff00000
+				>;
+			};
 		};
 		serial0: serial at 42000 {
 			status = "okay";
@@ -131,11 +145,6 @@
 			status = "okay";
 		};
 	};
-	nand_cs3 at 62000000 {
-		status = "okay";
-		pinctrl-names = "default";
-		pinctrl-0 = <&nand_cs3_pins>;
-	};
 	vbat: fixedregulator at 0 {
 		compatible = "regulator-fixed";
 		regulator-name = "vbat";
@@ -250,3 +259,33 @@
 &edma1 {
 	ti,edma-reserved-slot-ranges = <32 90>;
 };
+
+&aemif {
+	pinctrl-names = "default";
+	pinctrl-0 = <&nand_pins>;
+	status = "ok";
+	cs3 {
+		#address-cells = <2>;
+		#size-cells = <1>;
+		clock-ranges;
+		ranges;
+
+		ti,cs-chipselect = <3>;
+
+		nand at 2000000,0 {
+			compatible = "ti,davinci-nand";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			reg = <0 0x02000000 0x02000000
+			       1 0x00000000 0x00008000>;
+
+			ti,davinci-chipselect = <1>;
+			ti,davinci-mask-ale = <0>;
+			ti,davinci-mask-cle = <0>;
+			ti,davinci-mask-chipsel = <0>;
+			ti,davinci-ecc-mode = "hw";
+			ti,davinci-ecc-bits = <4>;
+			ti,davinci-nand-use-bbt;
+		};
+	};
+};
diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index bc10e7e..e73fd64 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -411,17 +411,14 @@
 			dma-names = "tx", "rx";
 		};
 	};
-	nand_cs3 at 62000000 {
-		compatible = "ti,davinci-nand";
-		reg = <0x62000000 0x807ff
-		       0x68000000 0x8000>;
-		ti,davinci-chipselect = <1>;
-		ti,davinci-mask-ale = <0>;
-		ti,davinci-mask-cle = <0>;
-		ti,davinci-mask-chipsel = <0>;
-		ti,davinci-ecc-mode = "hw";
-		ti,davinci-ecc-bits = <4>;
-		ti,davinci-nand-use-bbt;
+	aemif: aemif at 68000000 {
+		compatible = "ti,da850-aemif";
+		#address-cells = <2>;
+		#size-cells = <1>;
+
+		reg = <0x68000000 0x00008000>;
+		ranges = <0 0 0x60000000 0x08000000
+			  1 0 0x68000000 0x00008000>;
 		status = "disabled";
 	};
 };
-- 
2.9.2

[PATCH v2 3/4] ARM: dts: da850,da850-evm: Add an aemif node and use it for the NAND

[Karl Beldan]

On Wed, Aug 10, 2016 at 11:00:31AM +0000, Karl Beldan wrote:
> Currently the davinci da8xx boards use the mach-davinci aemif code.
> Instantiating an aemif node into the DT allows to use the ti-aemif
> memory driver and is another step to better DT support.
> This change adds an aemif node in the dtsi while retiring the nand_cs3
> node. The NAND is now instantiated in the dts as a subnode of the aemif
> one along with its pins.
> 
> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> ---
>  arch/arm/boot/dts/da850-evm.dts | 49 ++++++++++++++++++++++++++++++++++++-----
>  arch/arm/boot/dts/da850.dtsi    | 19 +++++++---------
>  2 files changed, 52 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
> index 1a15db8..eedcc59 100644
> --- a/arch/arm/boot/dts/da850-evm.dts
> +++ b/arch/arm/boot/dts/da850-evm.dts
> @@ -29,6 +29,20 @@
>  					0x04 0x00011000 0x000ff000
>  				>;
>  			};
> +			nand_pins: nand_pins {
> +				pinctrl-single,bits = <
> +					/* EMA_WAIT[0], EMA_OE, EMA_WE, EMA_CS[4], EMA_CS[3] */
> +					0x1c 0x10110110  0xf0ff0ff0
> +					/*
> +					 * EMA_D[0], EMA_D[1], EMA_D[2],
> +					 * EMA_D[3], EMA_D[4], EMA_D[5],
> +					 * EMA_D[6], EMA_D[7]
> +					 */
> +					0x24 0x11111111  0xffffffff
> +					/* EMA_A[1], EMA_A[2] */
> +					0x30 0x01100000  0x0ff00000
> +				>;
> +			};
>  		};
>  		serial0: serial at 42000 {
>  			status = "okay";
> @@ -131,11 +145,6 @@
>  			status = "okay";
>  		};
>  	};
> -	nand_cs3 at 62000000 {
> -		status = "okay";
> -		pinctrl-names = "default";
> -		pinctrl-0 = <&nand_cs3_pins>;
> -	};
>  	vbat: fixedregulator at 0 {
>  		compatible = "regulator-fixed";
>  		regulator-name = "vbat";

It has no functionnal impact but with this change nand_cs3_pins becomes
useless and I forgot to get rid of it, so it'll make a v3.
 
Karl

[PATCH v2 4/4] ARM: dts: da850-lcdk: Add NAND to DT

[Karl Beldan]

This adds DT support for the NAND connected to the SoC AEMIF.
The parameters (timings, ecc) are the same as what the board ships with
(default AEMIF timings, 1bit ECC) and improvements will be handled in
due course.
This passed elementary tests hashing a 20MB file on top of ubifs on my
LCDK.

Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
---
 arch/arm/boot/dts/da850-lcdk.dts | 107 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 107 insertions(+)

diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
index dbcca0b..a3f9845 100644
--- a/arch/arm/boot/dts/da850-lcdk.dts
+++ b/arch/arm/boot/dts/da850-lcdk.dts
@@ -27,6 +27,27 @@
 
 &pmx_core {
 	status = "okay";
+
+	nand_pins: nand_pins {
+		pinctrl-single,bits = <
+			/* EMA_WAIT[0], EMA_OE, EMA_WE, EMA_CS[3] */
+			0x1c 0x10110010  0xf0ff00f0
+			/*
+			 * EMA_D[0], EMA_D[1], EMA_D[2],
+			 * EMA_D[3], EMA_D[4], EMA_D[5],
+			 * EMA_D[6], EMA_D[7]
+			 */
+			0x24 0x11111111  0xffffffff
+			/*
+			 * EMA_D[8],  EMA_D[9],  EMA_D[10],
+			 * EMA_D[11], EMA_D[12], EMA_D[13],
+			 * EMA_D[14], EMA_D[15]
+			 */
+			0x20 0x11111111  0xffffffff
+			/* EMA_A[1], EMA_A[2] */
+			0x30 0x01100000  0x0ff00000
+		>;
+	};
 };
 
 &serial2 {
@@ -68,3 +89,89 @@
 	cd-gpios = <&gpio 64 GPIO_ACTIVE_HIGH>;
 	status = "okay";
 };
+
+&aemif {
+	pinctrl-names = "default";
+	pinctrl-0 = <&nand_pins>;
+	status = "ok";
+	cs3 {
+		#address-cells = <2>;
+		#size-cells = <1>;
+		clock-ranges;
+		ranges;
+
+		ti,cs-chipselect = <3>;
+
+		nand at 2000000,0 {
+			compatible = "ti,davinci-nand";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			reg = <0 0x02000000 0x02000000
+			       1 0x00000000 0x00008000>;
+
+			ti,davinci-chipselect = <1>;
+			ti,davinci-mask-ale = <0>;
+			ti,davinci-mask-cle = <0>;
+			ti,davinci-mask-chipsel = <0>;
+
+			/*
+			 * nand_ecc_strength_good will emit a warning
+			 * but the LCDK ships with these settings [1].
+			 * Also HW 4bits ECC with 16bits NAND seems to
+			 * require some attention.
+			 *
+			 * ATM nand_davinci_probe handling of nand-ecc-*
+			 * is broken, e.g.
+			 * chip.ecc.strength = pdata->ecc_bits occurs after
+			 * scan_ident(), otherwise I would have used:
+			 * 	nand-ecc-mode = "hw";
+			 * 	nand-ecc-strength = <1>;
+			 * 	nand-ecc-step-size = <512>;
+			 */
+			ti,davinci-nand-buswidth = <16>;
+			ti,davinci-ecc-mode = "hw";
+			ti,davinci-ecc-bits = <1>;
+			ti,davinci-nand-use-bbt;
+
+			/*
+			 * LCDK original partitions:
+			 * 0x000000000000-0x000000020000 : "u-boot env"
+			 * 0x000000020000-0x0000000a0000 : "u-boot"
+			 * 0x0000000a0000-0x0000002a0000 : "kernel"
+			 * 0x0000002a0000-0x000020000000 : "filesystem"
+			 *
+			 * The 1st NAND block being guaranted to be valid w/o ECC (> 1k cycles),
+			 * it makes a perfect candidate as an SPL for the BootROM to jump to.
+			 * However the OMAP-L132/L138 Bootloader doc SPRAB41E reads:
+			 * "To boot from NAND Flash, the AIS should be written to NAND block 1
+			 * (NAND block 0 is not used by default)", which matches the LCDK
+			 * original partitioning.
+			 * Also, the LCDK ships with only the u-boot partition provisioned and
+			 * boots on it in its default configuration while using the MMC for the
+			 * kernel and rootfs, so preserve that one as is for now.
+			 * [1]: Ensuring for example that U-Boot LCDK SPL can handle it properly
+			 * and a proper boot chain ROM->SPL->U-Boot->Linux wrt ECC, would allow
+			 * for a better partitioning.
+			 */
+			partitions {
+				compatible = "fixed-partitions";
+				#address-cells = <1>;
+				#size-cells = <1>;
+
+				partition at 0 {
+					label = "u-boot env";
+					reg = <0 0x020000>;
+				};
+				partition at 0x020000 {
+					/* The LCDK defaults to booting from this partition */
+					label = "u-boot";
+					reg = <0x020000 0x080000>;
+				};
+				partition at 0x0a0000 {
+					label = "space";
+					reg = <0x0a0000 0>;
+				};
+			};
+		};
+	};
+};
-- 
2.9.2

[PATCH v2 4/4] ARM: dts: da850-lcdk: Add NAND to DT

[Sekhar Nori]

On Wednesday 10 August 2016 04:30 PM, Karl Beldan wrote:
> This adds DT support for the NAND connected to the SoC AEMIF.
> The parameters (timings, ecc) are the same as what the board ships with
> (default AEMIF timings, 1bit ECC) and improvements will be handled in
> due course.
> This passed elementary tests hashing a 20MB file on top of ubifs on my
> LCDK.
> 
> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> ---
>  arch/arm/boot/dts/da850-lcdk.dts | 107 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 107 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
> index dbcca0b..a3f9845 100644
> --- a/arch/arm/boot/dts/da850-lcdk.dts
> +++ b/arch/arm/boot/dts/da850-lcdk.dts
> @@ -27,6 +27,27 @@

> +&aemif {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&nand_pins>;
> +	status = "ok";
> +	cs3 {
> +		#address-cells = <2>;
> +		#size-cells = <1>;
> +		clock-ranges;
> +		ranges;
> +
> +		ti,cs-chipselect = <3>;
> +
> +		nand at 2000000,0 {
> +			compatible = "ti,davinci-nand";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			reg = <0 0x02000000 0x02000000
> +			       1 0x00000000 0x00008000>;
> +
> +			ti,davinci-chipselect = <1>;
> +			ti,davinci-mask-ale = <0>;
> +			ti,davinci-mask-cle = <0>;
> +			ti,davinci-mask-chipsel = <0>;
> +
> +			/*
> +			 * nand_ecc_strength_good will emit a warning
> +			 * but the LCDK ships with these settings [1].
> +			 * Also HW 4bits ECC with 16bits NAND seems to
> +			 * require some attention.
> +			 *
> +			 * ATM nand_davinci_probe handling of nand-ecc-*
> +			 * is broken, e.g.
> +			 * chip.ecc.strength = pdata->ecc_bits occurs after
> +			 * scan_ident(), otherwise I would have used:
> +			 * 	nand-ecc-mode = "hw";
> +			 * 	nand-ecc-strength = <1>;
> +			 * 	nand-ecc-step-size = <512>;
> +			 */
> +			ti,davinci-nand-buswidth = <16>;
> +			ti,davinci-ecc-mode = "hw";
> +			ti,davinci-ecc-bits = <1>;
> +			ti,davinci-nand-use-bbt;

As discussed before, will apply this patch after 4-bit ECC support is
sorted out.

> +
> +			/*
> +			 * LCDK original partitions:
> +			 * 0x000000000000-0x000000020000 : "u-boot env"
> +			 * 0x000000020000-0x0000000a0000 : "u-boot"
> +			 * 0x0000000a0000-0x0000002a0000 : "kernel"
> +			 * 0x0000002a0000-0x000020000000 : "filesystem"
> +			 *
> +			 * The 1st NAND block being guaranted to be valid w/o ECC (> 1k cycles),
> +			 * it makes a perfect candidate as an SPL for the BootROM to jump to.
> +			 * However the OMAP-L132/L138 Bootloader doc SPRAB41E reads:
> +			 * "To boot from NAND Flash, the AIS should be written to NAND block 1
> +			 * (NAND block 0 is not used by default)", which matches the LCDK
> +			 * original partitioning.

FWIW, silicon version 2.1 supports booting from block 0, but needs new
boot pin settings not possible in stock LCDK.

> +			 * Also, the LCDK ships with only the u-boot partition provisioned and
> +			 * boots on it in its default configuration while using the MMC for the
> +			 * kernel and rootfs, so preserve that one as is for now.
> +			 * [1]: Ensuring for example that U-Boot LCDK SPL can handle it properly
> +			 * and a proper boot chain ROM->SPL->U-Boot->Linux wrt ECC, would allow
> +			 * for a better partitioning.
> +			 */

I would rather not refer to the software that LCDK ships with in the
comments at all. Because that can change without notice. At some point,
if mainline kernel works well enough on LCDK, TI might decide to ship
LCDKs with mainline kernel flashed.

> +			partitions {
> +				compatible = "fixed-partitions";
> +				#address-cells = <1>;
> +				#size-cells = <1>;
> +
> +				partition at 0 {
> +					label = "u-boot env";
> +					reg = <0 0x020000>;
> +				};
> +				partition at 0x020000 {
> +					/* The LCDK defaults to booting from this partition */
> +					label = "u-boot";
> +					reg = <0x020000 0x080000>;
> +				};
> +				partition at 0x0a0000 {
> +					label = "space";
> +					reg = <0x0a0000 0>;
> +				};
> +			};
> +		};
> +	};
> +};

Rest of the patch looks good to me.

Regards,
Sekhar

[PATCH v2 4/4] ARM: dts: da850-lcdk: Add NAND to DT

[Karl Beldan]

On Thu, Aug 18, 2016 at 03:27:52PM +0530, Sekhar Nori wrote:
> On Wednesday 10 August 2016 04:30 PM, Karl Beldan wrote:
> > This adds DT support for the NAND connected to the SoC AEMIF.
> > The parameters (timings, ecc) are the same as what the board ships with
> > (default AEMIF timings, 1bit ECC) and improvements will be handled in
> > due course.
> > This passed elementary tests hashing a 20MB file on top of ubifs on my
> > LCDK.
> > 
> > Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> > ---
> >  arch/arm/boot/dts/da850-lcdk.dts | 107 +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 107 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
> > index dbcca0b..a3f9845 100644
> > --- a/arch/arm/boot/dts/da850-lcdk.dts
> > +++ b/arch/arm/boot/dts/da850-lcdk.dts
> > @@ -27,6 +27,27 @@
> 
> > +&aemif {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&nand_pins>;
> > +	status = "ok";
> > +	cs3 {
> > +		#address-cells = <2>;
> > +		#size-cells = <1>;
> > +		clock-ranges;
> > +		ranges;
> > +
> > +		ti,cs-chipselect = <3>;
> > +
> > +		nand at 2000000,0 {
> > +			compatible = "ti,davinci-nand";
> > +			#address-cells = <1>;
> > +			#size-cells = <1>;
> > +			reg = <0 0x02000000 0x02000000
> > +			       1 0x00000000 0x00008000>;
> > +
> > +			ti,davinci-chipselect = <1>;
> > +			ti,davinci-mask-ale = <0>;
> > +			ti,davinci-mask-cle = <0>;
> > +			ti,davinci-mask-chipsel = <0>;
> > +
> > +			/*
> > +			 * nand_ecc_strength_good will emit a warning
> > +			 * but the LCDK ships with these settings [1].
> > +			 * Also HW 4bits ECC with 16bits NAND seems to
> > +			 * require some attention.
> > +			 *
> > +			 * ATM nand_davinci_probe handling of nand-ecc-*
> > +			 * is broken, e.g.
> > +			 * chip.ecc.strength = pdata->ecc_bits occurs after
> > +			 * scan_ident(), otherwise I would have used:
> > +			 * 	nand-ecc-mode = "hw";
> > +			 * 	nand-ecc-strength = <1>;
> > +			 * 	nand-ecc-step-size = <512>;
> > +			 */
> > +			ti,davinci-nand-buswidth = <16>;
> > +			ti,davinci-ecc-mode = "hw";
> > +			ti,davinci-ecc-bits = <1>;
> > +			ti,davinci-nand-use-bbt;
> 
> As discussed before, will apply this patch after 4-bit ECC support is
> sorted out.
> 

Ok. BTW it seems I might have the opportunity to look into this.

> > +
> > +			/*
> > +			 * LCDK original partitions:
> > +			 * 0x000000000000-0x000000020000 : "u-boot env"
> > +			 * 0x000000020000-0x0000000a0000 : "u-boot"
> > +			 * 0x0000000a0000-0x0000002a0000 : "kernel"
> > +			 * 0x0000002a0000-0x000020000000 : "filesystem"
> > +			 *
> > +			 * The 1st NAND block being guaranted to be valid w/o ECC (> 1k cycles),
> > +			 * it makes a perfect candidate as an SPL for the BootROM to jump to.
> > +			 * However the OMAP-L132/L138 Bootloader doc SPRAB41E reads:
> > +			 * "To boot from NAND Flash, the AIS should be written to NAND block 1
> > +			 * (NAND block 0 is not used by default)", which matches the LCDK
> > +			 * original partitioning.
> 
> FWIW, silicon version 2.1 supports booting from block 0, but needs new
> boot pin settings not possible in stock LCDK.
> 

That's a convenient improvement.

> > +			 * Also, the LCDK ships with only the u-boot partition provisioned and
> > +			 * boots on it in its default configuration while using the MMC for the
> > +			 * kernel and rootfs, so preserve that one as is for now.
> > +			 * [1]: Ensuring for example that U-Boot LCDK SPL can handle it properly
> > +			 * and a proper boot chain ROM->SPL->U-Boot->Linux wrt ECC, would allow
> > +			 * for a better partitioning.
> > +			 */
> 
> I would rather not refer to the software that LCDK ships with in the
> comments at all. Because that can change without notice. At some point,
> if mainline kernel works well enough on LCDK, TI might decide to ship
> LCDKs with mainline kernel flashed.
> 

That'd be a very good thing to get mainline well enough that it ships
with it. Given that information it would make sense to get rid of the
comments, which I'll keep in mind for when the ECC issue is sorted out.

Thanks for the review.

Regards, 
Karl

> > +			partitions {
> > +				compatible = "fixed-partitions";
> > +				#address-cells = <1>;
> > +				#size-cells = <1>;
> > +
> > +				partition at 0 {
> > +					label = "u-boot env";
> > +					reg = <0 0x020000>;
> > +				};
> > +				partition at 0x020000 {
> > +					/* The LCDK defaults to booting from this partition */
> > +					label = "u-boot";
> > +					reg = <0x020000 0x080000>;
> > +				};
> > +				partition at 0x0a0000 {
> > +					label = "space";
> > +					reg = <0x0a0000 0>;
> > +				};
> > +			};
> > +		};
> > +	};
> > +};
> 
> Rest of the patch looks good to me.
> 
> Regards,
> Sekhar
> 
>