devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ARM: dts: da850-lcdk: Add NAND to DT
@ 2016-08-29 15:32 Karl Beldan
       [not found] ` <20160829153242.18615-1-kbeldan-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
  0 siblings, 1 reply; 6+ messages in thread
From: Karl Beldan @ 2016-08-29 15:32 UTC (permalink / raw)
  To: devicetree, linux-arm-kernel
  Cc: Mark Rutland, Karl Beldan, Kevin Hilman, Sekhar Nori,
	Russell King, Rob Herring, Karl Beldan

This adds DT support for the NAND connected to the SoC AEMIF.
Passed torture hashing a 40MB file on top of UBIFS using subpages.

Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
---
v2:

- Removed comments pertaining to the BSP the LCDK ships with (Sekhar)
- s/"ok"/"okay"/
- Removed partitions since it relates to the BSP the LCDK ships with and
  also removed the references to the shortcomings of davinci_nand wrt DT
  parsing vs scan_ident (use of ti,davinci-ecc* vs nand-ecc*)
- use 4-bit ECC (since I have sent a fix for the driver)

 arch/arm/boot/dts/da850-lcdk.dts | 53 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
index 7563260..1665eee 100644
--- a/arch/arm/boot/dts/da850-lcdk.dts
+++ b/arch/arm/boot/dts/da850-lcdk.dts
@@ -63,6 +63,27 @@
 			0x04 0x00000110 0x00000ff0
 		>;
 	};
+
+	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 {
@@ -136,3 +157,35 @@
 	tx-num-evt = <32>;
 	rx-num-evt = <32>;
 };
+
+&aemif {
+	pinctrl-names = "default";
+	pinctrl-0 = <&nand_pins>;
+	status = "okay";
+	cs3 {
+		#address-cells = <2>;
+		#size-cells = <1>;
+		clock-ranges;
+		ranges;
+
+		ti,cs-chipselect = <3>;
+
+		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>;
+
+			ti,davinci-nand-buswidth = <16>;
+			ti,davinci-ecc-mode = "hw";
+			ti,davinci-ecc-bits = <4>;
+			ti,davinci-nand-use-bbt;
+		};
+	};
+};
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] ARM: dts: da850-lcdk: Add NAND to DT
       [not found] ` <20160829153242.18615-1-kbeldan-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
@ 2016-08-31 23:44   ` Kevin Hilman
       [not found]     ` <7h37lk5wrd.fsf-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
  0 siblings, 1 reply; 6+ messages in thread
From: Kevin Hilman @ 2016-08-31 23:44 UTC (permalink / raw)
  To: Karl Beldan
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Rob Herring,
	Mark Rutland, Russell King, Sekhar Nori, Karl Beldan

Karl Beldan <kbeldan-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> writes:

> This adds DT support for the NAND connected to the SoC AEMIF.
> Passed torture hashing a 40MB file on top of UBIFS using subpages.
>
> Signed-off-by: Karl Beldan <kbeldan-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> ---
> v2:
>
> - Removed comments pertaining to the BSP the LCDK ships with (Sekhar)
> - s/"ok"/"okay"/
> - Removed partitions since it relates to the BSP the LCDK ships with

IMO, some default partitions are nice to have, at least for u-boot and
u-boot env so factory-shipped u-boot is not accidentally overridden.
The left over space can be labeled "free space".

Anyways, after adding some partitions back, I tested with ubifs on my
LCDK:

Tested-by: Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ARM: dts: da850-lcdk: Add NAND to DT
       [not found]     ` <7h37lk5wrd.fsf-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
@ 2016-09-01  5:17       ` Sekhar Nori
       [not found]         ` <3945712b-d02f-d97a-2356-9383d7fe1984-l0cyMroinI0@public.gmane.org>
  0 siblings, 1 reply; 6+ messages in thread
From: Sekhar Nori @ 2016-09-01  5:17 UTC (permalink / raw)
  To: Kevin Hilman, Karl Beldan
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Rob Herring,
	Mark Rutland, Russell King, Karl Beldan

On Thursday 01 September 2016 05:14 AM, Kevin Hilman wrote:
> Karl Beldan <kbeldan-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> writes:
> 
>> This adds DT support for the NAND connected to the SoC AEMIF.
>> Passed torture hashing a 40MB file on top of UBIFS using subpages.
>>
>> Signed-off-by: Karl Beldan <kbeldan-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>> ---
>> v2:
>>
>> - Removed comments pertaining to the BSP the LCDK ships with (Sekhar)
>> - s/"ok"/"okay"/
>> - Removed partitions since it relates to the BSP the LCDK ships with
> 
> IMO, some default partitions are nice to have, at least for u-boot and
> u-boot env so factory-shipped u-boot is not accidentally overridden.
> The left over space can be labeled "free space".
> 
> Anyways, after adding some partitions back, I tested with ubifs on my
> LCDK:

This[1] is what I had commented on the partitions before. I did not 
mean to remove partitions altogether, just comments referring to 
compatibility with pre-flashed software shipping with LCDK (since that 
changes without notice).

Apologies for any confusion caused. The "Rest of the patch looks good
to me" in the end was to say that the partitions themselves look fine :)

Can you please submit a v2 with partitions added?

Thanks,
Sekhar

[1]

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

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

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

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

> +			partitions {
> +				compatible = "fixed-partitions";
> +				#address-cells = <1>;
> +				#size-cells = <1>;
> +
> +				partition@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>;
> +				};
> +			};
> +		};
> +	};
> +};

Rest of the patch looks good to me.

Regards,
Sekhar
--
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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ARM: dts: da850-lcdk: Add NAND to DT
       [not found]         ` <3945712b-d02f-d97a-2356-9383d7fe1984-l0cyMroinI0@public.gmane.org>
@ 2016-09-01 10:00           ` Karl Beldan
  2016-09-01 10:16             ` Sekhar Nori
  0 siblings, 1 reply; 6+ messages in thread
From: Karl Beldan @ 2016-09-01 10:00 UTC (permalink / raw)
  To: Sekhar Nori, Kevin Hilman
  Cc: Karl Beldan, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Rob Herring,
	Mark Rutland, Russell King, Karl Beldan

On Thu, Sep 01, 2016 at 10:47:04AM +0530, Sekhar Nori wrote:
> On Thursday 01 September 2016 05:14 AM, Kevin Hilman wrote:
> > Karl Beldan <kbeldan-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> writes:
> > 
> >> This adds DT support for the NAND connected to the SoC AEMIF.
> >> Passed torture hashing a 40MB file on top of UBIFS using subpages.
> >>
> >> Signed-off-by: Karl Beldan <kbeldan-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> >> ---
> >> v2:
> >>
> >> - Removed comments pertaining to the BSP the LCDK ships with (Sekhar)
> >> - s/"ok"/"okay"/
> >> - Removed partitions since it relates to the BSP the LCDK ships with
> > 
> > IMO, some default partitions are nice to have, at least for u-boot and
> > u-boot env so factory-shipped u-boot is not accidentally overridden.
> > The left over space can be labeled "free space".
> > 
> > Anyways, after adding some partitions back, I tested with ubifs on my
> > LCDK:
> 
> This[1] is what I had commented on the partitions before. I did not 
> mean to remove partitions altogether, just comments referring to 
> compatibility with pre-flashed software shipping with LCDK (since that 
> changes without notice).
> 
> Apologies for any confusion caused. The "Rest of the patch looks good
> to me" in the end was to say that the partitions themselves look fine :)
> 

None necessary, I just pushed the logic further on my own as specified
in the changelog. I can add the partitions back as I don't have any
strong preference on this, but not without mentioning that it is because
that is what the LCDK ships with, as in my original version, please see
if the adjustments below accomodate your preferences.

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

I'd add [1]:
Same doc reads for ROM "Silicon Revision 2.1", "Updated NAND boot mode
to offer boot from block 0 or block 1", (Sekhar:) "but needs new boot pin
settings not possible in stock LCDK".

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

If you want me to keep the partitions, I would change that to [2] "Here
we keep the first two provisioned default partitions and labels the LCDK
ships with as is." (although FWIU it conflicts with your request above).

 
Karl

> > +			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>;
> > +				};
> > +			};
> > +		};
> > +	};
> > +};
> 
> Rest of the patch looks good to me.
> 
> Regards,
> Sekhar
--
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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ARM: dts: da850-lcdk: Add NAND to DT
  2016-09-01 10:00           ` Karl Beldan
@ 2016-09-01 10:16             ` Sekhar Nori
       [not found]               ` <fdf48f3c-c7f5-4b9b-ff6b-f801b04f3f33-l0cyMroinI0@public.gmane.org>
  0 siblings, 1 reply; 6+ messages in thread
From: Sekhar Nori @ 2016-09-01 10:16 UTC (permalink / raw)
  To: Karl Beldan, Kevin Hilman
  Cc: Karl Beldan, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Rob Herring,
	Mark Rutland, Russell King, Karl Beldan

On Thursday 01 September 2016 03:30 PM, Karl Beldan wrote:
> On Thu, Sep 01, 2016 at 10:47:04AM +0530, Sekhar Nori wrote:
>> On Thursday 01 September 2016 05:14 AM, Kevin Hilman wrote:
>>> Karl Beldan <kbeldan-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> writes:
>>>
>>>> This adds DT support for the NAND connected to the SoC AEMIF.
>>>> Passed torture hashing a 40MB file on top of UBIFS using subpages.
>>>>
>>>> Signed-off-by: Karl Beldan <kbeldan-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>>>> ---
>>>> v2:
>>>>
>>>> - Removed comments pertaining to the BSP the LCDK ships with (Sekhar)
>>>> - s/"ok"/"okay"/
>>>> - Removed partitions since it relates to the BSP the LCDK ships with
>>>
>>> IMO, some default partitions are nice to have, at least for u-boot and
>>> u-boot env so factory-shipped u-boot is not accidentally overridden.
>>> The left over space can be labeled "free space".
>>>
>>> Anyways, after adding some partitions back, I tested with ubifs on my
>>> LCDK:
>>
>> This[1] is what I had commented on the partitions before. I did not 
>> mean to remove partitions altogether, just comments referring to 
>> compatibility with pre-flashed software shipping with LCDK (since that 
>> changes without notice).
>>
>> Apologies for any confusion caused. The "Rest of the patch looks good
>> to me" in the end was to say that the partitions themselves look fine :)
>>
> 
> None necessary, I just pushed the logic further on my own as specified
> in the changelog. I can add the partitions back as I don't have any
> strong preference on this, but not without mentioning that it is because
> that is what the LCDK ships with, as in my original version, please see
> if the adjustments below accomodate your preferences.
> 
>> Can you please submit a v2 with partitions added?
>>
>> Thanks,
>> Sekhar
>>
>> [1]
>>
>>> +
>>> +			/*
>>> +			 * LCDK original partitions:
>>> +			 * 0x000000000000-0x000000020000 : "u-boot env"
>>> +			 * 0x000000020000-0x0000000a0000 : "u-boot"
>>> +			 * 0x0000000a0000-0x0000002a0000 : "kernel"
>>> +			 * 0x0000002a0000-0x000020000000 : "filesystem"
>>> +			 *
>>> +			 * The 1st NAND block being guaranted to be valid w/o ECC (> 1k cycles),
>>> +			 * it makes a perfect candidate as an SPL for the BootROM to jump to.
>>> +			 * However the OMAP-L132/L138 Bootloader doc SPRAB41E reads:
>>> +			 * "To boot from NAND Flash, the AIS should be written to NAND block 1
>>> +			 * (NAND block 0 is not used by default)", which matches the LCDK
>>> +			 * original partitioning.
>>
>> FWIW, silicon version 2.1 supports booting from block 0, but needs new
>> boot pin settings not possible in stock LCDK.
>>
> 
> I'd add [1]:
> Same doc reads for ROM "Silicon Revision 2.1", "Updated NAND boot mode
> to offer boot from block 0 or block 1", (Sekhar:) "but needs new boot pin
> settings not possible in stock LCDK".
> 
>>> +			 * Also, the LCDK ships with only the u-boot partition provisioned and
>>> +			 * boots on it in its default configuration while using the MMC for the
>>> +			 * kernel and rootfs, so preserve that one as is for now.
>>> +			 * [1]: Ensuring for example that U-Boot LCDK SPL can handle it properly
>>> +			 * and a proper boot chain ROM->SPL->U-Boot->Linux wrt ECC, would allow
>>> +			 * for a better partitioning.
>>> +			 */
>>
>> I would rather not refer to the software that LCDK ships with in the
>> comments at all. Because that can change without notice. At some point,
>> if mainline kernel works well enough on LCDK, TI might decide to ship
>> LCDKs with mainline kernel flashed.
>>
> 
> If you want me to keep the partitions, I would change that to [2] "Here
> we keep the first two provisioned default partitions and labels the LCDK
> ships with as is." (although FWIU it conflicts with your request above).

The reason for choosing the two partitions like the way you do is not
because "LCDK ships with as is", but because:

a) LCDK cannot book from block 0 (needs a different boot mode)

b) Block 0 is guaranteed to be good and capable of ~1000 erase cycles on
most NANDs so using it to store environment variables makes sense.
Especially on development boards where environment might be updated
frequently during course of usage. Coupled with the fact that U-Boot
does not implement any sort of wear-leveling on environment block (AFAIK).

c) If block 0 is dedicated to env, block 1 is the next logical block to
be used for u-boot.ais

d) rest of the space is left to be used as the user deems fit.

I still disagree with any references to legacy software that ships with
LCDK. The partitioning scheme is used because it makes most sense. Not
because the board ships with it.

Thanks,
Sekhar
--
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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ARM: dts: da850-lcdk: Add NAND to DT
       [not found]               ` <fdf48f3c-c7f5-4b9b-ff6b-f801b04f3f33-l0cyMroinI0@public.gmane.org>
@ 2016-09-01 13:16                 ` Karl Beldan
  0 siblings, 0 replies; 6+ messages in thread
From: Karl Beldan @ 2016-09-01 13:16 UTC (permalink / raw)
  To: Sekhar Nori
  Cc: Karl Beldan, Kevin Hilman, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Rob Herring,
	Mark Rutland, Russell King, Karl Beldan

On Thu, Sep 01, 2016 at 03:46:44PM +0530, Sekhar Nori wrote:
> On Thursday 01 September 2016 03:30 PM, Karl Beldan wrote:
> > On Thu, Sep 01, 2016 at 10:47:04AM +0530, Sekhar Nori wrote:
> >> On Thursday 01 September 2016 05:14 AM, Kevin Hilman wrote:
> >>> Karl Beldan <kbeldan-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> writes:
> >>>
> >>>> This adds DT support for the NAND connected to the SoC AEMIF.
> >>>> Passed torture hashing a 40MB file on top of UBIFS using subpages.
> >>>>
> >>>> Signed-off-by: Karl Beldan <kbeldan-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> >>>> ---
> >>>> v2:
> >>>>
> >>>> - Removed comments pertaining to the BSP the LCDK ships with (Sekhar)
> >>>> - s/"ok"/"okay"/
> >>>> - Removed partitions since it relates to the BSP the LCDK ships with
> >>>
> >>> IMO, some default partitions are nice to have, at least for u-boot and
> >>> u-boot env so factory-shipped u-boot is not accidentally overridden.
> >>> The left over space can be labeled "free space".
> >>>
> >>> Anyways, after adding some partitions back, I tested with ubifs on my
> >>> LCDK:
> >>
> >> This[1] is what I had commented on the partitions before. I did not 
> >> mean to remove partitions altogether, just comments referring to 
> >> compatibility with pre-flashed software shipping with LCDK (since that 
> >> changes without notice).
> >>
> >> Apologies for any confusion caused. The "Rest of the patch looks good
> >> to me" in the end was to say that the partitions themselves look fine :)
> >>
> > 
> > None necessary, I just pushed the logic further on my own as specified
> > in the changelog. I can add the partitions back as I don't have any
> > strong preference on this, but not without mentioning that it is because
> > that is what the LCDK ships with, as in my original version, please see
> > if the adjustments below accomodate your preferences.
> > 
> >> Can you please submit a v2 with partitions added?
> >>
> >> Thanks,
> >> Sekhar
> >>
> >> [1]
> >>
> >>> +
> >>> +			/*
> >>> +			 * LCDK original partitions:
> >>> +			 * 0x000000000000-0x000000020000 : "u-boot env"
> >>> +			 * 0x000000020000-0x0000000a0000 : "u-boot"
> >>> +			 * 0x0000000a0000-0x0000002a0000 : "kernel"
> >>> +			 * 0x0000002a0000-0x000020000000 : "filesystem"
> >>> +			 *
> >>> +			 * The 1st NAND block being guaranted to be valid w/o ECC (> 1k cycles),
> >>> +			 * it makes a perfect candidate as an SPL for the BootROM to jump to.
> >>> +			 * However the OMAP-L132/L138 Bootloader doc SPRAB41E reads:
> >>> +			 * "To boot from NAND Flash, the AIS should be written to NAND block 1
> >>> +			 * (NAND block 0 is not used by default)", which matches the LCDK
> >>> +			 * original partitioning.
> >>
> >> FWIW, silicon version 2.1 supports booting from block 0, but needs new
> >> boot pin settings not possible in stock LCDK.
> >>
> > 
> > I'd add [1]:
> > Same doc reads for ROM "Silicon Revision 2.1", "Updated NAND boot mode
> > to offer boot from block 0 or block 1", (Sekhar:) "but needs new boot pin
> > settings not possible in stock LCDK".
> > 
> >>> +			 * Also, the LCDK ships with only the u-boot partition provisioned and
> >>> +			 * boots on it in its default configuration while using the MMC for the
> >>> +			 * kernel and rootfs, so preserve that one as is for now.
> >>> +			 * [1]: Ensuring for example that U-Boot LCDK SPL can handle it properly
> >>> +			 * and a proper boot chain ROM->SPL->U-Boot->Linux wrt ECC, would allow
> >>> +			 * for a better partitioning.
> >>> +			 */
> >>
> >> I would rather not refer to the software that LCDK ships with in the
> >> comments at all. Because that can change without notice. At some point,
> >> if mainline kernel works well enough on LCDK, TI might decide to ship
> >> LCDKs with mainline kernel flashed.
> >>
> > 
> > If you want me to keep the partitions, I would change that to [2] "Here
> > we keep the first two provisioned default partitions and labels the LCDK
> > ships with as is." (although FWIU it conflicts with your request above).
> 
> The reason for choosing the two partitions like the way you do is not
> because "LCDK ships with as is", but because:
> 

This is not exactly what I have written, but this all looks like a drag
to me so I'll pass.

> a) LCDK cannot book from block 0 (needs a different boot mode)
> 
> b) Block 0 is guaranteed to be good and capable of ~1000 erase cycles on
> most NANDs so using it to store environment variables makes sense.
> Especially on development boards where environment might be updated
> frequently during course of usage. Coupled with the fact that U-Boot
> does not implement any sort of wear-leveling on environment block (AFAIK).
> 
> c) If block 0 is dedicated to env, block 1 is the next logical block to
> be used for u-boot.ais
> 
> d) rest of the space is left to be used as the user deems fit.
> 

If I put those HW facts in my original patch before you stated them,
it is obviously because they strongly guide the partitioning, so I will
skip that (except for c) which I address below).

> I still disagree with any references to legacy software that ships with
> LCDK. The partitioning scheme is used because it makes most sense. Not
> because the board ships with it.
> 

I, along with most people I believe, would choose to use what makes most
sense ;).
If you insist on asserting this particular scheme makes most sense and
that is the reason it is used:
- First obviously, if you didn't provision the NAND, I wouldn't provide
  a partition, no matter your desiderata (if I saw your scheme was the
  one making the most sense I would though), would you ?
- Isn't it a matter of preserving the factory-shipped u-boot ?
- Next, you are reserving ~0xa0000 space for U-Boot which itself is
  several 100KBs for the boot ROM to jump to, can't you imagine someone
  else using way less space for an SPL ? Advantages include less chance
  to corrupt the 1st piece of SW the HW jumps to and proper U-Boot (or
  whatever) mirroring.

Some digression:
This morning I hesitated to send a patch to choose a proper OOB layout,
here's what the sprab41e.pdf (BootROM info for OMAP-L132/L138) I found
on the net states wrt OOB layout:
"Each segment of spare bytes contains 6 test bytes and 10 ECC
bytes. For those pages that are checked during bad block detection, all
the test bytes in each segment must equal FFh; any other value indicates
that the page (and its entire block) is bad and should not be used."
U-Boot has a provision to use this scheme, (although it doesn't really
match since it considers the 3 last segment 'test bytes' as free).
What I found is that it is only used for one board only and keystone has
a compile time define to use said layout while having to pay attention
that Linux is using a layout like nand_ooblayout_lp_ops. What makes even
less sense, is that the Linux driver still uses the 10bytes<->8bytes
(un)packing scheme unconditionally which is of no use except for saving
2 bytes per OOB 'section' (ie. 2 bytes / 512 bytes data) which moreover
would otherwise not be used and have no use except for compatibility
with the boot ROM. This has been the case for years.
What also made little sense is how you put during the nand DTS addition
that you would not accept 1bit ECC to switch to 4bit ECC right-away,
while ECC issues have been registered in TI forums with such issues for
years, and is also a 1 (among 17) entries in the MTD section of
bugzilla.kernel.org.
I think I have also raised some other issues along our recent exchanges.

This digression to say that it is a little bit difficult to eat such
nits while reading your "partitioning scheme is used because it makes
most sense.", and not getting a tad annoyed.

I can't remember the name of this guy who said, if you want to drag
research today, just grant more money, but via comitees, I guess it's
the same in many other areas.  It might well be something else, such
nits are sometimes just a pretext, though if they are bona fide, it's
all to your credit to take upon yourself that they might seem so.

It is not comfortable to doubt the attitude of a maintainer but it is
even less so to see them abusing their prerogatives, and do nothing
about it.

On my side I can't thrive on such things, especially what we are
presently discussing so I'll pass.

Thanks, 
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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-09-01 13:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-29 15:32 [PATCH] ARM: dts: da850-lcdk: Add NAND to DT Karl Beldan
     [not found] ` <20160829153242.18615-1-kbeldan-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
2016-08-31 23:44   ` Kevin Hilman
     [not found]     ` <7h37lk5wrd.fsf-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
2016-09-01  5:17       ` Sekhar Nori
     [not found]         ` <3945712b-d02f-d97a-2356-9383d7fe1984-l0cyMroinI0@public.gmane.org>
2016-09-01 10:00           ` Karl Beldan
2016-09-01 10:16             ` Sekhar Nori
     [not found]               ` <fdf48f3c-c7f5-4b9b-ff6b-f801b04f3f33-l0cyMroinI0@public.gmane.org>
2016-09-01 13:16                 ` Karl Beldan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).