Change in coreboot[master]: mb/google/zork: Use FW_CONFIG to enable/disable eMMC on Dalboz
Bhanu Prakash Maiya has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44421 )
Change subject: mb/google/zork: Use FW_CONFIG to enable/disable eMMC on Dalboz ......................................................................
mb/google/zork: Use FW_CONFIG to enable/disable eMMC on Dalboz
Currently sku_id is used to enable/disable eMMC as boot media on Dalboz. This patch will check eMMC bit in firmware configuration table to enable/disable eMMC. On Dalboz Proto and EVT devices with eMMC, there was an issue found after SMT. This patch checks for board_version instead of SKU_ID to configure eMMC in HS200. Configure HDMI based on daughterboard_id in FW_CONFIG.
BRANCH=none BUG=b:152817444 TEST=Check eMMC is enabled or disabled based on the eMMC bit in FW_CONFIG.
Signed-off-by: Bhanu Prakash Maiya bhanumaiya@google.com Change-Id: Ifa2a49a754d85fb6269f788c970bd9da58af1dad --- M src/mainboard/google/zork/variants/dalboz/variant.c 1 file changed, 19 insertions(+), 23 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/44421/1
diff --git a/src/mainboard/google/zork/variants/dalboz/variant.c b/src/mainboard/google/zork/variants/dalboz/variant.c index de4338e..1dab5fe 100644 --- a/src/mainboard/google/zork/variants/dalboz/variant.c +++ b/src/mainboard/google/zork/variants/dalboz/variant.c @@ -10,6 +10,8 @@ #include <string.h>
#define EC_PNP_ID 0x0c09 +#define DALBOZ_DB_USBC 0x0 +#define DALBOZ_DB_HDMI 0x1
/* Look for an EC device of type PNP with id 0x0c09 */ static bool match_ec_dev(DEVTREE_CONST struct device *dev) @@ -104,30 +106,22 @@ cfg->remote_bus = 5; }
-static int sku_has_emmc(void) -{ - uint32_t board_sku = sku_id(); - - /* Factory flow requires all OS boot media to be enabled. */ - if (boot_is_factory_unprovisioned()) - return 1; - - /* FIXME: This needs to be fw_config controlled. */ - /* Enable emmc0 for unknown skus. Only sku3/0xC really has it. */ - if (board_sku == 0x5A80000C || board_sku == 0x5A800003 || board_sku == CROS_SKU_UNKNOWN) - return 1; - - return 0; -} - void variant_devtree_update(void) { + uint32_t board_version; struct soc_amd_picasso_config *cfg;
cfg = config_of_soc();
- if (sku_has_emmc()) { - if ((sku_id() == 0x5A800003) || (sku_id() == 0x5A80000C)) { + /* + * If CBI board version cannot be read, assume this is an older revision + * of hardware. + */ + if (google_chromeec_cbi_get_board_version(&board_version) != 0) + board_version = 1; + + if (variant_has_emmc() || boot_is_factory_unprovisioned()) { + if (board_version <= 2) { /* * rev0 and rev1 boards have issues with HS400 * @@ -196,14 +190,16 @@ const fsp_ddi_descriptor **ddi_descs, size_t *ddi_num) { - uint32_t board_sku = sku_id(); + uint32_t daughterboard_id = variant_get_daughterboard_id();
*dxio_descs = baseboard_get_dxio_descriptors(dxio_num);
- /* SKU 1, A, and D DB have HDMI, as well as unknown */ - /* FIXME: this needs to be fw_config controlled. */ - if ((board_sku == 0x5A80000A) || (board_sku == 0x5A80000D) || (board_sku == 0x5A800001) - || (board_sku == CROS_SKU_UNKNOWN)) { + /* + * SKU# ending 1, A, and D DB have HDMI and enable for + * unprovisioned boards. + */ + if ((daughterboard_id == DALBOZ_DB_HDMI) || + boot_is_factory_unprovisioned()) { *ddi_descs = &hdmi_ddi_descriptors[0]; *ddi_num = ARRAY_SIZE(hdmi_ddi_descriptors); } else {
Bhanu Prakash Maiya has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/44421 )
Change subject: mb/google/zork: Use FW_CONFIG to enable/disable eMMC on Dalboz ......................................................................
Abandoned
Bhanu Prakash Maiya has restored this change. ( https://review.coreboot.org/c/coreboot/+/44421 )
Change subject: mb/google/zork: Use FW_CONFIG to enable/disable eMMC on Dalboz ......................................................................
Restored
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44421 )
Change subject: mb/google/zork: Use FW_CONFIG to enable/disable eMMC on Dalboz ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/44421/2/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/dalboz/variant.c:
https://review.coreboot.org/c/coreboot/+/44421/2/src/mainboard/google/zork/v... PS2, Line 199: * unprovisioned boards. same line?
https://review.coreboot.org/c/coreboot/+/44421/2/src/mainboard/google/zork/v... PS2, Line 202: boot_is_factory_unprovisioned()) { same line
Hello build bot (Jenkins), Furquan Shaikh, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44421
to look at the new patch set (#3).
Change subject: mb/google/zork: Use FW_CONFIG to enable/disable eMMC on Dalboz ......................................................................
mb/google/zork: Use FW_CONFIG to enable/disable eMMC on Dalboz
Currently sku_id is used to enable/disable eMMC as boot media on Dalboz. This patch will check eMMC bit in firmware configuration table to enable/disable eMMC. On Dalboz Proto and EVT devices with eMMC, there was an issue found after SMT. This patch checks for board_version instead of SKU_ID to configure eMMC in HS200. Configure HDMI based on daughterboard_id in FW_CONFIG.
BRANCH=none BUG=b:152817444 TEST=Check eMMC is enabled or disabled based on the eMMC bit in FW_CONFIG.
Signed-off-by: Bhanu Prakash Maiya bhanumaiya@google.com Change-Id: Ifa2a49a754d85fb6269f788c970bd9da58af1dad --- M src/mainboard/google/zork/variants/dalboz/variant.c 1 file changed, 18 insertions(+), 23 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/44421/3
Bhanu Prakash Maiya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44421 )
Change subject: mb/google/zork: Use FW_CONFIG to enable/disable eMMC on Dalboz ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/44421/2/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/dalboz/variant.c:
https://review.coreboot.org/c/coreboot/+/44421/2/src/mainboard/google/zork/v... PS2, Line 199: * unprovisioned boards.
same line?
Ack
https://review.coreboot.org/c/coreboot/+/44421/2/src/mainboard/google/zork/v... PS2, Line 202: boot_is_factory_unprovisioned()) {
same line
Ack
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44421 )
Change subject: mb/google/zork: Use FW_CONFIG to enable/disable eMMC on Dalboz ......................................................................
Patch Set 3: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44421 )
Change subject: mb/google/zork: Use FW_CONFIG to enable/disable eMMC on Dalboz ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44421/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44421/3//COMMIT_MSG@7 PS3, Line 7: Use FW_CONFIG to enable/disable eMMC on Dalboz nit: It might be better to have a summary that says "Switch to using FW_CONFIG instead of SKU_ID" since the change affects MMC as well as HDMI.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44421 )
Change subject: mb/google/zork: Use FW_CONFIG to enable/disable eMMC on Dalboz ......................................................................
Patch Set 3: Code-Review+2
Hello build bot (Jenkins), Furquan Shaikh, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44421
to look at the new patch set (#5).
Change subject: mb/google/zork: Switch to using FW_CONFIG instead of SKU_ID ......................................................................
mb/google/zork: Switch to using FW_CONFIG instead of SKU_ID
Currently sku_id is used to enable/disable eMMC as boot media on Dalboz. This patch will check eMMC bit in firmware configuration table to enable/disable eMMC. On Dalboz Proto and EVT devices with eMMC, there was an issue found after SMT. This patch checks for board_version instead of SKU_ID to configure eMMC in HS200. Configure HDMI based on daughterboard_id in FW_CONFIG.
BRANCH=none BUG=b:152817444 TEST=Check eMMC is enabled or disabled based on the eMMC bit in FW_CONFIG.
Signed-off-by: Bhanu Prakash Maiya bhanumaiya@google.com Change-Id: Ifa2a49a754d85fb6269f788c970bd9da58af1dad --- M src/mainboard/google/zork/variants/dalboz/variant.c 1 file changed, 18 insertions(+), 23 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/44421/5
Bhanu Prakash Maiya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44421 )
Change subject: mb/google/zork: Switch to using FW_CONFIG instead of SKU_ID ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44421/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44421/3//COMMIT_MSG@7 PS3, Line 7: Use FW_CONFIG to enable/disable eMMC on Dalboz
nit: It might be better to have a summary that says "Switch to using FW_CONFIG instead of SKU_ID" si […]
Ack
Aaron Durbin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44421 )
Change subject: mb/google/zork: Switch to using FW_CONFIG instead of SKU_ID ......................................................................
mb/google/zork: Switch to using FW_CONFIG instead of SKU_ID
Currently sku_id is used to enable/disable eMMC as boot media on Dalboz. This patch will check eMMC bit in firmware configuration table to enable/disable eMMC. On Dalboz Proto and EVT devices with eMMC, there was an issue found after SMT. This patch checks for board_version instead of SKU_ID to configure eMMC in HS200. Configure HDMI based on daughterboard_id in FW_CONFIG.
BRANCH=none BUG=b:152817444 TEST=Check eMMC is enabled or disabled based on the eMMC bit in FW_CONFIG.
Signed-off-by: Bhanu Prakash Maiya bhanumaiya@google.com Change-Id: Ifa2a49a754d85fb6269f788c970bd9da58af1dad Reviewed-on: https://review.coreboot.org/c/coreboot/+/44421 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/variants/dalboz/variant.c 1 file changed, 18 insertions(+), 23 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/variants/dalboz/variant.c b/src/mainboard/google/zork/variants/dalboz/variant.c index de4338e..5138782 100644 --- a/src/mainboard/google/zork/variants/dalboz/variant.c +++ b/src/mainboard/google/zork/variants/dalboz/variant.c @@ -10,6 +10,8 @@ #include <string.h>
#define EC_PNP_ID 0x0c09 +#define DALBOZ_DB_USBC 0x0 +#define DALBOZ_DB_HDMI 0x1
/* Look for an EC device of type PNP with id 0x0c09 */ static bool match_ec_dev(DEVTREE_CONST struct device *dev) @@ -104,30 +106,22 @@ cfg->remote_bus = 5; }
-static int sku_has_emmc(void) -{ - uint32_t board_sku = sku_id(); - - /* Factory flow requires all OS boot media to be enabled. */ - if (boot_is_factory_unprovisioned()) - return 1; - - /* FIXME: This needs to be fw_config controlled. */ - /* Enable emmc0 for unknown skus. Only sku3/0xC really has it. */ - if (board_sku == 0x5A80000C || board_sku == 0x5A800003 || board_sku == CROS_SKU_UNKNOWN) - return 1; - - return 0; -} - void variant_devtree_update(void) { + uint32_t board_version; struct soc_amd_picasso_config *cfg;
cfg = config_of_soc();
- if (sku_has_emmc()) { - if ((sku_id() == 0x5A800003) || (sku_id() == 0x5A80000C)) { + /* + * If CBI board version cannot be read, assume this is an older revision + * of hardware. + */ + if (google_chromeec_cbi_get_board_version(&board_version) != 0) + board_version = 1; + + if (variant_has_emmc() || boot_is_factory_unprovisioned()) { + if (board_version <= 2) { /* * rev0 and rev1 boards have issues with HS400 * @@ -196,14 +190,15 @@ const fsp_ddi_descriptor **ddi_descs, size_t *ddi_num) { - uint32_t board_sku = sku_id(); + uint32_t daughterboard_id = variant_get_daughterboard_id();
*dxio_descs = baseboard_get_dxio_descriptors(dxio_num);
- /* SKU 1, A, and D DB have HDMI, as well as unknown */ - /* FIXME: this needs to be fw_config controlled. */ - if ((board_sku == 0x5A80000A) || (board_sku == 0x5A80000D) || (board_sku == 0x5A800001) - || (board_sku == CROS_SKU_UNKNOWN)) { + /* + * Get daughterboard id from FW_CONFIG and configure descriptors accordingly. + * For unprovisioned boards use DB_HDMI as default. + */ + if ((daughterboard_id == DALBOZ_DB_HDMI) || boot_is_factory_unprovisioned()) { *ddi_descs = &hdmi_ddi_descriptors[0]; *ddi_num = ARRAY_SIZE(hdmi_ddi_descriptors); } else {
-
Aaron Durbin (Code Review) -
Bhanu Prakash Maiya (Code Review)
-
Furquan Shaikh (Code Review)