refactor(core): improve USB events handling, more extensible implementation

[no changelog]

core/embed/io/usb/inc/io/usb.h

@@ -28,6 +28,16 @@
 
 #define USB_PACKET_LEN 64
 
+typedef enum {
+  USB_EVENT_NONE = 0,
+  USB_EVENT_CONFIGURED = 1,
+  USB_EVENT_DECONFIGURED = 2,
+} usb_event_t;
+
+typedef union {
+  bool configured;
+} usb_state_t;
+
 // clang-format off
 //
 // USB stack high-level state machine
@@ -94,7 +104,7 @@ void usb_deinit(void);
 // Initializes the USB stack (and hardware) and starts all registered class
 // drivers.
 //
-// This function can called after all class drivers are registered or after
+// This function can be called after all class drivers are registered or after
 // `usb_stop()` is called.
 //
 // Returns `sectrue` if the USB stack is started successfully.
@@ -102,17 +112,18 @@ secbool usb_start(void);
 
 // Stops USB driver and its class drivers
 //
-// Unitializes the USB stack (and hardware) but leaves all configuration intact,
-// so it can be started again with `usb_start()`.
+// Uninitializes the USB stack (and hardware) but leaves all configuration
+// intact, so it can be started again with `usb_start()`.
 //
 // When the USB stack is stopped, it does not respond to any USB events and
 // the CPU can go to stop/standby mode.
 void usb_stop(void);
 
-// Returns `sectrue` if the device is connected to the host (or is expected to
-// be)
-//
-// TODO: Review and clarify the logic of this function in the future
-secbool usb_configured(void);
+// Reads USB event
+// Return USB_EVENT_NONE if no event is available
+usb_event_t usb_get_event(void);
+
+// Reads USB state into `state`
+void usb_get_state(usb_state_t *state);
 
 #endif

core/embed/io/usb/stm32/usb.c

@@ -84,6 +84,9 @@ typedef struct {
   // Set to `sectrue` if the USB stack was ready sinced the last start
   secbool was_ready;
 
+  // Current state of USB configuration
+  secbool configured;
+
 } usb_driver_t;
 
 // USB driver instance
@@ -118,8 +121,8 @@ secbool usb_init(const usb_dev_info_t *dev_info) {
   // Device descriptor
   drv->dev_desc.bLength = sizeof(usb_device_descriptor_t);
   drv->dev_desc.bDescriptorType = USB_DESC_TYPE_DEVICE;
-  drv->dev_desc.bcdUSB =
-      (sectrue == drv->usb21_enabled) ? 0x0210 : 0x0200;  // USB 2.1 or USB 2.0
+  // USB 2.1 or USB 2.0
+  drv->dev_desc.bcdUSB = (sectrue == drv->usb21_enabled) ? 0x0210 : 0x0200;
   drv->dev_desc.bDeviceClass = dev_info->device_class;
   drv->dev_desc.bDeviceSubClass = dev_info->device_subclass;
   drv->dev_desc.bDeviceProtocol = dev_info->device_protocol;
@@ -127,11 +130,12 @@ secbool usb_init(const usb_dev_info_t *dev_info) {
   drv->dev_desc.idVendor = dev_info->vendor_id;
   drv->dev_desc.idProduct = dev_info->product_id;
   drv->dev_desc.bcdDevice = dev_info->release_num;
-  drv->dev_desc.iManufacturer =
-      USBD_IDX_MFC_STR;  // Index of manufacturer string
-  drv->dev_desc.iProduct = USBD_IDX_PRODUCT_STR;  // Index of product string
-  drv->dev_desc.iSerialNumber =
-      USBD_IDX_SERIAL_STR;  // Index of serial number string
+  // Index of manufacturer string
+  drv->dev_desc.iManufacturer = USBD_IDX_MFC_STR;
+  // Index of product string
+  drv->dev_desc.iProduct = USBD_IDX_PRODUCT_STR;
+  // Index of serial number string
+  drv->dev_desc.iSerialNumber = USBD_IDX_SERIAL_STR;
   drv->dev_desc.bNumConfigurations = 1;
 
   // String table
@@ -158,17 +162,19 @@ secbool usb_init(const usb_dev_info_t *dev_info) {
   // Configuration descriptor
   drv->config_desc->bLength = sizeof(usb_config_descriptor_t);
   drv->config_desc->bDescriptorType = USB_DESC_TYPE_CONFIGURATION;
-  drv->config_desc->wTotalLength =
-      sizeof(usb_config_descriptor_t);  // will be updated later via
-                                        // usb_alloc_class_descriptors()
-  drv->config_desc->bNumInterfaces =
-      0;  // will be updated later via usb_set_iface_class()
+  // will be updated later via usb_alloc_class_descriptors()
+  drv->config_desc->wTotalLength = sizeof(usb_config_descriptor_t);
+  // will be updated later via usb_set_iface_class()
+  drv->config_desc->bNumInterfaces = 0;
   drv->config_desc->bConfigurationValue = 0x01;
   drv->config_desc->iConfiguration = 0;
-  drv->config_desc->bmAttributes =
-      0x80;  // 0x80 = bus powered; 0xC0 = self powered
-  drv->config_desc->bMaxPower = 0x32;  // Maximum Power Consumption in 2mA units
+  // 0x80 = bus powered; 0xC0 = self powered
+  drv->config_desc->bmAttributes = 0x80;
+  // Maximum Power Consumption in 2mA units
+  drv->config_desc->bMaxPower = 0x32;
 
+  // starting with this flag set, to avoid false warnings
+  drv->configured = sectrue;
   drv->initialized = sectrue;
 
   return sectrue;
@@ -241,7 +247,7 @@ void usb_stop(void) {
   memset(&drv->dev_handle, 0, sizeof(drv->dev_handle));
 }
 
-secbool usb_configured(void) {
+static secbool usb_configured(void) {
   usb_driver_t *drv = &g_usb_driver;
 
   if (drv->initialized != sectrue) {
@@ -295,6 +301,39 @@ secbool usb_configured(void) {
   return ready;
 }
 
+usb_event_t usb_get_event(void) {
+  usb_driver_t *drv = &g_usb_driver;
+
+  if (drv->initialized != sectrue) {
+    // The driver is not initialized
+    return false;
+  }
+
+  secbool configured = usb_configured();
+  if (configured != drv->configured) {
+    drv->configured = configured;
+    if (configured == sectrue) {
+      return USB_EVENT_CONFIGURED;
+    } else {
+      return USB_EVENT_DECONFIGURED;
+    }
+  }
+
+  return USB_EVENT_NONE;
+}
+
+void usb_get_state(usb_state_t *state) {
+  usb_driver_t *drv = &g_usb_driver;
+
+  usb_state_t s = {0};
+
+  if (drv->initialized == sectrue) {
+    s.configured = drv->configured == sectrue;
+  }
+
+  *state = s;
+}
+
 // ==========================================================================
 // Utility functions for USB class drivers
 // ==========================================================================

core/embed/io/usb/unix/usb.c

@@ -322,3 +322,9 @@ secbool usb_configured(void) {
 
   return sectrue;
 }
+
+usb_event_t usb_get_event(void) { return USB_EVENT_NONE; }
+
+void usb_get_state(usb_state_t *state) {
+  state->configured = usb_configured() == sectrue;
+}

core/embed/rust/build.rs

@@ -406,7 +406,8 @@ fn generate_trezorhal_bindings() {
         // toif
         .allowlist_type("toif_format_t")
         //usb
-        .allowlist_function("usb_configured")
+        .allowlist_type("usb_event_t")
+        .allowlist_function("usb_get_state")
         // touch
         .allowlist_function("touch_get_event")
         // button

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

@@ -1,5 +1,9 @@
 use super::ffi;
 
 pub fn usb_configured() -> bool {
-    unsafe { ffi::usb_configured() == ffi::sectrue }
+    unsafe {
+        let mut state: ffi::usb_state_t = core::mem::zeroed();
+        ffi::usb_get_state(&mut state as *mut ffi::usb_state_t);
+        state.configured
+    }
 }

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

@@ -3,14 +3,11 @@ pub mod button;
 #[cfg(feature = "touch")]
 pub mod touch;
 
+pub mod usb;
+
 #[cfg(feature = "button")]
 pub use button::{ButtonEvent, PhysicalButton};
 #[cfg(feature = "touch")]
 pub use touch::{SwipeEvent, TouchEvent};
 
-#[derive(Copy, Clone, PartialEq, Eq)]
-#[cfg_attr(feature = "debug", derive(ufmt::derive::uDebug))]
-pub enum USBEvent {
-    /// USB host has connected/disconnected.
-    Connected(bool),
-}
+pub use usb::USBEvent;

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

@@ -0,0 +1,20 @@
+use crate::error::Error;
+
+#[derive(Copy, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "debug", derive(ufmt::derive::uDebug))]
+pub enum USBEvent {
+    /// USB host has connected/disconnected.
+    Configured,
+    Deconfigured,
+}
+
+impl USBEvent {
+    pub fn new(event: u32) -> Result<Self, Error> {
+        let result = match event {
+            1 => Self::Configured,
+            2 => Self::Deconfigured,
+            _ => return Err(Error::OutOfRange),
+        };
+        Ok(result)
+    }
+}

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

@@ -545,7 +545,7 @@ extern "C" fn ui_layout_usb_event(n_args: usize, args: *const Obj) -> Obj {
             return Err(Error::TypeError);
         }
         let this: Gc<LayoutObj> = args[0].try_into()?;
-        let event = USBEvent::Connected(args[1].try_into()?);
+        let event = USBEvent::new(args[1].try_into()?)?;
         let msg = this.inner_mut().obj_event(Event::USB(event))?;
         Ok(msg)
     };

core/embed/rust/src/ui/model_mercury/component/homescreen.rs

@@ -7,7 +7,7 @@ use crate::{
     ui::{
         component::{Component, Event, EventCtx, Timer},
         display::{image::ImageInfo, Color, Font},
-        event::{TouchEvent, USBEvent},
+        event::TouchEvent,
         geometry::{Alignment, Alignment2D, Offset, Point, Rect},
         layout::util::get_user_custom_image,
         model_mercury::constant,
@@ -500,7 +500,7 @@ impl Homescreen {
     }
 
     fn event_usb(&mut self, ctx: &mut EventCtx, event: Event) {
-        if let Event::USB(USBEvent::Connected(_)) = event {
+        if let Event::USB(_) = event {
             ctx.request_paint();
         }
     }

core/embed/rust/src/ui/model_tr/component/homescreen.rs

@@ -10,7 +10,6 @@ use crate::{
             image::{ImageInfo, ToifFormat},
             Font, Icon,
         },
-        event::USBEvent,
         geometry::{Alignment, Alignment2D, Insets, Offset, Point, Rect},
         layout::util::get_user_custom_image,
         shape,
@@ -164,7 +163,7 @@ impl Homescreen {
     }
 
     fn event_usb(&mut self, ctx: &mut EventCtx, event: Event) {
-        if let Event::USB(USBEvent::Connected(_)) = event {
+        if let Event::USB(_) = event {
             ctx.request_paint();
         }
     }

core/embed/rust/src/ui/model_tt/component/homescreen.rs

@@ -11,7 +11,7 @@ use crate::{
             toif::Icon,
             Color, Font,
         },
-        event::{TouchEvent, USBEvent},
+        event::TouchEvent,
         geometry::{Alignment, Alignment2D, Insets, Offset, Point, Rect},
         layout::util::get_user_custom_image,
         model_tt::{constant, theme::IMAGE_HOMESCREEN},
@@ -130,7 +130,7 @@ impl Homescreen {
     }
 
     fn event_usb(&mut self, ctx: &mut EventCtx, event: Event) {
-        if let Event::USB(USBEvent::Connected(_)) = event {
+        if let Event::USB(_) = event {
             self.paint_notification_only = true;
             ctx.request_paint();
         }

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

@@ -212,8 +212,13 @@ __attribute((no_stack_protector)) void syscall_handler(uint32_t *args,
       usb_stop();
     } break;
 
-    case SYSCALL_USB_CONFIGURED: {
-      args[0] = usb_configured();
+    case SYSCALL_USB_GET_EVENT: {
+      args[0] = usb_get_event();
+    } break;
+
+    case SYSCALL_USB_GET_STATE: {
+      usb_state_t *state = (usb_state_t *)args[0];
+      usb_get_state__verified(state);
     } break;
 
     case SYSCALL_USB_HID_ADD: {

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

@@ -51,7 +51,8 @@ typedef enum {
   SYSCALL_USB_DEINIT,
   SYSCALL_USB_START,
   SYSCALL_USB_STOP,
-  SYSCALL_USB_CONFIGURED,
+  SYSCALL_USB_GET_EVENT,
+  SYSCALL_USB_GET_STATE,
 
   SYSCALL_USB_HID_ADD,
   SYSCALL_USB_HID_CAN_READ,

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

@@ -162,8 +162,12 @@ secbool usb_start(void) { return (secbool)syscall_invoke0(SYSCALL_USB_START); }
 
 void usb_stop(void) { syscall_invoke0(SYSCALL_USB_STOP); }
 
-secbool usb_configured(void) {
-  return (secbool)syscall_invoke0(SYSCALL_USB_CONFIGURED);
+usb_event_t usb_get_event(void) {
+  return (usb_event_t)syscall_invoke0(SYSCALL_USB_GET_EVENT);
+}
+
+void usb_get_state(usb_state_t *state) {
+  syscall_invoke1((uint32_t)state, SYSCALL_USB_GET_STATE);
 }
 
 // =============================================================================

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

@@ -193,6 +193,18 @@ access_violation:
 
 // ---------------------------------------------------------------------
 
+void usb_get_state__verified(usb_state_t *state) {
+  if (!probe_write_access(state, sizeof(*state))) {
+    goto access_violation;
+  }
+
+  usb_get_state(state);
+  return;
+
+access_violation:
+  apptask_access_violation();
+}
+
 int usb_hid_read__verified(uint8_t iface_num, uint8_t *buf, uint32_t len) {
   if (!probe_write_access(buf, len)) {
     goto access_violation;

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

@@ -50,6 +50,10 @@ void display_fill__verified(const gfx_bitblt_t *bb);
 
 void display_copy_rgb565__verified(const gfx_bitblt_t *bb);
 
+// ---------------------------------------------------------------------
+#include <io/usb.h>
+void usb_get_state__verified(usb_state_t *state);
+
 // ---------------------------------------------------------------------
 #include <io/usb_hid.h>
 

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

@@ -33,13 +33,12 @@
 #include "SDL.h"
 #endif
 
-#define USB_DATA_IFACE (253)
+#define USB_EVENT_IFACE (253)
 #define BUTTON_IFACE (254)
 #define TOUCH_IFACE (255)
 #define POLL_READ (0x0000)
 #define POLL_WRITE (0x0100)
 
-extern bool usb_connected_previously;
 extern uint32_t last_touch_sample_time;
 
 /// package: trezorio.__init__
@@ -138,12 +137,12 @@ STATIC mp_obj_t mod_trezorio_poll(mp_obj_t ifaces, mp_obj_t list_ref,
         }
       }
 #endif
-      else if (iface == USB_DATA_IFACE) {
-        bool usb_connected = usb_configured() == sectrue ? true : false;
-        if (usb_connected != usb_connected_previously) {
-          usb_connected_previously = usb_connected;
+      else if (iface == USB_EVENT_IFACE) {
+        usb_event_t event = usb_get_event();
+
+        if (event != USB_EVENT_NONE) {
           ret->items[0] = MP_OBJ_NEW_SMALL_INT(i);
-          ret->items[1] = usb_connected ? mp_const_true : mp_const_false;
+          ret->items[1] = MP_OBJ_NEW_SMALL_INT((int32_t)event);
           return mp_const_true;
         }
       }

core/embed/upymod/modtrezorio/modtrezorio.c

@@ -37,9 +37,6 @@
 
 #include <io/usb.h>
 
-// Whether USB data pins were connected on last check (USB configured)
-bool usb_connected_previously = true;
-
 uint32_t last_touch_sample_time = 0;
 
 #define CHECK_PARAM_RANGE(value, minimum, maximum)  \
@@ -79,7 +76,7 @@ uint32_t last_touch_sample_time = 0;
 /// BUTTON_LEFT: int  # button number of left button
 /// BUTTON_RIGHT: int  # button number of right button
 
-/// USB_CHECK: int # interface id for check of USB data connection
+/// USB_EVENT: int # interface id for USB events
 
 /// WireInterface = Union[HID, WebUSB]
 
@@ -120,7 +117,7 @@ STATIC const mp_rom_map_elem_t mp_module_trezorio_globals_table[] = {
     {MP_ROM_QSTR(MP_QSTR_POLL_READ), MP_ROM_INT(POLL_READ)},
     {MP_ROM_QSTR(MP_QSTR_POLL_WRITE), MP_ROM_INT(POLL_WRITE)},
 
-    {MP_ROM_QSTR(MP_QSTR_USB_CHECK), MP_ROM_INT(USB_DATA_IFACE)},
+    {MP_ROM_QSTR(MP_QSTR_USB_EVENT), MP_ROM_INT(USB_EVENT_IFACE)},
 };
 
 STATIC MP_DEFINE_CONST_DICT(mp_module_trezorio_globals,

core/mocks/generated/trezorio/__init__.pyi

@@ -171,5 +171,5 @@ BUTTON_PRESSED: int  # button down event
 BUTTON_RELEASED: int  # button up event
 BUTTON_LEFT: int  # button number of left button
 BUTTON_RIGHT: int  # button number of right button
-USB_CHECK: int # interface id for check of USB data connection
+USB_EVENT: int # interface id for USB events
 WireInterface = Union[HID, WebUSB]

core/src/trezor/ui/layouts/homescreen.py

@@ -68,10 +68,10 @@ class Homescreen(HomescreenBase):
     async def usb_checker_task(self) -> None:
         from trezor import io, loop
 
-        usbcheck = loop.wait(io.USB_CHECK)
+        usbcheck = loop.wait(io.USB_EVENT)
         while True:
-            is_connected = await usbcheck
-            self._event(self.layout.usb_event, is_connected)
+            event = await usbcheck
+            self._event(self.layout.usb_event, event)
 
     def create_tasks(self) -> Iterator[loop.Task]:
         yield from super().create_tasks()