Patchwork [v2] mtd: ofpart: support partitions of 4 GB and larger

login
register
mail settings
Submitter Aaron Sierra
Date Feb. 21, 2013, 10:29 p.m.
Message ID <9b66b475-1755-4842-b44e-4948ed7ed9d4@zimbra>
Download mbox | patch
Permalink /patch/222429/
State New
Headers show

Comments

Aaron Sierra - Feb. 21, 2013, 10:29 p.m.
From: Joe Schaack <jschaack@xes-inc.com>

Previously, partitions were limited to less than 4 GB in size because
the address and size were read as 32-bit values. Add support for 64-bit
values to support devices of 4 GB and larger.

Signed-off-by: Joe Schaack <jschaack@xes-inc.com>
Signed-off-by: Nate Case <ncase@xes-inc.com>
Signed-off-by: Aaron Sierra <asierra@xes-inc.com>
---
 .../devicetree/bindings/mtd/partition.txt          |   36 ++++++++++++++++++--
 drivers/mtd/ofpart.c                               |    7 ++--
 2 files changed, 39 insertions(+), 4 deletions(-)
Artem Bityutskiy - March 8, 2013, 12:01 p.m.
On Thu, 2013-02-21 at 16:29 -0600, Aaron Sierra wrote:
> From: Joe Schaack <jschaack@xes-inc.com>
> 
> Previously, partitions were limited to less than 4 GB in size because
> the address and size were read as 32-bit values. Add support for 64-bit
> values to support devices of 4 GB and larger.
> 
> Signed-off-by: Joe Schaack <jschaack@xes-inc.com>
> Signed-off-by: Nate Case <ncase@xes-inc.com>
> Signed-off-by: Aaron Sierra <asierra@xes-inc.com>

Pushed to l2-mtd.git, thanks!
David Woodhouse - April 5, 2013, 11:23 a.m.
On Thu, 2013-02-21 at 16:29 -0600, Aaron Sierra wrote:
> +       /* an 8 GB partition */
> +       partition@0 {
> +               label = "filesystem #1";
> +               reg = <0x0 0x00000000 0x2 0x00000000>;
> +       };
> +
> +       /* a 4 GB partition */
> +       partition@200000000 {
> +               label = "filesystem #2";
> +               reg = <0x2 0x00000000 0x1 0x00000000>;
> +       };
> +};

The comments here don't match the actual definitions. I fixed it to read
as follows:

flash@2 {
	#address-cells = <2>;
	#size-cells = <2>;

	/* an 8 GB partition */
	partition@0 {
		label = "filesystem #1";
		reg = <0x0 0x00000000 0x1 0xdcd65000>;
	};

	/* a 4 GB partition */
	partition@200000000 {
		label = "filesystem #2";
		reg = <0x1 0xdcd65000 0x0 0xee6b2800>;
	};
};
Aaron Sierra - April 5, 2013, 2:18 p.m.
> From: "David Woodhouse" <dwmw2@infradead.org>
> Sent: Friday, April 5, 2013 6:23:22 AM
> Subject: Re: [PATCH v2] mtd: ofpart: support partitions of 4 GB and larger
> 
> On Thu, 2013-02-21 at 16:29 -0600, Aaron Sierra wrote:
> > +       /* an 8 GB partition */
> > +       partition@0 {
> > +               label = "filesystem #1";
> > +               reg = <0x0 0x00000000 0x2 0x00000000>;
> > +       };
> > +
> > +       /* a 4 GB partition */
> > +       partition@200000000 {
> > +               label = "filesystem #2";
> > +               reg = <0x2 0x00000000 0x1 0x00000000>;
> > +       };
> > +};
> 
> The comments here don't match the actual definitions. I fixed it to
> read
> as follows:
> 
> flash@2 {
> 	#address-cells = <2>;
> 	#size-cells = <2>;
> 
> 	/* an 8 GB partition */
> 	partition@0 {
> 		label = "filesystem #1";
> 		reg = <0x0 0x00000000 0x1 0xdcd65000>;
> 	};
> 
> 	/* a 4 GB partition */
> 	partition@200000000 {
> 		label = "filesystem #2";
> 		reg = <0x1 0xdcd65000 0x0 0xee6b2800>;
> 	};
> };

David,
Blame this on my vague use of GB. I assume things would have been
crystal clear if I'd used GiB. Is this already committed with your
change? Your version doesn't seem like a realistic partitioning
scheme based on likely device erase block sizes.

Is is possible to just update the comments?

- 	/* an 8 GB partition */
+ 	/* an 8 GiB partition */
...
- 	/* a 4 GB partition */
+ 	/* a 4 GiB partition */

-Aaron
David Woodhouse - April 5, 2013, 2:20 p.m.
On Fri, 2013-04-05 at 09:18 -0500, Aaron Sierra wrote:
> 
> Is is possible to just update the comments?
> 
> -       /* an 8 GB partition */
> +       /* an 8 GiB partition */
> ...
> -       /* a 4 GB partition */
> +       /* a 4 GiB partition */

I have to admit that I lied in my previous email, and what you suggest
here is exactly what I actually *did* commit :)

Patch

diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
index 6e1f61f..8f354bc 100644
--- a/Documentation/devicetree/bindings/mtd/partition.txt
+++ b/Documentation/devicetree/bindings/mtd/partition.txt
@@ -5,8 +5,12 @@  on platforms which have strong conventions about which portions of a flash are
 used for what purposes, but which don't use an on-flash partition table such
 as RedBoot.
 
-#address-cells & #size-cells must both be present in the mtd device and be
-equal to 1.
+#address-cells & #size-cells must both be present in the mtd device. There are
+two valid values for both:
+<1>: for partitions that require a single 32-bit cell to represent their
+     size/address (aka the value is below 4 GB)
+<2>: for partitions that require two 32-bit cells to represent their
+     size/address (aka the value is 4 GB or greater).
 
 Required properties:
 - reg : The partition's offset and size within the mtd bank.
@@ -36,3 +40,31 @@  flash@0 {
 		reg = <0x0100000 0x200000>;
 	};
 };
+
+flash@1 {
+	#address-cells = <1>;
+	#size-cells = <2>;
+
+	/* a 4 GB partition */
+	partition@0 {
+		label = "filesystem";
+		reg = <0x00000000 0x1 0x00000000>;
+	};
+};
+
+flash@2 {
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	/* an 8 GB partition */
+	partition@0 {
+		label = "filesystem #1";
+		reg = <0x0 0x00000000 0x2 0x00000000>;
+	};
+
+	/* a 4 GB partition */
+	partition@200000000 {
+		label = "filesystem #2";
+		reg = <0x2 0x00000000 0x1 0x00000000>;
+	};
+};
diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c
index 30bd907..553d6d6 100644
--- a/drivers/mtd/ofpart.c
+++ b/drivers/mtd/ofpart.c
@@ -55,6 +55,7 @@  static int parse_ofpart_partitions(struct mtd_info *master,
 	while ((pp = of_get_next_child(node, pp))) {
 		const __be32 *reg;
 		int len;
+		int a_cells, s_cells;
 
 		reg = of_get_property(pp, "reg", &len);
 		if (!reg) {
@@ -62,8 +63,10 @@  static int parse_ofpart_partitions(struct mtd_info *master,
 			continue;
 		}
 
-		(*pparts)[i].offset = be32_to_cpu(reg[0]);
-		(*pparts)[i].size = be32_to_cpu(reg[1]);
+		a_cells = of_n_addr_cells(pp);
+		s_cells = of_n_size_cells(pp);
+		(*pparts)[i].offset = of_read_number(reg, a_cells);
+		(*pparts)[i].size = of_read_number(reg + a_cells, s_cells);
 
 		partname = of_get_property(pp, "label", &len);
 		if (!partname)