Skip to content

Commit 49576db

Browse files
committed
Revert "edtlib: fix "last modified" semantic for included ... specs"
[1] was introduced to get more valuable answers from the PropertySpec.path API, which is supposed to tell in which file the property's specification was "last modfied". Further work on related issues [2] showed that the approach chosen in [1] is dead end: we need to first rethink how bindings (and especially child-bindings) are initialized. [1] edtlib: fix last modified semantic in included property specs [2] edtlib: Preserve paths of properties from included child bindings See also: zephyrproject-rtos#65221, zephyrproject-rtos#78095 This reverts commit b3b5ad8. Signed-off-by: Christophe Dufaza <[email protected]>
1 parent 1a53371 commit 49576db

File tree

1 file changed

+15
-116
lines changed
  • scripts/dts/python-devicetree/src/devicetree

1 file changed

+15
-116
lines changed

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

Lines changed: 15 additions & 116 deletions
Original file line numberDiff line numberDiff line change
@@ -163,9 +163,7 @@ class Binding:
163163

164164
def __init__(self, path: Optional[str], fname2path: Dict[str, str],
165165
raw: Any = None, require_compatible: bool = True,
166-
require_description: bool = True,
167-
inc_allowlist: Optional[List[str]] = None,
168-
inc_blocklist: Optional[List[str]] = None):
166+
require_description: bool = True):
169167
"""
170168
Binding constructor.
171169
@@ -193,36 +191,16 @@ def __init__(self, path: Optional[str], fname2path: Dict[str, str],
193191
"description:" line. If False, a missing "description:" is
194192
not an error. Either way, "description:" must be a string
195193
if it is present in the binding.
196-
197-
inc_allowlist:
198-
The property-allowlist filter set by including bindings.
199-
200-
inc_blocklist:
201-
The property-blocklist filter set by including bindings.
202194
"""
203195
self.path: Optional[str] = path
204196
self._fname2path: Dict[str, str] = fname2path
205197

206-
self._inc_allowlist: Optional[List[str]] = inc_allowlist
207-
self._inc_blocklist: Optional[List[str]] = inc_blocklist
208-
209198
if raw is None:
210199
if path is None:
211200
_err("you must provide either a 'path' or a 'raw' argument")
212201
with open(path, encoding="utf-8") as f:
213202
raw = yaml.load(f, Loader=_BindingLoader)
214203

215-
# Get the properties this binding modifies
216-
# before we merge the included ones.
217-
last_modified_props = list(raw.get("properties", {}).keys())
218-
219-
# Map property names to their specifications:
220-
# - first, _merge_includes() will recursively populate prop2specs with
221-
# the properties specified by the included bindings
222-
# - eventually, we'll update prop2specs with the properties
223-
# this binding itself defines or modifies
224-
self.prop2specs: Dict[str, 'PropertySpec'] = {}
225-
226204
# Merge any included files into self.raw. This also pulls in
227205
# inherited child binding definitions, so it has to be done
228206
# before initializing those.
@@ -246,11 +224,10 @@ def __init__(self, path: Optional[str], fname2path: Dict[str, str],
246224
# Make sure this is a well defined object.
247225
self._check(require_compatible, require_description)
248226

249-
# Update specs with the properties this binding defines or modifies.
250-
for prop_name in last_modified_props:
251-
self.prop2specs[prop_name] = PropertySpec(prop_name, self)
252-
253227
# Initialize look up tables.
228+
self.prop2specs: Dict[str, 'PropertySpec'] = {}
229+
for prop_name in self.raw.get("properties", {}).keys():
230+
self.prop2specs[prop_name] = PropertySpec(prop_name, self)
254231
self.specifier2cells: Dict[str, List[str]] = {}
255232
for key, val in self.raw.items():
256233
if key.endswith("-cells"):
@@ -314,41 +291,18 @@ def _merge_includes(self, raw: dict, binding_path: Optional[str]) -> dict:
314291

315292
if isinstance(include, str):
316293
# Simple scalar string case
317-
# Load YAML file and register property specs into prop2specs.
318-
inc_raw = self._load_raw(include, self._inc_allowlist,
319-
self._inc_blocklist)
320-
321-
_merge_props(merged, inc_raw, None, binding_path, False)
294+
_merge_props(merged, self._load_raw(include), None, binding_path,
295+
False)
322296
elif isinstance(include, list):
323297
# List of strings and maps. These types may be intermixed.
324298
for elem in include:
325299
if isinstance(elem, str):
326-
# Load YAML file and register property specs into prop2specs.
327-
inc_raw = self._load_raw(elem, self._inc_allowlist,
328-
self._inc_blocklist)
329-
330-
_merge_props(merged, inc_raw, None, binding_path, False)
300+
_merge_props(merged, self._load_raw(elem), None,
301+
binding_path, False)
331302
elif isinstance(elem, dict):
332303
name = elem.pop('name', None)
333-
334-
# Merge this include property-allowlist filter
335-
# with filters from including bindings.
336304
allowlist = elem.pop('property-allowlist', None)
337-
if allowlist is not None:
338-
if self._inc_allowlist:
339-
allowlist.extend(self._inc_allowlist)
340-
else:
341-
allowlist = self._inc_allowlist
342-
343-
# Merge this include property-blocklist filter
344-
# with filters from including bindings.
345305
blocklist = elem.pop('property-blocklist', None)
346-
if blocklist is not None:
347-
if self._inc_blocklist:
348-
blocklist.extend(self._inc_blocklist)
349-
else:
350-
blocklist = self._inc_blocklist
351-
352306
child_filter = elem.pop('child-binding', None)
353307

354308
if elem:
@@ -359,12 +313,10 @@ def _merge_includes(self, raw: dict, binding_path: Optional[str]) -> dict:
359313
_check_include_dict(name, allowlist, blocklist,
360314
child_filter, binding_path)
361315

362-
# Load YAML file, and register (filtered) property specs
363-
# into prop2specs.
364-
contents = self._load_raw(name,
365-
allowlist, blocklist,
366-
child_filter)
316+
contents = self._load_raw(name)
367317

318+
_filter_properties(contents, allowlist, blocklist,
319+
child_filter, binding_path)
368320
_merge_props(merged, contents, None, binding_path, False)
369321
else:
370322
_err(f"all elements in 'include:' in {binding_path} "
@@ -384,17 +336,11 @@ def _merge_includes(self, raw: dict, binding_path: Optional[str]) -> dict:
384336

385337
return raw
386338

387-
388-
def _load_raw(self, fname: str,
389-
allowlist: Optional[List[str]] = None,
390-
blocklist: Optional[List[str]] = None,
391-
child_filter: Optional[dict] = None) -> dict:
339+
def _load_raw(self, fname: str) -> dict:
392340
# Returns the contents of the binding given by 'fname' after merging
393-
# any bindings it lists in 'include:' into it, according to the given
394-
# property filters.
395-
#
396-
# Will also register the (filtered) included property specs
397-
# into prop2specs.
341+
# any bindings it lists in 'include:' into it. 'fname' is just the
342+
# basename of the file, so we check that there aren't multiple
343+
# candidates.
398344

399345
path = self._fname2path.get(fname)
400346

@@ -406,55 +352,8 @@ def _load_raw(self, fname: str,
406352
if not isinstance(contents, dict):
407353
_err(f'{path}: invalid contents, expected a mapping')
408354

409-
# Apply constraints to included YAML contents.
410-
_filter_properties(contents,
411-
allowlist, blocklist,
412-
child_filter, self.path)
413-
414-
# Register included property specs.
415-
self._add_included_prop2specs(fname, contents, allowlist, blocklist)
416-
417355
return self._merge_includes(contents, path)
418356

419-
def _add_included_prop2specs(self, fname: str, contents: dict,
420-
allowlist: Optional[List[str]] = None,
421-
blocklist: Optional[List[str]] = None) -> None:
422-
# Registers the properties specified by an included binding file
423-
# into the properties this binding supports/requires (aka prop2specs).
424-
#
425-
# Consider "this" binding B includes I1 which itself includes I2.
426-
#
427-
# We assume to be called in that order:
428-
# 1) _add_included_prop2spec(B, I1)
429-
# 2) _add_included_prop2spec(B, I2)
430-
#
431-
# Where we don't want I2 "taking ownership" for properties
432-
# modified by I1.
433-
#
434-
# So we:
435-
# - first create a binding that represents the included file
436-
# - then add the property specs defined by this binding to prop2specs,
437-
# without overriding the specs modified by an including binding
438-
#
439-
# Note: Unfortunately, we can't cache these base bindings,
440-
# as a same YAML file may be included with different filters
441-
# (property-allowlist and such), leading to different contents.
442-
443-
inc_binding = Binding(
444-
self._fname2path[fname],
445-
self._fname2path,
446-
contents,
447-
require_compatible=False,
448-
require_description=False,
449-
# Recursively pass filters to included bindings.
450-
inc_allowlist=allowlist,
451-
inc_blocklist=blocklist,
452-
)
453-
454-
for prop, spec in inc_binding.prop2specs.items():
455-
if prop not in self.prop2specs:
456-
self.prop2specs[prop] = spec
457-
458357
def _check(self, require_compatible: bool, require_description: bool):
459358
# Does sanity checking on the binding.
460359

0 commit comments

Comments
 (0)