Messages in this thread | | | From | Daniel Kurtz <> | Date | Wed, 25 Jan 2017 18:24:12 +0800 | Subject | Re: [PATCH] spi: mediatek: Manually set dma_ops for spi_master device |
| |
Hi Robin,
On Tue, Jan 24, 2017 at 11:14 PM, Robin Murphy <robin.murphy@arm.com> wrote: > Hi Dan,
[snip...]
>> And I really don't know why we needed to set the coherent_dma_mask to 0 to >> avoid SPI transaction errors. > > Following what I mentioned above, "git grep dma_alloc drivers/spi" makes > it seem like coherent DMA isn't used anywhere relevant, which is rather > puzzling. Unless of course somewhere along the line somebody's done the > dev->dma_mask = &dev->dma_coherent_mask hack, with the result that > writing one mask hits both, making *all* DMA fail and big transfers fall > back to PIO.
You mean this last line?
>> @@ -575,6 +576,10 @@ static int mtk_spi_probe(struct platform_device *pdev) >> goto err_put_master; >> } >> >> + /* Call of_dma_configure to set up spi_master's dma_ops */ >> + of_dma_configure(&master->dev, master->dev.of_node); >> + /* But explicitly set the coherent_dma_mask to 0 */ >> + master->dev.coherent_dma_mask = 0; >> if (!pdev->dev.dma_mask) >> pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask; ^^^^^
As predicted, setting the dma_mask = 0 causes "all DMA to fail". In particular, dma_capable() always returns false, so swiotlb_map_sg_attrs() takes the map_single() path, instead of just assigning:
sg->dma_address = dev_addr;
swiotlb_map_sg_attrs(struct device *hwdev, struct scatterlist *sgl, int nelems, enum dma_data_direction dir, unsigned long attrs) { struct scatterlist *sg; int i;
BUG_ON(dir == DMA_NONE);
for_each_sg(sgl, sg, nelems, i) { phys_addr_t paddr = sg_phys(sg); dma_addr_t dev_addr = phys_to_dma(hwdev, paddr);
if (swiotlb_force == SWIOTLB_FORCE || !dma_capable(hwdev, dev_addr, sg->length)) { phys_addr_t map = map_single(hwdev, sg_phys(sg), sg->length, dir, attrs); ... } sg->dma_address = phys_to_dma(hwdev, map); } else sg->dma_address = dev_addr; sg_dma_len(sg) = sg->length; } return nelems; }
So, I think this means that the only reason the MTK SPI driver ever worked before was that it was tested on an older kernel, so the spi_master was defaulting to swiotlb_dma_ops with a 0 dma_mask, and therefore it was using SWIOTLB bounce buffers (via 'map_single'), and not actually ever doing real DMA.
I'm in a bit over my head here, any suggestions on how to fix this?
-Dan
| |