[PATCH 2/2] arch: m68k: Add DMA mapping operations for Coldfire M5441x

Jean-Michel Hautbois posted 2 patches 11 months ago
[PATCH 2/2] arch: m68k: Add DMA mapping operations for Coldfire M5441x
Posted by Jean-Michel Hautbois 11 months ago
Introduce `dma_ops` for the Coldfire M5441x platform, enabling both
coherent and streaming DMA operations. This addition should fill the gap
for DMA support integrates with the existing kernel DMA framework.

The key operations implemented include:
- Custom allocation using the global coherent DMA pool
- Mapping and unmapping functions for pages
- Scatter-gather support
- Resource mapping

Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
---
 arch/m68k/Kconfig.machine    |  19 +++++--
 arch/m68k/coldfire/dma_ops.c | 131 +++++++++++++++++++++++++++++++++++++++++++
 arch/m68k/include/asm/dma.h  |   2 -
 arch/m68k/kernel/dma.c       |   2 +-
 arch/m68k/mm/mcfmmu.c        |   6 +-
 5 files changed, 148 insertions(+), 12 deletions(-)

diff --git a/arch/m68k/Kconfig.machine b/arch/m68k/Kconfig.machine
index 1467d1ff349bb26714a3c99f593a2a71c9ad273d..9cecc5025401976897f815505e0bc74cba28eb0d 100644
--- a/arch/m68k/Kconfig.machine
+++ b/arch/m68k/Kconfig.machine
@@ -406,12 +406,19 @@ config KERNELBASE
 	  for the theoretical maximum number of 256 vectors.
 
 config DMASIZE
-       hex "Size of DRAM (in bytes) reserved pool"
-       default "0x1000000"
-       depends on DMA_GLOBAL_POOL
-       help
-         Define the DMA pool size, allocated at init time, which can then
-         be used by DMA engine. Defaults to 16MB.
+	hex "Size of DRAM (in bytes) reserved pool"
+	default "0x1000000"
+	depends on DMA_GLOBAL_POOL
+	help
+	  Define the DMA pool size, allocated at init time, which can then
+	  be used by DMA engine. Defaults to 16MB.
+
+config DMABASE
+	hex "Address of the base of RAM"
+	default RAMBASE
+	depends on DMA_GLOBAL_POOL
+	help
+	  Define the DMA base address for the global pool.
 
 comment "ROM configuration"
 
diff --git a/arch/m68k/coldfire/dma_ops.c b/arch/m68k/coldfire/dma_ops.c
new file mode 100644
index 0000000000000000000000000000000000000000..e9967adc8821fb06c828e010ef51ef4e9702925b
--- /dev/null
+++ b/arch/m68k/coldfire/dma_ops.c
@@ -0,0 +1,131 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * dma_timer.c -- Freescale ColdFire DMA Ops helpers.
+ *
+ * Copyright (C) 2024 Jean-Michel Hautbois, Yoseli
+ *
+ */
+
+#include <linux/device.h>
+#include <linux/dma-direct.h>
+#include <linux/dma-map-ops.h>
+#include <linux/dma-mapping.h>
+#include <linux/kernel.h>
+#include <linux/scatterlist.h>
+#include <linux/slab.h>
+#include <linux/vmalloc.h>
+#include <linux/pci.h>
+#include <asm/pgalloc.h>
+
+static void *coldfire_alloc_coherent(struct device *dev, size_t size,
+				     dma_addr_t *dma_handle, gfp_t gfp,
+				     unsigned long attrs)
+{
+	void *vaddr;
+
+	vaddr = dma_alloc_from_global_coherent(dev, size, dma_handle);
+	if (!vaddr) {
+		dev_err(dev, "Failed to allocate %zu bytes\n", size);
+		return NULL;
+	}
+
+	dev_dbg(dev, "Allocated %zu bytes at %p (dma %pad)\n", size, vaddr, dma_handle);
+
+	return vaddr;
+}
+
+static void coldfire_free_coherent(struct device *dev, size_t size, void *vaddr,
+				   dma_addr_t dma_handle, unsigned long attrs)
+{
+	unsigned int page_order = get_order(size);
+
+	if (!dma_release_from_global_coherent(page_order, vaddr))
+		WARN_ON_ONCE(1);
+}
+
+static dma_addr_t coldfire_map_page(struct device *dev, struct page *page,
+				    unsigned long offset, size_t size,
+				    enum dma_data_direction dir, unsigned long attrs)
+{
+	phys_addr_t phys = page_to_phys(page) + offset;
+	dma_addr_t dma_addr = phys_to_dma(dev, phys);
+
+	if (!dev_is_dma_coherent(dev) && !(attrs & DMA_ATTR_SKIP_CPU_SYNC))
+		arch_sync_dma_for_device(phys, size, dir);
+	return dma_addr;
+}
+
+static void coldfire_unmap_page(struct device *dev, dma_addr_t dma_handle,
+				size_t size, enum dma_data_direction dir,
+				unsigned long attrs)
+{
+	phys_addr_t phys = dma_to_phys(dev, dma_handle);
+
+	if (!dev_is_dma_coherent(dev) && !(attrs & DMA_ATTR_SKIP_CPU_SYNC))
+		arch_sync_dma_for_cpu(phys, size, dir);
+}
+
+static int coldfire_map_sg(struct device *dev, struct scatterlist *sg,
+			   int nents, enum dma_data_direction dir,
+			   unsigned long attrs)
+{
+	struct scatterlist *s;
+	dma_addr_t dma_handle;
+	int i;
+
+	if (!sg) {
+		dev_err(dev, "Invalid scatterlist, cannot map memory\n");
+		return 0;
+	}
+
+	for_each_sg(sg, s, nents, i) {
+		dma_handle = page_to_phys(sg_page(s)) + sg->offset;
+		dev_dbg(dev, "Mapped scatterlist %p (offset %u) to DMA address %pad\n",
+			sg_page(s), sg->offset, &dma_handle);
+	}
+
+	return nents;
+}
+
+static void coldfire_unmap_sg(struct device *dev, struct scatterlist *sg,
+			      int nents, enum dma_data_direction dir,
+			      unsigned long attrs)
+{
+}
+
+static dma_addr_t coldfire_map_resource(struct device *dev, phys_addr_t phys,
+					size_t size, enum dma_data_direction dir,
+					unsigned long attrs)
+{
+	dma_addr_t dma_handle;
+
+	if (!phys) {
+		dev_err(dev, "Invalid physical address, cannot map memory\n");
+		return 0;
+	}
+
+	dma_handle = phys;
+	dev_dbg(dev, "Mapped physical address %pa (size %zu) to DMA address %pad\n",
+		&phys, size, &dma_handle);
+	return dma_handle;
+}
+
+static void coldfire_unmap_resource(struct device *dev, dma_addr_t dma_handle,
+				    size_t size, enum dma_data_direction dir,
+				    unsigned long attrs)
+{
+}
+
+const struct dma_map_ops coldfire_dma_ops = {
+	.alloc			= coldfire_alloc_coherent,
+	.free			= coldfire_free_coherent,
+	.map_page		= coldfire_map_page,
+	.unmap_page		= coldfire_unmap_page,
+	.map_sg			= coldfire_map_sg,
+	.unmap_sg		= coldfire_unmap_sg,
+	.map_resource		= coldfire_map_resource,
+	.unmap_resource		= coldfire_unmap_resource,
+	.alloc_pages_op		= dma_common_alloc_pages,
+	.free_pages		= dma_common_free_pages,
+};
+EXPORT_SYMBOL(coldfire_dma_ops);
diff --git a/arch/m68k/include/asm/dma.h b/arch/m68k/include/asm/dma.h
index 53f9c1570ce62ddaa521d85d9ef64523e8ad0919..fc029b710c30bf23f8bf2657ea74a1c146b3f8b1 100644
--- a/arch/m68k/include/asm/dma.h
+++ b/arch/m68k/include/asm/dma.h
@@ -5,6 +5,4 @@
 /* it's useless on the m68k, but unfortunately needed by the new
    bootmem allocator (but this should do it for this) */
 #define MAX_DMA_ADDRESS PAGE_OFFSET
-
-extern size_t dma_coldfire_base;
 #endif /* _M68K_DMA_H */
diff --git a/arch/m68k/kernel/dma.c b/arch/m68k/kernel/dma.c
index bc66b012de79be842b55f3d82726a00c929416b9..6d41fdc4991589c30f9cad783fc43f56e8f8b99b 100644
--- a/arch/m68k/kernel/dma.c
+++ b/arch/m68k/kernel/dma.c
@@ -47,7 +47,7 @@ void arch_sync_dma_for_device(phys_addr_t handle, size_t size,
 #ifdef CONFIG_DMA_GLOBAL_POOL
 static int __init coldfire_dma_init(void)
 {
-	return dma_init_global_coherent(PFN_PHYS(PFN_DOWN(coldfire_dma_base)),
+	return dma_init_global_coherent(PFN_PHYS(PFN_DOWN(CONFIG_DMABASE)),
 					CONFIG_DMASIZE);
 }
 
diff --git a/arch/m68k/mm/mcfmmu.c b/arch/m68k/mm/mcfmmu.c
index f6c560ea17a68ff6546b0683461b6b9ffb587db7..f8006a70e0f58828bedb18690f9eecfa924c8059 100644
--- a/arch/m68k/mm/mcfmmu.c
+++ b/arch/m68k/mm/mcfmmu.c
@@ -156,8 +156,6 @@ int cf_tlb_miss(struct pt_regs *regs, int write, int dtlb, int extension_word)
 	return ret;
 }
 
-size_t coldfire_dma_base = _ramend - CONFIG_DMASIZE;
-
 void __init cf_bootmem_alloc(void)
 {
 	unsigned long memstart;
@@ -181,8 +179,10 @@ void __init cf_bootmem_alloc(void)
 	/* Reserve kernel text/data/bss */
 	memblock_reserve(_rambase, memstart - _rambase);
 
+#ifdef CONFIG_DMA_GLOBAL_POOL
 	/* Reserve DMA */
-	memblock_reserve(coldfire_dma_base, CONFIG_DMASIZE);
+	memblock_reserve(CONFIG_DMABASE, CONFIG_DMASIZE);
+#endif
 
 	m68k_virt_to_node_shift = fls(_ramend - 1) - 6;
 	module_fixup(NULL, __start_fixup, __stop_fixup);

-- 
2.39.5
Re: [PATCH 2/2] arch: m68k: Add DMA mapping operations for Coldfire M5441x
Posted by Christoph Hellwig 10 months, 3 weeks ago
On Tue, Jan 21, 2025 at 11:54:14AM +0100, Jean-Michel Hautbois wrote:
> Introduce `dma_ops` for the Coldfire M5441x platform, enabling both
> coherent and streaming DMA operations. This addition should fill the gap
> for DMA support integrates with the existing kernel DMA framework.

Arch dma code should not implement DMA OPS, but use the generic
dma-direct code with the right helpes for handling coherency.  This
variant looks like it should be using the DMA_COHERENT_POOL allocator
for dma coherent memory.
Re: [PATCH 2/2] arch: m68k: Add DMA mapping operations for Coldfire M5441x
Posted by Jean-Michel Hautbois 10 months, 3 weeks ago
Hi Christoph,

On 28/01/2025 07:10, Christoph Hellwig wrote:
> On Tue, Jan 21, 2025 at 11:54:14AM +0100, Jean-Michel Hautbois wrote:
>> Introduce `dma_ops` for the Coldfire M5441x platform, enabling both
>> coherent and streaming DMA operations. This addition should fill the gap
>> for DMA support integrates with the existing kernel DMA framework.
> 
> Arch dma code should not implement DMA OPS, but use the generic
> dma-direct code with the right helpes for handling coherency.  This
> variant looks like it should be using the DMA_COHERENT_POOL allocator
> for dma coherent memory.

Thanks for your answer.
I am not sure to understand it though :-) because I can see a few 
dma_map_ops implementations in arch code. I tried to let dma_direct do 
the work, I can't remember exactly what happened but it was not great :-).
I can give it a second try.

Thanks,
JM
Re: [PATCH 2/2] arch: m68k: Add DMA mapping operations for Coldfire M5441x
Posted by Christoph Hellwig 10 months, 3 weeks ago
On Tue, Jan 28, 2025 at 07:33:37AM +0100, Jean-Michel Hautbois wrote:
> > Arch dma code should not implement DMA OPS, but use the generic
> > dma-direct code with the right helpes for handling coherency.  This
> > variant looks like it should be using the DMA_COHERENT_POOL allocator
> > for dma coherent memory.
> 
> Thanks for your answer.
> I am not sure to understand it though :-) because I can see a few
> dma_map_ops implementations in arch code.

The last one left for the direct mapping are arm32 and parisc, and
they should go away eventually.

> I tried to let dma_direct do the
> work, I can't remember exactly what happened but it was not great :-).
> I can give it a second try.

It really should not be hard.  The dynamic mappings already work fine
as m68k is using the generic code.  So the only thing you want is
to dip into the glonal pool for coherent allocations. For that you
need to select the DMA_GLOBAL_POOL config option and fill the pool
with dma_init_global_coherent().
Re: [PATCH 2/2] arch: m68k: Add DMA mapping operations for Coldfire M5441x
Posted by Jean-Michel Hautbois 10 months, 3 weeks ago
On 28/01/2025 07:40, Christoph Hellwig wrote:
> On Tue, Jan 28, 2025 at 07:33:37AM +0100, Jean-Michel Hautbois wrote:
>>> Arch dma code should not implement DMA OPS, but use the generic
>>> dma-direct code with the right helpes for handling coherency.  This
>>> variant looks like it should be using the DMA_COHERENT_POOL allocator
>>> for dma coherent memory.
>>
>> Thanks for your answer.
>> I am not sure to understand it though :-) because I can see a few
>> dma_map_ops implementations in arch code.
> 
> The last one left for the direct mapping are arm32 and parisc, and
> they should go away eventually.
> 
>> I tried to let dma_direct do the
>> work, I can't remember exactly what happened but it was not great :-).
>> I can give it a second try.
> 
> It really should not be hard.  The dynamic mappings already work fine
> as m68k is using the generic code.  So the only thing you want is
> to dip into the glonal pool for coherent allocations. For that you
> need to select the DMA_GLOBAL_POOL config option and fill the pool
> with dma_init_global_coherent().
> 

Isn't it done in patch 1/2 ? Or did I miss something ?

JM
Re: [PATCH 2/2] arch: m68k: Add DMA mapping operations for Coldfire M5441x
Posted by Christoph Hellwig 10 months, 3 weeks ago
On Tue, Jan 28, 2025 at 07:43:23AM +0100, Jean-Michel Hautbois wrote:
> > It really should not be hard.  The dynamic mappings already work fine
> > as m68k is using the generic code.  So the only thing you want is
> > to dip into the glonal pool for coherent allocations. For that you
> > need to select the DMA_GLOBAL_POOL config option and fill the pool
> > with dma_init_global_coherent().
> > 
> 
> Isn't it done in patch 1/2 ? Or did I miss something ?

The point is that this is all you need.  No need for new dma ops.
Re: [PATCH 2/2] arch: m68k: Add DMA mapping operations for Coldfire M5441x
Posted by Jean-Michel Hautbois 10 months, 3 weeks ago
Hi Christoph,

On 28/01/2025 07:47, Christoph Hellwig wrote:
> On Tue, Jan 28, 2025 at 07:43:23AM +0100, Jean-Michel Hautbois wrote:
>>> It really should not be hard.  The dynamic mappings already work fine
>>> as m68k is using the generic code.  So the only thing you want is
>>> to dip into the glonal pool for coherent allocations. For that you
>>> need to select the DMA_GLOBAL_POOL config option and fill the pool
>>> with dma_init_global_coherent().
>>>
>>
>> Isn't it done in patch 1/2 ? Or did I miss something ?
> 
> The point is that this is all you need.  No need for new dma ops.
> 

Indeed, I tested it and it works fine.
I have removed the test in fec_main to be certain, but I think it would 
be better to change the COLDFIRE_COHERENT_DMA conditional ?
What sounds like the most efficient way ?
I can' do:

- #if defined(CONFIG_COLDFIRE) && !defined(CONFIG_COLDFIRE_COHERENT_DMA)
+ #if defined(CONFIG_COLDFIRE) && !defined(CONFIG_COLDFIRE_COHERENT_DMA) 
&& !defined(CONFIG_DMA_GLOBAL_POOL)

I suppose it would be better to modify it in the Kconfig, but I can't 
find a nice way to do it without having a circular dependency:
arch/m68k/Kconfig.cpu
@@ -555,6 +555,7 @@ config COLDFIRE_COHERENT_DMA
         default y
         depends on COLDFIRE
         depends on !HAVE_CACHE_CB && !CACHE_D && !CACHE_BOTH
+       depends on DMA_GLOBAL_POOL

=>
error: recursive dependency detected!
         symbol DMA_DIRECT_REMAP is selected by M68K_NONCOHERENT_DMA
         symbol M68K_NONCOHERENT_DMA depends on COLDFIRE_COHERENT_DMA
         symbol COLDFIRE_COHERENT_DMA depends on DMA_GLOBAL_POOL
         symbol DMA_GLOBAL_POOL depends on DMA_DIRECT_REMAP


I will send a v2 with the global pool.

Thanks !
JM