Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42079 )
Change subject: soc/intel/tigerlake: Add code for early tcss ......................................................................
Patch Set 10:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... PS10, Line 31: tcss_make_alt_mode_cmd_1
could we think of more descriptive names than alt_mode_cmd_1 and _2 ?
These literally correspond to Buffer 0 and buffer 1 I can name it as such there really isn't a more descriptive name thatn that though unfortunately
https://review.coreboot.org/c/coreboot/+/42079/10/src/soc/intel/tigerlake/ea... PS10, Line 81: static int send_conn_disc_req(const struct pmc_ipc_buffer *req, : struct pmc_ipc_buffer *res) : { : : uint32_t cmd_reg; : uint32_t res_reg; : int tries = 2; : int r; : : cmd_reg = pmc_make_ipc_cmd(PMC_IPC_USBC_CMD_ID, PMC_IPC_USBC_SUBCMD_ID, : PMC_IPC_CONN_REQ_SIZE); : : do { : r = pmc_send_ipc_cmd(cmd_reg, req, res); : if (r < 0) { : printk(BIOS_ERR, "pmc_send_ipc_cmd failed\n"); : return -1; : } : res_reg = res->buf[0]; : : if (!TCSS_STATUS_HAS_FAILED(res_reg)) { : printk(BIOS_DEBUG, "pmc_send_ipc_cmd succeeded\n"); : return 0; : } : : if (TCSS_STATUS_IS_FATAL(res_reg)) { : printk(BIOS_ERR, "pmc_send_ipc_cmd status: fatal\n"); : return -1; : } : } while (--tries >= 0); : : printk(BIOS_ERR, "pmc_send_ipc_cmd failed after retries\n"); : return -1; : } : : static int send_safe_mode_req(const struct pmc_ipc_buffer *req, : struct pmc_ipc_buffer *res) : { : uint32_t cmd_reg; : uint32_t res_reg; : int tries = 2; : int r; : : cmd_reg = pmc_make_ipc_cmd(PMC_IPC_USBC_CMD_ID, PMC_IPC_USBC_SUBCMD_ID, : PMC_IPC_ALT_REQ_SIZE); : : do { : r = pmc_send_ipc_cmd(cmd_reg, req, res); : if (r < 0) { : printk(BIOS_ERR, "pmc_send_ipc_cmd failed\n"); : return -1; : } : res_reg = res->buf[0]; : : if (!TCSS_STATUS_HAS_FAILED(res_reg)) { : printk(BIOS_DEBUG, "pmc_send_ipc_cmd succeeded\n"); : return 0; : } : : if (TCSS_STATUS_IS_FATAL(res_reg)) { : printk(BIOS_ERR, "pmc_send_ipc_cmd status: fatal\n"); : return -1; : } : } while (--tries >= 0); : : printk(BIOS_ERR, "pmc_send_ipc_cmd failed after retries\n"); : return -1; : } : : static int send_dp_mode_req(const struct pmc_ipc_buffer *req, : struct pmc_ipc_buffer *res) : { : uint32_t cmd_reg; : uint32_t res_reg; : int tries = 2; : int r; : : cmd_reg = pmc_make_ipc_cmd(PMC_IPC_USBC_CMD_ID, PMC_IPC_USBC_SUBCMD_ID, : PMC_IPC_ALT_REQ_SIZE); : : do { : r = pmc_send_ipc_cmd(cmd_reg, req, res); : if (r < 0) { : printk(BIOS_ERR, "pmc_send_ipc_cmd failed\n"); : return -1; : } : res_reg = res->buf[0]; : : if (!TCSS_STATUS_HAS_FAILED(res_reg)) { : printk(BIOS_DEBUG, "pmc_send_ipc_cmd succeeded\n"); : return 0; : } : : if (TCSS_STATUS_IS_FATAL(res_reg)) { : printk(BIOS_ERR, "pmc_send_ipc_cmd status: fatal\n"); : return -1; : } : } while (--tries >= 0); : : printk(BIOS_ERR, "pmc_send_ipc_cmd failed after retries\n"); : return -1; : }
All 3 of these are virtually the same, looks like the only difference is the 3rd argument to pmc_mak […]
Good point I'll consolidate this