[PATCH v3 0/4] Add DT support for NAND to LCDK via ti-aemif

[PATCH v3 0/4] Add DT support for NAND to LCDK via ti-aemif

[Karl Beldan]

Hi,

Here is another spin. I could get my hands on an EVM and test the
changes on it as well. Doing so, I could also observe that 4bit ECC
on 8bits NANDS is currently broken in mailine. Apparently there had
already been some alerts on the mailing list some months ago.

Rgds, 
Karl

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

Changes from v2:
 - remove now useless pins of nand_cs3 in v3_3/4


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

 arch/arm/boot/dts/da850-evm.dts        |  49 +++++++++++++--
 arch/arm/boot/dts/da850-lcdk.dts       | 108 +++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/da850.dtsi           |  35 +++--------
 arch/arm/configs/davinci_all_defconfig |   2 +
 arch/arm/mach-davinci/da850.c          |   1 +
 arch/arm/mach-davinci/da8xx-dt.c       |   1 +
 6 files changed, 164 insertions(+), 32 deletions(-)

-- 
2.9.2

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

[Karl Beldan]

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

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

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

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

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

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

[Karl Beldan]

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

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

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

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

[Karl Beldan]

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

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

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

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

[Karl Beldan]

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

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..fca5336 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";
+	cs3 {
+		#address-cells = <2>;
+		#size-cells = <1>;
+		clock-ranges;
+		ranges;
+
+		ti,cs-chipselect = <3>;
+
+		nand at 2000000,0 {
+			compatible = "ti,davinci-nand";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			reg = <0 0x02000000 0x02000000
+			       1 0x00000000 0x00008000>;
+
+			ti,davinci-chipselect = <1>;
+			ti,davinci-mask-ale = <0>;
+			ti,davinci-mask-cle = <0>;
+			ti,davinci-mask-chipsel = <0>;
+
+			/*
+			 * nand_ecc_strength_good will emit a warning
+			 * but the LCDK ships with these settings [1].
+			 * Also HW 4bits ECC with 16bits NAND seems to
+			 * require some attention, and it doesn't work
+			 * in mainline for 8bits ones.
+			 *
+			 * ATM nand_davinci_probe handling of nand-ecc-*
+			 * is broken, e.g.
+			 * chip.ecc.strength = pdata->ecc_bits occurs after
+			 * scan_ident(), otherwise I would have used:
+			 * 	nand-ecc-mode = "hw";
+			 * 	nand-ecc-strength = <1>;
+			 * 	nand-ecc-step-size = <512>;
+			 */
+			ti,davinci-nand-buswidth = <16>;
+			ti,davinci-ecc-mode = "hw";
+			ti,davinci-ecc-bits = <1>;
+			ti,davinci-nand-use-bbt;
+
+			/*
+			 * LCDK original partitions:
+			 * 0x000000000000-0x000000020000 : "u-boot env"
+			 * 0x000000020000-0x0000000a0000 : "u-boot"
+			 * 0x0000000a0000-0x0000002a0000 : "kernel"
+			 * 0x0000002a0000-0x000020000000 : "filesystem"
+			 *
+			 * The 1st NAND block being guaranted to be valid w/o ECC (> 1k cycles),
+			 * it makes a perfect candidate as an SPL for the BootROM to jump to.
+			 * However the OMAP-L132/L138 Bootloader doc SPRAB41E reads:
+			 * "To boot from NAND Flash, the AIS should be written to NAND block 1
+			 * (NAND block 0 is not used by default)", which matches the LCDK
+			 * original partitioning.
+			 * Also, the LCDK ships with only the u-boot partition provisioned and
+			 * boots on it in its default configuration while using the MMC for the
+			 * kernel and rootfs, so preserve that one as is for now.
+			 * [1]: Ensuring for example that U-Boot LCDK SPL can handle it properly
+			 * and a proper boot chain ROM->SPL->U-Boot->Linux wrt ECC, would allow
+			 * for a better partitioning.
+			 */
+			partitions {
+				compatible = "fixed-partitions";
+				#address-cells = <1>;
+				#size-cells = <1>;
+
+				partition at 0 {
+					label = "u-boot env";
+					reg = <0 0x020000>;
+				};
+				partition at 0x020000 {
+					/* The LCDK defaults to booting from this partition */
+					label = "u-boot";
+					reg = <0x020000 0x080000>;
+				};
+				partition at 0x0a0000 {
+					label = "space";
+					reg = <0x0a0000 0>;
+				};
+			};
+		};
+	};
+};
-- 
2.9.2

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

[Sekhar Nori]

On Wednesday 17 August 2016 04:03 AM, Karl Beldan wrote:
> Many davinci boards (da830 and da850 families) don't have their clocks
> in DT yet and won't be successful in getting an unnamed aemif clock

Actually none of DaVinci platforms have clocks in DT yet.

> without explicitly registering them via clk_lookups, failing the
> ti-aemif memory driver probe.

I am happy with the patch itself. But I think the terminology used in
the commit message can be made more accurate. clk_get() does not look up
a clock by name. It looks up a clock by consumer device and a consumer
id (used for multiple clocks used by same consumer device). The AEMIF
clock in DaVinci has a name already. Its assigned in da850.c as "aemif".
But the clock name itself does not matter in clock lookup.

So, IMO, saying "won't be successful in getting an unnamed aemif clock"
is inaccurate.

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

So the existing look-up does not recognize the AEMIF as a device (NULL
device name) and is using a "global" consumer id to look-up
"device-less" clocks. As you noted, this entry should remain for non-DT
mode and for backward compatibility.

> This change will allow to switch from the mach-davinci aemif code to
> the ti-aemif memory driver.
> 
> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>

Thanks,
Sekhar

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

[Sekhar Nori]

On Wednesday 17 August 2016 04:03 AM, Karl Beldan wrote:
> 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>

Applied to v4.9/defconfig.

Regards,
Sekhar

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

[Karl Beldan]

On Wed, Aug 17, 2016 at 08:15:41PM +0530, Sekhar Nori wrote:
> On Wednesday 17 August 2016 04:03 AM, Karl Beldan wrote:
> > Many davinci boards (da830 and da850 families) don't have their clocks
> > in DT yet and won't be successful in getting an unnamed aemif clock
> 
> Actually none of DaVinci platforms have clocks in DT yet.
> 

Indeed, I haven't got used to the TI platforms, I mistook some omap SoCs
for davinci ones.

> > without explicitly registering them via clk_lookups, failing the
> > ti-aemif memory driver probe.
> 
> I am happy with the patch itself. But I think the terminology used in
> the commit message can be made more accurate. clk_get() does not look up
> a clock by name. It looks up a clock by consumer device and a consumer
> id (used for multiple clocks used by same consumer device). The AEMIF
> clock in DaVinci has a name already. Its assigned in da850.c as "aemif".
> But the clock name itself does not matter in clock lookup.
> 

I will be happy to send a more accurate comment, here is my case for the
record and the feedback, although I try to keep my distance from
comments of comments ;).

Checking clk_get:

struct clk *clk_get(struct device *dev, const char *con_id)
{       
	[...]
	if (dev) {
		struct clk *__of_clk_get_by_name(struct device_node *np,
						 const char *dev_id,
						 const char *name)
		clk = __of_clk_get_by_name(dev->of_node, dev_id, con_id);
		[...]
	}
	return clk_get_sys(dev_id, con_id);
}

In DT case the con_id _is_ the clock name, so the assertion "clk_get()
does not look up a clock by name" would be false ?
Also, numerous commits refer to *clk_get(*, NULL) as getting an unnamed
clock, although it only really is accurate to the point in DT cases.

> So, IMO, saying "won't be successful in getting an unnamed aemif clock"
> is inaccurate.
> 

You are right, it is innacurate, because in that case it won't try
getting such a clock, ie. by name. Instead it will resort to looking it
up by dev_id / con_id, connecting back with your point.

I will resend the series with this commit message reworded.

Rgds, 
Karl

> > The current aemif lookup entry resolving to the same clock:
> >     'CLK(NULL,               "aemif",        &aemif_clk)'
> > remains, as it is currently used (davinci_nand is getting a named clock
> > "aemif").
> 
> So the existing look-up does not recognize the AEMIF as a device (NULL
> device name) and is using a "global" consumer id to look-up
> "device-less" clocks. As you noted, this entry should remain for non-DT
> mode and for backward compatibility.
> 
> > This change will allow to switch from the mach-davinci aemif code to
> > the ti-aemif memory driver.
> > 
> > Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> 
> Thanks,
> Sekhar

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

[Russell King - ARM Linux]

On Thu, Aug 18, 2016 at 07:33:19AM +0000, Karl Beldan wrote:
> Checking clk_get:
> 
> struct clk *clk_get(struct device *dev, const char *con_id)
> {       
> 	[...]
> 	if (dev) {
> 		struct clk *__of_clk_get_by_name(struct device_node *np,
> 						 const char *dev_id,
> 						 const char *name)
> 		clk = __of_clk_get_by_name(dev->of_node, dev_id, con_id);
> 		[...]
> 	}
> 	return clk_get_sys(dev_id, con_id);
> }
> 
> In DT case the con_id _is_ the clock name, so the assertion "clk_get()
> does not look up a clock by name" would be false ?

Wrong.

	clocks = <&provider 0>, <&provider 1>;
	clock-names = "fck", "ick";

	fck = clk_get(dev, "fck");

	ick = clk_get(dev, "ick");

Works just the same.  The whole point of the string is to identify an
_individual_ _input_ _to_ _the_ _device_ and not a global clock name.

Think what happens if you specify a clock name.  Where does that name
come from?  What if the clock is named differently on another SoC?
With that approach, we need to define a new set of APIs to allow a
device to determine the global clock name for the clock that it's
interested in - which is completely absurd when we've already got
that within clk_get().

The whole point of the clk API is to abstract those differences.  That's
why it takes a _connection_ _id_ and not a clock name.

I really can't understand why people keep getting this wrong in their
heads.  It makes no sense to me for this to take a global clock name.

> Also, numerous commits refer to *clk_get(*, NULL) as getting an unnamed
> clock, although it only really is accurate to the point in DT cases.

Table-driven clkdev copes fine with that, and will try to locate an
appropriate table entry with a NULL connection ID, not matching any
non-NULL connection ID entry.

In the DT case, it's always the first specified clock.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

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

[Sekhar Nori]

On Wednesday 17 August 2016 04:03 AM, Karl Beldan wrote:
> Currently the davinci da8xx boards use the mach-davinci aemif code.
> Instantiating an aemif node into the DT allows to use the ti-aemif
> memory driver and is another step to better DT support.
> This change adds an aemif node in the dtsi while retiring the nand_cs3
> node. The NAND is now instantiated in the dts as a subnode of the aemif
> one along with its pins.
> 
> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>

Applied to v4.9/dts branch of my tree.

Regards,
Sekhar

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

[Karl Beldan]

On Thu, Aug 18, 2016 at 10:04:37AM +0100, Russell King - ARM Linux wrote:
> On Thu, Aug 18, 2016 at 07:33:19AM +0000, Karl Beldan wrote:
> > Checking clk_get:
> > 
> > struct clk *clk_get(struct device *dev, const char *con_id)
> > {       
> > 	[...]
> > 	if (dev) {
> > 		struct clk *__of_clk_get_by_name(struct device_node *np,
> > 						 const char *dev_id,
> > 						 const char *name)
> > 		clk = __of_clk_get_by_name(dev->of_node, dev_id, con_id);
> > 		[...]
> > 	}
> > 	return clk_get_sys(dev_id, con_id);
> > }
> > 
> > In DT case the con_id _is_ the clock name, so the assertion "clk_get()
> > does not look up a clock by name" would be false ?
> 
> Wrong.
> 
> 	clocks = <&provider 0>, <&provider 1>;
> 	clock-names = "fck", "ick";
> 
> 	fck = clk_get(dev, "fck");
> 
> 	ick = clk_get(dev, "ick");
> 
> Works just the same.  The whole point of the string is to identify an
> _individual_ _input_ _to_ _the_ _device_ and not a global clock name.
> 

I hope so.
You are matching con_id with _a_ clock name, that's my point.

Maybe my writing 'the clock name' instead of 'a clock name' made you
deduce I was asserting in some way that the clock tree was addressed
by some kind of a unique root namespace via the clock-names ?? It was
not the case ('the' was contextual if that means anything).

> Think what happens if you specify a clock name.  Where does that name
> come from?  What if the clock is named differently on another SoC?
> With that approach, we need to define a new set of APIs to allow a
> device to determine the global clock name for the clock that it's
> interested in - which is completely absurd when we've already got
> that within clk_get().
> 
> The whole point of the clk API is to abstract those differences.  That's
> why it takes a _connection_ _id_ and not a clock name.
> 
> I really can't understand why people keep getting this wrong in their
> heads.  It makes no sense to me for this to take a global clock name.
> 

Nope, I am fine with that, it is a convenient scheme.

> > Also, numerous commits refer to *clk_get(*, NULL) as getting an unnamed
> > clock, although it only really is accurate to the point in DT cases.
> 
> Table-driven clkdev copes fine with that, and will try to locate an
> appropriate table entry with a NULL connection ID, not matching any
> non-NULL connection ID entry.
> 

Yes it does, my change relies on that and I was happy to be able to so.

It is good to see the maintainers like Sekhar and you seizing the
opportunity to throw light on such things.
 
Karl

> In the DT case, it's always the first specified clock.
> 
> -- 
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.

[PATCH v3 0/4] Add DT support for NAND to LCDK via ti-aemif

[Kevin Hilman]

Karl Beldan <kbeldan@baylibre.com> writes:

> Here is another spin. I could get my hands on an EVM and test the
> changes on it as well. Doing so, I could also observe that 4bit ECC
> on 8bits NANDS is currently broken in mailine. Apparently there had
> already been some alerts on the mailing list some months ago.

[...]

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

Tested on DA850-LCDK with UBIFS.

Tested-by: Kevin Hilman <khilman@baylibre.com>

Kevin

[PATCH v3 0/4] Add DT support for NAND to LCDK via ti-aemif

[Kevin Hilman]

Kevin Hilman <khilman@baylibre.com> writes:

> Karl Beldan <kbeldan@baylibre.com> writes:
>
>> Here is another spin. I could get my hands on an EVM and test the
>> changes on it as well. Doing so, I could also observe that 4bit ECC
>> on 8bits NANDS is currently broken in mailine. Apparently there had
>> already been some alerts on the mailing list some months ago.
>
> [...]
>
>> Karl Beldan (4):
>>   ARM: davinci: da8xx-dt: Add ti-aemif lookup for clock matching
>>   ARM: davinci_all_defconfig: Enable AEMIF as a module
>>   ARM: dts: da850,da850-evm: Add an aemif node and use it for the NAND
>>   ARM: dts: da850-lcdk: Add NAND to DT
>
> Tested on DA850-LCDK with UBIFS.
>
> Tested-by: Kevin Hilman <khilman@baylibre.com>
>

I forgot to mention that I had to enable:

CONFIG_UBIFS_FS=y
CONFIG_MTD_UBI=y

in order to test with ubifs. Care to send a patch adding these as
modules to davinci_all_defconfig?

Thanks,

Kevin

[PATCH v3 0/4] Add DT support for NAND to LCDK via ti-aemif

[Karl Beldan]

On Thu, Aug 18, 2016 at 04:15:59PM -0500, Kevin Hilman wrote:
> Kevin Hilman <khilman@baylibre.com> writes:
> 
> > Karl Beldan <kbeldan@baylibre.com> writes:
> >
> >> Here is another spin. I could get my hands on an EVM and test the
> >> changes on it as well. Doing so, I could also observe that 4bit ECC
> >> on 8bits NANDS is currently broken in mailine. Apparently there had
> >> already been some alerts on the mailing list some months ago.
> >
> > [...]
> >
> >> Karl Beldan (4):
> >>   ARM: davinci: da8xx-dt: Add ti-aemif lookup for clock matching
> >>   ARM: davinci_all_defconfig: Enable AEMIF as a module
> >>   ARM: dts: da850,da850-evm: Add an aemif node and use it for the NAND
> >>   ARM: dts: da850-lcdk: Add NAND to DT
> >
> > Tested on DA850-LCDK with UBIFS.
> >
> > Tested-by: Kevin Hilman <khilman@baylibre.com>
> >
> 
> I forgot to mention that I had to enable:
> 
> CONFIG_UBIFS_FS=y
> CONFIG_MTD_UBI=y
> 
> in order to test with ubifs. Care to send a patch adding these as
> modules to davinci_all_defconfig?
> 

Sure, I'll add it to the series when resending.
 
Karl

[PATCH v3 0/4] Add DT support for NAND to LCDK via ti-aemif

[Sekhar Nori]

On Friday 19 August 2016 07:16 PM, Karl Beldan wrote:

> Sure, I'll add it to the series when resending.

I just pushed an updated master branch on my tree with all the patches
accepted so far. Can you please rebase against that branch while sending?

Regards,
Sekhar

[PATCH v3 0/4] Add DT support for NAND to LCDK via ti-aemif

[Karl Beldan]

On Fri, Aug 19, 2016 at 07:40:35PM +0530, Sekhar Nori wrote:
> On Friday 19 August 2016 07:16 PM, Karl Beldan wrote:
> 
> > Sure, I'll add it to the series when resending.
> 
> I just pushed an updated master branch on my tree with all the patches
> accepted so far. Can you please rebase against that branch while sending?
> 

I rebased and sent
  [1] [PATCH] ARM: davinci_all_defconfig: Enable some UBI modules
  [2] [PATCH v2] ARM: davinci: da8xx-dt:
                 Add ti-aemif lookup for clock matching

Unfortunately while automating I messed up and sent both twice and [2] as
a reply to [1] while I intended to send them separately and preferably
[1] after [2].

You've already applied patches 2/4 and 3/4 of the original series
while 3/4 depends on [2] and 4/4 is on hold because of the ECC issue.

Please tell me if you want to deal with these patches or prefer a resend.

Regards, 
Karl