On Wed, Nov 13, 2013 at 12:28:09PM -0000, David Laight wrote:
> If URB_ZERO_PACKET is set on a transfer that is an integral number
> of maximum length packets (1k for USB3 bulk) then an additional
> zero length fragment must be sent.
>
> Merge together the functions that setup single buffer and scatter-gather
> transfers (they aren't that different) simplifying the logic as well.
>
> In particular we note that the number of TRB passed to prepare_ring()
> need only be an upper limit on the number required. However we do try
> to only request 1 TRB when we know one is sufficient.
>
> Signed-off-by: David Laight <david.laight@xxxxxxxxxx>
> ---
> Change for v2:
> - Ensure we check there is space for 1 TRB for zero length transfer requests.
> (The extra TRB is added in the same path that checks URB_ZERO_PACKET).
>
> I hope I've edited the patch correctly.
> I've NFI how to get git to generate modified patches.
git commit --amend
I'll look over this patch in a couple days. ISTR that someone else
submitted a zero-length packet fix patch, so I need to coordinate the
two.
Sarah Sharp
>
> drivers/usb/host/xhci-ring.c | 390 +++++++++++--------------------------------
> drivers/usb/host/xhci.h | 1 +
> 2 files changed, 102 insertions(+), 290 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index ace586c..7211223 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -3082,55 +3082,6 @@ static int prepare_transfer(struct xhci_hcd *xhci,
> return 0;
> }
>
> -static unsigned int count_sg_trbs_needed(struct xhci_hcd *xhci, struct urb *urb)
> -{
> - int num_sgs, num_trbs, running_total, temp, i;
> - struct scatterlist *sg;
> -
> - sg = NULL;
> - num_sgs = urb->num_mapped_sgs;
> - temp = urb->transfer_buffer_length;
> -
> - num_trbs = 0;
> - for_each_sg(urb->sg, sg, num_sgs, i) {
> - unsigned int len = sg_dma_len(sg);
> -
> - /* Scatter gather list entries may cross 64KB boundaries */
> - running_total = TRB_MAX_BUFF_SIZE -
> - (sg_dma_address(sg) & (TRB_MAX_BUFF_SIZE - 1));
> - running_total &= TRB_MAX_BUFF_SIZE - 1;
> - if (running_total != 0)
> - num_trbs++;
> -
> - /* How many more 64KB chunks to transfer, how many more TRBs? */
> - while (running_total < sg_dma_len(sg) && running_total < temp) {
> - num_trbs++;
> - running_total += TRB_MAX_BUFF_SIZE;
> - }
> - len = min_t(int, len, temp);
> - temp -= len;
> - if (temp == 0)
> - break;
> - }
> - return num_trbs;
> -}
> -
> -static void check_trb_math(struct urb *urb, int num_trbs, int running_total)
> -{
> - if (num_trbs != 0)
> - dev_err(&urb->dev->dev, "%s - ep %#x - Miscalculated number of "
> - "TRBs, %d left\n", __func__,
> - urb->ep->desc.bEndpointAddress, num_trbs);
> - if (running_total != urb->transfer_buffer_length)
> - dev_err(&urb->dev->dev, "%s - ep %#x - Miscalculated tx length, "
> - "queued %#x (%d), asked for %#x (%d)\n",
> - __func__,
> - urb->ep->desc.bEndpointAddress,
> - running_total, running_total,
> - urb->transfer_buffer_length,
> - urb->transfer_buffer_length);
> -}
> -
> static void giveback_first_trb(struct xhci_hcd *xhci, int slot_id,
> unsigned int ep_index, unsigned int stream_id,
> struct xhci_generic_trb *start_trb)
> @@ -3232,285 +3183,145 @@ static u32 xhci_v1_0_td_remainder(int running_total, int trb_buff_len,
> return (total_packet_count - packets_transferred) << 17;
> }
>
> -static int queue_bulk_sg_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
> +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;
> unsigned int num_trbs;
> struct urb_priv *urb_priv;
> - struct xhci_td *td;
> + u32 trb_buff_len, this_sg_len;
> struct scatterlist *sg;
> - int num_sgs;
> - int trb_buff_len, this_sg_len, running_total;
> - unsigned int total_packet_count;
> - u32 trb_cycle_invert;
> + u32 trb_type_flags;
> + u32 max_packet, td_residue, len_left;
> + u32 td_size;
> u64 addr;
> - bool more_trbs_coming;
> -
> - struct xhci_generic_trb *start_trb;
> + int ret;
>
> ep_ring = xhci_urb_to_transfer_ring(xhci, urb);
> - if (!ep_ring)
> + if (unlikely(!ep_ring))
> return -EINVAL;
>
> - num_trbs = count_sg_trbs_needed(xhci, urb);
> - num_sgs = urb->num_mapped_sgs;
> - total_packet_count = DIV_ROUND_UP(urb->transfer_buffer_length,
> - usb_endpoint_maxp(&urb->ep->desc));
> -
> - trb_buff_len = prepare_transfer(xhci, xhci->devs[slot_id],
> - ep_index, urb->stream_id,
> - 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
> - * state may change as we enqueue the other TRBs, so save it too.
> - */
> - start_trb = &ep_ring->enqueue->generic;
> -
> - running_total = 0;
> - /*
> - * How much data is in the first TRB?
> - *
> - * There are three forces at work for TRB buffer pointers and lengths:
> - * 1. We don't want to walk off the end of this sg-list entry buffer.
> - * 2. The transfer length that the driver requested may be smaller than
> - * the amount of memory allocated for this scatter-gather list.
> - * 3. TRBs buffers can't cross 64KB boundaries.
> - */
> - sg = urb->sg;
> - addr = (u64) sg_dma_address(sg);
> - this_sg_len = sg_dma_len(sg);
> - trb_buff_len = TRB_MAX_BUFF_SIZE - (addr & (TRB_MAX_BUFF_SIZE - 1));
> - trb_buff_len = min_t(int, trb_buff_len, this_sg_len);
> - if (trb_buff_len > urb->transfer_buffer_length)
> - trb_buff_len = urb->transfer_buffer_length;
> -
> - trb_cycle_invert = TRB_CYCLE;
> - /* Queue the first TRB, even if it's zero-length */
> - do {
> - u32 field = 0;
> - u32 length_field = 0;
> - u32 remainder = 0;
> -
> - field |= ep_ring->cycle_state;
> - /* Don't change the cycle bit of the first TRB until later */
> - field ^= trb_cycle_invert;
> - trb_cycle_invert = 0;
> -
> - /* Chain all the TRBs together; clear the chain bit in the last
> - * TRB to indicate it's the last TRB in the chain.
> - */
> - if (num_trbs > 1) {
> - field |= TRB_CHAIN;
> - } else {
> - /* FIXME - add check for ZERO_PACKET flag before this */
> - td->last_trb = ep_ring->enqueue;
> - field |= TRB_IOC;
> - }
> -
> - /* Only set interrupt on short packet for IN endpoints */
> - if (usb_urb_dir_in(urb))
> - field |= TRB_ISP;
> -
> - if (TRB_MAX_BUFF_SIZE -
> - (addr & (TRB_MAX_BUFF_SIZE - 1)) < trb_buff_len) {
> - xhci_warn(xhci, "WARN: sg dma xfer crosses 64KB boundaries!\n");
> - xhci_dbg(xhci, "Next boundary at %#x, end dma = %#x\n",
> - (unsigned int) (addr + TRB_MAX_BUFF_SIZE) & ~(TRB_MAX_BUFF_SIZE - 1),
> - (unsigned int) addr + trb_buff_len);
> - }
> -
> - /* Set the TRB length, TD size, and interrupter fields. */
> - if (xhci->hci_version < 0x100) {
> - remainder = xhci_td_remainder(
> - urb->transfer_buffer_length -
> - running_total);
> - } else {
> - remainder = xhci_v1_0_td_remainder(running_total,
> - trb_buff_len, total_packet_count, urb,
> - num_trbs - 1);
> - }
> - length_field = TRB_LEN(trb_buff_len) |
> - remainder |
> - TRB_INTR_TARGET(0);
> -
> - if (num_trbs > 1)
> - more_trbs_coming = true;
> - else
> - more_trbs_coming = false;
> - queue_trb(xhci, ep_ring, more_trbs_coming,
> - lower_32_bits(addr),
> - upper_32_bits(addr),
> - length_field,
> - field | TRB_TYPE(TRB_NORMAL));
> - --num_trbs;
> - running_total += trb_buff_len;
> -
> - /* Calculate length for next transfer --
> - * Are we done queueing all the TRBs for this sg entry?
> + if (urb->num_sgs == 0) {
> + sg = NULL;
> + addr = (u64)urb->transfer_dma;
> + this_sg_len = urb->transfer_buffer_length;
> + num_trbs = 0;
> + } else {
> + sg = urb->sg;
> + addr = (u64)sg_dma_address(sg);
> + this_sg_len = sg_dma_len(sg);
> + /*
> + * We only need an upper bound for the number of TRBs.
> + * Add in two extra TRB for each subsequent segment.
> */
> - this_sg_len -= trb_buff_len;
> - if (this_sg_len == 0) {
> - --num_sgs;
> - if (num_sgs == 0)
> - break;
> - sg = sg_next(sg);
> - addr = (u64) sg_dma_address(sg);
> - this_sg_len = sg_dma_len(sg);
> - } else {
> - addr += trb_buff_len;
> - }
> -
> - trb_buff_len = TRB_MAX_BUFF_SIZE -
> - (addr & (TRB_MAX_BUFF_SIZE - 1));
> - trb_buff_len = min_t(int, trb_buff_len, this_sg_len);
> - if (running_total + trb_buff_len > urb->transfer_buffer_length)
> - trb_buff_len =
> - urb->transfer_buffer_length - running_total;
> - } while (running_total < urb->transfer_buffer_length);
> -
> - check_trb_math(urb, num_trbs, running_total);
> - giveback_first_trb(xhci, slot_id, ep_index, urb->stream_id, start_trb);
> - return 0;
> -}
> -
> -/* This is very similar to what ehci-q.c qtd_fill() does */
> -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;
> - u32 trb_cycle_invert;
> - bool more_trbs_coming;
> - u32 field, length_field;
> -
> - int running_total, trb_buff_len, ret;
> - unsigned int total_packet_count;
> - u64 addr;
> -
> - if (urb->num_sgs)
> - return queue_bulk_sg_tx(xhci, mem_flags, urb, slot_id, ep_index);
> + num_trbs = (urb->num_mapped_sgs - 1) * 2;
> + }
>
> - ep_ring = xhci_urb_to_transfer_ring(xhci, urb);
> - if (!ep_ring)
> - return -EINVAL;
> + /* One TRB for each 64k 'page' that contains data */
> + num_trbs += DIV_ROUND_UP((addr & (TRB_MAX_BUFF_SIZE - 1)) +
> + urb->transfer_buffer_length, TRB_MAX_BUFF_SIZE);
>
> - num_trbs = 0;
> - /* How much data is (potentially) left before the 64KB boundary? */
> - running_total = TRB_MAX_BUFF_SIZE -
> - (urb->transfer_dma & (TRB_MAX_BUFF_SIZE - 1));
> - running_total &= TRB_MAX_BUFF_SIZE - 1;
> + max_packet = GET_MAX_PACKET(usb_endpoint_maxp(&urb->ep->desc));
>
> - /* If there's some data on this 64KB chunk, or we have to send a
> - * zero-length transfer, we need at least one TRB
> + /*
> + * For v 1.0 we need to calculate the number of TD needed to
> + * complete the transfer.
> + * The code here is equivalent to 4.11.2.4.
> */
> - if (running_total != 0 || urb->transfer_buffer_length == 0)
> - num_trbs++;
> - /* How many more 64KB chunks to transfer, how many more TRBs? */
> - while (running_total < urb->transfer_buffer_length) {
> + td_residue = urb->transfer_buffer_length;
> + if (unlikely(urb->transfer_flags & URB_ZERO_PACKET)
> + || unlikely(urb->transfer_buffer_length == 0)) {
> + /* We need an extra zero length TD */
> num_trbs++;
> - running_total += TRB_MAX_BUFF_SIZE;
> + td_residue++;
> }
> - /* FIXME: this doesn't deal with URB_ZERO_PACKET - need one more */
> + td_residue = ALIGN(td_residue, max_packet) + max_packet - 1;
>
> ret = prepare_transfer(xhci, xhci->devs[slot_id],
> ep_index, urb->stream_id,
> num_trbs, urb, 0, mem_flags);
> - if (ret < 0)
> + if (unlikely(ret < 0))
> return ret;
>
> - urb_priv = urb->hcpriv;
> - td = &urb_priv->td[0];
> + trb_type_flags = TRB_CYCLE | TRB_CHAIN | TRB_TYPE(TRB_NORMAL);
>
> - /*
> - * 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
> - * state may change as we enqueue the other TRBs, so save it too.
> - */
> - start_trb = &ep_ring->enqueue->generic;
> -
> - running_total = 0;
> - total_packet_count = DIV_ROUND_UP(urb->transfer_buffer_length,
> - usb_endpoint_maxp(&urb->ep->desc));
> - /* How much data is in the first TRB? */
> - addr = (u64) urb->transfer_dma;
> - trb_buff_len = TRB_MAX_BUFF_SIZE -
> - (urb->transfer_dma & (TRB_MAX_BUFF_SIZE - 1));
> - if (trb_buff_len > urb->transfer_buffer_length)
> - trb_buff_len = urb->transfer_buffer_length;
> -
> - trb_cycle_invert = TRB_CYCLE;
> + /* Set interrupt on short packet for IN endpoints */
> + if (usb_urb_dir_in(urb))
> + trb_type_flags |= TRB_ISP;
>
> /* Queue the first TRB, even if it's zero-length */
> - do {
> - u32 remainder = 0;
> - field = 0;
> + len_left = urb->transfer_buffer_length;
> + for (;;) {
> + /*
> + * How much data is in the TRB?
> + *
> + * 1. TRBs buffers can't cross 64KB boundaries.
> + * 2. We don't want to walk off the end of this sg-list buffer.
> + * 2. Don't exceed the driver-supplied transfer length.
> + * (May be smaller than the sg list.)
> + */
> + trb_buff_len = (~addr & (TRB_MAX_BUFF_SIZE - 1)) + 1;
> + trb_buff_len = min_t(u32, trb_buff_len, this_sg_len);
> + trb_buff_len = min_t(u32, trb_buff_len, len_left);
> + td_residue -= trb_buff_len;
>
> - field |= ep_ring->cycle_state;
> - /* Don't change the cycle bit of the first TRB until later */
> - field ^= trb_cycle_invert;
> - trb_cycle_invert = 0;
> + /* TRB_CYCLE is inverted in the first TRB */
> + trb_type_flags ^= ep_ring->cycle_state;
>
> - /* Chain all the TRBs together; clear the chain bit in the last
> + /*
> + * Chain all the TRBs together; clear the chain bit in the last
> * TRB to indicate it's the last TRB in the chain.
> + * The td_residue can only be 2 * max_packet - 1 if
> + * URB_ZERO_PACKET was set, this forces a zero length packet.
> + * For version 1.0 we could send the last fragment with
> + * td_size set to 1 - but that is just extra logic.
> */
> - if (num_trbs > 1) {
> - field |= TRB_CHAIN;
> + if (trb_buff_len == len_left && (trb_buff_len == 0 ||
> + td_residue < 2 * max_packet - 1)) {
> + /* The last buffer fragment */
> + urb_priv = urb->hcpriv;
> + urb_priv->td[0].last_trb = ep_ring->enqueue;
> + /* Set TRB_IOC and clear TRB_CHAIN */
> + trb_type_flags ^= TRB_IOC | TRB_CHAIN;
> + td_size = 0;
> } else {
> - /* FIXME - add check for ZERO_PACKET flag before this */
> - td->last_trb = ep_ring->enqueue;
> - field |= TRB_IOC;
> + if (xhci->hci_version < 0x100)
> + td_size = xhci_td_remainder(len_left);
> + else
> + td_size = TRB_TD_SIZE(td_residue / max_packet);
> }
>
> - /* Only set interrupt on short packet for IN endpoints */
> - if (usb_urb_dir_in(urb))
> - field |= TRB_ISP;
> + /* No need to specify 'more_trbs_coming', implied by CHAIN */
> + queue_trb(xhci, ep_ring, false,
> + lower_32_bits(addr),
> + upper_32_bits(addr),
> + TRB_LEN(trb_buff_len) | td_size |
> + TRB_INTR_TARGET(0),
> + trb_type_flags);
> +
> + if (!(trb_type_flags & TRB_CHAIN))
> + break;
> +
> + /* We want the current TRB_CYCLE in subsequent TRB */
> + trb_type_flags &= ~TRB_CYCLE;
>
> - /* Set the TRB length, TD size, and interrupter fields. */
> - if (xhci->hci_version < 0x100) {
> - remainder = xhci_td_remainder(
> - urb->transfer_buffer_length -
> - running_total);
> + /* Advance everything past this segment */
> + len_left -= trb_buff_len;
> + this_sg_len -= trb_buff_len;
> + if (this_sg_len == 0 && len_left != 0) {
> + sg = sg_next(sg);
> + addr = (u64)sg_dma_address(sg);
> + this_sg_len = sg_dma_len(sg);
> } else {
> - remainder = xhci_v1_0_td_remainder(running_total,
> - trb_buff_len, total_packet_count, urb,
> - num_trbs - 1);
> + /* Buffer was split on 64k boundary.
> + * Or we need to send a zero length packet. */
> + addr += trb_buff_len;
> }
> - length_field = TRB_LEN(trb_buff_len) |
> - remainder |
> - TRB_INTR_TARGET(0);
> + }
>
> - if (num_trbs > 1)
> - more_trbs_coming = true;
> - else
> - more_trbs_coming = false;
> - queue_trb(xhci, ep_ring, more_trbs_coming,
> - lower_32_bits(addr),
> - upper_32_bits(addr),
> - length_field,
> - field | TRB_TYPE(TRB_NORMAL));
> - --num_trbs;
> - running_total += trb_buff_len;
> -
> - /* Calculate length for next transfer */
> - addr += trb_buff_len;
> - trb_buff_len = urb->transfer_buffer_length - running_total;
> - if (trb_buff_len > TRB_MAX_BUFF_SIZE)
> - trb_buff_len = TRB_MAX_BUFF_SIZE;
> - } while (running_total < urb->transfer_buffer_length);
> -
> - check_trb_math(urb, num_trbs, running_total);
> - giveback_first_trb(xhci, slot_id, ep_index, urb->stream_id, start_trb);
> + urb_priv = urb->hcpriv;
> + giveback_first_trb(xhci, slot_id, ep_index, urb->stream_id,
> + &urb_priv->td[0].first_trb->generic);
> return 0;
> }
>
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index 118f90b..bbdbfed 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -1123,6 +1123,7 @@ struct xhci_event_cmd {
> /* Normal TRB fields */
> /* transfer_len bitmasks - bits 0:16 */
> #define TRB_LEN(p) ((p) & 0x1ffff)
> +#define TRB_TD_SIZE(p) (min_t(u32, (p), 31) << 17)
> /* Interrupter Target - which MSI-X vector to target the completion event at */
> #define TRB_INTR_TARGET(p) (((p) & 0x3ff) << 22)
> #define GET_INTR_TARGET(p) (((p) >> 22) & 0x3ff)
> --
> 1.8.1.2
>
>
>
--
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
