Re: [PATCH v3 2/2] dma: Add Xilinx AXI Direct Memory Access Engine driver support

2014-09-09 Thread Srikanth Thokala
Hi Vinod,

On Tue, Sep 9, 2014 at 9:27 PM, Vinod Koul  wrote:
> On Tue, Sep 09, 2014 at 12:52:16AM +0530, Srikanth Thokala wrote:
>> Hi Vinod,
>>
>> On Thu, Sep 4, 2014 at 12:06 PM, Vinod Koul  wrote:
>> > On Wed, Sep 03, 2014 at 12:17:43PM +0530, Srikanth Thokala wrote:
>> >> Hi Vinod,
>> >>
>> >> Apologies for the delay.
>> >>
>> >> On Tue, Aug 19, 2014 at 10:33 PM, Vinod Koul  wrote:
>> >> > On Mon, Jul 28, 2014 at 05:47:49PM +0530, Srikanth Thokala wrote:
>> >> >> +struct xilinx_dma_chan {
>> >> >> + struct xilinx_dma_device *xdev;
>> >> >> + u32 ctrl_offset;
>> >> >> + spinlock_t lock;
>> >> >> + struct list_head pending_list;
>> >> >> + struct xilinx_dma_tx_descriptor *active_desc;
>> >> >> + struct xilinx_dma_tx_descriptor *allocated_desc;
>> >> >> + struct list_head done_list;
>> >> >> + struct list_head free_seg_list;
>> >> >> + struct dma_chan common;
>> >> >> + struct xilinx_dma_tx_segment *seg_v;
>> >> >> + dma_addr_t seg_p;
>> >> >> + struct device *dev;
>> >> >> + int irq;
>> >> >> + int id;
>> >> >> + enum dma_transfer_direction direction;
>> >> > This looks suspect. Why should channel have direction, for a descriptor 
>> >> > it
>> >> > makes sense though.
>> >>
>> >> The channel only supports transfers in one direction. Either from memory 
>> >> to
>> >> peripheral or from peripheral to memory, that's fixed and can't be changed
>> >> at runtime.  So, the driver needs to know which direction the channel 
>> >> supports
>> >> and hence it can reject transfers with the wrong direction.
>> > But you already have this information in descriptor so why duplicate?
>>
>> Our descriptor doesn't have the information of channel direction, we have 
>> this
>> information only in the chan structure (struct xilinx_dma_chan) which
>> is populated
>> while parsing the DT.  So, we use this information to ensure if we are 
>> servicing
>> proper channel (in prep_slave_sg call).
> One of the argument of prep_slave_sg call is direction. You need to use that
> and store in in your descriptor for using it afterwards
>
>> FYI: We also need the direction of the channel so that we can select SRC/DST
>> register address in the issue_pending() call.
> And you have the descriptor there as well so can use it

Ok, got it. Thanks.  I will send v4 with the changes mentioned.

- Srikanth

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


Re: [PATCH v3 2/2] dma: Add Xilinx AXI Direct Memory Access Engine driver support

2014-09-09 Thread Vinod Koul
On Tue, Sep 09, 2014 at 12:52:16AM +0530, Srikanth Thokala wrote:
> Hi Vinod,
> 
> On Thu, Sep 4, 2014 at 12:06 PM, Vinod Koul  wrote:
> > On Wed, Sep 03, 2014 at 12:17:43PM +0530, Srikanth Thokala wrote:
> >> Hi Vinod,
> >>
> >> Apologies for the delay.
> >>
> >> On Tue, Aug 19, 2014 at 10:33 PM, Vinod Koul  wrote:
> >> > On Mon, Jul 28, 2014 at 05:47:49PM +0530, Srikanth Thokala wrote:
> >> >> +struct xilinx_dma_chan {
> >> >> + struct xilinx_dma_device *xdev;
> >> >> + u32 ctrl_offset;
> >> >> + spinlock_t lock;
> >> >> + struct list_head pending_list;
> >> >> + struct xilinx_dma_tx_descriptor *active_desc;
> >> >> + struct xilinx_dma_tx_descriptor *allocated_desc;
> >> >> + struct list_head done_list;
> >> >> + struct list_head free_seg_list;
> >> >> + struct dma_chan common;
> >> >> + struct xilinx_dma_tx_segment *seg_v;
> >> >> + dma_addr_t seg_p;
> >> >> + struct device *dev;
> >> >> + int irq;
> >> >> + int id;
> >> >> + enum dma_transfer_direction direction;
> >> > This looks suspect. Why should channel have direction, for a descriptor 
> >> > it
> >> > makes sense though.
> >>
> >> The channel only supports transfers in one direction. Either from memory to
> >> peripheral or from peripheral to memory, that's fixed and can't be changed
> >> at runtime.  So, the driver needs to know which direction the channel 
> >> supports
> >> and hence it can reject transfers with the wrong direction.
> > But you already have this information in descriptor so why duplicate?
> 
> Our descriptor doesn't have the information of channel direction, we have this
> information only in the chan structure (struct xilinx_dma_chan) which
> is populated
> while parsing the DT.  So, we use this information to ensure if we are 
> servicing
> proper channel (in prep_slave_sg call).
One of the argument of prep_slave_sg call is direction. You need to use that
and store in in your descriptor for using it afterwards

> FYI: We also need the direction of the channel so that we can select SRC/DST
> register address in the issue_pending() call.
And you have the descriptor there as well so can use it

-- 
~Vinod

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


Re: [PATCH v3 2/2] dma: Add Xilinx AXI Direct Memory Access Engine driver support

2014-09-08 Thread Srikanth Thokala
Hi Vinod,

On Thu, Sep 4, 2014 at 12:06 PM, Vinod Koul  wrote:
> On Wed, Sep 03, 2014 at 12:17:43PM +0530, Srikanth Thokala wrote:
>> Hi Vinod,
>>
>> Apologies for the delay.
>>
>> On Tue, Aug 19, 2014 at 10:33 PM, Vinod Koul  wrote:
>> > On Mon, Jul 28, 2014 at 05:47:49PM +0530, Srikanth Thokala wrote:
>> >> +struct xilinx_dma_chan {
>> >> + struct xilinx_dma_device *xdev;
>> >> + u32 ctrl_offset;
>> >> + spinlock_t lock;
>> >> + struct list_head pending_list;
>> >> + struct xilinx_dma_tx_descriptor *active_desc;
>> >> + struct xilinx_dma_tx_descriptor *allocated_desc;
>> >> + struct list_head done_list;
>> >> + struct list_head free_seg_list;
>> >> + struct dma_chan common;
>> >> + struct xilinx_dma_tx_segment *seg_v;
>> >> + dma_addr_t seg_p;
>> >> + struct device *dev;
>> >> + int irq;
>> >> + int id;
>> >> + enum dma_transfer_direction direction;
>> > This looks suspect. Why should channel have direction, for a descriptor it
>> > makes sense though.
>>
>> The channel only supports transfers in one direction. Either from memory to
>> peripheral or from peripheral to memory, that's fixed and can't be changed
>> at runtime.  So, the driver needs to know which direction the channel 
>> supports
>> and hence it can reject transfers with the wrong direction.
> But you already have this information in descriptor so why duplicate?

Our descriptor doesn't have the information of channel direction, we have this
information only in the chan structure (struct xilinx_dma_chan) which
is populated
while parsing the DT.  So, we use this information to ensure if we are servicing
proper channel (in prep_slave_sg call).

FYI: We also need the direction of the channel so that we can select SRC/DST
register address in the issue_pending() call.

Could you please elaborate, if I am missing something?

>
>> >> +/**
>> >> + * xilinx_dma_channel_set_config - Configure DMA channel
>> >> + * @dchan: DMA channel
>> >> + * @cfg: DMA device configuration pointer
>> >> + *
>> >> + * Return: '0' on success and failure value on error
>> >> + */
>> >> +int xilinx_dma_channel_set_config(struct dma_chan *dchan,
>> >> +   struct xilinx_dma_config *cfg)
>> >> +{
>> >> + struct xilinx_dma_chan *chan = to_xilinx_chan(dchan);
>> >> + u32 reg = dma_ctrl_read(chan, XILINX_DMA_REG_CONTROL);
>> >> +
>> >> + if (cfg->reset)
>> >> + return xilinx_dma_reset(chan);
>> >> +
>> >> + if (cfg->coalesc <= XILINX_DMA_CR_COALESCE_MAX)
>> >> + reg |= cfg->coalesc << XILINX_DMA_CR_COALESCE_SHIFT;
>> >> +
>> >> + if (cfg->delay <= XILINX_DMA_CR_DELAY_MAX)
>> >> + reg |= cfg->delay << XILINX_DMA_CR_DELAY_SHIFT;
>> >> +
>> >> + dma_ctrl_write(chan, XILINX_DMA_REG_CONTROL, reg);
>> > You aren't checking if a transaction is already running on this channel.
>> > Also don't you need other slave parameters, I see you have removed the
>> > dma_slave_config entirely
>>
>> All the parameters that are required for this DMA engine are provided by
>> SG list, so I don't see the need of dma_slave_config.  There are some
>> specific IP parameters that are not part of dma_slave_config and these
>> are being set by 'dma_channel_set_config' which is exported to slave
>> drivers.
> And that means you dont have any common parameters between dma_slave_config
> right?

Yes, true.

Thanks,
Srikanth

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


Re: [PATCH v3 2/2] dma: Add Xilinx AXI Direct Memory Access Engine driver support

2014-09-03 Thread Vinod Koul
On Wed, Sep 03, 2014 at 12:17:43PM +0530, Srikanth Thokala wrote:
> Hi Vinod,
> 
> Apologies for the delay.
> 
> On Tue, Aug 19, 2014 at 10:33 PM, Vinod Koul  wrote:
> > On Mon, Jul 28, 2014 at 05:47:49PM +0530, Srikanth Thokala wrote:
> >> +struct xilinx_dma_chan {
> >> + struct xilinx_dma_device *xdev;
> >> + u32 ctrl_offset;
> >> + spinlock_t lock;
> >> + struct list_head pending_list;
> >> + struct xilinx_dma_tx_descriptor *active_desc;
> >> + struct xilinx_dma_tx_descriptor *allocated_desc;
> >> + struct list_head done_list;
> >> + struct list_head free_seg_list;
> >> + struct dma_chan common;
> >> + struct xilinx_dma_tx_segment *seg_v;
> >> + dma_addr_t seg_p;
> >> + struct device *dev;
> >> + int irq;
> >> + int id;
> >> + enum dma_transfer_direction direction;
> > This looks suspect. Why should channel have direction, for a descriptor it
> > makes sense though.
> 
> The channel only supports transfers in one direction. Either from memory to
> peripheral or from peripheral to memory, that's fixed and can't be changed
> at runtime.  So, the driver needs to know which direction the channel supports
> and hence it can reject transfers with the wrong direction.
But you already have this information in descriptor so why duplicate?

> >> +/**
> >> + * xilinx_dma_channel_set_config - Configure DMA channel
> >> + * @dchan: DMA channel
> >> + * @cfg: DMA device configuration pointer
> >> + *
> >> + * Return: '0' on success and failure value on error
> >> + */
> >> +int xilinx_dma_channel_set_config(struct dma_chan *dchan,
> >> +   struct xilinx_dma_config *cfg)
> >> +{
> >> + struct xilinx_dma_chan *chan = to_xilinx_chan(dchan);
> >> + u32 reg = dma_ctrl_read(chan, XILINX_DMA_REG_CONTROL);
> >> +
> >> + if (cfg->reset)
> >> + return xilinx_dma_reset(chan);
> >> +
> >> + if (cfg->coalesc <= XILINX_DMA_CR_COALESCE_MAX)
> >> + reg |= cfg->coalesc << XILINX_DMA_CR_COALESCE_SHIFT;
> >> +
> >> + if (cfg->delay <= XILINX_DMA_CR_DELAY_MAX)
> >> + reg |= cfg->delay << XILINX_DMA_CR_DELAY_SHIFT;
> >> +
> >> + dma_ctrl_write(chan, XILINX_DMA_REG_CONTROL, reg);
> > You aren't checking if a transaction is already running on this channel.
> > Also don't you need other slave parameters, I see you have removed the
> > dma_slave_config entirely
> 
> All the parameters that are required for this DMA engine are provided by
> SG list, so I don't see the need of dma_slave_config.  There are some
> specific IP parameters that are not part of dma_slave_config and these
> are being set by 'dma_channel_set_config' which is exported to slave
> drivers.
And that means you dont have any common parameters between dma_slave_config
right?

-- 
~Vinod

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


Re: [PATCH v3 2/2] dma: Add Xilinx AXI Direct Memory Access Engine driver support

2014-09-02 Thread Srikanth Thokala
Hi Vinod,

Apologies for the delay.

On Tue, Aug 19, 2014 at 10:33 PM, Vinod Koul  wrote:
> On Mon, Jul 28, 2014 at 05:47:49PM +0530, Srikanth Thokala wrote:
>> +struct xilinx_dma_chan {
>> + struct xilinx_dma_device *xdev;
>> + u32 ctrl_offset;
>> + spinlock_t lock;
>> + struct list_head pending_list;
>> + struct xilinx_dma_tx_descriptor *active_desc;
>> + struct xilinx_dma_tx_descriptor *allocated_desc;
>> + struct list_head done_list;
>> + struct list_head free_seg_list;
>> + struct dma_chan common;
>> + struct xilinx_dma_tx_segment *seg_v;
>> + dma_addr_t seg_p;
>> + struct device *dev;
>> + int irq;
>> + int id;
>> + enum dma_transfer_direction direction;
> This looks suspect. Why should channel have direction, for a descriptor it
> makes sense though.

The channel only supports transfers in one direction. Either from memory to
peripheral or from peripheral to memory, that's fixed and can't be changed
at runtime.  So, the driver needs to know which direction the channel supports
and hence it can reject transfers with the wrong direction.

>
>> +static enum dma_status xilinx_dma_tx_status(struct dma_chan *dchan,
>> + dma_cookie_t cookie,
>> + struct dma_tx_state *txstate)
>> +{
>> + struct xilinx_dma_chan *chan = to_xilinx_chan(dchan);
>> + enum dma_status ret;
>> + unsigned long flags;
>> +
>> + ret = dma_cookie_status(dchan, cookie, txstate);
>> + if (ret != DMA_COMPLETE) {
>> + spin_lock_irqsave(&chan->lock, flags);
>> + dma_set_residue(txstate, chan->residue);
>> + spin_unlock_irqrestore(&chan->lock, flags);
>> + }
> No residue reporting?

I will fix this, thanks.

>
>> +static int xilinx_dma_device_control(struct dma_chan *dchan,
>> +  enum dma_ctrl_cmd cmd, unsigned long arg)
>> +{
>> + struct xilinx_dma_chan *chan = to_xilinx_chan(dchan);
>> + unsigned long flags;
>> +
>> + if (cmd != DMA_TERMINATE_ALL)
>> + return -ENXIO;
>> +
>> + /* Halt the DMA engine */
>> + xilinx_dma_halt(chan);
>> +
>> + spin_lock_irqsave(&chan->lock, flags);
>> +
>> + /* Remove and free all of the descriptors in the lists */
>> + xilinx_dma_free_desc_list(chan, &chan->pending_list);
>> + xilinx_dma_free_desc_list(chan, &chan->done_list);
>
>> +
>> + spin_unlock_irqrestore(&chan->lock, flags);
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * xilinx_dma_channel_set_config - Configure DMA channel
>> + * @dchan: DMA channel
>> + * @cfg: DMA device configuration pointer
>> + *
>> + * Return: '0' on success and failure value on error
>> + */
>> +int xilinx_dma_channel_set_config(struct dma_chan *dchan,
>> +   struct xilinx_dma_config *cfg)
>> +{
>> + struct xilinx_dma_chan *chan = to_xilinx_chan(dchan);
>> + u32 reg = dma_ctrl_read(chan, XILINX_DMA_REG_CONTROL);
>> +
>> + if (cfg->reset)
>> + return xilinx_dma_reset(chan);
>> +
>> + if (cfg->coalesc <= XILINX_DMA_CR_COALESCE_MAX)
>> + reg |= cfg->coalesc << XILINX_DMA_CR_COALESCE_SHIFT;
>> +
>> + if (cfg->delay <= XILINX_DMA_CR_DELAY_MAX)
>> + reg |= cfg->delay << XILINX_DMA_CR_DELAY_SHIFT;
>> +
>> + dma_ctrl_write(chan, XILINX_DMA_REG_CONTROL, reg);
> You aren't checking if a transaction is already running on this channel.
> Also don't you need other slave parameters, I see you have removed the
> dma_slave_config entirely

All the parameters that are required for this DMA engine are provided by
SG list, so I don't see the need of dma_slave_config.  There are some
specific IP parameters that are not part of dma_slave_config and these
are being set by 'dma_channel_set_config' which is exported to slave
drivers.

Thanks for the reveiw,
Srikanth

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


Re: [PATCH v3 2/2] dma: Add Xilinx AXI Direct Memory Access Engine driver support

2014-08-19 Thread Vinod Koul
On Mon, Jul 28, 2014 at 05:47:49PM +0530, Srikanth Thokala wrote:
> +struct xilinx_dma_chan {
> + struct xilinx_dma_device *xdev;
> + u32 ctrl_offset;
> + spinlock_t lock;
> + struct list_head pending_list;
> + struct xilinx_dma_tx_descriptor *active_desc;
> + struct xilinx_dma_tx_descriptor *allocated_desc;
> + struct list_head done_list;
> + struct list_head free_seg_list;
> + struct dma_chan common;
> + struct xilinx_dma_tx_segment *seg_v;
> + dma_addr_t seg_p;
> + struct device *dev;
> + int irq;
> + int id;
> + enum dma_transfer_direction direction;
This looks suspect. Why should channel have direction, for a descriptor it
makes sense though.

> +static enum dma_status xilinx_dma_tx_status(struct dma_chan *dchan,
> + dma_cookie_t cookie,
> + struct dma_tx_state *txstate)
> +{
> + struct xilinx_dma_chan *chan = to_xilinx_chan(dchan);
> + enum dma_status ret;
> + unsigned long flags;
> +
> + ret = dma_cookie_status(dchan, cookie, txstate);
> + if (ret != DMA_COMPLETE) {
> + spin_lock_irqsave(&chan->lock, flags);
> + dma_set_residue(txstate, chan->residue);
> + spin_unlock_irqrestore(&chan->lock, flags);
> + }
No residue reporting?

> +static int xilinx_dma_device_control(struct dma_chan *dchan,
> +  enum dma_ctrl_cmd cmd, unsigned long arg)
> +{
> + struct xilinx_dma_chan *chan = to_xilinx_chan(dchan);
> + unsigned long flags;
> +
> + if (cmd != DMA_TERMINATE_ALL)
> + return -ENXIO;
> +
> + /* Halt the DMA engine */
> + xilinx_dma_halt(chan);
> +
> + spin_lock_irqsave(&chan->lock, flags);
> +
> + /* Remove and free all of the descriptors in the lists */
> + xilinx_dma_free_desc_list(chan, &chan->pending_list);
> + xilinx_dma_free_desc_list(chan, &chan->done_list);

> +
> + spin_unlock_irqrestore(&chan->lock, flags);
> +
> + return 0;
> +}
> +
> +/**
> + * xilinx_dma_channel_set_config - Configure DMA channel
> + * @dchan: DMA channel
> + * @cfg: DMA device configuration pointer
> + *
> + * Return: '0' on success and failure value on error
> + */
> +int xilinx_dma_channel_set_config(struct dma_chan *dchan,
> +   struct xilinx_dma_config *cfg)
> +{
> + struct xilinx_dma_chan *chan = to_xilinx_chan(dchan);
> + u32 reg = dma_ctrl_read(chan, XILINX_DMA_REG_CONTROL);
> +
> + if (cfg->reset)
> + return xilinx_dma_reset(chan);
> +
> + if (cfg->coalesc <= XILINX_DMA_CR_COALESCE_MAX)
> + reg |= cfg->coalesc << XILINX_DMA_CR_COALESCE_SHIFT;
> +
> + if (cfg->delay <= XILINX_DMA_CR_DELAY_MAX)
> + reg |= cfg->delay << XILINX_DMA_CR_DELAY_SHIFT;
> +
> + dma_ctrl_write(chan, XILINX_DMA_REG_CONTROL, reg);
You aren't checking if a transaction is already running on this channel.
Also don't you need other slave parameters, I see you have removed the
dma_slave_config entirely

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


Re: [PATCH v3 2/2] dma: Add Xilinx AXI Direct Memory Access Engine driver support

2014-08-08 Thread Michal Simek
Hi Vinod,

any update on this one?

Thanks,
Michal

On 08/01/2014 06:59 AM, Srikanth Thokala wrote:
> Hi,
> 
> Kindly review this patch and please provide your inputs.
> 
> Thanks
> Srikanth
> 
> On Mon, Jul 28, 2014 at 5:47 PM, Srikanth Thokala  wrote:
>> This is the driver for the AXI Direct Memory Access (AXI DMA)
>> core, which is a soft Xilinx IP core that provides high-
>> bandwidth direct memory access between memory and AXI4-Stream
>> type target peripherals.
>>
>> This module works on Zynq (ARM Based SoC) and Microblaze platforms.
>>
>> Signed-off-by: Srikanth Thokala 
>> ---
>> Changes in v3:
>> - Rebased on 3.16-rc7
>>
>> Changes in v2:
>> - Simplified the logic to set SOP and APP words in prep_slave_sg().
>> - Corrected function description comments to match the return type.
>> - Fixed some minor comments as suggested by Andy, Thanks.
>> ---
>>  drivers/dma/Kconfig |   13 +
>>  drivers/dma/xilinx/Makefile |1 +
>>  drivers/dma/xilinx/xilinx_dma.c | 1225 
>> +++
>>  include/linux/amba/xilinx_dma.h |   17 +
>>  4 files changed, 1256 insertions(+)
>>  create mode 100644 drivers/dma/xilinx/xilinx_dma.c
>>
>> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
>> index 1eca7b9..b8e831e 100644
>> --- a/drivers/dma/Kconfig
>> +++ b/drivers/dma/Kconfig
>> @@ -375,6 +375,19 @@ config XILINX_VDMA
>>   channels, Memory Mapped to Stream (MM2S) and Stream to
>>   Memory Mapped (S2MM) for the data transfers.
>>
>> +config XILINX_DMA
>> +   tristate "Xilinx AXI DMA Engine"
>> +   depends on (ARCH_ZYNQ || MICROBLAZE)
>> +   select DMA_ENGINE
>> +   help
>> + Enable support for Xilinx AXI DMA Soft IP.
>> +
>> + This engine provides high-bandwidth direct memory access
>> + between memory and AXI4-Stream type target peripherals.
>> + It has two stream interfaces/channels, Memory Mapped to
>> + Stream (MM2S) and Stream to Memory Mapped (S2MM) for the
>> + data transfers.
>> +
>>  config DMA_ENGINE
>> bool
>>
>> diff --git a/drivers/dma/xilinx/Makefile b/drivers/dma/xilinx/Makefile
>> index 3c4e9f2..6224a49 100644
>> --- a/drivers/dma/xilinx/Makefile
>> +++ b/drivers/dma/xilinx/Makefile
>> @@ -1 +1,2 @@
>>  obj-$(CONFIG_XILINX_VDMA) += xilinx_vdma.o
>> +obj-$(CONFIG_XILINX_DMA) += xilinx_dma.o
>> diff --git a/drivers/dma/xilinx/xilinx_dma.c 
>> b/drivers/dma/xilinx/xilinx_dma.c
>> new file mode 100644
>> index 000..0500773
>> --- /dev/null
>> +++ b/drivers/dma/xilinx/xilinx_dma.c
>> @@ -0,0 +1,1225 @@
>> +/*
>> + * DMA driver for Xilinx DMA Engine
>> + *
>> + * Copyright (C) 2010 - 2014 Xilinx, Inc. All rights reserved.
>> + *
>> + * Based on the Freescale DMA driver.
>> + *
>> + * Description:
>> + *  The AXI DMA, is a soft IP, which provides high-bandwidth Direct Memory
>> + *  Access between memory and AXI4-Stream-type target peripherals. It can be
>> + *  configured to have one channel or two channels and if configured as two
>> + *  channels, one is to transmit data from memory to a device and another is
>> + *  to receive from a device.
>> + *
>> + * This program is free software: you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation, either version 2 of the License, or
>> + * (at your option) any later version.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include "../dmaengine.h"
>> +
>> +/* Register Offsets */
>> +#define XILINX_DMA_REG_CONTROL 0x00
>> +#define XILINX_DMA_REG_STATUS  0x04
>> +#define XILINX_DMA_REG_CURDESC 0x08
>> +#define XILINX_DMA_REG_TAILDESC0x10
>> +#define XILINX_DMA_REG_SRCADDR 0x18
>> +#define XILINX_DMA_REG_DSTADDR 0x20
>> +#define XILINX_DMA_REG_BTT 0x28
>> +
>> +/* Channel/Descriptor Offsets */
>> +#define XILINX_DMA_MM2S_CTRL_OFFSET0x00
>> +#define XILINX_DMA_S2MM_CTRL_OFFSET0x30
>> +
>> +/* General register bits definitions */
>> +#define XILINX_DMA_CR_RUNSTOP_MASK BIT(0)
>> +#define XILINX_DMA_CR_RESET_MASK   BIT(2)
>> +
>> +#define XILINX_DMA_CR_DELAY_SHIFT  24
>> +#define XILINX_DMA_CR_COALESCE_SHIFT   16
>> +
>> +#define XILINX_DMA_CR_DELAY_MAXGENMASK(7, 0)
>> +#define XILINX_DMA_CR_COALESCE_MAX GENMASK(7, 0)
>> +
>> +#define XILINX_DMA_SR_HALTED_MASK  BIT(0)
>> +#define XILINX_DMA_SR_IDLE_MASKBIT(1)
>> +
>> +#define XILINX_DMA_XR_IRQ_IOC_MASK BIT(12)
>> +#define XILINX_DMA_XR_IRQ_DELAY_MASK   BIT(13)
>> +#define XILINX_DMA_XR_IRQ_ERROR_MASK   BIT(14)
>> +#define XILINX_DMA_XR_IRQ_ALL_MASK GENMASK(14, 12)
>> +
>> +/* BD definitions */
>> +#define XILINX_DMA_BD_STS_ALL_MASK GENMASK(31, 28)
>> +#define XILINX_DMA_BD_SOP  BIT(27)
>> +#define XILINX_DMA_BD_E

Re: [PATCH v3 2/2] dma: Add Xilinx AXI Direct Memory Access Engine driver support

2014-07-31 Thread Srikanth Thokala
Hi,

Kindly review this patch and please provide your inputs.

Thanks
Srikanth

On Mon, Jul 28, 2014 at 5:47 PM, Srikanth Thokala  wrote:
> This is the driver for the AXI Direct Memory Access (AXI DMA)
> core, which is a soft Xilinx IP core that provides high-
> bandwidth direct memory access between memory and AXI4-Stream
> type target peripherals.
>
> This module works on Zynq (ARM Based SoC) and Microblaze platforms.
>
> Signed-off-by: Srikanth Thokala 
> ---
> Changes in v3:
> - Rebased on 3.16-rc7
>
> Changes in v2:
> - Simplified the logic to set SOP and APP words in prep_slave_sg().
> - Corrected function description comments to match the return type.
> - Fixed some minor comments as suggested by Andy, Thanks.
> ---
>  drivers/dma/Kconfig |   13 +
>  drivers/dma/xilinx/Makefile |1 +
>  drivers/dma/xilinx/xilinx_dma.c | 1225 
> +++
>  include/linux/amba/xilinx_dma.h |   17 +
>  4 files changed, 1256 insertions(+)
>  create mode 100644 drivers/dma/xilinx/xilinx_dma.c
>
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index 1eca7b9..b8e831e 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -375,6 +375,19 @@ config XILINX_VDMA
>   channels, Memory Mapped to Stream (MM2S) and Stream to
>   Memory Mapped (S2MM) for the data transfers.
>
> +config XILINX_DMA
> +   tristate "Xilinx AXI DMA Engine"
> +   depends on (ARCH_ZYNQ || MICROBLAZE)
> +   select DMA_ENGINE
> +   help
> + Enable support for Xilinx AXI DMA Soft IP.
> +
> + This engine provides high-bandwidth direct memory access
> + between memory and AXI4-Stream type target peripherals.
> + It has two stream interfaces/channels, Memory Mapped to
> + Stream (MM2S) and Stream to Memory Mapped (S2MM) for the
> + data transfers.
> +
>  config DMA_ENGINE
> bool
>
> diff --git a/drivers/dma/xilinx/Makefile b/drivers/dma/xilinx/Makefile
> index 3c4e9f2..6224a49 100644
> --- a/drivers/dma/xilinx/Makefile
> +++ b/drivers/dma/xilinx/Makefile
> @@ -1 +1,2 @@
>  obj-$(CONFIG_XILINX_VDMA) += xilinx_vdma.o
> +obj-$(CONFIG_XILINX_DMA) += xilinx_dma.o
> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> new file mode 100644
> index 000..0500773
> --- /dev/null
> +++ b/drivers/dma/xilinx/xilinx_dma.c
> @@ -0,0 +1,1225 @@
> +/*
> + * DMA driver for Xilinx DMA Engine
> + *
> + * Copyright (C) 2010 - 2014 Xilinx, Inc. All rights reserved.
> + *
> + * Based on the Freescale DMA driver.
> + *
> + * Description:
> + *  The AXI DMA, is a soft IP, which provides high-bandwidth Direct Memory
> + *  Access between memory and AXI4-Stream-type target peripherals. It can be
> + *  configured to have one channel or two channels and if configured as two
> + *  channels, one is to transmit data from memory to a device and another is
> + *  to receive from a device.
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "../dmaengine.h"
> +
> +/* Register Offsets */
> +#define XILINX_DMA_REG_CONTROL 0x00
> +#define XILINX_DMA_REG_STATUS  0x04
> +#define XILINX_DMA_REG_CURDESC 0x08
> +#define XILINX_DMA_REG_TAILDESC0x10
> +#define XILINX_DMA_REG_SRCADDR 0x18
> +#define XILINX_DMA_REG_DSTADDR 0x20
> +#define XILINX_DMA_REG_BTT 0x28
> +
> +/* Channel/Descriptor Offsets */
> +#define XILINX_DMA_MM2S_CTRL_OFFSET0x00
> +#define XILINX_DMA_S2MM_CTRL_OFFSET0x30
> +
> +/* General register bits definitions */
> +#define XILINX_DMA_CR_RUNSTOP_MASK BIT(0)
> +#define XILINX_DMA_CR_RESET_MASK   BIT(2)
> +
> +#define XILINX_DMA_CR_DELAY_SHIFT  24
> +#define XILINX_DMA_CR_COALESCE_SHIFT   16
> +
> +#define XILINX_DMA_CR_DELAY_MAXGENMASK(7, 0)
> +#define XILINX_DMA_CR_COALESCE_MAX GENMASK(7, 0)
> +
> +#define XILINX_DMA_SR_HALTED_MASK  BIT(0)
> +#define XILINX_DMA_SR_IDLE_MASKBIT(1)
> +
> +#define XILINX_DMA_XR_IRQ_IOC_MASK BIT(12)
> +#define XILINX_DMA_XR_IRQ_DELAY_MASK   BIT(13)
> +#define XILINX_DMA_XR_IRQ_ERROR_MASK   BIT(14)
> +#define XILINX_DMA_XR_IRQ_ALL_MASK GENMASK(14, 12)
> +
> +/* BD definitions */
> +#define XILINX_DMA_BD_STS_ALL_MASK GENMASK(31, 28)
> +#define XILINX_DMA_BD_SOP  BIT(27)
> +#define XILINX_DMA_BD_EOP  BIT(26)
> +
> +/* Hw specific definitions */
> +#define XILINX_DMA_MAX_CHANS_PER_DEVICE0x2
> +#define XILINX_DMA_MAX_TRANS_LEN   GENMASK(22, 0)
> +
> +/* Delay loop counter to prevent hardware failure */
> +#define XILINX_D