Clk wiz zephyr support

[sbeesams]

This patch series adds support for dual-mapped AMD/Xilinx Clocking Wizard (xlnx,clkx5-wiz-1.0) nodes in Lopper.

The first patch updates the Zephyr supported compatibility list to
include the "xlnx,clkx5-wiz-1.0" compatible entry, allowing proper
recognition of the Clocking Wizard node during device tree processing.

The second patch extends the gen_domain_dts assist script to handle
Clocking Wizard nodes whose reg property spans both the APU and RPU
domains, slicing it to the correct per-domain address instead of
dropping the node.

Tested on a Versal Gen2 platform with Lopper-generated dts.

[zeddii]

Thanks @sbeesams. The approach looks fine to me. i.e. preserve the dual-mapped clk-wiz node past the delete pass, then slice its reg down to the target domain. Also, I noticed that the propval(...) != [''] sentinel checks are handled correctly. A few things to sort out before this can merge; specifics are inline.

Blocking

• Rebase needed: the branch now conflicts with master (gen_domain_dts.py moved under recent merges). Please rebase onto current master and re-push so CI re-runs.
• Forced status = "okay" (inline) — same override that was reworked out of pull #816; status should come from the SDT, not be synthesized.
• Silent except Exception: pass around tree.sync() (inline). At least emit a warning, or decide if it's an error, but don't mask it completely.

Worth tightening

• It looks like the whitelist rebuild silently drops any other property on the node.
• The 8-cell preserve guard and the slice logic are a split invariant that must stay in lockstep .. we should at least leave a comment to remind us of that.

To me, this should be a single commit, so a squash would be appropriate. Thanks!

[zeddii]

This 8-cell preserve guard is one half of an invariant; the other half is the if n == 8 slice in _compact_clk_wiz_node. If the compatible match or the cell count ever changes, both sites have to change together. Worth a cross-reference comment, or factor the "is a dual-mapped clk-wiz" test into one helper so it can't drift.

[sbeesams]

Addressed with cross-reference comments at both sites: the preserve guard notes that _compact_clk_wiz_node is the consumer of the preserved node, and the if n == 8 slice in _compact_clk_wiz_node now points back to the preserve guard. Both comments call out the invariant explicitly so the two sites can't drift silently.

[zeddii]

Two small things in this branch:

1. addr_hex uses only final_reg[1] (the low address cell), dropping final_reg[0] — fine while addresses fit in 32 bits, but the node name will be wrong for a >4 GB address.
2. the APU/RPU split keys off "a78"/"a72" substrings in machine; that's the convention nearby, just flagging it's the assumption this relies on.

[sbeesams]

Point 1 fixed: addr_hex now combines both 32-bit cells (high << 32 | low) so the node name is correct for addresses above 4 GB. For all current clkx5-wiz instances (32-bit space), lstrip('0') removes the zero high word so existing output is unchanged. Point 2 acknowledged with a comment: the "a78"/"a72" substring check is the same convention used throughout this file for all processor identification.

[zeddii]

This deletes every property on the node and re-adds a fixed six. Any other property the SDT carried on the clk-wiz node (interrupts, interrupt-parent, labels, extra xlnx,* attrs) is silently dropped. If that's intentional for the Zephyr consumer, a one-line comment saying so would help; if not, a targeted transform (slice reg, rename) is safer than a rebuild-from-whitelist.

[sbeesams]

Intentional — the Zephyr xlnx,clkx5-wiz-1.0 binding schema only allows the six properties being written back; any additional SDT-carried attribute (extra xlnx,* vendor props, interrupts, etc.) would cause Zephyr's DTS compiler to fail with an unknown-property error.

[zeddii]

Forces status = "okay" unconditionally — the same override that was reworked out of #816. Note the whitelist rebuild just above already dropped the SDT's real status, so the - status entry you added to the keep-list YAML never takes effect for this node. status should follow the SDT: if the clk-wiz is enabled you already have it; if it's disabled, forcing okay is wrong.

[sbeesams]

Fixed — status is now snapshotted before the whitelist rebuild and written back as-is. The unconditional status = "okay" is removed; if the SDT had no status property the node gets none (treated as enabled by default), and a disabled node stays disabled.

[zeddii]

try: sdt.tree.sync() except Exception: pass swallows every failure. A sync error here means the tree is in a bad state, and hiding it will resurface later as a confusing downstream failure with no breadcrumb. Either let it raise, or catch a specific exception and log it. (Lopper convention is no silent fallbacks.)

[sbeesams]

Fixed — removed the try/except entirely. sdt.tree.sync() now raises on failure per Lopper convention

[zeddii]

First, the good news: I checked all five of your replies against the code and they're all genuinely done. Your rationale for the whitelist rebuild (the Zephyr binding only permits those six properties) is a fine answer; I'd suggest putting that sentence in the code as a comment so the next reader doesn't ask the same question.

So the review feedback is settled. Unfortunately the rebase isn't, and it's a bigger problem than the conflict it was meant to fix.

This branch would silently revert the zephyr_domain_dts split.

The PR is now +1911 -0, up from +66 -0. gen_domain_dts.py alone accounts for +1902 -0, against a file that is 609 lines on master. The second hunk (@@ -606,4 +609,1903 @@) appends the entire Zephyr generation subsystem back into gen_domain_dts.py — 15 function definitions including xlnx_generate_zephyr_domain_dts_arm, xlnx_remove_unsupported_nodes, _apply_pl_peripheral_transforms, board_symbol_for_machine, generate_board_kconfig_defconfig and xlnx_generate_zephyr_domain_dts.

Every one of those lives in zephyr_domain_dts.py on master and only there — they were moved by ef8313d3 ("lopper: assists: Add zephyr_domain_dts assist for Zephyr domain DTS"). I checked each:

xlnx_generate_zephyr_domain_dts_arm    gen_domain_dts=0  zephyr_domain_dts=1
_xlnx_zephyr_assign_ttc0               gen_domain_dts=0  zephyr_domain_dts=1
board_symbol_for_machine               gen_domain_dts=0  zephyr_domain_dts=1
_load_zephyr_compat_schema             gen_domain_dts=0  zephyr_domain_dts=1
_reset_board_kconfig_scratch           gen_domain_dts=0  zephyr_domain_dts=1

Merging this would put two copies of the Zephyr generator in the tree and undo that refactor without any commit saying so. The branch is based on a master from before ef8313d3, so what looks like a rebase has reintroduced the pre-split file.

Worth knowing: CI has not run on this. The workflow is sitting at action_required, so the only check reporting is DCO. Nothing automated would have caught the duplication before merge.

What the rework looks like

Your actual change is fine and splits across two files now:

• The preserve guard stays in gen_domain_dts.py — that call site still exists on master and your hunk applies to it cleanly. Nothing to change there.
• _compact_clk_wiz_node and its dispatch move to zephyr_domain_dts.py, into the xlnx_remove_unsupported_nodes that lives there now.
• zephyr_supported_comp.yaml is unaffected.

Rebase onto current master and the diff should land back around its original size. Please ping me when it's up and I'll re-check — the clk-wiz logic itself I'm happy with, so this should be the last round.

[zeddii]

This is the start of the 1903-line block that shouldn't be here.

Everything from this line to the end of the hunk is the Zephyr generation subsystem as it existed before ef8313d3 moved it into zephyr_domain_dts.py. xlnx_generate_zephyr_domain_dts_arm, _xlnx_zephyr_assign_ttc0, board_symbol_for_machine, _load_zephyr_compat_schema and the rest are all defined in that assist on master, and in no other file.

Re-adding them here produces two copies of each, with nothing to say which one runs — and it happens inside a PR whose stated purpose is clocking-wizard support, so a reader skimming the change would have no reason to look for it.

The cause is the branch point rather than anything you wrote: this branch started before the split, so replaying it onto master brings the old file body with it. A fresh rebase onto current master should drop this entire hunk, leaving your genuine additions — the preserve guard above, and _compact_clk_wiz_node, which needs to move to zephyr_domain_dts.py since that's where xlnx_remove_unsupported_nodes lives now.

[zeddii]

This hunk is correct and belongs in this file — the surrounding xlnx_generate_domain_dts is still here on master, so it survives the rebase as-is. Flagging that explicitly so you don't move it along with the rest.

The cross-reference comment does what I asked for. One small thing: it names xlnx_remove_unsupported_nodes as the consumer, and after the rebase that function will be in zephyr_domain_dts.py rather than this file. Worth saying so in the comment, since a cross-reference that silently spans two assists is exactly the kind of link that rots.

[zeddii]

The rebase is sorted and the split is exactly right — I checked all of it. The preserve guard stayed in gen_domain_dts.py, _compact_clk_wiz_node moved to zephyr_domain_dts.py next to xlnx_remove_unsupported_nodes, the 1900-line duplication is gone, and the diff is back to +70 -0. You also picked up the two smaller items.

One new thing came in with the rework, inline. It's a one-line fix and then I think this is done.

[zeddii]

The continue is outside the is_supported_periph check, so an unsupported clk-wiz falls out of the loop without being compacted and without being deleted.

The else at lines 1028-1033 is what removes peripherals that aren't in the schema. Reaching continue here skips it, so such a node survives into the Zephyr output carrying its raw 8-cell reg ... which is precisely the malformed node the preserve guard in gen_domain_dts.py deliberately kept alive for you to slice. Before this PR it would have been deleted there as unsupported, so this is a behaviour change, and a silent one.

Not reachable today: the only clkx5-wiz compatible in the wild is xlnx,clkx5-wiz-1.0, and this PR adds it to zephyr_supported_comp.yaml. But the guard is a substring match on clkx5-wiz while is_supported_periph is an exact key match, so the two disagree the moment a -2.0 or a variant shows up — and the failure mode is a bad node in the output rather than an error.

Folding the condition keeps the unsupported case on the existing path:

if (any('clkx5-wiz' in c for c in node.propval('compatible', list))
        and is_supported_periph):
    _compact_clk_wiz_node(node, machine, sdt)
    valid_alias_proplist.append(node.name)
    continue

An unsupported clk-wiz then falls through to the else and is deleted as it was before.

[sbeesams]

hi @zeddii, I addressed the final continue condition feedback in commit d46e350. Could you please take another look when you get a chance? Thanks.

[zeddii]

The dispatch fix in d46e350 is right — folding is_supported_periph into the condition means an unsupported clk-wiz now falls through to the deletion path instead of leaking out with its 8-cell reg, and the valid_alias_proplist append is preserved. The added comment explaining the fall-through is a nice touch.

This now needs a rebase, though. #815 (Versal SysMon) merged ahead of it and the two collide.

The good news is it's narrow — I test-merged it, and only zephyr_supported_comp.yaml conflicts. zephyr_domain_dts.py auto-merges cleanly, so your actual logic is untouched:

Auto-merging lopper/assists/zephyr_domain_dts.py
Auto-merging lopper/assists/zephyr_supported_comp.yaml
CONFLICT (content): Merge conflict in lopper/assists/zephyr_supported_comp.yaml

It's purely textual: both changes append a new top-level compatible block at the end of the file, right after remote-ipi-id. Master now ends with xlnx,versal-sysmon from #815, and yours adds xlnx,clkx5-wiz-1.0 in the same spot. Different keys, nothing semantically overlapping — keep both blocks and it's resolved.

[sbeesams]

Hi, @zeddii Rebased onto the current master branch and resolved the conflict in zephyr_supported_comp.yaml. Could you please take another look when you get a chance? Thanks.