Change in coreboot[master]: [TESTONLY] Revert "src/amd/common: Exclude biosram from psp_verstage"
Hello build bot (Jenkins), Raul Rangel, Martin Roth, Patrick Georgi, Julius Werner, Aaron Durbin,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/43067
to review the following change.
Change subject: [TESTONLY] Revert "src/amd/common: Exclude biosram from psp_verstage" ......................................................................
[TESTONLY] Revert "src/amd/common: Exclude biosram from psp_verstage"
This reverts commit f38af663d2c2c854859715803da249e6c24032db.
I just want to see where the build fails.
Change-Id: I0a995301404b67224be6addbeebf984c4b5c47d5 --- M src/soc/amd/common/block/acpimmio/Makefile.inc 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/43067/1
diff --git a/src/soc/amd/common/block/acpimmio/Makefile.inc b/src/soc/amd/common/block/acpimmio/Makefile.inc index 553d9e2..69253b9 100644 --- a/src/soc/amd/common/block/acpimmio/Makefile.inc +++ b/src/soc/amd/common/block/acpimmio/Makefile.inc @@ -6,7 +6,7 @@ smm-$(CONFIG_SOC_AMD_COMMON_BLOCK_ACPIMMIO) += mmio_util.c
bootblock-$(CONFIG_SOC_AMD_COMMON_BLOCK_ACPIMMIO) += biosram.c -verstage_x86-$(CONFIG_SOC_AMD_COMMON_BLOCK_ACPIMMIO) += biosram.c +verstage-$(CONFIG_SOC_AMD_COMMON_BLOCK_ACPIMMIO) += biosram.c romstage-$(CONFIG_SOC_AMD_COMMON_BLOCK_ACPIMMIO) += biosram.c postcar-$(CONFIG_SOC_AMD_COMMON_BLOCK_ACPIMMIO) += biosram.c ramstage-$(CONFIG_SOC_AMD_COMMON_BLOCK_ACPIMMIO) += biosram.c
Hello build bot (Jenkins), Raul Rangel, Martin Roth, Patrick Georgi, Julius Werner, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43067
to look at the new patch set (#2).
Change subject: Revert "src/amd/common: Exclude biosram from psp_verstage" ......................................................................
Revert "src/amd/common: Exclude biosram from psp_verstage"
This reverts commit f38af663d2c2c854859715803da249e6c24032db.
The build error was a spurious ENV_X86 guard in <cbmem.h> that called for a different clean up.
Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Change-Id: I0a995301404b67224be6addbeebf984c4b5c47d5 --- M src/soc/amd/common/block/acpimmio/Makefile.inc 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/43067/2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43067 )
Change subject: Revert "src/amd/common: Exclude biosram from psp_verstage" ......................................................................
Patch Set 2: Code-Review+2
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43067 )
Change subject: Revert "src/amd/common: Exclude biosram from psp_verstage" ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43067/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43067/2//COMMIT_MSG@11 PS2, Line 11: The build error was a spurious ENV_X86 guard in <cbmem.h> that : called for a different clean up. So which patch fixes this?
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43067 )
Change subject: Revert "src/amd/common: Exclude biosram from psp_verstage" ......................................................................
Patch Set 2: Code-Review+2
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43067 )
Change subject: Revert "src/amd/common: Exclude biosram from psp_verstage" ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43067/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43067/2//COMMIT_MSG@11 PS2, Line 11: The build error was a spurious ENV_X86 guard in <cbmem.h> that : called for a different clean up.
So which patch fixes this?
CB:43326
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43067 )
Change subject: Revert "src/amd/common: Exclude biosram from psp_verstage" ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43067/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43067/2//COMMIT_MSG@11 PS2, Line 11: The build error was a spurious ENV_X86 guard in <cbmem.h> that : called for a different clean up.
CB:43326
We can do something, since CB:43326 is good to go, add the commit hash here?
Kyösti Mälkki has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/43067 )
Change subject: Revert "src/amd/common: Exclude biosram from psp_verstage" ......................................................................
Abandoned
Kyösti Mälkki has restored this change. ( https://review.coreboot.org/c/coreboot/+/43067 )
Change subject: Revert "src/amd/common: Exclude biosram from psp_verstage" ......................................................................
Restored
Hello build bot (Jenkins), Raul Rangel, Martin Roth, Patrick Georgi, Angel Pons, Julius Werner, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43067
to look at the new patch set (#5).
Change subject: Revert "src/amd/common: Exclude biosram from psp_verstage" ......................................................................
Revert "src/amd/common: Exclude biosram from psp_verstage"
This reverts commit f38af663d2c2c854859715803da249e6c24032db.
The build error was a spurious ENV_X86 guard in <cbmem.h> that called for a different clean up.
Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Change-Id: I0a995301404b67224be6addbeebf984c4b5c47d5 --- M src/soc/amd/common/block/acpimmio/Makefile.inc 1 file changed, 1 insertion(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/43067/5
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43067 )
Change subject: Revert "src/amd/common: Exclude biosram from psp_verstage" ......................................................................
Patch Set 5:
the non-car SoCs (family 17h+) don't use the functions from biosram.c, so my plan was rather to add a Kconfig option that needs to be selected if a SoC needs those
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43067 )
Change subject: Revert "src/amd/common: Exclude biosram from psp_verstage" ......................................................................
Patch Set 5:
Patch Set 5:
the non-car SoCs (family 17h+) don't use the functions from biosram.c, so my plan was rather to add a Kconfig option that needs to be selected if a SoC needs those
We have garbage collection. I'd rather see you build as much of the common code as possible for ARCH_ARM; otherwise someone will just fork copies of them with minor modifications when need to use them on psp-verstage arises.
IMHO, when there is no real x86 dependency verstage_x86 should never be used.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43067 )
Change subject: Revert "src/amd/common: Exclude biosram from psp_verstage" ......................................................................
Patch Set 5: Code-Review+2
We have garbage collection. I'd rather see you build as much of the common code as possible for ARCH_ARM; otherwise someone will just fork copies of them with minor modifications when need to use them on psp-verstage arises.
point taken
IMHO, when there is no real x86 dependency verstage_x86 should never be used.
i wouldn't make a strict rule out of that, but yes, if it's not really really needed, it should be avoided if possible
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43067 )
Change subject: Revert "src/amd/common: Exclude biosram from psp_verstage" ......................................................................
Patch Set 5: Code-Review+2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43067 )
Change subject: Revert "src/amd/common: Exclude biosram from psp_verstage" ......................................................................
Patch Set 5: Code-Review+2
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43067 )
Change subject: Revert "src/amd/common: Exclude biosram from psp_verstage" ......................................................................
Revert "src/amd/common: Exclude biosram from psp_verstage"
This reverts commit f38af663d2c2c854859715803da249e6c24032db.
The build error was a spurious ENV_X86 guard in <cbmem.h> that called for a different clean up.
Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Change-Id: I0a995301404b67224be6addbeebf984c4b5c47d5 Reviewed-on: https://review.coreboot.org/c/coreboot/+/43067 Reviewed-by: Felix Held felix-coreboot@felixheld.de Reviewed-by: Raul Rangel rrangel@chromium.org Reviewed-by: Angel Pons th3fanbus@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/amd/common/block/acpimmio/Makefile.inc 1 file changed, 1 insertion(+), 5 deletions(-)
Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, approved Raul Rangel: Looks good to me, approved Angel Pons: Looks good to me, approved
diff --git a/src/soc/amd/common/block/acpimmio/Makefile.inc b/src/soc/amd/common/block/acpimmio/Makefile.inc index 8981231..6537d56 100644 --- a/src/soc/amd/common/block/acpimmio/Makefile.inc +++ b/src/soc/amd/common/block/acpimmio/Makefile.inc @@ -3,11 +3,7 @@ all-y += mmio_util.c smm-y += mmio_util.c
-bootblock-y += biosram.c -verstage_x86-y += biosram.c -romstage-y += biosram.c -postcar-y += biosram.c -ramstage-y += biosram.c +all-y += biosram.c smm-y += biosram.c
bootblock-y += print_reset_status.c
-
Angel Pons (Code Review) -
Felix Held (Code Review)
-
Kyösti Mälkki (Code Review)
-
Martin Roth (Code Review)
-
Raul Rangel (Code Review)