Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37792 )
Change subject: {drivers,soc}/intel/fsp2_0: Move chipset specific logo handling to SoC ......................................................................
Patch Set 5:
(3 comments)
https://review.coreboot.org/c/coreboot/+/37792/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37792/4//COMMIT_MSG@13 PS4, Line 13: load the logo
pass logo parameters to FSP?
The soc_load_logo actually passes the pointer and length to the FSP. The fsp_load_logo will load the logo into cbmem and return the pointer and the length. So I think the text is correct.
https://review.coreboot.org/c/coreboot/+/37792/4/src/drivers/intel/fsp2_0/Kc... File src/drivers/intel/fsp2_0/Kconfig:
https://review.coreboot.org/c/coreboot/+/37792/4/src/drivers/intel/fsp2_0/Kc... PS4, Line 159:
space
Done
https://review.coreboot.org/c/coreboot/+/37792/4/src/drivers/intel/fsp2_0/lo... File src/drivers/intel/fsp2_0/logo.c:
https://review.coreboot.org/c/coreboot/+/37792/4/src/drivers/intel/fsp2_0/lo... PS4, Line 29: if (logo_size) : *logo_ptr = (UINT32)logo_buffer;
else remove cbmem entry and set logo_entry=NULL?
No. The logo_entry points to the allocated cbmem structure. As this is allocated it should not be set to NULL without removing the area first. Removing of this area is done by the calling routine anyhow.
The returned logo_pointer and logo_size are the pointer passed back to the fsp. If the logo is not available both are set to 0.