On 19-10-07 13:06:18, Pawel Laszczak wrote:
> Dequeuing implementation in cdns3_gadget_ep_dequeu gets first request from
%s/cdns3_gadget_ep_dequeu/cdns3_gadget_ep_dequeue
> deferred_req_list and changed TRB associated with it to LINK TRB.
> This approach is incorrect because deferred_req_list contains requests
> that have not been placed on hardware RING. In this case driver should
> just giveback this request to gadget driver.
>
> The patch implements new approach that first checks where dequeuing
> request is located and only when it's on Transfer Ring then changes TRB
> associated with it to LINK TRB.
> During processing completed transfers such LINK TRB will be ignored.
>
> Reported-by: Peter Chen <peter.c...@nxp.com>
> Signed-off-by: Pawel Laszczak <paw...@cadence.com>
> Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver")
> ---
> drivers/usb/cdns3/gadget.c | 35 ++++++++++++++++++++---------------
> 1 file changed, 20 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
> index 2ca280f4c054..9050b380ab83 100644
> --- a/drivers/usb/cdns3/gadget.c
> +++ b/drivers/usb/cdns3/gadget.c
> @@ -1145,6 +1145,14 @@ static void cdns3_transfer_completed(struct
> cdns3_device *priv_dev,
> request = cdns3_next_request(&priv_ep->pending_req_list);
> priv_req = to_cdns3_request(request);
>
> + trb = priv_ep->trb_pool + priv_ep->dequeue;
> +
> + /* Request was dequeued and TRB was changed to TRB_LINK. */
> + if (TRB_FIELD_TO_TYPE(trb->control) == TRB_LINK) {
> + trace_cdns3_complete_trb(priv_ep, trb);
> + cdns3_move_deq_to_next_trb(priv_req);
> + }
If the request was dequeued, it should not be at request list, isn't it?
Peter
> +
> /* Re-select endpoint. It could be changed by other CPU during
> * handling usb_gadget_giveback_request.
> */
> @@ -2067,6 +2075,7 @@ int cdns3_gadget_ep_dequeue(struct usb_ep *ep,
> struct usb_request *req, *req_temp;
> struct cdns3_request *priv_req;
> struct cdns3_trb *link_trb;
> + u8 req_on_hw_ring = 0;
> unsigned long flags;
> int ret = 0;
>
> @@ -2083,8 +2092,10 @@ int cdns3_gadget_ep_dequeue(struct usb_ep *ep,
>
> list_for_each_entry_safe(req, req_temp, &priv_ep->pending_req_list,
> list) {
> - if (request == req)
> + if (request == req) {
> + req_on_hw_ring = 1;
> goto found;
> + }
> }
>
> list_for_each_entry_safe(req, req_temp, &priv_ep->deferred_req_list,
> @@ -2096,27 +2107,21 @@ int cdns3_gadget_ep_dequeue(struct usb_ep *ep,
> goto not_found;
>
> found:
> -
> - if (priv_ep->wa1_trb == priv_req->trb)
> - cdns3_wa1_restore_cycle_bit(priv_ep);
> -
> link_trb = priv_req->trb;
> - cdns3_move_deq_to_next_trb(priv_req);
> - cdns3_gadget_giveback(priv_ep, priv_req, -ECONNRESET);
> -
> - /* Update ring */
> - request = cdns3_next_request(&priv_ep->deferred_req_list);
> - if (request) {
> - priv_req = to_cdns3_request(request);
>
> + /* 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));
> link_trb->control = (link_trb->control & TRB_CYCLE) |
> - TRB_TYPE(TRB_LINK) | TRB_CHAIN | TRB_TOGGLE;
> - } else {
> - priv_ep->flags |= EP_UPDATE_EP_TRBADDR;
> + TRB_TYPE(TRB_LINK) | TRB_CHAIN;
> +
> + if (priv_ep->wa1_trb == priv_req->trb)
> + cdns3_wa1_restore_cycle_bit(priv_ep);
> }
>
> + cdns3_gadget_giveback(priv_ep, priv_req, -ECONNRESET);
> +
> not_found:
> spin_unlock_irqrestore(&priv_dev->lock, flags);
> return ret;
--
Thanks,
Peter Chen
