Messages in this thread | | | From | "Sosnowski, Maciej" <> | Date | Tue, 31 Mar 2009 15:03:07 +0100 | Subject | RE: [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
| |