Skip to content

lopper: assists: xlnx_overlay_pl_dt: add --user-dtsi option to append user overlay content#718

Open
arthokal wants to merge 1 commit intodevicetree-org:masterfrom
arthokal:master
Open

lopper: assists: xlnx_overlay_pl_dt: add --user-dtsi option to append user overlay content#718
arthokal wants to merge 1 commit intodevicetree-org:masterfrom
arthokal:master

Conversation

@arthokal
Copy link
Contributor

@arthokal arthokal commented Mar 13, 2026

Created a new PR with latest changes.

Add --user-dtsi=<path> option allowing users to append custom overlay content to the generated pl.dtsi output. The provided file undergoes lightweight validation: checks for presence of at least one reference node (&label { ... }) and rejects /dts-v1/ and /plugin/ headers.

Also refactor validate_and_parse_options() to return a named tuple (OverlayOptions) instead of a bare tuple, and warn on unrecognized options.

… user overlay content

Add --user-dtsi=<path> option allowing users to append custom overlay
content to the generated pl.dtsi output. The provided file undergoes
lightweight validation: checks for presence of at least one reference
node (&label { ... }) and rejects /dts-v1/ and /plugin/ headers.

Also refactor validate_and_parse_options() to return a named tuple
(OverlayOptions) instead of a bare tuple, and warn on unrecognized
options.

Signed-off-by: Aravind Thokala <aravind.thokala@amd.com>
@zeddii
Copy link
Collaborator

zeddii commented Mar 13, 2026

You closed the old pull request without answering my question. Those questions still stand.

@arthokal
Copy link
Contributor Author

arthokal commented Mar 13, 2026

Hi @zeddii,

Adding questions from previous closed PR here:

I don't have enough time right now to review this fully at the moment, but I do have some questions for more information.

What is the high level goal of this functionality ?

The user is supplying a dtsi that is then .. what ? Just read in and written out as-is to the generated overlay ? Something else ?

The reason I ask is that at a glance, this seems to be duplicating something that lopper-core can do, and then pass to the assist. i.e. lopper can read overlays, and would apply it to the core tree.

I also have some prototype work that completes the ability to read something in and hold it as a secondary tree until an assist or lop asks for it to be merged the main tree.

In either of those scenarios, you'd get the front end syntax validation, you'd get the core semantic checks, and shortly access to the audit framework.

After all those pass it would be merged into the working tree, so when you generate an overlay from lopper, the user changes would be incorporated into the output.

Adding @onkarharsh to the discussion.

@onkarharsh
Copy link
Contributor

Hi @zeddii,

This is more related to the prototype work that you mentioned above. In this case, the user dtsi must not be merged to the base system device tree, instead this needs to go to the pl overlay as is, which is generated after the assist operation. We were not aware that you already have something of this sort in the front-end. We do need this support especially for creating overlays if this PR is going to be rejected. Do you have a timeline for the proposed solution? This Pull Request is critical for our release timeline.

@zeddii
Copy link
Collaborator

zeddii commented Mar 16, 2026

It may not be ready for a few weeks yet, there are other pressing issues.

But so I understand better as to what you are trying to do. Why can't those changes be merged into the main tree, when the overlay's should be created/pulled out of the main tree.

Is it that those use supplied additions aren't fully "trusted" ? something else ?

What assists do you see / want in the pipeline when this overlay is created ?

@arthokal
Copy link
Contributor Author

Hi @zeddii

One use case of not merging changes into main tree is this:

Sometimes a user needs to update a PS block (like a display controller) to use clocks coming from the PL side. These PL clocks are provided by IPs in the FPGA design and only become available after the FPGA bitstream is loaded.

If these changes are added to the base device tree, Linux will try to use those PL clocks at boot — before the FPGA is programmed — and fail because the clocks don't exist yet.

That's why these changes need to go into the PL overlay, so they only take effect after the FPGA is up and the PL clocks are actually available.

Beyond PS-to-PL dependencies, it also lets users disable PL nodes, add custom properties or apply any overlay-level tweak.

@zeddii
Copy link
Collaborator

zeddii commented Mar 17, 2026

But if we create an overlay from the base tree, the logic of lopper-core is that those resources should be removed from the base device tree, and hence linux won't see them.

If that was the case, would that meet the need. Yes, I'll still do a feature to carry the supporting/secondary trees, but I don't think it is needed in this case.

The point is that, if it loads with the main tree, we can validate that it is correct, then pull it out as an overlay and again, validate that it is correct. If it arrives late like that to an assist, we can't do that.

@arthokal
Copy link
Contributor Author

Hi @zeddii

Using -i <user.dtsi>, Lopper merges user modifications into base tree before assist runs. This validates user content via dtc compilation as you recommended.

Three use cases are identified:

  1. PS-only modifications: Properties stay in base DT. This is correct, PS resources are always available.
  2. PL-only modifications: Properties automatically extracted with /amba_pl into overlay.
  3. PS-to-PL references: Example: PS display controller references PL clocks. This is the gap that we are discussing about.

Problem with use case 3:

After -i merges for example &zynqmp_dp = { clocks = <&pl_clk_0> to base tree, PS node has a PL dependency.
If this stays in base DT, Linux tries to resolve PL clocks at boot before FPGA loads causing boot failure.

Proposed solution:

  1. Collect all PL phandles
  2. Scan non-PL nodes for PL phandle references
  3. Create overlay nodes for those PS properties
  4. Remove PL-referencing properties from base DT
  5. Add PS-to-PL overlay nodes to pl.dtsi

Issue: Mixed PS+PL phandles

Step 4 has a problem when a property contains both PS and PL phandles.

Original:    &zynqmp_dp { clocks = <&ps_clk>; };
User adds:   &zynqmp_dp { clocks = <&ps_clk>, <&pl_clk_0>; };
After step 4: &zynqmp_dp { /* clocks deleted entirely — ps_clk lost! */ };

Deleting entire property loses original PS clock reference. This mixed PS+PL phandle use case appears valid(PS clock available at boot, PL clock added later).

Could you please let us know how we should handle properties containing both PS and PL phandle references?

CC: @onkarharsh , @sivadur

@zeddii
Copy link
Collaborator

zeddii commented Mar 20, 2026

Understood. Let me ask a clarifying question

you say: "After -i merges for example &zynqmp_dp = { clocks = <&pl_clk_0> to base tree, PS node has a PL dependency. If this stays in base DT, Linux tries to resolve PL clocks at boot before FPGA loads causing boot failure."

So the clock is in the pl (the overlay), lopper by default will delete the clock if the phandle target node isn't present in the device tree. If we pull that clock into the overlay, then it will no longer be resolved. Lopper can delete it. Lopper understands "records" in a property, so it is supposed to only delete the record with the undefined phandle.

A secondary pass isn't triggered now, but it could be (with some associated tweaks).

If what I describe starts working, would that meet the need ?

I know where it is likely going wrong and if you get me some sample device trees and expected results, I can get it working.

@arthokal
Copy link
Contributor Author

arthokal commented Mar 23, 2026

Hi @zeddii

Lopper's strict mode successfully drops invalid phandle records.

Input:  clocks = <&ps_clk>, <&ps_clk2>, <&pl_clk>;
Output: clocks = <&ps_clk>, <&ps_clk2>;  /* pl_clk dropped */

But, there are two gaps that I identified:

Gap 1: Property synchronization
Problem:
Related *-names properties not updated when phandle records are dropped. For example:

clock-names = "mmi_pll", "ps_vid_clk", "stc_ref_clk", "pl_vid_func_clk", "pl_aud_clk"; /* PL clocks names still appear */
clocks = <&versal2_clk 0xd7>, <&versal2_clk 0xda>, <&versal2_clk 0xdf>; /* PL clocks are dropped */

Does strict mode only process phandle-bearing properties and not string arrays?

Gap 2: No data capture for overlay
Problem:
Dropped phandle information is discarded. May I know how this dropped information be added to final overlay?

@zeddii
Copy link
Collaborator

zeddii commented Mar 23, 2026

We'll have to write something custom for the -names. Because the actual values of the -names are not part of the device tree spec, they can vary. If we have an input schema, then yes, we can enforce it.

But that's why it is left alone. You can't be sure which name to drop. That being said, I can do it by index and be right in almost all cases. That could be something that can something configurable (in case it gets it wrong).

Invalid phandles aren't dropped until an output is created. So they are in the tree, with their invalid numeric value, just on output they are resolved and dropped if they are invalid.

Remind me of the overlay phandles. Don't they have to be valid to the base device tree ? Or are they referring to resources in other overlays or external information ?

@onkarharsh
Copy link
Contributor

onkarharsh commented Mar 24, 2026

Hi @zeddii,

These phandles are rendered invalid in base device tree because they are part of the PL bus and the PL bus is carved out from the DT as part of the overlay. When we are dropping such properties from the base device tree, we need to put them back in the carved overlay so that they come into effect after FPGA programming.

Let me try to summarize the complete use case that we are trying to support:

  • Existing Usage:

SDT -> overlay assist ->

Outputs two files:

          a) linux device tree (without PL bus) 
          b) an overlay file pl.dtbo (PL bus converted to an overlay)

Now, when someone wants to insert an overlay on top of the existing pl.dtbo, we need to manually add an include file in the generated overlay and then check its correctness with DTC. That's where we are trying to make use of lopper.

  • Proposed Usage:

SDT -> overlay assist (with user overlay as input) ->
Outputs two files:

          a) linux device tree (without PL bus and without the user overlay) 
          b) an overlay file pl.dtbo (PL bus converted to an overlay + the user overlay)

Based on the above discussions, I see we are trying to focus on below two areas:

  1. The overlay passed by the user must be syntactically correct and that can only be checked when it is merged with the system device tree and compiled. This is what lopper front end is doing and it is needed.
  2. At the same time, we must preserve the info contained in overlay and not merge it to the generated linux device tree. This must be added only to the final PL overlay that we are carving out.

We see few issues in achieving step2 with the frontend:

  1. If we first merge the overlay DT to full SDT, removing those entries from the linux dt output is not easy. The suggestion is to rely on the frontend checks for the invalid phandle, invalid names etc. This works for phandles because those phandles are defined in the PL bus which is removed via the assist. This seems to be a reactive approach as the list of such properties can be an ever increasing one (after clocks, interrupt related props can be the next).

  2. Removing those entries from linux device tree still seems fine, but we need the overlay passed by the user in its actual form while creating the pl.dtbo without any manipulation. And this also seems difficult with the proposed suggestion.

Given the complexity involved here, the current suggestion doesn't seem to work. We only want to check the compatibility of this secondary tree coming from overlay with the system device tree, but we don't want to merge this secondary tree to the primary tree. That's where we thought your secondary tree related prototype may come to our rescue.

Another possible solution that I can think is to define a separate bus (say overlay-bus) for all such use cases and use that bus in our assists depending on our use cases. For overlays, that bus will be merged to the PL bus. And since they will be passed to lopper frontend, the phandle and other props syntax will also be in check.

Let me know your opinion on this summary and the introduction of this new bus. Any suggestions are most welcome.

@zeddii
Copy link
Collaborator

zeddii commented Mar 24, 2026

Hi @zeddii,

These phandles are rendered invalid in base device tree because they are part of the PL bus and the PL bus is carved out from the DT as part of the overlay. When we are dropping such properties from the base device tree, we need to put them back in the carved overlay so that they come into effect after FPGA programming.

Right. That's the core purpose of the overlay_of() routines, they can pull components out, and remove them from the base device tree. We shouldn't lose anything outside of invalid references in the base tree. If we pull those refs into the overlay, then we'll have the same set as the single tree, just split across the two places, and that's completely valid.

Let me try to summarize the complete use case that we are trying to support:

  • Existing Usage:

SDT -> overlay assist ->

Outputs two files:

          a) linux device tree (without PL bus) 
          b) an overlay file pl.dtbo (PL bus converted to an overlay)

Now, when someone wants to insert an overlay on top of the existing pl.dtbo, we need to manually add an include file in the generated overlay and then check its correctness with DTC. That's where we are trying to make use of lopper.

  • Proposed Usage:

SDT -> overlay assist (with user overlay as input) ->
Outputs two files:

          a) linux device tree (without PL bus and without the user overlay) 
          b) an overlay file pl.dtbo (PL bus converted to an overlay + the user overlay)

Those match my understanding of what we are trying to do.

Based on the above discussions, I see we are trying to focus on below two areas:

  1. The overlay passed by the user must be syntactically correct and that can only be checked when it is merged with the system device tree and compiled. This is what lopper front end is doing and it is needed.
  2. At the same time, we must preserve the info contained in overlay and not merge it to the generated linux device tree. This must be added only to the final PL overlay that we are carving out.

We see few issues in achieving step2 with the frontend:

  1. If we first merge the overlay DT to full SDT, removing those entries from the linux dt output is not easy. The suggestion is to rely on the frontend checks for the invalid phandle, invalid names etc. This works for phandles because those phandles are defined in the PL bus which is removed via the assist. This seems to be a reactive approach as the list of such properties can be an ever increasing one (after clocks, interrupt related props can be the next).

That's expected and handled by the core when creating an overlay, the complexity isn't anything than has been designed.

Lopper could track nodes and property on the way in, there's other uses of this in the future, but that's a complexity we don't want to add right now.

  1. Removing those entries from linux device tree still seems fine, but we need the overlay passed by the user in its actual form while creating the pl.dtbo without any manipulation. And this also seems difficult with the proposed suggestion.

Can you elaborate on why ? Do you want two overlays in the end ? The base tree + our overlay + the user overlay, all separate ? If so, then yes, we'd either create two separate overlays in the assist, or track the overlays as secondary trees in lopper (which is what I was hinting at before).

Given the complexity involved here, the current suggestion doesn't seem to work. We only want to check the compatibility of this secondary tree coming from overlay with the system device tree, but we don't want to merge this secondary tree to the primary tree. That's where we thought your secondary tree related prototype may come to our rescue.

It still may, assuming you confirm my understanding above about the two separate overlays. If the result is a single, merged overlay (dtbo), then either approach will work.

Another possible solution that I can think is to define a separate bus (say overlay-bus) for all such use cases and use that bus in our assists depending on our use cases. For overlays, that bus will be merged to the PL bus. And since they will be passed to lopper frontend, the phandle and other props syntax will also be in check.

Let me know your opinion on this summary and the introduction of this new bus. Any suggestions are most welcome.

It's not a bad idea, but let's see about the secondary/support trees and the other options first.

Bruce

@onkarharsh
Copy link
Contributor

Hi @zeddii,
These phandles are rendered invalid in base device tree because they are part of the PL bus and the PL bus is carved out from the DT as part of the overlay. When we are dropping such properties from the base device tree, we need to put them back in the carved overlay so that they come into effect after FPGA programming.

Right. That's the core purpose of the overlay_of() routines, they can pull components out, and remove them from the base device tree. We shouldn't lose anything outside of invalid references in the base tree. If we pull those refs into the overlay, then we'll have the same set as the single tree, just split across the two places, and that's completely valid.

Hi @zeddii ,

Thanks for the response. I am not sure if I understood this splitting of properties across 2 places correctly.

If my overlay has:
&mmi_dc {
clock-names = "ps-clk", "pl-clk";
clocks = <&ps_clk_0 0x0> , <&pl_clk_0 0x1>;
}

Dropping the <&pl_clk_0 0x1> from base tree (because we have removed pl clocks from the base tree) and dropping the "pl-clk" from clock names (for which we may need a new implementation as mentioned before) seems fine for the base tree consumption. But, in overlay, we expect both the entries of ps_clk and pl_clk in its given form and not the truncated one (with only pl_clk entries). When these properties are kept at two places (one in the base tree and one in the overlay), the overlay overwrite the base tree property. Therefore, splitting across two places is not what we expect, we want to preserve the overlay instead.

Let me try to summarize the complete use case that we are trying to support:

  • Existing Usage:

SDT -> overlay assist ->

Outputs two files:

          a) linux device tree (without PL bus) 
          b) an overlay file pl.dtbo (PL bus converted to an overlay)

Now, when someone wants to insert an overlay on top of the existing pl.dtbo, we need to manually add an include file in the generated overlay and then check its correctness with DTC. That's where we are trying to make use of lopper.

  • Proposed Usage:

SDT -> overlay assist (with user overlay as input) ->
Outputs two files:

          a) linux device tree (without PL bus and without the user overlay) 
          b) an overlay file pl.dtbo (PL bus converted to an overlay + the user overlay)

Those match my understanding of what we are trying to do.

Based on the above discussions, I see we are trying to focus on below two areas:

  1. The overlay passed by the user must be syntactically correct and that can only be checked when it is merged with the system device tree and compiled. This is what lopper front end is doing and it is needed.
  2. At the same time, we must preserve the info contained in overlay and not merge it to the generated linux device tree. This must be added only to the final PL overlay that we are carving out.

We see few issues in achieving step2 with the frontend:

  1. If we first merge the overlay DT to full SDT, removing those entries from the linux dt output is not easy. The suggestion is to rely on the frontend checks for the invalid phandle, invalid names etc. This works for phandles because those phandles are defined in the PL bus which is removed via the assist. This seems to be a reactive approach as the list of such properties can be an ever increasing one (after clocks, interrupt related props can be the next).

That's expected and handled by the core when creating an overlay, the complexity isn't anything than has been designed.

Lopper could track nodes and property on the way in, there's other uses of this in the future, but that's a complexity we don't want to add right now.

How many such properties we can cater to in the frontend. Removing clk-names based on the phandles if not done right can lead to erroneous trees and those errors will be seen only during run time. I feel, the onus for using the right clocks (and the overlay) should be on the user. We should stick to only dt compatibility check between the overlay and the base tree and not do any modification with the overlays passed.

  1. Removing those entries from linux device tree still seems fine, but we need the overlay passed by the user in its actual form while creating the pl.dtbo without any manipulation. And this also seems difficult with the proposed suggestion.

Can you elaborate on why ? Do you want two overlays in the end ? The base tree + our overlay + the user overlay, all separate ? If so, then yes, we'd either create two separate overlays in the assist, or track the overlays as secondary trees in lopper (which is what I was hinting at before).

We don't want two separate overlay files, the user overlay and our overlay have to be in one file. As stated in the first example and the above, I am afraid such manipulations can lead to erroneous trees even when the user overlay was correct and that is not ideal.

Let me know if I could answer your queries. Looking forward to your suggestions.

Regards,
Onkar

Given the complexity involved here, the current suggestion doesn't seem to work. We only want to check the compatibility of this secondary tree coming from overlay with the system device tree, but we don't want to merge this secondary tree to the primary tree. That's where we thought your secondary tree related prototype may come to our rescue.

It still may, assuming you confirm my understanding above about the two separate overlays. If the result is a single, merged overlay (dtbo), then either approach will work.

Another possible solution that I can think is to define a separate bus (say overlay-bus) for all such use cases and use that bus in our assists depending on our use cases. For overlays, that bus will be merged to the PL bus. And since they will be passed to lopper frontend, the phandle and other props syntax will also be in check.
Let me know your opinion on this summary and the introduction of this new bus. Any suggestions are most welcome.

It's not a bad idea, but let's see about the secondary/support trees and the other options first.

Bruce

@zeddii
Copy link
Collaborator

zeddii commented Mar 25, 2026 via email

@zeddii
Copy link
Collaborator

zeddii commented Mar 26, 2026

I've implemented an alternative approach for the user overlay use case in the master-next branch. This approach handles the core problem in lopper's tree infrastructure rather than requiring assist-specific logic.

Commits (on master-next)

 xlnx_overlay_pl_dt: add --nodes option for explicit overlay node selection
 xlnx_overlay_pl_dt: auto-generate fragments for user overlay references
 tree: add overlay fragment generation for cross-tree references

Note: The first commit (0ad5b751) contains the core infrastructure in lopper/base.py and lopper/tree.py. The assist changes in commits a8c47dcb and 4ea98322 are optional and provided for your review - the same functionality could be integrated into your PR approach if preferred.

How It Works

When user overlays (via -i) modify base tree nodes to reference nodes that will be extracted into the PL overlay, those properties would normally be lost because the overlay output processing strips unresolvable phandles.

The new infrastructure:

  1. Detects cross-tree references - Finds properties in the base tree that reference nodes in the overlay
  2. Auto-generates overlay fragments - Creates &label { ... } fragments with the complete property values
  3. Includes companion properties - Automatically pairs clocks with clock-names, resets with reset-names, etc.

Example

User overlay (mmi_dc.dtsi):

&mmi_dc{
    clock-names = "mmi_pll", "ps_vid_clk", "stc_ref_clk", "pl_vid_func_clk", "pl_aud_clk";
    clocks = <&versal2_clk 0xd7>, <&versal2_clk 0xda>, <&versal2_clk 0xdf>, <&clkx5_wiz_0 0>, <&clkx5_wiz_1 0>;
};

Here clkx5_wiz_0 and clkx5_wiz_1 are PL nodes that will be in the overlay. Without intervention, the clocks property would lose these references.

Command:

./lopper.py -i mmi_dc.dtsi -O lopper_output -f --enhanced \
    system-top.dts system-top-no-pl.dts -- \
    xlnx_overlay_pl_dt cortexa78_0 full

Output pl.dtsi (excerpt showing auto-generated fragment):

/dts-v1/;
/plugin/;

&fpga {
    firmware-name = "design_1_wrapper_pld.pdi";
    ...
};

&amba {
    clkx5_wiz_0: clkx5_wiz@b0a00000 { ... };
    clkx5_wiz_1: clkx5_wiz@b0a10000 { ... };
    ...
};

&mmi_dc {
    clock-names = "mmi_pll", "ps_vid_clk", "stc_ref_clk", "pl_vid_func_clk", "pl_aud_clk";
    clocks = <&versal2_clk 0xd7>,
             <&versal2_clk 0xda>,
             <&versal2_clk 0xdf>,
             <&clkx5_wiz_0 0x0>,
             <&clkx5_wiz_1 0x0>;
};

Validation

The overlay compiles and applies cleanly:

# Compile base and overlay
dtc -I dts -O dtb -@ system-top-no-pl.dts -o base.dtb
dtc -I dts -O dtb -@ pl.dtsi -o pl.dtbo

# Apply overlay
fdtoverlay -i base.dtb -o merged.dtb pl.dtbo

Merged DTB verification (mmi_dc has the complete clocks property):

mmi_dc@edd00000 {
    ...
    clock-names = "mmi_pll", "ps_vid_clk", "stc_ref_clk", "pl_vid_func_clk", "pl_aud_clk";
    clocks = <0x000000ad 0x000000d7 0x000000ad 0x000000da 0x000000ad 0x000000df 0x0000018d 0x00000000 0x0000018e 0x00000000>;
    ...
};

Optional: --nodes option

The third commit also adds a --nodes option for explicit control over overlay content:

# Extract only specific properties from mmi_dc
./lopper.py ... -- xlnx_overlay_pl_dt cortexa78_0 full --nodes="mmi_dc#clocks,clock-names"

# Combine whole nodes with property-specific extraction
./lopper.py ... -- xlnx_overlay_pl_dt cortexa78_0 full --nodes="/amba_pl;mmi_dc#clocks"

This completely overrides the default /amba_pl extraction when specified.


Please review and let me know if you'd like to integrate this approach or if you have questions about the implementation.

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