From 7c72a6c117853dff81357205d89ca79acf4219f8 Mon Sep 17 00:00:00 2001 From: Christophe Dufaza Date: Fri, 6 Sep 2024 09:29:50 +0200 Subject: [PATCH] edtlib: preserve paths of properties from included child-bindings All child bindings are initialized at once after included binding files have been merged, with the top-level binding file set as their origin. As a consequence, properties of child bindings appear to all come from ("last modified" semantic) the top-level file. For example, properties of a channel child of an ADC controller with compatible "nordic,nrf-saadc" will all appear last modified by nordic,nrf-saadc.yaml instead of adc-controller.yaml. We should use the same approach we've already come to for property specifications: - https://github.com/zephyrproject-rtos/zephyr/issues/65135 - https://github.com/zephyrproject-rtos/zephyr/pull/65221 Signed-off-by: Christophe Dufaza --- .../src/devicetree/edtlib.py | 139 ++++++++++++++++-- .../tests/test-bindings-include/digest.yaml | 59 ++++++++ .../test-bindings-include/digest_base.yaml | 34 +++++ .../tests/test-bindings-include/left.yaml | 18 +++ .../test-bindings-include/left_right.yaml | 31 ++++ .../tests/test-bindings-include/right.yaml | 18 +++ .../python-devicetree/tests/test_edtlib.py | 135 +++++++++++++++++ 7 files changed, 418 insertions(+), 16 deletions(-) create mode 100644 scripts/dts/python-devicetree/tests/test-bindings-include/digest.yaml create mode 100644 scripts/dts/python-devicetree/tests/test-bindings-include/digest_base.yaml create mode 100644 scripts/dts/python-devicetree/tests/test-bindings-include/left.yaml create mode 100644 scripts/dts/python-devicetree/tests/test-bindings-include/left_right.yaml create mode 100644 scripts/dts/python-devicetree/tests/test-bindings-include/right.yaml diff --git a/scripts/dts/python-devicetree/src/devicetree/edtlib.py b/scripts/dts/python-devicetree/src/devicetree/edtlib.py index e53925f96182d3c..c3ce5edeebc72cb 100644 --- a/scripts/dts/python-devicetree/src/devicetree/edtlib.py +++ b/scripts/dts/python-devicetree/src/devicetree/edtlib.py @@ -223,25 +223,31 @@ def __init__(self, path: Optional[str], fname2path: Dict[str, str], # this binding itself defines or modifies self.prop2specs: Dict[str, 'PropertySpec'] = {} + # This binding's child-binding, + # using the same approach as above for property specifications: + # - we initially have no child-binding + # - we may inherit child-binding definitions from other bindings + # when merging included files + # - eventually, we may update inherited child-binding definitions + # with the content of the "child-binding" element of this binding's file + self.child_binding: Optional[Binding] = None + # Save child-binding element before merging. + if "child-binding" in raw: + raw_child_binding = deepcopy(raw["child-binding"]) + else: + raw_child_binding = None + # Merge any included files into self.raw. This also pulls in - # inherited child binding definitions, so it has to be done - # before initializing those. + # inherited child binding definitions. self.raw: dict = self._merge_includes(raw, self.path) - # Recursively initialize any child bindings. These don't - # require a 'compatible' or 'description' to be well defined, - # but they must be dicts. - if "child-binding" in raw: - if not isinstance(raw["child-binding"], dict): - _err(f"malformed 'child-binding:' in {self.path}, " - "expected a binding (dictionary with keys/values)") - self.child_binding: Optional['Binding'] = Binding( - path, fname2path, - raw=raw["child-binding"], - require_compatible=False, - require_description=False) - else: - self.child_binding = None + # When this binding's file contains a child-binding element, + # its content will: + # - update the child-binding definitions inherited from included files, + # if any + # - or initialize this binding's child-binding + if raw_child_binding: + self._update_child_binding(raw_child_binding) # Make sure this is a well defined object. self._check(require_compatible, require_description) @@ -414,6 +420,17 @@ def _load_raw(self, fname: str, # Register included property specs. self._add_included_prop2specs(fname, contents, allowlist, blocklist) + # Pull in child-binding definitions we may inherit + # from the included file. + if "child-binding" in contents: + self._add_included_child_binding( + path, + contents["child-binding"], + # Properties in child-binding have already been filtered, + # child_filter is only used to determine their origin. + child_filter, + ) + return self._merge_includes(contents, path) def _add_included_prop2specs(self, fname: str, contents: dict, @@ -455,6 +472,96 @@ def _add_included_prop2specs(self, fname: str, contents: dict, if prop not in self.prop2specs: self.prop2specs[prop] = spec + # Add child-binding definitions inherited from an included binding. + # - path: YAML file containing the "child-binding" element + # - raw: The content of the "child-binding" element + # - child_filter: filters applied by the including binding + def _add_included_child_binding( + self, + path: Optional[str], + raw: Dict[str, Any], + child_filter: Optional[dict], + ) -> None: + inc_child_binding = self._mk_child_binding(path, raw) + + if self.child_binding or child_filter: + # Take "last modified" semantic when: + # - we've already included a child-binding + # - the including binding applies some filter + # to the included child-binding's properties + child_binding = self._mk_child_binding(self.path, {}) + if self.child_binding: + self._merge_child_binding(child_binding, self.child_binding) + self.child_binding = child_binding + + if self.child_binding: + self._merge_child_binding(self.child_binding, inc_child_binding) + else: + self.child_binding = inc_child_binding + + # Merge child bindings: + # - update child_binding's properties with properties + # from other_child_binding + # - update child_binding's cell specifiers with specifiers + # from other_child_binding + # - recursively merge child bindings of child_binding + # and other_child_binding + def _merge_child_binding( + self, + child_binding: "Binding", + other_child_binding: "Binding", + ) -> None: + child_binding.prop2specs.update(other_child_binding.prop2specs) + child_binding.specifier2cells.update( + other_child_binding.specifier2cells + ) + + other_grandchild_binding = other_child_binding.child_binding + if other_grandchild_binding: + if child_binding.child_binding: + self._merge_child_binding( + child_binding.child_binding, other_grandchild_binding + ) + else: + child_binding.child_binding = other_grandchild_binding + + def _update_child_binding(self, raw: Dict[str, Any]) -> None: + # Child-binding specifications from this binding's file. + last_child_binding = self._mk_child_binding(self.path, raw) + + if self.child_binding: + # This binding's child-binding is "last modified" here, + # create empty new one with "self" origin (path). + child_binding = self._mk_child_binding(self.path, {}) + # First, add child-binding specifications inherited + # from included files. + self._merge_child_binding(child_binding, self.child_binding) + # Then, add/modify child-binding specifications + # from this binding's file. + self._merge_child_binding(child_binding, last_child_binding) + self.child_binding = child_binding + else: + self.child_binding = last_child_binding + + def _mk_child_binding( + self, path: Optional[str], raw: Dict[str, Any] + ) -> "Binding": + if not isinstance(raw, dict): + _err( + f"malformed 'child-binding:' in {path}, " + "expected a binding (dictionary with keys/values)" + ) + return Binding( + path, + self._fname2path, + raw, + require_compatible=False, + require_description=False, + # Filters already applied. + inc_allowlist=None, + inc_blocklist=None, + ) + def _check(self, require_compatible: bool, require_description: bool): # Does sanity checking on the binding. diff --git a/scripts/dts/python-devicetree/tests/test-bindings-include/digest.yaml b/scripts/dts/python-devicetree/tests/test-bindings-include/digest.yaml new file mode 100644 index 000000000000000..65fb67db221e9a6 --- /dev/null +++ b/scripts/dts/python-devicetree/tests/test-bindings-include/digest.yaml @@ -0,0 +1,59 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright (c) 2023 Christophe Dufaza + +description: | + Extends allow-and-blocklist-multilevel.yaml test case. + +compatible: vnd,digest + +# child-binding: +# + child-prop-1 +# +# grandchild-binding: +# + gandchild-prop-1 +# + gandchild-prop-2 +include: + - name: digest_base.yaml + property-allowlist: [x] + + child-binding: + property-allowlist: [child-prop-1] + + child-binding: + property-blocklist: [grandchild-prop-blocked] + +properties: + top-prop: + type: int + +child-binding: + include: + # child-binding: + # ~ child-prop-1 + # + x + # + z + - name: digest_base.yaml + property-blocklist: [y] + + # grandchild-binding: + # ~ gandchild-prop-1 + # ~ gandchild-prop-2 + # + child-prop-1 + # + child-prop-1 + child-binding: + property-blocklist: [child-prop-blocked] + + # grandgrandchild-binding: + # + gandchild-prop-1 + # + gandchild-prop-2 + child-binding: + property-blocklist: [grandchild-prop-blocked] + + properties: + cb-prop: + type: int + + child-binding: + properties: + gcbb-prop: + type: int diff --git a/scripts/dts/python-devicetree/tests/test-bindings-include/digest_base.yaml b/scripts/dts/python-devicetree/tests/test-bindings-include/digest_base.yaml new file mode 100644 index 000000000000000..4864d61bc25bd15 --- /dev/null +++ b/scripts/dts/python-devicetree/tests/test-bindings-include/digest_base.yaml @@ -0,0 +1,34 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright (c) 2023 Christophe Dufaza + +description: | + Extends include.yaml with an additional property + for testing grandchild-binding filters. + + See digest.yaml. + +properties: + x: + type: int + y: + type: int + z: + type: int + +child-binding: + properties: + child-prop-1: + type: int + child-prop-2: + type: int + child-prop-blocked: + type: int + + child-binding: + properties: + grandchild-prop-1: + type: int + grandchild-prop-2: + type: int + grandchild-prop-blocked: + type: int diff --git a/scripts/dts/python-devicetree/tests/test-bindings-include/left.yaml b/scripts/dts/python-devicetree/tests/test-bindings-include/left.yaml new file mode 100644 index 000000000000000..937969c4fd624a3 --- /dev/null +++ b/scripts/dts/python-devicetree/tests/test-bindings-include/left.yaml @@ -0,0 +1,18 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright (c) 2023 Christophe Dufaza + +description: | + Left include file for the multi-includes test case. + +properties: + left-prop-1: + type: int + left-prop-2: + type: int + +child-binding: + properties: + left-child-prop-1: + type: int + left-child-prop-2: + type: int diff --git a/scripts/dts/python-devicetree/tests/test-bindings-include/left_right.yaml b/scripts/dts/python-devicetree/tests/test-bindings-include/left_right.yaml new file mode 100644 index 000000000000000..9c98654ec11beae --- /dev/null +++ b/scripts/dts/python-devicetree/tests/test-bindings-include/left_right.yaml @@ -0,0 +1,31 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright (c) 2023 Christophe Dufaza + +description: | + Test multi-includes from top-level binding. + +compatible: vnd,left_right + +include: + - name: left.yaml + property-allowlist: [left-prop-1] + child-binding: + property-allowlist: [left-child-prop-1] + + - name: right.yaml + property-allowlist: [right-prop-2] + child-binding: + property-allowlist: [right-child-prop-2] + +properties: + left-right-prop: + type: int + +child-binding: + properties: + child-prop: + type: int + child-binding: + properties: + grandchild-prop: + type: int diff --git a/scripts/dts/python-devicetree/tests/test-bindings-include/right.yaml b/scripts/dts/python-devicetree/tests/test-bindings-include/right.yaml new file mode 100644 index 000000000000000..1681db4b61ed5ef --- /dev/null +++ b/scripts/dts/python-devicetree/tests/test-bindings-include/right.yaml @@ -0,0 +1,18 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright (c) 2023 Christophe Dufaza + +description: | + Right include file for the multi-includes test case. + +properties: + right-prop-1: + type: int + right-prop-2: + type: int + +child-binding: + properties: + right-child-prop-1: + type: int + right-child-prop-2: + type: int diff --git a/scripts/dts/python-devicetree/tests/test_edtlib.py b/scripts/dts/python-devicetree/tests/test_edtlib.py index a81aa929370a076..a21fe10ce76f862 100644 --- a/scripts/dts/python-devicetree/tests/test_edtlib.py +++ b/scripts/dts/python-devicetree/tests/test_edtlib.py @@ -393,6 +393,138 @@ def test_include_filters_included_bindings(): assert not top_blocks.prop2specs.get("x") assert top_blocks.prop2specs.get("y") +def test_include_child_binding_digest() -> None: + '''Extends allow-and-blocklist-multilevel.yaml test case, + plus: + - check of bindings and property specs paths + - check of grand-grandchild-binding level + - check grandchild-binding filters + + All child-binding definitions come from a single file, + included multiple times, at different levels, + with different filters. + ''' + fname2path = { + "digest_base.yaml": "test-bindings-include/digest_base.yaml", + } + + binding = None + with from_here(): + binding = edtlib.Binding("test-bindings-include/digest.yaml", + fname2path) + assert "digest.yaml" == _basename(binding.path) + + assert {"x", "top-prop"} == set(binding.prop2specs.keys()) + assert "digest_base.yaml" == _basename(binding.prop2specs["x"].path) + assert "digest.yaml" == _basename( + binding.prop2specs["top-prop"].path + ) + + child_binding = binding.child_binding + assert child_binding + # Last modified semantic. + assert "digest.yaml" == _basename(child_binding.path) + + assert {"x", "z", "child-prop-1", "cb-prop"} == set( + child_binding.prop2specs.keys() + ) + + assert "digest_base.yaml" == _basename(child_binding.prop2specs["x"].path) + assert "digest_base.yaml" == _basename( + child_binding.prop2specs["child-prop-1"].path + ) + + grandchild_binding = child_binding.child_binding + assert grandchild_binding + assert "digest_base.yaml" == _basename(grandchild_binding.path) + + assert { + "child-prop-1", + "child-prop-2", + "grandchild-prop-1", + "grandchild-prop-2", + "gcbb-prop", + } == set(grandchild_binding.prop2specs.keys()) + + assert "digest_base.yaml" == _basename( + grandchild_binding.prop2specs["child-prop-1"].path + ) + assert "digest_base.yaml" == _basename( + grandchild_binding.prop2specs["grandchild-prop-1"].path + ) + assert "digest.yaml" == _basename( + grandchild_binding.prop2specs["gcbb-prop"].path + ) + + ggchild_binding = grandchild_binding.child_binding + assert ggchild_binding + assert "digest_base.yaml" == _basename(ggchild_binding.path) + + assert { + "grandchild-prop-1", + "grandchild-prop-2", + } == set(ggchild_binding.prop2specs.keys()) + + assert "digest_base.yaml" == _basename( + ggchild_binding.prop2specs["grandchild-prop-1"].path + ) + assert "digest_base.yaml" == _basename( + ggchild_binding.prop2specs["grandchild-prop-2"].path + ) + +def test_include_child_bindings_digest() -> None: + '''Complement test case above, child-binding definitions come from two files.''' + fname2path = { + "left.yaml": "test-bindings-include/left.yaml", + "right.yaml": "test-bindings-include/right.yaml", + } + + binding = None + with from_here(): + binding = edtlib.Binding("test-bindings-include/left_right.yaml", + fname2path) + assert "left_right.yaml" == _basename(binding.path) + assert {"left-right-prop", "left-prop-1", "right-prop-2"} == set( + binding.prop2specs.keys() + ) + assert "left_right.yaml" == _basename( + binding.prop2specs["left-right-prop"].path + ) + assert "left.yaml" == _basename(binding.prop2specs["left-prop-1"].path) + assert "right.yaml" == _basename(binding.prop2specs["right-prop-2"].path) + + # Child-binding. + child_binding = binding.child_binding + assert child_binding + # Last modified semantic. + assert "left_right.yaml" == _basename(child_binding.path) + + assert {"left-child-prop-1", "right-child-prop-2", "child-prop"} == set( + child_binding.prop2specs.keys() + ) + + assert "left.yaml" == _basename( + child_binding.prop2specs["left-child-prop-1"].path + ) + assert "right.yaml" == _basename( + child_binding.prop2specs["right-child-prop-2"].path + ) + assert "left_right.yaml" == _basename( + child_binding.prop2specs["child-prop"].path + ) + + # GrandChild-binding. + grandchild_binding = child_binding.child_binding + assert grandchild_binding + assert "left_right.yaml" == _basename(grandchild_binding.path) + + assert { + "grandchild-prop", + } == set(grandchild_binding.prop2specs.keys()) + + assert "left_right.yaml" == _basename( + grandchild_binding.prop2specs["grandchild-prop"].path + ) def test_bus(): @@ -882,3 +1014,6 @@ def verify_phandle_array_prop(node, name, values): assert actual.data == data else: assert actual is None + +def _basename(path): + return os.path.basename(path or "?")