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

Michal Simek michal.simek at amd.com
Thu Oct 12 16:46:24 CEST 2023



On 9/12/23 05:59, Venkatesh Yadav Abbarapu wrote:
> 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>
> ---
>   drivers/usb/host/xhci-mem.c  | 17 -----------------
>   drivers/usb/host/xhci-ring.c | 31 +++++++------------------------
>   drivers/usb/host/xhci.c      |  6 ------
>   include/usb/xhci.h           |  2 +-
>   4 files changed, 8 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..1bde0b9793 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,6 @@ 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();

There is compilation warning coming from this.

w+../drivers/usb/host/xhci-ring.c: In function ‘xhci_wait_for_event’:
w+../drivers/usb/host/xhci-ring.c:501:1: warning: control reaches end of 
non-void function [-Wreturn-type]

you should fix it.


diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 1bde0b9793c4..087ef63f86a4 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -498,6 +498,8 @@ 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);
+
+       return NULL;
  }

Thanks,
Michal

  /*


More information about the U-Boot mailing list