Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30684 )
Change subject: Use uintptr_t instead of (void *) where it makes sense ......................................................................
Patch Set 11:
(4 comments)
https://review.coreboot.org/#/c/30684/10/src/cpu/allwinner/a10/ram_segs.h File src/cpu/allwinner/a10/ram_segs.h:
https://review.coreboot.org/#/c/30684/10/src/cpu/allwinner/a10/ram_segs.h@27 PS10, Line 27: << 20
Seen this more often, maybe worth to trigger a follow up to replace […]
I'll do that.
https://review.coreboot.org/#/c/30684/10/src/cpu/allwinner/a10/ram_segs.h@30 PS10, Line 30: /* : * By CBFS cache, we mean a cached copy, in RAM, of the entire CBFS region. : */ : static inline uintptr_t a1x_get_cbfs_cache_top(void) : { : /* Arbitrary 16 MiB gap for cbmem tables and bouncebuffer */ : return a1x_get_cbmem_top() - (16 << 20); : } : : static inline uintptr_t a1x_get_cbfs_cache_base(void) : { : return a1x_get_cbfs_cache_top() - CONFIG_ROM_SIZE; : }
I was wondering why we don't have to update any caller... […]
this cpu/soc code was actually never completed. The code that would allow it to read cbfs from the bootmedium (typically SD/emmc) was never pushed for review, so it won't get past the bootblock (raminit is done in there).
https://review.coreboot.org/#/c/30684/10/src/drivers/intel/fsp1_1/raminit.c File src/drivers/intel/fsp1_1/raminit.c:
https://review.coreboot.org/#/c/30684/10/src/drivers/intel/fsp1_1/raminit.c@... PS10, Line 155: (unsigned int)
or use PRIxPTR instead
that macro is not implemented in coreboot.
https://review.coreboot.org/#/c/30684/10/src/mainboard/emulation/qemu-power8... File src/mainboard/emulation/qemu-power8/cbmem.c:
https://review.coreboot.org/#/c/30684/10/src/mainboard/emulation/qemu-power8... PS10, Line 21: /* For now, last 1M of 4G */
Is this a lie? CBMEM will be *below* the last 1MiB...
looks quite weird indeed.