[PATCH 0/3] ARM: dts: OMAP3: Use constants with MTD devices

[PATCH 0/3] ARM: dts: OMAP3: Use constants with MTD devices

[Florian Vaussard]

Hello,

Legacy board files use constants from sizes.h and mtd/partitions.h
to declare MTD partitions. This series performs the same with DT.

Necessary headers are added (patch 1), a NAND node is added to
omap3-overo (patch 2), and remaining DTS are converted (patch 3).

Patch 2 was tested on the real hardware. For patch 3, the resulting
DTB were diff'ed. The MTDPART_SIZ_FULL constant was used in DTS, when
it was the case in legacy board files. The size cell is thus changed
inside the binary output, so testing on the hardware is welcome, even
if it should work transparently.

Note that inside omap3430-sdp.dts (nor@0,0), it appears with
this series that partitions 'kernel-nor' and 'filesystem-nor' overlaps
by (2*SZ_128K), which is probably not desired.

Regards,

Florian

Florian Vaussard (3):
  ARM: dts: Add headers with constants for MTD partitions
  ARM: dts: Add omap3-overo NAND flash memory binding
  ARM: dts: OMAP3: Use MTD constants for OMAP3 boards

 arch/arm/boot/dts/omap3-devkit8000.dts |   10 +++---
 arch/arm/boot/dts/omap3-igep0020.dts   |   10 +++---
 arch/arm/boot/dts/omap3-igep0030.dts   |   10 +++---
 arch/arm/boot/dts/omap3-overo.dtsi     |   50 ++++++++++++++++++++++++++++++
 arch/arm/boot/dts/omap3.dtsi           |    2 +
 arch/arm/boot/dts/omap3430-sdp.dts     |   28 ++++++++--------
 include/dt-bindings/mtd/partitions.h   |   12 +++++++
 include/dt-bindings/sizes.h            |   52 ++++++++++++++++++++++++++++++++
 8 files changed, 145 insertions(+), 29 deletions(-)
 create mode 100644 include/dt-bindings/mtd/partitions.h
 create mode 100644 include/dt-bindings/sizes.h

-- 
1.7.5.4

[PATCH 1/3] ARM: dts: Add headers with constants for MTD partitions

[Florian Vaussard]

These constants can be used to easily declare MTD partitions inside
DTS.

The constants MTDPART_OFS_* are purposely not included. Indeed,
parse_ofpart_partitions() is expecting u64, but a DT cell is u32.
Negative constants, as defined by MTDPART_OFS_*, would be wrongly
interpreted by parse_ofpart_partitions(). Two cells should be
used to correctly encode the negative constants, but this breaks
current usage.

Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
---
 include/dt-bindings/mtd/partitions.h |   12 ++++++++
 include/dt-bindings/sizes.h          |   52 ++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 0 deletions(-)
 create mode 100644 include/dt-bindings/mtd/partitions.h
 create mode 100644 include/dt-bindings/sizes.h

diff --git a/include/dt-bindings/mtd/partitions.h b/include/dt-bindings/mtd/partitions.h
new file mode 100644
index 0000000..7dfa676
--- /dev/null
+++ b/include/dt-bindings/mtd/partitions.h
@@ -0,0 +1,12 @@
+/*
+ * This header provides constants used with MTD partitions.
+ */
+
+#ifndef _DT_BINDINGS_MTD_PARTITIONS_H
+#define _DT_BINDINGS_MTD_PARTITIONS_H
+
+/* Partition size */
+#define MTDPART_SIZ_FULL	0
+
+#endif
+
diff --git a/include/dt-bindings/sizes.h b/include/dt-bindings/sizes.h
new file mode 100644
index 0000000..995f2de
--- /dev/null
+++ b/include/dt-bindings/sizes.h
@@ -0,0 +1,52 @@
+/*
+ * This header provides size constants.
+ *
+ * Original version:
+ *   include/linux/sizes.h
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _DT_BINDINGS_SIZES_H
+#define _DT_BINDINGS_SIZES_H
+
+#define SZ_1				0x00000001
+#define SZ_2				0x00000002
+#define SZ_4				0x00000004
+#define SZ_8				0x00000008
+#define SZ_16				0x00000010
+#define SZ_32				0x00000020
+#define SZ_64				0x00000040
+#define SZ_128				0x00000080
+#define SZ_256				0x00000100
+#define SZ_512				0x00000200
+
+#define SZ_1K				0x00000400
+#define SZ_2K				0x00000800
+#define SZ_4K				0x00001000
+#define SZ_8K				0x00002000
+#define SZ_16K				0x00004000
+#define SZ_32K				0x00008000
+#define SZ_64K				0x00010000
+#define SZ_128K				0x00020000
+#define SZ_256K				0x00040000
+#define SZ_512K				0x00080000
+
+#define SZ_1M				0x00100000
+#define SZ_2M				0x00200000
+#define SZ_4M				0x00400000
+#define SZ_8M				0x00800000
+#define SZ_16M				0x01000000
+#define SZ_32M				0x02000000
+#define SZ_64M				0x04000000
+#define SZ_128M				0x08000000
+#define SZ_256M				0x10000000
+#define SZ_512M				0x20000000
+
+#define SZ_1G				0x40000000
+#define SZ_2G				0x80000000
+
+#endif
+
-- 
1.7.5.4

Re: [PATCH 1/3] ARM: dts: Add headers with constants for MTD partitions

[Stephen Warren]

On 06/11/2013 08:48 AM, Florian Vaussard wrote:
> These constants can be used to easily declare MTD partitions inside
> DTS.
> 
> The constants MTDPART_OFS_* are purposely not included. Indeed,
> parse_ofpart_partitions() is expecting u64, but a DT cell is u32.
> Negative constants, as defined by MTDPART_OFS_*, would be wrongly
> interpreted by parse_ofpart_partitions(). Two cells should be
> used to correctly encode the negative constants, but this breaks
> current usage.

I think addition of common headers like this needs an ack from
Grant/Rob. I CC'd them here.

> diff --git a/include/dt-bindings/mtd/partitions.h b/include/dt-bindings/mtd/partitions.h

> + * This header provides constants used with MTD partitions.
...
> +/* Partition size */
> +#define MTDPART_SIZ_FULL	0

Which binding document in Documentation/devicetree/bindings is this
definition associated with? The comment above should really mention
this. Documentation/devicetree/bindings/mtd/partition.txt doesn't seem
to mention this value.

> diff --git a/include/dt-bindings/sizes.h b/include/dt-bindings/sizes.h

...
> +#define SZ_1G				0x40000000
> +#define SZ_2G				0x80000000
> +
> +#endif

For MTD partitions specifically, SZ_4G and onwards would be useful in
theory, although that would end up putting two cell values into a single
macro. and then the values couldn't be added/or'd together. So, I'm not
really sure if we want to add those larger values, but food for thought...

Re: [PATCH 1/3] ARM: dts: Add headers with constants for MTD partitions

[Florian Vaussard]

Hello Stephen,

On 06/11/2013 06:24 PM, Stephen Warren wrote:
> On 06/11/2013 08:48 AM, Florian Vaussard wrote:
>> These constants can be used to easily declare MTD partitions inside
>> DTS.
>>
>> The constants MTDPART_OFS_* are purposely not included. Indeed,
>> parse_ofpart_partitions() is expecting u64, but a DT cell is u32.
>> Negative constants, as defined by MTDPART_OFS_*, would be wrongly
>> interpreted by parse_ofpart_partitions(). Two cells should be
>> used to correctly encode the negative constants, but this breaks
>> current usage.
>
> I think addition of common headers like this needs an ack from
> Grant/Rob. I CC'd them here.
>

Indeed. Thank you.

>> diff --git a/include/dt-bindings/mtd/partitions.h b/include/dt-bindings/mtd/partitions.h
>
>> + * This header provides constants used with MTD partitions.
> ...
>> +/* Partition size */
>> +#define MTDPART_SIZ_FULL	0
>
> Which binding document in Documentation/devicetree/bindings is this
> definition associated with? The comment above should really mention
> this. Documentation/devicetree/bindings/mtd/partition.txt doesn't seem
> to mention this value.
>

Mmmh I was not seeing this as a DT binding, strictly speaking. It was 
already
used with legacy board files. Otherwise we should also update the binding
for constants related to GPIO, IRQ,... But I agree that a line inside the
documentation never killed someone.

>> diff --git a/include/dt-bindings/sizes.h b/include/dt-bindings/sizes.h
>
> ...
>> +#define SZ_1G				0x40000000
>> +#define SZ_2G				0x80000000
>> +
>> +#endif
>
> For MTD partitions specifically, SZ_4G and onwards would be useful in
> theory, although that would end up putting two cell values into a single
> macro. and then the values couldn't be added/or'd together. So, I'm not
> really sure if we want to add those larger values, but food for thought...
>

It is maybe feasible to define a macro splitting the u64 into two
u32 cells? But this can be done afterwards when need arises.

Best regards,

Florian

Re: [PATCH 1/3] ARM: dts: Add headers with constants for MTD partitions

[Grant Likely]

On Tue, 11 Jun 2013 16:48:56 +0200, Florian Vaussard <florian.vaussard@epfl.ch> wrote:
> These constants can be used to easily declare MTD partitions inside
> DTS.
> 
> The constants MTDPART_OFS_* are purposely not included. Indeed,
> parse_ofpart_partitions() is expecting u64, but a DT cell is u32.
> Negative constants, as defined by MTDPART_OFS_*, would be wrongly

The DT binding uses the number of cells defined by #address-cells. It is
not fixed to a u32 or a u64

> interpreted by parse_ofpart_partitions(). Two cells should be
> used to correctly encode the negative constants, but this breaks
> current usage.

The binding doesn't even allow for shortcuts like MTDPART_SIZ_FULL. If a
partition fills the whole device, then the reg property should include
the actual size. If the code is allowing '0' to be used to mean
MTDPART_SIZ_FULL, then that is a bug that needs to be fixed.

Please drop the mtd/partitions.h hunk from this patch.

g.

> 
> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
> ---
>  include/dt-bindings/mtd/partitions.h |   12 ++++++++
>  include/dt-bindings/sizes.h          |   52 ++++++++++++++++++++++++++++++++++
>  2 files changed, 64 insertions(+), 0 deletions(-)
>  create mode 100644 include/dt-bindings/mtd/partitions.h
>  create mode 100644 include/dt-bindings/sizes.h
> 
> diff --git a/include/dt-bindings/mtd/partitions.h b/include/dt-bindings/mtd/partitions.h
> new file mode 100644
> index 0000000..7dfa676
> --- /dev/null
> +++ b/include/dt-bindings/mtd/partitions.h
> @@ -0,0 +1,12 @@
> +/*
> + * This header provides constants used with MTD partitions.
> + */
> +
> +#ifndef _DT_BINDINGS_MTD_PARTITIONS_H
> +#define _DT_BINDINGS_MTD_PARTITIONS_H
> +
> +/* Partition size */
> +#define MTDPART_SIZ_FULL	0
> +
> +#endif
> +
> diff --git a/include/dt-bindings/sizes.h b/include/dt-bindings/sizes.h
> new file mode 100644
> index 0000000..995f2de
> --- /dev/null
> +++ b/include/dt-bindings/sizes.h
> @@ -0,0 +1,52 @@
> +/*
> + * This header provides size constants.
> + *
> + * Original version:
> + *   include/linux/sizes.h
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef _DT_BINDINGS_SIZES_H
> +#define _DT_BINDINGS_SIZES_H
> +
> +#define SZ_1				0x00000001
> +#define SZ_2				0x00000002
> +#define SZ_4				0x00000004
> +#define SZ_8				0x00000008
> +#define SZ_16				0x00000010
> +#define SZ_32				0x00000020
> +#define SZ_64				0x00000040
> +#define SZ_128				0x00000080
> +#define SZ_256				0x00000100
> +#define SZ_512				0x00000200
> +
> +#define SZ_1K				0x00000400
> +#define SZ_2K				0x00000800
> +#define SZ_4K				0x00001000
> +#define SZ_8K				0x00002000
> +#define SZ_16K				0x00004000
> +#define SZ_32K				0x00008000
> +#define SZ_64K				0x00010000
> +#define SZ_128K				0x00020000
> +#define SZ_256K				0x00040000
> +#define SZ_512K				0x00080000
> +
> +#define SZ_1M				0x00100000
> +#define SZ_2M				0x00200000
> +#define SZ_4M				0x00400000
> +#define SZ_8M				0x00800000
> +#define SZ_16M				0x01000000
> +#define SZ_32M				0x02000000
> +#define SZ_64M				0x04000000
> +#define SZ_128M				0x08000000
> +#define SZ_256M				0x10000000
> +#define SZ_512M				0x20000000
> +
> +#define SZ_1G				0x40000000
> +#define SZ_2G				0x80000000
> +
> +#endif
> +
> -- 
> 1.7.5.4
> 
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

Re: [PATCH 1/3] ARM: dts: Add headers with constants for MTD partitions

[Florian Vaussard]

Hello,

Thank you for the review.

On 06/12/2013 03:05 PM, Grant Likely wrote:
> On Tue, 11 Jun 2013 16:48:56 +0200, Florian Vaussard <florian.vaussard@epfl.ch> wrote:
>> These constants can be used to easily declare MTD partitions inside
>> DTS.
>>
>> The constants MTDPART_OFS_* are purposely not included. Indeed,
>> parse_ofpart_partitions() is expecting u64, but a DT cell is u32.
>> Negative constants, as defined by MTDPART_OFS_*, would be wrongly
>
> The DT binding uses the number of cells defined by #address-cells. It is
> not fixed to a u32 or a u64
>

The message was ill-formatted, sorry. As an address cell is u32, and as
parse_ofpart_partitions() is storing the value inside u64 without checking
for sign extension (as one assumes to have only positive offsets),
passing a negative value would require 2 address cells, making it more 
difficult
for the DT user. But as Stephen Warren noticed, it is probably desirable
to specify sizes >= 4GB, thus I will think about a way to easily handle 
such case.

Anyway, MTDPART_OFS_* would probably face the same objection raised by 
you for
MTDPART_SIZ_FULL.

>> interpreted by parse_ofpart_partitions(). Two cells should be
>> used to correctly encode the negative constants, but this breaks
>> current usage.
>
> The binding doesn't even allow for shortcuts like MTDPART_SIZ_FULL. If a
> partition fills the whole device, then the reg property should include
> the actual size. If the code is allowing '0' to be used to mean
> MTDPART_SIZ_FULL, then that is a bug that needs to be fixed.
>

The root problem is that many System on Module, like the Gumstix Overo, are
shipped with various NAND sizes depending on the version or even the 
manufacturing
period. Supporting such a diversity would painfully duplicates lots of DT
code and clutter the arch/arm/boot/dts/ directory with dozens of slightly-
different versions. I believe that determining the NAND size is better done
at probe time, and this is what is currently done in legacy board files:

static struct mtd_partition overo_nand_partitions[] = {
         {
                 .name           = "xloader",
                 .offset         = 0,                    /* Offset = 
0x00000 */
                 .size           = 4 * NAND_BLOCK_SIZE,
                 .mask_flags     = MTD_WRITEABLE
         },

<snip...>

         {
                 .name           = "rootfs",
                 .offset         = MTDPART_OFS_APPEND,   /* Offset = 
0x680000 */
                 .size           = MTDPART_SIZ_FULL,
         },
};

Moreover, I do not see such strict restriction in the OF norm. If I 
refer to IEEE
1275-1994 page 174, for the definition of the "reg" property, it is written:

"The interpretation of the size entries is dependent on the parent bus."

Nevertheless, if such an approach is not acceptable, could we think about an
alternative solution? Like a boolean property "mtd,append-and-fill" that 
would
replace the "reg" property and tell the MTD core to compute the 
partition size
at probe time, like what is currently done with board files?

Best regards,

Florian

[PATCH 2/3] ARM: dts: Add omap3-overo NAND flash memory binding

[Florian Vaussard]

Add device-tree node for the on-board NAND memory.

Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
---
 arch/arm/boot/dts/omap3-overo.dtsi |   50 ++++++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/omap3.dtsi       |    2 +
 2 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/omap3-overo.dtsi b/arch/arm/boot/dts/omap3-overo.dtsi
index e112a42..811b74c 100644
--- a/arch/arm/boot/dts/omap3-overo.dtsi
+++ b/arch/arm/boot/dts/omap3-overo.dtsi
@@ -33,6 +33,56 @@
 	};
 };
 
+&gpmc {
+	ranges = <0 0 0x30000000 0x00000004>;       /* CS0: NAND */
+
+	nand@0,0 {
+		reg = <0 0 0>; /* CS0, offset 0 */
+		nand-bus-width = <16>;
+
+		ti,nand-ecc-opt = "sw";
+
+		gpmc,device-nand;
+		gpmc,sync-clk-ps = <0>;
+		gpmc,cs-on-ns = <0>;
+		gpmc,cs-rd-off-ns = <36>;
+		gpmc,cs-wr-off-ns = <36>;
+		gpmc,adv-on-ns = <6>;
+		gpmc,adv-rd-off-ns = <24>;
+		gpmc,adv-wr-off-ns = <36>;
+		gpmc,we-off-ns = <30>;
+		gpmc,oe-off-ns = <48>;
+		gpmc,access-ns = <54>;
+		gpmc,rd-cycle-ns = <72>;
+		gpmc,wr-cycle-ns = <72>;
+		gpmc,wr-access-ns = <30>;
+		gpmc,wr-data-mux-bus-ns = <0>;
+
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		xloader@0 {
+			reg = <(0 * SZ_128K) (4 * SZ_128K)>;
+		};
+
+		uboot@80000 {
+			reg = <(4 * SZ_128K) (14 * SZ_128K)>;
+		};
+
+		ubootenv@240000 {
+			reg = <(18 * SZ_128K) (2 * SZ_128K)>;
+		};
+
+		linux@280000 {
+			reg = <(20 * SZ_128K) (32 * SZ_128K)>;
+		};
+
+		rootfs@680000 {
+			reg = <(52 * SZ_128K) MTDPART_SIZ_FULL>;
+		};
+	};
+};
+
 &i2c1 {
 	clock-frequency = <2600000>;
 
diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
index 6d05ee0..65a9390 100644
--- a/arch/arm/boot/dts/omap3.dtsi
+++ b/arch/arm/boot/dts/omap3.dtsi
@@ -9,7 +9,9 @@
  */
 
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/mtd/partitions.h>
 #include <dt-bindings/pinctrl/omap.h>
+#include <dt-bindings/sizes.h>
 
 #include "skeleton.dtsi"
 
-- 
1.7.5.4

[PATCH 3/3] ARM: dts: OMAP3: Use MTD constants for OMAP3 boards

[Florian Vaussard]

Use the MTD constants for NAND and OneNAND nodes used in OMAP3
DTS.

Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
---
 arch/arm/boot/dts/omap3-devkit8000.dts |   10 +++++-----
 arch/arm/boot/dts/omap3-igep0020.dts   |   10 +++++-----
 arch/arm/boot/dts/omap3-igep0030.dts   |   10 +++++-----
 arch/arm/boot/dts/omap3430-sdp.dts     |   28 ++++++++++++++--------------
 4 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/arch/arm/boot/dts/omap3-devkit8000.dts b/arch/arm/boot/dts/omap3-devkit8000.dts
index 5be71b1..08699cb 100644
--- a/arch/arm/boot/dts/omap3-devkit8000.dts
+++ b/arch/arm/boot/dts/omap3-devkit8000.dts
@@ -143,27 +143,27 @@
 
 		x-loader@0 {
 			label = "X-Loader";
-			reg = <0 0x80000>;
+			reg = <(0 * SZ_128K) (4 * SZ_128K)>;
 		};
 
 		bootloaders@80000 {
 			label = "U-Boot";
-			reg = <0x80000 0x1e0000>;
+			reg = <(4 * SZ_128K) (15 * SZ_128K)>;
 		};
 
 		bootloaders_env@260000 {
 			label = "U-Boot Env";
-			reg = <0x260000 0x20000>;
+			reg = <(19 * SZ_128K) (1 * SZ_128K)>;
 		};
 
 		kernel@280000 {
 			label = "Kernel";
-			reg = <0x280000 0x400000>;
+			reg = <(20 * SZ_128K) (32 * SZ_128K)>;
 		};
 
 		filesystem@680000 {
 			label = "File System";
-			reg = <0x680000 0xf980000>;
+			reg = <(52 * SZ_128K) MTDPART_SIZ_FULL>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/omap3-igep0020.dts b/arch/arm/boot/dts/omap3-igep0020.dts
index e8c4828..3476b3c 100644
--- a/arch/arm/boot/dts/omap3-igep0020.dts
+++ b/arch/arm/boot/dts/omap3-igep0020.dts
@@ -97,23 +97,23 @@
 
 		partition@0 {
 			label = "SPL";
-			reg = <0 0x100000>;
+			reg = <(0 * SZ_256K) (4 * SZ_256K)>;
 		};
 		partition@0x80000 {
 			label = "U-Boot";
-			reg = <0x100000 0x180000>;
+			reg = <(4 * SZ_256K) (6 * SZ_256K)>;
 		};
 		partition@0x1c0000 {
 			label = "Environment";
-			reg = <0x280000 0x100000>;
+			reg = <(10 * SZ_256K) (4 * SZ_256K)>;
 		};
 		partition@0x280000 {
 			label = "Kernel";
-			reg = <0x380000 0x300000>;
+			reg = <(14 * SZ_256K) (12 * SZ_256K)>;
 		};
 		partition@0x780000 {
 			label = "Filesystem";
-			reg = <0x680000 0x1f980000>;
+			reg = <(26 * SZ_256K) MTDPART_SIZ_FULL>;
 		};
 	};
 
diff --git a/arch/arm/boot/dts/omap3-igep0030.dts b/arch/arm/boot/dts/omap3-igep0030.dts
index 644d053..e4f078c 100644
--- a/arch/arm/boot/dts/omap3-igep0030.dts
+++ b/arch/arm/boot/dts/omap3-igep0030.dts
@@ -72,23 +72,23 @@
 
 		partition@0 {
 			label = "SPL";
-			reg = <0 0x100000>;
+			reg = <(0 * SZ_256K) (4 * SZ_256K)>;
 		};
 		partition@0x80000 {
 			label = "U-Boot";
-			reg = <0x100000 0x180000>;
+			reg = <(4 * SZ_256K) (6 * SZ_256K)>;
 		};
 		partition@0x1c0000 {
 			label = "Environment";
-			reg = <0x280000 0x100000>;
+			reg = <(10 * SZ_256K) (4 * SZ_256K)>;
 		};
 		partition@0x280000 {
 			label = "Kernel";
-			reg = <0x380000 0x300000>;
+			reg = <(14 * SZ_256K) (12 * SZ_256K)>;
 		};
 		partition@0x780000 {
 			label = "Filesystem";
-			reg = <0x680000 0x1f980000>;
+			reg = <(26 * SZ_256K) MTDPART_SIZ_FULL>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/omap3430-sdp.dts b/arch/arm/boot/dts/omap3430-sdp.dts
index 2a725a0..dd69ee0 100644
--- a/arch/arm/boot/dts/omap3430-sdp.dts
+++ b/arch/arm/boot/dts/omap3430-sdp.dts
@@ -81,19 +81,19 @@
 
 		partition@0 {
 			label = "bootloader-nor";
-			reg = <0 0x40000>;
+			reg = <(0 * SZ_128K) (2 * SZ_128K)>;
 		};
 		partition@0x40000 {
 			label = "params-nor";
-			reg = <0x40000 0x40000>;
+			reg = <(2 * SZ_128K) (2 * SZ_128K)>;
 		};
 		partition@0x80000 {
 			label = "kernel-nor";
-			reg = <0x80000 0x200000>;
+			reg = <(4 * SZ_128K) (16 * SZ_128K)>;
 		};
 		partition@0x280000 {
 			label = "filesystem-nor";
-			reg = <0x240000 0x7d80000>;
+			reg = <(18 * SZ_128K) (1004 * SZ_128K)>;
 		};
 	};
 
@@ -123,23 +123,23 @@
 
 		partition@0 {
 			label = "xloader-nand";
-			reg = <0 0x80000>;
+			reg = <(0 * SZ_128K) (4 * SZ_128K)>;
 		};
 		partition@0x80000 {
 			label = "bootloader-nand";
-			reg = <0x80000 0x140000>;
+			reg = <(4 * SZ_128K) (10 * SZ_128K)>;
 		};
 		partition@0x1c0000 {
 			label = "params-nand";
-			reg = <0x1c0000 0xc0000>;
+			reg = <(14 * SZ_128K) (6 * SZ_128K)>;
 		};
 		partition@0x280000 {
 			label = "kernel-nand";
-			reg = <0x280000 0x500000>;
+			reg = <(20 * SZ_128K) (40 * SZ_128K)>;
 		};
 		partition@0x780000 {
 			label = "filesystem-nand";
-			reg = <0x780000 0x7880000>;
+			reg = <(60 * SZ_128K) (964 * SZ_128K)>;
 		};
 	};
 
@@ -168,23 +168,23 @@
 
 		partition@0 {
 			label = "xloader-onenand";
-			reg = <0 0x80000>;
+			reg = <(0 * SZ_128K) (4 * SZ_128K)>;
 		};
 		partition@0x80000 {
 			label = "bootloader-onenand";
-			reg = <0x80000 0x40000>;
+			reg = <(4 * SZ_128K) (2 * SZ_128K)>;
 		};
 		partition@0xc0000 {
 			label = "params-onenand";
-			reg = <0xc0000 0x20000>;
+			reg = <(6 * SZ_128K) (1 * SZ_128K)>;
 		};
 		partition@0xe0000 {
 			label = "kernel-onenand";
-			reg = <0xe0000 0x200000>;
+			reg = <(7 * SZ_128K) (16 * SZ_128K)>;
 		};
 		partition@0x2e0000 {
 			label = "filesystem-onenand";
-			reg = <0x2e0000 0xfd20000>;
+			reg = <(23 * SZ_128K) (2025 * SZ_128K)>;
 		};
 	};
 };
-- 
1.7.5.4

Re: [PATCH 3/3] ARM: dts: OMAP3: Use MTD constants for OMAP3 boards

[Javier Martinez Canillas]

On 06/11/2013 04:48 PM, Florian Vaussard wrote:
> Use the MTD constants for NAND and OneNAND nodes used in OMAP3
> DTS.
> 
> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
> ---
>  arch/arm/boot/dts/omap3-devkit8000.dts |   10 +++++-----
>  arch/arm/boot/dts/omap3-igep0020.dts   |   10 +++++-----
>  arch/arm/boot/dts/omap3-igep0030.dts   |   10 +++++-----
>  arch/arm/boot/dts/omap3430-sdp.dts     |   28 ++++++++++++++--------------
>  4 files changed, 29 insertions(+), 29 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/omap3-devkit8000.dts b/arch/arm/boot/dts/omap3-devkit8000.dts
> index 5be71b1..08699cb 100644
> --- a/arch/arm/boot/dts/omap3-devkit8000.dts
> +++ b/arch/arm/boot/dts/omap3-devkit8000.dts
> @@ -143,27 +143,27 @@
>  
>  		x-loader@0 {
>  			label = "X-Loader";
> -			reg = <0 0x80000>;
> +			reg = <(0 * SZ_128K) (4 * SZ_128K)>;
>  		};
>  
>  		bootloaders@80000 {
>  			label = "U-Boot";
> -			reg = <0x80000 0x1e0000>;
> +			reg = <(4 * SZ_128K) (15 * SZ_128K)>;
>  		};
>  
>  		bootloaders_env@260000 {
>  			label = "U-Boot Env";
> -			reg = <0x260000 0x20000>;
> +			reg = <(19 * SZ_128K) (1 * SZ_128K)>;
>  		};
>  
>  		kernel@280000 {
>  			label = "Kernel";
> -			reg = <0x280000 0x400000>;
> +			reg = <(20 * SZ_128K) (32 * SZ_128K)>;
>  		};
>  
>  		filesystem@680000 {
>  			label = "File System";
> -			reg = <0x680000 0xf980000>;
> +			reg = <(52 * SZ_128K) MTDPART_SIZ_FULL>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/omap3-igep0020.dts b/arch/arm/boot/dts/omap3-igep0020.dts
> index e8c4828..3476b3c 100644
> --- a/arch/arm/boot/dts/omap3-igep0020.dts
> +++ b/arch/arm/boot/dts/omap3-igep0020.dts
> @@ -97,23 +97,23 @@
>  
>  		partition@0 {
>  			label = "SPL";
> -			reg = <0 0x100000>;
> +			reg = <(0 * SZ_256K) (4 * SZ_256K)>;
>  		};
>  		partition@0x80000 {
>  			label = "U-Boot";
> -			reg = <0x100000 0x180000>;
> +			reg = <(4 * SZ_256K) (6 * SZ_256K)>;
>  		};
>  		partition@0x1c0000 {
>  			label = "Environment";
> -			reg = <0x280000 0x100000>;
> +			reg = <(10 * SZ_256K) (4 * SZ_256K)>;
>  		};
>  		partition@0x280000 {
>  			label = "Kernel";
> -			reg = <0x380000 0x300000>;
> +			reg = <(14 * SZ_256K) (12 * SZ_256K)>;
>  		};
>  		partition@0x780000 {
>  			label = "Filesystem";
> -			reg = <0x680000 0x1f980000>;
> +			reg = <(26 * SZ_256K) MTDPART_SIZ_FULL>;
>  		};
>  	};
>  
> diff --git a/arch/arm/boot/dts/omap3-igep0030.dts b/arch/arm/boot/dts/omap3-igep0030.dts
> index 644d053..e4f078c 100644
> --- a/arch/arm/boot/dts/omap3-igep0030.dts
> +++ b/arch/arm/boot/dts/omap3-igep0030.dts
> @@ -72,23 +72,23 @@
>  
>  		partition@0 {
>  			label = "SPL";
> -			reg = <0 0x100000>;
> +			reg = <(0 * SZ_256K) (4 * SZ_256K)>;
>  		};
>  		partition@0x80000 {
>  			label = "U-Boot";
> -			reg = <0x100000 0x180000>;
> +			reg = <(4 * SZ_256K) (6 * SZ_256K)>;
>  		};
>  		partition@0x1c0000 {
>  			label = "Environment";
> -			reg = <0x280000 0x100000>;
> +			reg = <(10 * SZ_256K) (4 * SZ_256K)>;
>  		};
>  		partition@0x280000 {
>  			label = "Kernel";
> -			reg = <0x380000 0x300000>;
> +			reg = <(14 * SZ_256K) (12 * SZ_256K)>;
>  		};
>  		partition@0x780000 {
>  			label = "Filesystem";
> -			reg = <0x680000 0x1f980000>;
> +			reg = <(26 * SZ_256K) MTDPART_SIZ_FULL>;
>  		};
>  	};
>  };

Hi Florian,

I don't have access to my IGEP board so I can test it right now but the patch
looks good to me.

In fact I wanted to use MTDPART_SIZ_FULL when added the NAND nodes since not all
IGEP boards have 512MB flash but I didn't know that a value of 0 meant that.

So thanks a lot for doing this!

Acked-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>

Re: [PATCH 3/3] ARM: dts: OMAP3: Use MTD constants for OMAP3 boards

[Florian Vaussard]

Hello Javier,

On 06/11/2013 05:29 PM, Javier Martinez Canillas wrote:
> On 06/11/2013 04:48 PM, Florian Vaussard wrote:
>> Use the MTD constants for NAND and OneNAND nodes used in OMAP3
>> DTS.
>>
>> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
>> ---
>>   arch/arm/boot/dts/omap3-devkit8000.dts |   10 +++++-----
>>   arch/arm/boot/dts/omap3-igep0020.dts   |   10 +++++-----
>>   arch/arm/boot/dts/omap3-igep0030.dts   |   10 +++++-----
>>   arch/arm/boot/dts/omap3430-sdp.dts     |   28 ++++++++++++++--------------
>>   4 files changed, 29 insertions(+), 29 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/omap3-devkit8000.dts b/arch/arm/boot/dts/omap3-devkit8000.dts
>> index 5be71b1..08699cb 100644
>> --- a/arch/arm/boot/dts/omap3-devkit8000.dts
>> +++ b/arch/arm/boot/dts/omap3-devkit8000.dts
>> @@ -143,27 +143,27 @@
>>
>>   		x-loader@0 {
>>   			label = "X-Loader";
>> -			reg = <0 0x80000>;
>> +			reg = <(0 * SZ_128K) (4 * SZ_128K)>;
>>   		};
>>
>>   		bootloaders@80000 {
>>   			label = "U-Boot";
>> -			reg = <0x80000 0x1e0000>;
>> +			reg = <(4 * SZ_128K) (15 * SZ_128K)>;
>>   		};
>>
>>   		bootloaders_env@260000 {
>>   			label = "U-Boot Env";
>> -			reg = <0x260000 0x20000>;
>> +			reg = <(19 * SZ_128K) (1 * SZ_128K)>;
>>   		};
>>
>>   		kernel@280000 {
>>   			label = "Kernel";
>> -			reg = <0x280000 0x400000>;
>> +			reg = <(20 * SZ_128K) (32 * SZ_128K)>;
>>   		};
>>
>>   		filesystem@680000 {
>>   			label = "File System";
>> -			reg = <0x680000 0xf980000>;
>> +			reg = <(52 * SZ_128K) MTDPART_SIZ_FULL>;
>>   		};
>>   	};
>>   };
>> diff --git a/arch/arm/boot/dts/omap3-igep0020.dts b/arch/arm/boot/dts/omap3-igep0020.dts
>> index e8c4828..3476b3c 100644
>> --- a/arch/arm/boot/dts/omap3-igep0020.dts
>> +++ b/arch/arm/boot/dts/omap3-igep0020.dts
>> @@ -97,23 +97,23 @@
>>
>>   		partition@0 {
>>   			label = "SPL";
>> -			reg = <0 0x100000>;
>> +			reg = <(0 * SZ_256K) (4 * SZ_256K)>;
>>   		};
>>   		partition@0x80000 {
>>   			label = "U-Boot";
>> -			reg = <0x100000 0x180000>;
>> +			reg = <(4 * SZ_256K) (6 * SZ_256K)>;
>>   		};
>>   		partition@0x1c0000 {
>>   			label = "Environment";
>> -			reg = <0x280000 0x100000>;
>> +			reg = <(10 * SZ_256K) (4 * SZ_256K)>;
>>   		};
>>   		partition@0x280000 {
>>   			label = "Kernel";
>> -			reg = <0x380000 0x300000>;
>> +			reg = <(14 * SZ_256K) (12 * SZ_256K)>;
>>   		};
>>   		partition@0x780000 {
>>   			label = "Filesystem";
>> -			reg = <0x680000 0x1f980000>;
>> +			reg = <(26 * SZ_256K) MTDPART_SIZ_FULL>;
>>   		};
>>   	};
>>
>> diff --git a/arch/arm/boot/dts/omap3-igep0030.dts b/arch/arm/boot/dts/omap3-igep0030.dts
>> index 644d053..e4f078c 100644
>> --- a/arch/arm/boot/dts/omap3-igep0030.dts
>> +++ b/arch/arm/boot/dts/omap3-igep0030.dts
>> @@ -72,23 +72,23 @@
>>
>>   		partition@0 {
>>   			label = "SPL";
>> -			reg = <0 0x100000>;
>> +			reg = <(0 * SZ_256K) (4 * SZ_256K)>;
>>   		};
>>   		partition@0x80000 {
>>   			label = "U-Boot";
>> -			reg = <0x100000 0x180000>;
>> +			reg = <(4 * SZ_256K) (6 * SZ_256K)>;
>>   		};
>>   		partition@0x1c0000 {
>>   			label = "Environment";
>> -			reg = <0x280000 0x100000>;
>> +			reg = <(10 * SZ_256K) (4 * SZ_256K)>;
>>   		};
>>   		partition@0x280000 {
>>   			label = "Kernel";
>> -			reg = <0x380000 0x300000>;
>> +			reg = <(14 * SZ_256K) (12 * SZ_256K)>;
>>   		};
>>   		partition@0x780000 {
>>   			label = "Filesystem";
>> -			reg = <0x680000 0x1f980000>;
>> +			reg = <(26 * SZ_256K) MTDPART_SIZ_FULL>;
>>   		};
>>   	};
>>   };
>
> Hi Florian,
>
> I don't have access to my IGEP board so I can test it right now but the patch
> looks good to me.
>
> In fact I wanted to use MTDPART_SIZ_FULL when added the NAND nodes since not all
> IGEP boards have 512MB flash but I didn't know that a value of 0 meant that.
>

I had the same problem, and found that 0 was correctly parsed and later 
expanded to
the correct value when probing the NAND.

> So thanks a lot for doing this!
>
> Acked-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>

Thank you.

Florian

Re: [PATCH 3/3] ARM: dts: OMAP3: Use MTD constants for OMAP3 boards

[Grant Likely]

On Tue, 11 Jun 2013 17:29:43 +0200, Javier Martinez Canillas <javier.martinez@collabora.co.uk> wrote:
> On 06/11/2013 04:48 PM, Florian Vaussard wrote:
> > Use the MTD constants for NAND and OneNAND nodes used in OMAP3
> > DTS.
> > 
> > Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
> > ---
> >  arch/arm/boot/dts/omap3-devkit8000.dts |   10 +++++-----
> >  arch/arm/boot/dts/omap3-igep0020.dts   |   10 +++++-----
> >  arch/arm/boot/dts/omap3-igep0030.dts   |   10 +++++-----
> >  arch/arm/boot/dts/omap3430-sdp.dts     |   28 ++++++++++++++--------------
> >  4 files changed, 29 insertions(+), 29 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/omap3-devkit8000.dts b/arch/arm/boot/dts/omap3-devkit8000.dts
> > index 5be71b1..08699cb 100644
> > --- a/arch/arm/boot/dts/omap3-devkit8000.dts
> > +++ b/arch/arm/boot/dts/omap3-devkit8000.dts
> > @@ -143,27 +143,27 @@
> >  
> >  		x-loader@0 {
> >  			label = "X-Loader";
> > -			reg = <0 0x80000>;
> > +			reg = <(0 * SZ_128K) (4 * SZ_128K)>;
> >  		};
> >  
> >  		bootloaders@80000 {
> >  			label = "U-Boot";
> > -			reg = <0x80000 0x1e0000>;
> > +			reg = <(4 * SZ_128K) (15 * SZ_128K)>;
> >  		};
> >  
> >  		bootloaders_env@260000 {
> >  			label = "U-Boot Env";
> > -			reg = <0x260000 0x20000>;
> > +			reg = <(19 * SZ_128K) (1 * SZ_128K)>;
> >  		};
> >  
> >  		kernel@280000 {
> >  			label = "Kernel";
> > -			reg = <0x280000 0x400000>;
> > +			reg = <(20 * SZ_128K) (32 * SZ_128K)>;
> >  		};
> >  
> >  		filesystem@680000 {
> >  			label = "File System";
> > -			reg = <0x680000 0xf980000>;
> > +			reg = <(52 * SZ_128K) MTDPART_SIZ_FULL>;
> >  		};
> >  	};
> >  };
> > diff --git a/arch/arm/boot/dts/omap3-igep0020.dts b/arch/arm/boot/dts/omap3-igep0020.dts
> > index e8c4828..3476b3c 100644
> > --- a/arch/arm/boot/dts/omap3-igep0020.dts
> > +++ b/arch/arm/boot/dts/omap3-igep0020.dts
> > @@ -97,23 +97,23 @@
> >  
> >  		partition@0 {
> >  			label = "SPL";
> > -			reg = <0 0x100000>;
> > +			reg = <(0 * SZ_256K) (4 * SZ_256K)>;
> >  		};
> >  		partition@0x80000 {
> >  			label = "U-Boot";
> > -			reg = <0x100000 0x180000>;
> > +			reg = <(4 * SZ_256K) (6 * SZ_256K)>;
> >  		};
> >  		partition@0x1c0000 {
> >  			label = "Environment";
> > -			reg = <0x280000 0x100000>;
> > +			reg = <(10 * SZ_256K) (4 * SZ_256K)>;
> >  		};
> >  		partition@0x280000 {
> >  			label = "Kernel";
> > -			reg = <0x380000 0x300000>;
> > +			reg = <(14 * SZ_256K) (12 * SZ_256K)>;
> >  		};
> >  		partition@0x780000 {
> >  			label = "Filesystem";
> > -			reg = <0x680000 0x1f980000>;
> > +			reg = <(26 * SZ_256K) MTDPART_SIZ_FULL>;
> >  		};
> >  	};
> >  
> > diff --git a/arch/arm/boot/dts/omap3-igep0030.dts b/arch/arm/boot/dts/omap3-igep0030.dts
> > index 644d053..e4f078c 100644
> > --- a/arch/arm/boot/dts/omap3-igep0030.dts
> > +++ b/arch/arm/boot/dts/omap3-igep0030.dts
> > @@ -72,23 +72,23 @@
> >  
> >  		partition@0 {
> >  			label = "SPL";
> > -			reg = <0 0x100000>;
> > +			reg = <(0 * SZ_256K) (4 * SZ_256K)>;
> >  		};
> >  		partition@0x80000 {
> >  			label = "U-Boot";
> > -			reg = <0x100000 0x180000>;
> > +			reg = <(4 * SZ_256K) (6 * SZ_256K)>;
> >  		};
> >  		partition@0x1c0000 {
> >  			label = "Environment";
> > -			reg = <0x280000 0x100000>;
> > +			reg = <(10 * SZ_256K) (4 * SZ_256K)>;
> >  		};
> >  		partition@0x280000 {
> >  			label = "Kernel";
> > -			reg = <0x380000 0x300000>;
> > +			reg = <(14 * SZ_256K) (12 * SZ_256K)>;
> >  		};
> >  		partition@0x780000 {
> >  			label = "Filesystem";
> > -			reg = <0x680000 0x1f980000>;
> > +			reg = <(26 * SZ_256K) MTDPART_SIZ_FULL>;
> >  		};
> >  	};
> >  };
> 
> Hi Florian,
> 
> I don't have access to my IGEP board so I can test it right now but the patch
> looks good to me.
> 
> In fact I wanted to use MTDPART_SIZ_FULL when added the NAND nodes since not all
> IGEP boards have 512MB flash but I didn't know that a value of 0 meant that.
> 
> So thanks a lot for doing this!
> 
> Acked-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>

However, the binding doesn't allow for that and so it is a bug in the
parser. Relying on '0' is not safe. Nor does it match device tree
convention for the reg property, so don't count on getting an extension
added to allow it.

NAK

Re: [PATCH 3/3] ARM: dts: OMAP3: Use MTD constants for OMAP3 boards

[Stephen Warren]

On 06/11/2013 08:48 AM, Florian Vaussard wrote:
> Use the MTD constants for NAND and OneNAND nodes used in OMAP3
> DTS.

I don't quite understand the split between patches 2/3 and 3/3; isn't
the edit to omap3-overo.dtsi (part of) a board file, and hence logically
part of this patch? I'd be tempted just to squash the two together.

But, this is a nit; not a big deal.

Re: [PATCH 3/3] ARM: dts: OMAP3: Use MTD constants for OMAP3 boards

[Florian Vaussard]

Hello,

On 06/11/2013 06:27 PM, Stephen Warren wrote:
> On 06/11/2013 08:48 AM, Florian Vaussard wrote:
>> Use the MTD constants for NAND and OneNAND nodes used in OMAP3
>> DTS.
>
> I don't quite understand the split between patches 2/3 and 3/3; isn't
> the edit to omap3-overo.dtsi (part of) a board file, and hence logically
> part of this patch? I'd be tempted just to squash the two together.
>
> But, this is a nit; not a big deal.
>

Patch 2/3 was adding a new node, whereas patch 3/3 was converting existing
nodes. But your point is perfectly valid.

Regards,

Florian