Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79985?usp=email )
Change subject: soc/amd/genoa_poc: rely less on boot state hooks
......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS2:
> still needs to be tested on hardware
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/79985?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ic752635da5eaa9e333cfb927836f0d260d2ac049
Gerrit-Change-Number: 79985
Gerrit-PatchSet: 4
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: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Comment-Date: Wed, 24 Jan 2024 21:51:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: comment
Attention is currently required from: Marshall Dawson.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80148?usp=email )
Change subject: vc/amd/opensil/genoa_poc: move configure_mpio call to setup_opensil
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS1:
> *hardware
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/80148?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: If37077c879e266763fd2748a1a8d71c63c94729b
Gerrit-Change-Number: 80148
Gerrit-PatchSet: 3
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-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Comment-Date: Wed, 24 Jan 2024 21:51:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: comment
Attention is currently required from: Felix Held.
Hello Marshall Dawson, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/79985?usp=email
to look at the new patch set (#4).
Change subject: soc/amd/genoa_poc: rely less on boot state hooks
......................................................................
soc/amd/genoa_poc: rely less on boot state hooks
Call setup_opensil, opensil_entry, and fch_init in the right order from
the init method of the SoC's chip operations. This brings this SoC both
more in line with the other SoCs and avoids using boot state hooks for
this which also makes the sequence in which those functions are called
easier to understand. Previously the boot states were used so that
setup_opensil was run before configure_mpio which was run before
opensil_entry(SIL_TP1), but since configure_mpio is called from
setup_opensil, this is no longer necessary.
TEST=Onyx still boots to the payload and the MPIO configuration reported
from the openSIL code is still the same. The FCH init code now runs
before the resource allocation like on the AMD SoCs that rely on FSP.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: Ic752635da5eaa9e333cfb927836f0d260d2ac049
---
M src/soc/amd/genoa_poc/chip.c
M src/soc/amd/genoa_poc/fch.c
M src/soc/amd/genoa_poc/include/soc/southbridge.h
M src/vendorcode/amd/opensil/genoa_poc/opensil.h
M src/vendorcode/amd/opensil/genoa_poc/ramstage.c
5 files changed, 16 insertions(+), 15 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/79985/4
--
To view, visit https://review.coreboot.org/c/coreboot/+/79985?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ic752635da5eaa9e333cfb927836f0d260d2ac049
Gerrit-Change-Number: 79985
Gerrit-PatchSet: 4
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: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: Marshall Dawson.
Hello Marshall Dawson, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80148?usp=email
to look at the new patch set (#3).
Change subject: vc/amd/opensil/genoa_poc: move configure_mpio call to setup_opensil
......................................................................
vc/amd/opensil/genoa_poc: move configure_mpio call to setup_opensil
Instead of calling configure_mpio from the init function of the MPIO
chip struct for the first device that has this struct as chip_ops, call
if from setup_opensil. This will allow to do the calls into openSIL from
the SoC's chip_ops init function instead of having to rely on boot state
hooks. configure_mpio needs to be called after the xSimAssignMemoryTp1
call which sets up the openSIL data structures, but before the
opensil_entry(SIL_TP1) call for which the MPIO data structures need to
be filled for it to be able to initialize the hardware accordingly.
Since the vendorcode_amd_opensil_genoa_poc_mpio_ops struct now no longer
assigns configure_mpio to the init function pointer, we have to check
if the device's chip_ops pointer points to
vendorcode_amd_opensil_genoa_poc_mpio_ops instead of checking if the
chip_ops' init function is configure_mpio to match for the devices below
the MPIO chips in the devicetree.
TEST=Onyx still boots to the payload and the MPIO configuration reported
from the openSIL code is still the same
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: If37077c879e266763fd2748a1a8d71c63c94729b
---
M src/vendorcode/amd/opensil/genoa_poc/mpio/chip.c
M src/vendorcode/amd/opensil/genoa_poc/opensil.h
M src/vendorcode/amd/opensil/genoa_poc/ramstage.c
3 files changed, 10 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/80148/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/80148?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: If37077c879e266763fd2748a1a8d71c63c94729b
Gerrit-Change-Number: 80148
Gerrit-PatchSet: 3
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-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Marshall Dawson.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80148?usp=email )
Change subject: vc/amd/opensil/genoa_poc: move configure_mpio call to setup_opensil
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80148/comment/e1eaf96a_148e6a38 :
PS1, Line 13: call
> nit: called
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/80148?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: If37077c879e266763fd2748a1a8d71c63c94729b
Gerrit-Change-Number: 80148
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-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Comment-Date: Wed, 24 Jan 2024 21:38:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-MessageType: comment
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76512?usp=email )
Change subject: vendorcode/amd/opensil/genoa: Implement console callback
......................................................................
Patch Set 12:
(1 comment)
File src/vendorcode/amd/opensil/genoa_poc/opensil_console.c:
https://review.coreboot.org/c/coreboot/+/76512/comment/bb839c9c_e2e327c3 :
PS10, Line 34: char prefix[60];
: snprintf(prefix, sizeof(prefix), "%s%s:%d:", SilPrefix, (uintptr_t)Function, Line);
> Done
printk is the wrong function here; vprintk would have been the correct one. CB:80203 fixes that
--
To view, visit https://review.coreboot.org/c/coreboot/+/76512?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I208eea37ffde64a2311cb9f51e2bcd1ac3dbad4d
Gerrit-Change-Number: 76512
Gerrit-PatchSet: 12
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Comment-Date: Wed, 24 Jan 2024 20:15:12 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: comment
Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80203?usp=email )
Change subject: vc/amd/opensil/genoa_poc/opensil_console: fix host debug print function
......................................................................
vc/amd/opensil/genoa_poc/opensil_console: fix host debug print function
Since we pass va_list list to the print function, we need to use vprintk
instead of printk. Earlier versions of this code used vsnprintf and a
local buffer, but when that code was reworked to not need the temporary
buffer, it was replaced by printk instead of the correct vprintk.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: Ia931cc80dea5b7eabb75cfb19f8baa9a09cd2dbf
---
M src/vendorcode/amd/opensil/genoa_poc/opensil_console.c
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/80203/1
diff --git a/src/vendorcode/amd/opensil/genoa_poc/opensil_console.c b/src/vendorcode/amd/opensil/genoa_poc/opensil_console.c
index 55a3521..35e5eeb 100644
--- a/src/vendorcode/amd/opensil/genoa_poc/opensil_console.c
+++ b/src/vendorcode/amd/opensil/genoa_poc/opensil_console.c
@@ -39,6 +39,6 @@
/* print formatted message */
va_list args;
va_start(args, Line);
- printk(loglevel, Message, args);
+ vprintk(loglevel, Message, args);
va_end(args);
}
--
To view, visit https://review.coreboot.org/c/coreboot/+/80203?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ia931cc80dea5b7eabb75cfb19f8baa9a09cd2dbf
Gerrit-Change-Number: 80203
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newchange
Attention is currently required from: Marshall Dawson.
Hello Marshall Dawson, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80148?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Code-Review+1 by Marshall Dawson, Verified+1 by build bot (Jenkins)
Change subject: vc/amd/opensil/genoa_poc: move configure_mpio call to setup_opensil
......................................................................
vc/amd/opensil/genoa_poc: move configure_mpio call to setup_opensil
Instead of calling configure_mpio from the init function of the MPIO
chip struct for the first device that has this struct as chip_ops, call
if from setup_opensil. This will allow to do the calls into openSIL from
the SoC's chip_ops init function instead of having to rely on boot state
hooks. configure_mpio needs to be called after the xSimAssignMemoryTp1
call which sets up the openSIL data structures, but before the
opensil_entry(SIL_TP1) call for which the MPIO data structures need to
be filled for it to be able to initialize the hardware accordingly.
Since the vendorcode_amd_opensil_genoa_poc_mpio_ops struct now no longer
assigns configure_mpio to the init function pointer, we have to check
if the device's chip_ops pointer points to
vendorcode_amd_opensil_genoa_poc_mpio_ops instead of checking if the
chip_ops' init function is configure_mpio to match for the devices below
the MPIO chips in the devicetree.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: If37077c879e266763fd2748a1a8d71c63c94729b
---
M src/vendorcode/amd/opensil/genoa_poc/mpio/chip.c
M src/vendorcode/amd/opensil/genoa_poc/opensil.h
M src/vendorcode/amd/opensil/genoa_poc/ramstage.c
3 files changed, 10 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/80148/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/80148?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: If37077c879e266763fd2748a1a8d71c63c94729b
Gerrit-Change-Number: 80148
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-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Fred Reitberger, Jason Glenesk, Varshit Pandya.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80161?usp=email )
Change subject: soc/amd/common: Fix typo
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/80161?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ida6e87908ae6996529057c8df12dbe046ee54b98
Gerrit-Change-Number: 80161
Gerrit-PatchSet: 1
Gerrit-Owner: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Comment-Date: Wed, 24 Jan 2024 19:07:29 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment