Hello Raul Rangel,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/40247
to review the following change.
Change subject: soc/amd/common: Determine # of I2C controllers at runtime ......................................................................
soc/amd/common: Determine # of I2C controllers at runtime
Dali only has 3 I2C controllers, while Pollock has 5. Both boards use the same socket. So we determine at runtime how many I2C controllers are available.
I2C0 and I2C1 are not documented in the PPR, so I took a guess at the addressed.
BUG=b:149572620 BRANCH=none TEST=Booted dalboz firmware on trembyle. Saw only three controllers initialized. Touchscreen was still working.
Change-Id: I397b074ef9c14bf6a4f6680696582f5173a5d0d3 Signed-off-by: Martin Roth martinroth@chromium.org Signed-off-by: Raul E Rangel rrangel@chromium.org Reviewed-on: https://chromium-review.googlesource.com/1897071 Reviewed-on: https://chromium-review.googlesource.com/2057468 --- A src/soc/amd/common/block/include/amdblocks/i2c.h M src/soc/amd/common/block/lpc/lpc.c M src/soc/amd/picasso/Makefile.inc M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/i2c.c M src/soc/amd/picasso/include/soc/iomap.h 6 files changed, 63 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/40247/1
diff --git a/src/soc/amd/common/block/include/amdblocks/i2c.h b/src/soc/amd/common/block/include/amdblocks/i2c.h new file mode 100644 index 0000000..f1ffa71 --- /dev/null +++ b/src/soc/amd/common/block/include/amdblocks/i2c.h @@ -0,0 +1,12 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* This file is part of the coreboot project. */ + +#ifndef __AMDBLOCKS_I2C_H__ +#define __AMDBLOCKS_I2C_H__ + +#include <device/resource.h> + +/* Some systems can only determine the number of i2c controllers at runtime. */ +void soc_update_i2c_resource(struct resource *res); + +#endif /* __AMDBLOCKS_I2C_H__ */ diff --git a/src/soc/amd/common/block/lpc/lpc.c b/src/soc/amd/common/block/lpc/lpc.c index 5346356..e84594b 100644 --- a/src/soc/amd/common/block/lpc/lpc.c +++ b/src/soc/amd/common/block/lpc/lpc.c @@ -16,6 +16,7 @@ #include <pc80/i8259.h> #include <amdblocks/acpimmio.h> #include <amdblocks/lpc.h> +#include <amdblocks/i2c.h> #include <soc/acpi.h> #include <soc/southbridge.h> #include <soc/nvs.h> @@ -24,6 +25,8 @@ /* Most systems should have already enabled the bridge */ void __weak soc_late_lpc_bridge_enable(void) { }
+void __weak soc_update_i2c_resource(struct resource *res) { } + static void lpc_init(struct device *dev) { u8 byte; @@ -118,12 +121,14 @@ res->size = 0x00001000; res->flags = IORESOURCE_MEM | IORESOURCE_ASSIGNED | IORESOURCE_FIXED;
- /* I2C devices (all 4 devices) */ + /* I2C devices */ res = new_resource(dev, 4); res->base = I2C_BASE_ADDRESS; res->size = I2C_DEVICE_SIZE * I2C_DEVICE_COUNT; res->flags = IORESOURCE_MEM | IORESOURCE_ASSIGNED | IORESOURCE_FIXED;
+ soc_update_i2c_resource(res); + compact_resources(dev);
/* Allocate ACPI NVS in CBMEM */ diff --git a/src/soc/amd/picasso/Makefile.inc b/src/soc/amd/picasso/Makefile.inc index 5cef9ba..0ded6dd 100644 --- a/src/soc/amd/picasso/Makefile.inc +++ b/src/soc/amd/picasso/Makefile.inc @@ -42,6 +42,7 @@ bootblock-y += tsc_freq.c bootblock-y += gpio.c bootblock-y += smi_util.c +bootblock-y += soc_util.c
romstage-y += i2c.c romstage-y += romstage.c diff --git a/src/soc/amd/picasso/chip.h b/src/soc/amd/picasso/chip.h index b2ff9d8..adc9160 100644 --- a/src/soc/amd/picasso/chip.h +++ b/src/soc/amd/picasso/chip.h @@ -9,11 +9,10 @@ #include <commonlib/helpers.h> #include <drivers/i2c/designware/dw_i2c.h> #include <soc/i2c.h> +#include <soc/iomap.h> #include <arch/acpi_device.h> #include <FspsUpd.h>
-#define PICASSO_I2C_DEV_MAX 4 - struct soc_amd_picasso_config { /* * If sb_reset_i2c_slaves() is called, this devicetree register @@ -24,7 +23,7 @@ * register i2c_scl_reset = (GPIO_I2C0_SCL | GPIO_I2C3_SCL) */ u8 i2c_scl_reset; - struct dw_i2c_bus_config i2c[PICASSO_I2C_DEV_MAX]; + struct dw_i2c_bus_config i2c[I2C_DEVICE_COUNT]; enum { I2S_PINS_MAX_HDA = 0, /* HDA w/reset 3xSDI, SW w/Data0 */ I2S_PINS_MAX_MHDA = 1, /* HDA no reset 3xSDI, SW w/Data0-1 */ diff --git a/src/soc/amd/picasso/i2c.c b/src/soc/amd/picasso/i2c.c index 6bbc7a7..2bd3116 100644 --- a/src/soc/amd/picasso/i2c.c +++ b/src/soc/amd/picasso/i2c.c @@ -7,34 +7,41 @@ #include <delay.h> #include <drivers/i2c/designware/dw_i2c.h> #include <amdblocks/acpimmio.h> +#include <amdblocks/i2c.h> +#include <soc/i2c.h> #include <soc/iomap.h> #include <soc/pci_devs.h> +#include <soc/soc_util.h> #include <soc/southbridge.h> -#include <soc/i2c.h> #include "chip.h"
/* Global to provide access to chip.c */ const char *i2c_acpi_name(const struct device *dev);
static const uintptr_t i2c_bus_address[] = { + APU_I2C0_BASE, + APU_I2C1_BASE, APU_I2C2_BASE, APU_I2C3_BASE, APU_I2C4_BASE, /* slave device only */ };
+_Static_assert(ARRAY_SIZE(i2c_bus_address) == I2C_DEVICE_COUNT, + "ARRAY_SIZE(i2c_bus_address) must equal I2C_DEVICE_COUNT"); + uintptr_t dw_i2c_base_address(unsigned int bus) { - if (bus < APU_I2C_MIN_BUS || bus > APU_I2C_MAX_BUS) + if (bus >= ARRAY_SIZE(i2c_bus_address)) return 0;
- return i2c_bus_address[bus - APU_I2C_MIN_BUS]; + return i2c_bus_address[bus]; }
const struct dw_i2c_bus_config *dw_i2c_get_soc_cfg(unsigned int bus) { const struct soc_amd_picasso_config *config;
- if (bus < APU_I2C_MIN_BUS || bus > APU_I2C_MAX_BUS) + if (bus >= ARRAY_SIZE(config->i2c)) return NULL;
config = get_soc_config(); @@ -47,6 +54,10 @@ const char *i2c_acpi_name(const struct device *dev) { switch (dev->path.mmio.addr) { + case APU_I2C0_BASE: + return "I2C0"; + case APU_I2C1_BASE: + return "I2C1"; case APU_I2C2_BASE: return "I2C2"; case APU_I2C3_BASE: @@ -61,6 +72,10 @@ int dw_i2c_soc_dev_to_bus(struct device *dev) { switch (dev->path.mmio.addr) { + case APU_I2C0_BASE: + return 0; + case APU_I2C1_BASE: + return 1; case APU_I2C2_BASE: return 2; case APU_I2C3_BASE: @@ -73,6 +88,14 @@
__weak void mainboard_i2c_override(int bus, uint32_t *pad_settings) { }
+/* Pollock has access to I2C0 and I2C1. */ +static unsigned int i2c_start_index(void) +{ + if (soc_is_pollock()) + return 0; + return 2; +} + static void dw_i2c_soc_init(bool is_early_init) { size_t i; @@ -85,7 +108,7 @@ if (config == NULL) return;
- for (i = 0; i < ARRAY_SIZE(config->i2c); i++) { + for (i = i2c_start_index(); i < ARRAY_SIZE(config->i2c); i++) { const struct dw_i2c_bus_config *cfg = &config->i2c[i];
if (cfg->early_init != is_early_init) @@ -126,6 +149,15 @@ dw_i2c_soc_init(false); }
+void soc_update_i2c_resource(struct resource *res) +{ + unsigned int index = i2c_start_index(); + unsigned int count = ARRAY_SIZE(i2c_bus_address) - index; + + res->base = dw_i2c_base_address(index); + res->size = I2C_DEVICE_SIZE * count; +} + struct device_operations picasso_i2c_mmio_ops = { /* TODO(teravest): Move I2C resource info here. */ .read_resources = DEVICE_NOOP, diff --git a/src/soc/amd/picasso/include/soc/iomap.h b/src/soc/amd/picasso/include/soc/iomap.h index 0a6e1a1..3f539a2 100644 --- a/src/soc/amd/picasso/include/soc/iomap.h +++ b/src/soc/amd/picasso/include/soc/iomap.h @@ -18,15 +18,15 @@
/* Reserved 0xfecd1000-0xfedc3fff */
+/* I2C0 and I2C1 are only available on Pollock */ +#define APU_I2C0_BASE 0xfedc2000 +#define APU_I2C1_BASE 0xfedc3000 #define APU_I2C2_BASE 0xfedc4000 #define APU_I2C3_BASE 0xfedc5000 #define APU_I2C4_BASE 0xfedc6000 -#define APU_I2C_MIN_BUS 2 -#define APU_I2C_MAX_BUS 4 -#define APU_I2C_BLOCK_SIZE 0x1000 -#define I2C_BASE_ADDRESS APU_I2C2_BASE +#define I2C_BASE_ADDRESS APU_I2C0_BASE #define I2C_DEVICE_SIZE 0x00001000 -#define I2C_DEVICE_COUNT 3 +#define I2C_DEVICE_COUNT 5
#define APU_DMAC0_BASE 0xfedc7000 #define APU_DMAC1_BASE 0xfedc8000