[Power.org:parch] devicetree: Musings on reserved regions

[Grant Likely]

On Mon, Feb 7, 2011 at 3:00 PM, Jimi Xenidis <jimix at pobox.com> wrote:
> On Feb 7, 2011, at 3:45 PM, Grant Likely wrote:
>>
>> On Mon, Feb 7, 2011 at 2:39 PM, Benjamin Herrenschmidt
>> <benh at kernel.crashing.org> wrote:
>>>
>>>> In addition to the reserved regions block in the header, define a set
>>>> of properties in the memory node that specify the reserved regions
>>>> with the name reflecting the usage.
>>>> For example:
>>>>
>>>>       memory at 0 {
>>>>               device_type = "memory";
>>>>               reg = <0 0x40000000>;
>>>>               reserved-ramdisk = <0xc00000 0x200000>; /* 2MB ramdisk */
>>>>               reserved-dtb = <0xbf0000 0x1000>; /* devicetree */
>>>>               reserved-fb0 = 0x1000000 0x400000>; /* framebuffer */
>>>>       };
>>>>
>>>> Each reserved property would start with "reserved-" followed by a
>>>> name.  reserved-ramdisk and reserved-dtb would be reserved for ramdisk
>>>> and dtb images respectively.  Other names could also be defined; for
>>>> example, reserved-openfirmware.
>>>
>>> I very much dislike the list of properties with magic names. I'd rather
>>> have a pair of properties containing lists (reserved-names and
>>> reserved-ranges).
>>
>> Heh, I have very much the opposite opinion.  I'd rather have property
>> names that match the usage than have the names and values split into
>> two properties.  To me, keeping them together is more tasteful.  Queue
>> the debate...  :-)
>
> ok.. here goes nothing...
> When I see these "reservations", it makes me think that they should be nodes anyway. (off root?)
> Once we have nodes for these interesting things, perhaps the next step is to just have a "reserved" property for these nodes?
> so:
>        ramdisk@<addr> {
>                reg = < addr, size >;
>                format = < "gzip", "cpio" >;  // optional, order of wrapper?
>                reserved;
>        }
>
> No, I'm not ready to propose the ramdisk node, but it is something to consider.

Hmmm, of all the ideas, I think I like this one the best.  It is a
little more verbose than I was thinking, but making each reserved
region (or set of reserved regions) have separate node has some very
real advantages.  For one, the address decoding *just works* with our
existing helpers.  Also it means that device nodes can directly
indicate the reserved regions associated with it by way of a phandle.

Naming I think could simply be "reserved@<addr>" since the usage of
each node can be indicated by phandles or driver-specific properties
within the node, plus it kind of follows the lead of the 'generic
names' recommended practise.  Something like 'format' I suppose could
be specified, but I don't think it needs to be part of the reserved
binding.  Should these nodes have a compatible property that describes
it's usage?  ie; something like:

for a data block: compatible = "reserved-data", "reserved-memory";
	(like for an initrd or the dtb)
for a framebuffer: compatible = "reserved-framebuffer", "reserved-memory":
for shared memory: compatible = "reserved-shared", "reserved-memory";

On Mon, Feb 7, 2011 at 3:50 PM, Dan Hettena <danh at ghs.com> wrote:
> I've seen three types of reserved regions so far:
>
> (1) Reserved regions that are permanently reserved. For example,
>    RAM used for configuration of a device not communicated in the
>    device tree. Nothing further needs to be said about this class.
> (2) Reserved regions that are in use by something in the device tree
>    (for example, a spin table, or a frame buffer for a video device).
>    The client program can free these if it recognizes the node that
>    is using them and reprograms it (e.g., boot all processors, or
>    reprogram the video device).
> (3) Reserved regions that contain data the client program might want to
>    use but are not actually being used by the hardware for anything.
>    For example, a RAM root file system.
>
> I think this points strongly at having type 2 and type 3
> reservations specified along with the nodes that make use of them,
> so that the client program can free the memory as soon as it has
> caused the reserved memory to no longer be in use for the purpose
> for which it was reserved (provided that we make it clear that
> reservations declared as such are not permitted to overlap).

Yes.  I think phandles would do the trick nicely for this.  It would
also be possible to allow an initrd-region = <&initrd_reserved>;
property in the chosen node, but there probably isn't much benefit in
doing so, and it would just create yet another new binding that client
programs need to support in addition to the old binding.

>
> Primarily because of type 1 reservations, I see no conflict between
> the conventional reservations list and the idea of explicitly
> identifying single-purpose reservations throughout the device tree
> where they are used.

IMO, type 1 reservations still need to be specified explicitly in the
memory nodes (instead of, or in addition to, in the header).  It just
makes life so much easier when all the data about the machine is fully
specified in the device tree proper.  Otherwise extra hoops need to be
jumped through to get at it.

However, if nothing in the tree actually references a reserved
region node, then the client must treat it as an 'anonymous' region
and it does not have the option of ignoring or freeing it.

> However, in the spirit of distributing memory nodes, I think I would
> rather see the conventional reservations list moved to the
> corresponding memory nodes (with an added rule that a reservation
> may not cross a reg entry boundary, since that's just a big pain).
> If we're going to work towards keeping crap out of the header, then
> I don't see why the reservations block should be treated any
> differently.

I 100% agree.  The reserved regions nodes should be children of the
associated memory node, and reservations should not cross boundaries.
Collecting all the reservations in one place also give the client a
fighting chance of locating them easily when it parses the memory
node.

g.