Re: [PATCH v1 0/3] Converting m68k WD33C93 drivers to DMA API

[Arnd Bergmann <arnd@kernel.org>]

On Wed, Jun 29, 2022 at 9:36 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
> Am 29.06.2022 um 18:19 schrieb Arnd Bergmann:
> > On Wed, Jun 29, 2022 at 3:16 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
> >>
> >> V1 of a patch series to convert m68k Amiga WD33C93 drivers to the
> >> DMA API.
> >>
> >> This series was precipitated by Arnd removing CONFIG_VIRT_TO_BUS. The
> >> m68k WD33C93 still used virt_to_bus to convert virtual addresses to
> >> physical addresses suitable for the DMA engines (note m68k does not
> >> have an IOMMU and uses a direct mapping for DMA addresses).
> >>
> >> Arnd suggested to use dma_map_single() to set up dma mappings instead
> >> of open-coding much the same in every driver dma_setup() function.
> >>
> >> It appears that m68k (MMU, except for coldfire) will set up pages for
> >> DMA transfers as non-cacheable, thus obviating the need for explicit
> >> cache management.
> >
> > To clarify, the dma-mapping API has two ways of dealing with this:
> >
> > - the streaming API (dma_map/unmap_...) uses explicit cache flushes
> >
> > - the coherent API (dma_alloc_coherent etc) uses page protections
> >   to prevent caching.
> >
> > These three drivers use the streaming API because they operate on
> > data passed in from the outside, so the non-cacheable protection bits
> > are not used here.
>
> I had feared you'd say something along these lines ...
>
> Now that throws up a possible problem for the gvp11 driver: due to the
> need to first map an allocated chunk, then possibly discard that and try
> another allocation strategy, copying of data to the bounce buffer is
> deferred until after the final mapping has been successful. This means
> for writes, we have done the cache flushing before we have actually
> written any data to the buffer.
>
> I don't think it is safe to omit the explicit cache flush for writes in
> this case.

I think it's fine as long as you do things in the correct order: the
copy into the bounce buffer has to be done before the
dma_map_single() here, and conversely, the copy out of the
bounce buffer must happen after the dma_unmap_single().

Regarding the amiga_chip_alloc(), I don't know what this means
for caching. If chip memory is cache-coherent (either uncached
or by snooping), then there should not be any
dma_map()/dma_unmap() for that case, but instead the
amiga_chip_alloc() function should return both the pointer
and the dma_addr_t token.

         Arnd