[riot-notifications] [RIOT-OS/RIOT] [WIP] sam0_common: Add USB peripheral driver (#10915)

Dylan Laduranty notifications at github.com
Fri Feb 1 21:32:56 CET 2019


dylad commented on this pull request.

some comments for the first round. I'll test on arduino-zero like board soon.

> @@ -0,0 +1,64 @@
+/*
+ * Copyright (C) 2018 Freie Universit├Ąt Berlin
+ * Copyright (C) 2018 Inria

You should probably update these copyrights.

> + * General Public License v2.1. See the file LICENSE in the top level
+ * directory for more details.
+ */
+
+/**
+ * @defgroup    sys_uuid RFC 4122 compliant UUID's
+ * @ingroup     sys
+ * @brief       Provides RFC 4122 compliant UUID's
+ *
+ * This module provides RFC 4122 compliant UUID generation. The UUID stored in
+ * @ref uuid_t struct is stored in network byte order.
+ *
+ * @{
+ *
+ * @file
+ * @brief       [RFC 4122](https://tools.ietf.org/html/rfc4122) UUID functions

Please also update Doxygen

> +#endif
+
+/**
+ * USB endpoint buffer space
+ *
+ * @todo global configurable?
+ */
+#define SAM_USB_BUF_SPACE   USBDEV_EP_BUF_SPACE
+
+/**
+ * Number of USB IN and OUT endpoints
+ */
+#define SAM_USB_NUM_EP      USBDEV_NUM_ENDPOINTS
+
+typedef struct {
+    usbdev_t usbdev;

missing description

> +static int usbdev_ep_ready(usbdev_ep_t *ep, size_t len);
+
+
+static usbdev_ep_t *usbdev_new_ep(usbdev_t *dev, usb_ep_type_t type,
+                                  usb_ep_dir_t dir, size_t buf_len);
+
+static void _ep_address(usbdev_ep_t *ep);
+static void _ep_size(usbdev_ep_t *ep);
+static int _ep_unready(usbdev_ep_t *ep);
+
+static inline unsigned _get_ep_num(unsigned num, usb_ep_dir_t dir)
+{
+    return 2 * num + (dir == USB_EP_DIR_OUT ? 0 : 1);
+}
+
+static inline unsigned _get_ep_num2(usbdev_ep_t *ep)

Why is there two  get_ep_num functions ?

> +            val = 0x3;
+            break;
+        case 128:
+            val = 0x4;
+            break;
+        case 256:
+            val = 0x5;
+            break;
+        case 512:
+            val = 0x6;
+            break;
+        case 1023:
+            val = 0x7;
+            break;
+        default:
+            return;

What about a single line to directly compute this value with a nice comment ? :)

> +    UsbDeviceDescBank *bank = &banks[_get_ep_num(ep->num, ep->dir)];
+
+    return (size_t)bank->PCKSIZE.bit.BYTE_COUNT;
+
+}
+
+int usbdev_ep_get(usbdev_ep_t *ep, usbopt_ep_t opt,
+                  void *value, size_t max_len)
+{
+    (void)ep;
+    (void)value;
+    (void)max_len;
+    int res = -ENOTSUP;
+    assert(ep != NULL);
+    switch (opt) {
+        case USBOPT_EP_ENABLE:

add fall-through comment

-- 
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/10915#pullrequestreview-199247877
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190201/2751e40f/attachment.html>


More information about the notifications mailing list