Change in ...coreboot[master]: mb/google/sarien: Update SMBIOS type17

[Lijian Zhao (Code Review)]

Lijian Zhao has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32280

Change subject: mb/google/sarien: Update SMBIOS type17
......................................................................

mb/google/sarien: Update SMBIOS type17

Make SMBIOS type 17 device locator to be match with motherboard
silkscreen, using "DIMM-A" and "DIMM-B" instead of "Channel-0-DIMM-0"
and "Chaneek-1-DIMM-0".

TEST=Boot up with sarien platform and run dmidecode to check SMBIOS
type 17 have expected output.

Signed-off-by: Lijian Zhao 
Change-Id: Ie2125c0381bd24d96f725f68cde93a53da8c94c8
---
M src/mainboard/google/sarien/ramstage.c
1 file changed, 22 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/32280/1

diff --git a/src/mainboard/google/sarien/ramstage.c b/src/mainboard/google/sarien/ramstage.c
index 96321f8..e1fe62b 100644
--- a/src/mainboard/google/sarien/ramstage.c
+++ b/src/mainboard/google/sarien/ramstage.c
@@ -14,11 +14,33 @@
  */

#include <arch/acpi.h>
+#include <smbios.h>
 #include <soc/gpio.h>
 #include <soc/ramstage.h>
 #include <variant/gpio.h>
 #include <vendorcode/google/chromeos/chromeos.h>

+/* mainboard silk screen shows DIMM-A and DIMM-B */
+void smbios_fill_dimm_locator(const struct dimm_info *dimm,
+	struct smbios_type17 *t)
+{
+	switch(dimm->channel_num) {
+	case 0:
+		t->device_locator = smbios_add_string(t->eos,
+			"DIMM-A");
+		break;
+	case 1:
+		t->device_locator = smbios_add_string(t->eos,
+			"DIMM-B");
+		break;
+	default: {
+		t->device_locator = smbios_add_string(t->eos,
+			"UNKOWN");
+			break;
+		}
+	}
+}
+
 void mainboard_silicon_init_params(FSP_S_CONFIG *params)
 {
    const struct pad_config *gpio_table;

[build bot (Jenkins) (Code Review)]

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32280 )

Change subject: mb/google/sarien: Update SMBIOS type17
......................................................................

Patch Set 1:

(2 comments)

https://review.coreboot.org/#/c/32280/1/src/mainboard/google/sarien/ramstage... 
File src/mainboard/google/sarien/ramstage.c:

https://review.coreboot.org/#/c/32280/1/src/mainboard/google/sarien/ramstage... 
PS1, Line 27: 	switch(dimm->channel_num) {
space required before the open parenthesis '('

https://review.coreboot.org/#/c/32280/1/src/mainboard/google/sarien/ramstage... 
PS1, Line 38: 			"UNKOWN");
'UNKOWN' may be misspelled - perhaps 'UNKNOWN'?

[Lijian Zhao (Code Review)]

Lijian Zhao has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/32280 )

Change subject: mb/google/sarien: Update SMBIOS type17
......................................................................

mb/google/sarien: Update SMBIOS type17

Make SMBIOS type 17 device locator to be match with motherboard
silkscreen, using "DIMM-A" and "DIMM-B" instead of "Channel-0-DIMM-0"
and "Chaneek-1-DIMM-0".

TEST=Boot up with sarien platform and run dmidecode to check SMBIOS
type 17 have expected output.

Signed-off-by: Lijian Zhao 
Change-Id: Ie2125c0381bd24d96f725f68cde93a53da8c94c8
---
M src/mainboard/google/sarien/ramstage.c
1 file changed, 22 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/32280/2

[Paul Menzel (Code Review)]

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

Change subject: mb/google/sarien: Update SMBIOS type17
......................................................................

Patch Set 2:

(4 comments)

https://review.coreboot.org/#/c/32280/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/32280/2//COMMIT_MSG@9 
PS2, Line 9: Make SMBIOS type 17 device locator to be match with motherboard
           : silkscreen
Match SMBIOS type 17 device locator with motherboard silkscreen, …

https://review.coreboot.org/#/c/32280/2//COMMIT_MSG@11 
PS2, Line 11: Chaneek
Channel?

https://review.coreboot.org/#/c/32280/2/src/mainboard/google/sarien/ramstage... 
File src/mainboard/google/sarien/ramstage.c:

https://review.coreboot.org/#/c/32280/2/src/mainboard/google/sarien/ramstage... 
PS2, Line 23: silk screen
You spell it silkscreen as one word in the commit messages.

https://review.coreboot.org/#/c/32280/2/src/mainboard/google/sarien/ramstage... 
PS2, Line 30: 			"DIMM-A");
In Gerrit it looks like it would fit on one line.

[Lijian Zhao (Code Review)]

Hello build bot (Jenkins),

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

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

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

Change subject: mb/google/sarien: Update SMBIOS type17
......................................................................

mb/google/sarien: Update SMBIOS type17

Match SMBIOS type 17 device locator with motherboard silk screen,using
"DIMM-A" and "DIMM-B" instead of "Channel-0-DIMM-0" and 
"Chaneel-1-DIMM-0".

TEST=Boot up with sarien platform and run dmidecode to check SMBIOS
type 17 have expected output.

Signed-off-by: Lijian Zhao 
Change-Id: Ie2125c0381bd24d96f725f68cde93a53da8c94c8
---
M src/mainboard/google/sarien/ramstage.c
1 file changed, 18 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/32280/3

[Duncan Laurie (Code Review)]

Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32280 )

Change subject: mb/google/sarien: Update SMBIOS type17
......................................................................

Patch Set 3: Code-Review+2

[Lijian Zhao (Code Review)]

Lijian Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32280 )

Change subject: mb/google/sarien: Update SMBIOS type17
......................................................................

Patch Set 2:

(3 comments)

https://review.coreboot.org/#/c/32280/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/32280/2//COMMIT_MSG@9 
PS2, Line 9: Make SMBIOS type 17 device locator to be match with motherboard
           : silkscreen
> Match SMBIOS type 17 device locator with motherboard silkscreen, …

I have a brief google and seems that's silk screen, means the PCB label on motherboard. So I changed the commit message.

https://review.coreboot.org/#/c/32280/2//COMMIT_MSG@11 
PS2, Line 11: Chaneek
> Channel?

Done

https://review.coreboot.org/#/c/32280/2/src/mainboard/google/sarien/ramstage... 
File src/mainboard/google/sarien/ramstage.c:

https://review.coreboot.org/#/c/32280/2/src/mainboard/google/sarien/ramstage... 
PS2, Line 30: 			"DIMM-A");
> In Gerrit it looks like it would fit on one line.

Done

[Lijian Zhao (Code Review)]

Hello Duncan Laurie, build bot (Jenkins),

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

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

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

Change subject: mb/google/sarien: Update SMBIOS type17
......................................................................

mb/google/sarien: Update SMBIOS type17

Match SMBIOS type 17 device locator with motherboard silk screen,using
"DIMM-A" and "DIMM-B" instead of "Channel-0-DIMM-0" and 
"Chaneel-1-DIMM-0".

TEST=Boot up with sarien platform and run dmidecode to check SMBIOS
type 17 have expected output.

Signed-off-by: Lijian Zhao 
Change-Id: Ie2125c0381bd24d96f725f68cde93a53da8c94c8
---
M src/mainboard/google/sarien/ramstage.c
1 file changed, 20 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/32280/4

[Duncan Laurie (Code Review)]

Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32280 )

Change subject: mb/google/sarien: Update SMBIOS type17
......................................................................

Patch Set 4: Code-Review+2

[Duncan Laurie (Code Review)]

Duncan Laurie has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32280 )

Change subject: mb/google/sarien: Update SMBIOS type17
......................................................................

mb/google/sarien: Update SMBIOS type17

Match SMBIOS type 17 device locator with motherboard silk screen,using
"DIMM-A" and "DIMM-B" instead of "Channel-0-DIMM-0" and 
"Chaneel-1-DIMM-0".

TEST=Boot up with sarien platform and run dmidecode to check SMBIOS
type 17 have expected output.

Signed-off-by: Lijian Zhao 
Change-Id: Ie2125c0381bd24d96f725f68cde93a53da8c94c8
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32280
Tested-by: build bot (Jenkins) 
Reviewed-by: Duncan Laurie 
---
M src/mainboard/google/sarien/ramstage.c
1 file changed, 20 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Duncan Laurie: Looks good to me, approved

diff --git a/src/mainboard/google/sarien/ramstage.c b/src/mainboard/google/sarien/ramstage.c
index 96321f8..d8d1c9d 100644
--- a/src/mainboard/google/sarien/ramstage.c
+++ b/src/mainboard/google/sarien/ramstage.c
@@ -14,11 +14,31 @@
  */

#include <arch/acpi.h>
+#include <smbios.h>
 #include <soc/gpio.h>
 #include <soc/ramstage.h>
 #include <variant/gpio.h>
 #include <vendorcode/google/chromeos/chromeos.h>

+#if CONFIG(GENERATE_SMBIOS_TABLES)
+/* mainboard silk screen shows DIMM-A and DIMM-B */
+void smbios_fill_dimm_locator(const struct dimm_info *dimm,
+	struct smbios_type17 *t)
+{
+	switch (dimm->channel_num) {
+	case 0:
+		t->device_locator = smbios_add_string(t->eos, "DIMM-A");
+		break;
+	case 1:
+		t->device_locator = smbios_add_string(t->eos, "DIMM-B");
+		break;
+	default:
+		t->device_locator = smbios_add_string(t->eos, "UNKNOWN");
+		break;
+	}
+}
+#endif
+
 void mainboard_silicon_init_params(FSP_S_CONFIG *params)
 {
    const struct pad_config *gpio_table;