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

Kees Bakker notifications at github.com
Fri Feb 1 23:38:17 CET 2019


keestux commented on this pull request.

Here are a few more comments. I hope to spend more time in the next days.

> +            USB_DEVICE_EPINTENSET_RXSTP |
+            USB_DEVICE_EPINTENSET_STALL0);
+}
+
+static bool _ep_in_flags_set(UsbDeviceEndpoint *ep_reg)
+{
+    return ep_reg->EPINTFLAG.reg &
+           ep_reg->EPINTENSET.reg &
+           (USB_DEVICE_EPINTENSET_TRFAIL1 |
+            USB_DEVICE_EPINTENSET_TRCPT1 |
+            USB_DEVICE_EPINTENSET_STALL1);
+}
+
+static bool usb_enable_syncing(void)
+{
+    if (USB->DEVICE.SYNCBUSY.reg & USB_SYNCBUSY_ENABLE) {

Personally I like the .bit notation easier to read.

    if (USB->DEVICE.SYNCBUSY.bit.ENABLE) {

> +           (USB_DEVICE_EPINTENSET_TRFAIL1 |
+            USB_DEVICE_EPINTENSET_TRCPT1 |
+            USB_DEVICE_EPINTENSET_STALL1);
+}
+
+static bool usb_enable_syncing(void)
+{
+    if (USB->DEVICE.SYNCBUSY.reg & USB_SYNCBUSY_ENABLE) {
+        return true;
+    }
+    return false;
+}
+
+static bool usb_swrst_syncing(void)
+{
+    if (USB->DEVICE.SYNCBUSY.reg & USB_SYNCBUSY_SWRST) {

    if (USB->DEVICE.SYNCBUSY.bit.SWRST) {

> +        res->cb = NULL;
+        res->driver = &driver_ep;
+    }
+    return res;
+}
+
+static inline void poweron(void)
+{
+#if defined(CPU_FAM_SAMD21)
+    PM->AHBMASK.reg |= PM_AHBMASK_USB;
+    PM->APBBMASK.reg |= PM_APBBMASK_USB;
+    GCLK->CLKCTRL.reg = (uint32_t)(GCLK_CLKCTRL_CLKEN |
+                                   GCLK_CLKCTRL_GEN_GCLK0 |
+                                   (GCLK_CLKCTRL_ID(USB_GCLK_ID)));
+#elif defined(CPU_FAM_SAML21)
+    MCLK->AHBMASK.reg |= (MCLK_AHBMASK_USB);

Why the parenthesis?

> +    PM->AHBMASK.reg |= PM_AHBMASK_USB;
+    PM->APBBMASK.reg |= PM_APBBMASK_USB;
+    GCLK->CLKCTRL.reg = (uint32_t)(GCLK_CLKCTRL_CLKEN |
+                                   GCLK_CLKCTRL_GEN_GCLK0 |
+                                   (GCLK_CLKCTRL_ID(USB_GCLK_ID)));
+#elif defined(CPU_FAM_SAML21)
+    MCLK->AHBMASK.reg |= (MCLK_AHBMASK_USB);
+    GCLK->PCHCTRL[USB_GCLK_ID].reg = GCLK_PCHCTRL_CHEN |
+                                     GCLK_PCHCTRL_GEN_GCLK0;
+#endif
+}
+
+static inline void poweroff(void)
+{
+#if defined(CPU_FAM_SAMD21)
+    PM->AHBMASK.reg |= PM_AHBMASK_USB;

This should be clearing the bit. And also the APBBMASK is missing here. (Besides, the poweroff function is not used)

> +        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;
+    _enable_irq();
+    /* Interrupt configuration */
+    NVIC_EnableIRQ(USB_IRQn);
+}
+
+void usb_attach(void)
+{
+    /* Datasheet is not clear whether device starts detached */

The comment does not belong here. This function is simply doing what it's been told to do.
BTW. I don't have a direct answer, but ASF usb init and Arduino bootloader do an attach. So I guess it is detached when the device starts.

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


More information about the notifications mailing list