use the generic dma-noncoherent code for sh

use the generic dma-noncoherent code for sh

[Christoph Hellwig]

Hi all,

can you review these patches to switch sh to use the generic
dma-noncoherent code?  All the requirements are in mainline already
and we've switched various architectures over to it already.

[PATCH 1/5] sh: simplify get_arch_dma_ops

[Christoph Hellwig]

Remove the indirection through the dma_ops variable, and just return
nommu_dma_ops directly from get_arch_dma_ops.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/sh/include/asm/dma-mapping.h |  5 ++---
 arch/sh/kernel/dma-nommu.c        |  8 +-------
 arch/sh/mm/consistent.c           |  3 ---
 arch/sh/mm/init.c                 | 10 ----------
 4 files changed, 3 insertions(+), 23 deletions(-)

diff --git a/arch/sh/include/asm/dma-mapping.h b/arch/sh/include/asm/dma-mapping.h
index 41167931e5d9..149e71f95be7 100644
--- a/arch/sh/include/asm/dma-mapping.h
+++ b/arch/sh/include/asm/dma-mapping.h
@@ -2,12 +2,11 @@
 #ifndef __ASM_SH_DMA_MAPPING_H
 #define __ASM_SH_DMA_MAPPING_H
 
-extern const struct dma_map_ops *dma_ops;
-extern void no_iommu_init(void);
+extern const struct dma_map_ops nommu_dma_ops;
 
 static inline const struct dma_map_ops *get_arch_dma_ops(struct bus_type *bus)
 {
-	return dma_ops;
+	return &nommu_dma_ops;
 }
 
 extern void *dma_generic_alloc_coherent(struct device *dev, size_t size,
diff --git a/arch/sh/kernel/dma-nommu.c b/arch/sh/kernel/dma-nommu.c
index 3e3a32fc676e..79a9edafa5b0 100644
--- a/arch/sh/kernel/dma-nommu.c
+++ b/arch/sh/kernel/dma-nommu.c
@@ -79,10 +79,4 @@ const struct dma_map_ops nommu_dma_ops = {
 	.sync_sg_for_device	= nommu_sync_sg_for_device,
 #endif
 };
-
-void __init no_iommu_init(void)
-{
-	if (dma_ops)
-		return;
-	dma_ops = &nommu_dma_ops;
-}
+EXPORT_SYMBOL(nommu_dma_ops);
diff --git a/arch/sh/mm/consistent.c b/arch/sh/mm/consistent.c
index fceb2adfcac7..e9d422bd42a5 100644
--- a/arch/sh/mm/consistent.c
+++ b/arch/sh/mm/consistent.c
@@ -20,9 +20,6 @@
 #include <asm/cacheflush.h>
 #include <asm/addrspace.h>
 
-const struct dma_map_ops *dma_ops;
-EXPORT_SYMBOL(dma_ops);
-
 void *dma_generic_alloc_coherent(struct device *dev, size_t size,
 				 dma_addr_t *dma_handle, gfp_t gfp,
 				 unsigned long attrs)
diff --git a/arch/sh/mm/init.c b/arch/sh/mm/init.c
index 4034035fbede..7713c084d040 100644
--- a/arch/sh/mm/init.c
+++ b/arch/sh/mm/init.c
@@ -339,22 +339,12 @@ void __init paging_init(void)
 	free_area_init_nodes(max_zone_pfns);
 }
 
-/*
- * Early initialization for any I/O MMUs we might have.
- */
-static void __init iommu_init(void)
-{
-	no_iommu_init();
-}
-
 unsigned int mem_init_done = 0;
 
 void __init mem_init(void)
 {
 	pg_data_t *pgdat;
 
-	iommu_init();
-
 	high_memory = NULL;
 	for_each_online_pgdat(pgdat)
 		high_memory = max_t(void *, high_memory,
-- 
2.18.0

[PATCH 2/5] sh: introduce a sh_cacheop_vaddr helper

[Christoph Hellwig]

And use it in the maple bus code to avoid a dma API dependency.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/sh/include/asm/cacheflush.h | 7 +++++++
 arch/sh/mm/consistent.c          | 6 +-----
 drivers/sh/maple/maple.c         | 7 ++++---
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/arch/sh/include/asm/cacheflush.h b/arch/sh/include/asm/cacheflush.h
index d103ab5a4e4b..b932e42ef028 100644
--- a/arch/sh/include/asm/cacheflush.h
+++ b/arch/sh/include/asm/cacheflush.h
@@ -101,5 +101,12 @@ void kunmap_coherent(void *kvaddr);
 
 void cpu_cache_init(void);
 
+static inline void *sh_cacheop_vaddr(void *vaddr)
+{
+	if (__in_29bit_mode())
+		vaddr = (void *)CAC_ADDR((unsigned long)vaddr);
+	return vaddr;
+}
+
 #endif /* __KERNEL__ */
 #endif /* __ASM_SH_CACHEFLUSH_H */
diff --git a/arch/sh/mm/consistent.c b/arch/sh/mm/consistent.c
index e9d422bd42a5..1622ae6b9dbd 100644
--- a/arch/sh/mm/consistent.c
+++ b/arch/sh/mm/consistent.c
@@ -74,10 +74,7 @@ void dma_generic_free_coherent(struct device *dev, size_t size,
 void sh_sync_dma_for_device(void *vaddr, size_t size,
 		    enum dma_data_direction direction)
 {
-	void *addr;
-
-	addr = __in_29bit_mode() ?
-	       (void *)CAC_ADDR((unsigned long)vaddr) : vaddr;
+	void *addr = sh_cacheop_vaddr(vaddr);
 
 	switch (direction) {
 	case DMA_FROM_DEVICE:		/* invalidate only */
@@ -93,7 +90,6 @@ void sh_sync_dma_for_device(void *vaddr, size_t size,
 		BUG();
 	}
 }
-EXPORT_SYMBOL(sh_sync_dma_for_device);
 
 static int __init memchunk_setup(char *str)
 {
diff --git a/drivers/sh/maple/maple.c b/drivers/sh/maple/maple.c
index 2e45988d1259..e5d7fb81ad66 100644
--- a/drivers/sh/maple/maple.c
+++ b/drivers/sh/maple/maple.c
@@ -300,8 +300,8 @@ static void maple_send(void)
 	mutex_unlock(&maple_wlist_lock);
 	if (maple_packets > 0) {
 		for (i = 0; i < (1 << MAPLE_DMA_PAGES); i++)
-			sh_sync_dma_for_device(maple_sendbuf + i * PAGE_SIZE,
-				       PAGE_SIZE, DMA_BIDIRECTIONAL);
+			__flush_purge_region(maple_sendbuf + i * PAGE_SIZE,
+					PAGE_SIZE);
 	}
 
 finish:
@@ -642,7 +642,8 @@ static void maple_dma_handler(struct work_struct *work)
 		list_for_each_entry_safe(mq, nmq, &maple_sentq, list) {
 			mdev = mq->dev;
 			recvbuf = mq->recvbuf->buf;
-			sh_sync_dma_for_device(recvbuf, 0x400, DMA_FROM_DEVICE);
+			__flush_invalidate_region(sh_cacheop_vaddr(recvbuf),
+					0x400);
 			code = recvbuf[0];
 			kfree(mq->sendbuf);
 			list_del_init(&mq->list);
-- 
2.18.0

[PATCH 3/5] sh: use dma_direct_ops for the CONFIG_DMA_COHERENT case

[Christoph Hellwig]

This is a slight change in behavior as we avoid the detour through the
virtual mapping for the coherent allocator, but if this CPU really is
coherent that should be the right thing to do.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/sh/Kconfig                   | 1 +
 arch/sh/include/asm/dma-mapping.h | 4 ++++
 arch/sh/kernel/Makefile           | 4 ++--
 arch/sh/kernel/dma-nommu.c        | 4 ----
 4 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
index dd4f3d3e644f..c9993a0cdc7e 100644
--- a/arch/sh/Kconfig
+++ b/arch/sh/Kconfig
@@ -159,6 +159,7 @@ config SWAP_IO_SPACE
 	bool
 
 config DMA_COHERENT
+	select DMA_DIRECT_OPS
 	bool
 
 config DMA_NONCOHERENT
diff --git a/arch/sh/include/asm/dma-mapping.h b/arch/sh/include/asm/dma-mapping.h
index 149e71f95be7..1ebc6a4eb1c5 100644
--- a/arch/sh/include/asm/dma-mapping.h
+++ b/arch/sh/include/asm/dma-mapping.h
@@ -6,7 +6,11 @@ extern const struct dma_map_ops nommu_dma_ops;
 
 static inline const struct dma_map_ops *get_arch_dma_ops(struct bus_type *bus)
 {
+#ifdef CONFIG_DMA_NONCOHERENT
 	return &nommu_dma_ops;
+#else
+	return &dma_direct_ops;
+#endif
 }
 
 extern void *dma_generic_alloc_coherent(struct device *dev, size_t size,
diff --git a/arch/sh/kernel/Makefile b/arch/sh/kernel/Makefile
index dc80041f7363..cb5f1bfb52de 100644
--- a/arch/sh/kernel/Makefile
+++ b/arch/sh/kernel/Makefile
@@ -12,7 +12,7 @@ endif
 
 CFLAGS_REMOVE_return_address.o = -pg
 
-obj-y	:= debugtraps.o dma-nommu.o dumpstack.o 		\
+obj-y	:= debugtraps.o dumpstack.o 		\
 	   idle.o io.o irq.o irq_$(BITS).o kdebugfs.o			\
 	   machvec.o nmi_debug.o process.o				\
 	   process_$(BITS).o ptrace.o ptrace_$(BITS).o			\
@@ -45,7 +45,7 @@ obj-$(CONFIG_DUMP_CODE)		+= disassemble.o
 obj-$(CONFIG_HIBERNATION)	+= swsusp.o
 obj-$(CONFIG_DWARF_UNWINDER)	+= dwarf.o
 obj-$(CONFIG_PERF_EVENTS)	+= perf_event.o perf_callchain.o
-
+obj-$(CONFIG_DMA_NONCOHERENT)	+= dma-nommu.o
 obj-$(CONFIG_HAVE_HW_BREAKPOINT)		+= hw_breakpoint.o
 
 ccflags-y := -Werror
diff --git a/arch/sh/kernel/dma-nommu.c b/arch/sh/kernel/dma-nommu.c
index 79a9edafa5b0..d8689b1cb743 100644
--- a/arch/sh/kernel/dma-nommu.c
+++ b/arch/sh/kernel/dma-nommu.c
@@ -51,7 +51,6 @@ static int nommu_map_sg(struct device *dev, struct scatterlist *sg,
 	return nents;
 }
 
-#ifdef CONFIG_DMA_NONCOHERENT
 static void nommu_sync_single_for_device(struct device *dev, dma_addr_t addr,
 			      size_t size, enum dma_data_direction dir)
 {
@@ -67,16 +66,13 @@ static void nommu_sync_sg_for_device(struct device *dev, struct scatterlist *sg,
 	for_each_sg(sg, s, nelems, i)
 		sh_sync_dma_for_device(sg_virt(s), s->length, dir);
 }
-#endif
 
 const struct dma_map_ops nommu_dma_ops = {
 	.alloc			= dma_generic_alloc_coherent,
 	.free			= dma_generic_free_coherent,
 	.map_page		= nommu_map_page,
 	.map_sg			= nommu_map_sg,
-#ifdef CONFIG_DMA_NONCOHERENT
 	.sync_single_for_device	= nommu_sync_single_for_device,
 	.sync_sg_for_device	= nommu_sync_sg_for_device,
-#endif
 };
 EXPORT_SYMBOL(nommu_dma_ops);
-- 
2.18.0

[PATCH 4/5] sh: split arch/sh/mm/consistent.c

[Christoph Hellwig]

Half of the file just contains platform device memory setup code which
is required for all builds, and half contains helpers for dma coherent
allocation, which is only needed if CONFIG_DMA_NONCOHERENT is enabled.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/sh/kernel/Makefile       |  2 +-
 arch/sh/kernel/dma-coherent.c | 85 +++++++++++++++++++++++++++++++++++
 arch/sh/mm/consistent.c       | 80 ---------------------------------
 3 files changed, 86 insertions(+), 81 deletions(-)
 create mode 100644 arch/sh/kernel/dma-coherent.c

diff --git a/arch/sh/kernel/Makefile b/arch/sh/kernel/Makefile
index cb5f1bfb52de..d5ddb64bfffe 100644
--- a/arch/sh/kernel/Makefile
+++ b/arch/sh/kernel/Makefile
@@ -45,7 +45,7 @@ obj-$(CONFIG_DUMP_CODE)		+= disassemble.o
 obj-$(CONFIG_HIBERNATION)	+= swsusp.o
 obj-$(CONFIG_DWARF_UNWINDER)	+= dwarf.o
 obj-$(CONFIG_PERF_EVENTS)	+= perf_event.o perf_callchain.o
-obj-$(CONFIG_DMA_NONCOHERENT)	+= dma-nommu.o
+obj-$(CONFIG_DMA_NONCOHERENT)	+= dma-nommu.o dma-coherent.o
 obj-$(CONFIG_HAVE_HW_BREAKPOINT)		+= hw_breakpoint.o
 
 ccflags-y := -Werror
diff --git a/arch/sh/kernel/dma-coherent.c b/arch/sh/kernel/dma-coherent.c
new file mode 100644
index 000000000000..0e7720b5d495
--- /dev/null
+++ b/arch/sh/kernel/dma-coherent.c
@@ -0,0 +1,85 @@
+/*
+ * Copyright (C) 2004 - 2007  Paul Mundt
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */
+#include <linux/mm.h>
+#include <linux/init.h>
+#include <linux/dma-mapping.h>
+#include <linux/module.h>
+#include <asm/cacheflush.h>
+#include <asm/addrspace.h>
+
+void *dma_generic_alloc_coherent(struct device *dev, size_t size,
+				 dma_addr_t *dma_handle, gfp_t gfp,
+				 unsigned long attrs)
+{
+	void *ret, *ret_nocache;
+	int order = get_order(size);
+
+	gfp |= __GFP_ZERO;
+
+	ret = (void *)__get_free_pages(gfp, order);
+	if (!ret)
+		return NULL;
+
+	/*
+	 * Pages from the page allocator may have data present in
+	 * cache. So flush the cache before using uncached memory.
+	 */
+	sh_sync_dma_for_device(ret, size, DMA_BIDIRECTIONAL);
+
+	ret_nocache = (void __force *)ioremap_nocache(virt_to_phys(ret), size);
+	if (!ret_nocache) {
+		free_pages((unsigned long)ret, order);
+		return NULL;
+	}
+
+	split_page(pfn_to_page(virt_to_phys(ret) >> PAGE_SHIFT), order);
+
+	*dma_handle = virt_to_phys(ret);
+	if (!WARN_ON(!dev))
+		*dma_handle - PFN_PHYS(dev->dma_pfn_offset);
+
+	return ret_nocache;
+}
+
+void dma_generic_free_coherent(struct device *dev, size_t size,
+			       void *vaddr, dma_addr_t dma_handle,
+			       unsigned long attrs)
+{
+	int order = get_order(size);
+	unsigned long pfn = (dma_handle >> PAGE_SHIFT);
+	int k;
+
+	if (!WARN_ON(!dev))
+		pfn + dev->dma_pfn_offset;
+
+	for (k = 0; k < (1 << order); k++)
+		__free_pages(pfn_to_page(pfn + k), 0);
+
+	iounmap(vaddr);
+}
+
+void sh_sync_dma_for_device(void *vaddr, size_t size,
+		    enum dma_data_direction direction)
+{
+	void *addr = sh_cacheop_vaddr(vaddr);
+
+	switch (direction) {
+	case DMA_FROM_DEVICE:		/* invalidate only */
+		__flush_invalidate_region(addr, size);
+		break;
+	case DMA_TO_DEVICE:		/* writeback only */
+		__flush_wback_region(addr, size);
+		break;
+	case DMA_BIDIRECTIONAL:		/* writeback and invalidate */
+		__flush_purge_region(addr, size);
+		break;
+	default:
+		BUG();
+	}
+}
+EXPORT_SYMBOL(sh_sync_dma_for_device);
diff --git a/arch/sh/mm/consistent.c b/arch/sh/mm/consistent.c
index 1622ae6b9dbd..792f36129062 100644
--- a/arch/sh/mm/consistent.c
+++ b/arch/sh/mm/consistent.c
@@ -1,10 +1,6 @@
 /*
- * arch/sh/mm/consistent.c
- *
  * Copyright (C) 2004 - 2007  Paul Mundt
  *
- * Declared coherent memory functions based on arch/x86/kernel/pci-dma_32.c
- *
  * This file is subject to the terms and conditions of the GNU General Public
  * License.  See the file "COPYING" in the main directory of this archive
  * for more details.
@@ -13,83 +9,7 @@
 #include <linux/init.h>
 #include <linux/platform_device.h>
 #include <linux/dma-mapping.h>
-#include <linux/dma-debug.h>
 #include <linux/io.h>
-#include <linux/module.h>
-#include <linux/gfp.h>
-#include <asm/cacheflush.h>
-#include <asm/addrspace.h>
-
-void *dma_generic_alloc_coherent(struct device *dev, size_t size,
-				 dma_addr_t *dma_handle, gfp_t gfp,
-				 unsigned long attrs)
-{
-	void *ret, *ret_nocache;
-	int order = get_order(size);
-
-	gfp |= __GFP_ZERO;
-
-	ret = (void *)__get_free_pages(gfp, order);
-	if (!ret)
-		return NULL;
-
-	/*
-	 * Pages from the page allocator may have data present in
-	 * cache. So flush the cache before using uncached memory.
-	 */
-	sh_sync_dma_for_device(ret, size, DMA_BIDIRECTIONAL);
-
-	ret_nocache = (void __force *)ioremap_nocache(virt_to_phys(ret), size);
-	if (!ret_nocache) {
-		free_pages((unsigned long)ret, order);
-		return NULL;
-	}
-
-	split_page(pfn_to_page(virt_to_phys(ret) >> PAGE_SHIFT), order);
-
-	*dma_handle = virt_to_phys(ret);
-	if (!WARN_ON(!dev))
-		*dma_handle -= PFN_PHYS(dev->dma_pfn_offset);
-
-	return ret_nocache;
-}
-
-void dma_generic_free_coherent(struct device *dev, size_t size,
-			       void *vaddr, dma_addr_t dma_handle,
-			       unsigned long attrs)
-{
-	int order = get_order(size);
-	unsigned long pfn = dma_handle >> PAGE_SHIFT;
-	int k;
-
-	if (!WARN_ON(!dev))
-		pfn += dev->dma_pfn_offset;
-
-	for (k = 0; k < (1 << order); k++)
-		__free_pages(pfn_to_page(pfn + k), 0);
-
-	iounmap(vaddr);
-}
-
-void sh_sync_dma_for_device(void *vaddr, size_t size,
-		    enum dma_data_direction direction)
-{
-	void *addr = sh_cacheop_vaddr(vaddr);
-
-	switch (direction) {
-	case DMA_FROM_DEVICE:		/* invalidate only */
-		__flush_invalidate_region(addr, size);
-		break;
-	case DMA_TO_DEVICE:		/* writeback only */
-		__flush_wback_region(addr, size);
-		break;
-	case DMA_BIDIRECTIONAL:		/* writeback and invalidate */
-		__flush_purge_region(addr, size);
-		break;
-	default:
-		BUG();
-	}
-}
 
 static int __init memchunk_setup(char *str)
 {
-- 
2.18.0

Re: [PATCH 4/5] sh: split arch/sh/mm/consistent.c

[jacopo mondi]

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

Hi Christoph,

I have a single comment on something I noticed which might be an
implementation bug. On the overall patch architecture, it's very hard
for me to provide a valuable opinion as it's all relatively new for me
here :)

On Thu, Jul 19, 2018 at 06:05:15AM -0700, Christoph Hellwig wrote:
> Half of the file just contains platform device memory setup code which
> is required for all builds, and half contains helpers for dma coherent
> allocation, which is only needed if CONFIG_DMA_NONCOHERENT is enabled.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  arch/sh/kernel/Makefile       |  2 +-
>  arch/sh/kernel/dma-coherent.c | 85 +++++++++++++++++++++++++++++++++++
>  arch/sh/mm/consistent.c       | 80 ---------------------------------
>  3 files changed, 86 insertions(+), 81 deletions(-)
>  create mode 100644 arch/sh/kernel/dma-coherent.c
>
> diff --git a/arch/sh/kernel/Makefile b/arch/sh/kernel/Makefile
> index cb5f1bfb52de..d5ddb64bfffe 100644
> --- a/arch/sh/kernel/Makefile
> +++ b/arch/sh/kernel/Makefile
> @@ -45,7 +45,7 @@ obj-$(CONFIG_DUMP_CODE)		+= disassemble.o
>  obj-$(CONFIG_HIBERNATION)	+= swsusp.o
>  obj-$(CONFIG_DWARF_UNWINDER)	+= dwarf.o
>  obj-$(CONFIG_PERF_EVENTS)	+= perf_event.o perf_callchain.o
> -obj-$(CONFIG_DMA_NONCOHERENT)	+= dma-nommu.o
> +obj-$(CONFIG_DMA_NONCOHERENT)	+= dma-nommu.o dma-coherent.o
>  obj-$(CONFIG_HAVE_HW_BREAKPOINT)		+= hw_breakpoint.o
>
>  ccflags-y := -Werror
> diff --git a/arch/sh/kernel/dma-coherent.c b/arch/sh/kernel/dma-coherent.c
> new file mode 100644
> index 000000000000..0e7720b5d495
> --- /dev/null
> +++ b/arch/sh/kernel/dma-coherent.c
> @@ -0,0 +1,85 @@
> +/*
> + * Copyright (C) 2004 - 2007  Paul Mundt
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License.  See the file "COPYING" in the main directory of this archive
> + * for more details.
> + */
> +#include <linux/mm.h>
> +#include <linux/init.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/module.h>
> +#include <asm/cacheflush.h>
> +#include <asm/addrspace.h>
> +
> +void *dma_generic_alloc_coherent(struct device *dev, size_t size,
> +				 dma_addr_t *dma_handle, gfp_t gfp,
> +				 unsigned long attrs)
> +{
> +	void *ret, *ret_nocache;
> +	int order = get_order(size);
> +
> +	gfp |= __GFP_ZERO;
> +
> +	ret = (void *)__get_free_pages(gfp, order);
> +	if (!ret)
> +		return NULL;
> +
> +	/*
> +	 * Pages from the page allocator may have data present in
> +	 * cache. So flush the cache before using uncached memory.
> +	 */
> +	sh_sync_dma_for_device(ret, size, DMA_BIDIRECTIONAL);
> +
> +	ret_nocache = (void __force *)ioremap_nocache(virt_to_phys(ret), size);
> +	if (!ret_nocache) {
> +		free_pages((unsigned long)ret, order);
> +		return NULL;
> +	}
> +
> +	split_page(pfn_to_page(virt_to_phys(ret) >> PAGE_SHIFT), order);
> +
> +	*dma_handle = virt_to_phys(ret);
> +	if (!WARN_ON(!dev))
> +		*dma_handle - PFN_PHYS(dev->dma_pfn_offset);

I guess this comes from below...

> +
> +	return ret_nocache;
> +}
> +
> +void dma_generic_free_coherent(struct device *dev, size_t size,
> +			       void *vaddr, dma_addr_t dma_handle,
> +			       unsigned long attrs)
> +{
> +	int order = get_order(size);
> +	unsigned long pfn = (dma_handle >> PAGE_SHIFT);
> +	int k;
> +
> +	if (!WARN_ON(!dev))
> +		pfn + dev->dma_pfn_offset;
> +
> +	for (k = 0; k < (1 << order); k++)
> +		__free_pages(pfn_to_page(pfn + k), 0);
> +
> +	iounmap(vaddr);
> +}
> +
> +void sh_sync_dma_for_device(void *vaddr, size_t size,
> +		    enum dma_data_direction direction)
> +{
> +	void *addr = sh_cacheop_vaddr(vaddr);
> +
> +	switch (direction) {
> +	case DMA_FROM_DEVICE:		/* invalidate only */
> +		__flush_invalidate_region(addr, size);
> +		break;
> +	case DMA_TO_DEVICE:		/* writeback only */
> +		__flush_wback_region(addr, size);
> +		break;
> +	case DMA_BIDIRECTIONAL:		/* writeback and invalidate */
> +		__flush_purge_region(addr, size);
> +		break;
> +	default:
> +		BUG();
> +	}
> +}
> +EXPORT_SYMBOL(sh_sync_dma_for_device);
> diff --git a/arch/sh/mm/consistent.c b/arch/sh/mm/consistent.c
> index 1622ae6b9dbd..792f36129062 100644
> --- a/arch/sh/mm/consistent.c
> +++ b/arch/sh/mm/consistent.c
> @@ -1,10 +1,6 @@
>  /*
> - * arch/sh/mm/consistent.c
> - *
>   * Copyright (C) 2004 - 2007  Paul Mundt
>   *
> - * Declared coherent memory functions based on arch/x86/kernel/pci-dma_32.c
> - *
>   * This file is subject to the terms and conditions of the GNU General Public
>   * License.  See the file "COPYING" in the main directory of this archive
>   * for more details.
> @@ -13,83 +9,7 @@
>  #include <linux/init.h>
>  #include <linux/platform_device.h>
>  #include <linux/dma-mapping.h>
> -#include <linux/dma-debug.h>
>  #include <linux/io.h>
> -#include <linux/module.h>
> -#include <linux/gfp.h>
> -#include <asm/cacheflush.h>
> -#include <asm/addrspace.h>
> -
> -void *dma_generic_alloc_coherent(struct device *dev, size_t size,
> -				 dma_addr_t *dma_handle, gfp_t gfp,
> -				 unsigned long attrs)
> -{
> -	void *ret, *ret_nocache;
> -	int order = get_order(size);
> -
> -	gfp |= __GFP_ZERO;
> -
> -	ret = (void *)__get_free_pages(gfp, order);
> -	if (!ret)
> -		return NULL;
> -
> -	/*
> -	 * Pages from the page allocator may have data present in
> -	 * cache. So flush the cache before using uncached memory.
> -	 */
> -	sh_sync_dma_for_device(ret, size, DMA_BIDIRECTIONAL);
> -
> -	ret_nocache = (void __force *)ioremap_nocache(virt_to_phys(ret), size);
> -	if (!ret_nocache) {
> -		free_pages((unsigned long)ret, order);
> -		return NULL;
> -	}
> -
> -	split_page(pfn_to_page(virt_to_phys(ret) >> PAGE_SHIFT), order);
> -
> -	*dma_handle = virt_to_phys(ret);
> -	if (!WARN_ON(!dev))
> -		*dma_handle -= PFN_PHYS(dev->dma_pfn_offset);

... here

Is the s/-=/- intended?

Snippets copied here below:

> +	*dma_handle = virt_to_phys(ret);
> +	if (!WARN_ON(!dev))
> +		*dma_handle - PFN_PHYS(dev->dma_pfn_offset);
vs
> -	*dma_handle = virt_to_phys(ret);
> -	if (!WARN_ON(!dev))
> -		*dma_handle -= PFN_PHYS(dev->dma_pfn_offset);



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 4/5] sh: split arch/sh/mm/consistent.c

[Geert Uytterhoeven]

Hi Jacopo,

On Mon, Jul 23, 2018 at 10:42 AM jacopo mondi <jacopo@jmondi.org> wrote:
> I have a single comment on something I noticed which might be an
> implementation bug. On the overall patch architecture, it's very hard
> for me to provide a valuable opinion as it's all relatively new for me
> here :)
>
> On Thu, Jul 19, 2018 at 06:05:15AM -0700, Christoph Hellwig wrote:
> > Half of the file just contains platform device memory setup code which
> > is required for all builds, and half contains helpers for dma coherent
> > allocation, which is only needed if CONFIG_DMA_NONCOHERENT is enabled.
> >
> > Signed-off-by: Christoph Hellwig <hch@lst.de>

> > --- /dev/null
> > +++ b/arch/sh/kernel/dma-coherent.c
> > @@ -0,0 +1,85 @@
> > +/*
> > + * Copyright (C) 2004 - 2007  Paul Mundt
> > + *
> > + * This file is subject to the terms and conditions of the GNU General Public
> > + * License.  See the file "COPYING" in the main directory of this archive
> > + * for more details.
> > + */
> > +#include <linux/mm.h>
> > +#include <linux/init.h>
> > +#include <linux/dma-mapping.h>
> > +#include <linux/module.h>
> > +#include <asm/cacheflush.h>
> > +#include <asm/addrspace.h>
> > +
> > +void *dma_generic_alloc_coherent(struct device *dev, size_t size,
> > +                              dma_addr_t *dma_handle, gfp_t gfp,
> > +                              unsigned long attrs)
> > +{
> > +     void *ret, *ret_nocache;
> > +     int order = get_order(size);
> > +
> > +     gfp |= __GFP_ZERO;
> > +
> > +     ret = (void *)__get_free_pages(gfp, order);
> > +     if (!ret)
> > +             return NULL;
> > +
> > +     /*
> > +      * Pages from the page allocator may have data present in
> > +      * cache. So flush the cache before using uncached memory.
> > +      */
> > +     sh_sync_dma_for_device(ret, size, DMA_BIDIRECTIONAL);
> > +
> > +     ret_nocache = (void __force *)ioremap_nocache(virt_to_phys(ret), size);
> > +     if (!ret_nocache) {
> > +             free_pages((unsigned long)ret, order);
> > +             return NULL;
> > +     }
> > +
> > +     split_page(pfn_to_page(virt_to_phys(ret) >> PAGE_SHIFT), order);
> > +
> > +     *dma_handle = virt_to_phys(ret);
> > +     if (!WARN_ON(!dev))
> > +             *dma_handle - PFN_PHYS(dev->dma_pfn_offset);
>
> I guess this comes from below...

> > -     *dma_handle = virt_to_phys(ret);
> > -     if (!WARN_ON(!dev))
> > -             *dma_handle -= PFN_PHYS(dev->dma_pfn_offset);
>
> ... here
>
> Is the s/-=/- intended?
>
> Snippets copied here below:
>
> > +     *dma_handle = virt_to_phys(ret);
> > +     if (!WARN_ON(!dev))
> > +             *dma_handle - PFN_PHYS(dev->dma_pfn_offset);
> vs
> > -     *dma_handle = virt_to_phys(ret);
> > -     if (!WARN_ON(!dev))
> > -             *dma_handle -= PFN_PHYS(dev->dma_pfn_offset);

Doesn't look right to me, neither.

No complaints for 0day? My gcc says:

    error: statement with no effect [-Werror=unused-value]

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

Re: [PATCH 4/5] sh: split arch/sh/mm/consistent.c

[Christoph Hellwig]

On Mon, Jul 23, 2018 at 10:49:39AM +0200, Geert Uytterhoeven wrote:
> > > +     *dma_handle = virt_to_phys(ret);
> > > +     if (!WARN_ON(!dev))
> > > +             *dma_handle - PFN_PHYS(dev->dma_pfn_offset);
> > vs
> > > -     *dma_handle = virt_to_phys(ret);
> > > -     if (!WARN_ON(!dev))
> > > -             *dma_handle -= PFN_PHYS(dev->dma_pfn_offset);
> 
> Doesn't look right to me, neither.
> 
> No complaints for 0day? My gcc says:
> 
>     error: statement with no effect [-Werror=unused-value]

For some reason 0day has failed me multiple times for sh.  Does the
0day bot even cover sh at all?

RE: [PATCH 4/5] sh: split arch/sh/mm/consistent.c

[Li, Philip]

> Subject: Re: [PATCH 4/5] sh: split arch/sh/mm/consistent.c
> 
> On Mon, Jul 23, 2018 at 10:49:39AM +0200, Geert Uytterhoeven wrote:
> > > > +     *dma_handle = virt_to_phys(ret);
> > > > +     if (!WARN_ON(!dev))
> > > > +             *dma_handle - PFN_PHYS(dev->dma_pfn_offset);
> > > vs
> > > > -     *dma_handle = virt_to_phys(ret);
> > > > -     if (!WARN_ON(!dev))
> > > > -             *dma_handle -= PFN_PHYS(dev->dma_pfn_offset);
> >
> > Doesn't look right to me, neither.
> >
> > No complaints for 0day? My gcc says:
> >
> >     error: statement with no effect [-Werror=unused-value]
> 
> For some reason 0day has failed me multiple times for sh.  Does the
> 0day bot even cover sh at all?
yes, we do cover the sh arch. Sorry for missing the reports for sh arch.

After check, for unknown reason, certain commit leads to large build error and makes
disc full, so that other sh builds are impacted (blocked). I will follow up this to see
how to solve the issue.

kernel/sh-allnoconfig/gcc-7# find -mindepth 1 -maxdepth 1|xargs -i du -sh {}
30G     ./f41ccf64b487381388c2b5ef8c13d79509dde76e
kernel/sh-rsk7269_defconfig/gcc-7# find -mindepth 1 -maxdepth 1|xargs -i du -sh {}
32G     ./f41ccf64b487381388c2b5ef8c13d79509dde76e

kernel/sh-rsk7269_defconfig/gcc-7/f41ccf64b487381388c2b5ef8c13d79509dde76e/build-error# ls
make-oldnoconfig
make-prepare
make-prepare.0
make-prepare.1
make-prepare.10
make-prepare.100
make-prepare.1000

Thanks

[PATCH 5/5] sh: use generic dma_noncoherent_ops

[Christoph Hellwig]

Switch to the generic noncoherent direct mapping implementation.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/sh/Kconfig                   |  3 +-
 arch/sh/include/asm/Kbuild        |  1 +
 arch/sh/include/asm/dma-mapping.h | 26 -----------
 arch/sh/kernel/Makefile           |  2 +-
 arch/sh/kernel/dma-coherent.c     | 23 +++++----
 arch/sh/kernel/dma-nommu.c        | 78 -------------------------------
 6 files changed, 15 insertions(+), 118 deletions(-)
 delete mode 100644 arch/sh/include/asm/dma-mapping.h
 delete mode 100644 arch/sh/kernel/dma-nommu.c

diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
index c9993a0cdc7e..da4db4b5359f 100644
--- a/arch/sh/Kconfig
+++ b/arch/sh/Kconfig
@@ -51,7 +51,6 @@ config SUPERH
 	select HAVE_ARCH_AUDITSYSCALL
 	select HAVE_FUTEX_CMPXCHG if FUTEX
 	select HAVE_NMI
-	select NEED_DMA_MAP_STATE
 	select NEED_SG_DMA_LENGTH
 
 	help
@@ -164,6 +163,8 @@ config DMA_COHERENT
 
 config DMA_NONCOHERENT
 	def_bool !DMA_COHERENT
+	select ARCH_HAS_SYNC_DMA_FOR_DEVICE
+	select DMA_NONCOHERENT_OPS
 
 config PGTABLE_LEVELS
 	default 3 if X2TLB
diff --git a/arch/sh/include/asm/Kbuild b/arch/sh/include/asm/Kbuild
index 46dd82ab2c29..6a5609a55965 100644
--- a/arch/sh/include/asm/Kbuild
+++ b/arch/sh/include/asm/Kbuild
@@ -2,6 +2,7 @@ generic-y += compat.h
 generic-y += current.h
 generic-y += delay.h
 generic-y += div64.h
+generic-y += dma-mapping.h
 generic-y += emergency-restart.h
 generic-y += exec.h
 generic-y += irq_regs.h
diff --git a/arch/sh/include/asm/dma-mapping.h b/arch/sh/include/asm/dma-mapping.h
deleted file mode 100644
index 1ebc6a4eb1c5..000000000000
--- a/arch/sh/include/asm/dma-mapping.h
+++ /dev/null
@@ -1,26 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __ASM_SH_DMA_MAPPING_H
-#define __ASM_SH_DMA_MAPPING_H
-
-extern const struct dma_map_ops nommu_dma_ops;
-
-static inline const struct dma_map_ops *get_arch_dma_ops(struct bus_type *bus)
-{
-#ifdef CONFIG_DMA_NONCOHERENT
-	return &nommu_dma_ops;
-#else
-	return &dma_direct_ops;
-#endif
-}
-
-extern void *dma_generic_alloc_coherent(struct device *dev, size_t size,
-					dma_addr_t *dma_addr, gfp_t flag,
-					unsigned long attrs);
-extern void dma_generic_free_coherent(struct device *dev, size_t size,
-				      void *vaddr, dma_addr_t dma_handle,
-				      unsigned long attrs);
-
-void sh_sync_dma_for_device(void *vaddr, size_t size,
-	    enum dma_data_direction dir);
-
-#endif /* __ASM_SH_DMA_MAPPING_H */
diff --git a/arch/sh/kernel/Makefile b/arch/sh/kernel/Makefile
index d5ddb64bfffe..59673f8a3379 100644
--- a/arch/sh/kernel/Makefile
+++ b/arch/sh/kernel/Makefile
@@ -45,7 +45,7 @@ obj-$(CONFIG_DUMP_CODE)		+= disassemble.o
 obj-$(CONFIG_HIBERNATION)	+= swsusp.o
 obj-$(CONFIG_DWARF_UNWINDER)	+= dwarf.o
 obj-$(CONFIG_PERF_EVENTS)	+= perf_event.o perf_callchain.o
-obj-$(CONFIG_DMA_NONCOHERENT)	+= dma-nommu.o dma-coherent.o
+obj-$(CONFIG_DMA_NONCOHERENT)	+= dma-coherent.o
 obj-$(CONFIG_HAVE_HW_BREAKPOINT)		+= hw_breakpoint.o
 
 ccflags-y := -Werror
diff --git a/arch/sh/kernel/dma-coherent.c b/arch/sh/kernel/dma-coherent.c
index 0e7720b5d495..088ecd188b71 100644
--- a/arch/sh/kernel/dma-coherent.c
+++ b/arch/sh/kernel/dma-coherent.c
@@ -7,14 +7,13 @@
  */
 #include <linux/mm.h>
 #include <linux/init.h>
-#include <linux/dma-mapping.h>
+#include <linux/dma-noncoherent.h>
 #include <linux/module.h>
 #include <asm/cacheflush.h>
 #include <asm/addrspace.h>
 
-void *dma_generic_alloc_coherent(struct device *dev, size_t size,
-				 dma_addr_t *dma_handle, gfp_t gfp,
-				 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 *ret, *ret_nocache;
 	int order = get_order(size);
@@ -29,7 +28,8 @@ void *dma_generic_alloc_coherent(struct device *dev, size_t size,
 	 * Pages from the page allocator may have data present in
 	 * cache. So flush the cache before using uncached memory.
 	 */
-	sh_sync_dma_for_device(ret, size, DMA_BIDIRECTIONAL);
+	arch_sync_dma_for_device(dev, virt_to_phys(ret), size,
+			DMA_BIDIRECTIONAL);
 
 	ret_nocache = (void __force *)ioremap_nocache(virt_to_phys(ret), size);
 	if (!ret_nocache) {
@@ -46,9 +46,8 @@ void *dma_generic_alloc_coherent(struct device *dev, size_t size,
 	return ret_nocache;
 }
 
-void dma_generic_free_coherent(struct device *dev, size_t size,
-			       void *vaddr, dma_addr_t dma_handle,
-			       unsigned long attrs)
+void arch_dma_free(struct device *dev, size_t size, void *vaddr,
+		dma_addr_t dma_handle, unsigned long attrs)
 {
 	int order = get_order(size);
 	unsigned long pfn = (dma_handle >> PAGE_SHIFT);
@@ -63,12 +62,12 @@ void dma_generic_free_coherent(struct device *dev, size_t size,
 	iounmap(vaddr);
 }
 
-void sh_sync_dma_for_device(void *vaddr, size_t size,
-		    enum dma_data_direction direction)
+void arch_sync_dma_for_device(struct device *dev, phys_addr_t paddr,
+		size_t size, enum dma_data_direction dir)
 {
-	void *addr = sh_cacheop_vaddr(vaddr);
+	void *addr = sh_cacheop_vaddr(phys_to_virt(paddr));
 
-	switch (direction) {
+	switch (dir) {
 	case DMA_FROM_DEVICE:		/* invalidate only */
 		__flush_invalidate_region(addr, size);
 		break;
diff --git a/arch/sh/kernel/dma-nommu.c b/arch/sh/kernel/dma-nommu.c
deleted file mode 100644
index d8689b1cb743..000000000000
--- a/arch/sh/kernel/dma-nommu.c
+++ /dev/null
@@ -1,78 +0,0 @@
-/*
- * DMA mapping support for platforms lacking IOMMUs.
- *
- * Copyright (C) 2009  Paul Mundt
- *
- * This file is subject to the terms and conditions of the GNU General Public
- * License.  See the file "COPYING" in the main directory of this archive
- * for more details.
- */
-#include <linux/dma-mapping.h>
-#include <linux/io.h>
-#include <asm/cacheflush.h>
-
-static dma_addr_t nommu_map_page(struct device *dev, struct page *page,
-				 unsigned long offset, size_t size,
-				 enum dma_data_direction dir,
-				 unsigned long attrs)
-{
-	dma_addr_t addr = page_to_phys(page) + offset
-		- PFN_PHYS(dev->dma_pfn_offset);
-
-	WARN_ON(size == 0);
-
-	if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
-		sh_sync_dma_for_device(page_address(page) + offset, size, dir);
-
-	return addr;
-}
-
-static int nommu_map_sg(struct device *dev, struct scatterlist *sg,
-			int nents, enum dma_data_direction dir,
-			unsigned long attrs)
-{
-	struct scatterlist *s;
-	int i;
-
-	WARN_ON(nents == 0 || sg[0].length == 0);
-
-	for_each_sg(sg, s, nents, i) {
-		dma_addr_t offset = PFN_PHYS(dev->dma_pfn_offset);
-
-		BUG_ON(!sg_page(s));
-
-		if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
-			sh_sync_dma_for_device(sg_virt(s), s->length, dir);
-
-		s->dma_address = sg_phys(s) - offset;
-		s->dma_length = s->length;
-	}
-
-	return nents;
-}
-
-static void nommu_sync_single_for_device(struct device *dev, dma_addr_t addr,
-			      size_t size, enum dma_data_direction dir)
-{
-	sh_sync_dma_for_device(phys_to_virt(addr), size, dir);
-}
-
-static void nommu_sync_sg_for_device(struct device *dev, struct scatterlist *sg,
-			  int nelems, enum dma_data_direction dir)
-{
-	struct scatterlist *s;
-	int i;
-
-	for_each_sg(sg, s, nelems, i)
-		sh_sync_dma_for_device(sg_virt(s), s->length, dir);
-}
-
-const struct dma_map_ops nommu_dma_ops = {
-	.alloc			= dma_generic_alloc_coherent,
-	.free			= dma_generic_free_coherent,
-	.map_page		= nommu_map_page,
-	.map_sg			= nommu_map_sg,
-	.sync_single_for_device	= nommu_sync_single_for_device,
-	.sync_sg_for_device	= nommu_sync_sg_for_device,
-};
-EXPORT_SYMBOL(nommu_dma_ops);
-- 
2.18.0