On Wed, 2016-04-13 at 21:18 +0200, Christian Lamparter wrote:
> This patch changes the driver to allocate DMA descriptors when
> needed. This stops memory resources to be wasted and letting
> them sit idle in the free_list structure when the device doesn't
> need it... This also solves the problem, that a driver has to
> guess the number of how many descriptors it needs to allocate
> in advance. Currently, the dma engine will just fail when put
> under load by sata_dwc_460ex.
Thanks! My comments below.
> --- a/drivers/dma/dw/core.c
> +++ b/drivers/dma/dw/core.c
> @@ -1166,48 +1143,15 @@ static int dwc_alloc_chan_resources(struct
> dma_chan *chan)
> dw_dma_on(dw);
> dw->in_use |= dwc->mask;
>
> - spin_lock_irqsave(&dwc->lock, flags);
> - i = dwc->descs_allocated;
> - while (dwc->descs_allocated < NR_DESCS_PER_CHANNEL) {
> - dma_addr_t phys;
> -
> - spin_unlock_irqrestore(&dwc->lock, flags);
> -
> - desc = dma_pool_alloc(dw->desc_pool, GFP_ATOMIC,
> &phys);
> - if (!desc)
> - goto err_desc_alloc;
> -
> - memset(desc, 0, sizeof(struct dw_desc));
> -
> - INIT_LIST_HEAD(&desc->tx_list);
> - dma_async_tx_descriptor_init(&desc->txd, chan);
> - desc->txd.tx_submit = dwc_tx_submit;
> - desc->txd.flags = DMA_CTRL_ACK;
> - desc->txd.phys = phys;
> -
> - dwc_desc_put(dwc, desc);
> -
> - spin_lock_irqsave(&dwc->lock, flags);
> - i = ++dwc->descs_allocated;
> - }
> -
> - spin_unlock_irqrestore(&dwc->lock, flags);
> -
> dev_dbg(chan2dev(chan), "%s: allocated %d descriptors\n",
> __func__, i);
>
> return i;
Looks like leftover. Since we do not allocate descriptors here we may
return 0 directly without any additional message printed.
> -
> -err_desc_alloc:
> - dev_info(chan2dev(chan), "only allocated %d descriptors\n",
> i);
> -
> - return i;
> }
> @@ -1244,11 +1186,6 @@ static void dwc_free_chan_resources(struct
> dma_chan *chan)
> if (!dw->in_use)
> dw_dma_off(dw);
>
> - list_for_each_entry_safe(desc, _desc, &list, desc_node) {
> - dev_vdbg(chan2dev(chan), " freeing descriptor %p\n",
> desc);
> - dma_pool_free(dw->desc_pool, desc, desc->txd.phys);
> - }
> -
> dev_vdbg(chan2dev(chan), "%s: done\n", __func__);
> }
I didn't check deeply, but is the case when user frees the active
channel covered here?
--
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
