ARM: dts: OMAP3: Use constants with MTD devices
cover ARM: dts: OMAP3: Use constants with MTD devices
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 at 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
Signature · 1 line
1.7.5.4
PATCH 1/3 ARM: dts: Add headers with constants for MTD partitions
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.
--- 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
include/dt-bindings/mtd/partitions.h new file +12 -0
@@ -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 +
include/dt-bindings/sizes.h new file +52 -0
52 lines not on this page · raw message
Signature · 1 line
1.7.5.4
PATCH 1/3 ARM: dts: Add headers with constants for MTD partitions
Quoted 9 lines · 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 byparse_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.
Quoted 3 lines
include/dt-bindings/mtd/partitions.h +1
-0+* This header provides constants used with MTD partitions.
...
include/dt-bindings/mtd/partitions.h line 9
Quoted 2 lines
include/dt-bindings/mtd/partitions.h +2
-0+/* 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.
Quoted 1 line
include/dt-bindings/sizes.h +0
-0
...
include/dt-bindings/sizes.h line 51
Quoted 4 lines
include/dt-bindings/sizes.h +4
-0+#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...
PATCH 1/3 ARM: dts: Add headers with constants for MTD partitions
Hello Stephen,
Quoted 11 lines · Stephen Warren
Quoted 9 lines · 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 byparse_ofpart_partitions(). Two cells should be used to correctly encode the negative constants, but this breaks current usage.
Quoted 3 lines
I think addition of common headers like this needs an ack from Grant/Rob. I CC'd them here.
Indeed. Thank you.
Quoted 12 lines
Quoted 3 lines
include/dt-bindings/mtd/partitions.h +1
-0+* This header provides constants used with MTD partitions. ...
include/dt-bindings/mtd/partitions.h line 9
Quoted 2 lines
include/dt-bindings/mtd/partitions.h +2
-0+/* Partition size */ +#define MTDPART_SIZ_FULL 0Which binding document in
Documentation/devicetree/bindingsis this definition associated with? The comment above should really mention this.Documentation/devicetree/bindings/mtd/partition.txtdoesn'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.
Quoted 13 lines
Quoted 1 line
include/dt-bindings/sizes.h +0
-0...
include/dt-bindings/sizes.h line 51
Quoted 4 lines
include/dt-bindings/sizes.h +4
-0+#define SZ_1G 0x40000000 +#define SZ_2G 0x80000000 + +#endif For MTD partitions specifically,
SZ_4Gand 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
PATCH 1/3 ARM: dts: Add headers with constants for MTD partitions
In reply to Florian VaussardQuoted 6 lines · 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
The DT binding uses the number of cells defined by #address-cells. It is not fixed to a u32 or a u64
Quoted 3 lines
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.
Quoted 92 lines
Signed-off-by Florian Vaussard <[email protected]>--- 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.hinclude/dt-bindings/mtd/partitions.h new file +12
-0@@ -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 +include/dt-bindings/sizes.h new file +52
-0@@ -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 +Signature · 6 lines
1.7.5.4 _______________________________________________ devicetree-discuss mailing list devicetree-discuss at lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Signature · 2 lines
Grant Likely, B.Sc, P.Eng. Secret Lab Technologies, Ltd.
PATCH 1/3 ARM: dts: Add headers with constants for MTD partitions
Hello,
Thank you for the review.
Quoted 11 lines · Grant Likely
Quoted 6 lines · 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 wronglyThe 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 forMTDPART_SIZ_FULL.
Quoted 9 lines
Quoted 3 lines
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 meanMTDPART_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
In reply to Florian VaussardAdd device-tree node for the on-board NAND memory.
--- arch/arm/boot/dts/omap3-overo.dtsi | 50 ++++++++++++++++++++++++++++++++++++ arch/arm/boot/dts/omap3.dtsi | 2 + 2 files changed, 52 insertions(+), 0 deletions(-)
arch/arm/boot/dts/omap3-overo.dtsi +50 -0
@@ -33,6 +33,56 @@ }; }; +&gpmc { + ranges = <0 0 0x30000000 0x00000004>; /* CS0: NAND */ + + nand at 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 at 0 { + reg = <(0 * SZ_128K) (4 * SZ_128K)>; + }; + + uboot at 80000 { + reg = <(4 * SZ_128K) (14 * SZ_128K)>; + }; + + ubootenv at 240000 { + reg = <(18 * SZ_128K) (2 * SZ_128K)>; + }; + + linux at 280000 { + reg = <(20 * SZ_128K) (32 * SZ_128K)>; + }; + + rootfs at 680000 { + reg = <(52 * SZ_128K) MTDPART_SIZ_FULL>; + }; + }; +}; + &i2c1 { clock-frequency = <2600000>;
arch/arm/boot/dts/omap3.dtsi +2 -0
@@ -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"
Signature · 1 line
1.7.5.4
PATCH 3/3 ARM: dts: OMAP3: Use MTD constants for OMAP3 boards
In reply to Florian VaussardUse the MTD constants for NAND and OneNAND nodes used in OMAP3 DTS.
--- 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(-)
arch/arm/boot/dts/omap3-devkit8000.dts +5 -5
@@ -143,27 +143,27 @@ x-loader at 0 { label = "X-Loader"; - reg = <0 0x80000>; + reg = <(0 * SZ_128K) (4 * SZ_128K)>; }; bootloaders at 80000 { label = "U-Boot"; - reg = <0x80000 0x1e0000>; + reg = <(4 * SZ_128K) (15 * SZ_128K)>; }; bootloaders_env at 260000 { label = "U-Boot Env"; - reg = <0x260000 0x20000>; + reg = <(19 * SZ_128K) (1 * SZ_128K)>; }; kernel at 280000 { label = "Kernel"; - reg = <0x280000 0x400000>; + reg = <(20 * SZ_128K) (32 * SZ_128K)>; }; filesystem at 680000 { label = "File System"; - reg = <0x680000 0xf980000>; + reg = <(52 * SZ_128K) MTDPART_SIZ_FULL>; }; }; };
arch/arm/boot/dts/omap3-igep0020.dts +5 -5
@@ -97,23 +97,23 @@ partition at 0 { label = "SPL"; - reg = <0 0x100000>; + reg = <(0 * SZ_256K) (4 * SZ_256K)>; }; partition at 0x80000 { label = "U-Boot"; - reg = <0x100000 0x180000>; + reg = <(4 * SZ_256K) (6 * SZ_256K)>; }; partition at 0x1c0000 { label = "Environment"; - reg = <0x280000 0x100000>; + reg = <(10 * SZ_256K) (4 * SZ_256K)>; }; partition at 0x280000 { label = "Kernel"; - reg = <0x380000 0x300000>; + reg = <(14 * SZ_256K) (12 * SZ_256K)>; }; partition at 0x780000 { label = "Filesystem"; - reg = <0x680000 0x1f980000>; + reg = <(26 * SZ_256K) MTDPART_SIZ_FULL>; }; };
arch/arm/boot/dts/omap3-igep0030.dts +5 -5
@@ -72,23 +72,23 @@ partition at 0 { label = "SPL"; - reg = <0 0x100000>; + reg = <(0 * SZ_256K) (4 * SZ_256K)>; }; partition at 0x80000 { label = "U-Boot"; - reg = <0x100000 0x180000>; + reg = <(4 * SZ_256K) (6 * SZ_256K)>; }; partition at 0x1c0000 { label = "Environment"; - reg = <0x280000 0x100000>; + reg = <(10 * SZ_256K) (4 * SZ_256K)>; }; partition at 0x280000 { label = "Kernel"; - reg = <0x380000 0x300000>; + reg = <(14 * SZ_256K) (12 * SZ_256K)>; }; partition at 0x780000 { label = "Filesystem"; - reg = <0x680000 0x1f980000>; + reg = <(26 * SZ_256K) MTDPART_SIZ_FULL>; }; }; };
arch/arm/boot/dts/omap3430-sdp.dts +14 -14
@@ -81,19 +81,19 @@ partition at 0 { label = "bootloader-nor"; - reg = <0 0x40000>; + reg = <(0 * SZ_128K) (2 * SZ_128K)>; }; partition at 0x40000 { label = "params-nor"; - reg = <0x40000 0x40000>; + reg = <(2 * SZ_128K) (2 * SZ_128K)>; }; partition at 0x80000 { label = "kernel-nor"; - reg = <0x80000 0x200000>; + reg = <(4 * SZ_128K) (16 * SZ_128K)>; }; partition at 0x280000 { label = "filesystem-nor"; - reg = <0x240000 0x7d80000>; + reg = <(18 * SZ_128K) (1004 * SZ_128K)>; }; };
@@ -123,23 +123,23 @@ partition at 0 { label = "xloader-nand"; - reg = <0 0x80000>; + reg = <(0 * SZ_128K) (4 * SZ_128K)>; }; partition at 0x80000 { label = "bootloader-nand"; - reg = <0x80000 0x140000>; + reg = <(4 * SZ_128K) (10 * SZ_128K)>; }; partition at 0x1c0000 { label = "params-nand"; - reg = <0x1c0000 0xc0000>; + reg = <(14 * SZ_128K) (6 * SZ_128K)>; }; partition at 0x280000 { label = "kernel-nand"; - reg = <0x280000 0x500000>; + reg = <(20 * SZ_128K) (40 * SZ_128K)>; }; partition at 0x780000 { label = "filesystem-nand"; - reg = <0x780000 0x7880000>; + reg = <(60 * SZ_128K) (964 * SZ_128K)>; }; };
@@ -168,23 +168,23 @@ partition at 0 { label = "xloader-onenand"; - reg = <0 0x80000>; + reg = <(0 * SZ_128K) (4 * SZ_128K)>; }; partition at 0x80000 { label = "bootloader-onenand"; - reg = <0x80000 0x40000>; + reg = <(4 * SZ_128K) (2 * SZ_128K)>; }; partition at 0xc0000 { label = "params-onenand"; - reg = <0xc0000 0x20000>; + reg = <(6 * SZ_128K) (1 * SZ_128K)>; }; partition at 0xe0000 { label = "kernel-onenand"; - reg = <0xe0000 0x200000>; + reg = <(7 * SZ_128K) (16 * SZ_128K)>; }; partition at 0x2e0000 { label = "filesystem-onenand"; - reg = <0x2e0000 0xfd20000>; + reg = <(23 * SZ_128K) (2025 * SZ_128K)>; }; }; };
Signature · 1 line
1.7.5.4
PATCH 3/3 ARM: dts: OMAP3: Use MTD constants for OMAP3 boards
Quoted 104 lines · Florian Vaussard
Use the MTD constants for NAND and OneNAND nodes used in OMAP3 DTS.
Signed-off-by Florian Vaussard <[email protected]>--- 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(-)arch/arm/boot/dts/omap3-devkit8000.dts +5
-5@@ -143,27 +143,27 @@ x-loader at 0 { label = "X-Loader"; - reg = <0 0x80000>; + reg = <(0 * SZ_128K) (4 * SZ_128K)>; }; bootloaders at 80000 { label = "U-Boot"; - reg = <0x80000 0x1e0000>; + reg = <(4 * SZ_128K) (15 * SZ_128K)>; }; bootloaders_env at 260000 { label = "U-Boot Env"; - reg = <0x260000 0x20000>; + reg = <(19 * SZ_128K) (1 * SZ_128K)>; }; kernel at 280000 { label = "Kernel"; - reg = <0x280000 0x400000>; + reg = <(20 * SZ_128K) (32 * SZ_128K)>; }; filesystem at 680000 { label = "File System"; - reg = <0x680000 0xf980000>; + reg = <(52 * SZ_128K) MTDPART_SIZ_FULL>; }; }; };arch/arm/boot/dts/omap3-igep0020.dts +5
-5@@ -97,23 +97,23 @@ partition at 0 { label = "SPL"; - reg = <0 0x100000>; + reg = <(0 * SZ_256K) (4 * SZ_256K)>; }; partition at 0x80000 { label = "U-Boot"; - reg = <0x100000 0x180000>; + reg = <(4 * SZ_256K) (6 * SZ_256K)>; }; partition at 0x1c0000 { label = "Environment"; - reg = <0x280000 0x100000>; + reg = <(10 * SZ_256K) (4 * SZ_256K)>; }; partition at 0x280000 { label = "Kernel"; - reg = <0x380000 0x300000>; + reg = <(14 * SZ_256K) (12 * SZ_256K)>; }; partition at 0x780000 { label = "Filesystem"; - reg = <0x680000 0x1f980000>; + reg = <(26 * SZ_256K) MTDPART_SIZ_FULL>; }; };arch/arm/boot/dts/omap3-igep0030.dts +3
-3@@ -72,23 +72,23 @@ partition at 0 { label = "SPL"; - reg = <0 0x100000>; + reg = <(0 * SZ_256K) (4 * SZ_256K)>; }; partition at 0x80000 { label = "U-Boot"; - reg = <0x100000 0x180000>; + reg = <(4 * SZ_256K) (6 * SZ_256K)>; }; partition at 0x1c0000 { label = "Environment"; - reg = <0x280000 0x100000>; + reg = <(10 * SZ_256K) (4 * SZ_256K)>; }; partition at 0x280000 { label = "Kernel";
arch/arm/boot/dts/omap3-igep0030.dts line 94
Quoted 10 lines
arch/arm/boot/dts/omap3-igep0030.dts +2
-2- reg = <0x380000 0x300000>; + reg = <(14 * SZ_256K) (12 * SZ_256K)>; }; partition at 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!
PATCH 3/3 ARM: dts: OMAP3: Use MTD constants for OMAP3 boards
Hello Javier,
Quoted 118 lines · Javier Martinez Canillas
Quoted 104 lines · Florian Vaussard
Use the MTD constants for NAND and OneNAND nodes used in OMAP3 DTS.
Signed-off-by Florian Vaussard <[email protected]>--- 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(-)arch/arm/boot/dts/omap3-devkit8000.dts +5
-5@@ -143,27 +143,27 @@ x-loader at 0 { label = "X-Loader"; - reg = <0 0x80000>; + reg = <(0 * SZ_128K) (4 * SZ_128K)>; }; bootloaders at 80000 { label = "U-Boot"; - reg = <0x80000 0x1e0000>; + reg = <(4 * SZ_128K) (15 * SZ_128K)>; }; bootloaders_env at 260000 { label = "U-Boot Env"; - reg = <0x260000 0x20000>; + reg = <(19 * SZ_128K) (1 * SZ_128K)>; }; kernel at 280000 { label = "Kernel"; - reg = <0x280000 0x400000>; + reg = <(20 * SZ_128K) (32 * SZ_128K)>; }; filesystem at 680000 { label = "File System"; - reg = <0x680000 0xf980000>; + reg = <(52 * SZ_128K) MTDPART_SIZ_FULL>; }; }; };arch/arm/boot/dts/omap3-igep0020.dts +5
-5@@ -97,23 +97,23 @@ partition at 0 { label = "SPL"; - reg = <0 0x100000>; + reg = <(0 * SZ_256K) (4 * SZ_256K)>; }; partition at 0x80000 { label = "U-Boot"; - reg = <0x100000 0x180000>; + reg = <(4 * SZ_256K) (6 * SZ_256K)>; }; partition at 0x1c0000 { label = "Environment"; - reg = <0x280000 0x100000>; + reg = <(10 * SZ_256K) (4 * SZ_256K)>; }; partition at 0x280000 { label = "Kernel"; - reg = <0x380000 0x300000>; + reg = <(14 * SZ_256K) (12 * SZ_256K)>; }; partition at 0x780000 { label = "Filesystem"; - reg = <0x680000 0x1f980000>; + reg = <(26 * SZ_256K) MTDPART_SIZ_FULL>; }; };arch/arm/boot/dts/omap3-igep0030.dts +3
-3@@ -72,23 +72,23 @@ partition at 0 { label = "SPL"; - reg = <0 0x100000>; + reg = <(0 * SZ_256K) (4 * SZ_256K)>; }; partition at 0x80000 { label = "U-Boot"; - reg = <0x100000 0x180000>; + reg = <(4 * SZ_256K) (6 * SZ_256K)>; }; partition at 0x1c0000 { label = "Environment"; - reg = <0x280000 0x100000>; + reg = <(10 * SZ_256K) (4 * SZ_256K)>; }; partition at 0x280000 { label = "Kernel";arch/arm/boot/dts/omap3-igep0030.dts line 94
Quoted 10 lines
arch/arm/boot/dts/omap3-igep0030.dts +2
-2- reg = <0x380000 0x300000>; + reg = <(14 * SZ_256K) (12 * SZ_256K)>; }; partition at 0x780000 { label = "Filesystem"; - reg = <0x680000 0x1f980000>; + reg = <(26 * SZ_256K) MTDPART_SIZ_FULL>; }; }; };Hi Florian,
Quoted 6 lines
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_FULLwhen 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.
Quoted 4 lines
So thanks a lot for doing this!
Thank you.
Florian
Grant Likely
NAKPATCH 3/3 ARM: dts: OMAP3: Use MTD constants for OMAP3 boards
In reply to Javier Martinez CanillasQuoted 121 lines · Javier Martinez Canillas
Quoted 104 lines · Florian Vaussard
Use the MTD constants for NAND and OneNAND nodes used in OMAP3 DTS.
Signed-off-by Florian Vaussard <[email protected]>--- 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(-)arch/arm/boot/dts/omap3-devkit8000.dts +5
-5@@ -143,27 +143,27 @@ x-loader at 0 { label = "X-Loader"; - reg = <0 0x80000>; + reg = <(0 * SZ_128K) (4 * SZ_128K)>; }; bootloaders at 80000 { label = "U-Boot"; - reg = <0x80000 0x1e0000>; + reg = <(4 * SZ_128K) (15 * SZ_128K)>; }; bootloaders_env at 260000 { label = "U-Boot Env"; - reg = <0x260000 0x20000>; + reg = <(19 * SZ_128K) (1 * SZ_128K)>; }; kernel at 280000 { label = "Kernel"; - reg = <0x280000 0x400000>; + reg = <(20 * SZ_128K) (32 * SZ_128K)>; }; filesystem at 680000 { label = "File System"; - reg = <0x680000 0xf980000>; + reg = <(52 * SZ_128K) MTDPART_SIZ_FULL>; }; }; };arch/arm/boot/dts/omap3-igep0020.dts +5
-5@@ -97,23 +97,23 @@ partition at 0 { label = "SPL"; - reg = <0 0x100000>; + reg = <(0 * SZ_256K) (4 * SZ_256K)>; }; partition at 0x80000 { label = "U-Boot"; - reg = <0x100000 0x180000>; + reg = <(4 * SZ_256K) (6 * SZ_256K)>; }; partition at 0x1c0000 { label = "Environment"; - reg = <0x280000 0x100000>; + reg = <(10 * SZ_256K) (4 * SZ_256K)>; }; partition at 0x280000 { label = "Kernel"; - reg = <0x380000 0x300000>; + reg = <(14 * SZ_256K) (12 * SZ_256K)>; }; partition at 0x780000 { label = "Filesystem"; - reg = <0x680000 0x1f980000>; + reg = <(26 * SZ_256K) MTDPART_SIZ_FULL>; }; };arch/arm/boot/dts/omap3-igep0030.dts +3
-3@@ -72,23 +72,23 @@ partition at 0 { label = "SPL"; - reg = <0 0x100000>; + reg = <(0 * SZ_256K) (4 * SZ_256K)>; }; partition at 0x80000 { label = "U-Boot"; - reg = <0x100000 0x180000>; + reg = <(4 * SZ_256K) (6 * SZ_256K)>; }; partition at 0x1c0000 { label = "Environment"; - reg = <0x280000 0x100000>; + reg = <(10 * SZ_256K) (4 * SZ_256K)>; }; partition at 0x280000 { label = "Kernel";arch/arm/boot/dts/omap3-igep0030.dts line 94
Quoted 10 lines
arch/arm/boot/dts/omap3-igep0030.dts +2
-2- reg = <0x380000 0x300000>; + reg = <(14 * SZ_256K) (12 * SZ_256K)>; }; partition at 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.
Quoted 6 lines
In fact I wanted to use
MTDPART_SIZ_FULLwhen 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!
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
PATCH 3/3 ARM: dts: OMAP3: Use MTD constants for OMAP3 boards
In reply to Florian VaussardQuoted 2 lines · Florian Vaussard
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 3/3 ARM: dts: OMAP3: Use MTD constants for OMAP3 boards
Hello,
Quoted 10 lines · Stephen Warren
Quoted 2 lines · Florian Vaussard
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