Messages in this thread | | | Subject | Re: [PATCH v3 2/4] dmaengine: tegra: Add tegra gpcdma driver | From | Jon Hunter <> | Date | Thu, 2 Sep 2021 11:57:19 +0100 |
| |
On 01/09/2021 21:56, Jon Hunter wrote: > > On 27/08/2021 07:04, Akhil R wrote: >> Adding GPC DMA controller driver for Tegra186 and Tegra194. The driver >> supports dma transfers between memory to memory, IO peripheral to memory >> and memory to IO peripheral. >> >> Signed-off-by: Pavan Kunapuli <pkunapuli@nvidia.com> >> Signed-off-by: Rajesh Gumasta <rgumasta@nvidia.com> >> Signed-off-by: Akhil R <akhilrajeev@nvidia.com> >> --- >> drivers/dma/Kconfig | 12 + >> drivers/dma/Makefile | 1 + >> drivers/dma/tegra-gpc-dma.c | 1343 >> +++++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 1356 insertions(+) >> create mode 100644 drivers/dma/tegra-gpc-dma.c
...
>> +static int tegra_dma_terminate_all(struct dma_chan *dc) >> +{ >> + struct tegra_dma_channel *tdc = to_tegra_dma_chan(dc); >> + unsigned long wcount = 0; >> + unsigned long status; >> + unsigned long flags; >> + bool was_busy; >> + int err; >> + >> + raw_spin_lock_irqsave(&tdc->lock, flags); >> + >> + if (!tdc->dma_desc) { >> + raw_spin_unlock_irqrestore(&tdc->lock, flags); >> + return 0; >> + } >> + >> + if (!tdc->busy) >> + goto skip_dma_stop; >> + >> + if (tdc->tdma->chip_data->hw_support_pause) { >> + err = tegra_dma_pause(tdc); >> + if (err) { >> + raw_spin_unlock_irqrestore(&tdc->lock, flags); >> + return err; >> + } >> + } else { >> + /* Before Reading DMA status to figure out number >> + * of bytes transferred by DMA channel: >> + * Change the client associated with the DMA channel >> + * to stop DMA engine from starting any more bursts for >> + * the given client and wait for in flight bursts to complete >> + */ >> + tegra_dma_reset_client(tdc); >> + >> + /* Wait for in flight data transfer to finish */ >> + udelay(TEGRA_GPCDMA_BURST_COMPLETE_TIME); >> + >> + /* If TX/RX path is still active wait till it becomes >> + * inactive >> + */ >> + >> + if (readl_relaxed_poll_timeout_atomic(tdc->tdma->base_addr + >> + tdc->chan_base_offset + >> + TEGRA_GPCDMA_CHAN_STATUS, >> + status, >> + !(status & (TEGRA_GPCDMA_STATUS_CHANNEL_TX | >> + TEGRA_GPCDMA_STATUS_CHANNEL_RX)), >> + 5, >> + TEGRA_GPCDMA_BURST_COMPLETION_TIMEOUT)) { >> + dev_dbg(tdc2dev(tdc), "Timeout waiting for DMA burst >> completion!\n"); >> + tegra_dma_dump_chan_regs(tdc); >> + } > > I would be tempted to make the code in the 'else' clause > tegra_dma_sw_pause(). Then you could have tegra_dma_hw_pause() and > tegra_dma_sw_pause().
Thinking some more tegra_dma_hw_pause() and tegra_dma_sw_pause() it not very clear/accurate. I would be tempted to call these tegra_dma_pause() and tegra_dma_stop_client() or tegra_dma_stop_transactions(), because without having a proper hardware pause, you are simply ignoring the client sync events.
Jon -- nvpublic
| |