button events refactor

core/embed/io/button/inc/io/button.h

@@ -17,8 +17,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#ifndef TREZORHAL_BUTTON_H
+#pragma once
-#define TREZORHAL_BUTTON_H
 
 #include <trezor_types.h>
 
@@ -31,9 +30,10 @@
 //
 //
 
-// Button events
+typedef enum {
-#define BTN_EVT_DOWN (1U << 24)
+  BTN_EVENT_UP,
-#define BTN_EVT_UP (1U << 25)
+  BTN_EVENT_DOWN,
+} button_event_type_t;
 
 // Button identifiers
 typedef enum {
@@ -42,6 +42,11 @@ typedef enum {
   BTN_POWER = 2,
 } button_t;
 
+typedef struct {
+  button_event_type_t event_type;
+  button_t button;
+} button_event_t;
+
 #ifdef KERNEL_MODE
 
 // Initializes button driver
@@ -59,8 +64,8 @@ void button_deinit(void);
 // It's expected there's just one consumer of the button events,
 // e.g. the main loop
 //
-// Returns 0 if no event is available
+// Returns false if no event is available
-uint32_t button_get_event(void);
+bool button_get_event(button_event_t* event);
 
 // Checks if the specified button is currently pressed
 //
@@ -68,5 +73,3 @@ uint32_t button_get_event(void);
 // `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/io/button/stm32/button.c

@@ -110,11 +110,13 @@ void button_deinit(void) {
 #endif
 }
 
-uint32_t button_get_event(void) {
+bool button_get_event(button_event_t *event) {
   button_driver_t *drv = &g_button_driver;
 
+  memset(event, 0, sizeof(*event));
+
   if (!drv->initialized) {
-    return 0;
+    return false;
   }
 
 #ifdef BTN_LEFT_PIN
@@ -124,9 +126,13 @@ uint32_t button_get_event(void) {
   if (drv->left_down != left_down) {
     drv->left_down = left_down;
     if (left_down) {
-      return BTN_EVT_DOWN | BTN_LEFT;
+      event->button = BTN_LEFT;
+      event->event_type = BTN_EVENT_DOWN;
+      return true;
     } else {
-      return BTN_EVT_UP | BTN_LEFT;
+      event->button = BTN_LEFT;
+      event->event_type = BTN_EVENT_UP;
+      return true;
     }
   }
 #endif
@@ -138,9 +144,13 @@ uint32_t button_get_event(void) {
   if (drv->right_down != right_down) {
     drv->right_down = right_down;
     if (right_down) {
-      return BTN_EVT_DOWN | BTN_RIGHT;
+      event->button = BTN_RIGHT;
+      event->event_type = BTN_EVENT_DOWN;
+      return true;
     } else {
-      return BTN_EVT_UP | BTN_RIGHT;
+      event->button = BTN_RIGHT;
+      event->event_type = BTN_EVENT_UP;
+      return true;
     }
   }
 #endif
@@ -152,9 +162,13 @@ uint32_t button_get_event(void) {
   if (drv->power_down != power_down) {
     drv->power_down = power_down;
     if (power_down) {
-      return BTN_EVT_DOWN | BTN_POWER;
+      event->button = BTN_POWER;
+      event->event_type = BTN_EVENT_DOWN;
+      return true;
     } else {
-      return BTN_EVT_UP | BTN_POWER;
+      event->button = BTN_POWER;
+      event->event_type = BTN_EVENT_UP;
+      return true;
     }
   }
 #endif

core/embed/projects/prodtest/cmd/prodtest_button.c

@@ -30,7 +30,9 @@ static void test_single_button(cli_t* cli, uint32_t timeout, button_t btn) {
 
   cli_trace(cli, "Waiting for the button press...");
 
-  while (button_get_event() != (btn | BTN_EVT_DOWN)) {
+  button_event_t btn_event = {0};
+  while (!button_get_event(&btn_event) || !(btn_event.button == btn) ||
+         !(btn_event.event_type == BTN_EVENT_DOWN)) {
     if (ticks_expired(expire_time)) {
       cli_error(cli, CLI_ERROR_TIMEOUT, "");
       return;
@@ -43,7 +45,8 @@ static void test_single_button(cli_t* cli, uint32_t timeout, button_t btn) {
 
   cli_trace(cli, "Waiting for the button release...");
 
-  while (button_get_event() != (btn | BTN_EVT_UP)) {
+  while (!button_get_event(&btn_event) || !(btn_event.button == btn) ||
+         !(btn_event.event_type == BTN_EVENT_UP)) {
     if (ticks_expired(expire_time)) {
       cli_error(cli, CLI_ERROR_TIMEOUT, "");
       return;
@@ -65,7 +68,8 @@ static void test_button_combination(cli_t* cli, uint32_t timeout, button_t btn1,
 
   while (true) {
     // Event must be read before calling `button_is_down()`
-    button_get_event();
+    button_event_t e = {0};
+    button_get_event(&e);
 
     if (button_is_down(btn1) && button_is_down(btn2)) {
       break;
@@ -81,7 +85,8 @@ static void test_button_combination(cli_t* cli, uint32_t timeout, button_t btn1,
 
   while (true) {
     // Event must be read before calling `button_is_down()`
-    button_get_event();
+    button_event_t e = {0};
+    button_get_event(&e);
 
     if (!button_is_down(btn1) && !button_is_down(btn2)) {
       break;

core/embed/rust/src/trezorhal/button.rs

@@ -0,0 +1,35 @@
+use super::ffi;
+
+use num_traits::FromPrimitive;
+
+#[derive(Copy, Clone, PartialEq, Eq, FromPrimitive)]
+#[cfg_attr(feature = "debug", derive(ufmt::derive::uDebug))]
+pub enum PhysicalButton {
+    Left = ffi::button_t_BTN_LEFT as _,
+    Right = ffi::button_t_BTN_RIGHT as _,
+    Power = ffi::button_t_BTN_POWER as _,
+}
+
+#[derive(Copy, Clone, PartialEq, Eq, FromPrimitive)]
+#[cfg_attr(feature = "debug", derive(ufmt::derive::uDebug))]
+pub enum PhysicalButtonEvent {
+    Down = ffi::button_event_type_t_BTN_EVENT_DOWN as _,
+    Up = ffi::button_event_type_t_BTN_EVENT_UP as _,
+}
+
+pub fn button_get_event() -> Option<(PhysicalButton, PhysicalButtonEvent)> {
+    unsafe {
+        let mut e = ffi::button_event_t {
+            event_type: ffi::button_event_type_t_BTN_EVENT_DOWN,
+            button: ffi::button_t_BTN_LEFT,
+        };
+        if ffi::button_get_event(&mut e as _) {
+            Some((
+                unwrap!(PhysicalButton::from_u8(e.button)),
+                unwrap!(PhysicalButtonEvent::from_u8(e.event_type)),
+            ))
+        } else {
+            None
+        }
+    }
+}

core/embed/rust/src/trezorhal/io.rs

@@ -1,20 +0,0 @@
-use super::ffi;
-
-#[cfg(feature = "touch")]
-pub fn io_touch_get_event() -> u32 {
-    unsafe { ffi::touch_get_event() }
-}
-
-#[cfg(feature = "button")]
-pub fn io_button_get_event() -> u32 {
-    unsafe { ffi::button_get_event() }
-}
-
-#[cfg(feature = "button")]
-#[derive(Copy, Clone, PartialEq, Eq, FromPrimitive)]
-#[cfg_attr(feature = "debug", derive(ufmt::derive::uDebug))]
-pub enum PhysicalButton {
-    Left = ffi::button_t_BTN_LEFT as _,
-    Right = ffi::button_t_BTN_RIGHT as _,
-    Power = ffi::button_t_BTN_POWER as _,
-}

core/embed/rust/src/trezorhal/mod.rs

@@ -12,7 +12,11 @@ mod ffi;
 #[cfg(feature = "haptic")]
 pub mod haptic;
 
-pub mod io;
+#[cfg(feature = "button")]
+pub mod button;
+
+#[cfg(feature = "touch")]
+pub mod touch;
 
 #[cfg(feature = "hw_jpeg_decoder")]
 pub mod jpegdec;

core/embed/rust/src/trezorhal/touch.rs

@@ -0,0 +1,5 @@
+use super::ffi;
+
+pub fn touch_get_event() -> u32 {
+    unsafe { ffi::touch_get_event() }
+}

core/embed/rust/src/ui/event/button.rs

@@ -1,7 +1,7 @@
 use crate::error::Error;
-use num_traits::FromPrimitive;
 
-pub use crate::trezorhal::io::PhysicalButton;
+pub use crate::trezorhal::button::PhysicalButton;
+use crate::trezorhal::button::PhysicalButtonEvent;
 
 #[derive(Copy, Clone, PartialEq, Eq)]
 #[cfg_attr(feature = "debug", derive(ufmt::derive::uDebug))]
@@ -19,15 +19,10 @@ pub enum ButtonEvent {
 }
 
 impl ButtonEvent {
-    pub fn new(event: u32, button: u32) -> Result<Self, Error> {
+    pub fn new(event: PhysicalButtonEvent, button: PhysicalButton) -> Result<Self, Error> {
-        let button = PhysicalButton::from_u32(button);
+        let result = match event {
-
+            PhysicalButtonEvent::Down => Self::ButtonPressed(button),
-        let button = button.ok_or(Error::OutOfRange)?;
+            PhysicalButtonEvent::Up => Self::ButtonReleased(button),
-
-        let result = match event & 0xFF {
-            1 => Self::ButtonPressed(button),
-            2 => Self::ButtonReleased(button),
-            _ => return Err(Error::OutOfRange),
         };
         Ok(result)
     }

core/embed/rust/src/ui/layout/obj.rs

@@ -4,11 +4,12 @@ use core::{
     marker::PhantomData,
     ops::{Deref, DerefMut},
 };
+use num_traits::FromPrimitive;
 
 #[cfg(feature = "touch")]
 use crate::ui::{event::TouchEvent, geometry::Direction};
 #[cfg(feature = "touch")]
-use num_traits::{FromPrimitive, ToPrimitive};
+use num_traits::ToPrimitive;
 
 #[cfg(feature = "ble")]
 use crate::micropython::buffer::get_buffer;
@@ -16,8 +17,12 @@ use crate::micropython::buffer::get_buffer;
 use crate::ui::event::BLEEvent;
 
 #[cfg(feature = "button")]
-use crate::ui::event::ButtonEvent;
+use crate::{
+    trezorhal::button::{PhysicalButton, PhysicalButtonEvent},
+    ui::event::ButtonEvent,
+};
 
+use super::base::{Layout, LayoutState};
 use crate::{
     error::Error,
     maybe_trace::MaybeTrace,
@@ -46,8 +51,6 @@ use crate::{
     },
 };
 
-use super::base::{Layout, LayoutState};
-
 impl AttachType {
     fn to_obj(self) -> Obj {
         match self {
@@ -516,7 +519,13 @@ extern "C" fn ui_layout_button_event(n_args: usize, args: *const Obj) -> Obj {
             return Err(Error::TypeError);
         }
         let this: Gc<LayoutObj> = args[0].try_into()?;
-        let event = ButtonEvent::new(args[1].try_into()?, args[2].try_into()?)?;
+        let event_type_num: u8 = args[1].try_into()?;
+        let button_num: u8 = args[2].try_into()?;
+
+        let event_type = unwrap!(PhysicalButtonEvent::from_u8(event_type_num));
+        let button = unwrap!(PhysicalButton::from_u8(button_num));
+
+        let event = ButtonEvent::new(event_type, button)?;
         let msg = this.inner_mut().obj_event(Event::Button(event))?;
         Ok(msg)
     };

core/embed/rust/src/ui/layout/simplified.rs

@@ -1,7 +1,7 @@
 #[cfg(feature = "button")]
-use crate::trezorhal::io::io_button_get_event;
+use crate::trezorhal::button::button_get_event;
 #[cfg(feature = "touch")]
-use crate::trezorhal::io::io_touch_get_event;
+use crate::trezorhal::touch::touch_get_event;
 #[cfg(feature = "button")]
 use crate::ui::event::ButtonEvent;
 #[cfg(feature = "touch")]
@@ -35,13 +35,10 @@ where
 }
 #[cfg(feature = "button")]
 fn button_eval() -> Option<ButtonEvent> {
-    let event = io_button_get_event();
+    let event = button_get_event();
-    if event == 0 {
+    let Some((event_btn, event_type)) = event else {
         return None;
-    }
+    };
-
-    let event_type = event >> 24;
-    let event_btn = event & 0xFFFFFF;
 
     let event = ButtonEvent::new(event_type, event_btn);
 
@@ -84,7 +81,7 @@ pub fn run(frame: &mut impl Component<Msg = impl ReturnToC>) -> u32 {
         #[cfg(all(feature = "button", not(feature = "touch")))]
         let event = button_eval();
         #[cfg(feature = "touch")]
-        let event = touch_unpack(io_touch_get_event());
+        let event = touch_unpack(touch_get_event());
         if let Some(e) = event {
             let mut ctx = EventCtx::new();
             #[cfg(all(feature = "button", not(feature = "touch")))]

core/embed/sys/syscall/stm32/syscall_dispatch.c

@@ -420,7 +420,8 @@ __attribute((no_stack_protector)) void syscall_handler(uint32_t *args,
 
 #ifdef USE_BUTTON
     case SYSCALL_BUTTON_GET_EVENT: {
-      args[0] = button_get_event();
+      button_event_t *event = (button_event_t *)args[0];
+      args[0] = button_get_event__verified(event);
     } break;
 #endif
 

core/embed/sys/syscall/stm32/syscall_stubs.c

@@ -363,8 +363,8 @@ secbool secret_bootloader_locked(void) {
 
 #include <io/button.h>
 
-uint32_t button_get_event(void) {
+bool button_get_event(button_event_t *event) {
-  return syscall_invoke0(SYSCALL_BUTTON_GET_EVENT);
+  return syscall_invoke1(SYSCALL_BUTTON_GET_EVENT, (uint32_t)event);
 }
 
 #endif

core/embed/sys/syscall/stm32/syscall_verifiers.c

@@ -861,6 +861,8 @@ access_violation:
 
 #endif  // USE_HW_JPEG_DECODER
 
+// ---------------------------------------------------------------------
+
 #ifdef USE_DMA2D
 
 bool dma2d_rgb565_fill__verified(const gfx_bitblt_t *bb) {
@@ -1050,4 +1052,24 @@ access_violation:
 
 #endif
 
+// ---------------------------------------------------------------------
+
+#ifdef USE_BUTTON
+
+#include <io/button.h>
+
+bool button_get_event__verified(button_event_t *event) {
+  if (!probe_write_access(event, sizeof(*event))) {
+    goto access_violation;
+  }
+
+  return button_get_event(event);
+
+access_violation:
+  apptask_access_violation();
+  return false;
+}
+
+#endif
+
 #endif  // SYSCALL_DISPATCH

core/embed/sys/syscall/stm32/syscall_verifiers.h

@@ -255,4 +255,13 @@ bool dma2d_rgba8888_blend_mono8__verified(const gfx_bitblt_t *bb);
 
 #endif
 
+// ---------------------------------------------------------------------
+#ifdef USE_BUTTON
+
+#include <io/button.h>
+
+bool button_get_event__verified(button_event_t *event);
+
+#endif
+
 #endif  // SYSCALL_DISPATCH

core/embed/upymod/modtrezorio/modtrezorio-poll.h

@@ -155,11 +155,12 @@ STATIC mp_obj_t mod_trezorio_poll(mp_obj_t ifaces, mp_obj_t list_ref,
       }
 #if USE_BUTTON
       else if (iface == BUTTON_IFACE) {
-        const uint32_t evt = button_get_event();
+        button_event_t btn_event = {0};
-        if (evt & (BTN_EVT_DOWN | BTN_EVT_UP)) {
+        const bool btn = button_get_event(&btn_event);
+        if (btn) {
           mp_obj_tuple_t *tuple = MP_OBJ_TO_PTR(mp_obj_new_tuple(2, NULL));
-          uint32_t etype = (evt >> 24) & 0x3U;  // button down/up
+          uint32_t etype = btn_event.event_type;
-          uint32_t en = evt & 0xFFFF;           // button number
+          uint32_t en = btn_event.button;
           if (display_get_orientation() == 180) {
             en = (en == BTN_LEFT) ? BTN_RIGHT : BTN_LEFT;
           }

core/embed/upymod/modtrezorio/modtrezorio.c

@@ -111,10 +111,8 @@ STATIC const mp_rom_map_elem_t mp_module_trezorio_globals_table[] = {
 #endif
 #ifdef USE_BUTTON
     {MP_ROM_QSTR(MP_QSTR_BUTTON), MP_ROM_INT(BUTTON_IFACE)},
-    {MP_ROM_QSTR(MP_QSTR_BUTTON_PRESSED),
+    {MP_ROM_QSTR(MP_QSTR_BUTTON_PRESSED), MP_ROM_INT(BTN_EVENT_DOWN)},
-     MP_ROM_INT((BTN_EVT_DOWN >> 24) & 0x3U)},
+    {MP_ROM_QSTR(MP_QSTR_BUTTON_RELEASED), MP_ROM_INT(BTN_EVENT_UP)},
-    {MP_ROM_QSTR(MP_QSTR_BUTTON_RELEASED),
-     MP_ROM_INT((BTN_EVT_UP >> 24) & 0x3U)},
     {MP_ROM_QSTR(MP_QSTR_BUTTON_LEFT), MP_ROM_INT(BTN_LEFT)},
     {MP_ROM_QSTR(MP_QSTR_BUTTON_RIGHT), MP_ROM_INT(BTN_RIGHT)},
 #endif