diff options
author | Randall Spangler <rspangler@chromium.org> | 2013-04-12 15:07:58 -0700 |
---|---|---|
committer | ChromeBot <chrome-bot@google.com> | 2013-04-15 14:27:45 -0700 |
commit | 108235225d2536f75a3100cd535f44f732b486c3 (patch) | |
tree | 3f111879d86eddbc6979af513ed466dc20faf13d | |
parent | 27459f8600d76d5a7ccc4cc1c396ef59cb26ff19 (diff) | |
download | chrome-ec-108235225d2536f75a3100cd535f44f732b486c3.tar.gz |
Refactor gpio_set_level() and gpio_pre_init()
gpio_set_level() now allows setting the pin level if GPIO_LOW or
GPIO_HIGH is specified. Previously, stm32 platforms did this even
though the definition of gpio_set_level() said it wouldn't work.
Fixed gpio_set_level() not setting level after warm reboot on stm32
because it was checking the GPIO_DEFAULT flag in the wrong place.
Fixed LM4 still mucking with alternate function settings and levels
even if GPIO_DEFAULT was specified.
And checked gpio_list[] and all of the calls to gpio_set_flags() to
make sure everything still behaves the same way it did before (or
better, in the case of actual bugs).
BUG=chrome-os-partner:18718
BRANCH=none
TEST=build all platforms; boot spring and link
Change-Id: I4b84815f76060252df235ff9a37da52c54a8eac5
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://gerrit.chromium.org/gerrit/48058
Reviewed-by: Bill Richardson <wfrichar@chromium.org>
-rw-r--r-- | board/mccroskey/board.c | 9 | ||||
-rw-r--r-- | board/pit/board.c | 2 | ||||
-rw-r--r-- | board/snow/board.c | 2 | ||||
-rw-r--r-- | board/spring/board.c | 6 | ||||
-rw-r--r-- | chip/lm4/gpio.c | 52 | ||||
-rw-r--r-- | chip/stm32/gpio-stm32f100.c | 50 | ||||
-rw-r--r-- | chip/stm32/gpio-stm32l15x.c | 97 | ||||
-rw-r--r-- | chip/stm32/i2c.c | 9 | ||||
-rw-r--r-- | include/gpio.h | 29 |
9 files changed, 138 insertions, 118 deletions
diff --git a/board/mccroskey/board.c b/board/mccroskey/board.c index fbfac0fb7a..0f56b2ebae 100644 --- a/board/mccroskey/board.c +++ b/board/mccroskey/board.c @@ -17,9 +17,8 @@ #include "timer.h" #include "util.h" -#define GPIO_OUTPUT_OD (GPIO_OUTPUT | GPIO_OPEN_DRAIN) #define GPIO_KB_INPUT (GPIO_INPUT | GPIO_PULL_UP | GPIO_INT_BOTH) -#define GPIO_KB_OUTPUT GPIO_OUTPUT_OD +#define GPIO_KB_OUTPUT (GPIO_OUTPUT | GPIO_OPEN_DRAIN | GPIO_OUT_LOW) #define HARD_RESET_TIMEOUT_MS 5 @@ -47,7 +46,7 @@ const struct gpio_info gpio_list[GPIO_COUNT] = { {"KBD_PWR_BUTTON", GPIO_B, (1<<2), GPIO_INPUT, kbd_power_on}, {"OMZO_RDY_L", GPIO_A, (1<<0), GPIO_INPUT, NULL}, /* PA0_WKUP */ - {"OZMO_RST_L", GPIO_A, (1<<2), GPIO_OUTPUT_OD, NULL}, + {"OZMO_RST_L", GPIO_A, (1<<2), GPIO_HI_Z, NULL}, {"VBUS_UP_DET", GPIO_A, (1<<3), GPIO_INPUT, NULL}, {"OZMO_REQ_L", GPIO_A, (1<<8), GPIO_INPUT, NULL}, {"CHARGE_ZERO", GPIO_B, (1<<0), GPIO_INPUT, NULL}, @@ -74,8 +73,8 @@ const struct gpio_info gpio_list[GPIO_COUNT] = { {"KB_OUT10", GPIO_C, (1<<10), GPIO_KB_OUTPUT, NULL}, {"KB_OUT11", GPIO_C, (1<<11), GPIO_KB_OUTPUT, NULL}, {"KB_OUT12", GPIO_C, (1<<12), GPIO_KB_OUTPUT, NULL}, - {"USB_VBUS_CTRL", GPIO_C, (1<<13), GPIO_OUTPUT, NULL}, - {"HUB_RESET", GPIO_C, (1<<14), GPIO_OUTPUT_OD, NULL}, + {"USB_VBUS_CTRL", GPIO_C, (1<<13), GPIO_OUT_LOW, NULL}, + {"HUB_RESET", GPIO_C, (1<<14), GPIO_HI_Z, NULL}, {"WP_L", GPIO_D, (1<<2), GPIO_INPUT, NULL}, /* FIXME: make this alt. function */ diff --git a/board/pit/board.c b/board/pit/board.c index 0ee1ac8bc7..cccd61c6f0 100644 --- a/board/pit/board.c +++ b/board/pit/board.c @@ -16,7 +16,7 @@ #include "util.h" #define GPIO_KB_INPUT (GPIO_INPUT | GPIO_PULL_UP | GPIO_INT_BOTH) -#define GPIO_KB_OUTPUT (GPIO_OUTPUT | GPIO_PULL_UP | GPIO_OPEN_DRAIN) +#define GPIO_KB_OUTPUT GPIO_HI_Z /* GPIO signal list. Must match order from enum gpio_signal. */ const struct gpio_info gpio_list[GPIO_COUNT] = { diff --git a/board/snow/board.c b/board/snow/board.c index aaca03f7fd..4df97cd617 100644 --- a/board/snow/board.c +++ b/board/snow/board.c @@ -22,7 +22,7 @@ #include "util.h" #define GPIO_KB_INPUT (GPIO_INPUT | GPIO_PULL_UP | GPIO_INT_BOTH) -#define GPIO_KB_OUTPUT (GPIO_OUTPUT | GPIO_OPEN_DRAIN) +#define GPIO_KB_OUTPUT GPIO_HI_Z #define INT_BOTH_FLOATING (GPIO_INPUT | GPIO_INT_BOTH) #define INT_BOTH_PULL_UP (GPIO_INPUT | GPIO_PULL_UP | GPIO_INT_BOTH) diff --git a/board/spring/board.c b/board/spring/board.c index 4a10237cd3..9326e2e92e 100644 --- a/board/spring/board.c +++ b/board/spring/board.c @@ -23,7 +23,7 @@ #include "util.h" #define GPIO_KB_INPUT (GPIO_INPUT | GPIO_PULL_UP | GPIO_INT_BOTH) -#define GPIO_KB_OUTPUT (GPIO_OUTPUT | GPIO_OPEN_DRAIN) +#define GPIO_KB_OUTPUT GPIO_HI_Z #define INT_BOTH_FLOATING (GPIO_INPUT | GPIO_INT_BOTH) #define INT_BOTH_PULL_UP (GPIO_INPUT | GPIO_PULL_UP | GPIO_INT_BOTH) @@ -56,7 +56,7 @@ const struct gpio_info gpio_list[GPIO_COUNT] = { keyboard_raw_gpio_interrupt}, {"USB_CHG_INT", GPIO_A, (1<<6), GPIO_INT_FALLING, extpower_interrupt}, /* Other inputs */ - {"BCHGR_VACG", GPIO_A, (1<<0), GPIO_INT_BOTH, NULL}, + {"BCHGR_VACG", GPIO_A, (1<<0), GPIO_INT_BOTH, NULL}, /* * I2C pins should be configured as inputs until I2C module is * initialized. This will avoid driving the lines unintentionally. @@ -67,7 +67,7 @@ const struct gpio_info gpio_list[GPIO_COUNT] = { {"I2C2_SDA", GPIO_B, (1<<11), GPIO_INPUT, NULL}, /* Outputs */ {"EN_PP1350", GPIO_A, (1<<14), GPIO_OUT_LOW, NULL}, - {"EN_PP5000", GPIO_A, (1<<11), GPIO_OUT_LOW, NULL}, + {"EN_PP5000", GPIO_A, (1<<11), GPIO_OUT_LOW, NULL}, {"EN_PP3300", GPIO_A, (1<<8), GPIO_OUT_LOW, NULL}, {"PMIC_PWRON_L",GPIO_A, (1<<12), GPIO_OUT_HIGH, NULL}, {"PMIC_RESET", GPIO_A, (1<<15), GPIO_OUT_LOW, NULL}, diff --git a/chip/lm4/gpio.c b/chip/lm4/gpio.c index 50fa9dbce0..d49eeae4f5 100644 --- a/chip/lm4/gpio.c +++ b/chip/lm4/gpio.c @@ -97,9 +97,6 @@ void gpio_set_flags(enum gpio_signal signal, int flags) { const struct gpio_info *g = gpio_list + signal; - if (flags & GPIO_DEFAULT) - return; - if (flags & GPIO_OUTPUT) { /* * Select open drain first, so that we don't glitch the signal @@ -111,21 +108,26 @@ void gpio_set_flags(enum gpio_signal signal, int flags) LM4_GPIO_ODR(g->port) &= ~g->mask; LM4_GPIO_DIR(g->port) |= g->mask; + + /* Set level if necessary */ + if (flags & GPIO_HIGH) + gpio_set_level(signal, 1); + else if (flags & GPIO_LOW) + gpio_set_level(signal, 0); } else { /* Input */ LM4_GPIO_DIR(g->port) &= ~g->mask; + } - if (g->flags & GPIO_PULL) { - /* With pull up/down */ - if (g->flags & GPIO_HIGH) - LM4_GPIO_PUR(g->port) |= g->mask; - else - LM4_GPIO_PDR(g->port) |= g->mask; - } else { - /* No pull up/down */ - LM4_GPIO_PUR(g->port) &= ~g->mask; - LM4_GPIO_PDR(g->port) &= ~g->mask; - } + /* Handle pullup / pulldown */ + if (g->flags & GPIO_PULL_UP) { + LM4_GPIO_PUR(g->port) |= g->mask; + } else if (g->flags & GPIO_PULL_DOWN) { + LM4_GPIO_PDR(g->port) |= g->mask; + } else { + /* No pull up/down */ + LM4_GPIO_PUR(g->port) &= ~g->mask; + LM4_GPIO_PDR(g->port) &= ~g->mask; } /* Set up interrupt type */ @@ -195,19 +197,23 @@ void gpio_pre_init(void) /* Set all GPIOs to defaults */ for (i = 0; i < GPIO_COUNT; i++, g++) { - /* Use as GPIO, not alternate function */ - gpio_set_alternate_function(g->port, g->mask, -1); + int flags = g->flags; - /* Set up GPIO based on flags */ - gpio_set_flags(i, g->flags); + if (flags & GPIO_DEFAULT) + continue; /* - * If this is a cold boot, set the level. On a warm reboot, - * leave things where they were or we'll shut off the main - * chipset. + * If this is a warm reboot, don't set the output levels or + * we'll shut off the main chipset. */ - if ((g->flags & GPIO_OUTPUT) && !is_warm) - gpio_set_level(i, g->flags & GPIO_HIGH); + if (is_warm) + flags &= ~(GPIO_LOW | GPIO_HIGH); + + /* Set up GPIO based on flags */ + gpio_set_flags(i, flags); + + /* Use as GPIO, not alternate function */ + gpio_set_alternate_function(g->port, g->mask, -1); } } diff --git a/chip/stm32/gpio-stm32f100.c b/chip/stm32/gpio-stm32f100.c index 55c2b309de..5e55ca8834 100644 --- a/chip/stm32/gpio-stm32f100.c +++ b/chip/stm32/gpio-stm32f100.c @@ -59,9 +59,6 @@ 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; - gpio_config_info(g, &addr, &mode, &cnf); mask = REG32(addr) & ~(cnf | mode); @@ -75,16 +72,17 @@ void gpio_set_flags(enum gpio_signal signal, int flags) 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) */ - if ((flags & GPIO_PULL_UP) == GPIO_PULL_UP) { + if (flags & GPIO_PULL_UP) { mask |= 0x88888888 & cnf; STM32_GPIO_BSRR_OFF(g->port) |= g->mask; gpio_set_level(signal, 1); - } else if ((flags & GPIO_PULL_DOWN) == GPIO_PULL_DOWN) { + } else if (flags & GPIO_PULL_DOWN) { mask |= 0x88888888 & cnf; gpio_set_level(signal, 0); } else { @@ -92,21 +90,19 @@ void gpio_set_flags(enum gpio_signal signal, int flags) } } - /* - * Set pin level after port has been set up as to avoid - * potential damage, e.g. driving an open-drain output - * high before it has been configured as such. - */ - if ((flags & GPIO_OUTPUT) && !is_warm_boot) + REG32(addr) = mask; + + if (flags & GPIO_OUTPUT) { /* - * 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. + * Set pin level after port has been set up as to avoid + * potential damage, e.g. driving an open-drain output high + * before it has been configured as such. */ - gpio_set_level(signal, flags & GPIO_HIGH); - - REG32(addr) = mask; + if (flags & GPIO_HIGH) + gpio_set_level(signal, 1); + else if (flags & GPIO_LOW) + gpio_set_level(signal, 0); + } /* Set up interrupts if necessary */ ASSERT(!(flags & GPIO_INT_LEVEL)); @@ -135,8 +131,22 @@ void gpio_pre_init(void) } /* Set all GPIOs to defaults */ - for (i = 0; i < GPIO_COUNT; i++, g++) - gpio_set_flags(i, g->flags); + for (i = 0; i < GPIO_COUNT; i++, g++) { + int flags = g->flags; + + if (flags & GPIO_DEFAULT) + continue; + + /* + * If this is a warm reboot, don't set the output levels or + * we'll shut off the AP. + */ + if (is_warm_boot) + flags &= ~(GPIO_LOW | GPIO_HIGH); + + /* Set up GPIO based on flags */ + gpio_set_flags(i, flags); + } } void gpio_init(void) diff --git a/chip/stm32/gpio-stm32l15x.c b/chip/stm32/gpio-stm32l15x.c index dbd7af174c..6d61d05fa2 100644 --- a/chip/stm32/gpio-stm32l15x.c +++ b/chip/stm32/gpio-stm32l15x.c @@ -22,10 +22,55 @@ static const struct gpio_info *exti_events[16]; void gpio_set_flags(enum gpio_signal signal, int flags) { + const struct gpio_info *g = gpio_list + signal; + + /* Bitmask for registers with 2 bits per GPIO pin */ + const uint32_t mask2 = (g->mask * g->mask) | (g->mask * g->mask * 2); + uint32_t val; + + /* Set up pullup / pulldown */ + val = STM32_GPIO_PUPDR_OFF(g->port) & ~mask2; + if (flags & GPIO_PULL_UP) + val |= 0x55555555 & mask2; /* Pull Up = 01 */ + else if (flags & GPIO_PULL_DOWN) + val |= 0xaaaaaaaa & mask2; /* Pull Down = 10 */ + STM32_GPIO_PUPDR_OFF(g->port) = val; + /* - * TODO(dhendrix): Move GPIO setup code from gpio_pre_init - * into here like we did for STM32F + * Select open drain first, so that we don't glitch the signal when + * changing the line to an output. */ + if (flags & GPIO_OPEN_DRAIN) + STM32_GPIO_OTYPER_OFF(g->port) |= g->mask; + + val = STM32_GPIO_MODER_OFF(g->port) & ~mask2; + if (flags & GPIO_OUTPUT) { + /* + * Set pin level first to avoid glitching. This is harmless on + * STM32L because the set/reset register isn't connected to the + * output drivers until the pin is made an output. + */ + if (flags & GPIO_HIGH) + gpio_set_level(signal, 1); + else if (flags & GPIO_LOW) + gpio_set_level(signal, 0); + + /* General purpose, MODE = 01 */ + val |= 0x55555555 & mask2; + STM32_GPIO_MODER_OFF(g->port) = val; + + } else if (flags & GPIO_INPUT) { + /* Input, MODE=00 */ + STM32_GPIO_MODER_OFF(g->port) = val; + } + + /* Set up interrupts if necessary */ + ASSERT(!(flags & GPIO_INT_LEVEL)); + if (flags & (GPIO_INT_RISING | GPIO_INT_BOTH)) + STM32_EXTI_RTSR |= g->mask; + if (flags & (GPIO_INT_FALLING | GPIO_INT_BOTH)) + STM32_EXTI_FTSR |= g->mask; + /* Interrupt is enabled by gpio_enable_interrupt() */ } void gpio_pre_init(void) @@ -49,52 +94,22 @@ void gpio_pre_init(void) STM32_RCC_AHBENR |= 0x3f; } + /* Set all GPIOs to defaults */ for (i = 0; i < GPIO_COUNT; i++, g++) { - /* bitmask for registers with 2 bits per GPIO pin */ - uint32_t mask2 = (g->mask * g->mask) | (g->mask * g->mask * 2); - uint32_t val; + int flags = g->flags; - if (g->mask & GPIO_DEFAULT) + if (flags & GPIO_DEFAULT) continue; - val = STM32_GPIO_PUPDR_OFF(g->port) & ~mask2; - if ((g->flags & GPIO_PULL_UP) == GPIO_PULL_UP) - /* Pull Up = 01 */ - val |= 0x55555555 & mask2; - else if ((g->flags & GPIO_PULL_DOWN) == GPIO_PULL_DOWN) - /* Pull Down = 10 */ - val |= 0xaaaaaaaa & mask2; - STM32_GPIO_PUPDR_OFF(g->port) = val; - - if (g->flags & GPIO_OPEN_DRAIN) - STM32_GPIO_OTYPER_OFF(g->port) |= g->mask; /* - * Set pin level after port has been set up as to avoid - * potential damage, e.g. driving an open-drain output - * high before it has been configured as such. + * If this is a warm reboot, don't set the output levels or + * we'll shut off the AP. */ - val = STM32_GPIO_MODER_OFF(g->port) & ~mask2; - 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 (!is_warm) - gpio_set_level(i, g->flags & GPIO_HIGH); - } else if (g->flags & GPIO_INPUT) { /* Input, MODE=00 */ - STM32_GPIO_MODER_OFF(g->port) = val; - } + if (is_warm) + flags &= ~(GPIO_LOW | GPIO_HIGH); - /* Set up interrupts if necessary */ - ASSERT(!(g->flags & GPIO_INT_LEVEL)); - if (g->flags & (GPIO_INT_RISING | GPIO_INT_BOTH)) - STM32_EXTI_RTSR |= g->mask; - if (g->flags & (GPIO_INT_FALLING | GPIO_INT_BOTH)) - STM32_EXTI_FTSR |= g->mask; - /* Interrupt is enabled by gpio_enable_interrupt() */ + /* Set up GPIO based on flags */ + gpio_set_flags(i, flags); } } diff --git a/chip/stm32/i2c.c b/chip/stm32/i2c.c index a682883c89..cbb8db3e1d 100644 --- a/chip/stm32/i2c.c +++ b/chip/stm32/i2c.c @@ -379,14 +379,9 @@ static void unwedge_i2c_bus(int port) /* * Reconfigure ports as general purpose open-drain outputs, initted * to high. - * - * We manually set the level first in addition to using GPIO_HIGH - * since gpio_set_flags() behaves strangely in the case of a warm boot. */ - gpio_set_level(scl, 1); - gpio_set_level(sda, 1); - gpio_set_flags(scl, GPIO_OUTPUT | GPIO_OPEN_DRAIN | GPIO_HIGH); - gpio_set_flags(sda, GPIO_OUTPUT | GPIO_OPEN_DRAIN | GPIO_HIGH); + gpio_set_flags(scl, GPIO_HI_Z); + gpio_set_flags(sda, GPIO_HI_Z); /* Try to send out pseudo-stop bit. See function description */ if (gpio_get_level(scl) && gpio_get_level(sda)) { diff --git a/include/gpio.h b/include/gpio.h index fca33231e0..793b54575d 100644 --- a/include/gpio.h +++ b/include/gpio.h @@ -13,23 +13,21 @@ /* Flag definitions for gpio_info. */ #define GPIO_INPUT 0x0000 /* Input */ #define GPIO_OUTPUT 0x0001 /* Output */ -#define GPIO_PULL 0x0002 /* Input with on-chip pullup/pulldown */ -#define GPIO_HIGH 0x0004 /* If GPIO_OUTPUT, default high; if GPIO_PULL, - * pull up (otherwise default low / pull - * down) */ -#define GPIO_OPEN_DRAIN 0x0008 /* Output type is open-drain */ -#define GPIO_INT_RISING 0x0010 /* Interrupt on rising edge */ -#define GPIO_INT_FALLING 0x0020 /* Interrupt on falling edge */ -#define GPIO_INT_BOTH 0x0040 /* Interrupt on both edges */ -#define GPIO_INT_LOW 0x0080 /* Interrupt on low level */ -#define GPIO_INT_HIGH 0x0100 /* Interrupt on high level */ -#define GPIO_DEFAULT 0x0200 /* Don't set up on boot */ +#define GPIO_OPEN_DRAIN 0x0002 /* Output type is open-drain */ +#define GPIO_PULL_UP 0x0004 /* Enable on-chip pullup */ +#define GPIO_PULL_DOWN 0x0008 /* Enable on-chip pulldown */ +#define GPIO_LOW 0x0010 /* If GPIO_OUTPUT, set level low */ +#define GPIO_HIGH 0x0020 /* If GPIO_OUTPUT, set level high */ +#define GPIO_INT_RISING 0x0040 /* Interrupt on rising edge */ +#define GPIO_INT_FALLING 0x0080 /* Interrupt on falling edge */ +#define GPIO_INT_BOTH 0x0100 /* Interrupt on both edges */ +#define GPIO_INT_LOW 0x0200 /* Interrupt on low level */ +#define GPIO_INT_HIGH 0x0400 /* Interrupt on high level */ +#define GPIO_DEFAULT 0x0800 /* Don't set up on boot */ /* Common flag combinations */ -#define GPIO_OUT_LOW GPIO_OUTPUT +#define GPIO_OUT_LOW (GPIO_OUTPUT | GPIO_LOW) #define GPIO_OUT_HIGH (GPIO_OUTPUT | GPIO_HIGH) -#define GPIO_PULL_DOWN GPIO_PULL -#define GPIO_PULL_UP (GPIO_PULL | GPIO_HIGH) #define GPIO_HI_Z (GPIO_OUTPUT | GPIO_OPEN_DRAIN | GPIO_HIGH) #define GPIO_INT_EDGE (GPIO_INT_RISING | GPIO_INT_FALLING | GPIO_INT_BOTH) #define GPIO_INT_LEVEL (GPIO_INT_LOW | GPIO_INT_HIGH) @@ -99,9 +97,6 @@ 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. - * * @param signal Signal to set flags for * @param flags New flags for the signal */ |