Skip to content

Commit 9aff5da

Browse files
tmon-nordickartben
authored andcommitted
drivers: udc_dwc2: Optimize incomplete iso handling
At High-Speed there is at most 25 us handling window for incomplete iso IN/OUT and therefore determining which endpoints are isochronous is too wasteful. Add lookup variable holding which isochronous endpoints are enabled and limit the search to only enabled endpoints. For applications with just one OUT and one IN isochronous endpoint this is optimal. The lookup variable is updated only when mutex is held. Interrupt handler accesses the variable read-only and in general there is no problem is incomplete iso handling interrupt hits when the lookup variable is updated, because: * when endpoint is just activated, it cannot be source of incomplete iso interrupt because the endpoint is not armed yet * when endpoint is just deactivated, it was first disabled If there is more than one isochronous endpoint same direction then just relying on endpoint enabled is not necessarily optimal. However, in order to be able to limit the search to only armed endpoints, the lookup variable would have to be updated on every transfer preparation and completion which would require more time-expensive synchronization. Signed-off-by: Tomasz Moń <tomasz.mon@nordicsemi.no>
1 parent 70cd0ab commit 9aff5da

File tree

1 file changed

+57
-43
lines changed

1 file changed

+57
-43
lines changed

drivers/usb/udc/udc_dwc2.c

Lines changed: 57 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,8 @@ struct udc_dwc2_data {
116116
uint32_t max_pktcnt;
117117
uint32_t tx_len[16];
118118
uint32_t rx_siz[16];
119+
/* Isochronous endpoint enabled (IN on bits 0-15, OUT on bits 16-31) */
120+
uint32_t iso_enabled;
119121
uint16_t txf_set;
120122
uint16_t pending_tx_flush;
121123
uint16_t dfifodepth;
@@ -1475,6 +1477,12 @@ static int udc_dwc2_ep_activate(const struct device *dev,
14751477
dwc2_set_epint(dev, cfg, true);
14761478
sys_write32(dxepctl, dxepctl_reg);
14771479

1480+
if (dwc2_ep_is_iso(cfg)) {
1481+
uint8_t ep_bit = ep_idx + (USB_EP_DIR_IS_OUT(cfg->addr) ? 16 : 0);
1482+
1483+
priv->iso_enabled |= BIT(ep_bit);
1484+
}
1485+
14781486
for (uint8_t i = 1U; i < priv->ineps; i++) {
14791487
LOG_DBG("DIEPTXF%u %08x DIEPCTL%u %08x",
14801488
i, sys_read32((mem_addr_t)&base->dieptxf[i - 1U]), i, dxepctl);
@@ -1668,6 +1676,7 @@ static void udc_dwc2_ep_disable(const struct device *dev,
16681676
static int udc_dwc2_ep_deactivate(const struct device *dev,
16691677
struct udc_ep_config *const cfg)
16701678
{
1679+
struct udc_dwc2_data *const priv = udc_get_private(dev);
16711680
uint8_t ep_idx = USB_EP_GET_IDX(cfg->addr);
16721681
mem_addr_t dxepctl_reg;
16731682
uint32_t dxepctl;
@@ -1696,6 +1705,12 @@ static int udc_dwc2_ep_deactivate(const struct device *dev,
16961705
sys_write32(dxepctl, dxepctl_reg);
16971706
dwc2_set_epint(dev, cfg, false);
16981707

1708+
if (dwc2_ep_is_iso(cfg)) {
1709+
uint8_t ep_bit = ep_idx + (USB_EP_DIR_IS_OUT(cfg->addr) ? 16 : 0);
1710+
1711+
priv->iso_enabled &= ~BIT(ep_bit);
1712+
}
1713+
16991714
return 0;
17001715
}
17011716

@@ -2306,6 +2321,7 @@ static int dwc2_driver_preinit(const struct device *dev)
23062321
k_event_init(&priv->drv_evt);
23072322
atomic_clear(&priv->xfer_new);
23082323
atomic_clear(&priv->xfer_finished);
2324+
priv->iso_enabled = 0;
23092325

23102326
data->caps.rwup = true;
23112327
data->caps.addr_before_status = true;
@@ -2762,38 +2778,37 @@ static void dwc2_handle_incompisoin(const struct device *dev)
27622778
USB_DWC2_DEPCTL_EPENA |
27632779
usb_dwc2_set_depctl_eptype(USB_DWC2_DEPCTL_EPTYPE_ISO) |
27642780
USB_DWC2_DEPCTL_USBACTEP;
2781+
uint32_t eps = priv->iso_enabled & 0x0000FFFFUL;
27652782

2766-
for (uint8_t i = 1U; i < priv->numdeveps; i++) {
2767-
uint32_t epdir = usb_dwc2_get_ghwcfg1_epdir(priv->ghwcfg1, i);
2768-
2769-
if (epdir == USB_DWC2_GHWCFG1_EPDIR_IN ||
2770-
epdir == USB_DWC2_GHWCFG1_EPDIR_BDIR) {
2771-
mem_addr_t diepctl_reg = dwc2_get_dxepctl_reg(dev, i | USB_EP_DIR_IN);
2772-
uint32_t diepctl;
2783+
while (eps) {
2784+
uint8_t i = find_lsb_set(eps) - 1;
2785+
mem_addr_t diepctl_reg = dwc2_get_dxepctl_reg(dev, i | USB_EP_DIR_IN);
2786+
uint32_t diepctl;
27732787

2774-
diepctl = sys_read32(diepctl_reg);
2788+
diepctl = sys_read32(diepctl_reg);
27752789

2776-
/* Check if endpoint didn't receive ISO OUT data */
2777-
if ((diepctl & mask) == val) {
2778-
struct udc_ep_config *cfg;
2779-
struct net_buf *buf;
2790+
/* Check if endpoint didn't receive ISO IN data */
2791+
if ((diepctl & mask) == val) {
2792+
struct udc_ep_config *cfg;
2793+
struct net_buf *buf;
27802794

2781-
cfg = udc_get_ep_cfg(dev, i | USB_EP_DIR_IN);
2782-
__ASSERT_NO_MSG(cfg && cfg->stat.enabled &&
2783-
dwc2_ep_is_iso(cfg));
2795+
cfg = udc_get_ep_cfg(dev, i | USB_EP_DIR_IN);
2796+
__ASSERT_NO_MSG(cfg && cfg->stat.enabled &&
2797+
dwc2_ep_is_iso(cfg));
27842798

2785-
udc_dwc2_ep_disable(dev, cfg, false);
2799+
udc_dwc2_ep_disable(dev, cfg, false);
27862800

2787-
buf = udc_buf_get(cfg);
2788-
if (buf) {
2789-
/* Data is no longer relevant */
2790-
udc_submit_ep_event(dev, buf, 0);
2801+
buf = udc_buf_get(cfg);
2802+
if (buf) {
2803+
/* Data is no longer relevant */
2804+
udc_submit_ep_event(dev, buf, 0);
27912805

2792-
/* Try to queue next packet before SOF */
2793-
dwc2_handle_xfer_next(dev, cfg);
2794-
}
2806+
/* Try to queue next packet before SOF */
2807+
dwc2_handle_xfer_next(dev, cfg);
27952808
}
27962809
}
2810+
2811+
eps &= ~BIT(i);
27972812
}
27982813

27992814
sys_write32(USB_DWC2_GINTSTS_INCOMPISOIN, gintsts_reg);
@@ -2817,34 +2832,33 @@ static void dwc2_handle_incompisoout(const struct device *dev)
28172832
usb_dwc2_set_depctl_eptype(USB_DWC2_DEPCTL_EPTYPE_ISO) |
28182833
((priv->sof_num & 1) ? USB_DWC2_DEPCTL_DPID : 0) |
28192834
USB_DWC2_DEPCTL_USBACTEP;
2835+
uint32_t eps = (priv->iso_enabled & 0xFFFF0000UL) >> 16;
28202836

2821-
for (uint8_t i = 1U; i < priv->numdeveps; i++) {
2822-
uint32_t epdir = usb_dwc2_get_ghwcfg1_epdir(priv->ghwcfg1, i);
2837+
while (eps) {
2838+
uint8_t i = find_lsb_set(eps) - 1;
2839+
mem_addr_t doepctl_reg = dwc2_get_dxepctl_reg(dev, i);
2840+
uint32_t doepctl;
28232841

2824-
if (epdir == USB_DWC2_GHWCFG1_EPDIR_OUT ||
2825-
epdir == USB_DWC2_GHWCFG1_EPDIR_BDIR) {
2826-
mem_addr_t doepctl_reg = dwc2_get_dxepctl_reg(dev, i);
2827-
uint32_t doepctl;
2828-
2829-
doepctl = sys_read32(doepctl_reg);
2842+
doepctl = sys_read32(doepctl_reg);
28302843

2831-
/* Check if endpoint didn't receive ISO OUT data */
2832-
if ((doepctl & mask) == val) {
2833-
struct udc_ep_config *cfg;
2834-
struct net_buf *buf;
2844+
/* Check if endpoint didn't receive ISO OUT data */
2845+
if ((doepctl & mask) == val) {
2846+
struct udc_ep_config *cfg;
2847+
struct net_buf *buf;
28352848

2836-
cfg = udc_get_ep_cfg(dev, i);
2837-
__ASSERT_NO_MSG(cfg && cfg->stat.enabled &&
2838-
dwc2_ep_is_iso(cfg));
2849+
cfg = udc_get_ep_cfg(dev, i);
2850+
__ASSERT_NO_MSG(cfg && cfg->stat.enabled &&
2851+
dwc2_ep_is_iso(cfg));
28392852

2840-
udc_dwc2_ep_disable(dev, cfg, false);
2853+
udc_dwc2_ep_disable(dev, cfg, false);
28412854

2842-
buf = udc_buf_get(cfg);
2843-
if (buf) {
2844-
udc_submit_ep_event(dev, buf, 0);
2845-
}
2855+
buf = udc_buf_get(cfg);
2856+
if (buf) {
2857+
udc_submit_ep_event(dev, buf, 0);
28462858
}
28472859
}
2860+
2861+
eps &= ~BIT(i);
28482862
}
28492863

28502864
sys_write32(USB_DWC2_GINTSTS_INCOMPISOOUT, gintsts_reg);

0 commit comments

Comments
 (0)