Re: [PATCH v9 RESEND 01/13] spi: imx: add dma_sync_sg_for_device after fallback from dma

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2020-06-08 16:31, Mark Brown wrote:
On Mon, Jun 08, 2020 at 03:08:45PM +0000, Robin Gong wrote:

+	if (transfer->rx_sg.sgl) {
+		struct device *rx_dev = spi->controller->dma_rx->device->dev;
+
+		dma_sync_sg_for_device(rx_dev, transfer->rx_sg.sgl,
+				       transfer->rx_sg.nents, DMA_TO_DEVICE);
+	}
+

This is confusing - why are we DMA mapping to the device after doing a PIO
transfer?

'transfer->rx_sg.sgl' condition check that's the case fallback PIO after DMA transfer
failed. But the spi core still think the buffer should be in 'device' while spi driver
touch it by PIO(CPU), so sync it back to device to ensure all received data flush to DDR.

So we sync it back to the device so that we can then do another sync to
CPU?  TBH I'm a bit surprised that there's a requirement that we
explicitly undo a sync and that a redundant double sync in the same
direction might be an issue but I've not had a need to care so I'm
perfectly prepared to believe there is.

At the very least this needs a comment.

Yeah, something's off here - at the very least, syncing with DMA_TO_DEVICE on the Rx buffer that was mapped with DMA_FROM_DEVICE is clearly wrong. CONFIG_DMA_API_DEBUG should scream about that.

If the device has written to the buffer at all since dma_map_sg() was called then you do need a dma_sync_sg_for_cpu() call before touching it from a CPU fallback path, but if nobody's going to touch it from that point until it's unmapped then there's no point syncing it again. The my_card_interrupt_handler() example in DMA-API_HOWTO.txt demonstrates this.

Robin.



[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux PCI]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux