lkml.org 
[lkml]   [2009]   [Mar]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRE: [PATCH 2/2] dw_dmac: add cyclic API to DW DMA driver
Hans-Christian Egtvedt wrote:
> This patch adds a cyclic DMA interface to the DW DMA driver. This is
> very useful if you want to use the DMA controller in combination with a
> sound device which uses cyclic buffers.
>
> Using a DMA channel for cyclic DMA will disable the possibility to use
> it as a normal DMA engine until the user calls the cyclic free function
> on the DMA channel. Also a cyclic DMA list can not be prepared if the
> channel is already active.
>
> Signed-off-by: Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com>
> ---
> drivers/dma/dw_dmac.c | 303 +++++++++++++++++++++++++++++++++++++++++++-
> drivers/dma/dw_dmac_regs.h | 7 +- include/linux/dw_dmac.h | 19 +++
> 3 files changed, 327 insertions(+), 2 deletions(-)
>

The patch looks good to me.
Just a few remarks below:

> + for (i = 0; i < dwc->cdesc->periods; i++)
> + dwc_dump_lli(dwc, &dwc->cdesc->desc[i]->lli);
> +
> + return;
> + }
> +}

This return is not needed

> +void dw_dma_cyclic_stop(struct dma_chan *chan)
> +{
> + struct dw_dma_chan *dwc = to_dw_dma_chan(chan);
> + struct dw_dma *dw = to_dw_dma(dwc->chan.device);
> +
> + channel_clear_bit(dw, CH_EN, dwc->mask);
> + while (dma_readl(dw, CH_EN) & dwc->mask)
> + cpu_relax();
> +}

Don't you need locks in dw_dma_cyclic_stop?

> + was_cyclic = test_and_set_bit(DW_DMA_IS_CYCLIC, &dwc->flags);
> + if (was_cyclic) {
> + spin_unlock_bh(&dwc->lock);
> + dev_dbg(chan2dev(&dwc->chan),
> + "channel already prepared for cyclic DMA\n");
> + return ERR_PTR(-EBUSY);
> + }
> + spin_unlock_bh(&dwc->lock);

In this case both spin_unlock_bh() could be replaced by single one called before "if (was_cyclic)":
+ was_cyclic = test_and_set_bit(DW_DMA_IS_CYCLIC, &dwc->flags);
+ spin_unlock_bh(&dwc->lock);
+ if (was_cyclic) {
+ dev_dbg(chan2dev(&dwc->chan),
+ "channel already prepared for cyclic DMA\n");
+ return ERR_PTR(-EBUSY);
+ }
Regards,
Maciej

\
 
 \ /
  Last update: 2009-03-31 16:05    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site