On 23-11-06 10:34:52, Frank Li wrote: > When IP version >= DEV_VER_V2, gadget:sg_supported is true. So uvc gadget > function driver will use sg to equeue data, first is 8bytes header, the > second is 1016bytes data. > > cdns3_prepare_trb: ep2in: trb 0000000000ac755f, dma buf: 0xbf455000, size: 8, burst: 128 ctrl: 0x00000415 (C=1, T=0, ISP, CHAIN, Normal) > cdns3_prepare_trb: ep2in: trb 00000000a574e693, dma buf: 0xc0200fe0, size: 1016, burst: 128 ctrl: 0x00000405 (C=1, T=0, ISP, Normal) > > But cdns3_ep_run_transfer() can't correctly handle this case, which only > support one TRB for ISO transfer. > > The controller requires duplicate the TD for each SOF if priv_ep->interval > is not 1. DMA will read data from DDR to internal FIFO when get SOF. Send > data to bus when receive IN token. DMA always refill FIFO when get SOF > regardless host send IN token or not. If host send IN token later, some > frames data will be lost. > > Fixed it by below major steps: > > 1. Calculate numembers of TRB base on sg_nums and priv_ep->interval. > 2. Remove CHAIN flags for each end TRB of TD when duplicate TD. > 3. The controller requires LINK TRB must be first TRB of TD. When check > there are not enough TRBs lefts, just fill LINK TRB for left TRBs. > > .... CHAIN_TRB DATA_TRB, CHAIN_TRB DATA_TRB, LINK_TRB ... LINK_TRB > ^End of TRB List > > Fixes: 7733f6c32e36f ("usb: cdns3: Add Cadence USB3 DRD Driver") > Signed-off-by: Frank Li <Frank.Li@xxxxxxx> You mentioned android still has some problems, so let's wait your qualify results. Since it is a big change, please test more classes (mtp/adb/uvc/ncm). > --- > > This patch is depend on https://lore.kernel.org/imx/20231031084521.GA1948529@nchen-desktop/T/#t > > drivers/usb/cdns3/cdns3-gadget.c | 51 +++++++++++++++++++++++++------- > 1 file changed, 40 insertions(+), 11 deletions(-) > > diff --git a/drivers/usb/cdns3/cdns3-gadget.c b/drivers/usb/cdns3/cdns3-gadget.c > index cd08897f8da8..23580ec33202 100644 > --- a/drivers/usb/cdns3/cdns3-gadget.c > +++ b/drivers/usb/cdns3/cdns3-gadget.c > @@ -1117,6 +1117,7 @@ static int cdns3_ep_run_transfer(struct cdns3_endpoint *priv_ep, > dma_addr_t trb_dma; > u32 togle_pcs = 1; > int sg_iter = 0; > + int num_trb_req; > int num_trb; > int address; > u32 control; > @@ -1126,15 +1127,13 @@ static int cdns3_ep_run_transfer(struct cdns3_endpoint *priv_ep, > bool sg_supported = !!(request->num_mapped_sgs); > u32 ioc = request->no_interrupt ? 0 : TRB_IOC; > > + num_trb_req = sg_supported ? request->num_mapped_sgs : 1; > + > + /* ISO transfer require each SOF have a TD, each TD include some TRBs */ > if (priv_ep->type == USB_ENDPOINT_XFER_ISOC) > - num_trb = priv_ep->interval; > + num_trb = priv_ep->interval * num_trb_req; > else > - num_trb = sg_supported ? request->num_mapped_sgs : 1; > - > - if (num_trb > priv_ep->free_trbs) { > - priv_ep->flags |= EP_RING_FULL; > - return -ENOBUFS; > - } > + num_trb = num_trb_req; > > priv_req = to_cdns3_request(request); > address = priv_ep->endpoint.desc->bEndpointAddress; > @@ -1183,14 +1182,31 @@ static int cdns3_ep_run_transfer(struct cdns3_endpoint *priv_ep, > > link_trb->control = cpu_to_le32(((priv_ep->pcs) ? TRB_CYCLE : 0) | > TRB_TYPE(TRB_LINK) | TRB_TOGGLE | ch_bit); > + > + if (priv_ep->type == USB_ENDPOINT_XFER_ISOC) { > + /* > + * ISO require LINK TRB must be first one of TD. > + * Fill LINK TRBs for left trb space to simply software process logic. > + */ > + while (priv_ep->enqueue) { > + *trb = *link_trb; > + trace_cdns3_prepare_trb(priv_ep, trb); > + > + cdns3_ep_inc_enq(priv_ep); > + trb = priv_ep->trb_pool + priv_ep->enqueue; > + priv_req->trb = trb; > + } > + } > + } > + > + if (num_trb > priv_ep->free_trbs) { > + priv_ep->flags |= EP_RING_FULL; > + return -ENOBUFS; > } > > if (priv_dev->dev_ver <= DEV_VER_V2) > togle_pcs = cdns3_wa1_update_guard(priv_ep, trb); > > - if (sg_supported) > - s = request->sg; > - > /* set incorrect Cycle Bit for first trb*/ > control = priv_ep->pcs ? 0 : TRB_CYCLE; > trb->length = 0; > @@ -1208,6 +1224,9 @@ static int cdns3_ep_run_transfer(struct cdns3_endpoint *priv_ep, > do { > u32 length; > > + if (!(sg_iter % num_trb_req) && sg_supported) > + s = request->sg; > + > /* fill TRB */ > control |= TRB_TYPE(TRB_NORMAL); > if (sg_supported) { > @@ -1249,7 +1268,7 @@ static int cdns3_ep_run_transfer(struct cdns3_endpoint *priv_ep, > if (sg_supported) { > trb->control |= cpu_to_le32(TRB_ISP); > /* Don't set chain bit for last TRB */ > - if (sg_iter < num_trb - 1) > + if ((sg_iter % num_trb_req) < num_trb_req - 1) > trb->control |= cpu_to_le32(TRB_CHAIN); > > s = sg_next(s); > @@ -1507,6 +1526,12 @@ static void cdns3_transfer_completed(struct cdns3_device *priv_dev, > > /* The TRB was changed as link TRB, and the request was handled at ep_dequeue */ > while (TRB_FIELD_TO_TYPE(le32_to_cpu(trb->control)) == TRB_LINK) { > + > + /* ISO ep_traddr may stop at LINK TRB */ > + if (priv_ep->dequeue == cdns3_get_dma_pos(priv_dev, priv_ep) && > + priv_ep->type == USB_ENDPOINT_XFER_ISOC) > + break; > + > trace_cdns3_complete_trb(priv_ep, trb); > cdns3_ep_inc_deq(priv_ep); > trb = priv_ep->trb_pool + priv_ep->dequeue; > @@ -1539,6 +1564,10 @@ static void cdns3_transfer_completed(struct cdns3_device *priv_dev, > } > > if (request_handled) { > + /* TRBs are duplicated by priv_ep->interval time for ISO IN */ > + if (priv_ep->type == USB_ENDPOINT_XFER_ISOC && priv_ep->dir) > + request->actual /= priv_ep->interval; > + > cdns3_gadget_giveback(priv_ep, priv_req, 0); > request_handled = false; > transfer_end = false; > -- > 2.34.1 > -- Thanks, Peter Chen