All of lore.kernel.org
 help / color / mirror / Atom feed
From: vinod.koul@intel.com (Vinod Koul)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v5] dma: Add Xilinx AXI Direct Memory Access Engine driver support
Date: Tue, 17 Mar 2015 16:37:39 +0530	[thread overview]
Message-ID: <20150317110739.GE32683@intel.com> (raw)
In-Reply-To: <6f3b34935c0f405199cacd5312e972ac@BN1BFFO11FD017.protection.gbl>

On Mon, Mar 02, 2015 at 11:25:11PM +0530, Kedareswara rao Appana 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.
> 
> Signed-off-by: Srikanth Thokala <sthokal@xilinx.com>
> Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
> ---
> This patch is rebased on top of dma: xilinx-dma: move header file
> to common location.
but not on slave-dma next some API update is required..


> +/*
> + * DMA driver for Xilinx DMA Engine
> + *
> + * Copyright (C) 2010 - 2014 Xilinx, Inc. All rights reserved.
2015?

> +static struct xilinx_dma_tx_descriptor *
> +xilinx_dma_alloc_tx_descriptor(struct xilinx_dma_chan *chan)
> +{
> +	struct xilinx_dma_tx_descriptor *desc;
> +	unsigned long flags;
> +
> +	if (chan->allocated_desc)
> +		return chan->allocated_desc;
> +
> +	desc = kzalloc(sizeof(*desc), GFP_KERNEL);
GFP_NOWAIT

> +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) {
txstate can be null

> +		spin_lock_irqsave(&chan->lock, flags);
> +		dma_set_residue(txstate, chan->residue);
the queried descriptor may not be submitted to HW. Also the expectations are
that you will read current value from HW and calculate residue

> +static int xilinx_dma_device_slave_caps(struct dma_chan *dchan,
> +					struct dma_slave_caps *caps)
> +{
> +	caps->directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
> +	caps->cmd_terminate = true;
> +	caps->residue_granularity = DMA_RESIDUE_GRANULARITY_SEGMENT;
> +
> +	return 0;
> +}
this is based on older API, pls update


> +static void xilinx_dma_complete_descriptor(struct xilinx_dma_chan *chan)
> +{
> +	struct xilinx_dma_tx_descriptor *desc;
> +	struct xilinx_dma_tx_segment *segment, *next;
> +	struct xilinx_dma_desc_hw *hw;
> +	unsigned long flags;
> +	u32 residue = 0;
> +
> +	spin_lock_irqsave(&chan->lock, flags);
> +
> +	desc = chan->active_desc;
> +	if (!desc) {
> +		dev_dbg(chan->dev, "no running descriptors\n");
> +		goto out_unlock;
> +	}
> +
> +	if (chan->has_sg) {
> +		list_for_each_entry_safe(segment, next, &desc->segments, node) {
> +			hw = &segment->hw;
> +			residue += (hw->control - hw->status) &
> +				   XILINX_DMA_MAX_TRANS_LEN;
> +		}
why are we calculating residue here?
> +	}
> +
> +	chan->residue = residue;
and this is used in status call, so completely wrong!

> +static dma_cookie_t xilinx_dma_tx_submit(struct dma_async_tx_descriptor *tx)
> +{
> +	struct xilinx_dma_tx_descriptor *desc = to_dma_tx_descriptor(tx);
> +	struct xilinx_dma_chan *chan = to_xilinx_chan(tx->chan);
> +	dma_cookie_t cookie;
> +	unsigned long flags;
> +	int err;
> +
> +	if (chan->err) {
> +		/*
> +		 * If reset fails, need to hard reset the system.
> +		 * Channel is no longer functional
> +		 */
> +		err = xilinx_dma_reset(chan);
> +		if (err < 0)
> +			return err;
> +	}
> +
> +	spin_lock_irqsave(&chan->lock, flags);
> +
> +	cookie = dma_cookie_assign(tx);
> +
> +	/* Append the transaction to the pending transactions queue. */
> +	list_add_tail(&desc->node, &chan->pending_list);
> +
> +	/* Free the allocated desc */
> +	chan->allocated_desc = NULL;
this bit is confusing, can you explain what is going on?

> +static struct dma_async_tx_descriptor *xilinx_dma_prep_slave_sg(
> +	struct dma_chan *dchan, struct scatterlist *sgl, unsigned int sg_len,
> +	enum dma_transfer_direction direction, unsigned long flags,
> +	void *context)
> +{
> +	struct xilinx_dma_chan *chan = to_xilinx_chan(dchan);
> +	struct xilinx_dma_tx_descriptor *desc;
> +	struct xilinx_dma_tx_segment *segment;
> +	struct xilinx_dma_desc_hw *hw;
> +	u32 *app_w = (u32 *)context;
> +	struct scatterlist *sg;
> +	size_t copy, sg_used;
> +	int i;
> +
> +	if (!is_slave_direction(direction))
> +		return NULL;
> +
> +	/* Allocate a transaction descriptor. */
> +	desc = xilinx_dma_alloc_tx_descriptor(chan);
> +	if (!desc)
> +		return NULL;
> +
> +	desc->direction = direction;
> +	dma_async_tx_descriptor_init(&desc->async_tx, &chan->common);
> +	desc->async_tx.tx_submit = xilinx_dma_tx_submit;
> +	desc->async_tx.cookie = 0;
?

> +	async_tx_ack(&desc->async_tx);
why?

> +
> +	/* Build transactions using information in the scatter gather list */
> +	for_each_sg(sgl, sg, sg_len, i) {
> +		sg_used = 0;
> +
> +		/* Loop until the entire scatterlist entry is used */
> +		while (sg_used < sg_dma_len(sg)) {
> +
> +			/* Get a free segment */
> +			segment = xilinx_dma_alloc_tx_segment(chan);
> +			if (!segment)
> +				goto error;
> +
> +			/*
> +			 * Calculate the maximum number of bytes to transfer,
> +			 * making sure it is less than the hw limit
> +			 */
> +			copy = min_t(size_t, sg_dma_len(sg) - sg_used,
> +				     XILINX_DMA_MAX_TRANS_LEN);
> +			hw = &segment->hw;
> +
> +			/* Fill in the descriptor */
> +			hw->buf_addr = sg_dma_address(sg) + sg_used;
> +
> +			hw->control = copy;
> +
> +			if (direction == DMA_MEM_TO_DEV) {
> +				if (app_w)
> +					memcpy(hw->app, app_w, sizeof(u32) *
> +					       XILINX_DMA_NUM_APP_WORDS);
> +
> +				/*
> +				 * For the first DMA_MEM_TO_DEV transfer,
> +				 * set SOP
> +				 */
> +				if (!i)
> +					hw->control |= XILINX_DMA_BD_SOP;
> +			}
no else?

> +static int xilinx_dma_remove(struct platform_device *pdev)
> +{
> +	struct xilinx_dma_device *xdev = platform_get_drvdata(pdev);
> +	int i;
> +
> +	of_dma_controller_free(pdev->dev.of_node);
> +	dma_async_device_unregister(&xdev->common);
> +
> +	for (i = 0; i < XILINX_DMA_MAX_CHANS_PER_DEVICE; i++)
> +		if (xdev->chan[i])
> +			xilinx_dma_chan_remove(xdev->chan[i]);
> +
at this point your irq is active and tasklet cna still be scheduled


-- 
~Vinod

WARNING: multiple messages have this Message-ID (diff)
From: Vinod Koul <vinod.koul@intel.com>
To: Kedareswara rao Appana <appana.durga.rao@xilinx.com>
Cc: dan.j.williams@intel.com, michal.simek@xilinx.com,
	soren.brinkmann@xilinx.com, dmaengine@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, appanad@xilinx.com,
	anirudh@xilinx.com, svemula@xilinx.com,
	Srikanth Thokala <sthokal@xilinx.com>
Subject: Re: [PATCH v5] dma: Add Xilinx AXI Direct Memory Access Engine driver support
Date: Tue, 17 Mar 2015 16:37:39 +0530	[thread overview]
Message-ID: <20150317110739.GE32683@intel.com> (raw)
In-Reply-To: <6f3b34935c0f405199cacd5312e972ac@BN1BFFO11FD017.protection.gbl>

On Mon, Mar 02, 2015 at 11:25:11PM +0530, Kedareswara rao Appana 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.
> 
> Signed-off-by: Srikanth Thokala <sthokal@xilinx.com>
> Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
> ---
> This patch is rebased on top of dma: xilinx-dma: move header file
> to common location.
but not on slave-dma next some API update is required..


> +/*
> + * DMA driver for Xilinx DMA Engine
> + *
> + * Copyright (C) 2010 - 2014 Xilinx, Inc. All rights reserved.
2015?

> +static struct xilinx_dma_tx_descriptor *
> +xilinx_dma_alloc_tx_descriptor(struct xilinx_dma_chan *chan)
> +{
> +	struct xilinx_dma_tx_descriptor *desc;
> +	unsigned long flags;
> +
> +	if (chan->allocated_desc)
> +		return chan->allocated_desc;
> +
> +	desc = kzalloc(sizeof(*desc), GFP_KERNEL);
GFP_NOWAIT

> +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) {
txstate can be null

> +		spin_lock_irqsave(&chan->lock, flags);
> +		dma_set_residue(txstate, chan->residue);
the queried descriptor may not be submitted to HW. Also the expectations are
that you will read current value from HW and calculate residue

> +static int xilinx_dma_device_slave_caps(struct dma_chan *dchan,
> +					struct dma_slave_caps *caps)
> +{
> +	caps->directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
> +	caps->cmd_terminate = true;
> +	caps->residue_granularity = DMA_RESIDUE_GRANULARITY_SEGMENT;
> +
> +	return 0;
> +}
this is based on older API, pls update


> +static void xilinx_dma_complete_descriptor(struct xilinx_dma_chan *chan)
> +{
> +	struct xilinx_dma_tx_descriptor *desc;
> +	struct xilinx_dma_tx_segment *segment, *next;
> +	struct xilinx_dma_desc_hw *hw;
> +	unsigned long flags;
> +	u32 residue = 0;
> +
> +	spin_lock_irqsave(&chan->lock, flags);
> +
> +	desc = chan->active_desc;
> +	if (!desc) {
> +		dev_dbg(chan->dev, "no running descriptors\n");
> +		goto out_unlock;
> +	}
> +
> +	if (chan->has_sg) {
> +		list_for_each_entry_safe(segment, next, &desc->segments, node) {
> +			hw = &segment->hw;
> +			residue += (hw->control - hw->status) &
> +				   XILINX_DMA_MAX_TRANS_LEN;
> +		}
why are we calculating residue here?
> +	}
> +
> +	chan->residue = residue;
and this is used in status call, so completely wrong!

> +static dma_cookie_t xilinx_dma_tx_submit(struct dma_async_tx_descriptor *tx)
> +{
> +	struct xilinx_dma_tx_descriptor *desc = to_dma_tx_descriptor(tx);
> +	struct xilinx_dma_chan *chan = to_xilinx_chan(tx->chan);
> +	dma_cookie_t cookie;
> +	unsigned long flags;
> +	int err;
> +
> +	if (chan->err) {
> +		/*
> +		 * If reset fails, need to hard reset the system.
> +		 * Channel is no longer functional
> +		 */
> +		err = xilinx_dma_reset(chan);
> +		if (err < 0)
> +			return err;
> +	}
> +
> +	spin_lock_irqsave(&chan->lock, flags);
> +
> +	cookie = dma_cookie_assign(tx);
> +
> +	/* Append the transaction to the pending transactions queue. */
> +	list_add_tail(&desc->node, &chan->pending_list);
> +
> +	/* Free the allocated desc */
> +	chan->allocated_desc = NULL;
this bit is confusing, can you explain what is going on?

> +static struct dma_async_tx_descriptor *xilinx_dma_prep_slave_sg(
> +	struct dma_chan *dchan, struct scatterlist *sgl, unsigned int sg_len,
> +	enum dma_transfer_direction direction, unsigned long flags,
> +	void *context)
> +{
> +	struct xilinx_dma_chan *chan = to_xilinx_chan(dchan);
> +	struct xilinx_dma_tx_descriptor *desc;
> +	struct xilinx_dma_tx_segment *segment;
> +	struct xilinx_dma_desc_hw *hw;
> +	u32 *app_w = (u32 *)context;
> +	struct scatterlist *sg;
> +	size_t copy, sg_used;
> +	int i;
> +
> +	if (!is_slave_direction(direction))
> +		return NULL;
> +
> +	/* Allocate a transaction descriptor. */
> +	desc = xilinx_dma_alloc_tx_descriptor(chan);
> +	if (!desc)
> +		return NULL;
> +
> +	desc->direction = direction;
> +	dma_async_tx_descriptor_init(&desc->async_tx, &chan->common);
> +	desc->async_tx.tx_submit = xilinx_dma_tx_submit;
> +	desc->async_tx.cookie = 0;
?

> +	async_tx_ack(&desc->async_tx);
why?

> +
> +	/* Build transactions using information in the scatter gather list */
> +	for_each_sg(sgl, sg, sg_len, i) {
> +		sg_used = 0;
> +
> +		/* Loop until the entire scatterlist entry is used */
> +		while (sg_used < sg_dma_len(sg)) {
> +
> +			/* Get a free segment */
> +			segment = xilinx_dma_alloc_tx_segment(chan);
> +			if (!segment)
> +				goto error;
> +
> +			/*
> +			 * Calculate the maximum number of bytes to transfer,
> +			 * making sure it is less than the hw limit
> +			 */
> +			copy = min_t(size_t, sg_dma_len(sg) - sg_used,
> +				     XILINX_DMA_MAX_TRANS_LEN);
> +			hw = &segment->hw;
> +
> +			/* Fill in the descriptor */
> +			hw->buf_addr = sg_dma_address(sg) + sg_used;
> +
> +			hw->control = copy;
> +
> +			if (direction == DMA_MEM_TO_DEV) {
> +				if (app_w)
> +					memcpy(hw->app, app_w, sizeof(u32) *
> +					       XILINX_DMA_NUM_APP_WORDS);
> +
> +				/*
> +				 * For the first DMA_MEM_TO_DEV transfer,
> +				 * set SOP
> +				 */
> +				if (!i)
> +					hw->control |= XILINX_DMA_BD_SOP;
> +			}
no else?

> +static int xilinx_dma_remove(struct platform_device *pdev)
> +{
> +	struct xilinx_dma_device *xdev = platform_get_drvdata(pdev);
> +	int i;
> +
> +	of_dma_controller_free(pdev->dev.of_node);
> +	dma_async_device_unregister(&xdev->common);
> +
> +	for (i = 0; i < XILINX_DMA_MAX_CHANS_PER_DEVICE; i++)
> +		if (xdev->chan[i])
> +			xilinx_dma_chan_remove(xdev->chan[i]);
> +
at this point your irq is active and tasklet cna still be scheduled


-- 
~Vinod

  parent reply	other threads:[~2015-03-17 11:07 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-02 17:55 [PATCH v5] dma: Add Xilinx AXI Direct Memory Access Engine driver support Kedareswara rao Appana
2015-03-02 17:55 ` Kedareswara rao Appana
2015-03-02 18:41 ` Josh Cartwright
2015-03-02 18:41   ` Josh Cartwright
2015-03-05  9:34   ` Appana Durga Kedareswara Rao
2015-03-05  9:34     ` Appana Durga Kedareswara Rao
2015-03-02 18:59 ` Nicolae Rosia
2015-03-02 18:59   ` Nicolae Rosia
2015-03-05  9:35   ` Appana Durga Kedareswara Rao
2015-03-05  9:35     ` Appana Durga Kedareswara Rao
2015-03-02 22:01 ` Paul Bolle
2015-03-02 22:01   ` Paul Bolle
2015-03-05  9:34   ` Appana Durga Kedareswara Rao
2015-03-05  9:34     ` Appana Durga Kedareswara Rao
2015-03-17 11:07 ` Vinod Koul [this message]
2015-03-17 11:07   ` Vinod Koul
2015-03-23 16:24   ` Appana Durga Kedareswara Rao
2015-03-23 16:24     ` Appana Durga Kedareswara Rao
2015-03-24 16:28     ` Vinod Koul
2015-03-24 16:28       ` Vinod Koul

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150317110739.GE32683@intel.com \
    --to=vinod.koul@intel.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.