Hi On 25.1.2021 12.18, Andreas Hartmann wrote: > > Hello! > > Meanwhile I found the culprit: > > https://www.spinics.net/lists/linux-usb/msg141467.html > and > https://www.spinics.net/lists/linux-usb/msg141468.html > > Especially the last change breaks things here completely. After removing them > by the attached patch, problems are gone and device works again as expected > (I tested with the original 24 kB bulk size which was horribly broken w/o the > attached patch). This means: the additional repair steps are not just breaking > things but are even unnecessary (it's working perfectly without those changes) > here. Unfortunately this isn't enough to remove the alignment code for those controllers. This is just once specific usecase. We need to figure out what really goes wrong. Looks like 0 bytes is copied from sg list to bounce buffer when we want 512 bytes copied. Just noticed the alignment code assumes sg lists are used without checking it first. Could you add the below code and test again, it should print more debugging info. Thanks -Mathias 8<--- diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 5677b81c0915..6a4578a45eb1 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -3253,13 +3253,13 @@ static int xhci_align_td(struct xhci_hcd *xhci, struct urb *urb, u32 enqd_len, if (unalign == 0) return 0; - xhci_dbg(xhci, "Unaligned %d bytes, buff len %d\n", + xhci_warn(xhci, "Unaligned %d bytes, buff len %d\n", unalign, *trb_buff_len); /* is the last nornal TRB alignable by splitting it */ if (*trb_buff_len > unalign) { *trb_buff_len -= unalign; - xhci_dbg(xhci, "split align, new buff len %d\n", *trb_buff_len); + xhci_warn(xhci, "split align, new buff len %d\n", *trb_buff_len); return 0; } @@ -3275,12 +3275,26 @@ static int xhci_align_td(struct xhci_hcd *xhci, struct urb *urb, u32 enqd_len, /* create a max max_pkt sized bounce buffer pointed to by last trb */ if (usb_urb_dir_out(urb)) { + unsigned int nents; len = sg_pcopy_to_buffer(urb->sg, urb->num_sgs, seg->bounce_buf, new_buff_len, enqd_len); - if (len != new_buff_len) + if (len != new_buff_len) { xhci_warn(xhci, "WARN Wrong bounce buffer write length: %zu != %d\n", len, new_buff_len); + xhci_warn(xhci, "urb->num_sgs %d\n", urb->num_sgs); + xhci_warn(xhci, "urb->num_mapped_sgs %d\n", urb->num_mapped_sgs); + xhci_warn(xhci, "urb->transfer_buffer_length %d\n", urb->transfer_buffer_length); + xhci_warn(xhci, "enqd_len %d\n", enqd_len); + xhci_warn(xhci, "max_pkt %d\n", max_pkt); + if (urb->sg) { + struct scatterlist *sg; + for (nents = 0; sg; sg = sg_next(sg)) { + nents++; + xhci_warn(xhci, "%d: sg->length %d\n", nents, sg->length); + } + } + } seg->bounce_dma = dma_map_single(dev, seg->bounce_buf, max_pkt, DMA_TO_DEVICE); } else {