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    [W:0.027 / U:0.464 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site