The dwc3 gadget driver is correctly checking the prepare and started
request lists for potential underruns and will stop the running transfer
in that case. However it is possible that the running pipeline will lead
into more underrun scenarios, which can be avoided and be detected. This
series is adding the corresponding code to ensure that an underrun
transfer will be handled properly.
Signed-off-by: Michael Grzeschik <[email protected]>
---
Michael Grzeschik (3):
usb: dwc3: gadget: reclaim the whole started list when request was missed
usb: dwc3: gadget: check drained isoc ep
usb: dwc3: gadget: check the whole started queue for missed requests in complete
drivers/usb/dwc3/gadget.c | 38 ++++++++++++++++++++++++++++++++++++--
1 file changed, 36 insertions(+), 2 deletions(-)
---
base-commit: dfea18989aa7beb42c2cb6344fe8787de35d9471
change-id: 20240307-dwc3-gadget-complete-irq-1a8ffa347fd1
Best regards,
--
Michael Grzeschik <[email protected]>
The function dwc3_gadget_endpoint_trbs_complete will be called with the
interrupt event. This event is only representing the interrupt reason of
the exact trb that had IOC enabled. In the current approach the function
dwc3_gadget_ep_cleanup_completed_requests will give back and complete
the requests with the corresponding trb status and therefor will
correctly return the missed requests that are already finished.
Since inbetween those complete functioncalls of the properly
finished and the missed trbs, a missed transfer could get new
trbs enqueued with the updatecmd, this will just lead to more missed
trbs.
To break the cascading scenario this patch is checking all trbs from the
started list for any trb that has the missed trb status before even
calling the complete handler but stopping the transfer instead.
Signed-off-by: Michael Grzeschik <[email protected]>
---
drivers/usb/dwc3/gadget.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index f22b68a0b2dac..ca87f4d988d41 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -3563,6 +3563,21 @@ static void dwc3_gadget_endpoint_frame_from_event(struct dwc3_ep *dep,
dep->frame_number = event->parameters;
}
+static int dwc3_gadget_ep_check_missed_requests(struct dwc3_ep *dep)
+{
+ struct dwc3_request *req;
+ struct dwc3_request *tmp;
+
+ list_for_each_entry_safe(req, tmp, &dep->started_list, list) {
+ struct dwc3_trb *trb = req->trb;
+
+ if (DWC3_TRB_SIZE_TRBSTS(trb->size) == DWC3_TRBSTS_MISSED_ISOC)
+ return -EXDEV;
+ }
+
+ return 0;
+}
+
static bool dwc3_gadget_endpoint_trbs_complete(struct dwc3_ep *dep,
const struct dwc3_event_depevt *event, int status)
{
@@ -3584,7 +3599,7 @@ static bool dwc3_gadget_endpoint_trbs_complete(struct dwc3_ep *dep,
transfer_in_flight = trb->ctrl & DWC3_TRB_CTRL_HWO;
}
- if (status == -EXDEV || !transfer_in_flight)
+ if (status == -EXDEV || !transfer_in_flight || dwc3_gadget_ep_check_missed_requests(dep))
dwc3_stop_active_transfer(dep, true, true);
dwc3_gadget_ep_cleanup_completed_requests(dep, event, status);
--
2.39.2
On EXDEV/Missed status interrupt, the handler has to stop the running
transfer and ensure that every request complete call will not run kick
on the current transfer. This is achieved by calling
dwc3_stop_active_transfer early before cleaning up the started request
list.
If the current transfer was stopped we can not keep all requests in the
hardware and have to reclaim them all. Since the requests could have
no_interrupt bit the cleanup on an stopped pipeline has to remove every
request and may not stop on the one that has the IOC bit set.
To ensure that the dwc3_gadget_ep_cleanup_completed_request will
iterate over every request in that case, we skip the stop condition
in reclaim_completed_trb if the current transfer was stopped by checking
for DWC3_EP_END_TRANSFER_PENDING bit set.
Signed-off-by: Michael Grzeschik <[email protected]>
---
drivers/usb/dwc3/gadget.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 40c52dbc28d3b..b9fce7b1dcdec 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -3404,7 +3404,8 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
DWC3_TRB_SIZE_TRBSTS(trb->size) == DWC3_TRBSTS_MISSED_ISOC)
return 1;
- if ((trb->ctrl & DWC3_TRB_CTRL_IOC) ||
+ if (((trb->ctrl & DWC3_TRB_CTRL_IOC) &&
+ !(dep->flags & DWC3_EP_END_TRANSFER_PENDING)) ||
(trb->ctrl & DWC3_TRB_CTRL_LST))
return 1;
@@ -3568,6 +3569,9 @@ static bool dwc3_gadget_endpoint_trbs_complete(struct dwc3_ep *dep,
struct dwc3 *dwc = dep->dwc;
bool no_started_trb = true;
+ if (status == -EXDEV)
+ dwc3_stop_active_transfer(dep, true, true);
+
dwc3_gadget_ep_cleanup_completed_requests(dep, event, status);
if (dep->flags & DWC3_EP_END_TRANSFER_PENDING)
@@ -3578,7 +3582,7 @@ static bool dwc3_gadget_endpoint_trbs_complete(struct dwc3_ep *dep,
if (usb_endpoint_xfer_isoc(dep->endpoint.desc) &&
list_empty(&dep->started_list) &&
- (list_empty(&dep->pending_list) || status == -EXDEV))
+ (list_empty(&dep->pending_list)))
dwc3_stop_active_transfer(dep, true, true);
else if (dwc3_gadget_ep_should_continue(dep))
if (__dwc3_gadget_kick_transfer(dep) == 0)
--
2.39.2
To avoid a potential underrun of an currently drained transfer
we add a check for that scenario in the dwc3_gadget_endpoint_trbs_complete
function. In the case of an empty trb ring, we call the stop_transfer
cmd and avoid the underrun to occur.
Signed-off-by: Michael Grzeschik <[email protected]>
---
drivers/usb/dwc3/gadget.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index b9fce7b1dcdec..f22b68a0b2dac 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -3568,8 +3568,23 @@ static bool dwc3_gadget_endpoint_trbs_complete(struct dwc3_ep *dep,
{
struct dwc3 *dwc = dep->dwc;
bool no_started_trb = true;
+ unsigned int transfer_in_flight = 0;
+
+ /* It is possible that the interrupt thread was delayed by
+ * scheduling in the system, and therefor the HW has already
+ * run dry. In that case the last trb in the queue is already
+ * handled by the hw. By checking the HWO bit we know to restart
+ * the whole transfer. The condition to appear is more likely
+ * if not every req has the IOC bit set and therefor does not
+ * trigger the interrupt thread frequently.
+ */
+ if (dep->number && usb_endpoint_xfer_isoc(dep->endpoint.desc)) {
+ struct dwc3_trb *trb = dwc3_ep_prev_trb(dep, dep->trb_enqueue);
+
+ transfer_in_flight = trb->ctrl & DWC3_TRB_CTRL_HWO;
+ }
- if (status == -EXDEV)
+ if (status == -EXDEV || !transfer_in_flight)
dwc3_stop_active_transfer(dep, true, true);
dwc3_gadget_ep_cleanup_completed_requests(dep, event, status);
--
2.39.2
On Thu, Mar 07, 2024 at 04:22:02PM +0100, Michael Grzeschik wrote:
>The dwc3 gadget driver is correctly checking the prepare and started
>request lists for potential underruns and will stop the running transfer
>in that case. However it is possible that the running pipeline will lead
>into more underrun scenarios, which can be avoided and be detected. This
>series is adding the corresponding code to ensure that an underrun
>transfer will be handled properly.
>
>Signed-off-by: Michael Grzeschik <[email protected]>
>---
>Michael Grzeschik (3):
> usb: dwc3: gadget: reclaim the whole started list when request was missed
> usb: dwc3: gadget: check drained isoc ep
> usb: dwc3: gadget: check the whole started queue for missed requests in complete
>
> drivers/usb/dwc3/gadget.c | 38 ++++++++++++++++++++++++++++++++++++--
> 1 file changed, 36 insertions(+), 2 deletions(-)
>---
>base-commit: dfea18989aa7beb42c2cb6344fe8787de35d9471
>change-id: 20240307-dwc3-gadget-complete-irq-1a8ffa347fd1
>
>Best regards,
Since it is not right to fully stop the ep after one missed transfer was
detected, this series is not correct anymore. We solve the overall issue
in the upper layer now.
Besides the patch "usb: dwc3: gadget: check drained isoc ep" is not
completely wrong, I will resend that one in another veriant.
mgr
--
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 |