Re: Proposal: new device-tree syntax and semantics for extending information from included dts files

Re: Proposal: new device-tree syntax and semantics for extending information from included dts files

[Grant Likely]

On Tue, Oct 12, 2010 at 02:56:01PM -0700, John Bonesio wrote:
> Objective:
> The primary problem is that the same information gets repeated in
> various dts file that describe variations on the same base SOC. The
> information becomes redundant and when changes to the device tree
> syntax is made, extra work is necessary to update the same
> information in multiple files.

Thanks John.  Some comments below, but a reasonable proposal.  Next
you need to post the proposal to the devicetree-discuss list and cc:
David Gibson, Jon Loeliger and me.

... actually, this discussion is important enough that I don't want to
send this reply without David and Jon seeing it; otherwise we just end
up repeating the same rationals over again.  I'm going to cc the
list right now.

> 
> A solution needs to allow:
> a) nodes to be updated
> b) nodes to be overridden
> c) nodes to be removed
> d) properties to be overridden
> e) properties to be removed
> 
> Overview:
> The main thrust of this proposal is to suggest an overlay system
> where existing device tree nodes can be modified by dts statements
> later on in the "file". The "file" is defined as a file in memory
> after all /include/ statements have been processed.
> 
> Much of the functionality described in this proposal is already
> present in a test version of the device tree compiler (dtc). This
> proposal describes

s/test/unreleased/

> the functionality in it's entirety so that the changed parts can be
> viewed in light of the whole. This proposal suggests extensions, and
> might inadvertently suggest small modifications to this existing
> functionality.
> 
> The main idea of this proposal suggests that when duplicate nodes are
> encountered, later nodes seen by the compiler will update or in some way
> modify the node already encountered.
> 
> This proposal suggest that there be 3 ways in which an existing node
> can be modified. The node can be merged with the new information, it
> can be superseded by the new information, or it can be deleted. The
> method used would be defined by a new directive.
> 
> This proposal recommends that properties are not updated directly but
> that this overlay functionality only apply to nodes.

I'm not sure what you mean by this.  There will be situations when a
single property needs to be removed without affecting the rest of the
node contents (as you state in the requirements).

> 
> Proposal:
> 
> 1) Add a new keyword constant for property values, <Undefined>.

Personally, I'd prefer all lowercase: <undefined>.  There may be some
dickering over the exact keyword name, but I think the concept is
sound.

> 
> 	The constant means 'not defined'. If a property is assigned
> 	the value of <Undefined>, then it's as if the property was
> 	never defined. [Solves requirement (e)]

Ack.

> 
> 	Rationale:
> 		When we want to use /include/ and then modify or
> 		extend an existing base dts (.dtsi) file, using
> 		<Undefined> provides a clear indication to anyone
> 		reading the dts file that this property is being
> 		removed.  By using <Undefined> as a value, this syntax
> 		become a part of the device tree language rather than
> 		a command telling the compiler to do something.
> 
> 	Example:
> 		#size-cells = <Undefined>;
> 
> 2) Allow 3 directives for modifying existing nodes in the
> system: /extend/,
>    /replace/, /remove/
> 
> 	The test version of the dtc already provides the /extend/
> 	method of merging

s/test/unreleased/

> 	existing nodes. The version of the dtc may provide a way to
> 	extend properties as well, but this proposal suggest that
> 	individual properties should not be allowed to be updated.

Again, I'm not sure what you mean by not allowing properties to be
updated.  The current behaviour, which I think is sane, is that a
redefinition of a property will replace the value.

> 
> 	[Solves requirements (a-d)]
> 
> 	A) /extend/

"/extend-node/" is possibly a clearer naming to avoid confusion with
working with properties.

> 
> 	This is the default directive. When no directive is provided,
> 	it is assumed that /extend/ is to be used. If a node is
> 	described twice in the system, the second instance of the node
> 	merges with the first. If the second node has duplicate
> 	properties, the property in the last instance of the node is
> 	the value used.
> 
> 	Example:
> 		/ {
> 			...
> 			memory {
> 				device_type = "memory";
> 				reg = <0x00000000 0x04000000>;	// 64MB
> 			};
> 			...
> 
> 			memory /extend/ {
> 				reg = <0x00000000 0x08000000>;	// 128MB RAM
> 			}
> 
> 			memory {
> 				reg = <0x00000000 0x10000000>;	// 256MB RAM
> 			}
> 
> 		};

This example isn't correct.  In the current design, nodes will only
get merged at the top-level and the same node cannot be redefined
within the same top-level tree structure.  In essence, device tree
nodes are unordered sets of properties and child nodes.  Overriding
a node within the same tree has been explicitly avoided.  This example
should be:

	/ {
		...
		memory {
			device_type = "memory";
			reg = <0x00000000 0x04000000>;	// 64MB
		};
		...
	};

	/ {
		memory /extend/ {
			reg = <0x00000000 0x08000000>;	// 128MB RAM
		};
	};

	/ {
		memory {
			reg = <0x00000000 0x10000000>;	// 256MB RAM
		};
	};

Each subsequent tree is merged into the main tree before going on to
process the next one.

Or alternately:

	/ {
		...
		memorynode: memory {
			device_type = "memory";
			reg = <0x00000000 0x04000000>;	// 64MB
		};
		...
	};

	&memorynode {
		reg = <0x00000000 0x08000000>;	// 128MB RAM
	};

Which is a shortcut to a particular place in the tree

> 
> 		In this example, first the 2nd instance of the node,
> 		memory, is merged with the first with the /extend/
> 		directive. The reg property is overridden to 128MB.
> 		Then the 3rd instance of the node, memory, is also
> 		merged. The reg property is overridden to 256MB. In
> 		this second case, /extend/ is assumed since it is the
> 		default method of modifying an existing node.
> 
> 	B) /replace/

/replace-node/ perhaps?

> 
> 	This directive tells the device tree compiler to drop all
> 	existing properties from the node and replace them with the
> 	new node information.
> 
> 	Example:
> 		/ {
> 			soc: soc5200@f0000000 {
> 				#address-cells = <1>;
> 				#size-cells = <1>;
> 				compatible = "fsl,mpc5200b-immr";
> 				...
> 
> 				timer@600 {	// General Purpose Timer
> 					compatible = "fsl,mpc5200b-gpt","fsl,mpc5200-gpt";
> 					reg = <0x600 0x10>;
> 					interrupts = <1 9 0>;
> 					fsl,has-wdt;
> 				};
> 
> 				timer@600 /replace/ {	// General Purpose Timer
> 					compatible = "fsl,mpc5200-gpt";
> 					reg = <0x600 0x10>;
> 					interrupts = <1 15 0>;
> 					fsl,has-wdt;
> 				};
> 		};

Ditto on example needs to be fixed.

> 
> 		In this simple example, all of the properties are
> 		dropped from the first definition of 'timer' and are
> 		replaced by the second instance.  The second instance
> 		in this example, has different compatible and
> 		interrupts properties.

What about the child nodes?  Do they get dropped too (I think they
should)?

> 
> 	C) /remove/

/remove-node/?

> 
> 	This directive provides a simple clear method for removing nodes that
> 	were previously defined.
> 
> 	Exmaple:
> 		/ {
> 			soc: soc5200@f0000000 {
> 				#address-cells = <1>;
> 				#size-cells = <1>;
> 				compatible = "fsl,mpc5200b-immr";
> 				...
> 
> 				serial@2600 {		// PSC4
> 					compatible = "fsl,mpc5200b-psc-uart","fsl,mpc5200-psc-uart";
> 					reg = <0x2600 0x100>;
> 					interrupts = <2 11 0>;
> 				};
> 
> 				serial@2800 {		// PSC5
> 					compatible = "fsl,mpc5200b-psc-uart","fsl,mpc5200-psc-uart";
> 					reg = <0x2800 0x100>;
> 					interrupts = <2 12 0>;
> 				};
> 
> 
> 				serial@2600 /remove/ { };		// PSC4
> 
> 				serial@2800 /remove/ { };		// PSC5
> 		};

Ditto on fixing example.

I'm not thrilled with the example because it still shows the empty
braces which could be construed as the node still being present.  How
about this syntax instead:
	serial@2800 /remove-node/;

I also wonder if it would be better to have the directives before the
node name.  ie:
	/extend/ node1 { ... };
	/replace/ node1 { ... };
	/remove/ node1;

> 
> 		In this example, two serial devices are removed.
> 
> 	Rationale:
> 
> 		By specifying the method for combining, overriding, or
> 		removing nodes, it should be easy to understand what
> 		is taking place.  /extend/ allows new properties to be
> 		added, existing properties to be replaced, and
> 		existing properties to be removed if the <Undefined>
> 		value is used.
> 
> 		/replace/ provides a convenient way of substituting
> 		node properties without having to remove each property
> 		by name with the /extend/ directive.
>
> 		/remove/ allows a method a node to be removed
> 		completely. /replace/ with an empty property list
> 		could be used, but this leaves an empty node still
> 		being defined.
> 
> 		The reason that only nodes are to be modified by the
> 		overlay system is that updates to specific properties
> 		will likely be confusing to a reader, especially when
> 		labels are used (which will be the common case). By
> 		updating whole nodes, a reader can more easily figure
> 		out what information is being updated.
> 
> 		For example, one might have:
> 			&reg5 = <3 0 0x10000>;
> 		A reader might be wonder which reg is this, it might be updated to:
> 			&uart3_reg = <3 0 0x10000>;
> 
> 		At this point we might as well just specify the node:
> 			&uart3 /extends/ {
> 				reg = <3 0 0x10000>;
> 			};

Ah, yes. I see.  Yes, I don't at all want to reference properties by
label for purpose of overlays.  It must always be done from the node
context.

> 
> 3) Use labels to specify node updates.
> 	This functionality is already present in the test dtc. The
> 	idea is that nodes can be specified by their label rather than
> 	their full path.
> 
> 	Rationale:
> 		In some systems (in FPGAs in particular) a node might
> 		be located in different places in the device tree, but
> 		be the same basic node. In this instance, the base
> 		device tree can still be generated by the FPGA tools
> 		and the node can still be overridden by referring them
> 		with the label
> 
> 		In addition, it is just a lot nicer to be able to
> 		specify labels rather than having to specify the full
> 		path all over again.
> 
> 	Example:
> 		Here is the serial example again, but not inlined with labels:
> 
> 		/ {
> 			soc: soc5200@f0000000 {
> 				#address-cells = <1>;
> 				#size-cells = <1>;
> 				compatible = "fsl,mpc5200b-immr";
> 				...
> 
> 				psc4: serial@2600 {		// PSC4
> 					compatible = "fsl,mpc5200b-psc-uart","fsl,mpc5200-psc-uart";
> 					reg = <0x2600 0x100>;
> 					interrupts = <2 11 0>;
> 				};
> 
> 				psc5: serial@2800 {		// PSC5
> 					compatible = "fsl,mpc5200b-psc-uart","fsl,mpc5200-psc-uart";
> 					reg = <0x2800 0x100>;
> 					interrupts = <2 12 0>;
> 				};
> 		};
> 
> 		&psc4 /remove/ { };		// PSC4
> 		&psc5 /remove/ { };		// PSC5
> 
> 4) Allow full paths to be used to override nodes.
> 
> 	This suggest that labels aren't required. Most people will
> 	want to use labels, but for the device tree syntax to be
> 	orthogonal, full paths should still be allowed.
> 
> 	Example:
> 		Here is the serial example again. Not inlined with paths:
> 
> 		/ {
> 			soc: soc5200@f0000000 {
> 				#address-cells = <1>;
> 				#size-cells = <1>;
> 				compatible = "fsl,mpc5200b-immr";
> 				...
> 
> 				psc4: serial@2600 {		// PSC4
> 					compatible = "fsl,mpc5200b-psc-uart","fsl,mpc5200-psc-uart";
> 					reg = <0x2600 0x100>;
> 					interrupts = <2 11 0>;
> 				};
> 
> 				psc5: serial@2800 {		// PSC5
> 					compatible = "fsl,mpc5200b-psc-uart","fsl,mpc5200-psc-uart";
> 					reg = <0x2800 0x100>;
> 					interrupts = <2 12 0>;
> 				};
> 		};
> 
> 		/soc5200@f0000000/serial@2600 /remove/ { };		// PSC4
> 		/soc5200@f0000000/serial@2800 /remove/ { };		// PSC5

I've thought about this too, and I'd like to implement it, but there
are some nasty gotchas here because /.../ is already used as the
keyword delimiter.  There would need to be a way to ensure the grammer
is unambiguous about which are keywords and which are node names.

On that line, it might also be possible to specify a node by both
label and path:

	/ {
		soc: soc5200@f0000000 {
			#address-cells = <1>;
			#size-cells = <1>;
			compatible = "fsl,mpc5200b-immr";
			...

			psc4: serial@2600 {		// PSC4
				compatible = "fsl,mpc5200b-psc-uart","fsl,mpc5200-psc-uart";
				reg = <0x2600 0x100>;
				interrupts = <2 11 0>;
			};

			psc5: serial@2800 {		// PSC5
				compatible = "fsl,mpc5200b-psc-uart","fsl,mpc5200-psc-uart";
				reg = <0x2800 0x100>;
				interrupts = <2 12 0>;
			};
	};

	&soc/serial@2600 { current-speed = <115200>; };
	&soc/serial@2800 /remove-node/;

g.

RE: Proposal: new device-tree syntax and semantics for extendinginformation from included dts files

[Stephen Neuendorffer]

> /remove-node/?
> 
> >
> > 	This directive provides a simple clear method for removing nodes
that
> > 	were previously defined.
> >
> > 	Exmaple:
> > 		/ {
> > 			soc: soc5200@f0000000 {
> > 				#address-cells = <1>;
> > 				#size-cells = <1>;
> > 				compatible = "fsl,mpc5200b-immr";
> > 				...
> >
> > 				serial@2600 {		// PSC4
> > 					compatible =
"fsl,mpc5200b-psc-uart","fsl,mpc5200-psc-uart";
> > 					reg = <0x2600 0x100>;
> > 					interrupts = <2 11 0>;
> > 				};
> >
> > 				serial@2800 {		// PSC5
> > 					compatible =
"fsl,mpc5200b-psc-uart","fsl,mpc5200-psc-uart";
> > 					reg = <0x2800 0x100>;
> > 					interrupts = <2 12 0>;
> > 				};
> >
> >
> > 				serial@2600 /remove/ { };
// PSC4
> >
> > 				serial@2800 /remove/ { };
// PSC5
> > 		};
> 
> Ditto on fixing example.
> 
> I'm not thrilled with the example because it still shows the empty
> braces which could be construed as the node still being present.  How
> about this syntax instead:
> 	serial@2800 /remove-node/;
> 
> I also wonder if it would be better to have the directives before the
> node name.  ie:
> 	/extend/ node1 { ... };
> 	/replace/ node1 { ... };
> 	/remove/ node1;

Or better yet, outside of the braces?

		/ {
		soc: soc5200@f0000000 {
				#address-cells = <1>;
				#size-cells = <1>;
 				compatible = "fsl,mpc5200b-immr";
 				...

 				serial@2600 {		// PSC4
 					compatible =
"fsl,mpc5200b-psc-uart","fsl,mpc5200-psc-uart";
					reg = <0x2600 0x100>;
					interrupts = <2 11 0>;
 				};

				serial@2800 {		// PSC5
 					compatible =
"fsl,mpc5200b-psc-uart","fsl,mpc5200-psc-uart";
 					reg = <0x2800 0x100>;
 					interrupts = <2 12 0>;
 				};
            };
            /remove/ {
 				serial@2600 { };		// PSC4

 				serial@2800 { };		// PSC5
 		};

I agree the empty braces are wierd....  And perhaps ambiguous (does this
mean remove the node, or remove the empty set of properties from the
node)?
When I've done this in the past, it seemed to make the most sense if
remove always applied to a specific, named node (and sub-children) or
property.
Is removing an entire node even interesting? (is there an interesting
ambiguity in .dts between
an empty node and a non-existent one?)

Personally, I hope to avoid replace and remove, since it is difficult to
tell if
assumptions about which nodes may be present in an included file if
parts of the tree
start getting removed.

Steve




This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

Re: Proposal: new device-tree syntax and semantics for extendinginformation from included dts files

[David Gibson]

On Wed, Oct 13, 2010 at 04:41:59PM -0700, Stephen Neuendorffer wrote:
[snip]
> > I'm not thrilled with the example because it still shows the empty
> > braces which could be construed as the node still being present.  How
> > about this syntax instead:
> > 	serial@2800 /remove-node/;
> > 
> > I also wonder if it would be better to have the directives before the
> > node name.  ie:
> > 	/extend/ node1 { ... };
> > 	/replace/ node1 { ... };
> > 	/remove/ node1;
> 
> Or better yet, outside of the braces?
> 
> 		/ {
> 		soc: soc5200@f0000000 {
> 				#address-cells = <1>;
> 				#size-cells = <1>;
>  				compatible = "fsl,mpc5200b-immr";
>  				...
> 
>  				serial@2600 {		// PSC4
>  					compatible =
> "fsl,mpc5200b-psc-uart","fsl,mpc5200-psc-uart";
> 					reg = <0x2600 0x100>;
> 					interrupts = <2 11 0>;
>  				};
> 
> 				serial@2800 {		// PSC5
>  					compatible =
> "fsl,mpc5200b-psc-uart","fsl,mpc5200-psc-uart";
>  					reg = <0x2800 0x100>;
>  					interrupts = <2 12 0>;
>  				};
>             };
>             /remove/ {
>  				serial@2600 { };		// PSC4
> 
>  				serial@2800 { };		// PSC5
>  		};

Um.. no.  That makes even less sense in the conceptual framework of a
stack of overlays.

> I agree the empty braces are wierd....  And perhaps ambiguous (does this
> mean remove the node, or remove the empty set of properties from the
> node)?

Yes.

> When I've done this in the past, it seemed to make the most sense if
> remove always applied to a specific, named node (and sub-children) or
> property.
> Is removing an entire node even interesting? (is there an interesting
> ambiguity in .dts between
> an empty node and a non-existent one?)

Well, the difference is well defined in both dts and the output dtb
format.  Whether device tree users are likely to care is another
question, but I think the answer is still yes, at least in some cases.

> Personally, I hope to avoid replace and remove, since it is difficult to
> tell if
> assumptions about which nodes may be present in an included file if
> parts of the tree
> start getting removed.

Hrm, that's a point.  We may want to make a distinction between the
operations "delete and give an error if it wasn't there before" and
"delete if present".

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

Re: Proposal: new device-tree syntax and semantics for extendinginformation from included dts files

[Grant Likely]

On Thu, Oct 14, 2010 at 11:45:50AM +1100, David Gibson wrote:
> On Wed, Oct 13, 2010 at 04:41:59PM -0700, Stephen Neuendorffer wrote:
> > Personally, I hope to avoid replace and remove, since it is difficult to
> > tell if
> > assumptions about which nodes may be present in an included file if
> > parts of the tree
> > start getting removed.
> 
> Hrm, that's a point.  We may want to make a distinction between the
> operations "delete and give an error if it wasn't there before" and
> "delete if present".

We don't know if this is going to be an issue yet.  I suggest start
with choosing a stance that covers the path of least surprise (or at
least what we think is the path of least surprise), and add the syntax
for the other behaviour only if it is actually needed.

I say that when trying to delete a node, pitch an error (or warning)
if the target node doesn't exist.  When adding or overriding an node,
don't worry about whether or not an original exists (current behaviour).

g.

Re: Proposal: new device-tree syntax and semantics for extendinginformation from included dts files

[David Gibson]

On Wed, Oct 13, 2010 at 07:46:57PM -0600, Grant Likely wrote:
> On Thu, Oct 14, 2010 at 11:45:50AM +1100, David Gibson wrote:
> > On Wed, Oct 13, 2010 at 04:41:59PM -0700, Stephen Neuendorffer wrote:
> > > Personally, I hope to avoid replace and remove, since it is difficult to
> > > tell if
> > > assumptions about which nodes may be present in an included file if
> > > parts of the tree
> > > start getting removed.
> > 
> > Hrm, that's a point.  We may want to make a distinction between the
> > operations "delete and give an error if it wasn't there before" and
> > "delete if present".
> 
> We don't know if this is going to be an issue yet.  I suggest start
> with choosing a stance that covers the path of least surprise (or at
> least what we think is the path of least surprise), and add the syntax
> for the other behaviour only if it is actually needed.

Heh, I agree in principle, though I'm not at all sure which is the
least surprising.

> I say that when trying to delete a node, pitch an error (or warning)
> if the target node doesn't exist.  When adding or overriding an node,
> don't worry about whether or not an original exists (current behaviour).

I'm certainly not suggesting a change in current semantics for the
existing add/merge/override behaviour.

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

RE: Proposal: new device-tree syntax and semantics for extendinginformation from included dts files

[Stephen Neuendorffer]

> -----Original Message-----
> From: David Gibson [mailto:david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org]
> Sent: Wednesday, October 13, 2010 5:46 PM
> To: Stephen Neuendorffer
> Cc: grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org; John Bonesio;
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> Subject: Re: Proposal: new device-tree syntax and semantics for
extendinginformation from included dts
> files
> 
> On Wed, Oct 13, 2010 at 04:41:59PM -0700, Stephen Neuendorffer wrote:
> [snip]
> > > I'm not thrilled with the example because it still shows the empty
> > > braces which could be construed as the node still being present.
How
> > > about this syntax instead:
> > > 	serial@2800 /remove-node/;
> > >
> > > I also wonder if it would be better to have the directives before
the
> > > node name.  ie:
> > > 	/extend/ node1 { ... };
> > > 	/replace/ node1 { ... };
> > > 	/remove/ node1;
> >
> > Or better yet, outside of the braces?
> >
> > 		/ {
> > 		soc: soc5200@f0000000 {
> > 				#address-cells = <1>;
> > 				#size-cells = <1>;
> >  				compatible = "fsl,mpc5200b-immr";
> >  				...
> >
> >  				serial@2600 {		// PSC4
> >  					compatible =
> > "fsl,mpc5200b-psc-uart","fsl,mpc5200-psc-uart";
> > 					reg = <0x2600 0x100>;
> > 					interrupts = <2 11 0>;
> >  				};
> >
> > 				serial@2800 {		// PSC5
> >  					compatible =
> > "fsl,mpc5200b-psc-uart","fsl,mpc5200-psc-uart";
> >  					reg = <0x2800 0x100>;
> >  					interrupts = <2 12 0>;
> >  				};
> >             };
> >             /remove/ {
> >  				serial@2600 { };		// PSC4
> >
> >  				serial@2800 { };		// PSC5
> >  		};
> 
> Um.. no.  That makes even less sense in the conceptual framework of a
> stack of overlays.

Why exactly?  Instead of being a stack of overlays, it seems to me like
a stack of trees with operators..
The point is exactly that operators make most sense at the stack of
trees level and not
at the individual node level.

Steve

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

Re: Proposal: new device-tree syntax and semantics for extendinginformation from included dts files

[Grant Likely]

On Thu, Oct 14, 2010 at 09:38:44AM -0700, Stephen Neuendorffer wrote:
[fixed quoting header]
> On Wed, Oct 13, 2010 5:46 PM, David Gibson wrote:
> > On Wed, Oct 13, 2010 at 04:41:59PM -0700, Stephen Neuendorffer wrote:
> > [snip]
> > > Or better yet, outside of the braces?
[...]
> > >             /remove/ {
> > >  				serial@2600 { };		// PSC4
> > >
> > >  				serial@2800 { };		// PSC5
> > >  		};
> > 
> > Um.. no.  That makes even less sense in the conceptual framework of a
> > stack of overlays.
> 
> Why exactly?  Instead of being a stack of overlays, it seems to me like
> a stack of trees with operators..
> The point is exactly that operators make most sense at the stack of
> trees level and not
> at the individual node level.

I don't think I'm understanding what you're trying to say.  How do you differentiate "stack of overlays" and "stack of trees"?

The reason I don't like this approach is that in many cases many
things will need to be changed by a single overlay, and not all those
changes will be the same operation.  For example, an overlay for a
board could add a bunch of nodes for i2c devices, and at the same time
remove an unused spi bus device.

The "stack of overlays" conceptual model that we've settled on uses
the concept that subsequent top level trees stack on top of the
preceding tree and can mask out or add/change nodes and properties.
The trees are merged together before going on to the next top level
tree.

g.

RE: Proposal: new device-tree syntax and semantics for extendinginformation from included dts files

[Stephen Neuendorffer]

> -----Original Message-----
> From: Grant Likely [mailto:glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org] On Behalf Of Grant
Likely
> Sent: Friday, October 15, 2010 8:19 AM
> To: Stephen Neuendorffer
> Cc: David Gibson; John Bonesio; devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> Subject: Re: Proposal: new device-tree syntax and semantics for
extendinginformation from included dts
> files
> 
> On Thu, Oct 14, 2010 at 09:38:44AM -0700, Stephen Neuendorffer wrote:
> [fixed quoting header]
> > On Wed, Oct 13, 2010 5:46 PM, David Gibson wrote:
> > > On Wed, Oct 13, 2010 at 04:41:59PM -0700, Stephen Neuendorffer
wrote:
> > > [snip]
> > > > Or better yet, outside of the braces?
> [...]
> > > >             /remove/ {
> > > >  				serial@2600 { };
// PSC4
> > > >
> > > >  				serial@2800 { };
// PSC5
> > > >  		};
> > >
> > > Um.. no.  That makes even less sense in the conceptual framework
of a
> > > stack of overlays.
> >
> > Why exactly?  Instead of being a stack of overlays, it seems to me
like
> > a stack of trees with operators..
> > The point is exactly that operators make most sense at the stack of
> > trees level and not
> > at the individual node level.
> 
> I don't think I'm understanding what you're trying to say.  How do you
differentiate "stack of
> overlays" and "stack of trees"?
> 
> The reason I don't like this approach is that in many cases many
> things will need to be changed by a single overlay, and not all those
> changes will be the same operation.  For example, an overlay for a
> board could add a bunch of nodes for i2c devices, and at the same time
> remove an unused spi bus device.

So why not have two trees stacked to do the job? 
 
> The "stack of overlays" conceptual model that we've settled on uses
> the concept that subsequent top level trees stack on top of the
> preceding tree and can mask out or add/change nodes and properties.
> The trees are merged together before going on to the next top level
> tree.
> 
> g.
> 

I guess I'm stuck on 'overlay' to me implies '/extend/', so I associate
the operations being on trees, not individual nodes.
(Although, there's still the tough part about /remove-node/ vs
/remove-property/,
which might meant that the operations have to be in the trees to
distinguish that).

Steve

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

Re: Proposal: new device-tree syntax and semantics for extendinginformation from included dts files

[Grant Likely]

On Fri, Oct 15, 2010 at 10:10 AM, Stephen Neuendorffer
<stephen.neuendorffer-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org> wrote:
>
>
>> -----Original Message-----
>> From: Grant Likely [mailto:glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org] On Behalf Of Grant
> Likely
>> Sent: Friday, October 15, 2010 8:19 AM
>> To: Stephen Neuendorffer
>> Cc: David Gibson; John Bonesio; devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
>> Subject: Re: Proposal: new device-tree syntax and semantics for
> extendinginformation from included dts
>> files
>>
>> On Thu, Oct 14, 2010 at 09:38:44AM -0700, Stephen Neuendorffer wrote:
>> [fixed quoting header]
>> > On Wed, Oct 13, 2010 5:46 PM, David Gibson wrote:
>> > > On Wed, Oct 13, 2010 at 04:41:59PM -0700, Stephen Neuendorffer
> wrote:
>> > > [snip]
>> > > > Or better yet, outside of the braces?
>> [...]
>> > > >             /remove/ {
>> > > >                                 serial@2600 { };
> // PSC4
>> > > >
>> > > >                                 serial@2800 { };
> // PSC5
>> > > >                 };
>> > >
>> > > Um.. no.  That makes even less sense in the conceptual framework
> of a
>> > > stack of overlays.
>> >
>> > Why exactly?  Instead of being a stack of overlays, it seems to me
> like
>> > a stack of trees with operators..
>> > The point is exactly that operators make most sense at the stack of
>> > trees level and not
>> > at the individual node level.
>>
>> I don't think I'm understanding what you're trying to say.  How do you
> differentiate "stack of
>> overlays" and "stack of trees"?
>>
>> The reason I don't like this approach is that in many cases many
>> things will need to be changed by a single overlay, and not all those
>> changes will be the same operation.  For example, an overlay for a
>> board could add a bunch of nodes for i2c devices, and at the same time
>> remove an unused spi bus device.
>
> So why not have two trees stacked to do the job?

Umm, isn't the suggestion currently on the table?

>> The "stack of overlays" conceptual model that we've settled on uses
>> the concept that subsequent top level trees stack on top of the
>> preceding tree and can mask out or add/change nodes and properties.
>> The trees are merged together before going on to the next top level
>> tree.
>>
>> g.
>>
>
> I guess I'm stuck on 'overlay' to me implies '/extend/', so I associate
> the operations being on trees, not individual nodes.
> (Although, there's still the tough part about /remove-node/ vs
> /remove-property/,
> which might meant that the operations have to be in the trees to
> distinguish that).

Yes, the operations would need to be on the individual nodes; whether
operating on the top level node, or on one of the child nodes.  I
think some examples are in order....

Just for argument, I'm going to assume that we define two new
keywords; /trim-property/ and /trim-node/.  The sole purpose of
/trim-property/ is to remove a property.  /trim-node/ can be used
either to either remove or replace a node.  We can still argue about
the name and the ordering, but the principle remains the same.

Example 1: using full tree overlay
---------
The following .dts file:

	/ {
		the-red: red {
			rgb = <255 0 0>;
		};
		the-blue: blue {
			rgb = <0 0 255>;
			favourite-colour;
		};
		the-green: green {
			rgb = <0 255 0>;
		};
	};

	/ {
		blue {
			rgb = <0 0 127>;
			/trim-property/ favourite-color;
		};

		/trim-node/ green;

		/trim-node/ red {
			vendor = "Benjamin Moore"
		};
	};

This example uses only one overlay tree.  It removes (trims) the
'favourite-colour' property from the blue node.  It removes the
green node outright, and it replaced the red node.

Would be collapse to:

	/ {
		the-red: red {
			vendor = "Benjamin Moore"
		};
		the-blue: blue {
			rgb = <0 0 127>;
		};
	};

Example 2: using label references
---------
The following .dts file:

	/ {
		the-red: red {
			rgb = <255 0 0>;
			firebrick {
				rgb = <178 34 34>;
			};
		};
		the-blue: blue {
			rgb = <0 0 255>;
			favourite-colour;
		};
		the-green: green {
			rgb = <0 255 0>;
		};
	};

	&the-red {
		rgb = <127 0 0>;
		pink {
			rgb = <255 192 203;
		};
		/trim-node/ firebrick {
			ugly-colour;
		};
	};

	/trim-node/ &the-blue;

	/trim-node/ &the-green {
		shade = "radioactive slime"
	};

This example uses three overlay trees.  The first overlay
modified the node pointed to by the label "the-red". It changes the
rgb property, it adds a 'pink' node, and it *replaces* the firebrick
node (by using both the /trim-node/ keyword and a set of { }  braces).

The second removes the node pointed to by "the-blue".

The third replaces the node pointed to by "the-green".

This tree collapses to:

	/ {
		the-red: red {
			rgb = <127 0 0>;
			pink {
				rgb = <255 192 203>;
			};
			firebrick {
				ugly-colour;
			};
		};
		green {
			shade = "radioactive slime"
		};
	};

The /trim-node/ keyword in this model is valid at both the top level
and inside a node.  Properties can never be defined at the top level,
so /trim-property/ only makes sense inside a node.

g.

RE: Proposal: new device-tree syntax and semantics for extendinginformation from included dts files

[Stephen Neuendorffer]

> -----Original Message-----
> From: glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org [mailto:glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org] On Behalf Of Grant Likely
> Sent: Friday, October 15, 2010 12:33 PM
> To: Stephen Neuendorffer
> Cc: David Gibson; John Bonesio; devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> Subject: Re: Proposal: new device-tree syntax and semantics for extendinginformation from included dts
> files
> 
> On Fri, Oct 15, 2010 at 10:10 AM, Stephen Neuendorffer
> <stephen.neuendorffer-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org> wrote:
> >
> >
> >> -----Original Message-----
> >> From: Grant Likely [mailto:glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org] On Behalf Of Grant
> > Likely
> >> Sent: Friday, October 15, 2010 8:19 AM
> >> To: Stephen Neuendorffer
> >> Cc: David Gibson; John Bonesio; devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> >> Subject: Re: Proposal: new device-tree syntax and semantics for
> > extendinginformation from included dts
> >> files
> >>
> >> On Thu, Oct 14, 2010 at 09:38:44AM -0700, Stephen Neuendorffer wrote:
> >> [fixed quoting header]
> >> > On Wed, Oct 13, 2010 5:46 PM, David Gibson wrote:
> >> > > On Wed, Oct 13, 2010 at 04:41:59PM -0700, Stephen Neuendorffer
> > wrote:
> >> > > [snip]
> >> > > > Or better yet, outside of the braces?
> >> [...]
> >> > > >             /remove/ {
> >> > > >                                 serial@2600 { };
> > // PSC4
> >> > > >
> >> > > >                                 serial@2800 { };
> > // PSC5
> >> > > >                 };
> >> > >
> >> > > Um.. no.  That makes even less sense in the conceptual framework
> > of a
> >> > > stack of overlays.
> >> >
> >> > Why exactly?  Instead of being a stack of overlays, it seems to me
> > like
> >> > a stack of trees with operators..
> >> > The point is exactly that operators make most sense at the stack of
> >> > trees level and not
> >> > at the individual node level.
> >>
> >> I don't think I'm understanding what you're trying to say.  How do you
> > differentiate "stack of
> >> overlays" and "stack of trees"?
> >>
> >> The reason I don't like this approach is that in many cases many
> >> things will need to be changed by a single overlay, and not all those
> >> changes will be the same operation.  For example, an overlay for a
> >> board could add a bunch of nodes for i2c devices, and at the same time
> >> remove an unused spi bus device.
> >
> > So why not have two trees stacked to do the job?
> 
> Umm, isn't the suggestion currently on the table?
> 
> >> The "stack of overlays" conceptual model that we've settled on uses
> >> the concept that subsequent top level trees stack on top of the
> >> preceding tree and can mask out or add/change nodes and properties.
> >> The trees are merged together before going on to the next top level
> >> tree.
> >>
> >> g.
> >>
> >
> > I guess I'm stuck on 'overlay' to me implies '/extend/', so I associate
> > the operations being on trees, not individual nodes.
> > (Although, there's still the tough part about /remove-node/ vs
> > /remove-property/,
> > which might meant that the operations have to be in the trees to
> > distinguish that).
> 
> Yes, the operations would need to be on the individual nodes; whether
> operating on the top level node, or on one of the child nodes.  I
> think some examples are in order....
> 
> Just for argument, I'm going to assume that we define two new
> keywords; /trim-property/ and /trim-node/.  The sole purpose of
> /trim-property/ is to remove a property.  /trim-node/ can be used
> either to either remove or replace a node.  We can still argue about
> the name and the ordering, but the principle remains the same.
> 
> Example 1: using full tree overlay
> ---------
> The following .dts file:
> 
> 	/ {
> 		the-red: red {
> 			rgb = <255 0 0>;
> 		};
> 		the-blue: blue {
> 			rgb = <0 0 255>;
> 			favourite-colour;
> 		};
> 		the-green: green {
> 			rgb = <0 255 0>;
> 		};
> 	};
> 
> 	/ {
> 		blue {
> 			rgb = <0 0 127>;
> 			/trim-property/ favourite-color;
> 		};
> 
> 		/trim-node/ green;
> 
> 		/trim-node/ red {
> 			vendor = "Benjamin Moore"
> 		};
> 	};
> 
> This example uses only one overlay tree.  It removes (trims) the
> 'favourite-colour' property from the blue node.  It removes the
> green node outright, and it replaced the red node.
> 
> Would be collapse to:
> 
> 	/ {
> 		the-red: red {
> 			vendor = "Benjamin Moore"
> 		};
> 		the-blue: blue {
> 			rgb = <0 0 127>;
> 		};
> 	};
> 
> Example 2: using label references
> ---------
> The following .dts file:
> 
> 	/ {
> 		the-red: red {
> 			rgb = <255 0 0>;
> 			firebrick {
> 				rgb = <178 34 34>;
> 			};
> 		};
> 		the-blue: blue {
> 			rgb = <0 0 255>;
> 			favourite-colour;
> 		};
> 		the-green: green {
> 			rgb = <0 255 0>;
> 		};
> 	};
> 
> 	&the-red {
> 		rgb = <127 0 0>;
> 		pink {
> 			rgb = <255 192 203;
> 		};
> 		/trim-node/ firebrick {
> 			ugly-colour;
> 		};
> 	};
> 
> 	/trim-node/ &the-blue;
> 
> 	/trim-node/ &the-green {
> 		shade = "radioactive slime"
> 	};
> 
> This example uses three overlay trees.  The first overlay
> modified the node pointed to by the label "the-red". It changes the
> rgb property, it adds a 'pink' node, and it *replaces* the firebrick
> node (by using both the /trim-node/ keyword and a set of { }  braces).
> 
> The second removes the node pointed to by "the-blue".
> 
> The third replaces the node pointed to by "the-green".
> 
> This tree collapses to:
> 
> 	/ {
> 		the-red: red {
> 			rgb = <127 0 0>;
> 			pink {
> 				rgb = <255 192 203>;
> 			};
> 			firebrick {
> 				ugly-colour;
> 			};
> 		};
> 		green {
> 			shade = "radioactive slime"
> 		};
> 	};
> 
> The /trim-node/ keyword in this model is valid at both the top level
> and inside a node.  Properties can never be defined at the top level,
> so /trim-property/ only makes sense inside a node.

I think this all makes sense.. :)

Maybe I can try to sum up in one sentence:
The mental model is still one of overlays, but if a /trim-*/ attribute is present
before a node or property, then the existing node or property is deleted before 
continuing the overlay.

Steve

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

Re: Proposal: new device-tree syntax and semantics for extendinginformation from included dts files

[Grant Likely]

On Fri, Oct 15, 2010 at 1:48 PM, Stephen Neuendorffer
<stephen.neuendorffer-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org> wrote:
>
>
>> -----Original Message-----
>> From: glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org [mailto:glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org] On Behalf Of Grant Likely
>> Sent: Friday, October 15, 2010 12:33 PM
>> To: Stephen Neuendorffer
>> Cc: David Gibson; John Bonesio; devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
>> Subject: Re: Proposal: new device-tree syntax and semantics for extendinginformation from included dts
>> files
>>
>> On Fri, Oct 15, 2010 at 10:10 AM, Stephen Neuendorffer
>> <stephen.neuendorffer-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org> wrote:
>> >
>> >
>> >> -----Original Message-----
>> >> From: Grant Likely [mailto:glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org] On Behalf Of Grant
>> > Likely
>> >> Sent: Friday, October 15, 2010 8:19 AM
>> >> To: Stephen Neuendorffer
>> >> Cc: David Gibson; John Bonesio; devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
>> >> Subject: Re: Proposal: new device-tree syntax and semantics for
>> > extendinginformation from included dts
>> >> files
>> >>
>> >> On Thu, Oct 14, 2010 at 09:38:44AM -0700, Stephen Neuendorffer wrote:
>> >> [fixed quoting header]
>> >> > On Wed, Oct 13, 2010 5:46 PM, David Gibson wrote:
>> >> > > On Wed, Oct 13, 2010 at 04:41:59PM -0700, Stephen Neuendorffer
>> > wrote:
>> >> > > [snip]
>> >> > > > Or better yet, outside of the braces?
>> >> [...]
>> >> > > >             /remove/ {
>> >> > > >                                 serial@2600 { };
>> > // PSC4
>> >> > > >
>> >> > > >                                 serial@2800 { };
>> > // PSC5
>> >> > > >                 };
>> >> > >
>> >> > > Um.. no.  That makes even less sense in the conceptual framework
>> > of a
>> >> > > stack of overlays.
>> >> >
>> >> > Why exactly?  Instead of being a stack of overlays, it seems to me
>> > like
>> >> > a stack of trees with operators..
>> >> > The point is exactly that operators make most sense at the stack of
>> >> > trees level and not
>> >> > at the individual node level.
>> >>
>> >> I don't think I'm understanding what you're trying to say.  How do you
>> > differentiate "stack of
>> >> overlays" and "stack of trees"?
>> >>
>> >> The reason I don't like this approach is that in many cases many
>> >> things will need to be changed by a single overlay, and not all those
>> >> changes will be the same operation.  For example, an overlay for a
>> >> board could add a bunch of nodes for i2c devices, and at the same time
>> >> remove an unused spi bus device.
>> >
>> > So why not have two trees stacked to do the job?
>>
>> Umm, isn't the suggestion currently on the table?
>>
>> >> The "stack of overlays" conceptual model that we've settled on uses
>> >> the concept that subsequent top level trees stack on top of the
>> >> preceding tree and can mask out or add/change nodes and properties.
>> >> The trees are merged together before going on to the next top level
>> >> tree.
>> >>
>> >> g.
>> >>
>> >
>> > I guess I'm stuck on 'overlay' to me implies '/extend/', so I associate
>> > the operations being on trees, not individual nodes.
>> > (Although, there's still the tough part about /remove-node/ vs
>> > /remove-property/,
>> > which might meant that the operations have to be in the trees to
>> > distinguish that).
>>
>> Yes, the operations would need to be on the individual nodes; whether
>> operating on the top level node, or on one of the child nodes.  I
>> think some examples are in order....
>>
>> Just for argument, I'm going to assume that we define two new
>> keywords; /trim-property/ and /trim-node/.  The sole purpose of
>> /trim-property/ is to remove a property.  /trim-node/ can be used
>> either to either remove or replace a node.  We can still argue about
>> the name and the ordering, but the principle remains the same.
>>
>> Example 1: using full tree overlay
>> ---------
>> The following .dts file:
>>
>>       / {
>>               the-red: red {
>>                       rgb = <255 0 0>;
>>               };
>>               the-blue: blue {
>>                       rgb = <0 0 255>;
>>                       favourite-colour;
>>               };
>>               the-green: green {
>>                       rgb = <0 255 0>;
>>               };
>>       };
>>
>>       / {
>>               blue {
>>                       rgb = <0 0 127>;
>>                       /trim-property/ favourite-color;
>>               };
>>
>>               /trim-node/ green;
>>
>>               /trim-node/ red {
>>                       vendor = "Benjamin Moore"
>>               };
>>       };
>>
>> This example uses only one overlay tree.  It removes (trims) the
>> 'favourite-colour' property from the blue node.  It removes the
>> green node outright, and it replaced the red node.
>>
>> Would be collapse to:
>>
>>       / {
>>               the-red: red {
>>                       vendor = "Benjamin Moore"
>>               };
>>               the-blue: blue {
>>                       rgb = <0 0 127>;
>>               };
>>       };
>>
>> Example 2: using label references
>> ---------
>> The following .dts file:
>>
>>       / {
>>               the-red: red {
>>                       rgb = <255 0 0>;
>>                       firebrick {
>>                               rgb = <178 34 34>;
>>                       };
>>               };
>>               the-blue: blue {
>>                       rgb = <0 0 255>;
>>                       favourite-colour;
>>               };
>>               the-green: green {
>>                       rgb = <0 255 0>;
>>               };
>>       };
>>
>>       &the-red {
>>               rgb = <127 0 0>;
>>               pink {
>>                       rgb = <255 192 203;
>>               };
>>               /trim-node/ firebrick {
>>                       ugly-colour;
>>               };
>>       };
>>
>>       /trim-node/ &the-blue;
>>
>>       /trim-node/ &the-green {
>>               shade = "radioactive slime"
>>       };
>>
>> This example uses three overlay trees.  The first overlay
>> modified the node pointed to by the label "the-red". It changes the
>> rgb property, it adds a 'pink' node, and it *replaces* the firebrick
>> node (by using both the /trim-node/ keyword and a set of { }  braces).
>>
>> The second removes the node pointed to by "the-blue".
>>
>> The third replaces the node pointed to by "the-green".
>>
>> This tree collapses to:
>>
>>       / {
>>               the-red: red {
>>                       rgb = <127 0 0>;
>>                       pink {
>>                               rgb = <255 192 203>;
>>                       };
>>                       firebrick {
>>                               ugly-colour;
>>                       };
>>               };
>>               green {
>>                       shade = "radioactive slime"
>>               };
>>       };
>>
>> The /trim-node/ keyword in this model is valid at both the top level
>> and inside a node.  Properties can never be defined at the top level,
>> so /trim-property/ only makes sense inside a node.
>
> I think this all makes sense.. :)
>
> Maybe I can try to sum up in one sentence:
> The mental model is still one of overlays, but if a /trim-*/ attribute is present
> before a node or property, then the existing node or property is deleted before
> continuing the overlay.

Good summary.

g.

RE: Proposal: new device-tree syntax and semantics for extendinginformation from included dts files

[Stephen Neuendorffer]

> -----Original Message-----
> From: glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org [mailto:glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org] On Behalf Of Grant Likely
> Sent: Friday, October 15, 2010 12:56 PM
> To: Stephen Neuendorffer
> Cc: David Gibson; John Bonesio; devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> Subject: Re: Proposal: new device-tree syntax and semantics for extendinginformation from included dts
> files
> 
> On Fri, Oct 15, 2010 at 1:48 PM, Stephen Neuendorffer
> <stephen.neuendorffer-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org> wrote:
> >
> >
> >> -----Original Message-----
> >> From: glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org [mailto:glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org] On Behalf Of Grant Likely
> >> Sent: Friday, October 15, 2010 12:33 PM
> >> To: Stephen Neuendorffer
> >> Cc: David Gibson; John Bonesio; devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> >> Subject: Re: Proposal: new device-tree syntax and semantics for extendinginformation from included
> dts
> >> files
> >>
> >> On Fri, Oct 15, 2010 at 10:10 AM, Stephen Neuendorffer
> >> <stephen.neuendorffer-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org> wrote:
> >> >
> >> >
> >> >> -----Original Message-----
> >> >> From: Grant Likely [mailto:glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org] On Behalf Of Grant
> >> > Likely
> >> >> Sent: Friday, October 15, 2010 8:19 AM
> >> >> To: Stephen Neuendorffer
> >> >> Cc: David Gibson; John Bonesio; devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> >> >> Subject: Re: Proposal: new device-tree syntax and semantics for
> >> > extendinginformation from included dts
> >> >> files
> >> >>
> >> >> On Thu, Oct 14, 2010 at 09:38:44AM -0700, Stephen Neuendorffer wrote:
> >> >> [fixed quoting header]
> >> >> > On Wed, Oct 13, 2010 5:46 PM, David Gibson wrote:
> >> >> > > On Wed, Oct 13, 2010 at 04:41:59PM -0700, Stephen Neuendorffer
> >> > wrote:
> >> >> > > [snip]
> >> >> > > > Or better yet, outside of the braces?
> >> >> [...]
> >> >> > > >             /remove/ {
> >> >> > > >                                 serial@2600 { };
> >> > // PSC4
> >> >> > > >
> >> >> > > >                                 serial@2800 { };
> >> > // PSC5
> >> >> > > >                 };
> >> >> > >
> >> >> > > Um.. no.  That makes even less sense in the conceptual framework
> >> > of a
> >> >> > > stack of overlays.
> >> >> >
> >> >> > Why exactly?  Instead of being a stack of overlays, it seems to me
> >> > like
> >> >> > a stack of trees with operators..
> >> >> > The point is exactly that operators make most sense at the stack of
> >> >> > trees level and not
> >> >> > at the individual node level.
> >> >>
> >> >> I don't think I'm understanding what you're trying to say.  How do you
> >> > differentiate "stack of
> >> >> overlays" and "stack of trees"?
> >> >>
> >> >> The reason I don't like this approach is that in many cases many
> >> >> things will need to be changed by a single overlay, and not all those
> >> >> changes will be the same operation.  For example, an overlay for a
> >> >> board could add a bunch of nodes for i2c devices, and at the same time
> >> >> remove an unused spi bus device.
> >> >
> >> > So why not have two trees stacked to do the job?
> >>
> >> Umm, isn't the suggestion currently on the table?
> >>
> >> >> The "stack of overlays" conceptual model that we've settled on uses
> >> >> the concept that subsequent top level trees stack on top of the
> >> >> preceding tree and can mask out or add/change nodes and properties.
> >> >> The trees are merged together before going on to the next top level
> >> >> tree.
> >> >>
> >> >> g.
> >> >>
> >> >
> >> > I guess I'm stuck on 'overlay' to me implies '/extend/', so I associate
> >> > the operations being on trees, not individual nodes.
> >> > (Although, there's still the tough part about /remove-node/ vs
> >> > /remove-property/,
> >> > which might meant that the operations have to be in the trees to
> >> > distinguish that).
> >>
> >> Yes, the operations would need to be on the individual nodes; whether
> >> operating on the top level node, or on one of the child nodes.  I
> >> think some examples are in order....
> >>
> >> Just for argument, I'm going to assume that we define two new
> >> keywords; /trim-property/ and /trim-node/.  The sole purpose of
> >> /trim-property/ is to remove a property.  /trim-node/ can be used
> >> either to either remove or replace a node.  We can still argue about
> >> the name and the ordering, but the principle remains the same.
> >>
> >> Example 1: using full tree overlay
> >> ---------
> >> The following .dts file:
> >>
> >>       / {
> >>               the-red: red {
> >>                       rgb = <255 0 0>;
> >>               };
> >>               the-blue: blue {
> >>                       rgb = <0 0 255>;
> >>                       favourite-colour;
> >>               };
> >>               the-green: green {
> >>                       rgb = <0 255 0>;
> >>               };
> >>       };
> >>
> >>       / {
> >>               blue {
> >>                       rgb = <0 0 127>;
> >>                       /trim-property/ favourite-color;
> >>               };
> >>
> >>               /trim-node/ green;
> >>
> >>               /trim-node/ red {
> >>                       vendor = "Benjamin Moore"
> >>               };
> >>       };
> >>
> >> This example uses only one overlay tree.  It removes (trims) the
> >> 'favourite-colour' property from the blue node.  It removes the
> >> green node outright, and it replaced the red node.
> >>
> >> Would be collapse to:
> >>
> >>       / {
> >>               the-red: red {
> >>                       vendor = "Benjamin Moore"
> >>               };
> >>               the-blue: blue {
> >>                       rgb = <0 0 127>;
> >>               };
> >>       };
> >>
> >> Example 2: using label references
> >> ---------
> >> The following .dts file:
> >>
> >>       / {
> >>               the-red: red {
> >>                       rgb = <255 0 0>;
> >>                       firebrick {
> >>                               rgb = <178 34 34>;
> >>                       };
> >>               };
> >>               the-blue: blue {
> >>                       rgb = <0 0 255>;
> >>                       favourite-colour;
> >>               };
> >>               the-green: green {
> >>                       rgb = <0 255 0>;
> >>               };
> >>       };
> >>
> >>       &the-red {
> >>               rgb = <127 0 0>;
> >>               pink {
> >>                       rgb = <255 192 203;
> >>               };
> >>               /trim-node/ firebrick {
> >>                       ugly-colour;
> >>               };
> >>       };
> >>
> >>       /trim-node/ &the-blue;
> >>
> >>       /trim-node/ &the-green {
> >>               shade = "radioactive slime"
> >>       };
> >>
> >> This example uses three overlay trees.  The first overlay
> >> modified the node pointed to by the label "the-red". It changes the
> >> rgb property, it adds a 'pink' node, and it *replaces* the firebrick
> >> node (by using both the /trim-node/ keyword and a set of { }  braces).
> >>
> >> The second removes the node pointed to by "the-blue".
> >>
> >> The third replaces the node pointed to by "the-green".
> >>
> >> This tree collapses to:
> >>
> >>       / {
> >>               the-red: red {
> >>                       rgb = <127 0 0>;
> >>                       pink {
> >>                               rgb = <255 192 203>;
> >>                       };
> >>                       firebrick {
> >>                               ugly-colour;
> >>                       };
> >>               };
> >>               green {
> >>                       shade = "radioactive slime"
> >>               };
> >>       };
> >>
> >> The /trim-node/ keyword in this model is valid at both the top level
> >> and inside a node.  Properties can never be defined at the top level,
> >> so /trim-property/ only makes sense inside a node.
> >
> > I think this all makes sense.. :)
> >
> > Maybe I can try to sum up in one sentence:
> > The mental model is still one of overlays, but if a /trim-*/ attribute is present
> > before a node or property, then the existing node or property is deleted before
> > continuing the overlay.
> 
> Good summary.
> 
> g.

Does

/ {
     foo {
         bar;
     }
}

mean add an empty attribute bar or reference a possibly existing node bar and don't do anything to it?
Or does it assert that a node bar exists?



This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

Re: Proposal: new device-tree syntax and semantics for extendinginformation from included dts files

[Grant Likely]

On Fri, Oct 15, 2010 at 1:59 PM, Stephen Neuendorffer
<stephen.neuendorffer-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org> wrote:
>
>
>> -----Original Message-----
>> From: glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org [mailto:glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org] On Behalf Of Grant Likely
>> Sent: Friday, October 15, 2010 12:56 PM
>> To: Stephen Neuendorffer
>> Cc: David Gibson; John Bonesio; devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
>> Subject: Re: Proposal: new device-tree syntax and semantics for extendinginformation from included dts
>> files
>>
>> On Fri, Oct 15, 2010 at 1:48 PM, Stephen Neuendorffer
>> <stephen.neuendorffer-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org> wrote:
>> >
>> >
>> >> -----Original Message-----
>> >> From: glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org [mailto:glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org] On Behalf Of Grant Likely
>> >> Sent: Friday, October 15, 2010 12:33 PM
>> >> To: Stephen Neuendorffer
>> >> Cc: David Gibson; John Bonesio; devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
>> >> Subject: Re: Proposal: new device-tree syntax and semantics for extendinginformation from included
>> dts
>> >> files
>> >>
>> >> On Fri, Oct 15, 2010 at 10:10 AM, Stephen Neuendorffer
>> >> <stephen.neuendorffer-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org> wrote:
>> >> >
>> >> >
>> >> >> -----Original Message-----
>> >> >> From: Grant Likely [mailto:glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org] On Behalf Of Grant
>> >> > Likely
>> >> >> Sent: Friday, October 15, 2010 8:19 AM
>> >> >> To: Stephen Neuendorffer
>> >> >> Cc: David Gibson; John Bonesio; devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
>> >> >> Subject: Re: Proposal: new device-tree syntax and semantics for
>> >> > extendinginformation from included dts
>> >> >> files
>> >> >>
>> >> >> On Thu, Oct 14, 2010 at 09:38:44AM -0700, Stephen Neuendorffer wrote:
>> >> >> [fixed quoting header]
>> >> >> > On Wed, Oct 13, 2010 5:46 PM, David Gibson wrote:
>> >> >> > > On Wed, Oct 13, 2010 at 04:41:59PM -0700, Stephen Neuendorffer
>> >> > wrote:
>> >> >> > > [snip]
>> >> >> > > > Or better yet, outside of the braces?
>> >> >> [...]
>> >> >> > > >             /remove/ {
>> >> >> > > >                                 serial@2600 { };
>> >> > // PSC4
>> >> >> > > >
>> >> >> > > >                                 serial@2800 { };
>> >> > // PSC5
>> >> >> > > >                 };
>> >> >> > >
>> >> >> > > Um.. no.  That makes even less sense in the conceptual framework
>> >> > of a
>> >> >> > > stack of overlays.
>> >> >> >
>> >> >> > Why exactly?  Instead of being a stack of overlays, it seems to me
>> >> > like
>> >> >> > a stack of trees with operators..
>> >> >> > The point is exactly that operators make most sense at the stack of
>> >> >> > trees level and not
>> >> >> > at the individual node level.
>> >> >>
>> >> >> I don't think I'm understanding what you're trying to say.  How do you
>> >> > differentiate "stack of
>> >> >> overlays" and "stack of trees"?
>> >> >>
>> >> >> The reason I don't like this approach is that in many cases many
>> >> >> things will need to be changed by a single overlay, and not all those
>> >> >> changes will be the same operation.  For example, an overlay for a
>> >> >> board could add a bunch of nodes for i2c devices, and at the same time
>> >> >> remove an unused spi bus device.
>> >> >
>> >> > So why not have two trees stacked to do the job?
>> >>
>> >> Umm, isn't the suggestion currently on the table?
>> >>
>> >> >> The "stack of overlays" conceptual model that we've settled on uses
>> >> >> the concept that subsequent top level trees stack on top of the
>> >> >> preceding tree and can mask out or add/change nodes and properties.
>> >> >> The trees are merged together before going on to the next top level
>> >> >> tree.
>> >> >>
>> >> >> g.
>> >> >>
>> >> >
>> >> > I guess I'm stuck on 'overlay' to me implies '/extend/', so I associate
>> >> > the operations being on trees, not individual nodes.
>> >> > (Although, there's still the tough part about /remove-node/ vs
>> >> > /remove-property/,
>> >> > which might meant that the operations have to be in the trees to
>> >> > distinguish that).
>> >>
>> >> Yes, the operations would need to be on the individual nodes; whether
>> >> operating on the top level node, or on one of the child nodes.  I
>> >> think some examples are in order....
>> >>
>> >> Just for argument, I'm going to assume that we define two new
>> >> keywords; /trim-property/ and /trim-node/.  The sole purpose of
>> >> /trim-property/ is to remove a property.  /trim-node/ can be used
>> >> either to either remove or replace a node.  We can still argue about
>> >> the name and the ordering, but the principle remains the same.
>> >>
>> >> Example 1: using full tree overlay
>> >> ---------
>> >> The following .dts file:
>> >>
>> >>       / {
>> >>               the-red: red {
>> >>                       rgb = <255 0 0>;
>> >>               };
>> >>               the-blue: blue {
>> >>                       rgb = <0 0 255>;
>> >>                       favourite-colour;
>> >>               };
>> >>               the-green: green {
>> >>                       rgb = <0 255 0>;
>> >>               };
>> >>       };
>> >>
>> >>       / {
>> >>               blue {
>> >>                       rgb = <0 0 127>;
>> >>                       /trim-property/ favourite-color;
>> >>               };
>> >>
>> >>               /trim-node/ green;
>> >>
>> >>               /trim-node/ red {
>> >>                       vendor = "Benjamin Moore"
>> >>               };
>> >>       };
>> >>
>> >> This example uses only one overlay tree.  It removes (trims) the
>> >> 'favourite-colour' property from the blue node.  It removes the
>> >> green node outright, and it replaced the red node.
>> >>
>> >> Would be collapse to:
>> >>
>> >>       / {
>> >>               the-red: red {
>> >>                       vendor = "Benjamin Moore"
>> >>               };
>> >>               the-blue: blue {
>> >>                       rgb = <0 0 127>;
>> >>               };
>> >>       };
>> >>
>> >> Example 2: using label references
>> >> ---------
>> >> The following .dts file:
>> >>
>> >>       / {
>> >>               the-red: red {
>> >>                       rgb = <255 0 0>;
>> >>                       firebrick {
>> >>                               rgb = <178 34 34>;
>> >>                       };
>> >>               };
>> >>               the-blue: blue {
>> >>                       rgb = <0 0 255>;
>> >>                       favourite-colour;
>> >>               };
>> >>               the-green: green {
>> >>                       rgb = <0 255 0>;
>> >>               };
>> >>       };
>> >>
>> >>       &the-red {
>> >>               rgb = <127 0 0>;
>> >>               pink {
>> >>                       rgb = <255 192 203;
>> >>               };
>> >>               /trim-node/ firebrick {
>> >>                       ugly-colour;
>> >>               };
>> >>       };
>> >>
>> >>       /trim-node/ &the-blue;
>> >>
>> >>       /trim-node/ &the-green {
>> >>               shade = "radioactive slime"
>> >>       };
>> >>
>> >> This example uses three overlay trees.  The first overlay
>> >> modified the node pointed to by the label "the-red". It changes the
>> >> rgb property, it adds a 'pink' node, and it *replaces* the firebrick
>> >> node (by using both the /trim-node/ keyword and a set of { }  braces).
>> >>
>> >> The second removes the node pointed to by "the-blue".
>> >>
>> >> The third replaces the node pointed to by "the-green".
>> >>
>> >> This tree collapses to:
>> >>
>> >>       / {
>> >>               the-red: red {
>> >>                       rgb = <127 0 0>;
>> >>                       pink {
>> >>                               rgb = <255 192 203>;
>> >>                       };
>> >>                       firebrick {
>> >>                               ugly-colour;
>> >>                       };
>> >>               };
>> >>               green {
>> >>                       shade = "radioactive slime"
>> >>               };
>> >>       };
>> >>
>> >> The /trim-node/ keyword in this model is valid at both the top level
>> >> and inside a node.  Properties can never be defined at the top level,
>> >> so /trim-property/ only makes sense inside a node.
>> >
>> > I think this all makes sense.. :)
>> >
>> > Maybe I can try to sum up in one sentence:
>> > The mental model is still one of overlays, but if a /trim-*/ attribute is present
>> > before a node or property, then the existing node or property is deleted before
>> > continuing the overlay.
>>
>> Good summary.
>>
>> g.
>
> Does
>
> / {
>     foo {
>         bar;
>     }
> }
>
> mean add an empty attribute bar or reference a possibly existing node bar and don't do anything to it?
> Or does it assert that a node bar exists?

It adds an empty attribute bar, or changes an existing bar property to be empty.

Both the forms "bar = foo;" and "bar;" always operate on a property.

foo {...}; is always a node.

The only exception is for the removal of a node.  ie. "/trim-node/
foo;"  The primary reason being it be confusing to show the braces
when the node will no longer exist.  There is also a syntactic
difference between "/trim-node/ foo" and "/trim-node/ foo { }" where
the former removes the node entirely and the later replaces it with an
empty node.

The grammer remains parse-able because the syntax only comes into play
when the /trim-node/ keyword appears at the beginning of a statement.

g.

RE: Proposal: new device-tree syntax and semantics for extendinginformation from included dts files

[Stephen Neuendorffer]

> -----Original Message-----
> From: glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org [mailto:glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org] On Behalf Of Grant Likely
> Sent: Friday, October 15, 2010 1:12 PM
> To: Stephen Neuendorffer
> Cc: David Gibson; John Bonesio; devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> Subject: Re: Proposal: new device-tree syntax and semantics for extendinginformation from included dts
> files
> 
> On Fri, Oct 15, 2010 at 1:59 PM, Stephen Neuendorffer
> <stephen.neuendorffer-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org> wrote:
> >
> >
> >> -----Original Message-----
> >> From: glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org [mailto:glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org] On Behalf Of Grant Likely
> >> Sent: Friday, October 15, 2010 12:56 PM
> >> To: Stephen Neuendorffer
> >> Cc: David Gibson; John Bonesio; devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> >> Subject: Re: Proposal: new device-tree syntax and semantics for extendinginformation from included
> dts
> >> files
> >>
> >> On Fri, Oct 15, 2010 at 1:48 PM, Stephen Neuendorffer
> >> <stephen.neuendorffer-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org> wrote:
> >> >
> >> >
> >> >> -----Original Message-----
> >> >> From: glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org [mailto:glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org] On Behalf Of Grant Likely
> >> >> Sent: Friday, October 15, 2010 12:33 PM
> >> >> To: Stephen Neuendorffer
> >> >> Cc: David Gibson; John Bonesio; devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> >> >> Subject: Re: Proposal: new device-tree syntax and semantics for extendinginformation from
> included
> >> dts
> >> >> files
> >> >>
> >> >> On Fri, Oct 15, 2010 at 10:10 AM, Stephen Neuendorffer
> >> >> <stephen.neuendorffer-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org> wrote:
> >> >> >
> >> >> >
> >> >> >> -----Original Message-----
> >> >> >> From: Grant Likely [mailto:glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org] On Behalf Of Grant
> >> >> > Likely
> >> >> >> Sent: Friday, October 15, 2010 8:19 AM
> >> >> >> To: Stephen Neuendorffer
> >> >> >> Cc: David Gibson; John Bonesio; devicetree-discuss-uLR06cmDAlaKREJ1Ck/qmQ@public.gmane.orgorg
> >> >> >> Subject: Re: Proposal: new device-tree syntax and semantics for
> >> >> > extendinginformation from included dts
> >> >> >> files
> >> >> >>
> >> >> >> On Thu, Oct 14, 2010 at 09:38:44AM -0700, Stephen Neuendorffer wrote:
> >> >> >> [fixed quoting header]
> >> >> >> > On Wed, Oct 13, 2010 5:46 PM, David Gibson wrote:
> >> >> >> > > On Wed, Oct 13, 2010 at 04:41:59PM -0700, Stephen Neuendorffer
> >> >> > wrote:
> >> >> >> > > [snip]
> >> >> >> > > > Or better yet, outside of the braces?
> >> >> >> [...]
> >> >> >> > > >             /remove/ {
> >> >> >> > > >                                 serial@2600 { };
> >> >> > // PSC4
> >> >> >> > > >
> >> >> >> > > >                                 serial@2800 { };
> >> >> > // PSC5
> >> >> >> > > >                 };
> >> >> >> > >
> >> >> >> > > Um.. no.  That makes even less sense in the conceptual framework
> >> >> > of a
> >> >> >> > > stack of overlays.
> >> >> >> >
> >> >> >> > Why exactly?  Instead of being a stack of overlays, it seems to me
> >> >> > like
> >> >> >> > a stack of trees with operators..
> >> >> >> > The point is exactly that operators make most sense at the stack of
> >> >> >> > trees level and not
> >> >> >> > at the individual node level.
> >> >> >>
> >> >> >> I don't think I'm understanding what you're trying to say.  How do you
> >> >> > differentiate "stack of
> >> >> >> overlays" and "stack of trees"?
> >> >> >>
> >> >> >> The reason I don't like this approach is that in many cases many
> >> >> >> things will need to be changed by a single overlay, and not all those
> >> >> >> changes will be the same operation.  For example, an overlay for a
> >> >> >> board could add a bunch of nodes for i2c devices, and at the same time
> >> >> >> remove an unused spi bus device.
> >> >> >
> >> >> > So why not have two trees stacked to do the job?
> >> >>
> >> >> Umm, isn't the suggestion currently on the table?
> >> >>
> >> >> >> The "stack of overlays" conceptual model that we've settled on uses
> >> >> >> the concept that subsequent top level trees stack on top of the
> >> >> >> preceding tree and can mask out or add/change nodes and properties.
> >> >> >> The trees are merged together before going on to the next top level
> >> >> >> tree.
> >> >> >>
> >> >> >> g.
> >> >> >>
> >> >> >
> >> >> > I guess I'm stuck on 'overlay' to me implies '/extend/', so I associate
> >> >> > the operations being on trees, not individual nodes.
> >> >> > (Although, there's still the tough part about /remove-node/ vs
> >> >> > /remove-property/,
> >> >> > which might meant that the operations have to be in the trees to
> >> >> > distinguish that).
> >> >>
> >> >> Yes, the operations would need to be on the individual nodes; whether
> >> >> operating on the top level node, or on one of the child nodes.  I
> >> >> think some examples are in order....
> >> >>
> >> >> Just for argument, I'm going to assume that we define two new
> >> >> keywords; /trim-property/ and /trim-node/.  The sole purpose of
> >> >> /trim-property/ is to remove a property.  /trim-node/ can be used
> >> >> either to either remove or replace a node.  We can still argue about
> >> >> the name and the ordering, but the principle remains the same.
> >> >>
> >> >> Example 1: using full tree overlay
> >> >> ---------
> >> >> The following .dts file:
> >> >>
> >> >>       / {
> >> >>               the-red: red {
> >> >>                       rgb = <255 0 0>;
> >> >>               };
> >> >>               the-blue: blue {
> >> >>                       rgb = <0 0 255>;
> >> >>                       favourite-colour;
> >> >>               };
> >> >>               the-green: green {
> >> >>                       rgb = <0 255 0>;
> >> >>               };
> >> >>       };
> >> >>
> >> >>       / {
> >> >>               blue {
> >> >>                       rgb = <0 0 127>;
> >> >>                       /trim-property/ favourite-color;
> >> >>               };
> >> >>
> >> >>               /trim-node/ green;
> >> >>
> >> >>               /trim-node/ red {
> >> >>                       vendor = "Benjamin Moore"
> >> >>               };
> >> >>       };
> >> >>
> >> >> This example uses only one overlay tree.  It removes (trims) the
> >> >> 'favourite-colour' property from the blue node.  It removes the
> >> >> green node outright, and it replaced the red node.
> >> >>
> >> >> Would be collapse to:
> >> >>
> >> >>       / {
> >> >>               the-red: red {
> >> >>                       vendor = "Benjamin Moore"
> >> >>               };
> >> >>               the-blue: blue {
> >> >>                       rgb = <0 0 127>;
> >> >>               };
> >> >>       };
> >> >>
> >> >> Example 2: using label references
> >> >> ---------
> >> >> The following .dts file:
> >> >>
> >> >>       / {
> >> >>               the-red: red {
> >> >>                       rgb = <255 0 0>;
> >> >>                       firebrick {
> >> >>                               rgb = <178 34 34>;
> >> >>                       };
> >> >>               };
> >> >>               the-blue: blue {
> >> >>                       rgb = <0 0 255>;
> >> >>                       favourite-colour;
> >> >>               };
> >> >>               the-green: green {
> >> >>                       rgb = <0 255 0>;
> >> >>               };
> >> >>       };
> >> >>
> >> >>       &the-red {
> >> >>               rgb = <127 0 0>;
> >> >>               pink {
> >> >>                       rgb = <255 192 203;
> >> >>               };
> >> >>               /trim-node/ firebrick {
> >> >>                       ugly-colour;
> >> >>               };
> >> >>       };
> >> >>
> >> >>       /trim-node/ &the-blue;
> >> >>
> >> >>       /trim-node/ &the-green {
> >> >>               shade = "radioactive slime"
> >> >>       };
> >> >>
> >> >> This example uses three overlay trees.  The first overlay
> >> >> modified the node pointed to by the label "the-red". It changes the
> >> >> rgb property, it adds a 'pink' node, and it *replaces* the firebrick
> >> >> node (by using both the /trim-node/ keyword and a set of { }  braces).
> >> >>
> >> >> The second removes the node pointed to by "the-blue".
> >> >>
> >> >> The third replaces the node pointed to by "the-green".
> >> >>
> >> >> This tree collapses to:
> >> >>
> >> >>       / {
> >> >>               the-red: red {
> >> >>                       rgb = <127 0 0>;
> >> >>                       pink {
> >> >>                               rgb = <255 192 203>;
> >> >>                       };
> >> >>                       firebrick {
> >> >>                               ugly-colour;
> >> >>                       };
> >> >>               };
> >> >>               green {
> >> >>                       shade = "radioactive slime"
> >> >>               };
> >> >>       };
> >> >>
> >> >> The /trim-node/ keyword in this model is valid at both the top level
> >> >> and inside a node.  Properties can never be defined at the top level,
> >> >> so /trim-property/ only makes sense inside a node.
> >> >
> >> > I think this all makes sense.. :)
> >> >
> >> > Maybe I can try to sum up in one sentence:
> >> > The mental model is still one of overlays, but if a /trim-*/ attribute is present
> >> > before a node or property, then the existing node or property is deleted before
> >> > continuing the overlay.
> >>
> >> Good summary.
> >>
> >> g.
> >
> > Does
> >
> > / {
> >     foo {
> >         bar;
> >     }
> > }
> >
> > mean add an empty attribute bar or reference a possibly existing node bar and don't do anything to
> it?
> > Or does it assert that a node bar exists?
> 
> It adds an empty attribute bar, or changes an existing bar property to be empty.
> 
> Both the forms "bar = foo;" and "bar;" always operate on a property.
> 
> foo {...}; is always a node.
> 
> The only exception is for the removal of a node.  ie. "/trim-node/
> foo;"  The primary reason being it be confusing to show the braces
> when the node will no longer exist.  There is also a syntactic
> difference between "/trim-node/ foo" and "/trim-node/ foo { }" where
> the former removes the node entirely and the later replaces it with an
> empty node.
> 
> The grammer remains parse-able because the syntax only comes into play
> when the /trim-node/ keyword appears at the beginning of a statement.
> 
> g.

The ambiguity between
bar; (referring to a property)
and 
/trim-node/ bar; (referring to a node)

Seems awkward, but perhaps unavoidable.  At the very least, I don't have anything better to suggest.. :)

perhaps in an ideal world, open firmware would originally have distinguished these by
having

bar =;   // Create an empty property
bar {};  // Create an empty node

or something...

Steve


This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

Re: Proposal: new device-tree syntax and semantics for extendinginformation from included dts files

[Grant Likely]

On Fri, Oct 15, 2010 at 2:20 PM, Stephen Neuendorffer
<stephen.neuendorffer-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org> wrote:
> The ambiguity between
> bar; (referring to a property)
> and
> /trim-node/ bar; (referring to a node)
>
> Seems awkward, but perhaps unavoidable.  At the very least, I don't have anything better to suggest.. :)
>
> perhaps in an ideal world, open firmware would originally have distinguished these by
> having
>
> bar =;   // Create an empty property
> bar {};  // Create an empty node
>
> or something...

Actually, none of this is OpenFirmware related.  This syntax was
created for use with dtc which is completely independent of
OpenFirmware.  Only the device tree structure and usage conventions
are shared with dtc.

However, we've already got a large userbase so making an incompatible
source file format change would be unbelievably painful.

g.

Re: Proposal: new device-tree syntax and semantics for extendinginformation from included dts files

[David Gibson]

On Fri, Oct 15, 2010 at 05:58:06PM -0600, Grant Likely wrote:
> On Fri, Oct 15, 2010 at 2:20 PM, Stephen Neuendorffer
> <stephen.neuendorffer-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org> wrote:
> > The ambiguity between
> > bar; (referring to a property)
> > and
> > /trim-node/ bar; (referring to a node)
> >
> > Seems awkward, but perhaps unavoidable.  At the very least, I don't have anything better to suggest.. :)
> >
> > perhaps in an ideal world, open firmware would originally have distinguished these by
> > having
> >
> > bar =;   // Create an empty property
> > bar {};  // Create an empty node
> >
> > or something...
> 
> Actually, none of this is OpenFirmware related.  This syntax was
> created for use with dtc which is completely independent of
> OpenFirmware.  Only the device tree structure and usage conventions
> are shared with dtc.

That's true.  However, I'm pretty sure there was actually a reason
related to existing OF usage why I made the syntax for an empty
property 'thing;' rather than 'thing =;'.  I no longer remember what
it was, though.

> However, we've already got a large userbase so making an incompatible
> source file format change would be unbelievably painful.

That too.

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

Re: Proposal: new device-tree syntax and semantics for extendinginformation from included dts files

[John Bonesio]

On Fri, 2010-10-15 at 14:11 -0600, Grant Likely wrote:
> On Fri, Oct 15, 2010 at 1:59 PM, Stephen Neuendorffer
> <stephen.neuendorffer-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org> wrote:
> >
> >
> >> -----Original Message-----
> >> From: glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org [mailto:glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org] On Behalf Of Grant Likely
> >> Sent: Friday, October 15, 2010 12:56 PM
> >> To: Stephen Neuendorffer
> >> Cc: David Gibson; John Bonesio; devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> >> Subject: Re: Proposal: new device-tree syntax and semantics for extendinginformation from included dts
> >> files
> >>
> >> On Fri, Oct 15, 2010 at 1:48 PM, Stephen Neuendorffer
> >> <stephen.neuendorffer-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org> wrote:
> >> >
> >> >
> >> >> -----Original Message-----
> >> >> From: glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org [mailto:glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org] On Behalf Of Grant Likely
> >> >> Sent: Friday, October 15, 2010 12:33 PM
> >> >> To: Stephen Neuendorffer
> >> >> Cc: David Gibson; John Bonesio; devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> >> >> Subject: Re: Proposal: new device-tree syntax and semantics for extendinginformation from included
> >> dts
> >> >> files
> >> >>
> >> >> On Fri, Oct 15, 2010 at 10:10 AM, Stephen Neuendorffer
> >> >> <stephen.neuendorffer-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org> wrote:
> >> >> >
> >> >> >
> >> >> >> -----Original Message-----
> >> >> >> From: Grant Likely [mailto:glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org] On Behalf Of Grant
> >> >> > Likely
> >> >> >> Sent: Friday, October 15, 2010 8:19 AM
> >> >> >> To: Stephen Neuendorffer
> >> >> >> Cc: David Gibson; John Bonesio; devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> >> >> >> Subject: Re: Proposal: new device-tree syntax and semantics for
> >> >> > extendinginformation from included dts
> >> >> >> files
> >> >> >>
> >> >> >> On Thu, Oct 14, 2010 at 09:38:44AM -0700, Stephen Neuendorffer wrote:
> >> >> >> [fixed quoting header]
> >> >> >> > On Wed, Oct 13, 2010 5:46 PM, David Gibson wrote:
> >> >> >> > > On Wed, Oct 13, 2010 at 04:41:59PM -0700, Stephen Neuendorffer
> >> >> > wrote:
> >> >> >> > > [snip]
> >> >> >> > > > Or better yet, outside of the braces?
> >> >> >> [...]
> >> >> >> > > >             /remove/ {
> >> >> >> > > >                                 serial@2600 { };
> >> >> > // PSC4
> >> >> >> > > >
> >> >> >> > > >                                 serial@2800 { };
> >> >> > // PSC5
> >> >> >> > > >                 };
> >> >> >> > >
> >> >> >> > > Um.. no.  That makes even less sense in the conceptual framework
> >> >> > of a
> >> >> >> > > stack of overlays.
> >> >> >> >
> >> >> >> > Why exactly?  Instead of being a stack of overlays, it seems to me
> >> >> > like
> >> >> >> > a stack of trees with operators..
> >> >> >> > The point is exactly that operators make most sense at the stack of
> >> >> >> > trees level and not
> >> >> >> > at the individual node level.
> >> >> >>
> >> >> >> I don't think I'm understanding what you're trying to say.  How do you
> >> >> > differentiate "stack of
> >> >> >> overlays" and "stack of trees"?
> >> >> >>
> >> >> >> The reason I don't like this approach is that in many cases many
> >> >> >> things will need to be changed by a single overlay, and not all those
> >> >> >> changes will be the same operation.  For example, an overlay for a
> >> >> >> board could add a bunch of nodes for i2c devices, and at the same time
> >> >> >> remove an unused spi bus device.
> >> >> >
> >> >> > So why not have two trees stacked to do the job?
> >> >>
> >> >> Umm, isn't the suggestion currently on the table?
> >> >>
> >> >> >> The "stack of overlays" conceptual model that we've settled on uses
> >> >> >> the concept that subsequent top level trees stack on top of the
> >> >> >> preceding tree and can mask out or add/change nodes and properties.
> >> >> >> The trees are merged together before going on to the next top level
> >> >> >> tree.
> >> >> >>
> >> >> >> g.
> >> >> >>
> >> >> >
> >> >> > I guess I'm stuck on 'overlay' to me implies '/extend/', so I associate
> >> >> > the operations being on trees, not individual nodes.
> >> >> > (Although, there's still the tough part about /remove-node/ vs
> >> >> > /remove-property/,
> >> >> > which might meant that the operations have to be in the trees to
> >> >> > distinguish that).
> >> >>
> >> >> Yes, the operations would need to be on the individual nodes; whether
> >> >> operating on the top level node, or on one of the child nodes.  I
> >> >> think some examples are in order....
> >> >>
> >> >> Just for argument, I'm going to assume that we define two new
> >> >> keywords; /trim-property/ and /trim-node/.  The sole purpose of
> >> >> /trim-property/ is to remove a property.  /trim-node/ can be used
> >> >> either to either remove or replace a node.  We can still argue about
> >> >> the name and the ordering, but the principle remains the same.
> >> >>
> >> >> Example 1: using full tree overlay
> >> >> ---------
> >> >> The following .dts file:
> >> >>
> >> >>       / {
> >> >>               the-red: red {
> >> >>                       rgb = <255 0 0>;
> >> >>               };
> >> >>               the-blue: blue {
> >> >>                       rgb = <0 0 255>;
> >> >>                       favourite-colour;
> >> >>               };
> >> >>               the-green: green {
> >> >>                       rgb = <0 255 0>;
> >> >>               };
> >> >>       };
> >> >>
> >> >>       / {
> >> >>               blue {
> >> >>                       rgb = <0 0 127>;
> >> >>                       /trim-property/ favourite-color;
> >> >>               };
> >> >>
> >> >>               /trim-node/ green;
> >> >>
> >> >>               /trim-node/ red {
> >> >>                       vendor = "Benjamin Moore"
> >> >>               };
> >> >>       };
> >> >>
> >> >> This example uses only one overlay tree.  It removes (trims) the
> >> >> 'favourite-colour' property from the blue node.  It removes the
> >> >> green node outright, and it replaced the red node.
> >> >>
> >> >> Would be collapse to:
> >> >>
> >> >>       / {
> >> >>               the-red: red {
> >> >>                       vendor = "Benjamin Moore"
> >> >>               };
> >> >>               the-blue: blue {
> >> >>                       rgb = <0 0 127>;
> >> >>               };
> >> >>       };
> >> >>
> >> >> Example 2: using label references
> >> >> ---------
> >> >> The following .dts file:
> >> >>
> >> >>       / {
> >> >>               the-red: red {
> >> >>                       rgb = <255 0 0>;
> >> >>                       firebrick {
> >> >>                               rgb = <178 34 34>;
> >> >>                       };
> >> >>               };
> >> >>               the-blue: blue {
> >> >>                       rgb = <0 0 255>;
> >> >>                       favourite-colour;
> >> >>               };
> >> >>               the-green: green {
> >> >>                       rgb = <0 255 0>;
> >> >>               };
> >> >>       };
> >> >>
> >> >>       &the-red {
> >> >>               rgb = <127 0 0>;
> >> >>               pink {
> >> >>                       rgb = <255 192 203;
> >> >>               };
> >> >>               /trim-node/ firebrick {
> >> >>                       ugly-colour;
> >> >>               };
> >> >>       };
> >> >>
> >> >>       /trim-node/ &the-blue;
> >> >>
> >> >>       /trim-node/ &the-green {
> >> >>               shade = "radioactive slime"
> >> >>       };
> >> >>
> >> >> This example uses three overlay trees.  The first overlay
> >> >> modified the node pointed to by the label "the-red". It changes the
> >> >> rgb property, it adds a 'pink' node, and it *replaces* the firebrick
> >> >> node (by using both the /trim-node/ keyword and a set of { }  braces).
> >> >>
> >> >> The second removes the node pointed to by "the-blue".
> >> >>
> >> >> The third replaces the node pointed to by "the-green".
> >> >>
> >> >> This tree collapses to:
> >> >>
> >> >>       / {
> >> >>               the-red: red {
> >> >>                       rgb = <127 0 0>;
> >> >>                       pink {
> >> >>                               rgb = <255 192 203>;
> >> >>                       };
> >> >>                       firebrick {
> >> >>                               ugly-colour;
> >> >>                       };
> >> >>               };
> >> >>               green {
> >> >>                       shade = "radioactive slime"
> >> >>               };
> >> >>       };
> >> >>
> >> >> The /trim-node/ keyword in this model is valid at both the top level
> >> >> and inside a node.  Properties can never be defined at the top level,
> >> >> so /trim-property/ only makes sense inside a node.
> >> >
> >> > I think this all makes sense.. :)
> >> >
> >> > Maybe I can try to sum up in one sentence:
> >> > The mental model is still one of overlays, but if a /trim-*/ attribute is present
> >> > before a node or property, then the existing node or property is deleted before
> >> > continuing the overlay.
> >>
> >> Good summary.
> >>
> >> g.
> >
> > Does
> >
> > / {
> >     foo {
> >         bar;
> >     }
> > }
> >
> > mean add an empty attribute bar or reference a possibly existing node bar and don't do anything to it?
> > Or does it assert that a node bar exists?
> 
> It adds an empty attribute bar, or changes an existing bar property to be empty.
> 
> Both the forms "bar = foo;" and "bar;" always operate on a property.
> 
> foo {...}; is always a node.
> 
> The only exception is for the removal of a node.  ie. "/trim-node/
> foo;"  The primary reason being it be confusing to show the braces
> when the node will no longer exist.  There is also a syntactic
> difference between "/trim-node/ foo" and "/trim-node/ foo { }" where
> the former removes the node entirely and the later replaces it with an
> empty node.
> 
> The grammer remains parse-able because the syntax only comes into play
> when the /trim-node/ keyword appears at the beginning of a statement.
> 
> g.

This is where I think it might be better to have a different syntax for
removal of a node. Let me explain by giving a example.

One of the things that trips people up with the C language is the use of
= vs ==. It all seems good until you have:
if (x = 3) ...
It's too easy for people to miss that it's a single (=), instead of a
double (==). And the assignment in the if statement is valid. So
experienced programmers may do some things to prevent the mistake such
as
	if (3 = x) ... /* which gives an error with the wrong '=' */
or turning on "attila the warnings" in the compiler.

I think the proposed /trim-node/ syntax for removal is going to create
the same style of trip-up for people. When someone looks at
/trim-node/ foo { };
are they going to think the node is gone, or empty? Even if they know
the syntax, are they going to make mistakes overlooking an error,
thinking it looks right?

With the proposed, there's not even an obvious way someone can do
anything that will alert them to possible erroneous statements.

If we had:
/remove-node/ foo;

Then we could make the parser give an error on:
/remove-node/ foo {};

We could even possibly make
/trim-node/ foo;
an error.

This way there is less room for confusion or trip-ups.

What are your thoughts?

Re: Proposal: new device-tree syntax and semantics for extendinginformation from included dts files

[John Bonesio]

Just for reference, here is a sample segment of the grammar for
the /trim-node/ and /remove-node/ syntax:

devicetree:
	  '/' nodedef
		{
			$$ = name_node($2, "");
		}
	| devicetree '/' nodedef
		{
			$$ = merge_nodes($1, $3);
		}
	| devicetree DT_REF nodedef
		{
			struct node *target;

			target = get_node_by_label($1, $2);
			if (target)
				merge_nodes(target, $3);
			else
				yyerror("label does not exist in "
					" node redefinition");
			$$ = $1;
		}
	| devicetree DT_REPLACENODE DT_REF nodedef
		{
			struct node *target;

			target = get_node_by_label($1, $3);
			if (target)
				replace_nodes(target, $4);
			else
				yyerror("label does not exist in "
					" node redefinition");
			$$ = $1;
		}
	| devicetree DT_REMOVENODE DT_REF ';'
		{
			struct node *target;

			target = get_node_by_label($1, $3);
			if (target)
				remove_nodes(target);
			else
				yyerror("label does not exist in "
					" node redefinition");
			$$ = $1;
		}
	;

With the /remove-node/ part, it's a syntax error to have anything but
';' after the node reference, and with /trim-node/, it's a syntax error
to have anything but '{ ... }' after it.

- John


On Fri, 2010-10-15 at 13:35 -0700, John Bonesio wrote:
> On Fri, 2010-10-15 at 14:11 -0600, Grant Likely wrote:
> > On Fri, Oct 15, 2010 at 1:59 PM, Stephen Neuendorffer
> > <stephen.neuendorffer-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org> wrote:
> > >
> > >
> > >> -----Original Message-----
> > >> From: glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org [mailto:glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org] On Behalf Of Grant Likely
> > >> Sent: Friday, October 15, 2010 12:56 PM
> > >> To: Stephen Neuendorffer
> > >> Cc: David Gibson; John Bonesio; devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> > >> Subject: Re: Proposal: new device-tree syntax and semantics for extendinginformation from included dts
> > >> files
> > >>
> > >> On Fri, Oct 15, 2010 at 1:48 PM, Stephen Neuendorffer
> > >> <stephen.neuendorffer-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org> wrote:
> > >> >
> > >> >
> > >> >> -----Original Message-----
> > >> >> From: glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org [mailto:glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org] On Behalf Of Grant Likely
> > >> >> Sent: Friday, October 15, 2010 12:33 PM
> > >> >> To: Stephen Neuendorffer
> > >> >> Cc: David Gibson; John Bonesio; devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> > >> >> Subject: Re: Proposal: new device-tree syntax and semantics for extendinginformation from included
> > >> dts
> > >> >> files
> > >> >>
> > >> >> On Fri, Oct 15, 2010 at 10:10 AM, Stephen Neuendorffer
> > >> >> <stephen.neuendorffer-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org> wrote:
> > >> >> >
> > >> >> >
> > >> >> >> -----Original Message-----
> > >> >> >> From: Grant Likely [mailto:glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org] On Behalf Of Grant
> > >> >> > Likely
> > >> >> >> Sent: Friday, October 15, 2010 8:19 AM
> > >> >> >> To: Stephen Neuendorffer
> > >> >> >> Cc: David Gibson; John Bonesio; devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> > >> >> >> Subject: Re: Proposal: new device-tree syntax and semantics for
> > >> >> > extendinginformation from included dts
> > >> >> >> files
> > >> >> >>
> > >> >> >> On Thu, Oct 14, 2010 at 09:38:44AM -0700, Stephen Neuendorffer wrote:
> > >> >> >> [fixed quoting header]
> > >> >> >> > On Wed, Oct 13, 2010 5:46 PM, David Gibson wrote:
> > >> >> >> > > On Wed, Oct 13, 2010 at 04:41:59PM -0700, Stephen Neuendorffer
> > >> >> > wrote:
> > >> >> >> > > [snip]
> > >> >> >> > > > Or better yet, outside of the braces?
> > >> >> >> [...]
> > >> >> >> > > >             /remove/ {
> > >> >> >> > > >                                 serial@2600 { };
> > >> >> > // PSC4
> > >> >> >> > > >
> > >> >> >> > > >                                 serial@2800 { };
> > >> >> > // PSC5
> > >> >> >> > > >                 };
> > >> >> >> > >
> > >> >> >> > > Um.. no.  That makes even less sense in the conceptual framework
> > >> >> > of a
> > >> >> >> > > stack of overlays.
> > >> >> >> >
> > >> >> >> > Why exactly?  Instead of being a stack of overlays, it seems to me
> > >> >> > like
> > >> >> >> > a stack of trees with operators..
> > >> >> >> > The point is exactly that operators make most sense at the stack of
> > >> >> >> > trees level and not
> > >> >> >> > at the individual node level.
> > >> >> >>
> > >> >> >> I don't think I'm understanding what you're trying to say.  How do you
> > >> >> > differentiate "stack of
> > >> >> >> overlays" and "stack of trees"?
> > >> >> >>
> > >> >> >> The reason I don't like this approach is that in many cases many
> > >> >> >> things will need to be changed by a single overlay, and not all those
> > >> >> >> changes will be the same operation.  For example, an overlay for a
> > >> >> >> board could add a bunch of nodes for i2c devices, and at the same time
> > >> >> >> remove an unused spi bus device.
> > >> >> >
> > >> >> > So why not have two trees stacked to do the job?
> > >> >>
> > >> >> Umm, isn't the suggestion currently on the table?
> > >> >>
> > >> >> >> The "stack of overlays" conceptual model that we've settled on uses
> > >> >> >> the concept that subsequent top level trees stack on top of the
> > >> >> >> preceding tree and can mask out or add/change nodes and properties.
> > >> >> >> The trees are merged together before going on to the next top level
> > >> >> >> tree.
> > >> >> >>
> > >> >> >> g.
> > >> >> >>
> > >> >> >
> > >> >> > I guess I'm stuck on 'overlay' to me implies '/extend/', so I associate
> > >> >> > the operations being on trees, not individual nodes.
> > >> >> > (Although, there's still the tough part about /remove-node/ vs
> > >> >> > /remove-property/,
> > >> >> > which might meant that the operations have to be in the trees to
> > >> >> > distinguish that).
> > >> >>
> > >> >> Yes, the operations would need to be on the individual nodes; whether
> > >> >> operating on the top level node, or on one of the child nodes.  I
> > >> >> think some examples are in order....
> > >> >>
> > >> >> Just for argument, I'm going to assume that we define two new
> > >> >> keywords; /trim-property/ and /trim-node/.  The sole purpose of
> > >> >> /trim-property/ is to remove a property.  /trim-node/ can be used
> > >> >> either to either remove or replace a node.  We can still argue about
> > >> >> the name and the ordering, but the principle remains the same.
> > >> >>
> > >> >> Example 1: using full tree overlay
> > >> >> ---------
> > >> >> The following .dts file:
> > >> >>
> > >> >>       / {
> > >> >>               the-red: red {
> > >> >>                       rgb = <255 0 0>;
> > >> >>               };
> > >> >>               the-blue: blue {
> > >> >>                       rgb = <0 0 255>;
> > >> >>                       favourite-colour;
> > >> >>               };
> > >> >>               the-green: green {
> > >> >>                       rgb = <0 255 0>;
> > >> >>               };
> > >> >>       };
> > >> >>
> > >> >>       / {
> > >> >>               blue {
> > >> >>                       rgb = <0 0 127>;
> > >> >>                       /trim-property/ favourite-color;
> > >> >>               };
> > >> >>
> > >> >>               /trim-node/ green;
> > >> >>
> > >> >>               /trim-node/ red {
> > >> >>                       vendor = "Benjamin Moore"
> > >> >>               };
> > >> >>       };
> > >> >>
> > >> >> This example uses only one overlay tree.  It removes (trims) the
> > >> >> 'favourite-colour' property from the blue node.  It removes the
> > >> >> green node outright, and it replaced the red node.
> > >> >>
> > >> >> Would be collapse to:
> > >> >>
> > >> >>       / {
> > >> >>               the-red: red {
> > >> >>                       vendor = "Benjamin Moore"
> > >> >>               };
> > >> >>               the-blue: blue {
> > >> >>                       rgb = <0 0 127>;
> > >> >>               };
> > >> >>       };
> > >> >>
> > >> >> Example 2: using label references
> > >> >> ---------
> > >> >> The following .dts file:
> > >> >>
> > >> >>       / {
> > >> >>               the-red: red {
> > >> >>                       rgb = <255 0 0>;
> > >> >>                       firebrick {
> > >> >>                               rgb = <178 34 34>;
> > >> >>                       };
> > >> >>               };
> > >> >>               the-blue: blue {
> > >> >>                       rgb = <0 0 255>;
> > >> >>                       favourite-colour;
> > >> >>               };
> > >> >>               the-green: green {
> > >> >>                       rgb = <0 255 0>;
> > >> >>               };
> > >> >>       };
> > >> >>
> > >> >>       &the-red {
> > >> >>               rgb = <127 0 0>;
> > >> >>               pink {
> > >> >>                       rgb = <255 192 203;
> > >> >>               };
> > >> >>               /trim-node/ firebrick {
> > >> >>                       ugly-colour;
> > >> >>               };
> > >> >>       };
> > >> >>
> > >> >>       /trim-node/ &the-blue;
> > >> >>
> > >> >>       /trim-node/ &the-green {
> > >> >>               shade = "radioactive slime"
> > >> >>       };
> > >> >>
> > >> >> This example uses three overlay trees.  The first overlay
> > >> >> modified the node pointed to by the label "the-red". It changes the
> > >> >> rgb property, it adds a 'pink' node, and it *replaces* the firebrick
> > >> >> node (by using both the /trim-node/ keyword and a set of { }  braces).
> > >> >>
> > >> >> The second removes the node pointed to by "the-blue".
> > >> >>
> > >> >> The third replaces the node pointed to by "the-green".
> > >> >>
> > >> >> This tree collapses to:
> > >> >>
> > >> >>       / {
> > >> >>               the-red: red {
> > >> >>                       rgb = <127 0 0>;
> > >> >>                       pink {
> > >> >>                               rgb = <255 192 203>;
> > >> >>                       };
> > >> >>                       firebrick {
> > >> >>                               ugly-colour;
> > >> >>                       };
> > >> >>               };
> > >> >>               green {
> > >> >>                       shade = "radioactive slime"
> > >> >>               };
> > >> >>       };
> > >> >>
> > >> >> The /trim-node/ keyword in this model is valid at both the top level
> > >> >> and inside a node.  Properties can never be defined at the top level,
> > >> >> so /trim-property/ only makes sense inside a node.
> > >> >
> > >> > I think this all makes sense.. :)
> > >> >
> > >> > Maybe I can try to sum up in one sentence:
> > >> > The mental model is still one of overlays, but if a /trim-*/ attribute is present
> > >> > before a node or property, then the existing node or property is deleted before
> > >> > continuing the overlay.
> > >>
> > >> Good summary.
> > >>
> > >> g.
> > >
> > > Does
> > >
> > > / {
> > >     foo {
> > >         bar;
> > >     }
> > > }
> > >
> > > mean add an empty attribute bar or reference a possibly existing node bar and don't do anything to it?
> > > Or does it assert that a node bar exists?
> > 
> > It adds an empty attribute bar, or changes an existing bar property to be empty.
> > 
> > Both the forms "bar = foo;" and "bar;" always operate on a property.
> > 
> > foo {...}; is always a node.
> > 
> > The only exception is for the removal of a node.  ie. "/trim-node/
> > foo;"  The primary reason being it be confusing to show the braces
> > when the node will no longer exist.  There is also a syntactic
> > difference between "/trim-node/ foo" and "/trim-node/ foo { }" where
> > the former removes the node entirely and the later replaces it with an
> > empty node.
> > 
> > The grammer remains parse-able because the syntax only comes into play
> > when the /trim-node/ keyword appears at the beginning of a statement.
> > 
> > g.
> 
> This is where I think it might be better to have a different syntax for
> removal of a node. Let me explain by giving a example.
> 
> One of the things that trips people up with the C language is the use of
> = vs ==. It all seems good until you have:
> if (x = 3) ...
> It's too easy for people to miss that it's a single (=), instead of a
> double (==). And the assignment in the if statement is valid. So
> experienced programmers may do some things to prevent the mistake such
> as
> 	if (3 = x) ... /* which gives an error with the wrong '=' */
> or turning on "attila the warnings" in the compiler.
> 
> I think the proposed /trim-node/ syntax for removal is going to create
> the same style of trip-up for people. When someone looks at
> /trim-node/ foo { };
> are they going to think the node is gone, or empty? Even if they know
> the syntax, are they going to make mistakes overlooking an error,
> thinking it looks right?
> 
> With the proposed, there's not even an obvious way someone can do
> anything that will alert them to possible erroneous statements.
> 
> If we had:
> /remove-node/ foo;
> 
> Then we could make the parser give an error on:
> /remove-node/ foo {};
> 
> We could even possibly make
> /trim-node/ foo;
> an error.
> 
> This way there is less room for confusion or trip-ups.
> 
> What are your thoughts?
> 
> 
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: Proposal: new device-tree syntax and semantics for extendinginformation from included dts files

[Grant Likely]

On Fri, Oct 15, 2010 at 2:35 PM, John Bonesio <bones-RkXMcIGFT/TR7s880joybQ@public.gmane.org> wrote:
[snip]
> If we had:
> /remove-node/ foo;
>
> Then we could make the parser give an error on:
> /remove-node/ foo {};
>
> We could even possibly make
> /trim-node/ foo;
> an error.
>
> This way there is less room for confusion or trip-ups.
>
> What are your thoughts?

I'm still partial to using a single keyword for both use-cases, but
most of the implementation will be the same regardless of which is
chosen.  Go ahead and code it and post a patch.

g.

Re: Proposal: new device-tree syntax and semantics for extendinginformation from included dts files

[David Gibson]

On Fri, Oct 15, 2010 at 09:10:55AM -0700, Stephen Neuendorffer wrote:
[snip]
> > > Why exactly?  Instead of being a stack of overlays, it seems to me
> like
> > > a stack of trees with operators..
> > > The point is exactly that operators make most sense at the stack of
> > > trees level and not
> > > at the individual node level.
> > 
> > I don't think I'm understanding what you're trying to say.  How do you
> differentiate "stack of
> > overlays" and "stack of trees"?
> > 
> > The reason I don't like this approach is that in many cases many
> > things will need to be changed by a single overlay, and not all those
> > changes will be the same operation.  For example, an overlay for a
> > board could add a bunch of nodes for i2c devices, and at the same time
> > remove an unused spi bus device.
> 
> So why not have two trees stacked to do the job? 
>  
> > The "stack of overlays" conceptual model that we've settled on uses
> > the concept that subsequent top level trees stack on top of the
> > preceding tree and can mask out or add/change nodes and properties.
> > The trees are merged together before going on to the next top level
> > tree.
> 
> I guess I'm stuck on 'overlay' to me implies '/extend/', so I associate
> the operations being on trees, not individual nodes.
> (Although, there's still the tough part about /remove-node/ vs
> /remove-property/,
> which might meant that the operations have to be in the trees to
> distinguish that).

I'm with Steve here, even though I still don't follow his reasoning
from here to the syntax he proposed earlier.  But I think we've
kind of moved past that option now, anyway.

There can be multiple overlays / top-level items per-file so there's
no reason the example above can't be done as multiple overlays with
different operaitons.

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

Re: Proposal: new device-tree syntax and semantics for extending information from included dts files

[David Gibson]

On Wed, Oct 13, 2010 at 05:17:47PM -0600, Grant Likely wrote:
> On Tue, Oct 12, 2010 at 02:56:01PM -0700, John Bonesio wrote:
> > Objective:
> > The primary problem is that the same information gets repeated in
> > various dts file that describe variations on the same base SOC. The
> > information becomes redundant and when changes to the device tree
> > syntax is made, extra work is necessary to update the same
> > information in multiple files.
> 
> Thanks John.  Some comments below, but a reasonable proposal.  Next
> you need to post the proposal to the devicetree-discuss list and cc:
> David Gibson, Jon Loeliger and me.
> 
> ... actually, this discussion is important enough that I don't want to
> send this reply without David and Jon seeing it; otherwise we just end
> up repeating the same rationals over again.  I'm going to cc the
> list right now.

Excellent :)

> > A solution needs to allow:
> > a) nodes to be updated
> > b) nodes to be overridden
> > c) nodes to be removed
> > d) properties to be overridden
> > e) properties to be removed
> > 
> > Overview:
> > The main thrust of this proposal is to suggest an overlay system
> > where existing device tree nodes can be modified by dts statements
> > later on in the "file". The "file" is defined as a file in memory
> > after all /include/ statements have been processed.

Um, I know what you mean, but the wording is incorrect on a
technicality - parsing and include processing are done at the same
time, so the full include-expanded "file" never exists as a whole in
memory, only conceptually.

> > Much of the functionality described in this proposal is already
> > present in a test version of the device tree compiler (dtc). This
> > proposal describes
> 
> s/test/unreleased/

Do you mean you have a test branch of your own, or do you mean the
partial overlay functionality that's already upstream?

Note that "released" is a somewhat fuzzy concept for dtc.  The overlay
code is not released in the sense that we haven't made a new version
number and tarball since it went in.  However, neither Jon nor I have
been particularly active about tagging and rolling up "releases".  The
small scope of dtc along with good test coverage have meant that for a
long time now just about any random git snapshot has been entirely
usable and stable.

> > the functionality in it's entirety so that the changed parts can be
> > viewed in light of the whole. This proposal suggests extensions, and
> > might inadvertently suggest small modifications to this existing
> > functionality.
> > 
> > The main idea of this proposal suggests that when duplicate nodes are
> > encountered, later nodes seen by the compiler will update or in some way
> > modify the node already encountered.
> > 
> > This proposal suggest that there be 3 ways in which an existing node
> > can be modified. The node can be merged with the new information, it
> > can be superseded by the new information, or it can be deleted. The
> > method used would be defined by a new directive.
> > 
> > This proposal recommends that properties are not updated directly but
> > that this overlay functionality only apply to nodes.
> 
> I'm not sure what you mean by this.  There will be situations when a
> single property needs to be removed without affecting the rest of the
> node contents (as you state in the requirements).

Yeah, the explanation you have below makes sense, but this wording is
confusing.

> > Proposal:
> > 
> > 1) Add a new keyword constant for property values, <Undefined>.
> 
> Personally, I'd prefer all lowercase: <undefined>.  There may be some
> dickering over the exact keyword name, but I think the concept is
> sound.

It should be /undefined/ to match existing keywords.  Or possibly
/delete/ or /remove-property/ or some such.  As Grant says there is
some dickering to be done there.

> > 	The constant means 'not defined'. If a property is assigned
> > 	the value of <Undefined>, then it's as if the property was
> > 	never defined. [Solves requirement (e)]
> 
> Ack.

Yes, this basic approach is also what I had in mind.

> > 	Rationale:
> > 		When we want to use /include/ and then modify or
> > 		extend an existing base dts (.dtsi) file, using
> > 		<Undefined> provides a clear indication to anyone
> > 		reading the dts file that this property is being
> > 		removed.  By using <Undefined> as a value, this syntax
> > 		become a part of the device tree language rather than
> > 		a command telling the compiler to do something.
> > 
> > 	Example:
> > 		#size-cells = <Undefined>;
> > 
> > 2) Allow 3 directives for modifying existing nodes in the
> > system: /extend/,
> >    /replace/, /remove/
> > 
> > 	The test version of the dtc already provides the /extend/
> > 	method of merging
> 
> s/test/unreleased/
> 
> > 	existing nodes. The version of the dtc may provide a way to
> > 	extend properties as well, but this proposal suggest that
> > 	individual properties should not be allowed to be updated.
> 
> Again, I'm not sure what you mean by not allowing properties to be
> updated.  The current behaviour, which I think is sane, is that a
> redefinition of a property will replace the value.
> 
> > 
> > 	[Solves requirements (a-d)]
> > 
> > 	A) /extend/
> 
> "/extend-node/" is possibly a clearer naming to avoid confusion with
> working with properties.

Better yet, since it is the default, just don't have the keyword at
all.  Upstream dtc already has "extend", adding an explicit keyword is
just extra syntax for no purpose.

> > 	This is the default directive. When no directive is provided,
> > 	it is assumed that /extend/ is to be used. If a node is
> > 	described twice in the system, the second instance of the node
> > 	merges with the first. If the second node has duplicate
> > 	properties, the property in the last instance of the node is
> > 	the value used.
> > 
> > 	Example:
> > 		/ {
> > 			...
> > 			memory {
> > 				device_type = "memory";
> > 				reg = <0x00000000 0x04000000>;	// 64MB
> > 			};
> > 			...
> > 
> > 			memory /extend/ {
> > 				reg = <0x00000000 0x08000000>;	// 128MB RAM
> > 			}
> > 
> > 			memory {
> > 				reg = <0x00000000 0x10000000>;	// 256MB RAM
> > 			}
> > 
> > 		};
> 
> This example isn't correct.  In the current design, nodes will only
> get merged at the top-level and the same node cannot be redefined
> within the same top-level tree structure.  In essence, device tree
> nodes are unordered sets of properties and child nodes.  Overriding
> a node within the same tree has been explicitly avoided.  This example
> should be:

What Grant said.

[snip]
> > 		In this example, first the 2nd instance of the node,
> > 		memory, is merged with the first with the /extend/
> > 		directive. The reg property is overridden to 128MB.
> > 		Then the 3rd instance of the node, memory, is also
> > 		merged. The reg property is overridden to 256MB. In
> > 		this second case, /extend/ is assumed since it is the
> > 		default method of modifying an existing node.
> > 
> > 	B) /replace/
> 
> /replace-node/ perhaps?

There is no need for this.  It's equivalent to a /remove/ then an
/extend/, which is barely more verbose.

[snip]
> > 	C) /remove/
> 
> /remove-node/?
> 
> > 	This directive provides a simple clear method for removing nodes that
> > 	were previously defined.
> > 
> > 	Exmaple:
> > 		/ {
> > 			soc: soc5200@f0000000 {
> > 				#address-cells = <1>;
> > 				#size-cells = <1>;
> > 				compatible = "fsl,mpc5200b-immr";
> > 				...
> > 
> > 				serial@2600 {		// PSC4
> > 					compatible = "fsl,mpc5200b-psc-uart","fsl,mpc5200-psc-uart";
> > 					reg = <0x2600 0x100>;
> > 					interrupts = <2 11 0>;
> > 				};
> > 
> > 				serial@2800 {		// PSC5
> > 					compatible = "fsl,mpc5200b-psc-uart","fsl,mpc5200-psc-uart";
> > 					reg = <0x2800 0x100>;
> > 					interrupts = <2 12 0>;
> > 				};
> > 
> > 
> > 				serial@2600 /remove/ { };		// PSC4
> > 
> > 				serial@2800 /remove/ { };		// PSC5
> > 		};
> 
> Ditto on fixing example.
> 
> I'm not thrilled with the example because it still shows the empty
> braces which could be construed as the node still being present.  How
> about this syntax instead:
> 	serial@2800 /remove-node/;

Absolutely, the braces should not be there on a remove.

> I also wonder if it would be better to have the directives before the
> node name.  ie:
> 	/extend/ node1 { ... };
> 	/replace/ node1 { ... };
> 	/remove/ node1;

Maybe, yes, although I think the first two should go away anyway.

Actually, how about:
	node1 /undefine/;

Using the same /undefine/ keyword as for removing individual
properties (but without the =).

[snip]
> > 4) Allow full paths to be used to override nodes.
> > 
> > 	This suggest that labels aren't required. Most people will
> > 	want to use labels, but for the device tree syntax to be
> > 	orthogonal, full paths should still be allowed.
> > 
> > 	Example:
> > 		Here is the serial example again. Not inlined with paths:
> > 
> > 		/ {
> > 			soc: soc5200@f0000000 {
> > 				#address-cells = <1>;
> > 				#size-cells = <1>;
> > 				compatible = "fsl,mpc5200b-immr";
> > 				...
> > 
> > 				psc4: serial@2600 {		// PSC4
> > 					compatible = "fsl,mpc5200b-psc-uart","fsl,mpc5200-psc-uart";
> > 					reg = <0x2600 0x100>;
> > 					interrupts = <2 11 0>;
> > 				};
> > 
> > 				psc5: serial@2800 {		// PSC5
> > 					compatible = "fsl,mpc5200b-psc-uart","fsl,mpc5200-psc-uart";
> > 					reg = <0x2800 0x100>;
> > 					interrupts = <2 12 0>;
> > 				};
> > 		};
> > 
> > 		/soc5200@f0000000/serial@2600 /remove/ { };		// PSC4
> > 		/soc5200@f0000000/serial@2800 /remove/ { };		// PSC5
> 
> I've thought about this too, and I'd like to implement it, but there
> are some nasty gotchas here because /.../ is already used as the
> keyword delimiter.  There would need to be a way to ensure the grammer
> is unambiguous about which are keywords and which are node names.

Yeah.  But I think there's an obvious way to fix it.  We already have
a full-path reference syntax we can use for inserting phandles.  So I
think we should make the syntax here:

&{/full/path/to/node} {
	property-to-override = "something";
};

Or of course you can already do

/ {
	full {
		path {
			to {
				node {
					etc.

> On that line, it might also be possible to specify a node by both
> label and path:

Well, this is sort of already possible:

&soc {
	serial@2600 {
		current-speed = <115200>;
	};
};

Directly allowing a label+relative path could be useful enough to
reduce verbosity, but it will require careful thinking about the
syntax.

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

Re: Proposal: new device-tree syntax and semantics for extending information from included dts files

[Grant Likely]

On Thu, Oct 14, 2010 at 11:38:33AM +1100, David Gibson wrote:
> On Wed, Oct 13, 2010 at 05:17:47PM -0600, Grant Likely wrote:
> > On Tue, Oct 12, 2010 at 02:56:01PM -0700, John Bonesio wrote:
> > > Objective:
> > > The primary problem is that the same information gets repeated in
> > > various dts file that describe variations on the same base SOC. The
> > > information becomes redundant and when changes to the device tree
> > > syntax is made, extra work is necessary to update the same
> > > information in multiple files.
> > 
> > Thanks John.  Some comments below, but a reasonable proposal.  Next
> > you need to post the proposal to the devicetree-discuss list and cc:
> > David Gibson, Jon Loeliger and me.
> > 
> > ... actually, this discussion is important enough that I don't want to
> > send this reply without David and Jon seeing it; otherwise we just end
> > up repeating the same rationals over again.  I'm going to cc the
> > list right now.
> 
> Excellent :)

:-D

> 
> > > A solution needs to allow:
> > > a) nodes to be updated
> > > b) nodes to be overridden
> > > c) nodes to be removed
> > > d) properties to be overridden
> > > e) properties to be removed
> > > 
> > > Overview:
> > > The main thrust of this proposal is to suggest an overlay system
> > > where existing device tree nodes can be modified by dts statements
> > > later on in the "file". The "file" is defined as a file in memory
> > > after all /include/ statements have been processed.
> 
> Um, I know what you mean, but the wording is incorrect on a
> technicality - parsing and include processing are done at the same
> time, so the full include-expanded "file" never exists as a whole in
> memory, only conceptually.
> 
> > > Much of the functionality described in this proposal is already
> > > present in a test version of the device tree compiler (dtc). This
> > > proposal describes
> > 
> > s/test/unreleased/
> 
> Do you mean you have a test branch of your own, or do you mean the
> partial overlay functionality that's already upstream?

We're talking about the already upstreamed bits.

> > > Proposal:
> > > 
> > > 1) Add a new keyword constant for property values, <Undefined>.
> > 
> > Personally, I'd prefer all lowercase: <undefined>.  There may be some
> > dickering over the exact keyword name, but I think the concept is
> > sound.
> 
> It should be /undefined/ to match existing keywords.  Or possibly
> /delete/ or /remove-property/ or some such.  As Grant says there is
> some dickering to be done there.

ah, of course.  I missed that.  <...> already has a meaning in dts
syntax, so that should definitely be avoided.  /.../ it is.

> > > 	B) /replace/
> > 
> > /replace-node/ perhaps?
> 
> There is no need for this.  It's equivalent to a /remove/ then an
> /extend/, which is barely more verbose.

Need to flush this out.  What would this look like then?  This?

&some-node {
	node-to-remove /remove/;

	node-to-replace /remove/;
	node-to-replace { prop = "blah"; };
};

I don't like this one because it implies ordering between the first
and second 'node-to-replace' definitions, and to this point the syntax
has not imposed any order assumptions.

Perhaps this:

&some-node {
	node-to-remove /remove/;
	node-to-replace /remove/ { prop = "blah"; };
};

Here the keyword essentially becomes a flag to drop the previous
definition of the node with the option to also provide a replacement
defintion.  I like this better, and the (lack of) ordering assumptions
are preserved.

How then would it look when compared with removing properties?

&some-node {
	property-to-remove = /remove/;
	property-to-replace = "new value";
	property-to-replace-with-empty-prop;

	node-to-remove /remove/;
	node-to-replace /remove/ { prop = "blah"; };
};

In this form, the only syntactic difference between removing
properties and removing nodes is that removing properties has an '='
token between them.  I think that this has the potential to be
confusing to users, and probably a common source of defects.  That is
part of why I like the idea of different keywords when dealing with
nodes and properties.  What about this?

&some-node {
	property-to-remove /remove-prop/;
	property-to-replace = "new value";
	property-to-replace-with-empty-prop;

	node-to-remove /remove-node/;
	node-to-replace /remove-node/ { prop = "blah"; };
};

Or perhaps with the keyword preceding:

&some-node {
	/remove-prop/ property-to-remove;
	property-to-replace = "new value";
	property-to-replace-with-empty-prop;

	/remove-node/ node-to-remove;
	/remove-node/ node-to-replace { prop = "blah"; };
};

And the following construct would be forbidden:

	/remove-prop/ property-to-remove = "new value;


Also, /remove-node/ feels awkward when working with replacing a node.
Would /clear-node/ or /trim-node/ be better?  '/replace-node/'
wouldn't be very because it doesn't make sense in the removal case,
and /delete-node/ has the same problem as /remove-node/.
/undefine-node/ as you suggested has possibilities too.  Anyone have
other suggestions for intuitive keyword names?

> > I also wonder if it would be better to have the directives before the
> > node name.  ie:
> > 	/extend/ node1 { ... };
> > 	/replace/ node1 { ... };
> > 	/remove/ node1;
> 
> Maybe, yes, although I think the first two should go away anyway.
> 
> Actually, how about:
> 	node1 /undefine/;
> 
> Using the same /undefine/ keyword as for removing individual
> properties (but without the =).

See discussion above.  :-)

> 
> [snip]
> > > 4) Allow full paths to be used to override nodes.
> > > 
> > > 	This suggest that labels aren't required. Most people will
> > > 	want to use labels, but for the device tree syntax to be
> > > 	orthogonal, full paths should still be allowed.
> > > 
> > > 	Example:
> > > 		Here is the serial example again. Not inlined with paths:
> > > 
> > > 		/ {
> > > 			soc: soc5200@f0000000 {
> > > 				#address-cells = <1>;
> > > 				#size-cells = <1>;
> > > 				compatible = "fsl,mpc5200b-immr";
> > > 				...
> > > 
> > > 				psc4: serial@2600 {		// PSC4
> > > 					compatible = "fsl,mpc5200b-psc-uart","fsl,mpc5200-psc-uart";
> > > 					reg = <0x2600 0x100>;
> > > 					interrupts = <2 11 0>;
> > > 				};
> > > 
> > > 				psc5: serial@2800 {		// PSC5
> > > 					compatible = "fsl,mpc5200b-psc-uart","fsl,mpc5200-psc-uart";
> > > 					reg = <0x2800 0x100>;
> > > 					interrupts = <2 12 0>;
> > > 				};
> > > 		};
> > > 
> > > 		/soc5200@f0000000/serial@2600 /remove/ { };		// PSC4
> > > 		/soc5200@f0000000/serial@2800 /remove/ { };		// PSC5
> > 
> > I've thought about this too, and I'd like to implement it, but there
> > are some nasty gotchas here because /.../ is already used as the
> > keyword delimiter.  There would need to be a way to ensure the grammer
> > is unambiguous about which are keywords and which are node names.
> 
> Yeah.  But I think there's an obvious way to fix it.  We already have
> a full-path reference syntax we can use for inserting phandles.  So I
> think we should make the syntax here:
> 
> &{/full/path/to/node} {
> 	property-to-override = "something";
> };

True.  That would work.  And I like the reuse of existing syntax
elements.

> 
> Or of course you can already do
> 
> / {
> 	full {
> 		path {
> 			to {
> 				node {
> 					etc.

Right.

> > On that line, it might also be possible to specify a node by both
> > label and path:
> 
> Well, this is sort of already possible:
> 
> &soc {
> 	serial@2600 {
> 		current-speed = <115200>;
> 	};
> };
> 
> Directly allowing a label+relative path could be useful enough to
> reduce verbosity, but it will require careful thinking about the
> syntax.

Yes, (as discussed previously) this is just syntactic sugar.  The
syntax already supports the functionality, but it would be nice to be
less verbose.

g.

Re: Proposal: new device-tree syntax and semantics for extending information from included dts files

[Grant Likely]

On Wed, Oct 13, 2010 at 07:40:36PM -0600, Grant Likely wrote:
> On Thu, Oct 14, 2010 at 11:38:33AM +1100, David Gibson wrote:
> > On Wed, Oct 13, 2010 at 05:17:47PM -0600, Grant Likely wrote:
> > > On Tue, Oct 12, 2010 at 02:56:01PM -0700, John Bonesio wrote:
> &some-node {
> 	property-to-remove /remove-prop/;
> 	property-to-replace = "new value";
> 	property-to-replace-with-empty-prop;
> 
> 	node-to-remove /remove-node/;
> 	node-to-replace /remove-node/ { prop = "blah"; };
> };
> 
> Or perhaps with the keyword preceding:
> 
> &some-node {
> 	/remove-prop/ property-to-remove;
> 	property-to-replace = "new value";
> 	property-to-replace-with-empty-prop;
> 
> 	/remove-node/ node-to-remove;
> 	/remove-node/ node-to-replace { prop = "blah"; };
> };

I should probably also mock-up what it would look like when the target
node is at the top level.  With keyword after the node name:

/ {
	...
};
&some-node-to-remove /remove-node/;
&some-node-to-replace /remove-node/ { prop = "blah"; };

And with keywords preceeding:

/ {
	...
};
/remove-node/ &some-node-to-remove;
/remove-node/ &some-node-to-replace { prop = "blah"; };

g.

Re: Proposal: new device-tree syntax and semantics for extending information from included dts files

[John Bonesio]

On Wed, 2010-10-13 at 19:40 -0600, Grant Likely wrote:
> On Thu, Oct 14, 2010 at 11:38:33AM +1100, David Gibson wrote:
> > On Wed, Oct 13, 2010 at 05:17:47PM -0600, Grant Likely wrote:
> > > On Tue, Oct 12, 2010 at 02:56:01PM -0700, John Bonesio wrote:
> > > > Objective:
> > > > The primary problem is that the same information gets repeated in
> > > > various dts file that describe variations on the same base SOC. The
> > > > information becomes redundant and when changes to the device tree
> > > > syntax is made, extra work is necessary to update the same
> > > > information in multiple files.
> > > 
> > > Thanks John.  Some comments below, but a reasonable proposal.  Next
> > > you need to post the proposal to the devicetree-discuss list and cc:
> > > David Gibson, Jon Loeliger and me.
> > > 
> > > ... actually, this discussion is important enough that I don't want to
> > > send this reply without David and Jon seeing it; otherwise we just end
> > > up repeating the same rationals over again.  I'm going to cc the
> > > list right now.
> > 
> > Excellent :)
> 
> :-D
> 
> > 
> > > > A solution needs to allow:
> > > > a) nodes to be updated
> > > > b) nodes to be overridden
> > > > c) nodes to be removed
> > > > d) properties to be overridden
> > > > e) properties to be removed
> > > > 
> > > > Overview:
> > > > The main thrust of this proposal is to suggest an overlay system
> > > > where existing device tree nodes can be modified by dts statements
> > > > later on in the "file". The "file" is defined as a file in memory
> > > > after all /include/ statements have been processed.
> > 
> > Um, I know what you mean, but the wording is incorrect on a
> > technicality - parsing and include processing are done at the same
> > time, so the full include-expanded "file" never exists as a whole in
> > memory, only conceptually.
> > 
> > > > Much of the functionality described in this proposal is already
> > > > present in a test version of the device tree compiler (dtc). This
> > > > proposal describes
> > > 
> > > s/test/unreleased/
> > 
> > Do you mean you have a test branch of your own, or do you mean the
> > partial overlay functionality that's already upstream?
> 
> We're talking about the already upstreamed bits.
> 
> > > > Proposal:
> > > > 
> > > > 1) Add a new keyword constant for property values, <Undefined>.
> > > 
> > > Personally, I'd prefer all lowercase: <undefined>.  There may be some
> > > dickering over the exact keyword name, but I think the concept is
> > > sound.
> > 
> > It should be /undefined/ to match existing keywords.  Or possibly
> > /delete/ or /remove-property/ or some such.  As Grant says there is
> > some dickering to be done there.
> 
> ah, of course.  I missed that.  <...> already has a meaning in dts
> syntax, so that should definitely be avoided.  /.../ it is.
> 
> > > > 	B) /replace/
> > > 
> > > /replace-node/ perhaps?
> > 
> > There is no need for this.  It's equivalent to a /remove/ then an
> > /extend/, which is barely more verbose.
> 
> Need to flush this out.  What would this look like then?  This?
> 
> &some-node {
> 	node-to-remove /remove/;
> 
> 	node-to-replace /remove/;
> 	node-to-replace { prop = "blah"; };
> };
> 
> I don't like this one because it implies ordering between the first
> and second 'node-to-replace' definitions, and to this point the syntax
> has not imposed any order assumptions.
> 
> Perhaps this:
> 
> &some-node {
> 	node-to-remove /remove/;
> 	node-to-replace /remove/ { prop = "blah"; };
> };

I think this is going to be really confusing. A person looking at this
for the first time will be wondering, "Why are we removing property,
'prop'. I thought we needed that one?"

I believe that if we want to be able to replace nodes, we should stick
with the /replace/ syntax.

The dts syntax is meant for humans to read, otherwise, we'd just encode
the bits in the dtb file by hand. The new syntax and semantics doesn't
have to be reduced to provide only one way to do each thing. I believe
we should optimize for human readability.

My vote is to go ahead with the 3 methods of modifying existing nodes
which is /extend/, /replace/, and /remove/. Use of the /extend/ syntax,
while optional as this is the default, should still be allowed in the
case where most of the updates are using the other methods and for
clarity, the writer wants to make /extend/ explicit.

So the syntax would be:
&some-node {
	node-to-remove /remove/;
	node-to-replace /replace/ { prop = "blah"; };
}

or even

&node-to-remove /remove/;
&node-to-replace /replace/ { prop = "blah"; };
// assumes &node-to-remove and &node-to-replace are labels

> 
> Here the keyword essentially becomes a flag to drop the previous
> definition of the node with the option to also provide a replacement
> defintion.  I like this better, and the (lack of) ordering assumptions
> are preserved.
> 
> How then would it look when compared with removing properties?
> 
> &some-node {
> 	property-to-remove = /remove/;
> 	property-to-replace = "new value";
> 	property-to-replace-with-empty-prop;
> 
> 	node-to-remove /remove/;
> 	node-to-replace /remove/ { prop = "blah"; };
> };

> In this form, the only syntactic difference between removing
> properties and removing nodes is that removing properties has an '='
> token between them.  I think that this has the potential to be
> confusing to users, and probably a common source of defects.

I agree with this assessment.

> That is
> part of why I like the idea of different keywords when dealing with
> nodes and properties.  What about this?
> 
> &some-node {
> 	property-to-remove /remove-prop/;
> 	property-to-replace = "new value";
> 	property-to-replace-with-empty-prop;
> 
> 	node-to-remove /remove-node/;
> 	node-to-replace /remove-node/ { prop = "blah"; };
> };
> 
> Or perhaps with the keyword preceding:
> 
> &some-node {
> 	/remove-prop/ property-to-remove;
> 	property-to-replace = "new value";
> 	property-to-replace-with-empty-prop;
> 
> 	/remove-node/ node-to-remove;
> 	/remove-node/ node-to-replace { prop = "blah"; };
> };
> 
> And the following construct would be forbidden:
> 
> 	/remove-prop/ property-to-remove = "new value;

Do we need to be able to set a property with an empty value?

I'm wondering if we have /remove-node/ and /remove-property/ if it
will lead people to make errors by accidentally using the wrong
syntax for the node or property.

If we're not going to use
	property-to-remove = /undefined/;

then I'd prefer if we used /remove/ for both nodes and properties.

I'm not sure about having the keyword in front for properties and
afterwards for nodes. I think we want the syntactic rules to be very
simple and easy to remember.

> 
> 
> Also, /remove-node/ feels awkward when working with replacing a node.
> Would /clear-node/ or /trim-node/ be better?  '/replace-node/'
> wouldn't be very because it doesn't make sense in the removal case,
> and /delete-node/ has the same problem as /remove-node/.
> /undefine-node/ as you suggested has possibilities too.  Anyone have
> other suggestions for intuitive keyword names?
> 
> > > I also wonder if it would be better to have the directives before the
> > > node name.  ie:
> > > 	/extend/ node1 { ... };
> > > 	/replace/ node1 { ... };
> > > 	/remove/ node1;
> > 
> > Maybe, yes, although I think the first two should go away anyway.
> > 
> > Actually, how about:
> > 	node1 /undefine/;
> > 
> > Using the same /undefine/ keyword as for removing individual
> > properties (but without the =).
> 
> See discussion above.  :-)

I think having the keywords in front work fine, especially if we're
consistent about it.

It could be like this:

For nodes:
/extend/ &node-to-extend { ... };
/replace/ &node-to-replace { ... };
/remove/ &node-to-remove;

For properties:
/extend/ &node-to-extend {
	/remove/ property-to-remove;
};



> 
> > 
> > [snip]
> > > > 4) Allow full paths to be used to override nodes.
> > > > 
> > > > 	This suggest that labels aren't required. Most people will
> > > > 	want to use labels, but for the device tree syntax to be
> > > > 	orthogonal, full paths should still be allowed.
> > > > 
> > > > 	Example:
> > > > 		Here is the serial example again. Not inlined with paths:
> > > > 
> > > > 		/ {
> > > > 			soc: soc5200@f0000000 {
> > > > 				#address-cells = <1>;
> > > > 				#size-cells = <1>;
> > > > 				compatible = "fsl,mpc5200b-immr";
> > > > 				...
> > > > 
> > > > 				psc4: serial@2600 {		// PSC4
> > > > 					compatible = "fsl,mpc5200b-psc-uart","fsl,mpc5200-psc-uart";
> > > > 					reg = <0x2600 0x100>;
> > > > 					interrupts = <2 11 0>;
> > > > 				};
> > > > 
> > > > 				psc5: serial@2800 {		// PSC5
> > > > 					compatible = "fsl,mpc5200b-psc-uart","fsl,mpc5200-psc-uart";
> > > > 					reg = <0x2800 0x100>;
> > > > 					interrupts = <2 12 0>;
> > > > 				};
> > > > 		};
> > > > 
> > > > 		/soc5200@f0000000/serial@2600 /remove/ { };		// PSC4
> > > > 		/soc5200@f0000000/serial@2800 /remove/ { };		// PSC5
> > > 
> > > I've thought about this too, and I'd like to implement it, but there
> > > are some nasty gotchas here because /.../ is already used as the
> > > keyword delimiter.  There would need to be a way to ensure the grammer
> > > is unambiguous about which are keywords and which are node names.
> > 
> > Yeah.  But I think there's an obvious way to fix it.  We already have
> > a full-path reference syntax we can use for inserting phandles.  So I
> > think we should make the syntax here:
> > 
> > &{/full/path/to/node} {
> > 	property-to-override = "something";
> > };
> 
> True.  That would work.  And I like the reuse of existing syntax
> elements.
> 
> > 
> > Or of course you can already do
> > 
> > / {
> > 	full {
> > 		path {
> > 			to {
> > 				node {
> > 					etc.
> 
> Right.
> 
> > > On that line, it might also be possible to specify a node by both
> > > label and path:
> > 
> > Well, this is sort of already possible:
> > 
> > &soc {
> > 	serial@2600 {
> > 		current-speed = <115200>;
> > 	};
> > };
> > 
> > Directly allowing a label+relative path could be useful enough to
> > reduce verbosity, but it will require careful thinking about the
> > syntax.
> 
> Yes, (as discussed previously) this is just syntactic sugar.  The
> syntax already supports the functionality, but it would be nice to be
> less verbose.
> 
> g.
> 

Re: Proposal: new device-tree syntax and semantics for extending information from included dts files

[Grant Likely]

On Wed, Oct 13, 2010 at 08:08:12PM -0700, John Bonesio wrote:
> On Wed, 2010-10-13 at 19:40 -0600, Grant Likely wrote:
> > On Thu, Oct 14, 2010 at 11:38:33AM +1100, David Gibson wrote:
> > > On Wed, Oct 13, 2010 at 05:17:47PM -0600, Grant Likely wrote:
> > > > On Tue, Oct 12, 2010 at 02:56:01PM -0700, John Bonesio wrote:
> The dts syntax is meant for humans to read, otherwise, we'd just encode
> the bits in the dtb file by hand. The new syntax and semantics doesn't
> have to be reduced to provide only one way to do each thing. I believe
> we should optimize for human readability.
> 
> My vote is to go ahead with the 3 methods of modifying existing nodes
> which is /extend/, /replace/, and /remove/. Use of the /extend/ syntax,
> while optional as this is the default, should still be allowed in the
> case where most of the updates are using the other methods and for
> clarity, the writer wants to make /extend/ explicit.
> 
> So the syntax would be:
> &some-node {
> 	node-to-remove /remove/;
> 	node-to-replace /replace/ { prop = "blah"; };
> }
> 
> or even
> 
> &node-to-remove /remove/;
> &node-to-replace /replace/ { prop = "blah"; };
> // assumes &node-to-remove and &node-to-replace are labels

I suppose by making /replace/ and /extend/ explicit would provide a
mechanism for dtc to throw a warning/error if the node it is supposed
to be replacing doesn't exist.  I'm not yet convinced it is a necessary
use case though.

> > &some-node {
> > 	property-to-remove /remove-prop/;
> > 	property-to-replace = "new value";
> > 	property-to-replace-with-empty-prop;
> > 
> > 	node-to-remove /remove-node/;
> > 	node-to-replace /remove-node/ { prop = "blah"; };
> > };
> > 
> > Or perhaps with the keyword preceding:
> > 
> > &some-node {
> > 	/remove-prop/ property-to-remove;
> > 	property-to-replace = "new value";
> > 	property-to-replace-with-empty-prop;
> > 
> > 	/remove-node/ node-to-remove;
> > 	/remove-node/ node-to-replace { prop = "blah"; };
> > };
> > 
> > And the following construct would be forbidden:
> > 
> > 	/remove-prop/ property-to-remove = "new value;
> 
> Do we need to be able to set a property with an empty value?

Yes.  They are used all the time.  Best example I can think of in this
case is if a board wanted to replace a detailed 'ranges' property with
an empty one to enable full address range translation.

> I'm wondering if we have /remove-node/ and /remove-property/ if it
> will lead people to make errors by accidentally using the wrong
> syntax for the node or property.

The problem is that it is legal (though rare) for a child node to have
the same name as a property in a single node.  There needs to be a
difference in the grammer to show which type the operation is intended
to be performed on.  I like encoding 'node' and 'property' into the
keyword because then it is blatantly obvious what is intended.  If a
user uses /remove-node/ on a property, or /remove-property/ on a node,
then dtc won't find the target and it can righteously pitch a fit.  :-)

Also, if someone does use the wrong keyword, anyone familiar with the
english language can probably guess that /remove-property/ won't work
on nodes and visa versa.

> 
> If we're not going to use
> 	property-to-remove = /undefined/;
> 
> then I'd prefer if we used /remove/ for both nodes and properties.
> 
> I'm not sure about having the keyword in front for properties and
> afterwards for nodes. I think we want the syntactic rules to be very
> simple and easy to remember.

Yeah, I'm not sure which is best either.  I need to mull over it some
more.  I agree that the rules need to be simple and easy to remember.

g.

Re: Proposal: new device-tree syntax and semantics for extending information from included dts files

[David Gibson]

On Wed, Oct 13, 2010 at 08:08:12PM -0700, John Bonesio wrote:
> On Wed, 2010-10-13 at 19:40 -0600, Grant Likely wrote:
> > On Thu, Oct 14, 2010 at 11:38:33AM +1100, David Gibson wrote:
> > > On Wed, Oct 13, 2010 at 05:17:47PM -0600, Grant Likely wrote:
> > > > On Tue, Oct 12, 2010 at 02:56:01PM -0700, John Bonesio wrote:
[snip]
> > > > > 	B) /replace/
> > > > 
> > > > /replace-node/ perhaps?
> > > 
> > > There is no need for this.  It's equivalent to a /remove/ then an
> > > /extend/, which is barely more verbose.
> > 
> > Need to flush this out.  What would this look like then?  This?
> > 
> > &some-node {
> > 	node-to-remove /remove/;
> > 
> > 	node-to-replace /remove/;
> > 	node-to-replace { prop = "blah"; };
> > };
> > 
> > I don't like this one because it implies ordering between the first
> > and second 'node-to-replace' definitions, and to this point the syntax
> > has not imposed any order assumptions.
> > 
> > Perhaps this:
> > 
> > &some-node {
> > 	node-to-remove /remove/;
> > 	node-to-replace /remove/ { prop = "blah"; };
> > };
> 
> I think this is going to be really confusing. A person looking at this
> for the first time will be wondering, "Why are we removing property,
> 'prop'. I thought we needed that one?"
> 
> I believe that if we want to be able to replace nodes, we should stick
> with the /replace/ syntax.
> 
> The dts syntax is meant for humans to read, otherwise, we'd just encode
> the bits in the dtb file by hand. The new syntax and semantics doesn't

Well, it's meant for programmers (specifically, C programmers) to read..

> have to be reduced to provide only one way to do each thing. I believe
> we should optimize for human readability.

It's because of readability that I want to get rid of duplicate syntax
with identical semantics.  That leads readers to go "what's the
difference" when there is none.

> My vote is to go ahead with the 3 methods of modifying existing nodes
> which is /extend/, /replace/, and /remove/. Use of the /extend/ syntax,
> while optional as this is the default, should still be allowed in the
> case where most of the updates are using the other methods and for
> clarity, the writer wants to make /extend/ explicit.

Nack.  Any new syntax we're stuck with more-or-less forever.  I know
you and others want these features, but adding new syntax without more
than half a day's attempt to reach consensus is not wise.

[snip]
> > &some-node {
> > 	/remove-prop/ property-to-remove;
> > 	property-to-replace = "new value";
> > 	property-to-replace-with-empty-prop;
> > 
> > 	/remove-node/ node-to-remove;
> > 	/remove-node/ node-to-replace { prop = "blah"; };
> > };
> > 
> > And the following construct would be forbidden:
> > 
> > 	/remove-prop/ property-to-remove = "new value;
> 
> Do we need to be able to set a property with an empty value?

Yes, absolutely.  Empty valued properties (which are distinct from
non-existent properties) are used routinely in real trees.

> I'm wondering if we have /remove-node/ and /remove-property/ if it
> will lead people to make errors by accidentally using the wrong
> syntax for the node or property.

You mean this?
	/remove-node/ some-property;

Not that likely an error, IMO.  And more to the point it will just
generate a syntax error for which we can easily provide an error
message pointing directly at the mistake.  It's not mistakes which
lead the compiler to reject the input which are scary, it's the ones
which will silently produce a valid tree that's not the one you
expected.

> If we're not going to use
> 	property-to-remove = /undefined/;
> 
> then I'd prefer if we used /remove/ for both nodes and properties.
> 
> I'm not sure about having the keyword in front for properties and
> afterwards for nodes. I think we want the syntactic rules to be very
> simple and easy to remember.

Um.. I don't think that was ever suggested.  Although if using an
explicit preceding /remove/ for nodes and '= /undefined/' for props
that's kind of the same thing but the other way around.

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

Re: Proposal: new device-tree syntax and semantics for extending information from included dts files

[David Gibson]

On Wed, Oct 13, 2010 at 07:40:36PM -0600, Grant Likely wrote:
> On Thu, Oct 14, 2010 at 11:38:33AM +1100, David Gibson wrote:
> > On Wed, Oct 13, 2010 at 05:17:47PM -0600, Grant Likely wrote:
> > > On Tue, Oct 12, 2010 at 02:56:01PM -0700, John Bonesio wrote:
[snip]
> > > > Proposal:
> > > > 
> > > > 1) Add a new keyword constant for property values, <Undefined>.
> > > 
> > > Personally, I'd prefer all lowercase: <undefined>.  There may be some
> > > dickering over the exact keyword name, but I think the concept is
> > > sound.
> > 
> > It should be /undefined/ to match existing keywords.  Or possibly
> > /delete/ or /remove-property/ or some such.  As Grant says there is
> > some dickering to be done there.
> 
> ah, of course.  I missed that.  <...> already has a meaning in dts
> syntax, so that should definitely be avoided.  /.../ it is.
> 
> > > > 	B) /replace/
> > > 
> > > /replace-node/ perhaps?
> > 
> > There is no need for this.  It's equivalent to a /remove/ then an
> > /extend/, which is barely more verbose.
> 
> Need to flush this out.  What would this look like then?  This?
> 
> &some-node {
> 	node-to-remove /remove/;
> 
> 	node-to-replace /remove/;
> 	node-to-replace { prop = "blah"; };
> };
> 
> I don't like this one because it implies ordering between the first
> and second 'node-to-replace' definitions, and to this point the syntax
> has not imposed any order assumptions.

Hrm.  Well, more precisely I was thinking of:

&{/some/node} /remove/;

&{/some/node} {
	new-contents;
};

Since I thought the original proposal was that these node /replace/,
/extend/ whatnot were only valid at the top level (for the reasons
we've discussed on previous occasions).  We do already have ordering
amongst the top level items, since the last one wins.  Now, if using
labels rather than full paths it does get a bit curlier on the
implementation side.  But I don't think it would be that hard to make
labels stick around attached to a path, even when the path itself is
removed.

> Perhaps this:
> 
> &some-node {
> 	node-to-remove /remove/;
> 	node-to-replace /remove/ { prop = "blah"; };
> };
> 
> Here the keyword essentially becomes a flag to drop the previous
> definition of the node with the option to also provide a replacement
> defintion.  I like this better, and the (lack of) ordering assumptions
> are preserved.

Urg.  I think that is confusing for much the same reasons John Bonesio
states in his later mail.  But, I can see a case for:

	node-to-replace /replace/ { prop = "blah"; };

and allowing removal with:

	node-to-replace /replace/ /undefined/;

So instead of substituting "remove+extend" for replace, we substitute
"replace with nothing" for remove.  Either way we remove one syntactic
element.  Have to think more about which I prefer.

> How then would it look when compared with removing properties?
> 
> &some-node {
> 	property-to-remove = /remove/;
> 	property-to-replace = "new value";
> 	property-to-replace-with-empty-prop;
> 
> 	node-to-remove /remove/;
> 	node-to-replace /remove/ { prop = "blah"; };
> };
> 
> In this form, the only syntactic difference between removing
> properties and removing nodes is that removing properties has an '='
> token between them.  I think that this has the potential to be
> confusing to users, and probably a common source of defects.  That is
> part of why I like the idea of different keywords when dealing with
> nodes and properties.  What about this?
> 
> &some-node {
> 	property-to-remove /remove-prop/;
> 	property-to-replace = "new value";
> 	property-to-replace-with-empty-prop;
> 
> 	node-to-remove /remove-node/;
> 	node-to-replace /remove-node/ { prop = "blah"; };
> };
> 
> Or perhaps with the keyword preceding:
> 
> &some-node {
> 	/remove-prop/ property-to-remove;
> 	property-to-replace = "new value";
> 	property-to-replace-with-empty-prop;
> 
> 	/remove-node/ node-to-remove;
> 	/remove-node/ node-to-replace { prop = "blah"; };
> };


I prefer this last example to the two previous ones.

> And the following construct would be forbidden:
> 
> 	/remove-prop/ property-to-remove = "new value;
> 
> 
> Also, /remove-node/ feels awkward when working with replacing a node.
> Would /clear-node/ or /trim-node/ be better?  '/replace-node/'

/clear-node/ suggests to me that it would leave the node in place,
just removing subnodes and properties.  /trim-node/ might work.

> wouldn't be very because it doesn't make sense in the removal case,
> and /delete-node/ has the same problem as /remove-node/.
> /undefine-node/ as you suggested has possibilities too.  Anyone have
> other suggestions for intuitive keyword names?

Well.. the "try not to surprise C programmers" rule of thumb has
served us well so far for dtc, so /undef-node/ has some merit.  In
principle I'd kind of like to use just preceding /undef/ for both, but
that will cause some parsing problems.

[snip]
> > > I've thought about this too, and I'd like to implement it, but there
> > > are some nasty gotchas here because /.../ is already used as the
> > > keyword delimiter.  There would need to be a way to ensure the grammer
> > > is unambiguous about which are keywords and which are node names.
> > 
> > Yeah.  But I think there's an obvious way to fix it.  We already have
> > a full-path reference syntax we can use for inserting phandles.  So I
> > think we should make the syntax here:
> > 
> > &{/full/path/to/node} {
> > 	property-to-override = "something";
> > };
> 
> True.  That would work.  And I like the reuse of existing syntax
> elements.
> 
> > 
> > Or of course you can already do
> > 
> > / {
> > 	full {
> > 		path {
> > 			to {
> > 				node {
> > 					etc.
> 
> Right.
> 
> > > On that line, it might also be possible to specify a node by both
> > > label and path:
> > 
> > Well, this is sort of already possible:
> > 
> > &soc {
> > 	serial@2600 {
> > 		current-speed = <115200>;
> > 	};
> > };
> > 
> > Directly allowing a label+relative path could be useful enough to
> > reduce verbosity, but it will require careful thinking about the
> > syntax.
> 
> Yes, (as discussed previously) this is just syntactic sugar.  The
> syntax already supports the functionality, but it would be nice to be
> less verbose.

Right, but I think that can wait until after we've decided and
implemented on the core functionality.

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

Re: Proposal: new device-tree syntax and semantics for extending information from included dts files

[Grant Likely]

On Thu, Oct 14, 2010 at 02:48:21PM +1100, David Gibson wrote:
> On Wed, Oct 13, 2010 at 07:40:36PM -0600, Grant Likely wrote:
> > &some-node {
> > 	node-to-remove /remove/;
> > 
> > 	node-to-replace /remove/;
> > 	node-to-replace { prop = "blah"; };
> > };
> > 
> > I don't like this one because it implies ordering between the first
> > and second 'node-to-replace' definitions, and to this point the syntax
> > has not imposed any order assumptions.
> 
> Hrm.  Well, more precisely I was thinking of:
> 
> &{/some/node} /remove/;
> 
> &{/some/node} {
> 	new-contents;
> };
> 
> Since I thought the original proposal was that these node /replace/,
> /extend/ whatnot were only valid at the top level (for the reasons
> we've discussed on previous occasions).

Hmmm, I thought we had figured out how to solve those issues by using
a flag to indicate that a node or property "masks out" the same node
or property in a preceding top-level tree.

> We do already have ordering
> amongst the top level items, since the last one wins.

Right, but the ordering has been explicitly restricted to the top
level trees using the "stack of overlays" conceptual model.

> Now, if using
> labels rather than full paths it does get a bit curlier on the
> implementation side.  But I don't think it would be that hard to make
> labels stick around attached to a path, even when the path itself is
> removed.

Ah right, the issue here was that a previous tree could be holding a
label reference that gets deleted in an overlay.  And there are
questions about what happens if a label is removed and then the same
label attached to a new node, specifically because labels aren't
resolved until after the trees are fully parsed.  (let me know if I'm
remembering incorrectly).  I think this is solvable though, but we're
going to need to define the semantics.

I need to review the details though.  I can't remember all the ways
that the code handles label resolution.

> 
> > Perhaps this:
> > 
> > &some-node {
> > 	node-to-remove /remove/;
> > 	node-to-replace /remove/ { prop = "blah"; };
> > };
> > 
> > Here the keyword essentially becomes a flag to drop the previous
> > definition of the node with the option to also provide a replacement
> > defintion.  I like this better, and the (lack of) ordering assumptions
> > are preserved.
> 
> Urg.  I think that is confusing for much the same reasons John Bonesio
> states in his later mail.  But, I can see a case for:
> 
> 	node-to-replace /replace/ { prop = "blah"; };
> 
> and allowing removal with:
> 
> 	node-to-replace /replace/ /undefined/;
> 
> So instead of substituting "remove+extend" for replace, we substitute
> "replace with nothing" for remove.  Either way we remove one syntactic
> element.  Have to think more about which I prefer.

Hmmm, this has possibilities.

> > Or perhaps with the keyword preceding:
> > 
> > &some-node {
> > 	/remove-prop/ property-to-remove;
> > 	property-to-replace = "new value";
> > 	property-to-replace-with-empty-prop;
> > 
> > 	/remove-node/ node-to-remove;
> > 	/remove-node/ node-to-replace { prop = "blah"; };
> > };
> 
> 
> I prefer this last example to the two previous ones.

Combining your last suggestion with this would give:

&some-node {
	/remove-prop/ property-to-remove;
	property-to-replace = "new value";
	property-to-replace-with-empty-prop;

	/replace-node/ node-to-remove /undefined/;
	/replace-node/ node-to-replace { prop = "blah"; };
};

I'm not sure.  It feels a little muddy.  It's late though.  I'm going
to go sleep on it, and take a fresh look in the morning.

> 
> > And the following construct would be forbidden:
> > 
> > 	/remove-prop/ property-to-remove = "new value;
> > 
> > 
> > Also, /remove-node/ feels awkward when working with replacing a node.
> > Would /clear-node/ or /trim-node/ be better?  '/replace-node/'
> 
> /clear-node/ suggests to me that it would leave the node in place,
> just removing subnodes and properties.  /trim-node/ might work.
> 
> > wouldn't be very because it doesn't make sense in the removal case,
> > and /delete-node/ has the same problem as /remove-node/.
> > /undefine-node/ as you suggested has possibilities too.  Anyone have
> > other suggestions for intuitive keyword names?
> 
> Well.. the "try not to surprise C programmers" rule of thumb has
> served us well so far for dtc, so /undef-node/ has some merit.  In
> principle I'd kind of like to use just preceding /undef/ for both, but
> that will cause some parsing problems.

Good point.  And yes, using the same for both it tricky.

> > > Directly allowing a label+relative path could be useful enough to
> > > reduce verbosity, but it will require careful thinking about the
> > > syntax.
> > 
> > Yes, (as discussed previously) this is just syntactic sugar.  The
> > syntax already supports the functionality, but it would be nice to be
> > less verbose.
> 
> Right, but I think that can wait until after we've decided and
> implemented on the core functionality.

Yes.

Re: Proposal: new device-tree syntax and semantics for extending information from included dts files

[David Gibson]

On Wed, Oct 13, 2010 at 10:54:13PM -0600, Grant Likely wrote:
> On Thu, Oct 14, 2010 at 02:48:21PM +1100, David Gibson wrote:
> > On Wed, Oct 13, 2010 at 07:40:36PM -0600, Grant Likely wrote:
> > > &some-node {
> > > 	node-to-remove /remove/;
> > > 
> > > 	node-to-replace /remove/;
> > > 	node-to-replace { prop = "blah"; };
> > > };
> > > 
> > > I don't like this one because it implies ordering between the first
> > > and second 'node-to-replace' definitions, and to this point the syntax
> > > has not imposed any order assumptions.
> > 
> > Hrm.  Well, more precisely I was thinking of:
> > 
> > &{/some/node} /remove/;
> > 
> > &{/some/node} {
> > 	new-contents;
> > };
> > 
> > Since I thought the original proposal was that these node /replace/,
> > /extend/ whatnot were only valid at the top level (for the reasons
> > we've discussed on previous occasions).
> 
> Hmmm, I thought we had figured out how to solve those issues by using
> a flag to indicate that a node or property "masks out" the same node
> or property in a preceding top-level tree.

Um.. I don't follow.

> > We do already have ordering
> > amongst the top level items, since the last one wins.
> 
> Right, but the ordering has been explicitly restricted to the top
> level trees using the "stack of overlays" conceptual model.

Right, my point exactly.  I'm suggesting we keep it restricted to the
top level by only allowing these replace/remove operators (whatever
they look like) at the top level.

> > Now, if using
> > labels rather than full paths it does get a bit curlier on the
> > implementation side.  But I don't think it would be that hard to make
> > labels stick around attached to a path, even when the path itself is
> > removed.
> 
> Ah right, the issue here was that a previous tree could be holding a
> label reference that gets deleted in an overlay.  And there are
> questions about what happens if a label is removed and then the same
> label attached to a new node, specifically because labels aren't
> resolved until after the trees are fully parsed.  (let me know if I'm
> remembering incorrectly).  I think this is solvable though, but we're
> going to need to define the semantics.
> 
> I need to review the details though.  I can't remember all the ways
> that the code handles label resolution.

Right.  Having labels move around seriously muddies the waters, since
the whole idea of labels is to unambiguously refer to a node.  I think
the sanest thing is to prohibit duplicate labels, even if the labelled
nodes have been removed between the duplicate declarations.  In the
later stages of processing, then, referring to a deleted label would
be detectable and trigger an error at that point.

[snip]
> > > Or perhaps with the keyword preceding:
> > > 
> > > &some-node {
> > > 	/remove-prop/ property-to-remove;
> > > 	property-to-replace = "new value";
> > > 	property-to-replace-with-empty-prop;
> > > 
> > > 	/remove-node/ node-to-remove;
> > > 	/remove-node/ node-to-replace { prop = "blah"; };
> > > };
> > 
> > 
> > I prefer this last example to the two previous ones.
> 
> Combining your last suggestion with this would give:

Uh, sorry, not all of my suggestions were intended to be used in
conjuction.  Some were alternate possibilities which do not mix well.

> &some-node {
> 	/remove-prop/ property-to-remove;

For example if we're doing node removal as a "replace with nothing",
then we'd certainly do the same for properties, i.e. "prop =
/undefine/;" not a /remove-prop/ keyword.

> 	property-to-replace = "new value";
> 	property-to-replace-with-empty-prop;
> 
> 	/replace-node/ node-to-remove /undefined/;
> 	/replace-node/ node-to-replace { prop = "blah"; };
> };
> 
> I'm not sure.  It feels a little muddy.  It's late though.  I'm going
> to go sleep on it, and take a fresh look in the morning.

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

Re: Proposal: new device-tree syntax and semantics for extending information from included dts files

[Grant Likely]

On Sat, Oct 16, 2010 at 1:02 AM, David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> On Wed, Oct 13, 2010 at 10:54:13PM -0600, Grant Likely wrote:
>> On Thu, Oct 14, 2010 at 02:48:21PM +1100, David Gibson wrote:
>> > On Wed, Oct 13, 2010 at 07:40:36PM -0600, Grant Likely wrote:
>> > > &some-node {
>> > >   node-to-remove /remove/;
>> > >
>> > >   node-to-replace /remove/;
>> > >   node-to-replace { prop = "blah"; };
>> > > };
>> > >
>> > > I don't like this one because it implies ordering between the first
>> > > and second 'node-to-replace' definitions, and to this point the syntax
>> > > has not imposed any order assumptions.
>> >
>> > Hrm.  Well, more precisely I was thinking of:
>> >
>> > &{/some/node} /remove/;
>> >
>> > &{/some/node} {
>> >     new-contents;
>> > };
>> >
>> > Since I thought the original proposal was that these node /replace/,
>> > /extend/ whatnot were only valid at the top level (for the reasons
>> > we've discussed on previous occasions).
>>
>> Hmmm, I thought we had figured out how to solve those issues by using
>> a flag to indicate that a node or property "masks out" the same node
>> or property in a preceding top-level tree.
>
> Um.. I don't follow.

Sorry, I was thinking about a different aspect of the problem and I
didn't make that clear.

>
>> > We do already have ordering
>> > amongst the top level items, since the last one wins.
>>
>> Right, but the ordering has been explicitly restricted to the top
>> level trees using the "stack of overlays" conceptual model.
>
> Right, my point exactly.  I'm suggesting we keep it restricted to the
> top level by only allowing these replace/remove operators (whatever
> they look like) at the top level.

Sounds like we're having an impedance mismatch.  Let me try to
clarify my position.  Ordering is still restricted to the top level
what I'm suggesting.  For example, the following would be legal:

	/ {
		node {
			prop;
		};
	};

	/{
		/trim-node/ node;
	};

But this would be illegal (enforced by dtc):

	/{
		node {
			prop;
		};
		/trim-node/ node;
	};

And this would also be illegal (also enforced):

	/ {
		node {
			prop;
		};
	};

	/{
		/trim-node/ node;
		node {
			newprop;
		};
	};

The model I'm suggesting is that each top level tree is fully
constructed before being merged into the preceding tree.  Additional
nodes and properties are easy to understand and it works already.
I'm suggesting that the above illegal examples are exactly the same as
the following illegal example:

	/ {
		node {
			prop;
		};
		node {
			another-prop;
		};
	};

It is illegal because the node is defined twice in the same block.  If
it were allowed, then it would raise the question of which node gets
processed first, and we have explicitly decided not to support that
model.

The removal or replacement of nodes is exactly the same.  However,
instead of adding to a node, we set a flag in the new node stating
that it replaces the node in the preceding tree.  Or in the case of
removal, we set a flag that states the node in the preceding tree is
to be removed without a replacement.

In terms of implementation this should be straight forward.  In the
merge_nodes function, check the new node for a replace/remove flag,
and if it is set then remove/replace the old node instead of merging
with the new.  This can probably be performed in the
while(new_node->children) loop.

This is what I was talking about when I mentioned using a flag
earlier.  The /*-node/ keyword could set a flag in the node structure
so that merge_nodes handles the operation correctly when merging the
trees.

>
>> > Now, if using
>> > labels rather than full paths it does get a bit curlier on the
>> > implementation side.  But I don't think it would be that hard to make
>> > labels stick around attached to a path, even when the path itself is
>> > removed.
>>
>> Ah right, the issue here was that a previous tree could be holding a
>> label reference that gets deleted in an overlay.  And there are
>> questions about what happens if a label is removed and then the same
>> label attached to a new node, specifically because labels aren't
>> resolved until after the trees are fully parsed.  (let me know if I'm
>> remembering incorrectly).  I think this is solvable though, but we're
>> going to need to define the semantics.
>>
>> I need to review the details though.  I can't remember all the ways
>> that the code handles label resolution.
>
> Right.  Having labels move around seriously muddies the waters, since
> the whole idea of labels is to unambiguously refer to a node.  I think
> the sanest thing is to prohibit duplicate labels, even if the labelled
> nodes have been removed between the duplicate declarations.  In the
> later stages of processing, then, referring to a deleted label would
> be detectable and trigger an error at that point.

Fair enough.  I can agree to that constraint.  It can be revisited
again if it is too restrictive, and it is easier to relax
restrictions later than it is to add new restrictions at a later date.

g.

Re: Proposal: new device-tree syntax and semantics for extending information from included dts files

[John Bonesio]

On Sat, 2010-10-16 at 12:57 -0600, Grant Likely wrote:
> On Sat, Oct 16, 2010 at 1:02 AM, David Gibson
> <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > On Wed, Oct 13, 2010 at 10:54:13PM -0600, Grant Likely wrote:
> >> On Thu, Oct 14, 2010 at 02:48:21PM +1100, David Gibson wrote:
> >> > On Wed, Oct 13, 2010 at 07:40:36PM -0600, Grant Likely wrote:
> >> > > &some-node {
> >> > >   node-to-remove /remove/;
> >> > >
> >> > >   node-to-replace /remove/;
> >> > >   node-to-replace { prop = "blah"; };
> >> > > };
> >> > >
> >> > > I don't like this one because it implies ordering between the first
> >> > > and second 'node-to-replace' definitions, and to this point the syntax
> >> > > has not imposed any order assumptions.
> >> >
> >> > Hrm.  Well, more precisely I was thinking of:
> >> >
> >> > &{/some/node} /remove/;
> >> >
> >> > &{/some/node} {
> >> >     new-contents;
> >> > };
> >> >
> >> > Since I thought the original proposal was that these node /replace/,
> >> > /extend/ whatnot were only valid at the top level (for the reasons
> >> > we've discussed on previous occasions).
> >>
> >> Hmmm, I thought we had figured out how to solve those issues by using
> >> a flag to indicate that a node or property "masks out" the same node
> >> or property in a preceding top-level tree.
> >
> > Um.. I don't follow.
> 
> Sorry, I was thinking about a different aspect of the problem and I
> didn't make that clear.
> 
> >
> >> > We do already have ordering
> >> > amongst the top level items, since the last one wins.
> >>
> >> Right, but the ordering has been explicitly restricted to the top
> >> level trees using the "stack of overlays" conceptual model.
> >
> > Right, my point exactly.  I'm suggesting we keep it restricted to the
> > top level by only allowing these replace/remove operators (whatever
> > they look like) at the top level.
> 
> Sounds like we're having an impedance mismatch.  Let me try to
> clarify my position.  Ordering is still restricted to the top level
> what I'm suggesting.  For example, the following would be legal:
> 
> 	/ {
> 		node {
> 			prop;
> 		};
> 	};
> 
> 	/{
> 		/trim-node/ node;
> 	};
> 
> But this would be illegal (enforced by dtc):
> 
> 	/{
> 		node {
> 			prop;
> 		};
> 		/trim-node/ node;
> 	};
> 
> And this would also be illegal (also enforced):
> 
> 	/ {
> 		node {
> 			prop;
> 		};
> 	};
> 
> 	/{
> 		/trim-node/ node;
> 		node {
> 			newprop;
> 		};
> 	};
> 
> The model I'm suggesting is that each top level tree is fully
> constructed before being merged into the preceding tree.  Additional
> nodes and properties are easy to understand and it works already.
> I'm suggesting that the above illegal examples are exactly the same as
> the following illegal example:
> 
> 	/ {
> 		node {
> 			prop;
> 		};
> 		node {
> 			another-prop;
> 		};
> 	};
> 
> It is illegal because the node is defined twice in the same block.  If
> it were allowed, then it would raise the question of which node gets
> processed first, and we have explicitly decided not to support that
> model.

I think it is natural for dtc to process nodes in the order they are
seen. I think it is also natural for people to assume that the things at
the top are processed first. I don't think there would be any confusion
at all, especially for "C" programmers. If there is ever a question, we
can just say, "they're processed top to bottom."

Probably I just don't understand some of the thinking behind why there
is a problem forcing an ordering for dtc to process the nodes.

> The removal or replacement of nodes is exactly the same.  However,
> instead of adding to a node, we set a flag in the new node stating
> that it replaces the node in the preceding tree.  Or in the case of
> removal, we set a flag that states the node in the preceding tree is
> to be removed without a replacement.
> 
> In terms of implementation this should be straight forward.  In the
> merge_nodes function, check the new node for a replace/remove flag,
> and if it is set then remove/replace the old node instead of merging
> with the new.  This can probably be performed in the
> while(new_node->children) loop.

This doesn't look straight forward to me at all. From what I can tell,
the dtc is pretty much a 2 pass compilation system. It parses and builds
up an in memory tree structure, then it runs through and generates the
bits for the dtb file.

Right now it merges trees during the parsing phase (not just my
implementation, but also with what was already implemented). So, if it
sees
	/trim-node/ node;
It's going to be looking for the node it will replace (or remove). At
this point it's not going to know if 'node' was defined in this instance
of the tree or if node is in a previous tree. Internally it just keeps
in memory the current merged tree it has processed so far.

It could be I'm misunderstanding the design of dtc, but enforcing this
looks to me like it will require:
1) building a data structure that holds the multiple trees unchanged
(changing the existing implementation as well as my additions).
2) creating another pass over the data structure to merge the parts that
need to be merged.

I can understand that we may want to prevent nodes from being defined
twice in a tree because that is likely an unintended error on the dts
writer's part.

Perhaps it would be better to require explicit extension of nodes. This
would be a change to the current implementation where two trees are
automatically merged. This idea would be to require one of the
keywords, /extend-node/, /trim-node/, /replace-node/ (or whatever we
decide these to be) when an existing node is modified. Otherwise, it
would be assumed there are duplicate nodes, and dtc will give an error.

So:

	/ {
		node {
			prop;
		};
		node {
			another-prop;
		};
	};

would be an error. But

	/ { 
		node {
			prop;
		};
		/extend-node/ node {
			another-prop;
		};
	};

would be allowed.

> 
> This is what I was talking about when I mentioned using a flag
> earlier.  The /*-node/ keyword could set a flag in the node structure
> so that merge_nodes handles the operation correctly when merging the
> trees.

See above - I don't think just setting a flag is going to be sufficient.

> 
> >
> >> > Now, if using
> >> > labels rather than full paths it does get a bit curlier on the
> >> > implementation side.  But I don't think it would be that hard to make
> >> > labels stick around attached to a path, even when the path itself is
> >> > removed.
> >>
> >> Ah right, the issue here was that a previous tree could be holding a
> >> label reference that gets deleted in an overlay.  And there are
> >> questions about what happens if a label is removed and then the same
> >> label attached to a new node, specifically because labels aren't
> >> resolved until after the trees are fully parsed.  (let me know if I'm
> >> remembering incorrectly).  I think this is solvable though, but we're
> >> going to need to define the semantics.
> >>
> >> I need to review the details though.  I can't remember all the ways
> >> that the code handles label resolution.
> >
> > Right.  Having labels move around seriously muddies the waters, since
> > the whole idea of labels is to unambiguously refer to a node.  I think
> > the sanest thing is to prohibit duplicate labels, even if the labelled
> > nodes have been removed between the duplicate declarations.  In the
> > later stages of processing, then, referring to a deleted label would
> > be detectable and trigger an error at that point.
> 
> Fair enough.  I can agree to that constraint.  It can be revisited
> again if it is too restrictive, and it is easier to relax
> restrictions later than it is to add new restrictions at a later date.
> 
> g.

Re: Proposal: new device-tree syntax and semantics for extending information from included dts files

[Grant Likely]

On Sat, Oct 16, 2010 at 12:50:43PM -0700, John Bonesio wrote:
> On Sat, 2010-10-16 at 12:57 -0600, Grant Likely wrote:
> > The removal or replacement of nodes is exactly the same.  However,
> > instead of adding to a node, we set a flag in the new node stating
> > that it replaces the node in the preceding tree.  Or in the case of
> > removal, we set a flag that states the node in the preceding tree is
> > to be removed without a replacement.
> > 
> > In terms of implementation this should be straight forward.  In the
> > merge_nodes function, check the new node for a replace/remove flag,
> > and if it is set then remove/replace the old node instead of merging
> > with the new.  This can probably be performed in the
> > while(new_node->children) loop.
> 
> This doesn't look straight forward to me at all. From what I can tell,
> the dtc is pretty much a 2 pass compilation system. It parses and builds
> up an in memory tree structure, then it runs through and generates the
> bits for the dtb file.
> 
> Right now it merges trees during the parsing phase (not just my
> implementation, but also with what was already implemented). So, if it
> sees
> 	/trim-node/ node;
> It's going to be looking for the node it will replace (or remove). At
> this point it's not going to know if 'node' was defined in this instance
> of the tree or if node is in a previous tree. Internally it just keeps
> in memory the current merged tree it has processed so far.

Yes, but dtc doesn't need to resolve the node immediately.  It can
defer until merge_nodes() time.  I took the code you wrote (which made
this really easy because I didn't have to fiddle with removing and
freeing attached nodes), and adapted it to do exactly this today.
I've attached the patch.

The patch isn't complete, and it is using the suffix keyword syntax
because that was the approach that seemed quickest to implement.  It
also needs more error checking and it doesn't have any test cases; but
those are easy to add.  The patch also includes the test file that I
used when hacking on this.

What's really nice about this approach is that the exact same code
handles both the top level and the subnode usage of the /*-node/
keywords.

g.


>From 1c823cab1d508355af3edae4f6d4e9439ef73eeb Mon Sep 17 00:00:00 2001
From: John Bonesio <bones-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Date: Fri, 15 Oct 2010 18:07:50 -0700
Subject: [PATCH] Implements new features for updating existing device tree nodes.

This is interesting when the /include/ "<filename>" feature is used. This way
we can create base device tree source files for a family of systems and modify
the device tree for a specific system.

The current system allows an existing node to be extended with new properties
and subnodes.

The new features allow an existing node to be replaced completely by the new
properties and subnodes. The new features also allow an existing node to be
deleted.

This patch does not yet provide a way to remove properties.

Signed-off-by: John Bonesio <bones-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Signed-off-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
---
 dtc-lexer.l  |   12 ++++++++
 dtc-parser.y |   12 ++++++++
 dtc.h        |   14 +++++++++
 livetree.c   |   76 +++++++++++++++++++++++++++++++++++++++++++++++++--
 mydts.dts    |   85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 196 insertions(+), 3 deletions(-)
 create mode 100644 mydts.dts

diff --git a/dtc-lexer.l b/dtc-lexer.l
index 081e13a..a24edf5 100644
--- a/dtc-lexer.l
+++ b/dtc-lexer.l
@@ -96,6 +96,18 @@ static int pop_input_file(void);
 			return DT_MEMRESERVE;
 		}
 
+<*>"/trim-node/"	{
+			DPRINT("Keyword: /trim-node/\n");
+			BEGIN_DEFAULT();
+			return DT_REPLACENODE;
+		}
+
+<*>"/remove-node/"	{
+			DPRINT("Keyword: /remove-node/\n");
+			BEGIN_DEFAULT();
+			return DT_REMOVENODE;
+		}
+
 <*>{LABEL}:	{
 			DPRINT("Label: %s\n", yytext);
 			yylval.labelref = xstrdup(yytext);
diff --git a/dtc-parser.y b/dtc-parser.y
index 0aaf8e8..343ac76 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -54,6 +54,8 @@ static unsigned long long eval_literal(const char *s, int base, int bits);
 
 %token DT_V1
 %token DT_MEMRESERVE
+%token DT_REPLACENODE
+%token DT_REMOVENODE
 %token <propnodename> DT_PROPNODENAME
 %token <literal> DT_LITERAL
 %token <cbase> DT_BASE
@@ -147,6 +149,16 @@ nodedef:
 		{
 			$$ = build_node($2, $3);
 		}
+	| DT_REPLACENODE nodedef
+		{
+			$2->merge_op = NODE_MERGE_OP_REPLACE;
+			$$ = $2;
+		}
+	| DT_REMOVENODE ';'
+		{
+			$$ = build_node(NULL, NULL);
+			$$->merge_op = NODE_MERGE_OP_REMOVE;
+		}
 	;
 
 proplist:
diff --git a/dtc.h b/dtc.h
index b36ac5d..56027d4 100644
--- a/dtc.h
+++ b/dtc.h
@@ -139,6 +139,12 @@ struct property {
 	struct label *labels;
 };
 
+enum node_merge_ops {
+	NODE_MERGE_OP_EXTEND = 0,
+	NODE_MERGE_OP_REMOVE,
+	NODE_MERGE_OP_REPLACE,
+};
+
 struct node {
 	char *name;
 	struct property *proplist;
@@ -154,6 +160,8 @@ struct node {
 	int addr_cells, size_cells;
 
 	struct label *labels;
+
+	int merge_op;
 };
 
 #define for_each_label(l0, l) \
@@ -165,6 +173,10 @@ struct node {
 #define for_each_child(n, c)	\
 	for ((c) = (n)->children; (c); (c) = (c)->next_sibling)
 
+#define for_each_child_safe(n, c, cn)	\
+	for ((c) = (n)->children, (cn) = (c) ? (c)->next_sibling : NULL; \
+	     (c); (c) = (cn), (cn) = (cn) ? (cn)->next_sibling : NULL)
+
 void add_label(struct label **labels, char *label);
 
 struct property *build_property(char *name, struct data val);
@@ -175,6 +187,8 @@ struct node *build_node(struct property *proplist, struct node *children);
 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 clear_node(struct node *node);
+void unlink_node(struct node *node);
 
 void add_property(struct node *node, struct property *prop);
 void add_child(struct node *parent, struct node *child);
diff --git a/livetree.c b/livetree.c
index 13c5f10..302827a 100644
--- a/livetree.c
+++ b/livetree.c
@@ -100,12 +100,82 @@ struct node *name_node(struct node *node, char *name)
 	return node;
 }
 
+void clear_node(struct node *node)
+{
+	struct property *prop;
+	struct node *child;
+
+	while (node->proplist) {
+		/* Pop the property off the list */
+		prop = node->proplist;
+		node->proplist = prop->next;
+		prop->next = NULL;
+
+		free(prop);
+	}
+
+	/* Free up the node's children */
+	/* This assumes we have a tree and not a cylic or acylic graph */
+	while (node->children) {
+		/* Pop the child node off the list */
+		child = node->children;
+		node->children = child->next_sibling;
+		child->parent = NULL;
+		child->next_sibling = NULL;
+
+		clear_node(child);
+		free(child);
+	}
+}
+
+void unlink_node(struct node *node)
+{
+	struct node *parent = node->parent;
+	struct node *next_sibling = node->next_sibling;
+	struct node *child;
+
+	assert(parent); /* can't unlink root node */
+
+	node->parent = NULL;
+	node->next_sibling = NULL;
+
+	/* if the node is the first child ... */
+	if (parent->children == node) {
+		parent->children = next_sibling;
+		return;
+	}
+
+	for_each_child(parent, child) {
+		if (child->next_sibling == node) {
+			child->next_sibling = next_sibling;
+			return;
+		}
+	}
+
+	assert(1); /* Woah! We've got an inconsistent tree here */
+}
+
 struct node *merge_nodes(struct node *old_node, struct node *new_node)
 {
 	struct property *new_prop, *old_prop;
-	struct node *new_child, *old_child;
+	struct node *new_child, *old_child, *next_child;
 	struct label *l;
 
+	if (new_node->merge_op == NODE_MERGE_OP_REMOVE) {
+		/* Make sure this doesn't attempt to remove the root */
+		assert(old_node->parent);
+
+		unlink_node(old_node);
+		clear_node(old_node);
+		free(old_node);
+		free(new_node);
+		return NULL;
+	};
+
+	/* remove the contents of the old node */
+	if (new_node->merge_op == NODE_MERGE_OP_REPLACE)
+		clear_node(old_node);
+
 	/* Add new node labels to old node */
 	for_each_label(new_node->labels, l)
 		add_label(&old_node->labels, l->label);
@@ -147,7 +217,7 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
 		new_child->next_sibling = NULL;
 
 		/* Search for a collision.  Merge if there is */
-		for_each_child(old_node, old_child) {
+		for_each_child_safe(old_node, old_child, next_child) {
 			if (streq(old_child->name, new_child->name)) {
 				merge_nodes(old_child, new_child);
 				new_child = NULL;
@@ -155,7 +225,7 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
 			}
 		}
 
-		/* if no collision occured, add child to the old node. */
+		/* if no collision occurred, add child to the old node. */
 		if (new_child)
 			add_child(old_node, new_child);
 	}
diff --git a/mydts.dts b/mydts.dts
new file mode 100644
index 0000000..bfa8829
--- /dev/null
+++ b/mydts.dts
@@ -0,0 +1,85 @@
+/dts-v1/;
+
+/ {
+	node1 {
+		expect-2-nodes--one-trimmed;
+		node1 { prop; node { }; };
+		remove-me { prop; node { }; };
+		node2 { prop; node { }; };
+	};
+	node2 {
+		expect-1-node;
+		node1 { };
+		remove-me { };
+	};
+	node3 {
+		expect-1-node;
+		remove-me { };
+		node1 { };
+	};
+	node4 {
+		expect-0-nodes;
+		remove-me { };
+	};
+	node5 {
+		expect-0-nodes;
+		remove-me { };
+		remove-me2 { };
+	};
+
+	node6 {
+		node1 {
+			expect-2-nodes--one-trimmed;
+			trim1: node1 { prop; node { }; };
+			remove1: remove-me { prop; node { }; };
+			node2 { prop; node { }; };
+		};
+		node2 {
+			expect-1-node;
+			node1 { };
+			remove2: remove-me { };
+		};
+		node3 {
+			expect-1-node;
+			remove3: remove-me { };
+			node1 { };
+		};
+		node4 {
+			expect-0-nodes;
+			remove4: remove-me { };
+		};
+		node5 {
+			expect-0-nodes;
+			remove5: remove-me { };
+			remove6: remove-me2 { };
+		};
+	};
+};
+
+/ {
+	node1 {
+		node1 /trim-node/ { trimmed; };
+		remove-me /remove-node/;
+	};
+	node2 {
+		remove-me /remove-node/;
+	};
+	node3 {
+		remove-me /remove-node/;
+	};
+	node4 {
+		remove-me /remove-node/;
+	};
+	node5 {
+		remove-me /remove-node/;
+		remove-me2 /remove-node/;
+	};
+};
+
+&remove1 /remove-node/;
+&trim1 /trim-node/ { trimmed; };
+&remove2 /remove-node/;
+&remove3 /remove-node/;
+&remove4 /remove-node/;
+&remove5 /remove-node/;
+&remove6 /remove-node/;
-- 
1.7.1

Re: Proposal: new device-tree syntax and semantics for extending information from included dts files

[David Gibson]

On Sat, Oct 16, 2010 at 12:57:21PM -0600, Grant Likely wrote:
> On Sat, Oct 16, 2010 at 1:02 AM, David Gibson
> <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > On Wed, Oct 13, 2010 at 10:54:13PM -0600, Grant Likely wrote:
> >> On Thu, Oct 14, 2010 at 02:48:21PM +1100, David Gibson wrote:
> >> > On Wed, Oct 13, 2010 at 07:40:36PM -0600, Grant Likely wrote:
[snip]
> >> > We do already have ordering
> >> > amongst the top level items, since the last one wins.
> >>
> >> Right, but the ordering has been explicitly restricted to the top
> >> level trees using the "stack of overlays" conceptual model.
> >
> > Right, my point exactly.  I'm suggesting we keep it restricted to the
> > top level by only allowing these replace/remove operators (whatever
> > they look like) at the top level.
> 
> Sounds like we're having an impedance mismatch.  Let me try to

Yes, I noticed that :)

> clarify my position.  Ordering is still restricted to the top level
> what I'm suggesting.  For example, the following would be legal:

[snip]
> The model I'm suggesting is that each top level tree is fully
> constructed before being merged into the preceding tree.  Additional
> nodes and properties are easy to understand and it works already.
> I'm suggesting that the above illegal examples are exactly the same as
> the following illegal example:
> 
> 	/ {
> 		node {
> 			prop;
> 		};
> 		node {
> 			another-prop;
> 		};
> 	};
> 
> It is illegal because the node is defined twice in the same block.  If
> it were allowed, then it would raise the question of which node gets
> processed first, and we have explicitly decided not to support that
> model.
> 
> The removal or replacement of nodes is exactly the same.  However,
> instead of adding to a node, we set a flag in the new node stating
> that it replaces the node in the preceding tree.  Or in the case of
> removal, we set a flag that states the node in the preceding tree is
> to be removed without a replacement.
> 
> In terms of implementation this should be straight forward.  In the
> merge_nodes function, check the new node for a replace/remove flag,
> and if it is set then remove/replace the old node instead of merging
> with the new.  This can probably be performed in the
> while(new_node->children) loop.

> This is what I was talking about when I mentioned using a flag
> earlier.  The /*-node/ keyword could set a flag in the node structure
> so that merge_nodes handles the operation correctly when merging the
> trees.

Right, I understand the model and, yes, it's simple enough to
implement.  I'm just not sure if it's a good idea - allowing the
replace versus extend flag to be per sub-tree rather than per toplevel
tree is a step up in model complexity, and hence a step down in
obviousness.  I'm not yet sure if it's worth it (or perhaps rather, if
there's a cleaner alternative).

> >> > Now, if using
> >> > labels rather than full paths it does get a bit curlier on the
> >> > implementation side.  But I don't think it would be that hard to make
> >> > labels stick around attached to a path, even when the path itself is
> >> > removed.
> >>
> >> Ah right, the issue here was that a previous tree could be holding a
> >> label reference that gets deleted in an overlay.  And there are
> >> questions about what happens if a label is removed and then the same
> >> label attached to a new node, specifically because labels aren't
> >> resolved until after the trees are fully parsed.  (let me know if I'm
> >> remembering incorrectly).  I think this is solvable though, but we're
> >> going to need to define the semantics.
> >>
> >> I need to review the details though.  I can't remember all the ways
> >> that the code handles label resolution.
> >
> > Right.  Having labels move around seriously muddies the waters, since
> > the whole idea of labels is to unambiguously refer to a node.  I think
> > the sanest thing is to prohibit duplicate labels, even if the labelled
> > nodes have been removed between the duplicate declarations.  In the
> > later stages of processing, then, referring to a deleted label would
> > be detectable and trigger an error at that point.
> 
> Fair enough.  I can agree to that constraint.  It can be revisited
> again if it is too restrictive, and it is easier to relax
> restrictions later than it is to add new restrictions at a later date.

Ok.

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