lopper:assists:baremetal_xparameters_xlnx: Fix CPU Compatible Detecti…

[sathishkumar-amd]

…on in baremetal_xparameters_xlnx by Scanning Full Compatible List

The baremetal_xparameters_xlnx assist previously identified MicroBlaze and MicroBlaze RISC-V CPUs by checking only the first entry of the compatible property (compatible[0]). On newer SDTs, the first compatible string is often a generic CPU identifier (for example, amd,mbv32 or riscv), while the MicroBlaze-specific compatible strings may appear later in the list. As a result, CPU type detection could fail, leading to generation of generic CPU macros instead of the appropriate MicroBlaze or MicroBlaze RISC-V definitions.

Update the CPU detection logic to search the entire compatible list using any()-based matching, ensuring that MicroBlaze and MicroBlaze RISC-V processors are correctly identified regardless of the position of their compatible strings within the property. This improves compatibility with newer SDTs and ensures the correct CPU-specific xparameters are generated.

[zeddii]

Fix looks correct to me, and the reasoning in the commit message matches what the code does.

The part that could easily have been got wrong is the ordering at lines 499/521 — microblaze-riscv has to be tested before the bare microblaze, or every RISC-V part would fall into the plain MicroBlaze branch, since "microblaze" in "microblaze-riscv" is true. That's right as written.

Worth noting the new form is also more robust than what it replaces on the empty case: propval('compatible')[0] would IndexError on an empty list, whereas any(...) over it is just False.

One optional nit inline — take that one or leave it.

What I would like before this merges is a test. The failure being fixed here is a silent one: detection quietly fell through and the wrong macros got generated, with nothing to indicate it. A case pinning "multi-entry compatible, microblaze string not first, is detected" is a short test to write and it stops exactly that from creeping back in unnoticed. Happy to merge once that's there.

[zeddii]

Optional, and it applies to all three of the changed lines equally.

propval('compatible') without the list cast isn't guaranteed to hand you a list. I checked, and on the path that matters here it does: a compatible parsed from an SDT comes back as a list even when it holds a single entry, so this works:

compatible = "xlnx,microblaze";            -> ['xlnx,microblaze']
compatible = "amd,mbv32", "xlnx,mb-riscv"; -> ['amd,mbv32', 'xlnx,mb-riscv']

But a property assigned in Python rather than parsed comes back as a bare string, and any("microblaze" in c for c in "xlnx,microblaze") then iterates characters and quietly evaluates False. Other assists do assign compatible that way (openamp_xlnx.py sets timer_node["compatible"] = "cdns,ttc"), so it isn't hypothetical in general — just not reachable at this particular call site today.

propval('compatible', list) normalizes both cases and is what the surrounding code tends to use, so it's cheap insurance against someone later feeding this a tree that's been through an assist. Your call — the current code is correct for the inputs it actually sees.

[sathishkumar-amd]

@zeddii, Thanks for the review. Agreed -while propval('compatible') returns a list for SDT-parsed nodes on the path we hit today, other assists can assign compatible as a bare string, and any(... for c in compat) would then iterate characters and evaluate to False.

Updated all three call sites to propval('compatible', list) for consistency with the rest of this assist and to keep the logic safe if the CPU node is ever constructed or modified by an assist upstream. No functional change for current SDT inputs.

[zeddii]

Thanks — the propval('compatible', list) conversion looks right across all three sites, and it's squashed to a single commit with CI green.

I want to walk back the test I asked for, since I set it as a condition and it wasn't a fair one.

When I asked, I assumed a case pinning "multi-entry compatible, microblaze string not first" would be short to write. Having looked properly: nothing under tests/ references baremetal_xparameters_xlnx or xlnx_generate_xparams today, and exercising that function needs either a .repo.yaml manifest or a populated <repo>/lib/bsp/standalone/data/*/*.yaml tree, on top of an SDT carrying a MicroBlaze CPU node. That's building the first test scaffolding for an assist that has none — a reasonable piece of work, but not something to attach to a three line detection fix.

So: no test needed here, and sorry for the detour. Coverage for this assist is worth doing on its own terms rather than as a tax on the next small change that happens to touch it.

Merging.