[1/3] mtd: create a partition type device tree binding

Message ID 1446123152-22666-1-git-send-email-linus.walleij@linaro.org
State New
Headers show

Commit Message

Linus Walleij Oct. 29, 2015, 12:52 p.m.
The Linux code in drivers/mtd/maps/physmap_of.c will optionally
look for the linux,part-probe binding for hints on a partition type to
look for in the MTD. It was added in commit 9d5da3a9b849
"mtd: extend physmap_of to let the device tree specify the parition probe"

This solution is too Linux-specific and depend on some
Linux kernel-internal naming conventions. We need a proper
way of describing partition types that follow the pattern set by
other device tree bindings.

Create a "partition-type" binding for this, and add "my" bindings
for "arm,arm-flash-structure" as a starter for others to follow.
A follow-on patch adds support to the Linux kernel to handle this
binding, with some infrastructure for others to use it too.

Cc: devicetree@vger.kernel.org
Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Cc: Liviu Dudau <Liviu.Dudau@arm.com>
Reported-by: Liviu Dudau <Liviu.Dudau@arm.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

---
 .../devicetree/bindings/mtd/mtd-physmap.txt        |  2 ++
 .../devicetree/bindings/mtd/partition.txt          | 35 +++++++++++++++++++---
 2 files changed, 33 insertions(+), 4 deletions(-)

-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Linus Walleij Oct. 30, 2015, 2 p.m. | #1
On Thu, Oct 29, 2015 at 5:29 PM, Brian Norris
<computersforpeace@gmail.com> wrote:

>> +Required properties:

>> +- partition-type : the type of partition. Only one type can be specified.

>

> You're supporting this as a list property (for future expansion,

> presumably), so I can only assume the "only one type" is referring to

> the number of different parsers available currently, not the behavior of

> the property itself?


I was thinking that it would not be a list actually.

The reason being that if you're anyways going to the trouble of
specifying exactly what partition type is going to be used, you're
not really interested in specifying a few different ones, you know
exactly what type it is going to be.

But if you think it needs to be a list, I'll specify that, no big
deal. I'll also try to get the Linux code to handle a list then.

> Also, this patch will conflict with [1]. I'll probably take [1] soon, so

> one of us will have to rebase this.


Sure I'll rebase on whatever you say.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring Nov. 6, 2015, 2:13 p.m. | #2
On Fri, Oct 30, 2015 at 12:51 PM, Brian Norris
<computersforpeace@gmail.com> wrote:
> On Fri, Oct 30, 2015 at 03:00:57PM +0100, Linus Walleij wrote:

>> On Thu, Oct 29, 2015 at 5:29 PM, Brian Norris

>> <computersforpeace@gmail.com> wrote:

>>

>> >> +Required properties:

>> >> +- partition-type : the type of partition. Only one type can be specified.

>> >

>> > You're supporting this as a list property (for future expansion,

>> > presumably), so I can only assume the "only one type" is referring to

>> > the number of different parsers available currently, not the behavior of

>> > the property itself?

>>

>> I was thinking that it would not be a list actually.

>>

>> The reason being that if you're anyways going to the trouble of

>> specifying exactly what partition type is going to be used, you're

>> not really interested in specifying a few different ones, you know

>> exactly what type it is going to be.

>

> OK, that makes sense. I think it's still *possible* that a board might

> have the option of more than one partition parser, and so they might

> just include both in the DTS, but that seems unlikely and so it makes

> sense not to (over)engineer for it before it's needed. Anyway, your

> binding can easily be expanded in the future if needed.


Since we now have partitions contained in a sub node, how about using
compatible for that sub node instead.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij Nov. 10, 2015, 8:43 a.m. | #3
On Tue, Nov 10, 2015 at 4:26 AM, Brian Norris
<computersforpeace@gmail.com> wrote:
> On Fri, Nov 06, 2015 at 08:13:13AM -0600, Rob Herring wrote:

>> Since we now have partitions contained in a sub node, how about using

>> compatible for that sub node instead.

>

> That seems like a pretty good idea to me.


Hm! That's a clever idea. I'll take a spin on that.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij Nov. 14, 2015, 10:46 a.m. | #4
On Fri, Nov 13, 2015 at 11:00 PM, Brian Norris
<computersforpeace@gmail.com> wrote:

(...)
>  (2) we should define a comptible property for the hard-coded

>  partitioning case (e.g., compatible = "partitions")

(...)
> If we went with option (2), then ofpart.c could just check only for

> 'compatible = "partitions"' (or similar), and if not found bail out.


So this "hard-coded partitioning case" the case is where all partitions
are defined in the device tree as described in
Documentation/devicetree/bindings/mtd/partition.txt ?

Or is it a way to indicate that this array
static const char * const part_probe_types_def[] = {
        "cmdlinepart", "RedBoot", "ofpart", "ofoldpart", NULL };
in physmap_of.c should be used?

Sorry if I don't fully follow, I'm a bit stupid when it comes to the MTD
helicopter  view of the situation.... :(

> I think option (2) makes more sense. But it would require an update to

> the binding and code for 4.4, since [1] was only introduced during this

> release cycle.


Does that mean all old device trees that specify no compatible
string all of a sudden stop working? We can't break the DT ABI, so I
guess not.

A bit confused here, I can't really see what I should do with the patch...

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/Documentation/devicetree/bindings/mtd/mtd-physmap.txt b/Documentation/devicetree/bindings/mtd/mtd-physmap.txt
index 4a0a48bf4ecb..863560bdbb19 100644
--- a/Documentation/devicetree/bindings/mtd/mtd-physmap.txt
+++ b/Documentation/devicetree/bindings/mtd/mtd-physmap.txt
@@ -23,6 +23,8 @@  file systems on embedded devices.
    unaligned accesses as implemented in the JFFS2 code via memcpy().
    By defining "no-unaligned-direct-access", the flash will not be
    exposed directly to the MTD users (e.g. JFFS2) any more.
+ - partition-type : a flash partition type to expect and probe for
+   on this device. See "partition.txt" for available partition types.
  - linux,mtd-name: allow to specify the mtd name for retro capability with
    physmap-flash drivers as boot loader pass the mtd partition via the old
    device name physmap-flash.
diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
index 8e5557da1955..85d45764a4b3 100644
--- a/Documentation/devicetree/bindings/mtd/partition.txt
+++ b/Documentation/devicetree/bindings/mtd/partition.txt
@@ -1,9 +1,36 @@ 
 Representing flash partitions in devicetree
 
-Partitions can be represented by sub-nodes of an mtd device. This can be used
-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.
+On-device partition types:
+
+It is possible for some drivers to indicate an on-device partition type, i.e.
+partition tables, footers or other binary pattern in the flash used to
+define how the flash is partitioned. This can be done in the device tree node
+of an MTD device by specifying partition-type = "foo"; This tells the operating
+system to look for the partition type indicated.
+
+Required properties:
+- partition-type : the type of partition. Only one type can be specified.
+  Valid types are:
+  "arm,arm-flash-structure" (also called AFS)
+
+Example:
+
+flash0@40000000 {
+	/* 2 * 32MiB NOR Flash memory */
+	compatible = "arm,vexpress-flash", "cfi-flash";
+	partition-type = "arm,arm-flash-structure";
+	reg = <0x40000000 0x04000000>;
+	bank-width = <4>;
+};
+
+
+Device Tree specified partitions:
+
+If there is no specified on-device binary format, partitions can be
+represented by sub-nodes of an mtd device. This can be used on platforms which
+have strong conventions about which portions of a flash are used for what
+purposes.
+
 NOTE: if the sub-node has a compatible string, then it is not a partition.
 
 #address-cells & #size-cells must both be present in the mtd device. There are