* Re: [PATCH 1/3] mtd: create a partition type device tree binding [not found] ` <1446123152-22666-1-git-send-email-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> @ 2015-10-29 16:29 ` Brian Norris 2015-10-30 14:00 ` Linus Walleij 0 siblings, 1 reply; 10+ messages in thread From: Brian Norris @ 2015-10-29 16:29 UTC (permalink / raw To: Linus Walleij, devicetree-spec-u79uwXL29TY76Z2rM5mHXA Cc: David Woodhouse, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree-u79uwXL29TY76Z2rM5mHXA, Jason Gunthorpe, Liviu Dudau, Rafał Miłecki, hauke-5/S+JYg5SzeELgA04lAiVw + devicetree-spec and others, since other non-driver-specific MTD partitioning stuff has gotten directed there [1], and this binding should probably get promoted in short order to the core mtdpart.c partitioning On Thu, Oct 29, 2015 at 01:52:30PM +0100, Linus Walleij wrote: > 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. Overall, I like this. And thanks for expanding the explanation of DT partitions vs. parsers. It'd be good if we make parsers like this easier to integrate, so we see less hard-coded partitions in device tree. > Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > Cc: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> > Cc: Liviu Dudau <Liviu.Dudau-5wv7dgnIgG8@public.gmane.org> > Reported-by: Liviu Dudau <Liviu.Dudau-5wv7dgnIgG8@public.gmane.org> > Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > --- > .../devicetree/bindings/mtd/mtd-physmap.txt | 2 ++ > .../devicetree/bindings/mtd/partition.txt | 35 +++++++++++++++++++--- > 2 files changed, 33 insertions(+), 4 deletions(-) > > 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. 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? This should probably be worded differently if that's the intention. Like, "a list of partition types ... ordered by priority .... We currently support the following: (list only the AFS type)" (fill in the blanks how you'd like). Also, this patch will conflict with [1]. I'll probably take [1] soon, so one of us will have to rebase this. Brian [1] http://thread.gmane.org/gmane.comp.devicetree.spec/191 > + 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 > -- > 2.4.3 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] mtd: create a partition type device tree binding 2015-10-29 16:29 ` [PATCH 1/3] mtd: create a partition type device tree binding Brian Norris @ 2015-10-30 14:00 ` Linus Walleij [not found] ` <CACRpkdYS5Jf8UC2K0bNdEQZDjTeZZx44Uu-kkof+9E=4Ny0oDg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Linus Walleij @ 2015-10-30 14:00 UTC (permalink / raw To: Brian Norris Cc: devicetree-spec-u79uwXL29TY76Z2rM5mHXA, David Woodhouse, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Jason Gunthorpe, Liviu Dudau, Rafał Miłecki, Hauke Mehrtens On Thu, Oct 29, 2015 at 5:29 PM, Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <CACRpkdYS5Jf8UC2K0bNdEQZDjTeZZx44Uu-kkof+9E=4Ny0oDg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 1/3] mtd: create a partition type device tree binding [not found] ` <CACRpkdYS5Jf8UC2K0bNdEQZDjTeZZx44Uu-kkof+9E=4Ny0oDg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2015-10-30 17:51 ` Brian Norris [not found] ` <20151030175145.GF13239-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Brian Norris @ 2015-10-30 17:51 UTC (permalink / raw To: Linus Walleij Cc: devicetree-spec-u79uwXL29TY76Z2rM5mHXA, David Woodhouse, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Jason Gunthorpe, Liviu Dudau, Rafał Miłecki, Hauke Mehrtens 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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. > 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. No, I think your proposal is OK then. It's possible there is some room for clarification in the binding, since I was confused, but that could mostly be attributed to my pre-existing assumptions, not a fault of the text. > > 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. OK, I'll let you know. Brian ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <20151030175145.GF13239-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH 1/3] mtd: create a partition type device tree binding [not found] ` <20151030175145.GF13239-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> @ 2015-11-06 14:13 ` Rob Herring [not found] ` <CAL_JsqL2F3p7qBPbOiGytVyzF_aNLWbrCT0mAX1WZo6SxE3JoA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Rob Herring @ 2015-11-06 14:13 UTC (permalink / raw To: Brian Norris Cc: Linus Walleij, devicetree-spec-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, David Woodhouse, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Jason Gunthorpe, Liviu Dudau, Rafał Miłecki, Hauke Mehrtens On Fri, Oct 30, 2015 at 12:51 PM, Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <CAL_JsqL2F3p7qBPbOiGytVyzF_aNLWbrCT0mAX1WZo6SxE3JoA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 1/3] mtd: create a partition type device tree binding [not found] ` <CAL_JsqL2F3p7qBPbOiGytVyzF_aNLWbrCT0mAX1WZo6SxE3JoA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2015-11-10 3:26 ` Brian Norris [not found] ` <20151110032626.GN12143-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> 2015-11-13 22:00 ` Brian Norris 2015-11-15 9:06 ` Geert Uytterhoeven 2 siblings, 1 reply; 10+ messages in thread From: Brian Norris @ 2015-11-10 3:26 UTC (permalink / raw To: Rob Herring Cc: Linus Walleij, devicetree-spec-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, David Woodhouse, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Jason Gunthorpe, Liviu Dudau, Rafał Miłecki, Hauke Mehrtens 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. Brian -- 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] 10+ messages in thread
[parent not found: <20151110032626.GN12143-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH 1/3] mtd: create a partition type device tree binding [not found] ` <20151110032626.GN12143-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> @ 2015-11-10 8:43 ` Linus Walleij 0 siblings, 0 replies; 10+ messages in thread From: Linus Walleij @ 2015-11-10 8:43 UTC (permalink / raw To: Brian Norris Cc: Rob Herring, devicetree-spec-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, David Woodhouse, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Jason Gunthorpe, Liviu Dudau, Rafał Miłecki, Hauke Mehrtens On Tue, Nov 10, 2015 at 4:26 AM, Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] mtd: create a partition type device tree binding [not found] ` <CAL_JsqL2F3p7qBPbOiGytVyzF_aNLWbrCT0mAX1WZo6SxE3JoA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2015-11-10 3:26 ` Brian Norris @ 2015-11-13 22:00 ` Brian Norris [not found] ` <20151113220039.GA74382-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> 2015-11-15 9:06 ` Geert Uytterhoeven 2 siblings, 1 reply; 10+ messages in thread From: Brian Norris @ 2015-11-13 22:00 UTC (permalink / raw To: Rob Herring Cc: Linus Walleij, devicetree-spec-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, David Woodhouse, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Jason Gunthorpe, Liviu Dudau, Rafał Miłecki, Hauke Mehrtens, Michal Suchanek 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. I see that Linus and I spoke up in agreement on this one. I took a little look at adding of_match_table support to the core MTD partitioning code (not sure if that's duplicating anything Linus was attempting on his own?), and I'm observing that there's potential for conflict with the new binding [1]. If we're going to start overloading the 'partitions' node to support other partitioning types via 'compatible' property, then we either need to: (1) go back to specifying that full ofpart specifications must have *no* compatible property or (2) we should define a comptible property for the hard-coded partitioning case (e.g., compatible = "partitions") IOW, I could imagine a new partition parser that needs a DT like this: flash@xxx { compatible = "vendor,flash-type"; ... partitions { compatible = "some-new-partition-parser"; ... subnode { // "some-new-partition-parser" might // need to put something here }; }; }; But currently, the binding would say that 'subnode' must be a partition, even if it's really something else auxiliary to "some-new-partition-parser" [2]. If we went with option (1), then we'd just have ofpart.c see that 'partitions' has a compatible property and bail out. That seems kinda hacky. If we went with option (2), then ofpart.c could just check only for 'compatible = "partitions"' (or similar), and if not found bail out. 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. Brian [1] fe2585e9c29a ("doc: dt: mtd: support partitions in a special 'partitions' subnode") [2] Possibilities: something relevant to partition splitting. See some previous work, which I haven't gotten around to fully addressing yet, but can hopefully be rolled into this work: http://patchwork.ozlabs.org/patch/476373/ http://patchwork.ozlabs.org/patch/473364/ http://patchwork.ozlabs.org/patch/475988/ ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <20151113220039.GA74382-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH 1/3] mtd: create a partition type device tree binding [not found] ` <20151113220039.GA74382-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> @ 2015-11-14 10:46 ` Linus Walleij [not found] ` <CACRpkdakyae95f_Lc-R7Uf7wndj5hyfCqqHziwmZjwm-uNkGAg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Linus Walleij @ 2015-11-14 10:46 UTC (permalink / raw To: Brian Norris Cc: Rob Herring, devicetree-spec-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, David Woodhouse, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Jason Gunthorpe, Liviu Dudau, Rafał Miłecki, Hauke Mehrtens, Michal Suchanek On Fri, Nov 13, 2015 at 11:00 PM, Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <CACRpkdakyae95f_Lc-R7Uf7wndj5hyfCqqHziwmZjwm-uNkGAg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 1/3] mtd: create a partition type device tree binding [not found] ` <CACRpkdakyae95f_Lc-R7Uf7wndj5hyfCqqHziwmZjwm-uNkGAg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2015-11-16 4:12 ` Brian Norris 0 siblings, 0 replies; 10+ messages in thread From: Brian Norris @ 2015-11-16 4:12 UTC (permalink / raw To: Linus Walleij Cc: Rob Herring, devicetree-spec-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, David Woodhouse, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Jason Gunthorpe, Liviu Dudau, Rafał Miłecki, Hauke Mehrtens, Michal Suchanek On Sat, Nov 14, 2015 at 11:46:59AM +0100, Linus Walleij wrote: > On Fri, Nov 13, 2015 at 11:00 PM, Brian Norris > <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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 ? Right. > 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? No. At this point, I would consider that to be a legacy method. We still have to support this for many cases (including the non-DT case; but hopefully we can do better than that soon), and that would be an option. > Sorry if I don't fully follow, I'm a bit stupid when it comes to the MTD > helicopter view of the situation.... :( Yeah...sorry if I wasn't too clear. And I definitely don't blame you for not understanding the mess that MTD often is :( > > 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. No, that's not what I was intending. The binding before commmit fe2585e9c29a ("doc: dt: mtd: support partitions in a special 'partitions' subnode") should still stay working as-is. That is, we don't mess with the way things worked for anything that doesn't have a 'partitions' subnode. But now that we have a 'partitions' subnode (in 4.4-rc1), I'm just suggesting that we enforce this always have a compatible property, so we can be more clear on the difference between: partitions { // do I have a // compatible = "partitons"; // here? partition@0 { label = "foo-partition"; reg = <0 0x100000>; }; }; and partitions { compatible = "arm,arm-flash-structure"; subnode { // what if we need something here eventually? }; }; This would require some modifications to partitions.txt and to drivers/mtd/ofpart.c. > A bit confused here, I can't really see what I should do with the patch... Hopefully that cleared up a bit? The code changes for my suggestion would just be something like this, I think. (Not tested in any way.) diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c index 669c3452f278..6811bc5440a4 100644 --- a/drivers/mtd/ofpart.c +++ b/drivers/mtd/ofpart.c @@ -50,6 +50,10 @@ static int parse_ofpart_partitions(struct mtd_info *master, master->name, mtd_node->full_name); ofpart_node = mtd_node; dedicated = false; + } else { + /* The "partitions" subnode may belong to some other parser */ + if (!of_device_is_compatible(ofpart_node, "partitions")) + return 0; } /* First count the subnodes */ I was just bringing this up for discussion, since it's related to your/Rob's new proposal. I'll send a proper (and tested) patch, along with a doc update, if that looks reasonable. Brian ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] mtd: create a partition type device tree binding [not found] ` <CAL_JsqL2F3p7qBPbOiGytVyzF_aNLWbrCT0mAX1WZo6SxE3JoA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2015-11-10 3:26 ` Brian Norris 2015-11-13 22:00 ` Brian Norris @ 2015-11-15 9:06 ` Geert Uytterhoeven 2 siblings, 0 replies; 10+ messages in thread From: Geert Uytterhoeven @ 2015-11-15 9:06 UTC (permalink / raw To: Rob Herring Cc: Brian Norris, Linus Walleij, devicetree-spec-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, David Woodhouse, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Jason Gunthorpe, Liviu Dudau, Rafał Miłecki, Hauke Mehrtens On Fri, Nov 6, 2015 at 3:13 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote: > On Fri, Oct 30, 2015 at 12:51 PM, Brian Norris > <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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. Why not? It is (was) not uncommon to have multiple partition table types on hard disks (e.g. bsd disklabel and msdos and/or amiga rdb). >>> 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. And "compatible" supports a list of multiple values. BTW, this means it also (can) becomes more generic. Will it be applicable to other block devices (e.g. hard disks), too? Integration with block/partitions? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-11-16 4:12 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1446123152-22666-1-git-send-email-linus.walleij@linaro.org>
[not found] ` <1446123152-22666-1-git-send-email-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2015-10-29 16:29 ` [PATCH 1/3] mtd: create a partition type device tree binding Brian Norris
2015-10-30 14:00 ` Linus Walleij
[not found] ` <CACRpkdYS5Jf8UC2K0bNdEQZDjTeZZx44Uu-kkof+9E=4Ny0oDg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-10-30 17:51 ` Brian Norris
[not found] ` <20151030175145.GF13239-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2015-11-06 14:13 ` Rob Herring
[not found] ` <CAL_JsqL2F3p7qBPbOiGytVyzF_aNLWbrCT0mAX1WZo6SxE3JoA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-11-10 3:26 ` Brian Norris
[not found] ` <20151110032626.GN12143-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2015-11-10 8:43 ` Linus Walleij
2015-11-13 22:00 ` Brian Norris
[not found] ` <20151113220039.GA74382-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2015-11-14 10:46 ` Linus Walleij
[not found] ` <CACRpkdakyae95f_Lc-R7Uf7wndj5hyfCqqHziwmZjwm-uNkGAg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-11-16 4:12 ` Brian Norris
2015-11-15 9:06 ` Geert Uytterhoeven
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 read-only IMAP folder(s) and NNTP newsgroup(s).