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

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

[Karl Beldan]

Hi,

This does not use the same way the current da8xx boards do, instead
it is using the more generic and DT friendly memory driver ti-aemif.
I can do the same for the da850-evm and retire the dts nandcs3 instances.

Karl Beldan (4):
  memory: ti-aemif: Get a named clock rather than an unnamed one
  ARM: dts: da850: Add an aemif node
  ARM: dts: da850-lcdk: Add NAND to DT
  ARM: davinci_all_defconfig: Enable AEMIF as a module

 arch/arm/boot/dts/da850-lcdk.dts       | 108 +++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/da850.dtsi           |  10 +++
 arch/arm/configs/davinci_all_defconfig |   2 +
 drivers/memory/ti-aemif.c              |   2 +-
 4 files changed, 121 insertions(+), 1 deletion(-)

-- 
2.9.2

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

[Karl Beldan]

Hi,

This does not use the same way the current da8xx boards do, instead
it is using the more generic and DT friendly memory driver ti-aemif.
I can do the same for the da850-evm and retire the dts nandcs3 instances.

Karl Beldan (4):
  memory: ti-aemif: Get a named clock rather than an unnamed one
  ARM: dts: da850: Add an aemif node
  ARM: dts: da850-lcdk: Add NAND to DT
  ARM: davinci_all_defconfig: Enable AEMIF as a module

 arch/arm/boot/dts/da850-lcdk.dts       | 108 +++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/da850.dtsi           |  10 +++
 arch/arm/configs/davinci_all_defconfig |   2 +
 drivers/memory/ti-aemif.c              |   2 +-
 4 files changed, 121 insertions(+), 1 deletion(-)

-- 
2.9.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Karl Beldan]

Hi,

This does not use the same way the current da8xx boards do, instead
it is using the more generic and DT friendly memory driver ti-aemif.
I can do the same for the da850-evm and retire the dts nandcs3 instances.

Karl Beldan (4):
  memory: ti-aemif: Get a named clock rather than an unnamed one
  ARM: dts: da850: Add an aemif node
  ARM: dts: da850-lcdk: Add NAND to DT
  ARM: davinci_all_defconfig: Enable AEMIF as a module

 arch/arm/boot/dts/da850-lcdk.dts       | 108 +++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/da850.dtsi           |  10 +++
 arch/arm/configs/davinci_all_defconfig |   2 +
 drivers/memory/ti-aemif.c              |   2 +-
 4 files changed, 121 insertions(+), 1 deletion(-)

-- 
2.9.2

[PATCH 1/4] memory: ti-aemif: Get a named clock rather than an unnamed one

[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.
Also the sole current users of ti-aemif (keystone boards) use 'aemif' as
their aemif device clock clock-name and should remain unaffected.

Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
---
 drivers/memory/ti-aemif.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/memory/ti-aemif.c b/drivers/memory/ti-aemif.c
index a579a0f..c251fc8 100644
--- a/drivers/memory/ti-aemif.c
+++ b/drivers/memory/ti-aemif.c
@@ -345,7 +345,7 @@ static int aemif_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, aemif);
 
-	aemif->clk = devm_clk_get(dev, NULL);
+	aemif->clk = devm_clk_get(dev, "aemif");
 	if (IS_ERR(aemif->clk)) {
 		dev_err(dev, "cannot get clock 'aemif'\n");
 		return PTR_ERR(aemif->clk);
-- 
2.9.2

[PATCH 1/4] memory: ti-aemif: Get a named clock rather than an unnamed one

[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.
Also the sole current users of ti-aemif (keystone boards) use 'aemif' as
their aemif device clock clock-name and should remain unaffected.

Signed-off-by: Karl Beldan <kbeldan-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
 drivers/memory/ti-aemif.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/memory/ti-aemif.c b/drivers/memory/ti-aemif.c
index a579a0f..c251fc8 100644
--- a/drivers/memory/ti-aemif.c
+++ b/drivers/memory/ti-aemif.c
@@ -345,7 +345,7 @@ static int aemif_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, aemif);
 
-	aemif->clk = devm_clk_get(dev, NULL);
+	aemif->clk = devm_clk_get(dev, "aemif");
 	if (IS_ERR(aemif->clk)) {
 		dev_err(dev, "cannot get clock 'aemif'\n");
 		return PTR_ERR(aemif->clk);
-- 
2.9.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4] memory: ti-aemif: Get a named clock rather than an unnamed one

[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.
Also the sole current users of ti-aemif (keystone boards) use 'aemif' as
their aemif device clock clock-name and should remain unaffected.

Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
---
 drivers/memory/ti-aemif.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/memory/ti-aemif.c b/drivers/memory/ti-aemif.c
index a579a0f..c251fc8 100644
--- a/drivers/memory/ti-aemif.c
+++ b/drivers/memory/ti-aemif.c
@@ -345,7 +345,7 @@ static int aemif_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, aemif);
 
-	aemif->clk = devm_clk_get(dev, NULL);
+	aemif->clk = devm_clk_get(dev, "aemif");
 	if (IS_ERR(aemif->clk)) {
 		dev_err(dev, "cannot get clock 'aemif'\n");
 		return PTR_ERR(aemif->clk);
-- 
2.9.2

[PATCH 2/4] ARM: dts: da850: Add an aemif node

[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.
Also it will allow to properly pass the emif timings via DT.

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

diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index bc10e7e..f62928c 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -411,6 +411,16 @@
 			dma-names = "tx", "rx";
 		};
 	};
+	aemif: aemif@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";
+	};
 	nand_cs3@62000000 {
 		compatible = "ti,davinci-nand";
 		reg = <0x62000000 0x807ff
-- 
2.9.2

[PATCH 2/4] ARM: dts: da850: Add an aemif node

[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.
Also it will allow to properly pass the emif timings via DT.

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

diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index bc10e7e..f62928c 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -411,6 +411,16 @@
 			dma-names = "tx", "rx";
 		};
 	};
+	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";
+	};
 	nand_cs3 at 62000000 {
 		compatible = "ti,davinci-nand";
 		reg = <0x62000000 0x807ff
-- 
2.9.2

[PATCH 3/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 | 108 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 108 insertions(+)

diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
index dbcca0b..033380b 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,90 @@
 	cd-gpios = <&gpio 64 GPIO_ACTIVE_HIGH>;
 	status = "okay";
 };
+
+&aemif {
+	pinctrl-names = "default";
+	pinctrl-0 = <&nand_pins>;
+	status = "ok";
+	cs2 {
+		#address-cells = <2>;
+		#size-cells = <1>;
+		clock-ranges;
+		ranges;
+
+		ti,cs-chipselect = <2>;
+
+		nand@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-ecc-mode = "hw";
+			ti,davinci-ecc-bits = <1>;
+
+			nand-bus-width= <16>;
+			nand-on-flash-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@0 {
+					label = "u-boot env";
+					reg = <0 0x020000>;
+				};
+				partition@0x020000 {
+					/* The LCDK defaults to booting from this partition */
+					label = "u-boot";
+					reg = <0x020000 0x080000>;
+				};
+				partition@0x0a0000 {
+					label = "space";
+					reg = <0x0a0000 0>;
+				};
+			};
+		};
+	};
+};
-- 
2.9.2

[PATCH 3/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 | 108 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 108 insertions(+)

diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
index dbcca0b..033380b 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,90 @@
 	cd-gpios = <&gpio 64 GPIO_ACTIVE_HIGH>;
 	status = "okay";
 };
+
+&aemif {
+	pinctrl-names = "default";
+	pinctrl-0 = <&nand_pins>;
+	status = "ok";
+	cs2 {
+		#address-cells = <2>;
+		#size-cells = <1>;
+		clock-ranges;
+		ranges;
+
+		ti,cs-chipselect = <2>;
+
+		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-ecc-mode = "hw";
+			ti,davinci-ecc-bits = <1>;
+
+			nand-bus-width= <16>;
+			nand-on-flash-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 4/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 4/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

Re: [PATCH 1/4] memory: ti-aemif: Get a named clock rather than an unnamed one

[Karl Beldan]

On Tue, Aug 09, 2016 at 05:15:15PM +0000, 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.
> Also the sole current users of ti-aemif (keystone boards) use 'aemif' as
> their aemif device clock clock-name and should remain unaffected.
> 
> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> ---
>  drivers/memory/ti-aemif.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/memory/ti-aemif.c b/drivers/memory/ti-aemif.c
> index a579a0f..c251fc8 100644
> --- a/drivers/memory/ti-aemif.c
> +++ b/drivers/memory/ti-aemif.c
> @@ -345,7 +345,7 @@ static int aemif_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, aemif);
>  
> -	aemif->clk = devm_clk_get(dev, NULL);
> +	aemif->clk = devm_clk_get(dev, "aemif");

Looking further it seems to me that the struct clk_lookup da850_clks
registered by davinci_clk_init() should be enough to clk_get() unnamed
clocks using only the dev name. I look into what's going on but it
would make this patch unnecessary.
 
Karl

Re: [PATCH 1/4] memory: ti-aemif: Get a named clock rather than an unnamed one

[Karl Beldan]

On Tue, Aug 09, 2016 at 05:15:15PM +0000, 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.
> Also the sole current users of ti-aemif (keystone boards) use 'aemif' as
> their aemif device clock clock-name and should remain unaffected.
> 
> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> ---
>  drivers/memory/ti-aemif.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/memory/ti-aemif.c b/drivers/memory/ti-aemif.c
> index a579a0f..c251fc8 100644
> --- a/drivers/memory/ti-aemif.c
> +++ b/drivers/memory/ti-aemif.c
> @@ -345,7 +345,7 @@ static int aemif_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, aemif);
>  
> -	aemif->clk = devm_clk_get(dev, NULL);
> +	aemif->clk = devm_clk_get(dev, "aemif");

Looking further it seems to me that the struct clk_lookup da850_clks
registered by davinci_clk_init() should be enough to clk_get() unnamed
clocks using only the dev name. I look into what's going on but it
would make this patch unnecessary.
 
Karl

[PATCH 1/4] memory: ti-aemif: Get a named clock rather than an unnamed one

[Karl Beldan]

On Tue, Aug 09, 2016 at 05:15:15PM +0000, 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.
> Also the sole current users of ti-aemif (keystone boards) use 'aemif' as
> their aemif device clock clock-name and should remain unaffected.
> 
> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> ---
>  drivers/memory/ti-aemif.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/memory/ti-aemif.c b/drivers/memory/ti-aemif.c
> index a579a0f..c251fc8 100644
> --- a/drivers/memory/ti-aemif.c
> +++ b/drivers/memory/ti-aemif.c
> @@ -345,7 +345,7 @@ static int aemif_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, aemif);
>  
> -	aemif->clk = devm_clk_get(dev, NULL);
> +	aemif->clk = devm_clk_get(dev, "aemif");

Looking further it seems to me that the struct clk_lookup da850_clks
registered by davinci_clk_init() should be enough to clk_get() unnamed
clocks using only the dev name. I look into what's going on but it
would make this patch unnecessary.
 
Karl

Re: [PATCH 1/4] memory: ti-aemif: Get a named clock rather than an unnamed one

[Karl Beldan]

On Wed, Aug 10, 2016 at 07:00:20AM +0000, Karl Beldan wrote:
> On Tue, Aug 09, 2016 at 05:15:15PM +0000, 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.
> > Also the sole current users of ti-aemif (keystone boards) use 'aemif' as
> > their aemif device clock clock-name and should remain unaffected.
> > 
> > Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> > ---
> >  drivers/memory/ti-aemif.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/memory/ti-aemif.c b/drivers/memory/ti-aemif.c
> > index a579a0f..c251fc8 100644
> > --- a/drivers/memory/ti-aemif.c
> > +++ b/drivers/memory/ti-aemif.c
> > @@ -345,7 +345,7 @@ static int aemif_probe(struct platform_device *pdev)
> >  
> >  	platform_set_drvdata(pdev, aemif);
> >  
> > -	aemif->clk = devm_clk_get(dev, NULL);
> > +	aemif->clk = devm_clk_get(dev, "aemif");
> 
> Looking further it seems to me that the struct clk_lookup da850_clks
> registered by davinci_clk_init() should be enough to clk_get() unnamed
> clocks using only the dev name. I look into what's going on but it
> would make this patch unnecessary.
>  
Ok, just saw what's happening, this patch is unnecessary, v2 will
follow.
 
Karl

Re: [PATCH 1/4] memory: ti-aemif: Get a named clock rather than an unnamed one

[Karl Beldan]

On Wed, Aug 10, 2016 at 07:00:20AM +0000, Karl Beldan wrote:
> On Tue, Aug 09, 2016 at 05:15:15PM +0000, 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.
> > Also the sole current users of ti-aemif (keystone boards) use 'aemif' as
> > their aemif device clock clock-name and should remain unaffected.
> > 
> > Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> > ---
> >  drivers/memory/ti-aemif.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/memory/ti-aemif.c b/drivers/memory/ti-aemif.c
> > index a579a0f..c251fc8 100644
> > --- a/drivers/memory/ti-aemif.c
> > +++ b/drivers/memory/ti-aemif.c
> > @@ -345,7 +345,7 @@ static int aemif_probe(struct platform_device *pdev)
> >  
> >  	platform_set_drvdata(pdev, aemif);
> >  
> > -	aemif->clk = devm_clk_get(dev, NULL);
> > +	aemif->clk = devm_clk_get(dev, "aemif");
> 
> Looking further it seems to me that the struct clk_lookup da850_clks
> registered by davinci_clk_init() should be enough to clk_get() unnamed
> clocks using only the dev name. I look into what's going on but it
> would make this patch unnecessary.
>  
Ok, just saw what's happening, this patch is unnecessary, v2 will
follow.
 
Karl

[PATCH 1/4] memory: ti-aemif: Get a named clock rather than an unnamed one

[Karl Beldan]

On Wed, Aug 10, 2016 at 07:00:20AM +0000, Karl Beldan wrote:
> On Tue, Aug 09, 2016 at 05:15:15PM +0000, 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.
> > Also the sole current users of ti-aemif (keystone boards) use 'aemif' as
> > their aemif device clock clock-name and should remain unaffected.
> > 
> > Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> > ---
> >  drivers/memory/ti-aemif.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/memory/ti-aemif.c b/drivers/memory/ti-aemif.c
> > index a579a0f..c251fc8 100644
> > --- a/drivers/memory/ti-aemif.c
> > +++ b/drivers/memory/ti-aemif.c
> > @@ -345,7 +345,7 @@ static int aemif_probe(struct platform_device *pdev)
> >  
> >  	platform_set_drvdata(pdev, aemif);
> >  
> > -	aemif->clk = devm_clk_get(dev, NULL);
> > +	aemif->clk = devm_clk_get(dev, "aemif");
> 
> Looking further it seems to me that the struct clk_lookup da850_clks
> registered by davinci_clk_init() should be enough to clk_get() unnamed
> clocks using only the dev name. I look into what's going on but it
> would make this patch unnecessary.
>  
Ok, just saw what's happening, this patch is unnecessary, v2 will
follow.
 
Karl

Re: [PATCH 2/4] ARM: dts: da850: Add an aemif node

[Sekhar Nori]

On Tuesday 09 August 2016 10:45 PM, 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.
> Also it will allow to properly pass the emif timings via DT.
> 
> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> ---
>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> index bc10e7e..f62928c 100644
> --- a/arch/arm/boot/dts/da850.dtsi
> +++ b/arch/arm/boot/dts/da850.dtsi
> @@ -411,6 +411,16 @@
>  			dma-names = "tx", "rx";
>  		};
>  	};
> +	aemif: aemif@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";
> +	};
>  	nand_cs3@62000000 {
>  		compatible = "ti,davinci-nand";
>  		reg = <0x62000000 0x807ff

The nand node should be part of aemif node like it is done for keystone
boards.

Regards,
Sekhar

Re: [PATCH 2/4] ARM: dts: da850: Add an aemif node

[Sekhar Nori]

On Tuesday 09 August 2016 10:45 PM, 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.
> Also it will allow to properly pass the emif timings via DT.
> 
> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> ---
>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> index bc10e7e..f62928c 100644
> --- a/arch/arm/boot/dts/da850.dtsi
> +++ b/arch/arm/boot/dts/da850.dtsi
> @@ -411,6 +411,16 @@
>  			dma-names = "tx", "rx";
>  		};
>  	};
> +	aemif: aemif@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";
> +	};
>  	nand_cs3@62000000 {
>  		compatible = "ti,davinci-nand";
>  		reg = <0x62000000 0x807ff

The nand node should be part of aemif node like it is done for keystone
boards.

Regards,
Sekhar

[PATCH 2/4] ARM: dts: da850: Add an aemif node

[Sekhar Nori]

On Tuesday 09 August 2016 10:45 PM, 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.
> Also it will allow to properly pass the emif timings via DT.
> 
> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> ---
>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> index bc10e7e..f62928c 100644
> --- a/arch/arm/boot/dts/da850.dtsi
> +++ b/arch/arm/boot/dts/da850.dtsi
> @@ -411,6 +411,16 @@
>  			dma-names = "tx", "rx";
>  		};
>  	};
> +	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";
> +	};
>  	nand_cs3 at 62000000 {
>  		compatible = "ti,davinci-nand";
>  		reg = <0x62000000 0x807ff

The nand node should be part of aemif node like it is done for keystone
boards.

Regards,
Sekhar

Re: [PATCH 2/4] ARM: dts: da850: Add an aemif node

[Karl Beldan]

On Wed, Aug 10, 2016 at 01:18:51PM +0530, Sekhar Nori wrote:
> On Tuesday 09 August 2016 10:45 PM, 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.
> > Also it will allow to properly pass the emif timings via DT.
> > 
> > Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> > ---
> >  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> > index bc10e7e..f62928c 100644
> > --- a/arch/arm/boot/dts/da850.dtsi
> > +++ b/arch/arm/boot/dts/da850.dtsi
> > @@ -411,6 +411,16 @@
> >  			dma-names = "tx", "rx";
> >  		};
> >  	};
> > +	aemif: aemif@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";
> > +	};
> >  	nand_cs3@62000000 {
> >  		compatible = "ti,davinci-nand";
> >  		reg = <0x62000000 0x807ff
> 
> The nand node should be part of aemif node like it is done for keystone
> boards.
> 
Hmm, this is exactly what I have done.
 
Karl

Re: [PATCH 2/4] ARM: dts: da850: Add an aemif node

[Karl Beldan]

On Wed, Aug 10, 2016 at 01:18:51PM +0530, Sekhar Nori wrote:
> On Tuesday 09 August 2016 10:45 PM, 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.
> > Also it will allow to properly pass the emif timings via DT.
> > 
> > Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> > ---
> >  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> > index bc10e7e..f62928c 100644
> > --- a/arch/arm/boot/dts/da850.dtsi
> > +++ b/arch/arm/boot/dts/da850.dtsi
> > @@ -411,6 +411,16 @@
> >  			dma-names = "tx", "rx";
> >  		};
> >  	};
> > +	aemif: aemif@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";
> > +	};
> >  	nand_cs3@62000000 {
> >  		compatible = "ti,davinci-nand";
> >  		reg = <0x62000000 0x807ff
> 
> The nand node should be part of aemif node like it is done for keystone
> boards.
> 
Hmm, this is exactly what I have done.
 
Karl

[PATCH 2/4] ARM: dts: da850: Add an aemif node

[Karl Beldan]

On Wed, Aug 10, 2016 at 01:18:51PM +0530, Sekhar Nori wrote:
> On Tuesday 09 August 2016 10:45 PM, 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.
> > Also it will allow to properly pass the emif timings via DT.
> > 
> > Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> > ---
> >  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> > index bc10e7e..f62928c 100644
> > --- a/arch/arm/boot/dts/da850.dtsi
> > +++ b/arch/arm/boot/dts/da850.dtsi
> > @@ -411,6 +411,16 @@
> >  			dma-names = "tx", "rx";
> >  		};
> >  	};
> > +	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";
> > +	};
> >  	nand_cs3 at 62000000 {
> >  		compatible = "ti,davinci-nand";
> >  		reg = <0x62000000 0x807ff
> 
> The nand node should be part of aemif node like it is done for keystone
> boards.
> 
Hmm, this is exactly what I have done.
 
Karl

Re: [PATCH 2/4] ARM: dts: da850: Add an aemif node

[Sekhar Nori]

On Wednesday 10 August 2016 01:18 PM, Sekhar Nori wrote:
> On Tuesday 09 August 2016 10:45 PM, 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.
>> Also it will allow to properly pass the emif timings via DT.
>>
>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
>> ---
>>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>> index bc10e7e..f62928c 100644
>> --- a/arch/arm/boot/dts/da850.dtsi
>> +++ b/arch/arm/boot/dts/da850.dtsi
>> @@ -411,6 +411,16 @@
>>  			dma-names = "tx", "rx";
>>  		};
>>  	};
>> +	aemif: aemif@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";
>> +	};
>>  	nand_cs3@62000000 {
>>  		compatible = "ti,davinci-nand";
>>  		reg = <0x62000000 0x807ff
> 
> The nand node should be part of aemif node like it is done for keystone
> boards.

Actually, can you move the nand node out of da850.dtsi completely. Its
much better to keep da850.dtsi restricted to soc-internal devices and
keep the board level devices like NAND flash in <board>.dts file.

Similarly, can you move the NAND pinmux definitions too to the
da850-evm.dts file?

There is advantage in keeping common pinmux definitions in da850.dtsi so
each board doe not have to repeat them. But AEMIF is an exception as its
usage can really be varied (NAND, NOR, SRAM, other). Plus, different
boards are likely to use different chip selects so coming up with some
pinmux definitions which will be reused widely is really unlikely.

Thanks,
Sekhar

Re: [PATCH 2/4] ARM: dts: da850: Add an aemif node

[Sekhar Nori]

On Wednesday 10 August 2016 01:18 PM, Sekhar Nori wrote:
> On Tuesday 09 August 2016 10:45 PM, 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.
>> Also it will allow to properly pass the emif timings via DT.
>>
>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
>> ---
>>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>> index bc10e7e..f62928c 100644
>> --- a/arch/arm/boot/dts/da850.dtsi
>> +++ b/arch/arm/boot/dts/da850.dtsi
>> @@ -411,6 +411,16 @@
>>  			dma-names = "tx", "rx";
>>  		};
>>  	};
>> +	aemif: aemif@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";
>> +	};
>>  	nand_cs3@62000000 {
>>  		compatible = "ti,davinci-nand";
>>  		reg = <0x62000000 0x807ff
> 
> The nand node should be part of aemif node like it is done for keystone
> boards.

Actually, can you move the nand node out of da850.dtsi completely. Its
much better to keep da850.dtsi restricted to soc-internal devices and
keep the board level devices like NAND flash in <board>.dts file.

Similarly, can you move the NAND pinmux definitions too to the
da850-evm.dts file?

There is advantage in keeping common pinmux definitions in da850.dtsi so
each board doe not have to repeat them. But AEMIF is an exception as its
usage can really be varied (NAND, NOR, SRAM, other). Plus, different
boards are likely to use different chip selects so coming up with some
pinmux definitions which will be reused widely is really unlikely.

Thanks,
Sekhar

[PATCH 2/4] ARM: dts: da850: Add an aemif node

[Sekhar Nori]

On Wednesday 10 August 2016 01:18 PM, Sekhar Nori wrote:
> On Tuesday 09 August 2016 10:45 PM, 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.
>> Also it will allow to properly pass the emif timings via DT.
>>
>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
>> ---
>>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>> index bc10e7e..f62928c 100644
>> --- a/arch/arm/boot/dts/da850.dtsi
>> +++ b/arch/arm/boot/dts/da850.dtsi
>> @@ -411,6 +411,16 @@
>>  			dma-names = "tx", "rx";
>>  		};
>>  	};
>> +	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";
>> +	};
>>  	nand_cs3 at 62000000 {
>>  		compatible = "ti,davinci-nand";
>>  		reg = <0x62000000 0x807ff
> 
> The nand node should be part of aemif node like it is done for keystone
> boards.

Actually, can you move the nand node out of da850.dtsi completely. Its
much better to keep da850.dtsi restricted to soc-internal devices and
keep the board level devices like NAND flash in <board>.dts file.

Similarly, can you move the NAND pinmux definitions too to the
da850-evm.dts file?

There is advantage in keeping common pinmux definitions in da850.dtsi so
each board doe not have to repeat them. But AEMIF is an exception as its
usage can really be varied (NAND, NOR, SRAM, other). Plus, different
boards are likely to use different chip selects so coming up with some
pinmux definitions which will be reused widely is really unlikely.

Thanks,
Sekhar

Re: [PATCH 2/4] ARM: dts: da850: Add an aemif node

[Karl Beldan]

On Wed, Aug 10, 2016 at 01:32:03PM +0530, Sekhar Nori wrote:
> On Wednesday 10 August 2016 01:18 PM, Sekhar Nori wrote:
> > On Tuesday 09 August 2016 10:45 PM, 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.
> >> Also it will allow to properly pass the emif timings via DT.
> >>
> >> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> >> ---
> >>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
> >>  1 file changed, 10 insertions(+)
> >>
> >> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> >> index bc10e7e..f62928c 100644
> >> --- a/arch/arm/boot/dts/da850.dtsi
> >> +++ b/arch/arm/boot/dts/da850.dtsi
> >> @@ -411,6 +411,16 @@
> >>  			dma-names = "tx", "rx";
> >>  		};
> >>  	};
> >> +	aemif: aemif@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";
> >> +	};
> >>  	nand_cs3@62000000 {
> >>  		compatible = "ti,davinci-nand";
> >>  		reg = <0x62000000 0x807ff
> > 
> > The nand node should be part of aemif node like it is done for keystone
> > boards.
> 
> Actually, can you move the nand node out of da850.dtsi completely. Its
> much better to keep da850.dtsi restricted to soc-internal devices and
> keep the board level devices like NAND flash in <board>.dts file.
> 
> Similarly, can you move the NAND pinmux definitions too to the
> da850-evm.dts file?
> 
> There is advantage in keeping common pinmux definitions in da850.dtsi so
> each board doe not have to repeat them. But AEMIF is an exception as its
> usage can really be varied (NAND, NOR, SRAM, other). Plus, different
> boards are likely to use different chip selects so coming up with some
> pinmux definitions which will be reused widely is really unlikely.
> 
This is exactly what I just did for the LCDK.
If everybody is happy with it I will do the same for the evm as I put it
in the cover letter.
 
Karl

Re: [PATCH 2/4] ARM: dts: da850: Add an aemif node

[Karl Beldan]

On Wed, Aug 10, 2016 at 01:32:03PM +0530, Sekhar Nori wrote:
> On Wednesday 10 August 2016 01:18 PM, Sekhar Nori wrote:
> > On Tuesday 09 August 2016 10:45 PM, 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.
> >> Also it will allow to properly pass the emif timings via DT.
> >>
> >> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> >> ---
> >>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
> >>  1 file changed, 10 insertions(+)
> >>
> >> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> >> index bc10e7e..f62928c 100644
> >> --- a/arch/arm/boot/dts/da850.dtsi
> >> +++ b/arch/arm/boot/dts/da850.dtsi
> >> @@ -411,6 +411,16 @@
> >>  			dma-names = "tx", "rx";
> >>  		};
> >>  	};
> >> +	aemif: aemif@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";
> >> +	};
> >>  	nand_cs3@62000000 {
> >>  		compatible = "ti,davinci-nand";
> >>  		reg = <0x62000000 0x807ff
> > 
> > The nand node should be part of aemif node like it is done for keystone
> > boards.
> 
> Actually, can you move the nand node out of da850.dtsi completely. Its
> much better to keep da850.dtsi restricted to soc-internal devices and
> keep the board level devices like NAND flash in <board>.dts file.
> 
> Similarly, can you move the NAND pinmux definitions too to the
> da850-evm.dts file?
> 
> There is advantage in keeping common pinmux definitions in da850.dtsi so
> each board doe not have to repeat them. But AEMIF is an exception as its
> usage can really be varied (NAND, NOR, SRAM, other). Plus, different
> boards are likely to use different chip selects so coming up with some
> pinmux definitions which will be reused widely is really unlikely.
> 
This is exactly what I just did for the LCDK.
If everybody is happy with it I will do the same for the evm as I put it
in the cover letter.
 
Karl

[PATCH 2/4] ARM: dts: da850: Add an aemif node

[Karl Beldan]

On Wed, Aug 10, 2016 at 01:32:03PM +0530, Sekhar Nori wrote:
> On Wednesday 10 August 2016 01:18 PM, Sekhar Nori wrote:
> > On Tuesday 09 August 2016 10:45 PM, 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.
> >> Also it will allow to properly pass the emif timings via DT.
> >>
> >> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> >> ---
> >>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
> >>  1 file changed, 10 insertions(+)
> >>
> >> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> >> index bc10e7e..f62928c 100644
> >> --- a/arch/arm/boot/dts/da850.dtsi
> >> +++ b/arch/arm/boot/dts/da850.dtsi
> >> @@ -411,6 +411,16 @@
> >>  			dma-names = "tx", "rx";
> >>  		};
> >>  	};
> >> +	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";
> >> +	};
> >>  	nand_cs3 at 62000000 {
> >>  		compatible = "ti,davinci-nand";
> >>  		reg = <0x62000000 0x807ff
> > 
> > The nand node should be part of aemif node like it is done for keystone
> > boards.
> 
> Actually, can you move the nand node out of da850.dtsi completely. Its
> much better to keep da850.dtsi restricted to soc-internal devices and
> keep the board level devices like NAND flash in <board>.dts file.
> 
> Similarly, can you move the NAND pinmux definitions too to the
> da850-evm.dts file?
> 
> There is advantage in keeping common pinmux definitions in da850.dtsi so
> each board doe not have to repeat them. But AEMIF is an exception as its
> usage can really be varied (NAND, NOR, SRAM, other). Plus, different
> boards are likely to use different chip selects so coming up with some
> pinmux definitions which will be reused widely is really unlikely.
> 
This is exactly what I just did for the LCDK.
If everybody is happy with it I will do the same for the evm as I put it
in the cover letter.
 
Karl

Re: [PATCH 2/4] ARM: dts: da850: Add an aemif node

[Sekhar Nori]

On Wednesday 10 August 2016 01:37 PM, Karl Beldan wrote:
> On Wed, Aug 10, 2016 at 01:32:03PM +0530, Sekhar Nori wrote:
>> On Wednesday 10 August 2016 01:18 PM, Sekhar Nori wrote:
>>> On Tuesday 09 August 2016 10:45 PM, 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.
>>>> Also it will allow to properly pass the emif timings via DT.
>>>>
>>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
>>>> ---
>>>>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
>>>>  1 file changed, 10 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>>>> index bc10e7e..f62928c 100644
>>>> --- a/arch/arm/boot/dts/da850.dtsi
>>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>>> @@ -411,6 +411,16 @@
>>>>  			dma-names = "tx", "rx";
>>>>  		};
>>>>  	};
>>>> +	aemif: aemif@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";
>>>> +	};
>>>>  	nand_cs3@62000000 {
>>>>  		compatible = "ti,davinci-nand";
>>>>  		reg = <0x62000000 0x807ff
>>>
>>> The nand node should be part of aemif node like it is done for keystone
>>> boards.
>>
>> Actually, can you move the nand node out of da850.dtsi completely. Its
>> much better to keep da850.dtsi restricted to soc-internal devices and
>> keep the board level devices like NAND flash in <board>.dts file.
>>
>> Similarly, can you move the NAND pinmux definitions too to the
>> da850-evm.dts file?
>>
>> There is advantage in keeping common pinmux definitions in da850.dtsi so
>> each board doe not have to repeat them. But AEMIF is an exception as its
>> usage can really be varied (NAND, NOR, SRAM, other). Plus, different
>> boards are likely to use different chip selects so coming up with some
>> pinmux definitions which will be reused widely is really unlikely.
>>
> This is exactly what I just did for the LCDK.
> If everybody is happy with it I will do the same for the evm as I put it
> in the cover letter.

Yes please. We dont want duplication of data between da850.dtsi and
da850-lcdk.dts files.

Thanks,
Sekhar

Re: [PATCH 2/4] ARM: dts: da850: Add an aemif node

[Sekhar Nori]

On Wednesday 10 August 2016 01:37 PM, Karl Beldan wrote:
> On Wed, Aug 10, 2016 at 01:32:03PM +0530, Sekhar Nori wrote:
>> On Wednesday 10 August 2016 01:18 PM, Sekhar Nori wrote:
>>> On Tuesday 09 August 2016 10:45 PM, 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.
>>>> Also it will allow to properly pass the emif timings via DT.
>>>>
>>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
>>>> ---
>>>>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
>>>>  1 file changed, 10 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>>>> index bc10e7e..f62928c 100644
>>>> --- a/arch/arm/boot/dts/da850.dtsi
>>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>>> @@ -411,6 +411,16 @@
>>>>  			dma-names = "tx", "rx";
>>>>  		};
>>>>  	};
>>>> +	aemif: aemif@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";
>>>> +	};
>>>>  	nand_cs3@62000000 {
>>>>  		compatible = "ti,davinci-nand";
>>>>  		reg = <0x62000000 0x807ff
>>>
>>> The nand node should be part of aemif node like it is done for keystone
>>> boards.
>>
>> Actually, can you move the nand node out of da850.dtsi completely. Its
>> much better to keep da850.dtsi restricted to soc-internal devices and
>> keep the board level devices like NAND flash in <board>.dts file.
>>
>> Similarly, can you move the NAND pinmux definitions too to the
>> da850-evm.dts file?
>>
>> There is advantage in keeping common pinmux definitions in da850.dtsi so
>> each board doe not have to repeat them. But AEMIF is an exception as its
>> usage can really be varied (NAND, NOR, SRAM, other). Plus, different
>> boards are likely to use different chip selects so coming up with some
>> pinmux definitions which will be reused widely is really unlikely.
>>
> This is exactly what I just did for the LCDK.
> If everybody is happy with it I will do the same for the evm as I put it
> in the cover letter.

Yes please. We dont want duplication of data between da850.dtsi and
da850-lcdk.dts files.

Thanks,
Sekhar

[PATCH 2/4] ARM: dts: da850: Add an aemif node

[Sekhar Nori]

On Wednesday 10 August 2016 01:37 PM, Karl Beldan wrote:
> On Wed, Aug 10, 2016 at 01:32:03PM +0530, Sekhar Nori wrote:
>> On Wednesday 10 August 2016 01:18 PM, Sekhar Nori wrote:
>>> On Tuesday 09 August 2016 10:45 PM, 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.
>>>> Also it will allow to properly pass the emif timings via DT.
>>>>
>>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
>>>> ---
>>>>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
>>>>  1 file changed, 10 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>>>> index bc10e7e..f62928c 100644
>>>> --- a/arch/arm/boot/dts/da850.dtsi
>>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>>> @@ -411,6 +411,16 @@
>>>>  			dma-names = "tx", "rx";
>>>>  		};
>>>>  	};
>>>> +	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";
>>>> +	};
>>>>  	nand_cs3 at 62000000 {
>>>>  		compatible = "ti,davinci-nand";
>>>>  		reg = <0x62000000 0x807ff
>>>
>>> The nand node should be part of aemif node like it is done for keystone
>>> boards.
>>
>> Actually, can you move the nand node out of da850.dtsi completely. Its
>> much better to keep da850.dtsi restricted to soc-internal devices and
>> keep the board level devices like NAND flash in <board>.dts file.
>>
>> Similarly, can you move the NAND pinmux definitions too to the
>> da850-evm.dts file?
>>
>> There is advantage in keeping common pinmux definitions in da850.dtsi so
>> each board doe not have to repeat them. But AEMIF is an exception as its
>> usage can really be varied (NAND, NOR, SRAM, other). Plus, different
>> boards are likely to use different chip selects so coming up with some
>> pinmux definitions which will be reused widely is really unlikely.
>>
> This is exactly what I just did for the LCDK.
> If everybody is happy with it I will do the same for the evm as I put it
> in the cover letter.

Yes please. We dont want duplication of data between da850.dtsi and
da850-lcdk.dts files.

Thanks,
Sekhar

Re: [PATCH 2/4] ARM: dts: da850: Add an aemif node

[Karl Beldan]

On Wed, Aug 10, 2016 at 01:42:01PM +0530, Sekhar Nori wrote:
> On Wednesday 10 August 2016 01:37 PM, Karl Beldan wrote:
> > On Wed, Aug 10, 2016 at 01:32:03PM +0530, Sekhar Nori wrote:
> >> On Wednesday 10 August 2016 01:18 PM, Sekhar Nori wrote:
> >>> On Tuesday 09 August 2016 10:45 PM, 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.
> >>>> Also it will allow to properly pass the emif timings via DT.
> >>>>
> >>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> >>>> ---
> >>>>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
> >>>>  1 file changed, 10 insertions(+)
> >>>>
> >>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> >>>> index bc10e7e..f62928c 100644
> >>>> --- a/arch/arm/boot/dts/da850.dtsi
> >>>> +++ b/arch/arm/boot/dts/da850.dtsi
> >>>> @@ -411,6 +411,16 @@
> >>>>  			dma-names = "tx", "rx";
> >>>>  		};
> >>>>  	};
> >>>> +	aemif: aemif@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";
> >>>> +	};
> >>>>  	nand_cs3@62000000 {
> >>>>  		compatible = "ti,davinci-nand";
> >>>>  		reg = <0x62000000 0x807ff
> >>>
> >>> The nand node should be part of aemif node like it is done for keystone
> >>> boards.
> >>
> >> Actually, can you move the nand node out of da850.dtsi completely. Its
> >> much better to keep da850.dtsi restricted to soc-internal devices and
> >> keep the board level devices like NAND flash in <board>.dts file.
> >>
> >> Similarly, can you move the NAND pinmux definitions too to the
> >> da850-evm.dts file?
> >>
> >> There is advantage in keeping common pinmux definitions in da850.dtsi so
> >> each board doe not have to repeat them. But AEMIF is an exception as its
> >> usage can really be varied (NAND, NOR, SRAM, other). Plus, different
> >> boards are likely to use different chip selects so coming up with some
> >> pinmux definitions which will be reused widely is really unlikely.
> >>
> > This is exactly what I just did for the LCDK.
> > If everybody is happy with it I will do the same for the evm as I put it
> > in the cover letter.
> 
> Yes please. We dont want duplication of data between da850.dtsi and
> da850-lcdk.dts files.
> 
Then I'll wait for this series to be applied and then apply my changes
to the EVM while retiring the nand_cs3 together.
 
Karl

Re: [PATCH 2/4] ARM: dts: da850: Add an aemif node

[Karl Beldan]

On Wed, Aug 10, 2016 at 01:42:01PM +0530, Sekhar Nori wrote:
> On Wednesday 10 August 2016 01:37 PM, Karl Beldan wrote:
> > On Wed, Aug 10, 2016 at 01:32:03PM +0530, Sekhar Nori wrote:
> >> On Wednesday 10 August 2016 01:18 PM, Sekhar Nori wrote:
> >>> On Tuesday 09 August 2016 10:45 PM, 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.
> >>>> Also it will allow to properly pass the emif timings via DT.
> >>>>
> >>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> >>>> ---
> >>>>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
> >>>>  1 file changed, 10 insertions(+)
> >>>>
> >>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> >>>> index bc10e7e..f62928c 100644
> >>>> --- a/arch/arm/boot/dts/da850.dtsi
> >>>> +++ b/arch/arm/boot/dts/da850.dtsi
> >>>> @@ -411,6 +411,16 @@
> >>>>  			dma-names = "tx", "rx";
> >>>>  		};
> >>>>  	};
> >>>> +	aemif: aemif@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";
> >>>> +	};
> >>>>  	nand_cs3@62000000 {
> >>>>  		compatible = "ti,davinci-nand";
> >>>>  		reg = <0x62000000 0x807ff
> >>>
> >>> The nand node should be part of aemif node like it is done for keystone
> >>> boards.
> >>
> >> Actually, can you move the nand node out of da850.dtsi completely. Its
> >> much better to keep da850.dtsi restricted to soc-internal devices and
> >> keep the board level devices like NAND flash in <board>.dts file.
> >>
> >> Similarly, can you move the NAND pinmux definitions too to the
> >> da850-evm.dts file?
> >>
> >> There is advantage in keeping common pinmux definitions in da850.dtsi so
> >> each board doe not have to repeat them. But AEMIF is an exception as its
> >> usage can really be varied (NAND, NOR, SRAM, other). Plus, different
> >> boards are likely to use different chip selects so coming up with some
> >> pinmux definitions which will be reused widely is really unlikely.
> >>
> > This is exactly what I just did for the LCDK.
> > If everybody is happy with it I will do the same for the evm as I put it
> > in the cover letter.
> 
> Yes please. We dont want duplication of data between da850.dtsi and
> da850-lcdk.dts files.
> 
Then I'll wait for this series to be applied and then apply my changes
to the EVM while retiring the nand_cs3 together.
 
Karl

[PATCH 2/4] ARM: dts: da850: Add an aemif node

[Karl Beldan]

On Wed, Aug 10, 2016 at 01:42:01PM +0530, Sekhar Nori wrote:
> On Wednesday 10 August 2016 01:37 PM, Karl Beldan wrote:
> > On Wed, Aug 10, 2016 at 01:32:03PM +0530, Sekhar Nori wrote:
> >> On Wednesday 10 August 2016 01:18 PM, Sekhar Nori wrote:
> >>> On Tuesday 09 August 2016 10:45 PM, 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.
> >>>> Also it will allow to properly pass the emif timings via DT.
> >>>>
> >>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> >>>> ---
> >>>>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
> >>>>  1 file changed, 10 insertions(+)
> >>>>
> >>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> >>>> index bc10e7e..f62928c 100644
> >>>> --- a/arch/arm/boot/dts/da850.dtsi
> >>>> +++ b/arch/arm/boot/dts/da850.dtsi
> >>>> @@ -411,6 +411,16 @@
> >>>>  			dma-names = "tx", "rx";
> >>>>  		};
> >>>>  	};
> >>>> +	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";
> >>>> +	};
> >>>>  	nand_cs3 at 62000000 {
> >>>>  		compatible = "ti,davinci-nand";
> >>>>  		reg = <0x62000000 0x807ff
> >>>
> >>> The nand node should be part of aemif node like it is done for keystone
> >>> boards.
> >>
> >> Actually, can you move the nand node out of da850.dtsi completely. Its
> >> much better to keep da850.dtsi restricted to soc-internal devices and
> >> keep the board level devices like NAND flash in <board>.dts file.
> >>
> >> Similarly, can you move the NAND pinmux definitions too to the
> >> da850-evm.dts file?
> >>
> >> There is advantage in keeping common pinmux definitions in da850.dtsi so
> >> each board doe not have to repeat them. But AEMIF is an exception as its
> >> usage can really be varied (NAND, NOR, SRAM, other). Plus, different
> >> boards are likely to use different chip selects so coming up with some
> >> pinmux definitions which will be reused widely is really unlikely.
> >>
> > This is exactly what I just did for the LCDK.
> > If everybody is happy with it I will do the same for the evm as I put it
> > in the cover letter.
> 
> Yes please. We dont want duplication of data between da850.dtsi and
> da850-lcdk.dts files.
> 
Then I'll wait for this series to be applied and then apply my changes
to the EVM while retiring the nand_cs3 together.
 
Karl

Re: [PATCH 2/4] ARM: dts: da850: Add an aemif node

[Sekhar Nori]

On Wednesday 10 August 2016 01:56 PM, Karl Beldan wrote:
> On Wed, Aug 10, 2016 at 01:42:01PM +0530, Sekhar Nori wrote:
>> On Wednesday 10 August 2016 01:37 PM, Karl Beldan wrote:
>>> On Wed, Aug 10, 2016 at 01:32:03PM +0530, Sekhar Nori wrote:
>>>> On Wednesday 10 August 2016 01:18 PM, Sekhar Nori wrote:
>>>>> On Tuesday 09 August 2016 10:45 PM, 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.
>>>>>> Also it will allow to properly pass the emif timings via DT.
>>>>>>
>>>>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
>>>>>> ---
>>>>>>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
>>>>>>  1 file changed, 10 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>>>>>> index bc10e7e..f62928c 100644
>>>>>> --- a/arch/arm/boot/dts/da850.dtsi
>>>>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>>>>> @@ -411,6 +411,16 @@
>>>>>>  			dma-names = "tx", "rx";
>>>>>>  		};
>>>>>>  	};
>>>>>> +	aemif: aemif@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";
>>>>>> +	};
>>>>>>  	nand_cs3@62000000 {
>>>>>>  		compatible = "ti,davinci-nand";
>>>>>>  		reg = <0x62000000 0x807ff
>>>>>
>>>>> The nand node should be part of aemif node like it is done for keystone
>>>>> boards.
>>>>
>>>> Actually, can you move the nand node out of da850.dtsi completely. Its
>>>> much better to keep da850.dtsi restricted to soc-internal devices and
>>>> keep the board level devices like NAND flash in <board>.dts file.
>>>>
>>>> Similarly, can you move the NAND pinmux definitions too to the
>>>> da850-evm.dts file?
>>>>
>>>> There is advantage in keeping common pinmux definitions in da850.dtsi so
>>>> each board doe not have to repeat them. But AEMIF is an exception as its
>>>> usage can really be varied (NAND, NOR, SRAM, other). Plus, different
>>>> boards are likely to use different chip selects so coming up with some
>>>> pinmux definitions which will be reused widely is really unlikely.
>>>>
>>> This is exactly what I just did for the LCDK.
>>> If everybody is happy with it I will do the same for the evm as I put it
>>> in the cover letter.
>>
>> Yes please. We dont want duplication of data between da850.dtsi and
>> da850-lcdk.dts files.
>>
> Then I'll wait for this series to be applied and then apply my changes
> to the EVM while retiring the nand_cs3 together.

No, I prefer the fixup happens first. In the same series, you can first
fixup existing EVM and then add LCDK support.

Regards,
Sekhar

Re: [PATCH 2/4] ARM: dts: da850: Add an aemif node

[Sekhar Nori]

On Wednesday 10 August 2016 01:56 PM, Karl Beldan wrote:
> On Wed, Aug 10, 2016 at 01:42:01PM +0530, Sekhar Nori wrote:
>> On Wednesday 10 August 2016 01:37 PM, Karl Beldan wrote:
>>> On Wed, Aug 10, 2016 at 01:32:03PM +0530, Sekhar Nori wrote:
>>>> On Wednesday 10 August 2016 01:18 PM, Sekhar Nori wrote:
>>>>> On Tuesday 09 August 2016 10:45 PM, 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.
>>>>>> Also it will allow to properly pass the emif timings via DT.
>>>>>>
>>>>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
>>>>>> ---
>>>>>>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
>>>>>>  1 file changed, 10 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>>>>>> index bc10e7e..f62928c 100644
>>>>>> --- a/arch/arm/boot/dts/da850.dtsi
>>>>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>>>>> @@ -411,6 +411,16 @@
>>>>>>  			dma-names = "tx", "rx";
>>>>>>  		};
>>>>>>  	};
>>>>>> +	aemif: aemif@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";
>>>>>> +	};
>>>>>>  	nand_cs3@62000000 {
>>>>>>  		compatible = "ti,davinci-nand";
>>>>>>  		reg = <0x62000000 0x807ff
>>>>>
>>>>> The nand node should be part of aemif node like it is done for keystone
>>>>> boards.
>>>>
>>>> Actually, can you move the nand node out of da850.dtsi completely. Its
>>>> much better to keep da850.dtsi restricted to soc-internal devices and
>>>> keep the board level devices like NAND flash in <board>.dts file.
>>>>
>>>> Similarly, can you move the NAND pinmux definitions too to the
>>>> da850-evm.dts file?
>>>>
>>>> There is advantage in keeping common pinmux definitions in da850.dtsi so
>>>> each board doe not have to repeat them. But AEMIF is an exception as its
>>>> usage can really be varied (NAND, NOR, SRAM, other). Plus, different
>>>> boards are likely to use different chip selects so coming up with some
>>>> pinmux definitions which will be reused widely is really unlikely.
>>>>
>>> This is exactly what I just did for the LCDK.
>>> If everybody is happy with it I will do the same for the evm as I put it
>>> in the cover letter.
>>
>> Yes please. We dont want duplication of data between da850.dtsi and
>> da850-lcdk.dts files.
>>
> Then I'll wait for this series to be applied and then apply my changes
> to the EVM while retiring the nand_cs3 together.

No, I prefer the fixup happens first. In the same series, you can first
fixup existing EVM and then add LCDK support.

Regards,
Sekhar

[PATCH 2/4] ARM: dts: da850: Add an aemif node

[Sekhar Nori]

On Wednesday 10 August 2016 01:56 PM, Karl Beldan wrote:
> On Wed, Aug 10, 2016 at 01:42:01PM +0530, Sekhar Nori wrote:
>> On Wednesday 10 August 2016 01:37 PM, Karl Beldan wrote:
>>> On Wed, Aug 10, 2016 at 01:32:03PM +0530, Sekhar Nori wrote:
>>>> On Wednesday 10 August 2016 01:18 PM, Sekhar Nori wrote:
>>>>> On Tuesday 09 August 2016 10:45 PM, 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.
>>>>>> Also it will allow to properly pass the emif timings via DT.
>>>>>>
>>>>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
>>>>>> ---
>>>>>>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
>>>>>>  1 file changed, 10 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>>>>>> index bc10e7e..f62928c 100644
>>>>>> --- a/arch/arm/boot/dts/da850.dtsi
>>>>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>>>>> @@ -411,6 +411,16 @@
>>>>>>  			dma-names = "tx", "rx";
>>>>>>  		};
>>>>>>  	};
>>>>>> +	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";
>>>>>> +	};
>>>>>>  	nand_cs3 at 62000000 {
>>>>>>  		compatible = "ti,davinci-nand";
>>>>>>  		reg = <0x62000000 0x807ff
>>>>>
>>>>> The nand node should be part of aemif node like it is done for keystone
>>>>> boards.
>>>>
>>>> Actually, can you move the nand node out of da850.dtsi completely. Its
>>>> much better to keep da850.dtsi restricted to soc-internal devices and
>>>> keep the board level devices like NAND flash in <board>.dts file.
>>>>
>>>> Similarly, can you move the NAND pinmux definitions too to the
>>>> da850-evm.dts file?
>>>>
>>>> There is advantage in keeping common pinmux definitions in da850.dtsi so
>>>> each board doe not have to repeat them. But AEMIF is an exception as its
>>>> usage can really be varied (NAND, NOR, SRAM, other). Plus, different
>>>> boards are likely to use different chip selects so coming up with some
>>>> pinmux definitions which will be reused widely is really unlikely.
>>>>
>>> This is exactly what I just did for the LCDK.
>>> If everybody is happy with it I will do the same for the evm as I put it
>>> in the cover letter.
>>
>> Yes please. We dont want duplication of data between da850.dtsi and
>> da850-lcdk.dts files.
>>
> Then I'll wait for this series to be applied and then apply my changes
> to the EVM while retiring the nand_cs3 together.

No, I prefer the fixup happens first. In the same series, you can first
fixup existing EVM and then add LCDK support.

Regards,
Sekhar

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

[Sekhar Nori]

On Tuesday 09 August 2016 10:45 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.

I disagree that we need to be compatible to the software that ships with
the board. Thats software was last updated 3 years ago. Instead I would
concern with what the hardware supports. So, if the hardware can support
4-bit ECC, I would use that.

If driver is broken for 4-bit ECC, please fix that up first.

> This passed elementary tests hashing a 20MB file on top of ubifs on my
> LCDK.
> 
> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>

Thanks,
Sekhar

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

[Sekhar Nori]

On Tuesday 09 August 2016 10:45 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.

I disagree that we need to be compatible to the software that ships with
the board. Thats software was last updated 3 years ago. Instead I would
concern with what the hardware supports. So, if the hardware can support
4-bit ECC, I would use that.

If driver is broken for 4-bit ECC, please fix that up first.

> This passed elementary tests hashing a 20MB file on top of ubifs on my
> LCDK.
> 
> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>

Thanks,
Sekhar

Re: [PATCH 2/4] ARM: dts: da850: Add an aemif node

[Karl Beldan]

On Wed, Aug 10, 2016 at 01:59:26PM +0530, Sekhar Nori wrote:
> On Wednesday 10 August 2016 01:56 PM, Karl Beldan wrote:
> > On Wed, Aug 10, 2016 at 01:42:01PM +0530, Sekhar Nori wrote:
> >> On Wednesday 10 August 2016 01:37 PM, Karl Beldan wrote:
> >>> On Wed, Aug 10, 2016 at 01:32:03PM +0530, Sekhar Nori wrote:
> >>>> On Wednesday 10 August 2016 01:18 PM, Sekhar Nori wrote:
> >>>>> On Tuesday 09 August 2016 10:45 PM, 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.
> >>>>>> Also it will allow to properly pass the emif timings via DT.
> >>>>>>
> >>>>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> >>>>>> ---
> >>>>>>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
> >>>>>>  1 file changed, 10 insertions(+)
> >>>>>>
> >>>>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> >>>>>> index bc10e7e..f62928c 100644
> >>>>>> --- a/arch/arm/boot/dts/da850.dtsi
> >>>>>> +++ b/arch/arm/boot/dts/da850.dtsi
> >>>>>> @@ -411,6 +411,16 @@
> >>>>>>  			dma-names = "tx", "rx";
> >>>>>>  		};
> >>>>>>  	};
> >>>>>> +	aemif: aemif@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";
> >>>>>> +	};
> >>>>>>  	nand_cs3@62000000 {
> >>>>>>  		compatible = "ti,davinci-nand";
> >>>>>>  		reg = <0x62000000 0x807ff
> >>>>>
> >>>>> The nand node should be part of aemif node like it is done for keystone
> >>>>> boards.
> >>>>
> >>>> Actually, can you move the nand node out of da850.dtsi completely. Its
> >>>> much better to keep da850.dtsi restricted to soc-internal devices and
> >>>> keep the board level devices like NAND flash in <board>.dts file.
> >>>>
> >>>> Similarly, can you move the NAND pinmux definitions too to the
> >>>> da850-evm.dts file?
> >>>>
> >>>> There is advantage in keeping common pinmux definitions in da850.dtsi so
> >>>> each board doe not have to repeat them. But AEMIF is an exception as its
> >>>> usage can really be varied (NAND, NOR, SRAM, other). Plus, different
> >>>> boards are likely to use different chip selects so coming up with some
> >>>> pinmux definitions which will be reused widely is really unlikely.
> >>>>
> >>> This is exactly what I just did for the LCDK.
> >>> If everybody is happy with it I will do the same for the evm as I put it
> >>> in the cover letter.
> >>
> >> Yes please. We dont want duplication of data between da850.dtsi and
> >> da850-lcdk.dts files.
> >>
> > Then I'll wait for this series to be applied and then apply my changes
> > to the EVM while retiring the nand_cs3 together.
> 
> No, I prefer the fixup happens first. In the same series, you can first
> fixup existing EVM and then add LCDK support.
> 

Well in that case you'll have to do the testing since I only have an
LCDK. I should be able to send the series within the hour.
 
Karl

Re: [PATCH 2/4] ARM: dts: da850: Add an aemif node

[Karl Beldan]

On Wed, Aug 10, 2016 at 01:59:26PM +0530, Sekhar Nori wrote:
> On Wednesday 10 August 2016 01:56 PM, Karl Beldan wrote:
> > On Wed, Aug 10, 2016 at 01:42:01PM +0530, Sekhar Nori wrote:
> >> On Wednesday 10 August 2016 01:37 PM, Karl Beldan wrote:
> >>> On Wed, Aug 10, 2016 at 01:32:03PM +0530, Sekhar Nori wrote:
> >>>> On Wednesday 10 August 2016 01:18 PM, Sekhar Nori wrote:
> >>>>> On Tuesday 09 August 2016 10:45 PM, 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.
> >>>>>> Also it will allow to properly pass the emif timings via DT.
> >>>>>>
> >>>>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> >>>>>> ---
> >>>>>>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
> >>>>>>  1 file changed, 10 insertions(+)
> >>>>>>
> >>>>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> >>>>>> index bc10e7e..f62928c 100644
> >>>>>> --- a/arch/arm/boot/dts/da850.dtsi
> >>>>>> +++ b/arch/arm/boot/dts/da850.dtsi
> >>>>>> @@ -411,6 +411,16 @@
> >>>>>>  			dma-names = "tx", "rx";
> >>>>>>  		};
> >>>>>>  	};
> >>>>>> +	aemif: aemif@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";
> >>>>>> +	};
> >>>>>>  	nand_cs3@62000000 {
> >>>>>>  		compatible = "ti,davinci-nand";
> >>>>>>  		reg = <0x62000000 0x807ff
> >>>>>
> >>>>> The nand node should be part of aemif node like it is done for keystone
> >>>>> boards.
> >>>>
> >>>> Actually, can you move the nand node out of da850.dtsi completely. Its
> >>>> much better to keep da850.dtsi restricted to soc-internal devices and
> >>>> keep the board level devices like NAND flash in <board>.dts file.
> >>>>
> >>>> Similarly, can you move the NAND pinmux definitions too to the
> >>>> da850-evm.dts file?
> >>>>
> >>>> There is advantage in keeping common pinmux definitions in da850.dtsi so
> >>>> each board doe not have to repeat them. But AEMIF is an exception as its
> >>>> usage can really be varied (NAND, NOR, SRAM, other). Plus, different
> >>>> boards are likely to use different chip selects so coming up with some
> >>>> pinmux definitions which will be reused widely is really unlikely.
> >>>>
> >>> This is exactly what I just did for the LCDK.
> >>> If everybody is happy with it I will do the same for the evm as I put it
> >>> in the cover letter.
> >>
> >> Yes please. We dont want duplication of data between da850.dtsi and
> >> da850-lcdk.dts files.
> >>
> > Then I'll wait for this series to be applied and then apply my changes
> > to the EVM while retiring the nand_cs3 together.
> 
> No, I prefer the fixup happens first. In the same series, you can first
> fixup existing EVM and then add LCDK support.
> 

Well in that case you'll have to do the testing since I only have an
LCDK. I should be able to send the series within the hour.
 
Karl

[PATCH 2/4] ARM: dts: da850: Add an aemif node

[Karl Beldan]

On Wed, Aug 10, 2016 at 01:59:26PM +0530, Sekhar Nori wrote:
> On Wednesday 10 August 2016 01:56 PM, Karl Beldan wrote:
> > On Wed, Aug 10, 2016 at 01:42:01PM +0530, Sekhar Nori wrote:
> >> On Wednesday 10 August 2016 01:37 PM, Karl Beldan wrote:
> >>> On Wed, Aug 10, 2016 at 01:32:03PM +0530, Sekhar Nori wrote:
> >>>> On Wednesday 10 August 2016 01:18 PM, Sekhar Nori wrote:
> >>>>> On Tuesday 09 August 2016 10:45 PM, 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.
> >>>>>> Also it will allow to properly pass the emif timings via DT.
> >>>>>>
> >>>>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> >>>>>> ---
> >>>>>>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
> >>>>>>  1 file changed, 10 insertions(+)
> >>>>>>
> >>>>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> >>>>>> index bc10e7e..f62928c 100644
> >>>>>> --- a/arch/arm/boot/dts/da850.dtsi
> >>>>>> +++ b/arch/arm/boot/dts/da850.dtsi
> >>>>>> @@ -411,6 +411,16 @@
> >>>>>>  			dma-names = "tx", "rx";
> >>>>>>  		};
> >>>>>>  	};
> >>>>>> +	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";
> >>>>>> +	};
> >>>>>>  	nand_cs3 at 62000000 {
> >>>>>>  		compatible = "ti,davinci-nand";
> >>>>>>  		reg = <0x62000000 0x807ff
> >>>>>
> >>>>> The nand node should be part of aemif node like it is done for keystone
> >>>>> boards.
> >>>>
> >>>> Actually, can you move the nand node out of da850.dtsi completely. Its
> >>>> much better to keep da850.dtsi restricted to soc-internal devices and
> >>>> keep the board level devices like NAND flash in <board>.dts file.
> >>>>
> >>>> Similarly, can you move the NAND pinmux definitions too to the
> >>>> da850-evm.dts file?
> >>>>
> >>>> There is advantage in keeping common pinmux definitions in da850.dtsi so
> >>>> each board doe not have to repeat them. But AEMIF is an exception as its
> >>>> usage can really be varied (NAND, NOR, SRAM, other). Plus, different
> >>>> boards are likely to use different chip selects so coming up with some
> >>>> pinmux definitions which will be reused widely is really unlikely.
> >>>>
> >>> This is exactly what I just did for the LCDK.
> >>> If everybody is happy with it I will do the same for the evm as I put it
> >>> in the cover letter.
> >>
> >> Yes please. We dont want duplication of data between da850.dtsi and
> >> da850-lcdk.dts files.
> >>
> > Then I'll wait for this series to be applied and then apply my changes
> > to the EVM while retiring the nand_cs3 together.
> 
> No, I prefer the fixup happens first. In the same series, you can first
> fixup existing EVM and then add LCDK support.
> 

Well in that case you'll have to do the testing since I only have an
LCDK. I should be able to send the series within the hour.
 
Karl

Re: [PATCH 2/4] ARM: dts: da850: Add an aemif node

[Sekhar Nori]

On Wednesday 10 August 2016 02:04 PM, Karl Beldan wrote:
> On Wed, Aug 10, 2016 at 01:59:26PM +0530, Sekhar Nori wrote:
>> On Wednesday 10 August 2016 01:56 PM, Karl Beldan wrote:
>>> On Wed, Aug 10, 2016 at 01:42:01PM +0530, Sekhar Nori wrote:
>>>> On Wednesday 10 August 2016 01:37 PM, Karl Beldan wrote:
>>>>> On Wed, Aug 10, 2016 at 01:32:03PM +0530, Sekhar Nori wrote:
>>>>>> On Wednesday 10 August 2016 01:18 PM, Sekhar Nori wrote:
>>>>>>> On Tuesday 09 August 2016 10:45 PM, 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.
>>>>>>>> Also it will allow to properly pass the emif timings via DT.
>>>>>>>>
>>>>>>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
>>>>>>>> ---
>>>>>>>>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
>>>>>>>>  1 file changed, 10 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>>>>>>>> index bc10e7e..f62928c 100644
>>>>>>>> --- a/arch/arm/boot/dts/da850.dtsi
>>>>>>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>>>>>>> @@ -411,6 +411,16 @@
>>>>>>>>  			dma-names = "tx", "rx";
>>>>>>>>  		};
>>>>>>>>  	};
>>>>>>>> +	aemif: aemif@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";
>>>>>>>> +	};
>>>>>>>>  	nand_cs3@62000000 {
>>>>>>>>  		compatible = "ti,davinci-nand";
>>>>>>>>  		reg = <0x62000000 0x807ff
>>>>>>>
>>>>>>> The nand node should be part of aemif node like it is done for keystone
>>>>>>> boards.
>>>>>>
>>>>>> Actually, can you move the nand node out of da850.dtsi completely. Its
>>>>>> much better to keep da850.dtsi restricted to soc-internal devices and
>>>>>> keep the board level devices like NAND flash in <board>.dts file.
>>>>>>
>>>>>> Similarly, can you move the NAND pinmux definitions too to the
>>>>>> da850-evm.dts file?
>>>>>>
>>>>>> There is advantage in keeping common pinmux definitions in da850.dtsi so
>>>>>> each board doe not have to repeat them. But AEMIF is an exception as its
>>>>>> usage can really be varied (NAND, NOR, SRAM, other). Plus, different
>>>>>> boards are likely to use different chip selects so coming up with some
>>>>>> pinmux definitions which will be reused widely is really unlikely.
>>>>>>
>>>>> This is exactly what I just did for the LCDK.
>>>>> If everybody is happy with it I will do the same for the evm as I put it
>>>>> in the cover letter.
>>>>
>>>> Yes please. We dont want duplication of data between da850.dtsi and
>>>> da850-lcdk.dts files.
>>>>
>>> Then I'll wait for this series to be applied and then apply my changes
>>> to the EVM while retiring the nand_cs3 together.
>>
>> No, I prefer the fixup happens first. In the same series, you can first
>> fixup existing EVM and then add LCDK support.
>>
> 
> Well in that case you'll have to do the testing since I only have an
> LCDK. I should be able to send the series within the hour.

Sure. I can test it.

Regards,
Sekhar

Re: [PATCH 2/4] ARM: dts: da850: Add an aemif node

[Sekhar Nori]

On Wednesday 10 August 2016 02:04 PM, Karl Beldan wrote:
> On Wed, Aug 10, 2016 at 01:59:26PM +0530, Sekhar Nori wrote:
>> On Wednesday 10 August 2016 01:56 PM, Karl Beldan wrote:
>>> On Wed, Aug 10, 2016 at 01:42:01PM +0530, Sekhar Nori wrote:
>>>> On Wednesday 10 August 2016 01:37 PM, Karl Beldan wrote:
>>>>> On Wed, Aug 10, 2016 at 01:32:03PM +0530, Sekhar Nori wrote:
>>>>>> On Wednesday 10 August 2016 01:18 PM, Sekhar Nori wrote:
>>>>>>> On Tuesday 09 August 2016 10:45 PM, 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.
>>>>>>>> Also it will allow to properly pass the emif timings via DT.
>>>>>>>>
>>>>>>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
>>>>>>>> ---
>>>>>>>>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
>>>>>>>>  1 file changed, 10 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>>>>>>>> index bc10e7e..f62928c 100644
>>>>>>>> --- a/arch/arm/boot/dts/da850.dtsi
>>>>>>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>>>>>>> @@ -411,6 +411,16 @@
>>>>>>>>  			dma-names = "tx", "rx";
>>>>>>>>  		};
>>>>>>>>  	};
>>>>>>>> +	aemif: aemif@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";
>>>>>>>> +	};
>>>>>>>>  	nand_cs3@62000000 {
>>>>>>>>  		compatible = "ti,davinci-nand";
>>>>>>>>  		reg = <0x62000000 0x807ff
>>>>>>>
>>>>>>> The nand node should be part of aemif node like it is done for keystone
>>>>>>> boards.
>>>>>>
>>>>>> Actually, can you move the nand node out of da850.dtsi completely. Its
>>>>>> much better to keep da850.dtsi restricted to soc-internal devices and
>>>>>> keep the board level devices like NAND flash in <board>.dts file.
>>>>>>
>>>>>> Similarly, can you move the NAND pinmux definitions too to the
>>>>>> da850-evm.dts file?
>>>>>>
>>>>>> There is advantage in keeping common pinmux definitions in da850.dtsi so
>>>>>> each board doe not have to repeat them. But AEMIF is an exception as its
>>>>>> usage can really be varied (NAND, NOR, SRAM, other). Plus, different
>>>>>> boards are likely to use different chip selects so coming up with some
>>>>>> pinmux definitions which will be reused widely is really unlikely.
>>>>>>
>>>>> This is exactly what I just did for the LCDK.
>>>>> If everybody is happy with it I will do the same for the evm as I put it
>>>>> in the cover letter.
>>>>
>>>> Yes please. We dont want duplication of data between da850.dtsi and
>>>> da850-lcdk.dts files.
>>>>
>>> Then I'll wait for this series to be applied and then apply my changes
>>> to the EVM while retiring the nand_cs3 together.
>>
>> No, I prefer the fixup happens first. In the same series, you can first
>> fixup existing EVM and then add LCDK support.
>>
> 
> Well in that case you'll have to do the testing since I only have an
> LCDK. I should be able to send the series within the hour.

Sure. I can test it.

Regards,
Sekhar

[PATCH 2/4] ARM: dts: da850: Add an aemif node

[Sekhar Nori]

On Wednesday 10 August 2016 02:04 PM, Karl Beldan wrote:
> On Wed, Aug 10, 2016 at 01:59:26PM +0530, Sekhar Nori wrote:
>> On Wednesday 10 August 2016 01:56 PM, Karl Beldan wrote:
>>> On Wed, Aug 10, 2016 at 01:42:01PM +0530, Sekhar Nori wrote:
>>>> On Wednesday 10 August 2016 01:37 PM, Karl Beldan wrote:
>>>>> On Wed, Aug 10, 2016 at 01:32:03PM +0530, Sekhar Nori wrote:
>>>>>> On Wednesday 10 August 2016 01:18 PM, Sekhar Nori wrote:
>>>>>>> On Tuesday 09 August 2016 10:45 PM, 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.
>>>>>>>> Also it will allow to properly pass the emif timings via DT.
>>>>>>>>
>>>>>>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
>>>>>>>> ---
>>>>>>>>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
>>>>>>>>  1 file changed, 10 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>>>>>>>> index bc10e7e..f62928c 100644
>>>>>>>> --- a/arch/arm/boot/dts/da850.dtsi
>>>>>>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>>>>>>> @@ -411,6 +411,16 @@
>>>>>>>>  			dma-names = "tx", "rx";
>>>>>>>>  		};
>>>>>>>>  	};
>>>>>>>> +	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";
>>>>>>>> +	};
>>>>>>>>  	nand_cs3 at 62000000 {
>>>>>>>>  		compatible = "ti,davinci-nand";
>>>>>>>>  		reg = <0x62000000 0x807ff
>>>>>>>
>>>>>>> The nand node should be part of aemif node like it is done for keystone
>>>>>>> boards.
>>>>>>
>>>>>> Actually, can you move the nand node out of da850.dtsi completely. Its
>>>>>> much better to keep da850.dtsi restricted to soc-internal devices and
>>>>>> keep the board level devices like NAND flash in <board>.dts file.
>>>>>>
>>>>>> Similarly, can you move the NAND pinmux definitions too to the
>>>>>> da850-evm.dts file?
>>>>>>
>>>>>> There is advantage in keeping common pinmux definitions in da850.dtsi so
>>>>>> each board doe not have to repeat them. But AEMIF is an exception as its
>>>>>> usage can really be varied (NAND, NOR, SRAM, other). Plus, different
>>>>>> boards are likely to use different chip selects so coming up with some
>>>>>> pinmux definitions which will be reused widely is really unlikely.
>>>>>>
>>>>> This is exactly what I just did for the LCDK.
>>>>> If everybody is happy with it I will do the same for the evm as I put it
>>>>> in the cover letter.
>>>>
>>>> Yes please. We dont want duplication of data between da850.dtsi and
>>>> da850-lcdk.dts files.
>>>>
>>> Then I'll wait for this series to be applied and then apply my changes
>>> to the EVM while retiring the nand_cs3 together.
>>
>> No, I prefer the fixup happens first. In the same series, you can first
>> fixup existing EVM and then add LCDK support.
>>
> 
> Well in that case you'll have to do the testing since I only have an
> LCDK. I should be able to send the series within the hour.

Sure. I can test it.

Regards,
Sekhar

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

[Karl Beldan]

On Wed, Aug 10, 2016 at 02:01:57PM +0530, Sekhar Nori wrote:
> On Tuesday 09 August 2016 10:45 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.
> 
> I disagree that we need to be compatible to the software that ships with
> the board. Thats software was last updated 3 years ago. Instead I would
> concern with what the hardware supports. So, if the hardware can support
> 4-bit ECC, I would use that.
> 
I am not saying we _need_ to be compatible.

> If driver is broken for 4-bit ECC, please fix that up first.
> 
Since this issue is completely separate from my DT improvements
I'll stick to resubmitting the series, applying my LCDK changes to the
EVM too, besides you'll be able to compare the behavior without ECC
discrepancies.
I took note that you are likely to not apply without the ECC fix.
 
Karl

> > This passed elementary tests hashing a 20MB file on top of ubifs on my
> > LCDK.
> > 
> > Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> 
> Thanks,
> Sekhar

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

[Karl Beldan]

On Wed, Aug 10, 2016 at 02:01:57PM +0530, Sekhar Nori wrote:
> On Tuesday 09 August 2016 10:45 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.
> 
> I disagree that we need to be compatible to the software that ships with
> the board. Thats software was last updated 3 years ago. Instead I would
> concern with what the hardware supports. So, if the hardware can support
> 4-bit ECC, I would use that.
> 
I am not saying we _need_ to be compatible.

> If driver is broken for 4-bit ECC, please fix that up first.
> 
Since this issue is completely separate from my DT improvements
I'll stick to resubmitting the series, applying my LCDK changes to the
EVM too, besides you'll be able to compare the behavior without ECC
discrepancies.
I took note that you are likely to not apply without the ECC fix.
 
Karl

> > This passed elementary tests hashing a 20MB file on top of ubifs on my
> > LCDK.
> > 
> > Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> 
> Thanks,
> Sekhar

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

[Karl Beldan]

On Wed, Aug 10, 2016 at 02:01:57PM +0530, Sekhar Nori wrote:
> On Tuesday 09 August 2016 10:45 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.
> 
> I disagree that we need to be compatible to the software that ships with
> the board. Thats software was last updated 3 years ago. Instead I would
> concern with what the hardware supports. So, if the hardware can support
> 4-bit ECC, I would use that.
> 
I am not saying we _need_ to be compatible.

> If driver is broken for 4-bit ECC, please fix that up first.
> 
Since this issue is completely separate from my DT improvements
I'll stick to resubmitting the series, applying my LCDK changes to the
EVM too, besides you'll be able to compare the behavior without ECC
discrepancies.
I took note that you are likely to not apply without the ECC fix.
 
Karl

> > This passed elementary tests hashing a 20MB file on top of ubifs on my
> > LCDK.
> > 
> > Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> 
> Thanks,
> Sekhar

Re: [PATCH 2/4] ARM: dts: da850: Add an aemif node

[Karl Beldan]

On Wed, Aug 10, 2016 at 02:04:48PM +0530, Sekhar Nori wrote:
> On Wednesday 10 August 2016 02:04 PM, Karl Beldan wrote:
> > On Wed, Aug 10, 2016 at 01:59:26PM +0530, Sekhar Nori wrote:
> >> On Wednesday 10 August 2016 01:56 PM, Karl Beldan wrote:
> >>> On Wed, Aug 10, 2016 at 01:42:01PM +0530, Sekhar Nori wrote:
> >>>> On Wednesday 10 August 2016 01:37 PM, Karl Beldan wrote:
> >>>>> On Wed, Aug 10, 2016 at 01:32:03PM +0530, Sekhar Nori wrote:
> >>>>>> On Wednesday 10 August 2016 01:18 PM, Sekhar Nori wrote:
> >>>>>>> On Tuesday 09 August 2016 10:45 PM, 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.
> >>>>>>>> Also it will allow to properly pass the emif timings via DT.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> >>>>>>>> ---
> >>>>>>>>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
> >>>>>>>>  1 file changed, 10 insertions(+)
> >>>>>>>>
> >>>>>>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> >>>>>>>> index bc10e7e..f62928c 100644
> >>>>>>>> --- a/arch/arm/boot/dts/da850.dtsi
> >>>>>>>> +++ b/arch/arm/boot/dts/da850.dtsi
> >>>>>>>> @@ -411,6 +411,16 @@
> >>>>>>>>  			dma-names = "tx", "rx";
> >>>>>>>>  		};
> >>>>>>>>  	};
> >>>>>>>> +	aemif: aemif@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";
> >>>>>>>> +	};
> >>>>>>>>  	nand_cs3@62000000 {
> >>>>>>>>  		compatible = "ti,davinci-nand";
> >>>>>>>>  		reg = <0x62000000 0x807ff
> >>>>>>>
> >>>>>>> The nand node should be part of aemif node like it is done for keystone
> >>>>>>> boards.
> >>>>>>
> >>>>>> Actually, can you move the nand node out of da850.dtsi completely. Its
> >>>>>> much better to keep da850.dtsi restricted to soc-internal devices and
> >>>>>> keep the board level devices like NAND flash in <board>.dts file.
> >>>>>>
> >>>>>> Similarly, can you move the NAND pinmux definitions too to the
> >>>>>> da850-evm.dts file?
> >>>>>>
> >>>>>> There is advantage in keeping common pinmux definitions in da850.dtsi so
> >>>>>> each board doe not have to repeat them. But AEMIF is an exception as its
> >>>>>> usage can really be varied (NAND, NOR, SRAM, other). Plus, different
> >>>>>> boards are likely to use different chip selects so coming up with some
> >>>>>> pinmux definitions which will be reused widely is really unlikely.
> >>>>>>
> >>>>> This is exactly what I just did for the LCDK.
> >>>>> If everybody is happy with it I will do the same for the evm as I put it
> >>>>> in the cover letter.
> >>>>
> >>>> Yes please. We dont want duplication of data between da850.dtsi and
> >>>> da850-lcdk.dts files.
> >>>>
> >>> Then I'll wait for this series to be applied and then apply my changes
> >>> to the EVM while retiring the nand_cs3 together.
> >>
> >> No, I prefer the fixup happens first. In the same series, you can first
> >> fixup existing EVM and then add LCDK support.
> >>
> > 
> > Well in that case you'll have to do the testing since I only have an
> > LCDK. I should be able to send the series within the hour.
> 
> Sure. I can test it.
> 
The aemif/davinci_nand drivers don't configure AWCCR, yet davinci_nand
relies on EM_WAIT for RDY/nBUSY, so for the moment I keep the default
settings, but I configure the EM_WAIT pins in the pinctrl. I did it for
the LCDK, and it is not done for the EVM. Since the EVM schematics are
not public can you tell which EM_WAIT pins are connected ?
 
Karl

Re: [PATCH 2/4] ARM: dts: da850: Add an aemif node

[Karl Beldan]

On Wed, Aug 10, 2016 at 02:04:48PM +0530, Sekhar Nori wrote:
> On Wednesday 10 August 2016 02:04 PM, Karl Beldan wrote:
> > On Wed, Aug 10, 2016 at 01:59:26PM +0530, Sekhar Nori wrote:
> >> On Wednesday 10 August 2016 01:56 PM, Karl Beldan wrote:
> >>> On Wed, Aug 10, 2016 at 01:42:01PM +0530, Sekhar Nori wrote:
> >>>> On Wednesday 10 August 2016 01:37 PM, Karl Beldan wrote:
> >>>>> On Wed, Aug 10, 2016 at 01:32:03PM +0530, Sekhar Nori wrote:
> >>>>>> On Wednesday 10 August 2016 01:18 PM, Sekhar Nori wrote:
> >>>>>>> On Tuesday 09 August 2016 10:45 PM, 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.
> >>>>>>>> Also it will allow to properly pass the emif timings via DT.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> >>>>>>>> ---
> >>>>>>>>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
> >>>>>>>>  1 file changed, 10 insertions(+)
> >>>>>>>>
> >>>>>>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> >>>>>>>> index bc10e7e..f62928c 100644
> >>>>>>>> --- a/arch/arm/boot/dts/da850.dtsi
> >>>>>>>> +++ b/arch/arm/boot/dts/da850.dtsi
> >>>>>>>> @@ -411,6 +411,16 @@
> >>>>>>>>  			dma-names = "tx", "rx";
> >>>>>>>>  		};
> >>>>>>>>  	};
> >>>>>>>> +	aemif: aemif@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";
> >>>>>>>> +	};
> >>>>>>>>  	nand_cs3@62000000 {
> >>>>>>>>  		compatible = "ti,davinci-nand";
> >>>>>>>>  		reg = <0x62000000 0x807ff
> >>>>>>>
> >>>>>>> The nand node should be part of aemif node like it is done for keystone
> >>>>>>> boards.
> >>>>>>
> >>>>>> Actually, can you move the nand node out of da850.dtsi completely. Its
> >>>>>> much better to keep da850.dtsi restricted to soc-internal devices and
> >>>>>> keep the board level devices like NAND flash in <board>.dts file.
> >>>>>>
> >>>>>> Similarly, can you move the NAND pinmux definitions too to the
> >>>>>> da850-evm.dts file?
> >>>>>>
> >>>>>> There is advantage in keeping common pinmux definitions in da850.dtsi so
> >>>>>> each board doe not have to repeat them. But AEMIF is an exception as its
> >>>>>> usage can really be varied (NAND, NOR, SRAM, other). Plus, different
> >>>>>> boards are likely to use different chip selects so coming up with some
> >>>>>> pinmux definitions which will be reused widely is really unlikely.
> >>>>>>
> >>>>> This is exactly what I just did for the LCDK.
> >>>>> If everybody is happy with it I will do the same for the evm as I put it
> >>>>> in the cover letter.
> >>>>
> >>>> Yes please. We dont want duplication of data between da850.dtsi and
> >>>> da850-lcdk.dts files.
> >>>>
> >>> Then I'll wait for this series to be applied and then apply my changes
> >>> to the EVM while retiring the nand_cs3 together.
> >>
> >> No, I prefer the fixup happens first. In the same series, you can first
> >> fixup existing EVM and then add LCDK support.
> >>
> > 
> > Well in that case you'll have to do the testing since I only have an
> > LCDK. I should be able to send the series within the hour.
> 
> Sure. I can test it.
> 
The aemif/davinci_nand drivers don't configure AWCCR, yet davinci_nand
relies on EM_WAIT for RDY/nBUSY, so for the moment I keep the default
settings, but I configure the EM_WAIT pins in the pinctrl. I did it for
the LCDK, and it is not done for the EVM. Since the EVM schematics are
not public can you tell which EM_WAIT pins are connected ?
 
Karl

[PATCH 2/4] ARM: dts: da850: Add an aemif node

[Karl Beldan]

On Wed, Aug 10, 2016 at 02:04:48PM +0530, Sekhar Nori wrote:
> On Wednesday 10 August 2016 02:04 PM, Karl Beldan wrote:
> > On Wed, Aug 10, 2016 at 01:59:26PM +0530, Sekhar Nori wrote:
> >> On Wednesday 10 August 2016 01:56 PM, Karl Beldan wrote:
> >>> On Wed, Aug 10, 2016 at 01:42:01PM +0530, Sekhar Nori wrote:
> >>>> On Wednesday 10 August 2016 01:37 PM, Karl Beldan wrote:
> >>>>> On Wed, Aug 10, 2016 at 01:32:03PM +0530, Sekhar Nori wrote:
> >>>>>> On Wednesday 10 August 2016 01:18 PM, Sekhar Nori wrote:
> >>>>>>> On Tuesday 09 August 2016 10:45 PM, 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.
> >>>>>>>> Also it will allow to properly pass the emif timings via DT.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> >>>>>>>> ---
> >>>>>>>>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
> >>>>>>>>  1 file changed, 10 insertions(+)
> >>>>>>>>
> >>>>>>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> >>>>>>>> index bc10e7e..f62928c 100644
> >>>>>>>> --- a/arch/arm/boot/dts/da850.dtsi
> >>>>>>>> +++ b/arch/arm/boot/dts/da850.dtsi
> >>>>>>>> @@ -411,6 +411,16 @@
> >>>>>>>>  			dma-names = "tx", "rx";
> >>>>>>>>  		};
> >>>>>>>>  	};
> >>>>>>>> +	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";
> >>>>>>>> +	};
> >>>>>>>>  	nand_cs3 at 62000000 {
> >>>>>>>>  		compatible = "ti,davinci-nand";
> >>>>>>>>  		reg = <0x62000000 0x807ff
> >>>>>>>
> >>>>>>> The nand node should be part of aemif node like it is done for keystone
> >>>>>>> boards.
> >>>>>>
> >>>>>> Actually, can you move the nand node out of da850.dtsi completely. Its
> >>>>>> much better to keep da850.dtsi restricted to soc-internal devices and
> >>>>>> keep the board level devices like NAND flash in <board>.dts file.
> >>>>>>
> >>>>>> Similarly, can you move the NAND pinmux definitions too to the
> >>>>>> da850-evm.dts file?
> >>>>>>
> >>>>>> There is advantage in keeping common pinmux definitions in da850.dtsi so
> >>>>>> each board doe not have to repeat them. But AEMIF is an exception as its
> >>>>>> usage can really be varied (NAND, NOR, SRAM, other). Plus, different
> >>>>>> boards are likely to use different chip selects so coming up with some
> >>>>>> pinmux definitions which will be reused widely is really unlikely.
> >>>>>>
> >>>>> This is exactly what I just did for the LCDK.
> >>>>> If everybody is happy with it I will do the same for the evm as I put it
> >>>>> in the cover letter.
> >>>>
> >>>> Yes please. We dont want duplication of data between da850.dtsi and
> >>>> da850-lcdk.dts files.
> >>>>
> >>> Then I'll wait for this series to be applied and then apply my changes
> >>> to the EVM while retiring the nand_cs3 together.
> >>
> >> No, I prefer the fixup happens first. In the same series, you can first
> >> fixup existing EVM and then add LCDK support.
> >>
> > 
> > Well in that case you'll have to do the testing since I only have an
> > LCDK. I should be able to send the series within the hour.
> 
> Sure. I can test it.
> 
The aemif/davinci_nand drivers don't configure AWCCR, yet davinci_nand
relies on EM_WAIT for RDY/nBUSY, so for the moment I keep the default
settings, but I configure the EM_WAIT pins in the pinctrl. I did it for
the LCDK, and it is not done for the EVM. Since the EVM schematics are
not public can you tell which EM_WAIT pins are connected ?
 
Karl

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

[Karl Beldan]

On Tue, Aug 09, 2016 at 05:15:17PM +0000, 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 | 108 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 108 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
> index dbcca0b..033380b 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,90 @@
>  	cd-gpios = <&gpio 64 GPIO_ACTIVE_HIGH>;
>  	status = "okay";
>  };
> +
> +&aemif {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&nand_pins>;
> +	status = "ok";
> +	cs2 {
Typo, should be cs3.
 
Karl
> +		#address-cells = <2>;
> +		#size-cells = <1>;
> +		clock-ranges;
> +		ranges;
> +
> +		ti,cs-chipselect = <2>;
> +
> +		nand@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-ecc-mode = "hw";
> +			ti,davinci-ecc-bits = <1>;
> +
> +			nand-bus-width= <16>;
> +			nand-on-flash-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@0 {
> +					label = "u-boot env";
> +					reg = <0 0x020000>;
> +				};
> +				partition@0x020000 {
> +					/* The LCDK defaults to booting from this partition */
> +					label = "u-boot";
> +					reg = <0x020000 0x080000>;
> +				};
> +				partition@0x0a0000 {
> +					label = "space";
> +					reg = <0x0a0000 0>;
> +				};
> +			};
> +		};
> +	};
> +};
> -- 
> 2.9.2
> 

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

[Karl Beldan]

On Tue, Aug 09, 2016 at 05:15:17PM +0000, 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 | 108 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 108 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
> index dbcca0b..033380b 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,90 @@
>  	cd-gpios = <&gpio 64 GPIO_ACTIVE_HIGH>;
>  	status = "okay";
>  };
> +
> +&aemif {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&nand_pins>;
> +	status = "ok";
> +	cs2 {
Typo, should be cs3.
 
Karl
> +		#address-cells = <2>;
> +		#size-cells = <1>;
> +		clock-ranges;
> +		ranges;
> +
> +		ti,cs-chipselect = <2>;
> +
> +		nand@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-ecc-mode = "hw";
> +			ti,davinci-ecc-bits = <1>;
> +
> +			nand-bus-width= <16>;
> +			nand-on-flash-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@0 {
> +					label = "u-boot env";
> +					reg = <0 0x020000>;
> +				};
> +				partition@0x020000 {
> +					/* The LCDK defaults to booting from this partition */
> +					label = "u-boot";
> +					reg = <0x020000 0x080000>;
> +				};
> +				partition@0x0a0000 {
> +					label = "space";
> +					reg = <0x0a0000 0>;
> +				};
> +			};
> +		};
> +	};
> +};
> -- 
> 2.9.2
> 

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

[Karl Beldan]

On Tue, Aug 09, 2016 at 05:15:17PM +0000, 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 | 108 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 108 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
> index dbcca0b..033380b 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,90 @@
>  	cd-gpios = <&gpio 64 GPIO_ACTIVE_HIGH>;
>  	status = "okay";
>  };
> +
> +&aemif {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&nand_pins>;
> +	status = "ok";
> +	cs2 {
Typo, should be cs3.
 
Karl
> +		#address-cells = <2>;
> +		#size-cells = <1>;
> +		clock-ranges;
> +		ranges;
> +
> +		ti,cs-chipselect = <2>;
> +
> +		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-ecc-mode = "hw";
> +			ti,davinci-ecc-bits = <1>;
> +
> +			nand-bus-width= <16>;
> +			nand-on-flash-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
> 

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

[Sekhar Nori]

On Wednesday 10 August 2016 02:34 PM, Karl Beldan wrote:
> On Wed, Aug 10, 2016 at 02:01:57PM +0530, Sekhar Nori wrote:
>> On Tuesday 09 August 2016 10:45 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.
>>
>> I disagree that we need to be compatible to the software that ships with
>> the board. Thats software was last updated 3 years ago. Instead I would
>> concern with what the hardware supports. So, if the hardware can support
>> 4-bit ECC, I would use that.
>>
> I am not saying we _need_ to be compatible.

Alright then, please drop references to what software the board ships
with in the commit message and in the patch itself.

> 
>> If driver is broken for 4-bit ECC, please fix that up first.
>>
> Since this issue is completely separate from my DT improvements
> I'll stick to resubmitting the series, applying my LCDK changes to the
> EVM too, besides you'll be able to compare the behavior without ECC
> discrepancies.
> I took note that you are likely to not apply without the ECC fix.

Yeah, I would not like to apply with 1-bit ECC now and then change to
4-bit ECC soon after.

Regards,
Sekhar

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

[Sekhar Nori]

On Wednesday 10 August 2016 02:34 PM, Karl Beldan wrote:
> On Wed, Aug 10, 2016 at 02:01:57PM +0530, Sekhar Nori wrote:
>> On Tuesday 09 August 2016 10:45 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.
>>
>> I disagree that we need to be compatible to the software that ships with
>> the board. Thats software was last updated 3 years ago. Instead I would
>> concern with what the hardware supports. So, if the hardware can support
>> 4-bit ECC, I would use that.
>>
> I am not saying we _need_ to be compatible.

Alright then, please drop references to what software the board ships
with in the commit message and in the patch itself.

> 
>> If driver is broken for 4-bit ECC, please fix that up first.
>>
> Since this issue is completely separate from my DT improvements
> I'll stick to resubmitting the series, applying my LCDK changes to the
> EVM too, besides you'll be able to compare the behavior without ECC
> discrepancies.
> I took note that you are likely to not apply without the ECC fix.

Yeah, I would not like to apply with 1-bit ECC now and then change to
4-bit ECC soon after.

Regards,
Sekhar

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

[Sekhar Nori]

On Wednesday 10 August 2016 02:34 PM, Karl Beldan wrote:
> On Wed, Aug 10, 2016 at 02:01:57PM +0530, Sekhar Nori wrote:
>> On Tuesday 09 August 2016 10:45 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.
>>
>> I disagree that we need to be compatible to the software that ships with
>> the board. Thats software was last updated 3 years ago. Instead I would
>> concern with what the hardware supports. So, if the hardware can support
>> 4-bit ECC, I would use that.
>>
> I am not saying we _need_ to be compatible.

Alright then, please drop references to what software the board ships
with in the commit message and in the patch itself.

> 
>> If driver is broken for 4-bit ECC, please fix that up first.
>>
> Since this issue is completely separate from my DT improvements
> I'll stick to resubmitting the series, applying my LCDK changes to the
> EVM too, besides you'll be able to compare the behavior without ECC
> discrepancies.
> I took note that you are likely to not apply without the ECC fix.

Yeah, I would not like to apply with 1-bit ECC now and then change to
4-bit ECC soon after.

Regards,
Sekhar

Re: [PATCH 2/4] ARM: dts: da850: Add an aemif node

[Sekhar Nori]

On Wednesday 10 August 2016 02:58 PM, Karl Beldan wrote:
> On Wed, Aug 10, 2016 at 02:04:48PM +0530, Sekhar Nori wrote:
>> On Wednesday 10 August 2016 02:04 PM, Karl Beldan wrote:
>>> On Wed, Aug 10, 2016 at 01:59:26PM +0530, Sekhar Nori wrote:
>>>> On Wednesday 10 August 2016 01:56 PM, Karl Beldan wrote:
>>>>> On Wed, Aug 10, 2016 at 01:42:01PM +0530, Sekhar Nori wrote:
>>>>>> On Wednesday 10 August 2016 01:37 PM, Karl Beldan wrote:
>>>>>>> On Wed, Aug 10, 2016 at 01:32:03PM +0530, Sekhar Nori wrote:
>>>>>>>> On Wednesday 10 August 2016 01:18 PM, Sekhar Nori wrote:
>>>>>>>>> On Tuesday 09 August 2016 10:45 PM, 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.
>>>>>>>>>> Also it will allow to properly pass the emif timings via DT.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
>>>>>>>>>> ---
>>>>>>>>>>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
>>>>>>>>>>  1 file changed, 10 insertions(+)
>>>>>>>>>>
>>>>>>>>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>>>>>>>>>> index bc10e7e..f62928c 100644
>>>>>>>>>> --- a/arch/arm/boot/dts/da850.dtsi
>>>>>>>>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>>>>>>>>> @@ -411,6 +411,16 @@
>>>>>>>>>>  			dma-names = "tx", "rx";
>>>>>>>>>>  		};
>>>>>>>>>>  	};
>>>>>>>>>> +	aemif: aemif@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";
>>>>>>>>>> +	};
>>>>>>>>>>  	nand_cs3@62000000 {
>>>>>>>>>>  		compatible = "ti,davinci-nand";
>>>>>>>>>>  		reg = <0x62000000 0x807ff
>>>>>>>>>
>>>>>>>>> The nand node should be part of aemif node like it is done for keystone
>>>>>>>>> boards.
>>>>>>>>
>>>>>>>> Actually, can you move the nand node out of da850.dtsi completely. Its
>>>>>>>> much better to keep da850.dtsi restricted to soc-internal devices and
>>>>>>>> keep the board level devices like NAND flash in <board>.dts file.
>>>>>>>>
>>>>>>>> Similarly, can you move the NAND pinmux definitions too to the
>>>>>>>> da850-evm.dts file?
>>>>>>>>
>>>>>>>> There is advantage in keeping common pinmux definitions in da850.dtsi so
>>>>>>>> each board doe not have to repeat them. But AEMIF is an exception as its
>>>>>>>> usage can really be varied (NAND, NOR, SRAM, other). Plus, different
>>>>>>>> boards are likely to use different chip selects so coming up with some
>>>>>>>> pinmux definitions which will be reused widely is really unlikely.
>>>>>>>>
>>>>>>> This is exactly what I just did for the LCDK.
>>>>>>> If everybody is happy with it I will do the same for the evm as I put it
>>>>>>> in the cover letter.
>>>>>>
>>>>>> Yes please. We dont want duplication of data between da850.dtsi and
>>>>>> da850-lcdk.dts files.
>>>>>>
>>>>> Then I'll wait for this series to be applied and then apply my changes
>>>>> to the EVM while retiring the nand_cs3 together.
>>>>
>>>> No, I prefer the fixup happens first. In the same series, you can first
>>>> fixup existing EVM and then add LCDK support.
>>>>
>>>
>>> Well in that case you'll have to do the testing since I only have an
>>> LCDK. I should be able to send the series within the hour.
>>
>> Sure. I can test it.
>>
> The aemif/davinci_nand drivers don't configure AWCCR, yet davinci_nand
> relies on EM_WAIT for RDY/nBUSY, so for the moment I keep the default
> settings, but I configure the EM_WAIT pins in the pinctrl. I did it for
> the LCDK, and it is not done for the EVM. Since the EVM schematics are
> not public can you tell which EM_WAIT pins are connected ?

On the EVM, the NAND ready/busy output is connected to EMA_WAIT0.

Regards,
Sekhar

[PATCH 2/4] ARM: dts: da850: Add an aemif node

[Sekhar Nori]

On Wednesday 10 August 2016 02:58 PM, Karl Beldan wrote:
> On Wed, Aug 10, 2016 at 02:04:48PM +0530, Sekhar Nori wrote:
>> On Wednesday 10 August 2016 02:04 PM, Karl Beldan wrote:
>>> On Wed, Aug 10, 2016 at 01:59:26PM +0530, Sekhar Nori wrote:
>>>> On Wednesday 10 August 2016 01:56 PM, Karl Beldan wrote:
>>>>> On Wed, Aug 10, 2016 at 01:42:01PM +0530, Sekhar Nori wrote:
>>>>>> On Wednesday 10 August 2016 01:37 PM, Karl Beldan wrote:
>>>>>>> On Wed, Aug 10, 2016 at 01:32:03PM +0530, Sekhar Nori wrote:
>>>>>>>> On Wednesday 10 August 2016 01:18 PM, Sekhar Nori wrote:
>>>>>>>>> On Tuesday 09 August 2016 10:45 PM, 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.
>>>>>>>>>> Also it will allow to properly pass the emif timings via DT.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
>>>>>>>>>> ---
>>>>>>>>>>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
>>>>>>>>>>  1 file changed, 10 insertions(+)
>>>>>>>>>>
>>>>>>>>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>>>>>>>>>> index bc10e7e..f62928c 100644
>>>>>>>>>> --- a/arch/arm/boot/dts/da850.dtsi
>>>>>>>>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>>>>>>>>> @@ -411,6 +411,16 @@
>>>>>>>>>>  			dma-names = "tx", "rx";
>>>>>>>>>>  		};
>>>>>>>>>>  	};
>>>>>>>>>> +	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";
>>>>>>>>>> +	};
>>>>>>>>>>  	nand_cs3 at 62000000 {
>>>>>>>>>>  		compatible = "ti,davinci-nand";
>>>>>>>>>>  		reg = <0x62000000 0x807ff
>>>>>>>>>
>>>>>>>>> The nand node should be part of aemif node like it is done for keystone
>>>>>>>>> boards.
>>>>>>>>
>>>>>>>> Actually, can you move the nand node out of da850.dtsi completely. Its
>>>>>>>> much better to keep da850.dtsi restricted to soc-internal devices and
>>>>>>>> keep the board level devices like NAND flash in <board>.dts file.
>>>>>>>>
>>>>>>>> Similarly, can you move the NAND pinmux definitions too to the
>>>>>>>> da850-evm.dts file?
>>>>>>>>
>>>>>>>> There is advantage in keeping common pinmux definitions in da850.dtsi so
>>>>>>>> each board doe not have to repeat them. But AEMIF is an exception as its
>>>>>>>> usage can really be varied (NAND, NOR, SRAM, other). Plus, different
>>>>>>>> boards are likely to use different chip selects so coming up with some
>>>>>>>> pinmux definitions which will be reused widely is really unlikely.
>>>>>>>>
>>>>>>> This is exactly what I just did for the LCDK.
>>>>>>> If everybody is happy with it I will do the same for the evm as I put it
>>>>>>> in the cover letter.
>>>>>>
>>>>>> Yes please. We dont want duplication of data between da850.dtsi and
>>>>>> da850-lcdk.dts files.
>>>>>>
>>>>> Then I'll wait for this series to be applied and then apply my changes
>>>>> to the EVM while retiring the nand_cs3 together.
>>>>
>>>> No, I prefer the fixup happens first. In the same series, you can first
>>>> fixup existing EVM and then add LCDK support.
>>>>
>>>
>>> Well in that case you'll have to do the testing since I only have an
>>> LCDK. I should be able to send the series within the hour.
>>
>> Sure. I can test it.
>>
> The aemif/davinci_nand drivers don't configure AWCCR, yet davinci_nand
> relies on EM_WAIT for RDY/nBUSY, so for the moment I keep the default
> settings, but I configure the EM_WAIT pins in the pinctrl. I did it for
> the LCDK, and it is not done for the EVM. Since the EVM schematics are
> not public can you tell which EM_WAIT pins are connected ?

On the EVM, the NAND ready/busy output is connected to EMA_WAIT0.

Regards,
Sekhar

Re: [PATCH 2/4] ARM: dts: da850: Add an aemif node

[Karl Beldan]

On Wed, Aug 10, 2016 at 09:28:48AM +0000, Karl Beldan wrote:
> On Wed, Aug 10, 2016 at 02:04:48PM +0530, Sekhar Nori wrote:
> > On Wednesday 10 August 2016 02:04 PM, Karl Beldan wrote:
> > > On Wed, Aug 10, 2016 at 01:59:26PM +0530, Sekhar Nori wrote:
> > >> On Wednesday 10 August 2016 01:56 PM, Karl Beldan wrote:
> > >>> On Wed, Aug 10, 2016 at 01:42:01PM +0530, Sekhar Nori wrote:
> > >>>> On Wednesday 10 August 2016 01:37 PM, Karl Beldan wrote:
> > >>>>> On Wed, Aug 10, 2016 at 01:32:03PM +0530, Sekhar Nori wrote:
> > >>>>>> On Wednesday 10 August 2016 01:18 PM, Sekhar Nori wrote:
> > >>>>>>> On Tuesday 09 August 2016 10:45 PM, 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.
> > >>>>>>>> Also it will allow to properly pass the emif timings via DT.
> > >>>>>>>>
> > >>>>>>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> > >>>>>>>> ---
> > >>>>>>>>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
> > >>>>>>>>  1 file changed, 10 insertions(+)
> > >>>>>>>>
> > >>>>>>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> > >>>>>>>> index bc10e7e..f62928c 100644
> > >>>>>>>> --- a/arch/arm/boot/dts/da850.dtsi
> > >>>>>>>> +++ b/arch/arm/boot/dts/da850.dtsi
> > >>>>>>>> @@ -411,6 +411,16 @@
> > >>>>>>>>  			dma-names = "tx", "rx";
> > >>>>>>>>  		};
> > >>>>>>>>  	};
> > >>>>>>>> +	aemif: aemif@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";
> > >>>>>>>> +	};
> > >>>>>>>>  	nand_cs3@62000000 {
> > >>>>>>>>  		compatible = "ti,davinci-nand";
> > >>>>>>>>  		reg = <0x62000000 0x807ff
> > >>>>>>>
> > >>>>>>> The nand node should be part of aemif node like it is done for keystone
> > >>>>>>> boards.
> > >>>>>>
> > >>>>>> Actually, can you move the nand node out of da850.dtsi completely. Its
> > >>>>>> much better to keep da850.dtsi restricted to soc-internal devices and
> > >>>>>> keep the board level devices like NAND flash in <board>.dts file.
> > >>>>>>
> > >>>>>> Similarly, can you move the NAND pinmux definitions too to the
> > >>>>>> da850-evm.dts file?
> > >>>>>>
> > >>>>>> There is advantage in keeping common pinmux definitions in da850.dtsi so
> > >>>>>> each board doe not have to repeat them. But AEMIF is an exception as its
> > >>>>>> usage can really be varied (NAND, NOR, SRAM, other). Plus, different
> > >>>>>> boards are likely to use different chip selects so coming up with some
> > >>>>>> pinmux definitions which will be reused widely is really unlikely.
> > >>>>>>
> > >>>>> This is exactly what I just did for the LCDK.
> > >>>>> If everybody is happy with it I will do the same for the evm as I put it
> > >>>>> in the cover letter.
> > >>>>
> > >>>> Yes please. We dont want duplication of data between da850.dtsi and
> > >>>> da850-lcdk.dts files.
> > >>>>
> > >>> Then I'll wait for this series to be applied and then apply my changes
> > >>> to the EVM while retiring the nand_cs3 together.
> > >>
> > >> No, I prefer the fixup happens first. In the same series, you can first
> > >> fixup existing EVM and then add LCDK support.
> > >>
> > > 
> > > Well in that case you'll have to do the testing since I only have an
> > > LCDK. I should be able to send the series within the hour.
> > 
> > Sure. I can test it.
> > 
> The aemif/davinci_nand drivers don't configure AWCCR, yet davinci_nand
> relies on EM_WAIT for RDY/nBUSY, so for the moment I keep the default
> settings, but I configure the EM_WAIT pins in the pinctrl. I did it for
> the LCDK, and it is not done for the EVM. Since the EVM schematics are
> not public can you tell which EM_WAIT pins are connected ?
>  
Also the device name is nand_cs3 but the pin muxing also enables CS4
both in Linux and U-Boot, can you tell whether it is needed ?
Or maybe you can share the schematics and I'll check it myself ?
 
Karl

Re: [PATCH 2/4] ARM: dts: da850: Add an aemif node

[Karl Beldan]

On Wed, Aug 10, 2016 at 09:28:48AM +0000, Karl Beldan wrote:
> On Wed, Aug 10, 2016 at 02:04:48PM +0530, Sekhar Nori wrote:
> > On Wednesday 10 August 2016 02:04 PM, Karl Beldan wrote:
> > > On Wed, Aug 10, 2016 at 01:59:26PM +0530, Sekhar Nori wrote:
> > >> On Wednesday 10 August 2016 01:56 PM, Karl Beldan wrote:
> > >>> On Wed, Aug 10, 2016 at 01:42:01PM +0530, Sekhar Nori wrote:
> > >>>> On Wednesday 10 August 2016 01:37 PM, Karl Beldan wrote:
> > >>>>> On Wed, Aug 10, 2016 at 01:32:03PM +0530, Sekhar Nori wrote:
> > >>>>>> On Wednesday 10 August 2016 01:18 PM, Sekhar Nori wrote:
> > >>>>>>> On Tuesday 09 August 2016 10:45 PM, 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.
> > >>>>>>>> Also it will allow to properly pass the emif timings via DT.
> > >>>>>>>>
> > >>>>>>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> > >>>>>>>> ---
> > >>>>>>>>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
> > >>>>>>>>  1 file changed, 10 insertions(+)
> > >>>>>>>>
> > >>>>>>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> > >>>>>>>> index bc10e7e..f62928c 100644
> > >>>>>>>> --- a/arch/arm/boot/dts/da850.dtsi
> > >>>>>>>> +++ b/arch/arm/boot/dts/da850.dtsi
> > >>>>>>>> @@ -411,6 +411,16 @@
> > >>>>>>>>  			dma-names = "tx", "rx";
> > >>>>>>>>  		};
> > >>>>>>>>  	};
> > >>>>>>>> +	aemif: aemif@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";
> > >>>>>>>> +	};
> > >>>>>>>>  	nand_cs3@62000000 {
> > >>>>>>>>  		compatible = "ti,davinci-nand";
> > >>>>>>>>  		reg = <0x62000000 0x807ff
> > >>>>>>>
> > >>>>>>> The nand node should be part of aemif node like it is done for keystone
> > >>>>>>> boards.
> > >>>>>>
> > >>>>>> Actually, can you move the nand node out of da850.dtsi completely. Its
> > >>>>>> much better to keep da850.dtsi restricted to soc-internal devices and
> > >>>>>> keep the board level devices like NAND flash in <board>.dts file.
> > >>>>>>
> > >>>>>> Similarly, can you move the NAND pinmux definitions too to the
> > >>>>>> da850-evm.dts file?
> > >>>>>>
> > >>>>>> There is advantage in keeping common pinmux definitions in da850.dtsi so
> > >>>>>> each board doe not have to repeat them. But AEMIF is an exception as its
> > >>>>>> usage can really be varied (NAND, NOR, SRAM, other). Plus, different
> > >>>>>> boards are likely to use different chip selects so coming up with some
> > >>>>>> pinmux definitions which will be reused widely is really unlikely.
> > >>>>>>
> > >>>>> This is exactly what I just did for the LCDK.
> > >>>>> If everybody is happy with it I will do the same for the evm as I put it
> > >>>>> in the cover letter.
> > >>>>
> > >>>> Yes please. We dont want duplication of data between da850.dtsi and
> > >>>> da850-lcdk.dts files.
> > >>>>
> > >>> Then I'll wait for this series to be applied and then apply my changes
> > >>> to the EVM while retiring the nand_cs3 together.
> > >>
> > >> No, I prefer the fixup happens first. In the same series, you can first
> > >> fixup existing EVM and then add LCDK support.
> > >>
> > > 
> > > Well in that case you'll have to do the testing since I only have an
> > > LCDK. I should be able to send the series within the hour.
> > 
> > Sure. I can test it.
> > 
> The aemif/davinci_nand drivers don't configure AWCCR, yet davinci_nand
> relies on EM_WAIT for RDY/nBUSY, so for the moment I keep the default
> settings, but I configure the EM_WAIT pins in the pinctrl. I did it for
> the LCDK, and it is not done for the EVM. Since the EVM schematics are
> not public can you tell which EM_WAIT pins are connected ?
>  
Also the device name is nand_cs3 but the pin muxing also enables CS4
both in Linux and U-Boot, can you tell whether it is needed ?
Or maybe you can share the schematics and I'll check it myself ?
 
Karl

[PATCH 2/4] ARM: dts: da850: Add an aemif node

[Karl Beldan]

On Wed, Aug 10, 2016 at 09:28:48AM +0000, Karl Beldan wrote:
> On Wed, Aug 10, 2016 at 02:04:48PM +0530, Sekhar Nori wrote:
> > On Wednesday 10 August 2016 02:04 PM, Karl Beldan wrote:
> > > On Wed, Aug 10, 2016 at 01:59:26PM +0530, Sekhar Nori wrote:
> > >> On Wednesday 10 August 2016 01:56 PM, Karl Beldan wrote:
> > >>> On Wed, Aug 10, 2016 at 01:42:01PM +0530, Sekhar Nori wrote:
> > >>>> On Wednesday 10 August 2016 01:37 PM, Karl Beldan wrote:
> > >>>>> On Wed, Aug 10, 2016 at 01:32:03PM +0530, Sekhar Nori wrote:
> > >>>>>> On Wednesday 10 August 2016 01:18 PM, Sekhar Nori wrote:
> > >>>>>>> On Tuesday 09 August 2016 10:45 PM, 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.
> > >>>>>>>> Also it will allow to properly pass the emif timings via DT.
> > >>>>>>>>
> > >>>>>>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> > >>>>>>>> ---
> > >>>>>>>>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
> > >>>>>>>>  1 file changed, 10 insertions(+)
> > >>>>>>>>
> > >>>>>>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> > >>>>>>>> index bc10e7e..f62928c 100644
> > >>>>>>>> --- a/arch/arm/boot/dts/da850.dtsi
> > >>>>>>>> +++ b/arch/arm/boot/dts/da850.dtsi
> > >>>>>>>> @@ -411,6 +411,16 @@
> > >>>>>>>>  			dma-names = "tx", "rx";
> > >>>>>>>>  		};
> > >>>>>>>>  	};
> > >>>>>>>> +	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";
> > >>>>>>>> +	};
> > >>>>>>>>  	nand_cs3 at 62000000 {
> > >>>>>>>>  		compatible = "ti,davinci-nand";
> > >>>>>>>>  		reg = <0x62000000 0x807ff
> > >>>>>>>
> > >>>>>>> The nand node should be part of aemif node like it is done for keystone
> > >>>>>>> boards.
> > >>>>>>
> > >>>>>> Actually, can you move the nand node out of da850.dtsi completely. Its
> > >>>>>> much better to keep da850.dtsi restricted to soc-internal devices and
> > >>>>>> keep the board level devices like NAND flash in <board>.dts file.
> > >>>>>>
> > >>>>>> Similarly, can you move the NAND pinmux definitions too to the
> > >>>>>> da850-evm.dts file?
> > >>>>>>
> > >>>>>> There is advantage in keeping common pinmux definitions in da850.dtsi so
> > >>>>>> each board doe not have to repeat them. But AEMIF is an exception as its
> > >>>>>> usage can really be varied (NAND, NOR, SRAM, other). Plus, different
> > >>>>>> boards are likely to use different chip selects so coming up with some
> > >>>>>> pinmux definitions which will be reused widely is really unlikely.
> > >>>>>>
> > >>>>> This is exactly what I just did for the LCDK.
> > >>>>> If everybody is happy with it I will do the same for the evm as I put it
> > >>>>> in the cover letter.
> > >>>>
> > >>>> Yes please. We dont want duplication of data between da850.dtsi and
> > >>>> da850-lcdk.dts files.
> > >>>>
> > >>> Then I'll wait for this series to be applied and then apply my changes
> > >>> to the EVM while retiring the nand_cs3 together.
> > >>
> > >> No, I prefer the fixup happens first. In the same series, you can first
> > >> fixup existing EVM and then add LCDK support.
> > >>
> > > 
> > > Well in that case you'll have to do the testing since I only have an
> > > LCDK. I should be able to send the series within the hour.
> > 
> > Sure. I can test it.
> > 
> The aemif/davinci_nand drivers don't configure AWCCR, yet davinci_nand
> relies on EM_WAIT for RDY/nBUSY, so for the moment I keep the default
> settings, but I configure the EM_WAIT pins in the pinctrl. I did it for
> the LCDK, and it is not done for the EVM. Since the EVM schematics are
> not public can you tell which EM_WAIT pins are connected ?
>  
Also the device name is nand_cs3 but the pin muxing also enables CS4
both in Linux and U-Boot, can you tell whether it is needed ?
Or maybe you can share the schematics and I'll check it myself ?
 
Karl

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

[Karl Beldan]

On Wed, Aug 10, 2016 at 03:01:30PM +0530, Sekhar Nori wrote:
> On Wednesday 10 August 2016 02:34 PM, Karl Beldan wrote:
> > On Wed, Aug 10, 2016 at 02:01:57PM +0530, Sekhar Nori wrote:
> >> On Tuesday 09 August 2016 10:45 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.
> >>
> >> I disagree that we need to be compatible to the software that ships with
> >> the board. Thats software was last updated 3 years ago. Instead I would
> >> concern with what the hardware supports. So, if the hardware can support
> >> 4-bit ECC, I would use that.
> >>
> > I am not saying we _need_ to be compatible.
> 
> Alright then, please drop references to what software the board ships
> with in the commit message and in the patch itself.
> 

I hadn't seen this comment before sending v2.

> > 
> >> If driver is broken for 4-bit ECC, please fix that up first.
> >>
> > Since this issue is completely separate from my DT improvements
> > I'll stick to resubmitting the series, applying my LCDK changes to the
> > EVM too, besides you'll be able to compare the behavior without ECC
> > discrepancies.
> > I took note that you are likely to not apply without the ECC fix.
> 
> Yeah, I would not like to apply with 1-bit ECC now and then change to
> 4-bit ECC soon after.
> 

Both mityomapl138 from mainline and hawkboard from TI's BSP release
include the comment:
- "4 bit mode is not supported with 16 bit NAND"
It is not clear whether they imply that the HW has issues or if it's SW
only, but 4-bits ECC is a different matter and I hope you'll integrate
the current changes prior to tackling it.
 
Karl

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

[Karl Beldan]

On Wed, Aug 10, 2016 at 03:01:30PM +0530, Sekhar Nori wrote:
> On Wednesday 10 August 2016 02:34 PM, Karl Beldan wrote:
> > On Wed, Aug 10, 2016 at 02:01:57PM +0530, Sekhar Nori wrote:
> >> On Tuesday 09 August 2016 10:45 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.
> >>
> >> I disagree that we need to be compatible to the software that ships with
> >> the board. Thats software was last updated 3 years ago. Instead I would
> >> concern with what the hardware supports. So, if the hardware can support
> >> 4-bit ECC, I would use that.
> >>
> > I am not saying we _need_ to be compatible.
> 
> Alright then, please drop references to what software the board ships
> with in the commit message and in the patch itself.
> 

I hadn't seen this comment before sending v2.

> > 
> >> If driver is broken for 4-bit ECC, please fix that up first.
> >>
> > Since this issue is completely separate from my DT improvements
> > I'll stick to resubmitting the series, applying my LCDK changes to the
> > EVM too, besides you'll be able to compare the behavior without ECC
> > discrepancies.
> > I took note that you are likely to not apply without the ECC fix.
> 
> Yeah, I would not like to apply with 1-bit ECC now and then change to
> 4-bit ECC soon after.
> 

Both mityomapl138 from mainline and hawkboard from TI's BSP release
include the comment:
- "4 bit mode is not supported with 16 bit NAND"
It is not clear whether they imply that the HW has issues or if it's SW
only, but 4-bits ECC is a different matter and I hope you'll integrate
the current changes prior to tackling it.
 
Karl

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

[Karl Beldan]

On Wed, Aug 10, 2016 at 03:01:30PM +0530, Sekhar Nori wrote:
> On Wednesday 10 August 2016 02:34 PM, Karl Beldan wrote:
> > On Wed, Aug 10, 2016 at 02:01:57PM +0530, Sekhar Nori wrote:
> >> On Tuesday 09 August 2016 10:45 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.
> >>
> >> I disagree that we need to be compatible to the software that ships with
> >> the board. Thats software was last updated 3 years ago. Instead I would
> >> concern with what the hardware supports. So, if the hardware can support
> >> 4-bit ECC, I would use that.
> >>
> > I am not saying we _need_ to be compatible.
> 
> Alright then, please drop references to what software the board ships
> with in the commit message and in the patch itself.
> 

I hadn't seen this comment before sending v2.

> > 
> >> If driver is broken for 4-bit ECC, please fix that up first.
> >>
> > Since this issue is completely separate from my DT improvements
> > I'll stick to resubmitting the series, applying my LCDK changes to the
> > EVM too, besides you'll be able to compare the behavior without ECC
> > discrepancies.
> > I took note that you are likely to not apply without the ECC fix.
> 
> Yeah, I would not like to apply with 1-bit ECC now and then change to
> 4-bit ECC soon after.
> 

Both mityomapl138 from mainline and hawkboard from TI's BSP release
include the comment:
- "4 bit mode is not supported with 16 bit NAND"
It is not clear whether they imply that the HW has issues or if it's SW
only, but 4-bits ECC is a different matter and I hope you'll integrate
the current changes prior to tackling it.
 
Karl

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

[Sekhar Nori]

On Wednesday 10 August 2016 04:49 PM, Karl Beldan wrote:
> On Wed, Aug 10, 2016 at 03:01:30PM +0530, Sekhar Nori wrote:
>> On Wednesday 10 August 2016 02:34 PM, Karl Beldan wrote:
>>> On Wed, Aug 10, 2016 at 02:01:57PM +0530, Sekhar Nori wrote:
>>>> On Tuesday 09 August 2016 10:45 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.
>>>>
>>>> I disagree that we need to be compatible to the software that ships with
>>>> the board. Thats software was last updated 3 years ago. Instead I would
>>>> concern with what the hardware supports. So, if the hardware can support
>>>> 4-bit ECC, I would use that.
>>>>
>>> I am not saying we _need_ to be compatible.
>>
>> Alright then, please drop references to what software the board ships
>> with in the commit message and in the patch itself.
>>
> 
> I hadn't seen this comment before sending v2.
> 
>>>
>>>> If driver is broken for 4-bit ECC, please fix that up first.
>>>>
>>> Since this issue is completely separate from my DT improvements
>>> I'll stick to resubmitting the series, applying my LCDK changes to the
>>> EVM too, besides you'll be able to compare the behavior without ECC
>>> discrepancies.
>>> I took note that you are likely to not apply without the ECC fix.
>>
>> Yeah, I would not like to apply with 1-bit ECC now and then change to
>> 4-bit ECC soon after.
>>
> 
> Both mityomapl138 from mainline and hawkboard from TI's BSP release
> include the comment:
> - "4 bit mode is not supported with 16 bit NAND"
> It is not clear whether they imply that the HW has issues or if it's SW

At least the TRM says "The EMIFA supports 4-bit ECC on 8-bit/16-bit NAND
Flash.". And then the TRM goes on to describe how 4-bit ECC is supposed
to work for 16-bit NAND. I could not find any errata related to this in
the errata document.

> only, but 4-bits ECC is a different matter and I hope you'll integrate
> the current changes prior to tackling it.

Lets apply everything together. This is a new feature, not a bug fix.
There is no need to rush it. Also, for the NAND part on LCDK, per the
micron datasheet, minimum of 4-bit ECC is needed.

I will take a look at other patches in this series.

Regards,
Sekhar

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

[Sekhar Nori]

On Wednesday 10 August 2016 04:49 PM, Karl Beldan wrote:
> On Wed, Aug 10, 2016 at 03:01:30PM +0530, Sekhar Nori wrote:
>> On Wednesday 10 August 2016 02:34 PM, Karl Beldan wrote:
>>> On Wed, Aug 10, 2016 at 02:01:57PM +0530, Sekhar Nori wrote:
>>>> On Tuesday 09 August 2016 10:45 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.
>>>>
>>>> I disagree that we need to be compatible to the software that ships with
>>>> the board. Thats software was last updated 3 years ago. Instead I would
>>>> concern with what the hardware supports. So, if the hardware can support
>>>> 4-bit ECC, I would use that.
>>>>
>>> I am not saying we _need_ to be compatible.
>>
>> Alright then, please drop references to what software the board ships
>> with in the commit message and in the patch itself.
>>
> 
> I hadn't seen this comment before sending v2.
> 
>>>
>>>> If driver is broken for 4-bit ECC, please fix that up first.
>>>>
>>> Since this issue is completely separate from my DT improvements
>>> I'll stick to resubmitting the series, applying my LCDK changes to the
>>> EVM too, besides you'll be able to compare the behavior without ECC
>>> discrepancies.
>>> I took note that you are likely to not apply without the ECC fix.
>>
>> Yeah, I would not like to apply with 1-bit ECC now and then change to
>> 4-bit ECC soon after.
>>
> 
> Both mityomapl138 from mainline and hawkboard from TI's BSP release
> include the comment:
> - "4 bit mode is not supported with 16 bit NAND"
> It is not clear whether they imply that the HW has issues or if it's SW

At least the TRM says "The EMIFA supports 4-bit ECC on 8-bit/16-bit NAND
Flash.". And then the TRM goes on to describe how 4-bit ECC is supposed
to work for 16-bit NAND. I could not find any errata related to this in
the errata document.

> only, but 4-bits ECC is a different matter and I hope you'll integrate
> the current changes prior to tackling it.

Lets apply everything together. This is a new feature, not a bug fix.
There is no need to rush it. Also, for the NAND part on LCDK, per the
micron datasheet, minimum of 4-bit ECC is needed.

I will take a look at other patches in this series.

Regards,
Sekhar

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

[Sekhar Nori]

On Wednesday 10 August 2016 04:49 PM, Karl Beldan wrote:
> On Wed, Aug 10, 2016 at 03:01:30PM +0530, Sekhar Nori wrote:
>> On Wednesday 10 August 2016 02:34 PM, Karl Beldan wrote:
>>> On Wed, Aug 10, 2016 at 02:01:57PM +0530, Sekhar Nori wrote:
>>>> On Tuesday 09 August 2016 10:45 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.
>>>>
>>>> I disagree that we need to be compatible to the software that ships with
>>>> the board. Thats software was last updated 3 years ago. Instead I would
>>>> concern with what the hardware supports. So, if the hardware can support
>>>> 4-bit ECC, I would use that.
>>>>
>>> I am not saying we _need_ to be compatible.
>>
>> Alright then, please drop references to what software the board ships
>> with in the commit message and in the patch itself.
>>
> 
> I hadn't seen this comment before sending v2.
> 
>>>
>>>> If driver is broken for 4-bit ECC, please fix that up first.
>>>>
>>> Since this issue is completely separate from my DT improvements
>>> I'll stick to resubmitting the series, applying my LCDK changes to the
>>> EVM too, besides you'll be able to compare the behavior without ECC
>>> discrepancies.
>>> I took note that you are likely to not apply without the ECC fix.
>>
>> Yeah, I would not like to apply with 1-bit ECC now and then change to
>> 4-bit ECC soon after.
>>
> 
> Both mityomapl138 from mainline and hawkboard from TI's BSP release
> include the comment:
> - "4 bit mode is not supported with 16 bit NAND"
> It is not clear whether they imply that the HW has issues or if it's SW

At least the TRM says "The EMIFA supports 4-bit ECC on 8-bit/16-bit NAND
Flash.". And then the TRM goes on to describe how 4-bit ECC is supposed
to work for 16-bit NAND. I could not find any errata related to this in
the errata document.

> only, but 4-bits ECC is a different matter and I hope you'll integrate
> the current changes prior to tackling it.

Lets apply everything together. This is a new feature, not a bug fix.
There is no need to rush it. Also, for the NAND part on LCDK, per the
micron datasheet, minimum of 4-bit ECC is needed.

I will take a look at other patches in this series.

Regards,
Sekhar

Re: [PATCH 2/4] ARM: dts: da850: Add an aemif node

[Karl Beldan]

On Wed, Aug 10, 2016 at 02:04:48PM +0530, Sekhar Nori wrote:
> On Wednesday 10 August 2016 02:04 PM, Karl Beldan wrote:
> > On Wed, Aug 10, 2016 at 01:59:26PM +0530, Sekhar Nori wrote:
> >> On Wednesday 10 August 2016 01:56 PM, Karl Beldan wrote:
> >>> On Wed, Aug 10, 2016 at 01:42:01PM +0530, Sekhar Nori wrote:
> >>>> On Wednesday 10 August 2016 01:37 PM, Karl Beldan wrote:
> >>>>> On Wed, Aug 10, 2016 at 01:32:03PM +0530, Sekhar Nori wrote:
> >>>>>> On Wednesday 10 August 2016 01:18 PM, Sekhar Nori wrote:
> >>>>>>> On Tuesday 09 August 2016 10:45 PM, 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.
> >>>>>>>> Also it will allow to properly pass the emif timings via DT.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> >>>>>>>> ---
> >>>>>>>>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
> >>>>>>>>  1 file changed, 10 insertions(+)
> >>>>>>>>
> >>>>>>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> >>>>>>>> index bc10e7e..f62928c 100644
> >>>>>>>> --- a/arch/arm/boot/dts/da850.dtsi
> >>>>>>>> +++ b/arch/arm/boot/dts/da850.dtsi
> >>>>>>>> @@ -411,6 +411,16 @@
> >>>>>>>>  			dma-names = "tx", "rx";
> >>>>>>>>  		};
> >>>>>>>>  	};
> >>>>>>>> +	aemif: aemif@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";
> >>>>>>>> +	};
> >>>>>>>>  	nand_cs3@62000000 {
> >>>>>>>>  		compatible = "ti,davinci-nand";
> >>>>>>>>  		reg = <0x62000000 0x807ff
> >>>>>>>
> >>>>>>> The nand node should be part of aemif node like it is done for keystone
> >>>>>>> boards.
> >>>>>>
> >>>>>> Actually, can you move the nand node out of da850.dtsi completely. Its
> >>>>>> much better to keep da850.dtsi restricted to soc-internal devices and
> >>>>>> keep the board level devices like NAND flash in <board>.dts file.
> >>>>>>
> >>>>>> Similarly, can you move the NAND pinmux definitions too to the
> >>>>>> da850-evm.dts file?
> >>>>>>
> >>>>>> There is advantage in keeping common pinmux definitions in da850.dtsi so
> >>>>>> each board doe not have to repeat them. But AEMIF is an exception as its
> >>>>>> usage can really be varied (NAND, NOR, SRAM, other). Plus, different
> >>>>>> boards are likely to use different chip selects so coming up with some
> >>>>>> pinmux definitions which will be reused widely is really unlikely.
> >>>>>>
> >>>>> This is exactly what I just did for the LCDK.
> >>>>> If everybody is happy with it I will do the same for the evm as I put it
> >>>>> in the cover letter.
> >>>>
> >>>> Yes please. We dont want duplication of data between da850.dtsi and
> >>>> da850-lcdk.dts files.
> >>>>
> >>> Then I'll wait for this series to be applied and then apply my changes
> >>> to the EVM while retiring the nand_cs3 together.
> >>
> >> No, I prefer the fixup happens first. In the same series, you can first
> >> fixup existing EVM and then add LCDK support.
> >>
> > 
> > Well in that case you'll have to do the testing since I only have an
> > LCDK. I should be able to send the series within the hour.
> 
> Sure. I can test it.
> 

Yesterday I got my hands on an EVM TI just sent and could test it on it.
The change proper is fine, but I was surprised mainline was broken wrt
4-bit ECC on top of 8bits NANDs, so I tested with 1-bit ECC, 'enough'
for this device.

FYI, the NAND socket had a
  nand: device found, Manufacturer ID: 0x2c, Chip ID: 0xdc
  nand: Micron MT29F4G08AAC
  nand: 512 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
 
Karl

[PATCH 2/4] ARM: dts: da850: Add an aemif node

[Karl Beldan]

On Wed, Aug 10, 2016 at 02:04:48PM +0530, Sekhar Nori wrote:
> On Wednesday 10 August 2016 02:04 PM, Karl Beldan wrote:
> > On Wed, Aug 10, 2016 at 01:59:26PM +0530, Sekhar Nori wrote:
> >> On Wednesday 10 August 2016 01:56 PM, Karl Beldan wrote:
> >>> On Wed, Aug 10, 2016 at 01:42:01PM +0530, Sekhar Nori wrote:
> >>>> On Wednesday 10 August 2016 01:37 PM, Karl Beldan wrote:
> >>>>> On Wed, Aug 10, 2016 at 01:32:03PM +0530, Sekhar Nori wrote:
> >>>>>> On Wednesday 10 August 2016 01:18 PM, Sekhar Nori wrote:
> >>>>>>> On Tuesday 09 August 2016 10:45 PM, 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.
> >>>>>>>> Also it will allow to properly pass the emif timings via DT.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> >>>>>>>> ---
> >>>>>>>>  arch/arm/boot/dts/da850.dtsi | 10 ++++++++++
> >>>>>>>>  1 file changed, 10 insertions(+)
> >>>>>>>>
> >>>>>>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> >>>>>>>> index bc10e7e..f62928c 100644
> >>>>>>>> --- a/arch/arm/boot/dts/da850.dtsi
> >>>>>>>> +++ b/arch/arm/boot/dts/da850.dtsi
> >>>>>>>> @@ -411,6 +411,16 @@
> >>>>>>>>  			dma-names = "tx", "rx";
> >>>>>>>>  		};
> >>>>>>>>  	};
> >>>>>>>> +	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";
> >>>>>>>> +	};
> >>>>>>>>  	nand_cs3 at 62000000 {
> >>>>>>>>  		compatible = "ti,davinci-nand";
> >>>>>>>>  		reg = <0x62000000 0x807ff
> >>>>>>>
> >>>>>>> The nand node should be part of aemif node like it is done for keystone
> >>>>>>> boards.
> >>>>>>
> >>>>>> Actually, can you move the nand node out of da850.dtsi completely. Its
> >>>>>> much better to keep da850.dtsi restricted to soc-internal devices and
> >>>>>> keep the board level devices like NAND flash in <board>.dts file.
> >>>>>>
> >>>>>> Similarly, can you move the NAND pinmux definitions too to the
> >>>>>> da850-evm.dts file?
> >>>>>>
> >>>>>> There is advantage in keeping common pinmux definitions in da850.dtsi so
> >>>>>> each board doe not have to repeat them. But AEMIF is an exception as its
> >>>>>> usage can really be varied (NAND, NOR, SRAM, other). Plus, different
> >>>>>> boards are likely to use different chip selects so coming up with some
> >>>>>> pinmux definitions which will be reused widely is really unlikely.
> >>>>>>
> >>>>> This is exactly what I just did for the LCDK.
> >>>>> If everybody is happy with it I will do the same for the evm as I put it
> >>>>> in the cover letter.
> >>>>
> >>>> Yes please. We dont want duplication of data between da850.dtsi and
> >>>> da850-lcdk.dts files.
> >>>>
> >>> Then I'll wait for this series to be applied and then apply my changes
> >>> to the EVM while retiring the nand_cs3 together.
> >>
> >> No, I prefer the fixup happens first. In the same series, you can first
> >> fixup existing EVM and then add LCDK support.
> >>
> > 
> > Well in that case you'll have to do the testing since I only have an
> > LCDK. I should be able to send the series within the hour.
> 
> Sure. I can test it.
> 

Yesterday I got my hands on an EVM TI just sent and could test it on it.
The change proper is fine, but I was surprised mainline was broken wrt
4-bit ECC on top of 8bits NANDs, so I tested with 1-bit ECC, 'enough'
for this device.

FYI, the NAND socket had a
  nand: device found, Manufacturer ID: 0x2c, Chip ID: 0xdc
  nand: Micron MT29F4G08AAC
  nand: 512 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
 
Karl

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

[Karl Beldan]

On Wed, Aug 10, 2016 at 05:23:23PM +0530, Sekhar Nori wrote:
> On Wednesday 10 August 2016 04:49 PM, Karl Beldan wrote:
> > On Wed, Aug 10, 2016 at 03:01:30PM +0530, Sekhar Nori wrote:
> >> On Wednesday 10 August 2016 02:34 PM, Karl Beldan wrote:
> >>> On Wed, Aug 10, 2016 at 02:01:57PM +0530, Sekhar Nori wrote:
> >>>> On Tuesday 09 August 2016 10:45 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.
> >>>>
> >>>> I disagree that we need to be compatible to the software that ships with
> >>>> the board. Thats software was last updated 3 years ago. Instead I would
> >>>> concern with what the hardware supports. So, if the hardware can support
> >>>> 4-bit ECC, I would use that.
> >>>>
> >>> I am not saying we _need_ to be compatible.
> >>
> >> Alright then, please drop references to what software the board ships
> >> with in the commit message and in the patch itself.
> >>
> > 
> > I hadn't seen this comment before sending v2.
> > 
> >>>
> >>>> If driver is broken for 4-bit ECC, please fix that up first.
> >>>>
> >>> Since this issue is completely separate from my DT improvements
> >>> I'll stick to resubmitting the series, applying my LCDK changes to the
> >>> EVM too, besides you'll be able to compare the behavior without ECC
> >>> discrepancies.
> >>> I took note that you are likely to not apply without the ECC fix.
> >>
> >> Yeah, I would not like to apply with 1-bit ECC now and then change to
> >> 4-bit ECC soon after.
> >>
> > 
> > Both mityomapl138 from mainline and hawkboard from TI's BSP release
> > include the comment:
> > - "4 bit mode is not supported with 16 bit NAND"
> > It is not clear whether they imply that the HW has issues or if it's SW
> 
> At least the TRM says "The EMIFA supports 4-bit ECC on 8-bit/16-bit NAND
> Flash.". And then the TRM goes on to describe how 4-bit ECC is supposed
> to work for 16-bit NAND. I could not find any errata related to this in
> the errata document.
> 
> > only, but 4-bits ECC is a different matter and I hope you'll integrate
> > the current changes prior to tackling it.
> 
> Lets apply everything together. This is a new feature, not a bug fix.
> There is no need to rush it. Also, for the NAND part on LCDK, per the
> micron datasheet, minimum of 4-bit ECC is needed.
> 

FYI my patches are related to a small contracting work spanning 20 days
amounting to 15 real days (from scratch).  ATM my SoW not only doesn't
include this 'new feature' but excludes it. So I cannot promise I'll
look into it, though it shouldn't be too big a thing.
It seemed clear that you wanted to squeeze it with the rest, but after
setting things up for the EVM and observing mainline was broken with
4bits ECC on an 8bits NAND I thought it was impossible nobody hadn't
noticed it .. so I dug in the mail archive, only to find there had
already been some alerts raised some months ago on the list, so I picked
up my next item.

> I will take a look at other patches in this series.
> 

Ok, thanks.

Rgds, 
Karl

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

[Karl Beldan]

On Wed, Aug 10, 2016 at 05:23:23PM +0530, Sekhar Nori wrote:
> On Wednesday 10 August 2016 04:49 PM, Karl Beldan wrote:
> > On Wed, Aug 10, 2016 at 03:01:30PM +0530, Sekhar Nori wrote:
> >> On Wednesday 10 August 2016 02:34 PM, Karl Beldan wrote:
> >>> On Wed, Aug 10, 2016 at 02:01:57PM +0530, Sekhar Nori wrote:
> >>>> On Tuesday 09 August 2016 10:45 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.
> >>>>
> >>>> I disagree that we need to be compatible to the software that ships with
> >>>> the board. Thats software was last updated 3 years ago. Instead I would
> >>>> concern with what the hardware supports. So, if the hardware can support
> >>>> 4-bit ECC, I would use that.
> >>>>
> >>> I am not saying we _need_ to be compatible.
> >>
> >> Alright then, please drop references to what software the board ships
> >> with in the commit message and in the patch itself.
> >>
> > 
> > I hadn't seen this comment before sending v2.
> > 
> >>>
> >>>> If driver is broken for 4-bit ECC, please fix that up first.
> >>>>
> >>> Since this issue is completely separate from my DT improvements
> >>> I'll stick to resubmitting the series, applying my LCDK changes to the
> >>> EVM too, besides you'll be able to compare the behavior without ECC
> >>> discrepancies.
> >>> I took note that you are likely to not apply without the ECC fix.
> >>
> >> Yeah, I would not like to apply with 1-bit ECC now and then change to
> >> 4-bit ECC soon after.
> >>
> > 
> > Both mityomapl138 from mainline and hawkboard from TI's BSP release
> > include the comment:
> > - "4 bit mode is not supported with 16 bit NAND"
> > It is not clear whether they imply that the HW has issues or if it's SW
> 
> At least the TRM says "The EMIFA supports 4-bit ECC on 8-bit/16-bit NAND
> Flash.". And then the TRM goes on to describe how 4-bit ECC is supposed
> to work for 16-bit NAND. I could not find any errata related to this in
> the errata document.
> 
> > only, but 4-bits ECC is a different matter and I hope you'll integrate
> > the current changes prior to tackling it.
> 
> Lets apply everything together. This is a new feature, not a bug fix.
> There is no need to rush it. Also, for the NAND part on LCDK, per the
> micron datasheet, minimum of 4-bit ECC is needed.
> 

FYI my patches are related to a small contracting work spanning 20 days
amounting to 15 real days (from scratch).  ATM my SoW not only doesn't
include this 'new feature' but excludes it. So I cannot promise I'll
look into it, though it shouldn't be too big a thing.
It seemed clear that you wanted to squeeze it with the rest, but after
setting things up for the EVM and observing mainline was broken with
4bits ECC on an 8bits NAND I thought it was impossible nobody hadn't
noticed it .. so I dug in the mail archive, only to find there had
already been some alerts raised some months ago on the list, so I picked
up my next item.

> I will take a look at other patches in this series.
> 

Ok, thanks.

Rgds, 
Karl

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

[Karl Beldan]

On Tue, Aug 16, 2016 at 11:20:29PM +0000, Karl Beldan wrote:
> On Wed, Aug 10, 2016 at 05:23:23PM +0530, Sekhar Nori wrote:
> > On Wednesday 10 August 2016 04:49 PM, Karl Beldan wrote:
> > > On Wed, Aug 10, 2016 at 03:01:30PM +0530, Sekhar Nori wrote:
> > >> On Wednesday 10 August 2016 02:34 PM, Karl Beldan wrote:
> > >>> On Wed, Aug 10, 2016 at 02:01:57PM +0530, Sekhar Nori wrote:
> > >>>> On Tuesday 09 August 2016 10:45 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.
> > >>>>
> > >>>> I disagree that we need to be compatible to the software that ships with
> > >>>> the board. Thats software was last updated 3 years ago. Instead I would
> > >>>> concern with what the hardware supports. So, if the hardware can support
> > >>>> 4-bit ECC, I would use that.
> > >>>>
> > >>> I am not saying we _need_ to be compatible.
> > >>
> > >> Alright then, please drop references to what software the board ships
> > >> with in the commit message and in the patch itself.
> > >>
> > > 
> > > I hadn't seen this comment before sending v2.
> > > 
> > >>>
> > >>>> If driver is broken for 4-bit ECC, please fix that up first.
> > >>>>
> > >>> Since this issue is completely separate from my DT improvements
> > >>> I'll stick to resubmitting the series, applying my LCDK changes to the
> > >>> EVM too, besides you'll be able to compare the behavior without ECC
> > >>> discrepancies.
> > >>> I took note that you are likely to not apply without the ECC fix.
> > >>
> > >> Yeah, I would not like to apply with 1-bit ECC now and then change to
> > >> 4-bit ECC soon after.
> > >>
> > > 
> > > Both mityomapl138 from mainline and hawkboard from TI's BSP release
> > > include the comment:
> > > - "4 bit mode is not supported with 16 bit NAND"
> > > It is not clear whether they imply that the HW has issues or if it's SW
> > 
> > At least the TRM says "The EMIFA supports 4-bit ECC on 8-bit/16-bit NAND
> > Flash.". And then the TRM goes on to describe how 4-bit ECC is supposed
> > to work for 16-bit NAND. I could not find any errata related to this in
> > the errata document.
> > 
> > > only, but 4-bits ECC is a different matter and I hope you'll integrate
> > > the current changes prior to tackling it.
> > 
> > Lets apply everything together. This is a new feature, not a bug fix.
> > There is no need to rush it. Also, for the NAND part on LCDK, per the
> > micron datasheet, minimum of 4-bit ECC is needed.
> > 
> 
> FYI my patches are related to a small contracting work spanning 20 days
> amounting to 15 real days (from scratch).  ATM my SoW not only doesn't
> include this 'new feature' but excludes it. So I cannot promise I'll
> look into it, though it shouldn't be too big a thing.

I took a look at it yesterday, cf. https://lkml.org/lkml/2016/8/29/71
 
Karl

> It seemed clear that you wanted to squeeze it with the rest, but after
> setting things up for the EVM and observing mainline was broken with
> 4bits ECC on an 8bits NAND I thought it was impossible nobody hadn't
> noticed it .. so I dug in the mail archive, only to find there had
> already been some alerts raised some months ago on the list, so I picked
> up my next item.
> 
> > I will take a look at other patches in this series.
> > 
> 
> Ok, thanks.
> 
> Rgds, 
> Karl

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

[Karl Beldan]

On Tue, Aug 16, 2016 at 11:20:29PM +0000, Karl Beldan wrote:
> On Wed, Aug 10, 2016 at 05:23:23PM +0530, Sekhar Nori wrote:
> > On Wednesday 10 August 2016 04:49 PM, Karl Beldan wrote:
> > > On Wed, Aug 10, 2016 at 03:01:30PM +0530, Sekhar Nori wrote:
> > >> On Wednesday 10 August 2016 02:34 PM, Karl Beldan wrote:
> > >>> On Wed, Aug 10, 2016 at 02:01:57PM +0530, Sekhar Nori wrote:
> > >>>> On Tuesday 09 August 2016 10:45 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.
> > >>>>
> > >>>> I disagree that we need to be compatible to the software that ships with
> > >>>> the board. Thats software was last updated 3 years ago. Instead I would
> > >>>> concern with what the hardware supports. So, if the hardware can support
> > >>>> 4-bit ECC, I would use that.
> > >>>>
> > >>> I am not saying we _need_ to be compatible.
> > >>
> > >> Alright then, please drop references to what software the board ships
> > >> with in the commit message and in the patch itself.
> > >>
> > > 
> > > I hadn't seen this comment before sending v2.
> > > 
> > >>>
> > >>>> If driver is broken for 4-bit ECC, please fix that up first.
> > >>>>
> > >>> Since this issue is completely separate from my DT improvements
> > >>> I'll stick to resubmitting the series, applying my LCDK changes to the
> > >>> EVM too, besides you'll be able to compare the behavior without ECC
> > >>> discrepancies.
> > >>> I took note that you are likely to not apply without the ECC fix.
> > >>
> > >> Yeah, I would not like to apply with 1-bit ECC now and then change to
> > >> 4-bit ECC soon after.
> > >>
> > > 
> > > Both mityomapl138 from mainline and hawkboard from TI's BSP release
> > > include the comment:
> > > - "4 bit mode is not supported with 16 bit NAND"
> > > It is not clear whether they imply that the HW has issues or if it's SW
> > 
> > At least the TRM says "The EMIFA supports 4-bit ECC on 8-bit/16-bit NAND
> > Flash.". And then the TRM goes on to describe how 4-bit ECC is supposed
> > to work for 16-bit NAND. I could not find any errata related to this in
> > the errata document.
> > 
> > > only, but 4-bits ECC is a different matter and I hope you'll integrate
> > > the current changes prior to tackling it.
> > 
> > Lets apply everything together. This is a new feature, not a bug fix.
> > There is no need to rush it. Also, for the NAND part on LCDK, per the
> > micron datasheet, minimum of 4-bit ECC is needed.
> > 
> 
> FYI my patches are related to a small contracting work spanning 20 days
> amounting to 15 real days (from scratch).  ATM my SoW not only doesn't
> include this 'new feature' but excludes it. So I cannot promise I'll
> look into it, though it shouldn't be too big a thing.

I took a look at it yesterday, cf. https://lkml.org/lkml/2016/8/29/71
 
Karl

> It seemed clear that you wanted to squeeze it with the rest, but after
> setting things up for the EVM and observing mainline was broken with
> 4bits ECC on an 8bits NAND I thought it was impossible nobody hadn't
> noticed it .. so I dug in the mail archive, only to find there had
> already been some alerts raised some months ago on the list, so I picked
> up my next item.
> 
> > I will take a look at other patches in this series.
> > 
> 
> Ok, thanks.
> 
> Rgds, 
> Karl
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Karl Beldan]

On Tue, Aug 16, 2016 at 11:20:29PM +0000, Karl Beldan wrote:
> On Wed, Aug 10, 2016 at 05:23:23PM +0530, Sekhar Nori wrote:
> > On Wednesday 10 August 2016 04:49 PM, Karl Beldan wrote:
> > > On Wed, Aug 10, 2016 at 03:01:30PM +0530, Sekhar Nori wrote:
> > >> On Wednesday 10 August 2016 02:34 PM, Karl Beldan wrote:
> > >>> On Wed, Aug 10, 2016 at 02:01:57PM +0530, Sekhar Nori wrote:
> > >>>> On Tuesday 09 August 2016 10:45 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.
> > >>>>
> > >>>> I disagree that we need to be compatible to the software that ships with
> > >>>> the board. Thats software was last updated 3 years ago. Instead I would
> > >>>> concern with what the hardware supports. So, if the hardware can support
> > >>>> 4-bit ECC, I would use that.
> > >>>>
> > >>> I am not saying we _need_ to be compatible.
> > >>
> > >> Alright then, please drop references to what software the board ships
> > >> with in the commit message and in the patch itself.
> > >>
> > > 
> > > I hadn't seen this comment before sending v2.
> > > 
> > >>>
> > >>>> If driver is broken for 4-bit ECC, please fix that up first.
> > >>>>
> > >>> Since this issue is completely separate from my DT improvements
> > >>> I'll stick to resubmitting the series, applying my LCDK changes to the
> > >>> EVM too, besides you'll be able to compare the behavior without ECC
> > >>> discrepancies.
> > >>> I took note that you are likely to not apply without the ECC fix.
> > >>
> > >> Yeah, I would not like to apply with 1-bit ECC now and then change to
> > >> 4-bit ECC soon after.
> > >>
> > > 
> > > Both mityomapl138 from mainline and hawkboard from TI's BSP release
> > > include the comment:
> > > - "4 bit mode is not supported with 16 bit NAND"
> > > It is not clear whether they imply that the HW has issues or if it's SW
> > 
> > At least the TRM says "The EMIFA supports 4-bit ECC on 8-bit/16-bit NAND
> > Flash.". And then the TRM goes on to describe how 4-bit ECC is supposed
> > to work for 16-bit NAND. I could not find any errata related to this in
> > the errata document.
> > 
> > > only, but 4-bits ECC is a different matter and I hope you'll integrate
> > > the current changes prior to tackling it.
> > 
> > Lets apply everything together. This is a new feature, not a bug fix.
> > There is no need to rush it. Also, for the NAND part on LCDK, per the
> > micron datasheet, minimum of 4-bit ECC is needed.
> > 
> 
> FYI my patches are related to a small contracting work spanning 20 days
> amounting to 15 real days (from scratch).  ATM my SoW not only doesn't
> include this 'new feature' but excludes it. So I cannot promise I'll
> look into it, though it shouldn't be too big a thing.

I took a look at it yesterday, cf. https://lkml.org/lkml/2016/8/29/71
 
Karl

> It seemed clear that you wanted to squeeze it with the rest, but after
> setting things up for the EVM and observing mainline was broken with
> 4bits ECC on an 8bits NAND I thought it was impossible nobody hadn't
> noticed it .. so I dug in the mail archive, only to find there had
> already been some alerts raised some months ago on the list, so I picked
> up my next item.
> 
> > I will take a look at other patches in this series.
> > 
> 
> Ok, thanks.
> 
> Rgds, 
> Karl