Re: [PATCH 3/3] usb: dwc3: gadget: EP_DELAY_START is only handled for non isoc eps

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Mar 01, 2022 at 01:12:37AM +0000, Thinh Nguyen wrote:
Hi,

Michael Grzeschik wrote:
Refactor the codepath for handling DWC3_EP_DELAY_START condition
only being checked on non isoc endpoints.

The DWC3_EP_DELAY_START should still be applicable to isoc and End
Transfer pending. While the End Transfer command is active, don't issue
Start Transfer command.

Previously I think we have a check for the isoc endpoint and
DWC3_EP_DELAY_START because it was intended to check against the halt
condition, but it was done incorrectly. (Note that isoc endpoint doesn't
halt and there's no STALL handshake).

This change should not be applied. If we're to apply the fix for isoc
and delay start check, it should be done separately.

Right! I just realized that we indeed at least also have to check for
DWC3_EP_END_TRANSFER_PENDING flag on isoc transfers. Without that,
we would open up a race where we kick(update) transfers, that are
potentially to late for that current transfer.

So yes, please drop that patch. The other patches on the other hand are
fine I think.

Regards,
Michael

Signed-off-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx>
---
 drivers/usb/dwc3/gadget.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index b89dadaef4db9d..d09bd66f498a69 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1901,17 +1901,6 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep, struct dwc3_request *req)
 	if (dep->flags & DWC3_EP_WAIT_TRANSFER_COMPLETE)
 		return 0;

-	/*
-	 * Start the transfer only after the END_TRANSFER is completed
-	 * and endpoint STALL is cleared.
-	 */
-	if ((dep->flags & DWC3_EP_END_TRANSFER_PENDING) ||
-	    (dep->flags & DWC3_EP_WEDGE) ||
-	    (dep->flags & DWC3_EP_STALL)) {
-		dep->flags |= DWC3_EP_DELAY_START;
-		return 0;
-	}
-
 	/*
 	 * NOTICE: Isochronous endpoints should NEVER be prestarted. We must
 	 * wait for a XferNotReady event so we will know what's the current
@@ -1927,6 +1916,17 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep, struct dwc3_request *req)

 			return 0;
 		}
+	} else {
+		/*
+		 * Start the transfer only after the END_TRANSFER is completed
+		 * and endpoint STALL is cleared.
+		 */
+		if ((dep->flags & DWC3_EP_END_TRANSFER_PENDING) ||
+		    (dep->flags & DWC3_EP_WEDGE) ||
+		    (dep->flags & DWC3_EP_STALL)) {
+			dep->flags |= DWC3_EP_DELAY_START;
+			return 0;
+		}
 	}

 	__dwc3_gadget_kick_transfer(dep);


--
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux