coreboot-gerrit December 2020

coreboot-gerrit@coreboot.org

1 participants
3168 discussions

Change in coreboot[master]: device + util/sconfig: introduce new device `gpio`
by Michael Niewöhner (Code Review) 12 Dec '20

12 Dec '20

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48582 ) Change subject: device + util/sconfig: introduce new device `gpio` ...................................................................... Patch Set 3: This change is ready for review. -- To view, visit https://review.coreboot.org/c/coreboot/+/48582 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic4572ad8b37bd1afd2fb213b2c67fb8aec536786 Gerrit-Change-Number: 48582 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 13 Dec 2020 01:02:15 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/ipmi: add code to set BMC/IPMI enablement from jumper
by Michael Niewöhner (Code Review) 12 Dec '20

12 Dec '20

Hello Felix Singer, build bot (Jenkins), Nico Huber, Furquan Shaikh, Paul Menzel, Tim Wawrzynczak, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/48094 to look at the new patch set (#18). Change subject: drivers/ipmi: add code to set BMC/IPMI enablement from jumper ...................................................................... drivers/ipmi: add code to set BMC/IPMI enablement from jumper Some boards, like the Supermicro X11SSM-F, have a jumper for enabling or disabling the BMC and IPMI. Add a new devicetree driver option to set the GPIO used for the jumper and enable or disable IPMI according to its value. This gets used in the follow-up change by Supermicro X11SSM-F. Test: Boot with jumper set to each enabled and disabled and see if IPMI gets enabled. Successfully tested on Supermicro X11SSM-F. Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de> Change-Id: Icde3232843a7138797a4b106560f170972edeb9c --- M src/drivers/ipmi/chip.h M src/drivers/ipmi/ipmi_kcs_ops.c 2 files changed, 26 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/48094/18 -- To view, visit https://review.coreboot.org/c/coreboot/+/48094 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Icde3232843a7138797a4b106560f170972edeb9c Gerrit-Change-Number: 48094 Gerrit-PatchSet: 18 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> 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]: soc/amd/picasso: move UART Kconfig options to common folder
by Felix Held (Code Review) 12 Dec '20

12 Dec '20

Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48514 ) Change subject: soc/amd/picasso: move UART Kconfig options to common folder ...................................................................... soc/amd/picasso: move UART Kconfig options to common folder The actual UART initialization code will be factored out in follow-up commits. Change-Id: Ie4ddf1951b230323c5480c4389376c62dd74b0e1 Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> --- A src/soc/amd/common/block/uart/Kconfig M src/soc/amd/picasso/Kconfig 2 files changed, 58 insertions(+), 47 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/48514/1 diff --git a/src/soc/amd/common/block/uart/Kconfig b/src/soc/amd/common/block/uart/Kconfig new file mode 100644 index 0000000..599594b --- /dev/null +++ b/src/soc/amd/common/block/uart/Kconfig @@ -0,0 +1,57 @@ +config SOC_AMD_COMMON_BLOCK_UART + bool + default n + help + Select this option to add the common functions for setting up the + UART configuration to the build. + +if SOC_AMD_COMMON_BLOCK_UART + +config AMD_SOC_CONSOLE_UART + bool "Use integrated AMD SoC UART controller for console" + default n + select DRIVERS_UART_8250MEM + select DRIVERS_UART_8250MEM_32 + select NO_UART_ON_SUPERIO + select UART_OVERRIDE_REFCLK + help + There are four memory-mapped UARTs controllers at: + 0: 0xfedc9000 + 1: 0xfedca000 + 2: 0xfedc3000 + 3: 0xfedcf000 + +choice + prompt "UART Frequency" + depends on AMD_SOC_CONSOLE_UART + default AMD_SOC_UART_48MZ + +config AMD_SOC_UART_48MZ + bool "48 MHz clock" + help + Select this option for the most compatibility. + +config AMD_SOC_UART_1_8MZ + bool "1.8432 MHz clock" + help + Select this option if an old payload or Linux ttyS0 arguments require + a 1.8432 MHz clock source for the UART. + +endchoice + +config AMD_SOC_UART_LEGACY + bool "Decode legacy I/O range" + help + Assign I/O 3F8, 2F8, etc. to an integrated AMD SoC UART. A UART + accessed with I/O does not allow all the features of MMIO. The MMIO + decode is still present when this option is used. + +config CONSOLE_UART_BASE_ADDRESS + depends on CONSOLE_SERIAL && AMD_SOC_CONSOLE_UART + hex + default 0xfedc9000 if UART_FOR_CONSOLE = 0 + default 0xfedca000 if UART_FOR_CONSOLE = 1 + default 0xfedc3000 if UART_FOR_CONSOLE = 2 + default 0xfedcf000 if UART_FOR_CONSOLE = 3 + +endif # SOC_AMD_COMMON_BLOCK_UART diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig index c4c7bae..e9930e9 100644 --- a/src/soc/amd/picasso/Kconfig +++ b/src/soc/amd/picasso/Kconfig @@ -42,6 +42,7 @@ select SOC_AMD_COMMON_BLOCK_SMI select SOC_AMD_COMMON_BLOCK_SMU select SOC_AMD_COMMON_BLOCK_TSC_FAM17H_19H + select SOC_AMD_COMMON_BLOCK_UART select SOC_AMD_COMMON_BLOCK_PSP_GEN2 select PROVIDES_ROM_SHARING select BOOT_DEVICE_SUPPORTS_WRITES if BOOT_DEVICE_SPI_FLASH @@ -273,53 +274,6 @@ help Base address for the ACPI registers. -config AMD_SOC_CONSOLE_UART - bool "Use Picasso UART controller for console" - default n - select DRIVERS_UART_8250MEM - select DRIVERS_UART_8250MEM_32 - select NO_UART_ON_SUPERIO - select UART_OVERRIDE_REFCLK - help - There are four memory-mapped UARTs controllers in Picasso at: - 0: 0xfedc9000 - 1: 0xfedca000 - 2: 0xfedc3000 - 3: 0xfedcf000 - -choice - prompt "UART Frequency" - depends on AMD_SOC_CONSOLE_UART - default AMD_SOC_UART_48MZ - -config AMD_SOC_UART_48MZ - bool "48 MHz clock" - help - Select this option for the most compatibility. - -config AMD_SOC_UART_1_8MZ - bool "1.8432 MHz clock" - help - Select this option if an old payload or Linux ttyS0 arguments - require it. - -endchoice - -config AMD_SOC_UART_LEGACY - bool "Decode legacy I/O range" - help - Assign I/O 3F8, 2F8, etc. to a Picasso UART. A UART accessed with I/O - does not allow all the features of MMIO. The MMIO decode is still - present when this option is used. - -config CONSOLE_UART_BASE_ADDRESS - depends on CONSOLE_SERIAL && AMD_SOC_CONSOLE_UART - hex - default 0xfedc9000 if UART_FOR_CONSOLE = 0 - default 0xfedca000 if UART_FOR_CONSOLE = 1 - default 0xfedc3000 if UART_FOR_CONSOLE = 2 - default 0xfedcf000 if UART_FOR_CONSOLE = 3 - config SMM_TSEG_SIZE hex default 0x800000 if SMM_TSEG && HAVE_SMI_HANDLER -- To view, visit https://review.coreboot.org/c/coreboot/+/48514 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie4ddf1951b230323c5480c4389376c62dd74b0e1 Gerrit-Change-Number: 48514 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newchange

2 3

Change in coreboot[master]: soc/amd/common: Remove SMBus host word accessors
by Kyösti Mälkki (Code Review) 12 Dec '20

12 Dec '20

Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48581 ) Change subject: soc/amd/common: Remove SMBus host word accessors ...................................................................... soc/amd/common: Remove SMBus host word accessors SMBus controller has byte-wide registers. Remove the word accessors. Change-Id: If396108308bc8303d84458039b9529ecd83276c9 Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- M src/soc/amd/common/block/include/amdblocks/acpimmio.h 1 file changed, 0 insertions(+), 20 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/48581/1 diff --git a/src/soc/amd/common/block/include/amdblocks/acpimmio.h b/src/soc/amd/common/block/include/amdblocks/acpimmio.h index 15445ba..134e053 100644 --- a/src/soc/amd/common/block/include/amdblocks/acpimmio.h +++ b/src/soc/amd/common/block/include/amdblocks/acpimmio.h @@ -244,41 +244,21 @@ return read8(acpimmio_asf + reg); } -static inline uint16_t asf_read16(uint8_t reg) -{ - return read16(acpimmio_asf + reg); -} - static inline void asf_write8(uint8_t reg, uint8_t value) { write8(acpimmio_asf + reg, value); } -static inline void asf_write16(uint8_t reg, uint16_t value) -{ - write16(acpimmio_asf + reg, value); -} - static inline uint8_t smbus_read8(uint8_t reg) { return read8(acpimmio_smbus + reg); } -static inline uint16_t smbus_read16(uint8_t reg) -{ - return read16(acpimmio_smbus + reg); -} - static inline void smbus_write8(uint8_t reg, uint8_t value) { write8(acpimmio_smbus + reg, value); } -static inline void smbus_write16(uint8_t reg, uint16_t value) -{ - write16(acpimmio_smbus + reg, value); -} - static inline uint8_t iomux_read8(uint8_t reg) { return read8(acpimmio_iomux + reg); -- To view, visit https://review.coreboot.org/c/coreboot/+/48581 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If396108308bc8303d84458039b9529ecd83276c9 Gerrit-Change-Number: 48581 Gerrit-PatchSet: 1 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: 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-MessageType: newchange

3 3

Change in coreboot[master]: soc/amd/common: Refactor ACPIMMIO posted writes
by Kyösti Mälkki (Code Review) 12 Dec '20

12 Dec '20

Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42825 ) Change subject: soc/amd/common: Refactor ACPIMMIO posted writes ...................................................................... soc/amd/common: Refactor ACPIMMIO posted writes Change-Id: Ic1a5c17c789dd79fea8f348d1a9d32d4301ced88 Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- M src/soc/amd/common/block/include/amdblocks/acpimmio.h M src/soc/amd/picasso/i2c.c M src/soc/amd/stoneyridge/i2c.c 3 files changed, 8 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/25/42825/1 diff --git a/src/soc/amd/common/block/include/amdblocks/acpimmio.h b/src/soc/amd/common/block/include/amdblocks/acpimmio.h index 2775b52..8546da9 100644 --- a/src/soc/amd/common/block/include/amdblocks/acpimmio.h +++ b/src/soc/amd/common/block/include/amdblocks/acpimmio.h @@ -369,12 +369,6 @@ write32(gpio_ctrl_ptr(gpio_num), value); } -static inline void gpio_write32_rb(uint8_t gpio_num, uint32_t value) -{ - write32(gpio_ctrl_ptr(gpio_num), value); - read32(gpio_ctrl_ptr(gpio_num)); -} - /* GPIO bank 0 */ static inline uint8_t gpio0_read8(uint8_t reg) { diff --git a/src/soc/amd/picasso/i2c.c b/src/soc/amd/picasso/i2c.c index 881278f..61a4f8a 100644 --- a/src/soc/amd/picasso/i2c.c +++ b/src/soc/amd/picasso/i2c.c @@ -161,9 +161,11 @@ static void restore_i2c_pin_registers(uint8_t gpio, struct soc_amd_i2c_save *save_table) { + /* Write and flush posted writes. */ iomux_write8(gpio, save_table->mux_value); - iomux_read8(gpio); - gpio_write32_rb(gpio, save_table->control_value); + iomux_read8(0); + gpio_write32(gpio, save_table->control_value); + gpio_read32(0); } /* Slaves to be reset are controlled by devicetree register i2c_scl_reset */ diff --git a/src/soc/amd/stoneyridge/i2c.c b/src/soc/amd/stoneyridge/i2c.c index 0327028..cd42cda 100644 --- a/src/soc/amd/stoneyridge/i2c.c +++ b/src/soc/amd/stoneyridge/i2c.c @@ -144,9 +144,11 @@ static void restore_i2c_pin_registers(uint8_t gpio, struct soc_amd_i2c_save *save_table) { + /* Write and flush posted writes. */ iomux_write8(gpio, save_table->mux_value); - iomux_read8(gpio); - gpio_write32_rb(gpio, save_table->control_value); + iomux_read8(0); + gpio_write32(gpio, save_table->control_value); + gpio_read32(0); } /* Slaves to be reset are controlled by devicetree register i2c_scl_reset */ -- To view, visit https://review.coreboot.org/c/coreboot/+/42825 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic1a5c17c789dd79fea8f348d1a9d32d4301ced88 Gerrit-Change-Number: 42825 Gerrit-PatchSet: 1 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: newchange

4 12

Change in coreboot[master]: [TESTONLY] Revert "src/amd/common: Exclude biosram from psp_verstage"
by Kyösti Mälkki (Code Review) 12 Dec '20

12 Dec '20

Hello build bot (Jenkins), Raul Rangel, Martin Roth, Patrick Georgi, Julius Werner, Aaron Durbin, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/43067 to review the following change. Change subject: [TESTONLY] Revert "src/amd/common: Exclude biosram from psp_verstage" ...................................................................... [TESTONLY] Revert "src/amd/common: Exclude biosram from psp_verstage" This reverts commit f38af663d2c2c854859715803da249e6c24032db. I just want to see where the build fails. Change-Id: I0a995301404b67224be6addbeebf984c4b5c47d5 --- M src/soc/amd/common/block/acpimmio/Makefile.inc 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/43067/1 diff --git a/src/soc/amd/common/block/acpimmio/Makefile.inc b/src/soc/amd/common/block/acpimmio/Makefile.inc index 553d9e2..69253b9 100644 --- a/src/soc/amd/common/block/acpimmio/Makefile.inc +++ b/src/soc/amd/common/block/acpimmio/Makefile.inc @@ -6,7 +6,7 @@ smm-$(CONFIG_SOC_AMD_COMMON_BLOCK_ACPIMMIO) += mmio_util.c bootblock-$(CONFIG_SOC_AMD_COMMON_BLOCK_ACPIMMIO) += biosram.c -verstage_x86-$(CONFIG_SOC_AMD_COMMON_BLOCK_ACPIMMIO) += biosram.c +verstage-$(CONFIG_SOC_AMD_COMMON_BLOCK_ACPIMMIO) += biosram.c romstage-$(CONFIG_SOC_AMD_COMMON_BLOCK_ACPIMMIO) += biosram.c postcar-$(CONFIG_SOC_AMD_COMMON_BLOCK_ACPIMMIO) += biosram.c ramstage-$(CONFIG_SOC_AMD_COMMON_BLOCK_ACPIMMIO) += biosram.c -- To view, visit https://review.coreboot.org/c/coreboot/+/43067 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0a995301404b67224be6addbeebf984c4b5c47d5 Gerrit-Change-Number: 43067 Gerrit-PatchSet: 1 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: newchange

5 15

Change in coreboot[master]: nb/intel/sandybridge: Clean up stepping logic
by Angel Pons (Code Review) 12 Dec '20

12 Dec '20

Hello Felix Singer, Nico Huber, Arthur Heymans, Patrick Rudolph, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/48408 to review the following change. Change subject: nb/intel/sandybridge: Clean up stepping logic ...................................................................... nb/intel/sandybridge: Clean up stepping logic Do not combine the host bridge device ID with the CPU stepping. Change-Id: I27ad609eb53b96987ad5445301b5392055fa4ea1 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/cpu/intel/model_206ax/model_206ax.h M src/northbridge/intel/sandybridge/gma.c M src/northbridge/intel/sandybridge/northbridge.c M src/northbridge/intel/sandybridge/sandybridge.h 4 files changed, 47 insertions(+), 41 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/48408/1 diff --git a/src/cpu/intel/model_206ax/model_206ax.h b/src/cpu/intel/model_206ax/model_206ax.h index eb340ad..b6e2d65 100644 --- a/src/cpu/intel/model_206ax/model_206ax.h +++ b/src/cpu/intel/model_206ax/model_206ax.h @@ -3,6 +3,7 @@ #ifndef _CPU_INTEL_MODEL_206AX_H #define _CPU_INTEL_MODEL_206AX_H +#include <arch/cpu.h> #include <stdint.h> /* SandyBridge/IvyBridge bus clock is fixed at 100MHz */ @@ -88,4 +89,9 @@ int cpu_config_tdp_levels(void); int get_platform_id(void); +static inline u8 cpu_stepping(void) +{ + return cpuid_eax(1) & 0xf; +} + #endif diff --git a/src/northbridge/intel/sandybridge/gma.c b/src/northbridge/intel/sandybridge/gma.c index f75dfda..86e12b0 100644 --- a/src/northbridge/intel/sandybridge/gma.c +++ b/src/northbridge/intel/sandybridge/gma.c @@ -4,6 +4,7 @@ #include <device/mmio.h> #include <console/console.h> #include <bootmode.h> +#include <cpu/intel/model_206ax/model_206ax.h> #include <delay.h> #include <device/device.h> #include <device/pci.h> @@ -301,6 +302,8 @@ static void gma_pm_init_pre_vbios(struct device *dev) { + const bool is_sandy = is_sandybridge(); + u32 reg32; printk(BIOS_DEBUG, "GT Power Management Init\n"); @@ -309,7 +312,7 @@ if (!gtt_res || !gtt_res->base) return; - if (bridge_silicon_revision() < IVB_STEP_C0) { + if (is_sandy || cpu_stepping() < IVB_STEP_C0) { /* 1: Enable force wake */ gtt_write(0xa18c, 0x00000001); gtt_poll(0x130090, (1 << 0), (1 << 0)); @@ -319,14 +322,14 @@ gtt_poll(0x130040, (1 << 0), (1 << 0)); } - if ((bridge_silicon_revision() & BASE_REV_MASK) == BASE_REV_SNB) { + if (is_sandy) { /* 1d: Set GTT+0x42004 [15:14]=11 (SnB C1+) */ reg32 = gtt_read(0x42004); reg32 |= (1 << 14) | (1 << 15); gtt_write(0x42004, reg32); } - if (bridge_silicon_revision() >= IVB_STEP_A0) { + if (!is_sandy) { /* Display Reset Acknowledge Settings */ reg32 = gtt_read(0x45010); reg32 |= (1 << 1) | (1 << 0); @@ -335,7 +338,7 @@ /* 2: Get GT SKU from GTT+0x911c[13] */ reg32 = gtt_read(0x911c); - if ((bridge_silicon_revision() & BASE_REV_MASK) == BASE_REV_SNB) { + if (is_sandy) { if (reg32 & (1 << 13)) { printk(BIOS_DEBUG, "SNB GT1 Power Meter Weights\n"); gtt_write_powermeter(snb_pm_gt1); @@ -384,13 +387,12 @@ reg32 = gtt_read(0xa180); reg32 |= (1 << 26) | (1 << 31); /* (bit 20=1 for SNB step D1+ / IVB A0+) */ - if (bridge_silicon_revision() >= SNB_STEP_D1) + if (!is_sandy || cpu_stepping() >= SNB_STEP_D1) reg32 |= (1 << 20); gtt_write(0xa180, reg32); /* 6a: for SnB step D2+ only */ - if (((bridge_silicon_revision() & BASE_REV_MASK) == BASE_REV_SNB) && - (bridge_silicon_revision() >= SNB_STEP_D2)) { + if (is_sandy && cpu_stepping() >= SNB_STEP_D2) { reg32 = gtt_read(0x9400); reg32 |= (1 << 7); gtt_write(0x9400, reg32); @@ -402,16 +404,16 @@ gtt_poll(0x941c, (1 << 1), (0 << 1)); } - if ((bridge_silicon_revision() & BASE_REV_MASK) == BASE_REV_IVB) { + if (is_sandy) { + /* 6b: Clocking reset controls */ + gtt_write(0x9424, 0x00000000); + } else { reg32 = gtt_read(0x907c); reg32 |= (1 << 16); gtt_write(0x907c, reg32); /* 6b: Clocking reset controls */ gtt_write(0x9424, 0x00000001); - } else { - /* 6b: Clocking reset controls */ - gtt_write(0x9424, 0x00000000); } /* 7 */ @@ -503,7 +505,7 @@ printk(BIOS_DEBUG, "GT Power Management Init (post VBIOS)\n"); /* 15: Deassert Force Wake */ - if (bridge_silicon_revision() < IVB_STEP_C0) { + if (is_sandybridge() || cpu_stepping() < IVB_STEP_C0) { gtt_write(0xa18c, gtt_read(0xa18c) & ~1); gtt_poll(0x130090, (1 << 0), (0 << 0)); } else { diff --git a/src/northbridge/intel/sandybridge/northbridge.c b/src/northbridge/intel/sandybridge/northbridge.c index 541bf73..942d135 100644 --- a/src/northbridge/intel/sandybridge/northbridge.c +++ b/src/northbridge/intel/sandybridge/northbridge.c @@ -4,31 +4,26 @@ #include <acpi/acpi.h> #include <commonlib/helpers.h> #include <device/pci_ops.h> -#include <stdint.h> #include <delay.h> #include <cpu/intel/model_206ax/model_206ax.h> #include <cpu/x86/msr.h> #include <device/device.h> #include <device/pci.h> #include <device/pci_ids.h> +#include <types.h> #include "chip.h" #include "sandybridge.h" #include <cpu/intel/smm_reloc.h> -static int bridge_revision_id = -1; - /* IGD UMA memory */ static uint64_t uma_memory_base = 0; static uint64_t uma_memory_size = 0; -int bridge_silicon_revision(void) +bool is_sandybridge(void) { - if (bridge_revision_id < 0) { - uint8_t stepping = cpuid_eax(1) & 0x0f; - uint8_t bridge_id = pci_read_config16(pcidev_on_root(0, 0), PCI_DEVICE_ID); - bridge_revision_id = (bridge_id & 0xf0) | stepping; - } - return bridge_revision_id; + const uint16_t bridge_id = pci_read_config16(pcidev_on_root(0, 0), PCI_DEVICE_ID); + + return (bridge_id & BASE_REV_MASK) == BASE_REV_SNB; } /* Reserve everything between A segment and 1MB: @@ -115,7 +110,7 @@ CONFIG_CHROMEOS_RAMOOPS_RAM_SIZE >> 10); #endif - if ((bridge_silicon_revision() & BASE_REV_MASK) == BASE_REV_SNB) { + if (is_sandybridge()) { /* Required for SandyBridge sighting 3715511 */ bad_ram_resource(dev, index++, 0x20000000 >> 10, 0x00200000 >> 10); bad_ram_resource(dev, index++, 0x40000000 >> 10, 0x00200000 >> 10); @@ -259,6 +254,8 @@ static void northbridge_dmi_init(struct device *dev) { + const bool is_sandy = is_sandybridge(); + u32 reg32; /* Clear error status bits */ @@ -266,7 +263,7 @@ DMIBAR32(DMICESTS) = 0xffffffff; /* Steps prior to DMI ASPM */ - if ((bridge_silicon_revision() & BASE_REV_MASK) == BASE_REV_SNB) { + if (is_sandy) { reg32 = DMIBAR32(0x250); reg32 &= ~((1 << 22) | (1 << 20)); reg32 |= (1 << 21); @@ -277,12 +274,12 @@ reg32 |= (1 << 29); DMIBAR32(DMILLTC) = reg32; - if (bridge_silicon_revision() >= SNB_STEP_D0) { + if (is_sandy && cpu_stepping() >= SNB_STEP_D0) { reg32 = DMIBAR32(0x1f8); reg32 |= (1 << 16); DMIBAR32(0x1f8) = reg32; - } else if (bridge_silicon_revision() >= SNB_STEP_D1) { + } else if (is_sandy && cpu_stepping() >= SNB_STEP_D1) { reg32 = DMIBAR32(0x1f8); reg32 &= ~(1 << 26); reg32 |= (1 << 16); @@ -294,7 +291,7 @@ } /* Enable ASPM on SNB link, should happen before PCH link */ - if ((bridge_silicon_revision() & BASE_REV_MASK) == BASE_REV_SNB) { + if (is_sandy) { reg32 = DMIBAR32(0xd04); reg32 |= (1 << 4); DMIBAR32(0xd04) = reg32; @@ -376,7 +373,10 @@ bridge_type = MCHBAR32(SAPMTIMERS); bridge_type &= ~0xff; - if ((bridge_silicon_revision() & BASE_REV_MASK) == BASE_REV_IVB) { + if (is_sandybridge()) { + /* 20h for Sandybridge */ + bridge_type |= 0x20; + } else { /* Enable Power Aware Interrupt Routing */ u8 pair = MCHBAR8(INTRDIRCTL); pair &= ~0x0f; /* Clear 3:0 */ @@ -385,9 +385,6 @@ /* 30h for IvyBridge */ bridge_type |= 0x30; - } else { - /* 20h for Sandybridge */ - bridge_type |= 0x20; } MCHBAR32(SAPMTIMERS) = bridge_type; diff --git a/src/northbridge/intel/sandybridge/sandybridge.h b/src/northbridge/intel/sandybridge/sandybridge.h index 8f9d118..e5f2132 100644 --- a/src/northbridge/intel/sandybridge/sandybridge.h +++ b/src/northbridge/intel/sandybridge/sandybridge.h @@ -9,22 +9,22 @@ #define BASE_REV_MASK 0x50 /* SandyBridge CPU stepping */ -#define SNB_STEP_D0 (BASE_REV_SNB + 5) /* Also J0 */ -#define SNB_STEP_D1 (BASE_REV_SNB + 6) -#define SNB_STEP_D2 (BASE_REV_SNB + 7) /* Also J1/Q0 */ +#define SNB_STEP_D0 5 /* Also J0 */ +#define SNB_STEP_D1 6 +#define SNB_STEP_D2 7 /* Also J1/Q0 */ /* IvyBridge CPU stepping */ -#define IVB_STEP_A0 (BASE_REV_IVB + 0) -#define IVB_STEP_B0 (BASE_REV_IVB + 2) -#define IVB_STEP_C0 (BASE_REV_IVB + 4) -#define IVB_STEP_K0 (BASE_REV_IVB + 5) -#define IVB_STEP_D0 (BASE_REV_IVB + 6) +#define IVB_STEP_A0 0 +#define IVB_STEP_B0 2 +#define IVB_STEP_C0 4 +#define IVB_STEP_K0 5 +#define IVB_STEP_D0 6 #include "memmap.h" /* Everything below this line is ignored in the DSDT */ #ifndef __ACPI__ -#include <stdint.h> +#include <types.h> /* Chipset types */ enum platform_type { @@ -87,8 +87,9 @@ #ifndef __ASSEMBLER__ +bool is_sandybridge(void); + void intel_sandybridge_finalize_smm(void); -int bridge_silicon_revision(void); void systemagent_early_init(void); void sandybridge_init_iommu(void); void sandybridge_late_initialization(void); -- To view, visit https://review.coreboot.org/c/coreboot/+/48408 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I27ad609eb53b96987ad5445301b5392055fa4ea1 Gerrit-Change-Number: 48408 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

2 10

Change in coreboot[master]: nb/intel/sandybridge: Fix blunder in MR2 shadow code
by Angel Pons (Code Review) 12 Dec '20

12 Dec '20

Hello Felix Singer, Nico Huber, Arthur Heymans, Patrick Rudolph, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/48550 to review the following change. Change subject: nb/intel/sandybridge: Fix blunder in MR2 shadow code ...................................................................... nb/intel/sandybridge: Fix blunder in MR2 shadow code Commit 7f1363d9b4 (nb/intel/sandybridge: Program MR2 shadow register) has a bug where the system locks up and power cycles when booting Linux, but is still able to pass memtest86+ with flying colors. The issue will occur when the following conditions are true: - CPU is Ivy Bridge - Memory speed is not greater than 1066 MHz (DDR3-2133 or slower) - System contains dual-rank DIMMs - The second rank of the dual-rank DIMMs is mirrored - All DIMMs support Extended Temperature Range - At least one of the DIMMs does not support Auto Self-Refresh If all of these conditions are met, the final value of the MR2 Shadow registers configures the memory controller to issue a MRS command to update MR2 before entering self-refresh mode, but indicates that rank mirroring is not required (the first rank on a DIMM is never mirrored). Before the memory controller enters self-refresh, it sends MRS commands to all ranks to update MR2, but the missing address and bank mirroring means DRAM chips on mirrored ranks instead clobber MR1 with junk data. With garbage in MR1, the mirrored ranks no longer function properly, which ultimately leads to all hell breaking loose (undefined behavior). The condition is backwards, since only odd ranks can be mirrored. To avoid this problem completely, simply remove the condition. The final register value will still be correct, since the bits are always ORed. Tested on Asus P8Z77-V LX2, fixes booting Linux with dual-rank DIMMs. Change-Id: Iceff741eb85fab0ae846e50af0080e5ff405404c Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/northbridge/intel/sandybridge/raminit_common.c 1 file changed, 6 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/48550/1 diff --git a/src/northbridge/intel/sandybridge/raminit_common.c b/src/northbridge/intel/sandybridge/raminit_common.c index 399ba5a..80c5186 100644 --- a/src/northbridge/intel/sandybridge/raminit_common.c +++ b/src/northbridge/intel/sandybridge/raminit_common.c @@ -815,13 +815,12 @@ reg32 |= mr2reg & ~(3 << 6); - if (rank & 1) { - if (srt) - reg32 |= 1 << (rank / 2 + 6); - } else { - if (ctrl->rank_mirror[channel][rank]) - reg32 |= 1 << (rank / 2 + 14); - } + if (srt) + reg32 |= 1 << (rank / 2 + 6); + + if (ctrl->rank_mirror[channel][rank]) + reg32 |= 1 << (rank / 2 + 14); + MCHBAR32(TC_MR2_SHADOW_ch(channel)) = reg32; } -- To view, visit https://review.coreboot.org/c/coreboot/+/48550 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iceff741eb85fab0ae846e50af0080e5ff405404c Gerrit-Change-Number: 48550 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

4 4

Change in coreboot[master]: cbfs: Introduce cbfs_ro_map() and cbfs_ro_load()
by Julius Werner (Code Review) 12 Dec '20

12 Dec '20

Hello Aaron Durbin, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/39306 to review the following change. Change subject: cbfs: Introduce cbfs_ro_map() and cbfs_ro_load() ...................................................................... cbfs: Introduce cbfs_ro_map() and cbfs_ro_load() This patch introduces two new CBFS API functions which are equivalent to cbfs_map() and cbfs_load(), respectively, with the difference that they always operate on the read-only CBFS region ("COREBOOT" FMAP section). Use it to replace some of the simple cases that needed to use cbfs_locate_file_in_region(). Signed-off-by: Julius Werner <jwerner(a)chromium.org> Change-Id: I9c55b022b6502a333a9805ab0e4891dd7b97ef7f --- M src/drivers/pc80/rtc/option.c M src/include/cbfs.h M src/lib/cbfs.c M src/northbridge/intel/haswell/raminit.c M src/soc/intel/broadwell/romstage/raminit.c M src/soc/intel/quark/romstage/romstage.c 6 files changed, 60 insertions(+), 89 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/39306/1 diff --git a/src/drivers/pc80/rtc/option.c b/src/drivers/pc80/rtc/option.c index 4789b15..1f9abb5 100644 --- a/src/drivers/pc80/rtc/option.c +++ b/src/drivers/pc80/rtc/option.c @@ -60,36 +60,28 @@ return CB_SUCCESS; } -static enum cb_err locate_cmos_layout(struct region_device *rdev) +static struct cmos_option_table *get_cmos_layout(struct region_device *rdev) { - uint32_t cbfs_type = CBFS_COMPONENT_CMOS_LAYOUT; - MAYBE_STATIC_BSS struct cbfsf fh = {}; + MAYBE_STATIC_BSS struct cmos_option_table ct = NULL; /* * In case VBOOT is enabled and this function is called from SMM, * we have multiple CMOS layout files and to locate them we'd need to * include VBOOT into SMM... * - * Support only one CMOS layout in the 'COREBOOT' region for now. + * Support only one CMOS layout in the RO CBFS for now. */ - if (!region_device_sz(&(fh.data))) { - if (cbfs_locate_file_in_region(&fh, "COREBOOT", "cmos_layout.bin", - &cbfs_type)) { - printk(BIOS_ERR, "RTC: cmos_layout.bin could not be found. " - "Options are disabled\n"); - return CB_CMOS_LAYOUT_NOT_FOUND; - } - } - - cbfs_file_data(rdev, &fh); - - return CB_SUCCESS; + if (!ct) + ct = cbfs_ro_map("cmos_layout.bin", NULL); + if (!ct) + printk(BIOS_ERR, "RTC: cmos_layout.bin could not be found. " + "Options are disabled\n"); + return ct; } enum cb_err cmos_get_option(void *dest, const char *name) { struct cmos_option_table *ct; - struct region_device rdev; struct cmos_entries *ce; size_t namelen; int found = 0; @@ -97,16 +89,9 @@ /* Figure out how long name is */ namelen = strnlen(name, CMOS_MAX_NAME_LENGTH); - if (locate_cmos_layout(&rdev) != CB_SUCCESS) { + ct = get_cmos_layout(); + if (!ct) return CB_CMOS_LAYOUT_NOT_FOUND; - } - ct = rdev_mmap_full(&rdev); - if (!ct) { - printk(BIOS_ERR, "RTC: cmos_layout.bin could not be mapped. " - "Options are disabled\n"); - - return CB_CMOS_LAYOUT_NOT_FOUND; - } /* find the requested entry record */ ce = (struct cmos_entries *)((unsigned char *)ct + ct->header_length); @@ -119,19 +104,15 @@ } if (!found) { printk(BIOS_DEBUG, "No CMOS option '%s'.\n", name); - rdev_munmap(&rdev, ct); return CB_CMOS_OPTION_NOT_FOUND; } - if (!cmos_checksum_valid(LB_CKS_RANGE_START, LB_CKS_RANGE_END, LB_CKS_LOC)) { - rdev_munmap(&rdev, ct); + if (!cmos_checksum_valid(LB_CKS_RANGE_START, LB_CKS_RANGE_END, LB_CKS_LOC)) return CB_CMOS_CHECKSUM_INVALID; - } - if (get_cmos_value(ce->bit, ce->length, dest) != CB_SUCCESS) { - rdev_munmap(&rdev, ct); +} + if (get_cmos_value(ce->bit, ce->length, dest) != CB_SUCCESS) return CB_CMOS_ACCESS_ERROR; - } - rdev_munmap(&rdev, ct); +} return CB_SUCCESS; } @@ -179,7 +160,6 @@ enum cb_err cmos_set_option(const char *name, void *value) { struct cmos_option_table *ct; - struct region_device rdev; struct cmos_entries *ce; unsigned long length; size_t namelen; @@ -188,16 +168,9 @@ /* Figure out how long name is */ namelen = strnlen(name, CMOS_MAX_NAME_LENGTH); - if (locate_cmos_layout(&rdev) != CB_SUCCESS) { + ct = get_cmos_layout(); + if (!ct) return CB_CMOS_LAYOUT_NOT_FOUND; - } - ct = rdev_mmap_full(&rdev); - if (!ct) { - printk(BIOS_ERR, "RTC: cmos_layout.bin could not be mapped. " - "Options are disabled\n"); - - return CB_CMOS_LAYOUT_NOT_FOUND; - } /* find the requested entry record */ ce = (struct cmos_entries *)((unsigned char *)ct + ct->header_length); @@ -210,7 +183,6 @@ } if (!found) { printk(BIOS_DEBUG, "WARNING: No CMOS option '%s'.\n", name); - rdev_munmap(&rdev, ct); return CB_CMOS_OPTION_NOT_FOUND; } @@ -219,18 +191,13 @@ length = MAX(strlen((const char *)value) * 8, ce->length - 8); /* make sure the string is null terminated */ if (set_cmos_value(ce->bit + ce->length - 8, 8, &(u8[]){0}) - != CB_SUCCESS) { - rdev_munmap(&rdev, ct); + != CB_SUCCESS) return CB_CMOS_ACCESS_ERROR; - } } - if (set_cmos_value(ce->bit, length, value) != CB_SUCCESS) { - rdev_munmap(&rdev, ct); + if (set_cmos_value(ce->bit, length, value) != CB_SUCCESS) return CB_CMOS_ACCESS_ERROR; - } - rdev_munmap(&rdev, ct); return CB_SUCCESS; } diff --git a/src/include/cbfs.h b/src/include/cbfs.h index cc61b16..8cd5713 100644 --- a/src/include/cbfs.h +++ b/src/include/cbfs.h @@ -36,9 +36,12 @@ NOTE: Since this may return a direct pointer to memory-mapped hardware, compressed files are NOT transparently decompressed (unlike cbfs_load()). */ void *cbfs_map(const char *name, size_t *size); -/* Removes a mapping previously allocated with cbfs_map(). Should try to unmap - mappings in strict LIFO order where possible, since mapping backends often - don't support more complicated cases. */ +/* Like cbfs_map(), except that it will always read from the read-only CBFS + ("COREBOOT" FMAP region), even when CONFIG_VBOOT is enabled. */ +void *cbfs_ro_map(const char *name, size_t *size); +/* Removes a previously allocated CBFS mapping. Should try to unmap mappings in + strict LIFO order where possible, since mapping backends often don't support + more complicated cases. */ int cbfs_unmap(void *mapping); /* Locate file in a specific region of fmap. Return 0 on success. < 0 on error*/ int cbfs_locate_file_in_region(struct cbfsf *fh, const char *region_name, @@ -47,6 +50,9 @@ success or 0 on error. File will get decompressed as necessary. Same decompression requirements as cbfs_load_and_decompress(). */ size_t cbfs_load(const char *name, void *buf, size_t buf_size); +/* Like cbfs_load(), except that it will always read from the read-only CBFS + ("COREBOOT" FMAP region), even when CONFIG_VBOOT is enabled. */ +size_t cbfs_ro_load(const char *name, void *buf, size_t buf_size); /* Load |in_size| bytes from |rdev| at |offset| to the |buffer_size| bytes * large |buffer|, decompressing it according to |compression| in the process. * Returns the decompressed file size, or 0 on error. diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c index c3660aa..ba50c34 100644 --- a/src/lib/cbfs.c +++ b/src/lib/cbfs.c @@ -79,9 +79,9 @@ return 0; } -void *cbfs_map(const char *name, size_t *size) +static void *_cbfs_map(const char *name, size_t *size, bool force_ro) { - const struct cbfs_boot_device *cbd = cbfs_get_boot_device(false); + const struct cbfs_boot_device *cbd = cbfs_get_boot_device(force_ro); union cbfs_mdata mdata; size_t data_offset; @@ -96,6 +96,16 @@ return rdev_mmap(&cbd->rdev, data_offset, data_size); } +void *cbfs_map(const char *name, size_t *size) +{ + return _cbfs_map(name, size, false); +} + +void *cbfs_ro_map(const char *name, size_t *size) +{ + return _cbfs_map(name, size, true); +} + int cbfs_unmap(void *mapping) { /* This works because munmap() only works on the root rdev and never @@ -215,7 +225,8 @@ return prog_entry(&stage); } -size_t cbfs_load(const char *name, void *buf, size_t buf_size) +static size_t _cbfs_load(const char *name, void *buf, size_t buf_size, + bool force_ro) { const struct cbfs_boot_device *cbd = cbfs_get_boot_device(false); union cbfs_mdata mdata; @@ -238,6 +249,16 @@ compression); } +size_t cbfs_load(const char *name, void *buf, size_t buf_size) +{ + return _cbfs_load(name, buf, buf_size, false); +} + +size_t cbfs_ro_load(const char *name, void *buf, size_t buf_size) +{ + return _cbfs_load(name, buf, buf_size, true); +} + size_t cbfs_prog_stage_section(struct prog *pstage, uintptr_t *base) { struct cbfs_stage stage; diff --git a/src/northbridge/intel/haswell/raminit.c b/src/northbridge/intel/haswell/raminit.c index 8267833..00211bd 100644 --- a/src/northbridge/intel/haswell/raminit.c +++ b/src/northbridge/intel/haswell/raminit.c @@ -118,8 +118,6 @@ void sdram_initialize(struct pei_data *pei_data) { unsigned long entry; - uint32_t type = CBFS_TYPE_MRC; - struct cbfsf f; printk(BIOS_DEBUG, "Starting UEFI PEI System Agent\n"); @@ -144,12 +142,11 @@ /* * Locate and call UEFI System Agent binary. The binary needs to be at * a fixed offset in the flash and can therefore only reside in the - * COREBOOT fmap region + * COREBOOT fmap region. We don't care about leaking the mapping. */ - if (cbfs_locate_file_in_region(&f, "COREBOOT", "mrc.bin", &type) < 0) + entry = (unsigned long)cbfs_ro_map("mrc.bin", NULL); + if (!entry) die("mrc.bin not found!"); - /* We don't care about leaking the mapping */ - entry = (unsigned long)rdev_mmap_full(&f.data); if (entry) { int rv; asm volatile ( diff --git a/src/soc/intel/broadwell/romstage/raminit.c b/src/soc/intel/broadwell/romstage/raminit.c index 3759c1f..bcf7148 100644 --- a/src/soc/intel/broadwell/romstage/raminit.c +++ b/src/soc/intel/broadwell/romstage/raminit.c @@ -43,8 +43,6 @@ struct memory_info *mem_info; pei_wrapper_entry_t entry; int ret; - struct cbfsf f; - uint32_t type = CBFS_TYPE_MRC; broadwell_fill_pei_data(pei_data); @@ -76,15 +74,10 @@ pei_data->saved_data_size = 0; } - /* Determine if mrc.bin is in the cbfs. */ - if (cbfs_locate_file_in_region(&f, "COREBOOT", "mrc.bin", &type) < 0) - die("mrc.bin not found!"); /* We don't care about leaking the mapping */ - entry = (pei_wrapper_entry_t)rdev_mmap_full(&f.data); - if (entry == NULL) { - printk(BIOS_DEBUG, "Couldn't find mrc.bin\n"); - return; - } + entry = cbfs_ro_map("mrc.bin", NULL); + if (entry == NULL) + die("mrc.bin not found!"); printk(BIOS_DEBUG, "Starting Memory Reference Code\n"); diff --git a/src/soc/intel/quark/romstage/romstage.c b/src/soc/intel/quark/romstage/romstage.c index 4ac580e..3e08663 100644 --- a/src/soc/intel/quark/romstage/romstage.c +++ b/src/soc/intel/quark/romstage/romstage.c @@ -63,29 +63,16 @@ void *locate_rmu_file(size_t *rmu_file_len) { - struct cbfsf fh; size_t fsize; void *rmu_data; - uint32_t type; /* Locate the rmu.bin file in the read-only region of the flash */ - type = CBFS_TYPE_RAW; - if (cbfs_locate_file_in_region(&fh, "COREBOOT", "rmu.bin", &type)) + rmu_data = cbfs_ro_map("rmu.bin", &fsize); + if (!rmu_data) return NULL; - /* Get the file size */ - fsize = region_device_sz(&fh.data); if (rmu_file_len != NULL) *rmu_file_len = fsize; - /* Get the data address */ - rmu_data = rdev_mmap(&fh.data, 0, fsize); - - /* Since the SPI flash is directly mapped into memory, we do not need - * the mapping provided by the rdev service. Unmap the file to prevent - * a memory leak. Return/leak the SPI flash address for the rmu.bin - * file data which will be directly accessed by FSP MemoryInit. - */ - rdev_munmap(&fh.data, rmu_data); return rmu_data; } -- To view, visit https://review.coreboot.org/c/coreboot/+/39306 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9c55b022b6502a333a9805ab0e4891dd7b97ef7f Gerrit-Change-Number: 39306 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

5 15

Change in coreboot[master]: mb/google/kukui: Fix no firmware beep issue
by Tao Xia (Code Review) 11 Dec '20

11 Dec '20

Tao Xia has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47586 ) Change subject: mb/google/kukui: Fix no firmware beep issue ...................................................................... mb/google/kukui: Fix no firmware beep issue There are no two beeps during the developer screen after changing from the normal mode to the developer mode. The RT1015 amplifier works with I2C mode on Kakadu. So the SCL6/SDA6 should be configured as I2C function instead of GPIO function. And the RT1015 amplifier initialization should be done through I2C6 interface. BUG=b:171763469 BRANCH=kukui TEST=There are two beeps during the developer screen after changing from the normal mode to the developer mode. Signed-off-by: Tao Xia <xiatao5(a)huaqin.corp-partner.google.com> Change-Id: If07674d94a0b63ac3174d16d11bf371e0bd8c8cf --- M src/mainboard/google/kukui/Kconfig M src/mainboard/google/kukui/chromeos.c M src/mainboard/google/kukui/mainboard.c 3 files changed, 26 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/47586/1 diff --git a/src/mainboard/google/kukui/Kconfig b/src/mainboard/google/kukui/Kconfig index cff94da..358d445 100644 --- a/src/mainboard/google/kukui/Kconfig +++ b/src/mainboard/google/kukui/Kconfig @@ -17,6 +17,12 @@ select EC_GOOGLE_CHROMEEC_SWITCHES select VBOOT_VBNV_FLASH +config RT1015_I2C_MODE + bool + default n + help + The platform that uses rt1015 I2C mode should select this option to enable it. + config BOARD_SPECIFIC_OPTIONS def_bool y select RTC @@ -37,6 +43,7 @@ select MAINBOARD_FORCE_NATIVE_VGA_INIT select HAVE_LINEAR_FRAMEBUFFER select MT8183_DRAM_EMCP if BOARD_GOOGLE_KRANE || BOARD_GOOGLE_KAKADU + select RT1015_I2C_MODE if BOARD_GOOGLE_KAKADU config MAINBOARD_DIR string diff --git a/src/mainboard/google/kukui/chromeos.c b/src/mainboard/google/kukui/chromeos.c index c5810d1..6fce1c3 100644 --- a/src/mainboard/google/kukui/chromeos.c +++ b/src/mainboard/google/kukui/chromeos.c @@ -25,7 +25,20 @@ {CR50_IRQ.id, ACTIVE_HIGH, -1, "TPM interrupt"}, {GPIO_EN_SPK_AMP.id, ACTIVE_HIGH, -1, "speaker enable"}, }; - lb_add_gpios(gpios, chromeos_gpios, ARRAY_SIZE(chromeos_gpios)); + + struct lb_gpio chromeos_gpios_1[] = { + {GPIO_WP.id, ACTIVE_LOW, + !get_write_protect_state(), "write protect"}, + {EC_IN_RW.id, ACTIVE_HIGH, -1, "EC in RW"}, + {EC_IRQ.id, ACTIVE_LOW, -1, "EC interrupt"}, + {CR50_IRQ.id, ACTIVE_HIGH, -1, "TPM interrupt"}, + }; + + if (CONFIG(RT1015_I2C_MODE)) + lb_add_gpios(gpios, chromeos_gpios_1, ARRAY_SIZE(chromeos_gpios_1)); + else + lb_add_gpios(gpios, chromeos_gpios, ARRAY_SIZE(chromeos_gpios)); + } int get_write_protect_state(void) diff --git a/src/mainboard/google/kukui/mainboard.c b/src/mainboard/google/kukui/mainboard.c index 268b7ca..746b3af 100644 --- a/src/mainboard/google/kukui/mainboard.c +++ b/src/mainboard/google/kukui/mainboard.c @@ -18,6 +18,7 @@ #include <soc/mtcmos.h> #include <soc/spm.h> #include <soc/usb.h> +#include <soc/i2c.h> #include <string.h> #include "gpio.h" @@ -54,6 +55,9 @@ gpio_set_mode(GPIO(CAM_PDN1), PAD_CAM_PDN1_FUNC_I2S2_BCK); gpio_set_mode(GPIO(CAM_PDN0), PAD_CAM_PDN0_FUNC_I2S2_MCK); gpio_set_mode(GPIO(EINT3), PAD_EINT3_FUNC_I2S3_DO); + + if (CONFIG(RT1015_I2C_MODE)) + mtk_i2c_bus_init(6); } static void configure_ec(void) @@ -91,7 +95,7 @@ gpio_output(GPIO_PPVARN_LCD_EN, 1); gpio_output(GPIO_PP1800_LCM_EN, 1); gpio_output(GPIO_PP3300_LCM_EN, 1); - mdelay(6); + mdelay(15); gpio_output(GPIO_LCM_RST_1V8, 1); mdelay(6); } -- To view, visit https://review.coreboot.org/c/coreboot/+/47586 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If07674d94a0b63ac3174d16d11bf371e0bd8c8cf Gerrit-Change-Number: 47586 Gerrit-PatchSet: 1 Gerrit-Owner: Tao Xia <xiatao5(a)huaqin.corp-partner.google.com> Gerrit-MessageType: newchange

3 20

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit December 2020