refactor(core): improve button driver (add interrupt support)

[no changelog]

core/embed/bootloader/bootui.c

@@ -168,14 +168,14 @@ void ui_click(void) {
 
 void ui_click(void) {
   for (;;) {
-    button_read();
+    button_get_event();
-    if (button_state(BTN_LEFT) != 0 && button_state(BTN_RIGHT) != 0) {
+    if (button_is_down(BTN_LEFT) && button_is_down(BTN_RIGHT)) {
       break;
     }
   }
   for (;;) {
-    button_read();
+    button_get_event();
-    if (button_state(BTN_LEFT) != 1 && button_state(BTN_RIGHT) != 1) {
+    if (!button_is_down(BTN_LEFT) && !button_is_down(BTN_RIGHT)) {
       break;
     }
   }

core/embed/bootloader/main.c

@@ -523,8 +523,8 @@ int bootloader_main(void) {
     }
   }
 #elif defined USE_BUTTON
-  button_read();
+  button_get_event();
-  if (button_state(BTN_LEFT) == 1) {
+  if (button_is_down(BTN_LEFT)) {
     touched = 1;
   }
 #endif

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

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;
     }

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")

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 {
+pub fn io_button_get_event() -> u32 {
-    unsafe { ffi::button_read() }
+    unsafe { ffi::button_get_event() }
 }
 
 #[cfg(feature = "button")]

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<ButtonEvent> {
-    let event = io_button_read();
+    let event = io_button_get_event();
     if event == 0 {
         return None;
     }

core/embed/trezorhal/button.h

@@ -22,18 +22,48 @@
 
 #include <trezor_types.h>
 
+// 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);
+#ifdef KERNEL_MODE
-uint32_t button_read(void);
 
-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

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 <http://www.gnu.org/licenses/>.
+ */
 
 #include <trezor_bsp.h>
 #include <trezor_rtl.h>
@@ -6,8 +24,29 @@
 
 #ifdef KERNEL_MODE
 
-static void init_btn(GPIO_TypeDef *port, uint16_t pin) {
+// Button driver state
-  GPIO_InitTypeDef GPIO_InitStructure;
+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
+bool button_init(void) {
-static bool last_left = 0;
+  button_driver_t *drv = &g_button_driver;
-bool button_state_left(void) { return last_left; }
-#endif
 
-#ifdef BTN_RIGHT_CLK_ENA
+  if (drv->initialized) {
-static bool last_right = 0;
+    return true;
-bool button_state_right(void) { return last_right; }
+  }
-#endif
 
-#ifdef BTN_POWER_CLK_ENA
+  memset(drv, 0, sizeof(button_driver_t));
-static bool last_power = 0;
-bool button_state_power(void) { return last_power; }
-#endif
 
-void button_init(void) {
+#ifdef BTN_LEFT_PIN
-#ifdef BTN_LEFT_CLK_ENA
   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) {
+uint32_t button_get_event(void) {
-#ifdef BTN_LEFT_CLK_ENA
+  button_driver_t *drv = &g_button_driver;
-  bool left = (GPIO_PIN_RESET == HAL_GPIO_ReadPin(BTN_LEFT_PORT, BTN_LEFT_PIN));
+
-  if (last_left != left) {
+  if (!drv->initialized) {
-    last_left = left;
+    return 0;
-    if (left) {
+  }
+
+#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 (drv->right_down != right_down) {
-    if (right) {
+    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 (drv->power_down != power_down) {
-    if (power) {
+    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

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: {
+    case SYSCALL_BUTTON_GET_EVENT: {
-      args[0] = button_read();
+      args[0] = button_get_event();
     } break;
 #endif
 

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,
 

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

core/embed/trezorhal/unix/button.c

@@ -24,106 +24,100 @@
 
 #include "button.h"
 
-#ifdef BTN_LEFT_KEY
+// Button driver state
-static bool last_left = 0;
+typedef struct {
-#endif
+  bool initialized;
-
-#ifdef BTN_RIGHT_KEY
-static bool last_right = 0;
-#endif
-
-#ifdef BTN_POWER_KEY
-static bool last_power = 0;
-#endif
 
 #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) {}

core/embed/unix/main.c

@@ -36,6 +36,7 @@
 #include <sys/types.h>
 #include <unistd.h>
 
+#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();