Re: [PATCH] xhci: add quirk for host controllers that don't update endpoint DCS

[kernel test robot <lkp@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 8537 bytes --]

Hi "Bjørn,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on v5.13 next-20210701]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Bj-rn-Mork/xhci-add-quirk-for-host-controllers-that-don-t-update-endpoint-DCS/20210702-151445
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: riscv-randconfig-r012-20210702 (attached as .config)
compiler: riscv32-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/61088f366a5c42caf8ce20c87355b61efc1b175d
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Bj-rn-Mork/xhci-add-quirk-for-host-controllers-that-don-t-update-endpoint-DCS/20210702-151445
        git checkout 61088f366a5c42caf8ce20c87355b61efc1b175d
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross O=build_dir ARCH=riscv SHELL=/bin/bash drivers/usb/host/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/usb/host/xhci-ring.c: In function 'xhci_move_dequeue_past_td':
>> drivers/usb/host/xhci-ring.c:613:32: error: 'cur_td' undeclared (first use in this function)
     613 |   halted_seg = trb_in_td(xhci, cur_td->start_seg,
         |                                ^~~~~~
   drivers/usb/host/xhci-ring.c:613:32: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/usb/host/xhci-ring.c:620:3: error: 'state' undeclared (first use in this function); did you mean 'statx'?
     620 |   state->new_cycle_state = halted_trb->generic.field[3] & 0x1;
         |   ^~~~~
         |   statx

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for LOCKDEP
   Depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT && (FRAME_POINTER || MIPS || PPC || S390 || MICROBLAZE || ARM || ARC || X86)
   Selected by
   - DEBUG_LOCK_ALLOC && DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT


vim +/cur_td +613 drivers/usb/host/xhci-ring.c

   552	
   553	static int xhci_move_dequeue_past_td(struct xhci_hcd *xhci,
   554					unsigned int slot_id, unsigned int ep_index,
   555					unsigned int stream_id, struct xhci_td *td)
   556	{
   557		struct xhci_virt_device *dev = xhci->devs[slot_id];
   558		struct xhci_virt_ep *ep = &dev->eps[ep_index];
   559		struct xhci_ring *ep_ring;
   560		struct xhci_command *cmd;
   561		struct xhci_segment *new_seg;
   562		struct xhci_segment *halted_seg = NULL;
   563		union xhci_trb *new_deq;
   564		int new_cycle;
   565		union xhci_trb *halted_trb;
   566		int index = 0;
   567		dma_addr_t addr;
   568		u64 hw_dequeue;
   569		bool cycle_found = false;
   570		bool td_last_trb_found = false;
   571		u32 trb_sct = 0;
   572		int ret;
   573	
   574		ep_ring = xhci_triad_to_transfer_ring(xhci, slot_id,
   575				ep_index, stream_id);
   576		if (!ep_ring) {
   577			xhci_warn(xhci, "WARN can't find new dequeue, invalid stream ID %u\n",
   578				  stream_id);
   579			return -ENODEV;
   580		}
   581		/*
   582		 * A cancelled TD can complete with a stall if HW cached the trb.
   583		 * In this case driver can't find td, but if the ring is empty we
   584		 * can move the dequeue pointer to the current enqueue position.
   585		 * We shouldn't hit this anymore as cached cancelled TRBs are given back
   586		 * after clearing the cache, but be on the safe side and keep it anyway
   587		 */
   588		if (!td) {
   589			if (list_empty(&ep_ring->td_list)) {
   590				new_seg = ep_ring->enq_seg;
   591				new_deq = ep_ring->enqueue;
   592				new_cycle = ep_ring->cycle_state;
   593				xhci_dbg(xhci, "ep ring empty, Set new dequeue = enqueue");
   594				goto deq_found;
   595			} else {
   596				xhci_warn(xhci, "Can't find new dequeue state, missing td\n");
   597				return -EINVAL;
   598			}
   599		}
   600	
   601		hw_dequeue = xhci_get_hw_deq(xhci, dev, ep_index, stream_id);
   602		new_seg = ep_ring->deq_seg;
   603		new_deq = ep_ring->dequeue;
   604		new_cycle = hw_dequeue & 0x1;
   605	
   606		/*
   607		 * Quirk: xHC write-back of the DCS field in the hardware dequeue
   608		 * pointer is wrong - use the cycle state of the TRB pointed to by
   609		 * the dequeue pointer.
   610		 */
   611		if (xhci->quirks & XHCI_EP_CTX_BROKEN_DCS &&
   612		    !(ep->ep_state & EP_HAS_STREAMS))
 > 613			halted_seg = trb_in_td(xhci, cur_td->start_seg,
   614					       cur_td->first_trb, cur_td->last_trb,
   615					       hw_dequeue & ~0xf, false);
   616		if (halted_seg) {
   617			index = ((dma_addr_t)(hw_dequeue & ~0xf) - halted_seg->dma) /
   618				 sizeof(*halted_trb);
   619			halted_trb = &halted_seg->trbs[index];
 > 620			state->new_cycle_state = halted_trb->generic.field[3] & 0x1;
   621			xhci_dbg(xhci, "Endpoint DCS = %d TRB index = %d cycle = %d\n",
   622				 (u8)(hw_dequeue & 0x1), index,
   623				 state->new_cycle_state);
   624		} else {
   625			state->new_cycle_state = hw_dequeue & 0x1;
   626		}
   627		state->stream_id = stream_id;
   628	
   629		/*
   630		 * We want to find the pointer, segment and cycle state of the new trb
   631		 * (the one after current TD's last_trb). We know the cycle state at
   632		 * hw_dequeue, so walk the ring until both hw_dequeue and last_trb are
   633		 * found.
   634		 */
   635		do {
   636			if (!cycle_found && xhci_trb_virt_to_dma(new_seg, new_deq)
   637			    == (dma_addr_t)(hw_dequeue & ~0xf)) {
   638				cycle_found = true;
   639				if (td_last_trb_found)
   640					break;
   641			}
   642			if (new_deq == td->last_trb)
   643				td_last_trb_found = true;
   644	
   645			if (cycle_found && trb_is_link(new_deq) &&
   646			    link_trb_toggles_cycle(new_deq))
   647				new_cycle ^= 0x1;
   648	
   649			next_trb(xhci, ep_ring, &new_seg, &new_deq);
   650	
   651			/* Search wrapped around, bail out */
   652			if (new_deq == ep->ring->dequeue) {
   653				xhci_err(xhci, "Error: Failed finding new dequeue state\n");
   654				return -EINVAL;
   655			}
   656	
   657		} while (!cycle_found || !td_last_trb_found);
   658	
   659	deq_found:
   660	
   661		/* Don't update the ring cycle state for the producer (us). */
   662		addr = xhci_trb_virt_to_dma(new_seg, new_deq);
   663		if (addr == 0) {
   664			xhci_warn(xhci, "Can't find dma of new dequeue ptr\n");
   665			xhci_warn(xhci, "deq seg = %p, deq ptr = %p\n", new_seg, new_deq);
   666			return -EINVAL;
   667		}
   668	
   669		if ((ep->ep_state & SET_DEQ_PENDING)) {
   670			xhci_warn(xhci, "Set TR Deq already pending, don't submit for 0x%pad\n",
   671				  &addr);
   672			return -EBUSY;
   673		}
   674	
   675		/* This function gets called from contexts where it cannot sleep */
   676		cmd = xhci_alloc_command(xhci, false, GFP_ATOMIC);
   677		if (!cmd) {
   678			xhci_warn(xhci, "Can't alloc Set TR Deq cmd 0x%pad\n", &addr);
   679			return -ENOMEM;
   680		}
   681	
   682		if (stream_id)
   683			trb_sct = SCT_FOR_TRB(SCT_PRI_TR);
   684		ret = queue_command(xhci, cmd,
   685			lower_32_bits(addr) | trb_sct | new_cycle,
   686			upper_32_bits(addr),
   687			STREAM_ID_FOR_TRB(stream_id), SLOT_ID_FOR_TRB(slot_id) |
   688			EP_ID_FOR_TRB(ep_index) | TRB_TYPE(TRB_SET_DEQ), false);
   689		if (ret < 0) {
   690			xhci_free_command(xhci, cmd);
   691			return ret;
   692		}
   693		ep->queued_deq_seg = new_seg;
   694		ep->queued_deq_ptr = new_deq;
   695	
   696		xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
   697			       "Set TR Deq ptr 0x%llx, cycle %u\n", addr, new_cycle);
   698	
   699		/* Stop the TD queueing code from ringing the doorbell until
   700		 * this command completes.  The HC won't set the dequeue pointer
   701		 * if the ring is running, and ringing the doorbell starts the
   702		 * ring running.
   703		 */
   704		ep->ep_state |= SET_DEQ_PENDING;
   705		xhci_ring_cmd_db(xhci);
   706		return 0;
   707	}
   708	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35791 bytes --]