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 30:
(8 comments)
https://review.coreboot.org/c/coreboot/+/37870/26/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/37870/26/src/soc/intel/tigerlake/ea... PS26, Line 29: int req_size;
cmd.ipc_cmd. […]
Done
https://review.coreboot.org/c/coreboot/+/37870/26/src/soc/intel/tigerlake/ea... PS26, Line 32: cmd.ipc_cmd.subcmd = 0;
I'd rather explicitly set the subcmd just to be sure. […]
Done
https://review.coreboot.org/c/coreboot/+/37870/26/src/soc/intel/tigerlake/ea... PS26, Line 86: tcss_req[1] |= (1 << 4);
this should be a Macro... […]
Done
https://review.coreboot.org/c/coreboot/+/37870/28/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/37870/28/src/soc/intel/tigerlake/ea... PS28, Line 40: int req_size
may be we can remove this variable in all 3 functions and use macro instead
Done
https://review.coreboot.org/c/coreboot/+/37870/28/src/soc/intel/tigerlake/ea... PS28, Line 103: mux_data.dp_mode
ffs(mux_data. […]
there is no ffs in coreboot that I'm aware of :(...is it actually needed here? I'm not sure it is
https://review.coreboot.org/c/coreboot/+/37870/28/src/soc/intel/tigerlake/ea... PS28, Line 125: r
return here too for error in case of dp?
Done
https://review.coreboot.org/c/coreboot/+/37870/28/src/soc/intel/tigerlake/ea... PS28, Line 172: dp_mode
&mux_data.dp_mode […]
Done
https://review.coreboot.org/c/coreboot/+/37870/28/src/soc/intel/tigerlake/ea... PS28, Line 182: (uint8_t)
remove
Done
-
Brandon Breitenstein (Code Review)