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 11:
(4 comments)
https://review.coreboot.org/c/coreboot/+/35040/11/src/soc/intel/common/block... File src/soc/intel/common/block/include/intelblocks/mmc.h:
https://review.coreboot.org/c/coreboot/+/35040/11/src/soc/intel/common/block... PS11, Line 64: SOC specific API This is not SoC specific API.
https://review.coreboot.org/c/coreboot/+/35040/11/src/soc/intel/common/block... PS11, Line 67: void *bar Can you please add comment indicating what this BAR really means.
https://review.coreboot.org/c/coreboot/+/35040/11/src/soc/intel/common/block... File src/soc/intel/common/block/scs/mmc.c:
https://review.coreboot.org/c/coreboot/+/35040/11/src/soc/intel/common/block... PS11, Line 73: cfg0 Just curious: Why is this named cfg0?
https://review.coreboot.org/c/coreboot/+/35040/11/src/soc/intel/common/block... PS11, Line 75: if (CONFIG(SOC_INTEL_COMMON_MMC_OVERRIDE)) Why not put this up in line 71:
if (!CONFIG(SOC_INTEL_COMMON_MMC_OVERRIDE)) return;
No need to calculate base address if its not going to be used.