Attention is currently required from: Marshall Dawson.
Hello Marshall Dawson,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/60196
to look at the new patch set (#2).
Change subject: soc/amd/stoneyridge/fch: add GNVS-related TODOs
......................................................................
soc/amd/stoneyridge/fch: add GNVS-related TODOs
The AOAC device states shouldn't be stored in GNVS, but be read from the
AOAC registers during runtime. Same for the EHCI controller's BAR0. The
location and size of the XHCI firmware can either be statically
determined at build-time or have coreboot generate ACPI objects that
contain the needed addresses. Since I can't easily test changes that
require booting to a desktop on Stoneyridge at the moment, only add
TODOs for now.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Suggested-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Change-Id: I3691b05606b9430cb60923780a6131993a9887d4
---
M src/soc/amd/stoneyridge/fch.c
1 file changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/60196/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/60196
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3691b05606b9430cb60923780a6131993a9887d4
Gerrit-Change-Number: 60196
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Marshall Dawson, Kyösti Mälkki.
Hello build bot (Jenkins), Marshall Dawson,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/60142
to look at the new patch set (#2).
Change subject: soc/amd/stoneyridge: split southbridge code
......................................................................
soc/amd/stoneyridge: split southbridge code
Split the southbridge code into a bootblock and a ramstage part to align
it more with Picasso and Cezanne. Also move the implementation of
fch_clk_output_48Mhz to the end of early_fch.c since it's not really
related to the functions that were previously around it.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: Ib660fbef8dc25ba0fab803ccd82b3408878d1588
---
M src/soc/amd/stoneyridge/Makefile.inc
A src/soc/amd/stoneyridge/early_fch.c
A src/soc/amd/stoneyridge/fch.c
D src/soc/amd/stoneyridge/southbridge.c
4 files changed, 373 insertions(+), 369 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/60142/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/60142
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib660fbef8dc25ba0fab803ccd82b3408878d1588
Gerrit-Change-Number: 60142
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Marshall Dawson.
Hello build bot (Jenkins), Marshall Dawson,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/60141
to look at the new patch set (#2).
Change subject: soc/amd/stoneyridge: factor out AGESA-wrapper related FCH functions
......................................................................
soc/amd/stoneyridge: factor out AGESA-wrapper related FCH functions
Split the code that gets called from the AGESA wrapper from the rest of
the FCH/southbridge code that directly interacts with the hardware.
Since the remaining parts of southbridge.c aren't used in romstage,
drop it from the list of build targets for romstage.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: I6197add0e1396a82545735653110e1e17bf9c303
---
M src/soc/amd/stoneyridge/Makefile.inc
A src/soc/amd/stoneyridge/fch_agesa.c
M src/soc/amd/stoneyridge/southbridge.c
3 files changed, 62 insertions(+), 57 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/60141/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/60141
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6197add0e1396a82545735653110e1e17bf9c303
Gerrit-Change-Number: 60141
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Marshall Dawson, Kyösti Mälkki.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60142 )
Change subject: soc/amd/stoneyridge: split southbridge code
......................................................................
Patch Set 1:
(1 comment)
File src/soc/amd/stoneyridge/fch.c:
https://review.coreboot.org/c/coreboot/+/60142/comment/0dfe6347_10938094
PS1, Line 175: if (gnvs) {
> There's probably no need to have them in GNVS or update on S3 resume path. […]
I agree that the AOAC and XHCI firmware address information don't need to be in GNVS and that it would be good to move them out of there. Since this patch is only about splitting the fch code in a bootblock and a ramstage part and i haven't gotten around to figure out what's exactly needed to get the stoneyridge device to boot to the linux desktop to verify that this sort of things still works, i'd add a patch on top of this patch train that adds a comment that this should eventually be done without using gnvs. see CB:60196
From the ACPI code, i'd assume that the XHCI controller needs to have it firmware reloaded when going from D3 to D0, but i'm not sure if that's a hardware requirement or just how things were done. Same problem here that i can't easily validate changes around this right now.
--
To view, visit https://review.coreboot.org/c/coreboot/+/60142
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib660fbef8dc25ba0fab803ccd82b3408878d1588
Gerrit-Change-Number: 60142
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Comment-Date: Fri, 17 Dec 2021 17:27:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-MessageType: comment
Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/60196 )
Change subject: soc/amd/stoneyridge/fch: add GNVS-related TODOs
......................................................................
soc/amd/stoneyridge/fch: add GNVS-related TODOs
The AOAC device states shouldn't be stored in GNVS, but be read from the
AOAC registers during runtime. Same for the EHCI controller's BAR0. The
location and size of the XHCI firmware can either be statically
determined at build-time or have coreboot generate ACPI objects that
contain the needed addresses. Since I can't easily test changes that
require booting to a desktop on Stoneyridge at the moment, only add
TODOs for now.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Suggested-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Change-Id: I3691b05606b9430cb60923780a6131993a9887d4
---
M src/soc/amd/stoneyridge/fch.c
1 file changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/60196/1
diff --git a/src/soc/amd/stoneyridge/fch.c b/src/soc/amd/stoneyridge/fch.c
index 16802ef..3037165 100644
--- a/src/soc/amd/stoneyridge/fch.c
+++ b/src/soc/amd/stoneyridge/fch.c
@@ -165,12 +165,14 @@
gnvs->fw02 = fwaddr + XHCI_FW_BOOTRAM_SIZE;
gnvs->fw03 = fwsize << 16;
+ /* TODO: This might break if the OS decides to re-allocate the PCI BARs. */
gnvs->eh10 = pci_read_config32(SOC_EHCI1_DEV, PCI_BASE_ADDRESS_0)
& ~PCI_BASE_ADDRESS_MEM_ATTR_MASK;
}
void fch_final(void *chip_info)
{
+ /* TODO: The AOAC states and EHCI/XHCI addresses should be moved out of GNVS */
struct global_nvs *gnvs = acpi_get_gnvs();
if (gnvs) {
set_sb_aoac(&gnvs->aoac);
--
To view, visit https://review.coreboot.org/c/coreboot/+/60196
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3691b05606b9430cb60923780a6131993a9887d4
Gerrit-Change-Number: 60196
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newchange
Attention is currently required from: Paul Menzel, Rizwan Qureshi, Sridhar Siricilla, Tim Wawrzynczak.
Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60195 )
Change subject: intel/common/block/cse: Move EOP early in boot sequence
......................................................................
Patch Set 1:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/60195/comment/c52e437f_dd8f9504
PS1, Line 10: reduced.This
> Missing space.
Done
https://review.coreboot.org/c/coreboot/+/60195/comment/49b5c54c_35c620b6
PS1, Line 14: timing requirement
> What is the timing requirement?
Done
https://review.coreboot.org/c/coreboot/+/60195/comment/0000242f_5da79200
PS1, Line 23: removing
> remove
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/60195
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9c7fe6f8f3fadb68310d4a09692f51f82c737c35
Gerrit-Change-Number: 60195
Gerrit-PatchSet: 1
Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)google.com>
Gerrit-Comment-Date: Fri, 17 Dec 2021 15:42:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: comment
Attention is currently required from: Maulik V Vaghela, Rizwan Qureshi, Sridhar Siricilla, Tim Wawrzynczak.
Hello build bot (Jenkins), Rizwan Qureshi, Sridhar Siricilla, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/60195
to look at the new patch set (#2).
Change subject: intel/common/block/cse: Move EOP early in boot sequence
......................................................................
intel/common/block/cse: Move EOP early in boot sequence
Earlier while trying to optimize boot time EOP time kept increasing when
boot time reduced. This was because CSE was busy.
When EOP was moved later in boot stage it again created issue since CSE
was busy with loading other payload, it delayed response to EOP command.
In order to avoid delayed response, coreboot has to send EOP in
stage when CSE is done with firmware init and it will be ready to
serve EOP as soon as possible. This also aligns with previous flow where
FSP used to send EOP once silicon init is done and coreboot used to
rely on FSP to send this message.
Moving EOP to BS_DEV_INIT boot state meets this requirement and CSE EOP
time reduces from ~60 ms to ~20 ms on Brya QS board.
Also remove commands to set CSE active/idle state since now EOP is
being sent before HECI disable.
BUG=b:211085685
BRANCH=None
TEST=Tested on Brya system before and after the changes. Observed ~40ms
savings in boot time.
Change-Id: I9c7fe6f8f3fadb68310d4a09692f51f82c737c35
Signed-off-by: MAULIK V VAGHELA <maulik.v.vaghela(a)intel.com>
---
M src/soc/intel/common/block/cse/cse_eop.c
1 file changed, 8 insertions(+), 15 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/60195/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/60195
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9c7fe6f8f3fadb68310d4a09692f51f82c737c35
Gerrit-Change-Number: 60195
Gerrit-PatchSet: 2
Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)google.com>
Gerrit-MessageType: newpatchset
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/60082 )
Change subject: mb/google/guybrush/var/nipperkin: config eSPI as dedicated alert
......................................................................
mb/google/guybrush/var/nipperkin: config eSPI as dedicated alert
Setup eSPI to dedicated alert per the latest schematic changes.
DUT won't hang up at power on boot due to eSPI alert is triggerred
unexpectedly.
BUG=b:199458949,b:203446084
BRANCH=guybrush
TEST=emerge-guybrush coreboot chromeos-bootimage
test power on/reboot on DUT (6 units) each 10 loops->pass
Change-Id: I55cda7a1af22e555a4f55285cb7e337a69e6c234
Signed-off-by: Kevin Chiu <kevin.chiu(a)quantatw.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/60082
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Rob Barnes <robbarnes(a)google.com>
---
M src/mainboard/google/guybrush/variants/nipperkin/overridetree.cb
1 file changed, 0 insertions(+), 2 deletions(-)
Approvals:
build bot (Jenkins): Verified
Rob Barnes: Looks good to me, approved
diff --git a/src/mainboard/google/guybrush/variants/nipperkin/overridetree.cb b/src/mainboard/google/guybrush/variants/nipperkin/overridetree.cb
index 2d3b162..19c6fe1 100644
--- a/src/mainboard/google/guybrush/variants/nipperkin/overridetree.cb
+++ b/src/mainboard/google/guybrush/variants/nipperkin/overridetree.cb
@@ -171,6 +171,4 @@
device generic 0.1 on end
end
- register "common_config.espi_config.alert_pin" = "ESPI_ALERT_PIN_IN_BAND"
-
end # chip soc/amd/cezanne
--
To view, visit https://review.coreboot.org/c/coreboot/+/60082
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I55cda7a1af22e555a4f55285cb7e337a69e6c234
Gerrit-Change-Number: 60082
Gerrit-PatchSet: 4
Gerrit-Owner: Kevin Chiu <kevin.chiu.17802(a)gmail.com>
Gerrit-Reviewer: Bhanu Prakash Maiya <bhanumaiya(a)google.com>
Gerrit-Reviewer: Eric Peers <epeers(a)google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Rob Barnes <robbarnes(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Kevin Chiu <Kevin.Chiu(a)quantatw.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: merged
Attention is currently required from: Maulik V Vaghela, Rizwan Qureshi, Sridhar Siricilla, Tim Wawrzynczak.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60195 )
Change subject: intel/common/block/cse: Move EOP early in boot sequence
......................................................................
Patch Set 1:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/60195/comment/99f5f4e8_09c70ff3
PS1, Line 10: reduced.This
Missing space.
https://review.coreboot.org/c/coreboot/+/60195/comment/a1fca81d_8fbd6244
PS1, Line 14: timing requirement
What is the timing requirement?
https://review.coreboot.org/c/coreboot/+/60195/comment/94b13111_0ed15ff3
PS1, Line 23: removing
remove
--
To view, visit https://review.coreboot.org/c/coreboot/+/60195
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9c7fe6f8f3fadb68310d4a09692f51f82c737c35
Gerrit-Change-Number: 60195
Gerrit-PatchSet: 1
Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)google.com>
Gerrit-Comment-Date: Fri, 17 Dec 2021 14:19:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment