Change in coreboot[master]: soc/amd/common/blocks/gpio: Introduce gpio_get_bar - coreboot-gerrit

9 Jun 2020

Martin Roth has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42224 )
Change subject: soc/amd/common/blocks/gpio: Introduce gpio_get_bar
......................................................................
soc/amd/common/blocks/gpio: Introduce gpio_get_bar
The PSP has a GPIO BAR mapped to virtual memory, so the address can't be
hard coded. This refactors the code to call gpio_get_bar.
BUG=b:123887623
BRANCH=none
TEST=Boot morphius and saw I2C and TPM working
Original-Signed-off-by: Raul E Rangel rrangel@chromium.org
Original-Reviewed-on:
https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+...
Change-Id: Id24f71bebd2dc9f6243e049c9f387a66b6d1d0c9
Signed-off-by: Martin Roth martin@coreboot.org
---
M src/soc/amd/common/acpi/gpio_bank_lib.asl
M src/soc/amd/common/block/acpimmio/mmio_util.c
M src/soc/amd/common/block/gpio_banks/gpio.c
M src/soc/amd/common/block/include/amdblocks/acpimmio.h
M src/soc/amd/common/block/include/amdblocks/acpimmio_map.h
M src/soc/amd/common/block/include/amdblocks/gpio_banks.h
M src/soc/amd/picasso/acpi.c
M src/soc/amd/picasso/i2c.c
M src/soc/amd/picasso/include/soc/i2c.h
M src/soc/amd/stoneyridge/i2c.c
M src/soc/amd/stoneyridge/include/soc/i2c.h
11 files changed, 129 insertions(+), 201 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/42224/1

diff --git a/src/soc/amd/common/acpi/gpio_bank_lib.asl b/src/soc/amd/common/acpi/gpio_bank_lib.asl
index f73340c..dc2da6a 100644
--- a/src/soc/amd/common/acpi/gpio_bank_lib.asl
+++ b/src/soc/amd/common/acpi/gpio_bank_lib.asl
@@ -6,7 +6,7 @@
 Method (GPAD, 0x1)
 {
    /* Arg0 - GPIO pin number */
-	Return (Add(Multiply(Arg0, 4), ACPIMMIO_GPIO0_BASE))
+	Return (Add(Multiply(Arg0, 4), ACPIMMIO_GPIO_BASE))
 }
/* Read pin control dword */
diff --git a/src/soc/amd/common/block/acpimmio/mmio_util.c b/src/soc/amd/common/block/acpimmio/mmio_util.c
index 7126851..48208ca 100644
--- a/src/soc/amd/common/block/acpimmio/mmio_util.c
+++ b/src/soc/amd/common/block/acpimmio/mmio_util.c
@@ -59,3 +59,8 @@
    value >>= 16;
    pm_io_write16(reg + sizeof(uint16_t), value & 0xffff);
 }
+
+void *gpio_get_bar(void)
+{
+	return (void *)ACPIMMIO_GPIO_BASE;
+}
diff --git a/src/soc/amd/common/block/gpio_banks/gpio.c b/src/soc/amd/common/block/gpio_banks/gpio.c
index 81ea725..872cf1c 100644
--- a/src/soc/amd/common/block/gpio_banks/gpio.c
+++ b/src/soc/amd/common/block/gpio_banks/gpio.c
@@ -81,82 +81,63 @@
    }
 }
-uintptr_t gpio_get_address(gpio_t gpio_num)
+static uint16_t gpio_get_offset(gpio_t gpio_num)
 {
-	uintptr_t gpio_address;
+	uint16_t block_offset = GPIO_BLOCK_SIZE * (gpio_num / GPIO_PINS_PER_BLOCK);
+	uint16_t pin_offset = (gpio_num % GPIO_PINS_PER_BLOCK) * sizeof(uint32_t);
-	if (gpio_num < 64)
-		gpio_address = GPIO_BANK0_CONTROL(gpio_num);
-	else if (gpio_num < 128)
-		gpio_address = GPIO_BANK1_CONTROL(gpio_num);
-	else
-		gpio_address = GPIO_BANK2_CONTROL(gpio_num);
+	return block_offset + pin_offset;
+}
-	return gpio_address;
+uint32_t *gpio_get_address(gpio_t gpio_num)
+{
+	uintptr_t bar = (uintptr_t)gpio_get_bar();
+	bar += gpio_get_offset(gpio_num);
+
+	return (uint32_t *)bar;
 }
int gpio_get(gpio_t gpio_num)
 {
-	uint32_t reg;
-	uintptr_t gpio_address = gpio_get_address(gpio_num);
-
-	reg = read32((void *)gpio_address);
+	uint32_t reg = read32(gpio_get_address(gpio_num));
return !!(reg & GPIO_PIN_STS);
 }
void gpio_set(gpio_t gpio_num, int value)
 {
-	uint32_t reg;
-	uintptr_t gpio_address = gpio_get_address(gpio_num);
-
-	reg = read32((void *)gpio_address);
-	reg &= ~GPIO_OUTPUT_MASK;
-	reg |=  !!value << GPIO_OUTPUT_SHIFT;
-	write32((void *)gpio_address, reg);
+	mem_read_write32(gpio_get_address(gpio_num),
+			 !!value << GPIO_OUTPUT_VALUE_SHIFT,
+			 GPIO_OUTPUT_VALUE_MASK);
 }
void gpio_input_pulldown(gpio_t gpio_num)
 {
-	uint32_t reg;
-	uintptr_t gpio_address = gpio_get_address(gpio_num);
-
-	reg = read32((void *)gpio_address);
-	reg &= ~GPIO_PULLUP_ENABLE;
-	reg |=  GPIO_PULLDOWN_ENABLE;
-	write32((void *)gpio_address, reg);
+	mem_read_write32(gpio_get_address(gpio_num),
+			 GPIO_PULLDOWN_ENABLE,
+			 GPIO_PULL_DIR_MASK);
 }
void gpio_input_pullup(gpio_t gpio_num)
 {
-	uint32_t reg;
-	uintptr_t gpio_address = gpio_get_address(gpio_num);
-
-	reg = read32((void *)gpio_address);
-	reg &= ~GPIO_PULLDOWN_ENABLE;
-	reg |=  GPIO_PULLUP_ENABLE;
-	write32((void *)gpio_address, reg);
+	mem_read_write32(gpio_get_address(gpio_num),
+			 GPIO_PULLUP_ENABLE,
+			 GPIO_PULL_DIR_MASK);
 }
void gpio_input(gpio_t gpio_num)
 {
-	uint32_t reg;
-	uintptr_t gpio_address = gpio_get_address(gpio_num);
-
-	reg = read32((void *)gpio_address);
-	reg &= ~GPIO_OUTPUT_ENABLE;
-	write32((void *)gpio_address, reg);
+	mem_read_write32(gpio_get_address(gpio_num),
+			 GPIO_INPUT_ENABLE,
+			 GPIO_DIR_MASK);
 }
void gpio_output(gpio_t gpio_num, int value)
 {
-	uint32_t reg;
-	uintptr_t gpio_address = gpio_get_address(gpio_num);
-
-	reg = read32((void *)gpio_address);
-	reg |=  GPIO_OUTPUT_ENABLE;
-	write32((void *)gpio_address, reg);
-	gpio_set(gpio_num, value);
+	mem_read_write32(gpio_get_address(gpio_num),
+			 GPIO_OUTPUT_ENABLE
+				 | !!value << GPIO_OUTPUT_VALUE_SHIFT,
+			 GPIO_OUTPUT_VALUE_MASK | GPIO_DIR_MASK);
 }
const char *gpio_acpi_path(gpio_t gpio)
@@ -169,6 +150,17 @@
    return gpio;
 }
+static int gevent_missing(uint8_t gpio, int gevent_num, const char *string)
+{
+	if (gevent_num < 0) {
+		printk(BIOS_WARNING, "Warning: GPIO pin %d has"
+			" no associated gevent!\n", gpio);
+		printk (BIOS_WARNING, "Not programming %s\n", string);
+		return 1;
+	}
+	return 0;
+}
+
 __weak void soc_gpio_hook(uint8_t gpio, uint8_t mux) {}
void program_gpios(const struct soc_amd_gpio *gpio_list_ptr, size_t size)
@@ -179,10 +171,11 @@
    uint8_t mux, index, gpio;
    int gevent_num;
    const struct soc_amd_event *gev_tbl;
-	size_t gev_items;
+	size_t gev_items = 0;
+	const bool can_set_smi_flags = !CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK) || !ENV_SEPARATE_VERSTAGE;
-	inter_master = (uint32_t *)(uintptr_t)(ACPIMMIO_GPIO0_BASE
-					       + GPIO_MASTER_SWITCH);
+	inter_master = (uint32_t *)((uintptr_t)gpio_get_bar() + GPIO_MASTER_SWITCH);
+
    direction = 0;
    edge_level = 0;
    mask = 0;
@@ -199,7 +192,9 @@
     */
    mem_read_write32(inter_master, 0, GPIO_MASK_STS_EN | GPIO_INTERRUPT_EN);
-	soc_get_gpio_event_table(&gev_tbl, &gev_items);
+	if (can_set_smi_flags) {
+		soc_get_gpio_event_table(&gev_tbl, &gev_items);
+	}
for (index = 0; index < size; index++) {
    	gpio = gpio_list_ptr[index].gpio;
@@ -212,15 +207,10 @@
soc_gpio_hook(gpio, mux);
-		gpio_ptr = (uint32_t *)gpio_get_address(gpio);
+		gpio_ptr = gpio_get_address(gpio);
if (control_flags & GPIO_SPECIAL_FLAG) {
    		gevent_num = get_gpio_gevent(gpio, gev_tbl, gev_items);
-			if (gevent_num < 0) {
-				printk(BIOS_WARNING, "Warning: GPIO pin %d has"
-					" no associated gevent!\n", gpio);
-				continue;
-			}
    		switch (control_flags & GPIO_SPECIAL_MASK) {
    		case GPIO_DEBOUNCE_FLAG:
    			mem_read_write32(gpio_ptr, control,
@@ -235,11 +225,23 @@
    					AMD_GPIO_CONTROL_MASK);
    			break;
    		case GPIO_SMI_FLAG:
+				/* Can't set SMI flags from PSP */
+				if (!can_set_smi_flags)
+					break;
+
+				if (gevent_missing(gpio, gevent_num, "SMI Flag"))
+					break;
    			mem_read_write32(gpio_ptr, control,
    					INT_SCI_SMI_MASK);
    			program_smi(control_flags, gevent_num);
    			break;
    		case GPIO_SCI_FLAG:
+				/* Can't set SCI flags from PSP */
+				if (!can_set_smi_flags)
+					break;
+
+				if (gevent_missing(gpio, gevent_num, "SCI Flag"))
+					break;
    			mem_read_write32(gpio_ptr, control,
    					INT_SCI_SMI_MASK);
    			get_sci_config_bits(control_flags, &bit_edge,
@@ -269,26 +271,28 @@
     */
    mem_read_write32(inter_master, GPIO_INTERRUPT_EN, GPIO_INTERRUPT_EN);
-	/* Set all SCI trigger direction (high/low) */
-	mem_read_write32((uint32_t *)
-			(uintptr_t)(ACPIMMIO_SMI_BASE + SMI_SCI_TRIG),
-					direction, mask);
+	if (can_set_smi_flags) {
+		/* Set all SCI trigger direction (high/low) */
+		mem_read_write32((uint32_t *)
+				(uintptr_t)(ACPIMMIO_SMI_BASE + SMI_SCI_TRIG),
+						direction, mask);
-	/* Set all SCI trigger level (edge/level) */
-	mem_read_write32((uint32_t *)
-			(uintptr_t)(ACPIMMIO_SMI_BASE + SMI_SCI_LEVEL),
-					edge_level, mask);
+		/* Set all SCI trigger level (edge/level) */
+		mem_read_write32((uint32_t *)
+				(uintptr_t)(ACPIMMIO_SMI_BASE + SMI_SCI_LEVEL),
+						edge_level, mask);
+	}
 }
int gpio_interrupt_status(gpio_t gpio)
 {
-	uintptr_t gpio_address = gpio_get_address(gpio);
-	uint32_t reg = read32((void *)gpio_address);
+	uint32_t *gpio_ptr = gpio_get_address(gpio);
+	uint32_t reg = read32(gpio_ptr);
if (reg & GPIO_INT_STATUS) {
    	/* Clear interrupt status, preserve wake status */
    	reg &= ~GPIO_WAKE_STATUS;
-		write32((void *)gpio_address, reg);
+		write32(gpio_ptr, reg);
    	return 1;
    }
diff --git a/src/soc/amd/common/block/include/amdblocks/acpimmio.h b/src/soc/amd/common/block/include/amdblocks/acpimmio.h
index a0ab615..fb5d743 100644
--- a/src/soc/amd/common/block/include/amdblocks/acpimmio.h
+++ b/src/soc/amd/common/block/include/amdblocks/acpimmio.h
@@ -413,100 +413,6 @@
    write32((void *)(ACPIMMIO_GPIO_BASE_100 + reg), value);
 }
-/* New GPIO banks configuration registers */
-/* GPIO bank 0 */
-static inline uint8_t gpio0_read8(uint8_t reg)
-{
-	return read8((void *)(ACPIMMIO_GPIO0_BASE + reg));
-}
-
-static inline uint16_t gpio0_read16(uint8_t reg)
-{
-	return read16((void *)(ACPIMMIO_GPIO0_BASE + reg));
-}
-
-static inline uint32_t gpio0_read32(uint8_t reg)
-{
-	return read32((void *)(ACPIMMIO_GPIO0_BASE + reg));
-}
-
-static inline void gpio0_write8(uint8_t reg, uint8_t value)
-{
-	write8((void *)(ACPIMMIO_GPIO0_BASE + reg), value);
-}
-
-static inline void gpio0_write16(uint8_t reg, uint16_t value)
-{
-	write16((void *)(ACPIMMIO_GPIO0_BASE + reg), value);
-}
-
-static inline void gpio0_write32(uint8_t reg, uint32_t value)
-{
-	write32((void *)(ACPIMMIO_GPIO0_BASE + reg), value);
-}
-
-/* GPIO bank 1 */
-static inline uint8_t gpio1_read8(uint8_t reg)
-{
-	return read8((void *)(ACPIMMIO_GPIO1_BASE + reg));
-}
-
-static inline uint16_t gpio1_read16(uint8_t reg)
-{
-	return read16((void *)(ACPIMMIO_GPIO1_BASE + reg));
-}
-
-static inline uint32_t gpio1_read32(uint8_t reg)
-{
-	return read32((void *)(ACPIMMIO_GPIO1_BASE + reg));
-}
-
-static inline void gpio1_write8(uint8_t reg, uint8_t value)
-{
-	write8((void *)(ACPIMMIO_GPIO1_BASE + reg), value);
-}
-
-static inline void gpio1_write16(uint8_t reg, uint16_t value)
-{
-	write16((void *)(ACPIMMIO_GPIO1_BASE + reg), value);
-}
-
-static inline void gpio1_write32(uint8_t reg, uint32_t value)
-{
-	write32((void *)(ACPIMMIO_GPIO1_BASE + reg), value);
-}
-
-/* GPIO bank 2 */
-static inline uint8_t gpio2_read8(uint8_t reg)
-{
-	return read8((void *)(ACPIMMIO_GPIO2_BASE + reg));
-}
-
-static inline uint16_t gpio2_read16(uint8_t reg)
-{
-	return read16((void *)(ACPIMMIO_GPIO2_BASE + reg));
-}
-
-static inline uint32_t gpio2_read32(uint8_t reg)
-{
-	return read32((void *)(ACPIMMIO_GPIO2_BASE + reg));
-}
-
-static inline void gpio2_write8(uint8_t reg, uint8_t value)
-{
-	write8((void *)(ACPIMMIO_GPIO2_BASE + reg), value);
-}
-
-static inline void gpio2_write16(uint8_t reg, uint16_t value)
-{
-	write16((void *)(ACPIMMIO_GPIO2_BASE + reg), value);
-}
-
-static inline void gpio2_write32(uint8_t reg, uint32_t value)
-{
-	write32((void *)(ACPIMMIO_GPIO2_BASE + reg), value);
-}
-
 static inline uint8_t xhci_pm_read8(uint8_t reg)
 {
    return read8((void *)(ACPIMMIO_XHCIPM_BASE + reg));
diff --git a/src/soc/amd/common/block/include/amdblocks/acpimmio_map.h b/src/soc/amd/common/block/include/amdblocks/acpimmio_map.h
index 9e44a85..c05c810 100644
--- a/src/soc/amd/common/block/include/amdblocks/acpimmio_map.h
+++ b/src/soc/amd/common/block/include/amdblocks/acpimmio_map.h
@@ -48,9 +48,7 @@
 #define ACPIMMIO_IOMUX_BASE		0xfed80d00
 #define ACPIMMIO_MISC_BASE		0xfed80e00
 #define ACPIMMIO_DPVGA_BASE		0xfed81400
-#define ACPIMMIO_GPIO0_BASE		0xfed81500
-#define ACPIMMIO_GPIO1_BASE		0xfed81600
-#define ACPIMMIO_GPIO2_BASE		0xfed81700
+#define ACPIMMIO_GPIO_BASE		0xfed81500
 #define ACPIMMIO_XHCIPM_BASE		0xfed81c00
 #define ACPIMMIO_ACDCTMR_BASE		0xfed81d00
 #define ACPIMMIO_AOAC_BASE		0xfed81e00
diff --git a/src/soc/amd/common/block/include/amdblocks/gpio_banks.h b/src/soc/amd/common/block/include/amdblocks/gpio_banks.h
index 572e639..9555856 100644
--- a/src/soc/amd/common/block/include/amdblocks/gpio_banks.h
+++ b/src/soc/amd/common/block/include/amdblocks/gpio_banks.h
@@ -19,9 +19,8 @@
    uint8_t event;
 };
-#define GPIO_BANK0_CONTROL(gpio) (ACPIMMIO_GPIO0_BASE + ((gpio) * 4))
-#define GPIO_BANK1_CONTROL(gpio) (ACPIMMIO_GPIO1_BASE + (((gpio) - 64) * 4))
-#define GPIO_BANK2_CONTROL(gpio) (ACPIMMIO_GPIO2_BASE + (((gpio) - 128) * 4))
+#define GPIO_BLOCK_SIZE		0x100
+#define GPIO_PINS_PER_BLOCK	64
#define GPIO_MASTER_SWITCH	0xFC
 #define   GPIO_MASK_STS_EN	BIT(28)
@@ -50,6 +49,14 @@
 #define GPIO_8KPULLUP_SELECT	(1 << 19)
 #define GPIO_PULLUP_ENABLE	(1 << 20)
 #define GPIO_PULLDOWN_ENABLE	(1 << 21)
+
+#define GPIO_PULL_DIR_MASK	(GPIO_PULLUP_ENABLE | GPIO_PULLDOWN_ENABLE)
+#define GPIO_OUTPUT_VALUE_SHIFT	22
+#define GPIO_OUTPUT_VALUE_MASK	(1 << GPIO_OUTPUT_VALUE_SHIFT)
+#define GPIO_OUTPUT_ENABLE	(1 << 23)
+#define GPIO_INPUT_ENABLE	0
+#define GPIO_DIR_MASK		(GPIO_OUTPUT_ENABLE | GPIO_INPUT_ENABLE)
+
 #define GPIO_OUTPUT_SHIFT	22
 #define GPIO_OUTPUT_MASK	(1 << GPIO_OUTPUT_SHIFT)
 #define GPIO_OUTPUT_VALUE	(1 << GPIO_OUTPUT_SHIFT)
@@ -93,8 +100,8 @@
    GEVENT_31,
 };
-#define GPIO_OUTPUT_OUT_HIGH (GPIO_OUTPUT_ENABLE | GPIO_OUTPUT_VALUE)
-#define GPIO_OUTPUT_OUT_LOW GPIO_OUTPUT_ENABLE
+#define GPIO_OUTPUT_OUT_HIGH	(GPIO_OUTPUT_ENABLE | (1 << GPIO_OUTPUT_VALUE_SHIFT))
+#define GPIO_OUTPUT_OUT_LOW	(GPIO_OUTPUT_ENABLE | (0 << GPIO_OUTPUT_VALUE_SHIFT))
#define GPIO_PULL_PULL_UP_8K (GPIO_PULLUP_ENABLE | GPIO_8KPULLUP_SELECT)
 #define GPIO_PULL_PULL_UP GPIO_PULLUP_ENABLE
@@ -291,8 +298,10 @@
    				const struct soc_amd_gpio *override_cfg,
    				size_t override_num_pads);
-/* Get the address of the control register of a particular pin */
-uintptr_t gpio_get_address(gpio_t gpio_num);
+/*
+ * Gets the raw memory address of the control register of a particular pin.
+ */
+uint32_t *gpio_get_address(gpio_t gpio_num);
/**
  * @brief program a particular set of GPIO
diff --git a/src/soc/amd/picasso/acpi.c b/src/soc/amd/picasso/acpi.c
index 817ec51..4cad929 100644
--- a/src/soc/amd/picasso/acpi.c
+++ b/src/soc/amd/picasso/acpi.c
@@ -322,7 +322,7 @@
    				" %d\n", gpio_num, SOC_GPIO_TOTAL_PINS);
    	return -1;
    }
-	uintptr_t addr = (uintptr_t) gpio_get_address(gpio_num);
+	uintptr_t addr = (uintptr_t)gpio_get_address(gpio_num);
acpigen_soc_get_gpio_in_local5(addr);
@@ -352,7 +352,7 @@
    				" %d\n", gpio_num, SOC_GPIO_TOTAL_PINS);
    	return -1;
    }
-	uintptr_t addr = (uintptr_t) gpio_get_address(gpio_num);
+	uintptr_t addr = (uintptr_t)gpio_get_address(gpio_num);
/* Store (0x40, Local0) */
    acpigen_write_store();
diff --git a/src/soc/amd/picasso/i2c.c b/src/soc/amd/picasso/i2c.c
index fa59b99..af17718 100644
--- a/src/soc/amd/picasso/i2c.c
+++ b/src/soc/amd/picasso/i2c.c
@@ -156,7 +156,7 @@
 {
    uint32_t *gpio_ptr;
-	gpio_ptr = (uint32_t *)gpio_get_address(gpio);
+	gpio_ptr = gpio_get_address(gpio);
    save_table->mux_value = iomux_read8(gpio);
    save_table->control_value = read32(gpio_ptr);
 }
@@ -166,7 +166,7 @@
 {
    uint32_t *gpio_ptr;
-	gpio_ptr = (uint32_t *)gpio_get_address(gpio);
+	gpio_ptr = gpio_get_address(gpio);
    iomux_write8(gpio, save_table->mux_value);
    iomux_read8(gpio);
    write32(gpio_ptr, save_table->control_value);
@@ -180,6 +180,13 @@
    const struct device *dev = pcidev_path_on_root(GNB_DEVFN);
    struct soc_amd_i2c_save save_table[saved_pins_count];
    uint8_t i, j, control;
+	/* I2C0-1 is not accessible from the x86. */
+	uint32_t *i2c_gpio_ptr[] = {
+		NULL,
+		NULL,
+		gpio_get_address(I2C2_SCL_PIN),
+		gpio_get_address(I2C3_SCL_PIN),
+	};
if (!dev || !dev->chip_info)
    	return;
@@ -199,19 +206,19 @@
     */
    for (j = 0; j < 9; j++) {
    	if (control & GPIO_I2C2_SCL)
-			write32((uint32_t *)GPIO_I2C2_ADDRESS, GPIO_SCL_LOW);
+			write32(i2c_gpio_ptr[2], GPIO_SCL_LOW);
    	if (control & GPIO_I2C3_SCL)
-			write32((uint32_t *)GPIO_I2C3_ADDRESS, GPIO_SCL_LOW);
+			write32(i2c_gpio_ptr[3], GPIO_SCL_LOW);
-		read32((uint32_t *)GPIO_I2C3_ADDRESS); /* Flush posted write */
+		read32(i2c_gpio_ptr[3]); /* Flush posted write */
    	udelay(4); /* 4usec gets 85KHz for 1 pin, 70KHz for 4 pins */
if (control & GPIO_I2C2_SCL)
-			write32((uint32_t *)GPIO_I2C2_ADDRESS, GPIO_SCL_HIGH);
+			write32(i2c_gpio_ptr[2], GPIO_SCL_HIGH);
    	if (control & GPIO_I2C3_SCL)
-			write32((uint32_t *)GPIO_I2C3_ADDRESS, GPIO_SCL_HIGH);
+			write32(i2c_gpio_ptr[3], GPIO_SCL_HIGH);
-		read32((uint32_t *)GPIO_I2C3_ADDRESS); /* Flush posted write */
+		read32(i2c_gpio_ptr[3]); /* Flush posted write */
    	udelay(4);
    }
diff --git a/src/soc/amd/picasso/include/soc/i2c.h b/src/soc/amd/picasso/include/soc/i2c.h
index 58c27a4..4b63712 100644
--- a/src/soc/amd/picasso/include/soc/i2c.h
+++ b/src/soc/amd/picasso/include/soc/i2c.h
@@ -17,8 +17,6 @@
 #define I2C2_SCL_PIN			GPIO_113
 #define I2C3_SCL_PIN			GPIO_19
-#define GPIO_I2C2_ADDRESS		GPIO_BANK1_CONTROL(I2C2_SCL_PIN)
-#define GPIO_I2C3_ADDRESS		GPIO_BANK0_CONTROL(I2C3_SCL_PIN)
 #define GPIO_SCL_HIGH			0
 #define GPIO_SCL_LOW			GPIO_OUTPUT_ENABLE
diff --git a/src/soc/amd/stoneyridge/i2c.c b/src/soc/amd/stoneyridge/i2c.c
index 8667d92..082084e 100644
--- a/src/soc/amd/stoneyridge/i2c.c
+++ b/src/soc/amd/stoneyridge/i2c.c
@@ -149,7 +149,7 @@
 {
    uint32_t *gpio_ptr;
-	gpio_ptr = (uint32_t *)gpio_get_address(gpio);
+	gpio_ptr = gpio_get_address(gpio);
    iomux_write8(gpio, save_table->mux_value);
    iomux_read8(gpio);
    write32(gpio_ptr, save_table->control_value);
@@ -163,6 +163,12 @@
    const struct device *dev = pcidev_path_on_root(GNB_DEVFN);
    struct soc_amd_i2c_save save_table[saved_pins_count];
    uint8_t i, j, control;
+	void *i2c_gpio_ptr[] = {
+		gpio_get_address(I2C0_SCL_PIN),
+		gpio_get_address(I2C1_SCL_PIN),
+		gpio_get_address(I2C2_SCL_PIN),
+		gpio_get_address(I2C3_SCL_PIN),
+	};
if (!dev || !dev->chip_info)
    	return;
@@ -182,27 +188,27 @@
     */
    for (j = 0; j < 9; j++) {
    	if (control & GPIO_I2C0_SCL)
-			write32((uint32_t *)GPIO_I2C0_ADDRESS, GPIO_SCL_LOW);
+			write32(i2c_gpio_ptr[0], GPIO_SCL_LOW);
    	if (control & GPIO_I2C1_SCL)
-			write32((uint32_t *)GPIO_I2C1_ADDRESS, GPIO_SCL_LOW);
+			write32(i2c_gpio_ptr[1], GPIO_SCL_LOW);
    	if (control & GPIO_I2C2_SCL)
-			write32((uint32_t *)GPIO_I2C2_ADDRESS, GPIO_SCL_LOW);
+			write32(i2c_gpio_ptr[2], GPIO_SCL_LOW);
    	if (control & GPIO_I2C3_SCL)
-			write32((uint32_t *)GPIO_I2C3_ADDRESS, GPIO_SCL_LOW);
+			write32(i2c_gpio_ptr[3], GPIO_SCL_LOW);
-		read32((uint32_t *)GPIO_I2C3_ADDRESS); /* Flush posted write */
+		read32(i2c_gpio_ptr[3]); /* Flush posted write */
    	udelay(4); /* 4usec gets 85KHz for 1 pin, 70KHz for 4 pins */
if (control & GPIO_I2C0_SCL)
-			write32((uint32_t *)GPIO_I2C0_ADDRESS, GPIO_SCL_HIGH);
+			write32(i2c_gpio_ptr[0], GPIO_SCL_HIGH);
    	if (control & GPIO_I2C1_SCL)
-			write32((uint32_t *)GPIO_I2C1_ADDRESS, GPIO_SCL_HIGH);
+			write32(i2c_gpio_ptr[1], GPIO_SCL_HIGH);
    	if (control & GPIO_I2C2_SCL)
-			write32((uint32_t *)GPIO_I2C2_ADDRESS, GPIO_SCL_HIGH);
+			write32(i2c_gpio_ptr[2], GPIO_SCL_HIGH);
    	if (control & GPIO_I2C3_SCL)
-			write32((uint32_t *)GPIO_I2C3_ADDRESS, GPIO_SCL_HIGH);
+			write32(i2c_gpio_ptr[3], GPIO_SCL_HIGH);
-		read32((uint32_t *)GPIO_I2C3_ADDRESS); /* Flush posted write */
+		read32(i2c_gpio_ptr[3]); /* Flush posted write */
    	udelay(4);
    }
diff --git a/src/soc/amd/stoneyridge/include/soc/i2c.h b/src/soc/amd/stoneyridge/include/soc/i2c.h
index 874f7d1..844ff1b 100644
--- a/src/soc/amd/stoneyridge/include/soc/i2c.h
+++ b/src/soc/amd/stoneyridge/include/soc/i2c.h
@@ -21,11 +21,6 @@
 #define I2C2_SCL_PIN			GPIO_113
 #define I2C3_SCL_PIN			GPIO_19
-#define GPIO_I2C0_ADDRESS		GPIO_BANK2_CONTROL(I2C0_SCL_PIN)
-#define GPIO_I2C1_ADDRESS		GPIO_BANK2_CONTROL(I2C1_SCL_PIN)
-#define GPIO_I2C2_ADDRESS		GPIO_BANK1_CONTROL(I2C2_SCL_PIN)
-#define GPIO_I2C3_ADDRESS		GPIO_BANK0_CONTROL(I2C3_SCL_PIN)
-
 #define I2C0_SCL_PIN_IOMUX_GPIOxx	GPIO_145_IOMUX_GPIOxx
 #define I2C1_SCL_PIN_IOMUX_GPIOxx	GPIO_147_IOMUX_GPIOxx
 #define I2C2_SCL_PIN_IOMUX_GPIOxx	GPIO_113_IOMUX_GPIOxx
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/42224
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Id24f71bebd2dc9f6243e049c9f387a66b6d1d0c9
Gerrit-Change-Number: 42224
Gerrit-PatchSet: 1
Gerrit-Owner: Martin Roth martinroth@google.com
Gerrit-MessageType: newchange



    