Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/79985?usp=email )
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@felixheld.de Change-Id: Ic752635da5eaa9e333cfb927836f0d260d2ac049 Reviewed-on: https://review.coreboot.org/c/coreboot/+/79985 Reviewed-by: Marshall Dawson marshalldawson3rd@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Arthur Heymans arthur@aheymans.xyz --- 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(-)
Approvals: Arthur Heymans: Looks good to me, approved build bot (Jenkins): Verified Marshall Dawson: Looks good to me, but someone else must approve
diff --git a/src/soc/amd/genoa_poc/chip.c b/src/soc/amd/genoa_poc/chip.c index eb35a25..5f29428 100644 --- a/src/soc/amd/genoa_poc/chip.c +++ b/src/soc/amd/genoa_poc/chip.c @@ -3,10 +3,17 @@ #include <device/device.h> #include <soc/southbridge.h> #include <soc/acpi.h> +#include <soc/southbridge.h> +#include <vendorcode/amd/opensil/genoa_poc/opensil.h>
static void soc_init(void *chip_info) { default_dev_ops_root.write_acpi_tables = soc_acpi_write_tables; + + setup_opensil(); + opensil_entry(SIL_TP1); + + fch_init(chip_info); }
static void soc_final(void *chip_info) diff --git a/src/soc/amd/genoa_poc/fch.c b/src/soc/amd/genoa_poc/fch.c index c977957..f340d82 100644 --- a/src/soc/amd/genoa_poc/fch.c +++ b/src/soc/amd/genoa_poc/fch.c @@ -81,14 +81,8 @@ configure_smi(SMITYPE_SMI_CMD_PORT, SMI_MODE_SMI); }
-static void fch_init(void *unused) +void fch_init(void *chip_info) { set_pci_irqs(); fch_init_acpi_ports(); } - -/* - * Hook this function into the PCI state machine on entry into BS_DEV_ENABLE. - * TODO: can this be done without using BOOT_STATE_INIT_ENTRY? - */ -BOOT_STATE_INIT_ENTRY(BS_DEV_ENABLE, BS_ON_ENTRY, fch_init, NULL); diff --git a/src/soc/amd/genoa_poc/include/soc/southbridge.h b/src/soc/amd/genoa_poc/include/soc/southbridge.h index a761d53..1027a58 100644 --- a/src/soc/amd/genoa_poc/include/soc/southbridge.h +++ b/src/soc/amd/genoa_poc/include/soc/southbridge.h @@ -117,5 +117,6 @@
void fch_pre_init(void); void fch_early_init(void); +void fch_init(void *chip_info);
#endif /* AMD_GENOA_POC_SOUTHBRIDGE_H */ diff --git a/src/vendorcode/amd/opensil/genoa_poc/opensil.h b/src/vendorcode/amd/opensil/genoa_poc/opensil.h index fbf46b3..473238d 100644 --- a/src/vendorcode/amd/opensil/genoa_poc/opensil.h +++ b/src/vendorcode/amd/opensil/genoa_poc/opensil.h @@ -4,6 +4,7 @@ #define _OPENSIL_H_
#include <acpi/acpi.h> +#include <xSIM-api.h>
void SIL_STATUS_report(const char *function, const int status); // Add the memory map to dev, starting at index idx, returns last use idx @@ -13,4 +14,7 @@
void configure_mpio(void);
+void setup_opensil(void); +void opensil_entry(SIL_TIMEPOINT timepoint); + #endif diff --git a/src/vendorcode/amd/opensil/genoa_poc/ramstage.c b/src/vendorcode/amd/opensil/genoa_poc/ramstage.c index 7d03b32..11289bb 100644 --- a/src/vendorcode/amd/opensil/genoa_poc/ramstage.c +++ b/src/vendorcode/amd/opensil/genoa_poc/ramstage.c @@ -112,7 +112,7 @@ } }
-static void setup_opensil(void *unused) +void setup_opensil(void) { const SIL_STATUS debug_ret = SilDebugSetup(HostDebugService); SIL_STATUS_report("SilDebugSetup", debug_ret); @@ -129,9 +129,7 @@ configure_mpio(); }
-BOOT_STATE_INIT_ENTRY(BS_DEV_INIT_CHIPS, BS_ON_ENTRY, setup_opensil, NULL); - -static void opensil_entry(void *timepoint) +void opensil_entry(SIL_TIMEPOINT timepoint) { SIL_STATUS ret; SIL_TIMEPOINT tp = (uintptr_t)timepoint; @@ -162,7 +160,4 @@ } }
-/* TODO: look into calling these functions from some SoC device operations instead of using - * BOOT_STATE_INIT_ENTRY */ -BOOT_STATE_INIT_ENTRY(BS_DEV_INIT_CHIPS, BS_ON_EXIT, opensil_entry, (void *)SIL_TP1); -/* TODO add other timepoints later. Are they NOOP? */ +/* TODO: also call timepoints 2 and 3 from coreboot. Are they NOOP? */