[once more, without Gmail being stupid]
Nice! This also solves problems with exhausting coherent DMA memory
when you plug too many devices in.
On Tue, Mar 17, 2015 at 2:54 AM, Mian Yousaf Kaukab
<yousaf.kaukab@xxxxxxxxx> wrote:
> From: Gregory Herrero <gregory.herrero@xxxxxxxxx>
>
> Align buffer must be allocated using kmalloc since irqs are disabled.
> Coherency is handled through dma_map_single which can be used with irqs
> disabled.
>
> Signed-off-by: Gregory Herrero <gregory.herrero@xxxxxxxxx>
> ---
> drivers/usb/dwc2/hcd.c | 7 ++++---
> drivers/usb/dwc2/hcd_intr.c | 10 ++++++++++
> drivers/usb/dwc2/hcd_queue.c | 7 ++++---
> 3 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index fd2ad25..54f58c1 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -710,9 +710,7 @@ static int dwc2_hc_setup_align_buf(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh,
> /* 3072 = 3 max-size Isoc packets */
> buf_size = 3072;
>
> - qh->dw_align_buf = dma_alloc_coherent(hsotg->dev, buf_size,
> - &qh->dw_align_buf_dma,
> - GFP_ATOMIC);
> + qh->dw_align_buf = kzalloc(buf_size, GFP_ATOMIC);
Shouldn't this also set GFP_DMA? (And does it need to be kzalloc()? I
think kmalloc() should be enough and might avoid another memory
transfer.)
> if (!qh->dw_align_buf)
> return -ENOMEM;
> qh->dw_align_buf_size = buf_size;
> @@ -737,6 +735,9 @@ static int dwc2_hc_setup_align_buf(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh,
> }
> }
>
> + qh->dw_align_buf_dma = dma_map_single(hsotg->dev,
> + qh->dw_align_buf, qh->dw_align_buf_size, DMA_TO_DEVICE);
Documentation/DMA-API.txt says that you must always use the same
arguments for matching dma_map_single() and dma_unmap_single()... so I
think this and all the unmaps should use DMA_BIDIRECTIONAL.
> +
> chan->align_buf = qh->dw_align_buf_dma;
> return 0;
> }
> diff --git a/drivers/usb/dwc2/hcd_intr.c b/drivers/usb/dwc2/hcd_intr.c
> index 6927bba..22f1476 100644
> --- a/drivers/usb/dwc2/hcd_intr.c
> +++ b/drivers/usb/dwc2/hcd_intr.c
> @@ -468,6 +468,8 @@ static int dwc2_update_urb_state(struct dwc2_hsotg *hsotg,
> /* Non DWORD-aligned buffer case handling */
> if (chan->align_buf && xfer_length && chan->ep_is_in) {
> dev_vdbg(hsotg->dev, "%s(): non-aligned buffer\n", __func__);
> + dma_unmap_single(hsotg->dev, chan->qh->dw_align_buf_dma,
> + chan->qh->dw_align_buf_size, DMA_FROM_DEVICE);
> memcpy(urb->buf + urb->actual_length, chan->qh->dw_align_buf,
> xfer_length);
> }
> @@ -559,6 +561,8 @@ static enum dwc2_halt_status dwc2_update_isoc_urb_state(
> chan->ep_is_in) {
> dev_vdbg(hsotg->dev, "%s(): non-aligned buffer\n",
> __func__);
> + dma_unmap_single(hsotg->dev, chan->qh->dw_align_buf_dma,
> + chan->qh->dw_align_buf_size, DMA_FROM_DEVICE);
> memcpy(urb->buf + frame_desc->offset +
> qtd->isoc_split_offset, chan->qh->dw_align_buf,
> frame_desc->actual_length);
> @@ -588,6 +592,8 @@ static enum dwc2_halt_status dwc2_update_isoc_urb_state(
> chan->ep_is_in) {
> dev_vdbg(hsotg->dev, "%s(): non-aligned buffer\n",
> __func__);
> + dma_unmap_single(hsotg->dev, chan->qh->dw_align_buf_dma,
> + chan->qh->dw_align_buf_size, DMA_FROM_DEVICE);
> memcpy(urb->buf + frame_desc->offset +
> qtd->isoc_split_offset, chan->qh->dw_align_buf,
> frame_desc->actual_length);
> @@ -926,6 +932,8 @@ static int dwc2_xfercomp_isoc_split_in(struct dwc2_hsotg *hsotg,
>
> if (chan->align_buf) {
> dev_vdbg(hsotg->dev, "%s(): non-aligned buffer\n", __func__);
> + dma_unmap_single(hsotg->dev, chan->qh->dw_align_buf_dma,
> + chan->qh->dw_align_buf_size, DMA_FROM_DEVICE);
> memcpy(qtd->urb->buf + frame_desc->offset +
> qtd->isoc_split_offset, chan->qh->dw_align_buf, len);
> }
> @@ -1155,6 +1163,8 @@ static void dwc2_update_urb_state_abn(struct dwc2_hsotg *hsotg,
> /* Non DWORD-aligned buffer case handling */
> if (chan->align_buf && xfer_length && chan->ep_is_in) {
> dev_vdbg(hsotg->dev, "%s(): non-aligned buffer\n", __func__);
> + dma_unmap_single(hsotg->dev, chan->qh->dw_align_buf_dma,
> + chan->qh->dw_align_buf_size, DMA_FROM_DEVICE);
> memcpy(urb->buf + urb->actual_length, chan->qh->dw_align_buf,
> xfer_length);
> }
> diff --git a/drivers/usb/dwc2/hcd_queue.c b/drivers/usb/dwc2/hcd_queue.c
> index 02103b66..5f206ab 100644
> --- a/drivers/usb/dwc2/hcd_queue.c
> +++ b/drivers/usb/dwc2/hcd_queue.c
> @@ -231,9 +231,10 @@ void dwc2_hcd_qh_free(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh)
> {
> if (hsotg->core_params->dma_desc_enable > 0)
> dwc2_hcd_qh_free_ddma(hsotg, qh);
> - else if (qh->dw_align_buf)
> - dma_free_coherent(hsotg->dev, qh->dw_align_buf_size,
> - qh->dw_align_buf, qh->dw_align_buf_dma);
> + else {
Why did you drop the 'if (qh->dw_align_buf)' check? I think you still
need it since not all endpoints use dwc2_hc_setup_align_buf()
(isochronous ones whose buffer is already aligned skip it).
> + kfree(qh->dw_align_buf);
> + qh->dw_align_buf_dma = (dma_addr_t)0;
> + }
> kfree(qh);
> }
>
> --
> 1.9.1
>
> --
> 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
--
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
