Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 24:
(8 comments)
https://review.coreboot.org/c/coreboot/+/42079/24/src/mainboard/google/volte... File src/mainboard/google/volteer/Kconfig:
PS24: This is a mainboard change. It should go in the next CL.
https://review.coreboot.org/c/coreboot/+/42079/24/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/42079/24/src/soc/intel/tigerlake/ea... PS24, Line 3: #include <bootstate.h> Why is this required?
https://review.coreboot.org/c/coreboot/+/42079/24/src/soc/intel/tigerlake/ea... PS24, Line 6: #include <ec/google/chromeec/ec.h> Why is this required?
https://review.coreboot.org/c/coreboot/+/42079/24/src/soc/intel/tigerlake/ea... PS24, Line 102: while (--tries >= 0) Just curious: Why are we trying 3 times? Is this operation known to fail for some reason?
https://review.coreboot.org/c/coreboot/+/42079/24/src/soc/intel/tigerlake/fs... File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/42079/24/src/soc/intel/tigerlake/fs... PS24, Line 376: DSK What does DSK mean?
https://review.coreboot.org/c/coreboot/+/42079/24/src/soc/intel/tigerlake/fs... PS24, Line 376: WEAK Why WEAK?
https://review.coreboot.org/c/coreboot/+/42079/24/src/soc/intel/tigerlake/fs... PS24, Line 378: vboot_recovery_mode_enabled What about developer mode? Wouldn't you need this in developer mode as well?
https://review.coreboot.org/c/coreboot/+/42079/24/src/soc/intel/tigerlake/in... File src/soc/intel/tigerlake/include/soc/early_tcss.h:
https://review.coreboot.org/c/coreboot/+/42079/24/src/soc/intel/tigerlake/in... PS24, Line 130: mainboard_early_tcss_enable What is the expectation from mainboard here? There should be a comment explaining the expectations.