Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35040 )
Change subject: soc/intel/common/block: Provide mmc.c for setting dll registers. ......................................................................
Patch Set 8:
(2 comments)
https://review.coreboot.org/c/coreboot/+/35040/8/src/soc/intel/common/block/... File src/soc/intel/common/block/scs/mmc.c:
https://review.coreboot.org/c/coreboot/+/35040/8/src/soc/intel/common/block/... PS8, Line 31: if (dll_params->emmc_tx_data_cntl1) { : write32(bar + EMMC_TX_DATA_CNTL1_OFFSET, : dll_params->emmc_tx_data_cntl1); : override = 1; : } You can make this slightly easier to read if you do something like:
static void write_mmc_dll_param(bar, reg, val) { if (val) write32(bar + reg, val); }
and call it with: write_mmc_dll_param(bar, EMMC_TX_DATA_CNTL2_OFFSET, dll_params->emmc_tx_data_cntl2);
and so on.
https://review.coreboot.org/c/coreboot/+/35040/8/src/soc/intel/common/block/... PS8, Line 67: if (override) { : printk(BIOS_INFO, "Skip Emmc dll value programming\n"); : return -1; : } : Why is this required? Return value is anyways not used.