From 3b390264a415ce121a8c6f8db9fa9c42c647aaec Mon Sep 17 00:00:00 2001 From: Vijay Hiremath Date: Fri, 25 Oct 2019 03:38:30 -0700 Subject: 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 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1880974 Reviewed-by: Keith Short --- chip/g/gpio.c | 3 ++- chip/host/gpio.c | 2 +- chip/it83xx/gpio.c | 27 +++++++++++++++++---------- chip/lm4/gpio.c | 8 +++++--- chip/max32660/gpio_chip.c | 12 +++++++----- chip/mchp/gpio.c | 10 ++++++---- chip/mec1322/gpio.c | 10 ++++++---- chip/mt_scp/gpio.c | 5 +++-- chip/npcx/gpio.c | 15 +++++++++------ chip/nrf51/gpio.c | 11 +++++++---- chip/stm32/gpio-f0-l.c | 8 ++++++-- common/gpio.c | 8 ++++---- common/keyboard_scan.c | 3 ++- include/gpio.h | 23 ++++++++++++++++++++--- 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 /** -- cgit v1.2.1