All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] dtc: Dynamic symbols & fixup support (v2)
       [not found] ` <1383073266-12017-1-git-send-email-panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org>
@ 2013-11-11 17:56   ` Grant Likely
       [not found]     ` <20131111175604.ECF33C4233A-WNowdnHR2B42iJbIjFUEsiwD8/FfD2ys@public.gmane.org>
       [not found]     ` < DABC2A65-BC07-4481-B80C-A62777BD8732@antoniou-consulting.com>
  0 siblings, 2 replies; 13+ messages in thread
From: Grant Likely @ 2013-11-11 17:56 UTC (permalink / raw)
  To: Pantelis Antoniou, Jon Loeliger
  Cc: Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA, Koen Kooi,
	Pantelis-s3s/WqlpOiPyB63q8FvJNQ

On Tue, 29 Oct 2013 21:01:06 +0200, Pantelis Antoniou <panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org> wrote:
> Enable the generation of symbol & fixup information for
> usage with dynamic DT loading.
> 
> Passing the -@ option generates a __symbols__ node at the
> root node of the resulting blob for any node labels used.
> 
> When using the /plugin/ tag all unresolved label references
> be tracked in the __fixups__ node, while all local phandle
> references will the tracked in the __local_fixups__ node.
> 
> This is sufficient to implement a dynamic DT object loader.
> 
> Changes since v1:
> 
> * Forward ported to 1.4 version
> * Added manual entries for the -@ option
> 
> Signed-off-by: Pantelis Antoniou <panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org>

Hi Pantelis,

This looks pretty good on first look. Comments below.

> ---
>  Documentation/dt-object-internal.txt | 295 +++++++++++++++++++++++++++++++++++
>  Documentation/manual.txt             |  10 ++
>  checks.c                             | 120 +++++++++++++-
>  dtc-lexer.l                          |   5 +
>  dtc-parser.y                         |  23 ++-
>  dtc.c                                |   9 +-
>  dtc.h                                |  38 +++++
>  flattree.c                           | 139 +++++++++++++++++
>  8 files changed, 630 insertions(+), 9 deletions(-)
>  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..eb5d4c0
> --- /dev/null
> +++ b/Documentation/dt-object-internal.txt
> @@ -0,0 +1,295 @@
> +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. 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.

Not /entirely/ true. It would be completely fine to bump up the binary
format version for the overlays since they will never be loaded
standalone. If there are specific things that you would like to have in
the marshalled format then go ahead and propose them. Then the overlay
would use the new protocol version, but the base tree would use the
existing one.

> +
> +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.
> +
> +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 {
> +		...
> +		linux,phandle = <0x00000001>;
> +		phandle = <0x00000001>;
> +		...
> +	};
> +	ocp {
> +		...
> +		linux,phandle = <0x00000002>;
> +		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 labels
> +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:
> +
> +/plugin/;	/* allow undefined label 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 */
> +			}
> +		};
> +	};
> +};
> +
> +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 label
> +in the foo.dts file.
> +
> +$ dtc -@ -O dtb -o bar.dtbo -b 0 bar.dts
> +$ fdtdump bar.dtbo
> +...
> +/ {
> +	... /* properties */
> +	fragment@0 {
> +		target = <0xdeadbeef>;
> +		__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 '0xdeadbeef', 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.
> +
> +/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 = <0xdeadbeef>;
> +		__overlay__ {
> +			res_baz {
> +				....
> +				linux,phandle = <0x00000001>;
> +				phandle = <0x00000001>;
> +			};
> +		};
> +	};
> +	fragment@1 {
> +		target = <0xdeadbeef>;
> +		__overlay__ {
> +			baz {
> +				compatible = "corp,baz";
> +				... /* various properties and child nodes */
> +				res=<0x00000001>;
> +			}
> +		};
> +	};
> +	__fixups__ {
> +		res = "/fragment@0:target:0";
> +		ocp = "/fragment@1:target:0";

What happens when multiple phandles in the overlay reference the same
node? This could result in a lot of strings for fixup references.

> +	};
> +	__local_fixups__ {
> +		fixup = </fragment@1/__overlay__/baz:res:0>;

Ditto here, I'm a little concerned that a ton of strings will end up in
this node.

> +	};

Another way to do this would be to not fixup the tree at all but instead
have a translation node and get Linux to keep track of which overlay a
node is a part of. Each overlay would have it's own set of phandle
translators. It /might/ result in simpler code on the OS implementation
side, particularly since all phandle resolutions should go through
of_find_node_by_phandle(). Doing so would mean all the above tree might
look something like:

/ {
	... /* properties */
	fragment@0 {
		target = <0x3>;
		__overlay__ {
			res_baz {
				....
				linux,phandle = <0x00000001>;
				phandle = <0x00000001>;
			};
		};
	};
	fragment@1 {
		target = <0x4>;
		__overlay__ {
			baz {
				compatible = "corp,baz";
				... /* various properties and child nodes */
				res=<0x00000001>;
			}
		};
	};
	__external_refs__ {
		res = 0x3;
		ocp = 0x4;
	};
};

Where the phandles within the tree are entirely self-consistent, and
external phandles are translated through the __external_refs__ node to
decode the phandle in the parent tree.

I'm not trying to force you down this path, but I'd like to know if you considered
other approaches such as this.

(Besides, there are a couple of unresolved items with my suggestion. It
is potentially problematic for kexec or anything else that reads the
tree from userspace. The phandle domains would need to be exposed via
sysfs/procfs. It also wouldn't be suitable for merging the tree at
runtime by firmware before passing to the kernel).

> +};
> +
> +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. Since phandles are allocated starting at 1
> +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.
> diff --git a/Documentation/manual.txt b/Documentation/manual.txt
> index 65c8540..9b4d329 100644
> --- a/Documentation/manual.txt
> +++ b/Documentation/manual.txt
> @@ -121,6 +121,14 @@ Options:
>  	Make space for <number> reserve map entries
>  	Relevant for dtb and asm output only.
>  
> +    -@
> +        Generates a __symbols__ node at the root node of the resulting blob
> +	for any node labels used.
> +
> +	When using the /plugin/ tag all unresolved label references
> +	be tracked in the __fixups__ node, while all local phandle
> +	references will the tracked in the __local_fixups__ node.
> +

I'm just curious: why did you choose '@'?

Overall the approach looks sane to me. I need to read through the
kernel patches now. I'll probably think of other things when I do.

Acked-in-principle-by: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

g.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] dtc: Dynamic symbols & fixup support (v2)
       [not found]     ` <20131111175604.ECF33C4233A-WNowdnHR2B42iJbIjFUEsiwD8/FfD2ys@public.gmane.org>
@ 2013-11-12 10:20       ` Pantelis Antoniou
  0 siblings, 0 replies; 13+ messages in thread
From: Pantelis Antoniou @ 2013-11-12 10:20 UTC (permalink / raw)
  To: Grant Likely
  Cc: Jon Loeliger, Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Koen Kooi, Pantelis-s3s/WqlpOiPyB63q8FvJNQ

Hi Grant,

On Nov 11, 2013, at 6:56 PM, Grant Likely wrote:

> On Tue, 29 Oct 2013 21:01:06 +0200, Pantelis Antoniou <panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org> wrote:
>> Enable the generation of symbol & fixup information for
>> usage with dynamic DT loading.
>> 
>> Passing the -@ option generates a __symbols__ node at the
>> root node of the resulting blob for any node labels used.
>> 
>> When using the /plugin/ tag all unresolved label references
>> be tracked in the __fixups__ node, while all local phandle
>> references will the tracked in the __local_fixups__ node.
>> 
>> This is sufficient to implement a dynamic DT object loader.
>> 
>> Changes since v1:
>> 
>> * Forward ported to 1.4 version
>> * Added manual entries for the -@ option
>> 
>> Signed-off-by: Pantelis Antoniou <panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org>
> 
> Hi Pantelis,
> 
> This looks pretty good on first look. Comments below.
> 
>> ---
>> Documentation/dt-object-internal.txt | 295 +++++++++++++++++++++++++++++++++++
>> Documentation/manual.txt             |  10 ++

[snip]

>> +-----------------------------------------
>> +
>> +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. 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.
> 
> Not /entirely/ true. It would be completely fine to bump up the binary
> format version for the overlays since they will never be loaded
> standalone. If there are specific things that you would like to have in
> the marshalled format then go ahead and propose them. Then the overlay
> would use the new protocol version, but the base tree would use the
> existing one.

OK, I see. I am very hesitant to bump the format version, because both the
base DTS and the overlay need to be compiled with symbol support.
We can bump the version format for the overlay fragment, but bumping the
version for the base DTS will affect bootloaders badly.

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

[snip]
>> +	};
>> +	__fixups__ {
>> +		res = "/fragment@0:target:0";
>> +		ocp = "/fragment@1:target:0";
> 
> What happens when multiple phandles in the overlay reference the same
> node? This could result in a lot of strings for fixup references.
> 

Yes. 

>> +	};
>> +	__local_fixups__ {
>> +		fixup = </fragment@1/__overlay__/baz:res:0>;
> 
> Ditto here, I'm a little concerned that a ton of strings will end up in
> this node.
> 

Same.

FWIW, it's a few bytes; to get to a waste of a K that's going to take
an overlay much more complex that I'm familiar with.

Maybe the FGPA guys can share about their cases and how long their strings
get.

>> +	};
> 
> Another way to do this would be to not fixup the tree at all but instead
> have a translation node and get Linux to keep track of which overlay a
> node is a part of. Each overlay would have it's own set of phandle
> translators. It /might/ result in simpler code on the OS implementation
> side, particularly since all phandle resolutions should go through
> of_find_node_by_phandle(). Doing so would mean all the above tree might
> look something like:
> 

> / {
> 	... /* properties */
> 	fragment@0 {
> 		target = <0x3>;
> 		__overlay__ {
> 			res_baz {
> 				....
> 				linux,phandle = <0x00000001>;
> 				phandle = <0x00000001>;
> 			};
> 		};
> 	};
> 	fragment@1 {
> 		target = <0x4>;
> 		__overlay__ {
> 			baz {
> 				compatible = "corp,baz";
> 				... /* various properties and child nodes */
> 				res=<0x00000001>;
> 			}
> 		};
> 	};
> 	__external_refs__ {
> 		res = 0x3;
> 		ocp = 0x4;
> 	};
> };
> 
> Where the phandles within the tree are entirely self-consistent, and
> external phandles are translated through the __external_refs__ node to
> decode the phandle in the parent tree.
> 
> I'm not trying to force you down this path, but I'd like to know if you considered
> other approaches such as this.
> 

It can be made to work, but I can't tell at a glance all the possible problems
that can crop up. It is much trickier that the current implementation.

> (Besides, there are a couple of unresolved items with my suggestion. It
> is potentially problematic for kexec or anything else that reads the
> tree from userspace. The phandle domains would need to be exposed via
> sysfs/procfs. It also wouldn't be suitable for merging the tree at
> runtime by firmware before passing to the kernel).
> 

See above: It can be made to work, and the gain will be a few hundred bytes
savings in blob size.

Whether this is worth it? IMHO no, not at this point.

We can tackle this as part of the next blob version update perhaps?

>> +};
>> +
>> +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. Since phandles are allocated starting at 1
>> +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.
>> diff --git a/Documentation/manual.txt b/Documentation/manual.txt
>> index 65c8540..9b4d329 100644
>> --- a/Documentation/manual.txt
>> +++ b/Documentation/manual.txt
>> @@ -121,6 +121,14 @@ Options:
>> 	Make space for <number> reserve map entries
>> 	Relevant for dtb and asm output only.
>> 
>> +    -@
>> +        Generates a __symbols__ node at the root node of the resulting blob
>> +	for any node labels used.
>> +
>> +	When using the /plugin/ tag all unresolved label references
>> +	be tracked in the __fixups__ node, while all local phandle
>> +	references will the tracked in the __local_fixups__ node.
>> +
> 
> I'm just curious: why did you choose '@'?
> 

Non-letter, and @=at. Symbols point _at_ something. Corny I know.

> Overall the approach looks sane to me. I need to read through the
> kernel patches now. I'll probably think of other things when I do.
> 
> Acked-in-principle-by: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> 

Thanks.

> g.

Regards

-- Pantelis

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] dtc: Dynamic symbols & fixup support (v2)
       [not found]       ` <DABC2A65-BC07-4481-B80C-A62777BD8732-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org>
@ 2013-11-15  7:01         ` Grant Likely
       [not found]           ` <20131115070131.78139C40785-WNowdnHR2B42iJbIjFUEsiwD8/FfD2ys@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Grant Likely @ 2013-11-15  7:01 UTC (permalink / raw)
  To: Pantelis Antoniou
  Cc: Jon Loeliger, Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Koen Kooi, Pantelis-s3s/WqlpOiPyB63q8FvJNQ

On Tue, 12 Nov 2013 11:20:04 +0100, Pantelis Antoniou <panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org> wrote:
> Hi Grant,
> 
> On Nov 11, 2013, at 6:56 PM, Grant Likely wrote:
> 
> > On Tue, 29 Oct 2013 21:01:06 +0200, Pantelis Antoniou <panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org> wrote:
> >> Enable the generation of symbol & fixup information for
> >> usage with dynamic DT loading.
> >> 
> >> Passing the -@ option generates a __symbols__ node at the
> >> root node of the resulting blob for any node labels used.
> >> 
> >> When using the /plugin/ tag all unresolved label references
> >> be tracked in the __fixups__ node, while all local phandle
> >> references will the tracked in the __local_fixups__ node.
> >> 
> >> This is sufficient to implement a dynamic DT object loader.
> >> 
> >> Changes since v1:
> >> 
> >> * Forward ported to 1.4 version
> >> * Added manual entries for the -@ option
> >> 
> >> Signed-off-by: Pantelis Antoniou <panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org>
> > 
> > Hi Pantelis,
> > 
> > This looks pretty good on first look. Comments below.
> > 
> >> ---
> >> Documentation/dt-object-internal.txt | 295 +++++++++++++++++++++++++++++++++++
> >> Documentation/manual.txt             |  10 ++
> 
> [snip]
> 
> >> +-----------------------------------------
> >> +
> >> +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. 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.
> > 
> > Not /entirely/ true. It would be completely fine to bump up the binary
> > format version for the overlays since they will never be loaded
> > standalone. If there are specific things that you would like to have in
> > the marshalled format then go ahead and propose them. Then the overlay
> > would use the new protocol version, but the base tree would use the
> > existing one.
> 
> OK, I see. I am very hesitant to bump the format version, because both the
> base DTS and the overlay need to be compiled with symbol support.
> We can bump the version format for the overlay fragment, but bumping the
> version for the base DTS will affect bootloaders badly.

Hmmm, I missed that detail. Why does the base tree require symbol
support? It looked to me like the overlay had all the resolution
information needed.

g.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] dtc: Dynamic symbols & fixup support (v2)
       [not found]           ` <20131115070131.78139C40785-WNowdnHR2B42iJbIjFUEsiwD8/FfD2ys@public.gmane.org>
@ 2013-11-16 17:47             ` Pantelis Antoniou
  0 siblings, 0 replies; 13+ messages in thread
From: Pantelis Antoniou @ 2013-11-16 17:47 UTC (permalink / raw)
  To: Grant Likely
  Cc: Jon Loeliger, Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Koen Kooi, Pantelis-s3s/WqlpOiPyB63q8FvJNQ

Hi Grant,

On Nov 15, 2013, at 9:01 AM, Grant Likely wrote:

> On Tue, 12 Nov 2013 11:20:04 +0100, Pantelis Antoniou <panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org> wrote:
>> Hi Grant,
>> 
>> On Nov 11, 2013, at 6:56 PM, Grant Likely wrote:
>> 
>>> On Tue, 29 Oct 2013 21:01:06 +0200, Pantelis Antoniou <panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org> wrote:
>>>> Enable the generation of symbol & fixup information for
>>>> usage with dynamic DT loading.
>>>> 
>>>> Passing the -@ option generates a __symbols__ node at the
>>>> root node of the resulting blob for any node labels used.
>>>> 
>>>> When using the /plugin/ tag all unresolved label references
>>>> be tracked in the __fixups__ node, while all local phandle
>>>> references will the tracked in the __local_fixups__ node.
>>>> 
>>>> This is sufficient to implement a dynamic DT object loader.
>>>> 
>>>> Changes since v1:
>>>> 
>>>> * Forward ported to 1.4 version
>>>> * Added manual entries for the -@ option
>>>> 
>>>> Signed-off-by: Pantelis Antoniou <panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org>
>>> 
>>> Hi Pantelis,
>>> 
>>> This looks pretty good on first look. Comments below.
>>> 
>>>> ---
>>>> Documentation/dt-object-internal.txt | 295 +++++++++++++++++++++++++++++++++++
>>>> Documentation/manual.txt             |  10 ++
>> 
>> [snip]
>> 
>>>> +-----------------------------------------
>>>> +
>>>> +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. 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.
>>> 
>>> Not /entirely/ true. It would be completely fine to bump up the binary
>>> format version for the overlays since they will never be loaded
>>> standalone. If there are specific things that you would like to have in
>>> the marshalled format then go ahead and propose them. Then the overlay
>>> would use the new protocol version, but the base tree would use the
>>> existing one.
>> 
>> OK, I see. I am very hesitant to bump the format version, because both the
>> base DTS and the overlay need to be compiled with symbol support.
>> We can bump the version format for the overlay fragment, but bumping the
>> version for the base DTS will affect bootloaders badly.
> 
> Hmmm, I missed that detail. Why does the base tree require symbol
> support? It looked to me like the overlay had all the resolution
> information needed.
> 

It is because the symbols are in our case the labels of the DTS. We need
to generate a node containing all the labels so that the overlays can use in order
to get the target of their insertion point.

> g.
> 

Regards

-- Pantelis

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] dtc: Dynamic symbols & fixup support (v2)
       [not found]           ` <972758FF-44EC-4C32-BE13-4E3E8361D063-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org>
@ 2013-11-17 22:16             ` Grant Likely
  0 siblings, 0 replies; 13+ messages in thread
From: Grant Likely @ 2013-11-17 22:16 UTC (permalink / raw)
  To: Pantelis Antoniou
  Cc: Jon Loeliger, Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Koen Kooi, Pantelis-s3s/WqlpOiPyB63q8FvJNQ

On Sat, 16 Nov 2013 19:47:45 +0200, Pantelis Antoniou <panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org> wrote:
> Hi Grant,
> 
> On Nov 15, 2013, at 9:01 AM, Grant Likely wrote:
> 
> > On Tue, 12 Nov 2013 11:20:04 +0100, Pantelis Antoniou <panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org> wrote:
> >> Hi Grant,
> >> 
> >> On Nov 11, 2013, at 6:56 PM, Grant Likely wrote:
> >> 
> >>> On Tue, 29 Oct 2013 21:01:06 +0200, Pantelis Antoniou <panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org> wrote:
> >>>> Enable the generation of symbol & fixup information for
> >>>> usage with dynamic DT loading.
> >>>> 
> >>>> Passing the -@ option generates a __symbols__ node at the
> >>>> root node of the resulting blob for any node labels used.
> >>>> 
> >>>> When using the /plugin/ tag all unresolved label references
> >>>> be tracked in the __fixups__ node, while all local phandle
> >>>> references will the tracked in the __local_fixups__ node.
> >>>> 
> >>>> This is sufficient to implement a dynamic DT object loader.
> >>>> 
> >>>> Changes since v1:
> >>>> 
> >>>> * Forward ported to 1.4 version
> >>>> * Added manual entries for the -@ option
> >>>> 
> >>>> Signed-off-by: Pantelis Antoniou <panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org>
> >>> 
> >>> Hi Pantelis,
> >>> 
> >>> This looks pretty good on first look. Comments below.
> >>> 
> >>>> ---
> >>>> Documentation/dt-object-internal.txt | 295 +++++++++++++++++++++++++++++++++++
> >>>> Documentation/manual.txt             |  10 ++
> >> 
> >> [snip]
> >> 
> >>>> +-----------------------------------------
> >>>> +
> >>>> +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. 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.
> >>> 
> >>> Not /entirely/ true. It would be completely fine to bump up the binary
> >>> format version for the overlays since they will never be loaded
> >>> standalone. If there are specific things that you would like to have in
> >>> the marshalled format then go ahead and propose them. Then the overlay
> >>> would use the new protocol version, but the base tree would use the
> >>> existing one.
> >> 
> >> OK, I see. I am very hesitant to bump the format version, because both the
> >> base DTS and the overlay need to be compiled with symbol support.
> >> We can bump the version format for the overlay fragment, but bumping the
> >> version for the base DTS will affect bootloaders badly.
> > 
> > Hmmm, I missed that detail. Why does the base tree require symbol
> > support? It looked to me like the overlay had all the resolution
> > information needed.
> > 
> 
> It is because the symbols are in our case the labels of the DTS. We need
> to generate a node containing all the labels so that the overlays can use in order
> to get the target of their insertion point.

For consistency, please use aliases or full paths to nodes instead. Make
DTC do the translation from labels to aliases/paths.

g.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] dtc: Dynamic symbols & fixup support (v2)
       [not found]   ` <CADhT+wdJfrRMDjm+jb_+_uuy8sPRd1B0F5mNVgOL2DmyHTMrYw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2013-11-08  7:03     ` Pantelis Antoniou
  0 siblings, 0 replies; 13+ messages in thread
From: Pantelis Antoniou @ 2013-11-08  7:03 UTC (permalink / raw)
  To: Dinh Nguyen; +Cc: devicetree-u79uwXL29TY76Z2rM5mHXA


On Nov 8, 2013, at 1:43 AM, Dinh Nguyen wrote:

> Hi Pantelis,
> 

Hi Dinh,

[snip]

> +
> +/plugin/;      /* allow undefined label references and record them */
> 
> Should the fragment also need to have the  /dts-v1/; tag as well? The dtc would fail for me if I did not have the /dts-v1/; tag.
> 
> Error: arch/arm/boot/dts/socfpga_overlay.dts:1.1-9 syntax error
> FATAL ERROR: Unable to parse input tree
> 

Yes. The plugin tag only turns on symbol tracking, it doesn't affect the syntax of the DTS input.

> Dinh

Regards

-- Pantelis--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] dtc: Dynamic symbols & fixup support (v2)
       [not found]         ` <53AA440E-7F33-4AC1-9323-7D109D4B5852-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org>
@ 2013-11-05  8:02           ` Sascha Hauer
  0 siblings, 0 replies; 13+ messages in thread
From: Sascha Hauer @ 2013-11-05  8:02 UTC (permalink / raw)
  To: Pantelis Antoniou; +Cc: devicetree-u79uwXL29TY76Z2rM5mHXA

On Mon, Nov 04, 2013 at 04:58:38PM +0200, Pantelis Antoniou wrote:
> Hi Sascha,
> 
> On Nov 4, 2013, at 4:53 PM, Sascha Hauer wrote:
> 
> > Hi Pantelis,
> > 
> > Thank you for working on this again. I would really appreciate seeing
> > this upstream.
> > 
> 
> Don't mention it. Let's try to make it happen then.
> 
> > On Mon, Nov 04, 2013 at 04:36:13PM +0200, Pantelis Antoniou wrote:
> >> +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 labels
> >> +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:
> >> +
> >> +/plugin/;	/* allow undefined label 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 */
> >> +			}
> >> +		};
> >> +	};
> >> +};
> > 
> > When last looking at this patch I created the attached follow up. What
> > I didn't like about this patch is that we currently have to write dts
> > files explicitely as overlays. With the attached patch the above could
> > be written as:
> > 
> > &ocp: {
> > 	compatible = "corp,bar";
> > };
> > 
> > This is easier to write and more important a dts snippet could be either
> > compiled into a dtb or used as an overlay.
> > 
> 
> I see your point.
> 
> In my use case I always needed some out-of-band properties for the loader/manager 
> to track. Let me test your and see if it doesn't break my use-cases.

You mean for unloading an overlay afterwards?

> 
> If it's fine, then I have no objections in including it.

Thanks.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] dtc: Dynamic symbols & fixup support (v2)
       [not found]       ` <CADhT+wcgs2cQv23+K7EooZaA0KK7Lk8zY-KAPc50xUdm5BFRZw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2013-11-05  7:49         ` Pantelis Antoniou
  0 siblings, 0 replies; 13+ messages in thread
From: Pantelis Antoniou @ 2013-11-05  7:49 UTC (permalink / raw)
  To: Dinh Nguyen; +Cc: Dinh Nguyen, devicetree-u79uwXL29TY76Z2rM5mHXA

Hi Dinh,

No worries :)

On Nov 4, 2013, at 11:26 PM, Dinh Nguyen wrote:

> 
> 
> 
> On Mon, Nov 4, 2013 at 3:17 PM, Dinh Nguyen <dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org> wrote:
> On Mon, 2013-11-04 at 16:36 +0200, Pantelis Antoniou wrote:
> > Enable the generation of symbol & fixup information for
> > usage with dynamic DT loading.
> >
> > Passing the -@ option generates a __symbols__ node at the
> > root node of the resulting blob for any node labels used.
> >
> > When using the /plugin/ tag all unresolved label references
> > be tracked in the __fixups__ node, while all local phandle
> > references will the tracked in the __local_fixups__ node.
> >
> > This is sufficient to implement a dynamic DT object loader.
> >
> > Changes since v1:
> >
> > * Forward ported to 1.4 version
> > * Added manual entries for the -@ option
> >
> > Signed-off-by: Pantelis Antoniou <panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org>
> > ---
> 
> Can I ask what repo/branch this is based on? It did not apply cleanly
> for me on 1.4.
> 
> Never mind...please ignore. It applies clean for me now. Giving it a go..
> Sorry for the noise.
> 
> Dinh 
> 
> I would like to test it along with the FPGA manager.
> 
> Thanks,
> Dinh
> 

Regards

-- Pantelis

> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] dtc: Dynamic symbols & fixup support (v2)
  2013-11-04 21:17   ` Dinh Nguyen
@ 2013-11-05  7:46     ` Sascha Hauer
       [not found]     ` <CADhT+wcgs2cQv23+K7EooZaA0KK7Lk8zY-KAPc50xUdm5BFRZw@mail.gmail.com>
  1 sibling, 0 replies; 13+ messages in thread
From: Sascha Hauer @ 2013-11-05  7:46 UTC (permalink / raw)
  To: Dinh Nguyen; +Cc: Pantelis Antoniou, devicetree-u79uwXL29TY76Z2rM5mHXA

On Mon, Nov 04, 2013 at 03:17:25PM -0600, Dinh Nguyen wrote:
> On Mon, 2013-11-04 at 16:36 +0200, Pantelis Antoniou wrote:
> > Enable the generation of symbol & fixup information for
> > usage with dynamic DT loading.
> > 
> > Passing the -@ option generates a __symbols__ node at the
> > root node of the resulting blob for any node labels used.
> > 
> > When using the /plugin/ tag all unresolved label references
> > be tracked in the __fixups__ node, while all local phandle
> > references will the tracked in the __local_fixups__ node.
> > 
> > This is sufficient to implement a dynamic DT object loader.
> > 
> > Changes since v1:
> > 
> > * Forward ported to 1.4 version
> > * Added manual entries for the -@ option
> > 
> > Signed-off-by: Pantelis Antoniou <panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org>
> > ---
> 
> Can I ask what repo/branch this is based on? It did not apply cleanly
> for me on 1.4.

Try harder ;)

The patch applies cleanly on v1.4.0 here.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] dtc: Dynamic symbols & fixup support (v2)
       [not found] ` <1383575773-7296-1-git-send-email-panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org>
  2013-11-04 14:53   ` Sascha Hauer
@ 2013-11-04 21:17   ` Dinh Nguyen
  2013-11-05  7:46     ` Sascha Hauer
       [not found]     ` <CADhT+wcgs2cQv23+K7EooZaA0KK7Lk8zY-KAPc50xUdm5BFRZw@mail.gmail.com>
  1 sibling, 2 replies; 13+ messages in thread
From: Dinh Nguyen @ 2013-11-04 21:17 UTC (permalink / raw)
  To: Pantelis Antoniou; +Cc: devicetree-u79uwXL29TY76Z2rM5mHXA

On Mon, 2013-11-04 at 16:36 +0200, Pantelis Antoniou wrote:
> Enable the generation of symbol & fixup information for
> usage with dynamic DT loading.
> 
> Passing the -@ option generates a __symbols__ node at the
> root node of the resulting blob for any node labels used.
> 
> When using the /plugin/ tag all unresolved label references
> be tracked in the __fixups__ node, while all local phandle
> references will the tracked in the __local_fixups__ node.
> 
> This is sufficient to implement a dynamic DT object loader.
> 
> Changes since v1:
> 
> * Forward ported to 1.4 version
> * Added manual entries for the -@ option
> 
> Signed-off-by: Pantelis Antoniou <panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org>
> ---

Can I ask what repo/branch this is based on? It did not apply cleanly
for me on 1.4.

I would like to test it along with the FPGA manager.

Thanks,
Dinh


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] dtc: Dynamic symbols & fixup support (v2)
       [not found]     ` <20131104145342.GW24559-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
@ 2013-11-04 14:58       ` Pantelis Antoniou
       [not found]         ` <53AA440E-7F33-4AC1-9323-7D109D4B5852-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Pantelis Antoniou @ 2013-11-04 14:58 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: devicetree-u79uwXL29TY76Z2rM5mHXA

Hi Sascha,

On Nov 4, 2013, at 4:53 PM, Sascha Hauer wrote:

> Hi Pantelis,
> 
> Thank you for working on this again. I would really appreciate seeing
> this upstream.
> 

Don't mention it. Let's try to make it happen then.

> On Mon, Nov 04, 2013 at 04:36:13PM +0200, Pantelis Antoniou wrote:
>> +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 labels
>> +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:
>> +
>> +/plugin/;	/* allow undefined label 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 */
>> +			}
>> +		};
>> +	};
>> +};
> 
> When last looking at this patch I created the attached follow up. What
> I didn't like about this patch is that we currently have to write dts
> files explicitely as overlays. With the attached patch the above could
> be written as:
> 
> &ocp: {
> 	compatible = "corp,bar";
> };
> 
> This is easier to write and more important a dts snippet could be either
> compiled into a dtb or used as an overlay.
> 

I see your point.

In my use case I always needed some out-of-band properties for the loader/manager 
to track. Let me test your and see if it doesn't break my use-cases.

If it's fine, then I have no objections in including it.


> Sascha
> 

Regards

-- Pantelis

> 8<-------------------------------------------------------------------
> 
> From bf78934e2bfe5c220d97d52e4effdd95164e071f Mon Sep 17 00:00:00 2001
> From: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Date: Thu, 8 Aug 2013 23:08:31 +0200
> Subject: [PATCH] dtc: convert unresolved labels into overlay nodes
> 
> Signed-off-by: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
> dtc-parser.y | 10 +++++++---
> dtc.h        |  3 ++-
> livetree.c   | 23 +++++++++++++++++++++++
> 3 files changed, 32 insertions(+), 4 deletions(-)
> 
> diff --git a/dtc-parser.y b/dtc-parser.y
> index e444acf..db1e9f8 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -166,10 +166,14 @@ devicetree:
> 		{
> 			struct node *target = get_node_by_ref($1, $2);
> 
> -			if (target)
> +			if (target) {
> 				merge_nodes(target, $3);
> -			else
> -				print_error("label or path, '%s', not found", $2);
> +			} else {
> +				if (symbol_fixup_support)
> +					add_orphan_node($1, $3, $2);
> +				else
> +					print_error("label or path, '%s', not found", $2);
> +			}
> 			$$ = $1;
> 		}
> 	| devicetree DT_DEL_NODE DT_REF ';'
> diff --git a/dtc.h b/dtc.h
> index 8c9059b..147ab35 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -20,7 +20,7 @@
>  *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307
>  *                                                                   USA
>  */
> -
> +#define _GNU_SOURCE
> #include <stdio.h>
> #include <string.h>
> #include <stdlib.h>
> @@ -232,6 +232,7 @@ struct node *build_node_delete(void);
> struct node *name_node(struct node *node, char *name);
> struct node *chain_node(struct node *first, struct node *list);
> struct node *merge_nodes(struct node *old_node, struct node *new_node);
> +void add_orphan_node(struct node *old_node, struct node *new_node, char *ref);
> 
> void add_property(struct node *node, struct property *prop);
> void delete_property_by_name(struct node *node, char *name);
> diff --git a/livetree.c b/livetree.c
> index b61465f..cc26fe1 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -216,6 +216,29 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
> 	return old_node;
> }
> 
> +void add_orphan_node(struct node *dt, struct node *new_node, char *ref)
> +{
> +	struct node *ovl = xmalloc(sizeof(*ovl));
> +	struct property *p;
> +	struct data d = empty_data;
> +	char *name;
> +
> +	memset(ovl, 0, sizeof(*ovl));
> +
> +	d = data_add_marker(d, REF_PHANDLE, ref);
> +	d = data_append_integer(d, 0xdeadbeef, 32);
> +
> +	p = build_property("target", d);
> +	add_property(ovl, p);
> +
> +	asprintf(&name, "fragment@%s", ref);
> +	name_node(ovl, name);
> +	name_node(new_node, "__overlay__");
> +
> +	add_child(dt, ovl);
> +	add_child(ovl, new_node);
> +}
> +
> struct node *chain_node(struct node *first, struct node *list)
> {
> 	assert(first->next_sibling == NULL);
> -- 
> 1.8.4.rc3
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] dtc: Dynamic symbols & fixup support (v2)
       [not found] ` <1383575773-7296-1-git-send-email-panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org>
@ 2013-11-04 14:53   ` Sascha Hauer
       [not found]     ` <20131104145342.GW24559-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
  2013-11-04 21:17   ` Dinh Nguyen
  1 sibling, 1 reply; 13+ messages in thread
From: Sascha Hauer @ 2013-11-04 14:53 UTC (permalink / raw)
  To: Pantelis Antoniou; +Cc: devicetree-u79uwXL29TY76Z2rM5mHXA

Hi Pantelis,

Thank you for working on this again. I would really appreciate seeing
this upstream.

On Mon, Nov 04, 2013 at 04:36:13PM +0200, Pantelis Antoniou wrote:
> +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 labels
> +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:
> +
> +/plugin/;	/* allow undefined label 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 */
> +			}
> +		};
> +	};
> +};

When last looking at this patch I created the attached follow up. What
I didn't like about this patch is that we currently have to write dts
files explicitely as overlays. With the attached patch the above could
be written as:

&ocp: {
	compatible = "corp,bar";
};

This is easier to write and more important a dts snippet could be either
compiled into a dtb or used as an overlay.

Sascha

8<-------------------------------------------------------------------

>From bf78934e2bfe5c220d97d52e4effdd95164e071f Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Date: Thu, 8 Aug 2013 23:08:31 +0200
Subject: [PATCH] dtc: convert unresolved labels into overlay nodes

Signed-off-by: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 dtc-parser.y | 10 +++++++---
 dtc.h        |  3 ++-
 livetree.c   | 23 +++++++++++++++++++++++
 3 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/dtc-parser.y b/dtc-parser.y
index e444acf..db1e9f8 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -166,10 +166,14 @@ devicetree:
 		{
 			struct node *target = get_node_by_ref($1, $2);
 
-			if (target)
+			if (target) {
 				merge_nodes(target, $3);
-			else
-				print_error("label or path, '%s', not found", $2);
+			} else {
+				if (symbol_fixup_support)
+					add_orphan_node($1, $3, $2);
+				else
+					print_error("label or path, '%s', not found", $2);
+			}
 			$$ = $1;
 		}
 	| devicetree DT_DEL_NODE DT_REF ';'
diff --git a/dtc.h b/dtc.h
index 8c9059b..147ab35 100644
--- a/dtc.h
+++ b/dtc.h
@@ -20,7 +20,7 @@
  *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307
  *                                                                   USA
  */
-
+#define _GNU_SOURCE
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
@@ -232,6 +232,7 @@ struct node *build_node_delete(void);
 struct node *name_node(struct node *node, char *name);
 struct node *chain_node(struct node *first, struct node *list);
 struct node *merge_nodes(struct node *old_node, struct node *new_node);
+void add_orphan_node(struct node *old_node, struct node *new_node, char *ref);
 
 void add_property(struct node *node, struct property *prop);
 void delete_property_by_name(struct node *node, char *name);
diff --git a/livetree.c b/livetree.c
index b61465f..cc26fe1 100644
--- a/livetree.c
+++ b/livetree.c
@@ -216,6 +216,29 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
 	return old_node;
 }
 
+void add_orphan_node(struct node *dt, struct node *new_node, char *ref)
+{
+	struct node *ovl = xmalloc(sizeof(*ovl));
+	struct property *p;
+	struct data d = empty_data;
+	char *name;
+
+	memset(ovl, 0, sizeof(*ovl));
+
+	d = data_add_marker(d, REF_PHANDLE, ref);
+	d = data_append_integer(d, 0xdeadbeef, 32);
+
+	p = build_property("target", d);
+	add_property(ovl, p);
+
+	asprintf(&name, "fragment@%s", ref);
+	name_node(ovl, name);
+	name_node(new_node, "__overlay__");
+
+	add_child(dt, ovl);
+	add_child(ovl, new_node);
+}
+
 struct node *chain_node(struct node *first, struct node *list)
 {
 	assert(first->next_sibling == NULL);
-- 
1.8.4.rc3

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH] dtc: Dynamic symbols & fixup support (v2)
@ 2013-11-04 14:36 Pantelis Antoniou
       [not found] ` <1383575773-7296-1-git-send-email-panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org>
       [not found] ` <CADhT+wdJfrRMDjm+jb_+_uuy8sPRd1B0F5mNVgOL2DmyHTMrYw@mail.gmail.com>
  0 siblings, 2 replies; 13+ messages in thread
From: Pantelis Antoniou @ 2013-11-04 14:36 UTC (permalink / raw)
  To: devicetree-u79uwXL29TY76Z2rM5mHXA; +Cc: Pantelis Antoniou

Enable the generation of symbol & fixup information for
usage with dynamic DT loading.

Passing the -@ option generates a __symbols__ node at the
root node of the resulting blob for any node labels used.

When using the /plugin/ tag all unresolved label references
be tracked in the __fixups__ node, while all local phandle
references will the tracked in the __local_fixups__ node.

This is sufficient to implement a dynamic DT object loader.

Changes since v1:

* Forward ported to 1.4 version
* Added manual entries for the -@ option

Signed-off-by: Pantelis Antoniou <panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org>
---
 Documentation/dt-object-internal.txt | 295 +++++++++++++++++++++++++++++++++++
 Documentation/manual.txt             |  10 ++
 checks.c                             | 120 +++++++++++++-
 dtc-lexer.l                          |   5 +
 dtc-parser.y                         |  23 ++-
 dtc.c                                |   9 +-
 dtc.h                                |  38 +++++
 flattree.c                           | 139 +++++++++++++++++
 8 files changed, 630 insertions(+), 9 deletions(-)
 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..eb5d4c0
--- /dev/null
+++ b/Documentation/dt-object-internal.txt
@@ -0,0 +1,295 @@
+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. 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.
+
+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.
+
+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 {
+		...
+		linux,phandle = <0x00000001>;
+		phandle = <0x00000001>;
+		...
+	};
+	ocp {
+		...
+		linux,phandle = <0x00000002>;
+		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 labels
+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:
+
+/plugin/;	/* allow undefined label 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 */
+			}
+		};
+	};
+};
+
+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 label
+in the foo.dts file.
+
+$ dtc -@ -O dtb -o bar.dtbo -b 0 bar.dts
+$ fdtdump bar.dtbo
+...
+/ {
+	... /* properties */
+	fragment@0 {
+		target = <0xdeadbeef>;
+		__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 '0xdeadbeef', 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.
+
+/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 = <0xdeadbeef>;
+		__overlay__ {
+			res_baz {
+				....
+				linux,phandle = <0x00000001>;
+				phandle = <0x00000001>;
+			};
+		};
+	};
+	fragment@1 {
+		target = <0xdeadbeef>;
+		__overlay__ {
+			baz {
+				compatible = "corp,baz";
+				... /* various properties and child nodes */
+				res=<0x00000001>;
+			}
+		};
+	};
+	__fixups__ {
+		res = "/fragment@0:target:0";
+		ocp = "/fragment@1:target:0";
+	};
+	__local_fixups__ {
+		fixup = </fragment@1/__overlay__/baz: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. Since phandles are allocated starting at 1
+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.
diff --git a/Documentation/manual.txt b/Documentation/manual.txt
index 65c8540..9b4d329 100644
--- a/Documentation/manual.txt
+++ b/Documentation/manual.txt
@@ -121,6 +121,14 @@ Options:
 	Make space for <number> reserve map entries
 	Relevant for dtb and asm output only.
 
+    -@
+        Generates a __symbols__ node at the root node of the resulting blob
+	for any node labels used.
+
+	When using the /plugin/ tag all unresolved label references
+	be tracked in the __fixups__ node, while all local phandle
+	references will the tracked in the __local_fixups__ node.
+
     -S <bytes>
 	Ensure the blob at least <bytes> long, adding additional
 	space if needed.
@@ -155,6 +163,8 @@ Here is a very rough overview of the layout of a DTS source file:
 
     devicetree:   '/' nodedef
 
+    plugindecl:   '/' 'plugin' '/' ';'
+
     nodedef:      '{' list_of_property list_of_subnode '}' ';'
 
     property:     label PROPNAME '=' propdata ';'
diff --git a/checks.c b/checks.c
index ee96a25..970c0a3 100644
--- a/checks.c
+++ b/checks.c
@@ -457,22 +457,93 @@ static void fixup_phandle_references(struct check *c, struct node *dt,
 				     struct node *node, struct property *prop)
 {
 	struct marker *m = prop->val.markers;
+	struct fixup *f, **fp;
+	struct fixup_entry *fe, **fep;
 	struct node *refnode;
 	cell_t phandle;
+	int has_phandle_refs;
+
+	has_phandle_refs = 0;
+	for_each_marker_of_type(m, REF_PHANDLE) {
+		has_phandle_refs = 1;
+		break;
+	}
+
+	if (!has_phandle_refs)
+		return;
 
 	for_each_marker_of_type(m, REF_PHANDLE) {
 		assert(m->offset + sizeof(cell_t) <= prop->val.len);
 
 		refnode = get_node_by_ref(dt, m->ref);
-		if (! refnode) {
+		if (!refnode && !symbol_fixup_support) {
 			FAIL(c, "Reference to non-existent node or label \"%s\"\n",
-			     m->ref);
+				m->ref);
 			continue;
 		}
 
-		phandle = get_node_phandle(dt, refnode);
-		*((cell_t *)(prop->val.val + m->offset)) = cpu_to_fdt32(phandle);
+		if (!refnode) {
+			/* allocate fixup entry */
+			fe = xmalloc(sizeof(*fe));
+
+			fe->node = node;
+			fe->prop = prop;
+			fe->offset = m->offset;
+			fe->next = NULL;
+
+			/* search for an already existing fixup */
+			for_each_fixup(dt, f)
+				if (strcmp(f->ref, m->ref) == 0)
+					break;
+
+			/* no fixup found, add new */
+			if (f == NULL) {
+				f = xmalloc(sizeof(*f));
+				f->ref = m->ref;
+				f->entries = NULL;
+				f->next = NULL;
+
+				/* add it to the tree */
+				fp = &dt->fixups;
+				while (*fp)
+					fp = &(*fp)->next;
+				*fp = f;
+			}
+
+			/* and now append fixup entry */
+			fep = &f->entries;
+			while (*fep)
+				fep = &(*fep)->next;
+			*fep = fe;
+
+			/* mark the entry as unresolved */
+			phandle = 0xdeadbeef;
+		} else {
+			phandle = get_node_phandle(dt, refnode);
+
+			/* if it's a plugin, we need to record it */
+			if (symbol_fixup_support && dt->is_plugin) {
+
+				/* allocate a new local fixup entry */
+				fe = xmalloc(sizeof(*fe));
+
+				fe->node = node;
+				fe->prop = prop;
+				fe->offset = m->offset;
+				fe->next = NULL;
+
+				/* append it to the local fixups */
+				fep = &dt->local_fixups;
+				while (*fep)
+					fep = &(*fep)->next;
+				*fep = fe;
+			}
+		}
+
+		*((cell_t *)(prop->val.val + m->offset)) =
+			cpu_to_fdt32(phandle);
 	}
+
 }
 ERROR(phandle_references, NULL, NULL, fixup_phandle_references, NULL,
       &duplicate_node_names, &explicit_phandles);
@@ -651,6 +722,45 @@ static void check_obsolete_chosen_interrupt_controller(struct check *c,
 }
 TREE_WARNING(obsolete_chosen_interrupt_controller, NULL);
 
+static void check_auto_label_phandles(struct check *c, struct node *dt,
+				       struct node *node)
+{
+	struct label *l;
+	struct symbol *s, **sp;
+	int has_label;
+
+	if (!symbol_fixup_support)
+		return;
+
+	has_label = 0;
+	for_each_label(node->labels, l) {
+		has_label = 1;
+		break;
+	}
+
+	if (!has_label)
+		return;
+
+	/* force allocation of a phandle for this node */
+	(void)get_node_phandle(dt, node);
+
+	/* add the symbol */
+	for_each_label(node->labels, l) {
+
+		s = xmalloc(sizeof(*s));
+		s->label = l;
+		s->node = node;
+		s->next = NULL;
+
+		/* add it to the symbols list */
+		sp = &dt->symbols;
+		while (*sp)
+			sp = &((*sp)->next);
+		*sp = s;
+	}
+}
+NODE_WARNING(auto_label_phandles, NULL);
+
 static struct check *check_table[] = {
 	&duplicate_node_names, &duplicate_property_names,
 	&node_name_chars, &node_name_format, &property_name_chars,
@@ -669,6 +779,8 @@ static struct check *check_table[] = {
 	&avoid_default_addr_size,
 	&obsolete_chosen_interrupt_controller,
 
+	&auto_label_phandles,
+
 	&always_fail,
 };
 
diff --git a/dtc-lexer.l b/dtc-lexer.l
index 3b41bfc..78d5132 100644
--- a/dtc-lexer.l
+++ b/dtc-lexer.l
@@ -112,6 +112,11 @@ static int pop_input_file(void);
 			return DT_V1;
 		}
 
+<*>"/plugin/"	{
+			DPRINT("Keyword: /plugin/\n");
+			return DT_PLUGIN;
+		}
+
 <*>"/memreserve/"	{
 			DPRINT("Keyword: /memreserve/\n");
 			BEGIN_DEFAULT();
diff --git a/dtc-parser.y b/dtc-parser.y
index f412460..e444acf 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -20,6 +20,7 @@
 
 %{
 #include <stdio.h>
+#include <inttypes.h>
 
 #include "dtc.h"
 #include "srcpos.h"
@@ -56,9 +57,11 @@ static unsigned char eval_char_literal(const char *s);
 	struct node *nodelist;
 	struct reserve_info *re;
 	uint64_t integer;
+	int is_plugin;
 }
 
 %token DT_V1
+%token DT_PLUGIN
 %token DT_MEMRESERVE
 %token DT_LSHIFT DT_RSHIFT DT_LE DT_GE DT_EQ DT_NE DT_AND DT_OR
 %token DT_BITS
@@ -76,6 +79,7 @@ static unsigned char eval_char_literal(const char *s);
 
 %type <data> propdata
 %type <data> propdataprefix
+%type <is_plugin> plugindecl
 %type <re> memreserve
 %type <re> memreserves
 %type <array> arrayprefix
@@ -106,10 +110,23 @@ static unsigned char eval_char_literal(const char *s);
 %%
 
 sourcefile:
-	  DT_V1 ';' memreserves devicetree
+	  DT_V1 ';' plugindecl memreserves devicetree
 		{
-			the_boot_info = build_boot_info($3, $4,
-							guess_boot_cpuid($4));
+			$5->is_plugin = $3;
+			$5->is_root = 1;
+			the_boot_info = build_boot_info($4, $5,
+							guess_boot_cpuid($5));
+		}
+	;
+
+plugindecl:
+	/* empty */
+		{
+			$$ = 0;
+		}
+	| DT_PLUGIN ';'
+		{
+			$$ = 1;
 		}
 	;
 
diff --git a/dtc.c b/dtc.c
index e3c9653..31d29e2 100644
--- a/dtc.c
+++ b/dtc.c
@@ -29,6 +29,7 @@ int reservenum;		/* Number of memory reservation slots */
 int minsize;		/* Minimum blob size */
 int padsize;		/* Additional padding to blob */
 int phandle_format = PHANDLE_BOTH;	/* Use linux,phandle or phandle properties */
+int symbol_fixup_support = 0;
 
 static void fill_fullpaths(struct node *tree, const char *prefix)
 {
@@ -49,7 +50,7 @@ static void fill_fullpaths(struct node *tree, const char *prefix)
 
 /* Usage related data. */
 static const char usage_synopsis[] = "dtc [options] <input file>";
-static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:fb:i:H:sW:E:hv";
+static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:fb:i:H:sW:E:@hv";
 static struct option const usage_long_opts[] = {
 	{"quiet",            no_argument, NULL, 'q'},
 	{"in-format",         a_argument, NULL, 'I'},
@@ -67,6 +68,7 @@ static struct option const usage_long_opts[] = {
 	{"phandle",           a_argument, NULL, 'H'},
 	{"warning",           a_argument, NULL, 'W'},
 	{"error",             a_argument, NULL, 'E'},
+	{"symbols",	     no_argument, NULL, '@'},
 	{"help",             no_argument, NULL, 'h'},
 	{"version",          no_argument, NULL, 'v'},
 	{NULL,               no_argument, NULL, 0x0},
@@ -97,6 +99,7 @@ static const char * const usage_opts_help[] = {
 	 "\t\tboth   - Both \"linux,phandle\" and \"phandle\" properties",
 	"\n\tEnable/disable warnings (prefix with \"no-\")",
 	"\n\tEnable/disable errors (prefix with \"no-\")",
+	"\n\tEnable symbols/fixup support",
 	"\n\tPrint this help and exit",
 	"\n\tPrint version and exit",
 	NULL,
@@ -184,7 +187,9 @@ int main(int argc, char *argv[])
 		case 'E':
 			parse_checks_option(false, true, optarg);
 			break;
-
+		case '@':
+			symbol_fixup_support = 1;
+			break;
 		case 'h':
 			usage(NULL);
 		default:
diff --git a/dtc.h b/dtc.h
index 264a20c..8c9059b 100644
--- a/dtc.h
+++ b/dtc.h
@@ -54,6 +54,7 @@ extern int reservenum;		/* Number of memory reservation slots */
 extern int minsize;		/* Minimum blob size */
 extern int padsize;		/* Additional padding to blob */
 extern int phandle_format;	/* Use linux,phandle or phandle properties */
+extern int symbol_fixup_support;/* enable symbols & fixup support */
 
 #define PHANDLE_LEGACY	0x1
 #define PHANDLE_EPAPR	0x2
@@ -132,6 +133,25 @@ struct label {
 	struct label *next;
 };
 
+struct fixup_entry {
+	int offset;
+	struct node *node;
+	struct property *prop;
+	struct fixup_entry *next;
+};
+
+struct fixup {
+	char *ref;
+	struct fixup_entry *entries;
+	struct fixup *next;
+};
+
+struct symbol {
+	struct label *label;
+	struct node *node;
+	struct symbol *next;
+};
+
 struct property {
 	int deleted;
 	char *name;
@@ -158,6 +178,12 @@ struct node {
 	int addr_cells, size_cells;
 
 	struct label *labels;
+
+	int is_root;
+	int is_plugin;
+	struct fixup *fixups;
+	struct symbol *symbols;
+	struct fixup_entry *local_fixups;
 };
 
 #define for_each_label_withdel(l0, l) \
@@ -181,6 +207,18 @@ struct node {
 	for_each_child_withdel(n, c) \
 		if (!(c)->deleted)
 
+#define for_each_fixup(n, f) \
+	for ((f) = (n)->fixups; (f); (f) = (f)->next)
+
+#define for_each_fixup_entry(f, fe) \
+	for ((fe) = (f)->entries; (fe); (fe) = (fe)->next)
+
+#define for_each_symbol(n, s) \
+	for ((s) = (n)->symbols; (s); (s) = (s)->next)
+
+#define for_each_local_fixup_entry(n, fe) \
+	for ((fe) = (n)->local_fixups; (fe); (fe) = (fe)->next)
+
 void add_label(struct label **labels, char *label);
 void delete_labels(struct label **labels);
 
diff --git a/flattree.c b/flattree.c
index 665dad7..5b2e75e 100644
--- a/flattree.c
+++ b/flattree.c
@@ -262,6 +262,12 @@ static void flatten_tree(struct node *tree, struct emitter *emit,
 	struct property *prop;
 	struct node *child;
 	int seen_name_prop = 0;
+	struct symbol *sym;
+	struct fixup *f;
+	struct fixup_entry *fe;
+	char *name, *s;
+	const char *fullpath;
+	int namesz, nameoff, vallen;
 
 	if (tree->deleted)
 		return;
@@ -310,6 +316,139 @@ static void flatten_tree(struct node *tree, struct emitter *emit,
 		flatten_tree(child, emit, etarget, strbuf, vi);
 	}
 
+	if (!symbol_fixup_support)
+		goto no_symbols;
+
+	/* add the symbol nodes (if any) */
+	if (tree->symbols) {
+
+		emit->beginnode(etarget, NULL);
+		emit->string(etarget, "__symbols__", 0);
+		emit->align(etarget, sizeof(cell_t));
+
+		for_each_symbol(tree, sym) {
+
+			vallen = strlen(sym->node->fullpath);
+
+			nameoff = stringtable_insert(strbuf, sym->label->label);
+
+			emit->property(etarget, NULL);
+			emit->cell(etarget, vallen + 1);
+			emit->cell(etarget, nameoff);
+
+			if ((vi->flags & FTF_VARALIGN) && vallen >= 8)
+				emit->align(etarget, 8);
+
+			emit->string(etarget, sym->node->fullpath,
+					strlen(sym->node->fullpath));
+			emit->align(etarget, sizeof(cell_t));
+		}
+
+		emit->endnode(etarget, NULL);
+	}
+
+	/* add the fixup nodes */
+	if (tree->fixups) {
+
+		/* emit the external fixups */
+		emit->beginnode(etarget, NULL);
+		emit->string(etarget, "__fixups__", 0);
+		emit->align(etarget, sizeof(cell_t));
+
+		for_each_fixup(tree, f) {
+
+			namesz = 0;
+			for_each_fixup_entry(f, fe) {
+				fullpath = fe->node->fullpath;
+				if (fullpath[0] == '\0')
+					fullpath = "/";
+				namesz += strlen(fullpath) + 1;
+				namesz += strlen(fe->prop->name) + 1;
+				namesz += 32;	/* space for :<number> + '\0' */
+			}
+
+			name = xmalloc(namesz);
+
+			s = name;
+			for_each_fixup_entry(f, fe) {
+				fullpath = fe->node->fullpath;
+				if (fullpath[0] == '\0')
+					fullpath = "/";
+				snprintf(s, name + namesz - s, "%s:%s:%d",
+						fullpath,
+						fe->prop->name, fe->offset);
+				s += strlen(s) + 1;
+			}
+
+			nameoff = stringtable_insert(strbuf, f->ref);
+			vallen = s - name - 1;
+
+			emit->property(etarget, NULL);
+			emit->cell(etarget, vallen + 1);
+			emit->cell(etarget, nameoff);
+
+			if ((vi->flags & FTF_VARALIGN) && vallen >= 8)
+				emit->align(etarget, 8);
+
+			emit->string(etarget, name, vallen);
+			emit->align(etarget, sizeof(cell_t));
+
+			free(name);
+		}
+
+		emit->endnode(etarget, tree->labels);
+	}
+
+	/* add the local fixup property */
+	if (tree->local_fixups) {
+
+		/* emit the external fixups */
+		emit->beginnode(etarget, NULL);
+		emit->string(etarget, "__local_fixups__", 0);
+		emit->align(etarget, sizeof(cell_t));
+
+		namesz = 0;
+		for_each_local_fixup_entry(tree, fe) {
+			fullpath = fe->node->fullpath;
+			if (fullpath[0] == '\0')
+				fullpath = "/";
+			namesz += strlen(fullpath) + 1;
+			namesz += strlen(fe->prop->name) + 1;
+			namesz += 32;	/* space for :<number> + '\0' */
+		}
+
+		name = xmalloc(namesz);
+
+		s = name;
+		for_each_local_fixup_entry(tree, fe) {
+			fullpath = fe->node->fullpath;
+			if (fullpath[0] == '\0')
+				fullpath = "/";
+			snprintf(s, name + namesz - s, "%s:%s:%d",
+					fullpath, fe->prop->name,
+					fe->offset);
+			s += strlen(s) + 1;
+		}
+
+		nameoff = stringtable_insert(strbuf, "fixup");
+		vallen = s - name - 1;
+
+		emit->property(etarget, NULL);
+		emit->cell(etarget, vallen + 1);
+		emit->cell(etarget, nameoff);
+
+		if ((vi->flags & FTF_VARALIGN) && vallen >= 8)
+			emit->align(etarget, 8);
+
+		emit->string(etarget, name, vallen);
+		emit->align(etarget, sizeof(cell_t));
+
+		free(name);
+
+		emit->endnode(etarget, tree->labels);
+	}
+
+no_symbols:
 	emit->endnode(etarget, tree->labels);
 }
 
-- 
1.7.12

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2013-11-17 22:16 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1383073266-12017-1-git-send-email-panto@antoniou-consulting.com>
     [not found] ` <1383073266-12017-1-git-send-email-panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org>
2013-11-11 17:56   ` [PATCH] dtc: Dynamic symbols & fixup support (v2) Grant Likely
     [not found]     ` <20131111175604.ECF33C4233A-WNowdnHR2B42iJbIjFUEsiwD8/FfD2ys@public.gmane.org>
2013-11-12 10:20       ` Pantelis Antoniou
     [not found]     ` < DABC2A65-BC07-4481-B80C-A62777BD8732@antoniou-consulting.com>
     [not found]       ` <DABC2A65-BC07-4481-B80C-A62777BD8732-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org>
2013-11-15  7:01         ` Grant Likely
     [not found]           ` <20131115070131.78139C40785-WNowdnHR2B42iJbIjFUEsiwD8/FfD2ys@public.gmane.org>
2013-11-16 17:47             ` Pantelis Antoniou
     [not found]       ` < 20131115070131.78139C40785@trevor.secretlab.ca>
     [not found]         ` < 972758FF-44EC-4C32-BE13-4E3E8361D063@antoniou-consulting.com>
     [not found]           ` <972758FF-44EC-4C32-BE13-4E3E8361D063-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org>
2013-11-17 22:16             ` Grant Likely
2013-11-04 14:36 Pantelis Antoniou
     [not found] ` <1383575773-7296-1-git-send-email-panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org>
2013-11-04 14:53   ` Sascha Hauer
     [not found]     ` <20131104145342.GW24559-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2013-11-04 14:58       ` Pantelis Antoniou
     [not found]         ` <53AA440E-7F33-4AC1-9323-7D109D4B5852-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org>
2013-11-05  8:02           ` Sascha Hauer
2013-11-04 21:17   ` Dinh Nguyen
2013-11-05  7:46     ` Sascha Hauer
     [not found]     ` <CADhT+wcgs2cQv23+K7EooZaA0KK7Lk8zY-KAPc50xUdm5BFRZw@mail.gmail.com>
     [not found]       ` <CADhT+wcgs2cQv23+K7EooZaA0KK7Lk8zY-KAPc50xUdm5BFRZw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-11-05  7:49         ` Pantelis Antoniou
     [not found] ` <CADhT+wdJfrRMDjm+jb_+_uuy8sPRd1B0F5mNVgOL2DmyHTMrYw@mail.gmail.com>
     [not found]   ` <CADhT+wdJfrRMDjm+jb_+_uuy8sPRd1B0F5mNVgOL2DmyHTMrYw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-11-08  7:03     ` Pantelis Antoniou

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.