From 8c9210b81e83c94d9d34497d202476c4fa6fb34b Mon Sep 17 00:00:00 2001 From: Shawn Nematbakhsh Date: Thu, 21 Jan 2016 14:37:53 -0800 Subject: mec1322: hibernate: Don't reconfigure all GPIOs Configuring all GPIOs to INPUT / PULL_UP in hibernate is not a good idea: - INPUT / PULL_UP is not necessarially the lowest-power state (for example, if there is an onboard pull-down). - Most GPIOs should already be in lowest-power state when we're in S5. - For the few GPIOs that need to be in a different state for hibernate, we can use a board-level callback. In addition, remove mec1322 code related to restoring from hibernate state, since we always reset coming out of hibernate. BUG=chrome-os-partner:49608 BRANCH=glados, strago TEST=`hibernate` on chell console when in S5 and AC removed. Verify that EC power is roughly equivalent to low-power idle power. Attach Zinger, verify that device wakes and boots, and charges from charger. Change-Id: Ib00ef035bec32cea3847eb38d743f5c0cec896ca Signed-off-by: Shawn Nematbakhsh Reviewed-on: https://chromium-review.googlesource.com/322937 Commit-Ready: Shawn N Tested-by: Shawn N Reviewed-by: Duncan Laurie Reviewed-by: Todd Broch --- board/chell/board.c | 38 +++------- board/glados/board.c | 34 +++------ board/wheatley/board.c | 2 +- chip/mec1322/registers.h | 8 --- chip/mec1322/system.c | 175 +++++++---------------------------------------- chip/npcx/registers.h | 6 -- chip/npcx/system.c | 4 +- include/config.h | 6 -- include/gpio.h | 3 + 9 files changed, 50 insertions(+), 226 deletions(-) diff --git a/board/chell/board.c b/board/chell/board.c index 27316719e0..b0e4ec2ac5 100644 --- a/board/chell/board.c +++ b/board/chell/board.c @@ -357,35 +357,17 @@ static void board_chipset_suspend(void) } DECLARE_HOOK(HOOK_CHIPSET_SUSPEND, board_chipset_suspend, HOOK_PRIO_DEFAULT); -uint32_t board_get_gpio_hibernate_state(uint32_t port, uint32_t pin) +void board_set_gpio_hibernate_state(void) { - int i; - const uint32_t out_low_gpios[][2] = { - /* Turn off LEDs in hibernate */ - GPIO_TO_PORT_MASK_PAIR(GPIO_CHARGE_LED_1), - GPIO_TO_PORT_MASK_PAIR(GPIO_CHARGE_LED_2), - /* - * Set PD wake low so that it toggles high to generate a wake - * event once we leave hibernate. - */ - GPIO_TO_PORT_MASK_PAIR(GPIO_USB_PD_WAKE), - /* The GPIO to reset PMIC is active high */ - GPIO_TO_PORT_MASK_PAIR(GPIO_PMIC_LDO_EN), - /* The GPIO to control RTCRST is active high. */ - GPIO_TO_PORT_MASK_PAIR(GPIO_PCH_RTCRST), - /* Keep keyboard backlight off, GPIO34 pin is in PWM mode */ - GPIO_TO_PORT_MASK_PAIR(GPIO_KEYBOARD_BACKLIGHT), - /* RSMRST to PCH should be low when rails are off */ - GPIO_TO_PORT_MASK_PAIR(GPIO_PCH_RSMRST_L), - }; - - /* Some GPIOs should be driven low in hibernate */ - for (i = 0; i < ARRAY_SIZE(out_low_gpios); ++i) - if (out_low_gpios[i][0] == port && out_low_gpios[i][1] == pin) - return GPIO_OUTPUT | GPIO_LOW; - - /* Other GPIOs should be put in a low-power state */ - return GPIO_INPUT | GPIO_PULL_UP; + /* Turn off LEDs in hibernate */ + gpio_set_level(GPIO_CHARGE_LED_1, 0); + gpio_set_level(GPIO_CHARGE_LED_2, 0); + + /* + * Set PD wake low so that it toggles high to generate a wake + * event once we leave hibernate. + */ + gpio_set_level(GPIO_USB_PD_WAKE, 0); } /* Make the pmic re-sequence the power rails under these conditions. */ diff --git a/board/glados/board.c b/board/glados/board.c index 534a8086aa..eb39af9f4e 100644 --- a/board/glados/board.c +++ b/board/glados/board.c @@ -419,31 +419,17 @@ static void board_chipset_suspend(void) } DECLARE_HOOK(HOOK_CHIPSET_SUSPEND, board_chipset_suspend, HOOK_PRIO_DEFAULT); -uint32_t board_get_gpio_hibernate_state(uint32_t port, uint32_t pin) +void board_set_gpio_hibernate_state(void) { - int i; - const uint32_t out_low_gpios[][2] = { - /* Turn off LEDs in hibernate */ - GPIO_TO_PORT_MASK_PAIR(GPIO_CHARGE_LED_1), - GPIO_TO_PORT_MASK_PAIR(GPIO_CHARGE_LED_2), - /* - * Set PD wake low so that it toggles high to generate a wake - * event once we leave hibernate. - */ - GPIO_TO_PORT_MASK_PAIR(GPIO_USB_PD_WAKE), - /* The GPIO to control RTCRST is active high. */ - GPIO_TO_PORT_MASK_PAIR(GPIO_PCH_RTCRST), - /* RSMRST to PCH should be low when rails are off */ - GPIO_TO_PORT_MASK_PAIR(GPIO_PCH_RSMRST_L), - }; - - /* LED GPIOs should be driven low to turn off LEDs */ - for (i = 0; i < ARRAY_SIZE(out_low_gpios); ++i) - if (out_low_gpios[i][0] == port && out_low_gpios[i][1] == pin) - return GPIO_OUTPUT | GPIO_LOW; - - /* Other GPIOs should be put in a low-power state */ - return GPIO_INPUT | GPIO_PULL_UP; + /* Turn off LEDs in hibernate */ + gpio_set_level(GPIO_CHARGE_LED_1, 0); + gpio_set_level(GPIO_CHARGE_LED_2, 0); + + /* + * Set PD wake low so that it toggles high to generate a wake + * event once we leave hibernate. + */ + gpio_set_level(GPIO_USB_PD_WAKE, 0); } /* Any glados boards post version 2 should have ROP_LDO_EN stuffed. */ diff --git a/board/wheatley/board.c b/board/wheatley/board.c index 143642c3e0..8fcd142efa 100644 --- a/board/wheatley/board.c +++ b/board/wheatley/board.c @@ -379,7 +379,7 @@ static void board_chipset_suspend(void) } DECLARE_HOOK(HOOK_CHIPSET_SUSPEND, board_chipset_suspend, HOOK_PRIO_DEFAULT); -void board_set_gpio_state_hibernate(void) +void board_set_gpio_hibernate_state(void) { int i; const uint32_t hibernate_pins[][2] = { diff --git a/chip/mec1322/registers.h b/chip/mec1322/registers.h index be2d5f4a90..b7a814cd8c 100644 --- a/chip/mec1322/registers.h +++ b/chip/mec1322/registers.h @@ -65,7 +65,6 @@ #define MEC1322_EC_WDT_CNT REG32(MEC1322_EC_BASE + 0x28) #define MEC1322_EC_ADC_VREF_PD REG32(MEC1322_EC_BASE + 0x38) - /* Interrupt aggregator */ #define MEC1322_INT_BASE 0x4000c000 #define MEC1322_INTx_BASE(x) (MEC1322_INT_BASE + ((x) - 8) * 0x14) @@ -498,11 +497,4 @@ typedef volatile struct mec1322_dma_regs mec1322_dma_regs_t; extern const enum gpio_signal hibernate_wake_pins[]; extern const int hibernate_wake_pins_used; -/* - * Optional board-level function to get hibernate GPIO state. - * Returns desired GPIO state in hibernate, or 0 to skip reconfiguration. - */ -uint32_t board_get_gpio_hibernate_state(uint32_t port, uint32_t pin) - __attribute__((weak)); - #endif /* __CROS_EC_REGISTERS_H */ diff --git a/chip/mec1322/system.c b/chip/mec1322/system.c index d97f5b0c4e..967d8eb03b 100644 --- a/chip/mec1322/system.c +++ b/chip/mec1322/system.c @@ -186,101 +186,9 @@ uint32_t system_get_scratchpad(void) return MEC1322_VBAT_RAM(HIBDATA_INDEX_SCRATCHPAD); } -/* Returns desired GPIO state in hibernate, or 0 to skip reconfiguration */ -static uint32_t system_get_gpio_hibernate_state(uint32_t port, uint32_t pin) -{ - int i; - const int skip[][2] = { - /* - * Leave USB-C charging enabled in hibernate, in order to - * allow wake-on-plug. 5V enable must be pulled low. - */ -#ifdef CONFIG_USB_PD_PORT_COUNT -#if CONFIG_USB_PD_PORT_COUNT > 0 - GPIO_TO_PORT_MASK_PAIR(GPIO_USB_C0_5V_EN), - GPIO_TO_PORT_MASK_PAIR(GPIO_USB_C0_CHARGE_EN_L), -#endif -#if CONFIG_USB_PD_PORT_COUNT > 1 - GPIO_TO_PORT_MASK_PAIR(GPIO_USB_C1_5V_EN), - GPIO_TO_PORT_MASK_PAIR(GPIO_USB_C1_CHARGE_EN_L), -#endif -#endif /* CONFIG_USB_PD_PORT_COUNT */ - - /* - * MEC1322 datasheet, sec. 20.6: VCC1_RST# cannot be used - * as a GPIO pin. - */ - {13, 1}, - /* GPIO 205 doesn't exist. */ - {20, 5}, - }; - - for (i = 0; i < ARRAY_SIZE(skip); ++i) - if (port == skip[i][0] && pin == skip[i][1]) - return 0; - - if (board_get_gpio_hibernate_state) - return board_get_gpio_hibernate_state(port, pin); - else - return GPIO_INPUT | GPIO_PULL_UP; -} - -static void system_set_gpio_power(int enabled, uint32_t *backup_gpio_ctl) -{ - int i, j; - uint32_t port, flags; - - const int pins[][2] = { - {0, 7}, {1, 7}, {2, 7}, {3, 6}, {4, 7}, {5, 7}, - {6, 7}, {10, 7}, {11, 7}, {12, 7}, {13, 6}, - {14, 7}, {15, 7}, {16, 5}, {20, 6}, {21, 1} - }; - - for (i = 0; i < ARRAY_SIZE(pins); ++i) { - port = pins[i][0]; - for (j = 0; j <= pins[i][1]; ++j) { - flags = system_get_gpio_hibernate_state(port, j); - if (flags == 0) - continue; - - if (enabled) { - MEC1322_GPIO_CTL(port, j) = - backup_gpio_ctl[i * 8 + j]; - } else { - if (backup_gpio_ctl != NULL) - backup_gpio_ctl[i * 8 + j] = - MEC1322_GPIO_CTL(port, j); - gpio_set_flags_by_mask(port, 1 << j, flags); - gpio_set_alternate_function(port, 1 << j, -1); - } - } - } - -#ifdef CONFIG_USB_PD_PORT_COUNT - if (!enabled) { - /* - * Leave USB-C charging enabled in hibernate, in order to - * allow wake-on-plug. 5V enable must be pulled low. - */ -#if CONFIG_USB_PD_PORT_COUNT > 0 - gpio_set_flags(GPIO_USB_C0_5V_EN, GPIO_PULL_DOWN | GPIO_INPUT); - gpio_set_level(GPIO_USB_C0_CHARGE_EN_L, 0); -#endif -#if CONFIG_USB_PD_PORT_COUNT > 1 - gpio_set_flags(GPIO_USB_C1_5V_EN, GPIO_PULL_DOWN | GPIO_INPUT); - gpio_set_level(GPIO_USB_C1_CHARGE_EN_L, 0); -#endif - } -#endif /* CONFIG_USB_PD_PORT_COUNT */ -} - void system_hibernate(uint32_t seconds, uint32_t microseconds) { int i; - uint32_t int_status[16]; - uint32_t int_block_status; - uint32_t nvic_status[3]; - char *backup_gpio_ctl; #ifdef CONFIG_HOSTCMD_PD /* Inform the PD MCU that we are going to hibernate. */ @@ -293,18 +201,14 @@ void system_hibernate(uint32_t seconds, uint32_t microseconds) /* Disable interrupts */ interrupt_disable(); - for (i = 0; i < 3; ++i) - nvic_status[i] = CPU_NVIC_EN(i); for (i = 0; i <= 92; ++i) { task_disable_irq(i); task_clear_pending_irq(i); } - for (i = 8; i <= 23; ++i) { - int_status[i - 8] = MEC1322_INT_ENABLE(i); + for (i = 8; i <= 23; ++i) MEC1322_INT_DISABLE(i) = 0xffffffff; - } - int_block_status = MEC1322_INT_BLK_EN; + MEC1322_INT_BLK_DIS |= 0xffff00; /* Power down ADC VREF */ @@ -345,12 +249,24 @@ void system_hibernate(uint32_t seconds, uint32_t microseconds) MEC1322_PCR_SYS_SLP_CTL = (MEC1322_PCR_SYS_SLP_CTL & ~0x7) | 0x2; CPU_SCB_SYSCTRL |= 0x4; - /* Attempt to backup GPIO states if we need to restore them on wake. */ -#ifndef CONFIG_HIBERNATE_RESET_ON_WAKE - if (shared_mem_acquire(512, &backup_gpio_ctl) != EC_SUCCESS) + /* Setup GPIOs for hibernate */ + if (board_set_gpio_hibernate_state) + board_set_gpio_hibernate_state(); + +#ifdef CONFIG_USB_PD_PORT_COUNT + /* + * Leave USB-C charging enabled in hibernate, in order to + * allow wake-on-plug. 5V enable must be pulled low. + */ +#if CONFIG_USB_PD_PORT_COUNT > 0 + gpio_set_flags(GPIO_USB_C0_5V_EN, GPIO_PULL_DOWN | GPIO_INPUT); + gpio_set_level(GPIO_USB_C0_CHARGE_EN_L, 0); #endif - backup_gpio_ctl = NULL; - system_set_gpio_power(0, (uint32_t *)backup_gpio_ctl); +#if CONFIG_USB_PD_PORT_COUNT > 1 + gpio_set_flags(GPIO_USB_C1_5V_EN, GPIO_PULL_DOWN | GPIO_INPUT); + gpio_set_level(GPIO_USB_C1_CHARGE_EN_L, 0); +#endif +#endif /* CONFIG_USB_PD_PORT_COUNT */ if (hibernate_wake_pins_used > 0) { for (i = 0; i < hibernate_wake_pins_used; ++i) { @@ -393,55 +309,12 @@ void system_hibernate(uint32_t seconds, uint32_t microseconds) /* Use 48MHz clock to speed through wake-up */ MEC1322_PCR_PROC_CLK_CTL = 1; - /* If we didn't back up GPIO status, just reboot. */ - if (backup_gpio_ctl == NULL) - _system_reset(0, 1); - - system_set_gpio_power(1, (uint32_t *)backup_gpio_ctl); - shared_mem_release(backup_gpio_ctl); + /* Reboot */ + _system_reset(0, 1); - /* Enable blocks */ - MEC1322_PCR_SLOW_CLK_CTL |= 0x1e0; - MEC1322_PCR_CHIP_SLP_EN &= ~0x3; - MEC1322_PCR_EC_SLP_EN &= MEC1322_PCR_EC_SLP_EN_WAKE; - MEC1322_PCR_HOST_SLP_EN &= MEC1322_PCR_HOST_SLP_EN_WAKE; - MEC1322_PCR_EC_SLP_EN2 &= MEC1322_PCR_EC_SLP_EN2_WAKE; - - /* Enable timer */ - MEC1322_TMR32_CTL(0) |= 1; - MEC1322_TMR32_CTL(1) |= 1; - MEC1322_TMR16_CTL(0) |= 1; - - /* Enable watchdog */ - MEC1322_WDG_CTL |= 1; - - /* Enable 32KHz clock */ - MEC1322_VBAT_CE |= 0x2; - - /* Enable JTAG */ - MEC1322_EC_JTAG_EN |= 1; - - /* Enable UART */ - MEC1322_LPC_ACT |= 1; - MEC1322_UART_ACT |= 1; - - /* Deassert nSIO_RESET */ - MEC1322_PCR_PWR_RST_CTL &= ~1; - - /* Enable ADC */ - MEC1322_EC_ADC_VREF_PD &= ~1; - MEC1322_ADC_CTRL |= 1 << 0; - - /* Restore interrupts */ - for (i = 8; i <= 23; ++i) - MEC1322_INT_ENABLE(i) = int_status[i - 8]; - MEC1322_INT_BLK_EN = int_block_status; - - for (i = 0; i < 3; ++i) - CPU_NVIC_EN(i) = nvic_status[i]; - - /* Restore processor clock */ - MEC1322_PCR_PROC_CLK_CTL = 4; + /* We should never get here. */ + while (1) + ; } void htimer_interrupt(void) diff --git a/chip/npcx/registers.h b/chip/npcx/registers.h index 35fb568080..abb8936c04 100644 --- a/chip/npcx/registers.h +++ b/chip/npcx/registers.h @@ -1400,10 +1400,4 @@ static inline void npcx_gpio2uart(void) extern const enum gpio_signal hibernate_wake_pins[]; extern const int hibernate_wake_pins_used; -/* - * Optional board-level function to set GPIOs state in hibernate. - */ -void board_set_gpio_state_hibernate(void) - __attribute__((weak)); - #endif /* __CROS_EC_REGISTERS_H */ diff --git a/chip/npcx/system.c b/chip/npcx/system.c index 5abd5ff04e..dd44fce62d 100644 --- a/chip/npcx/system.c +++ b/chip/npcx/system.c @@ -353,8 +353,8 @@ void system_set_gpios_and_wakeup_inputs_hibernate(void) #endif /* CONFIG_USB_PD_PORT_COUNT */ /* board-level function to set GPIOs state in hibernate */ - if (board_set_gpio_state_hibernate) - return board_set_gpio_state_hibernate(); + if (board_set_gpio_hibernate_state) + return board_set_gpio_hibernate_state(); } /** diff --git a/include/config.h b/include/config.h index 23aeb306ef..942608e6e4 100644 --- a/include/config.h +++ b/include/config.h @@ -1088,12 +1088,6 @@ #undef CONFIG_HIBERNATE_BATT_PCT #undef CONFIG_HIBERNATE_BATT_SEC -/* - * Perform a system reset on wake from hibernate. This is the default behavior, - * and the only chip-supported behavior for certain ECs. - */ -#define CONFIG_HIBERNATE_RESET_ON_WAKE - /* For ECs with multiple wakeup pins, define enabled wakeup pins */ #undef CONFIG_HIBERNATE_WAKEUP_PINS diff --git a/include/gpio.h b/include/gpio.h index 060ef97c33..8c7e420cd9 100644 --- a/include/gpio.h +++ b/include/gpio.h @@ -252,4 +252,7 @@ void gpio_set_alternate_function(uint32_t port, uint32_t mask, int func); */ void gpio_enable_clocks(void); +/* Optional board-level function to set hibernate GPIO states. */ +void board_set_gpio_hibernate_state(void) __attribute__((weak)); + #endif /* __CROS_EC_GPIO_H */ -- cgit v1.2.1