Skip to content

Commit

Permalink
edtlib: link child nodes to parent for nodes with child-bindings
Browse files Browse the repository at this point in the history
The current EDT graph logic only use properties directly under a
specific node to add dependencies. For nodes properties in
child-bindings, this means that the child phandles are only linked by
the child node itself, which does have an ordinal but no corresponding
"sturct device" in the code, causing those dependencies to be silently
ignored by gen_handles.py.

Fix that by adding the recursive logic to visit child bindings when
present, which causes all child node property handles to be linked to
the parent node.

Signed-off-by: Fabio Baltieri <fabiobaltieri@google.com>
  • Loading branch information
fabiobaltieri committed Sep 14, 2023
1 parent eea897a commit c5d86f8
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 24 deletions.
75 changes: 51 additions & 24 deletions scripts/dts/python-devicetree/src/devicetree/edtlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -2053,6 +2053,56 @@ def scc_order(self) -> List[List[Node]]:
except Exception as e:
raise EDTError(e)

def _process_properties_r(self, root_node, props_node):
"""
Process props_node properties for dependencies, and add those as
dependencies of root_node. Then walk through all the props_node childs
and do the same recursively, maintaining the same root_node.
This ensures that on a node with child nodes, the parent node includes
the dependencies of all the child nodes as well as its own.
"""
# A Node depends on any Nodes present in 'phandle',
# 'phandles', or 'phandle-array' property values.
for prop in props_node.props.values():
if prop.type == 'phandle':
self._graph.add_edge(root_node, prop.val)
elif prop.type == 'phandles':
if TYPE_CHECKING:
assert isinstance(prop.val, list)
for phandle_node in prop.val:
self._graph.add_edge(root_node, phandle_node)
elif prop.type == 'phandle-array':
if TYPE_CHECKING:
assert isinstance(prop.val, list)
for cd in prop.val:
if cd is None:
continue
if TYPE_CHECKING:
assert isinstance(cd, ControllerAndData)
self._graph.add_edge(root_node, cd.controller)

# A Node depends on whatever supports the interrupts it
# generates.
for intr in props_node.interrupts:
self._graph.add_edge(root_node, intr.controller)

# If the binding defines child bindings, link the child properties to
# the root_node as well.
if props_node._binding and props_node._binding.child_binding:
for child in props_node.children.values():
if "compatible" in child.props:
# Not a child node, normal node on a different binding.
continue
self._process_properties_r(root_node, child)

def _process_properties(self, node):
"""
Add node dependencies based on own as well as child node properties,
start from the node itself.
"""
self._process_properties_r(node, node)

def _init_graph(self) -> None:
# Constructs a graph of dependencies between Node instances,
# which is usable for computing a partial order over the dependencies.
Expand All @@ -2066,30 +2116,7 @@ def _init_graph(self) -> None:
for child in node.children.values():
self._graph.add_edge(child, node)

# A Node depends on any Nodes present in 'phandle',
# 'phandles', or 'phandle-array' property values.
for prop in node.props.values():
if prop.type == 'phandle':
self._graph.add_edge(node, prop.val)
elif prop.type == 'phandles':
if TYPE_CHECKING:
assert isinstance(prop.val, list)
for phandle_node in prop.val:
self._graph.add_edge(node, phandle_node)
elif prop.type == 'phandle-array':
if TYPE_CHECKING:
assert isinstance(prop.val, list)
for cd in prop.val:
if cd is None:
continue
if TYPE_CHECKING:
assert isinstance(cd, ControllerAndData)
self._graph.add_edge(node, cd.controller)

# A Node depends on whatever supports the interrupts it
# generates.
for intr in node.interrupts:
self._graph.add_edge(node, intr.controller)
self._process_properties(node)

def _init_compat2binding(self) -> None:
# Creates self._compat2binding, a dictionary that maps
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,14 @@ child-binding:
child-prop:
type: int
required: true
child-ref:
type: phandle

child-binding:
description: grandchild node
properties:
grandchild-prop:
type: int
required: true
grandchild-ref:
type: phandle
5 changes: 5 additions & 0 deletions scripts/dts/python-devicetree/tests/test.dts
Original file line number Diff line number Diff line change
Expand Up @@ -470,16 +470,21 @@
// 'child-binding:')
//

child-binding-dep {
};

child-binding {
compatible = "top-binding";
child-1 {
child-prop = <1>;
grandchild {
grandchild-prop = <2>;
grandchild-ref = < &{/child-binding-dep} >;
};
};
child-2 {
child-prop = <3>;
child-ref = < &{/child-binding-dep} >;
};
};

Expand Down
14 changes: 14 additions & 0 deletions scripts/dts/python-devicetree/tests/test_edtlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,20 @@ def test_dependencies():
assert edt.get_node("/") in edt.get_node("/in-dir-1").depends_on
assert edt.get_node("/in-dir-1") in edt.get_node("/").required_by

def test_child_dependencies():
'''Test dependencies relashionship with child nodes propagated to parent'''
with from_here():
edt = edtlib.EDT("test.dts", ["test-bindings"])

dep_node = edt.get_node("/child-binding-dep")

assert dep_node in edt.get_node("/child-binding").depends_on
assert dep_node in edt.get_node("/child-binding/child-1/grandchild").depends_on
assert dep_node in edt.get_node("/child-binding/child-2").depends_on
assert edt.get_node("/child-binding") in dep_node.required_by
assert edt.get_node("/child-binding/child-1/grandchild") in dep_node.required_by
assert edt.get_node("/child-binding/child-2") in dep_node.required_by

def test_slice_errs(tmp_path):
'''Test error messages from the internal _slice() helper'''

Expand Down

0 comments on commit c5d86f8

Please sign in to comment.