Change in ...coreboot[master]: nb/amd/pi: support for boards without BINARYPI_LEGACY_WRAPPER
Krystian Hebel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32421
Change subject: nb/amd/pi: support for boards without BINARYPI_LEGACY_WRAPPER ......................................................................
nb/amd/pi: support for boards without BINARYPI_LEGACY_WRAPPER
A stripped down version (without S3) of ../agesa/family*/state_machine.c is used to provide platform-specific hooks.
AmdGetValue() is required by cpu_bus_scan(), it is adapted from binaryPI wrapper to use new interface for dispatcher.
Tested only on 00730F01 (apu2) - no other platforms available.
Signed-off-by: Krystian Hebel krystian.hebel@3mdeb.com Change-Id: I700a7d8d3c77ee0525b2c764c720ab5bf39925f8 Signed-off-by: Krystian Hebel krystian.hebel@3mdeb.com --- M src/northbridge/amd/pi/00630F01/northbridge.c M src/northbridge/amd/pi/00660F01/northbridge.c M src/northbridge/amd/pi/00730F01/northbridge.c M src/northbridge/amd/pi/Makefile.inc A src/northbridge/amd/pi/state_machine.c 5 files changed, 164 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/32421/1
diff --git a/src/northbridge/amd/pi/00630F01/northbridge.c b/src/northbridge/amd/pi/00630F01/northbridge.c index eca2f0c..c64a7ba 100644 --- a/src/northbridge/amd/pi/00630F01/northbridge.c +++ b/src/northbridge/amd/pi/00630F01/northbridge.c @@ -2,6 +2,7 @@ * This file is part of the coreboot project. * * Copyright (C) 2012 Advanced Micro Devices, Inc. + * Copyright (C) 2019 3mdeb Embedded Systems Consulting * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -38,6 +39,8 @@ #if CONFIG(BINARYPI_LEGACY_WRAPPER) #include <northbridge/amd/pi/agesawrapper.h> #include <northbridge/amd/pi/agesawrapper_call.h> +#else +#include <northbridge/amd/agesa/state_machine.h> #endif
#define MAX_NODE_NUMS MAX_NODES @@ -1006,3 +1009,28 @@
return new_vendev; } + +#if !CONFIG_BINARYPI_LEGACY_WRAPPER +AGESA_STATUS +AmdGetValue(IN CONST AGESA_FIELD_NAME name, IN OUT VOID **value, IN UINT32 size) +{ + AGESA_STATUS status; + struct sysinfo cb; + AMD_ACCESSOR_PARAMS Params = {}; + + agesa_set_interface(&cb); + Params.StdHeader.ImageBasePtr = cb.StdHeader.ImageBasePtr; + + Params.AllocationMethod = ByHost; + Params.FieldName = name; + Params.FieldValue = *value; + Params.FieldSize = size; + + status = module_dispatch(AMD_GET_VALUE, (AMD_CONFIG_PARAMS*) &Params); + + *value = Params.FieldValue; + size = Params.FieldSize; + + return status; +} +#endif diff --git a/src/northbridge/amd/pi/00660F01/northbridge.c b/src/northbridge/amd/pi/00660F01/northbridge.c index 43df725..08dfd84 100644 --- a/src/northbridge/amd/pi/00660F01/northbridge.c +++ b/src/northbridge/amd/pi/00660F01/northbridge.c @@ -2,6 +2,7 @@ * This file is part of the coreboot project. * * Copyright (C) 2015 Advanced Micro Devices, Inc. + * Copyright (C) 2019 3mdeb Embedded Systems Consulting * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -38,6 +39,8 @@ #if CONFIG(BINARYPI_LEGACY_WRAPPER) #include <northbridge/amd/pi/agesawrapper.h> #include <northbridge/amd/pi/agesawrapper_call.h> +#else +#include <northbridge/amd/agesa/state_machine.h> #endif
#define MAX_NODE_NUMS MAX_NODES @@ -991,3 +994,28 @@
return new_vendev; } + +#if !CONFIG_BINARYPI_LEGACY_WRAPPER +AGESA_STATUS +AmdGetValue(IN CONST AGESA_FIELD_NAME name, IN OUT VOID **value, IN UINT32 size) +{ + AGESA_STATUS status; + struct sysinfo cb; + AMD_ACCESSOR_PARAMS Params = {}; + + agesa_set_interface(&cb); + Params.StdHeader.ImageBasePtr = cb.StdHeader.ImageBasePtr; + + Params.AllocationMethod = ByHost; + Params.FieldName = name; + Params.FieldValue = *value; + Params.FieldSize = size; + + status = module_dispatch(AMD_GET_VALUE, (AMD_CONFIG_PARAMS*) &Params); + + *value = Params.FieldValue; + size = Params.FieldSize; + + return status; +} +#endif diff --git a/src/northbridge/amd/pi/00730F01/northbridge.c b/src/northbridge/amd/pi/00730F01/northbridge.c index 926208c..f69f226 100644 --- a/src/northbridge/amd/pi/00730F01/northbridge.c +++ b/src/northbridge/amd/pi/00730F01/northbridge.c @@ -40,6 +40,8 @@ #if CONFIG(BINARYPI_LEGACY_WRAPPER) #include <northbridge/amd/pi/agesawrapper.h> #include <northbridge/amd/pi/agesawrapper_call.h> +#else +#include <northbridge/amd/agesa/state_machine.h> #endif
#define MAX_NODE_NUMS MAX_NODES @@ -1242,3 +1244,28 @@
return new_vendev; } + +#if !CONFIG_BINARYPI_LEGACY_WRAPPER +AGESA_STATUS +AmdGetValue(IN CONST AGESA_FIELD_NAME name, IN OUT VOID **value, IN UINT32 size) +{ + AGESA_STATUS status; + struct sysinfo cb; + AMD_ACCESSOR_PARAMS Params = {}; + + agesa_set_interface(&cb); + Params.StdHeader.ImageBasePtr = cb.StdHeader.ImageBasePtr; + + Params.AllocationMethod = ByHost; + Params.FieldName = name; + Params.FieldValue = *value; + Params.FieldSize = size; + + status = module_dispatch(AMD_GET_VALUE, (AMD_CONFIG_PARAMS*) &Params); + + *value = Params.FieldValue; + size = Params.FieldSize; + + return status; +} +#endif diff --git a/src/northbridge/amd/pi/Makefile.inc b/src/northbridge/amd/pi/Makefile.inc index c2c8d88..349761b 100644 --- a/src/northbridge/amd/pi/Makefile.inc +++ b/src/northbridge/amd/pi/Makefile.inc @@ -22,6 +22,9 @@ ifeq ($(CONFIG_BINARYPI_LEGACY_WRAPPER), y) romstage-y += agesawrapper.c ramstage-y += agesawrapper.c +else +romstage-y += state_machine.c +ramstage-y += state_machine.c endif
romstage-y += ramtop.c diff --git a/src/northbridge/amd/pi/state_machine.c b/src/northbridge/amd/pi/state_machine.c new file mode 100644 index 0000000..f42e97f --- /dev/null +++ b/src/northbridge/amd/pi/state_machine.c @@ -0,0 +1,78 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2016 Kyösti Mälkki + * Copyright (C) 2019 3mdeb Embedded Systems Consulting + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <Porting.h> +#include <AGESA.h> + +#include <cbmem.h> +#include <northbridge/amd/agesa/state_machine.h> +#include <northbridge/amd/agesa/agesa_helper.h> + +void platform_BeforeInitReset(struct sysinfo *cb, AMD_RESET_PARAMS *Reset) +{ +} + +void platform_BeforeInitEarly(struct sysinfo *cb, AMD_EARLY_PARAMS *Early) +{ +} + +void platform_BeforeInitPost(struct sysinfo *cb, AMD_POST_PARAMS *Post) +{ +} + +void platform_AfterInitPost(struct sysinfo *cb, AMD_POST_PARAMS *Post) +{ + backup_top_of_low_cacheable(Post->MemConfig.Sub4GCacheTop); +} + +void platform_BeforeInitResume(struct sysinfo *cb, AMD_RESUME_PARAMS *Resume) +{ +} + +void platform_AfterInitResume(struct sysinfo *cb, AMD_RESUME_PARAMS *Resume) +{ +} + +void platform_BeforeInitEnv(struct sysinfo *cb, AMD_ENV_PARAMS *Env) +{ + EmptyHeap(); +} + +void platform_AfterInitEnv(struct sysinfo *cb, AMD_ENV_PARAMS *Env) +{ +} + +void platform_BeforeS3LateRestore(struct sysinfo *cb, AMD_S3LATE_PARAMS *S3Late) +{ +} + +void platform_AfterS3LateRestore(struct sysinfo *cb, AMD_S3LATE_PARAMS *S3Late) +{ + amd_initcpuio(); +} + +void platform_BeforeInitMid(struct sysinfo *cb, AMD_MID_PARAMS *Mid) +{ + amd_initcpuio(); +} + +void platform_AfterInitLate(struct sysinfo *cb, AMD_LATE_PARAMS *Late) +{ +} + +void platform_AfterS3Save(struct sysinfo *cb, AMD_S3SAVE_PARAMS *S3Save) +{ +}
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32421 )
Change subject: nb/amd/pi: support for boards without BINARYPI_LEGACY_WRAPPER ......................................................................
Patch Set 1:
(6 comments)
https://review.coreboot.org/#/c/32421/1/src/northbridge/amd/pi/00630F01/nort... File src/northbridge/amd/pi/00630F01/northbridge.c:
https://review.coreboot.org/#/c/32421/1/src/northbridge/amd/pi/00630F01/nort... PS1, Line 1015: AmdGetValue(IN CONST AGESA_FIELD_NAME name, IN OUT VOID **value, IN UINT32 size) need consistent spacing around '*' (ctx:WxO)
https://review.coreboot.org/#/c/32421/1/src/northbridge/amd/pi/00630F01/nort... PS1, Line 1029: status = module_dispatch(AMD_GET_VALUE, (AMD_CONFIG_PARAMS*) &Params); "(foo*)" should be "(foo *)"
https://review.coreboot.org/#/c/32421/1/src/northbridge/amd/pi/00660F01/nort... File src/northbridge/amd/pi/00660F01/northbridge.c:
https://review.coreboot.org/#/c/32421/1/src/northbridge/amd/pi/00660F01/nort... PS1, Line 1000: AmdGetValue(IN CONST AGESA_FIELD_NAME name, IN OUT VOID **value, IN UINT32 size) need consistent spacing around '*' (ctx:WxO)
https://review.coreboot.org/#/c/32421/1/src/northbridge/amd/pi/00660F01/nort... PS1, Line 1014: status = module_dispatch(AMD_GET_VALUE, (AMD_CONFIG_PARAMS*) &Params); "(foo*)" should be "(foo *)"
https://review.coreboot.org/#/c/32421/1/src/northbridge/amd/pi/00730F01/nort... File src/northbridge/amd/pi/00730F01/northbridge.c:
https://review.coreboot.org/#/c/32421/1/src/northbridge/amd/pi/00730F01/nort... PS1, Line 1250: AmdGetValue(IN CONST AGESA_FIELD_NAME name, IN OUT VOID **value, IN UINT32 size) need consistent spacing around '*' (ctx:WxO)
https://review.coreboot.org/#/c/32421/1/src/northbridge/amd/pi/00730F01/nort... PS1, Line 1264: status = module_dispatch(AMD_GET_VALUE, (AMD_CONFIG_PARAMS*) &Params); "(foo*)" should be "(foo *)"
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32421 )
Change subject: nb/amd/pi: support for boards without BINARYPI_LEGACY_WRAPPER ......................................................................
Patch Set 1:
AmdGetValue() was invalid in the first place, it is not part of AGESA specification and was already removed during amd/stoneyridge reviews from that PI build.
That cpu_bus_scan() uses is just wrong and probably does not even do what it is supposed to.
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32421
to look at the new patch set (#2).
Change subject: nb/amd/pi: support for boards without BINARYPI_LEGACY_WRAPPER ......................................................................
nb/amd/pi: support for boards without BINARYPI_LEGACY_WRAPPER
A stripped down version (without S3) of ../agesa/family*/state_machine.c is used to provide platform-specific hooks.
AmdGetValue() is required by cpu_bus_scan(), it is adapted from binaryPI wrapper to use new interface for dispatcher.
Tested only on 00730F01 (apu2) - no other platforms available.
Signed-off-by: Krystian Hebel krystian.hebel@3mdeb.com Change-Id: I700a7d8d3c77ee0525b2c764c720ab5bf39925f8 Signed-off-by: Krystian Hebel krystian.hebel@3mdeb.com --- M src/northbridge/amd/pi/00630F01/northbridge.c M src/northbridge/amd/pi/00660F01/northbridge.c M src/northbridge/amd/pi/00730F01/northbridge.c M src/northbridge/amd/pi/Makefile.inc A src/northbridge/amd/pi/state_machine.c 5 files changed, 164 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/32421/2
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32421 )
Change subject: nb/amd/pi: support for boards without BINARYPI_LEGACY_WRAPPER ......................................................................
Patch Set 2:
(6 comments)
https://review.coreboot.org/c/coreboot/+/32421/2/src/northbridge/amd/pi/0063... File src/northbridge/amd/pi/00630F01/northbridge.c:
https://review.coreboot.org/c/coreboot/+/32421/2/src/northbridge/amd/pi/0063... PS2, Line 975: AmdGetValue(IN CONST AGESA_FIELD_NAME name, IN OUT VOID **value, IN UINT32 size) need consistent spacing around '*' (ctx:WxO)
https://review.coreboot.org/c/coreboot/+/32421/2/src/northbridge/amd/pi/0063... PS2, Line 989: status = module_dispatch(AMD_GET_VALUE, (AMD_CONFIG_PARAMS*) &Params); "(foo*)" should be "(foo *)"
https://review.coreboot.org/c/coreboot/+/32421/2/src/northbridge/amd/pi/0066... File src/northbridge/amd/pi/00660F01/northbridge.c:
https://review.coreboot.org/c/coreboot/+/32421/2/src/northbridge/amd/pi/0066... PS2, Line 962: AmdGetValue(IN CONST AGESA_FIELD_NAME name, IN OUT VOID **value, IN UINT32 size) need consistent spacing around '*' (ctx:WxO)
https://review.coreboot.org/c/coreboot/+/32421/2/src/northbridge/amd/pi/0066... PS2, Line 976: status = module_dispatch(AMD_GET_VALUE, (AMD_CONFIG_PARAMS*) &Params); "(foo*)" should be "(foo *)"
https://review.coreboot.org/c/coreboot/+/32421/2/src/northbridge/amd/pi/0073... File src/northbridge/amd/pi/00730F01/northbridge.c:
https://review.coreboot.org/c/coreboot/+/32421/2/src/northbridge/amd/pi/0073... PS2, Line 1210: AmdGetValue(IN CONST AGESA_FIELD_NAME name, IN OUT VOID **value, IN UINT32 size) need consistent spacing around '*' (ctx:WxO)
https://review.coreboot.org/c/coreboot/+/32421/2/src/northbridge/amd/pi/0073... PS2, Line 1224: status = module_dispatch(AMD_GET_VALUE, (AMD_CONFIG_PARAMS*) &Params); "(foo*)" should be "(foo *)"
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32421 )
Change subject: nb/amd/pi: support for boards without BINARYPI_LEGACY_WRAPPER ......................................................................
Patch Set 2:
Patch Set 1:
AmdGetValue() was invalid in the first place, it is not part of AGESA specification and was already removed during amd/stoneyridge reviews from that PI build.
That cpu_bus_scan() uses is just wrong and probably does not even do what it is supposed to.
Any suggestions on how to rewrite the cpu_bus_scan? What would be the best approach?
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32421 )
Change subject: nb/amd/pi: support for boards without BINARYPI_LEGACY_WRAPPER ......................................................................
Patch Set 3:
(6 comments)
https://review.coreboot.org/c/coreboot/+/32421/3/src/northbridge/amd/pi/0063... File src/northbridge/amd/pi/00630F01/northbridge.c:
https://review.coreboot.org/c/coreboot/+/32421/3/src/northbridge/amd/pi/0063... PS3, Line 975: AmdGetValue(IN CONST AGESA_FIELD_NAME name, IN OUT VOID **value, IN UINT32 size) need consistent spacing around '*' (ctx:WxO)
https://review.coreboot.org/c/coreboot/+/32421/3/src/northbridge/amd/pi/0063... PS3, Line 989: status = module_dispatch(AMD_GET_VALUE, (AMD_CONFIG_PARAMS*) &Params); "(foo*)" should be "(foo *)"
https://review.coreboot.org/c/coreboot/+/32421/3/src/northbridge/amd/pi/0066... File src/northbridge/amd/pi/00660F01/northbridge.c:
https://review.coreboot.org/c/coreboot/+/32421/3/src/northbridge/amd/pi/0066... PS3, Line 962: AmdGetValue(IN CONST AGESA_FIELD_NAME name, IN OUT VOID **value, IN UINT32 size) need consistent spacing around '*' (ctx:WxO)
https://review.coreboot.org/c/coreboot/+/32421/3/src/northbridge/amd/pi/0066... PS3, Line 976: status = module_dispatch(AMD_GET_VALUE, (AMD_CONFIG_PARAMS*) &Params); "(foo*)" should be "(foo *)"
https://review.coreboot.org/c/coreboot/+/32421/3/src/northbridge/amd/pi/0073... File src/northbridge/amd/pi/00730F01/northbridge.c:
https://review.coreboot.org/c/coreboot/+/32421/3/src/northbridge/amd/pi/0073... PS3, Line 1204: AmdGetValue(IN CONST AGESA_FIELD_NAME name, IN OUT VOID **value, IN UINT32 size) need consistent spacing around '*' (ctx:WxO)
https://review.coreboot.org/c/coreboot/+/32421/3/src/northbridge/amd/pi/0073... PS3, Line 1218: status = module_dispatch(AMD_GET_VALUE, (AMD_CONFIG_PARAMS*) &Params); "(foo*)" should be "(foo *)"
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32421
to look at the new patch set (#4).
Change subject: nb/amd/pi: support for boards without BINARYPI_LEGACY_WRAPPER ......................................................................
nb/amd/pi: support for boards without BINARYPI_LEGACY_WRAPPER
A stripped down version (without S3) of ../agesa/family*/state_machine.c is used to provide platform-specific hooks.
AmdGetValue() is required by cpu_bus_scan(), it is adapted from binaryPI wrapper to use new interface for dispatcher.
Tested only on 00730F01 (apu2) - no other platforms available.
Signed-off-by: Krystian Hebel krystian.hebel@3mdeb.com Change-Id: I700a7d8d3c77ee0525b2c764c720ab5bf39925f8 --- M src/northbridge/amd/pi/00630F01/northbridge.c M src/northbridge/amd/pi/00660F01/northbridge.c M src/northbridge/amd/pi/00730F01/northbridge.c M src/northbridge/amd/pi/Makefile.inc A src/northbridge/amd/pi/state_machine.c 5 files changed, 164 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/32421/4
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32421 )
Change subject: nb/amd/pi: support for boards without BINARYPI_LEGACY_WRAPPER ......................................................................
Patch Set 4:
(3 comments)
https://review.coreboot.org/c/coreboot/+/32421/4/src/northbridge/amd/pi/0063... File src/northbridge/amd/pi/00630F01/northbridge.c:
https://review.coreboot.org/c/coreboot/+/32421/4/src/northbridge/amd/pi/0063... PS4, Line 975: AmdGetValue(IN CONST AGESA_FIELD_NAME name, IN OUT VOID **value, IN UINT32 size) need consistent spacing around '*' (ctx:WxO)
https://review.coreboot.org/c/coreboot/+/32421/4/src/northbridge/amd/pi/0066... File src/northbridge/amd/pi/00660F01/northbridge.c:
https://review.coreboot.org/c/coreboot/+/32421/4/src/northbridge/amd/pi/0066... PS4, Line 962: AmdGetValue(IN CONST AGESA_FIELD_NAME name, IN OUT VOID **value, IN UINT32 size) need consistent spacing around '*' (ctx:WxO)
https://review.coreboot.org/c/coreboot/+/32421/4/src/northbridge/amd/pi/0073... File src/northbridge/amd/pi/00730F01/northbridge.c:
https://review.coreboot.org/c/coreboot/+/32421/4/src/northbridge/amd/pi/0073... PS4, Line 1204: AmdGetValue(IN CONST AGESA_FIELD_NAME name, IN OUT VOID **value, IN UINT32 size) need consistent spacing around '*' (ctx:WxO)
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32421 )
Change subject: nb/amd/pi: support for boards without BINARYPI_LEGACY_WRAPPER ......................................................................
Patch Set 4:
Patch Set 2:
Patch Set 1:
AmdGetValue() was invalid in the first place, it is not part of AGESA specification and was already removed during amd/stoneyridge reviews from that PI build.
That cpu_bus_scan() uses is just wrong and probably does not even do what it is supposed to.
Any suggestions on how to rewrite the cpu_bus_scan? What would be the best approach?
IIRC the code in cpu_bus_scan() was somewhat adapted from native Family 10h or 15h and intended for full flexibility on Opterons, so it wanted to look at each chip within an MCM, each processor in every neighboring socket, etc. However, Kaveri, Carrizo, and Mullins were all single-APU designs, and I don't recall any of these being an MCM, so the algorithm should be capable of being simplified quite a bit. The number of cores in CPUID Fn0000_0001 can imply the number of LAPICs.
It's been a while since I thought about it, but I believe we were able to remove this in ST by changing over to mp_init_with_smm(). It seems like each core updates the tree as it responds to the INIT/SIPI sequence.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32421 )
Change subject: nb/amd/pi: support for boards without BINARYPI_LEGACY_WRAPPER ......................................................................
Patch Set 4:
This opens up a can of worms and does not conform to AGESA API at all. Those AMD_CREATE_STRUCT calls inside AGESA are messy as hell to follow. We need something else. Could be just a constant or Kconfig to get over this here-and-now, as it is blocking CAR_GLOBAL removal now.
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32421 )
Change subject: nb/amd/pi: support for boards without BINARYPI_LEGACY_WRAPPER ......................................................................
Patch Set 4:
Looking at Marshall's comment, is CPUID-based CPU detection good enough to replace AmdGetValue Kyösti?
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32421 )
Change subject: nb/amd/pi: support for boards without BINARYPI_LEGACY_WRAPPER ......................................................................
Patch Set 4:
Michal, we have some green light to disable binaryPI mainboard builds temporarily, without yanking their sources out the tree just yet. I'll make the commit to explain the process.
Michał Żygowski has uploaded a new patch set (#5) to the change originally created by Krystian Hebel. ( https://review.coreboot.org/c/coreboot/+/32421 )
Change subject: nb/amd/pi: support for boards without BINARYPI_LEGACY_WRAPPER ......................................................................
nb/amd/pi: support for boards without BINARYPI_LEGACY_WRAPPER
A stripped down version (without S3) of ../agesa/family*/state_machine.c is used to provide platform-specific hooks.
TEST=boot PC Engines apu2 with POSTCAR_STAGE patch
Change-Id: I700a7d8d3c77ee0525b2c764c720ab5bf39925f8 Signed-off-by: Krystian Hebel krystian.hebel@3mdeb.com Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com --- M src/northbridge/amd/pi/00630F01/northbridge.c M src/northbridge/amd/pi/00660F01/northbridge.c M src/northbridge/amd/pi/00730F01/northbridge.c M src/northbridge/amd/pi/Makefile.inc A src/northbridge/amd/pi/state_machine.c 5 files changed, 89 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/32421/5
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32421 )
Change subject: nb/amd/pi: support for boards without BINARYPI_LEGACY_WRAPPER ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/32421/5/src/northbridge/amd/pi/stat... File src/northbridge/amd/pi/state_machine.c:
https://review.coreboot.org/c/coreboot/+/32421/5/src/northbridge/amd/pi/stat... PS5, Line 5: * Copyright (C) 2019 3mdeb Embedded Systems Consulting I don't think you changed the file at all? Line removals don't count. Anyways, these copyright lines are being yanked anyways.
This goes into per-family directory. There are some minor differences in AGESA headers and implementations why I want to do it that way.
Michał Żygowski has uploaded a new patch set (#6) to the change originally created by Krystian Hebel. ( https://review.coreboot.org/c/coreboot/+/32421 )
Change subject: nb/amd/pi: support for boards without BINARYPI_LEGACY_WRAPPER ......................................................................
nb/amd/pi: support for boards without BINARYPI_LEGACY_WRAPPER
A stripped down version (without S3) of ../agesa/family*/state_machine.c is used to provide platform-specific hooks.
TEST=boot PC Engines apu2 with POSTCAR_STAGE patch
Change-Id: I700a7d8d3c77ee0525b2c764c720ab5bf39925f8 Signed-off-by: Krystian Hebel krystian.hebel@3mdeb.com Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com --- M src/northbridge/amd/pi/00630F01/northbridge.c M src/northbridge/amd/pi/00660F01/northbridge.c M src/northbridge/amd/pi/00730F01/northbridge.c M src/northbridge/amd/pi/Makefile.inc A src/northbridge/amd/pi/state_machine.c 5 files changed, 85 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/32421/6
Kyösti Mälkki has uploaded a new patch set (#7) to the change originally created by Krystian Hebel. ( https://review.coreboot.org/c/coreboot/+/32421 )
Change subject: amd/pi/00730F01: Add support without BINARYPI_LEGACY_WRAPPER ......................................................................
amd/pi/00730F01: Add support without BINARYPI_LEGACY_WRAPPER
A stripped down version (without S3) of ../agesa/family*/state_machine.c is used to provide platform-specific hooks.
TEST=boot PC Engines apu2 with POSTCAR_STAGE patch
Change-Id: I700a7d8d3c77ee0525b2c764c720ab5bf39925f8 Signed-off-by: Krystian Hebel krystian.hebel@3mdeb.com Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com --- M src/northbridge/amd/pi/00730F01/Makefile.inc A src/northbridge/amd/pi/00730F01/state_machine.c 2 files changed, 91 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/32421/7
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32421 )
Change subject: amd/pi/00730F01: Add support without BINARYPI_LEGACY_WRAPPER ......................................................................
Patch Set 6:
I have got the branch done, pushing over patchset #6.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32421 )
Change subject: amd/pi/00730F01: Add support without BINARYPI_LEGACY_WRAPPER ......................................................................
Patch Set 7:
(1 comment)
Please set topic
https://review.coreboot.org/c/coreboot/+/32421/5/src/northbridge/amd/pi/stat... File src/northbridge/amd/pi/state_machine.c:
https://review.coreboot.org/c/coreboot/+/32421/5/src/northbridge/amd/pi/stat... PS5, Line 5: * Copyright (C) 2019 3mdeb Embedded Systems Consulting
I don't think you changed the file at all? Line removals don't count. […]
Done
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32421 )
Change subject: amd/pi/00730F01: Add support without BINARYPI_LEGACY_WRAPPER ......................................................................
Patch Set 7:
Patch Set 7:
(1 comment)
Please set topic
It is rejecting me when I try to change topic with: git push origin HEAD:refs/for/master%topic=binarypi-wrapper-killer
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32421 )
Change subject: amd/pi/00730F01: Add support without BINARYPI_LEGACY_WRAPPER ......................................................................
Patch Set 7:
Patch Set 7:
Patch Set 7:
(1 comment)
Please set topic
It is rejecting me when I try to change topic with: git push origin HEAD:refs/for/master%topic=binarypi-wrapper-killer
Right, might be that only owner (as identified in gerrit) is allowed to change it. Can you ping Krystian about this or should I ask a gerrit admin to do it?
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32421 )
Change subject: amd/pi/00730F01: Add support without BINARYPI_LEGACY_WRAPPER ......................................................................
Patch Set 7: Code-Review+2
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32421 )
Change subject: amd/pi/00730F01: Add support without BINARYPI_LEGACY_WRAPPER ......................................................................
Patch Set 8: Code-Review+2
Michal, the TEST= line is still valid I hope?
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32421 )
Change subject: amd/pi/00730F01: Add support without BINARYPI_LEGACY_WRAPPER ......................................................................
Patch Set 8:
Patch Set 8: Code-Review+2
Michal, the TEST= line is still valid I hope?
Tested on top of CB:37203, so it works perfectly
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/32421 )
Change subject: amd/pi/00730F01: Add support without BINARYPI_LEGACY_WRAPPER ......................................................................
amd/pi/00730F01: Add support without BINARYPI_LEGACY_WRAPPER
A stripped down version (without S3) of ../agesa/family*/state_machine.c is used to provide platform-specific hooks.
TEST=boot PC Engines apu2 with POSTCAR_STAGE patch
Change-Id: I700a7d8d3c77ee0525b2c764c720ab5bf39925f8 Signed-off-by: Krystian Hebel krystian.hebel@3mdeb.com Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/32421 Reviewed-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-by: Marshall Dawson marshalldawson3rd@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/northbridge/amd/pi/00730F01/Makefile.inc A src/northbridge/amd/pi/00730F01/state_machine.c 2 files changed, 91 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved Marshall Dawson: Looks good to me, approved
diff --git a/src/northbridge/amd/pi/00730F01/Makefile.inc b/src/northbridge/amd/pi/00730F01/Makefile.inc index 39c3ee6..94cf72e 100644 --- a/src/northbridge/amd/pi/00730F01/Makefile.inc +++ b/src/northbridge/amd/pi/00730F01/Makefile.inc @@ -17,3 +17,8 @@
ramstage-y += northbridge.c ramstage-y += iommu.c + +ifneq ($(CONFIG_BINARYPI_LEGACY_WRAPPER), y) +romstage-y += state_machine.c +ramstage-y += state_machine.c +endif diff --git a/src/northbridge/amd/pi/00730F01/state_machine.c b/src/northbridge/amd/pi/00730F01/state_machine.c new file mode 100644 index 0000000..b567f38 --- /dev/null +++ b/src/northbridge/amd/pi/00730F01/state_machine.c @@ -0,0 +1,86 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2016 Kyösti Mälkki + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include "Porting.h" +#include "AGESA.h" + +#include <cbmem.h> +#include <northbridge/amd/agesa/state_machine.h> +#include <northbridge/amd/agesa/agesa_helper.h> + +void platform_BeforeInitReset(struct sysinfo *cb, AMD_RESET_PARAMS *Reset) +{ +} + +void platform_BeforeInitEarly(struct sysinfo *cb, AMD_EARLY_PARAMS *Early) +{ +} + +void platform_BeforeInitPost(struct sysinfo *cb, AMD_POST_PARAMS *Post) +{ +} + +void platform_AfterInitPost(struct sysinfo *cb, AMD_POST_PARAMS *Post) +{ + /* If UMA is enabled we currently have it below TOP_MEM as well. + * UMA may or may not be cacheable, so Sub4GCacheTop could be + * higher than UmaBase. With UMA_NONE we see UmaBase==0. */ + if (Post->MemConfig.UmaBase) + backup_top_of_low_cacheable(Post->MemConfig.UmaBase << 16); + else + backup_top_of_low_cacheable(Post->MemConfig.Sub4GCacheTop); +} + + +void platform_BeforeInitEnv(struct sysinfo *cb, AMD_ENV_PARAMS *Env) +{ + EmptyHeap(); +} + +void platform_AfterInitEnv(struct sysinfo *cb, AMD_ENV_PARAMS *Env) +{ +} + +void platform_BeforeInitMid(struct sysinfo *cb, AMD_MID_PARAMS *Mid) +{ + amd_initcpuio(); +} + +void platform_AfterInitLate(struct sysinfo *cb, AMD_LATE_PARAMS *Late) +{ +} + + + +void platform_BeforeInitResume(struct sysinfo *cb, AMD_RESUME_PARAMS *Resume) +{ +} + +void platform_AfterInitResume(struct sysinfo *cb, AMD_RESUME_PARAMS *Resume) +{ +} + +void platform_BeforeS3LateRestore(struct sysinfo *cb, AMD_S3LATE_PARAMS *S3Late) +{ +} + +void platform_AfterS3LateRestore(struct sysinfo *cb, AMD_S3LATE_PARAMS *S3Late) +{ + amd_initcpuio(); +} + +void platform_AfterS3Save(struct sysinfo *cb, AMD_S3SAVE_PARAMS *S3Save) +{ +}
-
build bot (Jenkins) (Code Review) -
Krystian Hebel (Code Review)
-
Kyösti Mälkki (Code Review)
-
Marshall Dawson (Code Review)
-
Michał Żygowski (Code Review)
-
Patrick Georgi (Code Review)