Bhanu Prakash Maiya has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44422 )
Change subject: mb/google/zork: Remove validity checks for FW_CONFIG in CBI ......................................................................
mb/google/zork: Remove validity checks for FW_CONFIG in CBI
After confirming that all zork variants and phases have valid FW_CONFIG value in CBI, this patch is dropping FW_CONFIG validity checks like VARIANT_HAS_FW_CONFIG and VARIANT_BOARD_VER_FW_CONFIG_VALID in Kconfig and will also remove associated helper functions. Add a helper function to get daughterboard_id from FW_CONIFG.
BRANCH=none BUG=b:162344105,b:152817444 TEST=Check if FW_CONFIG bits can be read in coreboot and FW_CONIFG helper function do not return 0 if board has a valid FW_CONFIG in CBI.
Signed-off-by: Bhanu Prakash Maiya bhanumaiya@google.com Change-Id: I633dc7c500ef8759f3fffb0db6b76d96257c3c9a --- M src/mainboard/google/zork/Kconfig M src/mainboard/google/zork/variants/baseboard/helpers.c M src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h 3 files changed, 7 insertions(+), 40 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/44422/1
diff --git a/src/mainboard/google/zork/Kconfig b/src/mainboard/google/zork/Kconfig index bf2fe2e..3a17a38 100644 --- a/src/mainboard/google/zork/Kconfig +++ b/src/mainboard/google/zork/Kconfig @@ -143,18 +143,6 @@ Location of the AMD firmware in the RW_B region. This is the start of the RW-A region + 64 bytes for the cbfs header.
-config VARIANT_HAS_FW_CONFIG - bool - help - Honor FW_CONFIG in CBI. - -config VARIANT_BOARD_VER_FW_CONFIG_VALID - int - default 256 - depends on VARIANT_HAS_FW_CONFIG - help - Which board version did FW_CONFIG become valid in CBI. - config VARIANT_SUPPORTS_PRE_V3_SCHEMATICS bool default y if BOARD_GOOGLE_TREMBYLE diff --git a/src/mainboard/google/zork/variants/baseboard/helpers.c b/src/mainboard/google/zork/variants/baseboard/helpers.c index 7dc9fd3..d95ab82 100644 --- a/src/mainboard/google/zork/variants/baseboard/helpers.c +++ b/src/mainboard/google/zork/variants/baseboard/helpers.c @@ -48,36 +48,10 @@ FW_CONFIG_SHIFT_FAN = 27, };
-int variant_fw_config_valid(void) -{ - static uint32_t board_version; - const uint32_t bv_valid = CONFIG_VARIANT_BOARD_VER_FW_CONFIG_VALID; - - if (!CONFIG(VARIANT_HAS_FW_CONFIG)) - return 0; - - /* Fast path for non-zero board version. */ - if (board_version >= bv_valid) - return 1; - - if (google_chromeec_cbi_get_board_version(&board_version)) { - printk(BIOS_ERR, "Unable to obtain board version for FW_CONFIG\n"); - return 0; - } - - if (board_version >= bv_valid) - return 1; - - return 0; -} - static int get_fw_config(uint32_t *val) { static uint32_t known_value;
- if (!variant_fw_config_valid()) - return -1; - if (known_value) { *val = known_value; return 0; @@ -170,3 +144,8 @@
return true; } + +int variant_get_daughterboard_id(void) +{ + return extract_field(FW_CONFIG_MASK_DB_INDEX, FW_CONFIG_DB_INDEX_SHIFT); +} diff --git a/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h b/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h index 93aad0f..bedcb0d 100644 --- a/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h +++ b/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h @@ -56,8 +56,6 @@ const fsp_ddi_descriptor *baseboard_get_ddi_descriptors(size_t *num);
/* Retrieve attributes from FW_CONFIG in CBI. */ -/* Return 1 if FW_CONFIG expected to be valid, else 0. */ -int variant_fw_config_valid(void); /* Return 0 if non-existent, 1 if present. */ int variant_has_emmc(void); /* Return 0 if non-existent, 1 if present. */ @@ -74,5 +72,7 @@ bool variant_uses_codec_gpi(void); /* Return true if variant has active low power enable fow WiFi. */ bool variant_has_active_low_wifi_power(void); +/* Return value of daughterboard ID */ +int variant_get_daughterboard_id(void);
#endif /* __BASEBOARD_VARIANTS_H__ */
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44422 )
Change subject: mb/google/zork: Remove validity checks for FW_CONFIG in CBI ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44422/1/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/helpers.c:
https://review.coreboot.org/c/coreboot/+/44422/1/src/mainboard/google/zork/v... PS1, Line 150: return extract_field(FW_CONFIG_MASK_DB_INDEX, FW_CONFIG_DB_INDEX_SHIFT); This change appears to be unrelated to the subject of this particular patch. Please put this in its own patch.
Hello build bot (Jenkins), Furquan Shaikh, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44422
to look at the new patch set (#2).
Change subject: mb/google/zork: Remove validity checks for FW_CONFIG in CBI ......................................................................
mb/google/zork: Remove validity checks for FW_CONFIG in CBI
After confirming that all zork variants and phases have valid FW_CONFIG value in CBI, this patch is dropping FW_CONFIG validity checks like VARIANT_HAS_FW_CONFIG and VARIANT_BOARD_VER_FW_CONFIG_VALID in Kconfig and will also remove associated helper functions.
BRANCH=none BUG=b:162344105,b:152817444 TEST=Check if FW_CONFIG bits can be read in coreboot and FW_CONIFG helper function do not return 0 if board has a valid FW_CONFIG in CBI.
Signed-off-by: Bhanu Prakash Maiya bhanumaiya@google.com Change-Id: I633dc7c500ef8759f3fffb0db6b76d96257c3c9a --- M src/mainboard/google/zork/Kconfig M src/mainboard/google/zork/variants/baseboard/helpers.c M src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h 3 files changed, 0 insertions(+), 40 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/44422/2
Bhanu Prakash Maiya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44422 )
Change subject: mb/google/zork: Remove validity checks for FW_CONFIG in CBI ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44422/1/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/helpers.c:
https://review.coreboot.org/c/coreboot/+/44422/1/src/mainboard/google/zork/v... PS1, Line 150: return extract_field(FW_CONFIG_MASK_DB_INDEX, FW_CONFIG_DB_INDEX_SHIFT);
This change appears to be unrelated to the subject of this particular patch. […]
Done
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44422 )
Change subject: mb/google/zork: Remove validity checks for FW_CONFIG in CBI ......................................................................
Patch Set 2: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44422 )
Change subject: mb/google/zork: Remove validity checks for FW_CONFIG in CBI ......................................................................
Patch Set 2: Code-Review+2
Aaron Durbin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44422 )
Change subject: mb/google/zork: Remove validity checks for FW_CONFIG in CBI ......................................................................
mb/google/zork: Remove validity checks for FW_CONFIG in CBI
After confirming that all zork variants and phases have valid FW_CONFIG value in CBI, this patch is dropping FW_CONFIG validity checks like VARIANT_HAS_FW_CONFIG and VARIANT_BOARD_VER_FW_CONFIG_VALID in Kconfig and will also remove associated helper functions.
BRANCH=none BUG=b:162344105,b:152817444 TEST=Check if FW_CONFIG bits can be read in coreboot and FW_CONIFG helper function do not return 0 if board has a valid FW_CONFIG in CBI.
Signed-off-by: Bhanu Prakash Maiya bhanumaiya@google.com Change-Id: I633dc7c500ef8759f3fffb0db6b76d96257c3c9a Reviewed-on: https://review.coreboot.org/c/coreboot/+/44422 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Aaron Durbin adurbin@chromium.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/mainboard/google/zork/Kconfig M src/mainboard/google/zork/variants/baseboard/helpers.c M src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h 3 files changed, 0 insertions(+), 40 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved Furquan Shaikh: Looks good to me, approved
diff --git a/src/mainboard/google/zork/Kconfig b/src/mainboard/google/zork/Kconfig index 1e1b790..c4a8f33 100644 --- a/src/mainboard/google/zork/Kconfig +++ b/src/mainboard/google/zork/Kconfig @@ -143,18 +143,6 @@ Location of the AMD firmware in the RW_B region. This is the start of the RW-A region + 64 bytes for the cbfs header.
-config VARIANT_HAS_FW_CONFIG - bool - help - Honor FW_CONFIG in CBI. - -config VARIANT_BOARD_VER_FW_CONFIG_VALID - int - default 256 - depends on VARIANT_HAS_FW_CONFIG - help - Which board version did FW_CONFIG become valid in CBI. - config VARIANT_SUPPORTS_PRE_V3_SCHEMATICS bool default y if BOARD_GOOGLE_TREMBYLE diff --git a/src/mainboard/google/zork/variants/baseboard/helpers.c b/src/mainboard/google/zork/variants/baseboard/helpers.c index 7dc9fd3..3f2b068 100644 --- a/src/mainboard/google/zork/variants/baseboard/helpers.c +++ b/src/mainboard/google/zork/variants/baseboard/helpers.c @@ -48,36 +48,10 @@ FW_CONFIG_SHIFT_FAN = 27, };
-int variant_fw_config_valid(void) -{ - static uint32_t board_version; - const uint32_t bv_valid = CONFIG_VARIANT_BOARD_VER_FW_CONFIG_VALID; - - if (!CONFIG(VARIANT_HAS_FW_CONFIG)) - return 0; - - /* Fast path for non-zero board version. */ - if (board_version >= bv_valid) - return 1; - - if (google_chromeec_cbi_get_board_version(&board_version)) { - printk(BIOS_ERR, "Unable to obtain board version for FW_CONFIG\n"); - return 0; - } - - if (board_version >= bv_valid) - return 1; - - return 0; -} - static int get_fw_config(uint32_t *val) { static uint32_t known_value;
- if (!variant_fw_config_valid()) - return -1; - if (known_value) { *val = known_value; return 0; diff --git a/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h b/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h index 93aad0f..98ba565 100644 --- a/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h +++ b/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h @@ -56,8 +56,6 @@ const fsp_ddi_descriptor *baseboard_get_ddi_descriptors(size_t *num);
/* Retrieve attributes from FW_CONFIG in CBI. */ -/* Return 1 if FW_CONFIG expected to be valid, else 0. */ -int variant_fw_config_valid(void); /* Return 0 if non-existent, 1 if present. */ int variant_has_emmc(void); /* Return 0 if non-existent, 1 if present. */
-
Aaron Durbin (Code Review) -
Bhanu Prakash Maiya (Code Review)
-
Furquan Shaikh (Code Review)