diff options
author | Randall Spangler <rspangler@chromium.org> | 2012-10-24 12:58:33 -0700 |
---|---|---|
committer | Gerrit <chrome-bot@google.com> | 2012-10-25 11:24:02 -0700 |
commit | 2957c3cf8b2275a1a6803bc4e141a30533cc2f1e (patch) | |
tree | ffe990e9e523b93023375dc0cce28a767adbbd02 | |
parent | cbee62d01e94ef5fc8539742ca995cc62b0ee96a (diff) | |
download | chrome-ec-2957c3cf8b2275a1a6803bc4e141a30533cc2f1e.tar.gz |
Clean up GPIO module
Just code cleanup; no functional changes
BUG=chrome-os-partner:15579
BRANCH=none
TEST=build code; boot link; gpioget still works
Change-Id: If0770c1a5ce0d5c51ba528fbe2944a73fafa949b
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://gerrit.chromium.org/gerrit/36556
Reviewed-by: Simon Glass <sjg@chromium.org>
-rw-r--r-- | chip/lm4/gpio.c | 71 | ||||
-rw-r--r-- | chip/stm32/gpio-stm32f100.c | 40 | ||||
-rw-r--r-- | chip/stm32/gpio-stm32l15x.c | 19 | ||||
-rw-r--r-- | chip/stm32/gpio.c | 2 | ||||
-rw-r--r-- | common/gpio_commands.c | 35 | ||||
-rw-r--r-- | include/gpio.h | 74 |
6 files changed, 138 insertions, 103 deletions
diff --git a/chip/lm4/gpio.c b/chip/lm4/gpio.c index 23cedd52c3..b5b1105632 100644 --- a/chip/lm4/gpio.c +++ b/chip/lm4/gpio.c @@ -5,7 +5,8 @@ /* GPIO module for Chrome EC */ -#include "board.h" +#include "clock.h" +#include "common.h" #include "gpio.h" #include "hooks.h" #include "power_button.h" @@ -14,8 +15,7 @@ #include "timer.h" #include "util.h" - -/* 0-terminated list of GPIO bases */ +/* 0-terminated list of GPIO base addresses */ static const uint32_t gpio_bases[] = { LM4_GPIO_A, LM4_GPIO_B, LM4_GPIO_C, LM4_GPIO_D, LM4_GPIO_E, LM4_GPIO_F, LM4_GPIO_G, LM4_GPIO_H, @@ -23,9 +23,15 @@ static const uint32_t gpio_bases[] = { LM4_GPIO_N, LM4_GPIO_P, LM4_GPIO_Q, 0 }; - -/* Find the index of a GPIO port base address (LM4_GPIO_[A-Q]); this is used by - * the clock gating registers. Returns the index, or -1 if no match. */ +/** + * Find the index of a GPIO port base address + * + * This is used by the clock gating registers. + * + * @param port_base Base address to find (LM4_GPIO_[A-Q]) + * + * @return The index, or -1 if no match. + */ static int find_gpio_port_index(uint32_t port_base) { int i; @@ -36,8 +42,7 @@ static int find_gpio_port_index(uint32_t port_base) return -1; } - -int gpio_pre_init(void) +void gpio_pre_init(void) { volatile uint32_t scratch __attribute__((unused)); const struct gpio_info *g = gpio_list; @@ -84,8 +89,6 @@ int gpio_pre_init(void) if ((g->flags & GPIO_OUTPUT) && !is_warm) gpio_set_level(i, g->flags & GPIO_HIGH); } - - return EC_SUCCESS; } static void gpio_init(void) @@ -104,6 +107,7 @@ static void gpio_init(void) task_enable_irq(LM4_IRQ_GPIOL); task_enable_irq(LM4_IRQ_GPIOM); #if defined(KB_SCAN_ROW_IRQ) && (KB_SCAN_ROW_IRQ != LM4_IRQ_GPION) + /* Don't enable interrupts for the keyboard input GPIO bank */ task_enable_irq(LM4_IRQ_GPION); #endif task_enable_irq(LM4_IRQ_GPIOP); @@ -122,11 +126,9 @@ void gpio_set_alternate_function(int port, int mask, int func) /* Enable the GPIO port if necessary */ cgmask = 1 << port_index; if ((LM4_SYSTEM_RCGCGPIO & cgmask) != cgmask) { - volatile uint32_t scratch __attribute__((unused)); LM4_SYSTEM_RCGCGPIO |= cgmask; - /* Delay a few clocks before accessing GPIO registers on that - * port. */ - scratch = LM4_SYSTEM_RCGCGPIO; + /* Delay a few clocks before accessing registers */ + clock_wait_cycles(3); } if (func) { @@ -148,40 +150,39 @@ void gpio_set_alternate_function(int port, int mask, int func) LM4_GPIO_DEN(port) |= mask; } - const char *gpio_get_name(enum gpio_signal signal) { return gpio_list[signal].name; } - int gpio_get_level(enum gpio_signal signal) { return LM4_GPIO_DATA(gpio_list[signal].port, gpio_list[signal].mask) ? 1 : 0; } - -int gpio_set_level(enum gpio_signal signal, int value) +void gpio_set_level(enum gpio_signal signal, int value) { - /* Ok to write 0xff becuase LM4_GPIO_DATA bit-masks only the bit - * we care about. */ + /* + * Ok to write 0xff becuase LM4_GPIO_DATA bit-masks only the bit + * we care about. + */ LM4_GPIO_DATA(gpio_list[signal].port, gpio_list[signal].mask) = (value ? 0xff : 0); - return EC_SUCCESS; } - -int gpio_set_flags(enum gpio_signal signal, int flags) +void gpio_set_flags(enum gpio_signal signal, int flags) { const struct gpio_info *g = gpio_list + signal; if (flags & GPIO_DEFAULT) - return EC_SUCCESS; + return; + if (flags & GPIO_OUTPUT) { - /* Output */ - /* Select open drain first, so that we don't glitch the signal - * when changing the line to an output. */ + /* + * Select open drain first, so that we don't glitch the signal + * when changing the line to an output. + */ if (g->flags & GPIO_OPEN_DRAIN) LM4_GPIO_ODR(g->port) |= g->mask; else @@ -220,11 +221,8 @@ int gpio_set_flags(enum gpio_signal signal, int flags) LM4_GPIO_IBE(g->port) |= g->mask; else LM4_GPIO_IBE(g->port) &= ~g->mask; - - return EC_SUCCESS; } - int gpio_enable_interrupt(enum gpio_signal signal) { const struct gpio_info *g = gpio_list + signal; @@ -251,8 +249,10 @@ static void gpio_interrupt(int port, uint32_t mis) } } -/* Handlers for each GPIO port. These read and clear the interrupt bits for - * the port, then call the master handler above. */ +/** + * Handlers for each GPIO port. These read and clear the interrupt bits for + * the port, then call the master handler above. + */ #define GPIO_IRQ_FUNC(irqfunc, gpiobase) \ static void irqfunc(void) \ { \ @@ -281,9 +281,10 @@ GPIO_IRQ_FUNC(__gpio_q_interrupt, LM4_GPIO_Q); #undef GPIO_IRQ_FUNC -/* Declare IRQs */ -/* TODO: nesting this macro inside the GPIO_IRQ_FUNC macro works poorly because - * DECLARE_IRQ() stringizes its inputs. */ +/* + * Declare IRQs. Nesting this macro inside the GPIO_IRQ_FUNC macro works + * poorly because DECLARE_IRQ() stringizes its inputs. + */ DECLARE_IRQ(LM4_IRQ_GPIOA, __gpio_a_interrupt, 1); DECLARE_IRQ(LM4_IRQ_GPIOB, __gpio_b_interrupt, 1); DECLARE_IRQ(LM4_IRQ_GPIOC, __gpio_c_interrupt, 1); diff --git a/chip/stm32/gpio-stm32f100.c b/chip/stm32/gpio-stm32f100.c index 5d91ff343e..b308138e93 100644 --- a/chip/stm32/gpio-stm32f100.c +++ b/chip/stm32/gpio-stm32f100.c @@ -5,7 +5,7 @@ /* GPIO module for Chrome EC */ -#include "config.h" +#include "common.h" #include "console.h" #include "gpio.h" #include "hooks.h" @@ -32,7 +32,9 @@ struct port_config { uint32_t cnf; }; -/* helper function for generating bitmasks for STM32 GPIO config registers */ +/** + * Helper function for generating bitmasks for STM32 GPIO config registers + */ static void gpio_config_info(const struct gpio_info *g, uint32_t *addr, uint32_t *mode, uint32_t *cnf) { /* @@ -52,13 +54,14 @@ static void gpio_config_info(const struct gpio_info *g, uint32_t *addr, *cnf = *mode << 2; } -int gpio_set_flags(enum gpio_signal signal, int flags) +void gpio_set_flags(enum gpio_signal signal, int flags) { const struct gpio_info *g = gpio_list + signal; uint32_t addr, cnf, mode, mask; if (flags & GPIO_DEFAULT) - return EC_SUCCESS; + return; + gpio_config_info(g, &addr, &mode, &cnf); mask = REG32(addr) & ~(cnf | mode); @@ -68,13 +71,15 @@ int gpio_set_flags(enum gpio_signal signal, int flags) * output, or alternate function. */ if (flags & GPIO_OUTPUT) { - /* FIXME: This assumes output max speed of 10MHz */ + /* TODO: This assumes output max speed of 10MHz */ mask |= 0x11111111 & mode; if (flags & GPIO_OPEN_DRAIN) mask |= 0x44444444 & cnf; } else { - /* GPIOx_ODR determines which resistor to activate in - * input mode, see Table 16 (datasheet rm0041) */ + /* + * GPIOx_ODR determines which resistor to activate in + * input mode, see Table 16 (datasheet rm0041) + */ if ((flags & GPIO_PULL_UP) == GPIO_PULL_UP) { mask |= 0x88888888 & cnf; STM32_GPIO_BSRR_OFF(g->port) |= g->mask; @@ -95,9 +100,9 @@ int gpio_set_flags(enum gpio_signal signal, int flags) if ((flags & GPIO_OUTPUT) && !is_warm_boot) /* General purpose, MODE = 01 * - * If this is a cold boot, set the level. - * On a warm reboot, leave things where they were - * or we'll shut off the AP. */ + * If this is a cold boot, set the level. On a warm reboot, + * leave things where they were or we'll shut off the AP. + */ gpio_set_level(signal, flags & GPIO_HIGH); REG32(addr) = mask; @@ -109,12 +114,9 @@ int gpio_set_flags(enum gpio_signal signal, int flags) if (flags & (GPIO_INT_FALLING | GPIO_INT_BOTH)) STM32_EXTI_FTSR |= g->mask; /* Interrupt is enabled by gpio_enable_interrupt() */ - - return EC_SUCCESS; } - -int gpio_pre_init(void) +void gpio_pre_init(void) { const struct gpio_info *g = gpio_list; int i; @@ -123,7 +125,9 @@ int gpio_pre_init(void) /* This is a warm reboot : TIM2 is already active */ is_warm_boot = 1; } else { - /* Enable all GPIOs clocks + /* + * Enable all GPIOs clocks + * * TODO: more fine-grained enabling for power saving */ STM32_RCC_APB2ENR |= 0x1fd; @@ -132,8 +136,6 @@ int gpio_pre_init(void) /* Set all GPIOs to defaults */ for (i = 0; i < GPIO_COUNT; i++, g++) gpio_set_flags(i, g->flags); - - return EC_SUCCESS; } void gpio_init(void) @@ -163,12 +165,10 @@ int gpio_get_level(enum gpio_signal signal) } -int gpio_set_level(enum gpio_signal signal, int value) +void gpio_set_level(enum gpio_signal signal, int value) { STM32_GPIO_BSRR_OFF(gpio_list[signal].port) = gpio_list[signal].mask << (value ? 0 : 16); - - return EC_SUCCESS; } int gpio_enable_interrupt(enum gpio_signal signal) diff --git a/chip/stm32/gpio-stm32l15x.c b/chip/stm32/gpio-stm32l15x.c index 3984eccaf6..659e05df78 100644 --- a/chip/stm32/gpio-stm32l15x.c +++ b/chip/stm32/gpio-stm32l15x.c @@ -20,16 +20,15 @@ /* For each EXTI bit, record which GPIO entry is using it */ static const struct gpio_info *exti_events[16]; -int gpio_set_flags(enum gpio_signal signal, int flags) +void gpio_set_flags(enum gpio_signal signal, int flags) { /* * TODO(dhendrix): Move GPIO setup code from gpio_pre_init * into here like we did for STM32F */ - return EC_ERROR_UNIMPLEMENTED; } -int gpio_pre_init(void) +void gpio_pre_init(void) { const struct gpio_info *g = gpio_list; int is_warm = 0; @@ -73,9 +72,11 @@ int gpio_pre_init(void) if (g->flags & GPIO_OUTPUT) { /* General purpose, MODE = 01 */ val |= 0x55555555 & mask2; STM32_GPIO_MODER_OFF(g->port) = val; - /* If this is a cold boot, set the level. - * On a warm reboot, leave things where they were - * or we'll shut off the AP. */ + /* + * If this is a cold boot, set the level. On a warm + * reboot, leave things where they were or we'll shut + * off the AP. + */ if (!is_warm) gpio_set_level(i, g->flags & GPIO_HIGH); } else if (g->flags & GPIO_INPUT) { /* Input, MODE=00 */ @@ -90,8 +91,6 @@ int gpio_pre_init(void) STM32_EXTI_FTSR |= g->mask; /* Interrupt is enabled by gpio_enable_interrupt() */ } - - return EC_SUCCESS; } static void gpio_init(void) @@ -150,12 +149,10 @@ int gpio_get_level(enum gpio_signal signal) } -int gpio_set_level(enum gpio_signal signal, int value) +void gpio_set_level(enum gpio_signal signal, int value) { STM32_GPIO_BSRR_OFF(gpio_list[signal].port) = gpio_list[signal].mask << (value ? 0 : 16); - - return EC_SUCCESS; } int gpio_enable_interrupt(enum gpio_signal signal) diff --git a/chip/stm32/gpio.c b/chip/stm32/gpio.c index a2d5b4daf7..d488c9c619 100644 --- a/chip/stm32/gpio.c +++ b/chip/stm32/gpio.c @@ -5,7 +5,7 @@ /* Common GPIO module for Chrome EC */ -#include "board.h" +#include "common.h" #include "gpio.h" diff --git a/common/gpio_commands.c b/common/gpio_commands.c index 2768540115..e429bc09a9 100644 --- a/common/gpio_commands.c +++ b/common/gpio_commands.c @@ -12,12 +12,15 @@ #include "system.h" #include "util.h" - static uint8_t last_val[(GPIO_COUNT + 7) / 8]; - -/* Find a GPIO signal by name. Returns the signal index, or GPIO_COUNT if - * no match. */ +/** + * Find a GPIO signal by name. + * + * @param name Signal name to find + * + * @return the signal index, or GPIO_COUNT if no match. + */ static enum gpio_signal find_signal_by_name(const char *name) { const struct gpio_info *g = gpio_list; @@ -34,9 +37,14 @@ static enum gpio_signal find_signal_by_name(const char *name) return GPIO_COUNT; } - -/* If v is different from the last value for index i, updates the last value - * and returns 1; else returns 0. */ +/** + * Update last_val + * + * @param i Index of last_val[] to update + * @param v New value for last_val[i] + * + * @return 1 if last_val[i] was updated, 0 if last_val[i]==v. + */ static int last_val_changed(int i, int v) { if (v && !(last_val[i / 8] & (1 << (i % 8)))) { @@ -80,8 +88,7 @@ static int command_gpio_get(int argc, char **argv) changed = last_val_changed(i, v); ccprintf(" %d%c %s\n", v, (changed ? '*' : ' '), g->name); - /* We have enough GPIOs that we'll overflow the output buffer - * without flushing */ + /* Flush console to avoid truncating output */ cflush(); } return EC_SUCCESS; @@ -91,7 +98,6 @@ DECLARE_CONSOLE_COMMAND(gpioget, command_gpio_get, "Read GPIO value(s)", NULL); - static int command_gpio_set(int argc, char **argv) { const struct gpio_info *g; @@ -116,7 +122,9 @@ static int command_gpio_set(int argc, char **argv) if (*e) return EC_ERROR_PARAM2; - return gpio_set_level(i, v); + gpio_set_level(i, v); + + return EC_SUCCESS; } DECLARE_CONSOLE_COMMAND(gpioset, command_gpio_set, "name <0 | 1>", @@ -145,7 +153,6 @@ static int gpio_command_get(struct host_cmd_handler_args *args) } DECLARE_HOST_COMMAND(EC_CMD_GPIO_GET, gpio_command_get, EC_VER_MASK(0)); - static int gpio_command_set(struct host_cmd_handler_args *args) { const struct ec_params_gpio_set *p = args->params; @@ -166,8 +173,8 @@ static int gpio_command_set(struct host_cmd_handler_args *args) if (!(g->flags & GPIO_OUTPUT)) return EC_RES_ERROR; - if (gpio_set_level(i, p->val) != EC_SUCCESS) - return EC_RES_ERROR; + gpio_set_level(i, p->val); + return EC_RES_SUCCESS; } DECLARE_HOST_COMMAND(EC_CMD_GPIO_SET, gpio_command_set, EC_VER_MASK(0)); diff --git a/include/gpio.h b/include/gpio.h index 0007e07cfa..77923ef4cb 100644 --- a/include/gpio.h +++ b/include/gpio.h @@ -8,10 +8,8 @@ #ifndef __CROS_EC_GPIO_H #define __CROS_EC_GPIO_H -#include "board.h" /* For board-dependent enum gpio_signal list */ #include "common.h" - /* Flag definitions for gpio_info. */ #define GPIO_INPUT 0x0000 /* Input */ #define GPIO_OUTPUT 0x0001 /* Output */ @@ -45,9 +43,11 @@ struct gpio_info { int mask; /* Bitmask on that port (0x01 - 0x80; 0x00 = * signal not implemented) */ uint32_t flags; /* Flags (GPIO_*) */ - /* Interrupt handler. If non-NULL, and the signal's interrupt is + /* + * Interrupt handler. If non-NULL, and the signal's interrupt is * enabled, this will be called in the context of the GPIO interrupt - * handler. */ + * handler. + */ void (*irq_handler)(enum gpio_signal signal); }; @@ -57,12 +57,19 @@ extern const struct gpio_info gpio_list[GPIO_COUNT]; /* Macro for signals which don't exist */ #define GPIO_SIGNAL_NOT_IMPLEMENTED(name) {name, LM4_GPIO_A, 0, 0, NULL} +/** + * Pre-initializes the module. + * + * This occurs before clocks or tasks are set up. + */ +void gpio_pre_init(void); -/* Pre-initializes the module. This occurs before clocks or tasks are - * set up. */ -int gpio_pre_init(void); - -/* Get the current value of a signal (0=low, 1=hi). */ +/** + * Get the current value of a signal + * + * @param signal Signal to get + * @return 0 if low, 1 if high. + */ int gpio_get_level(enum gpio_signal signal); /** @@ -78,30 +85,53 @@ int gpio_get_level(enum gpio_signal signal); uint16_t *gpio_get_level_reg(enum gpio_signal signal, uint32_t *mask); /** - * Returns the name of a given GPIO signal. + * Return the name of a given GPIO signal. * - * @param signal Signal to return. + * @param signal Signal to name * @returns name of the given signal */ const char *gpio_get_name(enum gpio_signal signal); -/* Set the flags for a signal. Note that this does not set the signal level - * based on the presence/absence of GPIO_HIGH; call gpio_set_level() afterwards - * to do that if needed. */ -int gpio_set_flags(enum gpio_signal signal, int flags); +/** + * Set the flags for a signal. + * + * Note that this does not set the signal level based on the presence/absence + * of GPIO_HIGH; call gpio_set_level() afterwards to do that if needed. + * + * @param signal Signal to set flags for + * @param flags New flags for the signal + */ +void gpio_set_flags(enum gpio_signal signal, int flags); -/* Set the current value of a signal. */ -int gpio_set_level(enum gpio_signal signal, int value); +/** + * Set the value of a signal. + * + * @param signal Signal to set + * @param value New value for signal (0 = low, != high */ +void gpio_set_level(enum gpio_signal signal, int value); -/* Enable interrupts for the signal. The signal must have been defined with +/** + * Enable interrupts for the signal. + * + * The signal must have been defined with * an interrupt handler. Normally called by the module which handles the - * interrupt, once it's ready to start processing interrupts. */ + * interrupt, once it's ready to start processing interrupts. + * + * @param signal Signal to enable interrrupts for + * @return EC_SUCCESS, or non-zero if error. + */ int gpio_enable_interrupt(enum gpio_signal signal); -/* Set alternate function <func> for GPIO <port> (LM4_GPIO_*) and <mask>. If - * func==0, configures the specified GPIOs for normal GPIO operation. +/** + * Set alternate function for GPIO(s). * - * This is intended for use by other modules' configure_gpio() functions. */ + * This is intended for use by other modules' configure_gpio() functions. + * + * @param port GPIO port to set (LM4_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. + */ void gpio_set_alternate_function(int port, int mask, int func); #endif /* __CROS_EC_GPIO_H */ |