| Message ID | 20200901084454.28649-6-peter.chen@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: cdns3: improve the sg use case | expand |
Peter Chen <peter.chen@nxp.com> writes: > - Judge each TRB has been handled at cdns3_trb_handled, since > the DMA pointer may be at the middle of the TD, we can't consider > this TD has finished at that time. > - Calcuate req->actual according to finished TRBs. ^^^^^^^^ calculate > - Handle short transfer for sg list use case correctly. When the > short transfer occurs, we check OUT_SMM at TRB to see if it is > the last TRB. it looks like each of these should be split to its own patch. > Signed-off-by: Peter Chen <peter.chen@nxp.com> > --- > drivers/usb/cdns3/gadget.c | 85 +++++++++++++++++++++++++------------- > drivers/usb/cdns3/gadget.h | 9 ++++ > 2 files changed, 65 insertions(+), 29 deletions(-) > > diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c > index a308a694abc5..6cb44c354f40 100644 > --- a/drivers/usb/cdns3/gadget.c > +++ b/drivers/usb/cdns3/gadget.c > @@ -817,6 +817,8 @@ void cdns3_gadget_giveback(struct cdns3_endpoint *priv_ep, > request->length); > > priv_req->flags &= ~(REQUEST_PENDING | REQUEST_UNALIGNED); > + /* All TRBs have finished, clear the counter */ > + priv_req->finished_trb = 0; > trace_cdns3_gadget_giveback(priv_req); > > if (priv_dev->dev_ver < DEV_VER_V2) { > @@ -1241,6 +1243,7 @@ static int cdns3_ep_run_transfer(struct cdns3_endpoint *priv_ep, > trb = priv_req->trb; > > priv_req->flags |= REQUEST_PENDING; > + priv_req->num_of_trb = num_trb; > > if (sg_iter == 1) > trb->control |= cpu_to_le32(TRB_IOC | TRB_ISP); > @@ -1362,7 +1365,7 @@ void cdns3_set_hw_configuration(struct cdns3_device *priv_dev) > } > > /** > - * cdns3_request_handled - check whether request has been handled by DMA > + * cdns3_trb_handled - check whether trb has been handled by DMA > * > * @priv_ep: extended endpoint object. > * @priv_req: request object for checking > @@ -1379,32 +1382,28 @@ void cdns3_set_hw_configuration(struct cdns3_device *priv_dev) > * ET = priv_req->end_trb - index of last TRB in transfer ring > * CI = current_index - index of processed TRB by DMA. > * > - * As first step, function checks if cycle bit for priv_req->start_trb is > - * correct. > + * As first step, we check if the TRB between the ST and ET. > + * Then, we check if cycle bit for index priv_ep->dequeue > + * is correct. > * > * some rules: > - * 1. priv_ep->dequeue never exceed current_index. > + * 1. priv_ep->dequeue never equals to current_index. > * 2 priv_ep->enqueue never exceed priv_ep->dequeue > * 3. exception: priv_ep->enqueue == priv_ep->dequeue > * and priv_ep->free_trbs is zero. > * This case indicate that TR is full. > * > - * Then We can split recognition into two parts: > + * At below two cases, the request have been handled. > * Case 1 - priv_ep->dequeue < current_index > * SR ... EQ ... DQ ... CI ... ER > * SR ... DQ ... CI ... EQ ... ER > * > - * Request has been handled by DMA if ST and ET is between DQ and CI. > - * > * Case 2 - priv_ep->dequeue > current_index > - * This situation take place when CI go through the LINK TRB at the end of > + * This situation takes place when CI go through the LINK TRB at the end of not part of $subject
On 20-09-08 09:25:41, Felipe Balbi wrote: > Peter Chen <peter.chen@nxp.com> writes: > > > - Judge each TRB has been handled at cdns3_trb_handled, since > > the DMA pointer may be at the middle of the TD, we can't consider > > this TD has finished at that time. > > - Calcuate req->actual according to finished TRBs. > ^^^^^^^^ > calculate > > > - Handle short transfer for sg list use case correctly. When the > > short transfer occurs, we check OUT_SMM at TRB to see if it is > > the last TRB. > > it looks like each of these should be split to its own patch. When I debug sg use case, it indeed took several patches for all functions work, and some patches improved the old patches since some short transfers use case did not be considered well. Using this patch, it could let the completion work for both normal transfer and short transfer. So I prefer keeping one patch. > > + * Then, we check if cycle bit for index priv_ep->dequeue > > + * is correct. > > * > > * some rules: > > - * 1. priv_ep->dequeue never exceed current_index. > > + * 1. priv_ep->dequeue never equals to current_index. > > * 2 priv_ep->enqueue never exceed priv_ep->dequeue > > * 3. exception: priv_ep->enqueue == priv_ep->dequeue > > * and priv_ep->free_trbs is zero. > > * This case indicate that TR is full. > > * > > - * Then We can split recognition into two parts: > > + * At below two cases, the request have been handled. > > * Case 1 - priv_ep->dequeue < current_index > > * SR ... EQ ... DQ ... CI ... ER > > * SR ... DQ ... CI ... EQ ... ER > > * > > - * Request has been handled by DMA if ST and ET is between DQ and CI. > > - * > > * Case 2 - priv_ep->dequeue > current_index > > - * This situation take place when CI go through the LINK TRB at the end of > > + * This situation takes place when CI go through the LINK TRB at the end of > > not part of $subject > I will make another patch for comment improvement, thanks.
> > When I debug sg use case, it indeed took several patches for all functions work, > and some patches improved the old patches since some short transfers use > case did not be considered well. > > Using this patch, it could let the completion work for both normal transfer and > short transfer. So I prefer keeping one patch. > > > > + * Then, we check if cycle bit for index priv_ep->dequeue > > > + * is correct. > > > * > > > * some rules: > > > - * 1. priv_ep->dequeue never exceed current_index. > > > + * 1. priv_ep->dequeue never equals to current_index. > > > * 2 priv_ep->enqueue never exceed priv_ep->dequeue > > > * 3. exception: priv_ep->enqueue == priv_ep->dequeue > > > * and priv_ep->free_trbs is zero. > > > * This case indicate that TR is full. > > > * > > > - * Then We can split recognition into two parts: > > > + * At below two cases, the request have been handled. > > > * Case 1 - priv_ep->dequeue < current_index > > > * SR ... EQ ... DQ ... CI ... ER > > > * SR ... DQ ... CI ... EQ ... ER > > > * > > > - * Request has been handled by DMA if ST and ET is between DQ > and CI. > > > - * > > > * Case 2 - priv_ep->dequeue > current_index > > > - * This situation take place when CI go through the LINK TRB at the > > > end of > > > + * This situation takes place when CI go through the LINK TRB at > > > + the end of > > > > not part of $subject > > > > I will make another patch for comment improvement, thanks. > I find I change the function from handle request to handle TRB, so the comments for this function needs to be updated accordingly, it needs to be at the same patch. Peter
diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c index a308a694abc5..6cb44c354f40 100644 --- a/drivers/usb/cdns3/gadget.c +++ b/drivers/usb/cdns3/gadget.c @@ -817,6 +817,8 @@ void cdns3_gadget_giveback(struct cdns3_endpoint *priv_ep, request->length); priv_req->flags &= ~(REQUEST_PENDING | REQUEST_UNALIGNED); + /* All TRBs have finished, clear the counter */ + priv_req->finished_trb = 0; trace_cdns3_gadget_giveback(priv_req); if (priv_dev->dev_ver < DEV_VER_V2) { @@ -1241,6 +1243,7 @@ static int cdns3_ep_run_transfer(struct cdns3_endpoint *priv_ep, trb = priv_req->trb; priv_req->flags |= REQUEST_PENDING; + priv_req->num_of_trb = num_trb; if (sg_iter == 1) trb->control |= cpu_to_le32(TRB_IOC | TRB_ISP); @@ -1362,7 +1365,7 @@ void cdns3_set_hw_configuration(struct cdns3_device *priv_dev) } /** - * cdns3_request_handled - check whether request has been handled by DMA + * cdns3_trb_handled - check whether trb has been handled by DMA * * @priv_ep: extended endpoint object. * @priv_req: request object for checking @@ -1379,32 +1382,28 @@ void cdns3_set_hw_configuration(struct cdns3_device *priv_dev) * ET = priv_req->end_trb - index of last TRB in transfer ring * CI = current_index - index of processed TRB by DMA. * - * As first step, function checks if cycle bit for priv_req->start_trb is - * correct. + * As first step, we check if the TRB between the ST and ET. + * Then, we check if cycle bit for index priv_ep->dequeue + * is correct. * * some rules: - * 1. priv_ep->dequeue never exceed current_index. + * 1. priv_ep->dequeue never equals to current_index. * 2 priv_ep->enqueue never exceed priv_ep->dequeue * 3. exception: priv_ep->enqueue == priv_ep->dequeue * and priv_ep->free_trbs is zero. * This case indicate that TR is full. * - * Then We can split recognition into two parts: + * At below two cases, the request have been handled. * Case 1 - priv_ep->dequeue < current_index * SR ... EQ ... DQ ... CI ... ER * SR ... DQ ... CI ... EQ ... ER * - * Request has been handled by DMA if ST and ET is between DQ and CI. - * * Case 2 - priv_ep->dequeue > current_index - * This situation take place when CI go through the LINK TRB at the end of + * This situation takes place when CI go through the LINK TRB at the end of * transfer ring. * SR ... CI ... EQ ... DQ ... ER - * - * Request has been handled by DMA if ET is less then CI or - * ET is greater or equal DQ. */ -static bool cdns3_request_handled(struct cdns3_endpoint *priv_ep, +static bool cdns3_trb_handled(struct cdns3_endpoint *priv_ep, struct cdns3_request *priv_req) { struct cdns3_device *priv_dev = priv_ep->cdns3_dev; @@ -1416,7 +1415,25 @@ static bool cdns3_request_handled(struct cdns3_endpoint *priv_ep, current_index = cdns3_get_dma_pos(priv_dev, priv_ep); doorbell = !!(readl(&priv_dev->regs->ep_cmd) & EP_CMD_DRDY); - trb = &priv_ep->trb_pool[priv_req->start_trb]; + /* current trb doesn't belong to this request */ + if (priv_req->start_trb < priv_req->end_trb) { + if (priv_ep->dequeue > priv_req->end_trb) + goto finish; + + if (priv_ep->dequeue < priv_req->start_trb) + goto finish; + } + + if ((priv_req->start_trb > priv_req->end_trb) && + (priv_ep->dequeue > priv_req->end_trb) && + (priv_ep->dequeue < priv_req->start_trb)) + goto finish; + + if ((priv_req->start_trb == priv_req->end_trb) && + (priv_ep->dequeue != priv_req->end_trb)) + goto finish; + + trb = &priv_ep->trb_pool[priv_ep->dequeue]; if ((le32_to_cpu(trb->control) & TRB_CYCLE) != priv_ep->ccs) goto finish; @@ -1438,12 +1455,8 @@ static bool cdns3_request_handled(struct cdns3_endpoint *priv_ep, !priv_ep->dequeue) goto finish; - if (priv_req->end_trb >= priv_ep->dequeue && - priv_req->end_trb < current_index) - handled = 1; + handled = 1; } else if (priv_ep->dequeue > current_index) { - if (priv_req->end_trb < current_index || - priv_req->end_trb >= priv_ep->dequeue) handled = 1; } @@ -1459,6 +1472,8 @@ static void cdns3_transfer_completed(struct cdns3_device *priv_dev, struct cdns3_request *priv_req; struct usb_request *request; struct cdns3_trb *trb; + bool request_handled = false; + bool transfer_end = false; while (!list_empty(&priv_ep->pending_req_list)) { request = cdns3_next_request(&priv_ep->pending_req_list); @@ -1478,20 +1493,32 @@ static void cdns3_transfer_completed(struct cdns3_device *priv_dev, */ cdns3_select_ep(priv_dev, priv_ep->endpoint.address); - if (!cdns3_request_handled(priv_ep, priv_req)) - goto prepare_next_td; + while (cdns3_trb_handled(priv_ep, priv_req)) { + priv_req->finished_trb++; + if (priv_req->finished_trb >= priv_req->num_of_trb) + request_handled = true; - trb = priv_ep->trb_pool + priv_ep->dequeue; - trace_cdns3_complete_trb(priv_ep, trb); + trb = priv_ep->trb_pool + priv_ep->dequeue; + trace_cdns3_complete_trb(priv_ep, trb); - if (trb != priv_req->trb) - dev_warn(priv_dev->dev, - "request_trb=0x%p, queue_trb=0x%p\n", - priv_req->trb, trb); + if (!transfer_end) + request->actual += + TRB_LEN(le32_to_cpu(trb->length)); - request->actual = TRB_LEN(le32_to_cpu(trb->length)); - cdns3_move_deq_to_next_trb(priv_req); - cdns3_gadget_giveback(priv_ep, priv_req, 0); + if (priv_req->num_of_trb > 1 && + le32_to_cpu(trb->control) & TRB_SMM) + transfer_end = true; + + cdns3_ep_inc_deq(priv_ep); + } + + if (request_handled) { + cdns3_gadget_giveback(priv_ep, priv_req, 0); + request_handled = false; + transfer_end = false; + } else { + goto prepare_next_td; + } if (priv_ep->type != USB_ENDPOINT_XFER_ISOC && TRBS_PER_SEGMENT == 2) diff --git a/drivers/usb/cdns3/gadget.h b/drivers/usb/cdns3/gadget.h index 52765b098b9e..9f8bd452847e 100644 --- a/drivers/usb/cdns3/gadget.h +++ b/drivers/usb/cdns3/gadget.h @@ -1030,6 +1030,11 @@ struct cdns3_trb { * When set to '1', the device will toggle its interpretation of the Cycle bit */ #define TRB_TOGGLE BIT(1) +/* + * The controller will set it if OUTSMM (OUT size mismatch) is detected, + * this bit is for normal TRB + */ +#define TRB_SMM BIT(1) /* * Short Packet (SP). OUT EPs at DMULT=1 only. Indicates if the TRB was @@ -1215,6 +1220,8 @@ struct cdns3_aligned_buf { * this endpoint * @flags: flag specifying special usage of request * @list: used by internally allocated request to add to wa2_descmiss_req_list. + * @finished_trb: number of trb has already finished per request + * @num_of_trb: how many trbs in this request */ struct cdns3_request { struct usb_request request; @@ -1230,6 +1237,8 @@ struct cdns3_request { #define REQUEST_UNALIGNED BIT(4) u32 flags; struct list_head list; + int finished_trb; + int num_of_trb; }; #define to_cdns3_request(r) (container_of(r, struct cdns3_request, request))
- Judge each TRB has been handled at cdns3_trb_handled, since the DMA pointer may be at the middle of the TD, we can't consider this TD has finished at that time. - Calcuate req->actual according to finished TRBs. - Handle short transfer for sg list use case correctly. When the short transfer occurs, we check OUT_SMM at TRB to see if it is the last TRB. Signed-off-by: Peter Chen <peter.chen@nxp.com> --- drivers/usb/cdns3/gadget.c | 85 +++++++++++++++++++++++++------------- drivers/usb/cdns3/gadget.h | 9 ++++ 2 files changed, 65 insertions(+), 29 deletions(-)