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.