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

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-se...
Signed-off-by: Venkatesh Yadav Abbarapu venkatesh.abbarapu@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(); }
/* @@ -512,16 +516,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 +545,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 +771,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 +958,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 +977,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);

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-se...
Signed-off-by: Venkatesh Yadav Abbarapu venkatesh.abbarapu@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");
case EP_STATE_STOPPED: case EP_STATE_RUNNING: debug("EP STATE RUNNING.\n");return -EPIPE;
@@ -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
/*
participants (2)
-
Michal Simek
-
Venkatesh Yadav Abbarapu