Re: [PATCH v7 3/5] dtc: Document the dynamic plugin internals

[David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 10241 bytes --]

On Thu, May 26, 2016 at 09:31:20AM +0300, Pantelis Antoniou wrote:
> Hi David,
> 
> > On May 26, 2016, at 09:28 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > 
> > On Thu, May 26, 2016 at 09:14:49AM +0300, Pantelis Antoniou wrote:
> >> Hi Frank,
> >> 
> >>> On May 25, 2016, at 22:13 , Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >>> 
> >>> On 5/24/2016 10:50 AM, Pantelis Antoniou wrote:
> >>>> Provides the document explaining the internal mechanics of
> >>>> plugins and options.
> >>>> 
> >>>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> >>>> ---
> >>>> 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.
> >>>> +
> >>>> +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.
> >>>> +
> >>>> +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 */
> >>>> +			}
> >>> 
> >>>                       };
> >>> 
> >>>> +		};
> >>>> +	};
> >>>> +};
> >>> 
> >>> Other than the fact that the above syntax is already in the Linux
> >>> kernel overlay implementation, is there a need for the target
> >>> property and the __overlay__ node?  I haven't figured out what
> >>> extra value they provide.
> >>> 
> >>> Without those added, the overlay dts becomes simpler (though for a
> >>> multi-node target path example this would be more complex unless a label
> >>> was used for the target node):
> >>> 
> >>> +/dts-v1/ /plugin/;	/* allow undefined references and record them */
> >>> +/ {
> >>> +	....	/* various properties for loader use; i.e. part id etc. */
> >>> +	ocp {
> >>> +			/* bar peripheral */
> >>> +			bar {
> >>> +				compatible = "corp,bar";
> >>> +				... /* various properties and child nodes */
> >>> +			};
> >>> +	};
> >>> +};
> >>> 
> >> 
> >> No.
> >> 
> >> That only works if the overlay is applied in a single platform.
> >> 
> >> I have working cases where the same overlay is applied on a ppc and a x86
> >> platform.
> > 
> > Huh?  How so..
> > 
> 
> Yes, it does work. Yes it’s being used right now. It is a very valid use case.
> 
> Think carrier boards on enterprise routers, plugging to a main board
> that’s either ppc or x86 (or anything else for that matter).

Sorry, I wasn't clear.  I have no problem believing overlays can be
applied on multiple platforms.

What I can't see is how Frank's format breaks that.  AFAICT it
contains exactly the same information in a simpler encoding.

-- 
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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]