Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37783 )
Change subject: soc/intel/tigerlake: Update chip files ......................................................................
Patch Set 22:
(5 comments)
https://review.coreboot.org/c/coreboot/+/37783/22/src/soc/intel/tigerlake/ch... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/37783/22/src/soc/intel/tigerlake/ch... PS22, Line 39: GPP_[A:G] or GPD
This is not correct. […]
Ack
https://review.coreboot.org/c/coreboot/+/37783/22/src/soc/intel/tigerlake/ch... PS22, Line 40: gpe0_dw0
Can we please name these pmc_gpe0_dw0 and so on. […]
Ack
https://review.coreboot.org/c/coreboot/+/37783/22/src/soc/intel/tigerlake/ch... PS22, Line 135: /* Need to update DLL setting to get Emmc running at HS400 speed */ : uint8_t EmmcUseCustomDlls; : uint32_t EmmcTxCmdDelayRegValue; : uint32_t EmmcTxDataDelay1RegValue; : uint32_t EmmcTxDataDelay2RegValue; : uint32_t EmmcRxCmdDataDelay1RegValue; : uint32_t EmmcRxCmdDataDelay2RegValue; : uint32_t EmmcRxStrobeDelayRegValue;
soc_intel_common_config on line 36 already includes mmc_dll_params which has the same params as thes […]
This is not added but it just keep the chip.h for existing Japerlake build due to mainboard which is same as ICL. Will check it again with JSL team if JSL patch can add back for eMMC if it's needed. Need to have clean up patch for JSL mainboard(devicetree.cb)
https://review.coreboot.org/c/coreboot/+/37783/22/src/soc/intel/tigerlake/ch... PS22, Line 145: SdCardPowerEnableActiveHigh
Can we please add this when/if really required? Currently, all these params are being added which ar […]
This is not added but it just keep the chip.h for existing Japerlake build due to mainboard which is same as ICL. Will check it again with JSL team if JSL patch can add back if it's needed. Need to have clean up patch for JSL mainboard(devicetree.cb)
https://review.coreboot.org/c/coreboot/+/37783/22/src/soc/intel/tigerlake/ch... File src/soc/intel/tigerlake/chip.c:
https://review.coreboot.org/c/coreboot/+/37783/22/src/soc/intel/tigerlake/ch... PS22, Line 124: soc_fill_gpio_pm_configuration
Can you please move this change to a CL of its own. […]
Ack