by Michal Suchanek

[permalink] [raw]

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

On 5 December 2015 at 12:39, Jonas Gorski <[email protected]> wrote:
> On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
> <[email protected]> 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.

Also how do you plan to write partitioning schemes with parameters
like with non-zero offset of the partition table.

Thanks

Michal

2015-12-07 03:04:34

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 <[email protected]> wrote:
> > > > On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
> > > > <[email protected]> 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.

> Having everything in one doc would help ensure that the entire
> "partitions" binding is considered as a whole when extending it, in my
> (and an in my interpretation of Michal's) opinion.
>
> Brian

--
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

Attachments:

(No filename) (3.16 kB)
signature.asc (819.00 B)
Download all attachments

2015-12-14 10:15:29

by Geert Uytterhoeven

[permalink] [raw]

Subject: Re: [RFC PATCH 0/7] mtd: partitions: add of_match_table support

Hi Brian,

On Sat, Dec 12, 2015 at 2:33 AM, Brian Norris
<[email protected]> wrote:
> On Fri, Dec 11, 2015 at 09:44:37AM +0100, Geert Uytterhoeven wrote:
>> On Thu, Dec 10, 2015 at 9:54 PM, Brian Norris
>> <[email protected]> wrote:
>> > IOW, I wouldn't expect MBR or GPT to work well on large raw NAND flash,
>> > and so I don't plan to do that sort of work myself. If you can provide
>> > some better argument for it, and some nice maintainable code to go with
>> > it, then of course it could be considered :)
>>
>> There's also NOR FLASH (e.g. SPI-NOR), which is what most boards I'm
>> working on have.
>
> OK. But these flash are often used for the boot firmware, no? And then,
> does the boot code have to be provided at one end of the flash (e.g.,
> bottom)? If so, then something like MBR or GPT will likely not apply,
> since they reserve the first (and sometimes last) blocks of the medium.

If the boot firmware is in the first few blocks, Amiga RDB or BSD disklabel
come to the rescue...

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2015-12-14 10:22:55

by Geert Uytterhoeven

[permalink] [raw]

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

On Sat, Dec 12, 2015 at 6:51 AM, David Gibson
<[email protected]> 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 <[email protected]> wrote:
>> > > > On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
>> > > > <[email protected]> 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?

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 ...>;
...
};

...
};

If you do need additional properties, you'll have to add separate partition
table nodes.
Where? Outside the "partitions" subnode?
What with multiple partition tables
- some needing properties (outside "partitions"),
- others not (outside/inside "partitions"),
- others needing the offset (inside "partitions"?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2015-12-14 12:29:19

by Michal Suchanek

[permalink] [raw]

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

On 14 December 2015 at 11:22, Geert Uytterhoeven <[email protected]> wrote:
> On Sat, Dec 12, 2015 at 6:51 AM, David Gibson
> <[email protected]> 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 <[email protected]> wrote:
>>> > > > On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
>>> > > > <[email protected]> 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?
>
> 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 ...>;
> ...
> };
>
> ...
> };
>
> If you do need additional properties, you'll have to add separate partition
> table nodes.
> Where? Outside the "partitions" subnode?
> What with multiple partition tables
> - some needing properties (outside "partitions"),
> - others not (outside/inside "partitions"),
> - others needing the offset (inside "partitions"?

Hello,

IIRC there is RedBoot on-flash partition format support in kernel
which has the partition table offset as compile time configuration
parameter. Moving this to DT would make unified kernel for boards with
different RedBoot table offsets possible.

I don't use any board with RedBoot so I would not know if there is
other issue preventing such unified kernel.

It might be an actual use case for both the binding and the extra
argument with current partitioning scheme, though.

Thanks

Michal

2015-12-15 09:50:58

On Tue, Dec 15, 2015 at 11:03:48AM +0100, Geert Uytterhoeven wrote:
> Hi David,
>
> On Tue, Dec 15, 2015 at 7:00 AM, David Gibson
> <[email protected]> wrote:
> >> 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.
>
> Why? The partition is part of the FLASH. In this respect, it doesn't differ
> from other hardcoded partitions using the same DT syntax.
> It would just have a compatible value indicating it's a partition
> table.

Ah.. yes, fair enough. I'd forgotten that the encoding of explicit
partitions in the device tree already established the address space
here as being the flash blocks. So, no, it's not an abuse of unit
address.

Doesn't help for partition table types which require scanning the
device, of course.

--
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

Attachments:

(No filename) (1.26 kB)
signature.asc (819.00 B)
Download all attachments