coreboot-gerrit February 2022

coreboot-gerrit@coreboot.org

1 participants
7066 discussions

Change in coreboot[master]: [WIP] soc/amd/sabrina/i2c: only call fch_i2c_pad_init for bus 3
by Felix Held (Code Review) 02 Feb '22

02 Feb '22

1 0

Change in coreboot[master]: mb/amd/chausie/devicetree: update I2C RX levels to match board design
by Felix Held (Code Review) 02 Feb '22

02 Feb '22

Attention is currently required from: Jason Glenesk, Marshall Dawson. Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/61569 ) Change subject: mb/amd/chausie/devicetree: update I2C RX levels to match board design ...................................................................... mb/amd/chausie/devicetree: update I2C RX levels to match board design Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: Ie5d5f5441132e5b0d8991d07d4dde994fc17ab64 --- M src/mainboard/amd/chausie/devicetree.cb 1 file changed, 4 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/61569/1 diff --git a/src/mainboard/amd/chausie/devicetree.cb b/src/mainboard/amd/chausie/devicetree.cb index 6067b18..8cc23e1 100644 --- a/src/mainboard/amd/chausie/devicetree.cb +++ b/src/mainboard/amd/chausie/devicetree.cb @@ -14,10 +14,10 @@ }" # I2C Pad Control RX Select Configuration - register "i2c_pad[0].rx_level" = "I2C_PAD_RX_3_3V" - register "i2c_pad[1].rx_level" = "I2C_PAD_RX_3_3V" - register "i2c_pad[2].rx_level" = "I2C_PAD_RX_3_3V" - register "i2c_pad[3].rx_level" = "I2C_PAD_RX_3_3V" + register "i2c_pad[0].rx_level" = "I2C_PAD_RX_1_8V" + register "i2c_pad[1].rx_level" = "I2C_PAD_RX_1_8V" + register "i2c_pad[2].rx_level" = "I2C_PAD_RX_1_8V" + register "i2c_pad[3].rx_level" = "I2C_PAD_RX_1_8V" register "s0ix_enable" = "true" -- To view, visit https://review.coreboot.org/c/coreboot/+/61569 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie5d5f5441132e5b0d8991d07d4dde994fc17ab64 Gerrit-Change-Number: 61569 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-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: soc/amd/*/i2c: factor out common I2C pad configuration
by Felix Held (Code Review) 02 Feb '22

02 Feb '22

Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson. Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/61568 ) Change subject: soc/amd/*/i2c: factor out common I2C pad configuration ...................................................................... soc/amd/*/i2c: factor out common I2C pad configuration The I2C pad control registers of Picasso and Cezanne are identical and the one of Sabrina is a superset of it, so factor out the functionality. To avoid having devicetree settings that contain raw register bits, the i2c_pad_control struct is introduced and used. The old Picasso code for this had the RX level hard-coded for 3.3V I2C interfaces, so keep it this way in this patch but add a TODO for future improvements. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I1d70329644b68be3c4a1602f748e09db20cf6de1 --- M src/mainboard/amd/chausie/devicetree.cb M src/mainboard/amd/majolica/devicetree.cb M src/mainboard/google/guybrush/variants/baseboard/devicetree.cb M src/soc/amd/cezanne/Kconfig M src/soc/amd/cezanne/chip.h M src/soc/amd/cezanne/i2c.c M src/soc/amd/cezanne/include/soc/southbridge.h M src/soc/amd/common/block/i2c/Kconfig M src/soc/amd/common/block/i2c/Makefile.inc A src/soc/amd/common/block/i2c/i2c_pad_ctrl.c A src/soc/amd/common/block/i2c/i2c_pad_def.h M src/soc/amd/common/block/include/amdblocks/i2c.h M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/i2c.c M src/soc/amd/picasso/include/soc/southbridge.h M src/soc/amd/sabrina/Kconfig M src/soc/amd/sabrina/chip.h M src/soc/amd/sabrina/i2c.c M src/soc/amd/sabrina/include/soc/southbridge.h 19 files changed, 134 insertions(+), 143 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/61568/1 diff --git a/src/mainboard/amd/chausie/devicetree.cb b/src/mainboard/amd/chausie/devicetree.cb index 19815b3..6067b18 100644 --- a/src/mainboard/amd/chausie/devicetree.cb +++ b/src/mainboard/amd/chausie/devicetree.cb @@ -14,10 +14,10 @@ }" # I2C Pad Control RX Select Configuration - register "i2c_pad_ctrl_rx_sel[0]" = "I2C_PAD_CTRL_RX_SEL_3_3V" - register "i2c_pad_ctrl_rx_sel[1]" = "I2C_PAD_CTRL_RX_SEL_3_3V" - register "i2c_pad_ctrl_rx_sel[2]" = "I2C_PAD_CTRL_RX_SEL_3_3V" - register "i2c_pad_ctrl_rx_sel[3]" = "I2C_PAD_CTRL_RX_SEL_3_3V" + register "i2c_pad[0].rx_level" = "I2C_PAD_RX_3_3V" + register "i2c_pad[1].rx_level" = "I2C_PAD_RX_3_3V" + register "i2c_pad[2].rx_level" = "I2C_PAD_RX_3_3V" + register "i2c_pad[3].rx_level" = "I2C_PAD_RX_3_3V" register "s0ix_enable" = "true" diff --git a/src/mainboard/amd/majolica/devicetree.cb b/src/mainboard/amd/majolica/devicetree.cb index 7bb0db1..9758e9c 100644 --- a/src/mainboard/amd/majolica/devicetree.cb +++ b/src/mainboard/amd/majolica/devicetree.cb @@ -14,10 +14,10 @@ }" # I2C Pad Control RX Select Configuration - register "i2c_pad_ctrl_rx_sel[0]" = "I2C_PAD_CTRL_RX_SEL_3_3V" - register "i2c_pad_ctrl_rx_sel[1]" = "I2C_PAD_CTRL_RX_SEL_3_3V" - register "i2c_pad_ctrl_rx_sel[2]" = "I2C_PAD_CTRL_RX_SEL_3_3V" - register "i2c_pad_ctrl_rx_sel[3]" = "I2C_PAD_CTRL_RX_SEL_3_3V" + register "i2c_pad[0].rx_level" = "I2C_PAD_RX_3_3V" + register "i2c_pad[1].rx_level" = "I2C_PAD_RX_3_3V" + register "i2c_pad[2].rx_level" = "I2C_PAD_RX_3_3V" + register "i2c_pad[3].rx_level" = "I2C_PAD_RX_3_3V" register "s0ix_enable" = "true" diff --git a/src/mainboard/google/guybrush/variants/baseboard/devicetree.cb b/src/mainboard/google/guybrush/variants/baseboard/devicetree.cb index ac2856e..104ba79 100644 --- a/src/mainboard/google/guybrush/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/guybrush/variants/baseboard/devicetree.cb @@ -75,10 +75,10 @@ register "i2c_scl_reset" = "GPIO_I2C0_SCL | GPIO_I2C1_SCL | GPIO_I2C2_SCL | GPIO_I2C3_SCL" # I2C Pad Control RX Select Configuration - register "i2c_pad_ctrl_rx_sel[0]" = "I2C_PAD_CTRL_RX_SEL_3_3V" # Trackpad - register "i2c_pad_ctrl_rx_sel[1]" = "I2C_PAD_CTRL_RX_SEL_3_3V" # Touchscreen - register "i2c_pad_ctrl_rx_sel[2]" = "I2C_PAD_CTRL_RX_SEL_3_3V" # Audio/SAR - register "i2c_pad_ctrl_rx_sel[3]" = "I2C_PAD_CTRL_RX_SEL_1_8V" # GSC + register "i2c_pad[0].rx_level" = "I2C_PAD_RX_3_3V" # Trackpad + register "i2c_pad[1].rx_level" = "I2C_PAD_RX_3_3V" # Touchscreen + register "i2c_pad[2].rx_level" = "I2C_PAD_RX_3_3V" # Audio/SAR + register "i2c_pad[3].rx_level" = "I2C_PAD_RX_1_8V" # GSC # general purpose PCIe clock output configuration register "gpp_clk_config[0]" = "GPP_CLK_REQ" diff --git a/src/soc/amd/cezanne/Kconfig b/src/soc/amd/cezanne/Kconfig index f2fdc7f..8a42ff4 100644 --- a/src/soc/amd/cezanne/Kconfig +++ b/src/soc/amd/cezanne/Kconfig @@ -50,6 +50,7 @@ select SOC_AMD_COMMON_BLOCK_GRAPHICS select SOC_AMD_COMMON_BLOCK_HAS_ESPI select SOC_AMD_COMMON_BLOCK_I2C + select SOC_AMD_COMMON_BLOCK_I2C_PAD_CTRL select SOC_AMD_COMMON_BLOCK_IOMMU select SOC_AMD_COMMON_BLOCK_LPC select SOC_AMD_COMMON_BLOCK_MCAX diff --git a/src/soc/amd/cezanne/chip.h b/src/soc/amd/cezanne/chip.h index d007a84..1e59153 100644 --- a/src/soc/amd/cezanne/chip.h +++ b/src/soc/amd/cezanne/chip.h @@ -4,6 +4,7 @@ #define CEZANNE_CHIP_H #include <amdblocks/chip.h> +#include <amdblocks/i2c.h> #include <gpio.h> #include <soc/i2c.h> #include <soc/southbridge.h> @@ -15,7 +16,7 @@ struct soc_amd_common_config common_config; u8 i2c_scl_reset; struct dw_i2c_bus_config i2c[I2C_CTRLR_COUNT]; - u8 i2c_pad_ctrl_rx_sel[I2C_CTRLR_COUNT]; + struct i2c_pad_control i2c_pad[I2C_CTRLR_COUNT]; /* Enable S0iX support */ bool s0ix_enable; diff --git a/src/soc/amd/cezanne/i2c.c b/src/soc/amd/cezanne/i2c.c index 1fcab6a..9267469 100644 --- a/src/soc/amd/cezanne/i2c.c +++ b/src/soc/amd/cezanne/i2c.c @@ -36,24 +36,11 @@ void soc_i2c_misc_init(unsigned int bus, const struct dw_i2c_bus_config *cfg) { const struct soc_amd_cezanne_config *config = config_of_soc(); - uint32_t pad_ctrl; - if (bus >= ARRAY_SIZE(config->i2c_pad_ctrl_rx_sel)) + if (bus >= ARRAY_SIZE(config->i2c_pad)) return; - pad_ctrl = misc_read32(MISC_I2C_PAD_CTRL(bus)); - - pad_ctrl &= ~I2C_PAD_CTRL_NG_MASK; - pad_ctrl |= I2C_PAD_CTRL_NG_NORMAL; - - pad_ctrl &= ~I2C_PAD_CTRL_RX_SEL_MASK; - pad_ctrl |= config->i2c_pad_ctrl_rx_sel[bus]; - - pad_ctrl &= ~I2C_PAD_CTRL_FALLSLEW_MASK; - pad_ctrl |= cfg->speed == I2C_SPEED_STANDARD ? - I2C_PAD_CTRL_FALLSLEW_STD : I2C_PAD_CTRL_FALLSLEW_LOW; - pad_ctrl |= I2C_PAD_CTRL_FALLSLEW_EN; - misc_write32(MISC_I2C_PAD_CTRL(bus), pad_ctrl); + fch_i2c_pad_init(bus, cfg->speed, &config->i2c_pad[bus]); } const struct soc_i2c_ctrlr_info *soc_get_i2c_ctrlr_info(size_t *num_ctrlrs) diff --git a/src/soc/amd/cezanne/include/soc/southbridge.h b/src/soc/amd/cezanne/include/soc/southbridge.h index 9c68d38..4e44b89 100644 --- a/src/soc/amd/cezanne/include/soc/southbridge.h +++ b/src/soc/amd/cezanne/include/soc/southbridge.h @@ -112,33 +112,6 @@ #define MISC_CLK_CNTL0 0x40 /* named MISC_CLK_CNTL1 on Picasso */ #define BP_X48M0_S0I3_DIS BIT(4) #define BP_X48M0_OUTPUT_EN BIT(2) /* 1=En, unlike Hudson, Kern */ -#define MISC_I2C0_PAD_CTRL 0xd8 -#define MISC_I2C1_PAD_CTRL 0xdc -#define MISC_I2C2_PAD_CTRL 0xe0 -#define MISC_I2C3_PAD_CTRL 0xe4 -#define MISC_I2C_PAD_CTRL(bus) (MISC_I2C0_PAD_CTRL + 4 * (bus)) -#define I2C_PAD_CTRL_NG_MASK (BIT(0) | BIT(1) | BIT(2) | BIT(3)) -#define I2C_PAD_CTRL_NG_NORMAL 0xc -#define I2C_PAD_CTRL_RX_SEL_MASK (BIT(4) | BIT(5)) -#define I2C_PAD_CTRL_RX_SHIFT 4 -#define I2C_PAD_CTRL_RX_SEL_OFF (0 << I2C_PAD_CTRL_RX_SHIFT) -#define I2C_PAD_CTRL_RX_SEL_3_3V (1 << I2C_PAD_CTRL_RX_SHIFT) -#define I2C_PAD_CTRL_RX_SEL_1_8V (3 << I2C_PAD_CTRL_RX_SHIFT) -#define I2C_PAD_CTRL_PULLDOWN_EN BIT(6) -#define I2C_PAD_CTRL_FALLSLEW_MASK (BIT(7) | BIT(8)) -#define I2C_PAD_CTRL_FALLSLEW_SHIFT 7 -#define I2C_PAD_CTRL_FALLSLEW_STD (0 << I2C_PAD_CTRL_FALLSLEW_SHIFT) -#define I2C_PAD_CTRL_FALLSLEW_LOW (1 << I2C_PAD_CTRL_FALLSLEW_SHIFT) -#define I2C_PAD_CTRL_FALLSLEW_EN BIT(9) -#define I2C_PAD_CTRL_SPIKE_RC_EN BIT(10) -#define I2C_PAD_CTRL_SPIKE_RC_SEL BIT(11) /* 0 = 50ns, 1 = 20ns */ -#define I2C_PAD_CTRL_CAP_DOWN BIT(12) -#define I2C_PAD_CTRL_CAP_UP BIT(13) -#define I2C_PAD_CTRL_RES_DOWN BIT(14) -#define I2C_PAD_CTRL_RES_UP BIT(15) -#define I2C_PAD_CTRL_BIAS_CRT_EN BIT(16) -#define I2C_PAD_CTRL_SPARE0 BIT(17) -#define I2C_PAD_CTRL_SPARE1 BIT(18) void fch_pre_init(void); void fch_early_init(void); diff --git a/src/soc/amd/common/block/i2c/Kconfig b/src/soc/amd/common/block/i2c/Kconfig index 5d8498c..7c1ea0e 100644 --- a/src/soc/amd/common/block/i2c/Kconfig +++ b/src/soc/amd/common/block/i2c/Kconfig @@ -2,3 +2,9 @@ bool help Select this option to add FCH I2C controller functions to the build. + +config SOC_AMD_COMMON_BLOCK_I2C_PAD_CTRL + bool + help + Select this option to add FCH I2C pad configuration functions to the + build. diff --git a/src/soc/amd/common/block/i2c/Makefile.inc b/src/soc/amd/common/block/i2c/Makefile.inc index 8af7796..3d2cdf6 100644 --- a/src/soc/amd/common/block/i2c/Makefile.inc +++ b/src/soc/amd/common/block/i2c/Makefile.inc @@ -1 +1,2 @@ all-$(CONFIG_SOC_AMD_COMMON_BLOCK_I2C) += i2c.c +all-$(CONFIG_SOC_AMD_COMMON_BLOCK_I2C_PAD_CTRL) += i2c_pad_ctrl.c diff --git a/src/soc/amd/common/block/i2c/i2c_pad_ctrl.c b/src/soc/amd/common/block/i2c/i2c_pad_ctrl.c new file mode 100644 index 0000000..cdb7900 --- /dev/null +++ b/src/soc/amd/common/block/i2c/i2c_pad_ctrl.c @@ -0,0 +1,45 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <amdblocks/acpimmio.h> +#include <amdblocks/i2c.h> +#include <console/console.h> +#include <types.h> +#include "i2c_pad_def.h" + +void fch_i2c_pad_init(unsigned int bus, + enum i2c_speed speed, + const struct i2c_pad_control *ctrl) +{ + uint32_t pad_ctrl; + + pad_ctrl = misc_read32(MISC_I2C_PAD_CTRL(bus)); + + pad_ctrl &= ~I2C_PAD_CTRL_NG_MASK; + pad_ctrl |= I2C_PAD_CTRL_NG_NORMAL; + + switch (ctrl->rx_level) { + case I2C_PAD_RX_NO_CHANGE: + break; + case I2C_PAD_RX_OFF: + pad_ctrl &= ~I2C_PAD_CTRL_RX_SEL_MASK; + pad_ctrl |= I2C_PAD_CTRL_RX_SEL_OFF; + break; + case I2C_PAD_RX_3_3V: + pad_ctrl &= ~I2C_PAD_CTRL_RX_SEL_MASK; + pad_ctrl |= I2C_PAD_CTRL_RX_SEL_3_3V; + break; + case I2C_PAD_RX_1_8V: + pad_ctrl &= ~I2C_PAD_CTRL_RX_SEL_MASK; + pad_ctrl |= I2C_PAD_CTRL_RX_SEL_1_8V; + break; + default: + printk(BIOS_WARNING, "Invalid I2C pad RX level for bus %u\n", bus); + break; + } + + pad_ctrl &= ~I2C_PAD_CTRL_FALLSLEW_MASK; + pad_ctrl |= speed == I2C_SPEED_STANDARD ? + I2C_PAD_CTRL_FALLSLEW_STD : I2C_PAD_CTRL_FALLSLEW_LOW; + pad_ctrl |= I2C_PAD_CTRL_FALLSLEW_EN; + misc_write32(MISC_I2C_PAD_CTRL(bus), pad_ctrl); +} diff --git a/src/soc/amd/common/block/i2c/i2c_pad_def.h b/src/soc/amd/common/block/i2c/i2c_pad_def.h new file mode 100644 index 0000000..76e10cd --- /dev/null +++ b/src/soc/amd/common/block/i2c/i2c_pad_def.h @@ -0,0 +1,38 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef AMD_BLOCK_I2C_PAD_DEF_H +#define AMD_BLOCK_I2C_PAD_DEF_H + +#include <types.h> + +#define MISC_I2C0_PAD_CTRL 0xd8 +#define MISC_I2C_PAD_CTRL(bus) (MISC_I2C0_PAD_CTRL + 4 * (bus)) + +#define I2C_PAD_CTRL_NG_MASK (BIT(0) | BIT(1) | BIT(2) | BIT(3)) +#define I2C_PAD_CTRL_NG_NORMAL 0xc +#define I2C_PAD_CTRL_RX_SEL_MASK (BIT(4) | BIT(5)) +#define I2C_PAD_CTRL_RX_SHIFT 4 +#define I2C_PAD_CTRL_RX_SEL_OFF (0 << I2C_PAD_CTRL_RX_SHIFT) +#define I2C_PAD_CTRL_RX_SEL_3_3V (1 << I2C_PAD_CTRL_RX_SHIFT) +#define I2C_PAD_CTRL_RX_SEL_1_8V (3 << I2C_PAD_CTRL_RX_SHIFT) +#define I2C_PAD_CTRL_PULLDOWN_EN BIT(6) +#define I2C_PAD_CTRL_FALLSLEW_MASK (BIT(7) | BIT(8)) +#define I2C_PAD_CTRL_FALLSLEW_SHIFT 7 +#define I2C_PAD_CTRL_FALLSLEW_STD (0 << I2C_PAD_CTRL_FALLSLEW_SHIFT) +#define I2C_PAD_CTRL_FALLSLEW_LOW (1 << I2C_PAD_CTRL_FALLSLEW_SHIFT) +#define I2C_PAD_CTRL_FALLSLEW_EN BIT(9) +#define I2C_PAD_CTRL_SPIKE_RC_EN BIT(10) +#define I2C_PAD_CTRL_SPIKE_RC_SEL BIT(11) /* 0 = 50ns, 1 = 20ns */ +#define I2C_PAD_CTRL_CAP_DOWN BIT(12) +#define I2C_PAD_CTRL_CAP_UP BIT(13) +#define I2C_PAD_CTRL_RES_DOWN BIT(14) +#define I2C_PAD_CTRL_RES_UP BIT(15) +#define I2C_PAD_CTRL_BIAS_CRT_EN BIT(16) +#define I2C_PAD_CTRL_SPARE0 BIT(17) +#define I2C_PAD_CTRL_SPARE1 BIT(18) +/* The following bits are reserved in Picasso and Cezanne */ +#define I2C_PAD_CTRL_PD_EN BIT(19) +#define I2C_PAD_CTRL_COMP_SEL BIT(20) +#define I2C_PAD_CTRL_RES_BIAS_EN BIT(21) + +#endif /* AMD_BLOCK_I2C_PAD_DEF_H */ diff --git a/src/soc/amd/common/block/include/amdblocks/i2c.h b/src/soc/amd/common/block/include/amdblocks/i2c.h index 6660e37..54651a9 100644 --- a/src/soc/amd/common/block/include/amdblocks/i2c.h +++ b/src/soc/amd/common/block/include/amdblocks/i2c.h @@ -56,6 +56,21 @@ size_t num_pins; }; +enum i2c_pad_rx_level { + I2C_PAD_RX_NO_CHANGE, + I2C_PAD_RX_OFF, + I2C_PAD_RX_3_3V, + I2C_PAD_RX_1_8V, +}; + +struct i2c_pad_control { + enum i2c_pad_rx_level rx_level; +}; + +void fch_i2c_pad_init(unsigned int bus, + enum i2c_speed speed, + const struct i2c_pad_control *ctrl); + /* Helper function to perform misc I2C configuration specific to SoC. */ void soc_i2c_misc_init(unsigned int bus, const struct dw_i2c_bus_config *cfg); diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig index 4567d55..ebcbd62 100644 --- a/src/soc/amd/picasso/Kconfig +++ b/src/soc/amd/picasso/Kconfig @@ -41,6 +41,7 @@ select SOC_AMD_COMMON_BLOCK_HAS_ESPI select SOC_AMD_COMMON_BLOCK_HDA select SOC_AMD_COMMON_BLOCK_I2C + select SOC_AMD_COMMON_BLOCK_I2C_PAD_CTRL select SOC_AMD_COMMON_BLOCK_IOMMU select SOC_AMD_COMMON_BLOCK_LPC select SOC_AMD_COMMON_BLOCK_MCAX diff --git a/src/soc/amd/picasso/i2c.c b/src/soc/amd/picasso/i2c.c index 40cf556..ebc833d 100644 --- a/src/soc/amd/picasso/i2c.c +++ b/src/soc/amd/picasso/i2c.c @@ -40,22 +40,13 @@ void soc_i2c_misc_init(unsigned int bus, const struct dw_i2c_bus_config *cfg) { - uint32_t pad_ctrl; + /* TODO: Picasso supports I2C RX pad configurations 3.3V, 1.8V and off, so make this + configurable. */ + const struct i2c_pad_control ctrl = { + .rx_level = I2C_PAD_RX_3_3V, + }; - pad_ctrl = misc_read32(MISC_I2C_PAD_CTRL(bus)); - - pad_ctrl &= ~I2C_PAD_CTRL_NG_MASK; - pad_ctrl |= I2C_PAD_CTRL_NG_NORMAL; - - pad_ctrl &= ~I2C_PAD_CTRL_RX_SEL_MASK; - pad_ctrl |= I2C_PAD_CTRL_RX_SEL_3_3V; - - pad_ctrl &= ~I2C_PAD_CTRL_FALLSLEW_MASK; - pad_ctrl |= cfg->speed == I2C_SPEED_STANDARD ? - I2C_PAD_CTRL_FALLSLEW_STD : I2C_PAD_CTRL_FALLSLEW_LOW; - pad_ctrl |= I2C_PAD_CTRL_FALLSLEW_EN; - - misc_write32(MISC_I2C_PAD_CTRL(bus), pad_ctrl); + fch_i2c_pad_init(bus, cfg->speed, &ctrl); } const struct soc_i2c_ctrlr_info *soc_get_i2c_ctrlr_info(size_t *num_ctrlrs) diff --git a/src/soc/amd/picasso/include/soc/southbridge.h b/src/soc/amd/picasso/include/soc/southbridge.h index 2a3e38b..b8dd67b 100644 --- a/src/soc/amd/picasso/include/soc/southbridge.h +++ b/src/soc/amd/picasso/include/soc/southbridge.h @@ -92,33 +92,6 @@ #define CG1PLL_SPREAD_SPECTRUM_ENABLE BIT(0) #define MISC_CLK_CNTL1 0x40 #define BP_X48M0_OUTPUT_EN BIT(2) /* 1=En, unlike Hudson, Kern */ -#define MISC_I2C0_PAD_CTRL 0xd8 -#define MISC_I2C1_PAD_CTRL 0xdc -#define MISC_I2C2_PAD_CTRL 0xe0 -#define MISC_I2C3_PAD_CTRL 0xe4 -#define MISC_I2C_PAD_CTRL(bus) (MISC_I2C0_PAD_CTRL + 4 * (bus)) -#define I2C_PAD_CTRL_NG_MASK (BIT(0) | BIT(1) | BIT(2) | BIT(3)) -#define I2C_PAD_CTRL_NG_NORMAL 0xc -#define I2C_PAD_CTRL_RX_SEL_MASK (BIT(4) | BIT(5)) -#define I2C_PAD_CTRL_RX_SHIFT 4 -#define I2C_PAD_CTRL_RX_SEL_OFF (0 << I2C_PAD_CTRL_RX_SHIFT) -#define I2C_PAD_CTRL_RX_SEL_3_3V (1 << I2C_PAD_CTRL_RX_SHIFT) -#define I2C_PAD_CTRL_RX_SEL_1_8V (3 << I2C_PAD_CTRL_RX_SHIFT) -#define I2C_PAD_CTRL_PULLDOWN_EN BIT(6) -#define I2C_PAD_CTRL_FALLSLEW_MASK (BIT(7) | BIT(8)) -#define I2C_PAD_CTRL_FALLSLEW_SHIFT 7 -#define I2C_PAD_CTRL_FALLSLEW_STD (0 << I2C_PAD_CTRL_FALLSLEW_SHIFT) -#define I2C_PAD_CTRL_FALLSLEW_LOW (1 << I2C_PAD_CTRL_FALLSLEW_SHIFT) -#define I2C_PAD_CTRL_FALLSLEW_EN BIT(9) -#define I2C_PAD_CTRL_SPIKE_RC_EN BIT(10) -#define I2C_PAD_CTRL_SPIKE_RC_SEL BIT(11) /* 0 = 50ns, 1 = 20ns */ -#define I2C_PAD_CTRL_CAP_DOWN BIT(12) -#define I2C_PAD_CTRL_CAP_UP BIT(13) -#define I2C_PAD_CTRL_RES_DOWN BIT(14) -#define I2C_PAD_CTRL_RES_UP BIT(15) -#define I2C_PAD_CTRL_BIAS_CRT_EN BIT(16) -#define I2C_PAD_CTRL_SPARE0 BIT(17) -#define I2C_PAD_CTRL_SPARE1 BIT(18) #define FCH_LEGACY_UART_DECODE (ALINK_AHB_ADDRESS + 0x20) /* 0xfedc0020 */ diff --git a/src/soc/amd/sabrina/Kconfig b/src/soc/amd/sabrina/Kconfig index ca7a83c..2e34e3f 100644 --- a/src/soc/amd/sabrina/Kconfig +++ b/src/soc/amd/sabrina/Kconfig @@ -52,6 +52,7 @@ select SOC_AMD_COMMON_BLOCK_GRAPHICS # TODO: Check if this is still correct select SOC_AMD_COMMON_BLOCK_HAS_ESPI # TODO: Check if this is still correct select SOC_AMD_COMMON_BLOCK_I2C # TODO: Check if this is still correct + select SOC_AMD_COMMON_BLOCK_I2C_PAD_CTRL select SOC_AMD_COMMON_BLOCK_IOMMU # TODO: Check if this is still correct select SOC_AMD_COMMON_BLOCK_LPC # TODO: Check if this is still correct select SOC_AMD_COMMON_BLOCK_MCAX # TODO: Check if this is still correct diff --git a/src/soc/amd/sabrina/chip.h b/src/soc/amd/sabrina/chip.h index a7acb7f..c272f9f 100644 --- a/src/soc/amd/sabrina/chip.h +++ b/src/soc/amd/sabrina/chip.h @@ -6,6 +6,7 @@ #define SABRINA_CHIP_H #include <amdblocks/chip.h> +#include <amdblocks/i2c.h> #include <gpio.h> #include <soc/i2c.h> #include <soc/southbridge.h> @@ -17,7 +18,7 @@ struct soc_amd_common_config common_config; u8 i2c_scl_reset; struct dw_i2c_bus_config i2c[I2C_CTRLR_COUNT]; - u8 i2c_pad_ctrl_rx_sel[I2C_CTRLR_COUNT]; + struct i2c_pad_control i2c_pad[I2C_CTRLR_COUNT]; /* Enable S0iX support */ bool s0ix_enable; diff --git a/src/soc/amd/sabrina/i2c.c b/src/soc/amd/sabrina/i2c.c index 89751d5..076a0f2 100644 --- a/src/soc/amd/sabrina/i2c.c +++ b/src/soc/amd/sabrina/i2c.c @@ -38,24 +38,11 @@ void soc_i2c_misc_init(unsigned int bus, const struct dw_i2c_bus_config *cfg) { const struct soc_amd_sabrina_config *config = config_of_soc(); - uint32_t pad_ctrl; - if (bus >= ARRAY_SIZE(config->i2c_pad_ctrl_rx_sel)) + if (bus >= ARRAY_SIZE(config->i2c_pad)) return; - pad_ctrl = misc_read32(MISC_I2C_PAD_CTRL(bus)); - - pad_ctrl &= ~I2C_PAD_CTRL_NG_MASK; - pad_ctrl |= I2C_PAD_CTRL_NG_NORMAL; - - pad_ctrl &= ~I2C_PAD_CTRL_RX_SEL_MASK; - pad_ctrl |= config->i2c_pad_ctrl_rx_sel[bus]; - - pad_ctrl &= ~I2C_PAD_CTRL_FALLSLEW_MASK; - pad_ctrl |= cfg->speed == I2C_SPEED_STANDARD ? - I2C_PAD_CTRL_FALLSLEW_STD : I2C_PAD_CTRL_FALLSLEW_LOW; - pad_ctrl |= I2C_PAD_CTRL_FALLSLEW_EN; - misc_write32(MISC_I2C_PAD_CTRL(bus), pad_ctrl); + fch_i2c_pad_init(bus, cfg->speed, &config->i2c_pad[bus]); } const struct soc_i2c_ctrlr_info *soc_get_i2c_ctrlr_info(size_t *num_ctrlrs) diff --git a/src/soc/amd/sabrina/include/soc/southbridge.h b/src/soc/amd/sabrina/include/soc/southbridge.h index e85a052..e755d50 100644 --- a/src/soc/amd/sabrina/include/soc/southbridge.h +++ b/src/soc/amd/sabrina/include/soc/southbridge.h @@ -113,36 +113,6 @@ #define MISC_CLK_CNTL0 0x40 /* named MISC_CLK_CNTL1 on Picasso */ #define BP_X48M0_S0I3_DIS BIT(4) #define BP_X48M0_OUTPUT_EN BIT(2) /* 1=En, unlike Hudson, Kern */ -#define MISC_I2C0_PAD_CTRL 0xd8 -#define MISC_I2C1_PAD_CTRL 0xdc -#define MISC_I2C2_PAD_CTRL 0xe0 -#define MISC_I2C3_PAD_CTRL 0xe4 -#define MISC_I2C_PAD_CTRL(bus) (MISC_I2C0_PAD_CTRL + 4 * (bus)) -#define I2C_PAD_CTRL_NG_MASK (BIT(0) | BIT(1) | BIT(2) | BIT(3)) -#define I2C_PAD_CTRL_NG_NORMAL 0xc -#define I2C_PAD_CTRL_RX_SEL_MASK (BIT(4) | BIT(5)) -#define I2C_PAD_CTRL_RX_SHIFT 4 -#define I2C_PAD_CTRL_RX_SEL_OFF (0 << I2C_PAD_CTRL_RX_SHIFT) -#define I2C_PAD_CTRL_RX_SEL_3_3V (1 << I2C_PAD_CTRL_RX_SHIFT) -#define I2C_PAD_CTRL_RX_SEL_1_8V (3 << I2C_PAD_CTRL_RX_SHIFT) -#define I2C_PAD_CTRL_PULLDOWN_EN BIT(6) -#define I2C_PAD_CTRL_FALLSLEW_MASK (BIT(7) | BIT(8)) -#define I2C_PAD_CTRL_FALLSLEW_SHIFT 7 -#define I2C_PAD_CTRL_FALLSLEW_STD (0 << I2C_PAD_CTRL_FALLSLEW_SHIFT) -#define I2C_PAD_CTRL_FALLSLEW_LOW (1 << I2C_PAD_CTRL_FALLSLEW_SHIFT) -#define I2C_PAD_CTRL_FALLSLEW_EN BIT(9) -#define I2C_PAD_CTRL_SPIKE_RC_EN BIT(10) -#define I2C_PAD_CTRL_SPIKE_RC_SEL BIT(11) /* 0 = 50ns, 1 = 20ns */ -#define I2C_PAD_CTRL_CAP_DOWN BIT(12) -#define I2C_PAD_CTRL_CAP_UP BIT(13) -#define I2C_PAD_CTRL_RES_DOWN BIT(14) -#define I2C_PAD_CTRL_RES_UP BIT(15) -#define I2C_PAD_CTRL_BIAS_CRT_EN BIT(16) -#define I2C_PAD_CTRL_SPARE0 BIT(17) -#define I2C_PAD_CTRL_SPARE1 BIT(18) -#define I2C_PAD_CTRL_PD_EN BIT(19) -#define I2C_PAD_CTRL_COMP_SEL BIT(20) -#define I2C_PAD_CTRL_RES_BIAS_EN BIT(21) void fch_pre_init(void); void fch_early_init(void); -- To view, visit https://review.coreboot.org/c/coreboot/+/61568 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1d70329644b68be3c4a1602f748e09db20cf6de1 Gerrit-Change-Number: 61568 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: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: soc/amd/*/i2c: introduce and use MISC_I2CX_PAD_CTRL(bus) macro
by Felix Held (Code Review) 02 Feb '22

02 Feb '22

1 0

Change in coreboot[master]: soc/amd/*/i2c: drop unused mainboard_i2c_override
by Felix Held (Code Review) 02 Feb '22

02 Feb '22

Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/61566 ) Change subject: soc/amd/*/i2c: drop unused mainboard_i2c_override ...................................................................... soc/amd/*/i2c: drop unused mainboard_i2c_override No mainboard in the current tree implements mainboard_i2c_override. In a follow-up commit the i2c_pad_control struct is introduced to be able to make more parameters controllable by devicetree settings in the future. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I8f9ed5d50d26e4623dc5888cc8af090fdd00fc03 --- M src/soc/amd/cezanne/i2c.c M src/soc/amd/cezanne/include/soc/southbridge.h M src/soc/amd/picasso/i2c.c M src/soc/amd/picasso/include/soc/southbridge.h M src/soc/amd/sabrina/i2c.c M src/soc/amd/sabrina/include/soc/southbridge.h 6 files changed, 0 insertions(+), 20 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/61566/1 diff --git a/src/soc/amd/cezanne/i2c.c b/src/soc/amd/cezanne/i2c.c index 008b261..153fa7c 100644 --- a/src/soc/amd/cezanne/i2c.c +++ b/src/soc/amd/cezanne/i2c.c @@ -33,8 +33,6 @@ } #endif -__weak void mainboard_i2c_override(int bus, uint32_t *pad_settings) { } - void soc_i2c_misc_init(unsigned int bus, const struct dw_i2c_bus_config *cfg) { const struct soc_amd_cezanne_config *config = config_of_soc(); @@ -57,8 +55,6 @@ pad_ctrl |= cfg->speed == I2C_SPEED_STANDARD ? I2C_PAD_CTRL_FALLSLEW_STD : I2C_PAD_CTRL_FALLSLEW_LOW; pad_ctrl |= I2C_PAD_CTRL_FALLSLEW_EN; - - mainboard_i2c_override(bus, &pad_ctrl); misc_write32(misc_reg, pad_ctrl); } diff --git a/src/soc/amd/cezanne/include/soc/southbridge.h b/src/soc/amd/cezanne/include/soc/southbridge.h index 4f372ad..113395e 100644 --- a/src/soc/amd/cezanne/include/soc/southbridge.h +++ b/src/soc/amd/cezanne/include/soc/southbridge.h @@ -147,7 +147,4 @@ void enable_aoac_devices(void); void wait_for_aoac_enabled(unsigned int dev); -/* Allow the board to change the default I2C pad configuration */ -void mainboard_i2c_override(int bus, uint32_t *pad_settings); - #endif /* AMD_CEZANNE_SOUTHBRIDGE_H */ diff --git a/src/soc/amd/picasso/i2c.c b/src/soc/amd/picasso/i2c.c index 18660f6..925a832 100644 --- a/src/soc/amd/picasso/i2c.c +++ b/src/soc/amd/picasso/i2c.c @@ -38,8 +38,6 @@ } #endif -__weak void mainboard_i2c_override(int bus, uint32_t *pad_settings) { } - void soc_i2c_misc_init(unsigned int bus, const struct dw_i2c_bus_config *cfg) { uint32_t pad_ctrl; @@ -59,7 +57,6 @@ I2C_PAD_CTRL_FALLSLEW_STD : I2C_PAD_CTRL_FALLSLEW_LOW; pad_ctrl |= I2C_PAD_CTRL_FALLSLEW_EN; - mainboard_i2c_override(bus, &pad_ctrl); misc_write32(misc_reg, pad_ctrl); } diff --git a/src/soc/amd/picasso/include/soc/southbridge.h b/src/soc/amd/picasso/include/soc/southbridge.h index 2936600..d439e92 100644 --- a/src/soc/amd/picasso/include/soc/southbridge.h +++ b/src/soc/amd/picasso/include/soc/southbridge.h @@ -136,7 +136,4 @@ void enable_aoac_devices(void); void wait_for_aoac_enabled(unsigned int dev); -/* Allow the board to change the default I2C pad configuration */ -void mainboard_i2c_override(int bus, uint32_t *pad_settings); - #endif /* AMD_PICASSO_SOUTHBRIDGE_H */ diff --git a/src/soc/amd/sabrina/i2c.c b/src/soc/amd/sabrina/i2c.c index 2030f40..f3eaf4e 100644 --- a/src/soc/amd/sabrina/i2c.c +++ b/src/soc/amd/sabrina/i2c.c @@ -35,8 +35,6 @@ } #endif -__weak void mainboard_i2c_override(int bus, uint32_t *pad_settings) { } - void soc_i2c_misc_init(unsigned int bus, const struct dw_i2c_bus_config *cfg) { const struct soc_amd_sabrina_config *config = config_of_soc(); @@ -59,8 +57,6 @@ pad_ctrl |= cfg->speed == I2C_SPEED_STANDARD ? I2C_PAD_CTRL_FALLSLEW_STD : I2C_PAD_CTRL_FALLSLEW_LOW; pad_ctrl |= I2C_PAD_CTRL_FALLSLEW_EN; - - mainboard_i2c_override(bus, &pad_ctrl); misc_write32(misc_reg, pad_ctrl); } diff --git a/src/soc/amd/sabrina/include/soc/southbridge.h b/src/soc/amd/sabrina/include/soc/southbridge.h index 4b25c5e..7b5644b 100644 --- a/src/soc/amd/sabrina/include/soc/southbridge.h +++ b/src/soc/amd/sabrina/include/soc/southbridge.h @@ -151,7 +151,4 @@ void enable_aoac_devices(void); void wait_for_aoac_enabled(unsigned int dev); -/* Allow the board to change the default I2C pad configuration */ -void mainboard_i2c_override(int bus, uint32_t *pad_settings); - #endif /* AMD_SABRINA_SOUTHBRIDGE_H */ -- To view, visit https://review.coreboot.org/c/coreboot/+/61566 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8f9ed5d50d26e4623dc5888cc8af090fdd00fc03 Gerrit-Change-Number: 61566 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: soc/amd/common/fsp: check fsp image revision
by Name of user not set (Code Review) 02 Feb '22

02 Feb '22

Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Paul Menzel, Name of user not set #1003801, Andrey Petrov, Patrick Rudolph. Hello build bot (Jenkins), Jason Glenesk, Raul Rangel, Marshall Dawson, Andrey Petrov, Julian Schroeder, Patrick Rudolph, Felix Held, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/61281 to look at the new patch set (#12). Change subject: soc/amd/common/fsp: check fsp image revision ...................................................................... soc/amd/common/fsp: check fsp image revision Check if FSP binary and coreboot FSP structures (fspmupd.h) match sufficiently. A change in minor number denotes less critical changes or additions to the FSP API that still allow for the boot process to proceed. A change of the AMD image revision major number will halt boot. The Fspmupd.h header now defines IMAGE_REVISION_ macros for AMD Picasso, Cezanne and Sabrina APUs. BUG=b:184650244 TEST=build, boot and check fsp image revision info. Example: FSP major = 1 FSP minor = 0 FSP revision = 5 FSP build = 0 Signed-off-by: Julian Schroeder <julianmarcusschroeder(a)gmail.com> Change-Id: I0fbf9413b0cf3e6093ee9c61ff692ff78ebefebc --- M src/soc/amd/common/fsp/fsp_validate.c M src/vendorcode/amd/fsp/cezanne/FspmUpd.h M src/vendorcode/amd/fsp/picasso/FspmUpd.h M src/vendorcode/amd/fsp/sabrina/FspmUpd.h 4 files changed, 55 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/61281/12 -- To view, visit https://review.coreboot.org/c/coreboot/+/61281 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0fbf9413b0cf3e6093ee9c61ff692ff78ebefebc Gerrit-Change-Number: 61281 Gerrit-PatchSet: 12 Gerrit-Owner: Name of user not set #1003801 Gerrit-Reviewer: Andrey Petrov <andrey.petrov(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-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julian Schroeder <julianmarcusschroeder(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Name of user not set #1003801 Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: lib/spd_bin: Fix for LPDDR5
by Selma Bensaid (Code Review) 02 Feb '22

02 Feb '22

Attention is currently required from: Anil Kumar K, Cliff Huang, Selma Bensaid, Tim Wawrzynczak, Angel Pons, Bernardo Perez Priego. Selma Bensaid has uploaded a new patch set (#9) to the change originally created by Cliff Huang. ( https://review.coreboot.org/c/coreboot/+/58101 ) Change subject: lib/spd_bin: Fix for LPDDR5 ...................................................................... lib/spd_bin: Fix for LPDDR5 This addresses the warning message: Defaulting to using DDR4 params. Please add dram_type check for 19 to use_ddr4_params 1. Add LPDDR5 for the param 2. Fix SPD name for LPDDR5 Signed-off-by: Cliff Huang <cliff.huang(a)intel.com> Change-Id: I52ecf24a313d4cbdd0859c623533630c6a6c3713 --- M src/lib/spd_bin.c 1 file changed, 10 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/58101/9 -- To view, visit https://review.coreboot.org/c/coreboot/+/58101 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I52ecf24a313d4cbdd0859c623533630c6a6c3713 Gerrit-Change-Number: 58101 Gerrit-PatchSet: 9 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: drivers/intel/fsp2_0/include/fsp: fix fsp_header
by Felix Held (Code Review) 02 Feb '22

02 Feb '22

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/58869 ) Change subject: drivers/intel/fsp2_0/include/fsp: fix fsp_header ...................................................................... drivers/intel/fsp2_0/include/fsp: fix fsp_header This patch aligns fsp_header with the Intel specification 2.0 and 2.3. The main impetus for this change is to make the fsp_info_header fully accessible in soc/vendor code. Here items such as image_revision can be checked. TEST=verify image revision output in the coreboot serial log. compare to FSP version shown in serial debug output. verify Google Guybrush machine boots into OS. Signed-off-by: Julian Schroeder <julianmarcusschroeder(a)gmail.com> Change-Id: Ibf50f16b5e9793d946a95970fcdabc4c07289646 Reviewed-on: https://review.coreboot.org/c/coreboot/+/58869 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Felix Held <felix-coreboot(a)felixheld.de> Reviewed-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> --- M src/drivers/intel/fsp2_0/header_display.c M src/drivers/intel/fsp2_0/include/fsp/info_header.h M src/drivers/intel/fsp2_0/memory_init.c M src/drivers/intel/fsp2_0/silicon_init.c M src/drivers/intel/fsp2_0/temp_ram_exit.c M src/drivers/intel/fsp2_0/util.c 6 files changed, 52 insertions(+), 62 deletions(-) Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, but someone else must approve Tim Wawrzynczak: Looks good to me, approved diff --git a/src/drivers/intel/fsp2_0/header_display.c b/src/drivers/intel/fsp2_0/header_display.c index 4c8085e..d209e4c 100644 --- a/src/drivers/intel/fsp2_0/header_display.c +++ b/src/drivers/intel/fsp2_0/header_display.c @@ -8,13 +8,14 @@ union fsp_revision revision; union extended_fsp_revision ext_revision; ext_revision.val = 0; + int i; /* For FSP 2.3 and later use extended image revision field present in header * for build number and revision calculation */ if (CONFIG(PLATFORM_USES_FSP2_3)) - ext_revision.val = hdr->extended_fsp_revision; + ext_revision.val = hdr->extended_image_revision; - revision.val = hdr->fsp_revision; + revision.val = hdr->image_revision; printk(BIOS_SPEW, "Spec version: v%u.%u\n", (hdr->spec_version >> 4), hdr->spec_version & 0xf); printk(BIOS_SPEW, "Revision: %u.%u.%u, Build Number %u\n", @@ -25,22 +26,28 @@ printk(BIOS_SPEW, "Type: %s/%s\n", (hdr->component_attribute & 1) ? "release" : "debug", (hdr->component_attribute & 2) ? "official" : "test"); - printk(BIOS_SPEW, "image ID: %s, base 0x%zx + 0x%zx\n", - hdr->image_id, (size_t)hdr->image_base, (size_t)hdr->image_size); + + printk(BIOS_SPEW, "image ID: "); + for (i = 0; i < FSP_IMAGE_ID_LENGTH; i++) + printk(BIOS_SPEW, "%c", hdr->image_id[i]); + printk(BIOS_SPEW, "\n"); + + printk(BIOS_SPEW, " base 0x%zx + 0x%zx\n", + (size_t)hdr->image_base, (size_t)hdr->image_size); printk(BIOS_SPEW, "\tConfig region 0x%zx + 0x%zx\n", (size_t)hdr->cfg_region_offset, (size_t)hdr->cfg_region_size); if ((hdr->component_attribute >> 12) == FSP_HDR_ATTRIB_FSPM) { printk(BIOS_SPEW, "\tMemory init offset 0x%zx\n", - (size_t)hdr->memory_init_entry_offset); + (size_t)hdr->fsp_memory_init_entry_offset); } if ((hdr->component_attribute >> 12) == FSP_HDR_ATTRIB_FSPS) { printk(BIOS_SPEW, "\tSilicon init offset 0x%zx\n", - (size_t)hdr->silicon_init_entry_offset); + (size_t)hdr->fsp_silicon_init_entry_offset); if (CONFIG(PLATFORM_USES_FSP2_2)) printk(BIOS_SPEW, "\tMultiPhaseSiInit offset 0x%zx\n", - (size_t)hdr->multi_phase_si_init_entry_offset); + (size_t)hdr->fsp_multi_phase_si_init_entry_offset); printk(BIOS_SPEW, "\tNotify phase offset 0x%zx\n", (size_t)hdr->notify_phase_entry_offset); } diff --git a/src/drivers/intel/fsp2_0/include/fsp/info_header.h b/src/drivers/intel/fsp2_0/include/fsp/info_header.h index 136fc2b..fceebec 100644 --- a/src/drivers/intel/fsp2_0/include/fsp/info_header.h +++ b/src/drivers/intel/fsp2_0/include/fsp/info_header.h @@ -10,27 +10,34 @@ #define FSP_HDR_ATTRIB_FSPT 1 #define FSP_HDR_ATTRIB_FSPM 2 #define FSP_HDR_ATTRIB_FSPS 3 +#define FSP_IMAGE_ID_LENGTH 8 #if CONFIG(PLATFORM_USES_FSP2_X86_32) struct fsp_header { - uint32_t fsp_revision; - uint16_t extended_fsp_revision; - uint32_t image_size; - uint32_t image_base; - uint16_t image_attribute; - uint8_t spec_version; - uint16_t component_attribute; - uint32_t cfg_region_offset; - uint32_t cfg_region_size; - uint32_t temp_ram_init_entry; - uint32_t temp_ram_exit_entry; - uint32_t notify_phase_entry_offset; - uint32_t memory_init_entry_offset; - uint32_t silicon_init_entry_offset; - uint32_t multi_phase_si_init_entry_offset; - char image_id[sizeof(uint64_t) + 1]; - uint8_t revision; -} __packed; + uint32_t signature; //FSPH + uint32_t header_length; + uint8_t res1[2]; + uint8_t spec_version; + uint8_t header_revision; + uint32_t image_revision; + char image_id[FSP_IMAGE_ID_LENGTH]; // not zero terminated + uint32_t image_size; + uint32_t image_base; + uint16_t image_attribute; + uint16_t component_attribute; + uint32_t cfg_region_offset; + uint32_t cfg_region_size; + uint32_t res2; + uint32_t temp_ram_init_entry_offset; //initial stack + uint32_t res3; + uint32_t notify_phase_entry_offset; + uint32_t fsp_memory_init_entry_offset; + uint32_t temp_ram_exit_entry_offset; + uint32_t fsp_silicon_init_entry_offset; + uint32_t fsp_multi_phase_si_init_entry_offset; + uint16_t extended_image_revision; + uint16_t res4; +} __packed; #else #error You need to implement this struct for x86_64 FSP #endif diff --git a/src/drivers/intel/fsp2_0/memory_init.c b/src/drivers/intel/fsp2_0/memory_init.c index 3c1a088..3643866 100644 --- a/src/drivers/intel/fsp2_0/memory_init.c +++ b/src/drivers/intel/fsp2_0/memory_init.c @@ -206,7 +206,7 @@ static uint32_t fsp_memory_settings_version(const struct fsp_header *hdr) { /* Use the full FSP version by default. */ - uint32_t ver = hdr->fsp_revision; + uint32_t ver = hdr->image_revision; if (!CONFIG(FSP_PLATFORM_MEMORY_SETTINGS_VERSIONS)) return ver; @@ -291,7 +291,7 @@ post_code(POST_MEM_PREINIT_PREP_END); /* Call FspMemoryInit */ - fsp_raminit = (void *)(uintptr_t)(hdr->image_base + hdr->memory_init_entry_offset); + fsp_raminit = (void *)(uintptr_t)(hdr->image_base + hdr->fsp_memory_init_entry_offset); fsp_debug_before_memory_init(fsp_raminit, upd, &fspm_upd); post_code(POST_FSP_MEMORY_INIT); diff --git a/src/drivers/intel/fsp2_0/silicon_init.c b/src/drivers/intel/fsp2_0/silicon_init.c index 83d44b1..05cea11 100644 --- a/src/drivers/intel/fsp2_0/silicon_init.c +++ b/src/drivers/intel/fsp2_0/silicon_init.c @@ -75,7 +75,7 @@ bool fsp_is_multi_phase_init_enabled(void) { return CONFIG(FSPS_USE_MULTI_PHASE_INIT) && - (fsps_hdr.multi_phase_si_init_entry_offset != 0); + (fsps_hdr.fsp_multi_phase_si_init_entry_offset != 0); } static void fsp_fill_common_arch_params(FSPS_UPD *supd) @@ -127,7 +127,7 @@ /* Call SiliconInit */ silicon_init = (void *) (uintptr_t)(hdr->image_base + - hdr->silicon_init_entry_offset); + hdr->fsp_silicon_init_entry_offset); fsp_debug_before_silicon_init(silicon_init, supd, upd); timestamp_add_now(TS_FSP_SILICON_INIT_START); @@ -162,7 +162,7 @@ /* Call MultiPhaseSiInit */ multi_phase_si_init = (void *) (uintptr_t)(hdr->image_base + - hdr->multi_phase_si_init_entry_offset); + hdr->fsp_multi_phase_si_init_entry_offset); /* Implementing multi_phase_si_init() is optional as per FSP 2.2 spec */ if (multi_phase_si_init == NULL) diff --git a/src/drivers/intel/fsp2_0/temp_ram_exit.c b/src/drivers/intel/fsp2_0/temp_ram_exit.c index 2192543..87b77bc 100644 --- a/src/drivers/intel/fsp2_0/temp_ram_exit.c +++ b/src/drivers/intel/fsp2_0/temp_ram_exit.c @@ -25,7 +25,7 @@ if (fsp_validate_component(&hdr, mapping, size) != CB_SUCCESS) die("Invalid FSPM header!\n"); - temp_ram_exit = (void *)(hdr.image_base + hdr.temp_ram_exit_entry); + temp_ram_exit = (void *)(hdr.image_base + hdr.temp_ram_exit_entry_offset); printk(BIOS_DEBUG, "Calling TempRamExit: %p\n", temp_ram_exit); status = temp_ram_exit(NULL); diff --git a/src/drivers/intel/fsp2_0/util.c b/src/drivers/intel/fsp2_0/util.c index ff79fc7..2537b38 100644 --- a/src/drivers/intel/fsp2_0/util.c +++ b/src/drivers/intel/fsp2_0/util.c @@ -26,11 +26,9 @@ return dead_code_t(uint32_t); } -static bool looks_like_fsp_header(const uint8_t *raw_hdr) +static bool looks_like_fsp_header(struct fsp_header *hdr) { - uint32_t fsp_header_length = read32(raw_hdr + 4); - - if (memcmp(raw_hdr, FSP_HDR_SIGNATURE, 4)) { + if (memcmp(&hdr->signature, FSP_HDR_SIGNATURE, 4)) { printk(BIOS_ALERT, "Did not find a valid FSP signature\n"); return false; } @@ -39,8 +37,8 @@ fields in FSP_INFO_HEADER. The new fields will be ignored based on the reported FSP version. This check ensures that the reported header length is at least what the reported FSP version requires so that we do not access any out-of-bound bytes. */ - if (fsp_header_length < fsp_hdr_get_expected_min_length()) { - printk(BIOS_ALERT, "FSP header has invalid length: %d\n", fsp_header_length); + if (hdr->header_length < fsp_hdr_get_expected_min_length()) { + printk(BIOS_ALERT, "FSP header has invalid length: %d\n", hdr->header_length); return false; } @@ -49,32 +47,10 @@ enum cb_err fsp_identify(struct fsp_header *hdr, const void *fsp_blob) { - const uint8_t *raw_hdr = fsp_blob; - - if (!looks_like_fsp_header(raw_hdr)) + memcpy(hdr, fsp_blob, sizeof(struct fsp_header)); + if (!looks_like_fsp_header(hdr)) return CB_ERR; - hdr->spec_version = read8(raw_hdr + 10); - hdr->revision = read8(raw_hdr + 11); - hdr->fsp_revision = read32(raw_hdr + 12); - memcpy(hdr->image_id, raw_hdr + 16, ARRAY_SIZE(hdr->image_id)); - hdr->image_id[ARRAY_SIZE(hdr->image_id) - 1] = '\0'; - hdr->image_size = read32(raw_hdr + 24); - hdr->image_base = read32(raw_hdr + 28); - hdr->image_attribute = read16(raw_hdr + 32); - hdr->component_attribute = read16(raw_hdr + 34); - hdr->cfg_region_offset = read32(raw_hdr + 36); - hdr->cfg_region_size = read32(raw_hdr + 40); - hdr->temp_ram_init_entry = read32(raw_hdr + 48); - hdr->temp_ram_exit_entry = read32(raw_hdr + 64); - hdr->notify_phase_entry_offset = read32(raw_hdr + 56); - hdr->memory_init_entry_offset = read32(raw_hdr + 60); - hdr->silicon_init_entry_offset = read32(raw_hdr + 68); - if (CONFIG(PLATFORM_USES_FSP2_2)) - hdr->multi_phase_si_init_entry_offset = read32(raw_hdr + 72); - if (CONFIG(PLATFORM_USES_FSP2_3)) - hdr->extended_fsp_revision = read16(raw_hdr + 76); - return CB_SUCCESS; } @@ -192,7 +168,7 @@ struct fsp_header *hdr = &fsps_hdr; union fsp_revision revision; - revision.val = hdr->fsp_revision; + revision.val = hdr->image_revision; snprintf(buf, FSP_VER_LEN, "%u.%u-%u.%u.%u.%u", (hdr->spec_version >> 4), hdr->spec_version & 0xf, revision.rev.major, revision.rev.minor, revision.rev.revision, revision.rev.bld_num); -- To view, visit https://review.coreboot.org/c/coreboot/+/58869 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibf50f16b5e9793d946a95970fcdabc4c07289646 Gerrit-Change-Number: 58869 Gerrit-PatchSet: 10 Gerrit-Owner: Name of user not set #1003801 Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Julian Schroeder <julianmarcusschroeder(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: mb/google/skyrim: Add new mainboard
by Jon Murphy (Code Review) 02 Feb '22

02 Feb '22

Attention is currently required from: Rob Barnes, Karthik Ramasubramanian, Felix Held. Jon Murphy has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61565 ) Change subject: mb/google/skyrim: Add new mainboard ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/61565 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I008fea4aa163b8aa66e86735b29b3fdc4e08a327 Gerrit-Change-Number: 61565 Gerrit-PatchSet: 2 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Rob Barnes <robbarnes(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Rob Barnes <robbarnes(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 02 Feb 2022 21:42:19 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/skyrim: Add new mainboard
by Raul Rangel (Code Review) 02 Feb '22

02 Feb '22

Attention is currently required from: Jon Murphy, Rob Barnes, Karthik Ramasubramanian, Felix Held. Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61565 ) Change subject: mb/google/skyrim: Add new mainboard ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/61565 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I008fea4aa163b8aa66e86735b29b3fdc4e08a327 Gerrit-Change-Number: 61565 Gerrit-PatchSet: 2 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Rob Barnes <robbarnes(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jon Murphy <jpmurphy(a)google.com> Gerrit-Attention: Rob Barnes <robbarnes(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 02 Feb 2022 21:32:23 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2022