On 28.09.2017 19:22, Vinod Koul wrote:

On Thu, Sep 28, 2017 at 05:35:59PM +0300, Dmitry Osipenko wrote:

quoted

On 28.09.2017 17:06, Dmitry Osipenko wrote:

quoted

On 28.09.2017 12:29, Vinod Koul wrote:

quoted

quoted

+	default:
+		return -EINVAL;
+	}
+
+	ahb_seq  = burst << TEGRA_AHBDMA_CHANNEL_AHB_BURST_SHIFT;
+	ahb_seq |= TEGRA_AHBDMA_CHANNEL_ADDR_WRAP;
+	ahb_seq |= TEGRA_AHBDMA_CHANNEL_INTR_ENB;
+
+	writel_relaxed(ahb_seq,
+		       ahbdma_chan->regs + TEGRA_AHBDMA_CHANNEL_AHB_SEQ);
+
+	writel_relaxed(ahb_addr,
+		       ahbdma_chan->regs + TEGRA_AHBDMA_CHANNEL_AHB_PTR);

oh no, you don't write to HW here. This can be called anytime when you have
txn running! You should save these and use them in prep_ calls.

BTW, some of the DMA drivers have exactly the same problem. I now see that it is
actually documented explicitly in provider.txt, but that's inconsistent across
the actual drivers.

Also, shouldn't prep_ and dma_slave_config be protected with locking? I don't
see DMA core doing any locking and seems none of the drivers too.

In prep when you modify the list yes (with vchan I suspect that maybe taken
care), but in general yes driver needs to do that

I meant that one CPU could modify channels config, while other CPU is preparing
the new TX using config that is in process of the modification. On the other
hand, this looks like something that DMA client should take care of.

-- 
Dmitry