Skip to contentSkip to conversation

dmaengine: Add Arm DMA-350 driver

PATCH Superseded Robin Murphy 4 participants 10 messages Feb 28, 2025 – Mar 12, 2025

Subsystems (3)
Device tree bindings dmaengine Arm/nuvoton MA35 architecture

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Robin Murphy

author
#0 lore raw

cover dmaengine: Add Arm DMA-350 driver

Hi Vinod,

As it says on the tin, here's an initial driver for the Arm DMA-350[1]. This has the intentions of growing into a fully-featured driver, but so far my only development platform has been an Arm FVP (software model) which doesn't emulate peripheral requests, so this is as far as I've been able to get with dmatest alone. However, I figure it's enough to be worth posting for review, at the very least.

Thanks,
Robin.

[1] https://developer.arm.com/Processors/CoreLink%20DMA-350

Robin Murphy (2):
dt-bindings: dma: Add Arm DMA-350
dmaengine: Add Arm DMA-350 driver

 .../devicetree/bindings/dma/arm,dma-350.yaml  |  44 ++
 drivers/dma/Kconfig                           |   7 +
 drivers/dma/Makefile                          |   1 +
 drivers/dma/arm-dma350.c                      | 659 ++++++++++++++++++
 4 files changed, 711 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dma/arm,dma-350.yaml
 create mode 100644 drivers/dma/arm-dma350.c
Signature · 1 line
2.39.2.101.g768bb238c484.dirty
author
#1 lore raw

PATCH 1/2 dt-bindings: dma: Add Arm DMA-350

Arm CoreLink DMA-350 is a pleasantly straightforward DMA controller which, although highly configurable, lends itself to a simple binding thanks to plenty of self-describing ID registers.

Signed-off-by Robin Murphy <[email protected]>
---
 .../devicetree/bindings/dma/arm,dma-350.yaml  | 44 +++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dma/arm,dma-350.yaml
Documentation/devicetree/bindings/dma/arm,dma-350.yaml new file +44 -0
new file mode 100644
index 000000000000..429f682f15d8
@@ -0,0 +1,44 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/dma/arm,dma-350.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Arm CoreLink DMA-350 Controller
+
+maintainers:
+  - Robin Murphy <[email protected]>
+
+allOf:
+  - $ref: dma-controller.yaml#
+
+properties:
+  compatible:
+    const: arm,dma-350
+
+  reg:
+    items:
+      - description: Base and size of the full register map
+
+  interrupts:
+    minItems: 1
+    items:
+      - description: Channel 0 interrupt
+      - description: Channel 1 interrupt
+      - description: Channel 2 interrupt
+      - description: Channel 3 interrupt
+      - description: Channel 4 interrupt
+      - description: Channel 5 interrupt
+      - description: Channel 6 interrupt
+      - description: Channel 7 interrupt
+
+  "#dma-cells":
+    const: 1
+    description: The cell is the trigger input number
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+unevaluatedProperties: false
Signature · 1 line
2.39.2.101.g768bb238c484.dirty
maintainer Reviewed-by on v1 1/2
#2 lore raw

Re: [PATCH 1/2] dt-bindings: dma: Add Arm DMA-350

Quoted 10 lines · Robin Murphy

Arm CoreLink DMA-350 is a pleasantly straightforward DMA controller which, although highly configurable, lends itself to a simple binding thanks to plenty of self-describing ID registers.

Signed-off-by Robin Murphy <[email protected]>
---
 .../devicetree/bindings/dma/arm,dma-350.yaml  | 44 +++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dma/arm,dma-350.yaml
Reviewed-by Rob Herring (Arm) <[email protected]>
author
#3 lore raw

PATCH 2/2 dmaengine: Add Arm DMA-350 driver

In reply to Robin Murphy

Add an initial driver for the Arm Corelink DMA-350 controller, to support basic mem-to-mem async_tx. The design here leaves room for more fun things like peripheral support and scatter-gather chaining to come in future.

Signed-off-by Robin Murphy <[email protected]>
---
 drivers/dma/Kconfig      |   7 +
 drivers/dma/Makefile     |   1 +
 drivers/dma/arm-dma350.c | 659 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 667 insertions(+)
 create mode 100644 drivers/dma/arm-dma350.c
drivers/dma/Kconfig +7 -0
index 8afea2e23360..fa7d7f98e50f 100644
@@ -93,6 +93,13 @@ config APPLE_ADMAC
 	help
 	  Enable support for Audio DMA Controller found on Apple Silicon SoCs.
 
+config ARM_DMA350
+	tristate "Arm DMA-350 support"
+	select DMA_ENGINE
+	select DMA_VIRTUAL_CHANNELS
+	help
+	  Enable support for the Arm DMA-350 controller.
+
 config AT_HDMAC
 	tristate "Atmel AHB DMA support"
 	depends on ARCH_AT91
drivers/dma/Makefile +1 -0
index 19ba465011a6..ba9732644752 100644
@@ -17,6 +17,7 @@ obj-$(CONFIG_ALTERA_MSGDMA) += altera-msgdma.o
 obj-$(CONFIG_AMBA_PL08X) += amba-pl08x.o
 obj-$(CONFIG_AMCC_PPC440SPE_ADMA) += ppc4xx/
 obj-$(CONFIG_APPLE_ADMAC) += apple-admac.o
+obj-$(CONFIG_ARM_DMA350) += arm-dma350.o
 obj-$(CONFIG_AT_HDMAC) += at_hdmac.o
 obj-$(CONFIG_AT_XDMAC) += at_xdmac.o
 obj-$(CONFIG_AXI_DMAC) += dma-axi-dmac.o
drivers/dma/arm-dma350.c new file +659 -0

659 lines not on this page · raw message

Signature · 1 line
2.39.2.101.g768bb238c484.dirty
build bot
#4 lore raw
kernel test robot noticed the following build warnings:

Hi Robin,

kernel test robot noticed the following build warnings:

[auto build test WARNING on vkoul-dmaengine/next]
[also build test WARNING on linus/master v6.14-rc4 next-20250228] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Robin-Murphy/dt-bindings-dma-Add-Arm-DMA-350/20250301-012733
base:   https://git.kernel.org/pub/scm/linux/kernel/git/vkoul/dmaengine.git next
patch link:    https://lore.kernel.org/r/55e084dd2b5720bdddf503ffac560d111032aa96.1740762136.git.robin.murphy%40arm.com
patch subject: [PATCH 2/2] dmaengine: Add Arm DMA-350 driver
config: hexagon-randconfig-001-20250302 (https://download.01.org/0day-ci/archive/20250302/[email protected]/config)
compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project 14170b16028c087ca154878f5ed93d3089a965c6)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250302/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

>> drivers/dma/arm-dma350.c:485:13: warning: logical not is only applied to the left hand side of this bitwise operator [-Wlogical-not-parentheses]
     485 |         } else if (!ch_status & CH_STAT_INTR_DONE) {
         |                    ^          ~
   drivers/dma/arm-dma350.c:485:13: note: add parentheses after the '!' to evaluate the bitwise operator first
     485 |         } else if (!ch_status & CH_STAT_INTR_DONE) {
         |                    ^                             
         |                     (                            )
   drivers/dma/arm-dma350.c:485:13: note: add parentheses around left hand side expression to silence this warning
     485 |         } else if (!ch_status & CH_STAT_INTR_DONE) {
         |                    ^
         |                    (         )

1 warning generated.

vim +485 drivers/dma/arm-dma350.c

462
463 static irqreturn_t d350_irq(int irq, void *data)
464 {
465 struct d350_chan *dch = data;
466 struct device *dev = dch->vc.chan.device->dev;
467 struct virt_dma_desc *vd = &dch->desc->vd;
468 u32 ch_status;
469
470 ch_status = readl(dch->base + CH_STATUS);
471 if (!ch_status)
472 return IRQ_NONE;
473
474 if (ch_status & CH_STAT_INTR_ERR) {
475 u32 errinfo = readl_relaxed(dch->base + CH_ERRINFO);
476
477 if (errinfo & (CH_ERRINFO_AXIRDPOISERR | CH_ERRINFO_AXIRDRESPERR))
478 vd->tx_result.result = DMA_TRANS_READ_FAILED;
479 else if (errinfo & CH_ERRINFO_AXIWRRESPERR)
480 vd->tx_result.result = DMA_TRANS_WRITE_FAILED;
481 else
482 vd->tx_result.result = DMA_TRANS_ABORTED;
483
484 vd->tx_result.residue = d350_get_residue(dch);

 > 485		} else if (!ch_status & CH_STAT_INTR_DONE) {

486 dev_warn(dev, "Unexpected IRQ source? 0x%08x\n", ch_status);
487 }
488 writel_relaxed(ch_status, dch->base + CH_STATUS);
489
490 spin_lock(&dch->vc.lock);
491 vchan_cookie_complete(vd);
492 if (ch_status & CH_STAT_INTR_DONE) {
493 dch->status = DMA_COMPLETE;
494 dch->residue = 0;
495 d350_start_next(dch);
496 } else {
497 dch->status = DMA_ERROR;
498 dch->residue = vd->tx_result.residue;
499 }
500 spin_unlock(&dch->vc.lock);
501
502 return IRQ_HANDLED;
503 }
504

Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
build bot
#5 lore raw
In reply to Robin Murphy
kernel test robot noticed the following build warnings:

Hi Robin,

kernel test robot noticed the following build warnings:

[auto build test WARNING on vkoul-dmaengine/next]
[also build test WARNING on linus/master v6.14-rc4 next-20250228] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Robin-Murphy/dt-bindings-dma-Add-Arm-DMA-350/20250301-012733
base:   https://git.kernel.org/pub/scm/linux/kernel/git/vkoul/dmaengine.git next
patch link:    https://lore.kernel.org/r/55e084dd2b5720bdddf503ffac560d111032aa96.1740762136.git.robin.murphy%40arm.com
patch subject: [PATCH 2/2] dmaengine: Add Arm DMA-350 driver
config: parisc-randconfig-002-20250302 (https://download.01.org/0day-ci/archive/20250302/[email protected]/config)
compiler: hppa-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250302/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

>> drivers/dma/arm-dma350.c:641:34: warning: 'd350_of_match' defined but not used [-Wunused-const-variable=]
     641 | static const struct of_device_id d350_of_match[] = {
         |                                  ^~~~~~~~~~~~~

vim +/d350_of_match +641 drivers/dma/arm-dma350.c

640

 > 641	static const struct of_device_id d350_of_match[] = {

642 { .compatible = "arm,dma-350" },
643 {}
644 };
645 MODULE_DEVICE_TABLE(of, d350_of_match);
646

Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
build bot
#6 lore raw
In reply to Robin Murphy
kernel test robot noticed the following build warnings:

Hi Robin,

kernel test robot noticed the following build warnings:

[auto build test WARNING on vkoul-dmaengine/next]
[also build test WARNING on linus/master v6.14-rc5 next-20250305] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Robin-Murphy/dt-bindings-dma-Add-Arm-DMA-350/20250301-012733
base:   https://git.kernel.org/pub/scm/linux/kernel/git/vkoul/dmaengine.git next
patch link:    https://lore.kernel.org/r/55e084dd2b5720bdddf503ffac560d111032aa96.1740762136.git.robin.murphy%40arm.com
patch subject: [PATCH 2/2] dmaengine: Add Arm DMA-350 driver
config: nios2-randconfig-r111-20250306 (https://download.01.org/0day-ci/archive/20250306/[email protected]/config)
compiler: nios2-linux-gcc (GCC) 14.2.0
reproduce: (https://download.01.org/0day-ci/archive/20250306/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

sparse warnings: (new ones prefixed by >>)

>> drivers/dma/arm-dma350.c:485:31: sparse: sparse: dubious: !x & y

drivers/dma/arm-dma350.c: note: in included file (through include/linux/smp.h, include/linux/lockdep.h, include/linux/spinlock.h, ...):
include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true

vim +485 drivers/dma/arm-dma350.c

462
463 static irqreturn_t d350_irq(int irq, void *data)
464 {
465 struct d350_chan *dch = data;
466 struct device *dev = dch->vc.chan.device->dev;
467 struct virt_dma_desc *vd = &dch->desc->vd;
468 u32 ch_status;
469
470 ch_status = readl(dch->base + CH_STATUS);
471 if (!ch_status)
472 return IRQ_NONE;
473
474 if (ch_status & CH_STAT_INTR_ERR) {
475 u32 errinfo = readl_relaxed(dch->base + CH_ERRINFO);
476
477 if (errinfo & (CH_ERRINFO_AXIRDPOISERR | CH_ERRINFO_AXIRDRESPERR))
478 vd->tx_result.result = DMA_TRANS_READ_FAILED;
479 else if (errinfo & CH_ERRINFO_AXIWRRESPERR)
480 vd->tx_result.result = DMA_TRANS_WRITE_FAILED;
481 else
482 vd->tx_result.result = DMA_TRANS_ABORTED;
483
484 vd->tx_result.residue = d350_get_residue(dch);

 > 485		} else if (!ch_status & CH_STAT_INTR_DONE) {

486 dev_warn(dev, "Unexpected IRQ source? 0x%08x\n", ch_status);
487 }
488 writel_relaxed(ch_status, dch->base + CH_STATUS);
489
490 spin_lock(&dch->vc.lock);
491 vchan_cookie_complete(vd);
492 if (ch_status & CH_STAT_INTR_DONE) {
493 dch->status = DMA_COMPLETE;
494 dch->residue = 0;
495 d350_start_next(dch);
496 } else {
497 dch->status = DMA_ERROR;
498 dch->residue = vd->tx_result.residue;
499 }
500 spin_unlock(&dch->vc.lock);
501
502 return IRQ_HANDLED;
503 }
504

Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
maintainer
#7 lore raw
In reply to Robin Murphy

drivers/dma/arm-dma350.c line 328

Quoted 10 lines · Robin Murphy
drivers/dma/arm-dma350.c new file +10 -0
+static u32 d350_get_residue(struct d350_chan *dch)
+{
+	u32 res, xsize, xsizehi, hi_new;
+
+	hi_new = readl_relaxed(dch->base + CH_XSIZEHI);
+	do {
+		xsizehi = hi_new;
+		xsize = readl_relaxed(dch->base + CH_XSIZE);
+		hi_new = readl_relaxed(dch->base + CH_XSIZEHI);
+	} while (xsizehi != hi_new);

This can go forever, lets have some limits to this loop please

drivers/dma/arm-dma350.c line 509

Quoted 5 lines
drivers/dma/arm-dma350.c new file +5 -0
+static int d350_alloc_chan_resources(struct dma_chan *chan)
+{
+	struct d350_chan *dch = to_d350_chan(chan);
+	int ret = request_irq(dch->irq, d350_irq, IRQF_SHARED,
+			      dev_name(&dch->vc.chan.dev->device), dch);

This is interesting, any reason why the irq is allocated here? Would it be not better to do that in probe...

Signature · 1 line
~Vinod
author
#8 lore raw
Quoted 14 lines · Vinod Koul

drivers/dma/arm-dma350.c line 328

Quoted 10 lines · Robin Murphy
drivers/dma/arm-dma350.c new file +10 -0
+static u32 d350_get_residue(struct d350_chan *dch)
+{
+	u32 res, xsize, xsizehi, hi_new;
+
+	hi_new = readl_relaxed(dch->base + CH_XSIZEHI);
+	do {
+		xsizehi = hi_new;
+		xsize = readl_relaxed(dch->base + CH_XSIZE);
+		hi_new = readl_relaxed(dch->base + CH_XSIZEHI);
+	} while (xsizehi != hi_new);

This can go forever, lets have some limits to this loop please

Sure, in practice I doubt we're ever going to be continually preempted faster than the controller can move another 64KB of data, but I concur there's no harm in making the code easier to reason about at a glance either.

Quoted 8 lines

drivers/dma/arm-dma350.c line 509

Quoted 5 lines
drivers/dma/arm-dma350.c new file +5 -0
+static int d350_alloc_chan_resources(struct dma_chan *chan)
+{
+	struct d350_chan *dch = to_d350_chan(chan);
+	int ret = request_irq(dch->irq, d350_irq, IRQF_SHARED,
+			      dev_name(&dch->vc.chan.dev->device), dch);

This is interesting, any reason why the irq is allocated here? Would it be not better to do that in probe...

Well, I'd say technically the IRQ is a channel resource, and quite a few other drivers do the same... Here it's mostly so I can get the channel name - so the IRQs are nice and identifiable in /proc/interrupts - easily without making a big mess in probe, since the names don't exist until after the device is registered.

Thanks,
Robin.

maintainer
#9 lore raw
Quoted 16 lines · Robin Murphy
Quoted 14 lines · Vinod Koul

drivers/dma/arm-dma350.c line 328

Quoted 10 lines · Robin Murphy
drivers/dma/arm-dma350.c new file +10 -0
+static u32 d350_get_residue(struct d350_chan *dch)
+{
+	u32 res, xsize, xsizehi, hi_new;
+
+	hi_new = readl_relaxed(dch->base + CH_XSIZEHI);
+	do {
+		xsizehi = hi_new;
+		xsize = readl_relaxed(dch->base + CH_XSIZE);
+		hi_new = readl_relaxed(dch->base + CH_XSIZEHI);
+	} while (xsizehi != hi_new);

This can go forever, lets have some limits to this loop please

Quoted 4 lines

Sure, in practice I doubt we're ever going to be continually preempted faster than the controller can move another 64KB of data, but I concur there's no harm in making the code easier to reason about at a glance either.

Yes you are coreect but when things go bad, a bug in h/w or something, I would like to see a fail safe

Quoted 9 lines
Quoted 8 lines

drivers/dma/arm-dma350.c line 509

Quoted 5 lines
drivers/dma/arm-dma350.c new file +5 -0
+static int d350_alloc_chan_resources(struct dma_chan *chan)
+{
+	struct d350_chan *dch = to_d350_chan(chan);
+	int ret = request_irq(dch->irq, d350_irq, IRQF_SHARED,
+			      dev_name(&dch->vc.chan.dev->device), dch);

This is interesting, any reason why the irq is allocated here? Would it be not better to do that in probe...

Quoted 5 lines

Well, I'd say technically the IRQ is a channel resource, and quite a few other drivers do the same... Here it's mostly so I can get the channel name
- so the IRQs are nice and identifiable in /proc/interrupts - easily without making a big mess in probe, since the names don't exist until after the device is registered.

Ok

Signature · 1 line
~Vinod