[coreboot-gerrit] Change in ...coreboot[master]: {drivers,soc/intel/braswell}: Implement C_ENVIRONMENT_BOOTBLOCK support

[Arthur Heymans (Code Review)]

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)

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

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 112: 
              : 	movd	%edi, %mm2 /* BIST value */
why do you do this?

https://review.coreboot.org/#/c/29662/30/src/soc/intel/braswell/bootblock/ca... 
PS30, Line 140: movd	%mm2, %eax
              : 	push	%eax	/* BIST */
this is not used by the function you call.

https://review.coreboot.org/#/c/29662/30/src/soc/intel/braswell/bootblock/ca... 
PS30, Line 148: call	bootblock_c_entry
It seems a good idea to align the stack 16 bytes.