Attention is currently required from: Chiranjeevi Rapolu, Tim Wawrzynczak, John Zhao, Duncan Laurie, Brandon Breitenstein.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51195 )
Change subject: mb/google/volteer: Configure tcss port information for early tcss init
......................................................................
Patch Set 1:
(2 comments)
File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/51195/comment/26524ff8_d2f69754
PS1, Line 182: mux_info
mux_info is never really allocated space. So, this code ends up writing at address 0. As mentioned on the SoC CL, I think this should return a pointer to the table rather than expecting SoC to pass in allocated space:
const struct tcss_mux *mainboard_get_tcss_mux_info(size_t *num_ports)
{
static struct tcss_mux mux_info[MAX_TYPE_C_PORTS];
size_t port;
*num_ports = ARRAY_SIZE(mux_info);
for (port = 0; port < *num_ports; port++) {
...
}
return mux_info;
}
https://review.coreboot.org/c/coreboot/+/51195/comment/22bf1e66_3ce4af91
PS1, Line 186: get_connector_config
This function can be called with mux as a pointer rather than having to fetch mux pointer every time.
const struct device *pmc;
const struct device *mux;
*num_ports = 0;
pmc = pcidev_path_on_root(PCH_DEVFN_PMC);
if (!pmc)
return NULL;
mux = pmc->link_list->children;
if (!mux)
return NULL;
*num_ports = ARRAY_SIZE(mux_info);
for (port = 0; port < *num_ports; port++) {
mux_config = get_connector_config(mux, port);
if (mux_config == NULL)
continue;
mux_info[port].usb2_port = ...;
...
}
--
To view, visit https://review.coreboot.org/c/coreboot/+/51195
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I59e5c5a7d2ab5ef5293abe6c59c3a585b25f7b75
Gerrit-Change-Number: 51195
Gerrit-PatchSet: 1
Gerrit-Owner: Brandon Breitenstein <brandon.breitenstein(a)intel.com>
Gerrit-Reviewer: Chiranjeevi Rapolu <chiranjeevi.rapolu(a)intel.com>
Gerrit-Reviewer: Duncan Laurie <dlaurie(a)gmail.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: John Zhao <john.zhao(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Chiranjeevi Rapolu <chiranjeevi.rapolu(a)intel.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: John Zhao <john.zhao(a)intel.com>
Gerrit-Attention: Duncan Laurie <dlaurie(a)gmail.com>
Gerrit-Attention: Brandon Breitenstein <brandon.breitenstein(a)intel.com>
Gerrit-Comment-Date: Tue, 02 Mar 2021 22:57:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Chiranjeevi Rapolu, Tim Wawrzynczak, John Zhao, Duncan Laurie.
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51194 )
Change subject: soc/intel/tigerlake: Enable TCSS Muxes to disconnect mode during boot
......................................................................
Patch Set 1:
(1 comment)
File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/51194/comment/415551ed_8d87a32a
PS1, Line 322: mainboard_tcss_get_port_info
> I don't think you need another callback here. […]
The reason it was done this way was to avoid any calling to EC/other FWs to get the information for the entire mux configuration. This call simply gets only the mux and port map information to avoid any unwanted delays on systems that do not need the entire display stack implemented
--
To view, visit https://review.coreboot.org/c/coreboot/+/51194
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4352072a4a7d6ccb1364b38377831f3c22ae8fb4
Gerrit-Change-Number: 51194
Gerrit-PatchSet: 1
Gerrit-Owner: Brandon Breitenstein <brandon.breitenstein(a)intel.com>
Gerrit-Reviewer: Chiranjeevi Rapolu <chiranjeevi.rapolu(a)intel.com>
Gerrit-Reviewer: Duncan Laurie <dlaurie(a)gmail.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: John Zhao <john.zhao(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Chiranjeevi Rapolu <chiranjeevi.rapolu(a)intel.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: John Zhao <john.zhao(a)intel.com>
Gerrit-Attention: Duncan Laurie <dlaurie(a)gmail.com>
Gerrit-Comment-Date: Tue, 02 Mar 2021 22:54:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Chiranjeevi Rapolu, John Zhao, Duncan Laurie, Brandon Breitenstein.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51194 )
Change subject: soc/intel/tigerlake: Enable TCSS Muxes to disconnect mode during boot
......................................................................
Patch Set 1:
(6 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/51194/comment/37c34829_1db6f797
PS1, Line 12: the ports are not initialized till the ports are set to the appropriate mode.
In addition to setting the muxes to disconnected state, early_tcss driver is also setting up the port maps for TCSS USB2/3 ports. Can you please capture that information in the commit message too? As per the bug, it is the port map that seems to be critical rather than setting the disconnected state.
File src/soc/intel/tigerlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/51194/comment/78e03dd4_9e22c368
PS1, Line 250: Enables TCSS to initialize devices before Kernel.
Sets up USB2/3 port mapping in TCSS Mux and sets MUX state to disconnected.
File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/51194/comment/15de30af_ab985898
PS1, Line 267: initialize_mux_to_disc_mode
Probably just say `tcss_init_mux()`? It does two things:
1. Resets MUX to disconnected state
2. Sets up USB2/3 port mapping in the MUX.
https://review.coreboot.org/c/coreboot/+/51194/comment/1fde4cb0_150a8210
PS1, Line 274: inial
initial
https://review.coreboot.org/c/coreboot/+/51194/comment/424af763_39265726
PS1, Line 277: update_tcss_mux
I think this should be renamed to `tcss_configure_dp_mode()` so that the intent is clear.
https://review.coreboot.org/c/coreboot/+/51194/comment/14277f39_731e61c4
PS1, Line 322: mainboard_tcss_get_port_info
I don't think you need another callback here. `mainboard_tcss_fill_mux_info()` is obtaining the information about TCSS ports from the mainboard. That should be sufficient. Also, this code should be run before the early display check. Basically something like:
```
void tcss_early_configure(void)
{
// Step 1: Get MUX info using mainboard_tcss_fill_mux_info()
// Step 2: Reset MUX to disconnected state and set up USB2/3 port mapping
// Step 3: If display init is not required, return.
// Step 4: If display init is required, update MUX for DP mode.
}
```
--
To view, visit https://review.coreboot.org/c/coreboot/+/51194
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4352072a4a7d6ccb1364b38377831f3c22ae8fb4
Gerrit-Change-Number: 51194
Gerrit-PatchSet: 1
Gerrit-Owner: Brandon Breitenstein <brandon.breitenstein(a)intel.com>
Gerrit-Reviewer: Chiranjeevi Rapolu <chiranjeevi.rapolu(a)intel.com>
Gerrit-Reviewer: Duncan Laurie <dlaurie(a)gmail.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: John Zhao <john.zhao(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Chiranjeevi Rapolu <chiranjeevi.rapolu(a)intel.com>
Gerrit-Attention: John Zhao <john.zhao(a)intel.com>
Gerrit-Attention: Duncan Laurie <dlaurie(a)gmail.com>
Gerrit-Attention: Brandon Breitenstein <brandon.breitenstein(a)intel.com>
Gerrit-Comment-Date: Tue, 02 Mar 2021 22:42:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Nikolai Vyssotski, Felix Held.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51193 )
Change subject: amd_blobs: update submodule pointer
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/51193
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I11fc199ca7bc6ee7431c59d35a60d9ebd977bf10
Gerrit-Change-Number: 51193
Gerrit-PatchSet: 1
Gerrit-Owner: Nikolai Vyssotski <nikolai.vyssotski(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nikolai Vyssotski <nikolai.vyssotski(a)amd.corp-partner.google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Tue, 02 Mar 2021 22:41:24 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Marc Jones, Furquan Shaikh, Martin Roth, Jonathan Zhang, Johnny Lin, Rocky Phagura, Angel Pons, Patrick Rudolph.
Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49849 )
Change subject: soc/intel/xeon_sp: Add PCH lockdown
......................................................................
Patch Set 4: Code-Review+1
(1 comment)
File src/soc/intel/common/pch/Kconfig:
https://review.coreboot.org/c/coreboot/+/49849/comment/10f1a1e3_d5bd206d
PS4, Line 21: if !XEON_SP_COMMON_BASE
Is there a better way to categorize this, or to have a set of convenience groups for these options? This is one of the many many ways in which this common code mess is broken.DSP, graphics, LPSS etc are not just missing on XEON_SP. Graphics is most likely missing on everything that's servers.
We could have groups for
- SERVER
- DESKTOP
- MOBILE
or (cleaner), have each SOC/chipset actually select the blocks they need.
The advantage of the second solution would be that we also find messed up dependencies in this code more easily. E.G. The GPIO code is by no means generic to a large nunmber of Intel SOCs.
--
To view, visit https://review.coreboot.org/c/coreboot/+/49849
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iab97123e487f4f13f874f364a9c51723d234d4f0
Gerrit-Change-Number: 49849
Gerrit-PatchSet: 4
Gerrit-Owner: Marc Jones <marc(a)marcjonesconsulting.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Jay Talbott <JayTalbott(a)sysproconsulting.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Rocky Phagura
Gerrit-Reviewer: Rocky Phagura <rphagura(a)fb.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Attention: Marc Jones <marc(a)marcjonesconsulting.com>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Rocky Phagura <rphagura(a)fb.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Rocky Phagura
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Tue, 02 Mar 2021 22:31:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Jason Glenesk, Raul Rangel, Paul Menzel, Angel Pons.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51076 )
Change subject: soc/amd/cezanne: Disable legacy DMA IO ports
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/51076/comment/6a3b09d0_7be77597
PS2, Line 10: ports.
> ok
had a look and didn't find any mainboard that is compatible with the renoir/cezanne apus and still has a fdd connector. there are very few compatible or possibly compatible mainboards that still have a parallel port on an internal connector and since it can be operated in a mode that doesn't require the legacy isa dma to be present, i don't really expect this to break things. also the ports around 0x80 don't get decoded to the isa dma controller, so it's unclear it that has a chance to work with the default that doesn't decode those ports to isa dma
--
To view, visit https://review.coreboot.org/c/coreboot/+/51076
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7792d1f8ea40eb1c7f6cca67e9907208884ac694
Gerrit-Change-Number: 51076
Gerrit-PatchSet: 2
Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Tue, 02 Mar 2021 22:16:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)users.sourceforge.net>
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: comment
Attention is currently required from: Martin Roth, Henry Sun, zanxi chen, Paul Menzel, Andy Yeh, Weimin Wu, ShawnX Tu.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50995 )
Change subject: mb/google/dedede/var/storo: Add camera support
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
> Hi Karthik, […]
Even if camera EEPROM is going to be provisioned later, I would like SSFC in CBI to be provisioned now so that we can enable the MIPI camera for source value 1.
If we dont provision SSFC now, later when we add a second source we have to go add some unnnecessary code to handle the unprovisioned scenario.
I would rather prefer to spend some time now and than more time later.
--
To view, visit https://review.coreboot.org/c/coreboot/+/50995
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I98d5708d1955406c2e46db972903057bb3d12dcc
Gerrit-Change-Number: 50995
Gerrit-PatchSet: 2
Gerrit-Owner: zanxi chen <chenzanxi(a)huaqin.corp-partner.google.com>
Gerrit-Reviewer: Andy Yeh <andy.yeh(a)intel.com>
Gerrit-Reviewer: Henry Sun <henrysun(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: ShawnX Tu <shawnx.tu(a)intel.com>
Gerrit-Reviewer: Tao Xia <xiatao5(a)huaqin.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Martin Roth <martinroth(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-CC: Weimin Wu <wuweimin(a)huaqin.corp-partner.google.com>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Henry Sun <henrysun(a)google.com>
Gerrit-Attention: zanxi chen <chenzanxi(a)huaqin.corp-partner.google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Attention: Andy Yeh <andy.yeh(a)intel.com>
Gerrit-Attention: Weimin Wu <wuweimin(a)huaqin.corp-partner.google.com>
Gerrit-Attention: Andy Yeh <andy.yeh(a)intel.com>
Gerrit-Attention: ShawnX Tu <shawnx.tu(a)intel.com>
Gerrit-Comment-Date: Tue, 02 Mar 2021 22:07:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Henry Sun <henrysun(a)google.com>
Comment-In-Reply-To: zanxi chen <chenzanxi(a)huaqin.corp-partner.google.com>
Comment-In-Reply-To: Andy Yeh <andy.yeh(a)intel.com>
Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Chiranjeevi Rapolu, John Zhao.
Brandon Breitenstein has removed Patrick Rudolph from this change. ( https://review.coreboot.org/c/coreboot/+/51194 )
Change subject: soc/intel/tigerlake: Enable TCSS Muxes to disconnect mode during boot
......................................................................
Removed reviewer Patrick Rudolph.
--
To view, visit https://review.coreboot.org/c/coreboot/+/51194
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4352072a4a7d6ccb1364b38377831f3c22ae8fb4
Gerrit-Change-Number: 51194
Gerrit-PatchSet: 1
Gerrit-Owner: Brandon Breitenstein <brandon.breitenstein(a)intel.com>
Gerrit-Reviewer: Chiranjeevi Rapolu <chiranjeevi.rapolu(a)intel.com>
Gerrit-Reviewer: John Zhao <john.zhao(a)intel.com>
Gerrit-Attention: Chiranjeevi Rapolu <chiranjeevi.rapolu(a)intel.com>
Gerrit-Attention: John Zhao <john.zhao(a)intel.com>
Gerrit-MessageType: deleteReviewer