Hello Mathias,
I'll test it ASAP!
Thanks
Andreas
On 26.01.21 at 18:29 Mathias Nyman wrote:
>
>>> I'm not sure if it's important for you to know: The driver doesn't use struct scatterlist or num_mapped_sgs at all (if it's meant to be used by the sender at all).
>>>
>>> But it sets URB_NO_TRANSFER_DMA_MAP (for data transfer among others).
>>>
>>> Mlme packets are sent w/o bulk and w/o setting URB_NO_TRANSFER_DMA_MAP. All other packets are sent with URB_NO_TRANSFER_DMA_MAP turned on.
>>>
>>
>> Ok, thanks, I see what's going on here.
>>
>> Short recap of xhci alignment requirements.
>> 1. Data pointed to by a transfer request block (TRB) may not span 64k boundary
>> 2. If a transfer contains several TRBs, and spans over two TRB ringbuffer
>> segments, then the sum of the TRB data in the first segment must be a
>> multiple of max packets in size.
>>
>> Code assumes that if transfer is split into several blocks,(TRBs) and a block
>> in the middle of a transfer is smaller than max packet size, then it must be sg list.
>>
>> But this is not necessarily the case if data was already DMA mapped beforehand.
>> Data might start just before a 64k boundary, causing first TRB to be less than
>> packet size.
>>
>> I'll start implementing a fix for this.
>
> Got a first iteration ready,
> any change you could try it out?
>
> Thanks
> -Mathias
>
> 8<---
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 5677b81c0915..8df30618aaf1 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -699,11 +699,16 @@ static void xhci_unmap_td_bounce_buffer(struct xhci_hcd *xhci,
> dma_unmap_single(dev, seg->bounce_dma, ring->bounce_buf_len,
> DMA_FROM_DEVICE);
> /* for in tranfers we need to copy the data from bounce to sg */
> - len = sg_pcopy_from_buffer(urb->sg, urb->num_sgs, seg->bounce_buf,
> - seg->bounce_len, seg->bounce_offs);
> - if (len != seg->bounce_len)
> - xhci_warn(xhci, "WARN Wrong bounce buffer read length: %zu != %d\n",
> - len, seg->bounce_len);
> + if (urb->num_sgs) {
> + len = sg_pcopy_from_buffer(urb->sg, urb->num_sgs, seg->bounce_buf,
> + seg->bounce_len, seg->bounce_offs);
> + if (len != seg->bounce_len)
> + xhci_warn(xhci, "WARN Wrong bounce buffer read length: %zu != %d\n",
> + len, seg->bounce_len);
> + } else {
> + memcpy(urb->transfer_buffer + seg->bounce_offs, seg->bounce_buf,
> + seg->bounce_len);
> + }
> seg->bounce_len = 0;
> seg->bounce_offs = 0;
> }
> @@ -3275,12 +3280,16 @@ 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)) {
> - len = sg_pcopy_to_buffer(urb->sg, urb->num_sgs,
> - seg->bounce_buf, new_buff_len, enqd_len);
> - if (len != new_buff_len)
> - xhci_warn(xhci,
> - "WARN Wrong bounce buffer write length: %zu != %d\n",
> - len, new_buff_len);
> + if (urb->num_sgs) {
> + len = sg_pcopy_to_buffer(urb->sg, urb->num_sgs,
> + seg->bounce_buf, new_buff_len, enqd_len);
> + if (len != new_buff_len)
> + xhci_warn(xhci, "WARN Wrong bounce buffer write length: %zu != %d\n",
> + len, new_buff_len);
> + } else {
> + memcpy(seg->bounce_buf, urb->transfer_buffer + enqd_len, new_buff_len);
> + }
> +
> seg->bounce_dma = dma_map_single(dev, seg->bounce_buf,
> max_pkt, DMA_TO_DEVICE);
> } else {
>
