Change in coreboot[master]: soc/intel/xeon_sp/cpx/Kconfig: Relocate 'select CACHE_MRC_SETTINGS'

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44548 )

Change subject: soc/intel/xeon_sp/cpx/Kconfig: Relocate 'select CACHE_MRC_SETTINGS'
......................................................................

soc/intel/xeon_sp/cpx/Kconfig: Relocate 'select CACHE_MRC_SETTINGS'

This will remove the warning:
 "src/soc/intel/xeon_sp/cpx/Kconfig:79:warning: config symbol 'CPU_BCLK_MHZ' uses select, but is not boolean or tristate"

Change-Id: I2cfaf347b638e3847caa167e7efda89e9202960a
Signed-off-by: Elyes HAOUAS 
---
M src/soc/intel/xeon_sp/cpx/Kconfig
1 file changed, 2 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/44548/1

diff --git a/src/soc/intel/xeon_sp/cpx/Kconfig b/src/soc/intel/xeon_sp/cpx/Kconfig
index dcbadf8..3d764fe 100644
--- a/src/soc/intel/xeon_sp/cpx/Kconfig
+++ b/src/soc/intel/xeon_sp/cpx/Kconfig
@@ -72,12 +72,12 @@
 config SOC_INTEL_COMMON_BLOCK_P2SB
    def_bool y

+select CACHE_MRC_SETTINGS
+
 config CPU_BCLK_MHZ
    int
    default 100

-select CACHE_MRC_SETTINGS
-
 # CPX-SP has 2 IMCs, 3 channels per IMC, 2 DIMMs per channel
 # Default value is set to one socket, full config.
 config DIMM_MAX

[Nico Huber (Code Review)]

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

Change subject: soc/intel/xeon_sp/cpx/Kconfig: Relocate 'select CACHE_MRC_SETTINGS'
......................................................................

Patch Set 1:

This would restore the original behavior, but that didn't look
intentional. I guess the `select CACHE_MRC_SETTINGS` belongs
to `config SOC_INTEL_COOPERLAKE_SP` (and `_BLOCK_P2SB` should
probably be selected there too).

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins), Jonathan Zhang, Patrick Rudolph,

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

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

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

Change subject: soc/intel/xeon_sp/cpx/Kconfig: Relocate 'select CACHE_MRC_SETTINGS'
......................................................................

soc/intel/xeon_sp/cpx/Kconfig: Relocate 'select CACHE_MRC_SETTINGS'

This will remove the warning:
 "src/soc/intel/xeon_sp/cpx/Kconfig:79:warning: config symbol 'CPU_BCLK_MHZ' uses select, but is not boolean or tristate"

Change-Id: I2cfaf347b638e3847caa167e7efda89e9202960a
Signed-off-by: Elyes HAOUAS 
---
M src/soc/intel/xeon_sp/cpx/Kconfig
1 file changed, 5 insertions(+), 5 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/44548/2

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44548 )

Change subject: soc/intel/xeon_sp/cpx/Kconfig: Relocate 'select CACHE_MRC_SETTINGS'
......................................................................

Patch Set 1:
> Patch Set 1:
>
> This would restore the original behavior, but that didn't look
> intentional. I guess the `select CACHE_MRC_SETTINGS` belongs
> to `config SOC_INTEL_COOPERLAKE_SP` (and `_BLOCK_P2SB` should
> probably be selected there too).

done.
Thx

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins), Jonathan Zhang, Patrick Rudolph,

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

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

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

Change subject: soc/intel/xeon_sp/cpx/Kconfig: Relocate 'select CACHE_MRC_SETTINGS'
......................................................................

soc/intel/xeon_sp/cpx/Kconfig: Relocate 'select CACHE_MRC_SETTINGS'

This will remove the warning:
 "src/soc/intel/xeon_sp/cpx/Kconfig:79:warning: config symbol 'CPU_BCLK_MHZ' uses select, but is not boolean or tristate"

Change-Id: I2cfaf347b638e3847caa167e7efda89e9202960a
Signed-off-by: Elyes HAOUAS 
---
M src/soc/intel/xeon_sp/Kconfig
M src/soc/intel/xeon_sp/cpx/Kconfig
2 files changed, 1 insertion(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/44548/3

[Benjamin Doron (Code Review)]

Benjamin Doron has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44548 )

Change subject: soc/intel/xeon_sp/cpx/Kconfig: Relocate 'select CACHE_MRC_SETTINGS'
......................................................................

Patch Set 3: Code-Review+1

[Nico Huber (Code Review)]

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

Change subject: soc/intel/xeon_sp/cpx/Kconfig: Relocate 'select CACHE_MRC_SETTINGS'
......................................................................

Patch Set 3: Code-Review+2

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44548 )

Change subject: soc/intel/xeon_sp/cpx/Kconfig: Relocate 'select CACHE_MRC_SETTINGS'
......................................................................

Patch Set 3: Code-Review+1

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44548 )

Change subject: soc/intel/xeon_sp/cpx/Kconfig: Relocate 'select CACHE_MRC_SETTINGS'
......................................................................

Patch Set 3: Code-Review+2

[Jonathan Zhang (Code Review)]

Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44548 )

Change subject: soc/intel/xeon_sp/cpx/Kconfig: Relocate 'select CACHE_MRC_SETTINGS'
......................................................................

Patch Set 3: Code-Review+2

Tested on Deltalake.

[Angel Pons (Code Review)]

Angel Pons has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44548 )

Change subject: soc/intel/xeon_sp/cpx/Kconfig: Relocate 'select CACHE_MRC_SETTINGS'
......................................................................

soc/intel/xeon_sp/cpx/Kconfig: Relocate 'select CACHE_MRC_SETTINGS'

This will remove the warning:
 "src/soc/intel/xeon_sp/cpx/Kconfig:79:warning: config symbol 'CPU_BCLK_MHZ' uses select, but is not boolean or tristate"

Change-Id: I2cfaf347b638e3847caa167e7efda89e9202960a
Signed-off-by: Elyes HAOUAS 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/44548
Tested-by: build bot (Jenkins) 
Reviewed-by: Benjamin Doron 
Reviewed-by: Nico Huber 
Reviewed-by: Patrick Rudolph 
Reviewed-by: Angel Pons 
Reviewed-by: Jonathan Zhang 
---
M src/soc/intel/xeon_sp/Kconfig
M src/soc/intel/xeon_sp/cpx/Kconfig
2 files changed, 1 insertion(+), 2 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Nico Huber: Looks good to me, approved
  Patrick Rudolph: Looks good to me, but someone else must approve
  Angel Pons: Looks good to me, approved
  Jonathan Zhang: Looks good to me, approved
  Benjamin Doron: Looks good to me, but someone else must approve

diff --git a/src/soc/intel/xeon_sp/Kconfig b/src/soc/intel/xeon_sp/Kconfig
index cf9ba94..545b423 100644
--- a/src/soc/intel/xeon_sp/Kconfig
+++ b/src/soc/intel/xeon_sp/Kconfig
@@ -17,6 +17,7 @@
    bool
    select XEON_SP_COMMON_BASE
    select PLATFORM_USES_FSP2_2
+	select CACHE_MRC_SETTINGS
    help
      Intel Cooperlake-SP support

diff --git a/src/soc/intel/xeon_sp/cpx/Kconfig b/src/soc/intel/xeon_sp/cpx/Kconfig
index dcbadf8..8e7e6f1 100644
--- a/src/soc/intel/xeon_sp/cpx/Kconfig
+++ b/src/soc/intel/xeon_sp/cpx/Kconfig
@@ -76,8 +76,6 @@
    int
    default 100

-select CACHE_MRC_SETTINGS
-
 # CPX-SP has 2 IMCs, 3 channels per IMC, 2 DIMMs per channel
 # Default value is set to one socket, full config.
 config DIMM_MAX

[9elements QA (Code Review)]

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44548 )

Change subject: soc/intel/xeon_sp/cpx/Kconfig: Relocate 'select CACHE_MRC_SETTINGS'
......................................................................

Patch Set 4:

Automatic boot test returned (PASS/FAIL/TOTAL): 7/1/8
"QEMU x86 q35/ich9" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/15909
"QEMU x86 q35/ich9" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/15908
"QEMU x86 i440fx/piix4" (x86_64) using payload SeaBIOS : FAIL : https://lava.9esec.io/r/15907
"QEMU x86 i440fx/piix4" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/15906
"QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/15905
"HP Z220 SFF Workstation" (x86_32) using payload LinuxBoot_BusyBox_kexec : SUCCESS : https://lava.9esec.io/r/15912
"HP Compaq 8200 Elite SFF PC" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/15911
"HP Compaq 8200 Elite SFF PC" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/15910

Please note: This test is under development and might not be accurate at all!