Re: [PATCH 22/28] sgiseeq: convert from dma_cache_sync to dma_sync_single_for_device
From: Christoph Hellwig <hch@lst.de>
Date: 2020-09-01 17:16:48
Also in:
alsa-devel, linux-arm-kernel, linux-doc, linux-iommu, linux-media, linux-mips, linux-mm, linux-nvme, linux-samsung-soc, linux-scsi, lkml, nouveau
On Tue, Sep 01, 2020 at 07:12:41PM +0200, Thomas Bogendoerfer wrote:
On Tue, Sep 01, 2020 at 05:22:09PM +0200, Thomas Bogendoerfer wrote:quoted
On Wed, Aug 19, 2020 at 08:55:49AM +0200, Christoph Hellwig wrote:quoted
Use the proper modern API to transfer cache ownership for incoherent DMA. Signed-off-by: Christoph Hellwig <hch@lst.de> --- drivers/net/ethernet/seeq/sgiseeq.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)diff --git a/drivers/net/ethernet/seeq/sgiseeq.c b/drivers/net/ethernet/seeq/sgiseeq.c index 39599bbb5d45b6..f91dae16d69a19 100644 --- a/drivers/net/ethernet/seeq/sgiseeq.c +++ b/drivers/net/ethernet/seeq/sgiseeq.c@@ -112,14 +112,18 @@ struct sgiseeq_private { static inline void dma_sync_desc_cpu(struct net_device *dev, void *addr) { - dma_cache_sync(dev->dev.parent, addr, sizeof(struct sgiseeq_rx_desc), - DMA_FROM_DEVICE); + struct sgiseeq_private *sp = netdev_priv(dev); + + dma_sync_single_for_cpu(dev->dev.parent, VIRT_TO_DMA(sp, addr), + sizeof(struct sgiseeq_rx_desc), DMA_BIDIRECTIONAL); } static inline void dma_sync_desc_dev(struct net_device *dev, void *addr) { - dma_cache_sync(dev->dev.parent, addr, sizeof(struct sgiseeq_rx_desc), - DMA_TO_DEVICE); + struct sgiseeq_private *sp = netdev_priv(dev); + + dma_sync_single_for_device(dev->dev.parent, VIRT_TO_DMA(sp, addr), + sizeof(struct sgiseeq_rx_desc), DMA_BIDIRECTIONAL); }this breaks ethernet on IP22 completely, but I haven't figured out why, yet.the problem is that dma_sync_single_for_cpu() doesn't flush anything for IP22, because it only flushes for CPUs which do speculation. So either MIPS arch_sync_dma_for_cpu() should always flush or sgiseeq needs to use a different sync funktion, when it wants to re-read descriptors from memory.
Well, if IP22 doesn't speculate (which I'm pretty sure is the case), dma_sync_single_for_cpu should indeeed be a no-op. But then there also shouldn't be anything in the cache, as the previous dma_sync_single_for_device should have invalidated it. So it seems like we are missing one (or more) ownership transfers to the device. I'll try to look at the the ownership management in a little more detail tomorrow.