Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45192 )
Change subject: soc/intel/alderlake/romstage: Do initial SoC commit till romstage ......................................................................
Patch Set 9:
(6 comments)
https://review.coreboot.org/c/coreboot/+/45192/7/src/soc/intel/alderlake/mem... File src/soc/intel/alderlake/meminit.c:
https://review.coreboot.org/c/coreboot/+/45192/7/src/soc/intel/alderlake/mem... PS7, Line 109: uint8_t dimm_cfg = DISABLE_DIMM1; /* Use only DIMM0 */
I checked memory support for ADL and I see the following: […]
yes, will fix it now
https://review.coreboot.org/c/coreboot/+/45192/7/src/soc/intel/alderlake/mem... PS7, Line 136: ENABLE_BOTH_DIMMS
Since the second SPD pointer is zero, shouldn't this be `DISABLE_DIMM1` as well?
Ack
https://review.coreboot.org/c/coreboot/+/45192/7/src/soc/intel/alderlake/mem... PS7, Line 183: mem_cfg->ECT = board_cfg->ect;
Thank you. BTW, looks like the comments on those files are wrong. […]
lol https://review.coreboot.org/c/coreboot/+/45338
https://review.coreboot.org/c/coreboot/+/45192/7/src/soc/intel/alderlake/res... File src/soc/intel/alderlake/reset.c:
https://review.coreboot.org/c/coreboot/+/45192/7/src/soc/intel/alderlake/res... PS7, Line 31: die("unknown reset type");
Sounds good to me. […]
https://review.coreboot.org/c/coreboot/+/45336 https://review.coreboot.org/c/coreboot/+/45337/1
https://review.coreboot.org/c/coreboot/+/45192/7/src/soc/intel/alderlake/rom... File src/soc/intel/alderlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/45192/7/src/soc/intel/alderlake/rom... PS7, Line 91: 1
Sure. […]
sure, make sense
https://review.coreboot.org/c/coreboot/+/45192/8/src/soc/intel/alderlake/rom... File src/soc/intel/alderlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/45192/8/src/soc/intel/alderlake/rom... PS8, Line 69: /* DP port config */ : m_cfg->DdiPortAConfig = config->DdiPortAConfig; : m_cfg->DdiPortBConfig = config->DdiPortBConfig; : m_cfg->DdiPortAHpd = config->DdiPortAHpd; : m_cfg->DdiPortBHpd = config->DdiPortBHpd; : m_cfg->DdiPortCHpd = config->DdiPortCHpd; : m_cfg->DdiPort1Hpd = config->DdiPort1Hpd; : m_cfg->DdiPort2Hpd = config->DdiPort2Hpd; : m_cfg->DdiPort3Hpd = config->DdiPort3Hpd; : m_cfg->DdiPort4Hpd = config->DdiPort4Hpd; : m_cfg->DdiPortADdc = config->DdiPortADdc; : m_cfg->DdiPortBDdc = config->DdiPortBDdc; : m_cfg->DdiPortCDdc = config->DdiPortCDdc; : m_cfg->DdiPort1Ddc = config->DdiPort1Ddc; : m_cfg->DdiPort2Ddc = config->DdiPort2Ddc; : m_cfg->DdiPort3Ddc = config->DdiPort3Ddc; : m_cfg->DdiPort4Ddc = config->DdiPort4Ddc;
I think these settings only configure some GPIOs. […]
you are right but the problem is FSP UPD default value, unless we override those, it might again override those GPIO configuration as CB does GPIO program first then we are calling FSP-S hence FSP-S can override GPIO 😞