12 comments:
File src/soc/intel/tigerlake/early_tcss.c:
/*
* This file is part of the coreboot project.
*
* Copyright 2020 The coreboot project Authors.
*
* SPDX-License-Identifier: GPL-2.0-or-later
*/
/* SPDX-License-Identifier: GPL-2.0-or-later */
Patch Set #10, Line 31: tcss_make_alt_mode_cmd_1
could we think of more descriptive names than alt_mode_cmd_1 and _2 ?
/*
* commenting this section out for now as probably not needed
static void *iom_reg(unsigned int iom_reg_offset)
{
const uintptr_t iombase = IOM_BASE_ADDRESS;
return (void *)(iombase + iom_reg_offset);
}
static const void *port_status_reg(int port)
{
uintptr_t sts_offset;
sts_offset = IOM_PORT_STATUS_OFFSET + IOM_REG_LEN * port;
return (const void *)iom_reg(sts_offset);
}
static bool is_port_connected(int port)
{
uint32_t sts;
sts = read32(port_status_reg(port));
return !!(sts & IOM_PORT_STATUS_CONNECTED);
}
*/
then go ahead and delete it, if you need it later, you can find it here in gerrit 😊
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_make_ipc_cmd(), a refactor would be much easier to read
nit: extra blank line
nit: extra blank line
nit: extra blank line
Patch Set #10, Line 259: if (mux_data.dp_mode <= MODE_DP_PIN_F) {
suggestion: remove this if, and use `default` instead
nit: extra blank line
nit: extra blank line
Patch Set #10, Line 350: return;
probably should have another big error message here.
Patch Set #10, Line 371: //Add check for connected maybe?
I think we should check, otherwise we're wasting boot time sending another host command and a PMC IPC as well.
To view, visit change 42079. To unsubscribe, or for help writing mail filters, visit settings.