[PATCH 1/2] Fix a bug about BUG_ON() on DMA engine capability DMA_INTERRUPT. - Kernel

This is a discussion on [PATCH 1/2] Fix a bug about BUG_ON() on DMA engine capability DMA_INTERRUPT. - Kernel ; The device->device_prep_dma_interrupt function is used by DMA_INTERRUPT capability, not DMA_ZERO_SUM. Signed-off-by: Zhang Wei --- drivers/dma/dmaengine.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c index 2996523..8db0e7f 100644 --- a/drivers/dma/dmaengine.c +++ b/drivers/dma/dmaengine.c @@ -357,7 +357,7 @@ ...

+ Reply to Thread
Results 1 to 4 of 4

Thread: [PATCH 1/2] Fix a bug about BUG_ON() on DMA engine capability DMA_INTERRUPT.

  1. [PATCH 1/2] Fix a bug about BUG_ON() on DMA engine capability DMA_INTERRUPT.

    The device->device_prep_dma_interrupt function is used by
    DMA_INTERRUPT capability, not DMA_ZERO_SUM.

    Signed-off-by: Zhang Wei
    ---
    drivers/dma/dmaengine.c | 2 +-
    1 files changed, 1 insertions(+), 1 deletions(-)

    diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
    index 2996523..8db0e7f 100644
    --- a/drivers/dma/dmaengine.c
    +++ b/drivers/dma/dmaengine.c
    @@ -357,7 +357,7 @@ int dma_async_device_register(struct dma_device *device)
    !device->device_prep_dma_zero_sum);
    BUG_ON(dma_has_cap(DMA_MEMSET, device->cap_mask) &&
    !device->device_prep_dma_memset);
    - BUG_ON(dma_has_cap(DMA_ZERO_SUM, device->cap_mask) &&
    + BUG_ON(dma_has_cap(DMA_INTERRUPT, device->cap_mask) &&
    !device->device_prep_dma_interrupt);

    BUG_ON(!device->device_alloc_chan_resources);
    --
    1.5.4

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [PATCH 2/2] Add device_prep_dma_interrupt support to fsldma.c

    On Mon, Mar 10, 2008 at 8:25 PM, Zhang Wei wrote:
    > This is a bug that I assigned DMA_INTERRUPT capability to fsldma
    > but missing device_prep_dma_interrupt function. For a bug in
    > dmaengine.c the driver passed BUG_ON() checking. The patch fixes it.
    >
    > Signed-off-by: Zhang Wei
    > ---
    > drivers/dma/fsldma.c | 27 +++++++++++++++++++++++++++
    > 1 files changed, 27 insertions(+), 0 deletions(-)
    >
    > diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
    > index 5dfedf3..5428f81 100644
    > --- a/drivers/dma/fsldma.c
    > +++ b/drivers/dma/fsldma.c
    > @@ -406,6 +406,32 @@ static void fsl_dma_free_chan_resources(struct dma_chan *chan)
    > dma_pool_destroy(fsl_chan->desc_pool);
    > }
    >
    > +static struct dma_async_tx_descriptor *fsl_dma_prep_interrupt(
    > + struct dma_chan *chan)
    > +{
    > + struct fsl_dma_chan *fsl_chan;
    > + struct fsl_desc_sw *new;
    > +
    > + if (!chan)
    > + return NULL;
    > +
    > + fsl_chan = to_fsl_chan(chan);
    > +
    > + new = fsl_dma_alloc_descriptor(fsl_chan);
    > + if (!new) {
    > + dev_err(fsl_chan->dev, "No free memory for link descriptor\n");
    > + return NULL;
    > + }
    > +
    > + new->async_tx.cookie = -EBUSY;
    > + new->async_tx.ack = 0;
    > +
    > + /* Set End-of-link to the last link descriptor of new list*/
    > + set_ld_eol(fsl_chan, new);


    Question, is 'set_ld_eol' safe to call on descriptors that may not be
    the last in the list? For example what about the following sequence:

    /* prepare two descriptors */
    tx1 = fsl_dma_prep_interrupt(chan);
    tx2 = fsl_dma_prep_memcpy(chan);

    /* submit out of order */
    tx2->tx_submit(tx2);
    tx1->tx_submit(tx1);

    This is only a concern if you plan to support channel switching at
    some point. For example, switching from a memcpy channel to an xor
    channel.

    Thanks,
    Dan
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. RE: [PATCH 2/2] Add device_prep_dma_interrupt support to fsldma.c

    Hi, Dan,

    > -----Original Message-----
    > From: dan.j.williams@gmail.com
    > > +
    > > + /* Set End-of-link to the last link descriptor of

    > new list*/
    > > + set_ld_eol(fsl_chan, new);

    >
    > Question, is 'set_ld_eol' safe to call on descriptors that may not be
    > the last in the list? For example what about the following sequence:


    set_ld_eol() is a safe function, which is only for preparing the tx descriptors
    lists. When adding prepared tx descriptior list to DMA channel tx list,
    the function append_ld_queue() should be called.

    >
    > /* prepare two descriptors */
    > tx1 = fsl_dma_prep_interrupt(chan);
    > tx2 = fsl_dma_prep_memcpy(chan);
    >
    > /* submit out of order */
    > tx2->tx_submit(tx2);
    > tx1->tx_submit(tx1);
    >
    > This is only a concern if you plan to support channel switching at
    > some point. For example, switching from a memcpy channel to an xor
    > channel.
    >

    It's no problem. In fact, I've added out of order testing codes in fsl_dma_self_test()
    function.

    But I have a question about device_prep_dma_interrupt(), which is no way to assign
    dest and src address. Is it a null tx action dma_async_tx_descriptor except to trigger
    an interrupt?

    Thanks!
    Wei.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [PATCH 2/2] Add device_prep_dma_interrupt support to fsldma.c

    On Tue, Mar 11, 2008 at 7:28 PM, Zhang Wei wrote:
    [..]
    > But I have a question about device_prep_dma_interrupt(), which is no way to assign
    > dest and src address. Is it a null tx action dma_async_tx_descriptor except to trigger
    > an interrupt?
    >


    Yes, it is for sequences where we are scheduling a chain of operations
    of indeterminate length and want a callback after they all complete.
    See ops_run_biofill() in drivers/md/raid5.c for an example.

    > Thanks!
    > Wei.


    --
    Dan
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread