At least some hardware (eg ASMedia PCIe cards) don't like processing a LINK
TRB and then finding that the 'cycle' bit on the next TRB is 'unset'.
This is make much more likely by the code that adds NOP TRB in order
to avoid inappropriate data alignments at LINK TRB.
I also suspect there is a race between the xhci hardware and the host
when the termination of one command schedules the setup of the next.
This makes the the bugs very machine specific.
Fix this by remembering the address of the first trb in prepare_ring()
and using that saved pointer in giveback_first_trb().
Since the first call to queue_trb() might now not be for the actual
first trb, the callers cannot track the cycle bit so this must (and
can easily) be done by queue_trb().
Similarly queue_command() must now call giveback_first_trb(), which
must not ring the doorbell for command rings.
The 'cleanup' code at the end of xhci_queue_isoc_tx() looked problematical.
However close inspection shows that it should never be executed.
For isoc transfers prepare_ring() is called early to ensure that there
are enough ring entries for the entire transfer - so it cannot fail
when called from prepare_transfer() withing the loop. In fact the later
calls are actually unnecessary so predicate them on td_index beiong zero.
The other possible looking error return from prepare_tranfser() is if
xhci_stream_id_to_ring() fails, however all the callers have just done
this. Pass in the ep_ring as a parameter to prepare_transfer() instead
of stream_id which can be obtained from the ring.
The other 'goto cleanup' is from a coding sanity check, the test could
probably be removed, I've just left it as an xci_err() trace.
The cleanup code has been deleted.
Remove the 'flip_cycle' parameter to td_to_noop() since that was only
set when called from the cleanup code of xhci_queue_isoc_tx().
(Saves worrying about whether the flips are correct.)
The code has been simplified significantly by advancing the ring's
enqueue point past LINK TRB in queue_trb() prior to writing an entry
(instead of in prepare_ring() and inc_enq() when the caller has
indicated that another TRB will be written).
There is now only one copy of the 'advance past a LINK when writing
to a ring' code and inc_enq() collapses to 3 lines and can be inlined
directly into queue_trb().
The 'more_trbs_coming' parameter (and the code to set it) disappears
in a puff of logic.
While it doesn't matter if td->first_trb references a LINK, td->last_trb
must point to the correct place. This is now 'ring->enqueue - 1'
after queue_trb() has been called.
td->last_trb is set using a static inline function since a minor
implementation change might require a different scheme.
I've also taken the liberty of on tracing short receives when they
are unexpected. Otherwise every ethernet receive generates a trace
(when the xhci_debug() trace is enabled).
Signed-off-by: David Laight <david.laight@xxxxxxxxxx>
---
This patch may well fix some of the regressions that appear to have been
created by the patch that adds NOP TRB.
I've a private email that indicates that a minimal patch (that left a
lot of unused code lurking) helped one ASMedia system.
If that is the case (I don't have appropriate hardware) then this is a
strong candidate for 3.13 and stable (3.12).
I've run an ethernet soak test (asx88179_178a) for several hours (with
usbnet.c patched to correctly initialise the scatter-gather list).
drivers/usb/host/xhci-ring.c | 414 ++++++++++++-------------------------------
drivers/usb/host/xhci.h | 7 +
2 files changed, 123 insertions(+), 298 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 53c2e29..1fa40a8 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -117,12 +117,6 @@ static int last_trb(struct xhci_hcd *xhci, struct xhci_ring *ring,
return TRB_TYPE_LINK_LE32(trb->link.control);
}
-static int enqueue_is_link_trb(struct xhci_ring *ring)
-{
- struct xhci_link_trb *link = &ring->enqueue->link;
- return TRB_TYPE_LINK_LE32(link->control);
-}
-
union xhci_trb *xhci_find_next_enqueue(struct xhci_ring *ring)
{
/* Enqueue pointer can be left pointing to the link TRB,
@@ -191,83 +185,6 @@ static void inc_deq(struct xhci_hcd *xhci, struct xhci_ring *ring)
}
/*
- * See Cycle bit rules. SW is the consumer for the event ring only.
- * Don't make a ring full of link TRBs. That would be dumb and this would loop.
- *
- * If we've just enqueued a TRB that is in the middle of a TD (meaning the
- * chain bit is set), then set the chain bit in all the following link TRBs.
- * If we've enqueued the last TRB in a TD, make sure the following link TRBs
- * have their chain bit cleared (so that each Link TRB is a separate TD).
- *
- * Section 6.4.4.1 of the 0.95 spec says link TRBs cannot have the chain bit
- * set, but other sections talk about dealing with the chain bit set. This was
- * fixed in the 0.96 specification errata, but we have to assume that all 0.95
- * xHCI hardware can't handle the chain bit being cleared on a link TRB.
- *
- * @more_trbs_coming: Will you enqueue more TRBs before calling
- * prepare_transfer()?
- */
-static void inc_enq(struct xhci_hcd *xhci, struct xhci_ring *ring,
- bool more_trbs_coming)
-{
- u32 chain;
- union xhci_trb *next;
- unsigned long long addr;
-
- chain = le32_to_cpu(ring->enqueue->generic.field[3]) & TRB_CHAIN;
- /* If this is not event ring, there is one less usable TRB */
- if (ring->type != TYPE_EVENT &&
- !last_trb(xhci, ring, ring->enq_seg, ring->enqueue))
- ring->num_trbs_free--;
- next = ++(ring->enqueue);
-
- ring->enq_updates++;
- /* Update the dequeue pointer further if that was a link TRB or we're at
- * the end of an event ring segment (which doesn't have link TRBS)
- */
- while (last_trb(xhci, ring, ring->enq_seg, next)) {
- if (ring->type != TYPE_EVENT) {
- /*
- * If the caller doesn't plan on enqueueing more
- * TDs before ringing the doorbell, then we
- * don't want to give the link TRB to the
- * hardware just yet. We'll give the link TRB
- * back in prepare_ring() just before we enqueue
- * the TD at the top of the ring.
- */
- if (!chain && !more_trbs_coming)
- break;
-
- /* If we're not dealing with 0.95 hardware or
- * isoc rings on AMD 0.96 host,
- * carry over the chain bit of the previous TRB
- * (which may mean the chain bit is cleared).
- */
- if (!(ring->type == TYPE_ISOC &&
- (xhci->quirks & XHCI_AMD_0x96_HOST))
- && !xhci_link_trb_quirk(xhci)) {
- next->link.control &=
- cpu_to_le32(~TRB_CHAIN);
- next->link.control |=
- cpu_to_le32(chain);
- }
- /* Give this link TRB to the hardware */
- wmb();
- next->link.control ^= cpu_to_le32(TRB_CYCLE);
-
- /* Toggle the cycle bit after the last ring segment. */
- if (last_trb_on_last_seg(xhci, ring, ring->enq_seg, next)) {
- ring->cycle_state = (ring->cycle_state ? 0 : 1);
- }
- }
- ring->enq_seg = ring->enq_seg->next;
- ring->enqueue = ring->enq_seg->trbs;
- next = ring->enqueue;
- }
- addr = (unsigned long long) xhci_trb_virt_to_dma(ring->enq_seg, ring->enqueue);
-}
-
-/*
* Check to see if there's room to enqueue num_trbs on the ring and make sure
* enqueue pointer will not advance into dequeue segment. See rules above.
*/
@@ -624,12 +541,8 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci,
(unsigned long long) addr);
}
-/* flip_cycle means flip the cycle bit of all but the first and last TRB.
- * (The last TRB actually points to the ring enqueue pointer, which is not part
- * of this TD.) This is used to remove partially enqueued isoc TDs from a ring.
- */
static void td_to_noop(struct xhci_hcd *xhci, struct xhci_ring *ep_ring,
- struct xhci_td *cur_td, bool flip_cycle)
+ struct xhci_td *cur_td)
{
struct xhci_segment *cur_seg;
union xhci_trb *cur_trb;
@@ -645,9 +558,6 @@ static void td_to_noop(struct xhci_hcd *xhci, struct xhci_ring *ep_ring,
/* Flip the cycle bit (link TRBs can't be the first
* or last TRB).
*/
- if (flip_cycle)
- cur_trb->generic.field[3] ^=
- cpu_to_le32(TRB_CYCLE);
xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
"Cancel (unchain) link TRB");
xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
@@ -664,10 +574,6 @@ static void td_to_noop(struct xhci_hcd *xhci, struct xhci_ring *ep_ring,
/* Preserve only the cycle bit of this TRB */
cur_trb->generic.field[3] &= cpu_to_le32(TRB_CYCLE);
/* Flip the cycle bit except on the first or last TRB */
- if (flip_cycle && cur_trb != cur_td->first_trb &&
- cur_trb != cur_td->last_trb)
- cur_trb->generic.field[3] ^=
- cpu_to_le32(TRB_CYCLE);
cur_trb->generic.field[3] |= cpu_to_le32(
TRB_TYPE(TRB_TR_NOOP));
xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
@@ -841,7 +747,7 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id,
cur_td->urb->stream_id,
cur_td, &deq_state);
else
- td_to_noop(xhci, ep_ring, cur_td, false);
+ td_to_noop(xhci, ep_ring, cur_td);
remove_finished_td:
/*
* The event handler won't see a completion for this TD anymore,
@@ -2332,7 +2238,7 @@ static int process_bulk_intr_td(struct xhci_hcd *xhci, struct xhci_td *td,
/* Others already handled above */
break;
}
- if (trb_comp_code == COMP_SHORT_TX)
+ if (*status == -EREMOTEIO)
xhci_dbg(xhci, "ep %#x - asked for %d bytes, "
"%d bytes untransferred\n",
td->urb->ep->desc.bEndpointAddress,
@@ -2917,22 +2823,62 @@ irqreturn_t xhci_msi_irq(int irq, void *hcd)
/*
* Generic function for queueing a TRB on a ring.
* The caller must have checked to make sure there's room on the ring.
- *
- * @more_trbs_coming: Will you enqueue more TRBs before calling
- * prepare_transfer()?
*/
static void queue_trb(struct xhci_hcd *xhci, struct xhci_ring *ring,
- bool more_trbs_coming,
u32 field1, u32 field2, u32 field3, u32 field4)
{
- struct xhci_generic_trb *trb;
+ union xhci_trb *trb;
+
+ trb = ring->enqueue;
+
+ /* Don't overwrite a link TRB, just advance past it */
+ if (TRB_TYPE_LINK_LE32(trb->link.control)) {
+ /* If we're not dealing with 0.95 hardware or isoc rings
+ * on AMD 0.96 host, copy the chain bit from the previous TRB.
+ * (For 0.95 the chain bit is always set.)
+ */
+ __le32 link_control = trb->link.control;
+ if (!xhci_link_trb_quirk(xhci) &&
+ !(ring->type == TYPE_ISOC &&
+ (xhci->quirks & XHCI_AMD_0x96_HOST))) {
+ link_control &= cpu_to_le32(~TRB_CHAIN);
+ link_control |= trb[-1].link.control &
+ cpu_to_le32(TRB_CHAIN);
+ }
+
+ if (trb != ring->enqueue_first)
+ link_control ^= cpu_to_le32(TRB_CYCLE);
+ trb->link.control = link_control;
+
+ /* Advance to the next ring segment */
+ ring->enq_seg = ring->enq_seg->next;
+ ring->enqueue = ring->enq_seg->trbs;
+ trb = ring->enqueue;
- trb = &ring->enqueue->generic;
- trb->field[0] = cpu_to_le32(field1);
- trb->field[1] = cpu_to_le32(field2);
- trb->field[2] = cpu_to_le32(field3);
- trb->field[3] = cpu_to_le32(field4);
- inc_enq(xhci, ring, more_trbs_coming);
+ /* Toggle the cycle bit after the last ring segment. */
+ if (ring->enq_seg == ring->first_seg)
+ ring->cycle_state ^= TRB_CYCLE;
+ }
+
+ /*
+ * If this is the first TRB leave the cycle bit with its old value
+ * until all of the TRBs have been written.
+ * Note that the first TRB might be a NOP or LINK TRB.
+ */
+ field4 |= ring->cycle_state;
+ if (trb == ring->enqueue_first)
+ field4 ^= TRB_CYCLE;
+
+ trb->generic.field[0] = cpu_to_le32(field1);
+ trb->generic.field[1] = cpu_to_le32(field2);
+ trb->generic.field[2] = cpu_to_le32(field3);
+ trb->generic.field[3] = cpu_to_le32(field4);
+
+ ring->num_trbs_free--;
+ ring->enq_updates++;
+
+ /* Leave enqueue pointing to the next entry - which might be a LINK */
+ ring->enqueue = trb + 1;
}
/*
@@ -2972,6 +2918,8 @@ static int prepare_ring(struct xhci_hcd *xhci, struct xhci_ring *ep_ring,
return -EINVAL;
}
+ /* Save entry whose owner needs flipping at the end */
+ ep_ring->enqueue_first = ep_ring->enqueue;
while (1) {
if (room_on_ring(xhci, ep_ring, num_trbs)) {
union xhci_trb *trb = ep_ring->enqueue;
@@ -3014,13 +2962,16 @@ static int prepare_ring(struct xhci_hcd *xhci, struct xhci_ring *ep_ring,
nop_cmd = cpu_to_le32(TRB_TYPE(TRB_TR_NOOP) |
ep_ring->cycle_state);
ep_ring->num_trbs_free -= usable;
- do {
+ trb->generic.field[3] = nop_cmd ^ cpu_to_le32(TRB_CYCLE);
+ for (;;) {
trb->generic.field[0] = 0;
trb->generic.field[1] = 0;
trb->generic.field[2] = 0;
- trb->generic.field[3] = nop_cmd;
trb++;
- } while (--usable);
+ if (!--usable)
+ break;
+ trb->generic.field[3] = nop_cmd;
+ }
ep_ring->enqueue = trb;
if (room_on_ring(xhci, ep_ring, num_trbs))
break;
@@ -3041,35 +2992,7 @@ static int prepare_ring(struct xhci_hcd *xhci, struct xhci_ring *ep_ring,
}
}
- if (enqueue_is_link_trb(ep_ring)) {
- struct xhci_ring *ring = ep_ring;
- union xhci_trb *next;
-
- next = ring->enqueue;
-
- while (last_trb(xhci, ring, ring->enq_seg, next)) {
- /* If we're not dealing with 0.95 hardware or isoc rings
- * on AMD 0.96 host, clear the chain bit.
- */
- if (!xhci_link_trb_quirk(xhci) &&
- !(ring->type == TYPE_ISOC &&
- (xhci->quirks & XHCI_AMD_0x96_HOST)))
- next->link.control &= cpu_to_le32(~TRB_CHAIN);
- else
- next->link.control |= cpu_to_le32(TRB_CHAIN);
-
- wmb();
- next->link.control ^= cpu_to_le32(TRB_CYCLE);
-
- /* Toggle the cycle bit after the last ring segment. */
- if (last_trb_on_last_seg(xhci, ring, ring->enq_seg, next)) {
- ring->cycle_state = (ring->cycle_state ? 0 : 1);
- }
- ring->enq_seg = ring->enq_seg->next;
- ring->enqueue = ring->enq_seg->trbs;
- next = ring->enqueue;
- }
- }
+ /* queue_trb() will advance past a LINK TRB. */
return 0;
}
@@ -3077,7 +3000,7 @@ static int prepare_ring(struct xhci_hcd *xhci, struct xhci_ring *ep_ring,
static int prepare_transfer(struct xhci_hcd *xhci,
struct xhci_virt_device *xdev,
unsigned int ep_index,
- unsigned int stream_id,
+ struct xhci_ring *ep_ring,
unsigned int num_trbs,
struct urb *urb,
unsigned int td_index,
@@ -3086,22 +3009,16 @@ static int prepare_transfer(struct xhci_hcd *xhci,
int ret;
struct urb_priv *urb_priv;
struct xhci_td *td;
- struct xhci_ring *ep_ring;
struct xhci_ep_ctx *ep_ctx = xhci_get_ep_ctx(xhci, xdev->out_ctx, ep_index);
- ep_ring = xhci_stream_id_to_ring(xdev, ep_index, stream_id);
- if (!ep_ring) {
- xhci_dbg(xhci, "Can't prepare ring for bad stream ID %u\n",
- stream_id);
- return -EINVAL;
+ if (td_index == 0) {
+ ret = prepare_ring(xhci, ep_ring,
+ le32_to_cpu(ep_ctx->ep_info) & EP_STATE_MASK,
+ num_trbs, mem_flags);
+ if (ret)
+ return ret;
}
- ret = prepare_ring(xhci, ep_ring,
- le32_to_cpu(ep_ctx->ep_info) & EP_STATE_MASK,
- num_trbs, mem_flags);
- if (ret)
- return ret;
-
urb_priv = urb->hcpriv;
td = urb_priv->td[td_index];
@@ -3175,19 +3092,23 @@ static void check_trb_math(struct urb *urb, int num_trbs, int running_total)
}
static void giveback_first_trb(struct xhci_hcd *xhci, int slot_id,
- unsigned int ep_index, unsigned int stream_id, int start_cycle,
- struct xhci_generic_trb *start_trb)
+ unsigned int ep_index, struct xhci_ring *ring)
{
/*
* Pass all the TRBs to the hardware at once and make sure this write
* isn't reordered.
*/
wmb();
- if (start_cycle)
- start_trb->field[3] |= cpu_to_le32(start_cycle);
- else
- start_trb->field[3] &= cpu_to_le32(~TRB_CYCLE);
- xhci_ring_ep_doorbell(xhci, slot_id, ep_index, stream_id);
+ ring->enqueue_first->generic.field[3] ^= cpu_to_le32(TRB_CYCLE);
+ /*
+ * Make sure the hardware doesn't read the ring before the write
+ * above completes.
+ */
+ wmb();
+
+ /* The command doorbell is rung by the caller */
+ if (ring->type != TYPE_COMMAND)
+ xhci_ring_ep_doorbell(xhci, slot_id, ep_index, ring->stream_id);
}
/*
@@ -3289,12 +3210,7 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
int num_sgs;
int trb_buff_len, this_sg_len, running_total;
unsigned int total_packet_count;
- bool first_trb;
u64 addr;
- bool more_trbs_coming;
-
- struct xhci_generic_trb *start_trb;
- int start_cycle;
ep_ring = xhci_urb_to_transfer_ring(xhci, urb);
if (!ep_ring)
@@ -3306,22 +3222,13 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
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);
+ ep_index, ep_ring, 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;
- start_cycle = ep_ring->cycle_state;
-
running_total = 0;
/*
* How much data is in the first TRB?
@@ -3340,21 +3247,12 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
if (trb_buff_len > urb->transfer_buffer_length)
trb_buff_len = urb->transfer_buffer_length;
- first_trb = true;
/* Queue the first TRB, even if it's zero-length */
do {
u32 field = 0;
u32 length_field = 0;
u32 remainder = 0;
- /* Don't change the cycle bit of the first TRB until later */
- if (first_trb) {
- first_trb = false;
- if (start_cycle == 0)
- field |= 0x1;
- } else
- field |= ep_ring->cycle_state;
-
/* Chain all the TRBs together; clear the chain bit in the last
* TRB to indicate it's the last TRB in the chain.
*/
@@ -3362,7 +3260,6 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
field |= TRB_CHAIN;
} else {
/* FIXME - add check for ZERO_PACKET flag before this */
- td->last_trb = ep_ring->enqueue;
field |= TRB_IOC;
}
@@ -3392,11 +3289,7 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
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,
+ queue_trb(xhci, ep_ring,
lower_32_bits(addr),
upper_32_bits(addr),
length_field,
@@ -3426,10 +3319,10 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
trb_buff_len =
urb->transfer_buffer_length - running_total;
} while (running_total < urb->transfer_buffer_length);
+ td->last_trb = xhci_last_written_trb(ep_ring);
check_trb_math(urb, num_trbs, running_total);
- giveback_first_trb(xhci, slot_id, ep_index, urb->stream_id,
- start_cycle, start_trb);
+ giveback_first_trb(xhci, slot_id, ep_index, ep_ring);
return 0;
}
@@ -3441,10 +3334,6 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
struct urb_priv *urb_priv;
struct xhci_td *td;
int num_trbs;
- struct xhci_generic_trb *start_trb;
- bool first_trb;
- bool more_trbs_coming;
- int start_cycle;
u32 field, length_field;
int running_total, trb_buff_len, ret;
@@ -3477,22 +3366,13 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
/* FIXME: this doesn't deal with URB_ZERO_PACKET - need one more */
ret = prepare_transfer(xhci, xhci->devs[slot_id],
- ep_index, urb->stream_id,
- num_trbs, urb, 0, mem_flags);
+ ep_index, ep_ring, 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
- * state may change as we enqueue the other TRBs, so save it too.
- */
- start_trb = &ep_ring->enqueue->generic;
- start_cycle = ep_ring->cycle_state;
-
running_total = 0;
total_packet_count = DIV_ROUND_UP(urb->transfer_buffer_length,
usb_endpoint_maxp(&urb->ep->desc));
@@ -3503,21 +3383,11 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
if (trb_buff_len > urb->transfer_buffer_length)
trb_buff_len = urb->transfer_buffer_length;
- first_trb = true;
-
/* Queue the first TRB, even if it's zero-length */
do {
u32 remainder = 0;
field = 0;
- /* Don't change the cycle bit of the first TRB until later */
- if (first_trb) {
- first_trb = false;
- if (start_cycle == 0)
- field |= 0x1;
- } else
- field |= ep_ring->cycle_state;
-
/* Chain all the TRBs together; clear the chain bit in the last
* TRB to indicate it's the last TRB in the chain.
*/
@@ -3525,7 +3395,6 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
field |= TRB_CHAIN;
} else {
/* FIXME - add check for ZERO_PACKET flag before this */
- td->last_trb = ep_ring->enqueue;
field |= TRB_IOC;
}
@@ -3547,11 +3416,7 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
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,
+ queue_trb(xhci, ep_ring,
lower_32_bits(addr),
upper_32_bits(addr),
length_field,
@@ -3565,10 +3430,10 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
if (trb_buff_len > TRB_MAX_BUFF_SIZE)
trb_buff_len = TRB_MAX_BUFF_SIZE;
} while (running_total < urb->transfer_buffer_length);
+ td->last_trb = xhci_last_written_trb(ep_ring);
check_trb_math(urb, num_trbs, running_total);
- giveback_first_trb(xhci, slot_id, ep_index, urb->stream_id,
- start_cycle, start_trb);
+ giveback_first_trb(xhci, slot_id, ep_index, ep_ring);
return 0;
}
@@ -3580,8 +3445,6 @@ int xhci_queue_ctrl_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
int num_trbs;
int ret;
struct usb_ctrlrequest *setup;
- struct xhci_generic_trb *start_trb;
- int start_cycle;
u32 field, length_field;
struct urb_priv *urb_priv;
struct xhci_td *td;
@@ -3607,29 +3470,18 @@ 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, urb->stream_id,
- num_trbs, urb, 0, mem_flags);
+ ep_index, ep_ring, 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
- * state may change as we enqueue the other TRBs, so save it too.
- */
- start_trb = &ep_ring->enqueue->generic;
- start_cycle = ep_ring->cycle_state;
-
/* Queue setup TRB - see section 6.4.1.2.1 */
/* FIXME better way to translate setup_packet into two u32 fields? */
setup = (struct usb_ctrlrequest *) urb->setup_packet;
field = 0;
field |= TRB_IDT | TRB_TYPE(TRB_SETUP);
- if (start_cycle == 0)
- field |= 0x1;
/* xHCI 1.0 6.4.1.2.1: Transfer Type field */
if (xhci->hci_version == 0x100) {
@@ -3641,7 +3493,7 @@ int xhci_queue_ctrl_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
}
}
- queue_trb(xhci, ep_ring, true,
+ queue_trb(xhci, ep_ring,
setup->bRequestType | setup->bRequest << 8 | le16_to_cpu(setup->wValue) << 16,
le16_to_cpu(setup->wIndex) | le16_to_cpu(setup->wLength) << 16,
TRB_LEN(8) | TRB_INTR_TARGET(0),
@@ -3661,31 +3513,30 @@ int xhci_queue_ctrl_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
if (urb->transfer_buffer_length > 0) {
if (setup->bRequestType & USB_DIR_IN)
field |= TRB_DIR_IN;
- queue_trb(xhci, ep_ring, true,
+ queue_trb(xhci, ep_ring,
lower_32_bits(urb->transfer_dma),
upper_32_bits(urb->transfer_dma),
length_field,
- field | ep_ring->cycle_state);
+ field);
}
- /* Save the DMA address of the last TRB in the TD */
- td->last_trb = ep_ring->enqueue;
-
/* Queue status TRB - see Table 7 and sections 4.11.2.2 and 6.4.1.2.3 */
/* If the device sent data, the status stage is an OUT transfer */
if (urb->transfer_buffer_length > 0 && setup->bRequestType & USB_DIR_IN)
field = 0;
else
field = TRB_DIR_IN;
- queue_trb(xhci, ep_ring, false,
+ queue_trb(xhci, ep_ring,
0,
0,
TRB_INTR_TARGET(0),
/* Event on completion */
- field | TRB_IOC | TRB_TYPE(TRB_STATUS) | ep_ring->cycle_state);
+ field | TRB_IOC | TRB_TYPE(TRB_STATUS));
+
+ /* Save the address of the last TRB in the TD */
+ td->last_trb = xhci_last_written_trb(ep_ring);
- giveback_first_trb(xhci, slot_id, ep_index, 0,
- start_cycle, start_trb);
+ giveback_first_trb(xhci, slot_id, ep_index, ep_ring);
return 0;
}
@@ -3771,14 +3622,11 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
struct urb_priv *urb_priv;
struct xhci_td *td;
int num_tds, trbs_per_td;
- struct xhci_generic_trb *start_trb;
bool first_trb;
- int start_cycle;
u32 field, length_field;
int running_total, trb_buff_len, td_len, td_remain_len, ret;
u64 start_addr, addr;
int i, j;
- bool more_trbs_coming;
ep_ring = xhci->devs[slot_id]->eps[ep_index].ring;
@@ -3789,8 +3637,6 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
}
start_addr = (u64) urb->transfer_dma;
- start_trb = &ep_ring->enqueue->generic;
- start_cycle = ep_ring->cycle_state;
urb_priv = urb->hcpriv;
/* Queue the first TRB, even if it's zero-length */
@@ -3818,12 +3664,10 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
trbs_per_td = count_isoc_trbs_needed(xhci, urb, i);
ret = prepare_transfer(xhci, xhci->devs[slot_id], ep_index,
- urb->stream_id, trbs_per_td, urb, i, mem_flags);
- if (ret < 0) {
- if (i == 0)
- return ret;
- goto cleanup;
- }
+ ep_ring, trbs_per_td, urb, i, mem_flags);
+ if (ret < 0)
+ /* Can only fail for the first transfer */
+ return ret;
td = urb_priv->td[i];
for (j = 0; j < trbs_per_td; j++) {
@@ -3837,16 +3681,10 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
field |= TRB_TYPE(TRB_ISOC);
/* Assume URB_ISO_ASAP is set */
field |= TRB_SIA;
- if (i == 0) {
- if (start_cycle == 0)
- field |= 0x1;
- } else
- field |= ep_ring->cycle_state;
first_trb = false;
} else {
/* Queue other normal TRBs */
field |= TRB_TYPE(TRB_NORMAL);
- field |= ep_ring->cycle_state;
}
/* Only set interrupt on short packet for IN EPs */
@@ -3859,9 +3697,7 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
*/
if (j < trbs_per_td - 1) {
field |= TRB_CHAIN;
- more_trbs_coming = true;
} else {
- td->last_trb = ep_ring->enqueue;
field |= TRB_IOC;
if (xhci->hci_version == 0x100 &&
!(xhci->quirks &
@@ -3870,7 +3706,6 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
if (i < num_tds - 1)
field |= TRB_BEI;
}
- more_trbs_coming = false;
}
/* Calculate TRB length */
@@ -3893,7 +3728,7 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
remainder |
TRB_INTR_TARGET(0);
- queue_trb(xhci, ep_ring, more_trbs_coming,
+ queue_trb(xhci, ep_ring,
lower_32_bits(addr),
upper_32_bits(addr),
length_field,
@@ -3903,12 +3738,13 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
addr += trb_buff_len;
td_remain_len -= trb_buff_len;
}
+ td->last_trb = xhci_last_written_trb(ep_ring);
/* Check TD length */
if (running_total != td_len) {
- xhci_err(xhci, "ISOC TD length unmatch\n");
- ret = -EINVAL;
- goto cleanup;
+ /* This is a coding error */
+ xhci_err(xhci, "ISOC TD length unmatch %u != %u\n",
+ running_total, td_len);
}
}
@@ -3918,31 +3754,8 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
}
xhci_to_hcd(xhci)->self.bandwidth_isoc_reqs++;
- giveback_first_trb(xhci, slot_id, ep_index, urb->stream_id,
- start_cycle, start_trb);
+ giveback_first_trb(xhci, slot_id, ep_index, ep_ring);
return 0;
-cleanup:
- /* Clean up a partially enqueued isoc transfer. */
-
- for (i--; i >= 0; i--)
- list_del_init(&urb_priv->td[i]->td_list);
-
- /* Use the first TD as a temporary variable to turn the TDs we've queued
- * into No-ops with a software-owned cycle bit. That way the hardware
- * won't accidentally start executing bogus TDs when we partially
- * overwrite them. td->first_trb and td->start_seg are already set.
- */
- urb_priv->td[0]->last_trb = ep_ring->enqueue;
- /* Every TRB except the first & last will have its cycle bit flipped. */
- td_to_noop(xhci, ep_ring, urb_priv->td[0], true);
-
- /* Reset the ring enqueue back to the first TRB and its cycle bit. */
- ep_ring->enqueue = urb_priv->td[0]->first_trb;
- ep_ring->enq_seg = urb_priv->td[0]->start_seg;
- ep_ring->cycle_state = start_cycle;
- ep_ring->num_trbs_free = ep_ring->num_trbs_free_temp;
- usb_hcd_unlink_urb_from_ep(bus_to_hcd(urb->dev->bus), urb);
- return ret;
}
/*
@@ -4042,8 +3855,13 @@ static int queue_command(struct xhci_hcd *xhci, u32 field1, u32 field2,
"unfailable commands failed.\n");
return ret;
}
- queue_trb(xhci, xhci->cmd_ring, false, field1, field2, field3,
- field4 | xhci->cmd_ring->cycle_state);
+ queue_trb(xhci, xhci->cmd_ring, field1, field2, field3, field4);
+ /*
+ * Although we've only written a single TRB, the 'first' TRB can
+ * be a LINK TRB - which must not have its cycle bit modified
+ * until the command TRB has been written.
+ */
+ giveback_first_trb(xhci, 0, 0, xhci->cmd_ring);
return 0;
}
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 03c74b7..2e84a6f 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1315,6 +1315,7 @@ struct xhci_ring {
struct xhci_segment *first_seg;
struct xhci_segment *last_seg;
union xhci_trb *enqueue;
+ union xhci_trb *enqueue_first;
struct xhci_segment *enq_seg;
unsigned int enq_updates;
union xhci_trb *dequeue;
@@ -1853,6 +1854,12 @@ void xhci_ring_ep_doorbell(struct xhci_hcd *xhci, unsigned int slot_id,
unsigned int ep_index, unsigned int stream_id);
union xhci_trb *xhci_find_next_enqueue(struct xhci_ring *ring);
+static inline union xhci_trb *xhci_last_written_trb(struct xhci_ring *ring)
+{
+ /* The last trb that contains a real request */
+ return ring->enqueue - 1;
+}
+
/* xHCI roothub code */
void xhci_set_link_state(struct xhci_hcd *xhci, __le32 __iomem **port_array,
int port_id, u32 link_state);
--
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
