coreboot-gerrit December 2020

coreboot-gerrit@coreboot.org

1 participants
3168 discussions

Change in coreboot[master]: soc/amd/picasso: move chipset_handle_reset to common
by Felix Held (Code Review) 11 Dec '20

11 Dec '20

Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48537 ) Change subject: soc/amd/picasso: move chipset_handle_reset to common ...................................................................... soc/amd/picasso: move chipset_handle_reset to common The FSP integration code needs this function to be present. It's not supposed to be called, but if it is, it'll print an error and call the SoC's cold reset function. Change-Id: I15f2622d9d9d0f22e3cf8e6283b578f5933b1a9f Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> --- M src/soc/amd/common/Makefile.inc A src/soc/amd/common/fsp/Makefile.inc A src/soc/amd/common/fsp/fsp_reset.c M src/soc/amd/picasso/reset.c 4 files changed, 20 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/48537/1 diff --git a/src/soc/amd/common/Makefile.inc b/src/soc/amd/common/Makefile.inc index 418539b..4f1052d 100644 --- a/src/soc/amd/common/Makefile.inc +++ b/src/soc/amd/common/Makefile.inc @@ -1,4 +1,5 @@ ifeq ($(CONFIG_SOC_AMD_COMMON),y) subdirs-y += block +subdirs-y += fsp subdirs-y += vboot endif diff --git a/src/soc/amd/common/fsp/Makefile.inc b/src/soc/amd/common/fsp/Makefile.inc new file mode 100644 index 0000000..5523876 --- /dev/null +++ b/src/soc/amd/common/fsp/Makefile.inc @@ -0,0 +1,4 @@ +ifeq ($(CONFIG_PLATFORM_USES_FSP2_0),y) +romstage-y += fsp_reset.c +ramstage-y += fsp_reset.c +endif # CONFIG_PLATFORM_USES_FSP2_0 diff --git a/src/soc/amd/common/fsp/fsp_reset.c b/src/soc/amd/common/fsp/fsp_reset.c new file mode 100644 index 0000000..62480bf --- /dev/null +++ b/src/soc/amd/common/fsp/fsp_reset.c @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <amdblocks/reset.h> +#include <assert.h> +#include <console/console.h> +#include <fsp/util.h> +#include <stdint.h> + +void chipset_handle_reset(uint32_t status) +{ + printk(BIOS_ERR, "Error: unexpected call to %s(0x%08x). Doing cold reset.\n", + __func__, status); + BUG(); + do_cold_reset(); +} diff --git a/src/soc/amd/picasso/reset.c b/src/soc/amd/picasso/reset.c index e16223a..89b5b80 100644 --- a/src/soc/amd/picasso/reset.c +++ b/src/soc/amd/picasso/reset.c @@ -1,15 +1,12 @@ /* SPDX-License-Identifier: GPL-2.0-only */ #include <arch/io.h> -#include <console/console.h> #include <cf9_reset.h> #include <reset.h> #include <soc/reset.h> #include <soc/southbridge.h> #include <amdblocks/acpimmio.h> #include <amdblocks/reset.h> -#include <fsp/util.h> -#include <assert.h> void set_warm_reset_flag(void) { @@ -44,11 +41,3 @@ /* TODO: Would a warm_reset() suffice? */ do_cold_reset(); } - -void chipset_handle_reset(uint32_t status) -{ - printk(BIOS_ERR, "Error: unexpected call to %s(0x%08x). Doing cold reset.\n", - __func__, status); - BUG(); - do_cold_reset(); -} -- To view, visit https://review.coreboot.org/c/coreboot/+/48537 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I15f2622d9d9d0f22e3cf8e6283b578f5933b1a9f Gerrit-Change-Number: 48537 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-MessageType: newchange

3 3

Change in coreboot[master]: soc/intel/jasperlake: Enables CSE Lite driver for JSL platform in the...
by Sridhar Siricilla (Code Review) 11 Dec '20

11 Dec '20

Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48280 ) Change subject: soc/intel/jasperlake: Enables CSE Lite driver for JSL platform in the romstage ...................................................................... Patch Set 5: (2 comments) https://review.coreboot.org/c/coreboot/+/48280/2/src/soc/intel/jasperlake/r… File src/soc/intel/jasperlake/romstage/romstage.c: https://review.coreboot.org/c/coreboot/+/48280/2/src/soc/intel/jasperlake/r… PS2, Line 138: if (!s3wake) > I think it is safe to do the call to cse_fw_sync() before save_dimm_info() and also restrict the cal […] Ack https://review.coreboot.org/c/coreboot/+/48280/2/src/soc/intel/jasperlake/r… PS2, Line 142: cse_fw_sync(); > I wonder if we should add dram_init_done_hooks just like we have cbmem_init_hooks. […] Agree, I will explore the suggested method.. -- To view, visit https://review.coreboot.org/c/coreboot/+/48280 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I43030e77f6ede53c23e6c9e65d34db85c141e13a Gerrit-Change-Number: 48280 Gerrit-PatchSet: 5 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 11 Dec 2020 17:16:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/tigerlake: Enable CSE Lite driver for TGL platform in romstage
by Sridhar Siricilla (Code Review) 11 Dec '20

11 Dec '20

Hello V Sowmya, build bot (Jenkins), Furquan Shaikh, Jamie Ryu, Tim Wawrzynczak, Paul Menzel, Rizwan Qureshi, Sugnan Prabhu S, Krishna P Bhat D, Patrick Rudolph, Karthik Ramasubramanian, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/48281 to look at the new patch set (#5). Change subject: soc/intel/tigerlake: Enable CSE Lite driver for TGL platform in romstage ...................................................................... soc/intel/tigerlake: Enable CSE Lite driver for TGL platform in romstage This patch sets up cse_fw_sync() call in the romstage. The cse_fw_syncO() must be called after DRAM initialization. BUG=b:174694480 Test=Verified on Tigerlake platform Signed-off-by: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Change-Id: Ib485a4d1d15989b162105deb32bb317d7a0f2856 --- M src/soc/intel/tigerlake/romstage/romstage.c 1 file changed, 8 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/48281/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/48281 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib485a4d1d15989b162105deb32bb317d7a0f2856 Gerrit-Change-Number: 48281 Gerrit-PatchSet: 5 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/intel/jasperlake: Enables CSE Lite driver for JSL platform in the...
by Sridhar Siricilla (Code Review) 11 Dec '20

11 Dec '20

Hello V Sowmya, build bot (Jenkins), Furquan Shaikh, Jamie Ryu, Tim Wawrzynczak, Paul Menzel, Sugnan Prabhu S, Rizwan Qureshi, Krishna P Bhat D, Patrick Rudolph, Karthik Ramasubramanian, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/48280 to look at the new patch set (#5). Change subject: soc/intel/jasperlake: Enables CSE Lite driver for JSL platform in the romstage ...................................................................... soc/intel/jasperlake: Enables CSE Lite driver for JSL platform in the romstage This patch sets up cse_fw_sync() call in the romstage.The cse_fw_sync() must be called after DRAM initialization. BUG=b:174694480 Test=Verified on Drawlet Signed-off-by: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Change-Id: I43030e77f6ede53c23e6c9e65d34db85c141e13a --- M src/soc/intel/jasperlake/romstage/romstage.c 1 file changed, 7 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/48280/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/48280 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I43030e77f6ede53c23e6c9e65d34db85c141e13a Gerrit-Change-Number: 48280 Gerrit-PatchSet: 5 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/intel/common: Move CSE Lite driver functionality into romstage
by Sridhar Siricilla (Code Review) 11 Dec '20

11 Dec '20

Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Jamie Ryu, Tim Wawrzynczak, Rizwan Qureshi, Sugnan Prabhu S, Krishna P Bhat D, Patrick Rudolph, Karthik Ramasubramanian, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/48279 to look at the new patch set (#5). Change subject: soc/intel/common: Move CSE Lite driver functionality into romstage ...................................................................... soc/intel/common: Move CSE Lite driver functionality into romstage The patch sets up the CSE Lite driver in the romstage instead of ramstage. With this change, CSE Lite driver sets CSE's boot partition and triggers CSE FW update in the romstage. The cse_fw_sync() must be called after DRAM initialized. Test=Verified on JSL and TGL platforms BUG=b:174694480 Signed-off-by: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Change-Id: I2fd562a5c6c8501226abbcb68021d9356bcf0b73 --- M src/soc/intel/common/block/cse/Makefile.inc M src/soc/intel/common/block/cse/cse_lite.c M src/soc/intel/common/block/include/intelblocks/cse.h 3 files changed, 3 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/48279/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/48279 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2fd562a5c6c8501226abbcb68021d9356bcf0b73 Gerrit-Change-Number: 48279 Gerrit-PatchSet: 5 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/google/volteer: Switch to fw_config_probe_nodefault when appropriate
by Tim Wawrzynczak (Code Review) 11 Dec '20

11 Dec '20

Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47972 ) Change subject: mb/google/volteer: Switch to fw_config_probe_nodefault when appropriate ...................................................................... mb/google/volteer: Switch to fw_config_probe_nodefault when appropriate In cases when a volteer device is unprovisioned, the safest thing to do for GPIOs that will normally be used for audio codec buses is to leave them disabled (configured as PAD_CFG_NC). The same is true for USB4 support; if it is not know explicitly to be supported, then leave the iTBT PCIe RPs disabled. Change-Id: I8efd101174f6e3d7233d2bf803b680673cada81a Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> --- M src/mainboard/google/volteer/fw_config.c M src/mainboard/google/volteer/mainboard.c M src/mainboard/google/volteer/romstage.c 3 files changed, 26 insertions(+), 16 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/47972/1 diff --git a/src/mainboard/google/volteer/fw_config.c b/src/mainboard/google/volteer/fw_config.c index 50e8f93..7be84bf 100644 --- a/src/mainboard/google/volteer/fw_config.c +++ b/src/mainboard/google/volteer/fw_config.c @@ -72,33 +72,43 @@ static void fw_config_handle(void *unused) { + /* + * When FW_CONFIG is undefined (e.g., in an unprovisioned board), we + * don't yet know which audio codec is connected. Our policy here is: + * 1) Configure bus-specific GPIOs which are not yet assigned to a device as NC. + * 2) Expose all of the devices in the ACPI tables, so they can be probed at runtime + * discovery and provisioning. + * Here, that means using fw_config_probe for AUDIO,NONE because it will return true + * for an unprovisioned board, and all GPIOs will get enabled; fw_config_probe_nodefault + * can only return true on provisioned boards. + */ if (fw_config_probe(FW_CONFIG(AUDIO, NONE))) { printk(BIOS_INFO, "Configure GPIOs for no audio.\n"); gpio_configure_pads(i2s_disable_pads, ARRAY_SIZE(i2s_disable_pads)); gpio_configure_pads(dmic_disable_pads, ARRAY_SIZE(dmic_disable_pads)); gpio_configure_pads(sndw_disable_pads, ARRAY_SIZE(sndw_disable_pads)); } - if (fw_config_probe(FW_CONFIG(AUDIO, MAX98373_ALC5682_SNDW))) { + if (fw_config_probe_nodefault(FW_CONFIG(AUDIO, MAX98373_ALC5682_SNDW))) { printk(BIOS_INFO, "Configure GPIOs for SoundWire audio.\n"); gpio_configure_pads(sndw_enable_pads, ARRAY_SIZE(sndw_enable_pads)); gpio_configure_pads(dmic_enable_pads, ARRAY_SIZE(dmic_enable_pads)); gpio_configure_pads(i2s_disable_pads, ARRAY_SIZE(i2s_disable_pads)); } - if (fw_config_probe(FW_CONFIG(AUDIO, MAX98357_ALC5682I_I2S)) || - fw_config_probe(FW_CONFIG(AUDIO, MAX98373_ALC5682I_I2S)) || - fw_config_probe(FW_CONFIG(AUDIO, MAX98360_ALC5682I_I2S))) { + if (fw_config_probe_nodefault(FW_CONFIG(AUDIO, MAX98357_ALC5682I_I2S)) || + fw_config_probe_nodefault(FW_CONFIG(AUDIO, MAX98373_ALC5682I_I2S)) || + fw_config_probe_nodefault(FW_CONFIG(AUDIO, MAX98360_ALC5682I_I2S))) { printk(BIOS_INFO, "Configure GPIOs for I2S audio on UP3.\n"); gpio_configure_pads(i2s_up3_enable_pads, ARRAY_SIZE(i2s_up3_enable_pads)); gpio_configure_pads(dmic_enable_pads, ARRAY_SIZE(dmic_enable_pads)); gpio_configure_pads(sndw_disable_pads, ARRAY_SIZE(sndw_disable_pads)); } - if (fw_config_probe(FW_CONFIG(AUDIO, MAX98373_ALC5682I_I2S_UP4))) { + if (fw_config_probe_nodefault(FW_CONFIG(AUDIO, MAX98373_ALC5682I_I2S_UP4))) { printk(BIOS_INFO, "Configure GPIOs for I2S audio on UP4.\n"); gpio_configure_pads(i2s_up4_enable_pads, ARRAY_SIZE(i2s_up4_enable_pads)); gpio_configure_pads(dmic_enable_pads, ARRAY_SIZE(dmic_enable_pads)); gpio_configure_pads(sndw_disable_pads, ARRAY_SIZE(sndw_disable_pads)); } - if (fw_config_probe(FW_CONFIG(DB_SD, SD_GL9755S))) { + if (fw_config_probe_nodefault(FW_CONFIG(DB_SD, SD_GL9755S))) { printk(BIOS_INFO, "Configure GPIOs for SD GL9755S.\n"); gpio_configure_pads(sd_gl9755s_pads, ARRAY_SIZE(sd_gl9755s_pads)); } diff --git a/src/mainboard/google/volteer/mainboard.c b/src/mainboard/google/volteer/mainboard.c index ea9e08f..2f016ab 100644 --- a/src/mainboard/google/volteer/mainboard.c +++ b/src/mainboard/google/volteer/mainboard.c @@ -53,10 +53,10 @@ if (!conn) return; - if (fw_config_probe(FW_CONFIG(DB_USB, USB4_GEN2)) || - fw_config_probe(FW_CONFIG(DB_USB, USB3_ACTIVE)) || - fw_config_probe(FW_CONFIG(DB_USB, USB4_GEN3)) || - fw_config_probe(FW_CONFIG(DB_USB, USB3_NO_A))) { + if (fw_config_probe_nodefault(FW_CONFIG(DB_USB, USB4_GEN2)) || + fw_config_probe_nodefault(FW_CONFIG(DB_USB, USB3_ACTIVE)) || + fw_config_probe_nodefault(FW_CONFIG(DB_USB, USB4_GEN3)) || + fw_config_probe_nodefault(FW_CONFIG(DB_USB, USB3_NO_A))) { struct drivers_intel_pmc_mux_conn_config *config = conn->chip_info; if (config) { @@ -164,8 +164,8 @@ bool has_usb4; /* If device doesn't have USB4 hardware, disable tbt */ - has_usb4 = (fw_config_probe(FW_CONFIG(DB_USB, USB4_GEN2)) - || fw_config_probe(FW_CONFIG(DB_USB, USB4_GEN3))); + has_usb4 = (fw_config_probe_nodefault(FW_CONFIG(DB_USB, USB4_GEN2)) + || fw_config_probe_nodefault(FW_CONFIG(DB_USB, USB4_GEN3))); if (!has_usb4) memset(params->ITbtPcieRootPortEn, 0, diff --git a/src/mainboard/google/volteer/romstage.c b/src/mainboard/google/volteer/romstage.c index 315ec20..a529b75 100644 --- a/src/mainboard/google/volteer/romstage.c +++ b/src/mainboard/google/volteer/romstage.c @@ -22,15 +22,15 @@ }; bool half_populated = gpio_get(GPIO_MEM_CH_SEL); - /* Disable HDA device if no audio board is present. */ + /* Disable HDA device if no audio board is present (or FW_CONFIG is undefined). */ if (fw_config_probe(FW_CONFIG(AUDIO, NONE))) mem_cfg->PchHdaEnable = 0; meminit_ddr(mem_cfg, board_cfg, &spd_info, half_populated); - /* Disable TBT if no USB4 hardware */ - if (!(fw_config_probe(FW_CONFIG(DB_USB, USB4_GEN2)) || - fw_config_probe(FW_CONFIG(DB_USB, USB4_GEN3)))) { + /* Disable TBT if no USB4 hardware (or if unknown) */ + if (!(fw_config_probe_nodefault(FW_CONFIG(DB_USB, USB4_GEN2)) || + fw_config_probe_nodefault(FW_CONFIG(DB_USB, USB4_GEN3)))) { mem_cfg->TcssDma0En = 0; mem_cfg->TcssItbtPcie0En = 0; mem_cfg->TcssItbtPcie1En = 0; -- To view, visit https://review.coreboot.org/c/coreboot/+/47972 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8efd101174f6e3d7233d2bf803b680673cada81a Gerrit-Change-Number: 47972 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: newchange

5 12

Change in coreboot[master]: fw_config: Default to undefined fw_config value
by Tim Wawrzynczak (Code Review) 11 Dec '20

11 Dec '20

Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47956 ) Change subject: fw_config: Default to undefined fw_config value ...................................................................... fw_config: Default to undefined fw_config value If for some reason a value for fw_config cannot be located, set its value to a magic undefined value. The current `fw_config_probe` function is modified to return true if fw_config is undefined, and a new function `fw_config_probe_nodefault` is added which will return false if the fw_config value is undefined. A new Kconfig option is added, FW_CONFIG_IGNORE_UNDEFINED (defaults to n), which controls the behavior of fw_config probing the devicetree. Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Change-Id: Ib3046233667e97a5f78961fabacbeb3099b3d442 --- M src/Kconfig M src/include/fw_config.h M src/lib/fw_config.c 3 files changed, 43 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/56/47956/1 diff --git a/src/Kconfig b/src/Kconfig index dc98ca2..a0f692c 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -398,6 +398,16 @@ is not tried if FW_CONFIG_SOURCE_CBFS is enabled and the value was found in CBFS. +config FW_CONFIG_IGNORE_UNDEFINED + bool "Undefined Firmware Configuration value will not probe devices" + depends on FW_CONFIG + default n + help + When probing devices with fw_config, when the value is undefined or + unable to be located, all devices will be successfully probed, and + fw_config_probe will always return true. Set this to 'y' if you do + not want this behavior. + config HAVE_RAMPAYLOAD bool diff --git a/src/include/fw_config.h b/src/include/fw_config.h index 3c87725..e02a38e 100644 --- a/src/include/fw_config.h +++ b/src/include/fw_config.h @@ -45,11 +45,21 @@ * fw_config_probe() - Check if field and option matches. * @match: Structure containing field and option to probe. * - * Return %true if match is found, %false if match is not found. + * Return %true if match is found or fw_config value is undefined. + * %false if match is not found. */ bool fw_config_probe(const struct fw_config *match); /** + * fw_config_probe_nodefault() - Check if field and option matches. + * @match: Structure containing field and option to probe. + * + * Return %true if match is found. + * %false if match is not found or fw_config value is undefined + */ +bool fw_config_probe_nodefault(const struct fw_config *match); + +/** * fw_config_for_each_found() - Call a callback for each fw_config field found * @cb: The callback function * @arg: A context argument that is passed to the callback diff --git a/src/lib/fw_config.c b/src/lib/fw_config.c index e17d40e..efbee01 100644 --- a/src/lib/fw_config.c +++ b/src/lib/fw_config.c @@ -29,7 +29,7 @@ CBFS_TYPE_RAW) != sizeof(fw_config_value)) { printk(BIOS_WARNING, "%s: Could not get fw_config from CBFS\n", __func__); - fw_config_value = 0; + fw_config_value = UNDEFINED_FW_CONFIG; } else { printk(BIOS_INFO, "FW_CONFIG value from CBFS is 0x%" PRIx64 "\n", fw_config_value); @@ -39,16 +39,24 @@ /* Read the value from EC CBI. */ if (CONFIG(FW_CONFIG_SOURCE_CHROMEEC_CBI)) { - if (google_chromeec_cbi_get_fw_config(&fw_config_value)) + if (google_chromeec_cbi_get_fw_config(&fw_config_value)) { printk(BIOS_WARNING, "%s: Could not get fw_config from EC\n", __func__); + fw_config_value = UNDEFINED_FW_CONFIG; + } } printk(BIOS_INFO, "FW_CONFIG value is 0x%" PRIx64 "\n", fw_config_value); return fw_config_value; } -bool fw_config_probe(const struct fw_config *match) +static bool fw_config_match(const struct fw_config *match, bool undefined_match) { + uint64_t fw_config; + + fw_config = fw_config_get(); + if (fw_config == UNDEFINED_FW_CONFIG) + return undefined_match; + /* Compare to system value. */ if ((fw_config_get() & match->mask) == match->value) { if (match->field_name && match->option_name) @@ -64,6 +72,16 @@ return false; } +bool fw_config_probe(const struct fw_config *match) +{ + return fw_config_match(match, true); +} + +bool fw_config_probe_nodefault(const struct fw_config *match) +{ + return fw_config_match(match, false); +} + #if ENV_RAMSTAGE /* @@ -111,7 +129,7 @@ continue; for (probe = dev->probe_list; probe && probe->mask != 0; probe++) { - if (fw_config_probe(probe)) { + if (fw_config_match(probe, (bool)!CONFIG(FW_CONFIG_IGNORE_UNDEFINED))) { match = true; cached_configs[probe_index(probe->mask)] = probe; break; -- To view, visit https://review.coreboot.org/c/coreboot/+/47956 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib3046233667e97a5f78961fabacbeb3099b3d442 Gerrit-Change-Number: 47956 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: newchange

5 27

Change in coreboot[master]: soc/intel/common: Move CSE Lite driver functionality into romstage
by Sridhar Siricilla (Code Review) 11 Dec '20

11 Dec '20

Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48279 ) Change subject: soc/intel/common: Move CSE Lite driver functionality into romstage ...................................................................... Patch Set 4: (1 comment) https://review.coreboot.org/c/coreboot/+/48279/2//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/48279/2//COMMIT_MSG@13 PS2, Line 13: Verified on JSL and TGL platforms > Plan is to measure delta time between from romstage(after DRAM init) to ramstage(existing code locat […] From our experiment, we notice improvement of 154ms of time of cold boot time if we move cse_fw_sync() from ramstage to romstage. -- To view, visit https://review.coreboot.org/c/coreboot/+/48279 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2fd562a5c6c8501226abbcb68021d9356bcf0b73 Gerrit-Change-Number: 48279 Gerrit-PatchSet: 4 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 11 Dec 2020 16:55:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Comment-In-Reply-To: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/supermicro/x11ssm-f: disable unconnected and unused/strap-only pads
by Tim Wawrzynczak (Code Review) 11 Dec '20

11 Dec '20

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48100 ) Change subject: mb/supermicro/x11ssm-f: disable unconnected and unused/strap-only pads ...................................................................... Patch Set 14: (1 comment) https://review.coreboot.org/c/coreboot/+/48100/14/src/mainboard/supermicro/… File src/mainboard/supermicro/x11-lga1151-series/variants/x11ssm-f/gpio.c: https://review.coreboot.org/c/coreboot/+/48100/14/src/mainboard/supermicro/… PS14, Line 46: DN_20K > This one is the only expection in this patch. Actually, it is connected through a diode to HDA_SDO. […] Ack and lovely ASCII art ! -- To view, visit https://review.coreboot.org/c/coreboot/+/48100 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I06b942e3182469f87e41914c893e5b485ccca420 Gerrit-Change-Number: 48100 Gerrit-PatchSet: 14 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Maxim Polyakov <max.senia.poliak(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Fri, 11 Dec 2020 16:21:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/picasso: Add data fabric read helper function
by Felix Held (Code Review) 11 Dec '20

11 Dec '20

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47726 ) Change subject: soc/amd/picasso: Add data fabric read helper function ...................................................................... Patch Set 4: (1 comment) https://review.coreboot.org/c/coreboot/+/47726/2/src/soc/amd/picasso/includ… File src/soc/amd/picasso/include/soc/data_fabric.h: https://review.coreboot.org/c/coreboot/+/47726/2/src/soc/amd/picasso/includ… PS2, Line 62: BIT(0) > Hmm, we seem to oscillate back and forth. There was a big push before to use BIT(). i prefer the (1 << bit) notation; not sure if the BIT(bit) came from the UEFI side of things (or were there defines for each bit? not sure; all i remember is that i didn't like it). if in doubt, i tend to do what the native SNB/IVB code does; that one is what i consider being the most coreboot-ish code for a non-ancient platform. but yeah, i don't have too strong opinions on this one -- To view, visit https://review.coreboot.org/c/coreboot/+/47726 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If0dc72063fbb99efaeea3fccef16cc1b5b8526f1 Gerrit-Change-Number: 47726 Gerrit-PatchSet: 4 Gerrit-Owner: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)amd.corp-partner.google.com> Gerrit-Reviewer: Justin Frodsham <justin.frodsham(a)protonmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Matt Papageorge <matthewpapa07(a)gmail.com> Gerrit-Reviewer: Nikolai Vyssotski <nikolai.vyssotski(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 11 Dec 2020 16:20:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Marshall Dawson <marshalldawson3rd(a)gmail.com> Comment-In-Reply-To: Jason Glenesk <jason.glenesk(a)amd.corp-partner.google.com> Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit December 2020