Change in coreboot[master]: mb/google/volteer/v/volteer2: Config for passive USB-C DB on C1
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47501 )
Change subject: mb/google/volteer/v/volteer2: Config for passive USB-C DB on C1 ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/coreboot/+/47501/3/src/mainboard/google/voltee... File src/mainboard/google/volteer/Kconfig:
https://review.coreboot.org/c/coreboot/+/47501/3/src/mainboard/google/voltee... PS3, Line 153: VARIANT_HAS_PASSIVE_USB_DB I don't think this is necessary. The weak `variant_usb3_passive_gpio_table` returns 0 pads to configure, so devices w/o the passive DB aren't affected.
https://review.coreboot.org/c/coreboot/+/47501/3/src/mainboard/google/voltee... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/47501/3/src/mainboard/google/voltee... PS3, Line 136: BIT(2) I think we could make a nice set of #define for this, here's my suggestion: ``` #define RETIMER 0 #define PLATFORM 1 #define TCSS_PORT_SBU_ORIENTATION_CONTROL(port, cntrl) ((cntrl) << ((port) - 1) << 1) ```
then this would look like: ``` cfg->TcssAuxOri |= TCSS_PORT_SBU_ORIENTATION_CONTROL(2, PLATFORM) ```
https://review.coreboot.org/c/coreboot/+/47501/3/src/mainboard/google/voltee... PS3, Line 129: /* Set up port C1 for USB3 passive DB */ : if (fw_config_probe(FW_CONFIG(DB_USB, USB3_PASSIVE))) { : /* : * TGL UP3/UP4 Processor EDS vol. 2a rev. 1.2 : * section 3.6.10 : * set IOM_TYPEC_SW_CONFIGURATION_4.PORT2_HSL_ORIENTATION_OVRRD_EN : */ : cfg->TcssAuxOri |= BIT(2); : cfg->IomTypeCPortPadCfg[2] = GPIO_ID_TCP1_AUXP_DC; : cfg->IomTypeCPortPadCfg[3] = GPIO_ID_TCP1_AUXN_DC; : } I think this should go in fw_config.c as well. Then there's only one place to change any configurations that depend on FW_CONFIG values.
-
Tim Wawrzynczak (Code Review)