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 25:
(13 comments)
https://review.coreboot.org/c/coreboot/+/37870/10//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37870/10//COMMIT_MSG@10 PS10, Line 10: PMC IPC driver is needed to communicate with EC in order to : get status of the Type C ports
The example I mentioned is actually pre-kernel. It is still firmware. […]
Ack
https://review.coreboot.org/c/coreboot/+/37870/10//COMMIT_MSG@12 PS10, Line 12:
Can you please add relevant partner bug?
Done
https://review.coreboot.org/c/coreboot/+/37870/10/src/soc/intel/tigerlake/Ma... File src/soc/intel/tigerlake/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/37870/10/src/soc/intel/tigerlake/Ma... PS10, Line 33: $(CONFIG_TGL_CHROME_EC)
will change this to early_tcss config flag
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 37: bool usb; : bool polarity; : bool ufp; : bool acc; : uint8_t usb3_port; : uint8_t usb2_port;
Can you please add comments indicating what these members really mean?
Done
https://review.coreboot.org/c/coreboot/+/37870/10/src/soc/intel/tigerlake/ea... PS10, Line 78: int num_ports, i;
unsigned int
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
I assume all the flags only so usb, dp, cable, polarity, hpd_irq, and hpd_lvl? […]
Done
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
ok I'll have to change this up a bit then and add an early_tcss.h file. […]
Done
https://review.coreboot.org/c/coreboot/+/37870/19/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/37870/19/src/soc/intel/tigerlake/ea... PS19, Line 71: /* send USB connect request */ : tcss_req[0] = PMC_IPC_TCSS_CONN_REQ_RES | /* Usage */ : mux_data.usb3_port << 4; : tcss_req[1] = mux_data.usb2_port | : mux_data.ufp << 4 | /* 1=UFP/0=DFP */ : mux_data.polarity << 5 | /* ORI-HSL */ : mux_data.polarity << 6 | /* ORI-SBU */ : mux_data.acc << 7; : req_size = 2; : printk(BIOS_DEBUG, "tcss_req[0]-> 0x%x\n" : "tcss_req[1]-> 0x%x\n\t", tcss_req[0], tcss_req[1]); :
add a separate function, […]
Done
https://review.coreboot.org/c/coreboot/+/37870/22/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/37870/22/src/soc/intel/tigerlake/ea... PS22, Line 1: /* : * This file is part of the coreboot project. : * : * Copyright (C) 2020 Intel Corp. : * : * This program is free software; you can redistribute it and/or modify : * it under the terms of the GNU General Public License as published by : * the Free Software Foundation; version 2 of the License. : * : * This program is distributed in the hope that it will be useful, : * but WITHOUT ANY WARRANTY; without even the implied warranty of : * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the : * GNU General Public License for more details. : */
coreboot.org is switching to SPDX headers : […]
Done
https://review.coreboot.org/c/coreboot/+/37870/23/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/37870/23/src/soc/intel/tigerlake/ea... PS23, Line 123: tcss_req[0] = PMC_IPC_TCSS_CONN_REQ_RES | /* Usage */ : mux_data.usb3_port << 4; : tcss_req[1] = mux_data.usb2_port | : mux_data.ufp << 4 | /* 1=UFP/0=DFP */ : mux_data.polarity << 5 | /* ORI-HSL */ : mux_data.polarity << 6 | /* ORI-SBU */ : mux_data.acc << 7; : req_size = PMC_IPC_CONN_REQ_SIZE; : printk(BIOS_DEBUG, "tcss_req[0]-> 0x%x\n" : "tcss_req[1]-> 0x%x\n\t", tcss_req[0], tcss_req[1]); : : /* Copy the request into the buffer */ : memcpy(wbuf, tcss_req, req_size); : : cmd.ipc_cmd.len = req_size; :
pls add a function, tcss_mux_connect()
Ack
https://review.coreboot.org/c/coreboot/+/37870/23/src/soc/intel/tigerlake/ea... PS23, Line 145: ret = send_dp_required_ipc_commands(mux_data);
I was wondering about that one I will fix that
Done
https://review.coreboot.org/c/coreboot/+/37870/25/src/soc/intel/tigerlake/ea... File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/37870/25/src/soc/intel/tigerlake/ea... PS25, Line 49: printk(BIOS_DEBUG, "tcss_req[0]-> 0x%x\n" : "tcss_req[1]-> 0x%x\n\t", tcss_req[0], tcss_req[1]); : : /* Copy the request into the buffer */ : memcpy(wbuf, tcss_req, req_size); : : return pmc_send_ipc_cmd(cmd.cmd, wbuf, rbuf);
req_size can be passed, […]
Ack
https://review.coreboot.org/c/coreboot/+/37870/25/src/soc/intel/tigerlake/ea... PS25, Line 151: int mux_flags, dp_mode;
move to line 161, make uint8_t to keep consistent
Done
-
Brandon Breitenstein (Code Review)