diff mbox series

[1/1] usb: cdns3: fix uvc failure work since sg support enabled

Message ID 20231106153452.2397035-1-Frank.Li@nxp.com (mailing list archive)
State Accepted
Commit 1b8be5ecff26201bafb0a554c74e91571299fb94
Headers show
Series [1/1] usb: cdns3: fix uvc failure work since sg support enabled | expand

Commit Message

Frank Li Nov. 6, 2023, 3:34 p.m. UTC 
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@nxp.com>
---

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(-)

Comments

Peter Chen Nov. 9, 2023, 1:11 p.m. UTC | #1 
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@nxp.com>

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
>
diff mbox series

Patch

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;