Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38461 )
Change subject: soc/intel/tigerlake: Update fsp params for Jasper Lake ......................................................................
Patch Set 7:
(9 comments)
https://review.coreboot.org/c/coreboot/+/38461/5/src/soc/intel/tigerlake/fsp... File src/soc/intel/tigerlake/fsp_params_jsl.c:
https://review.coreboot.org/c/coreboot/+/38461/5/src/soc/intel/tigerlake/fsp... PS5, Line 4: 2020
2019-2020
Done
https://review.coreboot.org/c/coreboot/+/38461/5/src/soc/intel/tigerlake/fsp... PS5, Line 72: enable/disable devices
Nit: I am not seeing any enable/disable device actions besides parsing the devicetree. […]
Done
https://review.coreboot.org/c/coreboot/+/38461/5/src/soc/intel/tigerlake/fsp... PS5, Line 86: MP PPI
Just curious: What is MP PPI?
MP PPI is the coreboot implementation of initializing various CPU specific feature. We publish PPI to FSP and FSP can call coreboot PPI to run feature initialization on all APs. You can find more details here: https://review.coreboot.org/c/coreboot/+/31841
https://review.coreboot.org/c/coreboot/+/38461/5/src/soc/intel/tigerlake/fsp... PS5, Line 132: if (!dev)
Nit: Use "{" because else block has braces. Just to improve the readability. […]
Done
https://review.coreboot.org/c/coreboot/+/38461/5/src/soc/intel/tigerlake/fsp... PS5, Line 143: PCH_DEV_SLOT_STORAGE
Referring to this CL here: https://review.coreboot. […]
Done
https://review.coreboot.org/c/coreboot/+/38461/5/src/soc/intel/tigerlake/rom... File src/soc/intel/tigerlake/romstage/fsp_params_jsl.c:
https://review.coreboot.org/c/coreboot/+/38461/5/src/soc/intel/tigerlake/rom... PS5, Line 16: #include <assert.h>
needed?
Done
https://review.coreboot.org/c/coreboot/+/38461/5/src/soc/intel/tigerlake/rom... PS5, Line 19: #include <soc/gpio_soc_defs.h> : #include <soc/iomap.h>
needed?
Done
https://review.coreboot.org/c/coreboot/+/38461/5/src/soc/intel/tigerlake/rom... PS5, Line 57: for (i = 0; i < ARRAY_SIZE(config->PcieRpEnable); i++) {
new line […]
Done
https://review.coreboot.org/c/coreboot/+/38461/5/src/soc/intel/tigerlake/rom... PS5, Line 108: /* MbHob */ : m_cfg->SkipMbpHob = 0;
not needed.
Done