Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35348 )
Change subject: Rangeley: Fix incorrect BCLK ......................................................................
Patch Set 10: Code-Review-2
(3 comments)
A bit attention to details please, with your +1 and +2...
https://review.coreboot.org/c/coreboot/+/35348/10/src/cpu/intel/common/fsb.c File src/cpu/intel/common/fsb.c:
https://review.coreboot.org/c/coreboot/+/35348/10/src/cpu/intel/common/fsb.c... PS10, Line 51: ret = rangeley_bclk[rdmsr(MSR_FSB_FREQ).lo & 3]; This changes all the 5 cases that previously returned BCLK=100. Also fix Rangeley comment.
https://review.coreboot.org/c/coreboot/+/35348/10/src/cpu/intel/fsp_model_40... File src/cpu/intel/fsp_model_406dx/acpi.c:
https://review.coreboot.org/c/coreboot/+/35348/10/src/cpu/intel/fsp_model_40... PS10, Line 177: I wonder if the value returned here is actually what we would want for tsc_freq_mhz(). If so, find better place for this, outside acpi.c.
https://review.coreboot.org/c/coreboot/+/35348/10/src/northbridge/intel/fsp_... File src/northbridge/intel/fsp_rangeley/udelay.c:
https://review.coreboot.org/c/coreboot/+/35348/10/src/northbridge/intel/fsp_... PS10, Line 50: fsb = rangeley_bclk[rdmsr(MSR_FSB_FREQ).lo & 3]; Again, you should not repeat the array and rdmsr, but provide one implementation returning the frequency.