Change in coreboot[master]: mb/google/dedede: Read DRAM population strap

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Change in ...coreboot[master]:...

Karthik Ramasubramanian (Code Review)

25 Mar 2020 25 Mar '20

4:40 a.m.

Karthik Ramasubramanian has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39812 )

Change subject: mb/google/dedede: Read DRAM population strap ......................................................................

mb/google/dedede: Read DRAM population strap

Read DRAM population strap and pass that information to FSP for memory initialization.

BUG=b:152275658 TEST=Build the mainboard.

Change-Id: I69583f35ffc219bae9ce06bd4ba9898ed0d4d21d Signed-off-by: Karthikeyan Ramasubramanian kramasub@google.com --- M src/mainboard/google/dedede/romstage.c M src/mainboard/google/dedede/variants/baseboard/include/baseboard/gpio.h 2 files changed, 7 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/39812/1

diff --git a/src/mainboard/google/dedede/romstage.c b/src/mainboard/google/dedede/romstage.c index 9c220d4..ca81785 100644 --- a/src/mainboard/google/dedede/romstage.c +++ b/src/mainboard/google/dedede/romstage.c @@ -6,8 +6,10 @@ */

#include <baseboard/variants.h> +#include <gpio.h> #include <soc/meminit_jsl.h> #include <soc/romstage.h> +#include <variant/gpio.h>

void mainboard_memory_init_params(FSPM_UPD *memupd) { @@ -16,7 +18,7 @@ .read_type = READ_SPD_CBFS, .spd_spec.spd_index = variant_memory_sku(), }; - /* TODO: Read the resistor strap to get number of memory segments. */ - bool half_populated = 0; + bool half_populated = gpio_get(GPIO_MEM_CH_SEL); + memcfg_init(&memupd->FspmConfig, board_cfg, &spd_info, half_populated); } diff --git a/src/mainboard/google/dedede/variants/baseboard/include/baseboard/gpio.h b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/gpio.h index dfb2cd1..fff0489 100644 --- a/src/mainboard/google/dedede/variants/baseboard/include/baseboard/gpio.h +++ b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/gpio.h @@ -26,4 +26,7 @@ #define GPIO_MEM_CONFIG_2 GPP_C4 #define GPIO_MEM_CONFIG_3 GPP_C5

+/* Memory channel select strap - 0:fully-populated, 1:half-populated */ +#define GPIO_MEM_CH_SEL GPP_S0 + #endif /* __BASEBOARD_GPIO_H__ */

Attachments:

attachment.htm (text/html — 4.2 KB)

Show replies by date

Karthik Ramasubramanian (Code Review)

25 Mar 25 Mar

4:50 a.m.

Hello build bot (Jenkins),

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

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

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

Change subject: mb/google/dedede: Read DRAM population strap ......................................................................

mb/google/dedede: Read DRAM population strap

Configure DRAM population strap GPIO. Read the strap and pass that information to FSP for memory initialization.

BUG=b:152275658 TEST=Build the mainboard.

Change-Id: I69583f35ffc219bae9ce06bd4ba9898ed0d4d21d Signed-off-by: Karthikeyan Ramasubramanian kramasub@google.com --- M src/mainboard/google/dedede/romstage.c M src/mainboard/google/dedede/variants/baseboard/gpio.c M src/mainboard/google/dedede/variants/baseboard/include/baseboard/gpio.h 3 files changed, 8 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/39812/2

Karthik Ramasubramanian (Code Review)

4:53 a.m.

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

Change subject: mb/google/dedede: Read DRAM population strap ......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39812/2//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/39812/2//COMMIT_MSG@13 PS2, Line 13: TEST=Build the mainboard. Yet to test on the board where the strap is not stuffed.

Furquan Shaikh (Code Review)

5:32 a.m.

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

Change subject: mb/google/dedede: Read DRAM population strap ......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39812/2/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/gpio.c:

https://review.coreboot.org/c/coreboot/+/39812/2/src/mainboard/google/dedede... PS2, Line 351: GPP_S0 This will have to be configured in early_gpio_table as well since it is being used in romstage.

-- To view, visit https://review.coreboot.org/c/coreboot/+/39812 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I69583f35ffc219bae9ce06bd4ba9898ed0d4d21d Gerrit-Change-Number: 39812 Gerrit-PatchSet: 2 Gerrit-Owner: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Marco Chen marcochen@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Wed, 25 Mar 2020 04:32:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Karthik Ramasubramanian (Code Review)

5:41 a.m.

Hello build bot (Jenkins), Furquan Shaikh, Marco Chen,

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

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

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

Change subject: mb/google/dedede: Read DRAM population strap ......................................................................

mb/google/dedede: Read DRAM population strap

Configure DRAM population strap GPIO. Read the strap and pass that information to FSP for memory initialization.

BUG=b:152275658 TEST=Build the mainboard.

Change-Id: I69583f35ffc219bae9ce06bd4ba9898ed0d4d21d Signed-off-by: Karthikeyan Ramasubramanian kramasub@google.com --- M src/mainboard/google/dedede/romstage.c M src/mainboard/google/dedede/variants/baseboard/gpio.c M src/mainboard/google/dedede/variants/baseboard/include/baseboard/gpio.h 3 files changed, 11 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/39812/3

Karthik Ramasubramanian (Code Review)

5:42 a.m.

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

Change subject: mb/google/dedede: Read DRAM population strap ......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39812/2/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/gpio.c:

https://review.coreboot.org/c/coreboot/+/39812/2/src/mainboard/google/dedede... PS2, Line 351: GPP_S0

...

This will have to be configured in early_gpio_table as well since it is being used in romstage.

True, thanks for pointing out.

-- To view, visit https://review.coreboot.org/c/coreboot/+/39812 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I69583f35ffc219bae9ce06bd4ba9898ed0d4d21d Gerrit-Change-Number: 39812 Gerrit-PatchSet: 3 Gerrit-Owner: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Marco Chen marcochen@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Wed, 25 Mar 2020 04:42:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh furquan@google.com Gerrit-MessageType: comment

Furquan Shaikh (Code Review)

6:43 a.m.

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

Change subject: mb/google/dedede: Read DRAM population strap ......................................................................

Patch Set 3: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/39812 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I69583f35ffc219bae9ce06bd4ba9898ed0d4d21d Gerrit-Change-Number: 39812 Gerrit-PatchSet: 3 Gerrit-Owner: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Marco Chen marcochen@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Wed, 25 Mar 2020 05:43:11 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Dtrain Hsu (Code Review)

11:39 a.m.

Dtrain Hsu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39812 )

Change subject: mb/google/dedede: Read DRAM population strap ......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39812/3/src/mainboard/google/dedede... File src/mainboard/google/dedede/romstage.c:

https://review.coreboot.org/c/coreboot/+/39812/3/src/mainboard/google/dedede... PS3, Line 21: bool half_populated = gpio_get(GPIO_MEM_CH_SEL); A question for checking GPIO_MEM_CH_SEL behavior. High is for dual-channel and low for single-channel from schematic. Is it correct?

-- To view, visit https://review.coreboot.org/c/coreboot/+/39812 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I69583f35ffc219bae9ce06bd4ba9898ed0d4d21d Gerrit-Change-Number: 39812 Gerrit-PatchSet: 3 Gerrit-Owner: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Marco Chen marcochen@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Dtrain Hsu dtrain_hsu@compal.corp-partner.google.com Gerrit-Comment-Date: Wed, 25 Mar 2020 10:39:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Paul Menzel (Code Review)

7:18 p.m.

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

Change subject: mb/google/dedede: Read DRAM population strap ......................................................................

Patch Set 3:

(2 comments)

https://review.coreboot.org/c/coreboot/+/39812/3//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/39812/3//COMMIT_MSG@9 PS3, Line 9: Configure DRAM population strap GPIO. Read the strap and pass that … according to the schematics.

https://review.coreboot.org/c/coreboot/+/39812/3/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/include/baseboard/gpio.h:

https://review.coreboot.org/c/coreboot/+/39812/3/src/mainboard/google/dedede... PS3, Line 29: /* Memory channel select strap - 0:fully-populated, 1:half-populated */ Add one space after each colon?

-- To view, visit https://review.coreboot.org/c/coreboot/+/39812 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I69583f35ffc219bae9ce06bd4ba9898ed0d4d21d Gerrit-Change-Number: 39812 Gerrit-PatchSet: 3 Gerrit-Owner: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Marco Chen marcochen@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Dtrain Hsu dtrain_hsu@compal.corp-partner.google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 25 Mar 2020 18:18:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Karthik Ramasubramanian (Code Review)

26 Mar 26 Mar

12:34 a.m.

Hello build bot (Jenkins), Furquan Shaikh, Marco Chen,

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

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

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

Change subject: mb/google/dedede: Read DRAM population strap ......................................................................

mb/google/dedede: Read DRAM population strap

Configure DRAM population strap GPIO according to the schematics. Read the strap and pass that information to FSP for memory initialization.

BUG=b:152275658 TEST=Build the mainboard.

Change-Id: I69583f35ffc219bae9ce06bd4ba9898ed0d4d21d Signed-off-by: Karthikeyan Ramasubramanian kramasub@google.com --- M src/mainboard/google/dedede/romstage.c M src/mainboard/google/dedede/variants/baseboard/gpio.c M src/mainboard/google/dedede/variants/baseboard/include/baseboard/gpio.h 3 files changed, 11 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/39812/4

-- To view, visit https://review.coreboot.org/c/coreboot/+/39812 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I69583f35ffc219bae9ce06bd4ba9898ed0d4d21d Gerrit-Change-Number: 39812 Gerrit-PatchSet: 4 Gerrit-Owner: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Marco Chen marcochen@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Dtrain Hsu dtrain_hsu@compal.corp-partner.google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Karthik Ramasubramanian (Code Review)

12:35 a.m.

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

Change subject: mb/google/dedede: Read DRAM population strap ......................................................................

Patch Set 4:

(3 comments)

https://review.coreboot.org/c/coreboot/+/39812/3//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/39812/3//COMMIT_MSG@9 PS3, Line 9: Configure DRAM population strap GPIO. Read the strap and pass that

...

… according to the schematics.

Done

https://review.coreboot.org/c/coreboot/+/39812/3/src/mainboard/google/dedede... File src/mainboard/google/dedede/romstage.c:

https://review.coreboot.org/c/coreboot/+/39812/3/src/mainboard/google/dedede... PS3, Line 21: bool half_populated = gpio_get(GPIO_MEM_CH_SEL);

...

A question for checking GPIO_MEM_CH_SEL behavior. […]

Thanks for pointing it out.

https://review.coreboot.org/c/coreboot/+/39812/3/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/include/baseboard/gpio.h:

https://review.coreboot.org/c/coreboot/+/39812/3/src/mainboard/google/dedede... PS3, Line 29: /* Memory channel select strap - 0:fully-populated, 1:half-populated */

...

Add one space after each colon?

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/39812 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I69583f35ffc219bae9ce06bd4ba9898ed0d4d21d Gerrit-Change-Number: 39812 Gerrit-PatchSet: 4 Gerrit-Owner: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Marco Chen marcochen@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Dtrain Hsu dtrain_hsu@compal.corp-partner.google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 25 Mar 2020 23:35:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Dtrain Hsu dtrain_hsu@compal.corp-partner.google.com Comment-In-Reply-To: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: comment

Furquan Shaikh (Code Review)

12:37 a.m.

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

Change subject: mb/google/dedede: Read DRAM population strap ......................................................................

Patch Set 4: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/39812 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I69583f35ffc219bae9ce06bd4ba9898ed0d4d21d Gerrit-Change-Number: 39812 Gerrit-PatchSet: 4 Gerrit-Owner: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Marco Chen marcochen@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Dtrain Hsu dtrain_hsu@compal.corp-partner.google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 25 Mar 2020 23:37:35 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Marco Chen (Code Review)

12:38 a.m.

Marco Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39812 )

Change subject: mb/google/dedede: Read DRAM population strap ......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39812/4/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/include/baseboard/gpio.h:

https://review.coreboot.org/c/coreboot/+/39812/4/src/mainboard/google/dedede... PS4, Line 29: /* Memory channel select strap - 0: fully-populated, 1: half-populated */ 0: half-populated, 1: fully-populated ?

-- To view, visit https://review.coreboot.org/c/coreboot/+/39812 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I69583f35ffc219bae9ce06bd4ba9898ed0d4d21d Gerrit-Change-Number: 39812 Gerrit-PatchSet: 4 Gerrit-Owner: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Marco Chen marcochen@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Dtrain Hsu dtrain_hsu@compal.corp-partner.google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 25 Mar 2020 23:38:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Karthik Ramasubramanian (Code Review)

12:54 a.m.

Hello build bot (Jenkins), Furquan Shaikh, Marco Chen,

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

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

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

Change subject: mb/google/dedede: Read DRAM population strap ......................................................................

mb/google/dedede: Read DRAM population strap

Configure DRAM population strap GPIO according to the schematics. Read the strap and pass that information to FSP for memory initialization.

BUG=b:152275658 TEST=Build the mainboard.

Change-Id: I69583f35ffc219bae9ce06bd4ba9898ed0d4d21d Signed-off-by: Karthikeyan Ramasubramanian kramasub@google.com --- M src/mainboard/google/dedede/romstage.c M src/mainboard/google/dedede/variants/baseboard/gpio.c M src/mainboard/google/dedede/variants/baseboard/include/baseboard/gpio.h 3 files changed, 11 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/39812/5

-- To view, visit https://review.coreboot.org/c/coreboot/+/39812 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I69583f35ffc219bae9ce06bd4ba9898ed0d4d21d Gerrit-Change-Number: 39812 Gerrit-PatchSet: 5 Gerrit-Owner: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Marco Chen marcochen@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Dtrain Hsu dtrain_hsu@compal.corp-partner.google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Karthik Ramasubramanian (Code Review)

12:54 a.m.

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

Change subject: mb/google/dedede: Read DRAM population strap ......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39812/4/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/include/baseboard/gpio.h:

https://review.coreboot.org/c/coreboot/+/39812/4/src/mainboard/google/dedede... PS4, Line 29: /* Memory channel select strap - 0: fully-populated, 1: half-populated */

...

0: half-populated, 1: fully-populated ?

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/39812 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I69583f35ffc219bae9ce06bd4ba9898ed0d4d21d Gerrit-Change-Number: 39812 Gerrit-PatchSet: 5 Gerrit-Owner: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Marco Chen marcochen@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Dtrain Hsu dtrain_hsu@compal.corp-partner.google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 25 Mar 2020 23:54:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Marco Chen marcochen@google.com Gerrit-MessageType: comment

Marco Chen (Code Review)

12:55 a.m.

Marco Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39812 )

Change subject: mb/google/dedede: Read DRAM population strap ......................................................................

Patch Set 5: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/39812 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I69583f35ffc219bae9ce06bd4ba9898ed0d4d21d Gerrit-Change-Number: 39812 Gerrit-PatchSet: 5 Gerrit-Owner: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Marco Chen marcochen@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Dtrain Hsu dtrain_hsu@compal.corp-partner.google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 25 Mar 2020 23:55:55 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Furquan Shaikh (Code Review)

12:59 a.m.

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

Change subject: mb/google/dedede: Read DRAM population strap ......................................................................

Patch Set 5: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/39812 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I69583f35ffc219bae9ce06bd4ba9898ed0d4d21d Gerrit-Change-Number: 39812 Gerrit-PatchSet: 5 Gerrit-Owner: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Dtrain Hsu dtrain_hsu@compal.corp-partner.google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Marco Chen marcochen@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 25 Mar 2020 23:59:53 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Karthik Ramasubramanian (Code Review)

17 Apr 17 Apr

6:21 p.m.

Hello build bot (Jenkins), Furquan Shaikh, Dtrain Hsu, Marco Chen,

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

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

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

Change subject: mb/google/dedede: Read DRAM population strap ......................................................................

mb/google/dedede: Read DRAM population strap

Configure DRAM population strap GPIO according to the schematics. Configure an internal pull-up to support the boards in which the strap is not populated. Read the strap and pass that information to FSP for memory initialization.

BUG=b:152275658 TEST=Build and boot the mainboard. Ensure that the strap information is read as expected and passed to FSP.

Change-Id: I69583f35ffc219bae9ce06bd4ba9898ed0d4d21d Signed-off-by: Karthikeyan Ramasubramanian kramasub@google.com --- M src/mainboard/google/dedede/romstage.c M src/mainboard/google/dedede/variants/baseboard/gpio.c M src/mainboard/google/dedede/variants/baseboard/include/baseboard/gpio.h 3 files changed, 11 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/39812/6

-- To view, visit https://review.coreboot.org/c/coreboot/+/39812 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I69583f35ffc219bae9ce06bd4ba9898ed0d4d21d Gerrit-Change-Number: 39812 Gerrit-PatchSet: 6 Gerrit-Owner: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Dtrain Hsu dtrain_hsu@compal.corp-partner.google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Marco Chen marcochen@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Karthik Ramasubramanian (Code Review)

6:22 p.m.

Hello build bot (Jenkins), Furquan Shaikh, Dtrain Hsu, Marco Chen,

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

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

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

Change subject: mb/google/dedede: Read DRAM population strap ......................................................................

mb/google/dedede: Read DRAM population strap

BUG=b:152275658, b:154301008 TEST=Build and boot the mainboard. Ensure that the strap information is read as expected and passed to FSP.

Change-Id: I69583f35ffc219bae9ce06bd4ba9898ed0d4d21d Signed-off-by: Karthikeyan Ramasubramanian kramasub@google.com --- M src/mainboard/google/dedede/romstage.c M src/mainboard/google/dedede/variants/baseboard/gpio.c M src/mainboard/google/dedede/variants/baseboard/include/baseboard/gpio.h 3 files changed, 11 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/39812/7

-- To view, visit https://review.coreboot.org/c/coreboot/+/39812 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I69583f35ffc219bae9ce06bd4ba9898ed0d4d21d Gerrit-Change-Number: 39812 Gerrit-PatchSet: 7 Gerrit-Owner: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Dtrain Hsu dtrain_hsu@compal.corp-partner.google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Marco Chen marcochen@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Furquan Shaikh (Code Review)

6:22 p.m.

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

Change subject: mb/google/dedede: Read DRAM population strap ......................................................................

Patch Set 7: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/39812 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I69583f35ffc219bae9ce06bd4ba9898ed0d4d21d Gerrit-Change-Number: 39812 Gerrit-PatchSet: 7 Gerrit-Owner: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Dtrain Hsu dtrain_hsu@compal.corp-partner.google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Marco Chen marcochen@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 17 Apr 2020 16:22:44 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Karthik Ramasubramanian (Code Review)

6:23 p.m.

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

Change subject: mb/google/dedede: Read DRAM population strap ......................................................................

Patch Set 7:

(2 comments)

https://review.coreboot.org/c/coreboot/+/39812/2//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/39812/2//COMMIT_MSG@13 PS2, Line 13: TEST=Build the mainboard.

...

Yet to test on the board where the strap is not stuffed.

Done

https://review.coreboot.org/c/coreboot/+/39812/2/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/gpio.c:

https://review.coreboot.org/c/coreboot/+/39812/2/src/mainboard/google/dedede... PS2, Line 351: GPP_S0

...

True, thanks for pointing out.

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/39812 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I69583f35ffc219bae9ce06bd4ba9898ed0d4d21d Gerrit-Change-Number: 39812 Gerrit-PatchSet: 7 Gerrit-Owner: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Dtrain Hsu dtrain_hsu@compal.corp-partner.google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Marco Chen marcochen@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 17 Apr 2020 16:23:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh furquan@google.com Comment-In-Reply-To: Karthik Ramasubramanian kramasub@google.com Gerrit-MessageType: comment

Patrick Georgi (Code Review)

20 Apr 20 Apr

8:25 a.m.

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

Change subject: mb/google/dedede: Read DRAM population strap ......................................................................

mb/google/dedede: Read DRAM population strap

BUG=b:152275658, b:154301008 TEST=Build and boot the mainboard. Ensure that the strap information is read as expected and passed to FSP.

Change-Id: I69583f35ffc219bae9ce06bd4ba9898ed0d4d21d Signed-off-by: Karthikeyan Ramasubramanian kramasub@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/39812 Reviewed-by: Furquan Shaikh furquan@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/dedede/romstage.c M src/mainboard/google/dedede/variants/baseboard/gpio.c M src/mainboard/google/dedede/variants/baseboard/include/baseboard/gpio.h 3 files changed, 11 insertions(+), 3 deletions(-)

Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved

diff --git a/src/mainboard/google/dedede/romstage.c b/src/mainboard/google/dedede/romstage.c index f95e7aa..2efaaf1 100644 --- a/src/mainboard/google/dedede/romstage.c +++ b/src/mainboard/google/dedede/romstage.c @@ -6,8 +6,10 @@ */

#include <baseboard/variants.h> +#include <gpio.h> #include <soc/meminit.h> #include <soc/romstage.h> +#include <variant/gpio.h>

void mainboard_memory_init_params(FSPM_UPD *memupd) { @@ -16,7 +18,7 @@ .read_type = READ_SPD_CBFS, .spd_spec.spd_index = variant_memory_sku(), }; - /* TODO: Read the resistor strap to get number of memory segments. */ - bool half_populated = 0; + bool half_populated = !gpio_get(GPIO_MEM_CH_SEL); + memcfg_init(&memupd->FspmConfig, board_cfg, &spd_info, half_populated); } diff --git a/src/mainboard/google/dedede/variants/baseboard/gpio.c b/src/mainboard/google/dedede/variants/baseboard/gpio.c index d12d2c4..a4ce97d 100644 --- a/src/mainboard/google/dedede/variants/baseboard/gpio.c +++ b/src/mainboard/google/dedede/variants/baseboard/gpio.c @@ -348,7 +348,7 @@

/* S0 : RAM_STRAP_4 */ - PAD_NC(GPP_S0, NONE), + PAD_CFG_GPI(GPP_S0, UP_5K, DEEP), /* S1 : RSVD_STRAP */ PAD_NC(GPP_S1, NONE), /* S2 : DMIC1_CLK */ @@ -416,6 +416,9 @@

/* H19 : BT_DISABLE_L */ PAD_CFG_GPO(GPP_H19, 0, DEEP), + + /* S0 : RAM_STRAP_4 */ + PAD_CFG_GPI(GPP_S0, UP_5K, DEEP), };

const struct pad_config *__weak variant_gpio_table(size_t *num) diff --git a/src/mainboard/google/dedede/variants/baseboard/include/baseboard/gpio.h b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/gpio.h index dfb2cd1..98e4b27 100644 --- a/src/mainboard/google/dedede/variants/baseboard/include/baseboard/gpio.h +++ b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/gpio.h @@ -26,4 +26,7 @@ #define GPIO_MEM_CONFIG_2 GPP_C4 #define GPIO_MEM_CONFIG_3 GPP_C5

+/* Memory channel select strap - 0: half-populated, 1: fully-populated */ +#define GPIO_MEM_CH_SEL GPP_S0 + #endif /* __BASEBOARD_GPIO_H__ */

-- To view, visit https://review.coreboot.org/c/coreboot/+/39812 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I69583f35ffc219bae9ce06bd4ba9898ed0d4d21d Gerrit-Change-Number: 39812 Gerrit-PatchSet: 8 Gerrit-Owner: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Dtrain Hsu dtrain_hsu@compal.corp-partner.google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Marco Chen marcochen@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: merged

1476

days inactive

1502

days old

coreboot-gerrit@coreboot.org

21 comments

6 participants

tags (0)

participants (6)

Dtrain Hsu (Code Review)
Furquan Shaikh (Code Review)
Karthik Ramasubramanian (Code Review)
Marco Chen (Code Review)
Patrick Georgi (Code Review)
Paul Menzel (Code Review)