| Message ID | 1353662559-26515-1-git-send-email-ldewangan@nvidia.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 11/23/2012 02:22 AM, Laxman Dewangan wrote: > Spi starts transfer using dma with DMA_CTRL_ACK which is not require > becasue spi driver does not use completed dma_desc after transfer > done and so it does not ack the dma descriptor. Removing the > DMA_CTRL_ACK flag to avoid memory leak in dma driver. I'm not quite sure, but isn't this the opposite of what's wanted. I think that setting this flag in prep() means that the SPI driver need not explicitly ack it later? At least, tegra_dma_desc_get() returns an allocated descriptor if one exists and async_tx_test_ack()==true for it, and it's true when the DMA_CTRL_ACK flag is set, which happens either due to calling async_tx_ack(), or because tegra_dma_prep_slave_sg() was called with DMA_CTRL_ACK in flags. ------------------------------------------------------------------------------ Monitor your physical, virtual and cloud infrastructure from a single web console. Get in-depth insight into apps, servers, databases, vmware, SAP, cloud infrastructure, etc. Download 30-day Free Trial. Pricing starts from $795 for 25 servers or applications! http://p.sf.net/sfu/zoho_dev2dev_nov
On Tuesday 27 November 2012 03:33 AM, Stephen Warren wrote: > On 11/23/2012 02:22 AM, Laxman Dewangan wrote: >> Spi starts transfer using dma with DMA_CTRL_ACK which is not require >> becasue spi driver does not use completed dma_desc after transfer >> done and so it does not ack the dma descriptor. Removing the >> DMA_CTRL_ACK flag to avoid memory leak in dma driver. > I'm not quite sure, but isn't this the opposite of what's wanted. I > think that setting this flag in prep() means that the SPI driver need > not explicitly ack it later? > > At least, tegra_dma_desc_get() returns an allocated descriptor if one > exists and async_tx_test_ack()==true for it, and it's true when the > DMA_CTRL_ACK flag is set, which happens either due to calling > async_tx_ack(), or because tegra_dma_prep_slave_sg() was called with > DMA_CTRL_ACK in flags. I think DMA_CTRL_ACK flag is require if we want to free/reuse desc only when client ack it. Although some part of implementation is wrong in the tegra20-apb-dma.c. static struct tegra_dma_desc *tegra_dma_desc_get( struct tegra_dma_channel *tdc) { struct tegra_dma_desc *dma_desc; unsigned long flags; spin_lock_irqsave(&tdc->lock, flags); /* Do not allocate if desc are waiting for ack */ list_for_each_entry(dma_desc, &tdc->free_dma_desc, node) { if (async_tx_test_ack(&dma_desc->txd)) { list_del(&dma_desc->node); ::::::::::::::: } In above it should be if (!async_tx_test_ack()) And when client done with the desc, then it can say as async_tx_clear_ack(). ------------------------------------------------------------------------------ Monitor your physical, virtual and cloud infrastructure from a single web console. Get in-depth insight into apps, servers, databases, vmware, SAP, cloud infrastructure, etc. Download 30-day Free Trial. Pricing starts from $795 for 25 servers or applications! http://p.sf.net/sfu/zoho_dev2dev_nov
On Monday 01 April 2013 07:05 PM, Mark Brown wrote: > * PGP Signed by an unknown key > > On Fri, Nov 23, 2012 at 02:52:39PM +0530, Laxman Dewangan wrote: >> Spi starts transfer using dma with DMA_CTRL_ACK which is not require >> becasue spi driver does not use completed dma_desc after transfer >> done and so it does not ack the dma descriptor. Removing the >> DMA_CTRL_ACK flag to avoid memory leak in dma driver. > Applied, thanks. Mark, There was bug in dma driver about handling of DMA_CTRL_ACK flag which we fixed after Stephen pointed out. This change is no more require. Can you please drop this patch? Let me know if I need to send the revert patch. Sorry for inconvenience. Thanks, Laxan ------------------------------------------------------------------------------ Minimize network downtime and maximize team effectiveness. Reduce network management and security costs.Learn how to hire the most talented Cisco Certified professionals. Visit the Employer Resources Portal http://www.cisco.com/web/learning/employer_resources/index.html
On Monday 01 April 2013 07:05 PM, Mark Brown wrote: > * PGP Signed by an unknown key > > On Fri, Nov 23, 2012 at 02:52:39PM +0530, Laxman Dewangan wrote: >> Spi starts transfer using dma with DMA_CTRL_ACK which is not require >> becasue spi driver does not use completed dma_desc after transfer >> done and so it does not ack the dma descriptor. Removing the >> DMA_CTRL_ACK flag to avoid memory leak in dma driver. > Applied, thanks. Mark, There was bug in dma driver about handling of DMA_CTRL_ACK flag which we fixed after Stephen pointed out. This change is no more require. Can you please drop this patch? Let me know if I need to send the revert patch. Sorry for inconvenience. Thanks, Laxan ------------------------------------------------------------------------------ Minimize network downtime and maximize team effectiveness. Reduce network management and security costs.Learn how to hire the most talented Cisco Certified professionals. Visit the Employer Resources Portal http://www.cisco.com/web/learning/employer_resources/index.html
diff --git a/drivers/spi/spi-tegra20-slink.c b/drivers/spi/spi-tegra20-slink.c index 7882b50..5b4c8c8 100644 --- a/drivers/spi/spi-tegra20-slink.c +++ b/drivers/spi/spi-tegra20-slink.c @@ -473,7 +473,7 @@ static int tegra_slink_start_tx_dma(struct tegra_slink_data *tspi, int len) INIT_COMPLETION(tspi->tx_dma_complete); tspi->tx_dma_desc = dmaengine_prep_slave_single(tspi->tx_dma_chan, tspi->tx_dma_phys, len, DMA_MEM_TO_DEV, - DMA_PREP_INTERRUPT | DMA_CTRL_ACK); + DMA_PREP_INTERRUPT); if (!tspi->tx_dma_desc) { dev_err(tspi->dev, "Not able to get desc for Tx\n"); return -EIO; @@ -492,7 +492,7 @@ static int tegra_slink_start_rx_dma(struct tegra_slink_data *tspi, int len) INIT_COMPLETION(tspi->rx_dma_complete); tspi->rx_dma_desc = dmaengine_prep_slave_single(tspi->rx_dma_chan, tspi->rx_dma_phys, len, DMA_DEV_TO_MEM, - DMA_PREP_INTERRUPT | DMA_CTRL_ACK); + DMA_PREP_INTERRUPT); if (!tspi->rx_dma_desc) { dev_err(tspi->dev, "Not able to get desc for Rx\n"); return -EIO;
Spi starts transfer using dma with DMA_CTRL_ACK which is not require becasue spi driver does not use completed dma_desc after transfer done and so it does not ack the dma descriptor. Removing the DMA_CTRL_ACK flag to avoid memory leak in dma driver. Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com> --- drivers/spi/spi-tegra20-slink.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)