edtlib: PropertySpec.path API might be incorrect or misleading

[dottspina]

I'm working on some kind of DTS viewer (rfc-tsh), and, when listing the properties of a node, I'd like to show which binding files specify them.

The edtlib.PropertySpec.path attribute seemed promising:

    path:
      The file where this property was defined. In case a binding includes
      other bindings, this is the file where the property was last modified.

But path appears to always point to the top-level binding file, not the file where the property was last modified.

For example, let's look at a flash controller with bindings ("<" means "included by"):

pm.yaml < base.yaml < flash-controller.yaml < nordic,nrf52-flash-controller.yaml

In my understanding, enumerating the flash controller properties and the YAML files where they were last modified should then show:

Property	Spec from	Spec last modified by
wakeup-source	pm.yaml	pm.yaml
zephyr,pm-device-runtime-auto	pm.yaml	pm.yaml
compatible	base.yaml	base.yaml
reg	base.yaml	flash-controller.yaml
partial-erase	nordic,nrf52-flash-controller.yaml	nordic,nrf52-flash-controller.yaml

If this is correct, the issue is that, for all properties defined for the flash controller node, PropertySpec.path points to the top-level nordic,nrf52-flash-controller.yaml binding file.

And indeed, Binding constructor:

• 1st loads the top-level YAML binding file
• then recursively merges all included YAML files
• and eventually populates prop2specs with all the gathered property specs

Since PropertySpec.path is simply a shortcut to PropertySpec.binding.path, all these property specs will claim they were last modified by the top-level binding file, nordic,nrf52-flash-controller.yaml.

Unfortunately, this means we can't fix PropertySpec.path without fixing PropertySpec.binding.

After going around in circles a bit, I found that:

• Binding._load_raw() is called for every included YAML file, and only for included YAML files (not for the top-level bindings that just use yaml.load())
• it's there that Binding._merge_includes() turns recursive

And the approach I came to is:

• assuming only included bindings may have constraints like "property-allowlist", we can apply these in _load_raw()
• then also register the (filtered) included property specs into Binding.prop2specs from there, taking care of the last modified semantic: if B includes I1 which itself includes I2, processing I2 should not "take ownership" for a property that I1 modifies
• eventually, at the end of Binding constructor, once all _merge_includes() are done, register into prop2specs only the property specs the (top-level) binding file modifies

I drafted a patch (attached, see bottom of this issue), and it seems to work:

• property specs actually point to the files where they were last modified (according to the table above, still)
• python-devicetree unit tests still pass

I didn't open a PR, though:

• I don't like having to instantiate bindings for each included YAML file, especially since we can't cache re-usable instances (as the same file may be included with different filters, "property-allowlist" and such)
• I like touching the whole Binding.prop2specs thing even less, since this directly affects Zephyr everyday behavior (starting with west build): the safest patch might be to instead fix the API doc

I thought about other ways to implement my use case, but I always come back to no per-spec Binding, no per-spec file.

Would anyone familiar with EDT be kind enough to:

• help me achieve a proper patch, that can be confirmed to not mess with normal EDT models initialization
• or suggest an alternative approach (to retrieve the origin of a property) that will not require to patch edtlib

Thanks.

The patch: edtlib-issue-PropertySpec.zip
I had to zip the patch file to work-around "We don’t support that file type" errors, I don't know why. Sorry for the inconvenience.

[henrikbrixandersen]

Please open a PR for submitting the patch instead and people will review/comment on that.

[dottspina]

Closing this, fixed by #65221.