Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/18457 )
Change subject: soc/intel/common: Add bootblock common stage file ......................................................................
Patch Set 33:
Patch Set 33:
Patch Set 33:
Patch Set 33:
Patch Set 33:
Patch Set 33:
with CB:36711 cpu/intel/car/bootblock.c you have prototypes to more or less do the same. Maybe you can reuse it?
I am skeptical to move the SoC init into "cpu"/intel/car since this encompasses multiple IPs like SA and MCH and not only CPU.
It already covers those multiple IPs. It could be placed into arch/x86 if you think that is a better place (it's not very Intel specific indeed).
HI Arthur,
Your point is very valid and for that reason only we have created this "src/soc/intel/common/basecode/" to handle non-IP code block which has just glue logic to call various IP from "src/soc/intel/common/block/".
Right now Intel common code model has supported SOC starting from SKL till latest and all exist inside "src/soc/intel/common/block" hence keeping glue/non-IP code into "src/soc/intel/common/basecode/" makes more reasonable to me. I hope you will agree to this as well.
Thanks, Subrata
Hi
I don't understand it. Why would you want to replicate more or less existing glue logic just to keep things in a dir? There is a similar thing happening for the romstage entry which is common to all Intel targets and resides in cpu/intel/car/romstage.c.
In SoC side we are reusing a lot but FW code doesn't exactly shows that picture, if you compare atom bootblock.c and core bootblock.c you will find lots of different in programming sequence, we are trying to create an unified boot flow where we have defined entry point calls like bootblock_early_init_<for SA/for PCH/for CPU> (function gets called before UART initialization) and similarly bootblock_init_<for SA/for PCH/for CPU> which needs to be done in bootblock before romstage. Similar study been done for romstage as well, hence at end of this exercise, you will see IA-coreboot follows a certain code sequence between atom and cores platform which ease programmers to debug and enable future soc. This also helps folks from different sites to work on same project, also one platform to another porting would be much similar. having a common file like romstage.c is not my goal here.
-
Subrata Banik (Code Review)