On Tue, Jul 20, 2010 at 04:49:11PM +0800, Andiry Xu wrote:
> >From 08d56cd6a34dff12bc753c6b5022a277ec03ee0b Mon Sep 17 00:00:00 2001
> From: Andiry Xu <andiry.xu@xxxxxxx>
> Date: Thu, 15 Jul 2010 13:33:27 +0800
> Subject: [PATCH 01/10] xHCI: handle_tx_event() refactor: finish_td
>
> This patch moves the td universal processing part in handle_tx_event()
> into a separate function finish_td().
>
> if finish_td() returns 1, it indicates the urb can be given back.
>
> Signed-off-by: Andiry Xu <andiry.xu@xxxxxxx>
The patchset (with the updated patch #8) passes my testing. Thanks
Andiry!
Signed-off-by: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>
> ---
> drivers/usb/host/xhci-ring.c | 185 +++++++++++++++++++++++++-----------------
> 1 files changed, 112 insertions(+), 73 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index bfc99a9..691a108 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -1258,6 +1258,104 @@ int xhci_is_vendor_info_code(struct xhci_hcd *xhci, unsigned int trb_comp_code)
> }
>
> /*
> + * Finish the td processing, 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);
> +
> + ret = 1;
> + }
> +
> + return ret;
> +}
> +
> +/*
> * 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.
> @@ -1278,6 +1376,7 @@ 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);
> @@ -1308,7 +1407,6 @@ static int handle_tx_event(struct xhci_hcd *xhci,
> 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__);
> @@ -1379,7 +1477,6 @@ static int handle_tx_event(struct xhci_hcd *xhci,
> break;
> }
> xhci_warn(xhci, "ERROR Unknown event condition, HC probably busted\n");
> - urb = NULL;
> goto cleanup;
> }
> /* Now update the urb's actual_length and give back to the core */
> @@ -1427,7 +1524,10 @@ static int handle_tx_event(struct xhci_hcd *xhci,
>
> xhci_cleanup_halted_endpoint(xhci,
> slot_id, ep_index, 0, td, event_trb);
> - goto td_cleanup;
> +
> + ret = finish_td(xhci, td, event_trb, event, ep,
> + &status, true);
> + goto cleanup;
> }
> /*
> * Did we transfer any data, despite the errors that might have
> @@ -1456,7 +1556,6 @@ static int handle_tx_event(struct xhci_hcd *xhci,
> td->urb->transfer_buffer_length -
> TRB_LEN(event->transfer_len);
> xhci_dbg(xhci, "Waiting for status stage event\n");
> - urb = NULL;
> goto cleanup;
> }
> }
> @@ -1558,68 +1657,16 @@ static int handle_tx_event(struct xhci_hcd *xhci,
> TRB_LEN(event->transfer_len);
> }
> }
> - 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;
> - } 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);
> + ret = finish_td(xhci, td, event_trb, event, ep, &status, false);
>
> +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 (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
> @@ -1627,17 +1674,9 @@ td_cleanup:
> */
> if (usb_endpoint_xfer_control(&urb->ep->desc) ||
> (trb_comp_code != COMP_STALL &&
> - trb_comp_code != COMP_BABBLE)) {
> + trb_comp_code != COMP_BABBLE))
> kfree(td);
> - }
> - 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);
> --
> 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
