On Fri, Dec 02, 2016 at 11:09:49AM +0200, Pantelis Antoniou wrote:
> Hi David,
>
> > On Dec 2, 2016, at 05:25 , David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> wrote:
> >
> > On Tue, Nov 29, 2016 at 01:21:40PM +0200, Pantelis Antoniou wrote:
> >> Hi Frank,
> >>
> >>> On Nov 29, 2016, at 03:36 , Frank Rowand <frowand.list@xxxxxxxxx> wrote:
> >>>
> >>> On 11/25/16 04:32, Pantelis Antoniou wrote:
> >>>> Provides the document explaining the internal mechanics of
> >>>> plugins and options.
> >>>>
> >>>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou@xxxxxxxxxxxx>
> >>>> ---
> >>>> Documentation/dt-object-internal.txt | 318 +++++++++++++++++++++++++++++++++++
> >>>> 1 file changed, 318 insertions(+)
> >>>> create mode 100644 Documentation/dt-object-internal.txt
> >>>>
> >>>> diff --git a/Documentation/dt-object-internal.txt b/Documentation/dt-object-internal.txt
> >>>> new file mode 100644
> >>>> index 0000000..d5b841e
> >>>> --- /dev/null
> >>>> +++ b/Documentation/dt-object-internal.txt
> >>>> @@ -0,0 +1,318 @@
> >>>> +Device Tree Dynamic Object format internals
> >>>> +-------------------------------------------
> >>>> +
> >>>> +The Device Tree for most platforms is a static representation of
> >>>> +the hardware capabilities. This is insufficient for many platforms
> >>>> +that need to dynamically insert device tree fragments to the
> >>>> +running kernel's live tree.
> >>>> +
> >>>> +This document explains the the device tree object format and the
> >>>> +modifications made to the device tree compiler, which make it possible.
> >>>> +
> >>>> +1. Simplified Problem Definition
> >>>> +--------------------------------
> >>>> +
> >>>> +Assume we have a platform which boots using following simplified device tree.
> >>>> +
> >>>> +---- foo.dts -----------------------------------------------------------------
> >>>> + /* FOO platform */
> >>>> + / {
> >>>> + compatible = "corp,foo";
> >>>> +
> >>>> + /* shared resources */
> >>>> + res: res {
> >>>> + };
> >>>> +
> >>>> + /* On chip peripherals */
> >>>> + ocp: ocp {
> >>>> + /* peripherals that are always instantiated */
> >>>> + peripheral1 { ... };
> >>>> + };
> >>>> + };
> >>>> +---- foo.dts -----------------------------------------------------------------
> >>>> +
> >>>> +We have a number of peripherals that after probing (using some undefined method)
> >>>> +should result in different device tree configuration.
> >>>> +
> >>>> +We cannot boot with this static tree because due to the configuration of the
> >>>> +foo platform there exist multiple conficting peripherals DT fragments.
> >>>
> >>> ^^^^^^^^^^ conflicting
> >>>
> >>> I assume conflicting because, for instance, the different peripherals might
> >>> occupy the same address space, use the same interrupt, or use the same gpio.
> >>> Mentioning that would provide a fuller picture for the neophyte.
> >>>
> >>
> >> Yes, thanks for bringing this to my attention. This document is heavy on the neophyte for sure.
> >>
> >>>> +
> >>>> +So for the bar peripheral we would have this:
> >>>> +
> >>>> +---- foo+bar.dts -------------------------------------------------------------
> >>>> + /* FOO platform + bar peripheral */
> >>>> + / {
> >>>> + compatible = "corp,foo";
> >>>> +
> >>>> + /* shared resources */
> >>>> + res: res {
> >>>> + };
> >>>> +
> >>>> + /* On chip peripherals */
> >>>> + ocp: ocp {
> >>>> + /* peripherals that are always instantiated */
> >>>> + peripheral1 { ... };
> >>>> +
> >>>> + /* bar peripheral */
> >>>> + bar {
> >>>> + compatible = "corp,bar";
> >>>> + ... /* various properties and child nodes */
> >>>> + };
> >>>> + };
> >>>> + };
> >>>> +---- foo+bar.dts -------------------------------------------------------------
> >>>> +
> >>>> +While for the baz peripheral we would have this:
> >>>> +
> >>>> +---- foo+baz.dts -------------------------------------------------------------
> >>>> + /* FOO platform + baz peripheral */
> >>>> + / {
> >>>> + compatible = "corp,foo";
> >>>> +
> >>>> + /* shared resources */
> >>>> + res: res {
> >>>> + /* baz resources */
> >>>> + baz_res: res_baz { ... };
> >>>> + };
> >>>> +
> >>>> + /* On chip peripherals */
> >>>> + ocp: ocp {
> >>>> + /* peripherals that are always instantiated */
> >>>> + peripheral1 { ... };
> >>>> +
> >>>> + /* baz peripheral */
> >>>> + baz {
> >>>> + compatible = "corp,baz";
> >>>> + /* reference to another point in the tree */
> >>>> + ref-to-res = <&baz_res>;
> >>>> + ... /* various properties and child nodes */
> >>>> + };
> >>>> + };
> >>>> + };
> >>>> +---- foo+baz.dts -------------------------------------------------------------
> >>>> +
> >>>> +We note that the baz case is more complicated, since the baz peripheral needs to
> >>>> +reference another node in the DT tree.
> >>>
> >>> I know that there are other situations that can justify overlays, so not
> >>> contesting the basic need with this comment. But the above situation could
> >>> be handled in a much simpler fashion by setting the status property of each
> >>> of the conflicting devices to disabled, then after probing setting the status
> >>> to ok. That method removes a lot of complexity.
> >>>
> >>> A big driver for the concept of overlays was being able to describe different
> >>> add on boards at run time, instead of when the base dtb was created. I think
> >>> we have agreed that moving to a connector model instead of a raw overlay is
> >>> the proper way to address add on boards.
> >>>
> >>> Can you address how an overlay can be created that will work for a board
> >>> plugged into any of the identical sockets that is compatible with the
> >>> board?
> >>>
> >>>
> >>
> >> Yes, I will try to do so.
> >>
> >>>> +
> >>>> +2. Device Tree Object Format Requirements
> >>>> +-----------------------------------------
> >>>> +
> >>>> +Since the device tree is used for booting a number of very different hardware
> >>>> +platforms it is imperative that we tread very carefully.
> >>>> +
> >>>> +2.a) No changes to the Device Tree binary format for the base tree. We cannot
> >>>> +modify the tree format at all and all the information we require should be
> >>>> +encoded using device tree itself. We can add nodes that can be safely ignored
> >>>> +by both bootloaders and the kernel. The plugin dtb's are optionally tagged
> >>>> +with a different magic number in the header but otherwise they too are simple
> >>>> +blobs.
> >>>> +
> >>>> +2.b) Changes to the DTS source format should be absolutely minimal, and should
> >>>> +only be needed for the DT fragment definitions, and not the base boot DT.
> >>>> +
> >>>> +2.c) An explicit option should be used to instruct DTC to generate the required
> >>>> +information needed for object resolution. Platforms that don't use the
> >>>> +dynamic object format can safely ignore it.
> >>>> +
> >>>> +2.d) Finally, DT syntax changes should be kept to a minimum. It should be
> >>>> +possible to express everything using the existing DT syntax.
> >>>> +
> >>>> +3. Implementation
> >>>> +-----------------
> >>>> +
> >>>> +The basic unit of addressing in Device Tree is the phandle. Turns out it's
> >>>> +relatively simple to extend the way phandles are generated and referenced
> >>>> +so that it's possible to dynamically convert symbolic references (labels)
> >>>> +to phandle values. This is a valid assumption as long as the author uses
> >>>> +reference syntax and does not assign phandle values manually (which might
> >>>> +be a problem with decompiled source files).
> >>>> +
> >>>> +We can roughly divide the operation into two steps.
> >>>> +
> >>>> +3.a) Compilation of the base board DTS file using the '-@' option
> >>>> +generates a valid DT blob with an added __symbols__ node at the root node,
> >>>> +containing a list of all nodes that are marked with a label.
> >>>> +
> >>>> +Using the foo.dts file above the following node will be generated;
> >>>> +
> >>>> +$ dtc -@ -O dtb -o foo.dtb -b 0 foo.dts
> >>>> +$ fdtdump foo.dtb
> >>>> +...
> >>>> +/ {
> >>>> + ...
> >>>> + res {
> >>>> + ...
> >>>> + phandle = <0x00000001>;
> >>>> + ...
> >>>> + };
> >>>> + ocp {
> >>>> + ...
> >>>> + phandle = <0x00000002>;
> >>>> + ...
> >>>> + };
> >>>> + __symbols__ {
> >>>> + res="/res";
> >>>> + ocp="/ocp";
> >>>> + };
> >>>> +};
> >>>> +
> >>>> +Notice that all the nodes that had a label have been recorded, and that
> >>>> +phandles have been generated for them.
> >>>> +
> >>>> +This blob can be used to boot the board normally, the __symbols__ node will
> >>>> +be safely ignored both by the bootloader and the kernel (the only loss will
> >>>> +be a few bytes of memory and disk space).
> >>>> +
> >>>> +3.b) The Device Tree fragments must be compiled with the same option but they
> >>>> +must also have a tag (/plugin/) that allows undefined references to nodes
> >>>> +that are not present at compilation time to be recorded so that the runtime
> >>>> +loader can fix them.
> >>>> +
> >>>> +So the bar peripheral's DTS format would be of the form:
> >>>> +
> >>>> +/dts-v1/ /plugin/; /* allow undefined references and record them */
> >>>> +/ {
> >>>> + .... /* various properties for loader use; i.e. part id etc. */
> >>>> + fragment@0 {
> >>>> + target = <&ocp>;
> >>>> + __overlay__ {
> >>>> + /* bar peripheral */
> >>>> + bar {
> >>>> + compatible = "corp,bar";
> >>>> + ... /* various properties and child nodes */
> >>>> + }
> >>>> + };
> >>>> + };
> >>>> +};
> >>>
> >>> The last version of your patches that I tested did not require specifying
> >>> the target property, the fragment node, and the __overlay__ node. dtc
> >>> properly created all of those items automatically. For example, I could
> >>> go to all of the trouble of creating those items in a dts like:
> >>>
> >>> $ cat example_1_hand_coded.dts
> >>> /dts-v1/;
> >>> /plugin/;
> >>>
> >>> / {
> >>>
> >>> fragment@0 {
> >>> target = <&am3353x_pinmux>;
> >>>
> >>> __overlay__ {
> >>>
> >>> i2c1_pins: pinmux_i2c1_pins {
> >>> pinctrl-single,pins = <
> >>> 0x158 0x72
> >>> 0x15c 0x72
> >>> >;
> >>> };
> >>> };
> >>> };
> >>>
> >>> fragment@1 {
> >>> target = <&i2c1>;
> >>>
> >>> __overlay__ {
> >>> pinctrl-names = "default";
> >>> pinctrl-0 = <&i2c1_pins>;
> >>> clock-frequency = <400000>;
> >>> status = "okay";
> >>>
> >>> at24@50 {
> >>> compatible = "at,24c256";
> >>> pagesize = <64>;
> >>> reg = <0x50>;
> >>> };
> >>> };
> >>> };
> >>> };
> >>>
> >>>
> >>> Or I could let dtc automagically create all the special features
> >>> (target, fragment, __overlay__) from an equivalent dts:
> >>>
> >>> $ cat example_1.dts
> >>> /dts-v1/;
> >>> /plugin/;
> >>>
> >>>
> >>> &am3353x_pinmux {
> >>> i2c1_pins: pinmux_i2c1_pins {
> >>> pinctrl-single,pins = <
> >>> 0x158 0x72
> >>> 0x15c 0x72
> >>> >;
> >>> };
> >>> };
> >>>
> >>> &i2c1 {
> >>> #address-cells = <1>;
> >>> #size-cells = <0>;
> >>> pinctrl-names = "default";
> >>> pinctrl-0 = <&i2c1_pins>;
> >>> clock-frequency = <400000>;
> >>> status = "okay";
> >>>
> >>> at24@50 {
> >>> compatible = "at,24c256";
> >>> pagesize = <64>;
> >>> reg = <0x50>;
> >>> };
> >>> };
> >>>
> >>>
> >>> I would much prefer that people never hand code the target, fragment, and
> >>> __overlay__ in a dts source file. Exposing them at the source level adds
> >>> complexity, confusion, and an increased chance of creating an invalid
> >>> overlay dtb.
> >>>
> >>> If possible, I would prefer target, fragment, and __overlay__ not be valid
> >>> input to dtc. It would probably be difficult to prohibit target and fragment,
> >>> because however unlikely they are as property and node names, they are valid
> >>> dts syntax before adding the overlay enhancements to dtc. However __overlay__
> >>> is not a valid node name without the overlay enhancements and could remain
> >>> invalid dts input.
> >>>
> >>> I prefer that target, fragment, and __overlay__ be documented as a dtb to
> >>> target system API. In this case, for the normal developer, they are
> >>> hidden in the binary dtb format and in the kernel (or boot loader)
> >>> overlay framework code.
> >>>
> >>> I do recognize that if __overlay__ is not valid dtc input then it is not
> >>> possible to decompile an overlay into a dts containing __overlay__ and
> >>> then recompile that dts. This could be resolved by a more complex
> >>> decompile that turned the overlay dtb back into the form of example_1.dts
> >>> above.
> >>>
> >>> After reading to the end of this patch, I see that the simpler form of
> >>> .dts (like example_1.dts) is also noted as "an alternative syntax to
> >>> the expanded form for overlays".
> >>>
> >>>
> >>
> >> Phew.
> >>
> >> Let me address all that.
> >>
> >> When I started on this the main problem was that there was no support for applying
> >> overlays in the kernel. The original patch series for dtc is meant to support the
> >> encoding of the required information into device tree format.
> >>
> >> The syntax of overlays like this '&foo { };’ is a new thing that can be subject to
> >> change.
> >
> > Well.. yes and no. What I'm going to call "compile time overlays"
> > using that syntax have been around for ages (rather longer than
> > dynamic overlays). The semantics you hve for runtime overlay
> > application are pretty much identical to those for compile time
> > overlays, except (duh) applied later.
> >
> > That's why I want to unify the syntax between the two. And, up to a
> > point, to unify the concepts as well. This is why I want to treat
> > this as having dtc parse the source into a bundle of overlays which it
> > then decides whether it needs to apply immediately (compile time
> > overlay) or encode them into the dtbo format for the bootloader or
> > kernel to apply later (dynamic overlay).
> >
>
> It is a worthy goal, but it will require quite a lot of work (and time).
Hm.. I think you're overestimating the complexity of it. A reasonable
first chunk I've already done in that 'overlay' branch.
> One thing that comes to mind is mapping the semantics of the compile time
> ‘overlays’ to run time will require changes in the blob format.
>
> For instance the syntax for deleting nodes/properties has no mapping to
> runtime. We will need to figure out how to encode them, etc.
No support for deletions is a difference yes, but we could just give
an error when trying to encode a compile time overlay into a runtime
fragment if there are any deletions within. At least until we come up
with a runtime overlay encoding for deletions.
.. and, I'm pretty sure that's the *only* semantic difference between
compile time overlays and dtbo fragments.
> It is something that can wait while we get things right, we don’t have a
> pressing need right now.
>
> >> On the last patchset I’ve split it out so that it is clear.
> >
> > Yeah, but you're splitting it based on the history, rather than what I
> > think is the conceptually clearer approach: first, allow the overlay
> > (&ref { ... }) syntax to be either compile-time or dynamic. second,
> > add in backwards compatiblity hacks for manually encoded dts files.
> >
>
> I’m not sure what your point here is. First things first; encoding of runtime
> overlays without changes in syntax. Next comes the new syntax for defining them.
>
> There is no backward compatibility hack. The hack _is_ the &ref { } syntax since
> nothing uses it now.
>
> We intent to use from now on, true, but it’s been holding up the rest of the
> patchset for years now.
Hm, yeah, I guess.
> >> Now, since we’ve settled on the internal encoding format (__overlays__, target, etc)
> >> we can tackle the syntax cases and alternative target options.
> >
> > But that's not an internal encoding format, it's an _external_
> > encoding format.
>
> I concede that for the definition of internal/external from the viewpoint
> of the dtc compiler.
>
> >
> >> So, yes we should forbid __overlay__ to be a valid node name eventually along with
> >> a bunch of other syntax stuff.
> >>
> >> Having come to mind, we should see what we need for the connector
> >> format to work.
> >
> > No argument there.
> >
> >>
> >>>> +
> >>>> +Note that there's a target property that specifies the location where the
> >>>> +contents of the overlay node will be placed, and it references the node
> >>>> +in the foo.dts file.
> >>>> +
> >>>> +$ dtc -@ -O dtb -o bar.dtbo -b 0 bar.dts
> >>>> +$ fdtdump bar.dtbo
> >>>> +...
> >>>> +/ {
> >>>> + ... /* properties */
> >>>> + fragment@0 {
> >>>> + target = <0xffffffff>;
> >>>> + __overlay__ {
> >>>> + bar {
> >>>> + compatible = "corp,bar";
> >>>> + ... /* various properties and child nodes */
> >>>> + }
> >>>> + };
> >>>> + };
> >>>> + __fixups__ {
> >>>> + ocp = "/fragment@0:target:0";
> >>>> + };
> >>>> +};
> >>>> +
> >>>> +No __symbols__ has been generated (no label in bar.dts).
> >>>> +Note that the target's ocp label is undefined, so the phandle handle
> >>>> +value is filled with the illegal value '0xffffffff', while a __fixups__
> >>>> +node has been generated, which marks the location in the tree where
> >>>> +the label lookup should store the runtime phandle value of the ocp node.
> >>>> +
> >>>> +The format of the __fixups__ node entry is
> >>>> +
> >>>> + <label> = "<local-full-path>:<property-name>:<offset>";
> >>>> +
> >>>> +<label> Is the label we're referring
> >>>> +<local-full-path> Is the full path of the node the reference is
> >>>> +<property-name> Is the name of the property containing the
> >>>> + reference
> >>>> +<offset> The offset (in bytes) of where the property's
> >>>> + phandle value is located.
> >>>> +
> >>>> +Doing the same with the baz peripheral's DTS format is a little bit more
> >>>> +involved, since baz contains references to local labels which require
> >>>> +local fixups.
> >>>> +
> >>>> +/dts-v1/ /plugin/; /* allow undefined label references and record them */
> >>>> +/ {
> >>>> + .... /* various properties for loader use; i.e. part id etc. */
> >>>> + fragment@0 {
> >>>> + target = <&res>;
> >>>> + __overlay__ {
> >>>> + /* baz resources */
> >>>> + baz_res: res_baz { ... };
> >>>> + };
> >>>> + };
> >>>> + fragment@1 {
> >>>> + target = <&ocp>;
> >>>> + __overlay__ {
> >>>> + /* baz peripheral */
> >>>> + baz {
> >>>> + compatible = "corp,baz";
> >>>> + /* reference to another point in the tree */
> >>>> + ref-to-res = <&baz_res>;
> >>>> + ... /* various properties and child nodes */
> >>>> + }
> >>>> + };
> >>>> + };
> >>>> +};
> >>>> +
> >>>> +Note that &bar_res reference.
> >>>> +
> >>>> +$ dtc -@ -O dtb -o baz.dtbo -b 0 baz.dts
> >>>> +$ fdtdump baz.dtbo
> >>>> +...
> >>>> +/ {
> >>>> + ... /* properties */
> >>>> + fragment@0 {
> >>>> + target = <0xffffffff>;
> >>>> + __overlay__ {
> >>>> + res_baz {
> >>>> + ....
> >>>> + phandle = <0x00000001>;
> >>>> + };
> >>>> + };
> >>>> + };
> >>>> + fragment@1 {
> >>>> + target = <0xffffffff>;
> >>>> + __overlay__ {
> >>>> + baz {
> >>>> + compatible = "corp,baz";
> >>>> + ... /* various properties and child nodes */
> >>>> + ref-to-res = <0x00000001>;
> >>>> + }
> >>>> + };
> >>>> + };
> >>>> + __fixups__ {
> >>>> + res = "/fragment@0:target:0";
> >>>> + ocp = "/fragment@1:target:0";
> >>>> + };
> >>>> + __local_fixups__ {
> >>>> + fragment@1 {
> >>>> + __overlay__ {
> >>>> + baz {
> >>>> + ref-to-res = <0>;
> >>>> + };
> >>>> + };
> >>>> + };
> >>>> + };
> >>>> +};
> >>>> +
> >>>> +This is similar to the bar case, but the reference of a local label by the
> >>>> +baz node generates a __local_fixups__ entry that records the place that the
> >>>> +local reference is being made. No matter how phandles are allocated from dtc
> >>>> +the run time loader must apply an offset to each phandle in every dynamic
> >>>> +DT object loaded. The __local_fixups__ node records the place of every
> >>>> +local reference so that the loader can apply the offset.
> >>>> +
> >>>> +There is an alternative syntax to the expanded form for overlays with phandle
> >>>> +targets which makes the format similar to the one using in .dtsi include files.
> >>>> +
> >>>> +So for the &ocp target example above one can simply write:
> >>>> +
> >>>> +/dts-v1/ /plugin/;
> >>>> +&ocp {
> >>>> + /* bar peripheral */
> >>>> + bar {
> >>>> + compatible = "corp,bar";
> >>>> + ... /* various properties and child nodes */
> >>>> + }
> >>>> +};
> >>>> +
> >>>> +The resulting dtb object is identical.
> >>>>
> >>>
> >>
> >> Regards
> >>
> >> — Pantelis
> >>
> >
>
> Regards
>
> — Pantelis
>
--
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
