Change in coreboot[master]: cpu/amd/pi: Don't redefine XIP_ROM_SIZE

[Arthur Heymans (Code Review)]

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35825 )

Change subject: cpu/amd/pi: Don't redefine XIP_ROM_SIZE
......................................................................

cpu/amd/pi: Don't redefine XIP_ROM_SIZE

This is already done in cpu/amd/pi/Kconfig.

Change-Id: I548b9c9066d825c2f03749353b9990b2efddfd9c
Signed-off-by: Arthur Heymans 
---
M src/cpu/amd/pi/00630F01/Kconfig
M src/cpu/amd/pi/00660F01/Kconfig
M src/cpu/amd/pi/00730F01/Kconfig
3 files changed, 0 insertions(+), 12 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/25/35825/1

diff --git a/src/cpu/amd/pi/00630F01/Kconfig b/src/cpu/amd/pi/00630F01/Kconfig
index e5e27b5..37025ab 100644
--- a/src/cpu/amd/pi/00630F01/Kconfig
+++ b/src/cpu/amd/pi/00630F01/Kconfig
@@ -21,8 +21,4 @@
    int
    default 48

-config XIP_ROM_SIZE
-	hex
-	default 0x100000
-
 endif
diff --git a/src/cpu/amd/pi/00660F01/Kconfig b/src/cpu/amd/pi/00660F01/Kconfig
index 6470448..374672d 100644
--- a/src/cpu/amd/pi/00660F01/Kconfig
+++ b/src/cpu/amd/pi/00660F01/Kconfig
@@ -21,8 +21,4 @@
    int
    default 48

-config XIP_ROM_SIZE
-	hex
-	default 0x100000
-
 endif
diff --git a/src/cpu/amd/pi/00730F01/Kconfig b/src/cpu/amd/pi/00730F01/Kconfig
index 43abc80..7ba4943 100644
--- a/src/cpu/amd/pi/00730F01/Kconfig
+++ b/src/cpu/amd/pi/00730F01/Kconfig
@@ -23,8 +23,4 @@
    int
    default 40

-config XIP_ROM_SIZE
-	hex
-	default 0x100000
-
 endif

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35825 )

Change subject: cpu/amd/pi: Don't redefine XIP_ROM_SIZE
......................................................................

Patch Set 1: Code-Review+2

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35825 )

Change subject: cpu/amd/pi: Don't redefine XIP_ROM_SIZE
......................................................................

Patch Set 1:

I don't see XIP_ROM_SIZE referenced with AGESA or binaryPI anywhere, amd_initmmio() sets CACHE_ROM_SIZE as WRPROT early in romstage. Looks like you can remove them all and select NO_FIXED_XIP_ROM_SIZE.

Only non-AGESA AMD uses cpu/amd/car/ where you see CONFIG_XIP_ROM_SIZE.

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35825 )

Change subject: cpu/amd/pi: Don't redefine XIP_ROM_SIZE
......................................................................

Patch Set 1: Code-Review-1

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35825 )

Change subject: cpu/amd/pi: Don't redefine XIP_ROM_SIZE
......................................................................

Patch Set 1:
> Patch Set 1:
>
> I don't see XIP_ROM_SIZE referenced with AGESA or binaryPI anywhere, amd_initmmio() sets CACHE_ROM_SIZE as WRPROT early in romstage. Looks like you can remove them all and select NO_FIXED_XIP_ROM_SIZE.
>
> Only non-AGESA AMD uses cpu/amd/car/ where you see CONFIG_XIP_ROM_SIZE.

ack.

Change in coreboot[master]: cpu/amd/pi: Select NO_FIXED_XIP_ROM_SIZE

[Arthur Heymans (Code Review)]

Hello Kyösti Mälkki, Marshall Dawson, Richard Spiegel, build bot (Jenkins), Nico Huber, Michał Żygowski,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/35825

to look at the new patch set (#2).

Change subject: cpu/amd/pi: Select NO_FIXED_XIP_ROM_SIZE
......................................................................

cpu/amd/pi: Select NO_FIXED_XIP_ROM_SIZE

AGESA and binaryPI set the whole CACHE_ROM_SIZE to WRPROT during the
romstage and do not reference the CONFIG_XIP_ROM_SIZE symbol.

Change-Id: I548b9c9066d825c2f03749353b9990b2efddfd9c
Signed-off-by: Arthur Heymans 
---
M src/cpu/amd/pi/00630F01/Kconfig
M src/cpu/amd/pi/00660F01/Kconfig
M src/cpu/amd/pi/00730F01/Kconfig
M src/cpu/amd/pi/Kconfig
4 files changed, 1 insertion(+), 23 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/25/35825/2

Change in coreboot[master]: cpu/amd/pi: Select NO_FIXED_XIP_ROM_SIZE

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35825 )

Change subject: cpu/amd/pi: Select NO_FIXED_XIP_ROM_SIZE
......................................................................

Patch Set 2:

cpu/amd/agesa too?

Change in coreboot[master]: cpu/amd/pi: Select NO_FIXED_XIP_ROM_SIZE

[Arthur Heymans (Code Review)]

Hello Kyösti Mälkki, Marshall Dawson, Richard Spiegel, build bot (Jenkins), Nico Huber, Michał Żygowski,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/35825

to look at the new patch set (#3).

Change subject: cpu/amd/pi: Select NO_FIXED_XIP_ROM_SIZE
......................................................................

cpu/amd/pi: Select NO_FIXED_XIP_ROM_SIZE

AGESA and binaryPI set the whole CACHE_ROM_SIZE to WRPROT during the
romstage and do not reference the CONFIG_XIP_ROM_SIZE symbol.

Change-Id: I548b9c9066d825c2f03749353b9990b2efddfd9c
Signed-off-by: Arthur Heymans 
---
M src/cpu/amd/agesa/Kconfig
M src/cpu/amd/pi/00630F01/Kconfig
M src/cpu/amd/pi/00660F01/Kconfig
M src/cpu/amd/pi/00730F01/Kconfig
M src/cpu/amd/pi/Kconfig
5 files changed, 2 insertions(+), 34 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/25/35825/3

Change in coreboot[master]: cpu/amd/pi: Select NO_FIXED_XIP_ROM_SIZE

[Arthur Heymans (Code Review)]

Hello Kyösti Mälkki, Marshall Dawson, Richard Spiegel, build bot (Jenkins), Nico Huber, Michał Żygowski,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/35825

to look at the new patch set (#4).

Change subject: cpu/amd/pi: Select NO_FIXED_XIP_ROM_SIZE
......................................................................

cpu/amd/pi: Select NO_FIXED_XIP_ROM_SIZE

AGESA and binaryPI set the whole CACHE_ROM_SIZE to WRPROT during the
romstage and do not reference the CONFIG_XIP_ROM_SIZE symbol.

Change-Id: I548b9c9066d825c2f03749353b9990b2efddfd9c
Signed-off-by: Arthur Heymans 
---
M src/cpu/amd/agesa/Kconfig
M src/cpu/amd/agesa/family12/Kconfig
M src/cpu/amd/agesa/family14/Kconfig
M src/cpu/amd/agesa/family15tn/Kconfig
M src/cpu/amd/agesa/family16kb/Kconfig
M src/cpu/amd/pi/00630F01/Kconfig
M src/cpu/amd/pi/00660F01/Kconfig
M src/cpu/amd/pi/00730F01/Kconfig
M src/cpu/amd/pi/Kconfig
M src/cpu/amd/socket_F_1207/Kconfig
10 files changed, 2 insertions(+), 54 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/25/35825/4

Change in coreboot[master]: cpu/amd/pi: Select NO_FIXED_XIP_ROM_SIZE

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35825 )

Change subject: cpu/amd/pi: Select NO_FIXED_XIP_ROM_SIZE
......................................................................

Patch Set 4:

(2 comments)

https://review.coreboot.org/c/coreboot/+/35825/4//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/35825/4//COMMIT_MSG@7 
PS4, Line 7: cpu/amd/pi: Select NO_FIXED_XIP_ROM_SIZE
agesa,pi

https://review.coreboot.org/c/coreboot/+/35825/4/src/cpu/amd/socket_F_1207/K... 
File src/cpu/amd/socket_F_1207/Kconfig:

https://review.coreboot.org/c/coreboot/+/35825/4/src/cpu/amd/socket_F_1207/K... 
PS4, Line 25: config XIP_ROM_SIZE
This is amdfam10-15, keep.

Change in coreboot[master]: cpu/amd/{agesa,pi}: Select NO_FIXED_XIP_ROM_SIZE

[Arthur Heymans (Code Review)]

Hello Kyösti Mälkki, Marshall Dawson, Richard Spiegel, build bot (Jenkins), Nico Huber, Michał Żygowski,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/35825

to look at the new patch set (#5).

Change subject: cpu/amd/{agesa,pi}: Select NO_FIXED_XIP_ROM_SIZE
......................................................................

cpu/amd/{agesa,pi}: Select NO_FIXED_XIP_ROM_SIZE

AGESA and binaryPI set the whole CACHE_ROM_SIZE to WRPROT during the
romstage and do not reference the CONFIG_XIP_ROM_SIZE symbol.

Change-Id: I548b9c9066d825c2f03749353b9990b2efddfd9c
Signed-off-by: Arthur Heymans 
---
M src/cpu/amd/agesa/Kconfig
M src/cpu/amd/agesa/family12/Kconfig
M src/cpu/amd/agesa/family14/Kconfig
M src/cpu/amd/agesa/family15tn/Kconfig
M src/cpu/amd/agesa/family16kb/Kconfig
M src/cpu/amd/pi/00630F01/Kconfig
M src/cpu/amd/pi/00660F01/Kconfig
M src/cpu/amd/pi/00730F01/Kconfig
M src/cpu/amd/pi/Kconfig
9 files changed, 2 insertions(+), 50 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/25/35825/5

Change in coreboot[master]: cpu/amd/{agesa,pi}: Select NO_FIXED_XIP_ROM_SIZE

[Marshall Dawson (Code Review)]

Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35825 )

Change subject: cpu/amd/{agesa,pi}: Select NO_FIXED_XIP_ROM_SIZE
......................................................................

Patch Set 5: Code-Review+2

Change in coreboot[master]: cpu/amd/{agesa,pi}: Select NO_FIXED_XIP_ROM_SIZE

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35825 )

Change subject: cpu/amd/{agesa,pi}: Select NO_FIXED_XIP_ROM_SIZE
......................................................................

Patch Set 5:

(2 comments)

https://review.coreboot.org/c/coreboot/+/35825/4//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/35825/4//COMMIT_MSG@7 
PS4, Line 7: cpu/amd/pi: Select NO_FIXED_XIP_ROM_SIZE
> agesa,pi

Done

https://review.coreboot.org/c/coreboot/+/35825/4/src/cpu/amd/socket_F_1207/K... 
File src/cpu/amd/socket_F_1207/Kconfig:

https://review.coreboot.org/c/coreboot/+/35825/4/src/cpu/amd/socket_F_1207/K... 
PS4, Line 25: config XIP_ROM_SIZE
> This is amdfam10-15, keep.

Done

Change in coreboot[master]: cpu/amd/{agesa,pi}: Select NO_FIXED_XIP_ROM_SIZE

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35825 )

Change subject: cpu/amd/{agesa,pi}: Select NO_FIXED_XIP_ROM_SIZE
......................................................................

Patch Set 5: Code-Review+1

Change in coreboot[master]: cpu/amd/{agesa,pi}: Select NO_FIXED_XIP_ROM_SIZE

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35825 )

Change subject: cpu/amd/{agesa,pi}: Select NO_FIXED_XIP_ROM_SIZE
......................................................................

cpu/amd/{agesa,pi}: Select NO_FIXED_XIP_ROM_SIZE

AGESA and binaryPI set the whole CACHE_ROM_SIZE to WRPROT during the
romstage and do not reference the CONFIG_XIP_ROM_SIZE symbol.

Change-Id: I548b9c9066d825c2f03749353b9990b2efddfd9c
Signed-off-by: Arthur Heymans 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/35825
Tested-by: build bot (Jenkins) 
Reviewed-by: Marshall Dawson 
Reviewed-by: Paul Menzel 
---
M src/cpu/amd/agesa/Kconfig
M src/cpu/amd/agesa/family12/Kconfig
M src/cpu/amd/agesa/family14/Kconfig
M src/cpu/amd/agesa/family15tn/Kconfig
M src/cpu/amd/agesa/family16kb/Kconfig
M src/cpu/amd/pi/00630F01/Kconfig
M src/cpu/amd/pi/00660F01/Kconfig
M src/cpu/amd/pi/00730F01/Kconfig
M src/cpu/amd/pi/Kconfig
9 files changed, 2 insertions(+), 50 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Paul Menzel: Looks good to me, but someone else must approve
  Marshall Dawson: Looks good to me, approved

diff --git a/src/cpu/amd/agesa/Kconfig b/src/cpu/amd/agesa/Kconfig
index f21bf54..b1fde2d 100644
--- a/src/cpu/amd/agesa/Kconfig
+++ b/src/cpu/amd/agesa/Kconfig
@@ -28,20 +28,10 @@
    select LAPIC_MONOTONIC_TIMER
    select SPI_FLASH if HAVE_ACPI_RESUME
    select SMM_ASEG
+	select NO_FIXED_XIP_ROM_SIZE

if CPU_AMD_AGESA

-config XIP_ROM_SIZE
-	hex
-	default 0x100000
-	help
-	  Overwride the default write through caching size as 1M Bytes.
-	  On some AMD platforms, one socket supports 2 or more kinds of
-	  processor family, compiling several CPU families agesa code
-	  will increase the romstage size.
-	  In order to execute romstage in place on the flash ROM,
-	  more space is required to be set as write through caching.
-
 config UDELAY_LAPIC_FIXED_FSB
    int
    default 200
diff --git a/src/cpu/amd/agesa/family12/Kconfig b/src/cpu/amd/agesa/family12/Kconfig
index 4fc2ba07..0324d12 100644
--- a/src/cpu/amd/agesa/family12/Kconfig
+++ b/src/cpu/amd/agesa/family12/Kconfig
@@ -21,8 +21,4 @@
    int
    default 48

-config XIP_ROM_SIZE
-	hex
-	default 0x80000
-
 endif
diff --git a/src/cpu/amd/agesa/family14/Kconfig b/src/cpu/amd/agesa/family14/Kconfig
index adbe7fd..518235b 100644
--- a/src/cpu/amd/agesa/family14/Kconfig
+++ b/src/cpu/amd/agesa/family14/Kconfig
@@ -21,8 +21,4 @@
    int
    default 36

-config XIP_ROM_SIZE
-	hex
-	default 0x80000
-
 endif
diff --git a/src/cpu/amd/agesa/family15tn/Kconfig b/src/cpu/amd/agesa/family15tn/Kconfig
index fde1adf..93e9ab2 100644
--- a/src/cpu/amd/agesa/family15tn/Kconfig
+++ b/src/cpu/amd/agesa/family15tn/Kconfig
@@ -21,8 +21,4 @@
    int
    default 48

-config XIP_ROM_SIZE
-	hex
-	default 0x100000
-
 endif
diff --git a/src/cpu/amd/agesa/family16kb/Kconfig b/src/cpu/amd/agesa/family16kb/Kconfig
index 9fef94d..e87a2d4 100644
--- a/src/cpu/amd/agesa/family16kb/Kconfig
+++ b/src/cpu/amd/agesa/family16kb/Kconfig
@@ -21,10 +21,6 @@
    int
    default 40

-config XIP_ROM_SIZE
-	hex
-	default 0x100000
-
 config FORCE_AM1_SOCKET_SUPPORT
    bool
    default n
diff --git a/src/cpu/amd/pi/00630F01/Kconfig b/src/cpu/amd/pi/00630F01/Kconfig
index e5e27b5..37025ab 100644
--- a/src/cpu/amd/pi/00630F01/Kconfig
+++ b/src/cpu/amd/pi/00630F01/Kconfig
@@ -21,8 +21,4 @@
    int
    default 48

-config XIP_ROM_SIZE
-	hex
-	default 0x100000
-
 endif
diff --git a/src/cpu/amd/pi/00660F01/Kconfig b/src/cpu/amd/pi/00660F01/Kconfig
index 6470448..374672d 100644
--- a/src/cpu/amd/pi/00660F01/Kconfig
+++ b/src/cpu/amd/pi/00660F01/Kconfig
@@ -21,8 +21,4 @@
    int
    default 48

-config XIP_ROM_SIZE
-	hex
-	default 0x100000
-
 endif
diff --git a/src/cpu/amd/pi/00730F01/Kconfig b/src/cpu/amd/pi/00730F01/Kconfig
index 43abc80..7ba4943 100644
--- a/src/cpu/amd/pi/00730F01/Kconfig
+++ b/src/cpu/amd/pi/00730F01/Kconfig
@@ -23,8 +23,4 @@
    int
    default 40

-config XIP_ROM_SIZE
-	hex
-	default 0x100000
-
 endif
diff --git a/src/cpu/amd/pi/Kconfig b/src/cpu/amd/pi/Kconfig
index 973a086..b33302e 100644
--- a/src/cpu/amd/pi/Kconfig
+++ b/src/cpu/amd/pi/Kconfig
@@ -28,23 +28,13 @@
    select SPI_FLASH if HAVE_ACPI_RESUME
    select CAR_GLOBAL_MIGRATION if BINARYPI_LEGACY_WRAPPER
    select SMM_ASEG
+	select NO_FIXED_XIP_ROM_SIZE

if CPU_AMD_PI

config BINARYPI_LEGACY_WRAPPER
    def_bool n

-config XIP_ROM_SIZE
-	hex
-	default 0x100000
-	help
-	  Overwride the default write through caching size as 1M Bytes.
-	  On some AMD platforms, one socket supports 2 or more kinds of
-	  processor family, compiling several CPU families agesa code
-	  will increase the romstage size.
-	  In order to execute romstage in place on the flash ROM,
-	  more space is required to be set as write through caching.
-
 config UDELAY_LAPIC_FIXED_FSB
    int
    default 200