[riot-notifications] [RIOT-OS/RIOT] USBUS: Initial work towards an USB stack (#10916)

Dylan Laduranty notifications at github.com
Mon Apr 1 22:23:46 CEST 2019


dylad commented on this pull request.

Found small nitpicks again.
Tested work on arduino-zero !
Also, there is no available command from the console with `examples/usbus_minimal` (except `help`).  Does the shell is really needed ?

> +#define USB_SETUP_REQ_SET_INTERFACE         0x0b    /**< Set interface         */
+/** @} */
+
+/**
+ * @name USB descriptor types (USB 2.0 spec table 9-5)
+ * @{
+ */
+#define USB_TYPE_DESCRIPTOR_DEVICE          0x01    /**< Device descriptor         */
+#define USB_TYPE_DESCRIPTOR_CONFIGURATION   0x02    /**< Configuration Descriptor  */
+#define USB_TYPE_DESCRIPTOR_STRING          0x03    /**< String descriptor         */
+#define USB_TYPE_DESCRIPTOR_INTERFACE       0x04    /**< Interface descriptor      */
+#define USB_TYPE_DESCRIPTOR_ENDPOINT        0x05    /**< Endpoint descriptor       */
+#define USB_TYPE_DESCRIPTOR_DEV_QUALIFIER   0x06    /**< Device qualifier          */
+#define USB_TYPE_DESCRIPTOR_SPEED_CONFIG    0x07    /**< Other speed configuration */
+#define USB_TYPE_DESCRIPTOR_IFACE_POWER     0x08    /**< Interface power           */
+#define USB_TYPE_DESCRIPTOR_INTERFACE_ASSOC 0x0B    /**< Interface association     */

```suggestion
#define USB_TYPE_DESCRIPTOR_INTERFACE_ASSOC 0x0b    /**< Interface association     */
```
:)

> +    uint8_t max_packet_size;    /**< EP0 max packet size (8, 16, 32 or 64 bytes)        */
+    uint16_t vendor_id;         /**< Vendor ID (as assigned by the USB-IF)              */
+    uint16_t product_id;        /**< Product ID                                         */
+    uint16_t bcd_device;        /**< Binary-coded decimal device release                */
+    uint8_t manufacturer_idx;   /**< Manufacturer string index number                   */
+    uint8_t product_idx;        /**< Product string index number                        */
+    uint8_t serial_idx;         /**< Device serial number string index number           */
+    uint8_t num_configurations; /**< Number of possible configurations                  */
+} usb_descriptor_device_t;
+
+/**
+ * @brief USB configuration descriptor (USB 2.0 spec table 9-10)
+ */
+typedef struct __attribute__((packed)) {
+    uint8_t length;             /**< Size of this descriptor                                 */
+    uint8_t type;               /**< Descriptor type (@ref USB_TYPE_DESCRIPTOR_CONFIGURATION */

```suggestion
    uint8_t type;               /**< Descriptor type (@ref USB_TYPE_DESCRIPTOR_CONFIGURATION) */
```

> +typedef struct usbus usbus_t;
+
+/**
+ * @brief USBUS event handler forward declaration
+ */
+typedef struct usbus_handler usbus_handler_t;
+
+/**
+ * @brief USBUS generic header generator
+ *
+ * @param usbus The usbus context
+ * @param arg   Additional argument for the header generator
+ *
+ * @return      Length of the generated header
+ */
+typedef size_t (*gen_hdr)(usbus_t *usbus, void *arg);

typedef is required here ??

> +                             usbdev_ep_t *ep, usbus_event_transfer_t event);
+
+    /**
+     * @brief setup request handler function
+     *
+     * This function receives USB setup requests from the USBUS stack.
+     *
+     * @param usbus     USBUS context
+     * @param handler   handler context
+     * @param state     setup request state
+     * @param setup     setup packet
+     *
+     * @returns         Size of the returned data when the request is handled
+     * @returns         negative to have the stack return an USB stall to the
+     *                  host
+     * @returns         zero when the request is not handled by this handler

```suggestion
     * @return         zero when the request is not handled by this handler
```

> +                             usbdev_ep_t *ep, usbus_event_transfer_t event);
+
+    /**
+     * @brief setup request handler function
+     *
+     * This function receives USB setup requests from the USBUS stack.
+     *
+     * @param usbus     USBUS context
+     * @param handler   handler context
+     * @param state     setup request state
+     * @param setup     setup packet
+     *
+     * @returns         Size of the returned data when the request is handled
+     * @returns         negative to have the stack return an USB stall to the
+     *                  host
+     * @returns         zero when the request is not handled by this handler

same at other place.

> +}
+
+/* USB endpoint 0 callback */
+static void _handler_ep0_event(usbus_t *usbus, usbus_handler_t *handler,
+                              usbus_event_usb_t event)
+{
+    usbus_control_handler_t *ep0_handler = (usbus_control_handler_t *)handler;
+
+    (void)usbus;
+    switch (event) {
+        case USBUS_EVENT_USB_RESET:
+            DEBUG("usbus_control: Reset event triggered\n");
+            ep0_handler->setup_state = USBUS_SETUPRQ_READY;
+            _usbus_config_ep0(ep0_handler);
+            break;
+        default:

missing assert/debug.

> +            break;
+        default:
+            break;
+    }
+}
+
+static void _handler_ep0_transfer(usbus_t *usbus, usbus_handler_t *handler,
+                                 usbdev_ep_t *ep, usbus_event_transfer_t event)
+{
+    usbus_control_handler_t *ep0_handler = (usbus_control_handler_t *)handler;
+
+    switch (event) {
+        case USBUS_EVENT_TRANSFER_COMPLETE:
+            _handle_tr_complete(usbus, ep0_handler, ep);
+            break;
+        default:

missing default assert/debug.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/10916#pullrequestreview-221299684
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190401/c6935ec7/attachment.html>


More information about the notifications mailing list