Re: [PATCH 1/2] m68k: use the generic dma coherent remap allocator

[Geert Uytterhoeven]

Hi Christoph,

On Tue, Jun 25, 2019 at 11:01 AM Christoph Hellwig <hch@xxxxxx> wrote:
> This switche to using common code for the DMA allocations, including

switches m68k

> potential use of the CMA allocator if configure.  Also add a

configured

> comment where the existing behavior seems to be lacking.
>
> Switching to the generic code enables DMA allocations from atomic
> context, which is required by the DMA API documentation, and also
> adds various other minor features drivers start relying upon.  It
> also makes sure we have on tested code base for all architectures

a tested code base

> that require uncached pte bits for coherent DMA allocations.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>

Thanks, applying and queueing for v5.3.

> --- a/arch/m68k/kernel/dma.c
> +++ b/arch/m68k/kernel/dma.c
> @@ -18,57 +18,20 @@
>  #include <asm/pgalloc.h>
>
>  #if defined(CONFIG_MMU) && !defined(CONFIG_COLDFIRE)
> -
> -void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
> -               gfp_t flag, unsigned long attrs)
> +pgprot_t arch_dma_mmap_pgprot(struct device *dev, pgprot_t prot,
> +               unsigned long attrs)
>  {
> -       struct page *page, **map;
> -       pgprot_t pgprot;
> -       void *addr;
> -       int i, order;
> -
> -       pr_debug("dma_alloc_coherent: %d,%x\n", size, flag);
> -
> -       size = PAGE_ALIGN(size);
> -       order = get_order(size);
> -
> -       page = alloc_pages(flag | __GFP_ZERO, order);
> -       if (!page)
> -               return NULL;
> -
> -       *handle = page_to_phys(page);
> -       map = kmalloc(sizeof(struct page *) << order, flag & ~__GFP_DMA);
> -       if (!map) {
> -               __free_pages(page, order);
> -               return NULL;
> +       /*
> +        * XXX: this doesn't seem to handle the sun3 MMU at all.

Correct.  This file is not compiled on Sun-3, which selects NO_DMA, so
I'll drop the comment while applying.

> +        */
> +       if (CPU_IS_040_OR_060) {
> +               pgprot_val(prot) &= ~_PAGE_CACHE040;
> +               pgprot_val(prot) |= _PAGE_GLOBAL040 | _PAGE_NOCACHE_S;
> +       } else {
> +               pgprot_val(prot) |= _PAGE_NOCACHE030;
>         }
> -       split_page(page, order);
> -
> -       order = 1 << order;
> -       size >>= PAGE_SHIFT;
> -       map[0] = page;
> -       for (i = 1; i < size; i++)
> -               map[i] = page + i;
> -       for (; i < order; i++)
> -               __free_page(page + i);
> -       pgprot = __pgprot(_PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_DIRTY);
> -       if (CPU_IS_040_OR_060)
> -               pgprot_val(pgprot) |= _PAGE_GLOBAL040 | _PAGE_NOCACHE_S;
> -       else
> -               pgprot_val(pgprot) |= _PAGE_NOCACHE030;
> -       addr = vmap(map, size, VM_MAP, pgprot);
> -       kfree(map);
> -
> -       return addr;
> +       return prot;
>  }

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds