Greetings,
This is a note to let Dmitry, Paul, and Luben know that while testing
their patches against Greg's usb-linus branch, my kernel crashed when I
plugged in a USB 3.0 hard drive. This was the first device to be
plugged into the host after driver load, so it should be fairly easy to
reproduce.
At this point, I'm not sure whether one of the patches are to blame, or
if the problem lies somewhere in Greg's tree. I'm compiling a kernel to
do diagnosis now. Until I find the underlying cause, I'm not going to
be able to send any of your patches off to Greg. Thanks for your
patience.
Sarah Sharp
On Tue, Feb 08, 2011 at 01:55:59PM -0800, Dmitry Torokhov wrote:
> From 37858b400aef2472a85d21b067124864a8757534 Mon Sep 17 00:00:00 2001
> From: Dmitry Torokhov <dtor@xxxxxxxxxx>
> Date: Tue, 8 Feb 2011 13:16:51 -0800
> Subject: [PATCH] USB: xhci - mark local fucntions as static
>
> Functions that are not used outsde of the module they are defined
> should be marked as static.
>
> Signed-off-by: Dmitry Torokhov <dtor@xxxxxxxxxx>
> ---
> drivers/usb/host/xhci-dbg.c | 4 ++--
> drivers/usb/host/xhci-mem.c | 4 ++--
> drivers/usb/host/xhci.c | 8 ++++----
> 3 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-dbg.c b/drivers/usb/host/xhci-dbg.c
> index fcbf4ab..4a83350 100644
> --- a/drivers/usb/host/xhci-dbg.c
> +++ b/drivers/usb/host/xhci-dbg.c
> @@ -449,7 +449,7 @@ char *xhci_get_slot_state(struct xhci_hcd *xhci,
> }
> }
>
> -void xhci_dbg_slot_ctx(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx)
> +static void xhci_dbg_slot_ctx(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx)
> {
> /* Fields are 32 bits wide, DMA addresses are in bytes */
> int field_size = 32 / 8;
> @@ -488,7 +488,7 @@ void xhci_dbg_slot_ctx(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx)
> dbg_rsvd64(xhci, (u64 *)slot_ctx, dma);
> }
>
> -void xhci_dbg_ep_ctx(struct xhci_hcd *xhci,
> +static void xhci_dbg_ep_ctx(struct xhci_hcd *xhci,
> struct xhci_container_ctx *ctx,
> unsigned int last_ep)
> {
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index 1d0f45f..7604d61 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -307,7 +307,7 @@ struct xhci_ep_ctx *xhci_get_ep_ctx(struct xhci_hcd *xhci,
>
> /***************** Streams structures manipulation *************************/
>
> -void xhci_free_stream_ctx(struct xhci_hcd *xhci,
> +static void xhci_free_stream_ctx(struct xhci_hcd *xhci,
> unsigned int num_stream_ctxs,
> struct xhci_stream_ctx *stream_ctx, dma_addr_t dma)
> {
> @@ -335,7 +335,7 @@ void xhci_free_stream_ctx(struct xhci_hcd *xhci,
> * The stream context array must be a power of 2, and can be as small as
> * 64 bytes or as large as 1MB.
> */
> -struct xhci_stream_ctx *xhci_alloc_stream_ctx(struct xhci_hcd *xhci,
> +static struct xhci_stream_ctx *xhci_alloc_stream_ctx(struct xhci_hcd *xhci,
> unsigned int num_stream_ctxs, dma_addr_t *dma,
> gfp_t mem_flags)
> {
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 34cf4e1..4c05777 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -109,7 +109,7 @@ int xhci_halt(struct xhci_hcd *xhci)
> /*
> * Set the run bit and wait for the host to be running.
> */
> -int xhci_start(struct xhci_hcd *xhci)
> +static int xhci_start(struct xhci_hcd *xhci)
> {
> u32 temp;
> int ret;
> @@ -329,7 +329,7 @@ int xhci_init(struct usb_hcd *hcd)
>
>
> #ifdef CONFIG_USB_XHCI_HCD_DEBUGGING
> -void xhci_event_ring_work(unsigned long arg)
> +static void xhci_event_ring_work(unsigned long arg)
> {
> unsigned long flags;
> int temp;
> @@ -857,7 +857,7 @@ unsigned int xhci_last_valid_endpoint(u32 added_ctxs)
> /* Returns 1 if the arguments are OK;
> * returns 0 this is a root hub; returns -EINVAL for NULL pointers.
> */
> -int xhci_check_args(struct usb_hcd *hcd, struct usb_device *udev,
> +static int xhci_check_args(struct usb_hcd *hcd, struct usb_device *udev,
> struct usb_host_endpoint *ep, int check_ep, bool check_virt_dev,
> const char *func) {
> struct xhci_hcd *xhci;
> @@ -1693,7 +1693,7 @@ static void xhci_setup_input_ctx_for_config_ep(struct xhci_hcd *xhci,
> xhci_dbg_ctx(xhci, in_ctx, xhci_last_valid_endpoint(add_flags));
> }
>
> -void xhci_setup_input_ctx_for_quirk(struct xhci_hcd *xhci,
> +static void xhci_setup_input_ctx_for_quirk(struct xhci_hcd *xhci,
> unsigned int slot_id, unsigned int ep_index,
> struct xhci_dequeue_state *deq_state)
> {
> --
> 1.7.3.2
>
On Tue, Feb 08, 2011 at 04:29:33PM -0800, Dmitry Torokhov wrote:
> xhci->ir_set points to __iomem region, but xhci_print_ir_set accepts
> plain struct xhci_intr_reg * causing multiple sparse warning at call
> sites and inside the fucntion when we try to read that memory.
>
> Instead of adding __iomem qualifier to the argument let's rework the
> function so it itself gets needed register set from xhci and prints
> it.
>
> Signed-off-by: Dmitry Torokhov <dtor@xxxxxxxxxx>
> ---
> drivers/usb/host/xhci-dbg.c | 5 +++--
> drivers/usb/host/xhci-mem.c | 2 +-
> drivers/usb/host/xhci.c | 6 +++---
> drivers/usb/host/xhci.h | 2 +-
> 4 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-dbg.c b/drivers/usb/host/xhci-dbg.c
> index 4a83350..0231814 100644
> --- a/drivers/usb/host/xhci-dbg.c
> +++ b/drivers/usb/host/xhci-dbg.c
> @@ -169,9 +169,10 @@ static void xhci_print_ports(struct xhci_hcd *xhci)
> }
> }
>
> -void xhci_print_ir_set(struct xhci_hcd *xhci, struct xhci_intr_reg *ir_set, int set_num)
> +void xhci_print_ir_set(struct xhci_hcd *xhci, int set_num)
> {
> - void *addr;
> + struct xhci_intr_reg __iomem *ir_set = &xhci->run_regs->ir_set[set_num];
> + void __iomem *addr;
> u32 temp;
> u64 temp_64;
>
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index 7604d61..f2a34e6 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -1961,7 +1961,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
> /* Set the event ring dequeue address */
> xhci_set_hc_event_deq(xhci);
> xhci_dbg(xhci, "Wrote ERST address to ir_set 0.\n");
> - xhci_print_ir_set(xhci, xhci->ir_set, 0);
> + xhci_print_ir_set(xhci, 0);
>
> /*
> * XXX: Might need to set the Interrupter Moderation Register to
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 4c05777..2083fc2 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -473,7 +473,7 @@ int xhci_run(struct usb_hcd *hcd)
> xhci->ir_set, (unsigned int) ER_IRQ_ENABLE(temp));
> xhci_writel(xhci, ER_IRQ_ENABLE(temp),
> &xhci->ir_set->irq_pending);
> - xhci_print_ir_set(xhci, xhci->ir_set, 0);
> + xhci_print_ir_set(xhci, 0);
>
> if (NUM_TEST_NOOPS > 0)
> doorbell = xhci_setup_one_noop(xhci);
> @@ -528,7 +528,7 @@ void xhci_stop(struct usb_hcd *hcd)
> temp = xhci_readl(xhci, &xhci->ir_set->irq_pending);
> xhci_writel(xhci, ER_IRQ_DISABLE(temp),
> &xhci->ir_set->irq_pending);
> - xhci_print_ir_set(xhci, xhci->ir_set, 0);
> + xhci_print_ir_set(xhci, 0);
>
> xhci_dbg(xhci, "cleaning up memory\n");
> xhci_mem_cleanup(xhci);
> @@ -755,7 +755,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
> temp = xhci_readl(xhci, &xhci->ir_set->irq_pending);
> xhci_writel(xhci, ER_IRQ_DISABLE(temp),
> &xhci->ir_set->irq_pending);
> - xhci_print_ir_set(xhci, xhci->ir_set, 0);
> + xhci_print_ir_set(xhci, 0);
>
> xhci_dbg(xhci, "cleaning up memory\n");
> xhci_mem_cleanup(xhci);
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index 7f236fd..7f127df 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -1348,7 +1348,7 @@ static inline int xhci_link_trb_quirk(struct xhci_hcd *xhci)
> }
>
> /* xHCI debugging */
> -void xhci_print_ir_set(struct xhci_hcd *xhci, struct xhci_intr_reg *ir_set, int set_num);
> +void xhci_print_ir_set(struct xhci_hcd *xhci, int set_num);
> void xhci_print_registers(struct xhci_hcd *xhci);
> void xhci_dbg_regs(struct xhci_hcd *xhci);
> void xhci_print_run_regs(struct xhci_hcd *xhci);
> --
> 1.7.3.2
>
On Tue, Feb 08, 2011 at 04:29:34PM -0800, Dmitry Torokhov wrote:
> There is no point in casting to (void *) when setting up xhci->ir_set
> as it only makes us lose __iomem annotation and makes sparse unhappy.
>
> OTOH we do need to cast to (void *) when calculating xhci->dba from
> offset, but since it is IO memory we need to annotate it as such.
>
> Signed-off-by: Dmitry Torokhov <dtor@xxxxxxxxxx>
> ---
> drivers/usb/host/xhci-mem.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index f2a34e6..a953439 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -1900,11 +1900,11 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
> val &= DBOFF_MASK;
> xhci_dbg(xhci, "// Doorbell array is located at offset 0x%x"
> " from cap regs base addr\n", val);
> - xhci->dba = (void *) xhci->cap_regs + val;
> + xhci->dba = (void __iomem *) xhci->cap_regs + val;
> xhci_dbg_regs(xhci);
> xhci_print_run_regs(xhci);
> /* Set ir_set to interrupt register set 0 */
> - xhci->ir_set = (void *) xhci->run_regs->ir_set;
> + xhci->ir_set = &xhci->run_regs->ir_set[0];
>
> /*
> * Event ring setup: Allocate a normal ring, but also setup
> --
> 1.7.3.2
>
On Thu, Feb 10, 2011 at 04:10:41PM -0800, Luben Tuikov wrote:
> If the device isn't reset, the XHCI HCD sends
> SET ADDRESS to address 0 while the device is
> already in Addressed state, and the request is
> dropped on the floor as it is addressed to the
> default address. This sequence of events, which this
> patch fixes looks like this:
>
> usb_reset_and_verify_device()
> hub_port_init()
> hub_set_address()
> SET_ADDRESS to 0 with 1
> usb_get_device_descriptor(udev, 8)
> usb_get_device_descriptor(udev, 18)
> descriptors_changed() --> goto re_enumerate:
> hub_port_logical_disconnect()
> kick_khubd()
>
> And then:
>
> hub_events()
> hub_port_connect_change()
> usb_disconnect()
> usb_disable_device()
> new device struct
> sets device state to Powered
> choose_address()
> hub_port_init() <-- no reset, but SET ADDRESS to 0 with 1, timeout!
>
> The solution is to always reset the device in
> hub_port_init() to put it in a known state.
>
> Signed-off-by: Luben Tuikov <ltuikov@xxxxxxxxx>
> ---
> drivers/usb/core/hub.c | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 4310cc4..5c28bf9 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -2680,11 +2680,13 @@ hub_port_init (struct usb_hub *hub, struct usb_device *udev, int port1,
> delay = HUB_LONG_RESET_TIME;
>
> mutex_lock(&usb_address0_mutex);
> -
> +#if 0
> if (!udev->config && oldspeed == USB_SPEED_SUPER) {
> /* Don't reset USB 3.0 devices during an initial setup */
> usb_set_device_state(udev, USB_STATE_DEFAULT);
> - } else {
> + } else
> +#endif
> + {
> /* Reset the device; full speed may morph to high speed */
> /* FIXME a USB 2.0 device may morph into SuperSpeed on reset. */
> retval = hub_port_reset(hub, port1, udev, delay);
> --
> 1.7.0.1
>
On Sat, Feb 12, 2011 at 02:05:29PM -0800, Paul Zimmerman wrote:
> Hi Sarah,
>
> Sorry, I did not know about the 70-char limit for the first line of the
> commit message. So here is a respin of the patch series, with that fixed.
> Plus, I expanded the commit messages to better explain exactly what is being
> fixed, as you requested. I hope you like these better.
>
> These patches make the xhci driver work with the Synopsys xHCI controller.
>
> The first patch prevents a BUG in interrupt context from bringing down
> the box, so while not strictly required I think it is a good idea.
>
> The remaining patches fix some errors in the TRB math that caused
> problems for us. Specifically, the count of TRBs required would
> occasionally be too large, causing the loops that create the TRBs to
> exit prematurely. This resulted in the last TRB not having the IOC
> bit set.
>
> This seemed to be harmless with the NEC controller, but with our
> controller it resulted in a lost interrupt, causing the transfer to
> eventually time out. Plus, the "orphaned" TRBs remained in the ring,
> eventually causing the BUG in interrupt context mentioned above.
>
> --
> Paul
>
On Sat, Feb 12, 2011 at 02:06:06PM -0800, Paul Zimmerman wrote:
> Change the BUGs in xhci_find_new_dequeue_state() to WARN_ONs, to avoid
> bringing down the box if one of them is hit
>
> Signed-off-by: Paul Zimmerman <paulz@xxxxxxxxxxxx>
> ---
> drivers/usb/host/xhci-ring.c | 13 +++++++++----
> 1 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index df558f6..b4be841 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -479,8 +479,11 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci,
> state->new_deq_seg = find_trb_seg(cur_td->start_seg,
> dev->eps[ep_index].stopped_trb,
> &state->new_cycle_state);
> - if (!state->new_deq_seg)
> - BUG();
> + if (!state->new_deq_seg) {
> + WARN_ON(1);
> + return;
> + }
> +
> /* Dig out the cycle state saved by the xHC during the stop ep cmd */
> xhci_dbg(xhci, "Finding endpoint context\n");
> ep_ctx = xhci_get_ep_ctx(xhci, dev->out_ctx, ep_index);
> @@ -491,8 +494,10 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci,
> state->new_deq_seg = find_trb_seg(state->new_deq_seg,
> state->new_deq_ptr,
> &state->new_cycle_state);
> - if (!state->new_deq_seg)
> - BUG();
> + if (!state->new_deq_seg) {
> + WARN_ON(1);
> + return;
> + }
>
> trb = &state->new_deq_ptr->generic;
> if ((trb->field[3] & TRB_TYPE_BITMASK) == TRB_TYPE(TRB_LINK) &&
> --
> 1.6.5.1.69.g36942
>
On Sat, Feb 12, 2011 at 02:06:44PM -0800, Paul Zimmerman wrote:
> This makes it easier to spot some problems, which will be fixed by the
> next patch in the series. Also change dev_dbg to dev_err in
> check_trb_math(), so any math errors will be visible even when running
> with debug disabled.
>
> Note: This patch changes the expressions containing
> "((1 << TRB_MAX_BUFF_SHIFT) - 1)" to use the equivalent
> "(TRB_MAX_BUFF_SIZE - 1)". No change in behavior is intended for
> those expressions.
>
> Signed-off-by: Paul Zimmerman <paulz@xxxxxxxxxxxx>
> ---
> drivers/usb/host/xhci-ring.c | 22 ++++++++++------------
> 1 files changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index b4be841..fe6dd81 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -2374,7 +2374,7 @@ static unsigned int count_sg_trbs_needed(struct xhci_hcd *xhci, struct urb *urb)
>
> /* Scatter gather list entries may cross 64KB boundaries */
> running_total = TRB_MAX_BUFF_SIZE -
> - (sg_dma_address(sg) & ((1 << TRB_MAX_BUFF_SHIFT) - 1));
> + (sg_dma_address(sg) & (TRB_MAX_BUFF_SIZE - 1));
> if (running_total != 0)
> num_trbs++;
>
> @@ -2404,11 +2404,11 @@ static unsigned int count_sg_trbs_needed(struct xhci_hcd *xhci, struct urb *urb)
> static void check_trb_math(struct urb *urb, int num_trbs, int running_total)
> {
> if (num_trbs != 0)
> - dev_dbg(&urb->dev->dev, "%s - ep %#x - Miscalculated number of "
> + dev_err(&urb->dev->dev, "%s - ep %#x - Miscalculated number of "
> "TRBs, %d left\n", __func__,
> urb->ep->desc.bEndpointAddress, num_trbs);
> if (running_total != urb->transfer_buffer_length)
> - dev_dbg(&urb->dev->dev, "%s - ep %#x - Miscalculated tx length, "
> + dev_err(&urb->dev->dev, "%s - ep %#x - Miscalculated tx length, "
> "queued %#x (%d), asked for %#x (%d)\n",
> __func__,
> urb->ep->desc.bEndpointAddress,
> @@ -2540,8 +2540,7 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
> sg = urb->sg;
> addr = (u64) sg_dma_address(sg);
> this_sg_len = sg_dma_len(sg);
> - trb_buff_len = TRB_MAX_BUFF_SIZE -
> - (addr & ((1 << TRB_MAX_BUFF_SHIFT) - 1));
> + trb_buff_len = TRB_MAX_BUFF_SIZE - (addr & (TRB_MAX_BUFF_SIZE - 1));
> trb_buff_len = min_t(int, trb_buff_len, this_sg_len);
> if (trb_buff_len > urb->transfer_buffer_length)
> trb_buff_len = urb->transfer_buffer_length;
> @@ -2577,7 +2576,7 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
> (unsigned int) (addr + TRB_MAX_BUFF_SIZE) & ~(TRB_MAX_BUFF_SIZE - 1),
> (unsigned int) addr + trb_buff_len);
> if (TRB_MAX_BUFF_SIZE -
> - (addr & ((1 << TRB_MAX_BUFF_SHIFT) - 1)) < trb_buff_len) {
> + (addr & (TRB_MAX_BUFF_SIZE - 1)) < trb_buff_len) {
> xhci_warn(xhci, "WARN: sg dma xfer crosses 64KB boundaries!\n");
> xhci_dbg(xhci, "Next boundary at %#x, end dma = %#x\n",
> (unsigned int) (addr + TRB_MAX_BUFF_SIZE) & ~(TRB_MAX_BUFF_SIZE - 1),
> @@ -2621,7 +2620,7 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
> }
>
> trb_buff_len = TRB_MAX_BUFF_SIZE -
> - (addr & ((1 << TRB_MAX_BUFF_SHIFT) - 1));
> + (addr & (TRB_MAX_BUFF_SIZE - 1));
> trb_buff_len = min_t(int, trb_buff_len, this_sg_len);
> if (running_total + trb_buff_len > urb->transfer_buffer_length)
> trb_buff_len =
> @@ -2661,7 +2660,7 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
> num_trbs = 0;
> /* How much data is (potentially) left before the 64KB boundary? */
> running_total = TRB_MAX_BUFF_SIZE -
> - (urb->transfer_dma & ((1 << TRB_MAX_BUFF_SHIFT) - 1));
> + (urb->transfer_dma & (TRB_MAX_BUFF_SIZE - 1));
>
> /* If there's some data on this 64KB chunk, or we have to send a
> * zero-length transfer, we need at least one TRB
> @@ -2704,8 +2703,8 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
> /* How much data is in the first TRB? */
> addr = (u64) urb->transfer_dma;
> trb_buff_len = TRB_MAX_BUFF_SIZE -
> - (urb->transfer_dma & ((1 << TRB_MAX_BUFF_SHIFT) - 1));
> - if (urb->transfer_buffer_length < trb_buff_len)
> + (urb->transfer_dma & (TRB_MAX_BUFF_SIZE - 1));
> + if (trb_buff_len > urb->transfer_buffer_length)
> trb_buff_len = urb->transfer_buffer_length;
>
> first_trb = true;
> @@ -2877,8 +2876,7 @@ static int count_isoc_trbs_needed(struct xhci_hcd *xhci,
> addr = (u64) (urb->transfer_dma + urb->iso_frame_desc[i].offset);
> td_len = urb->iso_frame_desc[i].length;
>
> - running_total = TRB_MAX_BUFF_SIZE -
> - (addr & ((1 << TRB_MAX_BUFF_SHIFT) - 1));
> + running_total = TRB_MAX_BUFF_SIZE - (addr & (TRB_MAX_BUFF_SIZE - 1));
> if (running_total != 0)
> num_trbs++;
>
> --
> 1.6.5.1.69.g36942
>
On Sat, Feb 12, 2011 at 02:07:20PM -0800, Paul Zimmerman wrote:
> Calculations like
>
> running_total = TRB_MAX_BUFF_SIZE -
> (sg_dma_address(sg) & (TRB_MAX_BUFF_SIZE - 1));
> if (running_total != 0)
> num_trbs++;
>
> are incorrect, because running_total can never be zero, so the if()
> expression will never be true. I think the intention was that
> running_total be in the range of 0 to TRB_MAX_BUFF_SIZE-1, not 1
> to TRB_MAX_BUFF_SIZE. So adding a
>
> running_total &= TRB_MAX_BUFF_SIZE - 1;
>
> fixes the problem.
>
> Signed-off-by: Paul Zimmerman <paulz@xxxxxxxxxxxx>
> ---
> drivers/usb/host/xhci-ring.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index fe6dd81..8939086 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -2375,6 +2375,7 @@ static unsigned int count_sg_trbs_needed(struct xhci_hcd *xhci, struct urb *urb)
> /* Scatter gather list entries may cross 64KB boundaries */
> running_total = TRB_MAX_BUFF_SIZE -
> (sg_dma_address(sg) & (TRB_MAX_BUFF_SIZE - 1));
> + running_total &= TRB_MAX_BUFF_SIZE - 1;
> if (running_total != 0)
> num_trbs++;
>
> @@ -2661,6 +2662,7 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
> /* How much data is (potentially) left before the 64KB boundary? */
> running_total = TRB_MAX_BUFF_SIZE -
> (urb->transfer_dma & (TRB_MAX_BUFF_SIZE - 1));
> + running_total &= TRB_MAX_BUFF_SIZE - 1;
>
> /* If there's some data on this 64KB chunk, or we have to send a
> * zero-length transfer, we need at least one TRB
> @@ -2877,6 +2879,7 @@ static int count_isoc_trbs_needed(struct xhci_hcd *xhci,
> td_len = urb->iso_frame_desc[i].length;
>
> running_total = TRB_MAX_BUFF_SIZE - (addr & (TRB_MAX_BUFF_SIZE - 1));
> + running_total &= TRB_MAX_BUFF_SIZE - 1;
> if (running_total != 0)
> num_trbs++;
>
> --
> 1.6.5.1.69.g36942
>
--
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
