I don't get the call stack dump with this patch, but I'm still getting
a few odd errors ... both of these repeat pretty much constantly as
long as I have the tuner on the XHCI port. I'm going back to make
sure I didn't do anything wrong.
[ 4679.593698] xhci_hcd 0000:00:10.0: ERROR Transfer event TRB DMA ptr
not part of current TD
[ 5249.398157] xhci_hcd 0000:00:10.0: Failed finding new dequeue state
On Tue, Jul 29, 2014 at 8:28 PM, Maciej Puzio <mx34567@xxxxxxxxx> wrote:
> I tested the patch with kernel 3.2.61 and Asmedia ASM1042A controller
> (the problematic combination), and the patch fixes the regression.
>
> On Tue, Jul 29, 2014 at 11:35 AM, Mathias Nyman
> <mathias.nyman@xxxxxxxxxxxxxxx> wrote:
>> When we manually need to move the TR dequeue pointer we need to set the
>> correct cycle bit as well. Previously we used the trb pointer from the
>> last event received as a base, but this was changed in Commit 1f81b6d22
>> to use the dequeue pointer from the endpoint context instead.
>>
>> It turns out some Asmedia controllers advance the dequeue pointer
>> stored in the endpoint context past the event triggering TRB, and
>> this messed up the way the cycle bit was calculated.
>>
>> Instead of adding a quirk or complicating the already hard to follow cycle bit
>> code, the whole cycle bit calculation is now simplified and adapted to handle
>> event and endpoint context dequeue pointer differences.
>>
>> Signed-off-by: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
>> ---
>> drivers/usb/host/xhci-ring.c | 99 +++++++++++++++++---------------------------
>> 1 file changed, 37 insertions(+), 62 deletions(-)
>>
>> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
>> index 749fc68..a74e647 100644
>> --- a/drivers/usb/host/xhci-ring.c
>> +++ b/drivers/usb/host/xhci-ring.c
>> @@ -364,32 +364,6 @@ static void ring_doorbell_for_active_rings(struct xhci_hcd *xhci,
>> }
>> }
>>
>> -/*
>> - * Find the segment that trb is in. Start searching in start_seg.
>> - * If we must move past a segment that has a link TRB with a toggle cycle state
>> - * bit set, then we will toggle the value pointed at by cycle_state.
>> - */
>> -static struct xhci_segment *find_trb_seg(
>> - struct xhci_segment *start_seg,
>> - union xhci_trb *trb, int *cycle_state)
>> -{
>> - struct xhci_segment *cur_seg = start_seg;
>> - struct xhci_generic_trb *generic_trb;
>> -
>> - while (cur_seg->trbs > trb ||
>> - &cur_seg->trbs[TRBS_PER_SEGMENT - 1] < trb) {
>> - generic_trb = &cur_seg->trbs[TRBS_PER_SEGMENT - 1].generic;
>> - if (generic_trb->field[3] & cpu_to_le32(LINK_TOGGLE))
>> - *cycle_state ^= 0x1;
>> - cur_seg = cur_seg->next;
>> - if (cur_seg == start_seg)
>> - /* Looped over the entire list. Oops! */
>> - return NULL;
>> - }
>> - return cur_seg;
>> -}
>> -
>> -
>> static struct xhci_ring *xhci_triad_to_transfer_ring(struct xhci_hcd *xhci,
>> unsigned int slot_id, unsigned int ep_index,
>> unsigned int stream_id)
>> @@ -459,9 +433,12 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci,
>> struct xhci_virt_device *dev = xhci->devs[slot_id];
>> struct xhci_virt_ep *ep = &dev->eps[ep_index];
>> struct xhci_ring *ep_ring;
>> - struct xhci_generic_trb *trb;
>> + struct xhci_segment *new_seg;
>> + union xhci_trb *new_deq;
>> dma_addr_t addr;
>> u64 hw_dequeue;
>> + bool cycle_found = false;
>> + bool td_last_trb_found = false;
>>
>> ep_ring = xhci_triad_to_transfer_ring(xhci, slot_id,
>> ep_index, stream_id);
>> @@ -486,45 +463,43 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci,
>> hw_dequeue = le64_to_cpu(ep_ctx->deq);
>> }
>>
>> - /* Find virtual address and segment of hardware dequeue pointer */
>> - state->new_deq_seg = ep_ring->deq_seg;
>> - state->new_deq_ptr = ep_ring->dequeue;
>> - while (xhci_trb_virt_to_dma(state->new_deq_seg, state->new_deq_ptr)
>> - != (dma_addr_t)(hw_dequeue & ~0xf)) {
>> - next_trb(xhci, ep_ring, &state->new_deq_seg,
>> - &state->new_deq_ptr);
>> - if (state->new_deq_ptr == ep_ring->dequeue) {
>> - WARN_ON(1);
>> - return;
>> - }
>> - }
>> + new_seg = ep_ring->deq_seg;
>> + new_deq = ep_ring->dequeue;
>> + state->new_cycle_state = hw_dequeue & 0x1;
>> +
>> /*
>> - * Find cycle state for last_trb, starting at old cycle state of
>> - * hw_dequeue. If there is only one segment ring, find_trb_seg() will
>> - * return immediately and cannot toggle the cycle state if this search
>> - * wraps around, so add one more toggle manually in that case.
>> + * We want to find the pointer, segment and cycle state of the new trb
>> + * (the one after current TD's last_trb). We know the cycle state at
>> + * hw_dequeue, so walk the ring until both hw_dequeue and last_trb are
>> + * found.
>> */
>> - state->new_cycle_state = hw_dequeue & 0x1;
>> - if (ep_ring->first_seg == ep_ring->first_seg->next &&
>> - cur_td->last_trb < state->new_deq_ptr)
>> - state->new_cycle_state ^= 0x1;
>> + do {
>> + if (!cycle_found && xhci_trb_virt_to_dma(new_seg, new_deq)
>> + == (dma_addr_t)(hw_dequeue & ~0xf)) {
>> + cycle_found = true;
>> + if (td_last_trb_found)
>> + break;
>> + }
>> + if (new_deq == cur_td->last_trb)
>> + td_last_trb_found = true;
>>
>> - state->new_deq_ptr = cur_td->last_trb;
>> - xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
>> - "Finding segment containing last TRB in TD.");
>> - state->new_deq_seg = find_trb_seg(state->new_deq_seg,
>> - state->new_deq_ptr, &state->new_cycle_state);
>> - if (!state->new_deq_seg) {
>> - WARN_ON(1);
>> - return;
>> - }
>> + if (cycle_found &&
>> + TRB_TYPE_LINK_LE32(new_deq->generic.field[3]) &&
>> + new_deq->generic.field[3] & cpu_to_le32(LINK_TOGGLE))
>> + state->new_cycle_state ^= 0x1;
>> +
>> + next_trb(xhci, ep_ring, &new_seg, &new_deq);
>> +
>> + /* Search wrapped around, bail out */
>> + if (new_deq == ep->ring->dequeue) {
>> + xhci_err(xhci, "Failed finding new dequeue state\n");
>> + break;
>> + }
>> +
>> + } while (!cycle_found || !td_last_trb_found);
>>
>> - /* Increment to find next TRB after last_trb. Cycle if appropriate. */
>> - trb = &state->new_deq_ptr->generic;
>> - if (TRB_TYPE_LINK_LE32(trb->field[3]) &&
>> - (trb->field[3] & cpu_to_le32(LINK_TOGGLE)))
>> - state->new_cycle_state ^= 0x1;
>> - next_trb(xhci, ep_ring, &state->new_deq_seg, &state->new_deq_ptr);
>> + state->new_deq_seg = new_seg;
>> + state->new_deq_ptr = new_deq;
>>
>> /* Don't update the ring cycle state for the producer (us). */
>> xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
>> --
>> 1.8.3.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
