coreboot-gerrit June 2021

coreboot-gerrit@coreboot.org

1 participants
7127 discussions

Change in coreboot[master]: mb/google/brya: Add EC_HOST_EVENT_USB_MUX
by Tim Wawrzynczak (Code Review) 02 Jun '21

02 Jun '21

Attention is currently required from: John Zhao. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55126 ) Change subject: mb/google/brya: Add EC_HOST_EVENT_USB_MUX ...................................................................... Patch Set 1: Code-Review+1 (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/55126/comment/f533d46d_99f3ca88 PS1, Line 11: BUG=b:183140386 this is a bug for a different board (i don't think we need one for this) -- To view, visit https://review.coreboot.org/c/coreboot/+/55126 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6f4dcbc60a6cb131f28de205bd9ef436f2b508eb Gerrit-Change-Number: 55126 Gerrit-PatchSet: 1 Gerrit-Owner: 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: John Zhao <john.zhao(a)intel.com> Gerrit-Comment-Date: Wed, 02 Jun 2021 19:20:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/alderlake: Set SaIpuEnable UPD from device state
by Furquan Shaikh (Code Review) 02 Jun '21

02 Jun '21

Attention is currently required from: Tim Wawrzynczak, Patrick Rudolph. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55143 ) Change subject: soc/intel/alderlake: Set SaIpuEnable UPD from device state ...................................................................... Patch Set 1: (1 comment) File src/soc/intel/alderlake/romstage/fsp_params.c: https://review.coreboot.org/c/coreboot/+/55143/comment/f93160d9_d22d034e PS1, Line 214: m_cfg->SaIpuEnable This check can be left as is, right? Since we have already evaluated the state of SA_DEVFN_IPU device on line 208? -- To view, visit https://review.coreboot.org/c/coreboot/+/55143 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I53752f92c4b49093218cc34848727a72b63e84eb Gerrit-Change-Number: 55143 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Wed, 02 Jun 2021 19:19:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/guybrush/var/guybrush: Reconfigure left speaker amplifier
by Furquan Shaikh (Code Review) 02 Jun '21

02 Jun '21

Attention is currently required from: Martin Roth, Marshall Dawson, Karthik Ramasubramanian. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55028 ) Change subject: mb/google/guybrush/var/guybrush: Reconfigure left speaker amplifier ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/55028 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I69a7e7dd65a459c2e5629ada1dea1f1660dd9990 Gerrit-Change-Number: 55028 Gerrit-PatchSet: 2 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Eric Peers <epeers(a)google.com> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Wed, 02 Jun 2021 19:16:37 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya: Add firmware configuration probing for audio
by Tim Wawrzynczak (Code Review) 02 Jun '21

02 Jun '21

Attention is currently required from: Furquan Shaikh, Sugnan Prabhu S, Paul Menzel, Sathya Prakash M R, Patrick Rudolph. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54716 ) Change subject: mb/google/brya: Add firmware configuration probing for audio ...................................................................... Patch Set 8: Code-Review+2 (1 comment) File src/mainboard/google/brya/variants/brya0/fw_config.c: https://review.coreboot.org/c/coreboot/+/54716/comment/96a8a995_d4a10599 PS7, Line 82: BS_PRE_DEVICE > I have changed it to BS_DEV_ENABLE for now, PRE_DEVICE changes can be discussed on the corresponding […] Ack -- To view, visit https://review.coreboot.org/c/coreboot/+/54716 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6f159442516830f9d304d78c83f070e4fcff4a37 Gerrit-Change-Number: 54716 Gerrit-PatchSet: 8 Gerrit-Owner: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Sathya Prakash M R <sathya.prakash.m.r(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Sathya Prakash M R <sathya.prakash.m.r(a)intel.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Wed, 02 Jun 2021 19:15:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya: move MIPI camera setting into overridetree
by Tim Wawrzynczak (Code Review) 02 Jun '21

02 Jun '21

Attention is currently required from: Furquan Shaikh, Scott Chao. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55134 ) Change subject: mb/google/brya: move MIPI camera setting into overridetree ...................................................................... Patch Set 1: (1 comment) File src/mainboard/google/brya/variants/baseboard/devicetree.cb: https://review.coreboot.org/c/coreboot/+/55134/comment/b0b241bf_75f8b949 PS1, Line 132: device ref ipu on end > Ah yes. That will be required too. […] CB:55143 -- To view, visit https://review.coreboot.org/c/coreboot/+/55134 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4e64d078a8e39732ad29443c3b09ca008a7e902f Gerrit-Change-Number: 55134 Gerrit-PatchSet: 1 Gerrit-Owner: Scott Chao <scott_chao(a)wistron.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Casper Chang <casper_chang(a)wistron.corp-partner.google.com> Gerrit-CC: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Scott Chao <scott_chao(a)wistron.corp-partner.google.com> Gerrit-Comment-Date: Wed, 02 Jun 2021 19:11:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/alderlake: Set SaIpuEnable UPD from device state
by Tim Wawrzynczak (Code Review) 02 Jun '21

02 Jun '21

Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/55143 ) Change subject: soc/intel/alderlake: Set SaIpuEnable UPD from device state ...................................................................... soc/intel/alderlake: Set SaIpuEnable UPD from device state The SaIpuEnable UPD is not currently being touched by coreboot; set it according to the enabled status of the corresponding devicetree node. Change-Id: I53752f92c4b49093218cc34848727a72b63e84eb Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> --- M src/soc/intel/alderlake/romstage/fsp_params.c 1 file changed, 6 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/55143/1 diff --git a/src/soc/intel/alderlake/romstage/fsp_params.c b/src/soc/intel/alderlake/romstage/fsp_params.c index ba6e036..662db40 100644 --- a/src/soc/intel/alderlake/romstage/fsp_params.c +++ b/src/soc/intel/alderlake/romstage/fsp_params.c @@ -203,6 +203,10 @@ dev = pcidev_path_on_root(SA_DEVFN_TBT3); m_cfg->TcssItbtPcie3En = is_dev_enabled(dev); + /* IPU */ + dev = pcidev_path_on_root(SA_DEVFN_IPU); + m_cfg->SaIpuEnable = is_dev_enabled(dev); + /* VT-d config */ m_cfg->VtdBaseAddress[VTD_GFX] = GFXVT_BASE_ADDRESS; m_cfg->VtdBaseAddress[VTD_IPU] = IPUVT_BASE_ADDRESS; @@ -211,7 +215,8 @@ m_cfg->VtdDisable = 0; m_cfg->VtdIopEnable = !m_cfg->VtdDisable; m_cfg->VtdIgdEnable = m_cfg->InternalGfx; - m_cfg->VtdIpuEnable = m_cfg->SaIpuEnable; + dev = pcidev_path_on_root(SA_DEVFN_IPU); + m_cfg->VtdIpuEnable = is_dev_enabled(dev); if (m_cfg->VtdIgdEnable && m_cfg->VtdBaseAddress[VTD_GFX] == 0) { m_cfg->VtdIgdEnable = 0; -- To view, visit https://review.coreboot.org/c/coreboot/+/55143 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I53752f92c4b49093218cc34848727a72b63e84eb Gerrit-Change-Number: 55143 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: mb/google/guybrush: Move variant_has_fpmcu() after eSPI init
by Furquan Shaikh (Code Review) 02 Jun '21

02 Jun '21

Attention is currently required from: Martin Roth. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54740 ) Change subject: mb/google/guybrush: Move variant_has_fpmcu() after eSPI init ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/54740 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6de44119e92c8820b266f9f07287706c7d4eb505 Gerrit-Change-Number: 54740 Gerrit-PatchSet: 4 Gerrit-Owner: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.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)mailbox.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Comment-Date: Wed, 02 Jun 2021 19:07:55 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya: move MIPI camera setting into overridetree
by Furquan Shaikh (Code Review) 02 Jun '21

02 Jun '21

Attention is currently required from: Tim Wawrzynczak, Scott Chao. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55134 ) Change subject: mb/google/brya: move MIPI camera setting into overridetree ...................................................................... Patch Set 1: (1 comment) File src/mainboard/google/brya/variants/baseboard/devicetree.cb: https://review.coreboot.org/c/coreboot/+/55134/comment/8538c167_7fe5a0b0 PS1, Line 132: device ref ipu on end > Hmm, probably should, but also we still need to hook up this device's enabled status to the SaIpuEna […] Ah yes. That will be required too. Dropping this from baseboard/devicetree will set the device state correctly and the SoC code will have to take that into account to set the UPD. -- To view, visit https://review.coreboot.org/c/coreboot/+/55134 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4e64d078a8e39732ad29443c3b09ca008a7e902f Gerrit-Change-Number: 55134 Gerrit-PatchSet: 1 Gerrit-Owner: Scott Chao <scott_chao(a)wistron.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Casper Chang <casper_chang(a)wistron.corp-partner.google.com> Gerrit-CC: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Scott Chao <scott_chao(a)wistron.corp-partner.google.com> Gerrit-Comment-Date: Wed, 02 Jun 2021 19:05:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/guybrush: Add helpers for cbi fw_config settings.
by Furquan Shaikh (Code Review) 02 Jun '21

02 Jun '21

Attention is currently required from: Raul Rangel, Martin Roth, Paul Menzel, Karthik Ramasubramanian. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54639 ) Change subject: mb/google/guybrush: Add helpers for cbi fw_config settings. ...................................................................... Patch Set 5: (1 comment) File src/mainboard/google/guybrush/variants/guybrush/helpers.c: https://review.coreboot.org/c/coreboot/+/54639/comment/3cc9a4cc_35dd2d1f PS4, Line 44: get_variant_wwan_type > Furquan, I understand what you're saying. I just disagree with the approach. […] fw_config is used to allow OEMs to provide configuration information to software when there are multiple options they are choosing for a component on their project. This configuration information allows software to determine the appropriate steps to be performed (GPIO config, ACPI tables, etc.) at runtime. Since each OEM project is different, there is going to be a varied combination of options across the projects. Thus, the common code at the baseboard/mainboard level should not make any assumptions about the fw_config presence. In fact, it is totally fine if an OEM decides to not use fw_config at all if they intend to build just one single combination of components for the project. That is the reason why it is important to ensure that the common code at mainboard/baseboard level uses the information about devices rather than probing fw_config directly. In this case on guybrush, you can imagine different OEMs going with different combinations for their fingerprint support: - OEMs with no fingerprint support - OEMs with all SKUs having fingerprint support - OEMs with some SKUs having fingerprint support whereas others with no fingerprint support Out of these three, only the last case requires fw_config information. The suggestion that I made above is to ensure that the helper functions are organized in such a way at the baseboard level that there is maximum flexibility for the OEM to choose what fw_config options they want to go with and at the same time not have to implement support for varied helper functions specific to each variant. From your comments, it looks like the thing that you are most worried about is the runtime overhead of walking the device tree. That is not really a big problem to solve. We are already using unique device aliases in the device tree. It is just a matter of emitting all these aliases so that they can be referenced directly by the helpers. That should allow you to reduce the helper functions to a single line checks without having to rely on fw_config. I had made a comment about this earlier on this CL as well that it is possible to eliminate the device walk completely. I don't think this makes things more complicated. In fact, it makes it much simpler for the partners to manage their own variants in they way they have designed their boards. -- To view, visit https://review.coreboot.org/c/coreboot/+/54639 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I212e7f413b4d8a7d15122cde90100a0ec28e88a6 Gerrit-Change-Number: 54639 Gerrit-PatchSet: 5 Gerrit-Owner: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Furquan Shaikh <furquan(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Wed, 02 Jun 2021 19:03:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Martin Roth <martinroth(a)google.com> Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya: move MIPI camera setting into overridetree
by Tim Wawrzynczak (Code Review) 02 Jun '21

02 Jun '21

Attention is currently required from: Furquan Shaikh, Scott Chao. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55134 ) Change subject: mb/google/brya: move MIPI camera setting into overridetree ...................................................................... Patch Set 1: (1 comment) File src/mainboard/google/brya/variants/baseboard/devicetree.cb: https://review.coreboot.org/c/coreboot/+/55134/comment/b8f25104_40924517 PS1, Line 132: device ref ipu on end > Should this be dropped from baseboard/devicetree so that the ipu is default off? Hmm, probably should, but also we still need to hook up this device's enabled status to the SaIpuEnable UPD 😊 (we got lucky it defaults to 1) -- To view, visit https://review.coreboot.org/c/coreboot/+/55134 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4e64d078a8e39732ad29443c3b09ca008a7e902f Gerrit-Change-Number: 55134 Gerrit-PatchSet: 1 Gerrit-Owner: Scott Chao <scott_chao(a)wistron.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Casper Chang <casper_chang(a)wistron.corp-partner.google.com> Gerrit-CC: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Scott Chao <scott_chao(a)wistron.corp-partner.google.com> Gerrit-Comment-Date: Wed, 02 Jun 2021 19:01:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit June 2021