base: phandle_safe_name: replace all invalid label characters#753
Open
Rajukumar45 wants to merge 20 commits into
Open
base: phandle_safe_name: replace all invalid label characters#753Rajukumar45 wants to merge 20 commits into
Rajukumar45 wants to merge 20 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>
In to_tree()'s serialize_json path, 'ranges: true' was producing [1] when bool_as_int=True, which resolve() rendered as 'ranges = <0x1>;' instead of the correct 'ranges;'. The root cause: the pre-processing loop preserved [1] for boolean true under bool_as_int=True, and the resulting LopperProp was a fresh node (no merge guard fires when /reserved-memory doesn't already exist in the SDT), so [1] reached resolve() unmodified. Fix: boolean true always maps to [] (zero-length list) regardless of bool_as_int. The bool_as_int flag only controls false encoding ([0] vs skip). An empty list forces resolve() into its len==0 branch, which emits the correct bare 'ranges;' flag syntax. Also fix the non-serialize_json bool branch to apply bool_as_int when encoding false, matching the serialize_json path behaviour. Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
…ree paths Add TestYAMLBooleanFalseEncoding class with four tests: - test_bool_true_produces_empty_string_list: bool_as_int=False, True -> empty flag - test_bool_false_skips_property: bool_as_int=False, False -> property absent - test_bool_true_with_bool_as_int_produces_empty: bool_as_int=True, True -> empty flag - test_reserved_memory_ranges_true_no_existing_node_resolves_as_flag: regression for the serialize_json path where a fresh /reserved-memory node with 'ranges: true' must resolve to 'ranges;' not 'ranges = <0x1>;' Also update two pre-existing assertions from == [''] to in ([], ['']) to match the actual [] value produced by the fixed serialize_json path. Signed-off-by: Bruce Ashfield <bruce.ashfield@amd.com>
Add the optional xlnx,vpss property to the Xilinx HDMI 2.1 TX Subsystem dt-binding. The property holds a phandle to the VPSS node and is required only when VPSS is connected to HDMI as a bridge. Signed-off-by: Rajesh Gugulothu <rajesh.gugulothu@amd.com>
DTS labels must be valid C identifiers ([a-zA-Z_][a-zA-Z0-9_]*), but node names can contain characters from the set [0-9a-zA-Z,._+-]. Previously, phandle_safe_name() only replaced '@', '-', and '/'. This left ',' , '.' and '+' in the generated label, producing invalid DTS output (e.g., "pcie0,0:" for node pcie@0,0). Add replacements for ',', '.', and '+' so that all non-identifier characters in node names are converted to '_'. Signed-off-by: Raju Kumar Pothuraju <rajukumar.pothuraju@amd.com>
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) |
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
4 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.
DTS labels must be valid C identifiers ([a-zA-Z_][a-zA-Z0-9_]*), but node names can contain characters from the set [0-9a-zA-Z,._+-].
Previously, phandle_safe_name() only replaced '@', '-', and '/'. This left ',' , '.' and '+' in the generated label, producing invalid DTS output (e.g., "pcie0,0:" for node pcie@0,0).
Add replacements for ',', '.', and '+' so that all non-identifier characters in node names are converted to '_'.