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 devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
