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 23:
(14 comments)
https://review.coreboot.org/c/coreboot/+/37870/17/src/soc/intel/tigerlake/Kc... File src/soc/intel/tigerlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/37870/17/src/soc/intel/tigerlake/Kc... PS17, Line 220:
nit: Extra blank line not required.
Done
https://review.coreboot.org/c/coreboot/+/37870/9/src/soc/intel/tigerlake/ear... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/37870/9/src/soc/intel/tigerlake/ear... PS9, Line 29: * Flags representing mux state */
if there is a similar file, move these to that file […]
Done
https://review.coreboot.org/c/coreboot/+/37870/9/src/soc/intel/tigerlake/ear... PS9, Line 73: "Port %d tcss_res=0x%x"
please terminate with newline
Done
https://review.coreboot.org/c/coreboot/+/37870/10/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/37870/10/src/soc/intel/tigerlake/ea... PS10, Line 18: #include <console/post_codes.h>
This was being used in an earlier iteration will remove
Done
https://review.coreboot.org/c/coreboot/+/37870/10/src/soc/intel/tigerlake/ea... PS10, Line 27:
use tab
Done
https://review.coreboot.org/c/coreboot/+/37870/10/src/soc/intel/tigerlake/ea... PS10, Line 29: /* Flags representing mux state */ : #define USB_PD_MUX_USB_ENABLED BIT(0) /* USB connected */ : #define USB_PD_MUX_DP_ENABLED BIT(1) /* DP connected */ : #define USB_PD_MUX_POLARITY_INVERTED BIT(2) /* CC line Polarity inverted */ : #define USB_PD_MUX_HPD_IRQ BIT(3) /* HPD IRQ is asserted */ : #define USB_PD_MUX_HPD_LVL BIT(4) /* HPD level is asserted */ :
will address this today... […]
Done
https://review.coreboot.org/c/coreboot/+/37870/10/src/soc/intel/tigerlake/ea... PS10, Line 81: google_chromeec_usb_get_pd_ports
Humm.. I understand that this is something which is very Chrome OS/Chrome EC specific. […]
Done
https://review.coreboot.org/c/coreboot/+/37870/10/src/soc/intel/tigerlake/ea... PS10, Line 81: num_ports = google_chromeec_usb_get_pd_ports();
Maybe the signature should be changed.
Done
https://review.coreboot.org/c/coreboot/+/37870/10/src/soc/intel/tigerlake/ea... PS10, Line 93: port_map = google_chromeec_pd_get_port_info(i);
i mean need to return error in CL:37867 so that this code can use it
Done
https://review.coreboot.org/c/coreboot/+/37870/10/src/soc/intel/tigerlake/ea... PS10, Line 115: BS_OS_RESUME
My understanding is that coreboot needed to reconnect the devices on resume. […]
Done
https://review.coreboot.org/c/coreboot/+/37870/10/src/soc/intel/tigerlake/ea... PS10, Line 116: BS_PAYLOAD_LOAD
I will take that into account when adding in the early display code. […]
Done
https://review.coreboot.org/c/coreboot/+/37870/16/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/37870/16/src/soc/intel/tigerlake/ea... PS16, Line 155: mux_flags & USB_PD_MUX_USB_ENABLED
please change all the below items to bool […]
I assume all the flags only so usb, dp, cable, polarity, hpd_irq, and hpd_lvl?
and it should be mux_data.usb = !!(mux_flags & USB_PD_MUX_USB_ENABLED) correct?
https://review.coreboot.org/c/coreboot/+/37870/16/src/soc/intel/tigerlake/ea... PS16, Line 158: mux_data.polarity = mux_flags & USB_PD_MUX_POLARITY_INVERTED;
This is applicable to Virtual muxes only (SOC has the mux). […]
Ack
https://review.coreboot.org/c/coreboot/+/37870/17/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/37870/17/src/soc/intel/tigerlake/ea... PS17, Line 174: BS_DEV_ENABLE
I think this is still too late. FSP-S gets called as part of chip init. […]
ok I'll have to change this up a bit then and add an early_tcss.h file. will update in the next push