On 01/09/2021 21:56, Jon Hunter wrote:

On 27/08/2021 07:04, Akhil R wrote:

quoted

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 <redacted>
Signed-off-by: Rajesh Gumasta <redacted>
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

...

quoted

+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