Cleanup: Correct GPIO alternate function parameter

Added code to correct the GPIO alternate function parameter at Chipset
level. Optionally board level functions can cleanup the code in additional
change lists.

BUG=b:139427854
BRANCH=none
TEST=make buildall -j

Change-Id: I1171ca36a703291070fc89f972f84414adcf04fc
Signed-off-by: Vijay Hiremath <vijay.p.hiremath@intel.com>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1880974
Reviewed-by: Keith Short <keithshort@chromium.org>

14 files changed, 95 insertions, 50 deletions

diff --git a/chip/g/gpio.c b/chip/g/gpio.c
index 7d8afec2cb..d8ebe42d0d 100644
--- a/chip/g/gpio.c
+++ b/chip/g/gpio.c
@@ -123,7 +123,8 @@ void gpio_set_flags_by_mask(uint32_t port, uint32_t mask, uint32_t 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)
+void gpio_set_alternate_function(uint32_t port, uint32_t mask,
+				enum gpio_alternate_func func)
 {
 	/* This HW feature is not present in the Cr50 ARM core */
 }
diff --git a/chip/host/gpio.c b/chip/host/gpio.c
index 6033902720..ace8cbd482 100644
--- a/chip/host/gpio.c
+++ b/chip/host/gpio.c
@@ -80,7 +80,7 @@ test_mockable void gpio_set_flags_by_mask(uint32_t port, uint32_t mask,
 }
 
 test_mockable void gpio_set_alternate_function(uint32_t port, uint32_t mask,
-					       int func)
+						enum gpio_alternate_func func)
 {
 	/* Nothing */
 }
diff --git a/chip/it83xx/gpio.c b/chip/it83xx/gpio.c
index 158c267f9f..19a244c93a 100644
--- a/chip/it83xx/gpio.c
+++ b/chip/it83xx/gpio.c
@@ -368,21 +368,28 @@ static void gpio_1p8v_3p3v_sel_by_pin(uint8_t port, uint8_t pin, int sel_1p8v)
 		*reg_1p8v &= ~sel;
 }
 
-void gpio_set_alternate_function(uint32_t port, uint32_t mask, int func)
+static inline void it83xx_set_alt_func(uint32_t port, uint32_t pin,
+				enum gpio_alternate_func func)
+{
+	/*
+	 * If func is not ALT_FUNC_NONE, set for alternate function.
+	 * Otherwise, turn the pin into an input as it's default.
+	 */
+	if (func != GPIO_ALT_FUNC_NONE)
+		IT83XX_GPIO_CTRL(CTRL_BASE(port), pin) &= ~0xc0;
+	else
+		IT83XX_GPIO_CTRL(CTRL_BASE(port), pin) =
+			(IT83XX_GPIO_CTRL(CTRL_BASE(port), pin) | 0x80) & ~0x40;
+}
+void gpio_set_alternate_function(uint32_t port, uint32_t mask,
+				enum gpio_alternate_func func)
 {
 	uint32_t pin = 0;
 
 	/* For each bit high in the mask, set that pin to use alt. func. */
 	while (mask > 0) {
-		/*
-		 * If func is non-negative, set for alternate function.
-		 * Otherwise, turn the pin into an input as it's default.
-		 */
-		if ((mask & 1) && func >= 0)
-			IT83XX_GPIO_CTRL(CTRL_BASE(port), pin) &= ~0xc0;
-		else if ((mask & 1) && func < 0)
-			IT83XX_GPIO_CTRL(CTRL_BASE(port), pin) =
-			(IT83XX_GPIO_CTRL(CTRL_BASE(port), pin) | 0x80) & ~0x40;
+		if (mask & 1)
+			it83xx_set_alt_func(port, pin, func);
 
 		pin++;
 		mask >>= 1;
diff --git a/chip/lm4/gpio.c b/chip/lm4/gpio.c
index b3dff342e8..65d6548f90 100644
--- a/chip/lm4/gpio.c
+++ b/chip/lm4/gpio.c
@@ -42,7 +42,8 @@ static int find_gpio_port_index(uint32_t port_base)
 	return -1;
 }
 
-void gpio_set_alternate_function(uint32_t port, uint32_t mask, int func)
+void gpio_set_alternate_function(uint32_t port, uint32_t mask,
+				enum gpio_alternate_func func)
 {
 	int port_index = find_gpio_port_index(port);
 	int cgmask;
@@ -56,7 +57,7 @@ void gpio_set_alternate_function(uint32_t port, uint32_t mask, int func)
 	clock_enable_peripheral(CGC_OFFSET_GPIO, cgmask,
 			CGC_MODE_RUN | CGC_MODE_SLEEP);
 
-	if (func >= 0) {
+	if (func != GPIO_ALT_FUNC_NONE) {
 		int pctlmask = 0;
 		int i;
 		/* Expand mask from bits to nibbles */
@@ -261,7 +262,8 @@ void gpio_pre_init(void)
 		gpio_set_flags_by_mask(g->port, g->mask, flags);
 
 		/* Use as GPIO, not alternate function */
-		gpio_set_alternate_function(g->port, g->mask, -1);
+		gpio_set_alternate_function(g->port, g->mask,
+					GPIO_ALT_FUNC_NONE);
 	}
 
 #ifdef CONFIG_LOW_POWER_IDLE
diff --git a/chip/max32660/gpio_chip.c b/chip/max32660/gpio_chip.c
index af2e96fd29..1ed7d386ae 100644
--- a/chip/max32660/gpio_chip.c
+++ b/chip/max32660/gpio_chip.c
@@ -23,20 +23,21 @@
 /* 0-terminated list of GPIO base addresses */
 static mxc_gpio_regs_t *gpio_bases[] = {MXC_GPIO0, 0};
 
-void gpio_set_alternate_function(uint32_t port, uint32_t mask, int func)
+void gpio_set_alternate_function(uint32_t port, uint32_t mask,
+			enum gpio_alternate_func func)
 {
 	mxc_gpio_regs_t *gpio = MXC_GPIO_GET_GPIO(port);
 
 	switch (func) {
-	case 1:
+	case GPIO_ALT_FUNC_1:
 		gpio->en_clr = mask;
 		gpio->en1_clr = mask;
 		break;
-	case 2:
+	case GPIO_ALT_FUNC_2:
 		gpio->en_clr = mask;
 		gpio->en1_set = mask;
 		break;
-	case 3:
+	case GPIO_ALT_FUNC_3:
 		gpio->en_set = mask;
 		gpio->en1_set = mask;
 		break;
@@ -167,7 +168,8 @@ void gpio_pre_init(void)
 			continue;
 
 		/* Use as GPIO, not alternate function */
-		gpio_set_alternate_function(g->port, g->mask, -1);
+		gpio_set_alternate_function(g->port, g->mask,
+					GPIO_ALT_FUNC_NONE);
 
 		/* Set up GPIO based on flags */
 		gpio_set_flags_by_mask(g->port, g->mask, flags);
diff --git a/chip/mchp/gpio.c b/chip/mchp/gpio.c
index eed5c3efbc..1de74dafcc 100644
--- a/chip/mchp/gpio.c
+++ b/chip/mchp/gpio.c
@@ -49,7 +49,8 @@ static const struct gpio_int_mapping int_map[6] = {
  * NOTE: GCC __builtin_ffs(val) returns (index + 1) of least significant
  * 1-bit of val or if val == 0 returns 0
  */
-void gpio_set_alternate_function(uint32_t port, uint32_t mask, int func)
+void gpio_set_alternate_function(uint32_t port, uint32_t mask,
+				enum gpio_alternate_func func)
 {
 	int i;
 	uint32_t val;
@@ -58,8 +59,8 @@ void gpio_set_alternate_function(uint32_t port, uint32_t mask, int func)
 		i = __builtin_ffs(mask) - 1;
 		val = MCHP_GPIO_CTL(port, i);
 		val &= ~(BIT(12) | BIT(13));
-		/* mux_control = 0 indicates GPIO */
-		if (func > 0)
+		/* mux_control = DEFAULT, indicates GPIO */
+		if (func > GPIO_ALT_FUNC_DEFAULT)
 			val |= (func & 0x3) << 12;
 		MCHP_GPIO_CTL(port, i) = val;
 		mask &= ~BIT(i);
@@ -325,7 +326,8 @@ void gpio_pre_init(void)
 		gpio_set_flags_by_mask(g->port, g->mask, flags);
 
 		/* Use as GPIO, not alternate function */
-		gpio_set_alternate_function(g->port, g->mask, -1);
+		gpio_set_alternate_function(g->port, g->mask,
+					GPIO_ALT_FUNC_NONE);
 	}
 }
 
diff --git a/chip/mec1322/gpio.c b/chip/mec1322/gpio.c
index fee9bfa79e..331022c87c 100644
--- a/chip/mec1322/gpio.c
+++ b/chip/mec1322/gpio.c
@@ -29,7 +29,8 @@ static const struct gpio_int_mapping int_map[22] = {
 	{20, 20}, {20, 20}
 };
 
-void gpio_set_alternate_function(uint32_t port, uint32_t mask, int func)
+void gpio_set_alternate_function(uint32_t port, uint32_t mask,
+				enum gpio_alternate_func func)
 {
 	int i;
 	uint32_t val;
@@ -38,8 +39,8 @@ void gpio_set_alternate_function(uint32_t port, uint32_t mask, int func)
 		i = __builtin_ffs(mask) - 1;
 		val = MEC1322_GPIO_CTL(port, i);
 		val &= ~(BIT(12) | BIT(13));
-		/* mux_control = 0 indicates GPIO */
-		if (func > 0)
+		/* mux_control = DEFAULT, indicates GPIO */
+		if (func > GPIO_ALT_FUNC_DEFAULT)
 			val |= (func & 0x3) << 12;
 		MEC1322_GPIO_CTL(port, i) = val;
 		mask &= ~BIT(i);
@@ -215,7 +216,8 @@ void gpio_pre_init(void)
 		gpio_set_flags_by_mask(g->port, g->mask, flags);
 
 		/* Use as GPIO, not alternate function */
-		gpio_set_alternate_function(g->port, g->mask, -1);
+		gpio_set_alternate_function(g->port, g->mask,
+					GPIO_ALT_FUNC_NONE);
 	}
 }
 
diff --git a/chip/mt_scp/gpio.c b/chip/mt_scp/gpio.c
index 896baab93a..736a961951 100644
--- a/chip/mt_scp/gpio.c
+++ b/chip/mt_scp/gpio.c
@@ -12,13 +12,14 @@
 #include "task.h"
 #include "util.h"
 
-void gpio_set_alternate_function(uint32_t port, uint32_t mask, int func)
+void gpio_set_alternate_function(uint32_t port, uint32_t mask,
+				enum gpio_alternate_func func)
 {
 	int bit, mode_reg_index, shift;
 	uint32_t mode_bits, mode_mask;
 
 	/* Up to 8 alt functions per port */
-	if (func > 7)
+	if (func > GPIO_ALT_FUNC_7)
 		return;
 
 	while (mask) {
diff --git a/chip/npcx/gpio.c b/chip/npcx/gpio.c
index 4f7a50b672..e20319ab0f 100644
--- a/chip/npcx/gpio.c
+++ b/chip/npcx/gpio.c
@@ -113,7 +113,8 @@ static uint8_t gpio_is_alt_sel(uint8_t port, uint8_t bit)
 }
 #endif
 
-static int gpio_alt_sel(uint8_t port, uint8_t bit, int8_t func)
+static int gpio_alt_sel(uint8_t port, uint8_t bit,
+			enum gpio_alternate_func func)
 {
 	struct gpio_alt_map const *map;
 
@@ -124,10 +125,10 @@ static int gpio_alt_sel(uint8_t port, uint8_t bit, int8_t func)
 			uint8_t alt_mask = 1 << map->alt.bit;
 
 			/*
-			 * func < 0          -> GPIO functionality
+			 * func < GPIO_ALT_FUNC_DEFAULT -> GPIO functionality
 			 * map->alt.inverted -> Set DEVALT bit for GPIO
 			 */
-			if ((func < 0) ^ map->alt.inverted)
+			if ((func < GPIO_ALT_FUNC_DEFAULT) ^ map->alt.inverted)
 				NPCX_DEVALT(map->alt.group) &= ~alt_mask;
 			else
 				NPCX_DEVALT(map->alt.group) |=  alt_mask;
@@ -136,7 +137,7 @@ static int gpio_alt_sel(uint8_t port, uint8_t bit, int8_t func)
 		}
 	}
 
-	if (func > 0)
+	if (func > GPIO_ALT_FUNC_DEFAULT)
 		CPRINTS("Warn! No alter func in port%d, pin%d", port, bit);
 
 	return -1;
@@ -306,7 +307,8 @@ BUILD_ASSERT(ARRAY_SIZE(gpio_lvol_table[0].lvol_gpio) == 8);
 /*****************************************************************************/
 /* IC specific low-level driver */
 
-void gpio_set_alternate_function(uint32_t port, uint32_t mask, int func)
+void gpio_set_alternate_function(uint32_t port, uint32_t mask,
+				enum gpio_alternate_func func)
 {
 	/* Enable alternative pins by func*/
 	int pin;
@@ -583,7 +585,8 @@ void gpio_pre_init(void)
 		 * configured as a GPIO, and not left in its default state,
 		 * which may or may not be as a GPIO.
 		 */
-		gpio_set_alternate_function(g->port, g->mask, -1);
+		gpio_set_alternate_function(g->port, g->mask,
+					GPIO_ALT_FUNC_NONE);
 	}
 
 	/* The bypass of low voltage IOs for better power consumption */
diff --git a/chip/nrf51/gpio.c b/chip/nrf51/gpio.c
index 48c9f39895..53694b5a74 100644
--- a/chip/nrf51/gpio.c
+++ b/chip/nrf51/gpio.c
@@ -53,7 +53,8 @@ volatile uint32_t * const nrf51_alt_funcs[] = {
 const unsigned int nrf51_alt_func_count = ARRAY_SIZE(nrf51_alt_funcs);
 
 /* Make sure the function table and defines stay in sync */
-BUILD_ASSERT(NRF51_MAX_ALT_FUNCS == ARRAY_SIZE(nrf51_alt_funcs));
+BUILD_ASSERT(ARRAY_SIZE(nrf51_alt_funcs) == NRF51_MAX_ALT_FUNCS &&
+	NRF51_MAX_ALT_FUNCS <= GPIO_ALT_FUNC_MAX);
 
 void gpio_set_flags_by_mask(uint32_t port, uint32_t mask, uint32_t flags)
 {
@@ -160,16 +161,18 @@ void gpio_pre_init(void)
  * NRF51 doesn't have an alternate function table.
  * Use the pin select registers in place of the function number.
  */
-void gpio_set_alternate_function(uint32_t port, uint32_t mask, int func)
+void gpio_set_alternate_function(uint32_t port, uint32_t mask,
+				enum gpio_alternate_func func)
 {
 	uint32_t bit = GPIO_MASK_TO_NUM(mask);
 
 	ASSERT((~mask & BIT(bit)) == 0); /* Only one bit set. */
 	ASSERT(port == GPIO_0);
-	ASSERT((func >= 0 && func < nrf51_alt_func_count) || func == -1);
+	ASSERT((func >= GPIO_ALT_FUNC_DEFAULT && func < nrf51_alt_func_count) ||
+		func == GPIO_ALT_FUNC_NONE);
 
 	/* Remove the previous setting(s) */
-	if (func == -1) {
+	if (func == GPIO_ALT_FUNC_NONE) {
 		int i;
 		for (i = 0; i < nrf51_alt_func_count; i++) {
 			if (*(nrf51_alt_funcs[i]) == bit)
diff --git a/chip/stm32/gpio-f0-l.c b/chip/stm32/gpio-f0-l.c
index 89dd47455d..55628cb6d4 100644
--- a/chip/stm32/gpio-f0-l.c
+++ b/chip/stm32/gpio-f0-l.c
@@ -132,14 +132,18 @@ void gpio_set_flags_by_mask(uint32_t port, uint32_t mask, uint32_t flags)
 	/* Interrupt is enabled by gpio_enable_interrupt() */
 }
 
-void gpio_set_alternate_function(uint32_t port, uint32_t mask, int func)
+void gpio_set_alternate_function(uint32_t port, uint32_t mask,
+				enum gpio_alternate_func func)
 {
+	/* Ensure that the func parameter isn't overflowed */
+	BUILD_ASSERT((int) MODULE_COUNT <= (int) GPIO_ALT_FUNC_MAX);
+
 	int bit;
 	uint32_t half;
 	uint32_t afr;
 	uint32_t moder = STM32_GPIO_MODER(port);
 
-	if (func < 0) {
+	if (func == GPIO_ALT_FUNC_NONE) {
 		/* Return to normal GPIO function, defaulting to input. */
 		while (mask) {
 			bit = get_next_bit(&mask);
diff --git a/common/gpio.c b/common/gpio.c
index fe02a15248..3dc8bf0367 100644
--- a/common/gpio.c
+++ b/common/gpio.c
@@ -18,7 +18,7 @@ struct gpio_alt_func {
 	uint8_t module_id;
 
 	/* Alternate function number */
-	uint8_t func;
+	enum gpio_alternate_func func;
 
 	/* Port base address */
 	uint32_t port;
@@ -75,8 +75,8 @@ static int gpio_config_pins(enum module_id id, uint32_t port, uint32_t pin_mask,
 					af->port, (af->mask & pin_mask),
 					enable ? af->flags : GPIO_INPUT);
 			gpio_set_alternate_function(af->port,
-						    (af->mask & pin_mask),
-						    enable ? af->func : -1);
+				    (af->mask & pin_mask),
+				    enable ? af->func : GPIO_ALT_FUNC_NONE);
 			rv = EC_SUCCESS;
 			/* We're done here if we were just setting one port. */
 			if (port != GPIO_CONFIG_ALL_PORTS)
@@ -128,7 +128,7 @@ void gpio_reset(enum gpio_signal signal)
 	const struct gpio_info *g = gpio_list + signal;
 
 	gpio_set_flags_by_mask(g->port, g->mask, g->flags);
-	gpio_set_alternate_function(g->port, g->mask, -1);
+	gpio_set_alternate_function(g->port, g->mask, GPIO_ALT_FUNC_NONE);
 }
 
 const char *gpio_get_name(enum gpio_signal signal)
diff --git a/common/keyboard_scan.c b/common/keyboard_scan.c
index 7f7d44ceec..b0b182fb47 100644
--- a/common/keyboard_scan.c
+++ b/common/keyboard_scan.c
@@ -882,7 +882,8 @@ int keyboard_factory_test_scan(void)
 		port = keyboard_factory_scan_pins[i][0];
 		id = keyboard_factory_scan_pins[i][1];
 
-		gpio_set_alternate_function(port, 1 << id, -1);
+		gpio_set_alternate_function(port, 1 << id,
+					GPIO_ALT_FUNC_NONE);
 		gpio_set_flags_by_mask(port, 1 << id,
 			GPIO_INPUT | GPIO_PULL_UP);
 	}
diff --git a/include/gpio.h b/include/gpio.h
index 3b39ad9314..e5e8afe24a 100644
--- a/include/gpio.h
+++ b/include/gpio.h
@@ -11,6 +11,7 @@
 #include "common.h"
 
 /* Flag definitions for gpio_info and gpio_alt_func */
+#define GPIO_FLAG_NONE     0       /* No flag needed, default setting */
 /* The following are valid for both gpio_info and gpio_alt_func: */
 #define GPIO_OPEN_DRAIN    BIT(0)  /* Output type is open-drain */
 #define GPIO_PULL_UP       BIT(1)  /* Enable on-chip pullup */
@@ -64,6 +65,21 @@ enum gpio_signal {
 };
 #endif /* __CROS_EC_GPIO_SIGNAL_H */
 
+/* Alternate functions for GPIOs */
+enum gpio_alternate_func {
+	GPIO_ALT_FUNC_NONE = -1,
+	GPIO_ALT_FUNC_DEFAULT,
+	GPIO_ALT_FUNC_1,
+	GPIO_ALT_FUNC_2,
+	GPIO_ALT_FUNC_3,
+	GPIO_ALT_FUNC_4,
+	GPIO_ALT_FUNC_5,
+	GPIO_ALT_FUNC_6,
+	GPIO_ALT_FUNC_7,
+
+	GPIO_ALT_FUNC_MAX = 63,
+};
+
 /* GPIO signal definition structure, for use by board.c */
 struct gpio_info {
 	/* Signal name */
@@ -278,10 +294,11 @@ void gpio_set_flags_by_mask(uint32_t port, uint32_t mask, uint32_t flags);
  *
  * @param port		GPIO port to set (GPIO_*)
  * @param mask		Bitmask of pins on that port to affect
- * @param func		Alternate function; if <0, configures the specified
- *			GPIOs for normal GPIO operation.
+ * @param func		Alternate function; if GPIO_ALT_FUNC_NONE, configures
+ *                      the specified GPIOs for normal GPIO operation.
  */
-void gpio_set_alternate_function(uint32_t port, uint32_t mask, int func);
+void gpio_set_alternate_function(uint32_t port, uint32_t mask,
+				enum gpio_alternate_func func);
 
 #ifdef CONFIG_GPIO_POWER_DOWN
 /**