Change in coreboot[4.11_branch]: src/mainboard/ocp/monolake: Create SMBIOS type 16 for Monolake platform

[Morgan Jang (Code Review)]

Morgan Jang has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37233 )

Change subject: src/mainboard/ocp/monolake: Create SMBIOS type 16 for Monolake platform
......................................................................

src/mainboard/ocp/monolake: Create SMBIOS type 16 for Monolake platform

Create SMBIOS type 16 for Monolake platform, but the memory ECC setting can`t be
enabled by the FSP, so it`s hardcoded to MEMORY_ARRAY_ECC_SINGLE_BIT.

Change-Id: Ie057742112f14447b226d432417d9301d4aea958
Signed-off-by: Morgan Jang 
---
M src/mainboard/ocp/monolake/mainboard.c
1 file changed, 41 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/37233/1

diff --git a/src/mainboard/ocp/monolake/mainboard.c b/src/mainboard/ocp/monolake/mainboard.c
index dffd19f..3a11b55 100644
--- a/src/mainboard/ocp/monolake/mainboard.c
+++ b/src/mainboard/ocp/monolake/mainboard.c
@@ -30,6 +30,8 @@
 #define VPD_LEN 10
 /* Default countdown is 15 minutes. */
 #define DEFAULT_COUNTDOWN 9000
+#define MAX_IMC  1
+#define MAX_DIMM_SIZE_GB (32UL << 20)

static void init_frb2_wdt(void)
 {
@@ -60,6 +62,41 @@
    }
 }

+#if CONFIG(GENERATE_SMBIOS_TABLES)
+static int write_smbios_type16(struct device *dev, int *handle, unsigned long *current)
+{
+	struct smbios_type16 *t = (struct smbios_type16 *)*current;
+	u32 maximum_capacity;
+	int len = sizeof(struct smbios_type16);
+
+	printk(BIOS_INFO, "Create SMBIOS type 16\n");
+
+	memset(t, 0, sizeof(struct smbios_type16));
+	t->type = SMBIOS_PHYS_MEMORY_ARRAY;
+	t->location = MEMORY_ARRAY_LOCATION_SYSTEM_BOARD;
+	t->use = MEMORY_ARRAY_USE_SYSTEM;
+	/* The ECC setting can`t be confirmed in FSP, so hardcode it. */
+	t->memory_error_correction = MEMORY_ARRAY_ECC_SINGLE_BIT;
+	t->memory_error_information_handle = 0xFFFE;
+	t->number_of_memory_devices = CONFIG_DIMM_MAX/MAX_IMC;
+
+	maximum_capacity = (u32)(CONFIG_DIMM_MAX * MAX_DIMM_SIZE_GB);
+	if (maximum_capacity >= 0x80000000) {
+		t->maximum_capacity = 0x80000000;
+		t->extended_maximum_capacity = maximum_capacity << 10;
+	} else {
+		t->maximum_capacity = (u32)maximum_capacity;
+		t->extended_maximum_capacity = 0;
+	}
+
+	*current += len;
+	t->handle = *handle;
+	*handle += 1;
+	t->length = len - 2;
+	return len;
+}
+#endif
+
 /*
  * mainboard_enable is executed as first thing after enumerate_buses().
  * This is the earliest point to add customization.
@@ -76,6 +113,10 @@
    	clear_ipmi_flags(&rsp);
    	system_reset();
    }
+
+#if CONFIG(GENERATE_SMBIOS_TABLES)
+	dev->ops->get_smbios_data = write_smbios_type16;
+#endif
 }

struct chip_operations mainboard_ops = {

Change in coreboot[4.11_branch]: mb/ocp/monolake: Create SMBIOS type 16 for Monolake platform

[Morgan Jang (Code Review)]

Hello Jingle Hsu, Patrick Rudolph, Jonathan Zhang, Johnny Lin, David Hendricks, Philipp Deppenwiese, build bot (Jenkins),

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

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

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

Change subject: mb/ocp/monolake: Create SMBIOS type 16 for Monolake platform
......................................................................

mb/ocp/monolake: Create SMBIOS type 16 for Monolake platform

Change-Id: Ie057742112f14447b226d432417d9301d4aea958
Signed-off-by: Morgan Jang 
---
M src/mainboard/ocp/monolake/mainboard.c
1 file changed, 41 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/37233/2

Change in coreboot[4.11_branch]: mb/ocp/monolake: Create SMBIOS type 16 for Monolake platform

[David Hendricks (Code Review)]

David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37233 )

Change subject: mb/ocp/monolake: Create SMBIOS type 16 for Monolake platform
......................................................................

Patch Set 2: Code-Review-1

(3 comments)

Hi Morgan,
Thanks for doing this, and for cherry-picking to 4.11_branch. The comments below are just carried over from the original patch.

https://review.coreboot.org/c/coreboot/+/37233/2/src/mainboard/ocp/monolake/... 
File src/mainboard/ocp/monolake/mainboard.c:

https://review.coreboot.org/c/coreboot/+/37233/2/src/mainboard/ocp/monolake/... 
PS2, Line 34: (32UL << 20)
#define MAX_DIMM_SIZE_GB (32 * MiB)

(thanks to Paul for pointing this out - I should have known better when I suggested this form)

https://review.coreboot.org/c/coreboot/+/37233/2/src/mainboard/ocp/monolake/... 
PS2, Line 81: CONFIG_DIMM_MAX/MAX_IMC;
space around / operator

(as per Arthur's comment in CB:36764)

https://review.coreboot.org/c/coreboot/+/37233/2/src/mainboard/ocp/monolake/... 
PS2, Line 119: #endif
if (CONFIG(GENERATE_SMBIOS_TABLES))

(as per Paul's comment in CB:36764, see also CB:31773 for background information)

Change in coreboot[4.11_branch]: mb/ocp/monolake: Create SMBIOS type 16 for Monolake platform

[Morgan Jang (Code Review)]

Hello Jingle Hsu, Patrick Rudolph, Jonathan Zhang, Johnny Lin, David Hendricks, Philipp Deppenwiese, build bot (Jenkins),

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

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

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

Change subject: mb/ocp/monolake: Create SMBIOS type 16 for Monolake platform
......................................................................

mb/ocp/monolake: Create SMBIOS type 16 for Monolake platform

Change-Id: Ie057742112f14447b226d432417d9301d4aea958
Signed-off-by: Morgan Jang 
---
M src/mainboard/ocp/monolake/mainboard.c
1 file changed, 44 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/37233/3

Change in coreboot[4.11_branch]: mb/ocp/monolake: Create SMBIOS type 16 for Monolake platform

[David Hendricks (Code Review)]

David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37233 )

Change subject: mb/ocp/monolake: Create SMBIOS type 16 for Monolake platform
......................................................................

Patch Set 4: Code-Review+2

(3 comments)

Marked old comments as resolved. Thanks!

https://review.coreboot.org/c/coreboot/+/37233/2/src/mainboard/ocp/monolake/... 
File src/mainboard/ocp/monolake/mainboard.c:

https://review.coreboot.org/c/coreboot/+/37233/2/src/mainboard/ocp/monolake/... 
PS2, Line 34: (32UL << 20)
> #define MAX_DIMM_SIZE_GB (32 * MiB) […]

Done

https://review.coreboot.org/c/coreboot/+/37233/2/src/mainboard/ocp/monolake/... 
PS2, Line 81: CONFIG_DIMM_MAX/MAX_IMC;
> space around / operator […]

Done

https://review.coreboot.org/c/coreboot/+/37233/2/src/mainboard/ocp/monolake/... 
PS2, Line 119: #endif
> if (CONFIG(GENERATE_SMBIOS_TABLES)) […]

Done

Change in coreboot[4.11_branch]: mb/ocp/monolake: Create SMBIOS type 16 for Monolake platform

[Angel Pons (Code Review)]

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

Change subject: mb/ocp/monolake: Create SMBIOS type 16 for Monolake platform
......................................................................

Patch Set 4: Code-Review+2

Needs a rebase though?

Change in coreboot[4.11_branch]: mb/ocp/monolake: Create SMBIOS type 16 for Monolake platform

[Morgan Jang (Code Review)]

Morgan Jang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37233 )

Change subject: mb/ocp/monolake: Create SMBIOS type 16 for Monolake platform
......................................................................

Patch Set 4:

(3 comments)
> Patch Set 4: Code-Review+2
>
> Needs a rebase though?

It seems that I have no "Push" rights in this branch.

https://review.coreboot.org/c/coreboot/+/37233/2/src/mainboard/ocp/monolake/... 
File src/mainboard/ocp/monolake/mainboard.c:

https://review.coreboot.org/c/coreboot/+/37233/2/src/mainboard/ocp/monolake/... 
PS2, Line 34: (32UL << 20)
> #define MAX_DIMM_SIZE_GB (32 * MiB) […]

Done

https://review.coreboot.org/c/coreboot/+/37233/2/src/mainboard/ocp/monolake/... 
PS2, Line 81: CONFIG_DIMM_MAX/MAX_IMC;
> space around / operator […]

Done

https://review.coreboot.org/c/coreboot/+/37233/2/src/mainboard/ocp/monolake/... 
PS2, Line 119: #endif
> if (CONFIG(GENERATE_SMBIOS_TABLES)) […]

Done

Change in coreboot[4.11_branch]: mb/ocp/monolake: Create SMBIOS type 16 for Monolake platform

[Angel Pons (Code Review)]

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

Change subject: mb/ocp/monolake: Create SMBIOS type 16 for Monolake platform
......................................................................

Patch Set 4:
> Patch Set 4:
>
> (3 comments)
> > Patch Set 4: Code-Review+2
> >
> > Needs a rebase though?
>
> It seems that I have no "Push" rights in this branch.

Try this: git push origin HEAD:refs/for/4.11_branch

Change in coreboot[4.11_branch]: mb/ocp/monolake: Create SMBIOS type 16 for Monolake platform

[Morgan Jang (Code Review)]

Hello Philipp Deppenwiese, build bot (Jenkins), Patrick Rudolph, Jonathan Zhang, Johnny Lin, David Hendricks, Jingle Hsu, Angel Pons,

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

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

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

Change subject: mb/ocp/monolake: Create SMBIOS type 16 for Monolake platform
......................................................................

mb/ocp/monolake: Create SMBIOS type 16 for Monolake platform

TEST=Use "dmidecode -t 16" in Linux to check if SMBIOS type 16 exists

Change-Id: Ie057742112f14447b226d432417d9301d4aea958
Signed-off-by: Morgan Jang 
---
M src/mainboard/ocp/monolake/mainboard.c
1 file changed, 45 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/37233/5

Change in coreboot[4.11_branch]: mb/ocp/monolake: Create SMBIOS type 16 for Monolake platform

[Morgan Jang (Code Review)]

Morgan Jang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37233 )

Change subject: mb/ocp/monolake: Create SMBIOS type 16 for Monolake platform
......................................................................

Patch Set 5:
> Patch Set 4:
> > Patch Set 4:
> >
> > (3 comments)
> > > Patch Set 4: Code-Review+2
> > >
> > > Needs a rebase though?
> >
> > It seems that I have no "Push" rights in this branch.
>
> Try this: git push origin HEAD:refs/for/4.11_branch

Done, thanks for your help!

Change in coreboot[4.11_branch]: mb/ocp/monolake: Create SMBIOS type 16 for Monolake platform

[Angel Pons (Code Review)]

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

Change subject: mb/ocp/monolake: Create SMBIOS type 16 for Monolake platform
......................................................................

Patch Set 5: Code-Review+2

Change in coreboot[4.11_branch]: mb/ocp/monolake: Create SMBIOS type 16 for Monolake platform

[Angel Pons (Code Review)]

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

Change subject: mb/ocp/monolake: Create SMBIOS type 16 for Monolake platform
......................................................................

Patch Set 5:
> Patch Set 5:
> > Patch Set 4:
> > > Patch Set 4:
> > >
> > > (3 comments)
> > > > Patch Set 4: Code-Review+2
> > > >
> > > > Needs a rebase though?
> > >
> > > It seems that I have no "Push" rights in this branch.
> >
> > Try this: git push origin HEAD:refs/for/4.11_branch
>
> Done, thanks for your help!

No problem! Now, let's hope Jenkins likes this change 😄

Change in coreboot[4.11_branch]: mb/ocp/monolake: Create SMBIOS type 16 for Monolake platform

[Angel Pons (Code Review)]

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

Change subject: mb/ocp/monolake: Create SMBIOS type 16 for Monolake platform
......................................................................

mb/ocp/monolake: Create SMBIOS type 16 for Monolake platform

TEST=Use "dmidecode -t 16" in Linux to check if SMBIOS type 16 exists

Change-Id: Ie057742112f14447b226d432417d9301d4aea958
Signed-off-by: Morgan Jang 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/37233
Reviewed-by: Angel Pons 
Tested-by: build bot (Jenkins) 
---
M src/mainboard/ocp/monolake/mainboard.c
1 file changed, 45 insertions(+), 1 deletion(-)

Approvals:
  build bot (Jenkins): Verified
  Angel Pons: Looks good to me, approved

diff --git a/src/mainboard/ocp/monolake/mainboard.c b/src/mainboard/ocp/monolake/mainboard.c
index 9a1c995..5f3408e 100644
--- a/src/mainboard/ocp/monolake/mainboard.c
+++ b/src/mainboard/ocp/monolake/mainboard.c
@@ -31,8 +31,9 @@
 /* Default countdown is 15 minutes. */
 #define DEFAULT_COUNTDOWN 9000
 #define FRU_DEVICE_ID 0
-
 static struct fru_info_str fru_strings;
+#define MAX_IMC 1
+#define MAX_DIMM_SIZE_GB (32 * MiB)

static void init_frb2_wdt(void)
 {
@@ -63,6 +64,44 @@
    }
 }

+#if CONFIG(GENERATE_SMBIOS_TABLES)
+static int write_smbios_type16(struct device *dev, int *handle, unsigned long *current)
+{
+	struct smbios_type16 *t = (struct smbios_type16 *)*current;
+	u32 maximum_capacity;
+	int len = sizeof(struct smbios_type16);
+
+	printk(BIOS_INFO, "Creating SMBIOS tables type 16 (note, ECC information is hard-coded) ...");
+
+	memset(t, 0, sizeof(struct smbios_type16));
+	t->type = SMBIOS_PHYS_MEMORY_ARRAY;
+	t->location = MEMORY_ARRAY_LOCATION_SYSTEM_BOARD;
+	t->use = MEMORY_ARRAY_USE_SYSTEM;
+	/* The ECC setting can`t be confirmed in FSP, so hardcode it. */
+	t->memory_error_correction = MEMORY_ARRAY_ECC_SINGLE_BIT;
+	t->memory_error_information_handle = 0xFFFE;
+	t->number_of_memory_devices = CONFIG_DIMM_MAX / MAX_IMC;
+
+	maximum_capacity = (u32)(CONFIG_DIMM_MAX * MAX_DIMM_SIZE_GB);
+	if (maximum_capacity >= 0x80000000) {
+		t->maximum_capacity = 0x80000000;
+		t->extended_maximum_capacity = maximum_capacity << 10;
+	} else {
+		t->maximum_capacity = (u32)maximum_capacity;
+		t->extended_maximum_capacity = 0;
+	}
+
+	*current += len;
+	t->handle = *handle;
+	*handle += 1;
+	t->length = len - 2;
+
+	printk(BIOS_INFO, "done\n");
+
+	return len;
+}
+#endif
+
 /*
  * mainboard_enable is executed as first thing after enumerate_buses().
  * This is the earliest point to add customization.
@@ -79,7 +118,12 @@
    	clear_ipmi_flags(&rsp);
    	system_reset();
    }
+
    read_fru_areas(BMC_KCS_BASE, FRU_DEVICE_ID, 0, &fru_strings);
+
+#if (CONFIG(GENERATE_SMBIOS_TABLES))
+	dev->ops->get_smbios_data = write_smbios_type16;
+#endif
 }

struct chip_operations mainboard_ops = {