diff --git a/scripts/dts/python-devicetree/src/devicetree/edtlib.py b/scripts/dts/python-devicetree/src/devicetree/edtlib.py index 099f3672addc1..61db1c8af3437 100644 --- a/scripts/dts/python-devicetree/src/devicetree/edtlib.py +++ b/scripts/dts/python-devicetree/src/devicetree/edtlib.py @@ -163,9 +163,7 @@ class Binding: def __init__(self, path: Optional[str], fname2path: Dict[str, str], raw: Any = None, require_compatible: bool = True, - require_description: bool = True, - inc_allowlist: Optional[List[str]] = None, - inc_blocklist: Optional[List[str]] = None): + require_description: bool = True): """ Binding constructor. @@ -193,36 +191,16 @@ def __init__(self, path: Optional[str], fname2path: Dict[str, str], "description:" line. If False, a missing "description:" is not an error. Either way, "description:" must be a string if it is present in the binding. - - inc_allowlist: - The property-allowlist filter set by including bindings. - - inc_blocklist: - The property-blocklist filter set by including bindings. """ self.path: Optional[str] = path self._fname2path: Dict[str, str] = fname2path - self._inc_allowlist: Optional[List[str]] = inc_allowlist - self._inc_blocklist: Optional[List[str]] = inc_blocklist - if raw is None: if path is None: _err("you must provide either a 'path' or a 'raw' argument") with open(path, encoding="utf-8") as f: raw = yaml.load(f, Loader=_BindingLoader) - # Get the properties this binding modifies - # before we merge the included ones. - last_modified_props = list(raw.get("properties", {}).keys()) - - # Map property names to their specifications: - # - first, _merge_includes() will recursively populate prop2specs with - # the properties specified by the included bindings - # - eventually, we'll update prop2specs with the properties - # this binding itself defines or modifies - self.prop2specs: Dict[str, 'PropertySpec'] = {} - # Merge any included files into self.raw. This also pulls in # inherited child binding definitions, so it has to be done # before initializing those. @@ -246,11 +224,10 @@ def __init__(self, path: Optional[str], fname2path: Dict[str, str], # Make sure this is a well defined object. self._check(require_compatible, require_description) - # Update specs with the properties this binding defines or modifies. - for prop_name in last_modified_props: - self.prop2specs[prop_name] = PropertySpec(prop_name, self) - # Initialize look up tables. + self.prop2specs: Dict[str, 'PropertySpec'] = {} + for prop_name in self.raw.get("properties", {}).keys(): + self.prop2specs[prop_name] = PropertySpec(prop_name, self) self.specifier2cells: Dict[str, List[str]] = {} for key, val in self.raw.items(): if key.endswith("-cells"): @@ -314,41 +291,18 @@ def _merge_includes(self, raw: dict, binding_path: Optional[str]) -> dict: if isinstance(include, str): # Simple scalar string case - # Load YAML file and register property specs into prop2specs. - inc_raw = self._load_raw(include, self._inc_allowlist, - self._inc_blocklist) - - _merge_props(merged, inc_raw, None, binding_path, False) + _merge_props(merged, self._load_raw(include), None, binding_path, + False) elif isinstance(include, list): # List of strings and maps. These types may be intermixed. for elem in include: if isinstance(elem, str): - # Load YAML file and register property specs into prop2specs. - inc_raw = self._load_raw(elem, self._inc_allowlist, - self._inc_blocklist) - - _merge_props(merged, inc_raw, None, binding_path, False) + _merge_props(merged, self._load_raw(elem), None, + binding_path, False) elif isinstance(elem, dict): name = elem.pop('name', None) - - # Merge this include property-allowlist filter - # with filters from including bindings. allowlist = elem.pop('property-allowlist', None) - if allowlist is not None: - if self._inc_allowlist: - allowlist.extend(self._inc_allowlist) - else: - allowlist = self._inc_allowlist - - # Merge this include property-blocklist filter - # with filters from including bindings. blocklist = elem.pop('property-blocklist', None) - if blocklist is not None: - if self._inc_blocklist: - blocklist.extend(self._inc_blocklist) - else: - blocklist = self._inc_blocklist - child_filter = elem.pop('child-binding', None) if elem: @@ -359,12 +313,10 @@ def _merge_includes(self, raw: dict, binding_path: Optional[str]) -> dict: _check_include_dict(name, allowlist, blocklist, child_filter, binding_path) - # Load YAML file, and register (filtered) property specs - # into prop2specs. - contents = self._load_raw(name, - allowlist, blocklist, - child_filter) + contents = self._load_raw(name) + _filter_properties(contents, allowlist, blocklist, + child_filter, binding_path) _merge_props(merged, contents, None, binding_path, False) else: _err(f"all elements in 'include:' in {binding_path} " @@ -384,17 +336,11 @@ def _merge_includes(self, raw: dict, binding_path: Optional[str]) -> dict: return raw - - def _load_raw(self, fname: str, - allowlist: Optional[List[str]] = None, - blocklist: Optional[List[str]] = None, - child_filter: Optional[dict] = None) -> dict: + def _load_raw(self, fname: str) -> dict: # Returns the contents of the binding given by 'fname' after merging - # any bindings it lists in 'include:' into it, according to the given - # property filters. - # - # Will also register the (filtered) included property specs - # into prop2specs. + # any bindings it lists in 'include:' into it. 'fname' is just the + # basename of the file, so we check that there aren't multiple + # candidates. path = self._fname2path.get(fname) @@ -406,55 +352,8 @@ def _load_raw(self, fname: str, if not isinstance(contents, dict): _err(f'{path}: invalid contents, expected a mapping') - # Apply constraints to included YAML contents. - _filter_properties(contents, - allowlist, blocklist, - child_filter, self.path) - - # Register included property specs. - self._add_included_prop2specs(fname, contents, allowlist, blocklist) - return self._merge_includes(contents, path) - def _add_included_prop2specs(self, fname: str, contents: dict, - allowlist: Optional[List[str]] = None, - blocklist: Optional[List[str]] = None) -> None: - # Registers the properties specified by an included binding file - # into the properties this binding supports/requires (aka prop2specs). - # - # Consider "this" binding B includes I1 which itself includes I2. - # - # We assume to be called in that order: - # 1) _add_included_prop2spec(B, I1) - # 2) _add_included_prop2spec(B, I2) - # - # Where we don't want I2 "taking ownership" for properties - # modified by I1. - # - # So we: - # - first create a binding that represents the included file - # - then add the property specs defined by this binding to prop2specs, - # without overriding the specs modified by an including binding - # - # Note: Unfortunately, we can't cache these base bindings, - # as a same YAML file may be included with different filters - # (property-allowlist and such), leading to different contents. - - inc_binding = Binding( - self._fname2path[fname], - self._fname2path, - contents, - require_compatible=False, - require_description=False, - # Recursively pass filters to included bindings. - inc_allowlist=allowlist, - inc_blocklist=blocklist, - ) - - for prop, spec in inc_binding.prop2specs.items(): - if prop not in self.prop2specs: - self.prop2specs[prop] = spec - 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/base.yaml b/scripts/dts/python-devicetree/tests/test-bindings-include/base.yaml deleted file mode 100644 index f564578b48d0b..0000000000000 --- a/scripts/dts/python-devicetree/tests/test-bindings-include/base.yaml +++ /dev/null @@ -1,7 +0,0 @@ -# SPDX-License-Identifier: BSD-3-Clause - -properties: - x: - type: int - y: - type: int diff --git a/scripts/dts/python-devicetree/tests/test-bindings-include/inc-base.yaml b/scripts/dts/python-devicetree/tests/test-bindings-include/inc-base.yaml deleted file mode 100644 index 59dc45eab0dbb..0000000000000 --- a/scripts/dts/python-devicetree/tests/test-bindings-include/inc-base.yaml +++ /dev/null @@ -1,3 +0,0 @@ -# SPDX-License-Identifier: BSD-3-Clause - -include: base.yaml diff --git a/scripts/dts/python-devicetree/tests/test-bindings-include/modified.yaml b/scripts/dts/python-devicetree/tests/test-bindings-include/modified.yaml deleted file mode 100644 index 3d8130c1aedd0..0000000000000 --- a/scripts/dts/python-devicetree/tests/test-bindings-include/modified.yaml +++ /dev/null @@ -1,7 +0,0 @@ -# SPDX-License-Identifier: BSD-3-Clause - -include: base.yaml - -properties: - x: - required: true diff --git a/scripts/dts/python-devicetree/tests/test-bindings-include/simple.yaml b/scripts/dts/python-devicetree/tests/test-bindings-include/simple.yaml new file mode 100644 index 0000000000000..9ce3fe8b1ee26 --- /dev/null +++ b/scripts/dts/python-devicetree/tests/test-bindings-include/simple.yaml @@ -0,0 +1,30 @@ +# SPDX-License-Identifier: BSD-3-Clause +# +# Base properties for testing property filters up to +# the grandchild-binding level. + +properties: + prop-1: + type: int + prop-2: + type: int + prop-3: + type: int + +child-binding: + properties: + child-prop-1: + type: int + child-prop-2: + type: int + child-prop-3: + type: int + + child-binding: + properties: + grandchild-prop-1: + type: int + grandchild-prop-2: + type: int + grandchild-prop-3: + type: int diff --git a/scripts/dts/python-devicetree/tests/test-bindings-include/simple_filter_allowlist.yaml b/scripts/dts/python-devicetree/tests/test-bindings-include/simple_filter_allowlist.yaml new file mode 100644 index 0000000000000..cae1cb2800acc --- /dev/null +++ b/scripts/dts/python-devicetree/tests/test-bindings-include/simple_filter_allowlist.yaml @@ -0,0 +1,12 @@ +# SPDX-License-Identifier: BSD-3-Clause +# +# Filter inherited property specifications +# up to the grandchild-binding level. + +include: + - name: simple_inherit.yaml + property-allowlist: [prop-1] + child-binding: + property-allowlist: [child-prop-1] + child-binding: + property-allowlist: [grandchild-prop-1] diff --git a/scripts/dts/python-devicetree/tests/test-bindings-include/simple_filter_blocklist.yaml b/scripts/dts/python-devicetree/tests/test-bindings-include/simple_filter_blocklist.yaml new file mode 100644 index 0000000000000..e605c6a65dc5e --- /dev/null +++ b/scripts/dts/python-devicetree/tests/test-bindings-include/simple_filter_blocklist.yaml @@ -0,0 +1,12 @@ +# SPDX-License-Identifier: BSD-3-Clause +# +# Filter inherited property specifications +# up to the grandchild-binding level. + +include: + - name: simple_inherit.yaml + property-blocklist: [prop-2, prop-3] + child-binding: + property-blocklist: [child-prop-2, child-prop-3] + child-binding: + property-blocklist: [grandchild-prop-2, grandchild-prop-3] diff --git a/scripts/dts/python-devicetree/tests/test-bindings-include/simple_inherit.yaml b/scripts/dts/python-devicetree/tests/test-bindings-include/simple_inherit.yaml new file mode 100644 index 0000000000000..8a95ef38f9511 --- /dev/null +++ b/scripts/dts/python-devicetree/tests/test-bindings-include/simple_inherit.yaml @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: BSD-3-Clause +# +# Inherits property specifications without modification. + +include: simple.yaml diff --git a/scripts/dts/python-devicetree/tests/test-bindings-include/top-allows.yaml b/scripts/dts/python-devicetree/tests/test-bindings-include/top-allows.yaml deleted file mode 100644 index a4938eb0d6768..0000000000000 --- a/scripts/dts/python-devicetree/tests/test-bindings-include/top-allows.yaml +++ /dev/null @@ -1,10 +0,0 @@ -# SPDX-License-Identifier: BSD-3-Clause - -description: Test property-allowlist filters set by including bindings - -compatible: "top-allowlist" - -include: - - name: inc-base.yaml - property-allowlist: - - x diff --git a/scripts/dts/python-devicetree/tests/test-bindings-include/top-blocks.yaml b/scripts/dts/python-devicetree/tests/test-bindings-include/top-blocks.yaml deleted file mode 100644 index 787db223a939e..0000000000000 --- a/scripts/dts/python-devicetree/tests/test-bindings-include/top-blocks.yaml +++ /dev/null @@ -1,10 +0,0 @@ -# SPDX-License-Identifier: BSD-3-Clause - -description: Test property-blocklist filters set by including bindings. - -compatible: "top-blocklist" - -include: - - name: inc-base.yaml - property-blocklist: - - x diff --git a/scripts/dts/python-devicetree/tests/test-bindings-include/top.yaml b/scripts/dts/python-devicetree/tests/test-bindings-include/top.yaml deleted file mode 100644 index 8fb9320676d51..0000000000000 --- a/scripts/dts/python-devicetree/tests/test-bindings-include/top.yaml +++ /dev/null @@ -1,21 +0,0 @@ -# SPDX-License-Identifier: BSD-3-Clause - -description: | - Top-level binding file for testing included property spec paths. - - base.yaml: specifies properties "x" and "y" - modified.yaml: includes base.yaml, modifies property "x" - top.yaml (this file): includes modified.yaml, specifies property "p" - - From the top-level binding, we expect: - - "x" was last modified in modified.yaml - - "y" was last modified in base.yaml - - "p" was last modified in top.yaml - -compatible: top-level - -include: modified.yaml - -properties: - p: - type: int diff --git a/scripts/dts/python-devicetree/tests/test_edtlib.py b/scripts/dts/python-devicetree/tests/test_edtlib.py index d02bfd7fc6ddf..30fbc7c3fc36f 100644 --- a/scripts/dts/python-devicetree/tests/test_edtlib.py +++ b/scripts/dts/python-devicetree/tests/test_edtlib.py @@ -365,35 +365,118 @@ def test_include_filters(): assert set(child.prop2specs.keys()) == {'child-prop-1', 'child-prop-2', 'x', 'z'} # root level 'y' is blocked -def test_include_paths(): - '''Test "last modified" semantic for included bindings paths.''' - - fname2path = {'base.yaml': 'test-bindings-include/base.yaml', - 'modified.yaml': 'test-bindings-include/modified.yaml'} - +def test_include_filters_inherited_bindings() -> None: + '''Test the basics of filtering properties inherited via an intermediary binding file. + + Use-case "B includes I includes X": + - X is a base binding file, specifying common properties + - I is an intermediary binding file, which includes X without modification + nor filter + - B includes I, filtering the properties it chooses to inherit + with an allowlist or a blocklist + + Checks that the properties inherited from X via I are actually filtered + as B intends to. + ''' + fname2path = { + # Base binding file, specifies a few properties up to the grandchild-binding level. + "simple.yaml": "test-bindings-include/simple.yaml", + # 'include:'s the base file above, without modification nor filter + "simple_inherit.yaml": "test-bindings-include/simple_inherit.yaml", + } with from_here(): - top = edtlib.Binding('test-bindings-include/top.yaml', fname2path) - - assert 'modified.yaml' == os.path.basename(top.prop2specs["x"].path) - assert 'base.yaml' == os.path.basename(top.prop2specs["y"].path) - assert 'top.yaml' == os.path.basename(top.prop2specs["p"].path) - -def test_include_filters_included_bindings(): - '''Test filters set by including bindings.''' - fname2path = {'base.yaml': 'test-bindings-include/base.yaml', - 'inc-base.yaml': 'test-bindings-include/inc-base.yaml'} + binding = edtlib.Binding( + # Filters inherited specifications with an allowlist. + "test-bindings-include/simple_filter_allowlist.yaml", + fname2path, + require_compatible=False, + require_description=False, + ) + # Only property allowed. + assert {"prop-1"} == set(binding.prop2specs.keys()) with from_here(): - top_allows = edtlib.Binding('test-bindings-include/top-allows.yaml', fname2path) - assert top_allows.prop2specs.get("x") - assert not top_allows.prop2specs.get("y") - + binding = edtlib.Binding( + # Filters inherited specifications with a blocklist. + "test-bindings-include/simple_filter_blocklist.yaml", + fname2path, + require_compatible=False, + require_description=False, + ) + # Only non blocked property. + assert {"prop-1"} == set(binding.prop2specs.keys()) + +def test_include_filters_inherited_child_bindings() -> None: + '''Test the basics of filtering properties inherited via an intermediary binding file + (child-binding level). + + See also: test_include_filters_inherited_bindings() + ''' + fname2path = { + "simple.yaml": "test-bindings-include/simple.yaml", + "simple_inherit.yaml": "test-bindings-include/simple_inherit.yaml", + } with from_here(): - top_blocks = edtlib.Binding('test-bindings-include/top-blocks.yaml', fname2path) - assert not top_blocks.prop2specs.get("x") - assert top_blocks.prop2specs.get("y") + binding = edtlib.Binding( + "test-bindings-include/simple_filter_allowlist.yaml", + fname2path, + require_compatible=False, + require_description=False, + ) + assert binding.child_binding + child_binding = binding.child_binding + # Only property allowed. + assert {"child-prop-1"} == set(child_binding.prop2specs.keys()) + with from_here(): + binding = edtlib.Binding( + "test-bindings-include/simple_filter_blocklist.yaml", + fname2path, + require_compatible=False, + require_description=False, + ) + # Only non blocked property. + assert binding.child_binding + child_binding = binding.child_binding + assert {"child-prop-1"} == set(child_binding.prop2specs.keys()) + +def test_include_filters_inherited_grandchild_bindings() -> None: + '''Test the basics of filtering properties inherited via an intermediary binding file + (grandchild-binding level). + + See also: test_include_filters_inherited_bindings() + ''' + fname2path = { + "simple.yaml": "test-bindings-include/simple.yaml", + "simple_inherit.yaml": "test-bindings-include/simple_inherit.yaml", + } + with from_here(): + binding = edtlib.Binding( + "test-bindings-include/simple_filter_allowlist.yaml", + fname2path, + require_compatible=False, + require_description=False, + ) + assert binding.child_binding + child_binding = binding.child_binding + assert child_binding.child_binding + grandchild_binding = child_binding.child_binding + # Only property allowed. + assert {"grandchild-prop-1"} == set(grandchild_binding.prop2specs.keys()) + with from_here(): + binding = edtlib.Binding( + "test-bindings-include/simple_filter_blocklist.yaml", + fname2path, + require_compatible=False, + require_description=False, + ) + assert binding.child_binding + child_binding = binding.child_binding + assert child_binding.child_binding + grandchild_binding = child_binding.child_binding + # Only non blocked property. + assert {"grandchild-prop-1"} == set(grandchild_binding.prop2specs.keys()) def test_bus(): '''Test 'bus:' and 'on-bus:' in bindings'''