Martin Roth submitted this change.

View Change

Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Raul Rangel: Looks good to me, but someone else must approve Angel Pons: Looks good to me, approved
soc/amd/picasso/i2c: don't initialize I2C4 as master and refactor code

I2C0&1 are either not available or not functional. Add place holders
instead, so that the array index matches the I2C controller number. I2C4
is slave device only, so do not initialize it as I2C host controller.
Also do some slight refactoring.

BUG=b:153152871
BUG=b:153675916

Change-Id: I397b074ef9c14bf6a4f6680696582f5173a5d0d3
Signed-off-by: Martin Roth <martinroth@chromium.org>
Signed-off-by: Paul Ma <magf@bitland.corp-partner.google.com>
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
Reviewed-on: https://chromium-review.googlesource.com/2094855
Reviewed-on: https://chromium-review.googlesource.com/2149870
Reviewed-on: https://review.coreboot.org/c/coreboot/+/40247
Reviewed-by: Furquan Shaikh <furquan@google.com>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
---
M src/soc/amd/common/block/lpc/lpc.c
M src/soc/amd/picasso/chip.h
M src/soc/amd/picasso/i2c.c
M src/soc/amd/picasso/include/soc/iomap.h
4 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/src/soc/amd/common/block/lpc/lpc.c b/src/soc/amd/common/block/lpc/lpc.c
index 5346356..3df5ad0 100644
--- a/src/soc/amd/common/block/lpc/lpc.c
+++ b/src/soc/amd/common/block/lpc/lpc.c
@@ -118,7 +118,7 @@
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;
diff --git a/src/soc/amd/picasso/chip.h b/src/soc/amd/picasso/chip.h
index 53c0329..9c756ed 100644
--- a/src/soc/amd/picasso/chip.h
+++ b/src/soc/amd/picasso/chip.h
@@ -9,10 +9,9 @@
#include <commonlib/helpers.h>
#include <drivers/i2c/designware/dw_i2c.h>
#include <soc/i2c.h>
+#include <soc/iomap.h>
#include <arch/acpi_device.h>

-#define PICASSO_I2C_DEV_MAX 4
-
struct soc_amd_picasso_config {
/*
* If sb_reset_i2c_slaves() is called, this devicetree register
@@ -23,7 +22,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_MASTER_DEV_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 22c6216..8ba05aa 100644
--- a/src/soc/amd/picasso/i2c.c
+++ b/src/soc/amd/picasso/i2c.c
@@ -8,34 +8,39 @@
#include <device/device.h>
#include <drivers/i2c/designware/dw_i2c.h>
#include <amdblocks/acpimmio.h>
+#include <soc/i2c.h>
#include <soc/iomap.h>
#include <soc/pci_devs.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[] = {
+/*
+ * We don't have addresses for I2C0-1.
+ */
+static const uintptr_t i2c_bus_address[I2C_MASTER_DEV_COUNT + I2C_SLAVE_DEV_COUNT] = {
+ 0,
+ 0,
APU_I2C2_BASE,
APU_I2C3_BASE,
- APU_I2C4_BASE, /* slave device only */
+ APU_I2C4_BASE, /* Can only be used in slave mode */
};

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 is not NULL; if it was, config_of_soc calls die() internally */
@@ -83,7 +88,7 @@
/* config is not NULL; if it was, config_of_soc calls die() internally */
config = config_of_soc();

- for (i = 0; i < ARRAY_SIZE(config->i2c); i++) {
+ for (i = I2C_MASTER_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)
diff --git a/src/soc/amd/picasso/include/soc/iomap.h b/src/soc/amd/picasso/include/soc/iomap.h
index 1ff3440..5e1e6e2 100644
--- a/src/soc/amd/picasso/include/soc/iomap.h
+++ b/src/soc/amd/picasso/include/soc/iomap.h
@@ -18,15 +18,29 @@

/* Reserved 0xfecd1000-0xfedc3fff */

+/*
+ * Picasso/Dali have I2C0 and I2C1 wired to the Sensor Fusion Hub (SFH/MP2).
+ * The controllers are not directly accessible via the x86.
+ *
+ * On Pollock, I2C0 and I2C1 are routed to the x86 domain, but unfortunately the
+ * interrupts weren't. This effectively makes the I2C controllers useless, so we
+ * pretend they don't exist.
+ *
+ * We want the device tree numbering to match the I2C numbers, so we allocate
+ * I2C0 and I2C1 even though they are not functional.
+ */
+#define I2C_MASTER_DEV_COUNT 4
+#define I2C_MASTER_START_INDEX 2
+#define I2C_SLAVE_DEV_COUNT 1
+
#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_DEVICE_SIZE 0x00001000
-#define I2C_DEVICE_COUNT 3
+
+/* I2C parameters for lpc_read_resources */
+#define I2C_BASE_ADDRESS APU_I2C2_BASE
+#define I2C_DEVICE_SIZE 0x00001000
+#define I2C_DEVICE_COUNT 3

#define APU_DMAC0_BASE 0xfedc7000
#define APU_DMAC1_BASE 0xfedc8000

To view, visit change 40247. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I397b074ef9c14bf6a4f6680696582f5173a5d0d3
Gerrit-Change-Number: 40247
Gerrit-PatchSet: 8
Gerrit-Owner: Felix Held <felix-coreboot@felixheld.de>
Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de>
Gerrit-Reviewer: Furquan Shaikh <furquan@google.com>
Gerrit-Reviewer: Magf - <magf@bitland.corp-partner.google.com>
Gerrit-Reviewer: Martin Roth <martinroth@google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-Reviewer: Raul Rangel <rrangel@chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-MessageType: merged