On Fri, May 25, 2012 at 06:18:40PM +0530, Pratyush Anand wrote:
> If an IN transfer is missed on isoc endpoint, then driver must insure
> that next ep_queue is properly handled.
> This patch fixes this issue by starting a new transfer for next queued
> request.
>
> Signed-off-by: Pratyush Anand <pratyush.anand@xxxxxx>
> Signed-off-by: Felip Balbi <balbi@xxxxxx>
> ---
> drivers/usb/dwc3/core.h | 3 ++
> drivers/usb/dwc3/gadget.c | 72 ++++++++++++++++++++++++++++++---------------
> 2 files changed, 51 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index c51cece..1210dae 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -352,6 +352,7 @@ struct dwc3_event_buffer {
> * @number: endpoint number (1 - 15)
> * @type: set to bmAttributes & USB_ENDPOINT_XFERTYPE_MASK
> * @res_trans_idx: Resource transfer index
> + * @current_uf: Current uf received through last event parameter
> * @interval: the intervall on which the ISOC transfer is started
> * @name: a human readable name e.g. ep1out-bulk
> * @direction: true for TX, false for RX
> @@ -376,6 +377,7 @@ struct dwc3_ep {
> #define DWC3_EP_WEDGE (1 << 2)
> #define DWC3_EP_BUSY (1 << 4)
> #define DWC3_EP_PENDING_REQUEST (1 << 5)
> +#define DWC3_EP_MISSED_ISOC (1 << 6)
>
> /* This last one is specific to EP0 */
> #define DWC3_EP0_DIR_IN (1 << 31)
> @@ -385,6 +387,7 @@ struct dwc3_ep {
> u8 number;
> u8 type;
> u8 res_trans_idx;
> + u16 current_uf;
> u32 interval;
>
> char name[20];
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 4388d4f..9751b6b 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -991,6 +991,34 @@ static int __dwc3_gadget_kick_transfer(struct dwc3_ep *dep, u16 cmd_param,
> return 0;
> }
>
> +static void dwc3_gadget_start_isoc_uf(struct dwc3 *dwc,
> + struct dwc3_ep *dep, u32 cur_uf)
> +{
can you please keep consistency when re-factoring ? All functions when
are re-factored get a somewhat standard name. This one would become:
__dwc3_gadget_start_isoc(struct dwc3 *dwc, struct dwc3_ep *dep,
u32 current_uf);
> + u32 uf;
> +
> + if (list_empty(&dep->request_list)) {
> + dev_vdbg(dwc->dev, "ISOC ep %s run out for requests.\n",
> + dep->name);
> + return;
> + }
> +
> + /* 4 micro frames in the future */
> + uf = cur_uf + dep->interval * 4;
> +
> + __dwc3_gadget_kick_transfer(dep, uf, 1);
> +}
> +
> +static void dwc3_gadget_start_isoc(struct dwc3 *dwc,
> + struct dwc3_ep *dep, const struct dwc3_event_depevt *event)
> +{
> + u32 cur_uf, mask;
> +
> + mask = ~(dep->interval - 1);
> + cur_uf = event->parameters & mask;
> +
> + dwc3_gadget_start_isoc_uf(dwc, dep, cur_uf);
> +}
> +
> static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep, struct dwc3_request *req)
> {
> struct dwc3 *dwc = dep->dwc;
> @@ -1020,8 +1048,14 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep, struct dwc3_request *req)
>
> list_add_tail(&req->list, &dep->request_list);
>
> - if (usb_endpoint_xfer_isoc(dep->desc) && (dep->flags & DWC3_EP_BUSY))
> - dep->flags |= DWC3_EP_PENDING_REQUEST;
> + if (usb_endpoint_xfer_isoc(dep->desc)) {
> + if (dep->flags & DWC3_EP_BUSY) {
> + dep->flags |= DWC3_EP_PENDING_REQUEST;
> + } else if (dep->flags & DWC3_EP_MISSED_ISOC) {
> + dwc3_gadget_start_isoc_uf(dwc, dep, dep->current_uf);
> + dep->flags &= ~DWC3_EP_MISSED_ISOC;
> + }
> + }
>
> /*
> * There are two special cases:
> @@ -1611,9 +1645,18 @@ static int dwc3_cleanup_done_reqs(struct dwc3 *dwc, struct dwc3_ep *dep,
>
> if (dep->direction) {
> if (count) {
> - dev_err(dwc->dev, "incomplete IN transfer %s\n",
> - dep->name);
> - status = -ECONNRESET;
> + if (DWC3_TRB_SIZE_TRBSTS(trb->size) ==
now I'm going to nitpick, sorry... This would look a bit nicer if you
define a variable to receive the trb status. Something like:
unsigned trb_status = DWC3_TRB_SIZE_TRBSTS(trb->size);
if (trb_status == DWC3_TRBSTS_MISSED_ISOC)
apart from these two comments, patch looks ready.
--
balbi
Attachment:
signature.asc
Description: Digital signature
