Re: [PATCH v9 2/2] Add Intel LGM SoC DMA support.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Vinod,

Thanks for your valuable comments. My reply inline.

On 11/26/2020 12:50 PM, Vinod Koul wrote:
On 25-11-20, 18:39, Reddy, MallikarjunaX wrote:

desc needs to be configure for each dma channel and the remapped address of
the IGP & EGP is desc base adress.
Why should this address not passed as src_addr/dst_addr?
src_addr/dst_addr is the data pointer. Data pointer indicates address
pointer of data buffer.

ldma_chan_desc_cfg() carries the descriptor address.

The descriptor list entry contains the data pointer, which points to the
data section in the memory.

So we should not use src_addr/dst_addr as desc base address.
Okay sounds reasonable. why is this using in API here?
descriptor base address needs to be write into the dma register (DMA_CDBA).
Why cant descriptor be allocated by damenegine driver, passed to client
as we normally do in prep_* callbacks ? Why do you need a custom API
1)
client needs to set the descriptor base address and also number of descriptors used in the descriptor list. reg DMA_CDBA used to configure descriptor base address and reg DMA_CDLEN used to configure number of descriptors used in the descriptor list.

In case of (ver > DMA_VER22) all descriptor fields and data pointer will be set by client, so we just need to write desc base and num desc length in to corresponding registers from the driver side.

dma_async_tx_descriptor * data is not really needed from driver to client side , so i am not planned to return 'struct dma_async_tx_descriptor *'.

because of this reason i used custom API (return -Ve for error and ZERO for success) instead of standard dmaengine_prep_slave_sg() callback (return 'struct dma_async_tx_descriptor *' descriptor)

2)
We can also use the dmaengine_prep_slave_sg( ) to pass desc base addr & desc number from client. In that case we have to use (sg)->dma_address as desc base address and (sg)->length as desc length.

dmaengine prep_* callback return 'struct dma_async_tx_descriptor *, this can be used on client side as to check  prep_* callback SUCCESS/FAIL.

Example:
/* code snippet */

static struct dma_async_tx_descriptor *
ldma_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
                  unsigned int sglen, enum dma_transfer_direction dir,
                  unsigned long flags, void *context)
{

.....

    if (d->ver > DMA_VER22)
        return ldma_chan_desc_cfg(chan, sgl->dma_address, sglen);

.....

}

static struct dma_async_tx_descriptor *
ldma_chan_desc_cfg(struct dma_chan *chan, dma_addr_t desc_base, int desc_num)
{
        struct ldma_chan *c = to_ldma_chan(chan);
        struct ldma_dev *d = to_ldma_dev(c->vchan.chan.device);
        struct dma_async_tx_descriptor *tx;
        struct dw2_desc_sw *ds;

        if (!desc_num) {
                dev_err(d->dev, "Channel %d must allocate descriptor first\n",
                        c->nr);
                return NULL;
        }

        if (desc_num > DMA_MAX_DESC_NUM) {
                dev_err(d->dev, "Channel %d descriptor number out of range %d\n",
                        c->nr, desc_num);
                return NULL;
        }

        ldma_chan_desc_hw_cfg(c, desc_base, desc_num);
        c->flags |= DMA_HW_DESC;
        c->desc_cnt = desc_num;
        c->desc_phys = desc_base;

        ds = kzalloc(sizeof(*ds), GFP_NOWAIT);
        if (!ds)
                return NULL;

        tx = &ds->vdesc.tx;
        dma_async_tx_descriptor_init(tx, chan);

        return tx;
}
Please let me know if this is OK, So that i will include in the next patch.




[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux PCI]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux