Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29662 )
Change subject: {drivers,soc/intel/braswell}: Implement C_ENVIRONMENT_BOOTBLOCK support ......................................................................
Patch Set 30:
(3 comments)
Patch Set 30:
Patch Set 30:
(3 comments)
You might want to rebase on top of https://review.coreboot.org/q/topic:%22fsp1.1+cleanup . It cleans up a few things and should make the transition and the bootflow in general cleaner. It gets rid of the many functions that do nothing but calling the next one, like the one in car_stage_entry.S
I was wondering how to combine your cleanup and this patch. I tested a few of your patches, not all of them yet.
Will rebase on top of your patch next week.
I was curious if your patch properly applied on top of the branch. Simply cherry-picking your patch on top of my branch applies rather ok. Not too many changes needed (mostly keep stuff from my branch that was cleaned up). If you want I could push it. I can also comment on a few things that I noticed went wrong.
https://review.coreboot.org/#/c/29662/30/src/drivers/intel/fsp1_1/car.c File src/drivers/intel/fsp1_1/car.c:
https://review.coreboot.org/#/c/29662/30/src/drivers/intel/fsp1_1/car.c@a182 PS30, Line 182: You need to change the implementation in strago/com_init.c and cyan/com_init.c to bootblock_mainboard_early_init(void) and link that file in the bootblock instead romstage (and also link some gpio related stuff in the bootblock for it to compile)
https://review.coreboot.org/#/c/29662/30/src/soc/intel/braswell/bootblock/ca... File src/soc/intel/braswell/bootblock/cache_as_ram.S:
https://review.coreboot.org/#/c/29662/30/src/soc/intel/braswell/bootblock/ca... PS30, Line 1: /* I wouldn't move this file, since it is FSP-T specific, not braswell (I hope no-one does, but one could want to add other FSP1.1 targets). Simply guard it like before with the Kconfig option. (IMHO it's a bit ridiculous Kconfig option as you need to run that code anyway, Skylake simply does it from native CAR instead of assembly)
https://review.coreboot.org/#/c/29662/30/src/soc/intel/braswell/bootblock/ca... PS30, Line 49: .global cache_as_ram unneeded?