Change in coreboot[master]: mb/google/volteer: Implement variant_devtree_update for volteer sku's
Shaunak Saha has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44261 )
Change subject: mb/google/volteer: Implement variant_devtree_update for volteer sku's ......................................................................
mb/google/volteer: Implement variant_devtree_update for volteer sku's
HybridStorageMode FSP UPD needs to be set only for optane storage. Enabling HybridStorageMode causes some extra delay in FspSiliconInit due to HECI command and hence is avoided for NVMe and SATA scenerios. This change disables "HybridStorageMode" by default from baseboard devicetree. Any volteer variant which is using optane should enable "HybridStorageMode" from the override devicetree file.This change also provides an implementation of variant_devtree_update() for volteer that enables HybridStorageMode for SKU ID = 3 and 5.
BUG=b:158573805 TEST=Verify HybridStorageMode is disabled when SKU ID != 3 or 5 and enabled for those 2 sku's only.
Signed-off-by: Shaunak Saha shaunak.saha@intel.com Change-Id: I5c3bcfafe1710155bb47f706c3568dd9500c1975 --- M src/mainboard/google/volteer/Makefile.inc A src/mainboard/google/volteer/ramstage.c M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/volteer/variants/volteer/Makefile.inc A src/mainboard/google/volteer/variants/volteer/variant.c 6 files changed, 39 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/44261/1
diff --git a/src/mainboard/google/volteer/Makefile.inc b/src/mainboard/google/volteer/Makefile.inc index c01f993..b5579a7 100644 --- a/src/mainboard/google/volteer/Makefile.inc +++ b/src/mainboard/google/volteer/Makefile.inc @@ -10,6 +10,7 @@ ramstage-y += ec.c ramstage-$(CONFIG_FW_CONFIG) += fw_config.c ramstage-y += mainboard.c +ramstage-y += ramstage.c
smm-y += smihandler.c
diff --git a/src/mainboard/google/volteer/ramstage.c b/src/mainboard/google/volteer/ramstage.c new file mode 100644 index 0000000..084e69e --- /dev/null +++ b/src/mainboard/google/volteer/ramstage.c @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <baseboard/variants.h> +#include <soc/ramstage.h> + +void mainboard_silicon_init_params(FSP_S_CONFIG *params) +{ + variant_devtree_update(); +} + +void __weak variant_devtree_update(void) +{ + /* Override dev tree settings per board */ +} + diff --git a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb index 53bbe5a..ccd1877 100644 --- a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb @@ -81,7 +81,9 @@ # Enable Optane PCIE 11 using clk 0 register "PcieRpEnable[10]" = "1" register "PcieRpLtrEnable[10]" = "1" - register "HybridStorageMode" = "1" + # Disable it by default. Device tree overwrite needs to enable this feature + # for platforms using Optane memory. + register "HybridStorageMode" = "0"
# Enable SD Card PCIE 8 using clk 3 register "PcieRpEnable[7]" = "1" diff --git a/src/mainboard/google/volteer/variants/baseboard/include/baseboard/variants.h b/src/mainboard/google/volteer/variants/baseboard/include/baseboard/variants.h index 2f90a42..aa8ea27 100644 --- a/src/mainboard/google/volteer/variants/baseboard/include/baseboard/variants.h +++ b/src/mainboard/google/volteer/variants/baseboard/include/baseboard/variants.h @@ -21,4 +21,6 @@ const struct lpddr4x_cfg *variant_memory_params(void); int variant_memory_sku(void);
+void variant_devtree_update(void); + #endif /* __BASEBOARD_VARIANTS_H__ */ diff --git a/src/mainboard/google/volteer/variants/volteer/Makefile.inc b/src/mainboard/google/volteer/variants/volteer/Makefile.inc index 13269db..04af3ae 100644 --- a/src/mainboard/google/volteer/variants/volteer/Makefile.inc +++ b/src/mainboard/google/volteer/variants/volteer/Makefile.inc @@ -3,3 +3,4 @@ bootblock-y += gpio.c
ramstage-y += gpio.c +ramstage-y += variant.c diff --git a/src/mainboard/google/volteer/variants/volteer/variant.c b/src/mainboard/google/volteer/variants/volteer/variant.c new file mode 100644 index 0000000..8d3adef --- /dev/null +++ b/src/mainboard/google/volteer/variants/volteer/variant.c @@ -0,0 +1,17 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <baseboard/variants.h> +#include <chip.h> +#include <ec/google/chromeec/ec.h> + +void variant_devtree_update(void) +{ + uint32_t sku_id; + config_t *cfg = config_of_soc(); + + if (google_chromeec_cbi_get_sku_id(&sku_id)) + return; + + if ((sku_id == 3) || (sku_id == 5)) + cfg->HybridStorageMode = 1; +}
Hello Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44261
to look at the new patch set (#2).
Change subject: mb/google/volteer: Implement variant_devtree_update for volteer sku's ......................................................................
mb/google/volteer: Implement variant_devtree_update for volteer sku's
HybridStorageMode FSP UPD needs to be set only for optane storage. Enabling HybridStorageMode causes some extra delay in FspSiliconInit due to HECI command and hence is avoided for NVMe and SATA scenerios. This change disables "HybridStorageMode" by default from baseboard devicetree. Any volteer variant which is using optane should enable "HybridStorageMode" from the override devicetree file.This change also provides an implementation of variant_devtree_update() for volteer that enables HybridStorageMode for SKU ID = 3 and 5.
BUG=b:158573805 TEST=Verify HybridStorageMode is disabled when SKU ID != 3 or 5 and enabled for those 2 sku's only.
Signed-off-by: Shaunak Saha shaunak.saha@intel.com Change-Id: I5c3bcfafe1710155bb47f706c3568dd9500c1975 --- M src/mainboard/google/volteer/Makefile.inc A src/mainboard/google/volteer/ramstage.c M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/volteer/variants/volteer/Makefile.inc A src/mainboard/google/volteer/variants/volteer/variant.c 6 files changed, 39 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/44261/2
Hello Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44261
to look at the new patch set (#3).
Change subject: mb/google/volteer: Implement variant_devtree_update for volteer sku's ......................................................................
mb/google/volteer: Implement variant_devtree_update for volteer sku's
HybridStorageMode FSP UPD needs to be set only for optane storage. Enabling HybridStorageMode causes some extra delay in FspSiliconInit due to HECI command and hence is avoided for NVMe and SATA scenerios. This change disables "HybridStorageMode" by default from baseboard devicetree. Any volteer variant which is using optane should enable "HybridStorageMode" from the override devicetree file.This change also provides an implementation of variant_devtree_update() for volteer that enables HybridStorageMode for SKU ID = 3 and 5.
BUG=b:158573805 TEST=Verify HybridStorageMode is disabled when SKU ID != 3 or 5 and enabled for those 2 sku's only.
Signed-off-by: Shaunak Saha shaunak.saha@intel.com Change-Id: I5c3bcfafe1710155bb47f706c3568dd9500c1975 --- M src/mainboard/google/volteer/Makefile.inc A src/mainboard/google/volteer/ramstage.c M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/volteer/variants/volteer/Makefile.inc A src/mainboard/google/volteer/variants/volteer/variant.c 6 files changed, 39 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/44261/3
Hello Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44261
to look at the new patch set (#4).
Change subject: mb/google/volteer: Implement variant_devtree_update for volteer sku's ......................................................................
mb/google/volteer: Implement variant_devtree_update for volteer sku's
HybridStorageMode FSP UPD needs to be set only for optane storage. Enabling HybridStorageMode causes some extra delay in FspSiliconInit due to HECI command and hence is avoided for NVMe and SATA scenerios. This change disables "HybridStorageMode" by default from baseboard devicetree. Any volteer variant which is using optane should enable "HybridStorageMode" from the override devicetree file. This change also provides an implementation of variant_devtree_update() for volteer that enables HybridStorageMode for SKU ID = 3 and 5.
BUG=b:158573805 TEST=Verify HybridStorageMode is disabled when SKU ID != 3 or 5 and enabled for those 2 sku's only.
Signed-off-by: Shaunak Saha shaunak.saha@intel.com Change-Id: I5c3bcfafe1710155bb47f706c3568dd9500c1975 --- M src/mainboard/google/volteer/Makefile.inc A src/mainboard/google/volteer/ramstage.c M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/volteer/variants/volteer/Makefile.inc A src/mainboard/google/volteer/variants/volteer/variant.c 6 files changed, 39 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/44261/4
Hello Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44261
to look at the new patch set (#5).
Change subject: mb/google/volteer: Implement variant_devtree_update for volteer sku's ......................................................................
mb/google/volteer: Implement variant_devtree_update for volteer sku's
HybridStorageMode FSP UPD needs to be set only for optane storage. Enabling HybridStorageMode causes some extra delay in FspSiliconInit due to HECI command and hence is avoided for NVMe and SATA scenerios. This change disables "HybridStorageMode" by default from baseboard devicetree. Any volteer variant which is using optane should enable "HybridStorageMode" from the override devicetree file. This change also provides an implementation of variant_devtree_update() for volteer that enables HybridStorageMode for SKU ID = 3 and 5.
BUG=b:158573805 TEST=Verify HybridStorageMode is disabled when SKU ID != 3 or 5 and enabled for those 2 sku's only.
Signed-off-by: Shaunak Saha shaunak.saha@intel.com Change-Id: I5c3bcfafe1710155bb47f706c3568dd9500c1975 --- M src/mainboard/google/volteer/Makefile.inc A src/mainboard/google/volteer/ramstage.c M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/volteer/variants/volteer/Makefile.inc A src/mainboard/google/volteer/variants/volteer/variant.c 6 files changed, 39 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/44261/5
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44261
to look at the new patch set (#6).
Change subject: mb/google/volteer: Implement variant_devtree_update for volteer sku's ......................................................................
mb/google/volteer: Implement variant_devtree_update for volteer sku's
HybridStorageMode FSP UPD needs to be set only for optane storage. Enabling HybridStorageMode causes some extra delay in FspSiliconInit due to HECI command and hence is avoided for NVMe and SATA scenerios. This change disables "HybridStorageMode" default from baseboard devicetree. Any volteer variant which is using optane should enable "HybridStorageMode" from the override devicetree file. This change also provides an implementation of variant_devtree_update() for volteer that enables HybridStorageMode for SKU ID = 3 and 5.
BUG=b:158573805 TEST=Verify HybridStorageMode is disabled when SKU ID != 3 or 5 and enabled for those 2 sku's only.
Signed-off-by: Shaunak Saha shaunak.saha@intel.com Change-Id: I5c3bcfafe1710155bb47f706c3568dd9500c1975 --- M src/mainboard/google/volteer/Makefile.inc A src/mainboard/google/volteer/ramstage.c M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/volteer/variants/volteer/Makefile.inc A src/mainboard/google/volteer/variants/volteer/variant.c 6 files changed, 39 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/44261/6
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Tim Wawrzynczak,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44261
to look at the new patch set (#7).
Change subject: mb/google/volteer: Implement variant_devtree_update for volteer sku's ......................................................................
mb/google/volteer: Implement variant_devtree_update for volteer sku's
HybridStorageMode FSP UPD needs to be set only for optane storage. Enabling HybridStorageMode causes some extra delay in FspSiliconInit due to HECI command and hence is avoided for NVMe and SATA scenerios. This change disables "HybridStorageMode" default from baseboard devicetree. Any volteer variant which is using optane should enable FSP UPD "HybridStorageMode" from the overridetree.cb file. This change also provides an implementation of variant_devtree_update() for volteer that enables HybridStorageMode for SKU ID = 3 and 5.
BUG=b:158573805 TEST=Verify HybridStorageMode is disabled when SKU ID != 3 or 5 and enabled for those 2 sku's only.
Signed-off-by: Shaunak Saha shaunak.saha@intel.com Change-Id: I5c3bcfafe1710155bb47f706c3568dd9500c1975 --- M src/mainboard/google/volteer/Makefile.inc A src/mainboard/google/volteer/ramstage.c M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/volteer/variants/volteer/Makefile.inc A src/mainboard/google/volteer/variants/volteer/variant.c 6 files changed, 38 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/44261/7
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44261 )
Change subject: mb/google/volteer: Implement variant_devtree_update for volteer sku's ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44261/7/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/volteer/variant.c:
https://review.coreboot.org/c/coreboot/+/44261/7/src/mainboard/google/voltee... PS7, Line 15: (sku_id == 3) || (sku_id == 5) We have moved to using FW_CONFIG instead of SKU_ID for determining features that firmware cares about.
Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44261 )
Change subject: mb/google/volteer: Implement variant_devtree_update for volteer sku's ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44261/7/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/volteer/variant.c:
https://review.coreboot.org/c/coreboot/+/44261/7/src/mainboard/google/voltee... PS7, Line 15: (sku_id == 3) || (sku_id == 5)
We have moved to using FW_CONFIG instead of SKU_ID for determining features that firmware cares abou […]
I used this cause this info seems to be alreday in the cbi info. If i add some extra bit for sku info again in fw_config then won't that be a repetition. Please suggest
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44261 )
Change subject: mb/google/volteer: Implement variant_devtree_update for volteer sku's ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44261/7/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/volteer/variant.c:
https://review.coreboot.org/c/coreboot/+/44261/7/src/mainboard/google/voltee... PS7, Line 15: (sku_id == 3) || (sku_id == 5)
I used this cause this info seems to be alreday in the cbi info. […]
I think we could add a 1-bit FW_CONFIG field for Hybrid Storage.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44261 )
Change subject: mb/google/volteer: Implement variant_devtree_update for volteer sku's ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44261/7/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/volteer/variant.c:
https://review.coreboot.org/c/coreboot/+/44261/7/src/mainboard/google/voltee... PS7, Line 15: (sku_id == 3) || (sku_id == 5)
I think we could add a 1-bit FW_CONFIG field for Hybrid Storage.
Actually, can't we runtime probe for hybrid storage? I thought FSP already did that.
Martin L Roth has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/44261?usp=email )
Change subject: mb/google/volteer: Implement variant_devtree_update for volteer sku's ......................................................................
Abandoned
This patch has not been touched in over 12 months. Anyone who wants to take over work on this patch, please feel free to restore it and do any work needed to get it merged. If you create a new patch based on this work, please credit the original author.
-
Furquan Shaikh (Code Review) -
Martin L Roth (Code Review)
-
Shaunak Saha (Code Review)
-
Tim Wawrzynczak (Code Review)