Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36145 )
Change subject: arch/arm: Pass cbmem_top to ramstage via calling argument ......................................................................
Patch Set 20:
(7 comments)
https://review.coreboot.org/c/coreboot/+/36145/2/src/arch/arm/stages.c File src/arch/arm/stages.c:
https://review.coreboot.org/c/coreboot/+/36145/2/src/arch/arm/stages.c@34 PS2, Line 34: #endif
This sort of stuff shouldn't be here. You'll probably want to centralize it in src/lib/imd_cbmem. […]
Done
https://review.coreboot.org/c/coreboot/+/36145/15/src/arch/arm/stages.c File src/arch/arm/stages.c:
https://review.coreboot.org/c/coreboot/+/36145/15/src/arch/arm/stages.c@25 PS15, Line 25: #include <cbmem.h>
This compilation unit doesn't technically need this header.
Now it does with the extern symbol declared in there.
https://review.coreboot.org/c/coreboot/+/36145/15/src/arch/arm/stages.c@32 PS15, Line 32: uintptr_t _cbmem_top_ptr;
It doesn't need a similar function. It just needs to set the variable. […]
Done
https://review.coreboot.org/c/coreboot/+/36145/15/src/arch/arm/stages.c@35 PS15, Line 35: {
I think this needs an ENV_RAMSTAGE check like the arm64 version? […]
Done
https://review.coreboot.org/c/coreboot/+/36145/15/src/arch/arm/stages.c@36 PS15, Line 36: _cbmem_top_ptr = (uintptr_t)stage_arg;
Not really sure I understand the second part of what you said, maybe that would be clearer if you up […]
Done
https://review.coreboot.org/c/coreboot/+/36145/16/src/arch/arm/stages.c File src/arch/arm/stages.c:
https://review.coreboot.org/c/coreboot/+/36145/16/src/arch/arm/stages.c@34 PS16, Line 34: if (!ENV_ROMSTAGE)
This needs to be either ENV_RAMSTAGE or !ENV_ROMSTAGE_OR_BEFORE. […]
Done
https://review.coreboot.org/c/coreboot/+/36145/18/src/arch/arm/stages.c File src/arch/arm/stages.c:
https://review.coreboot.org/c/coreboot/+/36145/18/src/arch/arm/stages.c@32 PS18, Line 32: void
nit: If you want this as a uintptr_t, just make it a uintptr_t.
Done