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