Attention is currently required from: Jamie Chen, Henry Sun, Tim Wawrzynczak, Paul Menzel, Ren Kuo, Simon Yang, Kane Chen, Patrick Rudolph. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60009 )
Change subject: soc/intel/jasperlake: Add CdClock frequency config ......................................................................
Patch Set 17:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/60009/comment/1e2ce87e_53e54743 PS17, Line 14: When is this setting needed?
When does FSP apply the CdClock setting? always or only when the GOP blob runs?
Please also provide all those details in a comment in `chip.h`. Without such details, people usually fall into the trap to specify random values without knowing why.
File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/60009/comment/978d3cc6_6d4838b9 PS17, Line 212: params->CdClock = config->cd_clock ? config->cd_clock - 1 : 0xff; So is the comment in `src/vendorcode/intel/fsp/fsp2_0/jasperlake/FspsUpd.h` wrong?
Offset 0x0436 - CdClock Frequency selection 0: (Default) Auto (Max based on reference clock frequency), 1: 172.8 Mhz, 2: 180 Mhz, 3: 190 Mhz, 4: 307.2 Mhz, 5: 312 Mhz, 6: 552 Mhz, 7: 556.8 Mhz, 8: 648 Mhz, 9: 652.8 Mhz 0: Auto (Max based on reference clock frequency), 1: 172.8 Mhz, 2: 180 Mhz, 3: 190 Mhz, 4: 307.2 Mhz, 5: 312 Mhz, 6: 552 Mhz, 7: 556.8 Mhz, 8: 648 Mhz, 9: 652.8 Mhz
If so, why isn't it fixed?
-
Nico Huber (Code Review)