lopper: assist: zephyr: guard R52 reserved-memory ranges update

[dbingi-amd]

R52 Zephyr domain generation walks top-level nodes in xlnx_generate_zephyr_domain_dts_arm() and, for /reserved-memory, replaces any existing ranges property with an empty flag:
ranges;
That path was added so a malformed ranges value produced elsewhere in the pipeline -- typically ranges = <0x1> from boolean handling -- could be stripped and rewritten as the empty flag the output is expected to carry.
The rewrite always called node.delete('ranges') first. delete() raises KeyError when the named property is not on the node; it is not a quiet no-op. Platform SDT fragments that introduce /reserved-memory without a ranges property -- SCMI reserved regions are one example -- therefore fail before any rewrite happens:
assist xlnx_generate_domain_dts failed: 'ranges'
The failure is early enough that it matters for overlay merge. Board overlay content is merged after domain-specific generation in xlnx_generate_domain_dts(); an exception in the R52 arm path aborts the assist and the overlay never runs. Nodes supplied only by the fragment, such as GEM MDIO/PHY content on RPU designs, are dropped from the generated Zephyr domain DTS even when the overlay file is passed on the command line.

Only touch ranges when the node already has the property: delete the existing value and add the empty flag. If ranges is absent, leave the node as the SDT supplied it. That preserves the original normalization for inputs that carry a bad ranges value, and lets R52 RPU generation complete when reserved-memory arrives without ranges at all.

[dbingi-amd]

@kedareswararao please review

[zeddii]

Two things on the fix, inline. The first leaves the original crash reachable on a narrower input, so I'd like it addressed before this goes in; the second is a design question I don't have a strong opinion on.

This is also a crash fix with no test, and the failing condition is trivially reproducible — build a /reserved-memory node with no ranges, run the R52 path, assert it doesn't raise. Both of the cases inline are two-line tests. Given the failure mode is silent (the assist aborts and the overlay merge just doesn't happen), a regression test is worth having here.

[zeddii]

props() is a regex matcher, not an exact-name lookup, so this guard is wider than it looks and the original KeyError is still reachable.

LopperNode.props() (lopper/tree.py:3299) tries the exact key first and, on miss, falls back to a regex sweep over every property name:

try:
    pmatches = [self.__props__[name]]
except:
    # maybe it was a regex ?
    for p in self.__props__.keys():
        if re.search( name, p ):
            pmatches.append( self.__props__[p] )

So for a /reserved-memory node carrying dma-ranges but no ranges, the exact lookup misses, re.search('ranges', 'dma-ranges') matches, the guard is truthy — and node.delete('ranges') on the next line raises the same 'ranges' KeyError this PR is fixing.

Narrow input, granted, but it's the identical failure and it'd be maddening to debug a second time given this fix is already in the tree.

Either of these closes it. Anchoring the pattern is the smaller change and keeps you on props():

if node.props('^ranges$'):

The exact-key lookup misses (nothing is literally named ^ranges$), so it falls through to the regex sweep, where ^ranges$ matches ranges and correctly rejects dma-ranges.

Or use propval(), which is the exact-lookup counterpart — it indexes the property directly and returns the [''] sentinel on a miss, with no regex step at all:

if node.propval('ranges') != ['']:

That form also distinguishes the present cases correctly: a valueless ranges; comes back as [] rather than [''], so an already-normalized node still takes the branch. It's the same sentinel idiom used elsewhere in the assists (e.g. the propval("timer") != [''] checks in openamp_xlnx.py).

Either is fine by me — the point is just that the current unanchored props('ranges') is a substring match.

[dbingi-amd]

Thanks @zeddii - Updated to use propval('ranges') != [''] so the guard only runs when a property literally named ranges is present

[zeddii]

Design question rather than a defect, but worth a moment's thought since it's the behavioural half of the change.

With the add inside the guard, a /reserved-memory that arrives without ranges now leaves without one either. The reserved-memory binding wants an empty ranges present, so for the SCMI case you describe the output is arguably still not what a consumer expects ... it just no longer crashes on the way there.

If the intent of this block was "guarantee /reserved-memory ends up with ranges;", then only the delete needs guarding:

if node.props('^ranges$'):
    node.delete('ranges')
node + LopperProp(name='ranges')

That keeps the normalization you're preserving for bad ranges = <0x1> values, fixes the crash, and leaves the absent case standard-conformant.

If instead the deliberate call is "don't synthesize a property the SDT didn't ask for", that's a defensible position too. I'd just ask you to say so in the commit message, because the current wording ("leave the node as the SDT supplied it") reads as a consequence of the fix rather than a decision.

If you do go that way, please warn on it rather than passing silently. Emitting a /reserved-memory with no ranges is a notable outcome, and a silent skip is precisely how this class of problem stays invisible until someone hits it downstream ... which is the same shape as the bug you're fixing here. lopper.log._warning is the right API for it; the file has no log import today, so if you'd rather match local style it already does print(f"[WARNING] ...") at line 245.

[dbingi-amd]

I'm keeping the guarded rewrite as-is we only touch ranges when it is already on /reserved-memory, and we do not add ranges; when the SDT omits it.

Why: this block was meant to normalize malformed ranges values (e.g. ranges = <0x1>), not to always materialize ranges; For the SCMI case that triggered the bug, the input has no ranges property, and that matches the Versal2 SDTs we see. Adding ranges; unconditionally would change output beyond what is needed to fix the crash.
The commit message now states that explicitly

[zeddii]

All three points resolved. propval('ranges') != [''] is the exact-lookup form, so the dma-ranges substring collision is closed. Both regression tests are the right ones.

On the design question, your reasoning changes my mind and also retracts my follow-up ask. I'd said that if you kept the skip you should warn rather than pass silently. That was on the assumption a missing ranges was anomalous. You're saying it's normal for the Versal2 SDTs this runs against .. in which case a warning fires on every ordinary R52 run and trains people to ignore it, which is worse than saying nothing. So no warning, and the skip is right.