assists: zephyr: generate Cortex-R52 TCM configuration

[bentheredonethat]

Cortex-R52 firmware can use BTCM for writable data and exception stacks before normal C initialization. If its local TCM region remains disabled, those accesses can reach the system address map and corrupt memory owned by another processor.

Normalize the R52 local TCM layout to ATCM at 0x0, BTCM at 0x10000, and CTCM at 0x18000. Emit configuration words that Zephyr consumes early during reset to enable the selected B and C banks while leaving the existing ATCM configuration unchanged.

Update the R52 fixture and generator checks for the corrected CTCM address and generated configuration values.

[zeddii]

Nothing here blocks merging — the address change is coherent (ATCM [0, 0x10000), BTCM [0x10000, 0x18000), the fixture and the MPU-overlap expectations were updated consistently, and you updated lopper_sanity.py rather than only the pytest side, which is the half that usually gets forgotten.

Five comments inline. Only the first is one I'd genuinely like an answer to; the rest range from a one-line robustness fix to two pre-existing observations that this change happens to make more visible.

I had AI check one thing so you don't need to: it went looking for a collision between .tcm_config > ATCM and the vector table, and there isn't one. custom_lines are emitted into a second SECTIONS block appended after the template, and ld's location counter within a region carries across blocks, so the config words land after whatever the primary block already placed in ATCM rather than at offset 0.

AI review comments are tagged as such

[zeddii]

This is the one I'd like confirmed, AI found it, and I can't check it from this repo and the failure mode is severe.

The commit message describes this word as "leaving the existing ATCM configuration unchanged." That reading depends entirely on the Zephyr-side consumer treating a zero word as skip. If it instead writes the word verbatim to IMP_ATCMREGIONR the way it presumably does for the B and C values just below, then bit 0 clear means ATCM disabled — while the core is executing from ATCM, since DATA_LOAD_REGION is ATCM for this profile and this very section is placed there.

The B and C words encode enable explicitly via | 1, so a reader of this block reasonably infers all three are written the same way. If the consumer really does skip zeros, could you note that in the comment on line 115? Something like "a zero word means leave the bank's existing configuration alone" would stop the next person from concluding this disables ATCM.

If it doesn't skip zeros, then this needs to emit the current ATCM configuration rather than zero.

A pointer to the Zephyr-side code that consumes _TCM_A_REGION would settle it either way.

[zeddii]

AI finding: No alignment on a section built entirely from LONG() statements.

The generator already has this concept — the custom-section loop right below emits . = ALIGN(n) when a section carries an alignment (lines 116-117 of the loop). This hand-rolled block skips it. Since .tcm_config has no input sections, ld has nothing to derive an output alignment from and can default it to 1, so the three words land wherever the location counter happens to sit after the preceding section in ATCM.

They're read by reset-time code before stack setup, which is the worst place to discover an unaligned 32-bit load. One line:

"    .tcm_config :",
"    {",
"        . = ALIGN(4);",

Probably fine in practice today, given whatever precedes it is likely already word-aligned — but it costs nothing to not depend on that.

[zeddii]

AI Minor: | 1 is the enable bit, but nothing here says so. Between this and the bare 0x00000000 above, a reader has to already know the TCM region register layout to tell that these are address | enable and not just addresses.

A named constant or a few words in the comment on line 115 would carry it — something like TCM_REGION_ENABLE = 0x1 used as btcm.origin | TCM_REGION_ENABLE.

[zeddii]

AI comment: Not introduced by this change, but this PR is what makes it matter: the layout constant now lives in two places that have to be updated together, and the check that sits between them can't catch it if they drift.

This validation is unreachable. parse_layout normalizes at line 959 and only then infers at line 1001, and _normalized_memory has already forced BTCM and CTCM to exactly the values this compares against — so memory.origin != origin is never true. (I chased whether the two predicates could disagree, since this uses "r52" in processor.lower() while _normalized_memory uses processor == "cortexr52", but _processor_from_domain returns a canonical "cortexr52", so they always agree.) The ATCM-origin check a few lines above is dead for the same reason.

You updated both copies correctly here. The concern is the next person who updates one — the guard that looks like it would catch them is vacuous.

Worth either deriving expected from the same table _normalized_memory uses, or moving the check before normalization so it validates what the SDT actually said. The second is more useful, and connects to the point on line 758.

[zeddii]

AI comment: Also pre-existing, and the reason the fixture change in this PR is cosmetic: this silently overrides whatever origin the SDT declared. The R52 fixture said CTCM was at 0x20000 and would have been rewritten to 0x18000 regardless of the .dts edit.

That's fine when the SDT agrees. When it disagrees, the user gets a linker script describing memory at an address their hardware description didn't specify, with nothing on the console to say so — and if their SDT was the correct one, the result is silently wrong.

A warning when the declared origin differs from the normalized one would make that visible without changing behaviour:

if origin != declared_origin:
    _warning(f"{name}: SDT declares {kind} at 0x{declared_origin:x}; "
             f"normalizing to 0x{origin:x} for {processor}")

Entirely optional for this PR, and I'd understand leaving it — but it's the kind of silent substitution that's hard to debug from the other end.

[bentheredonethat]

@zeddii

Thanks for the detailed review. The corresponding Zephyr-side implementation is in zephyrproject-rtos/zephyr#118037 (zephyrproject-rtos/zephyr#118037).

Its reset path loads each configuration word, tests bit 0, and skips the register write when that bit is clear.

Therefore the zero ATCM word preserves the existing boot-firmware configuration rather than disabling ATCM.

I’ll make that contract explicit in the generated linker-script comment.

I’ll also add explicit four-byte alignment to .tcm_config and replace the literal | 1 with a named enable-bit constant.

The observations about normalization are also correct.

The existing validation runs after normalization and therefore cannot detect a conflicting SDT address.

I’ll consolidate the architectural TCM origins into one mapping and warn whenever an SDT-provided address is changed during normalization.

That preserves the current generated layout while making mismatches visible and removes the misleading duplicated validation.

[bentheredonethat]

@zeddii closing this for now - will re-open in the future