All right, so we have a couple sparse warnings with this patch.
On Thu, Oct 17, 2013 at 01:40:15PM +0800, kbuild test robot wrote:
> tree: git://git.kernel.org/pub/scm/linux/kernel/git/sarah/xhci.git fun-streams-fixes
> head: e79db70b0f7d028cb9e9cf8ee424a1e92b7d232f
> commit: 0f899d55a18ce75eed9dccd2cc2cd6d333f9ff16 [33/48] xhci: For streams the dequeue ptr must be read from the stream ctx
> reproduce: make C=1 CF=-D__CHECK_ENDIAN__
>
>
> sparse warnings: (new ones prefixed by >>)
>
> >> drivers/usb/host/xhci-ring.c:1154:29: sparse: incorrect type in assignment (different base types)
> drivers/usb/host/xhci-ring.c:1154:29: expected restricted __le64 [usertype] deq
> drivers/usb/host/xhci-ring.c:1154:29: got unsigned long long
> >> drivers/usb/host/xhci-ring.c:1156:29: sparse: incorrect type in assignment (different base types)
> drivers/usb/host/xhci-ring.c:1156:29: expected restricted __le64 [usertype] deq
> drivers/usb/host/xhci-ring.c:1156:29: got unsigned long long
> >> drivers/usb/host/xhci-ring.c:1161:65: sparse: restricted __le64 degrades to integer
> drivers/usb/host/xhci-ring.c:1674:19: sparse: restricted __le32 degrades to integer
>
> vim +1154 drivers/usb/host/xhci-ring.c
>
> 1148 } else {
> 1149 __le64 deq;
> 1150 /* 4.6.10 deq ptr is written to the stream ctx for streams */
> 1151 if (ep->ep_state & EP_HAS_STREAMS) {
> 1152 struct xhci_stream_ctx *ctx =
> 1153 &ep->stream_info->stream_ctx_array[stream_id];
> > 1154 deq = le64_to_cpu(ctx->stream_ring) & SCTX_DEQ_MASK;
> 1155 } else {
> 1156 deq = le64_to_cpu(ep_ctx->deq) & ~EP_CTX_CYCLE_MASK;
> 1157 }
> 1158 xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
> 1159 "Successful Set TR Deq Ptr cmd, deq = @%08llx", deq);
> 1160 if (xhci_trb_virt_to_dma(ep->queued_deq_seg,
> 1161 ep->queued_deq_ptr) == deq) {
> 1162 /* Update the ring's dequeue segment and dequeue pointer
> 1163 * to reflect the new position.
> 1164 */
>
> ---
> 0-DAY kernel build testing backend Open Source Technology Center
> http://lists.01.org/mailman/listinfo/kbuild Intel Corporation
Looking at the patch itself:
+ __le64 deq;
+ /* 4.6.10 deq ptr is written to the stream ctx for streams */
+ if (ep->ep_state & EP_HAS_STREAMS) {
+ struct xhci_stream_ctx *ctx =
+ &ep->stream_info->stream_ctx_array[stream_id];
+ deq = le64_to_cpu(ctx->stream_ring) & SCTX_DEQ_MASK;
+ } else {
+ deq = le64_to_cpu(ep_ctx->deq) & ~EP_CTX_CYCLE_MASK;
+ }
xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
- "Successful Set TR Deq Ptr cmd, deq = @%08llx",
- le64_to_cpu(ep_ctx->deq));
- if (xhci_trb_virt_to_dma(dev->eps[ep_index].queued_deq_seg,
- dev->eps[ep_index].queued_deq_ptr) ==
- (le64_to_cpu(ep_ctx->deq) & ~(EP_CTX_CYCLE_MASK))) {
+ "Successful Set TR Deq Ptr cmd, deq = @%08llx", deq);
+ if (xhci_trb_virt_to_dma(ep->queued_deq_seg,
+ ep->queued_deq_ptr) == deq) {
/* Update the ring's dequeue segment and dequeue pointer
* to reflect the new position.
*/
I don't think it makes sense to use __le64 for the deq variable. It
should probably be dma_addr_t, which is what xhci_trb_virt_to_dma
returns.
Can you fix this?
I also wondered if we could have multiple Set TR dequeue
commands pending for different stream rings, and thus overwrite the
endpoint queued_deq_seg and queued_dequeue_ptr fields. However, I
convinced myself that isn't possible.
When the Stop Endpoint command completes, all stream rings will stop,
but the host will indicate the stream ring that it was in the middle of
processing. That will get stored in xhci_virt_ep->stopped_td, and thus
only one of the stream rings could possibly need a Set TR Dequeue
command at a time. The rest will have canceled TDs get turned into
no-ops.
Sarah Sharp
--
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
