Re: [PATCH v4 1/2] usb: dwc2: alloc dma aligned buffer for isoc split in

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Dear Doug,

在 2018年05月11日 05:01, Doug Anderson 写道:
Hi,

On Wed, May 9, 2018 at 3:11 AM, William Wu <william.wu@xxxxxxxxxxxxxx> wrote:
The commit 3bc04e28a030 ("usb: dwc2: host: Get aligned DMA in
a more supported way") rips out a lot of code to simply the
allocation of aligned DMA. However, it also introduces a new
issue when use isoc split in transfer.

In my test case, I connect the dwc2 controller with an usb hs
Hub (GL852G-12), and plug an usb fs audio device (Plantronics
headset) into the downstream port of Hub. Then use the usb mic
to record, we can find noise when playback.

It's because that the usb Hub uses an MDATA for the first
transaction and a DATA0 for the second transaction for the isoc
split in transaction. An typical isoc split in transaction sequence
like this:

- SSPLIT IN transaction
- CSPLIT IN transaction
   - MDATA packet
- CSPLIT IN transaction
   - DATA0 packet

The DMA address of MDATA (urb->dma) is always DWORD-aligned, but
the DMA address of DATA0 (urb->dma + qtd->isoc_split_offset) may
not be DWORD-aligned, it depends on the qtd->isoc_split_offset (the
length of MDATA). In my test case, the length of MDATA is usually
unaligned, this cause DATA0 packet transmission error.

This patch use kmem_cache to allocate aligned DMA buf for isoc
split in transaction. Note that according to usb 2.0 spec, the
maximum data payload size is 1023 bytes for each fs isoc ep,
and the maximum allowable interrupt data payload size is 64 bytes
or less for fs interrupt ep. So we set the size of object to be
1024 bytes in the kmem cache.

Signed-off-by: William Wu <william.wu@xxxxxxxxxxxxxx>
---
Changes in v4:
- get rid of "qh->dw_align_buf_size"
- allocate unaligned_cache for Address DMA mode and Desc DMA mode
- freeing order opposite of allocation
You only did half of this.  You changed the order under "error4" but
forgot to make dwc2_hcd_remove() match.
Ah, sorry, I'm careless.


- do dma_map_single() with the size of DWC2_KMEM_UNALIGNED_BUF_SIZE

Changes in v3:
- Modify the commit message
- Use Kmem_cache to allocate aligned DMA buf
- Fix coding style

Changes in v2:
- None

  drivers/usb/dwc2/core.h      |  3 ++
  drivers/usb/dwc2/hcd.c       | 87 ++++++++++++++++++++++++++++++++++++++++++--
  drivers/usb/dwc2/hcd.h       |  8 ++++
  drivers/usb/dwc2/hcd_intr.c  |  8 ++++
  drivers/usb/dwc2/hcd_queue.c |  3 ++
  5 files changed, 105 insertions(+), 4 deletions(-)
My only remaining comment is a really nitty detail.  Unless someone
else feels strongly about it, I'm not sure it's worth spinning the
patch just for that nit unless someone else has review comments.

I believe I've looked at this enough to say:

Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
Thank you very much for your review. I will submit V5 patches to fix the order of memory free in dwc2_hcd_remove(), and also add Review-by.





--
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



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux