edtlib: fix "last modified" semantic for included property specs

Although the PropertySpec.path attribute is documented as
"the file where the property was last modified",
all property specs in Binding.prop2specs will claim
they were last modified by the top-level binding itself.

Consider:
- I1 is a base binding that specifies properties x and y
- I2 is an "intermediate" binding that includes I1,
  modifying the specification for property x
- B is a top-level bindings that includes I2,
  and specifies an additional property p

When enumerating the properties of B,
we expect the values of PropertySpec.path to tell us:
- y was last modified by I1
- x was last modified by I2
- p was last modified by B

However, the Binding constructor:
- first merges all included bindings into the top-level one
- eventually initializes specifications for all the defined properties

As a consequence, all defined properties claim they were last modified
by the top-level binding file.

We should instead:
- first, take into account their own specifications for the
  included properties
- eventually update these specifications with the properties
  the top-level binding adds or modifies

Signed-off-by: Christophe Dufaza <chris@openmarl.org>

scripts/dts/python-devicetree/src/devicetree/edtlib.py

@@ -163,7 +163,9 @@ class Binding:
 
     def __init__(self, path: Optional[str], fname2path: Dict[str, str],
                  raw: Any = None, require_compatible: bool = True,
-                 require_description: bool = True):
+                 require_description: bool = True,
+                 inc_allowlist: Optional[List[str]] = None,
+                 inc_blocklist: Optional[List[str]] = None):
         """
         Binding constructor.
 
@@ -191,16 +193,36 @@ class Binding:
           "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.
@@ -224,10 +246,11 @@ class Binding:
         # Make sure this is a well defined object.
         self._check(require_compatible, require_description)
 
-        # Initialize look up tables.
+        # Update specs with the properties this binding defines or modifies.
-        self.prop2specs: Dict[str, 'PropertySpec'] = {}
+        for prop_name in last_modified_props:
-        for prop_name in self.raw.get("properties", {}).keys():
             self.prop2specs[prop_name] = PropertySpec(prop_name, self)
+
+        # Initialize look up tables.
         self.specifier2cells: Dict[str, List[str]] = {}
         for key, val in self.raw.items():
             if key.endswith("-cells"):
@@ -291,18 +314,41 @@ class Binding:
 
         if isinstance(include, str):
             # Simple scalar string case
-            _merge_props(merged, self._load_raw(include), None, binding_path,
+            # Load YAML file and register property specs into prop2specs.
-                         False)
+            inc_raw = self._load_raw(include, self._inc_allowlist,
+                                     self._inc_blocklist)
+
+            _merge_props(merged, inc_raw, 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):
-                    _merge_props(merged, self._load_raw(elem), None,
+                    # Load YAML file and register property specs into prop2specs.
-                                 binding_path, False)
+                    inc_raw = self._load_raw(elem, self._inc_allowlist,
+                                             self._inc_blocklist)
+
+                    _merge_props(merged, inc_raw, 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:
@@ -313,10 +359,12 @@ class Binding:
                     _check_include_dict(name, allowlist, blocklist,
                                         child_filter, binding_path)
 
-                    contents = self._load_raw(name)
+                    # Load YAML file, and register (filtered) property specs
+                    # into prop2specs.
+                    contents = self._load_raw(name,
+                                              allowlist, blocklist,
+                                              child_filter)
 
-                    _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} "
@@ -336,11 +384,17 @@ class Binding:
 
         return raw
 
-    def _load_raw(self, fname: str) -> dict:
+
+    def _load_raw(self, fname: str,
+                  allowlist: Optional[List[str]] = None,
+                  blocklist: Optional[List[str]] = None,
+                  child_filter: Optional[dict] = None) -> dict:
         # Returns the contents of the binding given by 'fname' after merging
-        # any bindings it lists in 'include:' into it. 'fname' is just the
+        # any bindings it lists in 'include:' into it, according to the given
-        # basename of the file, so we check that there aren't multiple
+        # property filters.
-        # candidates.
+        #
+        # Will also register the (filtered) included property specs
+        # into prop2specs.
 
         path = self._fname2path.get(fname)
 
@@ -352,8 +406,55 @@ class Binding:
             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.