Re: [PATCH 2/2] MIPS: Loongson64: Add cache_sync to loongson_dma_map_ops

[James Hogan <jhogan@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3135 bytes --]

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@kernel.org> 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@lst.de> 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.
> >> >

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]