Attention is currently required from: Alexander Couzens.
Forkoz has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51231 )
Change subject: T440P: Add Back Cover LED, Make it pulse like original
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/51231
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia38bc5de36aa72f383e6be6de03df78187cbe93b
Gerrit-Change-Number: 51231
Gerrit-PatchSet: 3
Gerrit-Owner: Forkoz <Crabstorage(a)getbackinthe.kitchen>
Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Reviewer: Forkoz <Crabstorage(a)getbackinthe.kitchen>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Comment-Date: Wed, 03 Mar 2021 19:49:01 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/51190 )
Change subject: soc/amd/cezanne/chipset.cb: rename alias for SATA controllers
......................................................................
soc/amd/cezanne/chipset.cb: rename alias for SATA controllers
Renoir/Cezanne have two SATA controllers with 2 ports each, so call them
sata_0 and sata_1.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: I6ebfd3a85f9b513901f205bc299e92564fa329e5
Reviewed-on: https://review.coreboot.org/c/coreboot/+/51190
Reviewed-by: Furquan Shaikh <furquan(a)google.com>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M src/soc/amd/cezanne/chipset.cb
1 file changed, 2 insertions(+), 2 deletions(-)
Approvals:
build bot (Jenkins): Verified
Furquan Shaikh: Looks good to me, approved
diff --git a/src/soc/amd/cezanne/chipset.cb b/src/soc/amd/cezanne/chipset.cb
index e82d435..5e3d269 100644
--- a/src/soc/amd/cezanne/chipset.cb
+++ b/src/soc/amd/cezanne/chipset.cb
@@ -32,8 +32,8 @@
device pci 0.7 alias mp2 off end # Sensor Fusion Hub (MP2)
end
device pci 8.2 alias gpp_bridge_b off # Internal GPP Bridge 1 to Bus B
- device pci 0.0 alias sata_ahci off end # SATA AHCI Mode
- device pci 0.1 alias sata_raid off end # SATA Controller; SATA Raid/AHCI Mode
+ device pci 0.0 alias sata_0 off end # first SATA controller; AHCI Mode
+ device pci 0.1 alias sata_1 off end # second SATA Controller; SATA Raid/AHCI Mode
device pci 0.2 alias xgbe_0 off end # 10 GbE Controller Port 0 (XGBE0)
device pci 0.3 alias xgbe_1 off end # 10 GbE Controller Port 1 (XGBE1)
end
--
To view, visit https://review.coreboot.org/c/coreboot/+/51190
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6ebfd3a85f9b513901f205bc299e92564fa329e5
Gerrit-Change-Number: 51190
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Mathew King <mathewk(a)chromium.org>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Attention is currently required from: Furquan Shaikh, Martin Roth, Angel Pons, Michal Motyl.
Mariusz Szafrański has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51180 )
Change subject: util/sconfig: Fix for multidomain support sconfig/devicetree.cb
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/51180/comment/e6d3589c_b7a87fb4
PS2, Line 7: multidomain
> I am curious what the use cases for the multi-domain support are? Last when I looked into this, ther […]
Thx for question.
I think it will require a little longer elaboration and I`m not sure if this review is a good place for it. Should I continue here or there is other "better" place for it?
--
To view, visit https://review.coreboot.org/c/coreboot/+/51180
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Idd639d0cb3ed1a49ed7c7b1c77ac747ba6f77672
Gerrit-Change-Number: 51180
Gerrit-PatchSet: 2
Gerrit-Owner: Mariusz Szafrański <mariuszx.szafranski(a)intel.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Michal Motyl <michalx.motyl(a)intel.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Suresh Bellampalli <suresh.bellampalli(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Michal Motyl <michalx.motyl(a)intel.com>
Gerrit-Comment-Date: Wed, 03 Mar 2021 19:40:37 +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: Bora Guvendik, Anil Kumar K, Furquan Shaikh, Wonkyu Kim, Angel Pons, Bernardo Perez Priego, Srinidhi N Kaushik, Patrick Rudolph.
Cliff Huang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50897 )
Change subject: soc/intel/tigerlake: Add CNVi Bluetooth flag at devicetree entry
......................................................................
Patch Set 9:
(1 comment)
Patchset:
PS9:
> Sorry Angel. I had missed that. […]
@Angel, Sorry. I think I miss the one you addressed earlier. Regarding of the value for CnviBtAudioOffload, I just saw the changed been merged to changed from enum type CnviBtAudioOffload to 'bool'. The CB:51154 was just merged this morning, and I will rebase to take the changes. Fortunately, the final code of this review does not use the value of it, so rebase will be sufficient.
--
To view, visit https://review.coreboot.org/c/coreboot/+/50897
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I71c512fe884060e23ee26e7334c575c4c517b78d
Gerrit-Change-Number: 50897
Gerrit-PatchSet: 9
Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Reviewer: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com>
Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com>
Gerrit-Reviewer: Shaunak Saha <shaunak.saha(a)intel.com>
Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: AndreX Andraos <andrex.andraos(a)intel.com>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-CC: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com>
Gerrit-Attention: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Wed, 03 Mar 2021 19:39:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Marc Jones, Jonathan Zhang, Johnny Lin, Christian Walter, Subrata Banik, Patrick Rudolph, Deomid "rojer" Ryabkov.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51230 )
Change subject: soc/intel/xeon_sp/cpx: Set the MRC "cold boot required" status bit
......................................................................
Patch Set 4:
(6 comments)
Patchset:
PS4:
> Let me take a look Jonathan on client side
Uh, FSP shouldn't be using a hardcoded offset from CMOS... If MRC only needs to know if a cold boot is required, why not pass this information through a FSP UPD?
However, if CPX-SP FSP needs to reboot the system and cannot hand control back to coreboot before doing so (I imagine this is the case after doing KTI/UPI initialisation), then I can understand the need to use a simple non-volatile storage from within FSP. If possible, I'd prefer if the offset in CMOS was configurable through a UPD.
In short: I don't think it's a good idea, and I would not want to have this on client FSP. However, I can understand why server FSP may need this. It could still be made more flexible, too.
File src/soc/intel/xeon_sp/cpx/chip.h:
https://review.coreboot.org/c/coreboot/+/51230/comment/8bdc0dd0_de77f97b
PS4, Line 107: #define CMOS_MRC_STATUS_ADDR 0x47
Let's make this more robust.
/*
* Address of the MRC status byte in CMOS. Should be reserved
* in mainboards' cmos.layout and not covered by checksum.
*/
#define CMOS_OFFSET_MRC_STATUS 0x47
#if CONFIG(USE_OPTION_TABLE)
#include "option_table.h"
#if CMOS_VSTART_mrc_status != CMOS_OFFSET_MRC_STATUS * 8
#error "CMOS start for CPX-SP MRC status byte is not correct, check your cmos.layout"
#endif
#if CMOS_VLEN_mrc_status != 8
#error "CMOS length for CPX-SP MRC status byte is not correct, check your cmos.layout"
#endif
#endif
What this pile of preprocessor does: if `USE_OPTION_TABLE` is selected, it verifies that an option named `mrc_status` is at offset 0x47 and covers one byte (original comment says "status byte"). If this is not the case, it raises an error at build-time, prompting the user to fix their board's cmos.layout file.
File src/soc/intel/xeon_sp/cpx/romstage.c:
https://review.coreboot.org/c/coreboot/+/51230/comment/abd3e459_74c24565
PS4, Line 128:
Since the CMOS offset is only used in this file, I'd move its definition right here. Rationale: locality of reference, but applied to humans.
https://review.coreboot.org/c/coreboot/+/51230/comment/8e552d21_dd82fdad
PS4, Line 132: uint8_t new_mrc_status = (mrc_status & 0xfe) | ((uint8_t) cold_boot_required);
Wouldn't it make more sense if cold_boot_required == 0? This way, if a machine loses RTC power, it would automatically retrain.
(I understand that changing this may require FSP changes)
https://review.coreboot.org/c/coreboot/+/51230/comment/af009231_22e5207b
PS4, Line 132: ((uint8_t)
cast should not be necessary
https://review.coreboot.org/c/coreboot/+/51230/comment/38a62e02_f9c7512f
PS4, Line 207: set_cmos_mrc_cold_boot_flag(cold_boot_required);
Simpler:
set_cmos_mrc_cold_boot_flag(!mupd->FspmArchUpd.NvsBufferPtr);
--
To view, visit https://review.coreboot.org/c/coreboot/+/51230
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9c4191d2fa2e0203b3464dcf40d845ede5f14c6b
Gerrit-Change-Number: 51230
Gerrit-PatchSet: 4
Gerrit-Owner: Deomid "rojer" Ryabkov <rojer9(a)fb.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Marc Jones <marc(a)marcjonesconsulting.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Attention: Marc Jones <marc(a)marcjonesconsulting.com>
Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Deomid "rojer" Ryabkov <rojer9(a)fb.com>
Gerrit-Comment-Date: Wed, 03 Mar 2021 19:37:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Jonathan Zhang <jonzhang(a)fb.com>
Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-MessageType: comment
Attention is currently required from: Marc Jones, Jonathan Zhang, Johnny Lin, Patrick Rudolph.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51227 )
Change subject: soc/intel/xeon_sp: Lockdown SPI BIOS controls
......................................................................
Patch Set 3:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/51227/comment/04dd0a29_d3af2b17
PS3, Line 7: Lockdown
Nit: Spelled with a space: Lock down
--
To view, visit https://review.coreboot.org/c/coreboot/+/51227
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6999b7ad17615b8390f6c7b3d0a874e58bccc481
Gerrit-Change-Number: 51227
Gerrit-PatchSet: 3
Gerrit-Owner: Marc Jones <marc(a)marcjonesconsulting.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: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Attention: Marc Jones <marc(a)marcjonesconsulting.com>
Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Wed, 03 Mar 2021 19:27:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Martin Roth, Chiranjeevi Rapolu, Tim Wawrzynczak, John Zhao, Duncan Laurie, Patrick Rudolph.
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 5:
(1 comment)
File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/51194/comment/0de1e234_c272a3ca
PS5, Line 270: *rbuf = NULL
> This is problematic. […]
going to add that patch to this patch chain
--
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: 5
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: 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: 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: Martin Roth <martinroth(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-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Wed, 03 Mar 2021 19:20:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Gerrit-MessageType: comment