John Su has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38718 )
Change subject: mb/google/drallion: Update IA VR TDC current limit ......................................................................
Patch Set 4:
(7 comments)
Patch Set 3:
(2 comments)
fe
https://review.coreboot.org/c/coreboot/+/38718/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38718/3//COMMIT_MSG@10 PS3, Line 10: and follow b:148912093#comment1.
I guess what you want to say is Dralion HW design is baseline so need to set TDC to 48A for U42, U62 […]
Yes.
https://review.coreboot.org/c/coreboot/+/38718/1/src/mainboard/google/dralli... File src/mainboard/google/drallion/ramstage.c:
https://review.coreboot.org/c/coreboot/+/38718/1/src/mainboard/google/dralli... PS1, Line 36: config_t *cfg = config_of_soc();
please use a function like mainboard_config_vr etc.. to put those in it.
Done
https://review.coreboot.org/c/coreboot/+/38718/1/src/mainboard/google/dralli... PS1, Line 39: if (!mch_id) {
"if" is not require, and remove static above, we only program once. […]
Done
https://review.coreboot.org/c/coreboot/+/38718/1/src/mainboard/google/dralli... PS1, Line 44: if ((mch_id == PCI_DEVICE_ID_INTEL_CML_ULT) || (mch_id == PCI_DEVICE_ID_INTEL_CML_ULT_6_2)) {
line over 96 characters
Done
https://review.coreboot.org/c/coreboot/+/38718/1/src/mainboard/google/dralli... PS1, Line 44: if ((mch_id == PCI_DEVICE_ID_INTEL_CML_ULT) || (mch_id == PCI_DEVICE_ID_INTEL_CML_ULT_6_2)) {
You can use !=ULT_2_2.
Done
https://review.coreboot.org/c/coreboot/+/38718/1/src/mainboard/google/dralli... PS1, Line 45: if (cfg->cpu_pl2_4_cfg == baseline)
always true not need. you can remove it.
Done
https://review.coreboot.org/c/coreboot/+/38718/3/src/mainboard/google/dralli... File src/mainboard/google/drallion/ramstage.c:
https://review.coreboot.org/c/coreboot/+/38718/3/src/mainboard/google/dralli... PS3, Line 42: params->TdcPowerLimit[1] = VR_CFG_TDC(48);
Are you sure TdcEnable is set? […]
I check FSP log. CPU_POWER_MGMT_VR_CONFIG : TdcEnable[0] : 0x1 TdcEnable is set.