Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39035 )
Change subject: mainboard/google/octopus: Migrate onto SKU ID helpers ......................................................................
mainboard/google/octopus: Migrate onto SKU ID helpers
Leverage the common sku id space helper encoders and set the sku id max to 0xff for legacy to ensure we behave the same.
BUG=b:149348474 BRANCH=none TEST=tested on hatch
Change-Id: I60a37a5f9659b8df4018872956f95e07a3506440 Signed-off-by: Edward O'Callaghan quasisec@google.com --- M src/mainboard/google/octopus/Kconfig M src/mainboard/google/octopus/mainboard_misc.c M src/mainboard/google/octopus/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/octopus/variants/bloog/variant.c M src/mainboard/google/octopus/variants/bobba/gpio.c M src/mainboard/google/octopus/variants/bobba/variant.c M src/mainboard/google/octopus/variants/casta/variant.c M src/mainboard/google/octopus/variants/dood/gpio.c M src/mainboard/google/octopus/variants/dood/variant.c M src/mainboard/google/octopus/variants/foob/gpio.c M src/mainboard/google/octopus/variants/foob/variant.c M src/mainboard/google/octopus/variants/garg/gpio.c M src/mainboard/google/octopus/variants/garg/variant.c M src/mainboard/google/octopus/variants/meep/gpio.c M src/mainboard/google/octopus/variants/meep/variant.c 15 files changed, 27 insertions(+), 50 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/39035/1
diff --git a/src/mainboard/google/octopus/Kconfig b/src/mainboard/google/octopus/Kconfig index a4e4982..7c9694c 100644 --- a/src/mainboard/google/octopus/Kconfig +++ b/src/mainboard/google/octopus/Kconfig @@ -116,6 +116,10 @@ int default 63 # GPE0_DW1_31 (GPIO_63)
+config EC_GOOGLE_CHROMEEC_SKU_ID_MAX + hex + default 0xff # LEGACY SKU_ID space. + config DRAM_PART_NUM_NOT_ALWAYS_IN_CBI bool default y if BOARD_GOOGLE_BOBBA diff --git a/src/mainboard/google/octopus/mainboard_misc.c b/src/mainboard/google/octopus/mainboard_misc.c index 3672f66..8b281da 100644 --- a/src/mainboard/google/octopus/mainboard_misc.c +++ b/src/mainboard/google/octopus/mainboard_misc.c @@ -21,34 +21,7 @@ #include <smbios.h> #include <string.h>
-#define SKU_UNKNOWN 0xFFFFFFFF -#define SKU_MAX 255 - -uint32_t get_board_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) { - static char sku_str[7]; /* sku{0..255} */ - uint32_t sku_id = get_board_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(); } diff --git a/src/mainboard/google/octopus/variants/baseboard/include/baseboard/variants.h b/src/mainboard/google/octopus/variants/baseboard/include/baseboard/variants.h index 2132db5..bf08a85 100644 --- a/src/mainboard/google/octopus/variants/baseboard/include/baseboard/variants.h +++ b/src/mainboard/google/octopus/variants/baseboard/include/baseboard/variants.h @@ -35,8 +35,6 @@ const struct lpddr4_cfg *variant_lpddr4_config(void); /* Return memory SKU for the board. */ size_t variant_memory_sku(void); -/* Return board SKU */ -uint32_t get_board_sku(void); /* Return ChromeOS gpio table and fill in number of entries. */ const struct cros_gpio *variant_cros_gpios(size_t *num);
diff --git a/src/mainboard/google/octopus/variants/bloog/variant.c b/src/mainboard/google/octopus/variants/bloog/variant.c index 699385e..1e5c3ca 100644 --- a/src/mainboard/google/octopus/variants/bloog/variant.c +++ b/src/mainboard/google/octopus/variants/bloog/variant.c @@ -42,7 +42,7 @@ const char *get_wifi_sar_cbfs_filename(void) { const char *filename = NULL; - uint32_t sku_id = get_board_sku(); + uint32_t sku_id = google_chromeec_get_board_sku();
switch (sku_id) { case SKU_33_BLOOG: @@ -65,7 +65,7 @@ { uint32_t sku_id;
- sku_id = get_board_sku(); + sku_id = google_chromeec_get_board_sku();
switch (sku_id) { case SKU_33_BLOOG: diff --git a/src/mainboard/google/octopus/variants/bobba/gpio.c b/src/mainboard/google/octopus/variants/bobba/gpio.c index 7c522c7..dd10840 100644 --- a/src/mainboard/google/octopus/variants/bobba/gpio.c +++ b/src/mainboard/google/octopus/variants/bobba/gpio.c @@ -18,6 +18,7 @@ #include <boardid.h> #include <gpio.h> #include <soc/gpio.h> +#include <ec/google/chromeec/ec.h>
enum { SKU_37_DROID = 37, /* LTE */ @@ -60,7 +61,7 @@ const struct pad_config *variant_override_gpio_table(size_t *num) { uint32_t sku_id; - sku_id = get_board_sku(); + sku_id = google_chromeec_get_board_sku();
switch (sku_id) { case SKU_37_DROID: diff --git a/src/mainboard/google/octopus/variants/bobba/variant.c b/src/mainboard/google/octopus/variants/bobba/variant.c index 1f6e80d..936eac8 100644 --- a/src/mainboard/google/octopus/variants/bobba/variant.c +++ b/src/mainboard/google/octopus/variants/bobba/variant.c @@ -58,7 +58,7 @@ const char *get_wifi_sar_cbfs_filename(void) { const char *filename = NULL; - uint32_t sku_id = get_board_sku(); + uint32_t sku_id = google_chromeec_get_board_sku();
if (sku_id == 33 || sku_id == 34 || sku_id == 35 || sku_id == 36 || sku_id == 41 || sku_id == 42 || sku_id == 43 || sku_id == 44) @@ -74,7 +74,7 @@ if (slp_typ != ACPI_S5) return;
- switch (get_board_sku()) { + switch (google_chromeec_get_board_sku()) { case SKU_37_DROID: case SKU_38_DROID: case SKU_39_DROID: diff --git a/src/mainboard/google/octopus/variants/casta/variant.c b/src/mainboard/google/octopus/variants/casta/variant.c index 12c8dd7..ef50eaf 100644 --- a/src/mainboard/google/octopus/variants/casta/variant.c +++ b/src/mainboard/google/octopus/variants/casta/variant.c @@ -21,7 +21,7 @@ const char *get_wifi_sar_cbfs_filename(void) { const char *filename = NULL; - uint32_t sku_id = get_board_sku(); + uint32_t sku_id = google_chromeec_get_board_sku();
if (sku_id == 2) filename = "wifi_sar-bluebird.hex"; @@ -31,7 +31,7 @@
bool variant_ext_usb_status(unsigned int port_type, unsigned int port_id) { - uint32_t sku_id = get_board_sku(); + uint32_t sku_id = google_chromeec_get_board_sku();
if (sku_id == 2 && port_id == RIGHT_USB_C_PORT_ID) return false; diff --git a/src/mainboard/google/octopus/variants/dood/gpio.c b/src/mainboard/google/octopus/variants/dood/gpio.c index 5b567b3..07eeb32 100644 --- a/src/mainboard/google/octopus/variants/dood/gpio.c +++ b/src/mainboard/google/octopus/variants/dood/gpio.c @@ -18,6 +18,7 @@ #include <boardid.h> #include <gpio.h> #include <soc/gpio.h> +#include <ec/google/chromeec/ec.h>
enum { SKU_1_LTE = 1, /* Wifi + LTE */ @@ -58,7 +59,7 @@ const struct pad_config *variant_override_gpio_table(size_t *num) { uint32_t sku_id; - sku_id = get_board_sku(); + sku_id = google_chromeec_get_board_sku();
switch (sku_id) { case SKU_1_LTE: diff --git a/src/mainboard/google/octopus/variants/dood/variant.c b/src/mainboard/google/octopus/variants/dood/variant.c index 7116061..f4b4eaa 100644 --- a/src/mainboard/google/octopus/variants/dood/variant.c +++ b/src/mainboard/google/octopus/variants/dood/variant.c @@ -61,7 +61,7 @@ if (slp_typ != ACPI_S5) return;
- switch (get_board_sku()) { + switch (google_chromeec_get_board_sku()) { case SKU_1_LTE: power_off_lte_module(slp_typ); return; diff --git a/src/mainboard/google/octopus/variants/foob/gpio.c b/src/mainboard/google/octopus/variants/foob/gpio.c index dec2ff5..55f8196 100644 --- a/src/mainboard/google/octopus/variants/foob/gpio.c +++ b/src/mainboard/google/octopus/variants/foob/gpio.c @@ -20,8 +20,6 @@ #include <soc/gpio.h> #include <ec/google/chromeec/ec.h>
-#define SKU_UNKNOWN 0xFFFFFFFF - static const struct pad_config default_override_table[] = { PAD_NC(GPIO_52, UP_20K), PAD_NC(GPIO_53, UP_20K), @@ -70,9 +68,9 @@ const struct pad_config *variant_override_gpio_table(size_t *num) { const struct pad_config *c; - uint32_t sku_id = SKU_UNKNOWN; + uint32_t sku_id;
- sku_id = get_board_sku(); + sku_id = google_chromeec_get_board_sku(); if (no_touchscreen_sku(sku_id)) { c = non_touchscreen_override_table; *num = ARRAY_SIZE(non_touchscreen_override_table); diff --git a/src/mainboard/google/octopus/variants/foob/variant.c b/src/mainboard/google/octopus/variants/foob/variant.c index dcc11dd..47639f6 100644 --- a/src/mainboard/google/octopus/variants/foob/variant.c +++ b/src/mainboard/google/octopus/variants/foob/variant.c @@ -30,7 +30,7 @@ return;
/* SKU ID 1 does not have a touchscreen device, hence disable it. */ - sku_id = get_board_sku(); + sku_id = google_chromeec_get_board_sku(); if (no_touchscreen_sku(sku_id)) touchscreen_i2c_host->enabled = 0; } diff --git a/src/mainboard/google/octopus/variants/garg/gpio.c b/src/mainboard/google/octopus/variants/garg/gpio.c index eeeb466..987c69e 100644 --- a/src/mainboard/google/octopus/variants/garg/gpio.c +++ b/src/mainboard/google/octopus/variants/garg/gpio.c @@ -19,6 +19,7 @@ #include <gpio.h> #include <soc/gpio.h> #include <variant/sku.h> +#include <ec/google/chromeec/ec.h>
static const struct pad_config default_override_table[] = { PAD_NC(GPIO_104, UP_20K), @@ -72,7 +73,7 @@ const struct pad_config *variant_override_gpio_table(size_t *num) { uint32_t sku_id; - sku_id = get_board_sku(); + sku_id = google_chromeec_get_board_sku();
switch (sku_id) { case SKU_9_HDMI: diff --git a/src/mainboard/google/octopus/variants/garg/variant.c b/src/mainboard/google/octopus/variants/garg/variant.c index f5f350a..2afceb9 100644 --- a/src/mainboard/google/octopus/variants/garg/variant.c +++ b/src/mainboard/google/octopus/variants/garg/variant.c @@ -54,7 +54,7 @@ { uint32_t sku_id;
- sku_id = get_board_sku(); + sku_id = google_chromeec_get_board_sku();
switch (sku_id) { case SKU_9_HDMI: @@ -72,7 +72,7 @@ if (slp_typ != ACPI_S5) return;
- switch (get_board_sku()) { + switch (google_chromeec_get_board_sku()) { case SKU_17_LTE: case SKU_18_LTE_TS: power_off_lte_module(slp_typ); diff --git a/src/mainboard/google/octopus/variants/meep/gpio.c b/src/mainboard/google/octopus/variants/meep/gpio.c index 44d9fff..ed4eb05 100644 --- a/src/mainboard/google/octopus/variants/meep/gpio.c +++ b/src/mainboard/google/octopus/variants/meep/gpio.c @@ -18,6 +18,7 @@ #include <gpio.h> #include <soc/gpio.h> #include <variant/sku.h> +#include <ec/google/chromeec/ec.h>
static const struct pad_config default_override_table[] = { PAD_NC(GPIO_104, UP_20K), @@ -44,7 +45,7 @@ const struct pad_config *variant_override_gpio_table(size_t *num) { uint32_t sku_id; - sku_id = get_board_sku(); + sku_id = google_chromeec_get_board_sku();
switch (sku_id) { case SKU_33_DORP: diff --git a/src/mainboard/google/octopus/variants/meep/variant.c b/src/mainboard/google/octopus/variants/meep/variant.c index 20aaa0a..7cd1e47 100644 --- a/src/mainboard/google/octopus/variants/meep/variant.c +++ b/src/mainboard/google/octopus/variants/meep/variant.c @@ -22,7 +22,7 @@ const char *get_wifi_sar_cbfs_filename(void) { const char *filename = NULL; - uint32_t sku_id = get_board_sku(); + uint32_t sku_id = google_chromeec_get_board_sku();
switch (sku_id) { case SKU_1_MEEP: @@ -45,7 +45,7 @@ { uint32_t sku_id;
- sku_id = get_board_sku(); + sku_id = google_chromeec_get_board_sku();
switch (sku_id) { case SKU_33_DORP:
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39035 )
Change subject: mainboard/google/octopus: Migrate onto SKU ID helpers ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39035/1/src/mainboard/google/octopu... File src/mainboard/google/octopus/Kconfig:
https://review.coreboot.org/c/coreboot/+/39035/1/src/mainboard/google/octopu... PS1, Line 121: default 0xff # LEGACY SKU_ID space. Why not put this behind a Kconfig under the ec part that the mainboard just selects the "small id"?
Hello Jett Rink, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39035
to look at the new patch set (#2).
Change subject: mainboard/google/octopus: Migrate onto SKU ID helpers ......................................................................
mainboard/google/octopus: Migrate onto SKU ID helpers
Leverage the common sku id space helper encoders and set the sku id max to 0xff for legacy to ensure we behave the same.
BUG=b:149348474 BRANCH=none TEST=tested on hatch
Change-Id: I60a37a5f9659b8df4018872956f95e07a3506440 Signed-off-by: Edward O'Callaghan quasisec@google.com --- M src/mainboard/google/octopus/Kconfig M src/mainboard/google/octopus/mainboard_misc.c M src/mainboard/google/octopus/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/octopus/variants/bloog/variant.c M src/mainboard/google/octopus/variants/bobba/gpio.c M src/mainboard/google/octopus/variants/bobba/variant.c M src/mainboard/google/octopus/variants/casta/variant.c M src/mainboard/google/octopus/variants/dood/gpio.c M src/mainboard/google/octopus/variants/dood/variant.c M src/mainboard/google/octopus/variants/foob/gpio.c M src/mainboard/google/octopus/variants/foob/variant.c M src/mainboard/google/octopus/variants/garg/gpio.c M src/mainboard/google/octopus/variants/garg/variant.c M src/mainboard/google/octopus/variants/meep/gpio.c M src/mainboard/google/octopus/variants/meep/variant.c 15 files changed, 28 insertions(+), 50 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/39035/2
Hello Jett Rink, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39035
to look at the new patch set (#4).
Change subject: mainboard/google/octopus: Migrate onto SKU ID helpers ......................................................................
mainboard/google/octopus: Migrate onto SKU ID helpers
Leverage the common sku id space helper encoders and set the sku id max to 0xff for legacy to ensure we behave the same.
BUG=b:149348474 BRANCH=none TEST=tested on hatch
Change-Id: I60a37a5f9659b8df4018872956f95e07a3506440 Signed-off-by: Edward O'Callaghan quasisec@google.com --- M src/mainboard/google/octopus/Kconfig M src/mainboard/google/octopus/mainboard_misc.c M src/mainboard/google/octopus/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/octopus/variants/bloog/variant.c M src/mainboard/google/octopus/variants/bobba/gpio.c M src/mainboard/google/octopus/variants/bobba/variant.c M src/mainboard/google/octopus/variants/casta/variant.c M src/mainboard/google/octopus/variants/dood/gpio.c M src/mainboard/google/octopus/variants/dood/variant.c M src/mainboard/google/octopus/variants/foob/gpio.c M src/mainboard/google/octopus/variants/foob/variant.c M src/mainboard/google/octopus/variants/garg/gpio.c M src/mainboard/google/octopus/variants/garg/variant.c M src/mainboard/google/octopus/variants/meep/gpio.c M src/mainboard/google/octopus/variants/meep/variant.c 15 files changed, 24 insertions(+), 50 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/39035/4
Hello Jett Rink, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39035
to look at the new patch set (#9).
Change subject: mainboard/google/octopus: Migrate onto SKU ID helpers ......................................................................
mainboard/google/octopus: Migrate onto SKU ID helpers
Leverage the common sku id space helper encoders and set the sku id max to 0xff for legacy to ensure we behave the same.
BUG=b:149348474 BRANCH=none TEST=tested on hatch
Change-Id: I60a37a5f9659b8df4018872956f95e07a3506440 Signed-off-by: Edward O'Callaghan quasisec@google.com --- M src/mainboard/google/octopus/Kconfig M src/mainboard/google/octopus/Makefile.inc M src/mainboard/google/octopus/mainboard_misc.c M src/mainboard/google/octopus/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/octopus/variants/bloog/variant.c M src/mainboard/google/octopus/variants/bobba/gpio.c M src/mainboard/google/octopus/variants/bobba/variant.c M src/mainboard/google/octopus/variants/casta/variant.c M src/mainboard/google/octopus/variants/dood/gpio.c M src/mainboard/google/octopus/variants/dood/variant.c M src/mainboard/google/octopus/variants/foob/gpio.c M src/mainboard/google/octopus/variants/foob/variant.c M src/mainboard/google/octopus/variants/garg/gpio.c M src/mainboard/google/octopus/variants/garg/variant.c M src/mainboard/google/octopus/variants/meep/gpio.c M src/mainboard/google/octopus/variants/meep/variant.c 16 files changed, 24 insertions(+), 51 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/39035/9
Hello Jett Rink, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39035
to look at the new patch set (#10).
Change subject: mainboard/google/octopus: Migrate onto SKU ID helpers ......................................................................
mainboard/google/octopus: Migrate onto SKU ID helpers
Leverage the common sku id space helper encoders and set the sku id max to 0xff for legacy to ensure we behave the same.
BUG=b:149348474 BRANCH=none TEST=tested on hatch
Change-Id: I60a37a5f9659b8df4018872956f95e07a3506440 Signed-off-by: Edward O'Callaghan quasisec@google.com --- M src/mainboard/google/octopus/Kconfig M src/mainboard/google/octopus/Makefile.inc M src/mainboard/google/octopus/mainboard_misc.c M src/mainboard/google/octopus/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/octopus/variants/bloog/variant.c M src/mainboard/google/octopus/variants/bobba/gpio.c M src/mainboard/google/octopus/variants/bobba/variant.c M src/mainboard/google/octopus/variants/casta/variant.c M src/mainboard/google/octopus/variants/dood/gpio.c M src/mainboard/google/octopus/variants/dood/variant.c M src/mainboard/google/octopus/variants/foob/gpio.c M src/mainboard/google/octopus/variants/foob/variant.c M src/mainboard/google/octopus/variants/garg/gpio.c M src/mainboard/google/octopus/variants/garg/variant.c M src/mainboard/google/octopus/variants/meep/gpio.c M src/mainboard/google/octopus/variants/meep/variant.c 16 files changed, 27 insertions(+), 51 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/39035/10
Hello Jett Rink, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39035
to look at the new patch set (#11).
Change subject: mainboard/google/octopus: Migrate onto SKU ID helpers ......................................................................
mainboard/google/octopus: Migrate onto SKU ID helpers
Leverage the common sku id space helper encoders and set the sku id max to 0xff for legacy to ensure we behave the same.
BUG=b:149348474 BRANCH=none TEST=tested on hatch
Change-Id: I60a37a5f9659b8df4018872956f95e07a3506440 Signed-off-by: Edward O'Callaghan quasisec@google.com --- M src/ec/google/chromeec/Makefile.inc M src/mainboard/google/octopus/Kconfig M src/mainboard/google/octopus/Makefile.inc M src/mainboard/google/octopus/mainboard_misc.c M src/mainboard/google/octopus/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/octopus/variants/bloog/variant.c M src/mainboard/google/octopus/variants/bobba/gpio.c M src/mainboard/google/octopus/variants/bobba/variant.c M src/mainboard/google/octopus/variants/casta/variant.c M src/mainboard/google/octopus/variants/dood/gpio.c M src/mainboard/google/octopus/variants/dood/variant.c M src/mainboard/google/octopus/variants/foob/gpio.c M src/mainboard/google/octopus/variants/foob/variant.c M src/mainboard/google/octopus/variants/garg/gpio.c M src/mainboard/google/octopus/variants/garg/variant.c M src/mainboard/google/octopus/variants/meep/gpio.c M src/mainboard/google/octopus/variants/meep/variant.c 17 files changed, 28 insertions(+), 51 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/39035/11
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39035 )
Change subject: mainboard/google/octopus: Migrate onto SKU ID helpers ......................................................................
Patch Set 11:
Furquan, please note the usage of the symbol in smm here due to differentiation on sku for radios.
Jett Rink has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39035 )
Change subject: mainboard/google/octopus: Migrate onto SKU ID helpers ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39035/11/src/mainboard/google/octop... File src/mainboard/google/octopus/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/39035/11/src/mainboard/google/octop... PS11, Line 13: smm-y += mainboard_misc.c this is the only thing that looks out of place, but this is because of the radio? Can you explain that a little more for me please. I am not as familiar with AP FW
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39035 )
Change subject: mainboard/google/octopus: Migrate onto SKU ID helpers ......................................................................
Patch Set 11: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/39035/11/src/mainboard/google/octop... File src/mainboard/google/octopus/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/39035/11/src/mainboard/google/octop... PS11, Line 13: smm-y += mainboard_misc.c
this is the only thing that looks out of place, but this is because of the radio? Can you explain th […]
Yes, for the lte thing. variant_smi_sleep() is ran in smm where it's switch()ing on sku id.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39035 )
Change subject: mainboard/google/octopus: Migrate onto SKU ID helpers ......................................................................
Patch Set 11:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39035/11/src/mainboard/google/octop... File src/mainboard/google/octopus/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/39035/11/src/mainboard/google/octop... PS11, Line 13: smm-y += mainboard_misc.c
Yes, for the lte thing. variant_smi_sleep() is ran in smm where it's switch()ing on sku id.
Yes, it is used to identify the SKUs which have LTE so that in those SKUs we can toggle few GPIOs to turn off the LTE module when the system enters S5.
https://review.coreboot.org/c/coreboot/+/39035/11/src/mainboard/google/octop... File src/mainboard/google/octopus/mainboard_misc.c:
https://review.coreboot.org/c/coreboot/+/39035/11/src/mainboard/google/octop... PS11, Line 24: const char *smbios_system_sku(void) Nit: This function can be put back into mainboard.c and this file removed. The purpose of introducing this file is gone with the SKU helper.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39035 )
Change subject: mainboard/google/octopus: Migrate onto SKU ID helpers ......................................................................
Patch Set 11:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39035/11/src/mainboard/google/octop... File src/mainboard/google/octopus/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/39035/11/src/mainboard/google/octop... PS11, Line 13: smm-y += mainboard_misc.c
Yes, it is used to identify the SKUs which have LTE so that in those SKUs we can toggle few GPIOs to […]
Done
https://review.coreboot.org/c/coreboot/+/39035/11/src/mainboard/google/octop... File src/mainboard/google/octopus/mainboard_misc.c:
https://review.coreboot.org/c/coreboot/+/39035/11/src/mainboard/google/octop... PS11, Line 24: const char *smbios_system_sku(void)
Nit: This function can be put back into mainboard.c and this file removed. […]
This is already handled in a follow up where we plan to collapse all these wrapping strong symbols down into a single strong symbol from src/ec/google/chromeec/ec_skuid.{o}.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39035 )
Change subject: mainboard/google/octopus: Migrate onto SKU ID helpers ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39035/1/src/mainboard/google/octopu... File src/mainboard/google/octopus/Kconfig:
https://review.coreboot.org/c/coreboot/+/39035/1/src/mainboard/google/octopu... PS1, Line 121: default 0xff # LEGACY SKU_ID space.
Why not put this behind a Kconfig under the ec part that the mainboard just selects the "small id"?
Ack
Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39035 )
Change subject: mainboard/google/octopus: Migrate onto SKU ID helpers ......................................................................
mainboard/google/octopus: Migrate onto SKU ID helpers
Leverage the common sku id space helper encoders and set the sku id max to 0xff for legacy to ensure we behave the same.
BUG=b:149348474 BRANCH=none TEST=tested on hatch
Change-Id: I60a37a5f9659b8df4018872956f95e07a3506440 Signed-off-by: Edward O'Callaghan quasisec@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/39035 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Aaron Durbin adurbin@chromium.org --- M src/ec/google/chromeec/Makefile.inc M src/mainboard/google/octopus/Kconfig M src/mainboard/google/octopus/Makefile.inc M src/mainboard/google/octopus/mainboard_misc.c M src/mainboard/google/octopus/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/octopus/variants/bloog/variant.c M src/mainboard/google/octopus/variants/bobba/gpio.c M src/mainboard/google/octopus/variants/bobba/variant.c M src/mainboard/google/octopus/variants/casta/variant.c M src/mainboard/google/octopus/variants/dood/gpio.c M src/mainboard/google/octopus/variants/dood/variant.c M src/mainboard/google/octopus/variants/foob/gpio.c M src/mainboard/google/octopus/variants/foob/variant.c M src/mainboard/google/octopus/variants/garg/gpio.c M src/mainboard/google/octopus/variants/garg/variant.c M src/mainboard/google/octopus/variants/meep/gpio.c M src/mainboard/google/octopus/variants/meep/variant.c 17 files changed, 28 insertions(+), 51 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved
diff --git a/src/ec/google/chromeec/Makefile.inc b/src/ec/google/chromeec/Makefile.inc index b57333e..2833c87 100644 --- a/src/ec/google/chromeec/Makefile.inc +++ b/src/ec/google/chromeec/Makefile.inc @@ -6,6 +6,7 @@ ramstage-$(CONFIG_EC_GOOGLE_CHROMEEC_BOARDID) += ec_boardid.c smm-$(CONFIG_EC_GOOGLE_CHROMEEC_BOARDID) += ec_boardid.c
+smm-$(CONFIG_EC_GOOGLE_CHROMEEC_SKUID) += ec_skuid.c romstage-$(CONFIG_EC_GOOGLE_CHROMEEC_SKUID) += ec_skuid.c ramstage-$(CONFIG_EC_GOOGLE_CHROMEEC_SKUID) += ec_skuid.c
diff --git a/src/mainboard/google/octopus/Kconfig b/src/mainboard/google/octopus/Kconfig index a4e4982..e52e994 100644 --- a/src/mainboard/google/octopus/Kconfig +++ b/src/mainboard/google/octopus/Kconfig @@ -13,6 +13,7 @@ select DRIVERS_USB_ACPI select EC_GOOGLE_CHROMEEC select EC_GOOGLE_CHROMEEC_BOARDID + select EC_GOOGLE_CHROMEEC_SKUID select EC_GOOGLE_CHROMEEC_ESPI select HAVE_ACPI_RESUME select HAVE_ACPI_TABLES diff --git a/src/mainboard/google/octopus/Makefile.inc b/src/mainboard/google/octopus/Makefile.inc index d36d5f7..b8a7366 100644 --- a/src/mainboard/google/octopus/Makefile.inc +++ b/src/mainboard/google/octopus/Makefile.inc @@ -10,7 +10,6 @@
verstage-$(CONFIG_CHROMEOS) += chromeos.c smm-y += smihandler.c -smm-y += mainboard_misc.c
subdirs-y += variants/baseboard CPPFLAGS_common += -I$(src)/mainboard/$(MAINBOARDDIR)/variants/baseboard/include diff --git a/src/mainboard/google/octopus/mainboard_misc.c b/src/mainboard/google/octopus/mainboard_misc.c index 3672f66..8b281da 100644 --- a/src/mainboard/google/octopus/mainboard_misc.c +++ b/src/mainboard/google/octopus/mainboard_misc.c @@ -21,34 +21,7 @@ #include <smbios.h> #include <string.h>
-#define SKU_UNKNOWN 0xFFFFFFFF -#define SKU_MAX 255 - -uint32_t get_board_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) { - static char sku_str[7]; /* sku{0..255} */ - uint32_t sku_id = get_board_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(); } diff --git a/src/mainboard/google/octopus/variants/baseboard/include/baseboard/variants.h b/src/mainboard/google/octopus/variants/baseboard/include/baseboard/variants.h index 2132db5..bf08a85 100644 --- a/src/mainboard/google/octopus/variants/baseboard/include/baseboard/variants.h +++ b/src/mainboard/google/octopus/variants/baseboard/include/baseboard/variants.h @@ -35,8 +35,6 @@ const struct lpddr4_cfg *variant_lpddr4_config(void); /* Return memory SKU for the board. */ size_t variant_memory_sku(void); -/* Return board SKU */ -uint32_t get_board_sku(void); /* Return ChromeOS gpio table and fill in number of entries. */ const struct cros_gpio *variant_cros_gpios(size_t *num);
diff --git a/src/mainboard/google/octopus/variants/bloog/variant.c b/src/mainboard/google/octopus/variants/bloog/variant.c index 6c85e50..05a1542 100644 --- a/src/mainboard/google/octopus/variants/bloog/variant.c +++ b/src/mainboard/google/octopus/variants/bloog/variant.c @@ -42,7 +42,7 @@ const char *get_wifi_sar_cbfs_filename(void) { const char *filename = NULL; - uint32_t sku_id = get_board_sku(); + uint32_t sku_id = google_chromeec_get_board_sku();
switch (sku_id) { case SKU_33_BLOOG: @@ -67,7 +67,7 @@ { uint32_t sku_id;
- sku_id = get_board_sku(); + sku_id = google_chromeec_get_board_sku();
switch (sku_id) { case SKU_33_BLOOG: diff --git a/src/mainboard/google/octopus/variants/bobba/gpio.c b/src/mainboard/google/octopus/variants/bobba/gpio.c index 7c522c7..dd10840 100644 --- a/src/mainboard/google/octopus/variants/bobba/gpio.c +++ b/src/mainboard/google/octopus/variants/bobba/gpio.c @@ -18,6 +18,7 @@ #include <boardid.h> #include <gpio.h> #include <soc/gpio.h> +#include <ec/google/chromeec/ec.h>
enum { SKU_37_DROID = 37, /* LTE */ @@ -60,7 +61,7 @@ const struct pad_config *variant_override_gpio_table(size_t *num) { uint32_t sku_id; - sku_id = get_board_sku(); + sku_id = google_chromeec_get_board_sku();
switch (sku_id) { case SKU_37_DROID: diff --git a/src/mainboard/google/octopus/variants/bobba/variant.c b/src/mainboard/google/octopus/variants/bobba/variant.c index 1f6e80d..03eacbf 100644 --- a/src/mainboard/google/octopus/variants/bobba/variant.c +++ b/src/mainboard/google/octopus/variants/bobba/variant.c @@ -19,6 +19,7 @@ #include <baseboard/variants.h> #include <delay.h> #include <gpio.h> +#include <ec/google/chromeec/ec.h>
enum { SKU_37_DROID = 37, /* LTE */ @@ -58,7 +59,7 @@ const char *get_wifi_sar_cbfs_filename(void) { const char *filename = NULL; - uint32_t sku_id = get_board_sku(); + uint32_t sku_id = google_chromeec_get_board_sku();
if (sku_id == 33 || sku_id == 34 || sku_id == 35 || sku_id == 36 || sku_id == 41 || sku_id == 42 || sku_id == 43 || sku_id == 44) @@ -74,7 +75,7 @@ if (slp_typ != ACPI_S5) return;
- switch (get_board_sku()) { + switch (google_chromeec_get_board_sku()) { case SKU_37_DROID: case SKU_38_DROID: case SKU_39_DROID: diff --git a/src/mainboard/google/octopus/variants/casta/variant.c b/src/mainboard/google/octopus/variants/casta/variant.c index 12c8dd7..4b1e42d 100644 --- a/src/mainboard/google/octopus/variants/casta/variant.c +++ b/src/mainboard/google/octopus/variants/casta/variant.c @@ -13,6 +13,7 @@ * GNU General Public License for more details. */
+#include <ec/google/chromeec/ec.h> #include <baseboard/variants.h> #include <sar.h>
@@ -21,7 +22,7 @@ const char *get_wifi_sar_cbfs_filename(void) { const char *filename = NULL; - uint32_t sku_id = get_board_sku(); + uint32_t sku_id = google_chromeec_get_board_sku();
if (sku_id == 2) filename = "wifi_sar-bluebird.hex"; @@ -31,7 +32,7 @@
bool variant_ext_usb_status(unsigned int port_type, unsigned int port_id) { - uint32_t sku_id = get_board_sku(); + uint32_t sku_id = google_chromeec_get_board_sku();
if (sku_id == 2 && port_id == RIGHT_USB_C_PORT_ID) return false; diff --git a/src/mainboard/google/octopus/variants/dood/gpio.c b/src/mainboard/google/octopus/variants/dood/gpio.c index 96b8ac0..3d09a39 100644 --- a/src/mainboard/google/octopus/variants/dood/gpio.c +++ b/src/mainboard/google/octopus/variants/dood/gpio.c @@ -18,6 +18,7 @@ #include <boardid.h> #include <gpio.h> #include <soc/gpio.h> +#include <ec/google/chromeec/ec.h>
enum { SKU_1_LTE = 1, /* Wifi + LTE */ @@ -60,7 +61,7 @@ const struct pad_config *variant_override_gpio_table(size_t *num) { uint32_t sku_id; - sku_id = get_board_sku(); + sku_id = google_chromeec_get_board_sku();
switch (sku_id) { case SKU_1_LTE: diff --git a/src/mainboard/google/octopus/variants/dood/variant.c b/src/mainboard/google/octopus/variants/dood/variant.c index 694e190..b54b9fa 100644 --- a/src/mainboard/google/octopus/variants/dood/variant.c +++ b/src/mainboard/google/octopus/variants/dood/variant.c @@ -19,6 +19,7 @@ #include <baseboard/variants.h> #include <delay.h> #include <gpio.h> +#include <ec/google/chromeec/ec.h>
enum { SKU_1_LTE = 1, /* Wifi + LTE */ @@ -63,7 +64,7 @@ if (slp_typ != ACPI_S5) return;
- switch (get_board_sku()) { + switch (google_chromeec_get_board_sku()) { case SKU_1_LTE: case SKU_3_LTE_2CAM: power_off_lte_module(slp_typ); diff --git a/src/mainboard/google/octopus/variants/foob/gpio.c b/src/mainboard/google/octopus/variants/foob/gpio.c index dec2ff5..55f8196 100644 --- a/src/mainboard/google/octopus/variants/foob/gpio.c +++ b/src/mainboard/google/octopus/variants/foob/gpio.c @@ -20,8 +20,6 @@ #include <soc/gpio.h> #include <ec/google/chromeec/ec.h>
-#define SKU_UNKNOWN 0xFFFFFFFF - static const struct pad_config default_override_table[] = { PAD_NC(GPIO_52, UP_20K), PAD_NC(GPIO_53, UP_20K), @@ -70,9 +68,9 @@ const struct pad_config *variant_override_gpio_table(size_t *num) { const struct pad_config *c; - uint32_t sku_id = SKU_UNKNOWN; + uint32_t sku_id;
- sku_id = get_board_sku(); + sku_id = google_chromeec_get_board_sku(); if (no_touchscreen_sku(sku_id)) { c = non_touchscreen_override_table; *num = ARRAY_SIZE(non_touchscreen_override_table); diff --git a/src/mainboard/google/octopus/variants/foob/variant.c b/src/mainboard/google/octopus/variants/foob/variant.c index dcc11dd..47639f6 100644 --- a/src/mainboard/google/octopus/variants/foob/variant.c +++ b/src/mainboard/google/octopus/variants/foob/variant.c @@ -30,7 +30,7 @@ return;
/* SKU ID 1 does not have a touchscreen device, hence disable it. */ - sku_id = get_board_sku(); + sku_id = google_chromeec_get_board_sku(); if (no_touchscreen_sku(sku_id)) touchscreen_i2c_host->enabled = 0; } diff --git a/src/mainboard/google/octopus/variants/garg/gpio.c b/src/mainboard/google/octopus/variants/garg/gpio.c index eeeb466..987c69e 100644 --- a/src/mainboard/google/octopus/variants/garg/gpio.c +++ b/src/mainboard/google/octopus/variants/garg/gpio.c @@ -19,6 +19,7 @@ #include <gpio.h> #include <soc/gpio.h> #include <variant/sku.h> +#include <ec/google/chromeec/ec.h>
static const struct pad_config default_override_table[] = { PAD_NC(GPIO_104, UP_20K), @@ -72,7 +73,7 @@ const struct pad_config *variant_override_gpio_table(size_t *num) { uint32_t sku_id; - sku_id = get_board_sku(); + sku_id = google_chromeec_get_board_sku();
switch (sku_id) { case SKU_9_HDMI: diff --git a/src/mainboard/google/octopus/variants/garg/variant.c b/src/mainboard/google/octopus/variants/garg/variant.c index f5f350a..2afceb9 100644 --- a/src/mainboard/google/octopus/variants/garg/variant.c +++ b/src/mainboard/google/octopus/variants/garg/variant.c @@ -54,7 +54,7 @@ { uint32_t sku_id;
- sku_id = get_board_sku(); + sku_id = google_chromeec_get_board_sku();
switch (sku_id) { case SKU_9_HDMI: @@ -72,7 +72,7 @@ if (slp_typ != ACPI_S5) return;
- switch (get_board_sku()) { + switch (google_chromeec_get_board_sku()) { case SKU_17_LTE: case SKU_18_LTE_TS: power_off_lte_module(slp_typ); diff --git a/src/mainboard/google/octopus/variants/meep/gpio.c b/src/mainboard/google/octopus/variants/meep/gpio.c index 44d9fff..ed4eb05 100644 --- a/src/mainboard/google/octopus/variants/meep/gpio.c +++ b/src/mainboard/google/octopus/variants/meep/gpio.c @@ -18,6 +18,7 @@ #include <gpio.h> #include <soc/gpio.h> #include <variant/sku.h> +#include <ec/google/chromeec/ec.h>
static const struct pad_config default_override_table[] = { PAD_NC(GPIO_104, UP_20K), @@ -44,7 +45,7 @@ const struct pad_config *variant_override_gpio_table(size_t *num) { uint32_t sku_id; - sku_id = get_board_sku(); + sku_id = google_chromeec_get_board_sku();
switch (sku_id) { case SKU_33_DORP: diff --git a/src/mainboard/google/octopus/variants/meep/variant.c b/src/mainboard/google/octopus/variants/meep/variant.c index 20aaa0a..7cd1e47 100644 --- a/src/mainboard/google/octopus/variants/meep/variant.c +++ b/src/mainboard/google/octopus/variants/meep/variant.c @@ -22,7 +22,7 @@ const char *get_wifi_sar_cbfs_filename(void) { const char *filename = NULL; - uint32_t sku_id = get_board_sku(); + uint32_t sku_id = google_chromeec_get_board_sku();
switch (sku_id) { case SKU_1_MEEP: @@ -45,7 +45,7 @@ { uint32_t sku_id;
- sku_id = get_board_sku(); + sku_id = google_chromeec_get_board_sku();
switch (sku_id) { case SKU_33_DORP:
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39035 )
Change subject: mainboard/google/octopus: Migrate onto SKU ID helpers ......................................................................
Patch Set 13:
Automatic boot test returned (PASS/FAIL/TOTAL): 3/0/3 Emulation targets: EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/1084 EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/1083 EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/1082
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Aaron Durbin (Code Review)
-
Edward O'Callaghan (Code Review)
-
Jett Rink (Code Review)
-
Karthik Ramasubramanian (Code Review)