Skip to content

lopper: generate Zephyr board.overlay from SDT folder - #790

Open
dbingi-amd wants to merge 2 commits into
devicetree-org:masterfrom
dbingi-amd:lopper-zephyr
Open

lopper: generate Zephyr board.overlay from SDT folder#790
dbingi-amd wants to merge 2 commits into
devicetree-org:masterfrom
dbingi-amd:lopper-zephyr

Conversation

@dbingi-amd

Copy link
Copy Markdown
Contributor

SDTGEN now copies Zephyr board fragments into the SDT output folder as sidecar .dtsi files ({board}_zephyr.dtsi and optional user input via -user_zephyr_dts), instead of passing a single board .dts file to lopper.
This PR adds SDT-folder-based discovery and board.overlay generation in lopper, and replaces the old single-file overlay handling in gen_domain_dts. A follow-up commit adds pytest unit tests to validate discovery rules and all three overlay flows
• Add discover_zephyr_board_files() to find {board}_zephyr.dtsi and user .dtsi files.
• Treat user DTSI as any other .dtsi not #included from system-top.dts.
• Add generate_board_overlay_from_sdt() for board-only, board+user, and user-only flows.
• Filter board overlay against the SDT tree; append user overlay unchanged when present.
• Update gen_domain_dts.py to call the new helper instead of reading one board file.
• Add tests/test_zephyr_board_overlay.py for discovery and overlay generation unit tests

@dbingi-amd

Copy link
Copy Markdown
Contributor Author

@kedareswararao: please review the changes

Scenario 2: board + user DTS -> filter board, append user unchanged
Scenario 3: user DTS only -> copy user content unchanged
"""
sdt_folder = resolve_sdt_folder(options, sdt)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we should support legacy use case as well where users pass the board dts instead of sdt folder

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acknowledged. I will add backward compatibility

Comment thread lopper/assists/zephyr_board_dt.py Outdated
print(f"[INFO] Generated board.overlay from filtered '{os.path.basename(board_dtsi)}' and user '{os.path.basename(user_zephyr_dtsi)}'.")
elif board_dtsi:
overlay_parts.append(
process_overlay_with_lopper_api(

@kedareswararao kedareswararao Jul 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This process_overlay_with_lopper_api() uses pure python calls recently Bruce added support for handling this kind of use cases in lopper frontend can you see whether we can remove the python API's and replace it with lopper API's

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed the Lopper overlay frontend APIs, but they are designed for true Device Tree overlays that are merged into the SDT tree. In contrast, our Zephyr board use cases rely on .dtsi include fragments containing both root-level blocks and node-specific sections. The reference validation logic already uses the Lopper tree APIs, and the remaining text-filtering functionality does not have a direct equivalent in the overlay frontend APIs. Therefore, we chose to retain this helper implementation.

@kedareswararao kedareswararao left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes looks fine to me

@zeddii

zeddii commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the work here. Before this lands I'd like to redirect the overlay handling, because the current approach re-implements a large amount of what lopper core (and dtc) already do, with text processing that will be fragile to maintain.

What lopper core already does with .dtsi fragments

A .dtsi is an include-and-compile artifact. When it's included and compiled, core already gives you everything this PR does by hand:

  • &label references are resolved by lopper's symbol resolution against the base tree;
  • properties whose phandle references don't resolve are dropped on write (strict mode, the default);
  • a reference to a label that genuinely doesn't exist is a hard compile error — which is what we want surfaced, not silently deleted.

In fact a .dtsi handed to lopper as an input is already concatenated with the base and compiled — setup() routes non-/plugin/ .dtsi into the base compile unit. So this isn't new machinery; the assist just needs to do the same thing internally.

Where this PR duplicates that

process_overlay_with_lopper_api() and its helpers reimplement the above in Python/regex:

  • re.findall(r'&(\w+)', …) / re.findall(r'(\w+):\s*…{', …) extract references and labels by regex instead of parsing;
  • check_reference_with_lopper_api() / _check_subnodes_recursive() hand-walk /aliases, /__symbols__ and recurse the tree to test whether a symbol exists — that is core symbol resolution, re-derived;
  • clean_overlay_content() brace-counts over raw text to delete &ref { … } blocks;
  • apply_final_cleanup() regex-scrubs the formatting.

That's several hundred lines duplicating the compiler and lopper's resolver, and text/brace/regex handling of DTS will break on the first fragment that doesn't match the assumed formatting. The root problem is that these .dtsi are processed externally when they should be included and compiled.

Path forward — step 1: include + compile

Keep taking the fragment as an assist argument — the invocation doesn't need to change. The fix is what the assist does with it: hand it to the compiler instead of parsing it. Compose the base + fragment and compile (note dt_compile() doesn't consume i_files — includes are handled by concatenation, the same way setup() seeds base .dtsi inputs):

import os, shutil
from lopper import Lopper
from lopper.tree import LopperTree

system_top = os.path.join(sdt_folder, "system-top.dts")

# compose base + fragment into one source, then compile it
combined_src = os.path.join(sdt.outdir, "_board_combined.dts")
with open(combined_src, "wb") as out:
    for src in (system_top, board_dtsi):
        with open(src, "rb") as f:
            shutil.copyfileobj(f, out)

compiled, _ = Lopper.dt_compile(combined_src, "", "", True,
                                sdt.outdir, sdt.save_temps)

combined = LopperTree()
combined.load(Lopper.export(Lopper.dt_to_fdt(compiled)))
# combined = base + fragment, fully resolved by core:
#   &label refs resolved, unresolved phandle refs dropped on write,
#   an undefined label is a hard compile error. No regex, no
#   __symbols__ walking, no brace-counting.

(Equivalently, register the fragment into the SDT's input files so setup() concatenates it — whatever fits the assist structure.) Either way this needs nothing new from lopper core.

Step 2 depends on one question about the Zephyr side

What consumes board.overlay — does the Zephyr build require a separate overlay file, or can it take a complete generated board device tree?

  • If it can take a full board DT, we're done at step 1: emit combined and there's no overlay to filter at all.
  • If a separate overlay file is required (Zephyr owns a base board DT that must be augmented, not replaced), the right way to produce it is a tree-delta — compare combined against the base and emit the difference as an overlay fragment. That belongs as a general lopper capability (advanced tree delta / diff with an overlay output format), not text filtering in one assist. If that's the requirement, I'll take care of that piece.

Could you confirm the Zephyr consumption? That decides between "emit the combined tree" and "emit a computed delta," and either way the hand-rolled parsing should come out.

(+1 to keeping the legacy single-file path working.)

@kedareswararao

Copy link
Copy Markdown
Contributor

@zeddii : Thanks for the suggestion combined dt also works for zephyr consumption
@dbingi-amd : give a try and let us know your observations

Merge board and user Zephyr DTSI content into the domain tree instead of
emitting a separate board.overlay file. Use lopper plugin compile for
&label fragments and merge root /aliases and /chosen nodes with fragment
path rewriting so board aliases such as eeprom-0 appear in the final DTS.

Signed-off-by: Bingi Dinesh kumar <dineshkumar.bingi@amd.com>
Add pytest coverage for SDT-folder board/user DTSI discovery and combined
domain-tree merge paths in zephyr_board_dt.

Signed-off-by: Bingi Dinesh kumar <dineshkumar.bingi@amd.com>
@dbingi-amd

Copy link
Copy Markdown
Contributor Author

@zeddii,
Thanks for taking the time to review this.

Step 1: I switched to the lopper compile/include path instead of regex.

  • I removed process_overlay_with_lopper_api() and all regex/brace-based overlay text filtering. Merge now relies on the lopper core APIs:
    • &label { } content (for example, kcu105_zephyr.dtsi): compile_overlay_standalone → _unwrap_overlay_tree → _merge_node_into_tree → _resolve_overlay_fixups, plus root /aliases and /chosen merge with fragment path rewriting.
    • Root-only / { ... } content (no & refs): domain base + fragment concat through Lopper.dt_compile() / LopperSDT.setup() — the same pattern used for lopper include handling.

  • A plain concat + dt_compile path is not enough for a full board .dtsi with & refs, because optional or missing labels (for example, axi_ethernet_0 on kcu105) would cause the entire compile to fail; the plugin path merges the matching fragments and skips the rest.

Step 2: I split out overlay vs. combined board DT.

  • Per @kedareswararao's direction, I implemented a combined board DT. During gen_domain_dts, the Board/user Zephyr DTSI is merged into sdt.tree, and no board.overlay is generated. Zephyr uses the single merged board .dts (for example, mbv32.dts).

@kedareswararao:

  • I tested end to end and verified the combined DT for zephyr.
    Observation: the reference board .dtsi may include optional IP that is not present in every XSA; unmatched & fragments are skipped with a warning, while matched content and aliases are merged. We can switch to a hard failure on missing labels if preferred.

@zeddii

zeddii commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Thanks -- good progress, and glad the combined DT works for the Zephyr side.

On the missing-label question: default should be an error. dtc won't build a plain .dtsi with an unresolved label (I checked -- even -f is fatal: "Label or path ... not found"), so allowing it silently means the .dtsi isn't really a .dtsi anymore. If a file legitimately needs to reference optional IP that may be absent in a given XSA, that is overlay content -- dtc supports undefined labels there via __fixups__, and that's the mechanism you're already using through compile_overlay_standalone / _unwrap_overlay_tree. So those reference board files should be treated as overlays, not include fragments.

If we do want lopper to tolerate an unresolved label in the include/compile path, wire it to the existing --permissive flag and make it a warning, not an error (and not a silent skip in the strict/default path). That keeps the default honest -- an unresolved label fails the build -- while giving an explicit, opt-in escape hatch.

Net: default = hard error; intentional optional refs = overlay; --permissive = warn-and-continue.

@zeddii

zeddii commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

A clarification / correction to my --permissive suggestion above, now that we've pinned down what dtc actually allows.

--permissive can't make a concatenated .dtsi tolerate an unresolved label -- that's fatal at dtc parse, before any lopper leniency can run, and there's nothing to "skip" in a concatenated compile. The only way to get skip-with-warning is for the assist to compile the fragment separately as a /plugin/ overlay (unresolved refs become __fixups__; _unwrap_overlay_tree skips the ones absent from the base). So --permissive only has meaning if the assist does that conversion -- it is not something the normal include/concat path can offer.

And that conversion has a hard cost, because the two dtc models are complementary, not superset/subset:

Model unresolved labels delete base nodes/props
.dtsi (concatenated + recompiled) ❌ fatal /delete-node/, /delete-property/
/plugin/ overlay (compiled standalone) ✅ (fixups) /delete-node/ won't compile; /delete-property/ no-ops on apply

So the two capabilities are mutually exclusive:

  • Need to delete base nodes/properties → it must stay a .dtsi (concatenated) → no unresolved symbols.
  • Need to tolerate unresolved/optional labels → it must be an overlay → no base deletions.

No single fragment gets both. So the real per-fragment question is: does it need to remove base content, or reference optional IP that may be absent? It can't do both, and --permissive doesn't change that -- at most it would select the overlay path (gaining unresolved-tolerance, losing deletes).

@kedareswararao

Copy link
Copy Markdown
Contributor

@zeddii : The board dts contains all possible board-related information, such as the Ethernet node, flash info, eeprom info, and similar details. The purpose of passing this file to lopper assist is that, if a given design does not include Ethernet, the assist logic should remove the Ethernet node references from the user-provided board dts and, if needed, return an informational or warning message indicating that the relevant nodes were deleted because they are not mapped in the design. I hope this clarifies the use case; if not, please let me know and I’ll be happy to clarify further.

CC: @sivadur @dbingi-amd

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants