Change in coreboot[master]: mainboard/google/dedede: Migrate onto SKU ID helpers

[Edward O'Callaghan (Code Review)]

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39036 )

Change subject: mainboard/google/dedede: Migrate onto SKU ID helpers
......................................................................

mainboard/google/dedede: Migrate onto SKU ID helpers

Leverage the common sku id space helper encoders.
dedede uses the non-legacy SKU ID space.

BUG=b:149348474
BRANCH=none
TEST=tested on hatch

Change-Id: I0c21a748fddef0985022cb4e77a8db95d6692f4b
Signed-off-by: Edward O'Callaghan 
---
M src/mainboard/google/dedede/Kconfig
M src/mainboard/google/dedede/board_info.c
2 files changed, 7 insertions(+), 30 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/39036/1

diff --git a/src/mainboard/google/dedede/Kconfig b/src/mainboard/google/dedede/Kconfig
index 5254d16..6a511ba 100644
--- a/src/mainboard/google/dedede/Kconfig
+++ b/src/mainboard/google/dedede/Kconfig
@@ -7,6 +7,7 @@
    select HAVE_ACPI_TABLES
    select MAINBOARD_HAS_CHROMEOS
    select SOC_INTEL_JASPERLAKE
+	select EC_GOOGLE_CHROMEEC_SKU_ID_MAX

if BOARD_GOOGLE_BASEBOARD_DEDEDE

diff --git a/src/mainboard/google/dedede/board_info.c b/src/mainboard/google/dedede/board_info.c
index ee89beb..f9ed44d 100644
--- a/src/mainboard/google/dedede/board_info.c
+++ b/src/mainboard/google/dedede/board_info.c
@@ -12,37 +12,9 @@
 #include <smbios.h>
 #include <string.h>

-#define SKU_UNKNOWN	0xffffffff
-#define SKU_MAX		0x7fffffff
-
-static uint32_t board_info_get_sku(void)
-{
-	static uint32_t sku_id = SKU_UNKNOWN;
-
-	if (sku_id != SKU_UNKNOWN)
-		return sku_id;
-
-	if (google_chromeec_cbi_get_sku_id(&sku_id))
-		sku_id = SKU_UNKNOWN;
-
-	return sku_id;
-}
-
 const char *smbios_system_sku(void)
 {
-	/* sku{0..2147483647} */
-	static char sku_str[14];
-	uint32_t sku_id = board_info_get_sku();
-
-	if (sku_id == SKU_UNKNOWN || sku_id > SKU_MAX) {
-		printk(BIOS_ERR, "%s: Unexpected SKU ID %u\n",
-			__func__, sku_id);
-		return "";
-	}
-
-	snprintf(sku_str, sizeof(sku_str), "sku%u", sku_id);
-
-	return sku_str;
+	return google_chromeec_smbios_system_sku();
 }

const char *smbios_mainboard_manufacturer(void)
@@ -66,7 +38,11 @@

int board_info_get_fw_config(uint32_t *fw_config)
 {
-	uint32_t sku_id = board_info_get_sku();
+	uint32_t sku_id = google_chromeec_get_board_sku();
+
+	/* FIXME: Move get_fw_config() in with rest of sku id helpers */
+#define SKU_UNKNOWN    0xffffffff
+#define SKU_MAX        0x7fffffff

/*
     * FW_CONFIG can potentially have all the bits set. So check the

[Edward O'Callaghan (Code Review)]

Hello Jett Rink, build bot (Jenkins),

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

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

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

Change subject: mainboard/google/dedede: Migrate onto SKU ID helpers
......................................................................

mainboard/google/dedede: Migrate onto SKU ID helpers

Leverage the common sku id space helper encoders.
dedede uses the non-legacy SKU ID space.

BUG=b:149348474
BRANCH=none
TEST=only tested on hatch

Change-Id: I0c21a748fddef0985022cb4e77a8db95d6692f4b
Signed-off-by: Edward O'Callaghan 
---
M src/mainboard/google/dedede/Kconfig
M src/mainboard/google/dedede/board_info.c
2 files changed, 8 insertions(+), 30 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/39036/2

[Edward O'Callaghan (Code Review)]

Hello Jett Rink, build bot (Jenkins),

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

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

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

Change subject: mainboard/google/dedede: Migrate onto SKU ID helpers
......................................................................

mainboard/google/dedede: Migrate onto SKU ID helpers

Leverage the common sku id space helper encoders.
dedede uses the non-legacy SKU ID space.

BUG=b:149348474
BRANCH=none
TEST=only tested on hatch

Change-Id: I0c21a748fddef0985022cb4e77a8db95d6692f4b
Signed-off-by: Edward O'Callaghan 
---
M src/mainboard/google/dedede/Kconfig
M src/mainboard/google/dedede/board_info.c
2 files changed, 9 insertions(+), 30 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/39036/3

[Edward O'Callaghan (Code Review)]

Hello Jett Rink, build bot (Jenkins),

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

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

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

Change subject: mainboard/google/dedede: Migrate onto SKU ID helpers
......................................................................

mainboard/google/dedede: Migrate onto SKU ID helpers

Leverage the common sku id space helper encoders.
dedede uses the non-legacy SKU ID space.

BUG=b:149348474
BRANCH=none
TEST=only tested on hatch

Change-Id: I0c21a748fddef0985022cb4e77a8db95d6692f4b
Signed-off-by: Edward O'Callaghan 
---
M src/mainboard/google/dedede/Kconfig
M src/mainboard/google/dedede/board_info.c
2 files changed, 7 insertions(+), 30 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/39036/5

[Jett Rink (Code Review)]

Jett Rink has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39036 )

Change subject: mainboard/google/dedede: Migrate onto SKU ID helpers
......................................................................

Patch Set 12: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/39036/12/src/mainboard/google/deded... 
File src/mainboard/google/dedede/board_info.c:

https://review.coreboot.org/c/coreboot/+/39036/12/src/mainboard/google/deded... 
PS12, Line 52: sku_id == SKU_UNKNOWN ||
nit: sku_id > SKU_MAX is a superset of sku_id == SKU_UNKNOWN, so we could just write sku_id > SKU_MAX

I know you didn't write the code though.

[Karthik Ramasubramanian (Code Review)]

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

Change subject: mainboard/google/dedede: Migrate onto SKU ID helpers
......................................................................

Patch Set 12:

(2 comments)

https://review.coreboot.org/c/coreboot/+/39036/12/src/mainboard/google/deded... 
File src/mainboard/google/dedede/Kconfig:

https://review.coreboot.org/c/coreboot/+/39036/12/src/mainboard/google/deded... 
PS12, Line 8: select EC_GOOGLE_CHROMEEC_SKUID
Nit: Move it after EC_GOOGLE_CHROMEEC_ESPI. Just organized in alphabetical order.

https://review.coreboot.org/c/coreboot/+/39036/12/src/mainboard/google/deded... 
File src/mainboard/google/dedede/board_info.c:

https://review.coreboot.org/c/coreboot/+/39036/12/src/mainboard/google/deded... 
PS12, Line 52: sku_id == SKU_UNKNOWN ||
> nit: sku_id > SKU_MAX is a superset of sku_id == SKU_UNKNOWN, so we could just write sku_id > SKU_MA […]

It is removed altogether here - https://review.coreboot.org/c/coreboot/+/39039/11 and is a follow-up CL in this CL stack.

Change in coreboot[master]: mainboard/google/dedede: Migrate onto SKU ID/fw_config helpers

[Edward O'Callaghan (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Jett Rink, Karthik Ramasubramanian,

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

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

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

Change subject: mainboard/google/dedede: Migrate onto SKU ID/fw_config helpers
......................................................................

mainboard/google/dedede: Migrate onto SKU ID/fw_config helpers

Leverage the common sku id space helper encoders.
dedede uses the non-legacy SKU ID space.

squash in,
  mainboard/google/dedede: Migrate onto get fw_config helper

BUG=b:149348474
BRANCH=none
TEST=only tested on hatch

Change-Id: I0c21a748fddef0985022cb4e77a8db95d6692f4b
Signed-off-by: Edward O'Callaghan 
---
M src/mainboard/google/dedede/Kconfig
M src/mainboard/google/dedede/board_info.c
2 files changed, 2 insertions(+), 39 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/39036/13

Change in coreboot[master]: mainboard/google/dedede: Migrate onto SKU ID/fw_config helpers

[Edward O'Callaghan (Code Review)]

Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39036 )

Change subject: mainboard/google/dedede: Migrate onto SKU ID/fw_config helpers
......................................................................

Patch Set 13:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39036/12/src/mainboard/google/deded... 
File src/mainboard/google/dedede/board_info.c:

https://review.coreboot.org/c/coreboot/+/39036/12/src/mainboard/google/deded... 
PS12, Line 52: sku_id == SKU_UNKNOWN ||
> It is removed altogether here - https://review.coreboot. […]

I have squashed the two together to avoid further confusion so we can get this over the line.

Change in coreboot[master]: mainboard/google/dedede: Migrate onto SKU ID/fw_config helpers

[Aaron Durbin (Code Review)]

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

Change subject: mainboard/google/dedede: Migrate onto SKU ID/fw_config helpers
......................................................................

Patch Set 13: Code-Review+2

Change in coreboot[master]: mainboard/google/dedede: Migrate onto SKU ID/fw_config helpers

[Edward O'Callaghan (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Jett Rink, Aaron Durbin, Karthik Ramasubramanian,

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

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

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

Change subject: mainboard/google/dedede: Migrate onto SKU ID/fw_config helpers
......................................................................

mainboard/google/dedede: Migrate onto SKU ID/fw_config helpers

Leverage the common sku id space helper encoders.
dedede uses the non-legacy SKU ID space.

squash in,
  mainboard/google/dedede: Migrate onto get fw_config helper

BUG=b:149348474
BRANCH=none
TEST=only tested on hatch

Change-Id: I0c21a748fddef0985022cb4e77a8db95d6692f4b
Signed-off-by: Edward O'Callaghan 
---
M src/mainboard/google/dedede/Kconfig
M src/mainboard/google/dedede/board_info.c
2 files changed, 2 insertions(+), 39 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/39036/14

Change in coreboot[master]: mainboard/google/dedede: Migrate onto SKU ID/fw_config helpers

[Edward O'Callaghan (Code Review)]

Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39036 )

Change subject: mainboard/google/dedede: Migrate onto SKU ID/fw_config helpers
......................................................................

Patch Set 14:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39036/12/src/mainboard/google/deded... 
File src/mainboard/google/dedede/Kconfig:

https://review.coreboot.org/c/coreboot/+/39036/12/src/mainboard/google/deded... 
PS12, Line 8: select EC_GOOGLE_CHROMEEC_SKUID
> Nit: Move it after EC_GOOGLE_CHROMEEC_ESPI. Just organized in alphabetical order.

Done

Change in coreboot[master]: mainboard/google/dedede: Migrate onto SKU ID/fw_config helpers

[Karthik Ramasubramanian (Code Review)]

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

Change subject: mainboard/google/dedede: Migrate onto SKU ID/fw_config helpers
......................................................................

Patch Set 14: Code-Review+2

Change in coreboot[master]: mainboard/google/dedede: Migrate onto SKU ID/fw_config helpers

[Edward O'Callaghan (Code Review)]

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39036 )

Change subject: mainboard/google/dedede: Migrate onto SKU ID/fw_config helpers
......................................................................

mainboard/google/dedede: Migrate onto SKU ID/fw_config helpers

Leverage the common sku id space helper encoders.
dedede uses the non-legacy SKU ID space.

squash in,
  mainboard/google/dedede: Migrate onto get fw_config helper

BUG=b:149348474
BRANCH=none
TEST=only tested on hatch

Change-Id: I0c21a748fddef0985022cb4e77a8db95d6692f4b
Signed-off-by: Edward O'Callaghan 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/39036
Tested-by: build bot (Jenkins) 
Reviewed-by: Karthik Ramasubramanian 
---
M src/mainboard/google/dedede/Kconfig
M src/mainboard/google/dedede/board_info.c
2 files changed, 2 insertions(+), 39 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Karthik Ramasubramanian: Looks good to me, approved

diff --git a/src/mainboard/google/dedede/Kconfig b/src/mainboard/google/dedede/Kconfig
index ebca580..2606e57 100644
--- a/src/mainboard/google/dedede/Kconfig
+++ b/src/mainboard/google/dedede/Kconfig
@@ -6,6 +6,7 @@
    select EC_GOOGLE_CHROMEEC
    select EC_GOOGLE_CHROMEEC_BOARDID
    select EC_GOOGLE_CHROMEEC_ESPI
+	select EC_GOOGLE_CHROMEEC_SKUID
    select GENERIC_SPD_BIN
    select HAVE_ACPI_RESUME
    select HAVE_ACPI_TABLES
diff --git a/src/mainboard/google/dedede/board_info.c b/src/mainboard/google/dedede/board_info.c
index ee89beb..7b10b23 100644
--- a/src/mainboard/google/dedede/board_info.c
+++ b/src/mainboard/google/dedede/board_info.c
@@ -12,37 +12,9 @@
 #include <smbios.h>
 #include <string.h>

-#define SKU_UNKNOWN	0xffffffff
-#define SKU_MAX		0x7fffffff
-
-static uint32_t board_info_get_sku(void)
-{
-	static uint32_t sku_id = SKU_UNKNOWN;
-
-	if (sku_id != SKU_UNKNOWN)
-		return sku_id;
-
-	if (google_chromeec_cbi_get_sku_id(&sku_id))
-		sku_id = SKU_UNKNOWN;
-
-	return sku_id;
-}
-
 const char *smbios_system_sku(void)
 {
-	/* sku{0..2147483647} */
-	static char sku_str[14];
-	uint32_t sku_id = board_info_get_sku();
-
-	if (sku_id == SKU_UNKNOWN || sku_id > SKU_MAX) {
-		printk(BIOS_ERR, "%s: Unexpected SKU ID %u\n",
-			__func__, sku_id);
-		return "";
-	}
-
-	snprintf(sku_str, sizeof(sku_str), "sku%u", sku_id);
-
-	return sku_str;
+	return google_chromeec_smbios_system_sku();
 }

const char *smbios_mainboard_manufacturer(void)
@@ -66,15 +38,5 @@

int board_info_get_fw_config(uint32_t *fw_config)
 {
-	uint32_t sku_id = board_info_get_sku();
-
-	/*
-	 * FW_CONFIG can potentially have all the bits set. So check the
-	 * sku_id to ensure that the CBI is provisioned before reading the
-	 * FW_CONFIG.
-	 */
-	if (sku_id == SKU_UNKNOWN || sku_id > SKU_MAX)
-		return -1;
-
    return google_chromeec_cbi_get_fw_config(fw_config);
 }