cr50: Separate ARM core GPIOs from pinmux configuration

This separates the configuration of the ARM core GPIOs from the
routing of internal peripherals to external pins. Both are still
described in the gpio.inc file, but are less dependent on each
other.

BUG=chrome-os-partner:33818
BRANCH=none
TEST=manual

Before this CL, running "sysjump rw" or trying to use more than 8
GPIOs caused hangs and reboots. Now it doesn't.

Change-Id: If962a7c5ad4136837b2ea00ae016a440f07d7e23
Signed-off-by: Bill Richardson <wfrichar@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/251015
Reviewed-by: Sheng-liang Song <ssl@chromium.org>
Reviewed-by: Randall Spangler <rspangler@chromium.org>

8 files changed, 276 insertions, 179 deletions

diff --git a/board/cr50/board.c b/board/cr50/board.c
index 31bfdf036a..92a6808c11 100644
--- a/board/cr50/board.c
+++ b/board/cr50/board.c
@@ -11,14 +11,29 @@
 #include "task.h"
 #include "util.h"
 
+/*
+ * There's no way to trigger on both rising and falling edges, so force a
+ * compiler error if we try. The workaround is to use the pinmux to connect
+ * two GPIOs to the same input and configure each one for a separate edge.
+ */
+#undef GPIO_INT_BOTH
+#define GPIO_INT_BOTH NOT_SUPPORTED_ON_CR50
+
 #include "gpio_list.h"
 
+/* Interrupt handler for button pushes */
 void button_event(enum gpio_signal signal)
 {
-	int val = gpio_get_level(signal);
-	ccprintf("Button %d = %d\n", signal, gpio_get_level(signal));
+	int v;
+
+	/* We have two GPIOs on the same input (one rising edge, one falling
+	 * edge), so de-alias them */
+	if (signal >= GPIO_SW_N_)
+		signal -= (GPIO_SW_N_ - GPIO_SW_N);
 
-	gpio_set_level(GPIO_LED_4, val);
+	v = gpio_get_level(signal);
+	ccprintf("Button %d = %d\n", signal, v);
+	gpio_set_level(signal - GPIO_SW_N + GPIO_LED_4, v);
 }
 
 /* Initialize board. */
@@ -28,5 +43,9 @@ static void board_init(void)
 	gpio_enable_interrupt(GPIO_SW_S);
 	gpio_enable_interrupt(GPIO_SW_W);
 	gpio_enable_interrupt(GPIO_SW_E);
+	gpio_enable_interrupt(GPIO_SW_N_);
+	gpio_enable_interrupt(GPIO_SW_S_);
+	gpio_enable_interrupt(GPIO_SW_W_);
+	gpio_enable_interrupt(GPIO_SW_E_);
 }
 DECLARE_HOOK(HOOK_INIT, board_init, HOOK_PRIO_DEFAULT);
diff --git a/board/cr50/board.h b/board/cr50/board.h
index 83f871c8f6..dff77271b8 100644
--- a/board/cr50/board.h
+++ b/board/cr50/board.h
@@ -15,9 +15,6 @@
 #undef CONFIG_HIBERNATE
 #undef CONFIG_LID_SWITCH
 
-/* Need space to store the pin muxing configuration */
-#define CONFIG_GPIO_LARGE_ALT_INFO
-
 /*
  * Allow dangerous commands all the time, since we don't have a write protect
  * switch.
diff --git a/board/cr50/gpio.inc b/board/cr50/gpio.inc
index 69bfe4aec5..8c518b8cf7 100644
--- a/board/cr50/gpio.inc
+++ b/board/cr50/gpio.inc
@@ -7,24 +7,59 @@
 /* Inputs with interrupt handlers are first for efficiency */
 
 /* User Push buttons */
-GPIO(SW_N,   M,  0, GPIO_INT_BOTH, button_event)
-GPIO(SW_S,   M,  1, GPIO_INT_BOTH, button_event)
-GPIO(SW_W,   M,  2, GPIO_INT_BOTH, button_event)
-GPIO(SW_E,   M,  3, GPIO_INT_BOTH, button_event)
+GPIO(SW_N,   0, 0, GPIO_INT_RISING, button_event)
+GPIO(SW_S,   0, 1, GPIO_INT_RISING, button_event)
+GPIO(SW_W,   0, 2, GPIO_INT_RISING, button_event)
+GPIO(SW_E,   0, 3, GPIO_INT_RISING, button_event)
 
-/* Outputs */
+/* We can't trigger on both rising and falling edge, so attach each button
+ * to two input GPIOs. */
+GPIO(SW_N_,   1, 0, GPIO_INT_FALLING, button_event)
+GPIO(SW_S_,   1, 1, GPIO_INT_FALLING, button_event)
+GPIO(SW_W_,   1, 2, GPIO_INT_FALLING, button_event)
+GPIO(SW_E_,   1, 3, GPIO_INT_FALLING, button_event)
 
 /* User GPIO LEDs */
-GPIO(LED_2,  A,  9,  GPIO_OUT_HIGH,  NULL)
-GPIO(LED_3,  A, 10,  GPIO_OUT_LOW,   NULL)
-GPIO(LED_4,  A, 11,  GPIO_OUT_LOW,  NULL)
-GPIO(LED_5,  A, 12,  GPIO_OUT_LOW,  NULL)
-GPIO(LED_6,  A, 13,  GPIO_OUT_LOW,  NULL)
-/*GPIO(LED_7,  A, 14,  GPIO_OUT_LOW,  NULL)*/
+GPIO(LED_2,  0, 4, GPIO_OUT_LOW, NULL)
+GPIO(LED_3,  0, 5, GPIO_OUT_LOW, NULL)
+GPIO(LED_4,  0, 6, GPIO_OUT_LOW, NULL)
+GPIO(LED_5,  0, 7, GPIO_OUT_LOW, NULL)
+GPIO(LED_6,  0, 8, GPIO_OUT_LOW, NULL)
+GPIO(LED_7,  0, 9, GPIO_OUT_LOW, NULL)
 
 /* Unimplemented signals which we need to emulate for now */
 UNIMPLEMENTED(ENTERING_RW)
 
-/* Alternate functions */
-ALTERNATE(A, 0x0001, PINMUX(UART0_TX), MODULE_UART,   0) /* UART0 TX: DIOA0 */
-ALTERNATE(A, 0x0002, PINMUX(UART0_RX), MODULE_UART,   0) /* UART0 RX: DIOA1 */
+/* The Cr50 ARM core has no alternate functions, so we repurpose that
+ * macro to describe the PINMUX setup. The args are
+ *
+ * 1. The ARM core GPIO or SoC peripheral function to connect
+ * 2. The pinmux DIO pad to connect to
+ * 3. <ignored>
+ * 4. MODULE_GPIO, to prevent being called by gpio_config_module()
+ * 5. flags to specify the direction if the GPIO isn't enough
+ */
+
+/* The serial port is one of the SoC peripheral functions */
+ALTERNATE(FUNC(UART0_TX), DIO(A0), 0, MODULE_GPIO, DIO_OUTPUT)
+ALTERNATE(FUNC(UART0_RX), DIO(A1), 0, MODULE_GPIO, DIO_INPUT)
+
+/* Inputs */
+ALTERNATE(SW_N,           DIO(M0), 0, MODULE_GPIO, 0)
+ALTERNATE(SW_S,           DIO(M1), 0, MODULE_GPIO, 0)
+ALTERNATE(SW_W,           DIO(M2), 0, MODULE_GPIO, 0)
+ALTERNATE(SW_E,           DIO(M3), 0, MODULE_GPIO, 0)
+
+/* Aliased Inputs, connected to the same pins */
+ALTERNATE(SW_N_,           DIO(M0), 0, MODULE_GPIO, 0)
+ALTERNATE(SW_S_,           DIO(M1), 0, MODULE_GPIO, 0)
+ALTERNATE(SW_W_,           DIO(M2), 0, MODULE_GPIO, 0)
+ALTERNATE(SW_E_,           DIO(M3), 0, MODULE_GPIO, 0)
+
+/* Outputs - also mark as inputs so we can read back from the driven pin */
+ALTERNATE(LED_2,          DIO(A9),    0, MODULE_GPIO, DIO_INPUT)
+ALTERNATE(LED_3,          DIO(A10),   0, MODULE_GPIO, DIO_INPUT)
+ALTERNATE(LED_4,          DIO(A11),   0, MODULE_GPIO, DIO_INPUT)
+ALTERNATE(LED_5,          DIO(A12),   0, MODULE_GPIO, DIO_INPUT)
+ALTERNATE(LED_6,          DIO(A13),   0, MODULE_GPIO, DIO_INPUT)
+ALTERNATE(LED_7,          DIO(A14),   0, MODULE_GPIO, DIO_INPUT)
diff --git a/chip/g/gpio.c b/chip/g/gpio.c
index 5f2b682507..0dd6122d54 100644
--- a/chip/g/gpio.c
+++ b/chip/g/gpio.c
@@ -9,118 +9,137 @@
 #include "registers.h"
 #include "task.h"
 
-/* GPIOs are split into 2 words of 16 GPIOs */
-#define GPIO_WORD_SIZE_LOG2 4
-#define GPIO_WORD_SIZE      (1 << GPIO_WORD_SIZE_LOG2)
-#define GPIO_WORD_MASK      (GPIO_WORD_SIZE - 1)
+/*
+ * The Cr50's ARM core has two GPIO ports of 16 bits each. Each GPIO signal
+ * can be routed through a full NxM crossbar to any of a number of external
+ * pins. When setting up GPIOs, both the ARM core and the crossbar must be
+ * configured correctly. This file is only concerned with the ARM core.
+ */
 
 test_mockable int gpio_get_level(enum gpio_signal signal)
 {
-	uint32_t wi = signal >> GPIO_WORD_SIZE_LOG2;
-	uint32_t mask = 1 << (signal & GPIO_WORD_MASK);
-	return !!(GR_GPIO_DATAIN(wi) & mask);
+	const struct gpio_info *g = gpio_list + signal;
+	return !!(GR_GPIO_DATAIN(g->port) & g->mask);
 }
 
-void gpio_set_level(enum gpio_signal signal, int value)
+static void set_one_gpio_bit(uint32_t port, uint16_t mask, int value)
 {
-	uint32_t wi = signal >> GPIO_WORD_SIZE_LOG2;
-	uint32_t idx = signal & GPIO_WORD_MASK;
+	if (!mask)
+		return;
 
-	GR_GPIO_MASKBYTE(wi, 1 << idx) = value << idx;
+	/* Assumes mask has one and only one bit set */
+	if (mask & 0x00FF)
+		GR_GPIO_MASKLOWBYTE(port, mask) = value ? mask : 0;
+	else
+		GR_GPIO_MASKHIGHBYTE(port, mask >> 8) = value ? mask : 0;
 }
 
-static void gpio_set_flags_internal(enum gpio_signal signal, uint32_t port,
-				    uint32_t mask, uint32_t flags)
+void gpio_set_level(enum gpio_signal signal, int value)
 {
-	uint32_t wi = signal >> GPIO_WORD_SIZE_LOG2;
-	uint32_t idx = signal & GPIO_WORD_MASK;
-	uint32_t di = 31 - __builtin_clz(mask);
+	const struct gpio_info *g = gpio_list + signal;
+	set_one_gpio_bit(g->port, g->mask, value);
+}
 
-	/* Set up pullup / pulldown */
-	if (flags & GPIO_PULL_UP)
-		GR_PINMUX_DIO_CTL(port, di) |= PINMUX_DIO_CTL_PU;
-	else
-		GR_PINMUX_DIO_CTL(port, di) &= ~PINMUX_DIO_CTL_PU;
-	if (flags & GPIO_PULL_DOWN)
-		GR_PINMUX_DIO_CTL(port, di) |= PINMUX_DIO_CTL_PD;
+void gpio_set_flags_by_mask(uint32_t port, uint32_t mask, uint32_t flags)
+{
+	/* Output must be enabled; input is always enabled */
+	if (flags & GPIO_OUTPUT)
+		GR_GPIO_SETDOUTEN(port) = mask;
 	else
-		GR_PINMUX_DIO_CTL(port, di) &= ~PINMUX_DIO_CTL_PD;
-
-	if (flags & GPIO_OUTPUT) {
-		/* Set pin level first to avoid glitching. */
-		GR_GPIO_MASKBYTE(wi, 1 << idx) = !!(flags & GPIO_HIGH) << idx;
-		/* Switch Output Enable */
-		GR_GPIO_SETDOUTEN(wi) = 1 << idx;
-	} else if (flags & GPIO_INPUT) {
-		GR_GPIO_CLRDOUTEN(wi) = 1 << idx;
+		GR_GPIO_CLRDOUTEN(port) = mask;
+
+	/* Only matters for outputs */
+	if (flags & GPIO_LOW)
+		set_one_gpio_bit(port, mask, 0);
+	else if (flags & GPIO_HIGH)
+		set_one_gpio_bit(port, mask, 1);
+
+	/* Interrupt types */
+	if (flags & GPIO_INT_F_LOW) {
+		GR_GPIO_CLRINTTYPE(port) = mask;
+		GR_GPIO_CLRINTPOL(port) = mask;
+		GR_GPIO_SETINTEN(port) = mask;
 	}
-
-	/* Edge-triggered interrupt */
-	if (flags & (GPIO_INT_F_FALLING | GPIO_INT_F_RISING)) {
-		GR_GPIO_SETINTTYPE(wi) = 1 << idx;
-		if (flags & GPIO_INT_F_RISING)
-			GR_GPIO_SETINTPOL(wi) = 1 << idx;
-		else
-			GR_GPIO_CLRINTPOL(wi) = 1 << idx;
+	if (flags & GPIO_INT_F_HIGH) {
+		GR_GPIO_CLRINTTYPE(port) = mask;
+		GR_GPIO_SETINTPOL(port) = mask;
+		GR_GPIO_SETINTEN(port) = mask;
 	}
-	/* Level-triggered interrupt */
-	if (flags & (GPIO_INT_F_LOW | GPIO_INT_F_HIGH)) {
-		GR_GPIO_CLRINTTYPE(wi) = 1 << idx;
-		if (flags & GPIO_INT_F_HIGH)
-			GR_GPIO_SETINTPOL(wi) = 1 << idx;
-		else
-			GR_GPIO_CLRINTPOL(wi) = 1 << idx;
+	if (flags & GPIO_INT_F_FALLING) {
+		GR_GPIO_SETINTTYPE(port) = mask;
+		GR_GPIO_CLRINTPOL(port) = mask;
+		GR_GPIO_SETINTEN(port) = mask;
+	}
+	if (flags & GPIO_INT_F_RISING) {
+		GR_GPIO_SETINTTYPE(port) = mask;
+		GR_GPIO_SETINTPOL(port) = mask;
+		GR_GPIO_SETINTEN(port) = mask;
 	}
-	/* Interrupt is enabled by gpio_enable_interrupt() */
-}
-
-void gpio_set_flags_by_mask(uint32_t port, uint32_t mask, uint32_t flags)
-{
-	const struct gpio_info *g = gpio_list;
-	int i;
 
-	for (i = 0; i < GPIO_COUNT; i++, g++)
-		if ((g->port == port) && (g->mask & mask))
-			gpio_set_flags_internal(i, port, mask, flags);
+	/* No way to trigger on both rising and falling edges, darn it. */
 }
 
 void gpio_set_alternate_function(uint32_t port, uint32_t mask, int func)
 {
-	uint32_t di = 31 - __builtin_clz(mask);
+	/* This HW feature is not present in the Cr50 ARM core */
+}
 
-	/* Connect the DIO pad to a specific function */
-	GR_PINMUX_DIO_SEL(port, di) = PINMUX_FUNC(func);
-	/* Connect the specific function to the DIO pad */
-	PINMUX_SEL_REG(func) = PINMUX_DIO_SEL(port, di);
 
-	/* Input Enable (if pad used for digital function) */
-	GR_PINMUX_DIO_CTL(port, di) |= PINMUX_DIO_CTL_IE;
+static void connect_pinmux(uint32_t signal, uint32_t dio, uint16_t flags)
+{
+	if (FIELD_IS_FUNC(signal)) {
+		/* Connect peripheral function to DIO */
+		if (flags & DIO_OUTPUT) {
+			/* drive DIO from peripheral */
+			DIO_SEL_REG(dio) = PERIPH_FUNC(signal);
+		}
+
+		if (flags & DIO_INPUT) {
+			/* drive peripheral from DIO */
+			PERIPH_SEL_REG(signal) = DIO_FUNC(dio);
+			/* enable digital input */
+			REG_WRITE_MLV(DIO_CTL_REG(dio), DIO_CTL_IE_MASK,
+				      DIO_CTL_IE_LSB, 1);
+		}
+	} else {
+		/* Connect GPIO to DIO */
+		const struct gpio_info *g = gpio_list + FIELD_GET_GPIO(signal);
+		int bitnum = 31 - __builtin_clz(g->mask);
+
+		if ((g->flags & GPIO_OUTPUT) || (flags & DIO_OUTPUT)) {
+			/* drive DIO output from GPIO */
+			DIO_SEL_REG(dio) = GET_GPIO_FUNC(g->port, bitnum);
+		}
+
+		if ((g->flags & GPIO_INPUT) || (flags & DIO_INPUT)) {
+			/* drive GPIO input from DIO */
+			GET_GPIO_SEL_REG(g->port, bitnum) = DIO_FUNC(dio);
+			/* enable digital input */
+			REG_WRITE_MLV(DIO_CTL_REG(dio), DIO_CTL_IE_MASK,
+				      DIO_CTL_IE_LSB, 1);
+		}
+	}
 }
 
 int gpio_enable_interrupt(enum gpio_signal signal)
 {
-	uint32_t wi = signal >> GPIO_WORD_SIZE_LOG2;
-	uint32_t idx = signal & GPIO_WORD_MASK;
-
-	GR_GPIO_SETINTEN(wi) = 1 << idx;
-
+	const struct gpio_info *g = gpio_list + signal;
+	GR_GPIO_SETINTEN(g->port) = g->mask;
 	return EC_SUCCESS;
 }
 
 int gpio_disable_interrupt(enum gpio_signal signal)
 {
-	uint32_t wi = signal >> GPIO_WORD_SIZE_LOG2;
-	uint32_t idx = signal & GPIO_WORD_MASK;
-
-	GR_GPIO_CLRINTEN(wi) = 1 << idx;
-	GR_GPIO_CLRINTSTAT(wi) = 1 << idx;
-
+	const struct gpio_info *g = gpio_list + signal;
+	GR_GPIO_CLRINTEN(g->port) = g->mask;
 	return EC_SUCCESS;
 }
 
 void gpio_pre_init(void)
 {
 	const struct gpio_info *g = gpio_list;
+	const struct gpio_alt_func *af = gpio_alt_funcs;
+
 	int i;
 
 	/* Enable clocks */
@@ -131,22 +150,14 @@ void gpio_pre_init(void)
 		      GC_PMU_PERICLKSET0_DGPIO1_MASK,
 		      GC_PMU_PERICLKSET0_DGPIO1_LSB, 1);
 
-	/* Set all GPIOs to defaults */
-	for (i = 0; i < GPIO_COUNT; i++, g++) {
-		uint32_t di = 31 - __builtin_clz(g->mask);
-		uint32_t wi = i >> GPIO_WORD_SIZE_LOG2;
-		uint32_t idx = i & GPIO_WORD_MASK;
-
-		if (g->flags & GPIO_DEFAULT)
-			continue;
-
-		/* Set up GPIO based on flags */
-		gpio_set_flags_internal(i, g->port, g->mask, g->flags);
-		/* Connect the GPIO to the DIO pin through the muxing matrix */
-		GR_PINMUX_DIO_CTL(g->port, di) |= PINMUX_DIO_CTL_IE;
-		GR_PINMUX_DIO_SEL(g->port, di) = PINMUX_GPIO_SEL(wi, idx);
-		GR_PINMUX_GPIO_SEL(wi, idx) = PINMUX_DIO_SEL(g->port, di);
-	}
+	/* Set up the pinmux */
+	for (i = 0; i < gpio_alt_funcs_count; i++, af++)
+		connect_pinmux(af->port, af->mask, af->flags);
+
+	/* Set up ARM core GPIOs */
+	for (i = 0; i < GPIO_COUNT; i++, g++)
+		if (g->mask && !(g->flags & GPIO_DEFAULT))
+			gpio_set_flags_by_mask(g->port, g->mask, g->flags);
 }
 
 static void gpio_init(void)
@@ -157,30 +168,39 @@ static void gpio_init(void)
 DECLARE_HOOK(HOOK_INIT, gpio_init, HOOK_PRIO_DEFAULT);
 
 /*****************************************************************************/
-/* Interrupt handler */
+/* Interrupt handler stuff */
 
-static void gpio_interrupt(int wi)
+static void gpio_invoke_handler(uint32_t port, uint32_t mask)
 {
-	int idx;
 	const struct gpio_info *g = gpio_list;
-	uint32_t pending = GR_GPIO_CLRINTSTAT(wi);
+	int i;
+	for (i = 0; i < GPIO_COUNT; i++, g++)
+		if (g->irq_handler && port == g->port && (mask & g->mask))
+			g->irq_handler(i);
+}
+
+static void gpio_interrupt(int port)
+{
+	int bitnum;
+	uint32_t mask;
+	uint32_t pending = GR_GPIO_CLRINTSTAT(port);
 
 	while (pending) {
-		idx = get_next_bit(&pending) + wi * 16;
-		if (g[idx].irq_handler)
-			g[idx].irq_handler(idx);
-		/* clear the interrupt */
-		GR_GPIO_CLRINTSTAT(wi) = 1 << idx;
+		bitnum = 31 - __builtin_clz(pending);
+		mask = 1 << bitnum;
+		pending &= ~mask;
+		gpio_invoke_handler(port, mask);
+		GR_GPIO_CLRINTSTAT(port) = mask;
 	}
 }
 
 void _gpio0_interrupt(void)
 {
-	gpio_interrupt(0);
+	gpio_interrupt(GPIO_0);
 }
 void _gpio1_interrupt(void)
 {
-	gpio_interrupt(1);
+	gpio_interrupt(GPIO_1);
 }
 DECLARE_IRQ(GC_IRQNUM_GPIO0_GPIOCOMBINT, _gpio0_interrupt, 1);
 DECLARE_IRQ(GC_IRQNUM_GPIO1_GPIOCOMBINT, _gpio1_interrupt, 1);
diff --git a/chip/g/registers.h b/chip/g/registers.h
index 51c3ee0c35..e549dd091c 100644
--- a/chip/g/registers.h
+++ b/chip/g/registers.h
@@ -103,43 +103,94 @@ static inline int x_uart_addr(int ch, int offset)
 #define GR_UART_FIFO(ch)              X_UARTREG(ch, GC_UART_FIFO_OFFSET)
 #define GR_UART_RFIFO(ch)             X_UARTREG(ch, GC_UART_RFIFO_OFFSET)
 
-/* GPIOs & PIN muxing */
-#define GPIO_M_COUNT  5
-#define GPIO_A_COUNT 15
-#define GPIO_B_COUNT  9
 
-/* GPIO bank index is the number of the first GPIO of the bank */
-#define GPIO_M 0
-#define GPIO_A GPIO_M_COUNT
-#define GPIO_B (GPIO_M_COUNT + GPIO_A_COUNT)
-#define DUMMY_GPIO_BANK GPIO_M
+/* GPIO port naming scheme left over from the LM4. Must maintain tradition! */
+#define GPIO_0 0
+#define GPIO_1 1
+#define DUMMY_GPIO_BANK 0
 
-#define GR_PINMUX_DIO_SEL(bank, di) REG32(GC_PINMUX_BASE_ADDR + ((bank) + (di))*8)
-#define GR_PINMUX_DIO_CTL(bank, di) REG32(GC_PINMUX_BASE_ADDR + ((bank) + (di))*8 + 4)
-
-#define GR_PINMUX_GPIO_SEL(wi, idx) REG32(GC_PINMUX_BASE_ADDR + GC_PINMUX_GPIO0_GPIO0_SEL_OFFSET + ((wi)*16 + (idx))*4)
-
-#define PINMUX_DIO_SEL(bank, di) (GC_PINMUX_DIOM0_SEL + (di) + (bank))
-#define PINMUX_GPIO_SEL(wi, idx)  (GC_PINMUX_GPIO0_GPIO0_SEL + (idx) + (wi)*16)
+/*
+ * Our ARM core doesn't have GPIO alternate functions, but it does have a full
+ * NxM crossbar called the pinmux, which connects internal peripherals
+ * including GPIOs to external pins. We'll reuse the alternate function stuff
+ * from other ECs to configure the pinmux. This requires some clever macros
+ * that pack both a MUX selector offset (register address) and a MUX selector
+ * value (which input to choose) into a single uint32_t.
+ */
 
-#define PINMUX_DIO_CTL_DS(ds)  (((ds) & PINMUX_DIOA0_CTL_DS_GC_PINMUX_DIOA0_CTL_DS_MASK) << GC_PINMUX_DIOA0_CTL_DS_LSB)
-#define PINMUX_DIO_CTL_IE      (1 << GC_PINMUX_DIOA0_CTL_IE_LSB)
-#define PINMUX_DIO_CTL_PD      (1 << GC_PINMUX_DIOA0_CTL_PD_LSB)
-#define PINMUX_DIO_CTL_PU      (1 << GC_PINMUX_DIOA0_CTL_PU_LSB)
-#define PINMUX_DIO_CTL_INV     (1 << GC_PINMUX_DIOA0_CTL_INV_LSB)
+/* Flags to indicate the direction of the signal-to-pin connection */
+#define DIO_INPUT 0x0001
+#define DIO_OUTPUT 0x0002
 
 /*
- * To store the alternate function pin muxing in a 32-bit integer :
- * put the function index selector in the low 16 bits,
- * and the selector register offset in the high 16 bits.
+ * To store a pinmux DIO in the struct gpio_alt_func's mask field, we use:
+ *
+ *   bits 31-16: offset of the MUX selector register that drives this signal
+ *   bits 15-0:  value to write to any pinmux selector to choose this source
  */
-#define PINMUX(func) (int)(CONCAT3(GC_PINMUX_, func, _SEL) | \
-		(CONCAT3(GC_PINMUX_, func, _SEL_OFFSET) << 16))
-#define PINMUX_SEL_REG(word) REG32(GC_PINMUX_BASE_ADDR + ((uint32_t)(word) >> 16))
-#define PINMUX_FUNC(word)  ((uint32_t)(word) & 0xffff)
+#define DIO(name) (uint32_t)(CONCAT3(GC_PINMUX_DIO, name, _SEL) |	\
+			     (CONCAT3(GC_PINMUX_DIO, name, _SEL_OFFSET) << 16))
+/* Extract the MUX selector register addres for the DIO */
+#define DIO_SEL_REG(word) REG32(GC_PINMUX_BASE_ADDR +			\
+				(((uint32_t)(word) >> 16) & 0xffff))
+/* Extract the control register address for this MUX */
+#define DIO_CTL_REG(word) REG32(GC_PINMUX_BASE_ADDR + 0x4 +		\
+				(((uint32_t)(word) >> 16) & 0xffff))
+/* Extract the selector value to choose this DIO */
+#define DIO_FUNC(word) ((uint32_t)(word) & 0xffff)
+
+/*
+ * The struct gpio_alt_func's port field will either contain an enum
+ * gpio_signal from gpio_list[], or an internal peripheral function. If bit 31
+ * is clear, then bits 30-0 are the gpio_signal. If bit 31 is set, the
+ * peripheral function is packed like the DIO, above.
+ *
+ * gpio:
+ *   bit 31:     0
+ *   bit 30-0:   enum gpio_signal
+ *
+ * peripheral:
+ *   bit 31:     1
+ *   bits 30-16: offset of the MUX selector register that drives this signal
+ *   bits 15-0:  value to write to any pinmux selector to choose this source
+*/
+
+/* Which is it? */
+#define FIELD_IS_FUNC(port) (0x80000000 & (port))
+/* Encode a pinmux identifier (both a MUX and a signal name) */
+#define GPIO_FUNC(name) (uint32_t)(CONCAT3(GC_PINMUX_, name, _SEL) |	\
+				   (CONCAT3(GC_PINMUX_, name, _SEL_OFFSET) << 16) | \
+		0x80000000)
+/* Extract the MUX selector register address to drive this signal */
+#define PERIPH_SEL_REG(word) REG32(GC_PINMUX_BASE_ADDR +	\
+				   (((uint32_t)(word) >> 16) & 0x7fff))
+/* Extract the control register address for this MUX */
+#define PERIPH_CTL_REG(word) REG32(GC_PINMUX_BASE_ADDR + 0x4 +	\
+				 (((uint32_t)(word) >> 16) & 0x7fff))
+/* Extract the selector value to choose this input source */
+#define PERIPH_FUNC(word) ((uint32_t)(word) & 0xffff)
+/* Extract the GPIO signal */
+#define FIELD_GET_GPIO(word) ((uint32_t)(word) & 0x7fffffff)
+
+
+/* Map a GPIO <port,bitnum> to a selector value or register */
+#define GET_GPIO_FUNC(port, bitnum) \
+	(GC_PINMUX_GPIO0_GPIO0_SEL + 16 * port + bitnum)
+
+#define GET_GPIO_SEL_REG(port, bitnum) \
+	REG32(GC_PINMUX_BASE_ADDR + \
+	       GC_PINMUX_GPIO0_GPIO0_SEL_OFFSET + 64 * port + 4 * bitnum)
 
+/* Constants for setting MUX control bits (same bits for all DIO pins) */
+#define DIO_CTL_IE_LSB  GC_PINMUX_DIOA0_CTL_IE_LSB
+#define DIO_CTL_IE_MASK GC_PINMUX_DIOA0_CTL_IE_MASK
+#define DIO_CTL_PD_LSB  GC_PINMUX_DIOA0_CTL_PD_LSB
+#define DIO_CTL_PD_MASK GC_PINMUX_DIOA0_CTL_PD_MASK
+#define DIO_CTL_PU_LSB  GC_PINMUX_DIOA0_CTL_PU_LSB
+#define DIO_CTL_PU_MASK GC_PINMUX_DIOA0_CTL_PU_MASK
 
-#define GR_GPIO_REG(n, off)         REG16(GC_GPIO0_BASE_ADDR + (n)*0x10000 + (off))
+/* Registers controlling the ARM core GPIOs */
+#define GR_GPIO_REG(n, off)         REG16(GC_GPIO0_BASE_ADDR + (n) * 0x10000 + (off))
 #define GR_GPIO_DATAIN(n)           GR_GPIO_REG(n, GC_GPIO_DATAIN_OFFSET)
 #define GR_GPIO_DOUT(n)             GR_GPIO_REG(n, GC_GPIO_DOUT_OFFSET)
 #define GR_GPIO_SETDOUTEN(n)        GR_GPIO_REG(n, GC_GPIO_SETDOUTEN_OFFSET)
@@ -152,7 +203,8 @@ static inline int x_uart_addr(int ch, int offset)
 #define GR_GPIO_CLRINTPOL(n)        GR_GPIO_REG(n, GC_GPIO_CLRINTPOL_OFFSET)
 #define GR_GPIO_CLRINTSTAT(n)       GR_GPIO_REG(n, GC_GPIO_CLRINTSTAT_OFFSET)
 
-#define GR_GPIO_MASKBYTE(n, m)      GR_GPIO_REG(n, GC_GPIO_MASKLOWBYTE_400_OFFSET + (m)*4)
+#define GR_GPIO_MASKLOWBYTE(n, mask)  GR_GPIO_REG(n, GC_GPIO_MASKLOWBYTE_400_OFFSET + (mask) * 4)
+#define GR_GPIO_MASKHIGHBYTE(n, mask) GR_GPIO_REG(n, GC_GPIO_MASKHIGHBYTE_800_OFFSET + (mask) * 4)
 
 /*
  * High-speed timers. Two modules with two timers each; four timers total.
diff --git a/chip/g/uart.c b/chip/g/uart.c
index d43c4d4fb1..dfea855105 100644
--- a/chip/g/uart.c
+++ b/chip/g/uart.c
@@ -137,9 +137,6 @@ void uart_init(void)
 	/* turn on uart clock */
 	clock_enable_module(MODULE_UART, 1);
 
-	/* set up pinmux */
-	gpio_config_module(MODULE_UART, 1);
-
 	/* set frequency */
 	GR_UART_NCO(0) = setting;
 
diff --git a/include/config.h b/include/config.h
index 5ea6e7e4f7..18f2b45c89 100644
--- a/include/config.h
+++ b/include/config.h
@@ -659,17 +659,6 @@
 /*****************************************************************************/
 
 /*
- * Use a larger word (32 bits) to store the pin muxing configuration
- * (aka alternate function).
- *
- * Less optimal for storage size and alignment, but might be required for
- * platforms with very flexible pin muxing.
- */
-#undef CONFIG_GPIO_LARGE_ALT_INFO
-
-/*****************************************************************************/
-
-/*
  * Support the host asking the EC about the status of the most recent host
  * command.
  *
diff --git a/include/gpio.h b/include/gpio.h
index 96404a94de..f71622dfe6 100644
--- a/include/gpio.h
+++ b/include/gpio.h
@@ -70,18 +70,6 @@ struct gpio_info {
 /* Signal information from board.c.  Must match order from enum gpio_signal. */
 extern const struct gpio_info gpio_list[];
 
-/*
- * Define the storage size for the pin muxing information.
- *
- * int8_t is more optimal for storage size and alignment,
- * but some chips require to store more information.
- */
-#ifdef CONFIG_GPIO_LARGE_ALT_INFO
-typedef uint32_t alt_func_t;
-#else
-typedef int8_t alt_func_t;
-#endif
-
 /* GPIO alternate function structure, for use by board.c */
 struct gpio_alt_func {
 	/* Port base address */
@@ -91,7 +79,7 @@ struct gpio_alt_func {
 	uint32_t mask;
 
 	/* Alternate function number */
-	alt_func_t func;
+	uint8_t func;
 
 	/* Module ID (as uint8_t, since enum would be 32-bit) */
 	uint8_t module_id;