Attention is currently required from: Martin Roth, Chiranjeevi Rapolu, Tim Wawrzynczak, John Zhao, Duncan Laurie, Brandon Breitenstein, Patrick Rudolph. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51194 )
Change subject: soc/intel/tigerlake: Enable TCSS Muxes to disconnect mode during boot ......................................................................
Patch Set 5:
(7 comments)
File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/51194/comment/e103d5ac_f4ccf1c8 PS5, Line 109: res The caller doesn't even care about res. This can be just defined in this function rather than accepting from caller.
struct pmc_ipc_buffer req = { 0 }; struct pmc_ipc_buffer rsp;
https://review.coreboot.org/c/coreboot/+/51194/comment/26fa352c_99a4224e PS5, Line 115: PMC_IPC_TCSS_DISC_REQ_RES, : port_map.usb3_port, : port_map.usb2_port, : 0, 0, 0, 0); Can you please reflow this to fill in the 96-column limit?
https://review.coreboot.org/c/coreboot/+/51194/comment/a733e44a_3dcb766d PS5, Line 268: struct tcss_port_map port_map Instead of passing in the entire structure here, you can pass in a pointer to the entry?
const struct tcss_port_map *port_map
https://review.coreboot.org/c/coreboot/+/51194/comment/246adaab_65545036 PS5, Line 270: *rbuf = NULL This is problematic. rbuf is set to NULL here and the driver expects the caller to pass in a valid pointer. This whole file is passing it wrong. (Fixed in CB:51232).
https://review.coreboot.org/c/coreboot/+/51194/comment/6ce19571_2b1d10be PS5, Line 271: = 0 Not required. This is set on line 273.
https://review.coreboot.org/c/coreboot/+/51194/comment/9de1584d_ceae2ded PS5, Line 319: if (!display_init_required()) if (!CONFIG(EARLY_TCSS_DISPLAY) || !display_init_required()) return;
File src/soc/intel/tigerlake/include/soc/early_tcss.h:
https://review.coreboot.org/c/coreboot/+/51194/comment/f5e0781a_de47b4cc PS5, Line 129: uint8_t usb3_port; /* USB2 Port Number */ : uint8_t usb2_port; /* USB3 Port Number */ This can be dropped now that there is a separate structure for the port map?