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

Dylan Laduranty notifications at github.com
Sat Feb 2 22:44:07 CET 2019


dylad commented on this pull request.

Another small round !

> +
+void usbdev_init(usbdev_t *dev)
+{
+    /* Only one usb device on this board */
+    _usbdev = (sam0_common_usb_t *)dev;
+    _usbdev->used = 0;
+    /* Set GPIO */
+    gpio_init(GPIO_PIN(PA, 24), GPIO_IN);
+    gpio_init(GPIO_PIN(PA, 25), GPIO_IN);
+    gpio_init_mux(GPIO_PIN(PA, 24), GPIO_MUX_G);
+    gpio_init_mux(GPIO_PIN(PA, 25), GPIO_MUX_G);
+    poweron();
+    /* Reset peripheral */
+    USB->DEVICE.CTRLA.reg |= USB_CTRLA_SWRST;
+    while (usb_swrst_syncing()) {}
+    while (USB->DEVICE.CTRLA.bit.SWRST) {}

You can remove this line. The datasheet states that both bit will be clearer once reset is done, so the line above should be enough.

> +    while (USB->DEVICE.CTRLA.bit.SWRST) {}
+
+    /* Enable USB device */
+    USB->DEVICE.DESCADD.reg = (uint32_t)banks;
+    USB->DEVICE.CTRLA.reg |= USB_CTRLA_ENABLE | USB_CTRLA_RUNSTDBY;
+    while (usb_enable_syncing()) {}
+    /* Callibration values */
+    USB->DEVICE.PADCAL.reg =
+        USB_PADCAL_TRANSP((*(uint32_t *)USB_FUSES_TRANSP_ADDR >>
+                           USB_FUSES_TRANSP_Pos)) |
+        USB_PADCAL_TRANSN((*(uint32_t *)USB_FUSES_TRANSN_ADDR >>
+                           USB_FUSES_TRANSN_Pos)) |
+        USB_PADCAL_TRIM((*(uint32_t *)USB_FUSES_TRIM_ADDR >>
+                         USB_FUSES_TRIM_Pos));
+
+    USB->DEVICE.CTRLB.bit.SPDCONF = 0x0;

Replaces 0 by USB_DEVICE_CTRLB_SPDCONF_FS

> +    if (dev == NULL) {
+        return -ENODEV;
+    }
+
+    switch (opt) {
+        case USBOPT_MAX_VERSION:
+            assert(max_len == sizeof(usb_speed_t));
+            *(usb_version_t *)value = USB_VERSION_20;
+            res = sizeof(usb_speed_t);
+            break;
+        case USBOPT_MAX_SPEED:
+            assert(max_len == sizeof(usb_speed_t));
+            *(usb_speed_t *)value = USB_SPEED_FULL;
+            res = sizeof(usb_speed_t);
+            break;
+        default:

Maybe add a DEBUG info here ?

> +            }
+            else {
+                USB->DEVICE.DADD.bit.ADDEN = 0;
+            }
+            break;
+        case USBOPT_ATTACH:
+            assert(value_len == sizeof(usbopt_enable_t));
+            if (*((usbopt_enable_t *)value)) {
+                usb_attach();
+            }
+            else {
+                usb_detach();
+            }
+            res = sizeof(usbopt_enable_t);
+            break;
+        default:

Same here.

> +{
+    UsbDeviceDescBank *bank = _bank_from_ep(ep);
+
+    bank->ADDR.reg = (uint32_t)ep->buf;
+}
+
+void _ep_set_stall(usbdev_ep_t *ep, usbopt_enable_t enable)
+{
+    UsbDeviceEndpoint *ep_reg = &USB->DEVICE.DeviceEndpoint[ep->num];
+
+    if (ep->dir == USB_EP_DIR_IN) {
+        if (enable) {
+            ep_reg->EPSTATUSSET.reg = USB_DEVICE_EPSTATUSSET_STALLRQ1;
+        }
+        else {
+            ep_reg->EPSTATUSCLR.reg = USB_DEVICE_EPSTATUSSET_STALLRQ1;

```suggestion
            ep_reg->EPSTATUSCLR.reg = USB_DEVICE_EPSTATUSCLR_STALLRQ1;
```

> +    UsbDeviceEndpoint *ep_reg = &USB->DEVICE.DeviceEndpoint[ep->num];
+
+    if (ep->dir == USB_EP_DIR_IN) {
+        if (enable) {
+            ep_reg->EPSTATUSSET.reg = USB_DEVICE_EPSTATUSSET_STALLRQ1;
+        }
+        else {
+            ep_reg->EPSTATUSCLR.reg = USB_DEVICE_EPSTATUSSET_STALLRQ1;
+        }
+    }
+    else {
+        if (enable) {
+            ep_reg->EPSTATUSSET.reg = USB_DEVICE_EPSTATUSSET_STALLRQ0;
+        }
+        else {
+            ep_reg->EPSTATUSCLR.reg = USB_DEVICE_EPSTATUSSET_STALLRQ0;

```suggestion
            ep_reg->EPSTATUSCLR.reg = USB_DEVICE_EPSTATUSCLR_STALLRQ0;
```

> +        case USBOPT_EP_STALL:
+            assert(value_len == sizeof(usbopt_enable_t));
+            _ep_set_stall(ep, *(usbopt_enable_t *)value);
+            res = sizeof(usbopt_enable_t);
+            break;
+        case USBOPT_EP_READY:
+            assert(value_len == sizeof(usbopt_enable_t));
+            if (*((usbopt_enable_t *)value)) {
+                _ep_unready(ep);
+            }
+            else {
+                usbdev_ep_ready(ep, 0);
+            }
+            res = sizeof(usbopt_enable_t);
+            break;
+        default:

DEBUG line ?

> +                usbdev_ep_ready(ep, 0);
+            }
+            res = sizeof(usbopt_enable_t);
+            break;
+        default:
+            break;
+    }
+    return res;
+}
+
+static int _ep_unready(usbdev_ep_t *ep)
+{
+    UsbDeviceEndpoint *ep_reg = &USB->DEVICE.DeviceEndpoint[ep->num];
+
+    if (ep->dir == USB_EP_DIR_IN) {
+        ep_reg->EPSTATUSCLR.reg = USB_DEVICE_EPSTATUSSET_BK1RDY;

```suggestion
        ep_reg->EPSTATUSCLR.reg = USB_DEVICE_EPSTATUSCLR_BK1RDY;
```

> +            break;
+        default:
+            break;
+    }
+    return res;
+}
+
+static int _ep_unready(usbdev_ep_t *ep)
+{
+    UsbDeviceEndpoint *ep_reg = &USB->DEVICE.DeviceEndpoint[ep->num];
+
+    if (ep->dir == USB_EP_DIR_IN) {
+        ep_reg->EPSTATUSCLR.reg = USB_DEVICE_EPSTATUSSET_BK1RDY;
+    }
+    else {
+        ep_reg->EPSTATUSSET.reg = USB_DEVICE_EPSTATUSCLR_BK0RDY;

```suggestion
        ep_reg->EPSTATUSSET.reg = USB_DEVICE_EPSTATUSSET_BK0RDY;
```

-- 
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-199362752
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190202/a2160c62/attachment.html>


More information about the notifications mailing list