diff options
author | Hsu Wei-Cheng <mojahsu@chromium.org> | 2021-08-06 08:32:44 +0000 |
---|---|---|
committer | Hsu Wei-Cheng <mojahsu@chromium.org> | 2021-08-06 09:21:55 +0000 |
commit | 53c02958d338f8a4eabd2e5239800d1e7fbac60f (patch) | |
tree | ef49313fb86cf79cca0639ae317718f56309d104 | |
parent | c0121aadfa214f124f20e50c9fe2552f09d77dd1 (diff) | |
download | chrome-ec-53c02958d338f8a4eabd2e5239800d1e7fbac60f.tar.gz |
Revert "bloonchipper: Reintroduce sleep lines to RO"
This reverts commit 5e2a0808b7dfc736676daf8ba4d6cf683076d961.
Reason for revert: It may broke CQ
BUG=b:195718112
Original change's description:
> bloonchipper: Reintroduce sleep lines to RO
>
> Change https://crrev.com/c/2673909 removed the sleep lines from RO
> in an effort to minimize the RO complexity.
> Most notably, this isolated the deep-sleep/low-power-idle
> logic to RW only.
>
> Unfortunately, the sleep lines also control whether the SPI host interface
> is listening, which allows it to ignore spurious communication.
>
> It seems safer to reinstate the the sleep line with low power idle
> active and directly disable CONFIG_LOW_POWER_IDLE in subsequent CL.
>
> We reinstate the sleep line gpio logic from the following:
> https://crrev.com/c5545464431669029f42829d542fa491d767ee5f/board/hatch_fp/board.c
> This is the parent commit to the CL that refactors the sleep lines.
>
> BRANCH=none
> BUG=b:178746753
> TEST=# Connect servo_micro to a dragonclaw board.
> make proj-bloonchipper -j
> sudo servod --board=dragonclaw
> ./util/flash_ec --board=bloonchipper
>
> dut-control fpmcu_slp:off fpmcu_slp_alt:off
> dut-control pp3300_dx_mcu_mw # Should be around 20mw
> dut-control fpmcu_slp:on fpmcu_slp_alt:off
> dut-control pp3300_dx_mcu_mw # Should be less the 5mw
> dut-control fpmcu_slp:off fpmcu_slp_alt:on
> dut-control pp3300_dx_mcu_mw # Should be less the 5mw
> dut-control fpmcu_slp:on fpmcu_slp_alt:on
> dut-control pp3300_dx_mcu_mw # Should be less the 5mw
> dut-control fpmcu_slp:off fpmcu_slp_alt:off
>
> minicom -D$(dut-control -o raw_fpmcu_console_uart_pty)
> > reboot ro
> # Ctrl-A Q
>
> dut-control fpmcu_slp:off fpmcu_slp_alt:off
> dut-control pp3300_dx_mcu_mw # Should be around 20mw
> dut-control fpmcu_slp:on fpmcu_slp_alt:off
> dut-control pp3300_dx_mcu_mw # Should be less the 5mw
> dut-control fpmcu_slp:off fpmcu_slp_alt:on
> dut-control pp3300_dx_mcu_mw # Should be less the 5mw
> dut-control fpmcu_slp:on fpmcu_slp_alt:on
> dut-control pp3300_dx_mcu_mw # Should be less the 5mw
> dut-control fpmcu_slp:off fpmcu_slp_alt:off
>
> minicom -D$(dut-control -o raw_fpmcu_console_uart_pty)
> > reboot
> > fpenroll
> > fpmatch
> > reboot
> # Ctrl-A Q
>
> Signed-off-by: Craig Hesling <hesling@chromium.org>
> Change-Id: I503cb3b62740300b265a4ddb165e29d9e36727fd
> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3012241
> Commit-Queue: Josie Nordrum <josienordrum@google.com>
> Reviewed-by: Josie Nordrum <josienordrum@google.com>
Bug: b:178746753
Change-Id: If610fdffaef778f26c712bbad360e84651df61bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3077589
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Hsu Wei-Cheng <mojahsu@chromium.org>
Auto-Submit: Hsu Wei-Cheng <mojahsu@chromium.org>
-rw-r--r-- | board/hatch_fp/board.c | 66 | ||||
-rw-r--r-- | board/hatch_fp/board.h | 2 | ||||
-rw-r--r-- | board/hatch_fp/board_rw.c | 74 | ||||
-rw-r--r-- | board/hatch_fp/board_rw.h | 1 | ||||
-rw-r--r-- | board/hatch_fp/gpio.inc | 2 | ||||
-rw-r--r-- | board/hatch_fp/gpio_rw.inc | 2 |
6 files changed, 81 insertions, 66 deletions
diff --git a/board/hatch_fp/board.c b/board/hatch_fp/board.c index b48dce2b7e..c2a2e32107 100644 --- a/board/hatch_fp/board.c +++ b/board/hatch_fp/board.c @@ -26,45 +26,6 @@ int console_is_restricted(void) #include "gpio_list.h" -/* - * Some platforms have a broken SLP_S0_L signal (stuck to 0 in S0) - * if set, ignore it and only uses SLP_S3_L for the AP state. - */ -static bool broken_slp; - -static void ap_deferred(void) -{ - /* - * Behavior: - * AP Active (ex. Intel S0): SLP_L is 1 - * AP Suspend (ex. Intel S0ix): SLP_L is 0 - * The alternative SLP_ALT_L should be pulled high at all the times. - * - * Legacy Intel behavior: - * in S3: SLP_ALT_L is 0 and SLP_L is X. - * in S0ix: SLP_ALT_L is 1 and SLP_L is 0. - * in S0: SLP_ALT_L is 1 and SLP_L is 1. - * in S5/G3, the FP MCU should not be running. - */ - int running = gpio_get_level(GPIO_SLP_ALT_L) && - (gpio_get_level(GPIO_SLP_L) || broken_slp); - - if (running) { /* S0 */ - disable_sleep(SLEEP_MASK_AP_RUN); - hook_notify(HOOK_CHIPSET_RESUME); - } else { /* S0ix/S3 */ - hook_notify(HOOK_CHIPSET_SUSPEND); - enable_sleep(SLEEP_MASK_AP_RUN); - } -} -DECLARE_DEFERRED(ap_deferred); - -/* PCH power state changes */ -void slp_event(enum gpio_signal signal) -{ - hook_call_deferred(&ap_deferred_data, 0); -} - static void board_init_transport(void) { enum fp_transport_type ret_transport = get_fp_transport_type(); @@ -74,15 +35,6 @@ static void board_init_transport(void) /* Initialize transport based on bootstrap */ switch (ret_transport) { case FP_TRANSPORT_TYPE_UART: - /* - * The Zork variants currently have a broken SLP_S0_L signal - * (stuck to 0 in S0). For now, unconditionally ignore it here - * as they are the only UART users and the AP has no S0ix state. - * TODO(b/174695987) once the RW AP firmware has been updated - * on all those machines, remove this workaround. - */ - broken_slp = true; - /* Check if CONFIG_USART_HOST_COMMAND is enabled. */ if (IS_ENABLED(CONFIG_USART_HOST_COMMAND)) usart_host_command_init(); @@ -109,23 +61,13 @@ static void board_init_transport(void) /* Initialize board. */ static void board_init(void) { - /* Run until the first S3 entry */ + /* Run until the first S3 entry. + * No suspend-based power management in RO. + */ disable_sleep(SLEEP_MASK_AP_RUN); - + hook_notify(HOOK_CHIPSET_RESUME); board_init_transport(); - - /* Enable interrupt on PCH power signals */ - gpio_enable_interrupt(GPIO_SLP_ALT_L); - gpio_enable_interrupt(GPIO_SLP_L); - if (IS_ENABLED(SECTION_IS_RW)) board_init_rw(); - - /* - * Enable the SPI slave interface if the PCH is up. - * Do not use hook_call_deferred(), because ap_deferred() will be - * called after tasks with priority higher than HOOK task (very late). - */ - ap_deferred(); } DECLARE_HOOK(HOOK_INIT, board_init, HOOK_PRIO_DEFAULT); diff --git a/board/hatch_fp/board.h b/board/hatch_fp/board.h index d8d4062320..fd48157478 100644 --- a/board/hatch_fp/board.h +++ b/board/hatch_fp/board.h @@ -247,8 +247,6 @@ #include "gpio_signal.h" #include "board_rw.h" -void slp_event(enum gpio_signal signal); - #endif /* !__ASSEMBLER__ */ #endif /* __BOARD_H */ diff --git a/board/hatch_fp/board_rw.c b/board/hatch_fp/board_rw.c index 35709860e3..09c78c5e5c 100644 --- a/board/hatch_fp/board_rw.c +++ b/board/hatch_fp/board_rw.c @@ -7,8 +7,10 @@ #include "console.h" #include "fpsensor_detect.h" #include "gpio.h" +#include "hooks.h" #include "registers.h" #include "spi.h" +#include "system.h" #include "task.h" #include "usart_host_command.h" #include "util.h" @@ -17,6 +19,45 @@ #error "This file should only be built for RW." #endif +/* + * Some platforms have a broken SLP_S0_L signal (stuck to 0 in S0) + * if set, ignore it and only uses SLP_S3_L for the AP state. + */ +static bool broken_slp; + +static void ap_deferred(void) +{ + /* + * Behavior: + * AP Active (ex. Intel S0): SLP_L is 1 + * AP Suspend (ex. Intel S0ix): SLP_L is 0 + * The alternative SLP_ALT_L should be pulled high at all the times. + * + * Legacy Intel behavior: + * in S3: SLP_ALT_L is 0 and SLP_L is X. + * in S0ix: SLP_ALT_L is 1 and SLP_L is 0. + * in S0: SLP_ALT_L is 1 and SLP_L is 1. + * in S5/G3, the FP MCU should not be running. + */ + int running = gpio_get_level(GPIO_SLP_ALT_L) && + (gpio_get_level(GPIO_SLP_L) || broken_slp); + + if (running) { /* S0 */ + disable_sleep(SLEEP_MASK_AP_RUN); + hook_notify(HOOK_CHIPSET_RESUME); + } else { /* S0ix/S3 */ + hook_notify(HOOK_CHIPSET_SUSPEND); + enable_sleep(SLEEP_MASK_AP_RUN); + } +} +DECLARE_DEFERRED(ap_deferred); + +/* PCH power state changes */ +void slp_event(enum gpio_signal signal) +{ + hook_call_deferred(&ap_deferred_data, 0); +} + /* SPI devices */ struct spi_device_t spi_devices[] = { /* Fingerprint sensor (SCLK at 4Mhz) */ @@ -46,6 +87,39 @@ static void configure_fp_sensor_spi(void) void board_init_rw(void) { + enum fp_transport_type ret_transport = get_fp_transport_type(); + + /* + * FP_RST_ODL pin is defined in gpio_rw.inc (with GPIO_OUT_HIGH + * flag) but not in gpio.inc, so RO leaves this pin set to 0 (reset + * default), but RW doesn't initialize this pin to 1 because sysjump + * to RW is a warm reset (see gpio_pre_init() in chip/stm32/gpio.c). + * Explicitly reset FP_RST_ODL pin to default value. + */ + gpio_reset(GPIO_FP_RST_ODL); + + if (ret_transport == FP_TRANSPORT_TYPE_UART) { + /* + * The Zork variants currently have a broken SLP_S0_L signal + * (stuck to 0 in S0). For now, unconditionally ignore it here + * as they are the only UART users and the AP has no S0ix state. + * TODO(b/174695987) once the RW AP firmware has been updated + * on all those machines, remove this workaround. + */ + broken_slp = true; + } + /* Configure and enable SPI as master for FP sensor */ configure_fp_sensor_spi(); + + /* Enable interrupt on PCH power signals */ + gpio_enable_interrupt(GPIO_SLP_ALT_L); + gpio_enable_interrupt(GPIO_SLP_L); + + /* + * Enable the SPI slave interface if the PCH is up. + * Do not use hook_call_deferred(), because ap_deferred() will be + * called after tasks with priority higher than HOOK task (very late). + */ + ap_deferred(); } diff --git a/board/hatch_fp/board_rw.h b/board/hatch_fp/board_rw.h index 1bee6c947d..bcfa061b25 100644 --- a/board/hatch_fp/board_rw.h +++ b/board/hatch_fp/board_rw.h @@ -7,6 +7,7 @@ #define __CROS_EC_BOARD_HATCH_FP_BOARD_RW_H void fps_event(enum gpio_signal signal); +void slp_event(enum gpio_signal signal); void board_init_rw(void); diff --git a/board/hatch_fp/gpio.inc b/board/hatch_fp/gpio.inc index c5319c2bee..52c8f770f6 100644 --- a/board/hatch_fp/gpio.inc +++ b/board/hatch_fp/gpio.inc @@ -5,8 +5,6 @@ */ /* Interrupts */ -GPIO_INT(SLP_L, PIN(A, 8), GPIO_INT_BOTH, slp_event) -GPIO_INT(SLP_ALT_L, PIN(B, 6), GPIO_INT_BOTH, slp_event) GPIO_INT(SPI1_NSS, PIN(A, 4), GPIO_INPUT, spi_event) /* Inputs */ diff --git a/board/hatch_fp/gpio_rw.inc b/board/hatch_fp/gpio_rw.inc index 3dfe890c12..8dcadd68c2 100644 --- a/board/hatch_fp/gpio_rw.inc +++ b/board/hatch_fp/gpio_rw.inc @@ -10,6 +10,8 @@ /* Interrupts */ GPIO_INT(FPS_INT, PIN(A, 0), GPIO_INT_RISING, fps_event) +GPIO_INT(SLP_L, PIN(A, 8), GPIO_INT_BOTH, slp_event) +GPIO_INT(SLP_ALT_L, PIN(B, 6), GPIO_INT_BOTH, slp_event) /* Inputs */ GPIO(FP_SENSOR_SEL, PIN(B, 0), GPIO_INPUT) |