Frans Hendriks 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:
(2 comments)
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.
No problem if you push the update.
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 cou […]
Agree with move to drivers/intel/fsp1_1. Is there any need to add with Kconfig option? AFAIK currently only Braswell is using FSP1.1.
https://review.coreboot.org/#/c/29662/30/src/soc/intel/braswell/bootblock/ca... PS30, Line 49: .global cache_as_ram
unneeded?
Ack can be removed.