Re: [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding

From: David Gibson
Date: Tue Dec 15 2015 - 04:51:04 EST


On Mon, Dec 14, 2015 at 11:22:46AM +0100, Geert Uytterhoeven wrote:
> On Sat, Dec 12, 2015 at 6:51 AM, David Gibson
> <david@xxxxxxxxxxxxxxxxxxxxx> wrote:
> > On Thu, Dec 10, 2015 at 12:43:24PM -0800, Brian Norris wrote:
> >> On Mon, Dec 07, 2015 at 12:36:28PM +1100, David Gibson wrote:
> >> > On Sat, Dec 05, 2015 at 10:33:30PM +0100, Michal Suchanek wrote:
> >> > > On 5 December 2015 at 12:39, Jonas Gorski <jogo@xxxxxxxxxxx> wrote:
> >> > > > On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
> >> > > > <computersforpeace@xxxxxxxxx> wrote:
> >> > >
> >> > > >> +
> >> > > >> +Examples:
> >> > > >> +
> >> > > >> +flash@0 {
> >> > > >> + partitions {
> >> > > >> + compatible = "google,fmap";
> >> > > >> + };
> >> > > >> +};
> >> > > >
> >> > > > I wonder if this wouldn't be better served in a separate binding doc
> >> > > > with its compatible name as the filename, like we do with
> >> > > > driver^Whardware blocks, especially if we want to add more parsers.
> >> > >
> >> > >
> >> > > I find that *very* counter productive for bindings that go to the same
> >> > > node. You have a description of a node, and then suddenly there you
> >> > > have another file with another description of the same node. Totally
> >> > > awesome.
> >> >
> >> > I can't actually work out from that if you're agreeing with the
> >> > original post or the first reply.
> >>
> >> Perhaps I'm biased, but I think he was agreeing with the first reply.
> >> (Particularly, "I find that *very* counter productive" uses the word
> >> "that" to refer to "separate binding doc[s]".)
> >>
> >> > > Also how do you plan to write partitioning schemes with parameters
> >> > > like with non-zero offset of the partition table.
> >>
> >> If you are directing this question at me: I don't have a specific plan
> >> for it. MTD parsers don't currently take external input for this; many
> >> scan the whole device, but some might also have conventions built into
> >> the parser itself too, so this just gets hooked based on "compatible".
> >> But if the need arose, I would hope we could work out a common binding.
> >>
> >> > Presumably with properties in the patitions node. Not seeing the
> >> > problem here.
> >>
> >> I believe Michal is bringing up the (important, IMO) point that if
> >> distinct partition types are being described in the same node, then any
> >> use of additional properties *must* be closely coordinated. We can't
> >> have two parsers "foo" and "bar" defining conflicting uses of the same
> >> property in the same node, like this:
> >>
> >> partitions {
> >> compatible = "foo", "bar";
> >> property-baz = ...; // e.g., reg = <...>;
> >> };
> >>
> >> where if "foo" is not found, we fall back to "bar". But what if "foo"
> >> and "bar" use "property-baz" differently?
> >
> > Ah.. that is an excellent point, and leads me to realise that using
> > compatible in this way is wrong. The whole point of compatible is
> > that the node is, well, compatible with *all* the things in the list,
> > and therefore the things in the list are compatible with each other.
> >
> > Using it for a list of entirely different things to attempt in order
> > is not correct.
>
> Isn't the idea behind a partition table that all partition information is
> stored on the device in a well-known format, so you don't need additional
> properties?

I guess that's the idea, but I wouldn't like to count on it.

And more importantly, it's still abusing the 'compatible' property. A
node is supposed to be compatible with *everything* in 'compatible',
not just one of the things listed there.

> If the only property needed is the partition table offset, it can be encoded
> in the unit-address, and the "reg" property:
>
> partitions {
>
> partition-table@xxxx {
> reg = <0xxxx ...>;
> ...
> };
>
> ...
> };

Urgh.. and that's abusing the unit address.

--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson

Attachment: signature.asc
Description: PGP signature