From 79cf4959d3a637fbc45fa62ec6ef994fe3841e89 Mon Sep 17 00:00:00 2001 From: cepetr Date: Wed, 6 Nov 2024 11:22:24 +0100 Subject: [PATCH] refactor(core): improve button driver (add interrupt support) [no changelog] --- core/embed/bootloader/bootui.c | 8 +- core/embed/bootloader/main.c | 4 +- .../extmod/modtrezorio/modtrezorio-poll.h | 2 +- core/embed/prodtest/main.c | 8 +- core/embed/rust/build.rs | 2 +- core/embed/rust/src/trezorhal/io.rs | 4 +- core/embed/rust/src/ui/layout/simplified.rs | 4 +- core/embed/trezorhal/button.h | 38 +++- core/embed/trezorhal/stm32f4/button.c | 168 +++++++++++++----- .../trezorhal/stm32f4/syscall_dispatch.c | 4 +- .../embed/trezorhal/stm32f4/syscall_numbers.h | 2 +- core/embed/trezorhal/stm32f4/syscall_stubs.c | 4 +- core/embed/trezorhal/unix/button.c | 156 ++++++++-------- core/embed/unix/main.c | 5 + 14 files changed, 260 insertions(+), 149 deletions(-) diff --git a/core/embed/bootloader/bootui.c b/core/embed/bootloader/bootui.c index 9980272d6c..6b173e892a 100644 --- a/core/embed/bootloader/bootui.c +++ b/core/embed/bootloader/bootui.c @@ -168,14 +168,14 @@ void ui_click(void) { void ui_click(void) { for (;;) { - button_read(); - if (button_state(BTN_LEFT) != 0 && button_state(BTN_RIGHT) != 0) { + button_get_event(); + if (button_is_down(BTN_LEFT) && button_is_down(BTN_RIGHT)) { break; } } for (;;) { - button_read(); - if (button_state(BTN_LEFT) != 1 && button_state(BTN_RIGHT) != 1) { + button_get_event(); + if (!button_is_down(BTN_LEFT) && !button_is_down(BTN_RIGHT)) { break; } } diff --git a/core/embed/bootloader/main.c b/core/embed/bootloader/main.c index c237752bb4..d1ba77e81f 100644 --- a/core/embed/bootloader/main.c +++ b/core/embed/bootloader/main.c @@ -523,8 +523,8 @@ int bootloader_main(void) { } } #elif defined USE_BUTTON - button_read(); - if (button_state(BTN_LEFT) == 1) { + button_get_event(); + if (button_is_down(BTN_LEFT)) { touched = 1; } #endif diff --git a/core/embed/extmod/modtrezorio/modtrezorio-poll.h b/core/embed/extmod/modtrezorio/modtrezorio-poll.h index e7bbc6603e..c795282e56 100644 --- a/core/embed/extmod/modtrezorio/modtrezorio-poll.h +++ b/core/embed/extmod/modtrezorio/modtrezorio-poll.h @@ -144,7 +144,7 @@ STATIC mp_obj_t mod_trezorio_poll(mp_obj_t ifaces, mp_obj_t list_ref, #endif #if USE_BUTTON else if (iface == BUTTON_IFACE) { - const uint32_t evt = button_read(); + const uint32_t evt = button_get_event(); if (evt & (BTN_EVT_DOWN | BTN_EVT_UP)) { mp_obj_tuple_t *tuple = MP_OBJ_TO_PTR(mp_obj_new_tuple(2, NULL)); uint32_t etype = (evt >> 24) & 0x3U; // button down/up diff --git a/core/embed/prodtest/main.c b/core/embed/prodtest/main.c index cbd888477b..7011d9be7a 100644 --- a/core/embed/prodtest/main.c +++ b/core/embed/prodtest/main.c @@ -244,13 +244,13 @@ static void test_display(const char *colors) { #ifdef USE_BUTTON static secbool test_btn_press(uint32_t deadline, uint32_t btn) { - while (button_read() != (btn | BTN_EVT_DOWN)) { + while (button_get_event() != (btn | BTN_EVT_DOWN)) { if (systick_ms() > deadline) { vcp_println("ERROR TIMEOUT"); return secfalse; } } - while (button_read() != (btn | BTN_EVT_UP)) { + while (button_get_event() != (btn | BTN_EVT_UP)) { if (systick_ms() > deadline) { vcp_println("ERROR TIMEOUT"); return secfalse; @@ -264,7 +264,7 @@ static secbool test_btn_all(uint32_t deadline) { bool left_pressed = 0; bool right_pressed = 0; while (true) { - uint32_t buttons = button_read(); + uint32_t buttons = button_get_event(); if (buttons == (BTN_LEFT | BTN_EVT_DOWN)) { left_pressed = 1; } @@ -287,7 +287,7 @@ static secbool test_btn_all(uint32_t deadline) { } while (true) { - uint32_t buttons = button_read(); + uint32_t buttons = button_get_event(); if (buttons == (BTN_LEFT | BTN_EVT_DOWN)) { left_pressed = 1; } diff --git a/core/embed/rust/build.rs b/core/embed/rust/build.rs index 8a62626ca6..85d906b27e 100644 --- a/core/embed/rust/build.rs +++ b/core/embed/rust/build.rs @@ -451,7 +451,7 @@ fn generate_trezorhal_bindings() { .allowlist_function("touch_get_event") // button .allowlist_type("button_t") - .allowlist_function("button_read") + .allowlist_function("button_get_event") // haptic .allowlist_type("haptic_effect_t") .allowlist_function("haptic_play") diff --git a/core/embed/rust/src/trezorhal/io.rs b/core/embed/rust/src/trezorhal/io.rs index b77c6d4b03..4fbaaad161 100644 --- a/core/embed/rust/src/trezorhal/io.rs +++ b/core/embed/rust/src/trezorhal/io.rs @@ -6,8 +6,8 @@ pub fn io_touch_get_event() -> u32 { } #[cfg(feature = "button")] -pub fn io_button_read() -> u32 { - unsafe { ffi::button_read() } +pub fn io_button_get_event() -> u32 { + unsafe { ffi::button_get_event() } } #[cfg(feature = "button")] diff --git a/core/embed/rust/src/ui/layout/simplified.rs b/core/embed/rust/src/ui/layout/simplified.rs index a966e1083a..23fe4cf669 100644 --- a/core/embed/rust/src/ui/layout/simplified.rs +++ b/core/embed/rust/src/ui/layout/simplified.rs @@ -1,5 +1,5 @@ #[cfg(feature = "button")] -use crate::trezorhal::io::io_button_read; +use crate::trezorhal::io::io_button_get_event; #[cfg(feature = "touch")] use crate::trezorhal::io::io_touch_get_event; #[cfg(feature = "button")] @@ -38,7 +38,7 @@ where } #[cfg(feature = "button")] fn button_eval() -> Option { - let event = io_button_read(); + let event = io_button_get_event(); if event == 0 { return None; } diff --git a/core/embed/trezorhal/button.h b/core/embed/trezorhal/button.h index 91c43d0488..4b69b2bd61 100644 --- a/core/embed/trezorhal/button.h +++ b/core/embed/trezorhal/button.h @@ -22,18 +22,48 @@ #include +// Button event is packed 32-bit value +// +// 31 24 23 0 +// |--------|-------------------------| +// | event | button identifier | +// |--------|-------------------------| +// +// + +// Button events #define BTN_EVT_DOWN (1U << 24) #define BTN_EVT_UP (1U << 25) +// Button identifiers typedef enum { BTN_LEFT = 0, BTN_RIGHT = 1, BTN_POWER = 2, } button_t; -void button_init(void); -uint32_t button_read(void); +#ifdef KERNEL_MODE -bool button_state(button_t button); +// Initializes button driver +// +// Returns true in case of success, false otherwise +bool button_init(void); -#endif +#endif // KERNEL_MODE + +// Get the last button event +// +// It's expected there's just one consumer of the button events, +// e.g. the main loop +// +// Returns 0 if no event is available +uint32_t button_get_event(void); + +// Checks if the specified button is currently pressed +// +// The current implementation returns the state of the button at the time +// `button_get_event()` was called. In the future, we may fix this limitation. +// For now, `button_get_event()` must be called before `button_is_down()`. +bool button_is_down(button_t button); + +#endif // TREZORHAL_BUTTON_H diff --git a/core/embed/trezorhal/stm32f4/button.c b/core/embed/trezorhal/stm32f4/button.c index 697f755a5d..8374598a6c 100644 --- a/core/embed/trezorhal/stm32f4/button.c +++ b/core/embed/trezorhal/stm32f4/button.c @@ -1,3 +1,21 @@ +/* + * This file is part of the Trezor project, https://trezor.io/ + * + * Copyright (c) SatoshiLabs + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ #include #include @@ -6,8 +24,29 @@ #ifdef KERNEL_MODE -static void init_btn(GPIO_TypeDef *port, uint16_t pin) { - GPIO_InitTypeDef GPIO_InitStructure; +// Button driver state +typedef struct { + bool initialized; + +#ifdef BTN_LEFT_PIN + bool left_down; +#endif +#ifdef BTN_RIGHT_PIN + bool right_down; +#endif +#ifdef BTN_POWER_PIN + bool power_down; +#endif + +} button_driver_t; + +// Button driver instance +static button_driver_t g_button_driver = { + .initialized = false, +}; + +static void button_setup_pin(GPIO_TypeDef *port, uint16_t pin) { + GPIO_InitTypeDef GPIO_InitStructure = {0}; GPIO_InitStructure.Mode = GPIO_MODE_INPUT; GPIO_InitStructure.Pull = GPIO_PULLUP; @@ -16,68 +55,91 @@ static void init_btn(GPIO_TypeDef *port, uint16_t pin) { HAL_GPIO_Init(port, &GPIO_InitStructure); } -#ifdef BTN_LEFT_CLK_ENA -static bool last_left = 0; -bool button_state_left(void) { return last_left; } -#endif +bool button_init(void) { + button_driver_t *drv = &g_button_driver; -#ifdef BTN_RIGHT_CLK_ENA -static bool last_right = 0; -bool button_state_right(void) { return last_right; } -#endif + if (drv->initialized) { + return true; + } -#ifdef BTN_POWER_CLK_ENA -static bool last_power = 0; -bool button_state_power(void) { return last_power; } -#endif + memset(drv, 0, sizeof(button_driver_t)); -void button_init(void) { -#ifdef BTN_LEFT_CLK_ENA +#ifdef BTN_LEFT_PIN BTN_LEFT_CLK_ENA(); - init_btn(BTN_LEFT_PORT, BTN_LEFT_PIN); + button_setup_pin(BTN_LEFT_PORT, BTN_LEFT_PIN); #endif -#ifdef BTN_RIGHT_CLK_ENA +#ifdef BTN_RIGHT_PIN BTN_RIGHT_CLK_ENA(); - init_btn(BTN_RIGHT_PORT, BTN_RIGHT_PIN); + button_setup_pin(BTN_RIGHT_PORT, BTN_RIGHT_PIN); #endif -#ifdef BTN_POWER_CLK_ENA +#ifdef BTN_POWER_PIN BTN_POWER_CLK_ENA(); - init_btn(BTN_POWER_PORT, BTN_POWER_PIN); + button_setup_pin(BTN_POWER_PORT, BTN_POWER_PIN); #endif + +#ifdef BTN_EXTI_INTERRUPT_HANDLER + // Setup interrupt handler + EXTI_HandleTypeDef EXTI_Handle = {0}; + EXTI_ConfigTypeDef EXTI_Config = {0}; + EXTI_Config.GPIOSel = BTN_EXTI_INTERRUPT_GPIOSEL; + EXTI_Config.Line = BTN_EXTI_INTERRUPT_LINE; + EXTI_Config.Mode = EXTI_MODE_INTERRUPT; + EXTI_Config.Trigger = EXTI_TRIGGER_FALLING; + HAL_EXTI_SetConfigLine(&EXTI_Handle, &EXTI_Config); + NVIC_SetPriority(BTN_EXTI_INTERRUPT_NUM, IRQ_PRI_NORMAL); + __HAL_GPIO_EXTI_CLEAR_FLAG(BTN_INT_PIN); + NVIC_EnableIRQ(BTN_EXTI_INTERRUPT_NUM); +#endif // BTN_EXTI_INTERRUPT_HANDLER + + drv->initialized = true; + + return true; } -uint32_t button_read(void) { -#ifdef BTN_LEFT_CLK_ENA - bool left = (GPIO_PIN_RESET == HAL_GPIO_ReadPin(BTN_LEFT_PORT, BTN_LEFT_PIN)); - if (last_left != left) { - last_left = left; - if (left) { +uint32_t button_get_event(void) { + button_driver_t *drv = &g_button_driver; + + if (!drv->initialized) { + return 0; + } + +#ifdef BTN_LEFT_PIN + bool left_down = + (GPIO_PIN_RESET == HAL_GPIO_ReadPin(BTN_LEFT_PORT, BTN_LEFT_PIN)); + + if (drv->left_down != left_down) { + drv->left_down = left_down; + if (left_down) { return BTN_EVT_DOWN | BTN_LEFT; } else { return BTN_EVT_UP | BTN_LEFT; } } #endif -#ifdef BTN_RIGHT_CLK_ENA - bool right = + +#ifdef BTN_RIGHT_PIN + bool right_down = (GPIO_PIN_RESET == HAL_GPIO_ReadPin(BTN_RIGHT_PORT, BTN_RIGHT_PIN)); - if (last_right != right) { - last_right = right; - if (right) { + + if (drv->right_down != right_down) { + drv->right_down = right_down; + if (right_down) { return BTN_EVT_DOWN | BTN_RIGHT; } else { return BTN_EVT_UP | BTN_RIGHT; } } #endif -#ifdef BTN_POWER_CLK_ENA - bool power = + +#ifdef BTN_POWER_PIN + bool power_down = (GPIO_PIN_RESET == HAL_GPIO_ReadPin(BTN_POWER_PORT, BTN_POWER_PIN)); - if (last_power != power) { - last_power = power; - if (power) { + + if (drv->power_down != power_down) { + drv->power_down = power_down; + if (power_down) { return BTN_EVT_DOWN | BTN_POWER; } else { return BTN_EVT_UP | BTN_POWER; @@ -88,23 +150,41 @@ uint32_t button_read(void) { return 0; } -bool button_state(button_t button) { +bool button_is_down(button_t button) { + button_driver_t *drv = &g_button_driver; + + if (!drv->initialized) { + return false; + } + switch (button) { -#ifdef BTN_LEFT_CLK_ENA +#ifdef BTN_LEFT_PIN case BTN_LEFT: - return button_state_left(); + return drv->left_down; #endif -#ifdef BTN_RIGHT_CLK_ENA +#ifdef BTN_RIGHT_PIN case BTN_RIGHT: - return button_state_right(); + return drv->right_down; #endif -#ifdef BTN_POWER_CLK_ENA +#ifdef BTN_POWER_PIN case BTN_POWER: - return button_state_power(); + return drv->power_down; #endif default: return false; } } +#ifdef BTN_EXTI_INTERRUPT_HANDLER +void BTN_EXTI_INTERRUPT_HANDLER(void) { + // button_driver_t *drv = &g_button_driver; + + // Clear the EXTI line pending bit + __HAL_GPIO_EXTI_CLEAR_FLAG(BTN_INT_PIN); + + // Inform the powerctl module about button press + // wakeup_flags_set(WAKEUP_FLAGS_BUTTON); +} +#endif + #endif // KERNEL_MODE diff --git a/core/embed/trezorhal/stm32f4/syscall_dispatch.c b/core/embed/trezorhal/stm32f4/syscall_dispatch.c index ef238388b1..81fc736c73 100644 --- a/core/embed/trezorhal/stm32f4/syscall_dispatch.c +++ b/core/embed/trezorhal/stm32f4/syscall_dispatch.c @@ -387,8 +387,8 @@ __attribute((no_stack_protector)) void syscall_handler(uint32_t *args, } break; #ifdef USE_BUTTON - case SYSCALL_BUTTON_READ: { - args[0] = button_read(); + case SYSCALL_BUTTON_GET_EVENT: { + args[0] = button_get_event(); } break; #endif diff --git a/core/embed/trezorhal/stm32f4/syscall_numbers.h b/core/embed/trezorhal/stm32f4/syscall_numbers.h index 3d29f1f070..0f0c1ad666 100644 --- a/core/embed/trezorhal/stm32f4/syscall_numbers.h +++ b/core/embed/trezorhal/stm32f4/syscall_numbers.h @@ -88,7 +88,7 @@ typedef enum { SYSCALL_SECRET_BOOTLOADER_LOCKED, - SYSCALL_BUTTON_READ, + SYSCALL_BUTTON_GET_EVENT, SYSCALL_TOUCH_GET_EVENT, diff --git a/core/embed/trezorhal/stm32f4/syscall_stubs.c b/core/embed/trezorhal/stm32f4/syscall_stubs.c index 952808d0a7..f20aeb7db8 100644 --- a/core/embed/trezorhal/stm32f4/syscall_stubs.c +++ b/core/embed/trezorhal/stm32f4/syscall_stubs.c @@ -359,7 +359,9 @@ secbool secret_bootloader_locked(void) { #include "button.h" -uint32_t button_read(void) { return syscall_invoke0(SYSCALL_BUTTON_READ); } +uint32_t button_get_event(void) { + return syscall_invoke0(SYSCALL_BUTTON_GET_EVENT); +} // ============================================================================= // touch.h diff --git a/core/embed/trezorhal/unix/button.c b/core/embed/trezorhal/unix/button.c index 716ffeab70..8f5f3de166 100644 --- a/core/embed/trezorhal/unix/button.c +++ b/core/embed/trezorhal/unix/button.c @@ -24,106 +24,100 @@ #include "button.h" -#ifdef BTN_LEFT_KEY -static bool last_left = 0; -#endif - -#ifdef BTN_RIGHT_KEY -static bool last_right = 0; -#endif - -#ifdef BTN_POWER_KEY -static bool last_power = 0; -#endif +// Button driver state +typedef struct { + bool initialized; #ifdef BTN_LEFT_KEY -bool button_state_left(void) { return last_left; } + bool left_down; #endif - #ifdef BTN_RIGHT_KEY -bool button_state_right(void) { return last_right; } + bool right_down; #endif - #ifdef BTN_POWER_KEY -bool button_state_power(void) { return last_power; } + bool power_down; #endif +} button_driver_t; + +// Button driver instance +static button_driver_t g_button_driver = { + .initialized = false, +}; + +bool button_init(void) { + button_driver_t *drv = &g_button_driver; + + if (drv->initialized) { + return true; + } + + memset(drv, 0, sizeof(button_driver_t)); + + drv->initialized = true; + + return true; +} + +uint32_t button_get_event(void) { + button_driver_t *drv = &g_button_driver; + + if (!drv->initialized) { + return 0; + } + + SDL_Event event; + + if (SDL_PollEvent(&event) > 0 && + (event.type == SDL_KEYDOWN || event.type == SDL_KEYUP) && + !event.key.repeat) { + bool down = (event.type == SDL_KEYDOWN); + uint32_t evt = down ? BTN_EVT_DOWN : BTN_EVT_UP; + + switch (event.key.keysym.sym) { +#ifdef BTN_LEFT_KEY + case BTN_LEFT_KEY: + drv->left_down = down; + return evt | BTN_LEFT; +#endif +#ifdef BTN_RIGHT_KEY + case BTN_RIGHT_KEY: + drv->right_down = down; + return evt | BTN_RIGHT; +#endif +#ifdef BTN_POWER_KEY + case BTN_POWER_KEY: + drv->power_down = down; + return evt | BTN_POWER; +#endif + default: + break; + } + } + + return 0; +} + +bool button_is_down(button_t button) { + button_driver_t *drv = &g_button_driver; + + if (!drv->initialized) { + return false; + } -bool button_state(button_t button) { switch (button) { #ifdef BTN_LEFT_KEY case BTN_LEFT: - return button_state_left(); + return drv->left_down; #endif #ifdef BTN_RIGHT_KEY case BTN_RIGHT: - return button_state_right(); + return drv->right_down; #endif #ifdef BTN_POWER_KEY case BTN_POWER: - return button_state_power(); + return drv->power_down; #endif default: return false; } } - -uint32_t button_read(void) { - SDL_Event event; - if (SDL_PollEvent(&event) > 0) { - switch (event.type) { - case SDL_KEYDOWN: - if (event.key.repeat) { - break; - } - switch (event.key.keysym.sym) { -#ifdef BTN_LEFT_KEY - case BTN_LEFT_KEY: - last_left = 1; - return BTN_EVT_DOWN | BTN_LEFT; -#endif -#ifdef BTN_RIGHT_KEY - case BTN_RIGHT_KEY: - last_right = 1; - return BTN_EVT_DOWN | BTN_RIGHT; -#endif -#ifdef BTN_POWER_KEY - case BTN_POWER_KEY: - last_power = 1; - return BTN_EVT_DOWN | BTN_POWER; -#endif - default: - break; - } - break; - case SDL_KEYUP: - if (event.key.repeat) { - break; - } - switch (event.key.keysym.sym) { -#ifdef BTN_LEFT_KEY - case BTN_LEFT_KEY: - last_left = 0; - return BTN_EVT_UP | BTN_LEFT; -#endif -#ifdef BTN_RIGHT_KEY - case BTN_RIGHT_KEY: - last_right = 0; - return BTN_EVT_UP | BTN_RIGHT; -#endif -#ifdef BTN_POWER_KEY - case BTN_POWER_KEY: - last_power = 0; - return BTN_EVT_UP | BTN_POWER; -#endif - default: - break; - } - break; - default: - break; - } - } - return 0; -} - -void button_init(void) {} diff --git a/core/embed/unix/main.c b/core/embed/unix/main.c index ccf68db89a..6167aab1d1 100644 --- a/core/embed/unix/main.c +++ b/core/embed/unix/main.c @@ -36,6 +36,7 @@ #include #include +#include "button.h" #include "display.h" #include "extmod/misc.h" #include "extmod/vfs_posix.h" @@ -520,6 +521,10 @@ MP_NOINLINE int main_(int argc, char **argv) { touch_init(); #endif +#ifdef USE_BUTTON + button_init(); +#endif + // Map trezor.flash to memory. flash_init(); flash_otp_init();