From fea8d53b8586835fca0f9eb7c323852b45be8f27 Mon Sep 17 00:00:00 2001
From: Pavol Rusnak <stick@gk2.sk>
Date: Wed, 17 Jan 2018 16:10:31 +0100
Subject: [PATCH] trezor.io: cleanup usb code

---
 embed/extmod/modtrezorio/modtrezorio-msg.h | 61 +++++++---------------
 embed/trezorhal/usb_hid-defs.h             | 11 ----
 embed/trezorhal/usb_hid-impl.h             |  4 +-
 3 files changed, 21 insertions(+), 55 deletions(-)

diff --git a/embed/extmod/modtrezorio/modtrezorio-msg.h b/embed/extmod/modtrezorio/modtrezorio-msg.h
index 565d27d650..160cdbedd0 100644
--- a/embed/extmod/modtrezorio/modtrezorio-msg.h
+++ b/embed/extmod/modtrezorio/modtrezorio-msg.h
@@ -18,6 +18,11 @@ void mp_hal_set_vcp_iface(int iface_num);
 #define POLL_READ  (0x0000)
 #define POLL_WRITE (0x0100)
 
+#define CHECK_PARAM_RANGE(value, minimum, maximum) \
+    if (value < minimum || value > maximum) { \
+        mp_raise_ValueError(#value " is out of range"); \
+    }
+
 /// def poll(ifaces: Iterable[int], list_ref: List, timeout_us: int) -> bool:
 ///     '''
 ///     Wait until one of `ifaces` is ready to read or write (using masks
@@ -139,27 +144,13 @@ STATIC mp_obj_t mod_trezorio_HID_make_new(const mp_obj_type_t *type, size_t n_ar
     if (report_desc.buf == NULL || report_desc.len == 0 || report_desc.len > 255) {
         mp_raise_ValueError("report_desc is invalid");
     }
-    if (iface_num < 0 || iface_num > 32) {
-        mp_raise_ValueError("iface_num is invalid");
-    }
-    if (ep_in < 0 || ep_in > 255) {
-        mp_raise_ValueError("ep_in is invalid");
-    }
-    if (ep_out < 0 || ep_out > 255) {
-        mp_raise_ValueError("ep_out is invalid");
-    }
-    if (subclass < 0 || subclass > 255) {
-        mp_raise_ValueError("subclass is invalid");
-    }
-    if (protocol < 0 || protocol > 255) {
-        mp_raise_ValueError("protocol is invalid");
-    }
-    if (polling_interval < 1 || polling_interval > 255) {
-        mp_raise_ValueError("polling_interval is invalid");
-    }
-    if (max_packet_len != 64) {
-        mp_raise_ValueError("max_packet_len is invalid");
-    }
+    CHECK_PARAM_RANGE(iface_num, 0, 32)
+    CHECK_PARAM_RANGE(ep_in, 0, 255)
+    CHECK_PARAM_RANGE(ep_out, 0, 255)
+    CHECK_PARAM_RANGE(subclass, 0, 255)
+    CHECK_PARAM_RANGE(protocol, 0, 255)
+    CHECK_PARAM_RANGE(polling_interval, 1, 255)
+    CHECK_PARAM_RANGE(max_packet_len, 64, 64)
 
     mp_obj_HID_t *o = m_new_obj(mp_obj_HID_t);
     o->base.type = type;
@@ -249,21 +240,11 @@ STATIC mp_obj_t mod_trezorio_VCP_make_new(const mp_obj_type_t *type, size_t n_ar
     const mp_int_t ep_out         = vals[3].u_int;
     const mp_int_t ep_cmd         = vals[4].u_int;
 
-    if (iface_num < 0 || iface_num > 32) {
-        mp_raise_ValueError("iface_num is invalid");
-    }
-    if (data_iface_num < 0 || data_iface_num > 32) {
-        mp_raise_ValueError("iface_num is invalid");
-    }
-    if (ep_in < 0 || ep_in > 255) {
-        mp_raise_ValueError("ep_in is invalid");
-    }
-    if (ep_out < 0 || ep_out > 255) {
-        mp_raise_ValueError("ep_out is invalid");
-    }
-    if (ep_cmd < 0 || ep_cmd > 255) {
-        mp_raise_ValueError("ep_cmd is invalid");
-    }
+    CHECK_PARAM_RANGE(iface_num, 0, 32)
+    CHECK_PARAM_RANGE(data_iface_num, 0, 32)
+    CHECK_PARAM_RANGE(ep_in, 0, 255)
+    CHECK_PARAM_RANGE(ep_out, 0, 255)
+    CHECK_PARAM_RANGE(ep_cmd, 0, 255)
 
     const size_t vcp_buffer_len = 1024;
     const size_t vcp_packet_len = 64;
@@ -371,12 +352,8 @@ STATIC mp_obj_t mod_trezorio_USB_make_new(const mp_obj_type_t *type, size_t n_ar
     const char *product       = get_0str(vals[4].u_obj, 0, 32);
     const char *serial_number = get_0str(vals[5].u_obj, 0, 32);
 
-    if (vendor_id < 0 || vendor_id > 65535) {
-        mp_raise_ValueError("vendor_id is invalid");
-    }
-    if (product_id < 0 || product_id > 65535) {
-        mp_raise_ValueError("product_id is invalid");
-    }
+    CHECK_PARAM_RANGE(vendor_id, 0, 65535)
+    CHECK_PARAM_RANGE(product_id, 0, 65535)
     if (manufacturer == NULL) {
         mp_raise_ValueError("manufacturer is invalid");
     }
diff --git a/embed/trezorhal/usb_hid-defs.h b/embed/trezorhal/usb_hid-defs.h
index 5597e03288..fae9fcd75a 100644
--- a/embed/trezorhal/usb_hid-defs.h
+++ b/embed/trezorhal/usb_hid-defs.h
@@ -22,17 +22,6 @@ typedef struct __attribute__((packed)) {
     usb_endpoint_descriptor_t ep_out;
 } usb_hid_descriptor_block_t;
 
-typedef enum {
-    USB_HID_SUBCLASS_NONE = 0,
-    USB_HID_SUBCLASS_BOOT = 1,
-} usb_hid_subclass_t;
-
-typedef enum {
-    USB_HID_PROTOCOL_NONE = 0,
-    USB_HID_PROTOCOL_KEYBOARD = 1,
-    USB_HID_PROTOCOL_MOUSE = 2,
-} usb_hid_protocol_t;
-
 /* usb_hid_info_t contains all information for setting up a HID interface.  All
  * passed pointers need to live at least until the interface is disabled
  * (usb_stop is called). */
diff --git a/embed/trezorhal/usb_hid-impl.h b/embed/trezorhal/usb_hid-impl.h
index a562617609..1de1168996 100644
--- a/embed/trezorhal/usb_hid-impl.h
+++ b/embed/trezorhal/usb_hid-impl.h
@@ -10,9 +10,9 @@
 #define USB_DESC_TYPE_HID        0x21
 #define USB_DESC_TYPE_REPORT     0x22
 
-#define USB_HID_REQ_SET_PROTOCOL 0x0b
+#define USB_HID_REQ_SET_PROTOCOL 0x0B
 #define USB_HID_REQ_GET_PROTOCOL 0x03
-#define USB_HID_REQ_SET_IDLE     0x0a
+#define USB_HID_REQ_SET_IDLE     0x0A
 #define USB_HID_REQ_GET_IDLE     0x02
 
 /* usb_hid_add adds and configures new USB HID interface according to