| Message ID | 20220901184314.30481-2-quic_wcheng@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix controller halt and endxfer timeout issues | expand |
Hi Wesley, On Thu, Sep 01, 2022 at 11:43:10AM -0700, Wesley Cheng wrote: > If DWC3_EP_DELAYED_STOP is set during stop active transfers, then do not > continue attempting to unmap request buffers during dwc3_remove_requests(). > This can lead to SMMU faults, as the controller has not stopped the > processing of the TRB. Defer this sequence to the EP0 out start, which > ensures that there are no pending SETUP transactions before issuing the > endxfer. > > Reviewed-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> > Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com> > --- > drivers/usb/dwc3/core.h | 3 +++ > drivers/usb/dwc3/ep0.c | 5 ++++- > drivers/usb/dwc3/gadget.c | 6 +++++- > 3 files changed, 12 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > index 4fe4287dc934..7d5d2507244b 100644 > --- a/drivers/usb/dwc3/core.h > +++ b/drivers/usb/dwc3/core.h > @@ -1560,6 +1560,7 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned int cmd, > int dwc3_send_gadget_generic_command(struct dwc3 *dwc, unsigned int cmd, > u32 param); > void dwc3_gadget_clear_tx_fifos(struct dwc3 *dwc); > +void dwc3_remove_requests(struct dwc3 *dwc, struct dwc3_ep *dep, int status); > #else > static inline int dwc3_gadget_init(struct dwc3 *dwc) > { return 0; } > @@ -1581,6 +1582,8 @@ static inline int dwc3_send_gadget_generic_command(struct dwc3 *dwc, > { return 0; } > static inline void dwc3_gadget_clear_tx_fifos(struct dwc3 *dwc) > { } > +static inline void dwc3_remove_requests(struct dwc3 *dwc, struct dwc3_ep *dep) Oops, you missed updating this with the additional 'status' parameter as well. Jack
On Thu, Sep 01, 2022 at 12:22:47PM -0700, Jack Pham wrote: > Hi Wesley, > > On Thu, Sep 01, 2022 at 11:43:10AM -0700, Wesley Cheng wrote: > > If DWC3_EP_DELAYED_STOP is set during stop active transfers, then do not > > continue attempting to unmap request buffers during dwc3_remove_requests(). > > This can lead to SMMU faults, as the controller has not stopped the > > processing of the TRB. Defer this sequence to the EP0 out start, which > > ensures that there are no pending SETUP transactions before issuing the > > endxfer. > > > > Reviewed-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> > > Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com> > > --- > > drivers/usb/dwc3/core.h | 3 +++ > > drivers/usb/dwc3/ep0.c | 5 ++++- > > drivers/usb/dwc3/gadget.c | 6 +++++- > > 3 files changed, 12 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > > index 4fe4287dc934..7d5d2507244b 100644 > > --- a/drivers/usb/dwc3/core.h > > +++ b/drivers/usb/dwc3/core.h > > @@ -1560,6 +1560,7 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned int cmd, > > int dwc3_send_gadget_generic_command(struct dwc3 *dwc, unsigned int cmd, > > u32 param); > > void dwc3_gadget_clear_tx_fifos(struct dwc3 *dwc); > > +void dwc3_remove_requests(struct dwc3 *dwc, struct dwc3_ep *dep, int status); > > #else > > static inline int dwc3_gadget_init(struct dwc3 *dwc) > > { return 0; } > > @@ -1581,6 +1582,8 @@ static inline int dwc3_send_gadget_generic_command(struct dwc3 *dwc, > > { return 0; } > > static inline void dwc3_gadget_clear_tx_fifos(struct dwc3 *dwc) > > { } > > +static inline void dwc3_remove_requests(struct dwc3 *dwc, struct dwc3_ep *dep) > > Oops, you missed updating this with the additional 'status' parameter as > well. Actually, why even bother having this stub at all? This would only be when neither CONFIG_USB_DWC3_GADGET nor CONFIG_USB_DWC3_DUAL_ROLE are enabled, but ep0.c itself wouldn't even be compiled in that case. Jack
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 4fe4287dc934..7d5d2507244b 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -1560,6 +1560,7 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned int cmd, int dwc3_send_gadget_generic_command(struct dwc3 *dwc, unsigned int cmd, u32 param); void dwc3_gadget_clear_tx_fifos(struct dwc3 *dwc); +void dwc3_remove_requests(struct dwc3 *dwc, struct dwc3_ep *dep, int status); #else static inline int dwc3_gadget_init(struct dwc3 *dwc) { return 0; } @@ -1581,6 +1582,8 @@ static inline int dwc3_send_gadget_generic_command(struct dwc3 *dwc, { return 0; } static inline void dwc3_gadget_clear_tx_fifos(struct dwc3 *dwc) { } +static inline void dwc3_remove_requests(struct dwc3 *dwc, struct dwc3_ep *dep) +{ } #endif #if IS_ENABLED(CONFIG_USB_DWC3_DUAL_ROLE) diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c index 33cee0089609..61de693461da 100644 --- a/drivers/usb/dwc3/ep0.c +++ b/drivers/usb/dwc3/ep0.c @@ -293,7 +293,10 @@ void dwc3_ep0_out_start(struct dwc3 *dwc) continue; dwc3_ep->flags &= ~DWC3_EP_DELAY_STOP; - dwc3_stop_active_transfer(dwc3_ep, true, true); + if (dwc->connected) + dwc3_stop_active_transfer(dwc3_ep, true, true); + else + dwc3_remove_requests(dwc, dwc3_ep, -ESHUTDOWN); } } diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index d6c0cb79ace3..6f2491fc109e 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -965,12 +965,16 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep, unsigned int action) return 0; } -static void dwc3_remove_requests(struct dwc3 *dwc, struct dwc3_ep *dep, int status) +void dwc3_remove_requests(struct dwc3 *dwc, struct dwc3_ep *dep, int status) { struct dwc3_request *req; dwc3_stop_active_transfer(dep, true, false); + /* If endxfer is delayed, avoid unmapping requests */ + if (dep->flags & DWC3_EP_DELAY_STOP) + return; + /* - giveback all requests to gadget driver */ while (!list_empty(&dep->started_list)) { req = next_request(&dep->started_list);