Skip to contentSkip to conversation

usb: cdns3: two fixes for gadget

PATCH In review Peter Chen 3 participants 5 messages Feb 19, 2020 – Mar 4, 2020

Subsystems (3)
USB Cadence USB3 DRD IP driver Cadence usbssp DRD IP driver
lore
Share & tools
lore

Peter Chen

author
#0 lore raw

v2 cover usb: cdns3: two fixes for gadget

These two issues are found during run "Error Recovery Test" for the latest USB CV MSC test, the TRB doesn't advance correctly after dequeue and clear halt. With these two fixes, the test can be passed.

Peter Chen (2):
usb: cdns3: gadget: link trb should point to next request
usb: cdns3: gadget: toggle cycle bit before reset endpoint

 drivers/usb/cdns3/gadget.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)
Signature · 1 line
2.17.1
author
#1 lore raw

PATCH v2 1/2 usb: cdns3: gadget: link trb should point to next request

It has marked the dequeue trb as link trb, but its next segment pointer is still itself, it causes the transfer can't go on. Fix it by set its pointer as the trb address for the next request.

Fixes f616c3bda47e ("usb: cdns3: Fix dequeue implementation")
Signed-off-by Peter Chen <[email protected]>

---
No changes.

 drivers/usb/cdns3/gadget.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/cdns3/gadget.c +1 -1
index 736b0c6e27fe..1d8a2af35bb0 100644
@@ -2550,7 +2550,7 @@ int cdns3_gadget_ep_dequeue(struct usb_ep *ep,
 	/* Update ring only if removed request is on pending_req_list list */
 	if (req_on_hw_ring) {
 		link_trb->buffer = TRB_BUFFER(priv_ep->trb_pool_dma +
-					      (priv_req->start_trb * TRB_SIZE));
+			((priv_req->end_trb + 1) * TRB_SIZE));
 		link_trb->control = (link_trb->control & TRB_CYCLE) |
 				    TRB_TYPE(TRB_LINK) | TRB_CHAIN;
Signature · 1 line
2.17.1
author
#2 lore raw

PATCH v2 2/2 usb: cdns3: gadget: toggle cycle bit before reset endpoint

In reply to Peter Chen

If there are TRBs pending during reset endpoint operation, the DMA will advance after reset operation, but it isn't expected, since the data is not yet available (For OUT, the data is not yet available). After the data is ready, there won't be any interrupt since the EP_TRADDR already points to next TRB entry and doorbell is not set.

To fix it, it toggles cycle bit before reset operation, and restores it after reset, it could avoid unexpected DMA advance due to cycle bit is for software during the endpoint reset operation.

Fixes 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver")
Signed-off-by Peter Chen <[email protected]>

---
Changes for v2:
- Improve the comments.
- Using MACRO TRB_CYCLE at code

 drivers/usb/cdns3/gadget.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)
drivers/usb/cdns3/gadget.c +14 -3
index 1d8a2af35bb0..3574dbb09366 100644
@@ -2595,11 +2595,21 @@ int __cdns3_gadget_ep_clear_halt(struct cdns3_endpoint *priv_ep)
 {
 	struct cdns3_device *priv_dev = priv_ep->cdns3_dev;
 	struct usb_request *request;
+	struct cdns3_request *priv_req;
+	struct cdns3_trb *trb = NULL;
 	int ret;
 	int val;
 
 	trace_cdns3_halt(priv_ep, 0, 0);
 
+	request = cdns3_next_request(&priv_ep->pending_req_list);
+	if (request) {
+		priv_req = to_cdns3_request(request);
+		trb = priv_req->trb;
+		if (trb)
+			trb->control = trb->control ^ TRB_CYCLE;
+	}
+
 	writel(EP_CMD_CSTALL | EP_CMD_EPRST, &priv_dev->regs->ep_cmd);
 
 	/* wait for EPRST cleared */

@@ -2610,10 +2620,11 @@ int __cdns3_gadget_ep_clear_halt(struct cdns3_endpoint *priv_ep)
 
 	priv_ep->flags &= ~(EP_STALLED | EP_STALL_PENDING);
 
-	request = cdns3_next_request(&priv_ep->pending_req_list);
-
-	if (request)
+	if (request) {
+		if (trb)
+			trb->control = trb->control ^ TRB_CYCLE;
 		cdns3_rearm_transfer(priv_ep, 1);
+	}
 
 	cdns3_start_all_request(priv_dev, priv_ep);
 	return ret;
Signature · 1 line
2.17.1
#3 lore raw
In reply to Peter Chen
Quoted 6 lines · Peter Chen

These two issues are found during run "Error Recovery Test" for the latest USB CV MSC test, the TRB doesn't advance correctly after dequeue and clear halt. With these two fixes, the test can be passed.

Quoted 7 lines

Peter Chen (2):
usb: cdns3: gadget: link trb should point to next request
usb: cdns3: gadget: toggle cycle bit before reset endpoint

 drivers/usb/cdns3/gadget.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

A gentle ping...

Peter

maintainer
#4 lore raw
Quoted 11 lines · Peter Chen
Quoted 10 lines · Peter Chen

These two issues are found during run "Error Recovery Test" for the latest USB CV MSC test, the TRB doesn't advance correctly after dequeue and clear halt. With these two fixes, the test can be passed.

Peter Chen (2):
usb: cdns3: gadget: link trb should point to next request
usb: cdns3: gadget: toggle cycle bit before reset endpoint

Quoted 5 lines
Quoted 3 lines
 drivers/usb/cdns3/gadget.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

A gentle ping...

I can take these now, thanks.

greg k-h