[coreboot-gerrit] Change in coreboot[master]: soc/intel/common: Configure TCSS access through IOE P2SB

[Tim Wawrzynczak (Code Review)]

Attention is currently required from: Subrata Banik, John Zhao, Paul Menzel, Eric Lai, Angel Pons, Patrick Rudolph, EricR Lai.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62723 )

Change subject: soc/intel/common: Configure TCSS access through IOE P2SB
......................................................................

Patch Set 5:

(1 comment)

File src/soc/intel/common/block/tcss/tcss.c:

https://review.coreboot.org/c/coreboot/+/62723/comment/3400895d_95a1f3a8 
PS5, Line 371: static void ioe_tcss_configure_aux_bias_pads_sbi(
             : 	const struct typec_aux_bias_pads *pads)
             : {
             : 	for (size_t i = 0; i < MAX_TYPE_C_PORTS; i++) {
             : 		if (pads[i].pad_auxn_dc && pads[i].pad_auxp_dc) {
             : 			ioe_p2sb_sbi_write(PID_IOM, IOM_AUX_BIAS_CTRL_PULLUP_OFFSET(i),
             : 				calc_bias_ctrl_reg_value(pads[i].pad_auxp_dc));
             : 			ioe_p2sb_sbi_write(PID_IOM, IOM_AUX_BIAS_CTRL_PULLDOWN_OFFSET(i),
             : 				calc_bias_ctrl_reg_value(pads[i].pad_auxn_dc));
             : 		}
             : 	}
             : }
             : 
             : static void tcss_configure_aux_bias_pads(
             : 	const struct typec_aux_bias_pads *pads)
             : {
             : 	if (CONFIG(SOC_INTEL_COMMON_BLOCK_TCSS_REG_ACCESS_REGBAR))
             : 		tcss_configure_aux_bias_pads_regbar(pads);
             : 	else if (CONFIG(SOC_INTEL_COMMON_BLOCK_TCSS_REG_ACCESS_SBI)) {
             : 		if (CONFIG(SOC_INTEL_COMMON_BLOCK_IOE_P2SB))
             : 			ioe_tcss_configure_aux_bias_pads_sbi(pads);
             : 	}
             : 	else
             : 		printk(BIOS_ERR, "%s: Error: No TCSS configuration method is selected!\n",
             : 					__func__);
             : }
             : 
             : const struct tcss_port_map *tcss_get_port_info(size_t *num_ports)
             : {
             : 	static struct tcss_port_map port_map[MAX_TYPE_C_PORTS];
             : 	size_t active_ports = 0;
             : 	size_t port;
             : 
             : 	for (port = 0; port < MAX_TYPE_C_PORTS; port++) {
             : 		const struct device_path conn_path[] = {
             : 			{.type = DEVICE_PATH_PCI, .pci.devfn = PCH_DEVFN_PMC},
             : 			{.type = DEVICE_PATH_GENERIC, .generic.id = 0, .generic.subid = 0},
             : 			{.type = DEVICE_PATH_GENERIC, .generic.id = port},
             : 		};
             : 		const struct device *conn = find_dev_nested_path(pci_root_bus(), conn_path,
             : 								ARRAY_SIZE(conn_path));
             : 		unsigned int usb2_port, usb3_port;
             : 
             : 		if (!conn)
             : 			continue;
             : 
             : 		if (CONFIG(DRIVERS_INTEL_PMC) &&
             : 			intel_pmc_mux_conn_get_ports(conn, &usb2_port, &usb3_port)) {
             : 			port_map[active_ports].usb2_port = usb2_port;
             : 			port_map[active_ports].usb3_port = usb3_port;
             : 			++active_ports;
             : 		}
             : 	}
             : 
             : 	*num_ports = active_ports;
             : 	return port_map;
             : }
             : 
             : void tcss_configure(const struct typec_aux_bias_pads aux_bias_pads[MAX_TYPE_C_PORTS])
             : {
             : 	const struct tcss_port_map *port_map;
             : 	size_t num_ports;
             : 	size_t i;
             : 
             : 	port_map = tcss_get_port_info(&num_ports);
             : 	if (port_map == NULL)
             : 		return;
             : 
             : 	for (i = 0; i < num_ports; i++)
             : 		tcss_init_mux(i, &port_map[i]);
             : 
             : 	/* This should be performed before alternate modes are entered */
             : 	tcss_configure_aux_bias_pads(aux_bias_pads);
             : 
             : 	if (CONFIG(ENABLE_TCSS_DISPLAY_DETECTION))
             : 		tcss_configure_dp_mode(port_map, num_ports);
             : }
             : 
             : uint32_t tcss_valid_tbt_auth(void)
             : {
             : 	if (CONFIG(SOC_INTEL_COMMON_BLOCK_TCSS_REG_ACCESS_REGBAR)) {
             : 		return REGBAR32(PID_IOM, IOM_CSME_IMR_TBT_STATUS) & TBT_VALID_AUTHENTICATION;
             : 	} else if (CONFIG(SOC_INTEL_COMMON_BLOCK_TCSS_REG_ACCESS_SBI)) {
             : 		if (CONFIG(SOC_INTEL_COMMON_BLOCK_IOE_P2SB))
             : 			return (ioe_p2sb_sbi_read(PID_IOM, IOM_CSME_IMR_TBT_STATUS) &
             : 							TBT_VALID_AUTHENTICATION);
             : 	}
             : 
             : 	printk(BIOS_ERR, "%s: Error: No validation for Thunderbolt authentication!\n",
             : 					__func__);
             : 	return 0;
             : }
This is getting to be pretty complex logic to pick which method is used to access TCSS registers here...

I can think of a few ways to make this cleaner:
1) A table of function pointers e.g.
```
   struct soc_tcss_ops {
         void (*configure_aux_bias_pads)(const struct typec_aux_bias_pads *pads)
         bool (*valid_tbt_auth)(void);
   };
```

one each for TGL, ADL, and MTL. The common functions can just live in this file and be exported, and each SoC just selects the appropriate pointers.

or

2) Declare functions in common/block/intelblocks/tcss.h, e.g.

```
void soc_tcss_configure_aux_bias_pads(const struct typec_aux_bias_pads *pads)
bool soc_tcss_valid_tbt_auth(void);
```

and then each SoC that selects COMMON_BLOCK_TCSS has to also implement these functions (no __weak functions).