Can you please separate the refactoring, print removal, and adding new
features into different patches?
I would actually like to see the refactoring process in several smaller
patches. E.g. here is a patch to move the control transfer processing
into a separate function, here is another patch to move the bulk
processing into a separate function, here is a third patch to remove
redundant printing messages. That way reviewers can easily see whether
the refactoring looks correct. With this giant patch, I can't easily
tell what you did. (And it will definitely take me longer to review.)
Sarah Sharp
On Wed, Jun 23, 2010 at 04:58:43PM +0800, root wrote:
> >From 7e91fbdef63597cb50c4c788b6ebc0fb0357c8c9 Mon Sep 17 00:00:00 2001
> From: Andiry Xu <andiry.xu@xxxxxxx>
> Date: Tue, 22 Jun 2010 13:35:41 +0800
> Subject: [PATCH 1/4] xHCI: handle_tx_event() refine
>
> This patch break handle_tx_event() into smaller functions to increase
> readability. It also removes redundant print messages in the interrupt
> context.
>
> This patch adds mechanism to process Missed Service Error Event.
> Sometimes the xHC is unable to process the isoc TDs in time, it will
> generate Missed Service Error Event. In this case some TDs on the ring are
> not processed and missed. When encounter a Missed Servce Error Event, set
> the skip flag of the ep, and process the missed TDs until reach the next
> processed TD, then clear the skip flag.
>
> Signed-off-by: Andiry Xu <andiry.xu@xxxxxxx>
> ---
> drivers/usb/host/xhci-mem.c | 1 +
> drivers/usb/host/xhci-ring.c | 732 +++++++++++++++++++++++++-----------------
> drivers/usb/host/xhci.h | 8 +
> 3 files changed, 452 insertions(+), 289 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index fd9e03a..522ea49 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -1102,6 +1102,7 @@ int xhci_endpoint_init(struct xhci_hcd *xhci,
> virt_dev->num_rings_cached--;
> xhci_reinit_cached_ring(xhci, virt_dev->eps[ep_index].new_ring);
> }
> + virt_dev->eps[ep_index].skip = false;
> ep_ring = virt_dev->eps[ep_index].new_ring;
> ep_ctx->deq = ep_ring->first_seg->dma | ep_ring->cycle_state;
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index a2064a0..6a55e24 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -1213,6 +1213,319 @@ int xhci_is_vendor_info_code(struct xhci_hcd *xhci, unsigned int trb_comp_code)
> }
>
> /*
> + * Finish the td procession, remove the td from td list;
> + * Return 1 if the urb can be given back.
> + */
> +static int finish_td(struct xhci_hcd *xhci, struct xhci_td *td,
> + union xhci_trb *event_trb, struct xhci_transfer_event *event,
> + struct xhci_virt_ep *ep, int status, bool skip)
> +{
> + struct xhci_virt_device *xdev;
> + struct xhci_ring *ep_ring;
> + unsigned int slot_id;
> + int ep_index;
> + struct urb *urb = NULL;
> + struct xhci_ep_ctx *ep_ctx;
> + int ret = 0;
> + u32 trb_comp_code;
> +
> + slot_id = TRB_TO_SLOT_ID(event->flags);
> + xdev = xhci->devs[slot_id];
> + ep_index = TRB_TO_EP_ID(event->flags) - 1;
> + ep_ring = xhci_dma_to_transfer_ring(ep, event->buffer);
> + ep_ctx = xhci_get_ep_ctx(xhci, xdev->out_ctx, ep_index);
> + trb_comp_code = GET_COMP_CODE(event->transfer_len);
> +
> + if (skip)
> + goto td_cleanup;
> +
> + if (trb_comp_code == COMP_STOP_INVAL ||
> + trb_comp_code == COMP_STOP) {
> + /* The Endpoint Stop Command completion will take care of any
> + * stopped TDs. A stopped TD may be restarted, so don't update
> + * the ring dequeue pointer or take this TD off any lists yet.
> + */
> + ep->stopped_td = td;
> + ep->stopped_trb = event_trb;
> + return 0;
> + } else {
> + if (trb_comp_code == COMP_STALL) {
> + /* The transfer is completed from the driver's
> + * perspective, but we need to issue a set dequeue
> + * command for this stalled endpoint to move the dequeue
> + * pointer past the TD. We can't do that here because
> + * the halt condition must be cleared first. Let the
> + * USB class driver clear the stall later.
> + */
> + ep->stopped_td = td;
> + ep->stopped_trb = event_trb;
> + ep->stopped_stream = ep_ring->stream_id;
> + } else if (xhci_requires_manual_halt_cleanup(xhci,
> + ep_ctx, trb_comp_code)) {
> + /* Other types of errors halt the endpoint, but the
> + * class driver doesn't call usb_reset_endpoint() unless
> + * the error is -EPIPE. Clear the halted status in the
> + * xHCI hardware manually.
> + */
> + xhci_cleanup_halted_endpoint(xhci,
> + slot_id, ep_index, ep_ring->stream_id,
> + td, event_trb);
> + } else {
> + /* Update ring dequeue pointer */
> + while (ep_ring->dequeue != td->last_trb)
> + inc_deq(xhci, ep_ring, false);
> + inc_deq(xhci, ep_ring, false);
> + }
> +
> +td_cleanup:
> + /* Clean up the endpoint's TD list */
> + urb = td->urb;
> +
> + /* Do one last check of the actual transfer length.
> + * If the host controller said we transferred more data than
> + * the buffer length, urb->actual_length will be a very big
> + * number (since it's unsigned). Play it safe and say we didn't
> + * transfer anything.
> + */
> + if (urb->actual_length > urb->transfer_buffer_length) {
> + xhci_warn(xhci, "URB transfer length is wrong, "
> + "xHC issue? req. len = %u, "
> + "act. len = %u\n",
> + urb->transfer_buffer_length,
> + urb->actual_length);
> + urb->actual_length = 0;
> + if (td->urb->transfer_flags & URB_SHORT_NOT_OK)
> + status = -EREMOTEIO;
> + else
> + status = 0;
> + }
> + list_del(&td->td_list);
> + /* Was this TD slated to be cancelled but completed anyway? */
> + if (!list_empty(&td->cancelled_td_list))
> + list_del(&td->cancelled_td_list);
> +
> + urb->status = status;
> + ret = 1;
> + }
> +
> + return ret;
> +}
> +
> +/*
> + * Process control tds, update urb status and actual_length.
> + */
> +static int process_ctrl_td(struct xhci_hcd *xhci, struct xhci_td *td,
> + union xhci_trb *event_trb, struct xhci_transfer_event *event,
> + struct xhci_virt_ep *ep, int status)
> +{
> + struct xhci_virt_device *xdev;
> + struct xhci_ring *ep_ring;
> + unsigned int slot_id;
> + int ep_index;
> + struct xhci_ep_ctx *ep_ctx;
> + u32 trb_comp_code;
> +
> + slot_id = TRB_TO_SLOT_ID(event->flags);
> + xdev = xhci->devs[slot_id];
> + ep_index = TRB_TO_EP_ID(event->flags) - 1;
> + ep_ring = xhci_dma_to_transfer_ring(ep, event->buffer);
> + ep_ctx = xhci_get_ep_ctx(xhci, xdev->out_ctx, ep_index);
> + trb_comp_code = GET_COMP_CODE(event->transfer_len);
> +
> + xhci_debug_trb(xhci, xhci->event_ring->dequeue);
> + switch (trb_comp_code) {
> + case COMP_SUCCESS:
> + if (event_trb == ep_ring->dequeue) {
> + xhci_warn(xhci, "WARN: Success on ctrl setup TRB "
> + "without IOC set??\n");
> + status = -ESHUTDOWN;
> + } else if (event_trb != td->last_trb) {
> + xhci_warn(xhci, "WARN: Success on ctrl data TRB "
> + "without IOC set??\n");
> + status = -ESHUTDOWN;
> + } else {
> + xhci_dbg(xhci, "Successful control transfer!\n");
> + status = 0;
> + }
> + break;
> + case COMP_SHORT_TX:
> + xhci_warn(xhci, "WARN: short transfer on control ep\n");
> + if (td->urb->transfer_flags & URB_SHORT_NOT_OK)
> + status = -EREMOTEIO;
> + else
> + status = 0;
> + break;
> + default:
> + if (!xhci_requires_manual_halt_cleanup(xhci,
> + ep_ctx, trb_comp_code))
> + break;
> + xhci_dbg(xhci, "TRB error code %u, "
> + "halted endpoint index = %u\n",
> + trb_comp_code, ep_index);
> + /* else fall through */
> + case COMP_STALL:
> + /* Did we transfer part of the data (middle) phase? */
> + if (event_trb != ep_ring->dequeue &&
> + event_trb != td->last_trb)
> + td->urb->actual_length =
> + td->urb->transfer_buffer_length
> + - TRB_LEN(event->transfer_len);
> + else
> + td->urb->actual_length = 0;
> + xhci_cleanup_halted_endpoint(xhci,
> + slot_id, ep_index, 0, td, event_trb);
> + return finish_td(xhci, td, event_trb, event, ep, status, true);
> + }
> + /*
> + * Did we transfer any data, despite the errors that might have
> + * happened? I.e. did we get past the setup stage?
> + */
> + if (event_trb != ep_ring->dequeue) {
> + /* The event was for the status stage */
> + if (event_trb == td->last_trb) {
> + if (td->urb->actual_length != 0) {
> + /* Don't overwrite a previously set error code
> + */
> + if ((status == -EINPROGRESS || status == 0) &&
> + (td->urb->transfer_flags
> + & URB_SHORT_NOT_OK))
> + /* Did we already see a short data
> + * stage? */
> + status = -EREMOTEIO;
> + } else {
> + td->urb->actual_length =
> + td->urb->transfer_buffer_length;
> + }
> + } else {
> + /* Maybe the event was for the data stage? */
> + if (trb_comp_code != COMP_STOP_INVAL) {
> + /* We didn't stop on a link TRB in the middle */
> + td->urb->actual_length =
> + td->urb->transfer_buffer_length -
> + TRB_LEN(event->transfer_len);
> + xhci_dbg(xhci, "Waiting for status "
> + "stage event\n");
> + return 0;
> + }
> + }
> + }
> +
> + return finish_td(xhci, td, event_trb, event, ep, status, false);
> +}
> +
> +/*
> + * Process bulk and interrupt tds, update urb status and actual_length.
> + */
> +static int process_bulk_intr_td(struct xhci_hcd *xhci, struct xhci_td *td,
> + union xhci_trb *event_trb, struct xhci_transfer_event *event,
> + struct xhci_virt_ep *ep, int status)
> +{
> + struct xhci_ring *ep_ring;
> + union xhci_trb *cur_trb;
> + struct xhci_segment *cur_seg;
> + u32 trb_comp_code;
> +
> + ep_ring = xhci_dma_to_transfer_ring(ep, event->buffer);
> + trb_comp_code = GET_COMP_CODE(event->transfer_len);
> +
> + switch (trb_comp_code) {
> + case COMP_SUCCESS:
> + /* Double check that the HW transferred everything. */
> + if (event_trb != td->last_trb) {
> + xhci_warn(xhci, "WARN Successful completion "
> + "on short TX\n");
> + if (td->urb->transfer_flags & URB_SHORT_NOT_OK)
> + status = -EREMOTEIO;
> + else
> + status = 0;
> + } else {
> + if (usb_endpoint_xfer_bulk(&td->urb->ep->desc))
> + xhci_dbg(xhci, "Successful bulk "
> + "transfer!\n");
> + else
> + xhci_dbg(xhci, "Successful interrupt "
> + "transfer!\n");
> + status = 0;
> + }
> + break;
> + case COMP_SHORT_TX:
> + if (td->urb->transfer_flags & URB_SHORT_NOT_OK)
> + status = -EREMOTEIO;
> + else
> + status = 0;
> + break;
> + default:
> + /* Others already handled above */
> + break;
> + }
> + dev_dbg(&td->urb->dev->dev,
> + "ep %#x - asked for %d bytes, "
> + "%d bytes untransferred\n",
> + td->urb->ep->desc.bEndpointAddress,
> + td->urb->transfer_buffer_length,
> + TRB_LEN(event->transfer_len));
> + /* Fast path - was this the last TRB in the TD for this URB? */
> + if (event_trb == td->last_trb) {
> + if (TRB_LEN(event->transfer_len) != 0) {
> + td->urb->actual_length =
> + td->urb->transfer_buffer_length -
> + TRB_LEN(event->transfer_len);
> + if (td->urb->transfer_buffer_length <
> + td->urb->actual_length) {
> + xhci_warn(xhci, "HC gave bad length "
> + "of %d bytes left\n",
> + TRB_LEN(event->transfer_len));
> + td->urb->actual_length = 0;
> + if (td->urb->transfer_flags &
> + URB_SHORT_NOT_OK)
> + status = -EREMOTEIO;
> + else
> + status = 0;
> + }
> + /* Don't overwrite a previously set error code */
> + if (status == -EINPROGRESS) {
> + if (td->urb->transfer_flags & URB_SHORT_NOT_OK)
> + status = -EREMOTEIO;
> + else
> + status = 0;
> + }
> + } else {
> + td->urb->actual_length =
> + td->urb->transfer_buffer_length;
> + /* Ignore a short packet completion if the
> + * untransferred length was zero.
> + */
> + if (status == -EREMOTEIO)
> + status = 0;
> + }
> + } else {
> + /* Slow path - walk the list, starting from the dequeue
> + * pointer, to get the actual length transferred.
> + */
> + td->urb->actual_length = 0;
> + for (cur_trb = ep_ring->dequeue, cur_seg = ep_ring->deq_seg;
> + cur_trb != event_trb;
> + next_trb(xhci, ep_ring, &cur_seg, &cur_trb)) {
> + if ((cur_trb->generic.field[3] &
> + TRB_TYPE_BITMASK) != TRB_TYPE(TRB_TR_NOOP) &&
> + (cur_trb->generic.field[3] &
> + TRB_TYPE_BITMASK) != TRB_TYPE(TRB_LINK))
> + td->urb->actual_length +=
> + TRB_LEN(cur_trb->generic.field[2]);
> + }
> + /* If the ring didn't stop on a Link or No-op TRB, add
> + * in the actual bytes transferred from the Normal TRB
> + */
> + if (trb_comp_code != COMP_STOP_INVAL)
> + td->urb->actual_length +=
> + TRB_LEN(cur_trb->generic.field[2]) -
> + TRB_LEN(event->transfer_len);
> + }
> +
> + return finish_td(xhci, td, event_trb, event, ep, status, false);
> +}
> +
> +/*
> * If this function returns an error condition, it means it got a Transfer
> * event with a corrupted Slot ID, Endpoint ID, or TRB DMA address.
> * At this point, the host controller is probably hosed and should be reset.
> @@ -1233,8 +1546,8 @@ static int handle_tx_event(struct xhci_hcd *xhci,
> int status = -EINPROGRESS;
> struct xhci_ep_ctx *ep_ctx;
> u32 trb_comp_code;
> + int ret = 0;
>
> - xhci_dbg(xhci, "In %s\n", __func__);
> slot_id = TRB_TO_SLOT_ID(event->flags);
> xdev = xhci->devs[slot_id];
> if (!xdev) {
> @@ -1248,51 +1561,16 @@ static int handle_tx_event(struct xhci_hcd *xhci,
> ep = &xdev->eps[ep_index];
> ep_ring = xhci_dma_to_transfer_ring(ep, event->buffer);
> ep_ctx = xhci_get_ep_ctx(xhci, xdev->out_ctx, ep_index);
> - if (!ep_ring || (ep_ctx->ep_info & EP_STATE_MASK) == EP_STATE_DISABLED) {
> + if (!ep_ring ||
> + (ep_ctx->ep_info & EP_STATE_MASK) == EP_STATE_DISABLED) {
> xhci_err(xhci, "ERROR Transfer event for disabled endpoint "
> "or incorrect stream ring\n");
> return -ENODEV;
> }
>
> event_dma = event->buffer;
> - /* This TRB should be in the TD at the head of this ring's TD list */
> - xhci_dbg(xhci, "%s - checking for list empty\n", __func__);
> - if (list_empty(&ep_ring->td_list)) {
> - xhci_warn(xhci, "WARN Event TRB for slot %d ep %d with no TDs queued?\n",
> - TRB_TO_SLOT_ID(event->flags), ep_index);
> - xhci_dbg(xhci, "Event TRB with TRB type ID %u\n",
> - (unsigned int) (event->flags & TRB_TYPE_BITMASK)>>10);
> - xhci_print_trb_offsets(xhci, (union xhci_trb *) event);
> - urb = NULL;
> - goto cleanup;
> - }
> - xhci_dbg(xhci, "%s - getting list entry\n", __func__);
> - td = list_entry(ep_ring->td_list.next, struct xhci_td, td_list);
> -
> - /* Is this a TRB in the currently executing TD? */
> - xhci_dbg(xhci, "%s - looking for TD\n", __func__);
> - event_seg = trb_in_td(ep_ring->deq_seg, ep_ring->dequeue,
> - td->last_trb, event_dma);
> - xhci_dbg(xhci, "%s - found event_seg = %p\n", __func__, event_seg);
> - if (!event_seg) {
> - /* HC is busted, give up! */
> - xhci_err(xhci, "ERROR Transfer event TRB DMA ptr not part of current TD\n");
> - return -ESHUTDOWN;
> - }
> - event_trb = &event_seg->trbs[(event_dma - event_seg->dma) / sizeof(*event_trb)];
> - xhci_dbg(xhci, "Event TRB with TRB type ID %u\n",
> - (unsigned int) (event->flags & TRB_TYPE_BITMASK)>>10);
> - xhci_dbg(xhci, "Offset 0x00 (buffer lo) = 0x%x\n",
> - lower_32_bits(event->buffer));
> - xhci_dbg(xhci, "Offset 0x04 (buffer hi) = 0x%x\n",
> - upper_32_bits(event->buffer));
> - xhci_dbg(xhci, "Offset 0x08 (transfer length) = 0x%x\n",
> - (unsigned int) event->transfer_len);
> - xhci_dbg(xhci, "Offset 0x0C (flags) = 0x%x\n",
> - (unsigned int) event->flags);
> -
> - /* Look for common error cases */
> trb_comp_code = GET_COMP_CODE(event->transfer_len);
> + /* Look for common error cases */
> switch (trb_comp_code) {
> /* Skip codes that require special handling depending on
> * transfer type
> @@ -1328,278 +1606,154 @@ static int handle_tx_event(struct xhci_hcd *xhci,
> xhci_warn(xhci, "WARN: HC couldn't access mem fast enough\n");
> status = -ENOSR;
> break;
> + case COMP_BW_OVER:
> + xhci_warn(xhci, "WARN: bandwidth overrun event on endpoint\n");
> + break;
> + case COMP_BUFF_OVER:
> + xhci_warn(xhci, "WARN: buffer overrun event on endpoint\n");
> + break;
> + case COMP_UNDERRUN:
> + /*
> + * When the Isoch ring is empty, the xHC will generate
> + * a Ring Overrun Event for IN Isoch endpoint or Ring
> + * Underrun Event for OUT Isoch endpoint.
> + */
> + xhci_dbg(xhci, "underrun event on endpoint\n");
> + if (!list_empty(&ep_ring->td_list))
> + xhci_dbg(xhci, "Underrun Event for slot %d ep %d "
> + "still with TDs queued?\n",
> + TRB_TO_SLOT_ID(event->flags), ep_index);
> + goto cleanup;
> + case COMP_OVERRUN:
> + xhci_dbg(xhci, "overrun event on endpoint\n");
> + if (!list_empty(&ep_ring->td_list))
> + xhci_dbg(xhci, "Overrun Event for slot %d ep %d "
> + "still with TDs queued?\n",
> + TRB_TO_SLOT_ID(event->flags), ep_index);
> + goto cleanup;
> + case COMP_MISSED_INT:
> + /*
> + * When encounter missed service error, one or more isoc tds
> + * may be missed by xHC.
> + * Set skip flag of the ep_ring; Complete the missed tds as
> + * short transfer when process the ep_ring next time.
> + */
> + ep->skip = true;
> + xhci_dbg(xhci, "Miss service interval error! Set skip flag\n");
> + goto cleanup;
> default:
> if (xhci_is_vendor_info_code(xhci, trb_comp_code)) {
> status = 0;
> break;
> }
> - xhci_warn(xhci, "ERROR Unknown event condition, HC probably busted\n");
> - urb = NULL;
> + xhci_warn(xhci, "ERROR Unknown event condition, HC probably "
> + "busted\n");
> goto cleanup;
> }
> - /* Now update the urb's actual_length and give back to the core */
> - /* Was this a control transfer? */
> - if (usb_endpoint_xfer_control(&td->urb->ep->desc)) {
> - xhci_debug_trb(xhci, xhci->event_ring->dequeue);
> - switch (trb_comp_code) {
> - case COMP_SUCCESS:
> - if (event_trb == ep_ring->dequeue) {
> - xhci_warn(xhci, "WARN: Success on ctrl setup TRB without IOC set??\n");
> - status = -ESHUTDOWN;
> - } else if (event_trb != td->last_trb) {
> - xhci_warn(xhci, "WARN: Success on ctrl data TRB without IOC set??\n");
> - status = -ESHUTDOWN;
> - } else {
> - xhci_dbg(xhci, "Successful control transfer!\n");
> - status = 0;
> - }
> - break;
> - case COMP_SHORT_TX:
> - xhci_warn(xhci, "WARN: short transfer on control ep\n");
> - if (td->urb->transfer_flags & URB_SHORT_NOT_OK)
> - status = -EREMOTEIO;
> - else
> - status = 0;
> - break;
> -
> - default:
> - if (!xhci_requires_manual_halt_cleanup(xhci,
> - ep_ctx, trb_comp_code))
> - break;
> - xhci_dbg(xhci, "TRB error code %u, "
> - "halted endpoint index = %u\n",
> - trb_comp_code, ep_index);
> - /* else fall through */
> - case COMP_STALL:
> - /* Did we transfer part of the data (middle) phase? */
> - if (event_trb != ep_ring->dequeue &&
> - event_trb != td->last_trb)
> - td->urb->actual_length =
> - td->urb->transfer_buffer_length
> - - TRB_LEN(event->transfer_len);
> - else
> - td->urb->actual_length = 0;
>
> - xhci_cleanup_halted_endpoint(xhci,
> - slot_id, ep_index, 0, td, event_trb);
> - goto td_cleanup;
> - }
> - /*
> - * Did we transfer any data, despite the errors that might have
> - * happened? I.e. did we get past the setup stage?
> + do {
> + /* This TRB should be in the TD at the head of this ring's
> + * TD list.
> */
> - if (event_trb != ep_ring->dequeue) {
> - /* The event was for the status stage */
> - if (event_trb == td->last_trb) {
> - if (td->urb->actual_length != 0) {
> - /* Don't overwrite a previously set error code */
> - if ((status == -EINPROGRESS ||
> - status == 0) &&
> - (td->urb->transfer_flags
> - & URB_SHORT_NOT_OK))
> - /* Did we already see a short data stage? */
> - status = -EREMOTEIO;
> - } else {
> - td->urb->actual_length =
> - td->urb->transfer_buffer_length;
> - }
> - } else {
> - /* Maybe the event was for the data stage? */
> - if (trb_comp_code != COMP_STOP_INVAL) {
> - /* We didn't stop on a link TRB in the middle */
> - td->urb->actual_length =
> - td->urb->transfer_buffer_length -
> - TRB_LEN(event->transfer_len);
> - xhci_dbg(xhci, "Waiting for status stage event\n");
> - urb = NULL;
> - goto cleanup;
> - }
> + if (list_empty(&ep_ring->td_list)) {
> + xhci_warn(xhci, "WARN Event TRB for slot %d ep %d "
> + "with no TDs queued?\n",
> + TRB_TO_SLOT_ID(event->flags), ep_index);
> + xhci_dbg(xhci, "Event TRB with TRB type ID %u\n",
> + (unsigned int) (event->flags & TRB_TYPE_BITMASK)>>10);
> + xhci_print_trb_offsets(xhci, (union xhci_trb *) event);
> + if (ep->skip) {
> + ep->skip = false;
> + xhci_dbg(xhci, "td_list is empty while skip "
> + "flag set. Clear skip flag.\n");
> }
> + ret = 0;
> + goto cleanup;
> }
> - } else {
> - switch (trb_comp_code) {
> - case COMP_SUCCESS:
> - /* Double check that the HW transferred everything. */
> - if (event_trb != td->last_trb) {
> - xhci_warn(xhci, "WARN Successful completion "
> - "on short TX\n");
> - if (td->urb->transfer_flags & URB_SHORT_NOT_OK)
> - status = -EREMOTEIO;
> - else
> - status = 0;
> - } else {
> - if (usb_endpoint_xfer_bulk(&td->urb->ep->desc))
> - xhci_dbg(xhci, "Successful bulk "
> - "transfer!\n");
> - else
> - xhci_dbg(xhci, "Successful interrupt "
> - "transfer!\n");
> - status = 0;
> - }
> - break;
> - case COMP_SHORT_TX:
> - if (td->urb->transfer_flags & URB_SHORT_NOT_OK)
> - status = -EREMOTEIO;
> - else
> - status = 0;
> - break;
> - default:
> - /* Others already handled above */
> - break;
> +
> + td = list_entry(ep_ring->td_list.next, struct xhci_td, td_list);
> + /* Is this a TRB in the currently executing TD? */
> + event_seg = trb_in_td(ep_ring->deq_seg, ep_ring->dequeue,
> + td->last_trb, event_dma);
> + if (event_seg && ep->skip) {
> + xhci_dbg(xhci, "Found td. Clear skip flag.\n");
> + ep->skip = false;
> + }
> + if (!event_seg &&
> + (!ep->skip || !usb_endpoint_xfer_isoc(&td->urb->ep->desc))) {
> + /* HC is busted, give up! */
> + xhci_err(xhci, "ERROR Transfer event TRB DMA ptr not "
> + "part of current TD\n");
> + return -ESHUTDOWN;
> }
> - dev_dbg(&td->urb->dev->dev,
> - "ep %#x - asked for %d bytes, "
> - "%d bytes untransferred\n",
> - td->urb->ep->desc.bEndpointAddress,
> - td->urb->transfer_buffer_length,
> - TRB_LEN(event->transfer_len));
> - /* Fast path - was this the last TRB in the TD for this URB? */
> - if (event_trb == td->last_trb) {
> - if (TRB_LEN(event->transfer_len) != 0) {
> - td->urb->actual_length =
> - td->urb->transfer_buffer_length -
> - TRB_LEN(event->transfer_len);
> - if (td->urb->transfer_buffer_length <
> - td->urb->actual_length) {
> - xhci_warn(xhci, "HC gave bad length "
> - "of %d bytes left\n",
> - TRB_LEN(event->transfer_len));
> - td->urb->actual_length = 0;
> - if (td->urb->transfer_flags &
> - URB_SHORT_NOT_OK)
> - status = -EREMOTEIO;
> - else
> - status = 0;
> - }
> - /* Don't overwrite a previously set error code */
> - if (status == -EINPROGRESS) {
> - if (td->urb->transfer_flags & URB_SHORT_NOT_OK)
> - status = -EREMOTEIO;
> - else
> - status = 0;
> - }
> - } else {
> - td->urb->actual_length = td->urb->transfer_buffer_length;
> - /* Ignore a short packet completion if the
> - * untransferred length was zero.
> - */
> - if (status == -EREMOTEIO)
> - status = 0;
> - }
> - } else {
> - /* Slow path - walk the list, starting from the dequeue
> - * pointer, to get the actual length transferred.
> - */
> - union xhci_trb *cur_trb;
> - struct xhci_segment *cur_seg;
>
> - td->urb->actual_length = 0;
> - for (cur_trb = ep_ring->dequeue, cur_seg = ep_ring->deq_seg;
> - cur_trb != event_trb;
> - next_trb(xhci, ep_ring, &cur_seg, &cur_trb)) {
> - if ((cur_trb->generic.field[3] &
> - TRB_TYPE_BITMASK) != TRB_TYPE(TRB_TR_NOOP) &&
> - (cur_trb->generic.field[3] &
> - TRB_TYPE_BITMASK) != TRB_TYPE(TRB_LINK))
> - td->urb->actual_length +=
> - TRB_LEN(cur_trb->generic.field[2]);
> - }
> - /* If the ring didn't stop on a Link or No-op TRB, add
> - * in the actual bytes transferred from the Normal TRB
> + if (event_seg) {
> + event_trb = &event_seg->trbs[(event_dma -
> + event_seg->dma) / sizeof(*event_trb)];
> + /*
> + * No-op TRB should not trigger interrupts.
> + * If event_trb is a no-op TRB, it means the
> + * corresponding TD has been cancelled. Just ignore
> + * the TD.
> */
> - if (trb_comp_code != COMP_STOP_INVAL)
> - td->urb->actual_length +=
> - TRB_LEN(cur_trb->generic.field[2]) -
> - TRB_LEN(event->transfer_len);
> + if ((event_trb->generic.field[3] & TRB_TYPE_BITMASK)
> + == TRB_TYPE(TRB_TR_NOOP)) {
> + xhci_dbg(xhci, "event_trb is a no-op TRB. "
> + "Skip it\n");
> + goto cleanup;
> + }
> }
> - }
> - if (trb_comp_code == COMP_STOP_INVAL ||
> - trb_comp_code == COMP_STOP) {
> - /* The Endpoint Stop Command completion will take care of any
> - * stopped TDs. A stopped TD may be restarted, so don't update
> - * the ring dequeue pointer or take this TD off any lists yet.
> +
> + /* Now update the urb's actual_length and give back to
> + * the core
> */
> - ep->stopped_td = td;
> - ep->stopped_trb = event_trb;
> - } else {
> - if (trb_comp_code == COMP_STALL) {
> - /* The transfer is completed from the driver's
> - * perspective, but we need to issue a set dequeue
> - * command for this stalled endpoint to move the dequeue
> - * pointer past the TD. We can't do that here because
> - * the halt condition must be cleared first. Let the
> - * USB class driver clear the stall later.
> - */
> - ep->stopped_td = td;
> - ep->stopped_trb = event_trb;
> - ep->stopped_stream = ep_ring->stream_id;
> - } else if (xhci_requires_manual_halt_cleanup(xhci,
> - ep_ctx, trb_comp_code)) {
> - /* Other types of errors halt the endpoint, but the
> - * class driver doesn't call usb_reset_endpoint() unless
> - * the error is -EPIPE. Clear the halted status in the
> - * xHCI hardware manually.
> - */
> - xhci_cleanup_halted_endpoint(xhci,
> - slot_id, ep_index, ep_ring->stream_id, td, event_trb);
> - } else {
> - /* Update ring dequeue pointer */
> - while (ep_ring->dequeue != td->last_trb)
> - inc_deq(xhci, ep_ring, false);
> - inc_deq(xhci, ep_ring, false);
> - }
> + if (usb_endpoint_xfer_control(&td->urb->ep->desc))
> + ret = process_ctrl_td(xhci, td, event_trb, event, ep,
> + status);
> + else
> + ret = process_bulk_intr_td(xhci, td, event_trb, event,
> + ep, status);
>
> -td_cleanup:
> - /* Clean up the endpoint's TD list */
> - urb = td->urb;
> - /* Do one last check of the actual transfer length.
> - * If the host controller said we transferred more data than
> - * the buffer length, urb->actual_length will be a very big
> - * number (since it's unsigned). Play it safe and say we didn't
> - * transfer anything.
> +cleanup:
> + /*
> + * Do not update event ring dequeue pointer if ep->skip is set.
> + * Will roll back to continue process missed tds.
> */
> - if (urb->actual_length > urb->transfer_buffer_length) {
> - xhci_warn(xhci, "URB transfer length is wrong, "
> - "xHC issue? req. len = %u, "
> - "act. len = %u\n",
> - urb->transfer_buffer_length,
> - urb->actual_length);
> - urb->actual_length = 0;
> - if (td->urb->transfer_flags & URB_SHORT_NOT_OK)
> - status = -EREMOTEIO;
> - else
> - status = 0;
> + if (trb_comp_code == COMP_MISSED_INT || !ep->skip) {
> + inc_deq(xhci, xhci->event_ring, true);
> + xhci_set_hc_event_deq(xhci);
> }
> - list_del(&td->td_list);
> - /* Was this TD slated to be cancelled but completed anyway? */
> - if (!list_empty(&td->cancelled_td_list))
> - list_del(&td->cancelled_td_list);
>
> - /* Leave the TD around for the reset endpoint function to use
> - * (but only if it's not a control endpoint, since we already
> - * queued the Set TR dequeue pointer command for stalled
> - * control endpoints).
> - */
> - if (usb_endpoint_xfer_control(&urb->ep->desc) ||
> - (trb_comp_code != COMP_STALL &&
> - trb_comp_code != COMP_BABBLE)) {
> - kfree(td);
> + if (ret) {
> + urb = td->urb;
> + /* Leave the TD around for the reset endpoint function
> + * to use(but only if it's not a control endpoint,
> + * since we already queued the Set TR dequeue pointer
> + * command for stalled control endpoints).
> + */
> + if (usb_endpoint_xfer_control(&urb->ep->desc) ||
> + (trb_comp_code != COMP_STALL &&
> + trb_comp_code != COMP_BABBLE))
> + kfree(td);
> +
> + usb_hcd_unlink_urb_from_ep(xhci_to_hcd(xhci), urb);
> + xhci_dbg(xhci, "Giveback URB %p, len = %d, "
> + "status = %d\n",
> + urb, urb->actual_length, urb->status);
> + spin_unlock(&xhci->lock);
> + usb_hcd_giveback_urb(xhci_to_hcd(xhci), urb,
> + urb->status);
> + spin_lock(&xhci->lock);
> }
> - urb->hcpriv = NULL;
> - }
> -cleanup:
> - inc_deq(xhci, xhci->event_ring, true);
> - xhci_set_hc_event_deq(xhci);
>
> - /* FIXME for multi-TD URBs (who have buffers bigger than 64MB) */
> - if (urb) {
> - usb_hcd_unlink_urb_from_ep(xhci_to_hcd(xhci), urb);
> - xhci_dbg(xhci, "Giveback URB %p, len = %d, status = %d\n",
> - urb, urb->actual_length, status);
> - spin_unlock(&xhci->lock);
> - usb_hcd_giveback_urb(xhci_to_hcd(xhci), urb, status);
> - spin_lock(&xhci->lock);
> - }
> + /*
> + * If ep->skip is set, it means there are missed tds on the
> + * endpoint ring need to take care of.
> + * Process them as short transfer until reach the td pointed by
> + * the event.
> + */
> + } while (ep->skip && trb_comp_code != COMP_MISSED_INT);
> +
> return 0;
> }
>
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index fb3440b..5c63f10 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -720,6 +720,14 @@ struct xhci_virt_ep {
> struct timer_list stop_cmd_timer;
> int stop_cmds_pending;
> struct xhci_hcd *xhci;
> + /*
> + * Sometimes the xHC can not process isochronous endpoint ring quickly
> + * enough, and it will miss some isoc tds on the ring and generate
> + * a Missed Service Error Event.
> + * Set skip flag when receive a Missed Service Error Event and
> + * process the missed tds on the endpoint ring.
> + */
> + bool skip;
> };
>
> struct xhci_virt_device {
> --
> 1.7.0.4
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
