Change in ...coreboot[master]: mb/google/octopus: default DRAM_PART_NUM_IN_CBI to y

[Aaron Durbin (Code Review)]

Aaron Durbin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33370

Change subject: mb/google/octopus: default DRAM_PART_NUM_IN_CBI to y
......................................................................

mb/google/octopus: default DRAM_PART_NUM_IN_CBI to y

All new targets utilizing octopus mainboard support default
to using DRAM_PART_NUM_IN_CBI, including DRAM_PART_NUM_ALWAYS_IN_CBI.
This allows easier addition of new targets.

BUG=b:132668378
BRANCH=octopus

Change-Id: Idb136aa960260abe1657b16ded02a7dfb63c6849
Signed-off-by: Aaron Durbin 
---
M src/mainboard/google/octopus/Kconfig
1 file changed, 11 insertions(+), 9 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/33370/1

diff --git a/src/mainboard/google/octopus/Kconfig b/src/mainboard/google/octopus/Kconfig
index 48753ed..0ddfb3a 100644
--- a/src/mainboard/google/octopus/Kconfig
+++ b/src/mainboard/google/octopus/Kconfig
@@ -123,20 +123,22 @@

config DRAM_PART_NUM_IN_CBI
    bool
-	default y if BOARD_GOOGLE_PHASER
-	default y if BOARD_GOOGLE_MEEP
-	default y if BOARD_GOOGLE_AMPTON
-	default y if BOARD_GOOGLE_FLEEX
+	default y
+
+config DRAM_PART_NUM_NOT_ALWAYS_IN_CBI
+	bool
+	depends on DRAM_PART_NUM_IN_CBI
    default y if BOARD_GOOGLE_BOBBA
-	default y if BOARD_GOOGLE_CASTA
-	default y if BOARD_GOOGLE_BLOOG
+	default y if BOARD_GOOGLE_FLEEX
+	default y if BOARD_GOOGLE_MEEP
+	default y if BOARD_GOOGLE_OCTOPUS
+	default y if BOARD_GOOGLE_PHASER
+	default y if BOARD_GOOGLE_YORP

config DRAM_PART_NUM_ALWAYS_IN_CBI
    bool
    depends on DRAM_PART_NUM_IN_CBI
-	default y if BOARD_GOOGLE_AMPTON
-	default y if BOARD_GOOGLE_CASTA
-	default y if BOARD_GOOGLE_BLOOG
+	default y if !DRAM_PART_NUM_NOT_ALWAYS_IN_CBI

config DRAM_PART_IN_CBI_BOARD_ID_MIN
    int

[Kevin Chiu (Code Review)]

Kevin Chiu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33370 )

Change subject: mb/google/octopus: default DRAM_PART_NUM_IN_CBI to y
......................................................................

Patch Set 1: Code-Review+1

[Karthik Ramasubramanian (Code Review)]

Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33370 )

Change subject: mb/google/octopus: default DRAM_PART_NUM_IN_CBI to y
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/#/c/33370/1/src/mainboard/google/octopus/Kconfig 
File src/mainboard/google/octopus/Kconfig:

https://review.coreboot.org/#/c/33370/1/src/mainboard/google/octopus/Kconfig... 
PS1, Line 138: DRAM_PART_NUM_ALWAYS_IN_CBI
Just wondering if we can remove this config item altogether since we are checking only for NOT_ALWAYS_IN_CBI in the rest of the code and we have defined it.

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33370 )

Change subject: mb/google/octopus: default DRAM_PART_NUM_IN_CBI to y
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/#/c/33370/1/src/mainboard/google/octopus/Kconfig 
File src/mainboard/google/octopus/Kconfig:

https://review.coreboot.org/#/c/33370/1/src/mainboard/google/octopus/Kconfig... 
PS1, Line 138: DRAM_PART_NUM_ALWAYS_IN_CBI
> Just wondering if we can remove this config item altogether since we are checking only for NOT_ALWAY […]

We can. I was minimizing the churn/changes. I'll upload another CL that nukes it, and you can see if you like it.

[Aaron Durbin (Code Review)]

Hello Karthik Ramasubramanian, Marco Chen, Justin TerAvest, Kevin Chiu, build bot (Jenkins), Furquan Shaikh,

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

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

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

Change subject: mb/google/octopus: default DRAM_PART_NUM_IN_CBI to y
......................................................................

mb/google/octopus: default DRAM_PART_NUM_IN_CBI to y

All new targets utilizing octopus mainboard support default
to using DRAM_PART_NUM_IN_CBI, including DRAM_PART_NUM_ALWAYS_IN_CBI.
This allows easier addition of new targets.

BUG=b:132668378
BRANCH=octopus

Change-Id: Idb136aa960260abe1657b16ded02a7dfb63c6849
Signed-off-by: Aaron Durbin 
---
M src/mainboard/google/octopus/Kconfig
M src/mainboard/google/octopus/romstage.c
M src/mainboard/google/octopus/variants/baseboard/memory.c
3 files changed, 9 insertions(+), 25 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/33370/2

[Karthik Ramasubramanian (Code Review)]

Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33370 )

Change subject: mb/google/octopus: default DRAM_PART_NUM_IN_CBI to y
......................................................................

Patch Set 2: Code-Review+2

Change in ...coreboot[master]: mb/google/octopus: make new targets have DRAM part in CBI by default

[Aaron Durbin (Code Review)]

Hello Karthik Ramasubramanian, Marco Chen, Justin TerAvest, build bot (Jenkins), Kevin Chiu, Furquan Shaikh,

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

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

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

Change subject: mb/google/octopus: make new targets have DRAM part in CBI by default
......................................................................

mb/google/octopus: make new targets have DRAM part in CBI by default

All new targets utilizing octopus mainboard support default
to always using DRAM_PART_NUM_IN_CBI. This allows easier addition
of new targets.

BUG=b:132668378
BRANCH=octopus

Change-Id: Idb136aa960260abe1657b16ded02a7dfb63c6849
Signed-off-by: Aaron Durbin 
---
M src/mainboard/google/octopus/Kconfig
M src/mainboard/google/octopus/romstage.c
M src/mainboard/google/octopus/variants/baseboard/memory.c
3 files changed, 9 insertions(+), 25 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/33370/3

Change in ...coreboot[master]: mb/google/octopus: make new targets have DRAM part in CBI by default

[Justin TerAvest (Code Review)]

Justin TerAvest has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33370 )

Change subject: mb/google/octopus: make new targets have DRAM part in CBI by default
......................................................................

Patch Set 3: Code-Review+2

Change in ...coreboot[master]: mb/google/octopus: make new targets have DRAM part in CBI by default

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33370 )

Change subject: mb/google/octopus: make new targets have DRAM part in CBI by default
......................................................................

Patch Set 3: Code-Review+2

Change in ...coreboot[master]: mb/google/octopus: make new targets have DRAM part in CBI by default

[Kevin Chiu (Code Review)]

Kevin Chiu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33370 )

Change subject: mb/google/octopus: make new targets have DRAM part in CBI by default
......................................................................

Patch Set 3: Code-Review+2

Change in ...coreboot[master]: mb/google/octopus: make new targets have DRAM part in CBI by default

[Aaron Durbin (Code Review)]

Aaron Durbin has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/33370 )

Change subject: mb/google/octopus: make new targets have DRAM part in CBI by default
......................................................................

mb/google/octopus: make new targets have DRAM part in CBI by default

All new targets utilizing octopus mainboard support default
to always using DRAM_PART_NUM_IN_CBI. This allows easier addition
of new targets.

BUG=b:132668378
BRANCH=octopus

Change-Id: Idb136aa960260abe1657b16ded02a7dfb63c6849
Signed-off-by: Aaron Durbin 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/33370
Reviewed-by: Justin TerAvest 
Reviewed-by: Furquan Shaikh 
Reviewed-by: Kevin Chiu 
Reviewed-by: Karthik Ramasubramanian 
Tested-by: build bot (Jenkins) 
---
M src/mainboard/google/octopus/Kconfig
M src/mainboard/google/octopus/romstage.c
M src/mainboard/google/octopus/variants/baseboard/memory.c
3 files changed, 9 insertions(+), 25 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Furquan Shaikh: Looks good to me, approved
  Kevin Chiu: Looks good to me, approved
  Justin TerAvest: Looks good to me, approved
  Karthik Ramasubramanian: Looks good to me, approved

diff --git a/src/mainboard/google/octopus/Kconfig b/src/mainboard/google/octopus/Kconfig
index 48753ed..8ca9251 100644
--- a/src/mainboard/google/octopus/Kconfig
+++ b/src/mainboard/google/octopus/Kconfig
@@ -121,26 +121,18 @@
    int
    default 63 # GPE0_DW1_31 (GPIO_63)

-config DRAM_PART_NUM_IN_CBI
+config DRAM_PART_NUM_NOT_ALWAYS_IN_CBI
    bool
-	default y if BOARD_GOOGLE_PHASER
-	default y if BOARD_GOOGLE_MEEP
-	default y if BOARD_GOOGLE_AMPTON
-	default y if BOARD_GOOGLE_FLEEX
    default y if BOARD_GOOGLE_BOBBA
-	default y if BOARD_GOOGLE_CASTA
-	default y if BOARD_GOOGLE_BLOOG
-
-config DRAM_PART_NUM_ALWAYS_IN_CBI
-	bool
-	depends on DRAM_PART_NUM_IN_CBI
-	default y if BOARD_GOOGLE_AMPTON
-	default y if BOARD_GOOGLE_CASTA
-	default y if BOARD_GOOGLE_BLOOG
+	default y if BOARD_GOOGLE_FLEEX
+	default y if BOARD_GOOGLE_MEEP
+	default y if BOARD_GOOGLE_OCTOPUS
+	default y if BOARD_GOOGLE_PHASER
+	default y if BOARD_GOOGLE_YORP

config DRAM_PART_IN_CBI_BOARD_ID_MIN
    int
-	depends on DRAM_PART_NUM_IN_CBI && !DRAM_PART_NUM_ALWAYS_IN_CBI
+	depends on DRAM_PART_NUM_NOT_ALWAYS_IN_CBI
    default 255 if BOARD_GOOGLE_YORP
    default 2 if BOARD_GOOGLE_PHASER
    default 2 if BOARD_GOOGLE_FLEEX
diff --git a/src/mainboard/google/octopus/romstage.c b/src/mainboard/google/octopus/romstage.c
index ff0354d..d878d0d 100644
--- a/src/mainboard/google/octopus/romstage.c
+++ b/src/mainboard/google/octopus/romstage.c
@@ -37,12 +37,7 @@
    char part_num_store[DIMM_INFO_PART_NUMBER_SIZE];
    const char *part_num = NULL;

-	if (!CONFIG(DRAM_PART_NUM_IN_CBI)) {
-		save_dimm_info_by_sku_config();
-		return;
-	}
-
-	if (!CONFIG(DRAM_PART_NUM_ALWAYS_IN_CBI)) {
+	if (CONFIG(DRAM_PART_NUM_NOT_ALWAYS_IN_CBI)) {
    	/* Fall back on part numbers encoded in lp4cfg array. */
    	if ((int)board_id() < CONFIG_DRAM_PART_IN_CBI_BOARD_ID_MIN) {
    		save_dimm_info_by_sku_config();
diff --git a/src/mainboard/google/octopus/variants/baseboard/memory.c b/src/mainboard/google/octopus/variants/baseboard/memory.c
index fc7c87d..604295b 100644
--- a/src/mainboard/google/octopus/variants/baseboard/memory.c
+++ b/src/mainboard/google/octopus/variants/baseboard/memory.c
@@ -205,10 +205,7 @@

const struct lpddr4_cfg *__weak variant_lpddr4_config(void)
 {
-	if (!CONFIG(DRAM_PART_NUM_IN_CBI))
-		return &non_cbi_lp4cfg;
-
-	if (!CONFIG(DRAM_PART_NUM_ALWAYS_IN_CBI)) {
+	if (CONFIG(DRAM_PART_NUM_NOT_ALWAYS_IN_CBI)) {
    	/* Fall back non cbi memory config. */
    	if ((int)board_id() < CONFIG_DRAM_PART_IN_CBI_BOARD_ID_MIN)
    		return &non_cbi_lp4cfg;