Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37490 )
Change subject: soc/amd/picasso: Add bootblock support ......................................................................
Patch Set 11:
(6 comments)
https://review.coreboot.org/c/coreboot/+/37490/11//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37490/11//COMMIT_MSG@16 PS11, Line 16: Remove postcar support : completely from Makefile.inc. This is not really true for the change anymore.
https://review.coreboot.org/c/coreboot/+/37490/11//COMMIT_MSG@21 PS11, Line 21: Can you please add appropriate BUG= to commit message?
https://review.coreboot.org/c/coreboot/+/37490/11/src/soc/amd/picasso/Makefi... File src/soc/amd/picasso/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/37490/11/src/soc/amd/picasso/Makefi... PS11, Line 208: $(obj)/cbfs/fallback $(objcbfs)
https://review.coreboot.org/c/coreboot/+/37490/11/src/soc/amd/picasso/Makefi... PS11, Line 208: bootblock.elf Don't you really want to just use bootblock.bin here?
https://review.coreboot.org/c/coreboot/+/37490/11/src/soc/amd/picasso/Makefi... PS11, Line 209: $(CONFIG_C_ENV_BOOTBLOCK_SIZE) I believe you can just use file-size bootblock.bin here?
https://review.coreboot.org/c/coreboot/+/37490/11/src/soc/amd/picasso/bootbl... File src/soc/amd/picasso/bootblock/bootblock.c:
https://review.coreboot.org/c/coreboot/+/37490/11/src/soc/amd/picasso/bootbl... PS11, Line 24: pre_init( These are named in a weird way. bootblock_soc_early_init() calls function with *_pre_init() and bootblock_soc_init() calls functions with *_early_init(). I understand these were not added in this CL. But just noting the naming inconsistency.