+hdegoede
> I tried to apply this patch on top of 3.15-rc1, but it fails because of the
> streams support added to xhci_find_new_dequeue_state()
>
> After some manual editing the interesting parts of
> xhci_find_new_dequeue_state() looks like this:
>
> @@ -577,46 +568,57 @@ void xhci_find_new_dequeue_state(struct xhci_hcd
> *xhci,
> if (ep->ep_state & EP_HAS_STREAMS) {
> struct xhci_stream_ctx *ctx =
> &ep->stream_info->stream_ctx_array[stream_id];
> - state->new_cycle_state = 0x1 &
> le64_to_cpu(ctx->stream_ring);
> + hw_dequeue = le64_to_cpu(ctx->stream_ring);
> } else {
> struct xhci_ep_ctx *ep_ctx
>
> = xhci_get_ep_ctx(xhci, dev->out_ctx, ep_index);
> - state->new_cycle_state = 0x1 & le64_to_cpu(ep_ctx->deq);
> + 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 & ~0x1)) {
> + 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;
> + }
> + }
>
> Also the comparison of the dequeue pointers, using (hw_dequeue & ~0x1) might
> have some troubles with streams. Endpoint context TR dequeue pointer LO
> field has bits 3:1 reserved (probably zero) but stream context uses those
> bits. Would it make sense to use (hw_dequeue & ~0xf) here instead?
Ah, yes, looks like that patch wasn't in Linus' tree yet back when I
wrote this. I think your merge looks pretty good... just use
(hw_dequeue & ~0xf) instead of (hw_dequeue & ~0x1) to get the pointer
as you said, and this should work fine.
> But I'm still concerned about the dequeue pointer in the streams case.
> streams may be nested, we might be pointing at another stream context
> instead of the dequeue pointer.
>
> So there's still some work needed. Are you interested in re-working this to
> fit on top of 3.15-rc1 or should I add it to my todo list?
Hmmm... maybe the stream_id parameter is already pointing to the
correct secondary stream (if applicable) so you can rely on having a
normal dequeue pointer there? The xhci_triad_to_transfer_ring()
function seems to make the same assumption... At any rate, if there is
a problem here it would also be in the original c4bedb77e ("xhci: For
streams the css flag most be read from the stream-ctx on ep stop")
already, so I think you should follow up with that patch's author and
fix it in a separate commit if necessary.
I unfortunately don't have a device using streams to test with, so I
couldn't do more for this patch than you've already done. I think if
you change the hw_dequeue masking as you said that should guarantee
that the patch at least won't make things worse for streams, and that
should be good enough.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
