On Thursday, April 14, 2016 01:32:48 PM Andy Shevchenko wrote:
> 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.
>
> [...]
> 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;
> > }
Yes, this can be removed. (I'll post a updated patch once #2 has been
dealt with)
> > @@ -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?
I think so. Currently, dwc_free_chan_resources will actually OOPS if
the channel is not idle when the function is called:
static void dwc_free_chan_resources(struct dma_chan *chan)
{
[...]
dev_dbg(chan2dev(chan), "%s: descs allocated=%u\n", __func__, dwc->descs_allocated);
/* ASSERT: channel is idle */
BUG_ON(!list_empty(&dwc->active_list));
BUG_ON(!list_empty(&dwc->queue));
BUG_ON(dma_readl(to_dw_dma(chan->device), CH_EN) & dwc->mask);
[...]
I think Julian ran into this once when his controller was become stuck
and the active_list and queue couldn't be flushed fast enough, so the
BUG_ONs were triggered when he tried to reboot.
It would probably be nice to make the function wait for the channel to
empty out and not OOPs, but this will need it's own patch.
--
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
