Matt DeVillier has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32381
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
soc/intel/braswell: add default option to use public FSP
The current Braswell FSP 1.1 header in vendorcode/intel, for which there is no publicly available FSP binary, contains silicon init UPDs which are not found in the publicly available header/binary in the FSP Github repo. This prevents new boards from being added which use the public Braswell FSP header/binary.
To resolve this, move the UPDs not found in the public header from the soc's chip.c to ramstage.c for the boards which use them. Add a Kconfig option to use the current non-public FSP header and select it for boards which need it (google/cyan and intel/strago). Use the Kconfig option to set the FSP_HEADER_PATH to ensure the correct header is used.
Test: build/boot google/cyan
Change-Id: I43cf18b98c844175a87b61fdbe4b0b24484e5702 Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M src/mainboard/google/cyan/Kconfig M src/mainboard/google/cyan/Makefile.inc A src/mainboard/google/cyan/ramstage.c M src/mainboard/intel/strago/Kconfig M src/mainboard/intel/strago/ramstage.c M src/soc/intel/braswell/Kconfig M src/soc/intel/braswell/Makefile.inc M src/soc/intel/braswell/chip.c 8 files changed, 104 insertions(+), 33 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/32381/1
diff --git a/src/mainboard/google/cyan/Kconfig b/src/mainboard/google/cyan/Kconfig index 1d9b7b7..1e24795 100644 --- a/src/mainboard/google/cyan/Kconfig +++ b/src/mainboard/google/cyan/Kconfig @@ -16,6 +16,7 @@ select HAVE_ACPI_RESUME select PCIEXP_L1_SUB_STATE if !BOARD_GOOGLE_CYAN select SYSTEM_TYPE_LAPTOP + select USE_GOOGLE_FSP
if BOARD_GOOGLE_BASEBOARD_CYAN
diff --git a/src/mainboard/google/cyan/Makefile.inc b/src/mainboard/google/cyan/Makefile.inc index 92b0422..6879c8d 100644 --- a/src/mainboard/google/cyan/Makefile.inc +++ b/src/mainboard/google/cyan/Makefile.inc @@ -21,6 +21,7 @@ ramstage-$(CONFIG_CHROMEOS) += chromeos.c ramstage-y += ec.c ramstage-y += irqroute.c +ramstage-y += ramstage.c ramstage-y += w25q64.c
smm-$(CONFIG_HAVE_SMI_HANDLER) += smihandler.c diff --git a/src/mainboard/google/cyan/ramstage.c b/src/mainboard/google/cyan/ramstage.c new file mode 100644 index 0000000..ae6996a --- /dev/null +++ b/src/mainboard/google/cyan/ramstage.c @@ -0,0 +1,52 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2019 Eltan B.V. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <soc/ramstage.h> + +void mainboard_silicon_init_params(SILICON_INIT_UPD *params) +{ + struct device *dev = pcidev_on_root(LPC_DEV, LPC_FUNC); + struct soc_intel_braswell_config *config; + + config = dev->chip_info; + + if (config->D0Usb2Port0PerPortRXISet != 0) + params->D0Usb2Port0PerPortRXISet = + config->D0Usb2Port0PerPortRXISet; + + if (config->D0Usb2Port1PerPortRXISet != 0) + params->D0Usb2Port1PerPortRXISet = + config->D0Usb2Port1PerPortRXISet; + + if (config->D0Usb2Port2PerPortRXISet != 0) + params->D0Usb2Port2PerPortRXISet = + config->D0Usb2Port2PerPortRXISet; + + if (config->D0Usb2Port3PerPortRXISet != 0) + params->D0Usb2Port3PerPortRXISet = + config->D0Usb2Port3PerPortRXISet; + + if (config->D0Usb2Port4PerPortRXISet != 0) + params->D0Usb2Port4PerPortRXISet = + config->D0Usb2Port4PerPortRXISet; + + params->I2C0Frequency = config->I2C0Frequency; + params->I2C1Frequency = config->I2C1Frequency; + params->I2C2Frequency = config->I2C2Frequency; + params->I2C3Frequency = config->I2C3Frequency; + params->I2C4Frequency = config->I2C4Frequency; + params->I2C5Frequency = config->I2C5Frequency; + params->I2C6Frequency = config->I2C6Frequency; +} diff --git a/src/mainboard/intel/strago/Kconfig b/src/mainboard/intel/strago/Kconfig index 4aa7640..d943f98 100644 --- a/src/mainboard/intel/strago/Kconfig +++ b/src/mainboard/intel/strago/Kconfig @@ -15,6 +15,7 @@ select MAINBOARD_HAS_LPC_TPM select SOC_INTEL_BRASWELL select PCIEXP_L1_SUB_STATE + select USE_GOOGLE_FSP
config VBOOT select EC_GOOGLE_CHROMEEC_SWITCHES diff --git a/src/mainboard/intel/strago/ramstage.c b/src/mainboard/intel/strago/ramstage.c index a05cd90..e8f8069 100644 --- a/src/mainboard/intel/strago/ramstage.c +++ b/src/mainboard/intel/strago/ramstage.c @@ -2,6 +2,7 @@ * This file is part of the coreboot project. * * Copyright (C) 2014 Intel Corporation + * Copyright (C) 2019 Eltan B.V. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -19,6 +20,39 @@
void mainboard_silicon_init_params(SILICON_INIT_UPD *params) { + struct device *dev = pcidev_on_root(LPC_DEV, LPC_FUNC); + struct soc_intel_braswell_config *config; + + config = dev->chip_info; + params->ChvSvidConfig = SVID_PMIC_CONFIG; params->PMIC_I2CBus = BCRD2_PMIC_I2C_BUS; + + if (config->D0Usb2Port0PerPortRXISet != 0) + params->D0Usb2Port0PerPortRXISet = + config->D0Usb2Port0PerPortRXISet; + + if (config->D0Usb2Port1PerPortRXISet != 0) + params->D0Usb2Port1PerPortRXISet = + config->D0Usb2Port1PerPortRXISet; + + if (config->D0Usb2Port2PerPortRXISet != 0) + params->D0Usb2Port2PerPortRXISet = + config->D0Usb2Port2PerPortRXISet; + + if (config->D0Usb2Port3PerPortRXISet != 0) + params->D0Usb2Port3PerPortRXISet = + config->D0Usb2Port3PerPortRXISet; + + if (config->D0Usb2Port4PerPortRXISet != 0) + params->D0Usb2Port4PerPortRXISet = + config->D0Usb2Port4PerPortRXISet; + + params->I2C0Frequency = config->I2C0Frequency; + params->I2C1Frequency = config->I2C1Frequency; + params->I2C2Frequency = config->I2C2Frequency; + params->I2C3Frequency = config->I2C3Frequency; + params->I2C4Frequency = config->I2C4Frequency; + params->I2C5Frequency = config->I2C5Frequency; + params->I2C6Frequency = config->I2C6Frequency; } diff --git a/src/soc/intel/braswell/Kconfig b/src/soc/intel/braswell/Kconfig index 8db4795..46deb60 100644 --- a/src/soc/intel/braswell/Kconfig +++ b/src/soc/intel/braswell/Kconfig @@ -135,4 +135,18 @@ Enable this to disable the HPET support Solves the Linux MP-BIOS bug timer not connected.
+config USE_GOOGLE_FSP + bool "Use Google's Braswell FSP header, instead of public release" + default n + help + Select this to use Google's custom Braswell FSP header/binary + instead of the public release on Github. All google/cyan + variants and intel/strago require this; other boards should + use the public release. + +config FSP_HEADER_PATH + string "Location of FSP headers" + default "$(src)/vendorcode/intel/fsp/fsp1_1/braswell" if USE_GOOGLE_FSP + default "3rdparty/fsp/BraswellFspBinPkg/Include/" + endif diff --git a/src/soc/intel/braswell/Makefile.inc b/src/soc/intel/braswell/Makefile.inc index a538f7d..a7ed4c5 100644 --- a/src/soc/intel/braswell/Makefile.inc +++ b/src/soc/intel/braswell/Makefile.inc @@ -56,7 +56,7 @@
CPPFLAGS_common += -I$(src)/soc/intel/braswell/ CPPFLAGS_common += -I$(src)/soc/intel/braswell/include -CPPFLAGS_common += -I$(src)/vendorcode/intel/fsp/fsp1_1/braswell +CPPFLAGS_common += -I$(call strip_quotes,$(CONFIG_FSP_HEADER_PATH))
CPPFLAGS_common += -I3rdparty/blobs/mainboard/$(MAINBOARDDIR)
diff --git a/src/soc/intel/braswell/chip.c b/src/soc/intel/braswell/chip.c index 7617d53..b65196a 100644 --- a/src/soc/intel/braswell/chip.c +++ b/src/soc/intel/braswell/chip.c @@ -128,36 +128,26 @@ params->Usb2Port0PerPortTxiSet = config->Usb2Port0PerPortTxiSet; params->Usb2Port0IUsbTxEmphasisEn = config->Usb2Port0IUsbTxEmphasisEn; params->Usb2Port0PerPortTxPeHalf = config->Usb2Port0PerPortTxPeHalf; - if (config->D0Usb2Port0PerPortRXISet != 0) - params->D0Usb2Port0PerPortRXISet = config->D0Usb2Port0PerPortRXISet;
params->Usb2Port1PerPortPeTxiSet = config->Usb2Port1PerPortPeTxiSet; params->Usb2Port1PerPortTxiSet = config->Usb2Port1PerPortTxiSet; params->Usb2Port1IUsbTxEmphasisEn = config->Usb2Port1IUsbTxEmphasisEn; params->Usb2Port1PerPortTxPeHalf = config->Usb2Port1PerPortTxPeHalf; - if (config->D0Usb2Port1PerPortRXISet != 0) - params->D0Usb2Port1PerPortRXISet = config->D0Usb2Port1PerPortRXISet;
params->Usb2Port2PerPortPeTxiSet = config->Usb2Port2PerPortPeTxiSet; params->Usb2Port2PerPortTxiSet = config->Usb2Port2PerPortTxiSet; params->Usb2Port2IUsbTxEmphasisEn = config->Usb2Port2IUsbTxEmphasisEn; params->Usb2Port2PerPortTxPeHalf = config->Usb2Port2PerPortTxPeHalf; - if (config->D0Usb2Port2PerPortRXISet != 0) - params->D0Usb2Port2PerPortRXISet = config->D0Usb2Port2PerPortRXISet;
params->Usb2Port3PerPortPeTxiSet = config->Usb2Port3PerPortPeTxiSet; params->Usb2Port3PerPortTxiSet = config->Usb2Port3PerPortTxiSet; params->Usb2Port3IUsbTxEmphasisEn = config->Usb2Port3IUsbTxEmphasisEn; params->Usb2Port3PerPortTxPeHalf = config->Usb2Port3PerPortTxPeHalf; - if (config->D0Usb2Port3PerPortRXISet != 0) - params->D0Usb2Port3PerPortRXISet = config->D0Usb2Port3PerPortRXISet;
params->Usb2Port4PerPortPeTxiSet = config->Usb2Port4PerPortPeTxiSet; params->Usb2Port4PerPortTxiSet = config->Usb2Port4PerPortTxiSet; params->Usb2Port4IUsbTxEmphasisEn = config->Usb2Port4IUsbTxEmphasisEn; params->Usb2Port4PerPortTxPeHalf = config->Usb2Port4PerPortTxPeHalf; - if (config->D0Usb2Port4PerPortRXISet != 0) - params->D0Usb2Port4PerPortRXISet = config->D0Usb2Port4PerPortRXISet;
params->Usb3Lane0Ow2tapgen2deemph3p5 = config->Usb3Lane0Ow2tapgen2deemph3p5; @@ -179,13 +169,6 @@ params->ISPEnable = config->ISPEnable; params->ISPPciDevConfig = config->ISPPciDevConfig; params->PcdSdDetectChk = config->PcdSdDetectChk; - params->I2C0Frequency = config->I2C0Frequency; - params->I2C1Frequency = config->I2C1Frequency; - params->I2C2Frequency = config->I2C2Frequency; - params->I2C3Frequency = config->I2C3Frequency; - params->I2C4Frequency = config->I2C4Frequency; - params->I2C5Frequency = config->I2C5Frequency; - params->I2C6Frequency = config->I2C6Frequency;
board_silicon_USB2_override(params); } @@ -265,9 +248,6 @@ fsp_display_upd_value("Usb2Port0PerPortTxPeHalf", 1, old->Usb2Port0PerPortTxPeHalf, new->Usb2Port0PerPortTxPeHalf); - fsp_display_upd_value("D0Usb2Port0PerPortRXISet", 1, - old->D0Usb2Port0PerPortRXISet, - new->D0Usb2Port0PerPortRXISet); fsp_display_upd_value("Usb2Port1PerPortPeTxiSet", 1, old->Usb2Port1PerPortPeTxiSet, new->Usb2Port1PerPortPeTxiSet); @@ -280,9 +260,6 @@ fsp_display_upd_value("Usb2Port1PerPortTxPeHalf", 1, old->Usb2Port1PerPortTxPeHalf, new->Usb2Port1PerPortTxPeHalf); - fsp_display_upd_value("D0Usb2Port1PerPortRXISet", 1, - old->D0Usb2Port1PerPortRXISet, - new->D0Usb2Port1PerPortRXISet); fsp_display_upd_value("Usb2Port2PerPortPeTxiSet", 1, old->Usb2Port2PerPortPeTxiSet, new->Usb2Port2PerPortPeTxiSet); @@ -295,9 +272,6 @@ fsp_display_upd_value("Usb2Port2PerPortTxPeHalf", 1, old->Usb2Port2PerPortTxPeHalf, new->Usb2Port2PerPortTxPeHalf); - fsp_display_upd_value("D0Usb2Port2PerPortRXISet", 1, - old->D0Usb2Port2PerPortRXISet, - new->D0Usb2Port2PerPortRXISet); fsp_display_upd_value("Usb2Port3PerPortPeTxiSet", 1, old->Usb2Port3PerPortPeTxiSet, new->Usb2Port3PerPortPeTxiSet); @@ -310,9 +284,6 @@ fsp_display_upd_value("Usb2Port3PerPortTxPeHalf", 1, old->Usb2Port3PerPortTxPeHalf, new->Usb2Port3PerPortTxPeHalf); - fsp_display_upd_value("D0Usb2Port3PerPortRXISet", 1, - old->D0Usb2Port3PerPortRXISet, - new->D0Usb2Port3PerPortRXISet); fsp_display_upd_value("Usb2Port4PerPortPeTxiSet", 1, old->Usb2Port4PerPortPeTxiSet, new->Usb2Port4PerPortPeTxiSet); @@ -325,9 +296,6 @@ fsp_display_upd_value("Usb2Port4PerPortTxPeHalf", 1, old->Usb2Port4PerPortTxPeHalf, new->Usb2Port4PerPortTxPeHalf); - fsp_display_upd_value("D0Usb2Port4PerPortRXISet", 1, - old->D0Usb2Port4PerPortRXISet, - new->D0Usb2Port4PerPortRXISet); fsp_display_upd_value("Usb3Lane0Ow2tapgen2deemph3p5", 1, old->Usb3Lane0Ow2tapgen2deemph3p5, new->Usb3Lane0Ow2tapgen2deemph3p5);
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32381 )
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/#/c/32381/1/src/mainboard/intel/strago/ramstage.... File src/mainboard/intel/strago/ramstage.c:
https://review.coreboot.org/#/c/32381/1/src/mainboard/intel/strago/ramstage.... PS1, Line 31: if (config->D0Usb2Port0PerPortRXISet != 0) please, no spaces at the start of a line
https://review.coreboot.org/#/c/32381/1/src/mainboard/intel/strago/ramstage.... PS1, Line 31: if (config->D0Usb2Port0PerPortRXISet != 0) suspect code indent for conditional statements (4, 16)
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32381 )
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
Patch Set 1:
not sure if I should include here or separate patch, but was going to set the default FSP binary path for the case when using the public FSP, modeled after how it's done with FSP 2.0 for SKL/KBL
Hello Patrick Rudolph, Frans Hendriks, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32381
to look at the new patch set (#2).
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
soc/intel/braswell: add default option to use public FSP
The current Braswell FSP 1.1 header in vendorcode/intel, for which there is no publicly available FSP binary, contains silicon init UPDs which are not found in the publicly available header/binary in the FSP Github repo. This prevents new boards from being added which use the public Braswell FSP header/binary.
To resolve this, move the UPDs not found in the public header from the soc's chip.c to ramstage.c for the boards which use them. Add a Kconfig option to use the current non-public FSP header and select it for boards which need it (google/cyan and intel/strago). Use the Kconfig option to set the FSP_HEADER_PATH to ensure the correct header is used.
Additionally, set the default path for the FSP binary when the public FSP is selected.
Test: build google/cyan with both public and non-public FSP options (adjusting Kconfig/ramstage.c to allow the former)
Change-Id: I43cf18b98c844175a87b61fdbe4b0b24484e5702 Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M src/drivers/intel/fsp1_1/Kconfig M src/mainboard/google/cyan/Kconfig M src/mainboard/google/cyan/Makefile.inc A src/mainboard/google/cyan/ramstage.c M src/mainboard/intel/strago/Kconfig M src/mainboard/intel/strago/ramstage.c M src/soc/intel/braswell/Kconfig M src/soc/intel/braswell/Makefile.inc M src/soc/intel/braswell/chip.c 9 files changed, 113 insertions(+), 33 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/32381/2
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32381 )
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
Patch Set 2:
(1 comment)
This patch seems to be similar as https://review.coreboot.org/c/coreboot/+/29661 Would rebase the differences on that patch an alternative?
https://review.coreboot.org/#/c/32381/2/src/soc/intel/braswell/Makefile.inc File src/soc/intel/braswell/Makefile.inc:
https://review.coreboot.org/#/c/32381/2/src/soc/intel/braswell/Makefile.inc@... PS2, Line 59: CPPFLAGS_common += -I$(call strip_quotes,$(CONFIG_FSP_HEADER_PATH)) Did you also build and test after you removed the FspUpdVpd.h binary in vendorcode/../braswell? I recall from my patch that the original (vendorcode) FspUpdVph.h was still used, which causes a hang during boot of the system.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32381 )
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
Patch Set 2:
(1 comment)
This patch seems to be similar as https://review.coreboot.org/c/coreboot/+/29661 Would rebase the differences on that patch an alternative?
yes, I did base my patch on yours - was trying to see if a cleaner approach to which header to use could be found. I have no preference on whose patch is used, just that the issue is resolved as cleanly as possible.
https://review.coreboot.org/#/c/32381/2/src/soc/intel/braswell/Makefile.inc File src/soc/intel/braswell/Makefile.inc:
https://review.coreboot.org/#/c/32381/2/src/soc/intel/braswell/Makefile.inc@... PS2, Line 59: CPPFLAGS_common += -I$(call strip_quotes,$(CONFIG_FSP_HEADER_PATH))
Did you also build and test after you removed the FspUpdVpd.h binary in vendorcode/../braswell? […]
I just did a test build with the header in vendorcode deleted, and no issues building. I don't see how it would still be used since that path is no longer included. Searching the ./build directory confirms this
Hello Patrick Rudolph, Huang Jin, Frans Hendriks, Lee Leahy, build bot (Jenkins), Nico Huber, Michał Żygowski, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32381
to look at the new patch set (#3).
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
soc/intel/braswell: add default option to use public FSP
The current Braswell FSP 1.1 header in vendorcode/intel, for which there is no publicly available FSP binary, contains silicon init UPDs which are not found in the publicly available header/binary in the FSP Github repo. This prevents new boards from being added which use the public Braswell FSP header/binary.
To resolve this, move the UPDs not found in the public header from the soc's chip.c to ramstage.c for the boards which use them. Add a Kconfig option to use the current non-public FSP header and select it for boards which need it (google/cyan and intel/strago). Use the Kconfig option to set the FSP_HEADER_PATH to ensure the correct header is used.
Additionally, set the default path for the FSP binary when the public FSP is selected.
Test: build google/cyan with both public and non-public FSP options (adjusting Kconfig/ramstage.c to allow the former)
Change-Id: I43cf18b98c844175a87b61fdbe4b0b24484e5702 Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M src/drivers/intel/fsp1_1/Kconfig M src/mainboard/google/cyan/Kconfig M src/mainboard/google/cyan/Makefile.inc A src/mainboard/google/cyan/ramstage.c M src/mainboard/intel/strago/Kconfig M src/mainboard/intel/strago/ramstage.c M src/soc/intel/braswell/Kconfig M src/soc/intel/braswell/Makefile.inc M src/soc/intel/braswell/chip.c 9 files changed, 115 insertions(+), 33 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/32381/3
Hello Patrick Rudolph, Huang Jin, Frans Hendriks, Lee Leahy, build bot (Jenkins), Nico Huber, Michał Żygowski, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32381
to look at the new patch set (#4).
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
soc/intel/braswell: add default option to use public FSP
The current Braswell FSP 1.1 header in vendorcode/intel, for which there is no publicly available FSP binary, contains silicon init UPDs which are not found in the publicly available header/binary in the FSP Github repo. This prevents new boards from being added which use the public Braswell FSP header/binary.
To resolve this, move the UPDs not found in the public header from the soc's chip.c to ramstage.c for the boards which use them. Add a Kconfig option to use the current non-public FSP header and select it for boards which need it (google/cyan variants); set the public FSP option as the default. Use the Kconfig option to set FSP_HEADER_PATH to ensure the correct header is used.
Additionally, set the default path for the FSP binary when the public FSP is selected.
Test: build google/cyan and intel/strago using non-public and public FSP header/binaries respectively.
Change-Id: I43cf18b98c844175a87b61fdbe4b0b24484e5702 Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M src/drivers/intel/fsp1_1/Kconfig M src/mainboard/google/cyan/Kconfig M src/mainboard/google/cyan/variants/celes/devicetree.cb M src/mainboard/google/cyan/variants/celes/ramstage.c M src/mainboard/google/cyan/variants/kefka/Makefile.inc M src/mainboard/google/cyan/variants/kefka/devicetree.cb A src/mainboard/google/cyan/variants/kefka/ramstage.c M src/mainboard/google/cyan/variants/relm/devicetree.cb M src/mainboard/google/cyan/variants/relm/ramstage.c M src/soc/intel/braswell/Kconfig M src/soc/intel/braswell/Makefile.inc M src/soc/intel/braswell/chip.c M src/soc/intel/braswell/chip.h 13 files changed, 72 insertions(+), 51 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/32381/4
Philipp Deppenwiese has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32381 )
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
Patch Set 6: Code-Review+1
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32381 )
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
Patch Set 6: Code-Review-1
(1 comment)
I2CxFrequency must be removed from Braswell and moved to MR1 mainboard directory.
Please have a look into patch: https://review.coreboot.org/c/coreboot/+/29661
https://review.coreboot.org/#/c/32381/6/src/soc/intel/braswell/chip.c File src/soc/intel/braswell/chip.c:
https://review.coreboot.org/#/c/32381/6/src/soc/intel/braswell/chip.c@172 PS6, Line 172: params->I2C0Frequency = config->I2C0Frequency; I2C0Frequency -- I2C6Frequency do not exist in SILICON_INIT_UPD of FSP MR2 3rdparty Braswell FspUpdVpd.h
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32381 )
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
Patch Set 6:
(1 comment)
I2CxFrequency must be removed from Braswell and moved to MR1 mainboard directory.
this patch is against the recently-posted 1.1.8.0 release (see parent commit which updates the FSP submodule pointer), which does include the I2C frequency UPDs. No point in using the much older MR2 release now since it just makes a lot more work
https://review.coreboot.org/#/c/32381/6/src/soc/intel/braswell/chip.c File src/soc/intel/braswell/chip.c:
https://review.coreboot.org/#/c/32381/6/src/soc/intel/braswell/chip.c@172 PS6, Line 172: params->I2C0Frequency = config->I2C0Frequency;
I2C0Frequency -- I2C6Frequency do not exist in SILICON_INIT_UPD of FSP MR2 3rdparty Braswell FspUpdVpd.h
see top level comment
Frans Hendriks has removed a vote on this change.
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
Removed Code-Review-1 by Frans Hendriks fhendriks@eltan.com
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32381 )
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
Patch Set 6: Code-Review+1
Patch Set 6:
(1 comment)
I2CxFrequency must be removed from Braswell and moved to MR1 mainboard directory.
this patch is against the recently-posted 1.1.8.0 release (see parent commit which updates the FSP submodule pointer), which does include the I2C frequency UPDs. No point in using the much older MR2 release now since it just makes a lot more work
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32381 )
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
Patch Set 6:
Frans, are you able to validate this on one of your boards?
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32381 )
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
Patch Set 6:
Patch Set 6:
Frans, are you able to validate this on one of your boards?
Tested with FSP 1.1.4.0, without I2Cx frequencies, and it's working find (building/booting). Building fine with FSP 1.1.8.0. Not tested booting (yet).
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32381 )
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
Patch Set 6: Code-Review+2
Patch Set 6:
Patch Set 6:
Frans, are you able to validate this on one of your boards?
Tested with FSP 1.1.4.0, without I2Cx frequencies, and it's working find (building/booting). Building fine with FSP 1.1.8.0. Not tested booting (yet).
I verified and Facebook FBG-1701 is booting without problems using this patch.
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32381 )
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
Patch Set 6:
Patch Set 6:
(1 comment)
I2CxFrequency must be removed from Braswell and moved to MR1 mainboard directory.
this patch is against the recently-posted 1.1.8.0 release (see parent commit which updates the FSP submodule pointer), which does include the I2C frequency UPDs. No point in using the much older MR2 release now since it just makes a lot more work
Version 1.1.4.0 (FSP MR2) is supposed to be used for embedded market. We could find information only, that FSM MR2 (1.1.4.0) is latest version for embedded market.
Do you have information if this 1.1.8.0 is supposed to be used for Intel embedded Braswell platforms? Or is this version for Google Chromebooks?
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32381 )
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
Patch Set 6:
Do you have information if this 1.1.8.0 is supposed to be used for Intel embedded Braswell platforms? Or is this version for Google Chromebooks?
1.1.8.0 is just a minor update, adding a few UPDs without changing offsets. It supports all devices the previous MR2 release did. It appears Google/Intel modified this FSP for certain Chromebooks to address a specific USB issue. Can't see any reason why using 1.1.8.0 in place of MR2 would be problematic but Intel would be the one to answer that authoritatively
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32381 )
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
Patch Set 6: Code-Review+2
(3 comments)
Looks good. Left some inline comments how to reduce the (pre-existing) Kconfig noise.
https://review.coreboot.org/#/c/32381/6/src/drivers/intel/fsp1_1/Kconfig File src/drivers/intel/fsp1_1/Kconfig:
https://review.coreboot.org/#/c/32381/6/src/drivers/intel/fsp1_1/Kconfig@52 PS6, Line 52: depends on HAVE_FSP_BIN How about `select` instead? Then this would be enabled by default for systems that support it and automatically build tested.
https://review.coreboot.org/#/c/32381/6/src/drivers/intel/fsp1_1/Kconfig@58 PS6, Line 58: string "Intel FSP binary path and filename" IIRC, we can hide the prompt conditionally, something like
string prompt "Intel FSP binary path and filename" if !FSP_USE_REPO
https://review.coreboot.org/#/c/32381/6/src/mainboard/google/cyan/variants/c... File src/mainboard/google/cyan/variants/celes/ramstage.c:
https://review.coreboot.org/#/c/32381/6/src/mainboard/google/cyan/variants/c... PS6, Line 23: //BSW D-stepping PERPORTRXISET 2 (low strength) NB. Now that I read the comment I wonder if the upstream FSP does it automatically.
Hello Patrick Rudolph, Huang Jin, Frans Hendriks, Lee Leahy, Philipp Deppenwiese, build bot (Jenkins), Nico Huber, Michał Żygowski, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32381
to look at the new patch set (#7).
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
soc/intel/braswell: add default option to use public FSP
The current Braswell FSP 1.1 header in vendorcode/intel, for which there is no publicly available FSP binary, contains silicon init UPDs which are not found in the publicly available header/binary in the FSP Github repo. This prevents new boards from being added which use the public Braswell FSP header/binary.
To resolve this, move the UPDs not found in the public header from the soc's chip.c to ramstage.c for the boards which use them. Add a Kconfig option to use the current non-public FSP header and select it for boards which need it (google/cyan variants); set the public FSP option as the default. Use the Kconfig option to set FSP_HEADER_PATH to ensure the correct header is used.
Additionally, set the default path for the FSP binary when the public FSP is selected.
Test: build google/cyan and intel/strago using non-public and public FSP header/binaries respectively.
Change-Id: I43cf18b98c844175a87b61fdbe4b0b24484e5702 Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M src/drivers/intel/fsp1_1/Kconfig M src/mainboard/google/cyan/Kconfig M src/mainboard/google/cyan/variants/celes/devicetree.cb M src/mainboard/google/cyan/variants/celes/ramstage.c M src/mainboard/google/cyan/variants/kefka/Makefile.inc M src/mainboard/google/cyan/variants/kefka/devicetree.cb A src/mainboard/google/cyan/variants/kefka/ramstage.c M src/mainboard/google/cyan/variants/relm/devicetree.cb M src/mainboard/google/cyan/variants/relm/ramstage.c M src/soc/intel/braswell/Kconfig M src/soc/intel/braswell/Makefile.inc M src/soc/intel/braswell/chip.c M src/soc/intel/braswell/chip.h 13 files changed, 74 insertions(+), 52 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/32381/7
Hello Patrick Rudolph, Huang Jin, Frans Hendriks, Lee Leahy, Philipp Deppenwiese, build bot (Jenkins), Nico Huber, Michał Żygowski, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32381
to look at the new patch set (#8).
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
soc/intel/braswell: add default option to use public FSP
The current Braswell FSP 1.1 header in vendorcode/intel, for which there is no publicly available FSP binary, contains silicon init UPDs which are not found in the publicly available header/binary in the FSP Github repo. This prevents new boards from being added which use the public Braswell FSP header/binary.
To resolve this, move the UPDs not found in the public header from the soc's chip.c to ramstage.c for the boards which use them. Add a Kconfig option to use the current non-public FSP header and select it for boards which need it (google/cyan variants); set the public FSP option as the default. Use the Kconfig option to set FSP_HEADER_PATH to ensure the correct header is used.
Additionally, set the default path for the FSP binary when the public FSP is selected.
Test: build google/cyan and intel/strago using non-public and public FSP header/binaries respectively.
Change-Id: I43cf18b98c844175a87b61fdbe4b0b24484e5702 Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M src/drivers/intel/fsp1_1/Kconfig M src/mainboard/google/cyan/Kconfig M src/mainboard/google/cyan/variants/celes/devicetree.cb M src/mainboard/google/cyan/variants/celes/ramstage.c M src/mainboard/google/cyan/variants/kefka/Makefile.inc M src/mainboard/google/cyan/variants/kefka/devicetree.cb A src/mainboard/google/cyan/variants/kefka/ramstage.c M src/mainboard/google/cyan/variants/relm/devicetree.cb M src/mainboard/google/cyan/variants/relm/ramstage.c M src/soc/intel/braswell/Kconfig M src/soc/intel/braswell/Makefile.inc M src/soc/intel/braswell/chip.c M src/soc/intel/braswell/chip.h 13 files changed, 74 insertions(+), 53 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/32381/8
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32381 )
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
Patch Set 8:
Hmmm, it doesn't build? I have no idea why it does try to add the file for Cyan variants (and only if not building for cros???).
The cbfstool issue seems to be fixed with FSP_LOC 0xfff2000 (no idea why we have to specify this when it depends on the binary).
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32381 )
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
Patch Set 8:
The cbfstool issue seems to be fixed with FSP_LOC 0xfff2000 (no idea why we have to specify this when it depends on the binary).
CBFS_SIZE, uh-huh
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32381 )
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
Patch Set 8:
There is another issue lurking: Makefile.inc:199 The submodule is only updated for certain FSP versions. I wouldn't mind if we simply always update it in case of USE_BLOBS.
Hello Patrick Rudolph, Huang Jin, Frans Hendriks, Lee Leahy, Philipp Deppenwiese, build bot (Jenkins), Nico Huber, Michał Żygowski, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32381
to look at the new patch set (#9).
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
soc/intel/braswell: add default option to use public FSP
The current Braswell FSP 1.1 header in vendorcode/intel, for which there is no publicly available FSP binary, contains silicon init UPDs which are not found in the publicly available header/binary in the FSP Github repo. This prevents new boards from being added which use the public Braswell FSP header/binary.
To resolve this, move the UPDs not found in the public header from the soc's chip.c to ramstage.c for the boards which use them. Add a Kconfig option to use the current non-public FSP header and select it for boards which need it (google/cyan variants); set the public FSP option as the default. Use the Kconfig option to set FSP_HEADER_PATH to ensure the correct header is used.
Additionally, reorder FSP 1.1 Kconfig entries for improved flow/ readability, and set the default path for the FSP binary when the public FSP is selected.
Test: build google/cyan and intel/strago using non-public and public FSP header/binaries respectively.
Change-Id: I43cf18b98c844175a87b61fdbe4b0b24484e5702 Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M src/drivers/intel/fsp1_1/Kconfig M src/mainboard/google/cyan/Kconfig M src/mainboard/google/cyan/variants/celes/devicetree.cb M src/mainboard/google/cyan/variants/celes/ramstage.c M src/mainboard/google/cyan/variants/kefka/Makefile.inc M src/mainboard/google/cyan/variants/kefka/devicetree.cb A src/mainboard/google/cyan/variants/kefka/ramstage.c M src/mainboard/google/cyan/variants/relm/devicetree.cb M src/mainboard/google/cyan/variants/relm/ramstage.c M src/soc/intel/braswell/Kconfig M src/soc/intel/braswell/Makefile.inc M src/soc/intel/braswell/chip.c M src/soc/intel/braswell/chip.h 13 files changed, 87 insertions(+), 65 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/32381/9
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32381 )
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
Patch Set 10: Code-Review+2
(3 comments)
https://review.coreboot.org/#/c/32381/6/src/drivers/intel/fsp1_1/Kconfig File src/drivers/intel/fsp1_1/Kconfig:
https://review.coreboot.org/#/c/32381/6/src/drivers/intel/fsp1_1/Kconfig@52 PS6, Line 52: depends on HAVE_FSP_BIN
How about `select` instead? Then this would be enabled by […]
Done
https://review.coreboot.org/#/c/32381/6/src/drivers/intel/fsp1_1/Kconfig@58 PS6, Line 58: string "Intel FSP binary path and filename"
IIRC, we can hide the prompt conditionally, something like […]
Done
https://review.coreboot.org/#/c/32381/10/src/drivers/intel/fsp1_1/Kconfig File src/drivers/intel/fsp1_1/Kconfig:
https://review.coreboot.org/#/c/32381/10/src/drivers/intel/fsp1_1/Kconfig@57 PS10, Line 57: This must match the : value that is set in the FSP binary The public FSP has different numbers in the integration guide. I don't have the tools to check / hardware to test. If somebody could check if this needs a different default for the public binaries, that would be nice.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32381 )
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/#/c/32381/10/src/drivers/intel/fsp1_1/Kconfig File src/drivers/intel/fsp1_1/Kconfig:
https://review.coreboot.org/#/c/32381/10/src/drivers/intel/fsp1_1/Kconfig@57 PS10, Line 57: This must match the : value that is set in the FSP binary
The public FSP has different numbers in the integration guide. […]
I checked the current binary with BCT, it's 0x0xfff20000, so I'll adjust based on use of CONFIG_USE_GOOGLE_FSP
Hello Patrick Rudolph, Huang Jin, Frans Hendriks, Lee Leahy, Philipp Deppenwiese, build bot (Jenkins), Nico Huber, Michał Żygowski, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32381
to look at the new patch set (#11).
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
soc/intel/braswell: add default option to use public FSP
The current Braswell FSP 1.1 header in vendorcode/intel, for which there is no publicly available FSP binary, contains silicon init UPDs which are not found in the publicly available header/binary in the FSP Github repo. This prevents new boards from being added which use the public Braswell FSP header/binary.
To resolve this, move the UPDs not found in the public header from the soc's chip.c to ramstage.c for the boards which use them. Add a Kconfig option to use the current non-public FSP header and select it for boards which need it (google/cyan variants); set the public FSP option as the default. Use the Kconfig option to set FSP_HEADER_PATH to ensure the correct header is used.
Additionally: - reorder FSP 1.1 Kconfig entries for improved flow/readability - set the default path for the FSP binary based on use of public FSP and platform - set the CBFS location for the FSP binary based on platform
Test: build google/cyan and intel/strago using non-public and public FSP header/binaries respectively.
Change-Id: I43cf18b98c844175a87b61fdbe4b0b24484e5702 Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M src/drivers/intel/fsp1_1/Kconfig M src/mainboard/google/cyan/Kconfig M src/mainboard/google/cyan/variants/celes/devicetree.cb M src/mainboard/google/cyan/variants/celes/ramstage.c M src/mainboard/google/cyan/variants/kefka/Makefile.inc M src/mainboard/google/cyan/variants/kefka/devicetree.cb A src/mainboard/google/cyan/variants/kefka/ramstage.c M src/mainboard/google/cyan/variants/relm/devicetree.cb M src/mainboard/google/cyan/variants/relm/ramstage.c M src/soc/intel/braswell/Kconfig M src/soc/intel/braswell/Makefile.inc M src/soc/intel/braswell/chip.c M src/soc/intel/braswell/chip.h 13 files changed, 90 insertions(+), 65 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/32381/11
Hello Patrick Rudolph, Huang Jin, Frans Hendriks, Lee Leahy, Philipp Deppenwiese, build bot (Jenkins), Nico Huber, Michał Żygowski, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32381
to look at the new patch set (#12).
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
soc/intel/braswell: add default option to use public FSP
The current Braswell FSP 1.1 header in vendorcode/intel, for which there is no publicly available FSP binary, contains silicon init UPDs which are not found in the publicly available header/binary in the FSP Github repo. This prevents new boards from being added which use the public Braswell FSP header/binary.
To resolve this, move the UPDs not found in the public header from the soc's chip.c to ramstage.c for the boards which use them. Add a Kconfig option to use the current non-public FSP header and select it for boards which need it (google/cyan variants); set the public FSP option as the default. Use the Kconfig option to set FSP_HEADER_PATH to ensure the correct header is used.
Test: build google/cyan and intel/strago using non-public and public FSP header/binaries respectively.
Change-Id: I43cf18b98c844175a87b61fdbe4b0b24484e5702 Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M src/mainboard/google/cyan/Kconfig M src/mainboard/google/cyan/variants/celes/devicetree.cb M src/mainboard/google/cyan/variants/celes/ramstage.c M src/mainboard/google/cyan/variants/kefka/Makefile.inc M src/mainboard/google/cyan/variants/kefka/devicetree.cb A src/mainboard/google/cyan/variants/kefka/ramstage.c M src/mainboard/google/cyan/variants/relm/devicetree.cb M src/mainboard/google/cyan/variants/relm/ramstage.c M src/soc/intel/braswell/Kconfig M src/soc/intel/braswell/Makefile.inc M src/soc/intel/braswell/chip.c M src/soc/intel/braswell/chip.h 12 files changed, 62 insertions(+), 51 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/32381/12
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32381 )
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
Patch Set 12:
(1 comment)
Patch Set 8:
Hmmm, it doesn't build? I have no idea why it does try to add the file for Cyan variants (and only if not building for cros???).
The cbfstool issue seems to be fixed with FSP_LOC 0xfff2000 (no idea why we have to specify this when it depends on the binary).
https://review.coreboot.org/#/c/32381/10/src/drivers/intel/fsp1_1/Kconfig File src/drivers/intel/fsp1_1/Kconfig:
https://review.coreboot.org/#/c/32381/10/src/drivers/intel/fsp1_1/Kconfig@57 PS10, Line 57: This must match the : value that is set in the FSP binary
I checked the current binary with BCT, it's 0x0xfff20000, so I'll adjust based on use of CONFIG_USE_ […]
This base if for the non-SecureBoot only! The SecureBootEnabled default FSP_LOC is 0xfff9c000.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32381 )
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/#/c/32381/10/src/drivers/intel/fsp1_1/Kconfig File src/drivers/intel/fsp1_1/Kconfig:
https://review.coreboot.org/#/c/32381/10/src/drivers/intel/fsp1_1/Kconfig@57 PS10, Line 57: This must match the : value that is set in the FSP binary
This base if for the non-SecureBoot only! […]
this has been moved to CB:32535 I'm not sure what you're referring to exactly, but if you're using a value other than the default for the 1.1.8.0 FSP binary, then you'd simply continue to override at either the board level or in your .config
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32381 )
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
Patch Set 12:
Patch Set 12:
(1 comment)
In previous comment you wrote current binary is at 0xfff2000, I just mentioned that the SecureBootEnabled binary uses another value.
For sure override will be used when required.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32381 )
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
Patch Set 12:
In previous comment you wrote current binary is at 0xfff2000, I just mentioned that the SecureBootEnabled binary uses another value. For sure override will be used when required.
ok, well since that's now in another patch, let's move discussion there and adjust as needed.
I think this one is good as-is now, once you've booted on one of your boards
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32381 )
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
Patch Set 12: Code-Review+1
Patch Set 12:
In previous comment you wrote current binary is at 0xfff2000, I just mentioned that the SecureBootEnabled binary uses another value. For sure override will be used when required.
ok, well since that's now in another patch, let's move discussion there and adjust as needed.
I think this one is good as-is now, once you've booted on one of your boards
This patch set is working fine on our boards.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32381 )
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
Patch Set 12: Code-Review+2
Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32381 )
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
soc/intel/braswell: add default option to use public FSP
The current Braswell FSP 1.1 header in vendorcode/intel, for which there is no publicly available FSP binary, contains silicon init UPDs which are not found in the publicly available header/binary in the FSP Github repo. This prevents new boards from being added which use the public Braswell FSP header/binary.
To resolve this, move the UPDs not found in the public header from the soc's chip.c to ramstage.c for the boards which use them. Add a Kconfig option to use the current non-public FSP header and select it for boards which need it (google/cyan variants); set the public FSP option as the default. Use the Kconfig option to set FSP_HEADER_PATH to ensure the correct header is used.
Test: build google/cyan and intel/strago using non-public and public FSP header/binaries respectively.
Change-Id: I43cf18b98c844175a87b61fdbe4b0b24484e5702 Signed-off-by: Matt DeVillier matt.devillier@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/32381 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Frans Hendriks fhendriks@eltan.com Reviewed-by: Nico Huber nico.h@gmx.de --- M src/mainboard/google/cyan/Kconfig M src/mainboard/google/cyan/variants/celes/devicetree.cb M src/mainboard/google/cyan/variants/celes/ramstage.c M src/mainboard/google/cyan/variants/kefka/Makefile.inc M src/mainboard/google/cyan/variants/kefka/devicetree.cb A src/mainboard/google/cyan/variants/kefka/ramstage.c M src/mainboard/google/cyan/variants/relm/devicetree.cb M src/mainboard/google/cyan/variants/relm/ramstage.c M src/soc/intel/braswell/Kconfig M src/soc/intel/braswell/Makefile.inc M src/soc/intel/braswell/chip.c M src/soc/intel/braswell/chip.h 12 files changed, 62 insertions(+), 51 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Frans Hendriks: Looks good to me, but someone else must approve
diff --git a/src/mainboard/google/cyan/Kconfig b/src/mainboard/google/cyan/Kconfig index b3c6790..aac14c0 100644 --- a/src/mainboard/google/cyan/Kconfig +++ b/src/mainboard/google/cyan/Kconfig @@ -16,6 +16,7 @@ select HAVE_ACPI_RESUME select PCIEXP_L1_SUB_STATE if !BOARD_GOOGLE_CYAN select SYSTEM_TYPE_LAPTOP + select USE_GOOGLE_FSP
if BOARD_GOOGLE_BASEBOARD_CYAN
diff --git a/src/mainboard/google/cyan/variants/celes/devicetree.cb b/src/mainboard/google/cyan/variants/celes/devicetree.cb index 2e708af..a1ab510 100644 --- a/src/mainboard/google/cyan/variants/celes/devicetree.cb +++ b/src/mainboard/google/cyan/variants/celes/devicetree.cb @@ -73,12 +73,6 @@ register "ISPEnable" = "0" # Disable IUNIT register "ISPPciDevConfig" = "3" register "PcdSdDetectChk" = "0" # Disable SD card detect - # Follow Intel recommendation to set BSW D-stepping PERPORTRXISET 2 (low strength) - register "D0Usb2Port0PerPortRXISet" = "2" - register "D0Usb2Port1PerPortRXISet" = "2" - register "D0Usb2Port2PerPortRXISet" = "2" - register "D0Usb2Port3PerPortRXISet" = "2" - register "D0Usb2Port4PerPortRXISet" = "2"
# LPE audio codec settings register "lpe_codec_clk_src" = "LPE_CLK_SRC_XTAL" # 19.2MHz clock diff --git a/src/mainboard/google/cyan/variants/celes/ramstage.c b/src/mainboard/google/cyan/variants/celes/ramstage.c index 88b17f5..6c522a1 100644 --- a/src/mainboard/google/cyan/variants/celes/ramstage.c +++ b/src/mainboard/google/cyan/variants/celes/ramstage.c @@ -19,29 +19,36 @@ { if (SocStepping() >= SocD0) {
+ //Follow Intel recommendation to set + //BSW D-stepping PERPORTRXISET 2 (low strength) params->Usb2Port0PerPortPeTxiSet = 7; params->Usb2Port0PerPortTxiSet = 0; params->Usb2Port0IUsbTxEmphasisEn = 3; params->Usb2Port0PerPortTxPeHalf = 1; + params->D0Usb2Port0PerPortRXISet = 2;
params->Usb2Port1PerPortPeTxiSet = 7; params->Usb2Port1PerPortTxiSet = 0; params->Usb2Port1IUsbTxEmphasisEn = 3; params->Usb2Port1PerPortTxPeHalf = 1; + params->D0Usb2Port1PerPortRXISet = 2;
params->Usb2Port2PerPortPeTxiSet = 7; params->Usb2Port2PerPortTxiSet = 6; params->Usb2Port2IUsbTxEmphasisEn = 3; params->Usb2Port2PerPortTxPeHalf = 1; + params->D0Usb2Port2PerPortRXISet = 2;
params->Usb2Port3PerPortPeTxiSet = 7; params->Usb2Port3PerPortTxiSet = 6; params->Usb2Port3IUsbTxEmphasisEn = 3; params->Usb2Port3PerPortTxPeHalf = 1; + params->D0Usb2Port3PerPortRXISet = 2;
params->Usb2Port4PerPortPeTxiSet = 7; params->Usb2Port4PerPortTxiSet = 6; params->Usb2Port4IUsbTxEmphasisEn = 3; params->Usb2Port4PerPortTxPeHalf = 1; + params->D0Usb2Port4PerPortRXISet = 2; } } diff --git a/src/mainboard/google/cyan/variants/kefka/Makefile.inc b/src/mainboard/google/cyan/variants/kefka/Makefile.inc index 5e94e71..7799e8d 100644 --- a/src/mainboard/google/cyan/variants/kefka/Makefile.inc +++ b/src/mainboard/google/cyan/variants/kefka/Makefile.inc @@ -18,6 +18,7 @@ romstage-y += spd_util.c
ramstage-y += gpio.c +ramstage-y += ramstage.c
SPD_BIN = $(obj)/spd.bin
diff --git a/src/mainboard/google/cyan/variants/kefka/devicetree.cb b/src/mainboard/google/cyan/variants/kefka/devicetree.cb index 1ce056f..807dbcb 100644 --- a/src/mainboard/google/cyan/variants/kefka/devicetree.cb +++ b/src/mainboard/google/cyan/variants/kefka/devicetree.cb @@ -80,13 +80,6 @@ register "I2C5Frequency" = "1" register "I2C6Frequency" = "1"
- # Follow Intel recommendation to set BSW D-stepping PERPORTRXISET 2 (low strength) - register "D0Usb2Port0PerPortRXISet" = "2" - register "D0Usb2Port1PerPortRXISet" = "2" - register "D0Usb2Port2PerPortRXISet" = "2" - register "D0Usb2Port3PerPortRXISet" = "2" - register "D0Usb2Port4PerPortRXISet" = "2" - # LPE audio codec settings register "lpe_codec_clk_src" = "LPE_CLK_SRC_XTAL" # 19.2MHz clock
diff --git a/src/mainboard/google/cyan/variants/kefka/ramstage.c b/src/mainboard/google/cyan/variants/kefka/ramstage.c new file mode 100644 index 0000000..d790708 --- /dev/null +++ b/src/mainboard/google/cyan/variants/kefka/ramstage.c @@ -0,0 +1,30 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2014 Intel Corporation + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <soc/ramstage.h> + +void board_silicon_USB2_override(SILICON_INIT_UPD *params) +{ + if (SocStepping() >= SocD0) { + + //Follow Intel recommendation to set + //BSW D-stepping PERPORTRXISET 2 (low strength) + params->D0Usb2Port0PerPortRXISet = 2; + params->D0Usb2Port1PerPortRXISet = 2; + params->D0Usb2Port2PerPortRXISet = 2; + params->D0Usb2Port3PerPortRXISet = 2; + params->D0Usb2Port4PerPortRXISet = 2; + } +} diff --git a/src/mainboard/google/cyan/variants/relm/devicetree.cb b/src/mainboard/google/cyan/variants/relm/devicetree.cb index 65e662c..e1bbb0a 100644 --- a/src/mainboard/google/cyan/variants/relm/devicetree.cb +++ b/src/mainboard/google/cyan/variants/relm/devicetree.cb @@ -80,13 +80,6 @@ register "I2C5Frequency" = "1" register "I2C6Frequency" = "1"
- # Follow Intel recommendation to set BSW D-stepping PERPORTRXISET 2 (low strength) - register "D0Usb2Port0PerPortRXISet" = "2" - register "D0Usb2Port1PerPortRXISet" = "2" - register "D0Usb2Port2PerPortRXISet" = "2" - register "D0Usb2Port3PerPortRXISet" = "2" - register "D0Usb2Port4PerPortRXISet" = "2" - # LPE audio codec settings register "lpe_codec_clk_src" = "LPE_CLK_SRC_XTAL" # 19.2MHz clock
diff --git a/src/mainboard/google/cyan/variants/relm/ramstage.c b/src/mainboard/google/cyan/variants/relm/ramstage.c index 27f9dfa..3fbd2ae 100644 --- a/src/mainboard/google/cyan/variants/relm/ramstage.c +++ b/src/mainboard/google/cyan/variants/relm/ramstage.c @@ -36,5 +36,13 @@ params->Usb2Port3PerPortTxiSet = 0; params->Usb2Port3IUsbTxEmphasisEn = 2; params->Usb2Port3PerPortTxPeHalf = 1; + + //Follow Intel recommendation to set + //BSW D-stepping PERPORTRXISET 2 (low strength) + params->D0Usb2Port0PerPortRXISet = 2; + params->D0Usb2Port1PerPortRXISet = 2; + params->D0Usb2Port2PerPortRXISet = 2; + params->D0Usb2Port3PerPortRXISet = 2; + params->D0Usb2Port4PerPortRXISet = 2; } } diff --git a/src/soc/intel/braswell/Kconfig b/src/soc/intel/braswell/Kconfig index a0c0708..ed5c972 100644 --- a/src/soc/intel/braswell/Kconfig +++ b/src/soc/intel/braswell/Kconfig @@ -135,4 +135,18 @@ Enable this to disable the HPET support Solves the Linux MP-BIOS bug timer not connected.
+config USE_GOOGLE_FSP + bool + help + Select this to use Google's custom Braswell FSP header/binary + instead of the public release on Github. Only google/cyan + variants require this; all other boards should use the public release. + +config FSP_HEADER_PATH + string + default "$(src)/vendorcode/intel/fsp/fsp1_1/braswell" if USE_GOOGLE_FSP + default "3rdparty/fsp/BraswellFspBinPkg/Include/" + help + Location of FSP header file FspUpdVpd.h + endif diff --git a/src/soc/intel/braswell/Makefile.inc b/src/soc/intel/braswell/Makefile.inc index a538f7d..a7ed4c5 100644 --- a/src/soc/intel/braswell/Makefile.inc +++ b/src/soc/intel/braswell/Makefile.inc @@ -56,7 +56,7 @@
CPPFLAGS_common += -I$(src)/soc/intel/braswell/ CPPFLAGS_common += -I$(src)/soc/intel/braswell/include -CPPFLAGS_common += -I$(src)/vendorcode/intel/fsp/fsp1_1/braswell +CPPFLAGS_common += -I$(call strip_quotes,$(CONFIG_FSP_HEADER_PATH))
CPPFLAGS_common += -I3rdparty/blobs/mainboard/$(MAINBOARDDIR)
diff --git a/src/soc/intel/braswell/chip.c b/src/soc/intel/braswell/chip.c index 4be13cd..900b2f3 100644 --- a/src/soc/intel/braswell/chip.c +++ b/src/soc/intel/braswell/chip.c @@ -129,36 +129,26 @@ params->Usb2Port0PerPortTxiSet = config->Usb2Port0PerPortTxiSet; params->Usb2Port0IUsbTxEmphasisEn = config->Usb2Port0IUsbTxEmphasisEn; params->Usb2Port0PerPortTxPeHalf = config->Usb2Port0PerPortTxPeHalf; - if (config->D0Usb2Port0PerPortRXISet != 0) - params->D0Usb2Port0PerPortRXISet = config->D0Usb2Port0PerPortRXISet;
params->Usb2Port1PerPortPeTxiSet = config->Usb2Port1PerPortPeTxiSet; params->Usb2Port1PerPortTxiSet = config->Usb2Port1PerPortTxiSet; params->Usb2Port1IUsbTxEmphasisEn = config->Usb2Port1IUsbTxEmphasisEn; params->Usb2Port1PerPortTxPeHalf = config->Usb2Port1PerPortTxPeHalf; - if (config->D0Usb2Port1PerPortRXISet != 0) - params->D0Usb2Port1PerPortRXISet = config->D0Usb2Port1PerPortRXISet;
params->Usb2Port2PerPortPeTxiSet = config->Usb2Port2PerPortPeTxiSet; params->Usb2Port2PerPortTxiSet = config->Usb2Port2PerPortTxiSet; params->Usb2Port2IUsbTxEmphasisEn = config->Usb2Port2IUsbTxEmphasisEn; params->Usb2Port2PerPortTxPeHalf = config->Usb2Port2PerPortTxPeHalf; - if (config->D0Usb2Port2PerPortRXISet != 0) - params->D0Usb2Port2PerPortRXISet = config->D0Usb2Port2PerPortRXISet;
params->Usb2Port3PerPortPeTxiSet = config->Usb2Port3PerPortPeTxiSet; params->Usb2Port3PerPortTxiSet = config->Usb2Port3PerPortTxiSet; params->Usb2Port3IUsbTxEmphasisEn = config->Usb2Port3IUsbTxEmphasisEn; params->Usb2Port3PerPortTxPeHalf = config->Usb2Port3PerPortTxPeHalf; - if (config->D0Usb2Port3PerPortRXISet != 0) - params->D0Usb2Port3PerPortRXISet = config->D0Usb2Port3PerPortRXISet;
params->Usb2Port4PerPortPeTxiSet = config->Usb2Port4PerPortPeTxiSet; params->Usb2Port4PerPortTxiSet = config->Usb2Port4PerPortTxiSet; params->Usb2Port4IUsbTxEmphasisEn = config->Usb2Port4IUsbTxEmphasisEn; params->Usb2Port4PerPortTxPeHalf = config->Usb2Port4PerPortTxPeHalf; - if (config->D0Usb2Port4PerPortRXISet != 0) - params->D0Usb2Port4PerPortRXISet = config->D0Usb2Port4PerPortRXISet;
params->Usb3Lane0Ow2tapgen2deemph3p5 = config->Usb3Lane0Ow2tapgen2deemph3p5; @@ -266,9 +256,6 @@ fsp_display_upd_value("Usb2Port0PerPortTxPeHalf", 1, old->Usb2Port0PerPortTxPeHalf, new->Usb2Port0PerPortTxPeHalf); - fsp_display_upd_value("D0Usb2Port0PerPortRXISet", 1, - old->D0Usb2Port0PerPortRXISet, - new->D0Usb2Port0PerPortRXISet); fsp_display_upd_value("Usb2Port1PerPortPeTxiSet", 1, old->Usb2Port1PerPortPeTxiSet, new->Usb2Port1PerPortPeTxiSet); @@ -281,9 +268,6 @@ fsp_display_upd_value("Usb2Port1PerPortTxPeHalf", 1, old->Usb2Port1PerPortTxPeHalf, new->Usb2Port1PerPortTxPeHalf); - fsp_display_upd_value("D0Usb2Port1PerPortRXISet", 1, - old->D0Usb2Port1PerPortRXISet, - new->D0Usb2Port1PerPortRXISet); fsp_display_upd_value("Usb2Port2PerPortPeTxiSet", 1, old->Usb2Port2PerPortPeTxiSet, new->Usb2Port2PerPortPeTxiSet); @@ -296,9 +280,6 @@ fsp_display_upd_value("Usb2Port2PerPortTxPeHalf", 1, old->Usb2Port2PerPortTxPeHalf, new->Usb2Port2PerPortTxPeHalf); - fsp_display_upd_value("D0Usb2Port2PerPortRXISet", 1, - old->D0Usb2Port2PerPortRXISet, - new->D0Usb2Port2PerPortRXISet); fsp_display_upd_value("Usb2Port3PerPortPeTxiSet", 1, old->Usb2Port3PerPortPeTxiSet, new->Usb2Port3PerPortPeTxiSet); @@ -311,9 +292,6 @@ fsp_display_upd_value("Usb2Port3PerPortTxPeHalf", 1, old->Usb2Port3PerPortTxPeHalf, new->Usb2Port3PerPortTxPeHalf); - fsp_display_upd_value("D0Usb2Port3PerPortRXISet", 1, - old->D0Usb2Port3PerPortRXISet, - new->D0Usb2Port3PerPortRXISet); fsp_display_upd_value("Usb2Port4PerPortPeTxiSet", 1, old->Usb2Port4PerPortPeTxiSet, new->Usb2Port4PerPortPeTxiSet); @@ -326,9 +304,6 @@ fsp_display_upd_value("Usb2Port4PerPortTxPeHalf", 1, old->Usb2Port4PerPortTxPeHalf, new->Usb2Port4PerPortTxPeHalf); - fsp_display_upd_value("D0Usb2Port4PerPortRXISet", 1, - old->D0Usb2Port4PerPortRXISet, - new->D0Usb2Port4PerPortRXISet); fsp_display_upd_value("Usb3Lane0Ow2tapgen2deemph3p5", 1, old->Usb3Lane0Ow2tapgen2deemph3p5, new->Usb3Lane0Ow2tapgen2deemph3p5); diff --git a/src/soc/intel/braswell/chip.h b/src/soc/intel/braswell/chip.h index 4afaf44..bb06dd5 100644 --- a/src/soc/intel/braswell/chip.h +++ b/src/soc/intel/braswell/chip.h @@ -172,11 +172,6 @@ UINT8 I2C4Frequency; UINT8 I2C5Frequency; UINT8 I2C6Frequency; - UINT8 D0Usb2Port0PerPortRXISet; /*setting for D0 stepping SOC*/ - UINT8 D0Usb2Port1PerPortRXISet; /*setting for D0 stepping SOC*/ - UINT8 D0Usb2Port2PerPortRXISet; /*setting for D0 stepping SOC*/ - UINT8 D0Usb2Port3PerPortRXISet; /*setting for D0 stepping SOC*/ - UINT8 D0Usb2Port4PerPortRXISet; /*setting for D0 stepping SOC*/ };
extern struct chip_operations soc_intel_braswell_ops;
-
build bot (Jenkins) (Code Review) -
Frans Hendriks (Code Review)
-
Matt DeVillier (Code Review)
-
Nico Huber (Code Review)
-
Philipp Deppenwiese (Code Review)