Skip to content

lopper:assist: Generate xparameters and CMake variables from the misc… - #804

Open
sathishkumar-amd wants to merge 1 commit into
devicetree-org:masterfrom
sathishkumar-amd:misc_props
Open

lopper:assist: Generate xparameters and CMake variables from the misc…#804
sathishkumar-amd wants to merge 1 commit into
devicetree-org:masterfrom
sathishkumar-amd:misc_props

Conversation

@sathishkumar-amd

Copy link
Copy Markdown
Contributor

…_props node

Update Lopper to consume the misc_props node generated by SDT in pcw.dtsi and automatically propagate its properties into bare-metal xparameters.h macros and CMake metadata. This enables new user-defined properties added under misc_props to flow through to the BSP without requiring additional Lopper assist changes.

@sathishkumar-amd

Copy link
Copy Markdown
Contributor Author

Hi @onkarharsh,

Please review this?

@onkarharsh

Copy link
Copy Markdown
Contributor

The changes look good to me.
The Integration test shows some failures which do not seem to be related to the changes done here.

@zeddii

zeddii commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Thanks @sathishkumar-amd — forwarding misc_props into xparameters + CMake so new SDT properties reach the BSP without an assist change is a nice additive approach, and it's a clean no-op for boards without a misc_props node. A few things before it goes in.

1. Identifier sanitization is incomplete (the main one). Both generators build the macro/variable name with:

var_name = prop_name.replace('xlnx,', '').replace('-', '_').upper()

That only handles the xlnx,foo-bar case. Any other DT-legal name emits an invalid C/CMake identifier: a non-xlnx vendor prefix (arm,foo#define ARM,FOO), a dot (foo.barFOO.BAR), or a leading digit (2nd-x2ND_X). Since the goal is "arbitrary user-defined properties flow through without assist changes," this defeats the premise for anything outside that one naming pattern. A general scrub — replace any non-[A-Za-z0-9_] with _, and prefix a leading digit — makes it robust.

2. _iter_misc_props — use the list cast. It does prop_val = node.propval(prop_name) then len(prop_val). An untyped propval() can return a bare scalar (not a list), on which len() raises. Everywhere else in lopper the pattern is node.propval(name, list); using it here guarantees safe iteration for int / string / empty / list values alike.

3. Minor — de-duplicate the name transform. The var_name = … line is copy-pasted in both gen_misc_props_macros and gen_misc_props_cmake_vars; yielding the sanitized name from _iter_misc_props keeps one source of truth (and folds in the fix for #1). The isinstance(val, bool) branches are also effectively dead — DT propvals surface as int/str, not Python bool.

4. Tests. Nothing exercises the misc_props path — a small fixture with a misc_props node covering int, string, list, and empty properties, asserting the emitted macros/CMake vars, would lock it in and catch #1.

The red CI is the cpu-cluster-prune bug that was on master (fixed now) — a rebase clears it; onkarharsh's LGTM is informal (it doesn't set a formal review verdict). Otherwise the approach is sound.

…_props node

Update Lopper to consume the misc_props node generated by SDT in pcw.dtsi and automatically propagate its properties
into bare-metal xparameters.h macros and CMake metadata. This enables new user-defined properties added under misc_props
to flow through to the BSP without requiring additional Lopper assist changes.

Signed-off-by: Sathish Kumar Kamishettigari <sathishkumar.kamishettigari@amd.com>
@sathishkumar-amd

Copy link
Copy Markdown
Contributor Author

@zeddii Thanks for the review, all points are addressed in the latest push:

1. Identifier sanitization
Added misc_prop_var_name() with a general scrub: strip xlnx,/amd, prefixes, replace any non-[A-Za-z0-9] with _, and prefix _ when the name starts with a digit. This covers vendor prefixes (arm,foo → ARM_FOO), dots (foo.bar → FOO_BAR), and leading digits (2nd-x → _2ND_X).

2. Safe propval iteration
_iter_misc_props now uses node.propval(prop_name, list) so scalar values are always list-wrapped before len() / iteration.

3. Single source of truth for name transform
_iter_misc_props yields (prop_name, var_name, value) with var_name from the shared helper; both gen_misc_props_macros and gen_misc_props_cmake_vars
consume it. Removed the dead isinstance(val, bool) branches — lopper surfaces booleans as empty flags or string "true"/"false" values, which are handled explicitly.

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