Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37870 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 5:
(10 comments)
https://review.coreboot.org/c/coreboot/+/37870/3/src/soc/intel/tigerlake/ear... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/37870/3/src/soc/intel/tigerlake/ear... PS3, Line 25: #define PMC_IPC_USBC_CMD_ID 0xA7
Please align values of these three defines to the same columm as line 29 below
Done
https://review.coreboot.org/c/coreboot/+/37870/3/src/soc/intel/tigerlake/ear... PS3, Line 49: u8
uint8_t
Done
https://review.coreboot.org/c/coreboot/+/37870/3/src/soc/intel/tigerlake/ear... PS3, Line 50: u32
uint32_t […]
Done
https://review.coreboot.org/c/coreboot/+/37870/3/src/soc/intel/tigerlake/ear... PS3, Line 65: BIOS_ERR
BIOS_INFO or BIOS_DEBUG ?
Done
https://review.coreboot.org/c/coreboot/+/37870/3/src/soc/intel/tigerlake/ear... PS3, Line 69: )&
nit - space needed between ")" and "&"
Done
https://review.coreboot.org/c/coreboot/+/37870/3/src/soc/intel/tigerlake/ear... PS3, Line 70: )t
nit - space needed between ")" and "t"
Done
https://review.coreboot.org/c/coreboot/+/37870/3/src/soc/intel/tigerlake/ear... PS3, Line 75: TODO
Remove TODO. If there is still a TODO, please file a bug for it.
removing this for now as it is not needed for type-c operation
https://review.coreboot.org/c/coreboot/+/37870/3/src/soc/intel/tigerlake/ear... PS3, Line 77: BIOS_ERR
BIOS_DEBUG ?
Done
https://review.coreboot.org/c/coreboot/+/37870/3/src/soc/intel/tigerlake/ear... PS3, Line 86: if (num_ports > 0) { : for (i = 0; i < num_ports; i++) {
I think if you change the if condition a little bit, code would be more readable. […]
Done
https://review.coreboot.org/c/coreboot/+/37870/3/src/soc/intel/tigerlake/ear... PS3, Line 114: ,
move comma to end of previous line, delete space
Done