g: Clean up pinmux initialization

Now that the pinmux information isn't packed into the GPIO alternate
function table, we can expand it a bit and give the fields nice names,
making the code easier to read and removing a number of bit packing
macros defined in registers.h.

Signed-off-by: Anton Staaf <robotboy@chromium.org>

BRANCH=None
BUG=None
TEST=make buildall -j
     Verified on cr50 hardware
CQ-DEPEND=CL:*249229

Change-Id: I9984bc37faf69b1ba9f1ba66a49596dd22e3b601
Reviewed-on: https://chromium-review.googlesource.com/328979
Commit-Ready: Bill Richardson <wfrichar@chromium.org>
Tested-by: Anton Staaf <robotboy@chromium.org>
Tested-by: Bill Richardson <wfrichar@chromium.org>
Reviewed-by: Bill Richardson <wfrichar@chromium.org>

2 files changed, 93 insertions, 99 deletions

diff --git a/chip/g/gpio.c b/chip/g/gpio.c
index ad5f199215..acb13c74c0 100644
--- a/chip/g/gpio.c
+++ b/chip/g/gpio.c
@@ -84,61 +84,103 @@ void gpio_set_alternate_function(uint32_t port, uint32_t mask, int func)
 	/* This HW feature is not present in the Cr50 ARM core */
 }
 
+/*
+ * A pinmux_config contains the selector offset and selector value for a
+ * particular pinmux entry.
+ */
+struct pinmux_config {
+	uint16_t offset;
+	uint16_t value;
+};
+
+#define PINMUX_CONFIG(name) {						\
+		.offset = CONCAT3(GC_PINMUX_, name, _SEL_OFFSET),	\
+		.value  = CONCAT3(GC_PINMUX_, name, _SEL),		\
+	}
+
+/*
+ * The pinmux struct contains a full description of the connection of a DIO to
+ * a GPIO, an internal peripheral, or as a direct input.  The flag
+ * DIO_TO_PERIPHERAL is used to select between the two union entries.  There
+ * is no union entry for direct input because it requires no parameters.
+ */
 struct pinmux {
-	uint32_t signal;
-	uint32_t dio;
-	uint16_t flags;
+	union {
+		enum gpio_signal     signal;
+		struct pinmux_config peripheral;
+	};
+	struct pinmux_config dio;
+	uint16_t             flags;
 };
 
-#define GPIO_GPIO(name) GPIO_##name
-#define PINMUX(signal, dio, flags) {GPIO_##signal, DIO(dio), flags},
+/*
+ * These macros are used to add flags indicating the type of mapping requested.
+ * DIO_TO_PERIPHERAL for FUNC mappings.
+ * DIO_ENABLE_DIRECT_INPUT for DIRECT mappings.
+ */
+#define FLAGS_FUNC(name) DIO_TO_PERIPHERAL
+#define FLAGS_GPIO(name) 0
+#define FLAGS_DIRECT     DIO_ENABLE_DIRECT_INPUT
+
+/*
+ * These macros are used to selectively initialize the anonymous union based
+ * on the type of pinmux mapping requested (FUNC, GPIO, or DIRECT).
+ */
+#define PINMUX_FUNC(name) .peripheral = PINMUX_CONFIG(name),
+#define PINMUX_GPIO(name) .signal     = CONCAT2(GPIO_, name),
+#define PINMUX_DIRECT
+
+/*
+ * Initialize an entry for the pinmux list.  The first parameter can be either
+ * FUNC(name) or GPIO(name) depending on the type of mapping required.  The
+ * second argument is the DIO name to map to.  And the final argument is the
+ * flags set for this mapping, this macro adds the DIO_TO_PERIPHERAL flag for
+ * a FUNC mapping.
+ */
+#define PINMUX(name, dio_name, dio_flags) {		\
+		PINMUX_##name				\
+		.dio   = PINMUX_CONFIG(DIO##dio_name),	\
+		.flags = dio_flags | FLAGS_##name	\
+	},
 
 static const struct pinmux pinmux_list[] = {
 	#include "gpio.wrap"
 };
 
-static void connect_pinmux(struct pinmux const *p)
+static int connect_dio_to_peripheral(struct pinmux const *p)
+{
+	if (p->flags & DIO_OUTPUT)
+		DIO_SEL_REG(p->dio.offset) = p->peripheral.value;
+
+	if (p->flags & DIO_INPUT)
+		DIO_SEL_REG(p->peripheral.offset) = p->dio.value;
+
+	return p->flags & DIO_INPUT;
+}
+
+static int connect_dio_to_gpio(struct pinmux const *p)
 {
-	uint32_t signal = p->signal;
-	uint32_t dio    = p->dio;
-	uint16_t flags  = p->flags;
+	const struct gpio_info *g = gpio_list + p->signal;
+	int bitnum = GPIO_MASK_TO_NUM(g->mask);
 
-	if (flags & DIO_ENABLE_DIRECT_INPUT) {
-		/* enable digital input for direct wired peripheral */
-		REG_WRITE_MLV(DIO_CTL_REG(dio), DIO_CTL_IE_MASK,
+	if ((g->flags & GPIO_OUTPUT) || (p->flags & DIO_OUTPUT))
+		DIO_SEL_REG(p->dio.offset) = GET_GPIO_FUNC(g->port, bitnum);
+
+	if ((g->flags & GPIO_INPUT) || (p->flags & DIO_INPUT))
+		GET_GPIO_SEL_REG(g->port, bitnum) = p->dio.value;
+
+	return (g->flags & GPIO_INPUT) || (p->flags & DIO_INPUT);
+}
+
+static void connect_pinmux(struct pinmux const *p)
+{
+	if ((p->flags & DIO_ENABLE_DIRECT_INPUT) ||
+	    ((p->flags & DIO_TO_PERIPHERAL) ?
+	     connect_dio_to_peripheral(p) :
+	     connect_dio_to_gpio(p)))
+		REG_WRITE_MLV(DIO_CTL_REG(p->dio.offset),
+			      DIO_CTL_IE_MASK,
 			      DIO_CTL_IE_LSB, 1);
-	} else 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 = GPIO_MASK_TO_NUM(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)
diff --git a/chip/g/registers.h b/chip/g/registers.h
index 566bf063b3..a03b9af31b 100644
--- a/chip/g/registers.h
+++ b/chip/g/registers.h
@@ -176,67 +176,19 @@ static inline int x_uart_addr(int ch, int offset)
 /*
  * 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.
+ * including GPIOs to external pins.
  */
 
-/* Flags to indicate the direction of the signal-to-pin connection */
+/* Flags to indicate the direction and type of the signal-to-pin connection */
 #define DIO_INPUT 0x0001
 #define DIO_OUTPUT 0x0002
 #define DIO_ENABLE_DIRECT_INPUT 0x0004
+#define DIO_TO_PERIPHERAL 0x0008
 
-/*
- * 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 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)
-
+/* Generate the MUX selector register address for the DIO */
+#define DIO_SEL_REG(offset) REG32(GC_PINMUX_BASE_ADDR + offset)
+/* Generate the control register address for this MUX */
+#define DIO_CTL_REG(offset) REG32(GC_PINMUX_BASE_ADDR + 0x4 + offset)
 
 /* Map a GPIO <port,bitnum> to a selector value or register */
 #define GET_GPIO_FUNC(port, bitnum) \