[PATCH v2] usb: xhci: Workaround to fix the USB halted endpoint issues

Venkatesh Yadav Abbarapu venkatesh.abbarapu at amd.com
Fri Oct 13 06:53:56 CEST 2023


The xhci host controller driver trying to queue the URB's and it is
getting halted at the endpoint, thereby hitting the BUG_ON's.
Mostly these kind of random issues are seen on faulty boards.
Removing these BUG_ON's from the U-Boot xhci code, as in Linux kernel
xhci code BUG_ON/BUG's are removed entirely.
Please also note, that BUG_ON() is not recommended any more in the Linux
kernel.
Similar issue has been observed on TI AM437x board and they created a patch
in Linux kernel as below
https://patches.linaro.org/project/linux-usb/patch/1390250711-25840-1-git-send-email-balbi@ti.com/

Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbarapu at amd.com>
---
Changes in v2:
 -Fixed the compilation warning.
---
 drivers/usb/host/xhci-mem.c  | 17 -----------------
 drivers/usb/host/xhci-ring.c | 32 ++++++++------------------------
 drivers/usb/host/xhci.c      |  6 ------
 include/usb/xhci.h           |  2 +-
 4 files changed, 9 insertions(+), 48 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 72b7530626..0efb4bd7ba 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -36,8 +36,6 @@
  */
 void xhci_flush_cache(uintptr_t addr, u32 len)
 {
-	BUG_ON((void *)addr == NULL || len == 0);
-
 	flush_dcache_range(addr & ~(CACHELINE_SIZE - 1),
 				ALIGN(addr + len, CACHELINE_SIZE));
 }
@@ -51,8 +49,6 @@ void xhci_flush_cache(uintptr_t addr, u32 len)
  */
 void xhci_inval_cache(uintptr_t addr, u32 len)
 {
-	BUG_ON((void *)addr == NULL || len == 0);
-
 	invalidate_dcache_range(addr & ~(CACHELINE_SIZE - 1),
 				ALIGN(addr + len, CACHELINE_SIZE));
 }
@@ -84,8 +80,6 @@ static void xhci_ring_free(struct xhci_ctrl *ctrl, struct xhci_ring *ring)
 	struct xhci_segment *seg;
 	struct xhci_segment *first_seg;
 
-	BUG_ON(!ring);
-
 	first_seg = ring->first_seg;
 	seg = first_seg->next;
 	while (seg != first_seg) {
@@ -210,7 +204,6 @@ static void *xhci_malloc(unsigned int size)
 	size_t cacheline_size = max(XHCI_ALIGNMENT, CACHELINE_SIZE);
 
 	ptr = memalign(cacheline_size, ALIGN(size, cacheline_size));
-	BUG_ON(!ptr);
 	memset(ptr, '\0', size);
 
 	xhci_flush_cache((uintptr_t)ptr, size);
@@ -291,7 +284,6 @@ static struct xhci_segment *xhci_segment_alloc(struct xhci_ctrl *ctrl)
 	struct xhci_segment *seg;
 
 	seg = malloc(sizeof(struct xhci_segment));
-	BUG_ON(!seg);
 
 	seg->trbs = xhci_malloc(SEGMENT_SIZE);
 	seg->dma = xhci_dma_map(ctrl, seg->trbs, SEGMENT_SIZE);
@@ -323,13 +315,11 @@ struct xhci_ring *xhci_ring_alloc(struct xhci_ctrl *ctrl, unsigned int num_segs,
 	struct xhci_segment *prev;
 
 	ring = malloc(sizeof(struct xhci_ring));
-	BUG_ON(!ring);
 
 	if (num_segs == 0)
 		return ring;
 
 	ring->first_seg = xhci_segment_alloc(ctrl);
-	BUG_ON(!ring->first_seg);
 
 	num_segs--;
 
@@ -338,7 +328,6 @@ struct xhci_ring *xhci_ring_alloc(struct xhci_ctrl *ctrl, unsigned int num_segs,
 		struct xhci_segment *next;
 
 		next = xhci_segment_alloc(ctrl);
-		BUG_ON(!next);
 
 		xhci_link_segments(ctrl, prev, next, link_trbs);
 
@@ -399,7 +388,6 @@ static int xhci_scratchpad_alloc(struct xhci_ctrl *ctrl)
 			break;
 		page_size = page_size >> 1;
 	}
-	BUG_ON(i == 16);
 
 	ctrl->page_size = 1 << (i + 12);
 	buf = memalign(ctrl->page_size, num_sp * ctrl->page_size);
@@ -444,9 +432,7 @@ static struct xhci_container_ctx
 	struct xhci_container_ctx *ctx;
 
 	ctx = malloc(sizeof(struct xhci_container_ctx));
-	BUG_ON(!ctx);
 
-	BUG_ON((type != XHCI_CTX_TYPE_DEVICE) && (type != XHCI_CTX_TYPE_INPUT));
 	ctx->type = type;
 	ctx->size = (MAX_EP_CTX_NUM + 1) *
 			CTX_SIZE(xhci_readl(&ctrl->hccr->cr_hccparams));
@@ -638,7 +624,6 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
 struct xhci_input_control_ctx
 		*xhci_get_input_control_ctx(struct xhci_container_ctx *ctx)
 {
-	BUG_ON(ctx->type != XHCI_CTX_TYPE_INPUT);
 	return (struct xhci_input_control_ctx *)ctx->bytes;
 }
 
@@ -760,8 +745,6 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl,
 
 	virt_dev = ctrl->devs[slot_id];
 
-	BUG_ON(!virt_dev);
-
 	/* Extract the EP0 and Slot Ctrl */
 	ep0_ctx = xhci_get_ep_ctx(ctrl, virt_dev->in_ctx, 0);
 	slot_ctx = xhci_get_slot_ctx(ctrl, virt_dev->in_ctx);
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index c8260cbdf9..5e9273c5a6 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -244,6 +244,7 @@ static int prepare_ring(struct xhci_ctrl *ctrl, struct xhci_ring *ep_ring,
 		return -EINVAL;
 	case EP_STATE_HALTED:
 		puts("WARN halted endpoint, queueing URB anyway.\n");
+		return -EPIPE;
 	case EP_STATE_STOPPED:
 	case EP_STATE_RUNNING:
 		debug("EP STATE RUNNING.\n");
@@ -287,12 +288,15 @@ static int prepare_ring(struct xhci_ctrl *ctrl, struct xhci_ring *ep_ring,
  * @param cmd		Command type to enqueue
  * Return: none
  */
-void xhci_queue_command(struct xhci_ctrl *ctrl, dma_addr_t addr, u32 slot_id,
+int xhci_queue_command(struct xhci_ctrl *ctrl, dma_addr_t addr, u32 slot_id,
 			u32 ep_index, trb_type cmd)
 {
 	u32 fields[4];
+	int ret;
 
-	BUG_ON(prepare_ring(ctrl, ctrl->cmd_ring, EP_STATE_RUNNING));
+	ret = prepare_ring(ctrl, ctrl->cmd_ring, EP_STATE_RUNNING);
+	if (ret < 0)
+		return ret;
 
 	fields[0] = lower_32_bits(addr);
 	fields[1] = upper_32_bits(addr);
@@ -311,6 +315,7 @@ void xhci_queue_command(struct xhci_ctrl *ctrl, dma_addr_t addr, u32 slot_id,
 
 	/* Ring the command ring doorbell */
 	xhci_writel(&ctrl->dba->doorbell[0], DB_VALUE_HOST);
+	return 0;
 }
 
 /*
@@ -493,7 +498,7 @@ union xhci_trb *xhci_wait_for_event(struct xhci_ctrl *ctrl, trb_type expected)
 		return NULL;
 
 	printf("XHCI timeout on event type %d... cannot recover.\n", expected);
-	BUG();
+	return NULL;
 }
 
 /*
@@ -512,16 +517,12 @@ static void reset_ep(struct usb_device *udev, int ep_index)
 	xhci_queue_command(ctrl, 0, udev->slot_id, ep_index, TRB_RESET_EP);
 	event = xhci_wait_for_event(ctrl, TRB_COMPLETION);
 	field = le32_to_cpu(event->trans_event.flags);
-	BUG_ON(TRB_TO_SLOT_ID(field) != udev->slot_id);
 	xhci_acknowledge_event(ctrl);
 
 	addr = xhci_trb_virt_to_dma(ring->enq_seg,
 		(void *)((uintptr_t)ring->enqueue | ring->cycle_state));
 	xhci_queue_command(ctrl, addr, udev->slot_id, ep_index, TRB_SET_DEQ);
 	event = xhci_wait_for_event(ctrl, TRB_COMPLETION);
-	BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags))
-		!= udev->slot_id || GET_COMP_CODE(le32_to_cpu(
-		event->event_cmd.status)) != COMP_SUCCESS);
 	xhci_acknowledge_event(ctrl);
 }
 
@@ -545,25 +546,15 @@ static void abort_td(struct usb_device *udev, int ep_index)
 
 	event = xhci_wait_for_event(ctrl, TRB_TRANSFER);
 	field = le32_to_cpu(event->trans_event.flags);
-	BUG_ON(TRB_TO_SLOT_ID(field) != udev->slot_id);
-	BUG_ON(TRB_TO_EP_INDEX(field) != ep_index);
-	BUG_ON(GET_COMP_CODE(le32_to_cpu(event->trans_event.transfer_len
-		!= COMP_STOP)));
 	xhci_acknowledge_event(ctrl);
 
 	event = xhci_wait_for_event(ctrl, TRB_COMPLETION);
-	BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags))
-		!= udev->slot_id || GET_COMP_CODE(le32_to_cpu(
-		event->event_cmd.status)) != COMP_SUCCESS);
 	xhci_acknowledge_event(ctrl);
 
 	addr = xhci_trb_virt_to_dma(ring->enq_seg,
 		(void *)((uintptr_t)ring->enqueue | ring->cycle_state));
 	xhci_queue_command(ctrl, addr, udev->slot_id, ep_index, TRB_SET_DEQ);
 	event = xhci_wait_for_event(ctrl, TRB_COMPLETION);
-	BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags))
-		!= udev->slot_id || GET_COMP_CODE(le32_to_cpu(
-		event->event_cmd.status)) != COMP_SUCCESS);
 	xhci_acknowledge_event(ctrl);
 }
 
@@ -781,8 +772,6 @@ again:
 	}
 
 	field = le32_to_cpu(event->trans_event.flags);
-	BUG_ON(TRB_TO_SLOT_ID(field) != slot_id);
-	BUG_ON(TRB_TO_EP_INDEX(field) != ep_index);
 
 	record_transfer_result(udev, event, available_length);
 	xhci_acknowledge_event(ctrl);
@@ -970,9 +959,6 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
 		goto abort;
 	field = le32_to_cpu(event->trans_event.flags);
 
-	BUG_ON(TRB_TO_SLOT_ID(field) != slot_id);
-	BUG_ON(TRB_TO_EP_INDEX(field) != ep_index);
-
 	record_transfer_result(udev, event, length);
 	xhci_acknowledge_event(ctrl);
 	if (udev->status == USB_ST_STALLED) {
@@ -992,8 +978,6 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
 		event = xhci_wait_for_event(ctrl, TRB_TRANSFER);
 		if (!event)
 			goto abort;
-		BUG_ON(TRB_TO_SLOT_ID(field) != slot_id);
-		BUG_ON(TRB_TO_EP_INDEX(field) != ep_index);
 		xhci_acknowledge_event(ctrl);
 	}
 
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 5cacf0769e..13b226a3f5 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -451,8 +451,6 @@ static int xhci_configure_endpoints(struct usb_device *udev, bool ctx_change)
 	xhci_queue_command(ctrl, in_ctx->dma, udev->slot_id, 0,
 			   ctx_change ? TRB_EVAL_CONTEXT : TRB_CONFIG_EP);
 	event = xhci_wait_for_event(ctrl, TRB_COMPLETION);
-	BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags))
-		!= udev->slot_id);
 
 	switch (GET_COMP_CODE(le32_to_cpu(event->event_cmd.status))) {
 	case COMP_SUCCESS:
@@ -647,7 +645,6 @@ static int xhci_address_device(struct usb_device *udev, int root_portnr)
 	xhci_queue_command(ctrl, virt_dev->in_ctx->dma,
 			   slot_id, 0, TRB_ADDR_DEV);
 	event = xhci_wait_for_event(ctrl, TRB_COMPLETION);
-	BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags)) != slot_id);
 
 	switch (GET_COMP_CODE(le32_to_cpu(event->event_cmd.status))) {
 	case COMP_CTX_STATE:
@@ -722,8 +719,6 @@ static int _xhci_alloc_device(struct usb_device *udev)
 
 	xhci_queue_command(ctrl, 0, 0, 0, TRB_ENABLE_SLOT);
 	event = xhci_wait_for_event(ctrl, TRB_COMPLETION);
-	BUG_ON(GET_COMP_CODE(le32_to_cpu(event->event_cmd.status))
-		!= COMP_SUCCESS);
 
 	udev->slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags));
 
@@ -1325,7 +1320,6 @@ static int xhci_update_hub_device(struct udevice *dev, struct usb_device *udev)
 		return 0;
 
 	virt_dev = ctrl->devs[slot_id];
-	BUG_ON(!virt_dev);
 
 	out_ctx = virt_dev->out_ctx;
 	in_ctx = virt_dev->in_ctx;
diff --git a/include/usb/xhci.h b/include/usb/xhci.h
index 4a4ac10229..3046b9e411 100644
--- a/include/usb/xhci.h
+++ b/include/usb/xhci.h
@@ -1253,7 +1253,7 @@ void xhci_slot_copy(struct xhci_ctrl *ctrl,
 		    struct xhci_container_ctx *out_ctx);
 void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl,
 				     struct usb_device *udev, int hop_portnr);
-void xhci_queue_command(struct xhci_ctrl *ctrl, dma_addr_t addr,
+int xhci_queue_command(struct xhci_ctrl *ctrl, dma_addr_t addr,
 			u32 slot_id, u32 ep_index, trb_type cmd);
 void xhci_acknowledge_event(struct xhci_ctrl *ctrl);
 union xhci_trb *xhci_wait_for_event(struct xhci_ctrl *ctrl, trb_type expected);
-- 
2.17.1



More information about the U-Boot mailing list