lopper: assists: zephyr: Fix UFS clock-name and clocks assignment#752
Closed
neeliajay wants to merge 17 commits into
Closed
lopper: assists: zephyr: Fix UFS clock-name and clocks assignment#752neeliajay wants to merge 17 commits into
neeliajay wants to merge 17 commits into
Conversation
Add new assist that scans the System Device Tree for devices under bus
nodes (e.g., simple-bus) and generates a YAML file containing a domain
with all devices in its access list. This enables glob patterns like
"dev: '*serial*'" to work without requiring a pre-existing parent domain.
The assist discovers all addressable devices (nodes with @ in name) under
bus-compatible nodes and generates a YAML structure:
domains:
sdt_all_devices:
compatible: lopper,sdt-devices-v1
access:
- dev: serial@ff000000
label: serial0
...
Usage:
lopper system.dts - -- sdt_devices -o /tmp/sdt-devices.yaml
lopper -f --permissive --enhanced \
-x '*.yaml' \
-i /tmp/sdt-devices.yaml \
-i my-domain.yaml \
system.dts output.dts
Options:
-b, --bus-types Comma-separated bus compatible strings (default: simple-bus)
-n, --domain-name Name for generated domain (default: sdt_all_devices)
-o Output file path
Also adds lop-sdt-devices.dts for automatic invocation and comprehensive
test coverage in tests/test_sdt_devices.py.
Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
When a domain node carries a 'lopper,activate' property, core_domain_access() now calls sdt.tree.overlay_tree(name) to obtain the merged tree for that condition before any processing begins. This makes conditional properties defined with the sigil syntax (e.g. 'chosen!linux:' or 'bootargs!linux!append') visible to all downstream processing — ref-counting, memory filtering, chosen node injection, etc. — without mutating the base tree. If 'lopper,activate' is absent, 'os,type' is used as the fallback so existing domain YAML files continue to work unchanged. If the named overlay does not exist (no sigil properties were defined for that condition), the base tree is used unmodified. Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
lnodes() builds __lnodes__ from label attributes present in DTS source. When a tree is loaded from a compiled DTB (without re-parsing source), labels may only survive in the __symbols__ node written by dtc -@. Add a __symbols__ fallback so lnodes() finds those nodes too. This fixes a potential regression introduced when resolve_node_by_label_or_path was removed from xlnx_overlay_pl_dt — that helper consulted __symbols__ explicitly, and the equivalent coverage now lives where it belongs: in the tree's own label-lookup primitive. Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
…omain driver binding
The "openamp pattern" places compatible!linux (and similar sigils) directly
on actual device nodes (e.g. /axi/timer@f1e90000) rather than under
/domains/. Domains opt into an overlay via lopper,activate; domains without
it see the base tree unchanged.
Ten new tests in TestOpenAMPPattern verify:
- base tree carries the vendor-neutral binding (cdns,ttc)
- linux overlay_subtrees includes the device node
- overlay_tree('linux') replaces compatible with uio
- base tree is not mutated after overlay_tree() is built
- APU_Linux carries lopper,activate; RPU1_BM does not
- DTS output per-OS reflects the correct binding
Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
…ions Sigil YAML (compatible!linux: "uio") builds per-condition overlay subtrees in memory at parse time. These were discarded when lopper wrote the output DTS, so a second invocation performing domain processing had no overlay_tree() data and silently used the base tree bindings for every domain, regardless of lopper,activate. This is the common split-pass workflow used in Vitis and gen-machine-conf pipelines: one lopper call translates YAML to DTS, a second runs domain_access to generate per-OS device trees. Three preservation mechanisms are added: 1. DTS embed (default, automatic) _serialize_overlay_subtrees() writes overlay data as JSON into a /__lopper-overlays__ node before DTS output. _deserialize_overlay_subtrees() reads and removes the node on the next invocation's load, making overlay_tree() work without any extra files or CLI flags. The node is absent from the final output DTS. 2. --emit-embedded-overlays (opt-in) Re-embeds /__lopper-overlays__ on every pass. Required for pipelines with more than two lopper invocations; without this flag the node is consumed and removed on the first deserializing pass. 3. --emit-overlay-sidecar (opt-in) Writes <out>.overlays.yaml alongside the output DTS. Feed with -i on subsequent passes. Full merge-scheme fidelity (append/prepend/delete) because it round-trips through the sigil YAML parser. 4. --emit-overlay-dtso (opt-in) Writes one <out>.<condition>.dtso per condition. Standard DTS overlay format, human-inspectable; merge schemes are not representable so output is always replace-mode. The three opt-in flags are tracked as a set (sdt.overlay_emit) rather than separate booleans so future modes can be added without API churn. Lopper.path_to_prop_name() / prop_name_to_path() are added to base.py as reversible encoders for embedding abs_paths as DTS property names inside /__lopper-overlays__. _LOPPER_INTERNAL_NODES frozenset replaces the per-path string check in LopperNode.print(), making it easy to add future internal nodes. /__lopper-overlays__ is intentionally excluded so it is emitted to DTS for pass-2 consumption. Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
Two related bugs caused cpu_expand to crash with 'NoneType has no attribute subnodes' when processing domains where the cluster label resolves to None in the current tree context: 1. cpu_expand initialised cluster_cpu = None before the loop but not cluster_node. If cpus[0] contained only empty/falsy entries (all skipped by the 'if not c: continue' guard), cluster_node was never assigned and the call to openamp_remote_cpu_expand at the end of the function passed an uninitialised name, which Python resolves to whatever cluster_node was in the enclosing scope — or raises UnboundLocalError. Initialise cluster_node = None alongside cluster_cpu. 2. openamp_remote_cpu_expand returned early when cluster_cpu is None but did not guard the cluster_node.subnodes() call on line 1025 against cluster_node being None. If cluster_cpu is set but the label lookup for the cluster failed (cluster_node = None from the except branch), the function would crash. Guard the subnodes() call with 'if cluster_node is not None'. Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
On Versal2 SoCs the TTC timer nodes live at the device tree root
rather than under an /axi bus. The previous implementation hard-coded
tree["/axi"].subnodes() as the only search scope, causing an error
("xlnx_timer_expand requires timer label reference") and dropping the
timer from the output whenever the label could not be resolved there.
Fall back to a whole-tree search via tree["/"].subnodes() when the /axi-
scoped lookup returns nothing. The /axi path is still tried first so
the behaviour is unchanged for SoCs where the timer genuinely lives
under /axi.
Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
YAML sigil nodes are placed by the YAML parser under the structure declared in the YAML file (e.g. /axi/timer@f1e90000). When the base SDT locates the same physical device at a different path (e.g. /timer@f1e90000 on Versal2) the direct path lookup fails and the conditional property override is silently dropped. Add a fallback: when tree[ov_node.abs_path] raises, search tree.__nodes__ for the unique node whose name matches the last path component. Only apply the fallback when exactly one match is found, to avoid ambiguous merges on SoCs that have multiple nodes with the same name at different addresses. Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
The existing overlay tests exercise serialize/deserialize in-process but
never actually invoke lopper twice as separate processes. Add
TestTwoPassSubprocess to validate the full real-world workflow:
Pass 1 — lopper translates system-top.dts + sigil YAML to
intermediate.dts with /__lopper-overlays__ embedded.
Pass 2 — lopper runs domain_access on intermediate.dts; the overlay
node is deserialized, overlay_tree('linux') activates via
lopper,activate, and the final DTS has uio (not cdns,ttc).
Five assertions cover: pass-1 exit status, /__lopper-overlays__ present
in intermediate, uio binding in APU_Linux output, /__lopper-overlays__
absent from final output, and cdns,ttc preserved for the base-tree
RPU1_BM domain.
Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
- Add PS-I3C handler to set compatible - zephyr_supported_comp.yaml: Add snps,dw-i3c-master-1.00a entry with required properties. Signed-off-by: Shubham Patil <ShubhamSanjay.Patil@amd.com>
…for_refs()
Previously overlay_of() generated fragments via fragment_add_for_refs()
and then deleted excluded properties from them afterward. Any fragment
whose only property was excluded ended up as an empty shell (&label {})
in the output, requiring a post-processing regex to clean it up.
Pass exclude_props into fragment_add_for_refs() so excluded properties
are skipped at collection time. A fragment is only created when at least
one non-excluded property remains, so empty fragments are never produced.
Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
…al data
When lopper.ini sets bool_as_int=True, YAML boolean properties (e.g.
"ranges: true") are encoded as [1]. If the base tree already has that
property as a zero-length/flag DT property (ptype EMPTY, value ['']),
merging [1] via LopperProp.merge() would clobber [''] with bare int 1,
causing resolve() to output "ranges = <0x1>;" instead of "ranges;".
Fix merge() to detect when an EMPTY-typed property receives a boolean-
numeric value and apply proper DT boolean semantics regardless of source:
- incoming [1]/True → preserve the flag property unchanged
- incoming [0]/False → remove the property (absent = false in DT)
- incoming real data → adopt the value and update ptype away from
EMPTY so the property encodes/resolves correctly
The guard is source-agnostic: it also fixes the previously broken case
of merging a real ranges tuple from DTS into an empty ranges; property,
which would have produced ['', 0x0, 0x0, 0x40000000] via concatenation.
Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
…ag property When bool_as_int=False, a YAML boolean true was stored as None (then [None] via __setattr__ list-wrapping) instead of [''] — the canonical lopper representation for an EMPTY/flag DT property. The in-code comment already said "a true is just an empty list" but the code assigned None instead. Fix: assign [''] so the YAML path produces the same representation as the FDT path for zero-length properties, flowing correctly through merge and resolve regardless of bool_as_int setting. Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
…ean encoding
Add tests for two related bug fixes:
TestEmptyPropMergeBooleanBug (covers ff48f7cf):
- [1]/True incoming preserves EMPTY flag property unchanged
- [0]/False incoming removes EMPTY flag property from its node
- real multi-element data adopts value and updates ptype from EMPTY
- [''] -> [''] merge (DTS flag into DTS flag) is a no-op
TestYAMLBooleanFalseEncoding (covers 7e2114f2):
- bool_as_int=False + true -> [''] (was [None], broken)
- bool_as_int=False + false -> property absent (skip=True path, correct)
- bool_as_int=True + true -> [1] (existing behaviour, unchanged)
Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
…y node Other parts of lopper handle /reserved-memory node 'ranges' property setup. Remove such handling from this plugin. Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>
Ensures that the clock-names property for UFS nodes is set correctly. UFS nodes to have two clock_nodes defined in generated DTS. Signed-off-by: Ajay Neeli <ajay.neeli@amd.com>
kedareswararao
approved these changes
May 4, 2026
Collaborator
|
sorry to make work, but my screw up on master has made this conflict. Can you rebase it onto current master and resend. There's a non FF merge causing issues (me fixing things) |
|
Please close this PR as i have created a new PR: #754 CC: @kedareswararao |
Collaborator
|
closing as requested. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ensures that the clock-names property for UFS nodes is set correctly.
UFS nodes to have two clock_nodes defined in generated DTS.