On Tue, Mar 02, 2010 at 06:28:45PM +0800, Libin Yang wrote:
> >From d598ff50c8908bd62c1871ca9bfe68d952e0a949 Mon Sep 17 00:00:00 2001
> From: Andiry Xu <andiry.xu@xxxxxxx>
> Date: Fri, 26 Feb 2010 17:34:47 +0800
> Subject: [PATCH 1/7] xHCI: Introduce urb_priv structure
>
> Introduce urb_priv data structure to allow one URB contains multiple TDs.
> This structure is necessary for isoc transfers, because xHCI can only
> transfer one isoc TD in a interval, and one isoc URB needs to be split
> into multiple isoc TDs.
>
> Some codes are borrowed from OHCI driver.
Hi Andiry,
Thanks for submitting these patches! I haven't had a chance to test
them yet because I've been reviewing the patchset.
I like that you broke the patchset up into functional changes, but can
you think about how you would reorder them such that if someone picked a
random patch to compile and run on (say #2) that they would only be
allowed to enqueue an isochronous URB if all the xHCI functionality is
there? This is for the people that have to run git-bisect. I think if
you move the modification of xhci_urb_enqueue() to accept isoc URB into
a separate patch at the end, that will be enough.
I'll comment on each patch separately.
Sarah Sharp
> Signed-off-by: Andiry Xu <andiry.xu@xxxxxxx>
> Signed-off-by: Crane Cai <crane.cai@xxxxxxx>
> Signed-off-by: Libin Yang <libin.yang@xxxxxxx>
> ---
> drivers/usb/host/xhci-hcd.c | 58 ++++++++++++++++++++++++++++++---
> drivers/usb/host/xhci-mem.c | 17 ++++++++++
> drivers/usb/host/xhci-ring.c | 71 +++++++++++++++++++++++++-----------------
> drivers/usb/host/xhci.h | 10 ++++++
> 4 files changed, 121 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-hcd.c b/drivers/usb/host/xhci-hcd.c
> index 4cb69e0..36b7fa1 100644
> --- a/drivers/usb/host/xhci-hcd.c
> +++ b/drivers/usb/host/xhci-hcd.c
> @@ -681,7 +681,8 @@ int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flags)
> unsigned long flags;
> int ret = 0;
> unsigned int slot_id, ep_index;
> -
> + struct urb_priv *urb_priv;
> + int size, i;
>
> if (!urb || xhci_check_args(hcd, urb->dev, urb->ep, true, __func__) <= 0)
> return -EINVAL;
> @@ -701,6 +702,37 @@ int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flags)
> ret = -ESHUTDOWN;
> goto exit;
> }
> +
> + if (usb_endpoint_xfer_isoc(&urb->ep->desc))
> + size = urb->number_of_packets;
> + else
> + size = 1;
> +
> + urb_priv = kzalloc(sizeof(struct urb_priv) +
> + size * sizeof(struct xhci_td *), mem_flags);
> + if (!urb_priv)
> + return -ENOMEM;
> +
> + for (i = 0; i < size; i++) {
> + urb_priv->td[i] = kzalloc(sizeof(struct xhci_td), mem_flags);
> + if (!urb_priv->td[i]) {
> + urb_priv->length = i;
> + urb_free_priv(xhci, urb_priv);
> + return -ENOMEM;
> + }
> + }
> +
> + spin_lock_irqsave(&xhci->lock, flags);
> + ret = usb_hcd_link_urb_to_ep(hcd, urb);
> + spin_unlock_irqrestore(&xhci->lock, flags);
> +
> + if (unlikely(ret)) {
> + urb_free_priv(xhci, urb_priv);
> + goto exit;
> + }
> + urb_priv->length = size;
> + urb->hcpriv = urb_priv;
> +
> if (usb_endpoint_xfer_control(&urb->ep->desc)) {
> /* Check to see if the max packet size for the default control
> * endpoint changed during FS device enumeration
> @@ -741,6 +773,7 @@ int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flags)
> exit:
> return ret;
> dying:
> + urb_free_priv(xhci, urb_priv);
> xhci_dbg(xhci, "Ep 0x%x: URB %p submitted for "
> "non-responsive xHCI host.\n",
> urb->ep->desc.bEndpointAddress, urb);
> @@ -782,9 +815,10 @@ dying:
> int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
> {
> unsigned long flags;
> - int ret;
> + int ret, i;
> u32 temp;
> struct xhci_hcd *xhci;
> + struct urb_priv *urb_priv;
> struct xhci_td *td;
> unsigned int ep_index;
> struct xhci_ring *ep_ring;
> @@ -799,12 +833,15 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
> temp = xhci_readl(xhci, &xhci->op_regs->status);
> if (temp == 0xffffffff) {
> xhci_dbg(xhci, "HW died, freeing TD.\n");
> - td = (struct xhci_td *) urb->hcpriv;
> + urb_priv = urb->hcpriv;
>
> usb_hcd_unlink_urb_from_ep(hcd, urb);
> spin_unlock_irqrestore(&xhci->lock, flags);
> usb_hcd_giveback_urb(xhci_to_hcd(xhci), urb, -ESHUTDOWN);
> - kfree(td);
> + if (urb_priv) {
> + urb_free_priv(xhci, urb_priv);
> + urb->hcpriv = NULL;
> + }
> return ret;
> }
> if (xhci->xhc_state & XHCI_STATE_DYING) {
> @@ -827,9 +864,18 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
> ep_ring = ep->ring;
> xhci_dbg(xhci, "Endpoint ring:\n");
> xhci_debug_ring(xhci, ep_ring);
> - td = (struct xhci_td *) urb->hcpriv;
>
> - list_add_tail(&td->cancelled_td_list, &ep->cancelled_td_list);
> + urb_priv = urb->hcpriv;
> +
> + if (urb_priv) {
> + for (i = 0; i < urb_priv->length; i++) {
> + td = urb_priv->td[i];
> + if (td)
> + list_add_tail(&td->cancelled_td_list,
> + &ep->cancelled_td_list);
> + }
> + }
> +
> /* Queue a stop endpoint command, but only if this is
> * the first cancellation to be handled.
> */
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index 8045bc6..a645c3a 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -881,6 +881,23 @@ struct xhci_command *xhci_alloc_command(struct xhci_hcd *xhci,
> return command;
> }
>
> +void urb_free_priv(struct xhci_hcd *xhci, struct urb_priv *urb_priv)
> +{
> + int last = urb_priv->length - 1;
> +
> + if (last >= 0) {
> + int i;
> + struct xhci_td *td;
> +
> + for (i = 0; i <= last; i++) {
> + td = urb_priv->td[i];
> + kfree(td);
> + }
> + }
> +
> + kfree(urb_priv);
> +}
> +
> void xhci_free_command(struct xhci_hcd *xhci,
> struct xhci_command *command)
> {
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 6ba841b..79041b9 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -492,16 +492,18 @@ static void xhci_giveback_urb_in_irq(struct xhci_hcd *xhci,
> struct xhci_td *cur_td, int status, char *adjective)
> {
> struct usb_hcd *hcd = xhci_to_hcd(xhci);
> + struct urb *urb;
>
> - cur_td->urb->hcpriv = NULL;
> - usb_hcd_unlink_urb_from_ep(hcd, cur_td->urb);
> - xhci_dbg(xhci, "Giveback %s URB %p\n", adjective, cur_td->urb);
> + urb = cur_td->urb;
> + if (urb->hcpriv) {
> + usb_hcd_unlink_urb_from_ep(hcd, urb);
> + xhci_dbg(xhci, "Giveback %s URB %p\n", adjective, urb);
>
> - spin_unlock(&xhci->lock);
> - usb_hcd_giveback_urb(hcd, cur_td->urb, status);
> - kfree(cur_td);
> - spin_lock(&xhci->lock);
> - xhci_dbg(xhci, "%s URB given back\n", adjective);
> + spin_unlock(&xhci->lock);
> + usb_hcd_giveback_urb(hcd, urb, status);
> + spin_lock(&xhci->lock);
> + xhci_dbg(xhci, "%s URB given back\n", adjective);
> + }
> }
>
> /*
> @@ -1472,9 +1474,9 @@ td_cleanup:
> if (usb_endpoint_xfer_control(&urb->ep->desc) ||
> (trb_comp_code != COMP_STALL &&
> trb_comp_code != COMP_BABBLE)) {
> - kfree(td);
> + urb_free_priv(xhci, urb->hcpriv);
> + urb->hcpriv = NULL;
> }
> - urb->hcpriv = NULL;
> }
> cleanup:
> inc_deq(xhci, xhci->event_ring, true);
> @@ -1628,34 +1630,32 @@ static int prepare_transfer(struct xhci_hcd *xhci,
> unsigned int ep_index,
> unsigned int num_trbs,
> struct urb *urb,
> - struct xhci_td **td,
> + unsigned int td_index,
> gfp_t mem_flags)
> {
> int ret;
> + struct urb_priv *urb_priv;
> + struct xhci_td *td;
> struct xhci_ep_ctx *ep_ctx = xhci_get_ep_ctx(xhci, xdev->out_ctx, ep_index);
> ret = prepare_ring(xhci, xdev->eps[ep_index].ring,
> ep_ctx->ep_info & EP_STATE_MASK,
> num_trbs, mem_flags);
> if (ret)
> return ret;
> - *td = kzalloc(sizeof(struct xhci_td), mem_flags);
> - if (!*td)
> - return -ENOMEM;
> - INIT_LIST_HEAD(&(*td)->td_list);
> - INIT_LIST_HEAD(&(*td)->cancelled_td_list);
>
> - ret = usb_hcd_link_urb_to_ep(xhci_to_hcd(xhci), urb);
> - if (unlikely(ret)) {
> - kfree(*td);
> - return ret;
> - }
> + urb_priv = urb->hcpriv;
> + td = urb_priv->td[td_index];
> +
> + INIT_LIST_HEAD(&td->td_list);
> + INIT_LIST_HEAD(&td->cancelled_td_list);
>
> - (*td)->urb = urb;
> - urb->hcpriv = (void *) (*td);
> + td->urb = urb;
> /* Add this TD to the tail of the endpoint ring's TD list */
> - list_add_tail(&(*td)->td_list, &xdev->eps[ep_index].ring->td_list);
> - (*td)->start_seg = xdev->eps[ep_index].ring->enq_seg;
> - (*td)->first_trb = xdev->eps[ep_index].ring->enqueue;
> + list_add_tail(&td->td_list, &xdev->eps[ep_index].ring->td_list);
> + td->start_seg = xdev->eps[ep_index].ring->enq_seg;
> + td->first_trb = xdev->eps[ep_index].ring->enqueue;
> +
> + urb_priv->td[td_index] = td;
>
> return 0;
> }
> @@ -1794,6 +1794,7 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
> {
> struct xhci_ring *ep_ring;
> unsigned int num_trbs;
> + struct urb_priv *urb_priv;
> struct xhci_td *td;
> struct scatterlist *sg;
> int num_sgs;
> @@ -1809,9 +1810,13 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
> num_sgs = urb->num_sgs;
>
> trb_buff_len = prepare_transfer(xhci, xhci->devs[slot_id],
> - ep_index, num_trbs, urb, &td, mem_flags);
> + ep_index, num_trbs, urb, 0, mem_flags);
> if (trb_buff_len < 0)
> return trb_buff_len;
> +
> + urb_priv = urb->hcpriv;
> + td = urb_priv->td[0];
> +
> /*
> * Don't give the first TRB to the hardware (by toggling the cycle bit)
> * until we've finished creating all the other TRBs. The ring's cycle
> @@ -1927,6 +1932,7 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
> struct urb *urb, int slot_id, unsigned int ep_index)
> {
> struct xhci_ring *ep_ring;
> + struct urb_priv *urb_priv;
> struct xhci_td *td;
> int num_trbs;
> struct xhci_generic_trb *start_trb;
> @@ -1968,10 +1974,13 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
> num_trbs);
>
> ret = prepare_transfer(xhci, xhci->devs[slot_id], ep_index,
> - num_trbs, urb, &td, mem_flags);
> + num_trbs, urb, 0, mem_flags);
> if (ret < 0)
> return ret;
>
> + urb_priv = urb->hcpriv;
> + td = urb_priv->td[0];
> +
> /*
> * Don't give the first TRB to the hardware (by toggling the cycle bit)
> * until we've finished creating all the other TRBs. The ring's cycle
> @@ -2052,6 +2061,7 @@ int xhci_queue_ctrl_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
> struct xhci_generic_trb *start_trb;
> int start_cycle;
> u32 field, length_field;
> + struct urb_priv *urb_priv;
> struct xhci_td *td;
>
> ep_ring = xhci->devs[slot_id]->eps[ep_index].ring;
> @@ -2076,10 +2086,13 @@ int xhci_queue_ctrl_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
> if (urb->transfer_buffer_length > 0)
> num_trbs++;
> ret = prepare_transfer(xhci, xhci->devs[slot_id], ep_index, num_trbs,
> - urb, &td, mem_flags);
> + urb, 0, mem_flags);
> if (ret < 0)
> return ret;
>
> + urb_priv = urb->hcpriv;
> + td = urb_priv->td[0];
> +
> /*
> * Don't give the first TRB to the hardware (by toggling the cycle bit)
> * until we've finished creating all the other TRBs. The ring's cycle
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index e5eb09b..6d4b21c 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -858,6 +858,9 @@ struct xhci_event_cmd {
> /* Control transfer TRB specific fields */
> #define TRB_DIR_IN (1<<16)
>
> +/* Isoc TRB specific fields */
> +#define TRB_SIA (1<<31)
> +
> struct xhci_generic_trb {
> u32 field[4];
> };
> @@ -1015,6 +1018,12 @@ struct xhci_scratchpad {
> dma_addr_t *sp_dma_buffers;
> };
>
> +struct urb_priv {
> + u16 length;
> + int td_cnt;
> + struct xhci_td *td[0];
> +};
> +
> /*
> * Each segment table entry is 4*32bits long. 1K seems like an ok size:
> * (1K bytes * 8bytes/bit) / (4*32 bits) = 64 segment entries in the table,
> @@ -1241,6 +1250,7 @@ void xhci_free_or_cache_endpoint_ring(struct xhci_hcd *xhci,
> struct xhci_command *xhci_alloc_command(struct xhci_hcd *xhci,
> bool allocate_in_ctx, bool allocate_completion,
> gfp_t mem_flags);
> +void urb_free_priv(struct xhci_hcd *xhci, struct urb_priv *urb_priv);
> void xhci_free_command(struct xhci_hcd *xhci,
> struct xhci_command *command);
>
> --
> 1.6.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
--
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
