diff --git a/drivers/usb/common/nrf_usbd_common/nrf_usbd_common.c b/drivers/usb/common/nrf_usbd_common/nrf_usbd_common.c index fcd5cff5ef6..9e502483765 100644 --- a/drivers/usb/common/nrf_usbd_common/nrf_usbd_common.c +++ b/drivers/usb/common/nrf_usbd_common/nrf_usbd_common.c @@ -834,9 +834,10 @@ static void usbd_dmareq_process(void) { if ((m_ep_dma_waiting & m_ep_ready) && (k_sem_take(&dma_available, K_NO_WAIT) == 0)) { + const bool low_power = nrf_usbd_common_suspend_check(); uint32_t req; - while (0 != (req = m_ep_dma_waiting & m_ep_ready)) { + while (!low_power && 0 != (req = m_ep_dma_waiting & m_ep_ready)) { uint8_t pos; if (NRFX_USBD_CONFIG_DMASCHEDULER_ISO_BOOST && @@ -1310,7 +1311,11 @@ bool nrf_usbd_common_is_started(void) bool nrf_usbd_common_suspend(void) { bool suspended = false; - unsigned int irq_lock_key = irq_lock(); + unsigned int irq_lock_key; + + /* DMA doesn't work in Low Power mode, ensure there is no active DMA */ + k_sem_take(&dma_available, K_FOREVER); + irq_lock_key = irq_lock(); if (m_bus_suspend) { if (!(NRF_USBD->EVENTCAUSE & USBD_EVENTCAUSE_RESUME_Msk)) { @@ -1327,6 +1332,7 @@ bool nrf_usbd_common_suspend(void) } irq_unlock(irq_lock_key); + k_sem_give(&dma_available); return suspended; } diff --git a/drivers/usb/udc/udc_dwc2.c b/drivers/usb/udc/udc_dwc2.c index 7e716201d3c..7f1256a10f0 100644 --- a/drivers/usb/udc/udc_dwc2.c +++ b/drivers/usb/udc/udc_dwc2.c @@ -1387,11 +1387,12 @@ static void udc_dwc2_ep_disable(const struct device *dev, uint8_t ep_idx = USB_EP_GET_IDX(cfg->addr); mem_addr_t dxepctl_reg; uint32_t dxepctl; + const bool is_iso = dwc2_ep_is_iso(cfg); dxepctl_reg = dwc2_get_dxepctl_reg(dev, cfg->addr); dxepctl = sys_read32(dxepctl_reg); - if (dxepctl & USB_DWC2_DEPCTL_NAKSTS) { + if (!is_iso && (dxepctl & USB_DWC2_DEPCTL_NAKSTS)) { /* Endpoint already sends forced NAKs. STALL if necessary. */ if (stall) { dxepctl |= USB_DWC2_DEPCTL_STALL; @@ -1401,6 +1402,19 @@ static void udc_dwc2_ep_disable(const struct device *dev, return; } + /* FIXME: This function needs to be changed to not synchronously wait + * for the events to happen because the actions here are racing against + * the USB host packets. It is possible that the IN token or OUT DATA + * gets sent shortly before this function disables the endpoint. If this + * happens, the XferCompl would be set and driver will incorrectly think + * that either: + * * never queued transfer finished, OR + * * transfer queued in incompisoin handler finished (before it really + * does and then it'll "double"-finish when it actually finishes) + * + * For the time being XferCompl is cleared as a workaround. + */ + if (USB_EP_DIR_IS_OUT(cfg->addr)) { mem_addr_t dctl_reg, gintsts_reg, doepint_reg; uint32_t dctl; @@ -1439,7 +1453,7 @@ static void udc_dwc2_ep_disable(const struct device *dev, } /* Clear Endpoint Disabled interrupt */ - sys_write32(USB_DWC2_DIEPINT_EPDISBLD, doepint_reg); + sys_write32(USB_DWC2_DOEPINT_EPDISBLD | USB_DWC2_DOEPINT_XFERCOMPL, doepint_reg); dctl |= USB_DWC2_DCTL_CGOUTNAK; sys_write32(dctl, dctl_reg); @@ -1463,7 +1477,7 @@ static void udc_dwc2_ep_disable(const struct device *dev, dwc2_wait_for_bit(dev, diepint_reg, USB_DWC2_DIEPINT_EPDISBLD); /* Clear Endpoint Disabled interrupt */ - sys_write32(USB_DWC2_DIEPINT_EPDISBLD, diepint_reg); + sys_write32(USB_DWC2_DIEPINT_EPDISBLD | USB_DWC2_DIEPINT_XFERCOMPL, diepint_reg); /* TODO: Read DIEPTSIZn here? Programming Guide suggest it to * let application know how many bytes of interrupted transfer @@ -2571,7 +2585,11 @@ static void dwc2_handle_incompisoin(const struct device *dev) buf = udc_buf_get(dev, cfg->addr); if (buf) { + /* Data is no longer relevant */ udc_submit_ep_event(dev, buf, 0); + + /* Try to queue next packet before SOF */ + dwc2_handle_xfer_next(dev, cfg); } } } diff --git a/drivers/usb/udc/udc_dwc2_vendor_quirks.h b/drivers/usb/udc/udc_dwc2_vendor_quirks.h index d12d64ef848..f45404c2b84 100644 --- a/drivers/usb/udc/udc_dwc2_vendor_quirks.h +++ b/drivers/usb/udc/udc_dwc2_vendor_quirks.h @@ -201,6 +201,9 @@ static inline int usbhs_enable_core(const struct device *dev) wrapper->ENABLE = USBHS_ENABLE_PHY_Msk | USBHS_ENABLE_CORE_Msk; wrapper->TASKS_START = 1UL; + /* Wait for clock to start to avoid hang on too early register read */ + k_busy_wait(1); + /* Enable interrupts */ wrapper->INTENSET = 1UL; diff --git a/drivers/usb/udc/udc_nrf.c b/drivers/usb/udc/udc_nrf.c index bf04a274ad6..e7229174463 100644 --- a/drivers/usb/udc/udc_nrf.c +++ b/drivers/usb/udc/udc_nrf.c @@ -443,6 +443,22 @@ static void udc_nrf_thread(void *p1, void *p2, void *p3) case UDC_NRF_EVT_HAL: ep = evt.hal_evt.data.eptransfer.ep; switch (evt.hal_evt.type) { + case NRF_USBD_COMMON_EVT_SUSPEND: + LOG_INF("SUSPEND state detected"); + nrf_usbd_common_suspend(); + udc_set_suspended(udc_nrf_dev, true); + udc_submit_event(udc_nrf_dev, UDC_EVT_SUSPEND, 0); + break; + case NRF_USBD_COMMON_EVT_RESUME: + LOG_INF("RESUMING from suspend"); + udc_set_suspended(udc_nrf_dev, false); + udc_submit_event(udc_nrf_dev, UDC_EVT_RESUME, 0); + break; + case NRF_USBD_COMMON_EVT_WUREQ: + LOG_INF("Remote wakeup initiated"); + udc_set_suspended(udc_nrf_dev, false); + udc_submit_event(udc_nrf_dev, UDC_EVT_RESUME, 0); + break; case NRF_USBD_COMMON_EVT_EPTRANSFER: start_xfer = true; if (USB_EP_DIR_IS_IN(ep)) { @@ -499,22 +515,6 @@ static void udc_sof_check_iso_out(const struct device *dev) static void usbd_event_handler(nrf_usbd_common_evt_t const *const hal_evt) { switch (hal_evt->type) { - case NRF_USBD_COMMON_EVT_SUSPEND: - LOG_INF("SUSPEND state detected"); - nrf_usbd_common_suspend(); - udc_set_suspended(udc_nrf_dev, true); - udc_submit_event(udc_nrf_dev, UDC_EVT_SUSPEND, 0); - break; - case NRF_USBD_COMMON_EVT_RESUME: - LOG_INF("RESUMING from suspend"); - udc_set_suspended(udc_nrf_dev, false); - udc_submit_event(udc_nrf_dev, UDC_EVT_RESUME, 0); - break; - case NRF_USBD_COMMON_EVT_WUREQ: - LOG_INF("Remote wakeup initiated"); - udc_set_suspended(udc_nrf_dev, false); - udc_submit_event(udc_nrf_dev, UDC_EVT_RESUME, 0); - break; case NRF_USBD_COMMON_EVT_RESET: LOG_INF("Reset"); udc_submit_event(udc_nrf_dev, UDC_EVT_RESET, 0); @@ -523,6 +523,9 @@ static void usbd_event_handler(nrf_usbd_common_evt_t const *const hal_evt) udc_submit_event(udc_nrf_dev, UDC_EVT_SOF, 0); udc_sof_check_iso_out(udc_nrf_dev); break; + case NRF_USBD_COMMON_EVT_SUSPEND: + case NRF_USBD_COMMON_EVT_RESUME: + case NRF_USBD_COMMON_EVT_WUREQ: case NRF_USBD_COMMON_EVT_EPTRANSFER: case NRF_USBD_COMMON_EVT_SETUP: { struct udc_nrf_evt evt = { diff --git a/subsys/usb/device_next/class/usbd_uac2.c b/subsys/usb/device_next/class/usbd_uac2.c index 0e7144ec622..f3e1a707b33 100644 --- a/subsys/usb/device_next/class/usbd_uac2.c +++ b/subsys/usb/device_next/class/usbd_uac2.c @@ -20,14 +20,15 @@ LOG_MODULE_REGISTER(usbd_uac2, CONFIG_USBD_UAC2_LOG_LEVEL); #define DT_DRV_COMPAT zephyr_uac2 -#define COUNT_UAC2_AS_ENDPOINTS(node) \ +#define COUNT_UAC2_AS_ENDPOINT_BUFFERS(node) \ IF_ENABLED(DT_NODE_HAS_COMPAT(node, zephyr_uac2_audio_streaming), ( \ + AS_HAS_ISOCHRONOUS_DATA_ENDPOINT(node) + \ + + AS_IS_USB_ISO_IN(node) /* ISO IN double buffering */ + \ AS_HAS_EXPLICIT_FEEDBACK_ENDPOINT(node))) -#define COUNT_UAC2_ENDPOINTS(i) \ +#define COUNT_UAC2_EP_BUFFERS(i) \ + DT_PROP(DT_DRV_INST(i), interrupt_endpoint) \ - DT_INST_FOREACH_CHILD(i, COUNT_UAC2_AS_ENDPOINTS) -#define UAC2_NUM_ENDPOINTS DT_INST_FOREACH_STATUS_OKAY(COUNT_UAC2_ENDPOINTS) + DT_INST_FOREACH_CHILD(i, COUNT_UAC2_AS_ENDPOINT_BUFFERS) +#define UAC2_NUM_EP_BUFFERS DT_INST_FOREACH_STATUS_OKAY(COUNT_UAC2_EP_BUFFERS) /* Net buf is used mostly with external data. The main reason behind external * data is avoiding unnecessary isochronous data copy operations. @@ -40,7 +41,7 @@ LOG_MODULE_REGISTER(usbd_uac2, CONFIG_USBD_UAC2_LOG_LEVEL); * "wasted memory" here is likely to be smaller than the memory overhead for * more complex "only as much as needed" schemes (e.g. heap). */ -UDC_BUF_POOL_DEFINE(uac2_pool, UAC2_NUM_ENDPOINTS, 6, +UDC_BUF_POOL_DEFINE(uac2_pool, UAC2_NUM_EP_BUFFERS, 6, sizeof(struct udc_buf_info), NULL); /* 5.2.2 Control Request Layout */ @@ -80,6 +81,7 @@ struct uac2_ctx { */ atomic_t as_active; atomic_t as_queued; + atomic_t as_double; uint32_t fb_queued; }; @@ -247,6 +249,7 @@ int usbd_uac2_send(const struct device *dev, uint8_t terminal, struct uac2_ctx *ctx = dev->data; struct net_buf *buf; const struct usb_ep_descriptor *desc; + atomic_t *queued_bits = &ctx->as_queued; uint8_t ep = 0; int as_idx = terminal_to_as_interface(dev, terminal); int ret; @@ -267,9 +270,12 @@ int usbd_uac2_send(const struct device *dev, uint8_t terminal, return 0; } - if (atomic_test_and_set_bit(&ctx->as_queued, as_idx)) { - LOG_ERR("Previous send not finished yet on 0x%02x", ep); - return -EAGAIN; + if (atomic_test_and_set_bit(queued_bits, as_idx)) { + queued_bits = &ctx->as_double; + if (atomic_test_and_set_bit(queued_bits, as_idx)) { + LOG_DBG("Already double queued on 0x%02x", ep); + return -EAGAIN; + } } buf = uac2_buf_alloc(ep, data, size); @@ -278,7 +284,7 @@ int usbd_uac2_send(const struct device *dev, uint8_t terminal, * enough, but if it does all we loose is just single packet. */ LOG_ERR("No netbuf for send"); - atomic_clear_bit(&ctx->as_queued, as_idx); + atomic_clear_bit(queued_bits, as_idx); ctx->ops->buf_release_cb(dev, terminal, data, ctx->user_data); return -ENOMEM; } @@ -287,7 +293,7 @@ int usbd_uac2_send(const struct device *dev, uint8_t terminal, if (ret) { LOG_ERR("Failed to enqueue net_buf for 0x%02x", ep); net_buf_unref(buf); - atomic_clear_bit(&ctx->as_queued, as_idx); + atomic_clear_bit(queued_bits, as_idx); ctx->ops->buf_release_cb(dev, terminal, data, ctx->user_data); } @@ -761,8 +767,8 @@ static int uac2_request(struct usbd_class_data *const c_data, struct net_buf *bu if (is_feedback) { ctx->fb_queued &= ~BIT(as_idx); - } else { - atomic_clear_bit(&ctx->as_queued, as_idx); + } else if (!atomic_test_and_clear_bit(&ctx->as_queued, as_idx) || buf->frags) { + atomic_clear_bit(&ctx->as_double, as_idx); } if (USB_EP_DIR_IS_OUT(ep)) { diff --git a/subsys/usb/device_next/usbd_ch9.c b/subsys/usb/device_next/usbd_ch9.c index 6910c6df147..ed7cf14f68d 100644 --- a/subsys/usb/device_next/usbd_ch9.c +++ b/subsys/usb/device_next/usbd_ch9.c @@ -858,7 +858,13 @@ static int sreq_get_interface(struct usbd_context *const uds_ctx, return 0; } + /* Treat as error in default (not specified) and addressed states. */ cfg_nd = usbd_config_get_current(uds_ctx); + if (cfg_nd == NULL) { + errno = -EPERM; + return 0; + } + cfg_desc = cfg_nd->desc; if (setup->wIndex > UINT8_MAX ||