lib/dma-debug.c +5 -0

index 7f5cdc1e6b29..712a897174e4 100644
@@ -41,6 +41,11 @@
  #define HASH_FN_SHIFT   13
  #define HASH_FN_MASK    (HASH_SIZE - 1)
 
+/* allow architectures to override this if absolutely required */
+#ifndef PREALLOC_DMA_DEBUG_ENTRIES
+#define PREALLOC_DMA_DEBUG_ENTRIES (1 << 16)
+#endif
+
  enum {
  	dma_debug_single,
  	dma_debug_page,

lib/dma-debug.c +5 -3

  	int i;
 
  	/* Do not use dma_debug_initialized here, since we really want to be
  	 * called to set dma_debug_initialized
  	 */
  	if (global_disable)
-		return;
+		return 0;
 
  	for (i = 0; i < HASH_SIZE; ++i) {
  		INIT_LIST_HEAD(&dma_entry_hash[i].list);

@@ -1026,17 +1029,19 @@ void dma_debug_init(u32 num_entries)
  		pr_err("DMA-API: error creating debugfs entries - disabling\n");
  		global_disable = true;
 
-		return;
+		return 0;
  	}
 
  	if (req_entries)
  		num_entries = req_entries;
+	else
+		num_entries = PREALLOC_DMA_DEBUG_ENTRIES;
 
  	if (prealloc_memory(num_entries) != 0) {
  		pr_err("DMA-API: debugging out of memory error - disabled\n");
  		global_disable = true;
 
-		return;
+		return 0;
  	}
 
  	nr_total_entries = num_free_entries;

lib/dma-debug.c +0 -0

  static __init int dma_debug_cmdline(char *str)
  {
 

lib/dma-debug.c void dma_debug_add_bus(struct bus_type *bus) · line 1014

Quoted 4 lines

lib/dma-debug.c +2 -1

-void dma_debug_init(u32 num_entries)
+static int dma_debug_init(void)
  {
+	u32 num_entries;

Maybe initialise it to PREALLOC_DMA_DEBUG_ENTRIES?

Quoted 1 line

+core_initcall(dma_debug_init);

I think it's worth noting that for most users this now happens much earlier than before. In general that's probably good (e.g. on arm64 it should prevent false-positives from the Arm SMMU drivers under ACPI), and I can't imagine it's high-risk, but it is a behaviour change.

Switch to the generic noncoherent direct mapping implementation.

Signed-off-by Christoph Hellwig <[email protected]>

---
 arch/arc/Kconfig                   |   4 +
 arch/arc/include/asm/Kbuild        |   1 +
 arch/arc/include/asm/dma-mapping.h |  21 -----
 arch/arc/mm/dma.c                  | 141 +++--------------------------
 4 files changed, 19 insertions(+), 148 deletions(-)
 delete mode 100644 arch/arc/include/asm/dma-mapping.h

Which is actually strange as I would expect ARC code to be built by bots.

Quoted 1 line · Alexey Brodkin

Which is actually strange as I would expect ARC code to be built by bots.

I don't think I got any notification. Thank for the fixes!

I think I found the bug, based on the fact that so far all tests for architectures that also need a cache op for device to cpu transitions failed. I did a stupid typo when changing kconfig symbols, so please try the patch below.

Switch to the generic noncoherent direct mapping implementation.

This makes sure kmap_atomic_pfn is consistently used for access to virtual addresses instead of either using the slower plain kmap or blindly expecting page_address() to work.

This makes sure the cache_sync routines is called in the unmap_sg case, to match the unmap_single and sync_{single,sg}_to_cpu cases.

Signed-off-by Christoph Hellwig <[email protected]>

---
 arch/nds32/Kconfig                   |   3 +
 arch/nds32/include/asm/Kbuild        |   1 +
 arch/nds32/include/asm/dma-mapping.h |  14 ---
 arch/nds32/kernel/dma.c              | 182 ++++++---------------------
 4 files changed, 39 insertions(+), 161 deletions(-)
 delete mode 100644 arch/nds32/include/asm/dma-mapping.h

arch/nds32/Kconfig +3 -0

index 249f38d3388f..67d0ac0a989c 100644
@@ -5,10 +5,13 @@
 
 config NDS32
         def_bool y
+       select ARCH_HAS_SYNC_DMA_FOR_CPU
+       select ARCH_HAS_SYNC_DMA_FOR_DEVICE
        select ARCH_WANT_FRAME_POINTERS if FTRACE
        select CLKSRC_MMIO
        select CLONE_BACKWARDS
        select COMMON_CLK
+       select DMA_NONCOHERENT_OPS
        select GENERIC_ATOMIC64
        select GENERIC_CPU_DEVICES
        select GENERIC_CLOCKEVENTS

arch/nds32/include/asm/Kbuild +1 -0

index 06bdf8167f5a..b3e951f805f8 100644
@@ -13,6 +13,7 @@ generic-y += cputime.h
 generic-y += device.h
 generic-y += div64.h
 generic-y += dma.h
+generic-y += dma-mapping.h
 generic-y += emergency-restart.h
 generic-y += errno.h
 generic-y += exec.h

arch/nds32/include/asm/dma-mapping.h deleted +0 -14

deleted file mode 100644
index 2dd47d245c25..000000000000
@@ -1,14 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-// Copyright (C) 2005-2017 Andes Technology Corporation
-
-#ifndef ASMNDS32_DMA_MAPPING_H
-#define ASMNDS32_DMA_MAPPING_H
-
-extern struct dma_map_ops nds32_dma_ops;
-
-static inline struct dma_map_ops *get_arch_dma_ops(struct bus_type *bus)
-{
-       return &nds32_dma_ops;
-}
-
-#endif

arch/nds32/kernel/dma.c +25 -120

index d291800fc621..688f1a03dee6 100644
@@ -3,17 +3,14 @@
 
 #include <linux/types.h>
 #include <linux/mm.h>
-#include <linux/export.h>
 #include <linux/string.h>
-#include <linux/scatterlist.h>
-#include <linux/dma-mapping.h>
+#include <linux/dma-noncoherent.h>
 #include <linux/io.h>
 #include <linux/cache.h>
 #include <linux/highmem.h>
 #include <linux/slab.h>
 #include <asm/cacheflush.h>
 #include <asm/tlbflush.h>
-#include <asm/dma-mapping.h>
 #include <asm/proc-fns.h>
 
 /*

@@ -22,11 +19,6 @@
 static pte_t *consistent_pte;
 static DEFINE_RAW_SPINLOCK(consistent_lock);
 
-enum master_type {
-       FOR_CPU = 0,
-       FOR_DEVICE = 1,
-};
-
 /*
  * VM region handling support.
  *

@@ -124,10 +116,8 @@ static struct arch_vm_region *vm_region_find(struct arch_vm_region *head,
        return c;
 }
 
-/* FIXME: attrs is not used. */
-static void *nds32_dma_alloc_coherent(struct device *dev, size_t size,
-                                     dma_addr_t * handle, gfp_t gfp,
-                                     unsigned long attrs)
+void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
+               gfp_t gfp, unsigned long attrs)
 {
        struct page *page;
        struct arch_vm_region *c;

@@ -232,8 +222,8 @@ static void *nds32_dma_alloc_coherent(struct device *dev, size_t size,
        return NULL;
 }
 
-static void nds32_dma_free(struct device *dev, size_t size, void *cpu_addr,
-                          dma_addr_t handle, unsigned long attrs)
+void arch_dma_free(struct device *dev, size_t size, void *cpu_addr,
+               dma_addr_t handle, unsigned long attrs)
 {
        struct arch_vm_region *c;
        unsigned long flags, addr;

@@ -333,145 +323,43 @@ static int __init consistent_init(void)
 }
 
 core_initcall(consistent_init);
-static void consistent_sync(void *vaddr, size_t size, int direction, int master_type);
-static dma_addr_t nds32_dma_map_page(struct device *dev, struct page *page,
-                                    unsigned long offset, size_t size,
-                                    enum dma_data_direction dir,
-                                    unsigned long attrs)
-{
-       if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
-               consistent_sync((void *)(page_address(page) + offset), size, dir, FOR_DEVICE);
-       return page_to_phys(page) + offset;
-}
-
-static void nds32_dma_unmap_page(struct device *dev, dma_addr_t handle,
-                                size_t size, enum dma_data_direction dir,
-                                unsigned long attrs)
-{
-       if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
-               consistent_sync(phys_to_virt(handle), size, dir, FOR_CPU);
-}
 
-/*
- * Make an area consistent for devices.
- */
-static void consistent_sync(void *vaddr, size_t size, int direction, int master_type)
+void arch_sync_dma_for_device(struct device *dev, phys_addr_t paddr,
+               size_t size, enum dma_data_direction dir)
 {
-       unsigned long start = (unsigned long)vaddr;
-       unsigned long end = start + size;
-
-       if (master_type == FOR_CPU) {
-               switch (direction) {
-               case DMA_TO_DEVICE:
-                       break;
-               case DMA_FROM_DEVICE:
-               case DMA_BIDIRECTIONAL:
-                       cpu_dma_inval_range(start, end);
-                       break;
-               default:
-                       BUG();
-               }
-       } else {
-               /* FOR_DEVICE */
-               switch (direction) {
-               case DMA_FROM_DEVICE:
-                       break;
-               case DMA_TO_DEVICE:
-               case DMA_BIDIRECTIONAL:
-                       cpu_dma_wb_range(start, end);
-                       break;
-               default:
-                       BUG();
-               }
+       void *addr = kmap_atomic_pfn(PHYS_PFN(paddr));
+       unsigned long start = (unsigned long)addr;
+
+       switch (direction) {
+       case DMA_FROM_DEVICE:
+               break;
+       case DMA_TO_DEVICE:
+       case DMA_BIDIRECTIONAL:
+               cpu_dma_wb_range(start, start + size);
+               break;
+       default:
+               BUG();
        }
-}
 
-static int nds32_dma_map_sg(struct device *dev, struct scatterlist *sg,
-                           int nents, enum dma_data_direction dir,
-                           unsigned long attrs)
-{
-       int i;
-
-       for (i = 0; i < nents; i++, sg++) {
-               void *virt;
-               unsigned long pfn;
-               struct page *page = sg_page(sg);
-
-               sg->dma_address = sg_phys(sg);
-               pfn = page_to_pfn(page) + sg->offset / PAGE_SIZE;
-               page = pfn_to_page(pfn);
-               if (PageHighMem(page)) {
-                       virt = kmap_atomic(page);
-                       consistent_sync(virt, sg->length, dir, FOR_CPU);
-                       kunmap_atomic(virt);
-               } else {
-                       if (sg->offset > PAGE_SIZE)
-                               panic("sg->offset:%08x > PAGE_SIZE\n",
-                                     sg->offset);
-                       virt = page_address(page) + sg->offset;
-                       consistent_sync(virt, sg->length, dir, FOR_CPU);
-               }
-       }
-       return nents;
-}
-
-static void nds32_dma_unmap_sg(struct device *dev, struct scatterlist *sg,
-                              int nhwentries, enum dma_data_direction dir,
-                              unsigned long attrs)
-{
+       kunmap_atomic(addr);
 }
 
-static void
-nds32_dma_sync_single_for_cpu(struct device *dev, dma_addr_t handle,
-                             size_t size, enum dma_data_direction dir)
+void arch_sync_dma_for_cpu(struct device *dev, phys_addr_t paddr,
+               size_t size, enum dma_data_direction dir)
 {
-       consistent_sync((void *)phys_to_virt(handle), size, dir, FOR_CPU);
-}
-
-static void
-nds32_dma_sync_single_for_device(struct device *dev, dma_addr_t handle,
-                                size_t size, enum dma_data_direction dir)
-{
-       consistent_sync((void *)phys_to_virt(handle), size, dir, FOR_DEVICE);
-}
-
-static void
-nds32_dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sg, int nents,
-                         enum dma_data_direction dir)
-{
-       int i;
-
-       for (i = 0; i < nents; i++, sg++) {
-               char *virt =
-                   page_address((struct page *)sg->page_link) + sg->offset;
-               consistent_sync(virt, sg->length, dir, FOR_CPU);
+       void *addr = kmap_atomic_pfn(PHYS_PFN(paddr));
+       unsigned long start = (unsigned long)addr;
+

2018-04-20 16:03 GMT+08:00 Christoph Hellwig <[email protected]>:

Quoted 10 lines

Switch to the generic noncoherent direct mapping implementation.

This makes sure kmap_atomic_pfn is consistently used for access to virtual addresses instead of either using the slower plain kmap or blindly expecting page_address() to work.

This makes sure the cache_sync routines is called in the unmap_sg case, to match the unmap_single and sync_{single,sg}_to_cpu cases.

Signed-off-by Christoph Hellwig <[email protected]>

CC arch/nds32/kernel/dma.o
arch/nds32/kernel/dma.c: In function 'arch_sync_dma_for_device':
arch/nds32/kernel/dma.c:333:10: error: 'direction' undeclared (first use in this function)

  switch (direction) {
          ^~~~~~~~~

arch/nds32/kernel/dma.c:333:10: note: each undeclared identifier is reported only once for each function it appears in arch/nds32/kernel/dma.c: In function 'arch_sync_dma_for_cpu':
arch/nds32/kernel/dma.c:353:10: error: 'direction' undeclared (first use in this function)

  switch (direction) {
          ^~~~~~~~~
arch/nds32/kernel/dma.c:358:30: error: 'end' undeclared (first use in
this function)
   cpu_dma_inval_range(start, end);
                              ^~~

make[1]: *** [arch/nds32/kernel/dma.o] Error 1
make: *** [arch/nds32/kernel] Error 2

After this building error, the ftmac100.c driver is broken. Not sure what happened.

--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Hi

Am 23.04.2018 um 18:49 schrieb Greentime Hu:

Quoted 29 lines

2018-04-20 16:03 GMT+08:00 Christoph Hellwig <[email protected]>:

Quoted 10 lines

Switch to the generic noncoherent direct mapping implementation.

This makes sure kmap_atomic_pfn is consistently used for access to virtual addresses instead of either using the slower plain kmap or blindly expecting page_address() to work.

This makes sure the cache_sync routines is called in the unmap_sg case, to match the unmap_single and sync_{single,sg}_to_cpu cases.

Signed-off-by Christoph Hellwig <[email protected]>

CC arch/nds32/kernel/dma.o
arch/nds32/kernel/dma.c: In function 'arch_sync_dma_for_device':
arch/nds32/kernel/dma.c:333:10: error: 'direction' undeclared (first use in this function)

  switch (direction) {
          ^~~~~~~~~

arch/nds32/kernel/dma.c:333:10: note: each undeclared identifier is reported only once for each function it appears in arch/nds32/kernel/dma.c: In function 'arch_sync_dma_for_cpu':
arch/nds32/kernel/dma.c:353:10: error: 'direction' undeclared (first use in this function)

  switch (direction) {
          ^~~~~~~~~
arch/nds32/kernel/dma.c:358:30: error: 'end' undeclared (first use in
this function)
   cpu_dma_inval_range(start, end);
                              ^~~

Quoted 5 lines

make[1]: *** [arch/nds32/kernel/dma.o] Error 1
make: *** [arch/nds32/kernel] Error 2

After this building error, the ftmac100.c driver is broken. Not sure what happened.

Christoph used 'direction' as in the original consistent_sync(), but the parameter is now named 'dir' in the new function. Changing

arch/nds32/kernel/dma.c +2 -0

+void arch_sync_dma_for_device(struct device *dev, phys_addr_t paddr,
+               size_t size, enum dma_data_direction dir)

to

(quoted diff) +2 -0

+void arch_sync_dma_for_device(struct device *dev, phys_addr_t paddr,
+               size_t size, enum dma_data_direction direction)

(or using 'dir' in place of 'direction' in the switch clauses) should fix that.

Christoph also forgot to declare and assign 'end'. Adding back the

unsigned long end = start + size;

after

+ unsigned long start = (unsigned long)addr;

should fix the second error.

If your ftmac100 driver relies on the DMA API, it wouldn't be expected to work if kernel and module views of the DMA API disagree.

HTH,

Michael

Hi Greentime,

thanks for testing the patch!

It looks like nds32 doesn't have a buildbot yet, so this code didn't even get syntax checkin, sorry.

Below is the incremental fixes based on this thread.

Can you check if my tree works if you just revert the "nds32: use generic dma_noncoherent_ops" commit?

arch/nds32/kernel/dma.c +1 -2

index 688f1a03dee6..48018275e7f4 100644
@@ -330,7 +330,7 @@ void arch_sync_dma_for_device(struct device *dev, phys_addr_t paddr,
        void *addr = kmap_atomic_pfn(PHYS_PFN(paddr));
        unsigned long start = (unsigned long)addr;
 
-       switch (direction) {
+       switch (dir) {
        case DMA_FROM_DEVICE:
                break;
        case DMA_TO_DEVICE:

@@ -350,12 +350,12 @@ void arch_sync_dma_for_cpu(struct device *dev, phys_addr_t paddr,
        void *addr = kmap_atomic_pfn(PHYS_PFN(paddr));
        unsigned long start = (unsigned long)addr;
 
-       switch (direction) {

Hi Crhistoph,

The ftmac100 works if I revert this commit.

Quoted 3 lines

Hi Crhistoph,

The ftmac100 works if I revert this commit.

Thanks. ftmac100 only use dma_map_page, which in the old nds32 code is just doing a plain page_address and never kmaps. Can you apply the patch below on the tree with the origin "nds32: use generic dma_noncoherent_ops" reverted? This always just uses page_address, although that, just like the original code is broken if you actually have highmem that needs to be mapped:

---
From 1dc5d1cae4cd7b9ce03d0e2943364ed4cca938d7 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <[email protected]>
Date: Mon, 16 Apr 2018 19:20:30 +0200
Subject: nds32: use generic dma_noncoherent_ops

Switch to the generic noncoherent direct mapping implementation.

This makes sure the cache_sync routines is called in the unmap_sg case, to match the unmap_single and sync_{single,sg}_to_cpu cases.

Signed-off-by Christoph Hellwig <[email protected]>

---
 arch/nds32/Kconfig                   |   3 +
 arch/nds32/include/asm/Kbuild        |   1 +
 arch/nds32/include/asm/dma-mapping.h |  14 ---
 arch/nds32/kernel/dma.c              | 182 +++++----------------------
 4 files changed, 37 insertions(+), 163 deletions(-)
 delete mode 100644 arch/nds32/include/asm/dma-mapping.h

arch/nds32/Kconfig +3 -0

index 249f38d3388f..67d0ac0a989c 100644
@@ -5,10 +5,13 @@
 
 config NDS32
         def_bool y
+       select ARCH_HAS_SYNC_DMA_FOR_CPU
+       select ARCH_HAS_SYNC_DMA_FOR_DEVICE
        select ARCH_WANT_FRAME_POINTERS if FTRACE
        select CLKSRC_MMIO
        select CLONE_BACKWARDS
        select COMMON_CLK
+       select DMA_NONCOHERENT_OPS
        select GENERIC_ATOMIC64
        select GENERIC_CPU_DEVICES
        select GENERIC_CLOCKEVENTS

arch/nds32/include/asm/Kbuild +1 -0

index 06bdf8167f5a..b3e951f805f8 100644
@@ -13,6 +13,7 @@ generic-y += cputime.h
 generic-y += device.h
 generic-y += div64.h
 generic-y += dma.h
+generic-y += dma-mapping.h
 generic-y += emergency-restart.h
 generic-y += errno.h
 generic-y += exec.h

arch/nds32/include/asm/dma-mapping.h deleted +0 -14

deleted file mode 100644
index 2dd47d245c25..000000000000
@@ -1,14 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-// Copyright (C) 2005-2017 Andes Technology Corporation
-
-#ifndef ASMNDS32_DMA_MAPPING_H
-#define ASMNDS32_DMA_MAPPING_H
-
-extern struct dma_map_ops nds32_dma_ops;
-
-static inline struct dma_map_ops *get_arch_dma_ops(struct bus_type *bus)
-{
-       return &nds32_dma_ops;
-}
-
-#endif

arch/nds32/kernel/dma.c +33 -150

index d291800fc621..78311a1e6fd1 100644
@@ -3,17 +3,14 @@
 
 #include <linux/types.h>
 #include <linux/mm.h>
-#include <linux/export.h>
 #include <linux/string.h>
-#include <linux/scatterlist.h>
-#include <linux/dma-mapping.h>
+#include <linux/dma-noncoherent.h>
 #include <linux/io.h>
 #include <linux/cache.h>
 #include <linux/highmem.h>
 #include <linux/slab.h>
 #include <asm/cacheflush.h>
 #include <asm/tlbflush.h>
-#include <asm/dma-mapping.h>
 #include <asm/proc-fns.h>
 
 /*

@@ -22,11 +19,6 @@
 static pte_t *consistent_pte;
 static DEFINE_RAW_SPINLOCK(consistent_lock);
 
-enum master_type {
-       FOR_CPU = 0,
-       FOR_DEVICE = 1,
-};
-
 /*
  * VM region handling support.
  *

@@ -124,10 +116,8 @@ static struct arch_vm_region *vm_region_find(struct arch_vm_region *head,
        return c;
 }
 
-/* FIXME: attrs is not used. */
-static void *nds32_dma_alloc_coherent(struct device *dev, size_t size,
-                                     dma_addr_t * handle, gfp_t gfp,
-                                     unsigned long attrs)
+void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
+               gfp_t gfp, unsigned long attrs)
 {
        struct page *page;
        struct arch_vm_region *c;

@@ -232,8 +222,8 @@ static void *nds32_dma_alloc_coherent(struct device *dev, size_t size,
        return NULL;
 }
 
-static void nds32_dma_free(struct device *dev, size_t size, void *cpu_addr,
-                          dma_addr_t handle, unsigned long attrs)
+void arch_dma_free(struct device *dev, size_t size, void *cpu_addr,
+               dma_addr_t handle, unsigned long attrs)
 {
        struct arch_vm_region *c;
        unsigned long flags, addr;

@@ -333,145 +323,39 @@ static int __init consistent_init(void)
 }
 
 core_initcall(consistent_init);
-static void consistent_sync(void *vaddr, size_t size, int direction, int master_type);
-static dma_addr_t nds32_dma_map_page(struct device *dev, struct page *page,
-                                    unsigned long offset, size_t size,
-                                    enum dma_data_direction dir,
-                                    unsigned long attrs)
-{
-       if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
-               consistent_sync((void *)(page_address(page) + offset), size, dir, FOR_DEVICE);
-       return page_to_phys(page) + offset;
-}
 
-static void nds32_dma_unmap_page(struct device *dev, dma_addr_t handle,
-                                size_t size, enum dma_data_direction dir,
-                                unsigned long attrs)
+void arch_sync_dma_for_device(struct device *dev, phys_addr_t paddr,
+               size_t size, enum dma_data_direction dir)
 {
-       if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
-               consistent_sync(phys_to_virt(handle), size, dir, FOR_CPU);
-}
-
-/*
- * Make an area consistent for devices.
- */
-static void consistent_sync(void *vaddr, size_t size, int direction, int master_type)
-{
-       unsigned long start = (unsigned long)vaddr;
-       unsigned long end = start + size;
-
-       if (master_type == FOR_CPU) {
-               switch (direction) {
-               case DMA_TO_DEVICE:
-                       break;
-               case DMA_FROM_DEVICE:
-               case DMA_BIDIRECTIONAL:
-                       cpu_dma_inval_range(start, end);
-                       break;
-               default:
-                       BUG();
-               }
-       } else {
-               /* FOR_DEVICE */
-               switch (direction) {
-               case DMA_FROM_DEVICE:
-                       break;
-               case DMA_TO_DEVICE:
-               case DMA_BIDIRECTIONAL:
-                       cpu_dma_wb_range(start, end);
-                       break;
-               default:
-                       BUG();
-               }
+       void *addr = phys_to_virt(paddr);
+       unsigned long start = (unsigned long)addr;
+
+       switch (dir) {
+       case DMA_FROM_DEVICE:
+               break;
+       case DMA_TO_DEVICE:
+       case DMA_BIDIRECTIONAL:
+               cpu_dma_wb_range(start, start + size);
+               break;
+       default:
+               BUG();
        }
 }
 
-static int nds32_dma_map_sg(struct device *dev, struct scatterlist *sg,
-                           int nents, enum dma_data_direction dir,
-                           unsigned long attrs)
+void arch_sync_dma_for_cpu(struct device *dev, phys_addr_t paddr,
+               size_t size, enum dma_data_direction dir)
 {
-       int i;
-
-       for (i = 0; i < nents; i++, sg++) {
-               void *virt;
-               unsigned long pfn;
-               struct page *page = sg_page(sg);
-
-               sg->dma_address = sg_phys(sg);
-               pfn = page_to_pfn(page) + sg->offset / PAGE_SIZE;
-               page = pfn_to_page(pfn);
-               if (PageHighMem(page)) {
-                       virt = kmap_atomic(page);
-                       consistent_sync(virt, sg->length, dir, FOR_CPU);
-                       kunmap_atomic(virt);
-               } else {
-                       if (sg->offset > PAGE_SIZE)
-                               panic("sg->offset:%08x > PAGE_SIZE\n",
-                                     sg->offset);
-                       virt = page_address(page) + sg->offset;
-                       consistent_sync(virt, sg->length, dir, FOR_CPU);
-               }
+       void *addr = phys_to_virt(paddr);
+       unsigned long start = (unsigned long)addr;
+
+       switch (dir) {
+       case DMA_TO_DEVICE:
+               break;
+       case DMA_FROM_DEVICE:
+       case DMA_BIDIRECTIONAL:
+               cpu_dma_inval_range(start, start + size);
+               break;
+       default:
+               BUG();
        }
-       return nents;
 }
-
-static void nds32_dma_unmap_sg(struct device *dev, struct scatterlist *sg,
-                              int nhwentries, enum dma_data_direction dir,
-                              unsigned long attrs)
-{
-}
-
-static void
-nds32_dma_sync_single_for_cpu(struct device *dev, dma_addr_t handle,
-                             size_t size, enum dma_data_direction dir)
-{
-       consistent_sync((void *)phys_to_virt(handle), size, dir, FOR_CPU);
-}
-
-static void
-nds32_dma_sync_single_for_device(struct device *dev, dma_addr_t handle,
-                                size_t size, enum dma_data_direction dir)
-{
-       consistent_sync((void *)phys_to_virt(handle), size, dir, FOR_DEVICE);
-}
-
-static void
-nds32_dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sg, int nents,
-                         enum dma_data_direction dir)
-{
-       int i;
-
-       for (i = 0; i < nents; i++, sg++) {
-               char *virt =
-                   page_address((struct page *)sg->page_link) + sg->offset;
-               consistent_sync(virt, sg->length, dir, FOR_CPU);
-       }
-}
-
-static void
-nds32_dma_sync_sg_for_device(struct device *dev, struct scatterlist *sg,
-                            int nents, enum dma_data_direction dir)
-{
-       int i;
-
-       for (i = 0; i < nents; i++, sg++) {
-               char *virt =
-                   page_address((struct page *)sg->page_link) + sg->offset;
-               consistent_sync(virt, sg->length, dir, FOR_DEVICE);
-       }
-}
-
-struct dma_map_ops nds32_dma_ops = {
-       .alloc = nds32_dma_alloc_coherent,
-       .free = nds32_dma_free,
-       .map_page = nds32_dma_map_page,
-       .unmap_page = nds32_dma_unmap_page,
-       .map_sg = nds32_dma_map_sg,
-       .unmap_sg = nds32_dma_unmap_sg,
-       .sync_single_for_device = nds32_dma_sync_single_for_device,
-       .sync_single_for_cpu = nds32_dma_sync_single_for_cpu,
-       .sync_sg_for_cpu = nds32_dma_sync_sg_for_cpu,
-       .sync_sg_for_device = nds32_dma_sync_sg_for_device,
-};
-
-EXPORT_SYMBOL(nds32_dma_ops);
--

2.17.0

Can you try this patch ontop of either the new or original one?

---

lib/dma-noncoherent.c +1 -1

index f4b8532c20ac..a2c192b3508d 100644
@@ -48,7 +48,7 @@ static int dma_noncoherent_map_sg(struct device *dev, struct scatterlist *sgl,
        return nents;
 }
 
-#ifdef CONFIG_DMA_NONCOHERENT_SYNC_FOR_CPU
+#ifdef CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU
 static void dma_noncoherent_sync_single_for_cpu(struct device *dev,
                dma_addr_t addr, size_t size, enum dma_data_direction dir)
 {

It works!!!

Quoted 1 line

It works!!!

Thanks!

Can you retest the updated tree here with all the fixes and give me your Tested-by: for the generic and nds32 patches?

git://git.infradead.org/users/hch/misc.git generic-dma-noncoherent

Switch to the generic noncoherent direct mapping implementation.

Parisc previously had two different non-coherent dma ops implementation that just different in the way coherent allocations were handled or not handled. The different behavior is not selected at runtime in the arch_dma_alloc and arch_dma_free routines. The non-coherent allocation in the pcx cases now uses the dma_direct helpers that are a little more sophisticated and used by a lot of other architectures.

Fix sync_single_for_cpu to do skip the cache flush unless the transfer is to the device to match the more tested unmap_single path which should have the same cache coherency implications.

This also now consistenly uses flush_kernel_dcache_range for cache flushing while previously some of the SG based operations used flush_kernel_vmap_range instead.

Signed-off-by Christoph Hellwig <[email protected]>

---
 arch/parisc/Kconfig                   |   4 +
 arch/parisc/include/asm/dma-mapping.h |   5 -
 arch/parisc/kernel/pci-dma.c          | 181 ++++----------------------
 arch/parisc/kernel/setup.c            |   8 +-
 arch/parisc/mm/init.c                 |  11 +-
 5 files changed, 35 insertions(+), 174 deletions(-)

arch/parisc/Kconfig +4 -0

index 47047f0cbe35..80166a1cbcb7 100644
@@ -188,6 +188,10 @@ config PA20
 config PA11
 	def_bool y
 	depends on PA7000 || PA7100LC || PA7200 || PA7300LC
+	select ARCH_HAS_SYNC_DMA_FOR_CPU
+	select ARCH_HAS_SYNC_DMA_FOR_DEVICE
+	select DMA_NONCOHERENT_OPS
+	select DMA_NONCOHERENT_CACHE_SYNC
 
 config PREFETCH
 	def_bool y

arch/parisc/include/asm/dma-mapping.h +0 -5

index 01e1fc057c83..44a9f97194aa 100644
@@ -21,11 +21,6 @@
 ** flush/purge and allocate "regular" cacheable pages for everything.
 */
 
-#ifdef CONFIG_PA11
-extern const struct dma_map_ops pcxl_dma_ops;
-extern const struct dma_map_ops pcx_dma_ops;
-#endif
-
 extern const struct dma_map_ops *hppa_dma_ops;
 
 static inline const struct dma_map_ops *get_arch_dma_ops(struct bus_type *bus)

arch/parisc/kernel/pci-dma.c +25 -156

index 91bc0cac03a1..235e2e53959e 100644
@@ -21,13 +21,12 @@
 #include <linux/init.h>
 #include <linux/gfp.h>
 #include <linux/mm.h>
-#include <linux/pci.h>
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
 #include <linux/string.h>
 #include <linux/types.h>
-#include <linux/scatterlist.h>
-#include <linux/export.h>
+#include <linux/dma-direct.h>
+#include <linux/dma-noncoherent.h>
 
 #include <asm/cacheflush.h>
 #include <asm/dma.h>    /* for DMA_CHUNK_SIZE */

@@ -447,178 +446,48 @@ static void pa11_dma_free(struct device *dev, size_t size, void *vaddr,
 	free_pages((unsigned long)__va(dma_handle), order);
 }
 
-static dma_addr_t pa11_dma_map_page(struct device *dev, struct page *page,
-		unsigned long offset, size_t size,
-		enum dma_data_direction direction, unsigned long attrs)
+void arch_sync_dma_for_device(struct device *dev, phys_addr_t paddr,
+		size_t size, enum dma_data_direction dir)
 {
-	void *addr = page_address(page) + offset;
-	BUG_ON(direction == DMA_NONE);
-
-	if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
-		flush_kernel_dcache_range((unsigned long) addr, size);
-
-	return virt_to_phys(addr);
+	flush_kernel_dcache_range((unsigned long)phys_to_virt(paddr), size);
 }
 
-static void pa11_dma_unmap_page(struct device *dev, dma_addr_t dma_handle,
-		size_t size, enum dma_data_direction direction,
-		unsigned long attrs)
+void arch_sync_dma_for_cpu(struct device *dev, phys_addr_t paddr,
+		size_t size, enum dma_data_direction dir)
 {
-	BUG_ON(direction == DMA_NONE);
-
-	if (attrs & DMA_ATTR_SKIP_CPU_SYNC)
-		return;
-
-	if (direction == DMA_TO_DEVICE)
+	if (dir == DMA_TO_DEVICE)
 		return;
 
 	/*
-	 * For PCI_DMA_FROMDEVICE this flush is not necessary for the
+	 * For DMA_FROM_DEVICE this flush is not necessary for the
 	 * simple map/unmap case. However, it IS necessary if if
-	 * pci_dma_sync_single_* has been called and the buffer reused.
+	 * dma_sync_single_* has been called and the buffer reused.
 	 */
 
-	flush_kernel_dcache_range((unsigned long) phys_to_virt(dma_handle), size);
-}
-
-static int pa11_dma_map_sg(struct device *dev, struct scatterlist *sglist,
-		int nents, enum dma_data_direction direction,
-		unsigned long attrs)
-{
-	int i;
-	struct scatterlist *sg;
-
-	BUG_ON(direction == DMA_NONE);
-
-	for_each_sg(sglist, sg, nents, i) {
-		unsigned long vaddr = (unsigned long)sg_virt(sg);
-
-		sg_dma_address(sg) = (dma_addr_t) virt_to_phys(vaddr);
-		sg_dma_len(sg) = sg->length;
-
-		if (attrs & DMA_ATTR_SKIP_CPU_SYNC)
-			continue;
-
-		flush_kernel_dcache_range(vaddr, sg->length);
-	}
-	return nents;
+	flush_kernel_dcache_range((unsigned long)phys_to_virt(paddr), size);
 }
 
-static void pa11_dma_unmap_sg(struct device *dev, struct scatterlist *sglist,
-		int nents, enum dma_data_direction direction,
-		unsigned long attrs)
-{
-	int i;
-	struct scatterlist *sg;
-
-	BUG_ON(direction == DMA_NONE);
-
-	if (attrs & DMA_ATTR_SKIP_CPU_SYNC)
-		return;
-
-	if (direction == DMA_TO_DEVICE)
-		return;
-
-	/* once we do combining we'll need to use phys_to_virt(sg_dma_address(sglist)) */
-
-	for_each_sg(sglist, sg, nents, i)
-		flush_kernel_vmap_range(sg_virt(sg), sg->length);
-}
-
-static void pa11_dma_sync_single_for_cpu(struct device *dev,
-		dma_addr_t dma_handle, size_t size,
-		enum dma_data_direction direction)
-{
-	BUG_ON(direction == DMA_NONE);
-
-	flush_kernel_dcache_range((unsigned long) phys_to_virt(dma_handle),
-			size);
-}
-
-static void pa11_dma_sync_single_for_device(struct device *dev,
-		dma_addr_t dma_handle, size_t size,
-		enum dma_data_direction direction)
-{
-	BUG_ON(direction == DMA_NONE);
-
-	flush_kernel_dcache_range((unsigned long) phys_to_virt(dma_handle),
-			size);
-}
-
-static void pa11_dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sglist, int nents, enum dma_data_direction direction)
-{
-	int i;
-	struct scatterlist *sg;
-
-	/* once we do combining we'll need to use phys_to_virt(sg_dma_address(sglist)) */
-
-	for_each_sg(sglist, sg, nents, i)
-		flush_kernel_vmap_range(sg_virt(sg), sg->length);
-}
-
-static void pa11_dma_sync_sg_for_device(struct device *dev, struct scatterlist *sglist, int nents, enum dma_data_direction direction)
-{
-	int i;
-	struct scatterlist *sg;
-
-	/* once we do combining we'll need to use phys_to_virt(sg_dma_address(sglist)) */
-
-	for_each_sg(sglist, sg, nents, i)
-		flush_kernel_vmap_range(sg_virt(sg), sg->length);
-}
-
-static void pa11_dma_cache_sync(struct device *dev, void *vaddr, size_t size,
+void arch_dma_cache_sync(struct device *dev, void *vaddr, size_t size,
 	       enum dma_data_direction direction)
 {
 	flush_kernel_dcache_range((unsigned long)vaddr, size);
 }
 
-const struct dma_map_ops pcxl_dma_ops = {
-	.alloc =		pa11_dma_alloc,
-	.free =			pa11_dma_free,
-	.map_page =		pa11_dma_map_page,
-	.unmap_page =		pa11_dma_unmap_page,
-	.map_sg =		pa11_dma_map_sg,
-	.unmap_sg =		pa11_dma_unmap_sg,
-	.sync_single_for_cpu =	pa11_dma_sync_single_for_cpu,
-	.sync_single_for_device = pa11_dma_sync_single_for_device,
-	.sync_sg_for_cpu =	pa11_dma_sync_sg_for_cpu,
-	.sync_sg_for_device =	pa11_dma_sync_sg_for_device,
-	.cache_sync =		pa11_dma_cache_sync,
-};
-
-static void *pcx_dma_alloc(struct device *dev, size_t size,
-		dma_addr_t *dma_handle, gfp_t flag, unsigned long attrs)
+void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle,
+		gfp_t gfp, unsigned long attrs)
 {
-	void *addr;
-
-	if ((attrs & DMA_ATTR_NON_CONSISTENT) == 0)
-		return NULL;
-
-	addr = (void *)__get_free_pages(flag, get_order(size));
-	if (addr)
-		*dma_handle = (dma_addr_t)virt_to_phys(addr);
-
-	return addr;
+	if (boot_cpu_data.cpu_type == pcxl2 || boot_cpu_data.cpu_type == pcxl)
+		return pa11_dma_alloc(dev, size, dma_handle, gfp, attrs);
+	if (attrs & DMA_ATTR_NON_CONSISTENT)
+		return dma_direct_alloc(dev, size, dma_handle, gfp, attrs);
+	return NULL;
 }
 
-static void pcx_dma_free(struct device *dev, size_t size, void *vaddr,
-		dma_addr_t iova, unsigned long attrs)
+void arch_dma_free(struct device *dev, size_t size, void *cpu_addr,
+		dma_addr_t dma_addr, unsigned long attrs)
 {
-	free_pages((unsigned long)vaddr, get_order(size));
-	return;
+	if (boot_cpu_data.cpu_type == pcxl2 || boot_cpu_data.cpu_type == pcxl)
+		pa11_dma_free(dev, size, cpu_addr, dma_addr, attrs);
+	else
+		dma_direct_free(dev, size, cpu_addr, dma_addr, attrs);
 }
-
-const struct dma_map_ops pcx_dma_ops = {
-	.alloc =		pcx_dma_alloc,
-	.free =			pcx_dma_free,
-	.map_page =		pa11_dma_map_page,
-	.unmap_page =		pa11_dma_unmap_page,
-	.map_sg =		pa11_dma_map_sg,
-	.unmap_sg =		pa11_dma_unmap_sg,
-	.sync_single_for_cpu =	pa11_dma_sync_single_for_cpu,
-	.sync_single_for_device = pa11_dma_sync_single_for_device,
-	.sync_sg_for_cpu =	pa11_dma_sync_sg_for_cpu,
-	.sync_sg_for_device =	pa11_dma_sync_sg_for_device,
-	.cache_sync =		pa11_dma_cache_sync,
-};

arch/parisc/kernel/setup.c +3 -5

index 8d3a7b80ac42..4e87c35c22b7 100644
@@ -97,14 +97,12 @@ void __init dma_ops_init(void)
 		panic(	"PA-RISC Linux currently only supports machines that conform to\n"
 			"the PA-RISC 1.1 or 2.0 architecture specification.\n");
 
-	case pcxs:
-	case pcxt:
-		hppa_dma_ops = &pcx_dma_ops;
-		break;
 	case pcxl2:
 		pa7300lc_init();
 	case pcxl: /* falls through */
-		hppa_dma_ops = &pcxl_dma_ops;
+	case pcxs:
+	case pcxt:
+		hppa_dma_ops = &dma_noncoherent_ops;
 		break;
 	default:
 		break;

arch/parisc/mm/init.c +3 -8

index cab32ee824d2..4ad91c28ecbe 100644
@@ -19,7 +19,6 @@
 #include <linux/gfp.h>
 #include <linux/delay.h>
 #include <linux/init.h>
-#include <linux/pci.h>		/* for hppa_dma_ops and pcxl_dma_ops */
 #include <linux/initrd.h>
 #include <linux/swap.h>
 #include <linux/unistd.h>

@@ -616,17 +615,13 @@ void __init mem_init(void)
 	free_all_bootmem();
 
 #ifdef CONFIG_PA11
-	if (hppa_dma_ops == &pcxl_dma_ops) {
+	if (boot_cpu_data.cpu_type == pcxl2 || boot_cpu_data.cpu_type == pcxl) {
 		pcxl_dma_start = (unsigned long)SET_MAP_OFFSET(MAP_START);
 		parisc_vmalloc_start = SET_MAP_OFFSET(pcxl_dma_start
 						+ PCXL_DMA_MAP_SIZE);
-	} else {
-		pcxl_dma_start = 0;
-		parisc_vmalloc_start = SET_MAP_OFFSET(MAP_START);
-	}
-#else
-	parisc_vmalloc_start = SET_MAP_OFFSET(MAP_START);
+	} else
 #endif
+		parisc_vmalloc_start = SET_MAP_OFFSET(MAP_START);
 
 	mem_init_print_info(NULL);
 

Quoted 10 lines · Christoph Hellwig

Switch to the generic noncoherent direct mapping implementation.

Parisc previously had two different non-coherent dma ops implementation that just different in the way coherent allocations were handled or not handled.  The different behavior is not selected at runtime in the arch_dma_alloc and arch_dma_free routines.  The non-coherent allocation in the pcx cases now uses the dma_direct helpers that are a little more sophisticated and used by a lot of other architectures.

Quoted 7 lines

Fix sync_single_for_cpu to do skip the cache flush unless the transfer is to the device to match the more tested unmap_single path which should have the same cache coherency implications.

This also now consistenly uses flush_kernel_dcache_range for cache flushing while previously some of the SG based operations used flush_kernel_vmap_range instead.

This patch breaks a 32bit kernel on a B160L machine (PA7300LC CPU, "pcxl2"). After applying this patch series the lasi82956 network driver works unreliable.  NIC gets IP, but ping doesn't work. See drivers/net/ethernet/i825xx/lasi_82596.c, it uses dma*sync() functions.