Change in coreboot[master]: soc/amd/common/gpio: Make macro names for GPIO flags consistent - coreboot-gerrit

29 Jun 2020

Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42869 )
Change subject: soc/amd/common/gpio: Make macro names for GPIO flags consistent
......................................................................
soc/amd/common/gpio: Make macro names for GPIO flags consistent
`soc_amd_gpio` structure uses a flag field to store additional
information about GPIO configuration that does not end up directly in
the GPIO control register. However, the naming for these flags is not
consistent across event triggers and special configurations. This
change updates the flag names to be consistent (starting with
GPIO_FLAG_*) and adds some helper functions for GPIO events.
In the following CLs, more changes will be made to drop some of the
special flags which are not really required.
BUG=b:159944426
Change-Id: Idca795c3e594eb956d297d5ba5d08f75b5563ee5
Signed-off-by: Furquan Shaikh furquan@google.com
---
M src/soc/amd/common/block/gpio_banks/gpio.c
M src/soc/amd/common/block/include/amdblocks/gpio_banks.h
2 files changed, 72 insertions(+), 47 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/42869/1

diff --git a/src/soc/amd/common/block/gpio_banks/gpio.c b/src/soc/amd/common/block/gpio_banks/gpio.c
index 4d6d164..d7ac896 100644
--- a/src/soc/amd/common/block/gpio_banks/gpio.c
+++ b/src/soc/amd/common/block/gpio_banks/gpio.c
@@ -32,21 +32,22 @@
    write32(address, reg32);
 }
-static void program_smi(uint32_t trigger, int gevent_num)
+static void program_smi(uint32_t flags, int gevent_num)
 {
-	/*
-	 * Only level trigger is allowed for SMI. Trigger values are 0
-	 * through 3, with 0-1 being level trigger and 2-3 being edge
-	 * trigger. GPIO_TRIGGER_EDGE_LOW is 2, so trigger has to be
-	 * less than GPIO_TRIGGER_EDGE_LOW.
-	 */
-	assert(trigger < GPIO_TRIGGER_EDGE_LOW);
+	uint8_t level;
-	if (trigger == GPIO_TRIGGER_LEVEL_HIGH)
-		configure_gevent_smi(gevent_num, SMI_MODE_SMI, SMI_SCI_LVL_HIGH);
+	if (!is_gpio_event_level_triggered(flags)) {
+		printk(BIOS_ERR, "ERROR: %s - Only level trigger allowed for SMI!\n", __func__);
+		assert(0);
+		return;
+	}
-	if (trigger == GPIO_TRIGGER_LEVEL_LOW)
-		configure_gevent_smi(gevent_num, SMI_MODE_SMI, SMI_SCI_LVL_LOW);
+	if (is_gpio_event_active_high(flags))
+		level = SMI_SCI_LVL_HIGH;
+	else
+		level = SMI_SCI_LVL_LOW;
+
+	configure_gevent_smi(gevent_num, SMI_MODE_SMI, level);
 }
struct sci_trigger_regs {
@@ -62,20 +63,18 @@
  * In a similar fashion, polarity (rising/falling, hi/lo) of each GPE is
  * represented as a single bit in SMI_SCI_TRIG register.
  */
-static void fill_sci_trigger(uint32_t trigger, int gpe, struct sci_trigger_regs *regs)
+static void fill_sci_trigger(uint32_t flags, int gpe, struct sci_trigger_regs *regs)
 {
    uint32_t mask = 1 << gpe;
regs->mask |= mask;
-	/* Select level vs. edge triggered event. */
-	if ((trigger == GPIO_TRIGGER_LEVEL_LOW) || (trigger == GPIO_TRIGGER_LEVEL_HIGH))
+	if (is_gpio_event_level_triggered(flags))
    	regs->level |= mask;
    else
    	regs->level &= ~mask;
-	/* Select rising/high vs falling/low trigger. */
-	if ((trigger == GPIO_TRIGGER_EDGE_HIGH) || (trigger == GPIO_TRIGGER_LEVEL_HIGH))
+	if (is_gpio_event_active_high(flags))
    	regs->polarity |= mask;
    else
    	regs->polarity &= ~mask;
@@ -217,38 +216,37 @@
gpio_ptr = gpio_ctrl_ptr(gpio);
-		if (control_flags & GPIO_SPECIAL_FLAG) {
+		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_SPECIAL_MASK) {
-			case GPIO_DEBOUNCE_FLAG:
+			switch (control_flags & GPIO_FLAG_SPECIAL_MASK) {
+			case GPIO_FLAG_DEBOUNCE:
    			mem_read_write32(gpio_ptr, control,
    					GPIO_DEBOUNCE_MASK);
    			break;
-			case GPIO_WAKE_FLAG:
+			case GPIO_FLAG_WAKE:
    			mem_read_write32(gpio_ptr, control,
    					INT_WAKE_MASK);
    			break;
-			case GPIO_INT_FLAG:
+			case GPIO_FLAG_INT:
    			mem_read_write32(gpio_ptr, control,
    					AMD_GPIO_CONTROL_MASK);
    			break;
-			case GPIO_SMI_FLAG:
+			case GPIO_FLAG_SMI:
    			mem_read_write32(gpio_ptr, control,
    					INT_SCI_SMI_MASK);
-				program_smi(control_flags & FLAGS_TRIGGER_MASK, gevent_num);
+				program_smi(control_flags, gevent_num);
    			break;
-			case GPIO_SCI_FLAG:
+			case GPIO_FLAG_SCI:
    			mem_read_write32(gpio_ptr, control,
    					INT_SCI_SMI_MASK);
-				fill_sci_trigger(control_flags & FLAGS_TRIGGER_MASK, gevent_num,
-						 &sci_trigger_cfg);
+				fill_sci_trigger(control_flags, gevent_num, &sci_trigger_cfg);
soc_route_sci(gevent_num);
    			break;
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 92aa8a2..2664ba5 100644
--- a/src/soc/amd/common/block/include/amdblocks/gpio_banks.h
+++ b/src/soc/amd/common/block/include/amdblocks/gpio_banks.h
@@ -4,6 +4,7 @@
 #define __AMDBLOCK_GPIO_BANKS_H__
#include <stdint.h>
+#include <stdbool.h>
 #include <stddef.h>
struct soc_amd_gpio {
@@ -106,27 +107,53 @@
 #define GPIO_INT_LEVEL_HIGH		(GPIO_ACTIVE_HIGH | GPIO_LEVEL_TRIG)
 #define GPIO_INT_LEVEL_LOW		(GPIO_ACTIVE_LOW | GPIO_LEVEL_TRIG)
-enum {
-	GPIO_TRIGGER_LEVEL_LOW,
-	GPIO_TRIGGER_LEVEL_HIGH,
-	GPIO_TRIGGER_EDGE_LOW,
-	GPIO_TRIGGER_EDGE_HIGH,
-};
+/*
+ * Flags used for GPIO configuration. These provide additional information that does not go
+ * directly into GPIO control register. These are stored in `flags` field in soc_amd_gpio.
+ */
+#define GPIO_FLAG_EVENT_TRIGGER_LEVEL	(1 << 0)
+#define GPIO_FLAG_EVENT_TRIGGER_EDGE	(0 << 0)
+#define GPIO_FLAG_EVENT_TRIGGER_MASK	(1 << 0)
+#define GPIO_FLAG_EVENT_ACTIVE_HIGH	(1 << 1)
+#define GPIO_FLAG_EVENT_ACTIVE_LOW	(0 << 1)
+#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)
-#define GPIO_SPECIAL_FLAG		(1 << 31)
-#define GPIO_DEBOUNCE_FLAG		(1 << 30)
-#define GPIO_WAKE_FLAG			(1 << 29)
-#define GPIO_INT_FLAG			(1 << 28)
-#define GPIO_SMI_FLAG			(1 << 27)
-#define GPIO_SCI_FLAG			(1 << 26)
-#define GPIO_FLAG_DEBOUNCE		(GPIO_SPECIAL_FLAG | GPIO_DEBOUNCE_FLAG)
-#define GPIO_FLAG_WAKE			(GPIO_SPECIAL_FLAG | GPIO_WAKE_FLAG)
-#define GPIO_FLAG_INT			(GPIO_SPECIAL_FLAG | GPIO_INT_FLAG)
-#define GPIO_FLAG_SCI			(GPIO_SPECIAL_FLAG | GPIO_SCI_FLAG)
-#define GPIO_FLAG_SMI			(GPIO_SPECIAL_FLAG | GPIO_SMI_FLAG)
+/* Trigger configuration for GPIO SCI/SMI events. */
+#define GPIO_FLAG_EVENT_TRIGGER_LEVEL_HIGH	(GPIO_FLAG_EVENT_TRIGGER_LEVEL | \
+						 GPIO_FLAG_EVENT_ACTIVE_HIGH)
+#define GPIO_FLAG_EVENT_TRIGGER_LEVEL_LOW	(GPIO_FLAG_EVENT_TRIGGER_LEVEL | \
+						 GPIO_FLAG_EVENT_ACTIVE_LOW)
+#define GPIO_FLAG_EVENT_TRIGGER_EDGE_HIGH	(GPIO_FLAG_EVENT_TRIGGER_EDGE | \
+						 GPIO_FLAG_EVENT_ACTIVE_HIGH)
+#define GPIO_FLAG_EVENT_TRIGGER_EDGE_LOW	(GPIO_FLAG_EVENT_TRIGGER_EDGE | \
+						 GPIO_FLAG_EVENT_ACTIVE_LOW)
-#define FLAGS_TRIGGER_MASK		0x00000003
-#define GPIO_SPECIAL_MASK		0x7c000000
+static inline bool is_gpio_event_level_triggered(int flags)
+{
+	return (flags & GPIO_FLAG_EVENT_TRIGGER_MASK) == GPIO_FLAG_EVENT_TRIGGER_LEVEL;
+}
+
+static inline bool is_gpio_event_edge_triggered(int flags)
+{
+	return (flags & GPIO_FLAG_EVENT_TRIGGER_MASK) == GPIO_FLAG_EVENT_TRIGGER_EDGE;
+}
+
+static inline bool is_gpio_event_active_high(int flags)
+{
+	return (flags & GPIO_FLAG_EVENT_ACTIVE_MASK) == GPIO_FLAG_EVENT_ACTIVE_HIGH;
+}
+
+static inline bool is_gpio_event_active_low(int 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
@@ -238,7 +265,7 @@
    		    GPIO_EVENT_INT ## _ ## action), \
    	.flags = GPIO_FLAG_INT }
 /* Auxiliary macro for SCI and SMI */
-#define PAD_AUX2(trigger, flag) (GPIO_TRIGGER ## _ ## trigger | flag)
+#define PAD_AUX2(trigger, flag) (GPIO_FLAG_EVENT_TRIGGER ## _ ## trigger | flag)
 /* SCI pad configuration */
 #define PAD_SCI(pin, pull, trigger) \
    { .gpio = (pin), \
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/42869
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Idca795c3e594eb956d297d5ba5d08f75b5563ee5
Gerrit-Change-Number: 42869
Gerrit-PatchSet: 1
Gerrit-Owner: Furquan Shaikh furquan@google.com
Gerrit-MessageType: newchange



    