Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35595 )
Change subject: soc/intel/cannonlake: FSP UPDs settings are wrong when GBE is disabled ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/c/coreboot/+/35595/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35595/1//COMMIT_MSG@7 PS1, Line 7: soc/intel/cannonlake: FSP UPDs settings are wrong when GBE is disabled Please make this a statement about the change, and not the problem.
Fix FSP UPDs with disabled GBE
https://review.coreboot.org/c/coreboot/+/35595/1//COMMIT_MSG@9 PS1, Line 9: even even when?
https://review.coreboot.org/c/coreboot/+/35595/1//COMMIT_MSG@10 PS1, Line 10: device tree. What affects does this have? Does something not work?
https://review.coreboot.org/c/coreboot/+/35595/1/src/soc/intel/cannonlake/fs... File src/soc/intel/cannonlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/35595/1/src/soc/intel/cannonlake/fs... PS1, Line 207: /* Override the settings when GBE and s0ix are enabled */ I believe the comment is not necessary, as the code is self-explanatory. If you want to keep it, please order the conditions in the comment to the if statement condition.