Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36627 )
Change subject: soc/intel/icelake: Refactor pch_early_init() code ......................................................................
Patch Set 5:
(3 comments)
https://review.coreboot.org/c/coreboot/+/36627/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36627/2//COMMIT_MSG@10 PS2, Line 10: to romstage/pch.c.
We had a similar topic on IRC and the question was what does the bootblock need to initialize? As there's no checklist, the code is usually just copied from earlier generations.
checklist is not there because its bootloader design to decide what we need and from soc over soc we copy because chrome (as our main vehicle for coreboot development holds same design). For an example. platform might need to have uart console enable and for that we need below programming in bootblock
1. LPSS UART controller init code 2. GPIO programming 3. GPIO programming need PCR programming 4. PCR programming needs P2SB programming 5. P2SB programming needs PMC programming
so bootblock_early_pch_init() should do those programming now if some design doesn't need that they don;t do the same. But that doesn't change with soc rather its bootloader or platform design
While it might be SoC/mainboard specific, having a checklist seems a good idea. I'll create a change for that.
yes, may be something like minimum bootloader requirement for IA soc. i had a white paper in past on the same i remember
https://review.coreboot.org/c/coreboot/+/36627/2/src/soc/intel/icelake/romst... File src/soc/intel/icelake/romstage/pch.c:
https://review.coreboot.org/c/coreboot/+/36627/2/src/soc/intel/icelake/romst... PS2, Line 85: pmc_gpe_init
What do the GPE registers look like in verstage? I am curious how GPE STS bits are getting set.
we were lucky it was working but realized it might not hold good for any cases. hence pulled in gpe programming into bootblock itself
https://review.coreboot.org/c/coreboot/+/36627/4/src/soc/intel/icelake/romst... File src/soc/intel/icelake/romstage/pch.c:
https://review.coreboot.org/c/coreboot/+/36627/4/src/soc/intel/icelake/romst... PS4, Line 65: TCO
When should the TCO watchdog be disabled?
Inside FSP-S or finalize.c
Will there be a platform reset if verstage takes too many TCO cycles measuring the flash regions?
Are you trying to related how kernel triggers soft reset due to TCO waterdog expires. i don't see the same in FW space as we use PCH Timer for timeouts