If call dma_cache_sync() is incorrect, then which is the correct way? 1, call mips_dma_cache_sync()? 2, remove static of __dma_sync_virtual() and call it? Huacai On Thu, Jan 25, 2018 at 6:54 PM, James Hogan <jhogan@xxxxxxxxxx> wrote: > On Thu, Jan 25, 2018 at 04:44:36PM +0800, Huacai Chen wrote: >> On Thu, Jan 25, 2018 at 3:55 PM, James Hogan <jhogan@xxxxxxxxxx> wrote: >> > Hi Huacai, >> > >> > On Thu, Jan 25, 2018 at 03:09:33PM +0800, Huacai Chen wrote: >> >> Hi, James and Christoph, >> >> >> >> We have modified arch/mips/loongson64/common/dma-swiotlb.c to make >> >> Loongson support coherent&non-coherent devices co-exist. You can see >> >> code here: >> >> https://github.com/linux-loongson/linux-loongson/commit/3f212e36b2432a7c4a843649e4b79c9c0837d1d2 >> >> When device is non-coherent, we will call dma_cache_sync(). >> >> >> >> Then, if dma_cache_sync() is only designed for >> >> DMA_ATTR_NON_CONSISTENT, what can I use? >> > >> > How did you allocate the memory? The appropriate generic API for the >> > type of memory should always be used in drivers over arch specific >> > stuff. >> > >> > AFAIK (see e.g. Documentation/DMA-API.txt): >> > - dma_alloc_coherent() shouldn't require syncing, though it may require >> > flushing of write combiners >> > - dma_alloc_attrs() only requires syncing when DMA_ATTR_NON_CONSISTENT >> > is used, otherwise is the same as dma_alloc_coherent() >> > - guaranteed contiguous memory within PA range (e.g. kmalloc()'d memory >> > with the appropriate GFP_DMA flags) can be synced using the >> > dma_map_*() and dma_unmap_*() functions. > > I also see there is a dma_sync_*_for_{cpu,device} to avoid "mapping" and > "unmapping" the same area repeatedly if it is reused. > >> Yes, kmalloc()'d memory with the appropriate GFP_DMA flags can be >> synced using the dma_map_*() and dma_unmap_*() functions. So, >> loongson_dma_map_page()/loongson_dma_unmap_page() (which is the >> backend of dma_map_*() and dma_unmap_*()) should call dma_cache_sync() >> for non-coherent devices, right? > > Yeh, I suppose thats effectively what it should do (though probably via > an arch specific function rather than dma_cache_sync() itself). Also the > sync_*_for_{cpu,device} callbacks should probably do it too. See all the > calls to __dma_sync() in the MIPS dma-default.c implementation. > > Cheers > James > >> >> Huacai >> >> > >> > Cheers >> > James >> > >> >> 1, __dma_sync_virtual() and __dma_sync() are both static functions, so >> >> can't be called in other files. >> >> 2, mips_dma_cache_sync() is not static, but I think it isn't designed >> >> to be called directly (So it should be static). >> >> 3, dma_cache_wback(), dma_cache_inv() and dma_cache_wback_inv() does't >> >> take a "direction" argument, so I should determine the direction first >> >> (then I will reimplement __dma_cache_sync for myself). >> >> So, It seems that I can only use dma_cache_sync(). >> >> >> >> Huacai >> >> >> >> On Wed, Jan 24, 2018 at 11:29 PM, Christoph Hellwig <hch@xxxxxx> wrote: >> >> > On Wed, Jan 24, 2018 at 03:03:05PM +0000, James Hogan wrote: >> >> >> I see, that makes sense. Thanks Christoph. I'll assume this patch isn't >> >> >> applicable then unless Huacai adds some explanation. >> >> > >> >> > In addition there also is no driver that can be used on loongsoon >> >> > that actually calls dma_cache_sync either. >> >> >