Re: [PATCH v2] dtc: parser: Add label while overriding nodes
On Fri, Feb 20, 2015 at 02:11:02AM +0530, Nikhil Devshatwar wrote:
This patch changes the dtc grammar to allow following syntax
i2cexp: i2c2 {
...
};
Current device tree compiler allows to define multiple labels when defining
the device node the first time. Typically device nodes are defined in
DTSI files. Now these nodes can be overwritten for updating some of the
properties. Typically, device nodes are overridden in DTS files.
When working with adapter boards, most of the time adapter board can fit to
multiple base boards. But depending on which base board it is connected to,
the devices on the adapter board would be children of different devices.
e.g. On dra7-evm.dts, i2c2 is exported for expansion connector whereas
on dra72-evm.dts, i2c5 is exported for expansion connector.
This causes a problem when writing a generic device tree file for
the adapter board. Because, you cannot know whether all the devices on
adapter board are present on i2c or i2c5.
The problem can be solved by adding a common label (e.g. i2cexp) in both
of the DTS files when overriding the device nodes for i2c2 or i2c5.
This way, generic adapter board file would override the i2cexp. And
depending on which base board you use the adapter board, all the devices
are automatically added for correct device nodes.
Signed-off-by: Nikhil Devshatwar nikhil...@ti.com
Applied, thanks.
I did tweak the testcase to use dtbs_unordered_equal instead of going
through the set of tree1 tests.
The fact that you can only apply a single label by this method is a
bit of a wart, but it's something we can fix later. I had a look at
it and (as I suspect you already discovered) it's surprisingly
complicated to do so.
--
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
pgpdYN9Zuaw5H.pgp
Description: PGP signature
Re: [PATCH] dtc: parser: Add label while overriding nodes
On Thu, Jan 29, 2015 at 09:07:31PM +1100, David Gibson wrote:
On Wed, Jan 28, 2015 at 08:20:11PM +0530, Nikhil Devshatwar wrote:
This patch changes the dtc grammar to allow following syntax
i2cexp: i2c2 {
...
};
Current device tree compiler allows to define multiple labels when defining
the device node the first time. Typically device nodes are defined in
DTSI files. Now these nodes can be overwritten for updating some of the
properties. Typically, device nodes are overridden in DTS files.
When working with adapter boards, most of the time adapter board can fit to
multiple base boards. But depending on which base board it is connected to,
the devices on the adapter board would be children of different devices.
e.g. On dra7-evm.dts, i2c2 is exported for expansion connector whereas
on dra72-evm.dts, i2c5 is exported for expansion connector.
This causes a problem when writing a generic device tree file for
the adapter board. Because, you cannot know whether all the devices on
adapter board are present on i2c or i2c5.
The problem can be solved by adding a common label (e.g. i2cexp) in both
of the DTS files when overriding the device nodes for i2c2 or i2c5.
This way, generic adapter board file would override the i2cexp. And
depending on which base board you use the adapter board, all the devices
are automatically added for correct device nodes.
Signed-off-by: Nikhil Devshatwar nikhil...@ti.com
Hi, sorry I didn't get around to responding to your earlier posting of
this.
The concept is good, and the implementation looks fine.
Only 2 things before I'm ready to merge:
1) It wants a testcase
2) I need to stare at the syntax for a while and convince myself
it's as good as we can reasonably do.
..and.. #2 is now done. Give me a testcase, and I'll merge.
--
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
pgpGXkdDjjSKG.pgp
Description: PGP signature
Re: [PATCH] dtc: parser: Add label while overriding nodes
On Wed, Jan 28, 2015 at 08:20:11PM +0530, Nikhil Devshatwar wrote:
This patch changes the dtc grammar to allow following syntax
i2cexp: i2c2 {
...
};
Current device tree compiler allows to define multiple labels when defining
the device node the first time. Typically device nodes are defined in
DTSI files. Now these nodes can be overwritten for updating some of the
properties. Typically, device nodes are overridden in DTS files.
When working with adapter boards, most of the time adapter board can fit to
multiple base boards. But depending on which base board it is connected to,
the devices on the adapter board would be children of different devices.
e.g. On dra7-evm.dts, i2c2 is exported for expansion connector whereas
on dra72-evm.dts, i2c5 is exported for expansion connector.
This causes a problem when writing a generic device tree file for
the adapter board. Because, you cannot know whether all the devices on
adapter board are present on i2c or i2c5.
The problem can be solved by adding a common label (e.g. i2cexp) in both
of the DTS files when overriding the device nodes for i2c2 or i2c5.
This way, generic adapter board file would override the i2cexp. And
depending on which base board you use the adapter board, all the devices
are automatically added for correct device nodes.
Signed-off-by: Nikhil Devshatwar nikhil...@ti.com
Hi, sorry I didn't get around to responding to your earlier posting of
this.
The concept is good, and the implementation looks fine.
Only 2 things before I'm ready to merge:
1) It wants a testcase
2) I need to stare at the syntax for a while and convince myself
it's as good as we can reasonably do.
--
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
pgp60gkTy92Ff.pgp
Description: PGP signature
Re: [RFC 00/15] Device Tree schemas and validation
On Tue, Oct 01, 2013 at 10:17:53AM -0500, Jon Loeliger wrote: Hi Rob, On 01/10/2013 15:17, Rob Herring wrote: On 10/01/2013 03:06 AM, Benoit Cousson wrote: + more DT maintainers folks Hi all, I know this is mostly boring user space code, but I was expecting a little bit of comments about at least the bindings syntax:-( I'd like to know if this is the right direction and if it worth pursuing in that direction. The idea was to have at least some base for further discussion during ARM KS 2013. I feel alone :-( If you have any comment, go ahead! Benoit, Sorry, I meant to ask earlier but forgot. Shouldn't this development be based on the upstream DTC repository and not the in-kernel copy of the DTC? Absolutely. Please work against upstream dtc. -- 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 pgpvQSOHC9Pwv.pgp Description: PGP signature
Re: [RFC 00/15] Device Tree schemas and validation
On Tue, Oct 01, 2013 at 03:54:20PM -0500, Rob Herring wrote: On Tue, Oct 1, 2013 at 10:06 AM, Benoit Cousson bcous...@baylibre.com wrote: Hi Rob, On 01/10/2013 15:17, Rob Herring wrote: On 10/01/2013 03:06 AM, Benoit Cousson wrote: + more DT maintainers folks Hi all, I know this is mostly boring user space code, but I was expecting a little bit of comments about at least the bindings syntax:-( I'd like to know if this is the right direction and if it worth pursuing in that direction. The idea was to have at least some base for further discussion during ARM KS 2013. I feel alone :-( If you have any comment, go ahead! Thanks for taking this on! This is interesting approach using the dts syntax, Well, this was discussed a little bit on the list, and it has the big advantage of re-using the parser already included in DTC for free. In term or readability, it avoids to re-defining a brand new syntax for people who are already familiar with the DTS one. but I worry that the validation will only be as good as the schema written and the review of the schema. Well, sure, but unfortunately, that will always be the case :-( The bindings definition being quite open, there is no easy way to ensure proper schema / bindings without careful review of the schema. There is no such thing as a free beer... Unfortunately :-) I think the schema needs to define the binding rather than define the checks. Then the schema can feed the validation checks. This format does not seem to me as easily being able to generate documentation from the schema which I believe is one of the goals. Indeed, but I think is it easy to generate any kind of readable format for the documentation purpose if needed from the actual format. Otherwise, we should consider a schema format based on kerneldoc type of syntax to improve readability. I'm just afraid it will become harder then to define complex schema. BTW, what kind of documentation are you expecting here? Is is a text that can be added on top of each schema? I would expect the schema to replace Documentation/devicetree/bindings/* over time. I think the thing that needs to be worked out here is how to add free form multi-line text. I'm not convinced that's a realistic goal. As I see it, the fundamental difference between a binding document and a formal schema is that a binding defines both the syntax required of a node, and its semantics, whereas a schema defines only syntax - the semantics still need to be defined somewhere. -- 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 pgp0yD2z4Q1JA.pgp Description: PGP signature
Re: [RFC 00/15] Device Tree schemas and validation
On Tue, Oct 01, 2013 at 08:17:42AM -0500, Rob Herring wrote: On 10/01/2013 03:06 AM, Benoit Cousson wrote: + more DT maintainers folks Hi all, I know this is mostly boring user space code, but I was expecting a little bit of comments about at least the bindings syntax:-( I'd like to know if this is the right direction and if it worth pursuing in that direction. The idea was to have at least some base for further discussion during ARM KS 2013. I feel alone :-( If you have any comment, go ahead! Thanks for taking this on! This is interesting approach using the dts syntax, but I worry that the validation will only be as good as the schema written and the review of the schema. I think the schema needs to define the binding rather than define the checks. Then the schema can feed the validation checks. This format does not seem to me as easily being able to generate documentation from the schema which I believe is one of the goals. I for one don't care to review the documentation and the schema for every binding. Hrm. I'm less optimistic about entirely replacing human-readable bindings with machine-readable schemas. But I do think the schema language needs to be substantially more flexible than the draft presented here. While I think a schema syntax which mirrors dts syntax makes a lot of sense, actually defining schemas as device trees doesn't seem quite right to me. -- 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 pgps4GNtO_6_u.pgp Description: PGP signature
Re: [RFC 01/15] scripts/dtc: fix most memory leaks in dtc
);
+ free_property(new_prop);
continue;
}
@@ -165,7 +169,7 @@ struct node *merge_nodes(struct node *old_node, struct
node *new_node)
if (streq(old_prop-name, new_prop-name)) {
/* Add new labels to old property */
for_each_label_withdel(new_prop-labels, l)
- add_label(old_prop-labels, l-label);
+ add_label(old_prop-labels,
xstrdup(l-label));
old_prop-val = new_prop-val;
old_prop-deleted = 0;
@@ -191,7 +195,7 @@ struct node *merge_nodes(struct node *old_node, struct
node *new_node)
if (new_child-deleted) {
delete_node_by_name(old_node, new_child-name);
- free(new_child);
+ free_node(new_child);
continue;
}
@@ -211,7 +215,7 @@ struct node *merge_nodes(struct node *old_node, struct
node *new_node)
/* The new node contents are now merged into the old node. Free
* the new node. */
- free(new_node);
+ free_node(new_node);
return old_node;
}
@@ -532,13 +536,13 @@ cell_t get_node_phandle(struct node *root, struct node
*node)
if (!get_property(node, linux,phandle)
(phandle_format PHANDLE_LEGACY))
add_property(node,
- build_property(linux,phandle,
+ build_property(xstrdup(linux,phandle),
data_append_cell(empty_data,
phandle)));
if (!get_property(node, phandle)
(phandle_format PHANDLE_EPAPR))
add_property(node,
- build_property(phandle,
+ build_property(xstrdup(phandle),
data_append_cell(empty_data,
phandle)));
/* If the node *does* have a phandle property, we must
@@ -707,3 +711,93 @@ void sort_tree(struct boot_info *bi)
sort_reserve_entries(bi);
sort_node(bi-dt);
}
+
+static void free_marker_list(struct marker *m)
+{
+ struct marker *marker, *marker_next;
+
+ if (!m)
+ return;
+
+ for (marker = m, marker_next = marker ? marker-next : NULL;
+ marker;
+ marker = marker_next, marker_next = marker ? marker-next : NULL) {
Rather hard to read version of safe-against-free list iteration.
+ free(marker-ref);
+ free(marker);
+ }
+}
+
+static void free_label_list(struct label *l)
+{
+ struct label *label, *label_next;
+
+ if (!l)
+ return;
+
+ for (label = l, label_next = label ? label-next : NULL;
+ label;
+ label = label_next, label_next = label ? label-next : NULL) {
+ free(label-label);
+ free(label);
+ }
+}
+
+static void free_property(struct property *p)
+{
+ if (!p)
+ return;
+
+ free_label_list(p-labels);
+ free_marker_list(p-val.markers);
+ free(p-val.val);
+ free(p-name);
+ free(p);
+}
+
+static void free_property_list(struct property *p)
+{
+ struct property *next;
+
+ if (!p)
+ return;
+
+ for (next = p-next; p; p = next, next = p ? p-next : NULL)
+ free_property(p);
+}
+
+static void free_node(struct node *n)
+{
+ if (!n)
+ return;
+
+ free_node_list(n-children);
+ free_label_list(n-labels);
+ free_property_list(n-proplist);
+ free(n-fullpath);
+ if (n-name *n-name)
*n-name .. so, the name can (and must) be statically allocated if
it's exactly . Not a useful optimization, I think.
+ free(n-name);
+ free(n);
+}
+
+static void free_node_list(struct node *n)
+{
+ struct node *next;
+
+ if (!n)
+ return;
+
+ for (next = n-next_sibling;
+ n;
+ n = next, next = n ? n-next_sibling : NULL) {
+ free_node(n);
+ }
+}
+
+void free_dt(struct boot_info *bi)
+{
+ if (!bi)
+ return;
+
+ free_node_list(bi-dt);
+ free(bi);
+}
--
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
pgpIqVVeRdOcK.pgp
Description: PGP signature
Re: [RFC 00/15] Device Tree schemas and validation
that the name property override the node name.
Override implies that dtc would change the node name during compilation.
I think s/override/validate/ or s/override/overrides the validation
rules for/?
Actually, dtc already contains checks that a name property (if
present) matches the unit name. Name properties vs. node names work a
bit differently in the flat-tree world versus traditional OF, and this
checks ensures that flat trees don't do (at least some) things which
would break the OF traditional approach.
* Require the presence of a property inside a node or inside one of its
parents
...
/dts-v1/;
/ {
compatible = ti,twl[0-9]+-rtc;
interrupt-controller {
is-required;
can-be-inherited;
interrupt-controller isn't a good example here, since it isn't a
property that would typically be inherited. Why not use interrupt-parent
instead?
One can check if 'node' has the following subnode 'subnode1', 'subnode2',
and 'abc' with the schema below:
/dts-v1/;
/ {
compatible = comp;
children = abc, subnode[0-9];
};
How is the schema for each sub-node specified?
What if some nodes are optional and some required? The conditions where
a sub-node is required might be complex, and I think we'd always want to
be able to represent them in whatever schema language we chose.
The most obvious way would be to make each sub-node's schema appear as a
sub-node within the main node's schema, but then how do you tell if a
schema node describes a property or a node?
Note that the following DT file is currently accepted by dtc even if it
may not be the best choice of property and node names:
==
/dts-v1/;
/ {
foo = 1;
foo {};
};
==
Note that node / property name collisions are not entirely theoretical
either. They are permitted in IEEE1275 and there are real Apple
device trees in the wild which have them. It's rare and discouraged,
obviously.
* Constraints on array size
One can specify the following constraints on array size:
- length: specify the exact length that an array must have.
- min-length: specify the minimum number of elements an array must have.
- max-length: specify the maximum number of elements an array must have.
This seems rather inflexible; it'll cover a lot of the simple cases, but
hit a wall pretty soon. For example, how would it validate a property
that is supposed to include 3 GPIO specifiers, where the GPIO specifiers
are going to have DT-specific lengths, since the length of each
specifier is defined by the node that the phandles reference?
Overall, I believe perhaps the single most important aspect of any DT
schema is schema inheritance or instancing, and this proposal doesn't
appear to address that issue at all.
Inheritance of schemas:
For example, any node that is addressed must contain a reg property. The
constraints on that property are identical in all bindings; it must
consist of #address-cells + #size-cells integer values (cells). We don't
want to have to cut/paste that rule into every single binding
definition. Rather, we should simply say something like this binding
uses the reg property, and the schema validation tool will look up the
definition of reg property, and hence know how to validate it.
Similarly, any binding that describes a GPIO controller will have some
similar requirements; the gpio-controller and #gpio-cells properties
must be present. The schema should simply say I'm a GPIO controller,
and the schema tool should add some extra requirements to nodes of that
type.
Instancing of schemas:
Any binding that uses GPIOs should be able to say that a particular
property (e.g. enable-gpios) is-a GPIO-specifier (with parameters
enable for the property name, min/max/expression length, etc.), and
then the schema validation tool would know to apply rules for a
specifier list to that property (and be able to check the property name).
Yes, I agree both of those are important.
So, here's a counter-proposal of at least a rough outline of how I
think schemas could work, in a way that's still based generally on dt
syntax.
First, define the notion of dt patterns or templates. A dt
pattern is to a dt node or subtree as a regex is to a string - it
provides a reasonably expressive way of defining a family of dt
nodes. These would be defined in an extension / superset of dt
syntax.
A schema would then be defined as a set of implications:
If node X matches pattern A, = it must also match pattern B
For example:
If a node has a compatible property with string foodev
= it must have various foodev properties.
If a node has a reg property (at all)
= it must have the format required by reg
If a node has an interrupts property
= it must have either interrupt-parent or interrupt-map
--
David Gibson| I'll have my music baroque, and my
Re: [RFC 00/15] Device Tree schemas and validation
On Wed, Oct 02, 2013 at 07:08:41PM +0100, Mark Brown wrote: On Wed, Oct 02, 2013 at 11:54:50PM +1000, David Gibson wrote: On Tue, Oct 01, 2013 at 03:54:20PM -0500, Rob Herring wrote: I would expect the schema to replace Documentation/devicetree/bindings/* over time. I think the thing that needs to be worked out here is how to add free form multi-line text. I'm not convinced that's a realistic goal. As I see it, the fundamental difference between a binding document and a formal schema is that a binding defines both the syntax required of a node, and its semantics, whereas a schema defines only syntax - the semantics still need to be defined somewhere. So long as the schema lets you include free form text to define the semantics I'm not sure there's an incompatibility there - the same document can cover both. True, there's no reason the machine-readable schema and human-readable documentation can't be contained in the same file. -- 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 pgpv8VA08IhkS.pgp Description: PGP signature
Re: [PATCH 6/6] OF: Introduce DT overlay support.
On Wed, Jan 23, 2013 at 01:01:58PM +0200, Pantelis Antoniou wrote:
On Jan 23, 2013, at 7:12 AM, David Gibson wrote:
On Tue, Jan 22, 2013 at 01:08:04PM +0200, Pantelis Antoniou wrote:
Hi
On Jan 22, 2013, at 5:50 AM, David Gibson wrote:
On Fri, Jan 04, 2013 at 09:31:10PM +0200, Pantelis Antoniou wrote:
Introduce DT overlay support.
Using this functionality it is possible to dynamically overlay a part of
the kernel's tree with another tree that's been dynamically loaded.
It is also possible to remove node and properties.
Signed-off-by: Pantelis Antoniou pa...@antoniou-consulting.com
---
Documentation/devicetree/overlay-notes.txt | 179 +++
drivers/of/Kconfig | 10 +
drivers/of/Makefile| 1 +
drivers/of/overlay.c | 831
+
include/linux/of.h | 107
5 files changed, 1128 insertions(+)
create mode 100644 Documentation/devicetree/overlay-notes.txt
create mode 100644 drivers/of/overlay.c
diff --git a/Documentation/devicetree/overlay-notes.txt
b/Documentation/devicetree/overlay-notes.txt
new file mode 100644
index 000..5289cbb
--- /dev/null
+++ b/Documentation/devicetree/overlay-notes.txt
@@ -0,0 +1,179 @@
+Device Tree Overlay Notes
+-
+
+This document describes the implementation of the in-kernel
+device tree overlay functionality residing in drivers/of/overlay.c and
is a
+companion document to
Documentation/devicetree/dt-object-internal.txt[1]
+Documentation/devicetree/dynamic-resolution-notes.txt[2]
+
+How overlays work
+-
+
+A Device Tree's overlay purpose is to modify the kernel's live tree, and
+have the modification affecting the state of the the kernel in a way
that
+is reflecting the changes.
Um.. I'm having a great deal of trouble parsing that sentence.
+Since the kernel mainly deals with devices, any new device node that
result
+in an active device should have it created while if the device node is
either
+disabled or removed all together, the affected device should be
deregistered.
+
+Lets take an example where we have a foo board with the following base
tree
+which is taken from [1].
+
+ foo.dts
-
+/* FOO platform */
+/ {
+compatible = corp,foo;
+
+/* shared resources */
+res: res {
+};
+
+/* On chip peripherals */
+ocp: ocp {
+/* peripherals that are always instantiated */
+peripheral1 { ... };
+}
+};
+ foo.dts
-
+
+The overlay bar.dts, when loaded (and resolved as described in [2])
should
+
+ bar.dts
-
+/plugin/; /* allow undefined label references and record them */
+/ {
+/* various properties for loader use; i.e. part id etc.
*/
+fragment@0 {
+target = ocp;
+__overlay__ {
+/* bar peripheral */
+bar {
+compatible = corp,bar;
+... /* various properties and child
nodes */
+}
+};
+};
+};
+ bar.dts
-
+
+result in foo+bar.dts
+
+ foo+bar.dts
-
+/* FOO platform + bar peripheral */
+/ {
+compatible = corp,foo;
+
+/* shared resources */
+res: res {
+};
+
+/* On chip peripherals */
+ocp: ocp {
+/* peripherals that are always instantiated */
+peripheral1 { ... };
+
+/* bar peripheral */
+bar {
+compatible = corp,bar;
+... /* various properties and child
nodes */
+}
+}
+};
+ foo+bar.dts
-
+
+As a result of the the overlay, a new device node (bar) has been created
+so a bar platform device will be registered and if a matching device
driver
+is loaded the device will be created as expected.
Hrm. This all seems rather complicated. Maybe it needs to be, but
I'm not entirely convinced yet.
One other point - both of these patches
Re: [PATCH 5/6] OF: Introduce Device Tree resolve support.
On Tue, Jan 22, 2013 at 01:06:09PM +0200, Pantelis Antoniou wrote: Hi On Jan 22, 2013, at 6:05 AM, David Gibson wrote: On Mon, Jan 21, 2013 at 12:59:15PM +0200, Pantelis Antoniou wrote: Hi David On Jan 21, 2013, at 6:48 AM, David Gibson wrote: On Fri, Jan 04, 2013 at 09:31:09PM +0200, Pantelis Antoniou wrote: Introduce support for dynamic device tree resolution. Using it, it is possible to prepare a device tree that's been loaded on runtime to be modified and inserted at the kernel live tree. Signed-off-by: Pantelis Antoniou pa...@antoniou-consulting.com --- .../devicetree/dynamic-resolution-notes.txt| 25 ++ drivers/of/Kconfig | 9 + drivers/of/Makefile| 1 + drivers/of/resolver.c | 394 + include/linux/of.h | 17 + 5 files changed, 446 insertions(+) create mode 100644 Documentation/devicetree/dynamic-resolution-notes.txt create mode 100644 drivers/of/resolver.c diff --git a/Documentation/devicetree/dynamic-resolution-notes.txt b/Documentation/devicetree/dynamic-resolution-notes.txt new file mode 100644 index 000..0b396c4 --- /dev/null +++ b/Documentation/devicetree/dynamic-resolution-notes.txt @@ -0,0 +1,25 @@ +Device Tree Dynamic Resolver Notes +-- + +This document describes the implementation of the in-kernel +Device Tree resolver, residing in drivers/of/resolver.c and is a +companion document to Documentation/devicetree/dt-object-internal.txt[1] + +How the resolver works +-- + +The resolver is given as an input an arbitrary tree compiled with the +proper dtc option and having a /plugin/ tag. This generates the +appropriate __fixups__ __local_fixups__ nodes as described in [1]. + +In sequence the resolver works by the following steps: + +1. Get the maximum device tree phandle value from the live tree + 1. +2. Adjust all the local phandles of the tree to resolve by that amount. +3. Using the __local__fixups__ node information adjust all local references + by the same amount. +4. For each property in the __fixups__ node locate the node it references + in the live tree. This is the label used to tag the node. +5. Retrieve the phandle of the target of the fixup. +5. For each fixup in the property locate the node:property:offset location + and replace it with the phandle value. Hrm. So, I'm really still not convinced by this approach. First, I think it's unwise to allow overlays to change essentially anything in the base tree, rather than having the base tree define sockets of some sort where things can be attached. One could say that the labels define the sockets. It's not just things to be attached, properties might have to change, or something more complex, as we've found out in practice. Hrm. I know a number of these have come up previously in that big thread, but can you summarise some of these cases here. If things need modification in the base tree that still seems to me like the base tree hasn't properly described the socket arrangement (I realise that allowing such descriptions may require extensions to some of our device tree conventions). It would be pointless to number all the use-cases that Grant put in that long document, but I can summarize the cases that we've seen on the bone. * Addition of child device nodes to the ocp node, creates new platform devices of various kind (audio/video/pwms/timers) - almost any kind of platform device that the SoC supports. Removing the overlay unregisters the devices (but precious few drivers support that cleanly ATM). Since the capes don't support hotplug that's not a big deal. Ok, that's just adding nodes, which is straightforward. * Addition of pinctrl configuration nodes. Ok, do you know where I can look to see how the pinctrl stuff works? * Addition of i2c/spi etc device nodes and modification of the parent's node status property to okay, Ok. I'm assuming this is basically just to enable the bus controller which was previously disabled because it had nothing attached to it? creates the bus platform device registers the devices on the bus. Slight complication with i2c client devices which are not platform devices need special handling. And this part is again just adding nodes. * Modification of configuration parameters of a disabled device and subsequent enablement. What sorts of modification are necessary to the parameters? Other than changing status = disabled of course. This is the only case I see here which might be changing data other than the status property, which if it were the only one could reasonably be special cased, I think. As far as the unwise part, a good deal of care has been taken so that people that don't
Re: [PATCH 6/6] OF: Introduce DT overlay support.
On Tue, Jan 22, 2013 at 01:08:04PM +0200, Pantelis Antoniou wrote:
Hi
On Jan 22, 2013, at 5:50 AM, David Gibson wrote:
On Fri, Jan 04, 2013 at 09:31:10PM +0200, Pantelis Antoniou wrote:
Introduce DT overlay support.
Using this functionality it is possible to dynamically overlay a part of
the kernel's tree with another tree that's been dynamically loaded.
It is also possible to remove node and properties.
Signed-off-by: Pantelis Antoniou pa...@antoniou-consulting.com
---
Documentation/devicetree/overlay-notes.txt | 179 +++
drivers/of/Kconfig | 10 +
drivers/of/Makefile| 1 +
drivers/of/overlay.c | 831
+
include/linux/of.h | 107
5 files changed, 1128 insertions(+)
create mode 100644 Documentation/devicetree/overlay-notes.txt
create mode 100644 drivers/of/overlay.c
diff --git a/Documentation/devicetree/overlay-notes.txt
b/Documentation/devicetree/overlay-notes.txt
new file mode 100644
index 000..5289cbb
--- /dev/null
+++ b/Documentation/devicetree/overlay-notes.txt
@@ -0,0 +1,179 @@
+Device Tree Overlay Notes
+-
+
+This document describes the implementation of the in-kernel
+device tree overlay functionality residing in drivers/of/overlay.c and is
a
+companion document to Documentation/devicetree/dt-object-internal.txt[1]
+Documentation/devicetree/dynamic-resolution-notes.txt[2]
+
+How overlays work
+-
+
+A Device Tree's overlay purpose is to modify the kernel's live tree, and
+have the modification affecting the state of the the kernel in a way that
+is reflecting the changes.
Um.. I'm having a great deal of trouble parsing that sentence.
+Since the kernel mainly deals with devices, any new device node that
result
+in an active device should have it created while if the device node is
either
+disabled or removed all together, the affected device should be
deregistered.
+
+Lets take an example where we have a foo board with the following base
tree
+which is taken from [1].
+
+ foo.dts
-
+ /* FOO platform */
+ / {
+ compatible = corp,foo;
+
+ /* shared resources */
+ res: res {
+ };
+
+ /* On chip peripherals */
+ ocp: ocp {
+ /* peripherals that are always instantiated */
+ peripheral1 { ... };
+ }
+ };
+ foo.dts
-
+
+The overlay bar.dts, when loaded (and resolved as described in [2]) should
+
+ bar.dts
-
+/plugin/; /* allow undefined label references and record them */
+/ {
+ /* various properties for loader use; i.e. part id etc. */
+ fragment@0 {
+ target = ocp;
+ __overlay__ {
+ /* bar peripheral */
+ bar {
+ compatible = corp,bar;
+ ... /* various properties and child nodes */
+ }
+ };
+ };
+};
+ bar.dts
-
+
+result in foo+bar.dts
+
+ foo+bar.dts
-
+ /* FOO platform + bar peripheral */
+ / {
+ compatible = corp,foo;
+
+ /* shared resources */
+ res: res {
+ };
+
+ /* On chip peripherals */
+ ocp: ocp {
+ /* peripherals that are always instantiated */
+ peripheral1 { ... };
+
+ /* bar peripheral */
+ bar {
+ compatible = corp,bar;
+ ... /* various properties and child nodes */
+ }
+ }
+ };
+ foo+bar.dts
-
+
+As a result of the the overlay, a new device node (bar) has been created
+so a bar platform device will be registered and if a matching device
driver
+is loaded the device will be created as expected.
Hrm. This all seems rather complicated. Maybe it needs to be, but
I'm not entirely convinced yet.
One other point - both of these patches are assuming that the overlay
is in the live tree format, but it still needs a bunch of extra
mangling. Would it simplify things to just go straight from the
overlay in flat tree form to modifications to the system-wide live
tree.
Sorry, I can't parse this. You mean apply the overlay without converting
to live tree format?
Yes.
--
David Gibson| I'll
Re: [PATCH 6/6] OF: Introduce DT overlay support.
On Fri, Jan 04, 2013 at 09:31:10PM +0200, Pantelis Antoniou wrote:
Introduce DT overlay support.
Using this functionality it is possible to dynamically overlay a part of
the kernel's tree with another tree that's been dynamically loaded.
It is also possible to remove node and properties.
Signed-off-by: Pantelis Antoniou pa...@antoniou-consulting.com
---
Documentation/devicetree/overlay-notes.txt | 179 +++
drivers/of/Kconfig | 10 +
drivers/of/Makefile| 1 +
drivers/of/overlay.c | 831
+
include/linux/of.h | 107
5 files changed, 1128 insertions(+)
create mode 100644 Documentation/devicetree/overlay-notes.txt
create mode 100644 drivers/of/overlay.c
diff --git a/Documentation/devicetree/overlay-notes.txt
b/Documentation/devicetree/overlay-notes.txt
new file mode 100644
index 000..5289cbb
--- /dev/null
+++ b/Documentation/devicetree/overlay-notes.txt
@@ -0,0 +1,179 @@
+Device Tree Overlay Notes
+-
+
+This document describes the implementation of the in-kernel
+device tree overlay functionality residing in drivers/of/overlay.c and is a
+companion document to Documentation/devicetree/dt-object-internal.txt[1]
+Documentation/devicetree/dynamic-resolution-notes.txt[2]
+
+How overlays work
+-
+
+A Device Tree's overlay purpose is to modify the kernel's live tree, and
+have the modification affecting the state of the the kernel in a way that
+is reflecting the changes.
Um.. I'm having a great deal of trouble parsing that sentence.
+Since the kernel mainly deals with devices, any new device node that result
+in an active device should have it created while if the device node is either
+disabled or removed all together, the affected device should be deregistered.
+
+Lets take an example where we have a foo board with the following base tree
+which is taken from [1].
+
+ foo.dts
-
+ /* FOO platform */
+ / {
+ compatible = corp,foo;
+
+ /* shared resources */
+ res: res {
+ };
+
+ /* On chip peripherals */
+ ocp: ocp {
+ /* peripherals that are always instantiated */
+ peripheral1 { ... };
+ }
+ };
+ foo.dts
-
+
+The overlay bar.dts, when loaded (and resolved as described in [2]) should
+
+ bar.dts
-
+/plugin/;/* allow undefined label references and record them */
+/ {
+ /* various properties for loader use; i.e. part id etc. */
+ fragment@0 {
+ target = ocp;
+ __overlay__ {
+ /* bar peripheral */
+ bar {
+ compatible = corp,bar;
+ ... /* various properties and child nodes */
+ }
+ };
+ };
+};
+ bar.dts
-
+
+result in foo+bar.dts
+
+ foo+bar.dts
-
+ /* FOO platform + bar peripheral */
+ / {
+ compatible = corp,foo;
+
+ /* shared resources */
+ res: res {
+ };
+
+ /* On chip peripherals */
+ ocp: ocp {
+ /* peripherals that are always instantiated */
+ peripheral1 { ... };
+
+ /* bar peripheral */
+ bar {
+ compatible = corp,bar;
+ ... /* various properties and child nodes */
+ }
+ }
+ };
+ foo+bar.dts
-
+
+As a result of the the overlay, a new device node (bar) has been created
+so a bar platform device will be registered and if a matching device driver
+is loaded the device will be created as expected.
Hrm. This all seems rather complicated. Maybe it needs to be, but
I'm not entirely convinced yet.
One other point - both of these patches are assuming that the overlay
is in the live tree format, but it still needs a bunch of extra
mangling. Would it simplify things to just go straight from the
overlay in flat tree form to modifications to the system-wide live
tree.
--
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
signature.asc
Description: Digital signature
Re: [PATCH 5/6] OF: Introduce Device Tree resolve support.
On Mon, Jan 21, 2013 at 12:59:15PM +0200, Pantelis Antoniou wrote: Hi David On Jan 21, 2013, at 6:48 AM, David Gibson wrote: On Fri, Jan 04, 2013 at 09:31:09PM +0200, Pantelis Antoniou wrote: Introduce support for dynamic device tree resolution. Using it, it is possible to prepare a device tree that's been loaded on runtime to be modified and inserted at the kernel live tree. Signed-off-by: Pantelis Antoniou pa...@antoniou-consulting.com --- .../devicetree/dynamic-resolution-notes.txt| 25 ++ drivers/of/Kconfig | 9 + drivers/of/Makefile| 1 + drivers/of/resolver.c | 394 + include/linux/of.h | 17 + 5 files changed, 446 insertions(+) create mode 100644 Documentation/devicetree/dynamic-resolution-notes.txt create mode 100644 drivers/of/resolver.c diff --git a/Documentation/devicetree/dynamic-resolution-notes.txt b/Documentation/devicetree/dynamic-resolution-notes.txt new file mode 100644 index 000..0b396c4 --- /dev/null +++ b/Documentation/devicetree/dynamic-resolution-notes.txt @@ -0,0 +1,25 @@ +Device Tree Dynamic Resolver Notes +-- + +This document describes the implementation of the in-kernel +Device Tree resolver, residing in drivers/of/resolver.c and is a +companion document to Documentation/devicetree/dt-object-internal.txt[1] + +How the resolver works +-- + +The resolver is given as an input an arbitrary tree compiled with the +proper dtc option and having a /plugin/ tag. This generates the +appropriate __fixups__ __local_fixups__ nodes as described in [1]. + +In sequence the resolver works by the following steps: + +1. Get the maximum device tree phandle value from the live tree + 1. +2. Adjust all the local phandles of the tree to resolve by that amount. +3. Using the __local__fixups__ node information adjust all local references + by the same amount. +4. For each property in the __fixups__ node locate the node it references + in the live tree. This is the label used to tag the node. +5. Retrieve the phandle of the target of the fixup. +5. For each fixup in the property locate the node:property:offset location + and replace it with the phandle value. Hrm. So, I'm really still not convinced by this approach. First, I think it's unwise to allow overlays to change essentially anything in the base tree, rather than having the base tree define sockets of some sort where things can be attached. One could say that the labels define the sockets. It's not just things to be attached, properties might have to change, or something more complex, as we've found out in practice. Hrm. I know a number of these have come up previously in that big thread, but can you summarise some of these cases here. If things need modification in the base tree that still seems to me like the base tree hasn't properly described the socket arrangement (I realise that allowing such descriptions may require extensions to some of our device tree conventions). As far as the unwise part, a good deal of care has been taken so that people that don't use the overlay functionality have absolutely no overhead, or anything modified in the way they use DT. Yeah, that's not what I'm concerned about. I'm concerned about hard to debug problems because some subtle change in the base tree or the overlay or both causes the overlay to alter something in the base tree it really shouldn't. Second, even allowing overlays to change anything, I don't see a lot of reason to do this kind of resolution within the kernel and with data stored in the dtb itself, rather than doing the resolution in userspace from an annotated overlay dts or dtb, then inserting the fully resolved product into the kernel. In either case, the overlay needs to be constructed with pretty intimate knowledge of the base tree. Fair enough, but that's one more thing of user-space crud to drag along, which will get enabled pretty late in the boot sequence. Meaning a whole bunch of devices, like consoles, and root filesystems on the devices that need an overlay to operate won't work easily enough. Hrm. But doesn't your scheme already require userspace to identify the hardware and load the overlay? So why is having it resolve the overlay significantly harder? AFAICT devices wanted early can be handled in several possible ways without having the resolved in kernel: an initramfs is the most obvious, but for things you want really early, it should be possible to do the resolution from the platform's bootloader update tool - so the pre-resolved overlay gets bundled with the kernel/initrd/whatever to get fired up from the bootloader. That said, I have some implementation comments below
Re: [PATCH 5/6] OF: Introduce Device Tree resolve support.
(propval, rprop-value, rprop-length);
+
+ propend = propval + rprop-length;
+ for (propcur = propval; propcur propend;
+ propcur += propcurlen + 1) {
+ propcurlen = strlen(propcur);
+
+ nodestr = propcur;
+ s = strchr(propcur, ':');
+ if (s == NULL) {
+ pr_err(%s: Illegal symbol
+ entry '%s' (1)\n,
+ __func__, (char *)rprop-value);
+ kfree(propval);
+ err = -EINVAL;
+ goto err_fail;
+ }
+ *s++ = '\0';
+
+ propstr = s;
+ s = strchr(s, ':');
+ if (s == NULL) {
+ pr_err(%s: Illegal symbol
+ entry '%s' (2)\n,
+ __func__, (char *)rprop-value);
+ kfree(propval);
+ err = -EINVAL;
+ goto err_fail;
+ }
+
+ *s++ = '\0';
+ offset = simple_strtoul(s, NULL, 10);
+
+ /* look into the resolve node for the full path */
+ refnode = __of_find_node_by_full_name(resolve,
+ nodestr);
Re-using the 'refnode' variable here is pretty confusing, since it
means very different things earlier and here (node pointed to, versus
node containing the property which points).
+ if (refnode == NULL) {
+ pr_err(%s: Could not find refnode '%s'\n,
+ __func__, (char *)rprop-value);
+ kfree(propval);
+ err = -ENOENT;
+ goto err_fail;
+ }
+
+ /* now find the property */
+ for_each_property_of_node(refnode, sprop) {
+ if (of_prop_cmp(sprop-name, propstr) == 0)
+ break;
+ }
+
+ if (sprop == NULL) {
+ pr_err(%s: Could not find property '%s'\n,
+ __func__, (char *)rprop-value);
+ kfree(propval);
+ err = -ENOENT;
+ goto err_fail;
+ }
+
+ *(uint32_t *)(sprop-value + offset) =
+ cpu_to_be32(phandle);
+ }
+
+ kfree(propval);
+ }
+
+merge_sym:
+
+ of_node_put(root_sym);
+
+ return 0;
+
+err_fail:
+
+ if (root_sym != NULL)
+ of_node_put(root_sym);
+
+ return err;
+}
diff --git a/include/linux/of.h b/include/linux/of.h
index c38e41a..ab52243 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -650,4 +650,21 @@ static inline int of_multi_prop_cmp(const struct
property *prop, const char *val
#endif /* !CONFIG_OF */
+
+/* illegal phandle value (set when unresolved) */
+#define OF_PHANDLE_ILLEGAL 0xdeadbeef
Ugh. 0 and -1 are already reserved as illegal phandle values, don't
invent a new one.
+
+#ifdef CONFIG_OF_RESOLVE
+
+int of_resolve(struct device_node *resolve);
+
+#else
+
+static inline int of_resolve(struct device_node *resolve)
+{
+ return -ENOTSUPP;
+}
+
+#endif
+
#endif /* _LINUX_OF_H */
--
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
signature.asc
Description: Digital signature
Re: [RFC] Device Tree Overlays Proposal (Was Re: capebus moving omap_devices to mach-omap2)
On Tue, Nov 13, 2012 at 03:38:18PM +0200, Pantelis Antoniou wrote: Hi Grant, On Nov 13, 2012, at 2:24 PM, Grant Likely wrote: On Tue, Nov 13, 2012 at 8:09 AM, Pantelis Antoniou [snip] My intention wasn't never to make overlays overly portable. My intention was to make them in a way that portability can be introduced if the boards are 'close' enough, but not for arbitrary boards. There have to be compatible interfaces both on the base, and the overlay dtbs. Right. And this is why I'm arguing that those interfaces should be described explicitly - using existing OF mechanisms like interrupt-map where possible, rather than having a very general, but very low-level interface to make arbitrary changes to the DT. -- 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 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Device Tree Overlays Proposal (Was Re: capebus moving omap_devices to mach-omap2)
On Tue, Nov 13, 2012 at 10:09:28AM +0200, Pantelis Antoniou wrote:
Hi David,
On Nov 13, 2012, at 9:25 AM, David Gibson wrote:
On Mon, Nov 12, 2012 at 09:52:32AM -0700, Stephen Warren wrote:
On 11/12/2012 05:10 AM, Pantelis Antoniou wrote:
[snip]
Oh yes. In fact if one was to use a single kernel image for beagleboard
and beaglebone, for the cape to work for both, it is required for it's
dtb to be compatible.
Well, as Grant pointed out, it's not actually strictly necessary for the
.dtb to be compatible; only the .dts /need/ be compatible, and the .dtb
can be generated at run-time using dtc for example.
So, actually, I think a whole bunch of problems with phandle
resolution disappear if we don't try to define an overlay .dtb format,
or at least treat it only as a very shortlived object. A more precise
proposal below. Note that this works more or less equally well with
either the original overlay approach or the graft/graft-bundle
proposal I made elsewhere.
1) We annotate the base tree with some extra label information for
nodes which overlays are likely to want to reference by phandle. e.g.
beaglebone_pic: interrupt-controller@X {
...
phandle,symbolic-name = beaglebone_pic;
};
We could extend dtc to (optionally?) auto-generate those properties
from its existing label syntax. Not sure if that's a good idea or
not yet. In any case, we compile this augmented base tree to .dtb as
normal and boot our kernel with it.
I'm fine with that. You can auto-generate when there's a label to a node.
The cape dt fragment will use the label name to reference a node.
More details below...
2) The information for the capes/modules/whatever is
distributed/packaged as .dts, never .dtb. When userspace detects the
new module (or the user explicitly tells it, if it's not probeable) it
picks up the correct dts and runs it through dtc in a special mode.
In this mode dtc takes the existing base tree (from /proc/device-tree,
say) as well as the new dts. In this mode, dtc allocates phandles for
the new tree fragment so as not to collide with anything from the
supplied base tree (as well as avoiding internal conflicts,
obviously). It also allows node references to the base tree by using
those label annotations from (1) to match symbolic names to the
phandle values in the base tree.
Not good to rely on userspace kicking off dtc and compiling from source.
Some capes/expansion boards might have your root fs device, for example
there is an eMMC cape coming up, while networking capes are common too.
So? dtc can go in an initramfs, just like mdadm or whatever other
tools are there.
However I have a compromise.
I agree that compiling from source can be an option for a runtime definable
cape, but for built-in capes we could do a bit better.
In particular, I don't see any particular need to have a DT fragment
reference anything that dependent of the runtime device tree. It should
be possible to compile the DT fragment in kernel, against the generated
flattened device tree, producing a flattened DT fragment with the phandles
already resolved.
Um..? Sorry, I can't parse that paragraph.
So the sequence could be something like this:
$ dtc -O dtb -o am335x-bone.dtb -b 0 am335x-bone.dts -@ ${LAST_PHANDLE_FILE}
$ dtc -O dtbf -R am335x-bone.dtb -o weather-cape.dtb -b 0 weather-cape.dts -@
${LAST_PHANDLE_FILE}
$ dtc -O dtbf -R am335x-bone.dtb -o geiger-cape.dtb -b 0 geiger-cape.dts -@
${LAST_PHANDLE_FILE}
The ${LAST_PHANDLE_FILE} can be updated with the last phandle value generated.
Alternatively we could have a way to statically assign a phandle range
for well known capes. All the others will have to use the runtime compile
mechanism.
$ dtc -O dtb -o am335x-bone.dtb -b 0 am335x-bone.dts
$ dtc -O dtbf -R am335x-bone.dtb -o weather-cape.dtb -b 0 weather-cape.dts
$ dtc -O dtbf -R am335x-bone.dtb -o geiger-cape.dtb -b 0 geiger-cape.dts
With the cape dtses having a /phandle-range/ statement at the top.
This can work because the cape dts do not cross-reference each other, and
neither the boot dts references the capes.
That way we can use request_firmware() pretty early in the boot sequence
and get the DT fragment we need even before user-space starts and root fs
has mounted. request_firmware() can locate the fragments in the kernel
image before rootfs.
I don't know if this will cover all the cases Grant has in mind though.
So just to make sure I got it right, this could work for our case.
i2c2: i2c@4819c000 {
compatible = ti,omap4-i2c;
#address-cells = 1;
#size-cells = 0;
ti,hwmods = i2c3;
reg = 0x4819c000 0x1000;
interrupt-parent = intc;
interrupts = 30;
status = disabled;
};
And in the cape definition (when compiled with the special mode I describe
below
Re: [RFC] Device Tree Overlays Proposal (Was Re: capebus moving omap_devices to mach-omap2)
On Fri, Nov 09, 2012 at 04:40:15PM +0100, Pantelis Antoniou wrote:
Hi David,
[snip]
I think graft is basically a safer operation, particular if we're
doing this at runtime with userspace passing in these fdt fragments.
In fact I'd go so far as to say if you really need the full overlay
functionality, then you really ought to be working at the bootloader
or early kernel load level to assemble the correct full device tree.
And as Mitch says, an existing programming language (C, OFW Forth or
whatever as you please) will serve you better for this sort of general
manipulation than a limited template system.
I also think graft will handle most of your use cases, although as I
said I don't fully understand the implications of some of them, so I
could be wrong. So, the actual insertion of the subtree is pretty
trivial to implement. phandles are the obvious other matter to be
dealt with. I haven't found the right thread where what you've
envisaged so far is discussed, so here are things I can see:
An overlay is more generic and can handle more complex cases.
For our use case, a graft should work - with a few caveats.
Obviously an overlay is more generic. But the ability to arbitrarily
modify existing tree nodes, without any obvious way to enforce rules
about what can be altered and what can't frankly gives me the
heebie-jeebies.
Due to the insertion/removal of the DT fragments other node's state
change from 'disabled' - 'okay' and platform devices created or
removed. This can be handled either via overlays, or via special
casing it.
Ah, yes, the status transition is a good point.
[snip]
1) Avoiding phandle collisions between main tree and subtree (or
between subtrees).
I'm hopeful that this can be resolved just by establishing some
conventions about the ranges of phandles to be used for various
components. I'd certainly be happy to add a directive to dtc which
enforces allocation of phandles within a specified range.
Really doubtful IMHO. That will impose yet more structure on the DT
syntax, and it will make it even more difficult for users.
Um.. I really don't see what's so hard about adding an incantation
like:
/phandle-range/ 1-0x;
at the top of your base dts. Then
/phandle-range/ 0x1-0x1;
to the plugin dts.
Especially if we made the most common base range the default.
We're talking about users that do understand the hardware, but can't
really grok linux kernel development.
DT is used a structured h/w definition and seems to work just
fine for these kind of users.
2) Resolving phandle references within a subtree
If we can handle (1) by convention, we don't need anything here, the
references are fine as is.
(3) Resolving phandle references from the subtree to the main tree.
So, I think this can actually be avoided, at least in cases where what
physical connections are available to the expansion module is well
defined. The main causes to have external references are interrupts
and gpios. Interrupts we handle by defining an interrupt specifier
space for the interrupts available on the expansion
socket/connector/bus/whatever. In the master tree we then have
something like:
...
expansion-socket@ {
expansion-id = SlotA;
interrupt-map = /* map expansion irq specs to
board interrupt controllers */ ;
interrupt-map-mask = ... ;
ranges = /* map expansion local addresses to global
mmio */ ;
};
The subtree for the expansion module gets attached as a subnode of
this one. It doesn't use explicit interrupt-parents but instead just
uses the expansion local irq specifiers, letting the parent be the
default which will bubble up to this socket node where the
interrupt-map will send them to the right places.
I don't recall the gpio bindings off hand, but as I recall we based
them off the irq tree bindings so we ought to be able to do the same
thing for them.
Please, don't try to do it this way. It is very debatable that we'll
identify all the types that will need special handling.
For example for our cape use, we already have references that do not
follow this example.
A generic way to resolve references is what we need.
Hrm... *mutters dubiously*.
--
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
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Device Tree Overlays Proposal (Was Re: capebus moving omap_devices to mach-omap2)
On Fri, Nov 09, 2012 at 09:08:14PM +, Grant Likely wrote:
On Fri, Nov 9, 2012 at 2:26 AM, David Gibson
da...@gibson.dropbear.id.au wrote:
Summary points:
- Create an FDT overlay data format and usage model
- SHALL reliable resolve or validate of phandles between base and
overlay trees
So, I'm not at all clear on what this proposed phandle validation
would involve. I'm also not convinced it's as necessary as you
think, more on that below.
Simply this: I'm taking this example from the omap3-beagle-xm.dts. It
has the following stanza which is currently rolled into the resulting
.dtb when compiled.
i2c1 {
clock-frequency = 260;
twl: twl@48 {
reg = 0x48;
interrupts = 7; /* SYS_NIRQ cascaded to intc */
interrupt-parent = intc;
vsim: regulator-vsim {
compatible = ti,twl4030-vsim;
regulator-min-microvolt = 180;
regulator-max-microvolt = 300;
};
twl_audio: audio {
compatible = ti,twl4030-audio;
codec {
};
};
};
};
However, if it were compiled into a separate dtb overlay it might look
like this:
/ {
.readonly;
ocp {
.readonly;
interrupt-controller@4820 {
phandle = 0x1234; /* EXPECTED PHANDLE */
.readonly;
};
i2c@4807 {
.must-exist;
clock-frequency = 260;
twl@48 {
reg = 0x48;
interrupts = 7;
interrupt-parent = 0x1234; /* RESOLVED PHANDLE */
vsim: regulator-vsim {
compatible = ti,twl4030-vsim;
regulator-min-microvolt = 180;
regulator-max-microvolt = 300;
};
twl_audio: audio {
compatible = ti,twl4030-audio;
codec {
};
};
};
};
};
};
Notice I've included the intc node and it's phandle. By phandle
validation I merely mean that when applying an overly the firmware or
kernel must verify that the phandles in the overlay match the phandle
in the base tree. If they don't match, then refuse to apply the
overhead. This approach avoids the need to find and fixup phandles in
the overlay. And if the phandle is generated from a hash of the
full_name, then the resulting phandle will only change if the node
moves.
Similarly, at application time it should be verified that the nodes
with a .readonly or .must-exist property could be verified to actually
exist before attempting to apply the overlay. I used two different
properties with the idea that only certain nodes would need to be
modified... exactly what the policies should be is yet to be
determined.
Ok, I see. I really don't like it much, but I understand.
--
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
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Device Tree Overlays Proposal (Was Re: capebus moving omap_devices to mach-omap2)
On Fri, Nov 09, 2012 at 09:42:37PM +, Grant Likely wrote:
On Fri, Nov 9, 2012 at 2:26 AM, David Gibson
da...@gibson.dropbear.id.au wrote:
(3) Resolving phandle references from the subtree to the main tree.
So, I think this can actually be avoided, at least in cases where what
physical connections are available to the expansion module is well
defined. The main causes to have external references are interrupts
and gpios. Interrupts we handle by defining an interrupt specifier
space for the interrupts available on the expansion
socket/connector/bus/whatever. In the master tree we then have
something like:
...
expansion-socket@ {
expansion-id = SlotA;
interrupt-map = /* map expansion irq specs to
board interrupt controllers */ ;
interrupt-map-mask = ... ;
ranges = /* map expansion local addresses to global
mmio */ ;
};
The subtree for the expansion module gets attached as a subnode of
this one. It doesn't use explicit interrupt-parents but instead just
uses the expansion local irq specifiers, letting the parent be the
default which will bubble up to this socket node where the
interrupt-map will send them to the right places.
I don't recall the gpio bindings off hand, but as I recall we based
them off the irq tree bindings so we ought to be able to do the same
thing for them.
Likewise, if there are several interchangeable expansion sockets that
have some address bits hard wired to distinguish them, we can just use
socket local mmio addresses within the subtree and the ranges property
here will sort that out.
If I'm reading correctly, the suggestion is that everything be grafted
below a single node and all connections route through that node using
mapping. Correct?
For interrupts that works today
For gpios, it isn't currently supported, but we could do it.
For SPI it would mean that the new spi devices would not appear below
the actual spi bus they are attached to
For I2C, MDIO, and one wire, same problem.
For memory mapped devices, the expansion node would need to a ranges
for all the windows that map through it, and it assumes only one
memory mapped bus (or at least it prefers only one memory mapped bus.
If there were more than one then the expansion node placement wouldn't
have a natural place to sit)
The problem is that this is not like a PCI bus where there is only one
kind of interface. It is a whole bunch of interfaces that happen to be
grouped together loosely (as an expansion connector in the beaglebone
case, but expansion isn't the only problem that I'm hearing about).
So, with a group of i2c, spi, memory mapped and other devices than are
all plugged in together, how does that look? They really should not
sit on the same level. An spi device cannot be a peer of an i2c device
for instance, the address mapping is entirely different. The kernel
really wants i2c devices to be a child of the actual i2c bus which may
already have an i2c device or too on the main board. Does the
expansion node need to have some kind of redirect node for each of the
busses where the children of it need to create devices as children of
the master bus?
To me that seems to get really complex in a hurry. More complex than
the overlay approach.
Ah, yes, I see. Yeah, that's a genuine showstopper for my original
proposal. Ok, let me offer a couple of counter-proposals:
1) bus-ranges
The notion of bus-reg and bus-ranges properties is something I've
toyed with before for other reasons, as a way of augmenting the
normal DT tree structure with interrupt-tree like DAG sections.
bus-reg and bus-ranges would act like the normal reg and ranges
properties except that each entry includes a phandle explicitly giving
the parent bus. In that case a suitable 'bus-ranges' in the socket
node would resurrect my graft proposal.
I'm not particularly sold on this idea, but I mention it because it
has applications other than this one. In particular it would mean we
could avoid having two different nodes for a device which is mostly
accessed via MMIO but also has a few sideband registers controlled by
i2c (or whatever).
2) graft bundle
The base tree has something like this:
...
i2c@XXX {
...
cape-socket {
compatible = vendor,cape-socket;
id = Socket-A;
piece-id = i2c;
ranges = ... ;
};
};
...
spi@YYY {
...
cape-socket {
compatible = vendor,cape-socket;
id = Socket-A;
piece-id = spi;
ranges = ... ;
};
};
...
cape-socket
Re: [RFC] Device Tree Overlays Proposal (Was Re: capebus moving omap_devices to mach-omap2)
On Fri, Nov 09, 2012 at 09:36:26PM -0600, Joel A Fernandes wrote: Hi Pantelis, On Fri, Nov 9, 2012 at 2:13 AM, Pantelis Antoniou pa...@antoniou-consulting.com wrote: Option C: U-Boot loads both the base and overlay FDT files, merges them, and passes the resolved tree to the kernel. Could be made to work. Only really required if Joanne wants the cape interface to work for u-boot too. For example if the cape has some kind of network interface that u-boot will use to boot from. I love Grant's hashing idea a lot keeping the phandle problem for compile time and not requiring fixups. IMO it is still a cleaner approach if u-boot does the tree merging for all cases, and not the kernel. That way from a development standpoint, very little or nothing will have to be changed in kernel (except for scripts/dtc) considering we are moving forward with hashing. Also this discussed a while back but at some point is going to brought up again- loading of dt fragment directly from EEPROM and merging at run time. If we were to implement this in kernel, we would have to add cape specific EEPROM reading code, merge the tree before it is unflattened and parse. I think doing tree merging in kernel is messy and we should do it in uboot. Ideally reading the fragment from the EEPROM for all capes and merging without worrying about version detection, Doing the merge and passing the merged blob to the kernel which (kernel) works the same way it does today. Not going to work, for a lot of cases. Doing it in the kernel seems to be the cleaner option. There are valid use cases for doing in u-boot too. True, if dynamic runtime stuff from userspace is what we're talking about, then yeah I see the important need for kernel to do the merge. Alternatively to hashing, reading david gibsons paper I followed, phandle is supposed to 'uniquely' identity node. I wonder why the node name itself is not sufficient to unquiely identify. The code that does the tree walking can then just strcmp the node name while it walks the tree instead of having to find a node with a phandle number. I guess the reason is phandles are small to store as data values. Another approach can be to arrange the string block in alphabetical order (unless it already is), and store phandle as index of the node name referenced relative to the starting of the strong block. This will not affect nodes in dtb being moved around since they will still have the same index value. the problem being adding or removing nodes Changes the offsets of all other nodes in the string block as well.. Hmm. This is pretty radical change to the DT format, no? Yes, true and the only way hypothetically to replace the phandle tree-walking mechanism is to store node paths instead of phandle which David pointed is too long to store, so I guess this wont work after all. Anyway this was an interesting exercise, thanks. They're not too long to store, but changing to paths would break years of existing OF practice. -- 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 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Device Tree Overlays Proposal (Was Re: capebus moving omap_devices to mach-omap2)
On Mon, Nov 12, 2012 at 10:22:07PM -0700, Stephen Warren wrote:
On 11/12/2012 06:05 PM, David Gibson wrote:
On Fri, Nov 09, 2012 at 09:42:37PM +, Grant Likely wrote:
...
2) graft bundle
The base tree has something like this:
...
i2c@XXX {
...
cape-socket {
compatible = vendor,cape-socket;
id = Socket-A;
piece-id = i2c;
ranges = ... ;
};
};
...
spi@YYY {
...
cape-socket {
compatible = vendor,cape-socket;
id = Socket-A;
piece-id = spi;
ranges = ... ;
};
};
...
cape-socket {
compatible = vendor,cape-socket;
id = Socket-A;
piece-id = misc;
interrupt-map = ... ;
interrupt-map-mask = ... ;
gpio-map = ... ;
gpio-map-mask = ... ;
};
Then instead of grafting a single subtree for the socket, we install a
bundle of subtrees, one each for each of the pieces within the
socket. That bundle could either be an actual set of multiple fdts,
or they could be placed into one fdt with a dummy root node, something like:
/ {
plugin-bundle;
compatible = vendor,cape-plugin;
version = ...;
i2c-piece = {
piece-id = i2c;
...
};
misc-piece = {
piece-id = misc;
...
};
};
I do like this approach; it's the kind of thing I proposed at:
http://www.mail-archive.com/devicetree-discuss@lists.ozlabs.org/msg20414.html
Roughly, yes, though a little streamlined from the syntax suggested
there.
One question though: Perhaps the base board has two instances of the
same type of connector vendor,cape-socket, allowing 2 independent capes
to be plugged in. When overlaying/grafting the child board's .dts, we'd
need some way to specify the socket ID that was being plugged into. Is
that the intent of the id property in your base board example above?
Yes, that's exactly what I had in mind for the id property.
Property names and other details entirely negotiable at this stage,
of course.
By the by, I think having multiple interchangable sockets could break
the convention based approach for avoiding collisions between phandles
I suggested, but another mail with some better thoughts on that
shortly to be posted.
--
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
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Device Tree Overlays Proposal (Was Re: capebus moving omap_devices to mach-omap2)
On Mon, Nov 12, 2012 at 09:52:32AM -0700, Stephen Warren wrote:
On 11/12/2012 05:10 AM, Pantelis Antoniou wrote:
[snip]
Oh yes. In fact if one was to use a single kernel image for beagleboard
and beaglebone, for the cape to work for both, it is required for it's
dtb to be compatible.
Well, as Grant pointed out, it's not actually strictly necessary for the
.dtb to be compatible; only the .dts /need/ be compatible, and the .dtb
can be generated at run-time using dtc for example.
So, actually, I think a whole bunch of problems with phandle
resolution disappear if we don't try to define an overlay .dtb format,
or at least treat it only as a very shortlived object. A more precise
proposal below. Note that this works more or less equally well with
either the original overlay approach or the graft/graft-bundle
proposal I made elsewhere.
1) We annotate the base tree with some extra label information for
nodes which overlays are likely to want to reference by phandle. e.g.
beaglebone_pic: interrupt-controller@X {
...
phandle,symbolic-name = beaglebone_pic;
};
We could extend dtc to (optionally?) auto-generate those properties
from its existing label syntax. Not sure if that's a good idea or
not yet. In any case, we compile this augmented base tree to .dtb as
normal and boot our kernel with it.
2) The information for the capes/modules/whatever is
distributed/packaged as .dts, never .dtb. When userspace detects the
new module (or the user explicitly tells it, if it's not probeable) it
picks up the correct dts and runs it through dtc in a special mode.
In this mode dtc takes the existing base tree (from /proc/device-tree,
say) as well as the new dts. In this mode, dtc allocates phandles for
the new tree fragment so as not to collide with anything from the
supplied base tree (as well as avoiding internal conflicts,
obviously). It also allows node references to the base tree by using
those label annotations from (1) to match symbolic names to the
phandle values in the base tree.
3) The resulting partial .dtb for the module is highly specific to the
base tree (which if the base tree was generated at runtime by firmware
could even be specific to a particular boot). But that's ok, because
we just spit it into the kernel, absolute phandle values and all, then
throw it away. Next time we need the module info, we recompile it
again.
Of course, relying on .dts compatibility rather than .dtb compatibility
might negatively impact the complexity of an initrd environment if we
end up loading overlays from there...
Well, it does mean we'd need dtc in the initrd. But dtc has no
library dependencies except libc, so that really shouldn't be too
bad. In return we entirely avoid inventing a new phandle resolution
protocol.
--
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
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Device Tree Overlays Proposal (Was Re: capebus moving omap_devices to mach-omap2)
On Fri, Nov 09, 2012 at 12:32:09AM -0500, Joel A Fernandes wrote: Hi Pantelis, I hope I'm not too late to reply as I'm traveling. On Nov 6, 2012, at 5:30 AM, Pantelis Antoniou pa...@antoniou-consulting.com wrote: Joanne has purchased one of Jane's capes and packaged it into a rugged case for data logging. As far as Joanne is concerned, the BeagleBone and cape together are a single unit and she'd prefer a single monolithic FDT instead of using an FDT overlay. Option A: Using dtc, she uses the BeagleBone and cape .dts source files to generate a single .dtb for the entire system which is loaded by U-Boot. -or- Unlikely. Option B: Joanne uses a tool to merge the BeagleBone and cape .dtb files (instead of .dts files), -or- Possible but low probability. Option C: U-Boot loads both the base and overlay FDT files, merges them, and passes the resolved tree to the kernel. Could be made to work. Only really required if Joanne wants the cape interface to work for u-boot too. For example if the cape has some kind of network interface that u-boot will use to boot from. I love Grant's hashing idea a lot keeping the phandle problem for compile time and not requiring fixups. Well, using a hash only moves the problem of fixed phandles to a problem of fixed node paths. The details of node paths are, if anything, more mutable than phandles. [snip] Alternatively to hashing, reading David Gibson's paper I followed, phandle is supposed to 'uniquely' identity node. I wonder why the node name itself is not sufficient to uniquely identify. Node names are not unique, not even close. If you have two similar NICs in slot 0 of two different PCI domains, they'll almost certainly both be called 'ethernet@0,0'. Similar examples abound on other buses. Node paths are unique, but they are long. The other big reason for phandles in OF history is that they would be more stable than paths. The device tree could be manipulated during OF runtime, but phandles would generally be internal pointers in OF and so remain a consistent handle even if the node moved in the tree. That's not really relevant for flat trees, but we need to work with the same structures. The code that does the tree walking can then just strcmp the node name while it walks the tree instead of having to find a node with a phandle number. I guess the reason is phandles are small to store as data values. Another approach can be to arrange the string block in alphabetical order (unless it already is), They're not, and doing so would be a painful change to maintain compatibility across. And in any case only property names use the strings block, not node names. and store phandle as index of the node name referenced relative to the starting of the strong block. This will not affect nodes in dtb being moved around since they will still have the same index value. the problem being adding or removing nodes Changes the index of all other nodes in the string block as well.. Hmm. -- 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 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Device Tree Overlays Proposal (Was Re: capebus moving omap_devices to mach-omap2)
, letting the parent be the default which will bubble up to this socket node where the interrupt-map will send them to the right places. I don't recall the gpio bindings off hand, but as I recall we based them off the irq tree bindings so we ought to be able to do the same thing for them. Likewise, if there are several interchangeable expansion sockets that have some address bits hard wired to distinguish them, we can just use socket local mmio addresses within the subtree and the ranges property here will sort that out. -- 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 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Mis?use of aliases
On Sat, Jul 14, 2012 at 07:07:17AM -1000, Mitch Bradley wrote: On 7/14/2012 6:37 AM, David Gibson wrote: On Fri, Jul 13, 2012 at 07:30:42PM -1000, Mitch Bradley wrote: I'm not sure this is really a good use of aliases. UARTs use aliases because it is important that the UART number to tty number is known and fixed. This brings up an issue that I've been meaning to comment on. The use of phandle-valued properties in the aliases node causes real OFW implementations some amount of heartburn. The Open Firmware standard says that the properties in /aliases are string-valued. That's important, because aliases are shorthand for fragments of full device specifiers (pathnames that can include arguments to nodes). Phandles can point to nodes, but can't be relative, and can't encode per-node-component arguments. Um, so, properties in /aliases should not have phandle values, flat tree or otherwise. Has this been seen in the wild, or are you being misled by the fact that dtc's reference-to-phandle and reference-to-path syntax is very similar: Yes, I was indeed being misled. Thanks for the clarification. The fred syntax is present in the .dts files that I have looked at. Right, it's all about the context. label is always a reference to another node, but in cell context ... that's expanded as a phandle, in bare context it's expanded as a path. prop = fred; Will generate a phandle valued property, but prop = fred; Will generate a string (path) valued property. For binding a Linux unit number to a device node, I would prefer to decorate the node with a property like linux,unit#, instead of breaking the standard semantics of /aliases. I don't see how using aliases for unit numbering (inherently) breaks the semantics of /aliases. If phandle valued properties are being used that is wrong, but it's not necessary for the unit numbering anyway. I agree, the use of string-valued /aliases is not a semantic problem. That said, I still think that decorating individual nodes is a better approach, for locality reasons. But, now that my misunderstanding has been cleared up, it's a mild preference instead of heartburn. So, I think aliases was suggested for this sort of numbering precisely because it _isn't_ local. Particularly when numbering similar devices across unrelated busses, the ordering more or less has to be a global platform convention, and may not be derived from - or even contradict - local numbering conventions from individual busses or components. -- 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 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Mis?use of aliases
On Fri, Jul 13, 2012 at 07:30:42PM -1000, Mitch Bradley wrote: I'm not sure this is really a good use of aliases. UARTs use aliases because it is important that the UART number to tty number is known and fixed. This brings up an issue that I've been meaning to comment on. The use of phandle-valued properties in the aliases node causes real OFW implementations some amount of heartburn. The Open Firmware standard says that the properties in /aliases are string-valued. That's important, because aliases are shorthand for fragments of full device specifiers (pathnames that can include arguments to nodes). Phandles can point to nodes, but can't be relative, and can't encode per-node-component arguments. Um, so, properties in /aliases should not have phandle values, flat tree or otherwise. Has this been seen in the wild, or are you being misled by the fact that dtc's reference-to-phandle and reference-to-path syntax is very similar: prop = fred; Will generate a phandle valued property, but prop = fred; Will generate a string (path) valued property. For binding a Linux unit number to a device node, I would prefer to decorate the node with a property like linux,unit#, instead of breaking the standard semantics of /aliases. I don't see how using aliases for unit numbering (inherently) breaks the semantics of /aliases. If phandle valued properties are being used that is wrong, but it's not necessary for the unit numbering anyway. -- 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 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How to handle named resources with DT?
On Tue, Aug 30, 2011 at 12:27:24PM +0300, Felipe Balbi wrote: Hi, On Tue, Aug 30, 2011 at 12:29:12PM +1000, David Gibson wrote: So, I actually agree that in the long term getting resource names in the DT would be a generally good thing. But doing so is a *huge* change in one of the very core semantics of all the DT bindings. It's not something that should be done lightly or quickly. It absolutely should not be tied to how this is handled the longer you take to change, the more complex will it be to change. No, not really. The longer we spend discussing the validity of _byname(), more boards/archs/whatnot will be converted to DT without _byname() and after a certain amount of them are converted, noone will be willing to change and validate everything again. I'm not discussing the validity of _byname (Russell King is, but that's not an argument I have a position of). What I'm saying is that the kernel internal use of byname, and named resources in the DT are different things which should be approached independently. -- 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 signature.asc Description: Digital signature
Re: How to handle named resources with DT?
On Fri, Aug 26, 2011 at 04:13:15PM +0200, Cousson, Benoit wrote: On 8/26/2011 12:58 PM, Arnd Bergmann wrote: On Friday 26 August 2011, David Gibson wrote: Seriously, how many times do I have to say it? [...] Insisting that the names come from the DT is to mistakenly think of the DT as an extension of the kernel's internal interfaces, instead of as the external and OS neutral data structure it actually is. You are wrongly interpreting the consequence: a smart Linux guy added a _byname API, without seeing the real cause: most HW resources have a name to identify them and not a number. No, I'm really not. Agreed, that was my main point anyway: Getting the name from the device tree would be a huge mistake. No, it would not. In fact, storing name in DT is even much more aligned with the goal of DT for my point of view, since it is supposed to describe the HW without any assumption about the OS usage. DT data are supposed to be driven by the HW specs. So, I actually agree that in the long term getting resource names in the DT would be a generally good thing. But doing so is a *huge* change in one of the very core semantics of all the DT bindings. It's not something that should be done lightly or quickly. It absolutely should not be tied to how this is handled on the Linux side. Therefore *for now* it is much better to have glue which binds existing DT practice to the new Linux interface; then thinking about this from the DT point of view can be a long term project. The ordering you are relying on for the moment is purely arbitrary and do not have any signification for the HW point of view. Just have a look at a HW spec and you will see that most signals have a *name*, not a number, to identify them. Sure, but assigning that arbitrary order is well-established practice for device DT bindings. Without that, you have to add some unnecessary and error prone processing to the original information: - Encode an information that is there originally (resource name from the HW spec) into a arbitrary number without any meaning: Why tx_irq should be before the rx_irq and after the err_irq??? - Remove the original name to confuse people. - Expect the driver to use a number that does not come from the HW spec but from a DT binding text file to figure out what resource it has to use. No, I don't expect that bit - see the discussion of how we can glue the DT order to names inside the kernel. - Pray that the driver guy didn't wrongly interpret the irq #2138469 to be the tx_irq instead of the rx_irq. Um, this is an order, not an arbitrary int number. So try irq #2. Maybe #3 or #4 on a really complicated device. If you extend a little bit the scope of the discussion and start considering that clocks, voltage rails, reset lines are as well a resources for the IP point of view, do you really think that using a number to identify a clock or a voltage will really make sense? Guess what? The current clock binding is using clock name... Yes. There's a big difference between creating a new binding to use names, and creating a new practice in the core DT semantics used by every single existing binding. In order to have a consistent way of using resources in DT, it makes sense to have the ability to provide a name for any kind of resources. BTW, adding a name should not prevent people to use the legacy by index method. Moreover, anybody deserve to have a name... Otherwise we will end up with situation like that: resource #6: Who are you? resource #2: The new #2. resource #6: Who is #1? resource #2: You are #6. resource #6: I am not a number, I am a free resource. Allusions to TV programs do not an argument make, no matter how interesting and influential they might have been. -- 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 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How to handle named resources with DT?
On Sat, Aug 27, 2011 at 08:13:59PM +0200, Arnd Bergmann wrote:
On Sunday 28 August 2011 00:37:36 David Gibson wrote:
On Fri, Aug 26, 2011 at 05:41:29PM +0200, Arnd Bergmann wrote:
On Friday 26 August 2011, Arnd Bergmann wrote:
On Friday 26 August 2011, David Gibson wrote:
If you open code it this way then yes, it's silly. But what about
something like this:
static struct of_device_id foodevice_of_match[] __devinitdata = {
{ .compatible = foocorp,foodevice1234,
.resource_names = {base_regs, extra_regs, }, },
{ .compatible = foocorp,foodevice1239,
.resource_names = {base_regs, extra_regs, more_regs,
}, },
{ },
};
Hmm, I hadn't thought of that. This looks quite nice indeed. No
objections
to this from my side.
Ah well, one objection on second thought:
This assumes that there is just one type of resource, but named resources
may be used for iomem, ioport and irq resources. If you have multiple
IRQs and multiple IOMEM resources, I don't see how the index in the
resource_names array can be used for both of them.
Details, shmetails, so you have both 'reg_names' and 'interrupt_names'.
Right, and I guess we can simply ignore DMA and ioport resources because they
are extremely rare, right?
Well, remember it's where resources can appear in the DT node that
matters, not what the types are in the platform device. ioports will
typically appear suitably encoded in 'reg', so that's covered. I've
never been very clear on what exactly DMA resources cover, but yeah,
you might need something for dma-reg or other device tree properties.
One more detail: IIRC struct of_device_id is exported to module_init_tools
for purposes of autoloading, so changing the structure breaks user
space.
Ah. That is a bit more of a problem.
--
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
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How to handle named resources with DT?
On Fri, Aug 26, 2011 at 05:41:29PM +0200, Arnd Bergmann wrote:
On Friday 26 August 2011, Arnd Bergmann wrote:
On Friday 26 August 2011, David Gibson wrote:
If you open code it this way then yes, it's silly. But what about
something like this:
static struct of_device_id foodevice_of_match[] __devinitdata = {
{ .compatible = foocorp,foodevice1234,
.resource_names = {base_regs, extra_regs, }, },
{ .compatible = foocorp,foodevice1239,
.resource_names = {base_regs, extra_regs, more_regs, },
},
{ },
};
Hmm, I hadn't thought of that. This looks quite nice indeed. No objections
to this from my side.
Ah well, one objection on second thought:
This assumes that there is just one type of resource, but named resources
may be used for iomem, ioport and irq resources. If you have multiple
IRQs and multiple IOMEM resources, I don't see how the index in the
resource_names array can be used for both of them.
Details, shmetails, so you have both 'reg_names' and 'interrupt_names'.
--
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
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How to handle named resources with DT?
On Fri, Aug 26, 2011 at 12:58:32PM +0200, Arnd Bergmann wrote:
On Friday 26 August 2011, David Gibson wrote:
Seriously, how many times do I have to say it?
Using _byname in drivers DOES NOT require adding names to the DT.
All it needs is some glue logic to attach names as the device tree is
read. This is properly thought of as part of the code which
translates the device tree into struct platform_device, not as part of
drivers proper.
But if you do such code, the only logical place for it to live would
be in that driver, otherwise you end up with multiple places in the
kernel source tree that deal with the same devices and need to be
updated in lock-step. Getting away from this mess is one of the main
reasons for converting to device tree based probing in the first place.
Obviously it would be in the driver file, but I'd think of it more as
some metadata attached to the driver, rather than truly part of the
driver.
FURTHERMORE, even if there were names in the DT, you'd still need
this glue logic, so that drivers converted to _byname could still use
old device trees.
I don't think anyone was talking about converting driver /to/ the
_byname method, the debate is mostly whether we should move away from
that while we convert drivers to no longer rely on board code but
instead allow them to be probed from the device tree.
Insisting that the names come from the DT is to mistakenly think of
the DT as an extension of the kernel's internal interfaces, instead of
as the external and OS neutral data structure it actually is.
Agreed, that was my main point anyway: Getting the name from the
device tree would be a huge mistake. By comparison, the scenario you
describe -- adding another identifier to struct resource and initializing
from the driver that consumes it -- would not be harmful at all, just a
little silly when you end up with a probe function like
If you open code it this way then yes, it's silly. But what about
something like this:
static struct of_device_id foodevice_of_match[] __devinitdata = {
{ .compatible = foocorp,foodevice1234,
.resource_names = {base_regs, extra_regs, }, },
{ .compatible = foocorp,foodevice1239,
.resource_names = {base_regs, extra_regs, more_regs, }, },
{ },
};
--
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
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How to handle named resources with DT?
On Thu, Aug 25, 2011 at 11:16:25AM -0700, Kevin Hilman wrote: Arnd Bergmann a...@arndb.de writes: On Thursday 25 August 2011, Russell King - ARM Linux wrote: On Thu, Aug 25, 2011 at 02:16:14AM +0300, Felipe Balbi wrote: on top of all that, for IPs which are used on many SoCs (such as MUSB) it's quite silly to force all users to provide resources in a certain order. It sounds to me that this will be prone to error in many ways until everything is synced up and on the correct order. Ditching _byname is a very bad idea. I continue to disagree. The current _byname is an abonimation and hack to try to fix this problem. _byname should have been implemented differently - rather than overriding the resources name field (which is normally specified to be the device or driver name), a new field should have been introduced in struct resource to carry the resource sub-name (which is really what it is.) That would have avoided making /proc/iomem completely illegible with multiple devices using this feature. I agree 100%. Please clarify. What I hear Russell saying is a problem with the *implementation* of the _byname API. What I hear you sating is that since DT doesn't support this, we need to remove it's usage completely from platform_devices also. These are two very different approaches. Fixing the implementation as Russell suggested seems relatively easy, and conceptually similar to adding it to the DT. Removing _byname all together seems like significant work just to avoid adding a feature to the DT core. Seriously, how many times do I have to say it? Using _byname in drivers DOES NOT require adding names to the DT. All it needs is some glue logic to attach names as the device tree is read. This is properly thought of as part of the code which translates the device tree into struct platform_device, not as part of drivers proper. FURTHERMORE, even if there were names in the DT, you'd *still* need this glue logic, so that drivers converted to _byname could still use old device trees. Insisting that the names come from the DT is to mistakenly think of the DT as an extension of the kernel's internal interfaces, instead of as the external and OS neutral data structure it actually is. -- 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 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How to handle named resources with DT?
On Thu, Aug 11, 2011 at 02:28:55PM +0200, Cousson, Benoit wrote: On 8/10/2011 9:22 PM, Grant Likely wrote: On Tue, Aug 9, 2011 at 7:52 PM, David Gibson da...@gibson.dropbear.id.au wrote: On Tue, Aug 09, 2011 at 11:53:32PM +0200, Cousson, Benoit wrote: On 8/9/2011 11:49 PM, Grant Likely wrote: That won't work either because that also breaks the existing 'reg' binding. Anything you do will need to supplement the existing binding without changing it in an incompatible way. OK, but can we add a new attribute then? reg2, reg_ng, reg_plusplus, reg_named...? He already suggested reg-names to be interpreted in parallel with the existing reg property. The (serious) problem with replacing the reg property is that it will break all existing OSes (including old Linux versions) that don't understand the new property. Of course, the problem with reg-names is that it will be ignored by older OSes, and so 'reg' must still be in the correct order. In which case you could argue it's more sensible to just have a static place to name mapping in the Linux driver. In short, yes, named reg elements in the DT would be nice in theory, but I'm not convinced it's worth a DT flag day to accomplish it. I'm inclined the same way, though I agree with the replies that point out it wouldn't result in a 'flag day' because existing bindings cannot become incompatible. The problem I have is that adding reg-names or similar implies that ordering of the reg property is no longer defined which I absolutely do not think is a good idea. That will not be an issue if reg-named is a completely new node. It will replace the reg node only when a named entry is needed. Most devices will use the regular reg entry, and only the one that need extra information will use the reg-named. Um, you seem to be confusing nodes and properties here. That seems to be pretty straightforward to implement, and as soon as it is useful even for a couple of drivers, it worth adding it. It is anyway better than having to add a custom property to get the information we will miss otherwise. Moreover, since some drivers are relying on that call, it will avoid having to add extra code for nothing if CONFIG_OF is set. It will allow the driver to use a pretty standard API in anycase vs using platform_get_resource + some extra optional calls to of_ functions + some code to get the information for non-DT build. You don't need to add stuff to the DT to use the byname interface. Really. All you need is a way for the driver (well match table entry, really) to provide a list of names to attach to the reg entries in order. Please, stick with the established convention and explicitly order 'reg' and 'interrupts' properties. If there is a specific use case where this doesn't work, then bring it up, but I haven't seen any yet. There will always be some alternatives, but they will be uglier, and the effort to add some extra node to DT is so small, that it is better to do that instead of adding some useless extra code in the driver. The current users of _byname that I've looked seem to only use it for convenience, not because a register range may be optional. ie. they all fail out if a reg resource isn't present. If that API can help the driver writer and can avoid adding 10 lines of code, it is still useful to use it. To be honest, I still do not understand why you are so reluctant to add that small feature. Because this is a totally new basic feature to add to the DT bindings. Once in there, we're stuck with it indefinitely, which means it warrants considerably more conservatism than mere internal code changes which can be easily updated or reverted. That will not break compatibility and it will make our life easier. Don't you want to make our life easier? :-) Benoit -- 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 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How to handle named resources with DT?
On Tue, Aug 09, 2011 at 11:23:45AM -0600, Grant Likely wrote: On Tue, Aug 9, 2011 at 10:57 AM, Cousson, Benoit b-cous...@ti.com wrote: Hi Manju, On 8/9/2011 6:29 PM, G, Manjunath Kondaiah wrote: Hi Benoit, On Tue, Aug 09, 2011 at 11:23:20AM +0200, Cousson, Benoit wrote: Hi Grant, Trying to bind hwmod informations with DT, I'm facing a little limitation. A bunch of drivers are using the platform_get_resource_byname, so the name for the resource is needed. The name is used so far for IORESOURCE_MEM, IORESOURCE_IRQ and IORESOURCE_DMA types of resources. IORESOURCE_MEM and IORESOURCE_IRQ's are fetched from dt blob and it will be part of pdev. Yes, but without the proper name in the resource structure. It will be then impossible to use the platform_get_resource_byname function that is currently used by a bunch of drivers. There is no analogous mechanism for _byname in the device tree. The DT binding for a device must explicitly state what order the register ranges are in. The driver will need to be adapted. Well, the driver proper shouldn't need changing. It should be possible to assign the correct names in the glue which translates the DT reg entries into the Linux resource structures. The difficulty with adding th enames to the DT itself is that it exposes the essentially Linux-specific names in what's supposed to be an OS neutral data structure. -- 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 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How to handle named resources with DT?
On Tue, Aug 09, 2011 at 11:53:32PM +0200, Cousson, Benoit wrote:
On 8/9/2011 11:49 PM, Grant Likely wrote:
On Tue, Aug 09, 2011 at 11:44:35PM +0200, Cousson, Benoit wrote:
On 8/9/2011 11:17 PM, Grant Likely wrote:
On Tue, Aug 09, 2011 at 11:08:09PM +0200, Cousson, Benoit wrote:
On 8/9/2011 10:57 PM, Grant Likely wrote:
On Tue, Aug 09, 2011 at 01:26:29PM -0500, Scott Wood wrote:
On 08/09/2011 12:47 PM, Cousson, Benoit wrote:
On 8/9/2011 7:23 PM, Grant Likely wrote:
There is no analogous mechanism for _byname in the device tree. The
DT binding for a device must explicitly state what order the register
ranges are in. The driver will need to be adapted.
That seems to be a small regression for my point of view. Relying on
the
order is not super safe. This is not very readable either. That's for
that exact reason that we changed our drivers to use
platform_get_resource_byname. That's probably the reason why that API
is
there as well.
For the same IP, the number of entries can vary depending of the SoC
revision.
By using the _byname, we can check if the resource is there or not
without having to care about the position.
You could have a named u32 property that contains the reg index, e.g.:
dev {
reg =0x2 0x200 0x24000 0x200;
foo-reg =0;
bar-reg =1;
};
That's a little nasty. A reg-names = foo, bar; would probably be
better.
Yep, I agree.
And what about something like that?
reg =0x2 0x200, foo,
0x2 0x200, bar ;
It is doable?
Definitely not. It would break all existing 'reg' parsing
implementations quite badly.
OK, so what about extending the reg attribute to be a reg node?
dev {
reg {
name = foo_wrapper;
start =0x1;
end =0x200;
}
reg {
name = foo;
start =0x2;
end =0x200;
}
}
A little bit more verbose, but at least we can add any attribute we want.
That won't work either because that also breaks the existing 'reg'
binding. Anything you do will need to supplement the existing
binding without changing it in an incompatible way.
OK, but can we add a new attribute then? reg2, reg_ng, reg_plusplus,
reg_named...?
He already suggested reg-names to be interpreted in parallel with the
existing reg property. The (serious) problem with replacing the reg
property is that it will break all existing OSes (including old Linux
versions) that don't understand the new property.
Of course, the problem with reg-names is that it will be ignored by
older OSes, and so 'reg' must still be in the correct order. In which
case you could argue it's more sensible to just have a static place to
name mapping in the Linux driver.
In short, yes, named reg elements in the DT would be nice in theory,
but I'm not convinced it's worth a DT flag day to accomplish it.
--
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
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
