From a9db319dd46fee25a11abcd55d2c8d3c9d012794 Mon Sep 17 00:00:00 2001 From: Vadim Bendebury Date: Thu, 2 Feb 2017 20:10:02 -0800 Subject: [PATCH] cr50: rework sleep state and tpm reset triggers The cr50 needs to be aware of the power state of the system and of the moment when the AP is reset, because this is when the TPM needs to be reset too. Arm and x86 platforms provide different hints in these cases. In case of x86 there is a single signal cr50 can rely on: PLT_RST_L. This active low signal is asserted when the system is going into any power state deeper than s0ix. The cr50 can fall into deep sleep when PLT_RST_L is asserted, and has to wake up and reset the TPM when this signal is deasserted. There could be other wake triggers, but the tpm should not be reset unless PLT_RST_L is inactive. It is also important not to fall into deep sleep when PLT_RST_L is pulsed to reboot the system. In case of ARM there are two separate signals. Deasserting SYS_RST_L signal is the trigger to reset the TPM, The GPIO_DETECT_AP going low for a duration of time is the indication of the AP going into some kind of sleep mode. The ARM case requires more clarification. This adds run time configuration of the the sleep state control input. Once the input turns low, the CHIPSET_SHUTDOWN signal is sent and deep sleep mode is enabled. Again, this will require adjustment for ARM platforms. The wake from deep sleep state is controlled by the wake pins as before, but by level instead of edge. This makes sure that in case the trigger for deep sleep goes away while deep sleep preparation is under way, the device resumes immediately instead of getting stuck missing the edge. The TPM_RST_ input is now triggering interrupts on deassertion - this is the moment when the TPM needs to be reset. The ISR is being renamed accordingly. The processing previously happening inside the ISR is being moved into a deferred function running on the hooks task context. There is no need to invoke TPM reset related functions from the PMU wake up ISR anymore. BRANCH=none BUG=chrome-os-partner:59007 TEST=as follows: 1. make buildall -j succeeds 2. started on Reef, still in progress after 100 iterations, early to call suspend_stress_test --suspend_min 40 --suspend_max 45 \ --wake_max 15 wake_min 10 (note that reef does not fall into s3 any more, so the test does not verify H1 deep sleep) 3. modified the target to fall into s3 during the test and successfully repeated it for 100 iterations 4. tried battery disconnect a few times and observe successful boot. Change-Id: Ica06ec0d363b53eede3be327404ff5807fa3a610 Signed-off-by: Vadim Bendebury Reviewed-on: https://chromium-review.googlesource.com/436865 Reviewed-by: Mary Ruthven --- board/cr50/board.c | 159 ++++++++++++++++++++++------------------ board/cr50/board.h | 2 +- board/cr50/gpio.inc | 3 +- common/tpm_registers.c | 25 +++++-- include/tpm_registers.h | 7 -- 5 files changed, 109 insertions(+), 87 deletions(-) diff --git a/board/cr50/board.c b/board/cr50/board.c index 01d9a0d5a9..3597014071 100644 --- a/board/cr50/board.c +++ b/board/cr50/board.c @@ -79,6 +79,9 @@ uint32_t nvmem_user_sizes[NVMEM_NUM_USERS] = { NVMEM_CR50_SIZE }; +static int device_state_changed(enum device_type device, + enum device_state state); + /* Board specific configuration settings */ static uint32_t board_properties; static uint8_t reboot_request_posted; @@ -312,7 +315,6 @@ static void init_pmu(void) void pmu_wakeup_interrupt(void) { int exiten, wakeup_src; - int wake_on_low; delay_sleep_by(1 * MSEC); @@ -337,18 +339,7 @@ void pmu_wakeup_interrupt(void) * Delay sleep long enough for a SPI slave transaction to start * or for the system to be reset. */ - delay_sleep_by(20 * SECOND); - - /* - * If sys_rst_l or plt_rst_l (if signal is present) is - * configured to wake on low and the signal is low, then call - * tpm_rst_asserted - */ - wake_on_low = board_use_plt_rst() ? - GREAD_FIELD(PINMUX, EXITINV0, DIOM3) : - GREAD_FIELD(PINMUX, EXITINV0, DIOM0); - if (!gpio_get_level(GPIO_TPM_RST_L) && wake_on_low) - tpm_rst_asserted(GPIO_TPM_RST_L); + delay_sleep_by(5 * SECOND); } /* Trigger timer0 interrupt */ @@ -376,17 +367,9 @@ void board_configure_deep_sleep_wakepins(void) GWRITE_FIELD(PINMUX, DIOB5_CTL, IE, 0); /* - * DIOA3 is GPIO_DETECT_AP which is used to detect if the AP is in S0. - * If the AP is in s0, cr50 should not be in deep sleep so wake up. - */ - GWRITE_FIELD(PINMUX, EXITEDGE0, DIOA3, 1); /* edge sensitive */ - GWRITE_FIELD(PINMUX, EXITINV0, DIOA3, 0); /* wake on high */ - GWRITE_FIELD(PINMUX, EXITEN0, DIOA3, 1); /* GPIO_DETECT_AP */ - - /* - * Whether it is a short pulse or long one waking on the rising edge is - * fine because the goal of the system reset signal is to reset the TPM - * and after resuming from deep sleep the TPM will be reset. Cr50 + * Whether it is a short pulse or long one waking on the high level is + * fine because the goal of the system reset signal is to reset the + * TPM and after resuming from deep sleep the TPM will be reset. Cr50 * doesn't need to read the low value and then reset. */ if (board_use_plt_rst()) { @@ -396,12 +379,25 @@ void board_configure_deep_sleep_wakepins(void) */ /* Disable plt_rst_l as a wake pin */ GWRITE_FIELD(PINMUX, EXITEN0, DIOM3, 0); - /* Reconfigure and reenable it. */ - GWRITE_FIELD(PINMUX, EXITEDGE0, DIOM3, 1); /* edge sensitive */ + /* + * Reconfigure it to be level sensitive so that we are + * guaranteed to wake up if the level turns up, no need to + * worry about missing the rising edge. + */ + GWRITE_FIELD(PINMUX, EXITEDGE0, DIOM3, 0); GWRITE_FIELD(PINMUX, EXITINV0, DIOM3, 0); /* wake on high */ /* enable powerdown exit */ GWRITE_FIELD(PINMUX, EXITEN0, DIOM3, 1); } else { + /* + * DIOA3 is GPIO_DETECT_AP which is used to detect if the AP + * is in S0. If the AP is in s0, cr50 should not be in deep + * sleep so wake up. + */ + GWRITE_FIELD(PINMUX, EXITEDGE0, DIOA3, 0); /* level sensitive */ + GWRITE_FIELD(PINMUX, EXITINV0, DIOA3, 0); /* wake on high */ + GWRITE_FIELD(PINMUX, EXITEN0, DIOA3, 1); + /* Configure cr50 to resume on the rising edge of sys_rst_l */ /* Disable sys_rst_l as a wake pin */ GWRITE_FIELD(PINMUX, EXITEN0, DIOM0, 0); @@ -439,32 +435,49 @@ static void configure_board_specific_gpios(void) * board type. This signal is used to monitor AP resets and reset the * TPM. * + * Also configure these pins to be wake triggers on the rising edge, + * this will apply to regular sleep only, entering deep sleep would + * reconfigure this. + * * plt_rst_l is on diom3, and sys_rst_l is on diom0. */ if (board_use_plt_rst()) { + /* Use plt_rst_l for device detect purposes. */ + device_states[DEVICE_AP].detect = GPIO_TPM_RST_L; + /* Use plt_rst_l as the tpm reset signal. */ GWRITE(PINMUX, GPIO1_GPIO0_SEL, GC_PINMUX_DIOM3_SEL); + + /* No interrupts from AP UART TX state change are needed. */ + gpio_disable_interrupt(GPIO_DETECT_AP); + /* Enbale the input */ GWRITE_FIELD(PINMUX, DIOM3_CTL, IE, 1); - /* Set power down for the equivalent of DIO_WAKE_FALLING */ /* Set to be edge sensitive */ GWRITE_FIELD(PINMUX, EXITEDGE0, DIOM3, 1); - /* Select failling edge polarity */ - GWRITE_FIELD(PINMUX, EXITINV0, DIOM3, 1); + /* Select rising edge polarity */ + GWRITE_FIELD(PINMUX, EXITINV0, DIOM3, 0); /* Enable powerdown exit on DIOM3 */ GWRITE_FIELD(PINMUX, EXITEN0, DIOM3, 1); } else { + /* Use AP UART TX for device detect purposes. */ + device_states[DEVICE_AP].detect = GPIO_DETECT_AP; + /* Use sys_rst_l as the tpm reset signal. */ GWRITE(PINMUX, GPIO1_GPIO0_SEL, GC_PINMUX_DIOM0_SEL); /* Enbale the input */ GWRITE_FIELD(PINMUX, DIOM0_CTL, IE, 1); - /* Set power down for the equivalent of DIO_WAKE_FALLING */ + /* Use AP UART TX as the DETECT AP signal. */ + GWRITE(PINMUX, GPIO1_GPIO1_SEL, GC_PINMUX_DIOA3_SEL); + /* Enbale the input */ + GWRITE_FIELD(PINMUX, DIOA3_CTL, IE, 1); + /* Set to be edge sensitive */ GWRITE_FIELD(PINMUX, EXITEDGE0, DIOM0, 1); - /* Select failling edge polarity */ - GWRITE_FIELD(PINMUX, EXITINV0, DIOM0, 1); + /* Select rising edge polarity */ + GWRITE_FIELD(PINMUX, EXITINV0, DIOM0, 0); /* Enable powerdown exit on DIOM0 */ GWRITE_FIELD(PINMUX, EXITEN0, DIOM0, 1); } @@ -576,31 +589,33 @@ int flash_regions_to_enable(struct g_flash_region *regions, return 3; } -/* This is the interrupt handler to react to TPM_RST_L */ -void tpm_rst_asserted(enum gpio_signal signal) +static void deferred_tpm_rst_isr(void) { - /* - * Cr50 drives SYS_RST_L in certain scenarios, in those cases - * this signal's assertion should be ignored here. - */ - CPRINTS("%s from %d", __func__, signal); - if (usb_spi_update_in_progress() || - tpm_is_resetting()) { - CPRINTS("%s ignored", __func__); + ccprintf("%T %s\n", __func__); + + if (board_use_plt_rst() && + device_state_changed(DEVICE_AP, DEVICE_STATE_ON)) + hook_notify(HOOK_CHIPSET_RESUME); + + if (!reboot_request_posted) { + /* Reset TPM, no need to wait for completion. */ + tpm_reset_request(0, 0); return; } - if (reboot_request_posted) { - /* - * Reset TPM and wait to completion to make sure nvmem is - * committed before reboot. - */ - tpm_reset_request(1, 0); - system_reset(SYSTEM_RESET_HARD); /* This will never return. */ - } else { - /* Reset TPM, no need to wait for completion. */ - tpm_reset_request(0, 0); - } + /* + * Reset TPM and wait to completion to make sure nvmem is + * committed before reboot. + */ + tpm_reset_request(1, 0); + system_reset(SYSTEM_RESET_HARD); /* This will never return. */ +} +DECLARE_DEFERRED(deferred_tpm_rst_isr); + +/* This is the interrupt handler to react to TPM_RST_L */ +void tpm_rst_deasserted(enum gpio_signal signal) +{ + hook_call_deferred(&deferred_tpm_rst_isr_data, 0); } void assert_sys_rst(void) @@ -656,13 +671,9 @@ int is_ec_rst_asserted(void) } static int device_state_changed(enum device_type device, - enum device_state state) + enum device_state state) { - /* - * We've determined the device state, so cancel any deferred callbacks. - */ hook_call_deferred(device_states[device].deferred, -1); - return device_set_state(device, state); } @@ -752,7 +763,6 @@ struct device_config device_states[] = { }, [DEVICE_AP] = { .deferred = &ap_deferred_data, - .detect = GPIO_DETECT_AP, .name = "AP" }, [DEVICE_EC] = { @@ -784,7 +794,7 @@ void device_state_on(enum gpio_signal signal) gpio_disable_interrupt(signal); switch (signal) { - case GPIO_DETECT_AP: + case GPIO_DETECT_AP: /* Would happen only on non plt_rst_l devices. */ if (device_state_changed(DEVICE_AP, DEVICE_STATE_ON)) hook_notify(HOOK_CHIPSET_RESUME); break; @@ -796,7 +806,7 @@ void device_state_on(enum gpio_signal signal) servo_attached(); break; default: - CPRINTS("Device not supported"); + CPRINTS("Device %d not supported", signal); return; } } @@ -810,21 +820,28 @@ void board_update_device_state(enum device_type device) * If the device is currently on set its state immediately. If it * thinks the device is powered off debounce the signal. */ - if (gpio_get_level(device_states[device].detect)) + if (gpio_get_level(device_states[device].detect)) { + if (device_get_state(device) == DEVICE_STATE_ON) + return; device_state_on(device_states[device].detect); - else { + } else { + if (device_get_state(device) == DEVICE_STATE_OFF) + return; device_set_state(device, DEVICE_STATE_UNKNOWN); - - gpio_enable_interrupt(device_states[device].detect); + if ((device != DEVICE_AP) || !board_use_plt_rst()) + gpio_enable_interrupt(device_states[device].detect); /* - * The signal is low now, but the detect signals are on UART RX - * which may be receiving something. Wait long enough for an - * entire data chunk to be sent to declare that the device is - * off. If the detect signal remains low for 100us then the - * signal is low because the device is off. + * The signal is low now, but this could be just an AP UART + * transmitting or PLT_RST_L pulsing. Let's wait long enough + * to debounce in both cases, pickng duration slightly shorter + * than the device polling iterval. + * + * Interrupts from the appropriate source (platform dependent) + * will cancel the deferred function if the signal is + * deasserted within the deferral interval. */ - hook_call_deferred(device_states[device].deferred, 100); + hook_call_deferred(device_states[device].deferred, 900 * MSEC); } } diff --git a/board/cr50/board.h b/board/cr50/board.h index c5d11a196f..bc48b31d26 100644 --- a/board/cr50/board.h +++ b/board/cr50/board.h @@ -157,7 +157,7 @@ enum usb_spi { void board_configure_deep_sleep_wakepins(void); /* Interrupt handler */ -void tpm_rst_asserted(enum gpio_signal signal); +void tpm_rst_deasserted(enum gpio_signal signal); void device_state_on(enum gpio_signal signal); void post_reboot_request(void); diff --git a/board/cr50/gpio.inc b/board/cr50/gpio.inc index f7fbcc3f51..f2ac78ae1d 100644 --- a/board/cr50/gpio.inc +++ b/board/cr50/gpio.inc @@ -55,7 +55,7 @@ * two internal GPIOs to the same pad if sys_rst_l is also being used to detect * system resets. */ -GPIO_INT(TPM_RST_L, PIN(1, 0), GPIO_INT_FALLING, tpm_rst_asserted) +GPIO_INT(TPM_RST_L, PIN(1, 0), GPIO_INT_RISING, tpm_rst_deasserted) GPIO_INT(DETECT_AP, PIN(1, 1), GPIO_INT_HIGH, device_state_on) GPIO_INT(DETECT_EC, PIN(1, 2), GPIO_INT_HIGH, device_state_on) GPIO_INT(DETECT_SERVO, PIN(1, 3), GPIO_INT_HIGH | GPIO_PULL_DOWN, @@ -145,7 +145,6 @@ PINMUX(FUNC(UART2_RX), B6, DIO_INPUT) /* EC console */ * driving the cr50 uart TX at the same time as servo is driving those pins may * damage both servo and cr50. */ -PINMUX(GPIO(DETECT_AP), A3, DIO_INPUT) PINMUX(GPIO(DETECT_EC), B6, DIO_INPUT) PINMUX(GPIO(DETECT_SERVO), B5, DIO_INPUT) diff --git a/common/tpm_registers.c b/common/tpm_registers.c index e5258df420..e2e286136f 100644 --- a/common/tpm_registers.c +++ b/common/tpm_registers.c @@ -11,6 +11,7 @@ #include "byteorder.h" #include "console.h" +#include "device_state.h" #include "extension.h" #include "link_defs.h" #include "nvmem.h" @@ -697,11 +698,6 @@ int tpm_reset_request(int wait_until_done, int wipe_nvmem_first) return EC_ERROR_TIMEOUT; } -int tpm_is_resetting(void) -{ - return reset_in_progress; -} - /* * A timeout hook to reinstate NVMEM commits soon after reset. * @@ -787,13 +783,30 @@ static void tpm_reset_now(int wipe_first) void tpm_task(void) { + uint32_t evt; + + /* + * Just in case there is a resume from deep sleep where AP is not out + * of reset, let's not proceed until AP is actually up. + */ + while (device_get_state(DEVICE_AP) != DEVICE_STATE_ON) { + /* + * The only event we should expect at this point would be the + * reset request. + */ + evt = task_wait_event(-1); + if (evt & TPM_EVENT_RESET) + break; + + cprints(CC_TASK, "%s: unexpected event %x\n", __func__, evt); + } + tpm_reset_now(0); while (1) { uint8_t *response; unsigned response_size; uint32_t command_code; struct tpm_cmd_header *tpmh; - uint32_t evt; /* Wait for the next command event */ evt = task_wait_event(-1); diff --git a/include/tpm_registers.h b/include/tpm_registers.h index 293a0a99ac..9d5fba4cf5 100644 --- a/include/tpm_registers.h +++ b/include/tpm_registers.h @@ -42,13 +42,6 @@ void tpm_register_interface(interface_restart_func interface_restart); */ int tpm_reset_request(int wait_until_done, int wipe_nvmem_first); -/* - * Return true if the TPM is being reset. Usually this helps to avoid - * unnecessary extra reset early at startup time, when TPM could be busy - * installing endorsement certificates. - */ -int tpm_is_resetting(void); - /* * This structure describes the header of all commands and responses sent and * received over TPM FIFO.