soc/amd/gpio, mb/{amd,google}: Configure pads using a single entry in GPIO configuration table

Currently, for Stoneyridge and Picasso mainboards, pads that are
configured for SCI/SMI/WAKE need to have multiple entries in the
configuration table - one for PAD_GPI and other for the special
configuration that is required. This requires a very specific ordering
of pads within the table and is prone to errors because of conflicting
params provided to the different entries for the same pad. This also
does not work very well with the concept of override GPIOs where the
entry in base table is overridden with the first matched entry from
the override table.

This change updates the way GPIO configuration is handled for special
routing like SCI/SMI/WAKE/DEBOUNCE by setting the control field of
soc_amd_gpio structure in the macros performing these
configurations. Also, program_gpios() is updated to perform a write to
GPIO control register instead of read-modify-write. This is because
mainboard is expected to provide only a single configuration entry for
each pad within a given table. Thus, there is no need to preserve
earlier configuration.

Mainboards that were providing multiple entries for a single pad are
updated accordingly.

BUG=b:159944426

Change-Id: I3364dc2982d66c4e33c2b4e6b0b97641ebea27f0
Signed-off-by: Furquan Shaikh <furquan@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/42875
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>

4 files changed, 46 insertions, 87 deletions

diff --git a/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c b/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c
index ba50362685..5874e89c06 100644
--- a/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c
+++ b/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c
@@ -13,7 +13,6 @@ static const struct soc_amd_gpio gpio_set_stage_rom_pre_v3[] = {
 	/* PEN_POWER_EN - reset */
 	PAD_GPO(GPIO_5, LOW),
 	/* EC_FCH_WAKE_L */
-	PAD_GPI(GPIO_24, PULL_UP),
 	PAD_WAKE(GPIO_24, PULL_UP, EDGE_LOW, S3_S4_S5),
 	/* PCIE_RST1_L - Variable timings (May remove) */
 	PAD_NF(GPIO_27, PCIE_RST1_L, PULL_NONE),
@@ -47,7 +46,6 @@ static const struct soc_amd_gpio gpio_set_stage_rom_v3[] = {
 	/* EN_PWR_TOUCHPAD_PS2 - reset */
 	PAD_GPO(GPIO_6, LOW),
 	/* EC_FCH_WAKE_L */
-	PAD_GPI(GPIO_24, PULL_UP),
 	PAD_WAKE(GPIO_24, PULL_UP, EDGE_LOW, S3_S4_S5),
 	/* PCIE_RST1_L - Variable timings (May remove) */
 	PAD_NF(GPIO_27, PCIE_RST1_L, PULL_NONE),
diff --git a/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c b/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c
index 01aef2bb11..21917939ae 100644
--- a/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c
+++ b/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c
@@ -13,7 +13,6 @@ static const struct soc_amd_gpio gpio_set_stage_rom_pre_v3[] = {
 	/* PEN_POWER_EN - reset */
 	PAD_GPO(GPIO_5, LOW),
 	/* EC_FCH_WAKE_L */
-	PAD_GPI(GPIO_24, PULL_UP),
 	PAD_WAKE(GPIO_24, PULL_UP, EDGE_LOW, S3_S4_S5),
 	/* PCIE_RST1_L - Variable timings (May remove) */
 	PAD_NF(GPIO_27, PCIE_RST1_L, PULL_NONE),
@@ -45,7 +44,6 @@ static const struct soc_amd_gpio gpio_set_stage_rom_v3[] = {
 	/* EN_PWR_TOUCHPAD_PS2 - reset */
 	PAD_GPO(GPIO_13, LOW),
 	/* EC_FCH_WAKE_L */
-	PAD_GPI(GPIO_24, PULL_UP),
 	PAD_WAKE(GPIO_24, PULL_UP, EDGE_LOW, S3_S4_S5),
 	/* PCIE_RST1_L - Variable timings (May remove) */
 	PAD_NF(GPIO_27, PCIE_RST1_L, PULL_NONE),
@@ -82,7 +80,6 @@ static const struct soc_amd_gpio gpio_set_stage_ram[] = {
 	/* PEN_POWER_EN - Enabled*/
 	PAD_GPO(GPIO_5, HIGH),
 	/* FPMCU_INT_L */
-	PAD_GPI(GPIO_6, PULL_UP),
 	PAD_WAKE(GPIO_6, PULL_UP, EDGE_LOW, S3_S4_S5),
 	/* I2S_SDIN */
 	PAD_NF(GPIO_7, ACP_I2S_SDIN, PULL_NONE),
diff --git a/src/soc/amd/common/block/gpio_banks/gpio.c b/src/soc/amd/common/block/gpio_banks/gpio.c
index caf07c0192..883dcfa783 100644
--- a/src/soc/amd/common/block/gpio_banks/gpio.c
+++ b/src/soc/amd/common/block/gpio_banks/gpio.c
@@ -21,17 +21,6 @@ static int get_gpio_gevent(uint8_t gpio, const struct soc_amd_event *table,
 	return -1;
 }
 
-static void mem_read_write32(uint32_t *address, uint32_t value, uint32_t mask)
-{
-	uint32_t reg32;
-
-	value &= mask;
-	reg32 = read32(address);
-	reg32 &= ~mask;
-	reg32 |= value;
-	write32(address, reg32);
-}
-
 static void program_smi(uint32_t flags, int gevent_num)
 {
 	uint8_t level;
@@ -187,7 +176,6 @@ __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)
 {
-	uint32_t *gpio_ptr;
 	uint32_t control, control_flags;
 	uint8_t mux, index, gpio;
 	int gevent_num;
@@ -219,51 +207,23 @@ void program_gpios(const struct soc_amd_gpio *gpio_list_ptr, size_t size)
 		iomux_read8(gpio); /* Flush posted write */
 
 		soc_gpio_hook(gpio, mux);
+		__gpio_setbits32(gpio, PAD_CFG_MASK, control);
+
+		if (control_flags == 0)
+			continue;
+
+		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;
+		}
 
-		gpio_ptr = gpio_ctrl_ptr(gpio);
-
-		if (control_flags & GPIO_FLAG_SPECIAL_MASK) {
-			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_FLAG_SPECIAL_MASK) {
-			case GPIO_FLAG_DEBOUNCE:
-				mem_read_write32(gpio_ptr, control,
-						GPIO_DEBOUNCE_MASK);
-				break;
-			case GPIO_FLAG_WAKE:
-				mem_read_write32(gpio_ptr, control,
-						INT_WAKE_MASK);
-				break;
-			case GPIO_FLAG_INT:
-				mem_read_write32(gpio_ptr, control,
-						AMD_GPIO_CONTROL_MASK);
-				break;
-			case GPIO_FLAG_SMI:
-				mem_read_write32(gpio_ptr, control,
-						INT_SCI_SMI_MASK);
-
-				program_smi(control_flags, gevent_num);
-				break;
-			case GPIO_FLAG_SCI:
-				mem_read_write32(gpio_ptr, control,
-						INT_SCI_SMI_MASK);
-
-				fill_sci_trigger(control_flags, gevent_num, &sci_trigger_cfg);
-
-				soc_route_sci(gevent_num);
-				break;
-			default:
-				printk(BIOS_WARNING, "Error, flags 0x%08x\n",
-							control_flags);
-				break;
-			}
-		} else {
-			mem_read_write32(gpio_ptr, control,
-						AMD_GPIO_CONTROL_MASK);
+		if (control_flags & GPIO_FLAG_SMI) {
+			program_smi(control_flags, gevent_num);
+		} else if (control_flags & GPIO_FLAG_SCI) {
+			fill_sci_trigger(control_flags, gevent_num, &sci_trigger_cfg);
+			soc_route_sci(gevent_num);
 		}
 	}
 
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 546d55aa53..f02c8e86b4 100644
--- a/src/soc/amd/common/block/include/amdblocks/gpio_banks.h
+++ b/src/soc/amd/common/block/include/amdblocks/gpio_banks.h
@@ -74,6 +74,7 @@ struct soc_amd_event {
 
 #define GPIO_INT_STATUS		(1 << 28)
 #define GPIO_WAKE_STATUS	(1 << 29)
+#define GPIO_STATUS_MASK	(3 << 28)
 
 enum {
 	GEVENT_0,
@@ -118,7 +119,6 @@ enum {
 #define GPIO_PULL_PULL_DOWN GPIO_PULLDOWN_ENABLE
 #define GPIO_PULL_PULL_NONE 0
 
-#define AMD_GPIO_CONTROL_MASK			0x00f4ff00
 #define AMD_GPIO_MUX_MASK			0x03
 
 
@@ -134,10 +134,6 @@ enum {
 #define GPIO_FLAG_EVENT_ACTIVE_MASK	(1 << 1)
 #define GPIO_FLAG_SCI			(1 << 2)
 #define GPIO_FLAG_SMI			(1 << 3)
-#define GPIO_FLAG_DEBOUNCE		(1 << 4)
-#define GPIO_FLAG_WAKE			(1 << 5)
-#define GPIO_FLAG_INT			(1 << 6)
-#define GPIO_FLAG_SPECIAL_MASK	        (0x1f << 2)
 
 /* Trigger configuration for GPIO SCI/SMI events. */
 #define GPIO_FLAG_EVENT_TRIGGER_LEVEL_HIGH	(GPIO_FLAG_EVENT_TRIGGER_LEVEL | \
@@ -169,11 +165,6 @@ static inline bool is_gpio_event_active_low(uint32_t flags)
 	return (flags & GPIO_FLAG_EVENT_ACTIVE_MASK) == GPIO_FLAG_EVENT_ACTIVE_LOW;
 }
 
-#define GPIO_DEBOUNCE_MASK		0x000000ff
-#define INT_TRIGGER_MASK		0x00000700
-#define INT_WAKE_MASK			0x0000e700
-#define INT_SCI_SMI_MASK		0x00f40000
-
 #define DEB_GLITCH_SHIFT		5
 #define DEB_GLITCH_LOW			1
 #define DEB_GLITCH_HIGH			2
@@ -200,18 +191,29 @@ static inline bool is_gpio_event_active_low(uint32_t flags)
 #define GPIO_DEB_200mS			(13 | GPIO_TIMEBASE_15560uS)
 #define GPIO_DEB_500mS			(8 | GPIO_TIMEBASE_62440uS)
 
+#define GPIO_DEB_MASK			0xff
+
 #define GPIO_WAKE_S0i3			(1 << 13)
 #define GPIO_WAKE_S3			(1 << 14)
 #define GPIO_WAKE_S4_S5			(1 << 15)
 #define GPIO_WAKE_S0i3_S4_S5		(GPIO_WAKE_S0i3 | GPIO_WAKE_S4_S5)
 #define GPIO_WAKE_S3_S4_S5		(GPIO_WAKE_S3 | GPIO_WAKE_S4_S5)
+#define GPIO_WAKE_MASK			(7 << 13)
+
+/*
+ * Mask used to reset bits in GPIO control register when configuring pad using `program_gpios()`
+ * Bits that are preserved/untouched:
+ * - Reserved bits
+ * - Drive strength bits
+ * - Read only bits
+ */
+#define PAD_CFG_MASK		(GPIO_DEB_MASK | GPIO_TRIGGER_MASK | GPIO_ACTIVE_MASK | \
+				 GPIO_INT_ENABLE_MASK | GPIO_WAKE_MASK | GPIO_PULL_MASK | \
+				 GPIO_OUTPUT_MASK | GPIO_STATUS_MASK)
 
 /*
- * Several macros are available to declare programming of GPIO pins, and if
- * needed, more than 1 macro can be used for any pin. However, some macros
- * will have no effect if combined. For example debounce only affects input
- * or one of the interrupts. Some macros should not be combined, such as SMI
- * and regular interrupt. The defined macros and their parameters are:
+ * Several macros are available to declare programming of GPIO pins. The defined macros and
+ * their parameters are:
  * PAD_NF		Define native alternate function for the pin.
  *	pin		the pin to be programmed
  *	function	the native function
@@ -230,19 +232,19 @@ static inline bool is_gpio_event_active_low(uint32_t flags)
  * PAD_SCI		The pin is a SCI source
  *	pin		the pin to be programmed
  *	pull		pull up, pull down or no pull
- *	trigger		LEVEL_LOW, LEVEL_HIGH, EDGE_LOW, EDGE_HIGH
+ *	event trigger		LEVEL_LOW, LEVEL_HIGH, EDGE_LOW, EDGE_HIGH
  * PAD_SMI		The pin is a SMI source
  *	pin		the pin to be programmed
  *	pull		pull up, pull down or no pull
- *	trigger		LEVEL_LOW, LEVEL_HIGH
+ *	event trigger		LEVEL_LOW, LEVEL_HIGH
  * PAD_WAKE		The pin can wake, use after PAD_INT or PAD_SCI
  *	pin		the pin to be programmed
  *	pull		pull up, pull down or no pull
  *	trigger		LEVEL_LOW, LEVEL_HIGH, EDGE_LOW, EDGE_HIGH, BOTH_EDGES
  *	type		S0i3, S3, S4_S5 or S4_S5 combinations (S0i3_S3 invalid)
- * PAD_DEBOUNCE		The input or interrupt will be debounced, invalid after
- *			PAD_NF
+ * PAD_DEBOUNCE		The input or interrupt will be debounced
  *	pin		the pin to be programmed
+ *	pull		pull up, pull down or no pull
  *	debounce_type	preserve low glitch, preserve high glitch, no glitch
  *	debounce_time	the debounce time
  */
@@ -279,29 +281,31 @@ static inline bool is_gpio_event_active_low(uint32_t flags)
 #define PAD_INT(pin, pull, trigger, action)				\
 	PAD_CFG_STRUCT(pin, pin ## _IOMUX_GPIOxx,			\
 		PAD_PULL(pull) | PAD_TRIGGER(trigger) | PAD_INT_ENABLE(action), \
-		GPIO_FLAG_INT)
+		0)
 
 /* SCI pad configuration */
 #define PAD_SCI(pin, pull, trigger)					\
-	PAD_CFG_STRUCT(pin, pin ## _IOMUX_GPIOxx, PAD_PULL(pull),	\
+	PAD_CFG_STRUCT(pin, pin ## _IOMUX_GPIOxx,			\
+		PAD_PULL(pull) | PAD_TRIGGER(LEVEL_HIGH),		\
 		PAD_FLAG_EVENT_TRIGGER(trigger) | GPIO_FLAG_SCI)
 
 /* SMI pad configuration */
 #define PAD_SMI(pin, pull, trigger)					\
-	PAD_CFG_STRUCT(pin, pin ## _IOMUX_GPIOxx, PAD_PULL(pull),	\
+	PAD_CFG_STRUCT(pin, pin ## _IOMUX_GPIOxx,			\
+		PAD_PULL(pull) | PAD_TRIGGER(LEVEL_HIGH),		\
 		PAD_FLAG_EVENT_TRIGGER(trigger) | GPIO_FLAG_SMI)
 
 /* WAKE pad configuration */
 #define PAD_WAKE(pin, pull, trigger, type)				\
 	PAD_CFG_STRUCT(pin, pin ## _IOMUX_GPIOxx,			\
 		PAD_PULL(pull) | PAD_TRIGGER(trigger) | PAD_WAKE_ENABLE(type), \
-		GPIO_FLAG_WAKE)
+		0)
 
 /* pin debounce configuration */
-#define PAD_DEBOUNCE(pin, type, time)					\
+#define PAD_DEBOUNCE(pin, pull, type, time)				\
 	PAD_CFG_STRUCT(pin, pin ## _IOMUX_GPIOxx,			\
-			PAD_DEBOUNCE_CONFIG(type) | PAD_DEBOUNCE_CONFIG(time), \
-			GPIO_FLAG_DEBOUNCE)
+		PAD_PULL(pull) | PAD_DEBOUNCE_CONFIG(type) | PAD_DEBOUNCE_CONFIG(time), \
+		0)
 
 typedef uint32_t gpio_t;