[PATCH 0/2] dmaengine: ma35d1-pdma: Add MA35D1 PDMA driver

[PATCH 0/2] dmaengine: ma35d1-pdma: Add MA35D1 PDMA driver

[Shan-Chun Hung]

This series adds the PDMA controller driver and DT binding for the
Nuvoton MA35D1 SoC.

The driver supports memcpy, slave scatter-gather and cyclic transfers,
and a per-channel timeout configured through
dma_slave_config.peripheral_config. The timeout is needed by the
MA35D1 UART driver, to be submitted separately.

Tested on the MA35D1 SOM board with Linux v7.4-rc1, using local
(not yet submitted) QSPI and UART patches that use this PDMA driver.

Signed-off-by: Shan-Chun Hung <shanchun1218@gmail.com>

Shan-Chun Hung (2):
  dt-bindings: dma: nuvoton,ma35d1-pdma: Document MA35D1 PDMA controller
  dmaengine: ma35d1-pdma: Add MA35D1 PDMA driver

 .../bindings/dma/nuvoton,ma35d1-pdma.yaml     |  72 ++
 drivers/dma/Kconfig                           |  12 +
 drivers/dma/Makefile                          |   1 +
 drivers/dma/ma35d1-pdma.c                     | 780 ++++++++++++++++++
 include/dt-bindings/dma/ma35d1-pdma.h         | 113 +++
 include/linux/dma/ma35d1-pdma.h               |  24 +
 6 files changed, 1002 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dma/nuvoton,ma35d1-pdma.yaml
 create mode 100644 drivers/dma/ma35d1-pdma.c
 create mode 100644 include/dt-bindings/dma/ma35d1-pdma.h
 create mode 100644 include/linux/dma/ma35d1-pdma.h

--
2.25.1

[PATCH 1/2] dt-bindings: dma: nuvoton,ma35d1-pdma: Document MA35D1 PDMA controller

[Shan-Chun Hung]

Add binding for Nuvoton MA35D1 PDMA controller.

Signed-off-by: Shan-Chun Hung <shanchun1218@gmail.com>
---
 .../bindings/dma/nuvoton,ma35d1-pdma.yaml     |  72 +++++++++++
 include/dt-bindings/dma/ma35d1-pdma.h         | 113 ++++++++++++++++++
 2 files changed, 185 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dma/nuvoton,ma35d1-pdma.yaml
 create mode 100644 include/dt-bindings/dma/ma35d1-pdma.h

diff --git a/Documentation/devicetree/bindings/dma/nuvoton,ma35d1-pdma.yaml b/Documentation/devicetree/bindings/dma/nuvoton,ma35d1-pdma.yaml
new file mode 100644
index 000000000000..cd752571242c
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/nuvoton,ma35d1-pdma.yaml
@@ -0,0 +1,72 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/dma/nuvoton,ma35d1-pdma.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Nuvoton MA35D1 PDMA controller
+
+description:
+  This describes the devicetree bindings for four Peripheral DMA engines to perform
+  memory-to-memory, memory-to-peripheral, or peripheral-to-memory data transfer
+  capable of supporting 10 channels, implemented in Nuvoton MA35D1 SoCs.
+
+maintainers:
+  - Shan-Chun Hung <shanchun1218@gmail.com>
+
+allOf:
+  - $ref: dma-controller.yaml#
+
+properties:
+  compatible:
+    const: nuvoton,ma35d1-pdma
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  "#dma-cells":
+    const: 2
+    description:
+      The first cell is the channel number. The second cell is the
+      peripheral request source (REQSRC) selection, which defines which
+      peripheral is connected to that PDMA channel.
+
+  dma-channels:
+    const: 10
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - interrupts
+  - "#dma-cells"
+  - dma-channels
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
+
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        dma-controller@40080000 {
+            compatible = "nuvoton,ma35d1-pdma";
+            reg = <0x0 0x40080000 0x0 0x2000>;
+            interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
+            clocks = <&clk PDMA0_GATE>;
+            #dma-cells = <2>;
+            dma-channels = <10>;
+        };
+    };
+
+...
diff --git a/include/dt-bindings/dma/ma35d1-pdma.h b/include/dt-bindings/dma/ma35d1-pdma.h
new file mode 100644
index 000000000000..0ddc99d29661
--- /dev/null
+++ b/include/dt-bindings/dma/ma35d1-pdma.h
@@ -0,0 +1,113 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (C) 2026 Nuvoton Technology Corp.
+ */
+
+#ifndef __DT_BINDINGS_DMA_MA35D1_PDMA_H__
+#define __DT_BINDINGS_DMA_MA35D1_PDMA_H__
+
+#define MA35D1_PDMA_MEM			0
+#define MA35D1_PDMA_UART0_TX		4
+#define MA35D1_PDMA_UART0_RX		5
+#define MA35D1_PDMA_UART1_TX		6
+#define MA35D1_PDMA_UART1_RX		7
+#define MA35D1_PDMA_UART2_TX		8
+#define MA35D1_PDMA_UART2_RX		9
+#define MA35D1_PDMA_UART3_TX		10
+#define MA35D1_PDMA_UART3_RX		11
+#define MA35D1_PDMA_UART4_TX		12
+#define MA35D1_PDMA_UART4_RX		13
+#define MA35D1_PDMA_UART5_TX		14
+#define MA35D1_PDMA_UART5_RX		15
+#define MA35D1_PDMA_UART6_TX		16
+#define MA35D1_PDMA_UART6_RX		17
+#define MA35D1_PDMA_UART7_TX		18
+#define MA35D1_PDMA_UART7_RX		19
+#define MA35D1_PDMA_UART8_TX		20
+#define MA35D1_PDMA_UART8_RX		21
+#define MA35D1_PDMA_UART9_TX		22
+#define MA35D1_PDMA_UART9_RX		23
+#define MA35D1_PDMA_UART10_TX		24
+#define MA35D1_PDMA_UART10_RX		25
+#define MA35D1_PDMA_UART11_TX		26
+#define MA35D1_PDMA_UART11_RX		27
+#define MA35D1_PDMA_UART12_TX		28
+#define MA35D1_PDMA_UART12_RX		29
+#define MA35D1_PDMA_UART13_TX		30
+#define MA35D1_PDMA_UART13_RX		31
+#define MA35D1_PDMA_UART14_TX		32
+#define MA35D1_PDMA_UART14_RX		33
+#define MA35D1_PDMA_UART15_TX		34
+#define MA35D1_PDMA_UART15_RX		35
+#define MA35D1_PDMA_UART16_TX		36
+#define MA35D1_PDMA_UART16_RX		37
+#define MA35D1_PDMA_QSPI0_TX		54
+#define MA35D1_PDMA_QSPI0_RX		55
+#define MA35D1_PDMA_QSPI1_TX		56
+#define MA35D1_PDMA_QSPI1_RX		57
+#define MA35D1_PDMA_SPI0_TX		60
+#define MA35D1_PDMA_SPI0_RX		61
+#define MA35D1_PDMA_SPI1_TX		62
+#define MA35D1_PDMA_SPI1_RX		63
+#define MA35D1_PDMA_SPI2_TX		64
+#define MA35D1_PDMA_SPI2_RX		65
+#define MA35D1_PDMA_SPI3_TX		66
+#define MA35D1_PDMA_SPI3_RX		67
+#define MA35D1_PDMA_I2C0_TX		70
+#define MA35D1_PDMA_I2C0_RX		71
+#define MA35D1_PDMA_I2C1_TX		72
+#define MA35D1_PDMA_I2C1_RX		73
+#define MA35D1_PDMA_I2C2_TX		74
+#define MA35D1_PDMA_I2C2_RX		75
+#define MA35D1_PDMA_I2C3_TX		76
+#define MA35D1_PDMA_I2C3_RX		77
+#define MA35D1_PDMA_I2C4_TX		78
+#define MA35D1_PDMA_I2C4_RX		79
+#define MA35D1_PDMA_I2C5_TX		80
+#define MA35D1_PDMA_I2C5_RX		81
+#define MA35D1_PDMA_I2S0_TX		86
+#define MA35D1_PDMA_I2S0_RX		87
+#define MA35D1_PDMA_I2S1_TX		88
+#define MA35D1_PDMA_I2S1_RX		89
+#define MA35D1_PDMA_TMR0		94
+#define MA35D1_PDMA_TMR1		95
+#define MA35D1_PDMA_TMR2		96
+#define MA35D1_PDMA_TMR3		97
+#define MA35D1_PDMA_TMR4		98
+#define MA35D1_PDMA_TMR5		99
+#define MA35D1_PDMA_TMR6		100
+#define MA35D1_PDMA_TMR7		101
+#define MA35D1_PDMA_TMR8		102
+#define MA35D1_PDMA_TMR9		103
+#define MA35D1_PDMA_TMR10		104
+#define MA35D1_PDMA_TMR11		105
+#define MA35D1_PDMA_EPWM0_P1_RX		110
+#define MA35D1_PDMA_EPWM0_P2_RX		111
+#define MA35D1_PDMA_EPWM0_P3_RX		112
+#define MA35D1_PDMA_EPWM1_P1_RX		113
+#define MA35D1_PDMA_EPWM1_P2_RX		114
+#define MA35D1_PDMA_EPWM1_P3_RX		115
+#define MA35D1_PDMA_EPWM2_P1_RX		116
+#define MA35D1_PDMA_EPWM2_P2_RX		117
+#define MA35D1_PDMA_EPWM2_P3_RX		118
+#define MA35D1_PDMA_EPWM0_CH0_TX	122
+#define MA35D1_PDMA_EPWM0_CH1_TX	123
+#define MA35D1_PDMA_EPWM0_CH2_TX	124
+#define MA35D1_PDMA_EPWM0_CH3_TX	125
+#define MA35D1_PDMA_EPWM0_CH4_TX	126
+#define MA35D1_PDMA_EPWM0_CH5_TX	127
+#define MA35D1_PDMA_EPWM1_CH0_TX	128
+#define MA35D1_PDMA_EPWM1_CH1_TX	129
+#define MA35D1_PDMA_EPWM1_CH2_TX	130
+#define MA35D1_PDMA_EPWM1_CH3_TX	131
+#define MA35D1_PDMA_EPWM1_CH4_TX	132
+#define MA35D1_PDMA_EPWM1_CH5_TX	133
+#define MA35D1_PDMA_EPWM2_CH0_TX	134
+#define MA35D1_PDMA_EPWM2_CH1_TX	135
+#define MA35D1_PDMA_EPWM2_CH2_TX	136
+#define MA35D1_PDMA_EPWM2_CH3_TX	137
+#define MA35D1_PDMA_EPWM2_CH4_TX	138
+#define MA35D1_PDMA_EPWM2_CH5_TX	139
+#define MA35D1_PDMA_EADC0_RX		146
+
+#endif /* __DT_BINDINGS_DMA_MA35D1_PDMA_H__ */
--
2.25.1

[PATCH 2/2] dmaengine: ma35d1-pdma: Add MA35D1 PDMA driver

[Shan-Chun Hung]

Add a driver for the PDMA controller found in the Nuvoton MA35D1 SoC.
It supports up to 10 channels with memory-to-memory, slave scatter-gather
and cyclic transfers, and a per-channel timeout for variable-length
peripheral RX.

Signed-off-by: Shan-Chun Hung <shanchun1218@gmail.com>
---
 drivers/dma/Kconfig             |  12 +
 drivers/dma/Makefile            |   1 +
 drivers/dma/ma35d1-pdma.c       | 780 ++++++++++++++++++++++++++++++++
 include/linux/dma/ma35d1-pdma.h |  24 +
 4 files changed, 817 insertions(+)
 create mode 100644 drivers/dma/ma35d1-pdma.c
 create mode 100644 include/linux/dma/ma35d1-pdma.h

diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 01f96a8257e5..3d4d09e459f1 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -404,6 +404,18 @@ config LPC32XX_DMAMUX
 	  Support for PL080 multiplexed DMA request lines on
 	  LPC32XX platrofm.

+config MA35D1_PDMA
+	tristate "Nuvoton MA35D1 PDMA Engine"
+	depends on ARCH_MA35 || COMPILE_TEST
+	select DMA_ENGINE
+	select DMA_VIRTUAL_CHANNELS
+	help
+	  Support the Nuvoton MA35D1 PDMA engine. This engine is integrated
+	  into Nuvoton MA35D1 chips. This DMA controller can transfer data
+	  from memory to memory, memory to peripheral and peripheral to memory.
+	  If you have a board based on such a SoC and wish to use DMA, say Y
+	  or M here.
+
 config MCF_EDMA
 	tristate "Freescale eDMA engine support, ColdFire mcf5441x SoCs"
 	depends on M5441x || (COMPILE_TEST && FSL_EDMA=n)
diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
index f62d12b08e15..dd99f7d12d4d 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile
@@ -52,6 +52,7 @@ obj-y += idxd/
 obj-$(CONFIG_K3_DMA) += k3dma.o
 obj-$(CONFIG_LPC18XX_DMAMUX) += lpc18xx-dmamux.o
 obj-$(CONFIG_LPC32XX_DMAMUX) += lpc32xx-dmamux.o
+obj-$(CONFIG_MA35D1_PDMA) += ma35d1-pdma.o
 obj-$(CONFIG_MILBEAUT_HDMAC) += milbeaut-hdmac.o
 obj-$(CONFIG_MILBEAUT_XDMAC) += milbeaut-xdmac.o
 obj-$(CONFIG_MMP_PDMA) += mmp_pdma.o
diff --git a/drivers/dma/ma35d1-pdma.c b/drivers/dma/ma35d1-pdma.c
new file mode 100644
index 000000000000..176bc56c8063
--- /dev/null
+++ b/drivers/dma/ma35d1-pdma.c
@@ -0,0 +1,780 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Nuvoton MA35D1 PDMA controller driver
+ *
+ * Copyright (C) 2026 Nuvoton Technology Corp.
+ *
+ * Author: Shan-Chun Hung <shanchun1218@gmail.com>
+ */
+
+#include <linux/align.h>
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/compiler_types.h>
+#include <linux/container_of.h>
+#include <linux/dev_printk.h>
+#include <linux/device.h>
+#include <linux/dma-direction.h>
+#include <linux/dma-mapping.h>
+#include <linux/dma/ma35d1-pdma.h>
+#include <linux/dmaengine.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/gfp_types.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/irqreturn.h>
+#include <linux/limits.h>
+#include <linux/list.h>
+#include <linux/math.h>
+#include <linux/minmax.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_dma.h>
+#include <linux/overflow.h>
+#include <linux/platform_device.h>
+#include <linux/scatterlist.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/spinlock_types.h>
+#include <linux/stddef.h>
+#include <linux/string.h>
+#include <linux/types.h>
+
+#include "dmaengine.h"
+#include "virt-dma.h"
+
+/* PDMA registers */
+#define PDMA_DSCT_CTL		0x000
+#define PDMA_DSCT_SA		0x004
+#define PDMA_DSCT_DA		0x008
+#define PDMA_DSCT_NEXT		0x00c
+#define PDMA_CHCTL		0x400
+#define PDMA_SWREQ		0x408
+#define PDMA_INTEN		0x418
+#define PDMA_INTSTS		0x41C
+#define PDMA_ABTSTS		0x420
+#define PDMA_TDSTS		0x424
+#define PDMA_TOUTEN		0x434
+#define PDMA_TOUTIEN		0x438
+#define PDMA_TOC		0x440
+#define PDMA_CHRST		0x460
+#define PDMA_TOUTPSC		0x470
+#define PDMA_REQSEL		0x480
+
+/* Bitfields in PDMA registers */
+#define PDMA_OP_BASIC		BIT(0)
+#define PDMA_OP_SCATTER		BIT(1)
+#define PDMA_OP_MSK		GENMASK(1, 0)
+#define PDMA_TXTYPE		BIT(2)
+#define PDMA_TBINTDIS		BIT(7)
+#define PDMA_SAFIX		GENMASK(9, 8)
+#define PDMA_DAFIX		GENMASK(11, 10)
+
+#define PDMA_MAX_CHANS		10
+#define PDMA_MAX_CHAN_BEATS	(U16_MAX + 1)
+
+#define PDMA_WIDTH(w)		FIELD_PREP(GENMASK(13, 12), (w) >> 1)
+#define PDMA_TXCNT(cnt)		FIELD_PREP(GENMASK(31, 16), (cnt) - 1)
+#define PDMA_GET_TXCNT(ctrl)	FIELD_GET(GENMASK(31, 16), readl(ctrl))
+#define PDMA_CH_OFFSET(ch)	((ch) * 0x10)
+
+#define PDMA_SLAVE_BUSWIDTHS	(BIT(DMA_SLAVE_BUSWIDTH_1_BYTE)  | \
+				 BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) | \
+				 BIT(DMA_SLAVE_BUSWIDTH_4_BYTES))
+
+struct ma35d1_sg {
+	u32 ctl;
+	u32 src;
+	u32 dst;
+	u32 next;
+};
+
+struct ma35d1_desc {
+	struct ma35d1_chan *ch;
+	bool cyclic;
+	/* bytes per beat */
+	u32 width;
+	struct virt_dma_desc vd;
+	dma_addr_t dma_addr;
+	size_t dma_len;
+	u32 sg_len;
+	struct ma35d1_sg sg[] __counted_by(sg_len);
+};
+
+struct ma35d1_chan {
+	struct device *dev;
+	struct ma35d1_pdmadev *pdmadev;
+	struct virt_dma_chan vc;
+	void __iomem *base;
+	int id;
+	struct ma35d1_desc *desc;
+	struct dma_slave_config cfg;
+	struct ma35d1_peripheral pcfg;
+	/* second #dma-cells argument from devicetree */
+	u32 request;
+	/* set when the current descriptor completed with an abort */
+	bool error;
+	/* bytes left after a timeout */
+	u32 remain;
+};
+
+struct ma35d1_pdmadev {
+	struct dma_device ddev;
+	/* protects read-modify-write of shared controller registers */
+	spinlock_t lock;
+	int nr_chans;
+	struct ma35d1_chan channels[] __counted_by(nr_chans);
+};
+
+static inline struct ma35d1_chan *to_ma35d1_pdma_chan(struct dma_chan *c)
+{
+	return container_of(c, struct ma35d1_chan, vc.chan);
+}
+
+static inline struct ma35d1_desc *to_ma35d1_pdma_desc(struct virt_dma_desc *vd)
+{
+	return container_of(vd, struct ma35d1_desc, vd);
+}
+
+static inline void ma35d1_pdma_reg_set(struct ma35d1_chan *ch, u32 offset, u32 mask)
+{
+	writel(readl(ch->base + offset) | mask, ch->base + offset);
+}
+
+static inline void ma35d1_pdma_reg_clear(struct ma35d1_chan *ch, u32 offset, u32 mask)
+{
+	writel(readl(ch->base + offset) & ~mask, ch->base + offset);
+}
+
+static void ma35d1_pdma_desc_free(struct virt_dma_desc *vd)
+{
+	struct ma35d1_desc *d = to_ma35d1_pdma_desc(vd);
+
+	dma_unmap_single(d->ch->dev, d->dma_addr, d->dma_len, DMA_TO_DEVICE);
+	kfree(d);
+}
+
+static int ma35d1_terminate_all(struct dma_chan *chan)
+{
+	LIST_HEAD(head);
+	struct ma35d1_chan *ch = to_ma35d1_pdma_chan(chan);
+	unsigned long flags, pdmadev_flags;
+
+	spin_lock_irqsave(&ch->vc.lock, flags);
+
+	spin_lock_irqsave(&ch->pdmadev->lock, pdmadev_flags);
+	ma35d1_pdma_reg_clear(ch, PDMA_CHCTL, BIT(ch->id));
+	spin_unlock_irqrestore(&ch->pdmadev->lock, pdmadev_flags);
+
+	if (ch->desc) {
+		vchan_terminate_vdesc(&ch->desc->vd);
+		ch->desc = NULL;
+	}
+
+	vchan_get_all_descriptors(&ch->vc, &head);
+	spin_unlock_irqrestore(&ch->vc.lock, flags);
+
+	vchan_dma_desc_free_list(&ch->vc, &head);
+
+	return 0;
+}
+
+static void ma35d1_synchronize(struct dma_chan *chan)
+{
+	struct ma35d1_chan *ch = to_ma35d1_pdma_chan(chan);
+
+	vchan_synchronize(&ch->vc);
+}
+
+static void ma35d1_pdma_timeout(struct ma35d1_chan *ch)
+{
+	struct ma35d1_peripheral *pcfg = ch->cfg.peripheral_config;
+	u32 offset, shift;
+	unsigned long flags;
+
+	spin_lock_irqsave(&ch->pdmadev->lock, flags);
+	if (!pcfg || (pcfg->prescaler == 0 && pcfg->counter == 0)) {
+		ma35d1_pdma_reg_clear(ch, PDMA_TOUTIEN, BIT(ch->id));
+		ma35d1_pdma_reg_clear(ch, PDMA_TOUTEN, BIT(ch->id));
+		goto spin_unlock;
+	}
+
+	/*
+	 * Calculate the positions of the timeout prescaler's bit fields and registers,
+	 * then write them.
+	 */
+	offset = PDMA_TOUTPSC + 4 * (ch->id >> 3);
+	shift = (ch->id & GENMASK(2, 0)) * 4;
+	ma35d1_pdma_reg_clear(ch, offset, GENMASK(2, 0) << shift);
+	ma35d1_pdma_reg_set(ch, offset, pcfg->prescaler << shift);
+
+	/*
+	 * Calculate the positions of the timeout counter's bit fields and registers,
+	 * then write them.
+	 */
+	offset = PDMA_TOC + 4 * (ch->id >> 1);
+	shift = (ch->id & GENMASK(0, 0)) * 16;
+	ma35d1_pdma_reg_clear(ch, offset, GENMASK(15, 0) << shift);
+	ma35d1_pdma_reg_set(ch, offset, pcfg->counter << shift);
+
+	ma35d1_pdma_reg_set(ch, PDMA_TOUTEN, BIT(ch->id));
+	ma35d1_pdma_reg_set(ch, PDMA_TOUTIEN, BIT(ch->id));
+
+spin_unlock:
+	spin_unlock_irqrestore(&ch->pdmadev->lock, flags);
+}
+
+static void ma35d1_pdma_setup_channel(struct ma35d1_chan *ch)
+{
+	u32 offset, reg, shift;
+	unsigned long flags;
+
+	spin_lock_irqsave(&ch->pdmadev->lock, flags);
+
+	if ((readl(ch->base + PDMA_CH_OFFSET(ch->id)) & GENMASK(1, 0)) != 0) {
+		reg = readl(ch->base + PDMA_CHCTL);
+		writel(0, ch->base + PDMA_CH_OFFSET(ch->id));
+		ma35d1_pdma_reg_set(ch, PDMA_CHRST, BIT(ch->id));
+		writel(reg | BIT(ch->id), ch->base + PDMA_CHCTL);
+	} else {
+		writel(0, ch->base + PDMA_CH_OFFSET(ch->id));
+		ma35d1_pdma_reg_set(ch, PDMA_CHCTL, BIT(ch->id));
+	}
+	ma35d1_pdma_reg_set(ch, PDMA_INTEN, BIT(ch->id));
+
+	offset = PDMA_REQSEL + (ch->id & ~GENMASK(1, 0));
+	shift = (ch->id & GENMASK(1, 0)) * 8;
+	ma35d1_pdma_reg_clear(ch, offset, GENMASK(7, 0) << shift);
+	ma35d1_pdma_reg_set(ch, offset, ch->request << shift);
+
+	spin_unlock_irqrestore(&ch->pdmadev->lock, flags);
+}
+
+static int ma35d1_slave_config(struct dma_chan *chan, struct dma_slave_config *cfg)
+{
+	struct ma35d1_chan *ch = to_ma35d1_pdma_chan(chan);
+	struct ma35d1_peripheral *pcfg = cfg->peripheral_config;
+
+	memcpy(&ch->cfg, cfg, sizeof(*cfg));
+	ch->cfg.peripheral_config = NULL;
+
+	if (pcfg) {
+		if (cfg->peripheral_size != sizeof(*pcfg)) {
+			dev_err(ch->dev, "Invalid peripheral size %zu, expected %zu\n",
+				cfg->peripheral_size, sizeof(*pcfg));
+			return -EINVAL;
+		}
+
+		if (pcfg->counter > GENMASK(15, 0) || pcfg->prescaler > GENMASK(2, 0)) {
+			dev_err(ch->dev, "Maximum prescaler or counter exceeded\n");
+			return -EINVAL;
+		}
+
+		memcpy(&ch->pcfg, pcfg, sizeof(ch->pcfg));
+		ch->cfg.peripheral_config = &ch->pcfg;
+	}
+
+	return 0;
+}
+
+static int ma35d1_slave_width(struct ma35d1_chan *ch, enum dma_transfer_direction dir)
+{
+	enum dma_slave_buswidth width;
+
+	if (dir == DMA_MEM_TO_DEV) {
+		width = ch->cfg.dst_addr_width;
+	} else if (dir == DMA_DEV_TO_MEM) {
+		width = ch->cfg.src_addr_width;
+	} else {
+		dev_err(ch->dev, "invalid DMA direction %d\n", dir);
+		return -EINVAL;
+	}
+
+	if (width != DMA_SLAVE_BUSWIDTH_1_BYTE && width != DMA_SLAVE_BUSWIDTH_2_BYTES &&
+	    width != DMA_SLAVE_BUSWIDTH_4_BYTES) {
+		dev_err(ch->dev, "invalid DMA width %d\n", width);
+		return -EINVAL;
+	}
+
+	return width;
+}
+
+static struct ma35d1_desc *ma35d1_desc_alloc(struct ma35d1_chan *ch, u32 sg_len)
+{
+	struct ma35d1_desc *d;
+
+	d = kzalloc(struct_size(d, sg, sg_len), GFP_ATOMIC);
+	if (!d)
+		return NULL;
+
+	d->sg_len = sg_len;
+	d->dma_len = sizeof(*d->sg) * sg_len;
+	d->dma_addr = dma_map_single(ch->dev, d->sg, d->dma_len, DMA_TO_DEVICE);
+	if (dma_mapping_error(ch->dev, d->dma_addr)) {
+		dev_err(ch->dev, "dma mapping error for channel\n");
+		kfree(d);
+		return NULL;
+	}
+
+	writel(d->dma_addr, ch->base + PDMA_CH_OFFSET(ch->id) + PDMA_DSCT_NEXT);
+
+	return d;
+}
+
+static struct dma_async_tx_descriptor *
+ma35d1_desc_submit(struct ma35d1_chan *ch, struct ma35d1_desc *d, bool cyclic,
+		   unsigned long tx_flags)
+{
+	dma_sync_single_for_device(ch->dev, d->dma_addr, d->dma_len, DMA_TO_DEVICE);
+	d->cyclic = cyclic;
+	d->ch = ch;
+	ch->error = false;
+
+	return vchan_tx_prep(&ch->vc, &d->vd, tx_flags);
+}
+
+static struct dma_async_tx_descriptor *
+ma35d1_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dst, dma_addr_t src,
+		       size_t len, unsigned long tx_flags)
+{
+	int i;
+	size_t tlen, max_chunk;
+	struct ma35d1_chan *ch = to_ma35d1_pdma_chan(chan);
+	struct ma35d1_desc *d;
+	u32 sg_len, width = 1;
+
+	if (ch->request != 0) {
+		dev_err(ch->dev, "%s: channel is bound to a peripheral request\n", __func__);
+		return NULL;
+	}
+
+	if (!len) {
+		dev_err(ch->dev, "invalid memcpy length\n");
+		return NULL;
+	}
+
+	if (IS_ALIGNED(src, 4) && IS_ALIGNED(dst, 4) && IS_ALIGNED(len, 4))
+		width = 4;
+
+	max_chunk = (size_t)PDMA_MAX_CHAN_BEATS * width;
+
+	sg_len = DIV_ROUND_UP_ULL(len, max_chunk);
+	d = ma35d1_desc_alloc(ch, sg_len);
+	if (!d)
+		return NULL;
+	d->width = width;
+
+	ma35d1_pdma_setup_channel(ch);
+
+	for (i = 0; i < sg_len; i++) {
+		tlen = min_t(size_t, len, max_chunk);
+		d->sg[i].src = src;
+		d->sg[i].dst = dst;
+		d->sg[i].ctl = PDMA_WIDTH(width) | PDMA_TXCNT(tlen / width) | PDMA_OP_SCATTER;
+		d->sg[i].next = d->dma_addr + PDMA_CH_OFFSET(i + 1);
+		src += tlen;
+		dst += tlen;
+		len -= tlen;
+	}
+	d->sg[sg_len - 1].ctl &= ~(PDMA_OP_MSK | PDMA_TBINTDIS);
+	d->sg[sg_len - 1].ctl |= PDMA_OP_BASIC;
+
+	return ma35d1_desc_submit(ch, d, false, tx_flags);
+}
+
+static struct dma_async_tx_descriptor *
+ma35d1_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
+		     u32 sg_len, enum dma_transfer_direction dir,
+		     unsigned long tx_flags, void *context)
+{
+	int i, width;
+	struct ma35d1_chan *ch = to_ma35d1_pdma_chan(chan);
+	struct ma35d1_desc *d;
+	struct scatterlist *sgent;
+
+	width = ma35d1_slave_width(ch, dir);
+	if (width < 0)
+		return NULL;
+
+	if (!sg_len) {
+		dev_err(ch->dev, "SG entry length is zero\n");
+		return NULL;
+	}
+
+	d = ma35d1_desc_alloc(ch, sg_len);
+	if (!d)
+		return NULL;
+	d->width = width;
+
+	ma35d1_pdma_setup_channel(ch);
+
+	for_each_sg(sgl, sgent, sg_len, i) {
+		if (dir == DMA_MEM_TO_DEV) {
+			d->sg[i].ctl = PDMA_WIDTH(width) | PDMA_DAFIX;
+			d->sg[i].src = sg_dma_address(sgent);
+			d->sg[i].dst = ch->cfg.dst_addr;
+		} else {
+			d->sg[i].ctl = PDMA_WIDTH(width) | PDMA_SAFIX;
+			d->sg[i].src = ch->cfg.src_addr;
+			d->sg[i].dst = sg_dma_address(sgent);
+		}
+		d->sg[i].ctl |= PDMA_TXCNT(sg_dma_len(sgent) / width) | PDMA_TBINTDIS |
+				PDMA_TXTYPE | PDMA_OP_SCATTER;
+		d->sg[i].next = d->dma_addr + PDMA_CH_OFFSET(i + 1);
+	}
+
+	d->sg[sg_len - 1].ctl &= ~(PDMA_OP_MSK | PDMA_TBINTDIS);
+	d->sg[sg_len - 1].ctl |= PDMA_OP_BASIC;
+
+	return ma35d1_desc_submit(ch, d, false, tx_flags);
+}
+
+static struct dma_async_tx_descriptor *
+ma35d1_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t dma_addr, size_t buf_len,
+		       size_t period_len, enum dma_transfer_direction dir,
+		       unsigned long flags)
+{
+	int i, sg_len, width;
+	size_t offset = 0;
+	struct ma35d1_chan *ch = to_ma35d1_pdma_chan(chan);
+	struct ma35d1_desc *d;
+
+	width = ma35d1_slave_width(ch, dir);
+	if (width < 0)
+		return NULL;
+
+	if (!buf_len || !period_len || period_len / width > PDMA_MAX_CHAN_BEATS) {
+		dev_err(ch->dev, "Invalid buffer/period length\n");
+		return NULL;
+	}
+
+	if (buf_len % period_len) {
+		dev_err(ch->dev, "Buffer length not multiple of period length\n");
+		return NULL;
+	}
+
+	if (period_len % width) {
+		dev_err(ch->dev, "Period length must be a multiple of DMA width\n");
+		return NULL;
+	}
+
+	sg_len = buf_len / period_len;
+	d = ma35d1_desc_alloc(ch, sg_len);
+	if (!d)
+		return NULL;
+	d->width = width;
+
+	ma35d1_pdma_setup_channel(ch);
+
+	/* Split the buffer into period size chunks. */
+	for (offset = 0, i = 0; offset < buf_len; offset += period_len, i++) {
+		if (dir == DMA_MEM_TO_DEV) {
+			d->sg[i].ctl = PDMA_WIDTH(width) | PDMA_TXCNT(period_len / width) |
+					PDMA_DAFIX;
+			d->sg[i].src = dma_addr + offset;
+			d->sg[i].dst = ch->cfg.dst_addr;
+		} else {
+			d->sg[i].ctl = PDMA_WIDTH(width) | PDMA_TXCNT(period_len / width) |
+					PDMA_SAFIX;
+			d->sg[i].src = ch->cfg.src_addr;
+			d->sg[i].dst = dma_addr + offset;
+		}
+		d->sg[i].ctl |= PDMA_TXTYPE | PDMA_OP_SCATTER;
+		d->sg[i].next = d->dma_addr + PDMA_CH_OFFSET(i + 1);
+	}
+	d->sg[sg_len - 1].next = d->dma_addr;
+
+	return ma35d1_desc_submit(ch, d, true, flags);
+}
+
+static struct dma_chan *ma35d1_of_xlate(struct of_phandle_args *dma_spec, struct of_dma *of)
+{
+	struct ma35d1_pdmadev *pdmadev = of->of_dma_data;
+	unsigned int chan_id, reqsel;
+
+	if (dma_spec->args_count != 2)
+		return NULL;
+
+	chan_id = dma_spec->args[0];
+	reqsel = dma_spec->args[1];
+	if (chan_id >= pdmadev->nr_chans || reqsel > GENMASK(7, 0))
+		return NULL;
+
+	pdmadev->channels[chan_id].request = reqsel;
+
+	return dma_get_slave_channel(&pdmadev->channels[chan_id].vc.chan);
+}
+
+static void ma35d1_free_chan_resources(struct dma_chan *chan)
+{
+	struct ma35d1_chan *ch = to_ma35d1_pdma_chan(chan);
+
+	vchan_free_chan_resources(&ch->vc);
+	ch->request = 0;
+}
+
+static void ma35d1_pdma_start(struct ma35d1_chan *ch)
+{
+	struct ma35d1_desc *d = ch->desc;
+
+	if (d->sg_len == 1 && !d->cyclic) {
+		/* Trigger basic mode. */
+		writel(d->sg[0].src, ch->base + PDMA_CH_OFFSET(ch->id) + PDMA_DSCT_SA);
+		writel(d->sg[0].dst, ch->base + PDMA_CH_OFFSET(ch->id) + PDMA_DSCT_DA);
+		writel(d->sg[0].ctl, ch->base + PDMA_CH_OFFSET(ch->id) + PDMA_DSCT_CTL);
+	} else {
+		/* Trigger scatter-gather mode. */
+		ma35d1_pdma_reg_clear(ch, PDMA_CH_OFFSET(ch->id) + PDMA_DSCT_CTL, PDMA_OP_MSK);
+		ma35d1_pdma_reg_set(ch, PDMA_CH_OFFSET(ch->id) + PDMA_DSCT_CTL, PDMA_OP_SCATTER);
+	}
+
+	/* Memory-to-memory has no peripheral request, so trigger it by software. */
+	if (ch->request == 0)
+		writel(BIT(ch->id), ch->base + PDMA_SWREQ);
+}
+
+static void ma35d1_pdma_start_desc(struct dma_chan *chan)
+{
+	struct ma35d1_chan *ch = to_ma35d1_pdma_chan(chan);
+	struct virt_dma_desc *vd;
+
+	vd = vchan_next_desc(&ch->vc);
+	if (!vd) {
+		ch->desc = NULL;
+		return;
+	}
+
+	list_del(&vd->node);
+	ch->desc = to_ma35d1_pdma_desc(vd);
+	ma35d1_pdma_timeout(ch);
+	ma35d1_pdma_start(ch);
+}
+
+static void ma35d1_issue_pending(struct dma_chan *chan)
+{
+	struct ma35d1_chan *ch = to_ma35d1_pdma_chan(chan);
+	unsigned long flags;
+
+	spin_lock_irqsave(&ch->vc.lock, flags);
+
+	if (vchan_issue_pending(&ch->vc) && !ch->desc)
+		ma35d1_pdma_start_desc(chan);
+
+	spin_unlock_irqrestore(&ch->vc.lock, flags);
+}
+
+static enum dma_status ma35d1_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
+					struct dma_tx_state *txstate)
+{
+	enum dma_status ret;
+	struct ma35d1_chan *ch = to_ma35d1_pdma_chan(chan);
+	unsigned long flags;
+
+	spin_lock_irqsave(&ch->vc.lock, flags);
+
+	ret = dma_cookie_status(chan, cookie, txstate);
+	if (ret == DMA_COMPLETE) {
+		if (ch->error)
+			ret = DMA_ERROR;
+		else
+			dma_set_residue(txstate, ch->remain);
+	}
+
+	spin_unlock_irqrestore(&ch->vc.lock, flags);
+
+	return ret;
+}
+
+static void ma35d1_pdma_complete(struct ma35d1_chan *ch)
+{
+	if (!ch->desc)
+		return;
+
+	vchan_cookie_complete(&ch->desc->vd);
+	ma35d1_pdma_start_desc(&ch->vc.chan);
+}
+
+static irqreturn_t ma35d1_pdma_interrupt(int irq, void *devid)
+{
+	int i;
+	struct ma35d1_pdmadev *pdmadev = devid;
+	struct ma35d1_chan *ch = &pdmadev->channels[pdmadev->nr_chans - 1];
+	unsigned int timeout_sts, done_sts, abort_sts;
+	unsigned long pending;
+
+	/*
+	 * Read the interrupt flags. Normally, only one is set per channel.
+	 * Check and clear each flag separately.
+	 */
+	done_sts = readl(ch->base + PDMA_TDSTS);
+	timeout_sts = readl(ch->base + PDMA_INTSTS);
+	abort_sts = readl(ch->base + PDMA_ABTSTS);
+
+	pending = (done_sts | abort_sts | (timeout_sts >> 8)) &
+		  GENMASK(pdmadev->nr_chans - 1, 0);
+
+	for_each_set_bit(i, &pending, pdmadev->nr_chans) {
+		unsigned long flags;
+
+		ch = &pdmadev->channels[i];
+
+		spin_lock_irqsave(&ch->vc.lock, flags);
+
+		if (done_sts & BIT(i)) {
+			writel(BIT(i), ch->base + PDMA_TDSTS);
+			if (ch->desc) {
+				ch->remain = 0;
+				if (ch->desc->cyclic)
+					vchan_cyclic_callback(&ch->desc->vd);
+				else
+					ma35d1_pdma_complete(ch);
+			}
+		}
+
+		if (timeout_sts & BIT(i + 8)) {
+			if (ch->desc)
+				ch->remain = PDMA_GET_TXCNT(ch->base + PDMA_CH_OFFSET(ch->id)) *
+					     ch->desc->width;
+			ma35d1_pdma_reg_clear(ch, PDMA_TOUTEN, BIT(i));
+			writel(BIT(i + 8), ch->base + PDMA_INTSTS);
+			ma35d1_pdma_complete(ch);
+		}
+
+		if (abort_sts & BIT(i)) {
+			writel(BIT(i), ch->base + PDMA_ABTSTS);
+			ch->error = true;
+			ma35d1_pdma_complete(ch);
+		}
+
+		spin_unlock_irqrestore(&ch->vc.lock, flags);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int ma35d1_pdma_probe(struct platform_device *pdev)
+{
+	int i, irq, nr_chans, ret;
+	struct clk *pdma_clk;
+	struct device *dev = &pdev->dev;
+	struct device_node *node = dev->of_node;
+	struct ma35d1_chan *ch;
+	struct ma35d1_pdmadev *pdmadev;
+	void __iomem *reg_base;
+
+	pdma_clk = devm_clk_get_enabled(dev, NULL);
+	if (IS_ERR(pdma_clk))
+		return dev_err_probe(dev, PTR_ERR(pdma_clk), "Failed to get clk\n");
+
+	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
+	if (ret)
+		return ret;
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return irq;
+
+	ret = of_property_read_u32(node, "dma-channels", &nr_chans);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to get dma-channels\n");
+
+	if (nr_chans > PDMA_MAX_CHANS) {
+		dev_warn(dev, "dma-channels %d exceeds max %d, clamping\n",
+			 nr_chans, PDMA_MAX_CHANS);
+		nr_chans = PDMA_MAX_CHANS;
+	}
+
+	pdmadev = devm_kzalloc(dev, struct_size(pdmadev, channels, nr_chans), GFP_KERNEL);
+	if (!pdmadev)
+		return -ENOMEM;
+
+	pdmadev->nr_chans = nr_chans;
+
+	reg_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(reg_base))
+		return dev_err_probe(dev, PTR_ERR(reg_base),
+				     "Failed to map pdma global registers\n");
+
+	dma_cap_zero(pdmadev->ddev.cap_mask);
+	dma_cap_set(DMA_SLAVE, pdmadev->ddev.cap_mask);
+	dma_cap_set(DMA_PRIVATE, pdmadev->ddev.cap_mask);
+	dma_cap_set(DMA_MEMCPY, pdmadev->ddev.cap_mask);
+	dma_cap_set(DMA_CYCLIC, pdmadev->ddev.cap_mask);
+
+	pdmadev->ddev.dev = dev;
+	pdmadev->ddev.src_addr_widths = PDMA_SLAVE_BUSWIDTHS;
+	pdmadev->ddev.dst_addr_widths = PDMA_SLAVE_BUSWIDTHS;
+	pdmadev->ddev.directions = BIT(DMA_MEM_TO_DEV) | BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_MEM);
+	pdmadev->ddev.device_prep_dma_memcpy = ma35d1_prep_dma_memcpy;
+	pdmadev->ddev.device_prep_slave_sg = ma35d1_prep_slave_sg;
+	pdmadev->ddev.device_prep_dma_cyclic = ma35d1_prep_dma_cyclic;
+	pdmadev->ddev.device_free_chan_resources = ma35d1_free_chan_resources;
+	pdmadev->ddev.device_issue_pending = ma35d1_issue_pending;
+	pdmadev->ddev.device_tx_status = ma35d1_tx_status;
+	pdmadev->ddev.device_config = ma35d1_slave_config;
+	pdmadev->ddev.device_terminate_all = ma35d1_terminate_all;
+	pdmadev->ddev.device_synchronize = ma35d1_synchronize;
+
+	spin_lock_init(&pdmadev->lock);
+	INIT_LIST_HEAD(&pdmadev->ddev.channels);
+	for (i = 0; i < nr_chans; i++) {
+		ch = &pdmadev->channels[i];
+		ch->dev = dev;
+		ch->pdmadev = pdmadev;
+		ch->id = i;
+		ch->base = reg_base;
+		ch->vc.desc_free = ma35d1_pdma_desc_free;
+		vchan_init(&ch->vc, &pdmadev->ddev);
+	}
+
+	ret = devm_request_irq(dev, irq, ma35d1_pdma_interrupt, 0, pdev->name, pdmadev);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to request IRQ\n");
+
+	ret = dmaenginem_async_device_register(&pdmadev->ddev);
+	if (ret)
+		return ret;
+
+	ret = of_dma_controller_register(node, ma35d1_of_xlate, pdmadev);
+	if (ret)
+		return dev_err_probe(dev, ret, "of_dma_controller_register failed\n");
+
+	platform_set_drvdata(pdev, pdmadev);
+
+	return 0;
+}
+
+static void ma35d1_pdma_remove(struct platform_device *pdev)
+{
+	struct ma35d1_pdmadev *pdmadev = platform_get_drvdata(pdev);
+	int i;
+
+	of_dma_controller_free(pdev->dev.of_node);
+	writel(0, pdmadev->channels[0].base + PDMA_INTEN);
+
+	for (i = 0; i < pdmadev->nr_chans; i++)
+		tasklet_kill(&pdmadev->channels[i].vc.task);
+}
+
+static const struct of_device_id ma35d1_pdma_match[] = {
+	{ .compatible = "nuvoton,ma35d1-pdma" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ma35d1_pdma_match);
+
+static struct platform_driver ma35d1_pdma_driver = {
+	.probe = ma35d1_pdma_probe,
+	.remove = ma35d1_pdma_remove,
+	.driver = {
+		.name = "ma35d1-pdma",
+		.of_match_table = ma35d1_pdma_match,
+	},
+};
+module_platform_driver(ma35d1_pdma_driver);
+
+MODULE_DESCRIPTION("PDMA controller driver for Nuvoton MA35D1");
+MODULE_AUTHOR("Shan-Chun Hung <shanchun1218@gmail.com>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/dma/ma35d1-pdma.h b/include/linux/dma/ma35d1-pdma.h
new file mode 100644
index 000000000000..13f1410d74da
--- /dev/null
+++ b/include/linux/dma/ma35d1-pdma.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Header file for the MA35D1 PDMA Controller driver
+ *
+ * Copyright (C) 2026 Nuvoton Technology Corp.
+ */
+#ifndef _MA35D1_PDMA_H
+#define _MA35D1_PDMA_H
+
+/**
+ * struct ma35d1_peripheral - PDMA peripheral configuration
+ * @prescaler: timeout prescaler value
+ * @counter: timeout counter value
+ *
+ * Slave device drivers fill in this structure and pass it via
+ * dma_slave_config.peripheral_config/peripheral_size to configure
+ * the MA35D1 PDMA engine's per-channel timeout detection block.
+ */
+struct ma35d1_peripheral {
+	unsigned int prescaler;
+	unsigned int counter;
+};
+
+#endif /* _MA35D1_PDMA_H */
--
2.25.1

Re: [PATCH 1/2] dt-bindings: dma: nuvoton,ma35d1-pdma: Document MA35D1 PDMA controller

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] dt-bindings: dma: nuvoton,ma35d1-pdma: additionalProperties blocks common props
- [Low] dt-bindings: dma: nuvoton,ma35d1-pdma: missing resets property in schema
--

commit 5c880c71a9c2f4fa473d23c5e2bcf77d9794575d
Author: Shan-Chun Hung <shanchun1218@gmail.com>

dt-bindings: dma: nuvoton,ma35d1-pdma: Document MA35D1 PDMA controller

This commit adds the devicetree binding schema and request source header for
the Nuvoton MA35D1 PDMA controller.

> diff --git a/Documentation/devicetree/bindings/dma/nuvoton,ma35d1-pdma.yaml b/Documentation/devicetree/bindings/dma/nuvoton,ma35d1-pdma.yaml
> new file mode 100644
> index 0000000000000..cd752571242c6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/nuvoton,ma35d1-pdma.yaml
> @@ -0,0 +1,72 @@

[ ... ]

> +allOf:
> +  - $ref: dma-controller.yaml#
> +
> +properties:
> +  compatible:
> +    const: nuvoton,ma35d1-pdma
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1

[Severity: Low]
Should this binding also document the resets property?

The MA35D1 reset binding and driver define dedicated hardware reset lines for
the four PDMA controllers:

include/dt-bindings/reset/nuvoton,ma35d1-reset.h:
    #define MA35D1_RESET_PDMA0	4
    #define MA35D1_RESET_PDMA1	5
    #define MA35D1_RESET_PDMA2	6
    #define MA35D1_RESET_PDMA3	7

drivers/reset/reset-ma35d1.c:
    static const struct ma35d1_reset_map ma35d1_resets[] = {
        ...
        [MA35D1_RESET_PDMA0] =   {0x20, 4},
        [MA35D1_RESET_PDMA1] =   {0x20, 5},
        [MA35D1_RESET_PDMA2] =   {0x20, 6},
        [MA35D1_RESET_PDMA3] =   {0x20, 7},

Without resets in the properties block, describing the controller's hardware
reset line in a devicetree node will fail dtbs_check validation.

> +
> +  "#dma-cells":
> +    const: 2
> +    description:
> +      The first cell is the channel number. The second cell is the
> +      peripheral request source (REQSRC) selection, which defines which
> +      peripheral is connected to that PDMA channel.
> +
> +  dma-channels:
> +    const: 10
> +
> +additionalProperties: false

[Severity: Low]
Since this schema references dma-controller.yaml# under allOf, shouldn't it
use unevaluatedProperties: false instead of additionalProperties: false?

Using additionalProperties: false causes dtbs_check to reject standard DMA
controller properties inherited from dma-controller.yaml# and
dma-common.yaml# (such as dma-channel-mask or dma-requests) unless they are
duplicated in the local properties block.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103022202.2139674-1-shanchun1218@gmail.com?part=1

Re: [PATCH 2/2] dmaengine: ma35d1-pdma: Add MA35D1 PDMA driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 11 potential issue(s) to consider:
- [Critical] dmaengine: ma35d1: premature register setup in ma35d1_desc_alloc() and prep
- [Critical] dmaengine: ma35d1: active DMA after timeout and double completion in IRQ
- [High] dmaengine: ma35d1: missing PDMA_TBINTDIS in ma35d1_prep_dma_memcpy()
- [High] dmaengine: ma35d1: unvalidated SG segment lengths in ma35d1_prep_slave_sg()
- [High] dmaengine: ma35d1: broken residue and status reporting in ma35d1_tx_status()
- [High] dmaengine: ma35d1: unlocked PDMA_TOUTEN RMW in ma35d1_pdma_interrupt()
- [High] dmaengine: ma35d1: state bugs in ma35d1_of_xlate() and free_chan_resources()
- [High] dmaengine: ma35d1-pdma: fix signed dma-channels pointer and zero bounds check
- [High] dmaengine: ma35d1-pdma: unsafe teardown order in ma35d1_pdma_remove()
- [Medium] dmaengine: ma35d1-pdma: disable channel timeout in ma35d1_terminate_all()
- [Medium] dmaengine: ma35d1-pdma: unreleased ch->desc and stale ch->cfg in ma35d1_free_chan_resources()
--

commit f0cfa08c80802c32fef4b715415e04126d30e654
Author: Shan-Chun Hung <shanchun1218@gmail.com>

dmaengine: ma35d1-pdma: Add MA35D1 PDMA driver

This commit adds a DMAEngine driver for the PDMA controller in the Nuvoton
MA35D1 SoC, supporting up to 10 channels with memory-to-memory, slave
scatter-gather, and cyclic transfers, as well as per-channel peripheral RX
timeouts.

> diff --git a/drivers/dma/ma35d1-pdma.c b/drivers/dma/ma35d1-pdma.c
> new file mode 100644
> index 0000000000000..176bc56c80639
> --- /dev/null
> +++ b/drivers/dma/ma35d1-pdma.c
> @@ -0,0 +1,780 @@

[ ... ]

> +#define PDMA_WIDTH(w)		FIELD_PREP(GENMASK(13, 12), (w) >> 1)
> +#define PDMA_TXCNT(cnt)		FIELD_PREP(GENMASK(31, 16), (cnt) - 1)
> +#define PDMA_GET_TXCNT(ctrl)	FIELD_GET(GENMASK(31, 16), readl(ctrl))
> +#define PDMA_CH_OFFSET(ch)	((ch) * 0x10)

[ ... ]

> +static int ma35d1_terminate_all(struct dma_chan *chan)
> +{
> +	LIST_HEAD(head);
> +	struct ma35d1_chan *ch = to_ma35d1_pdma_chan(chan);
> +	unsigned long flags, pdmadev_flags;
> +
> +	spin_lock_irqsave(&ch->vc.lock, flags);
> +
> +	spin_lock_irqsave(&ch->pdmadev->lock, pdmadev_flags);
> +	ma35d1_pdma_reg_clear(ch, PDMA_CHCTL, BIT(ch->id));
> +	spin_unlock_irqrestore(&ch->pdmadev->lock, pdmadev_flags);

[Severity: Medium]
When a channel configured with a peripheral timeout is terminated in
ma35d1_terminate_all(), only BIT(ch->id) in PDMA_CHCTL is cleared, leaving
BIT(ch->id) enabled in PDMA_TOUTEN and PDMA_TOUTIEN from
ma35d1_pdma_start_desc() -> ma35d1_pdma_timeout().

Could the hardware timeout counter fire after termination and cause
ma35d1_pdma_interrupt() to prematurely complete a subsequently started
descriptor on the same channel? Should PDMA_TOUTEN and PDMA_TOUTIEN also be
cleared here?

> +
> +	if (ch->desc) {
> +		vchan_terminate_vdesc(&ch->desc->vd);
> +		ch->desc = NULL;
> +	}

[ ... ]

> +static void ma35d1_pdma_setup_channel(struct ma35d1_chan *ch)
> +{
> +	u32 offset, reg, shift;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&ch->pdmadev->lock, flags);
> +
> +	if ((readl(ch->base + PDMA_CH_OFFSET(ch->id)) & GENMASK(1, 0)) != 0) {
> +		reg = readl(ch->base + PDMA_CHCTL);
> +		writel(0, ch->base + PDMA_CH_OFFSET(ch->id));
> +		ma35d1_pdma_reg_set(ch, PDMA_CHRST, BIT(ch->id));
> +		writel(reg | BIT(ch->id), ch->base + PDMA_CHCTL);
> +	} else {
> +		writel(0, ch->base + PDMA_CH_OFFSET(ch->id));
> +		ma35d1_pdma_reg_set(ch, PDMA_CHCTL, BIT(ch->id));
> +	}
> +	ma35d1_pdma_reg_set(ch, PDMA_INTEN, BIT(ch->id));
> +
> +	offset = PDMA_REQSEL + (ch->id & ~GENMASK(1, 0));
> +	shift = (ch->id & GENMASK(1, 0)) * 8;
> +	ma35d1_pdma_reg_clear(ch, offset, GENMASK(7, 0) << shift);
> +	ma35d1_pdma_reg_set(ch, offset, ch->request << shift);
> +
> +	spin_unlock_irqrestore(&ch->pdmadev->lock, flags);
> +}

[ ... ]

> +static struct ma35d1_desc *ma35d1_desc_alloc(struct ma35d1_chan *ch, u32 sg_len)
> +{
> +	struct ma35d1_desc *d;
> +
> +	d = kzalloc(struct_size(d, sg, sg_len), GFP_ATOMIC);
> +	if (!d)
> +		return NULL;
> +
> +	d->sg_len = sg_len;
> +	d->dma_len = sizeof(*d->sg) * sg_len;
> +	d->dma_addr = dma_map_single(ch->dev, d->sg, d->dma_len, DMA_TO_DEVICE);
> +	if (dma_mapping_error(ch->dev, d->dma_addr)) {
> +		dev_err(ch->dev, "dma mapping error for channel\n");
> +		kfree(d);
> +		return NULL;
> +	}
> +
> +	writel(d->dma_addr, ch->base + PDMA_CH_OFFSET(ch->id) + PDMA_DSCT_NEXT);
> +
> +	return d;
> +}
> +
> +static struct dma_async_tx_descriptor *
> +ma35d1_desc_submit(struct ma35d1_chan *ch, struct ma35d1_desc *d, bool cyclic,
> +		   unsigned long tx_flags)
> +{
> +	dma_sync_single_for_device(ch->dev, d->dma_addr, d->dma_len, DMA_TO_DEVICE);
> +	d->cyclic = cyclic;
> +	d->ch = ch;
> +	ch->error = false;
> +
> +	return vchan_tx_prep(&ch->vc, &d->vd, tx_flags);
> +}

[Severity: Critical]
Hardware registers and per-channel state are being modified during descriptor
preparation (ma35d1_prep_dma_memcpy(), ma35d1_prep_slave_sg(), and
ma35d1_prep_dma_cyclic()) rather than when the descriptor is actually started
in ma35d1_pdma_start_desc() -> ma35d1_pdma_start():

1. ma35d1_desc_alloc() writes d->dma_addr directly to PDMA_DSCT_NEXT before
d->sg is populated or synced with dma_sync_single_for_device(), and without
holding ch->vc.lock.

2. Immediately after ma35d1_desc_alloc(), the prep functions call
ma35d1_pdma_setup_channel(ch). If bits 1:0 of PDMA_DSCT_CTL are non-zero (for
example, while an active transfer is running, or after a timeout or
ma35d1_terminate_all()), ma35d1_pdma_setup_channel() writes 0 to PDMA_DSCT_CTL
and asserts BIT(ch->id) in PDMA_CHRST. Doesn't resetting the channel here
abort any in-flight transfer and wipe out the PDMA_DSCT_NEXT value that
ma35d1_desc_alloc() just programmed?

3. Conversely, when a descriptor is dequeued in ma35d1_pdma_start_desc() ->
ma35d1_pdma_start(), neither ma35d1_pdma_setup_channel(ch) nor the write of
d->dma_addr to PDMA_DSCT_NEXT is performed:

    static void ma35d1_pdma_start(struct ma35d1_chan *ch)
    {
        struct ma35d1_desc *d = ch->desc;

        if (d->sg_len == 1 && !d->cyclic) {
            ...
        } else {
            /* Trigger scatter-gather mode. */
            ma35d1_pdma_reg_clear(ch, PDMA_CH_OFFSET(ch->id) + PDMA_DSCT_CTL, PDMA_OP_MSK);
            ma35d1_pdma_reg_set(ch, PDMA_CH_OFFSET(ch->id) + PDMA_DSCT_CTL, PDMA_OP_SCATTER);
        }

If multiple descriptors are prepared before dma_async_issue_pending(), won't
PDMA_DSCT_NEXT be overwritten by the last prepared descriptor, and subsequent
descriptors started from ma35d1_pdma_complete() never program PDMA_DSCT_NEXT
or re-enable BIT(ch->id) in PDMA_CHCTL?

4. In ma35d1_desc_submit(), ch->error = false is written without holding
ch->vc.lock, which races with ma35d1_pdma_interrupt() and ma35d1_tx_status().

Should channel setup and writing PDMA_DSCT_NEXT be moved into
ma35d1_pdma_start() under ch->vc.lock?

> +
> +static struct dma_async_tx_descriptor *
> +ma35d1_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dst, dma_addr_t src,
> +		       size_t len, unsigned long tx_flags)
> +{

[ ... ]

> +	for (i = 0; i < sg_len; i++) {
> +		tlen = min_t(size_t, len, max_chunk);
> +		d->sg[i].src = src;
> +		d->sg[i].dst = dst;
> +		d->sg[i].ctl = PDMA_WIDTH(width) | PDMA_TXCNT(tlen / width) | PDMA_OP_SCATTER;
> +		d->sg[i].next = d->dma_addr + PDMA_CH_OFFSET(i + 1);
> +		src += tlen;
> +		dst += tlen;
> +		len -= tlen;
> +	}
> +	d->sg[sg_len - 1].ctl &= ~(PDMA_OP_MSK | PDMA_TBINTDIS);
> +	d->sg[sg_len - 1].ctl |= PDMA_OP_BASIC;

[Severity: High]
In ma35d1_prep_dma_memcpy(), the final SG entry clears PDMA_TBINTDIS, but
intermediate entries in the loop never set PDMA_TBINTDIS in d->sg[i].ctl
(unlike ma35d1_prep_slave_sg()).

When len > max_chunk (so sg_len > 1), won't the controller raise a transfer
done interrupt (PDMA_TDSTS) as soon as the first chunk (d->sg[0]) finishes,
causing ma35d1_pdma_interrupt() -> ma35d1_pdma_complete() to complete and
free the descriptor while the remaining chunks are still in flight?

> +
> +	return ma35d1_desc_submit(ch, d, false, tx_flags);
> +}
> +
> +static struct dma_async_tx_descriptor *
> +ma35d1_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
> +		     u32 sg_len, enum dma_transfer_direction dir,
> +		     unsigned long tx_flags, void *context)
> +{

[ ... ]

> +	for_each_sg(sgl, sgent, sg_len, i) {
> +		if (dir == DMA_MEM_TO_DEV) {
> +			d->sg[i].ctl = PDMA_WIDTH(width) | PDMA_DAFIX;
> +			d->sg[i].src = sg_dma_address(sgent);
> +			d->sg[i].dst = ch->cfg.dst_addr;
> +		} else {
> +			d->sg[i].ctl = PDMA_WIDTH(width) | PDMA_SAFIX;
> +			d->sg[i].src = ch->cfg.src_addr;
> +			d->sg[i].dst = sg_dma_address(sgent);
> +		}
> +		d->sg[i].ctl |= PDMA_TXCNT(sg_dma_len(sgent) / width) | PDMA_TBINTDIS |
> +				PDMA_TXTYPE | PDMA_OP_SCATTER;
> +		d->sg[i].next = d->dma_addr + PDMA_CH_OFFSET(i + 1);
> +	}

[Severity: High]
In ma35d1_prep_slave_sg(), sg_dma_len(sgent) / width is passed directly to
PDMA_TXCNT(cnt), which encodes FIELD_PREP(GENMASK(31, 16), (cnt) - 1), without
validating the segment length (unlike ma35d1_prep_dma_cyclic() and
ma35d1_prep_dma_memcpy()).

If sg_dma_len(sgent) < width (or 0), sg_dma_len(sgent) / width is 0 and
(cnt) - 1 underflows to 0xffff (65536 beats), causing an out-of-bounds DMA
transfer. Conversely, if sg_dma_len(sgent) / width > PDMA_MAX_CHAN_BEATS,
the 16-bit field overflows and truncates the transfer, and if
sg_dma_len(sgent) % width != 0, trailing bytes are dropped.

Should ma35d1_prep_slave_sg() validate (or split) each SG entry's length and
alignment, and should ma35d1_pdma_probe() also advertise the maximum segment
size via dma_set_max_seg_size()?

[ ... ]

> +static struct dma_chan *ma35d1_of_xlate(struct of_phandle_args *dma_spec, struct of_dma *of)
> +{
> +	struct ma35d1_pdmadev *pdmadev = of->of_dma_data;
> +	unsigned int chan_id, reqsel;
> +
> +	if (dma_spec->args_count != 2)
> +		return NULL;
> +
> +	chan_id = dma_spec->args[0];
> +	reqsel = dma_spec->args[1];
> +	if (chan_id >= pdmadev->nr_chans || reqsel > GENMASK(7, 0))
> +		return NULL;
> +
> +	pdmadev->channels[chan_id].request = reqsel;
> +
> +	return dma_get_slave_channel(&pdmadev->channels[chan_id].vc.chan);
> +}

[Severity: High]
In ma35d1_of_xlate(), pdmadev->channels[chan_id].request = reqsel is written
before calling dma_get_slave_channel().

If chan_id is already in use by another consumer, dma_get_slave_channel()
will return NULL, but the active channel's request field will already have
been overwritten with reqsel. Should request only be assigned after
dma_get_slave_channel() succeeds?

> +
> +static void ma35d1_free_chan_resources(struct dma_chan *chan)
> +{
> +	struct ma35d1_chan *ch = to_ma35d1_pdma_chan(chan);
> +
> +	vchan_free_chan_resources(&ch->vc);
> +	ch->request = 0;
> +}

[Severity: Medium]
When a channel is released in ma35d1_free_chan_resources(), ch->request is
reset to 0, but ch->cfg (including ch->cfg.peripheral_config) is not cleared,
and any active descriptor in ch->desc (which ma35d1_pdma_start_desc() unlinked
from vc.desc_issued via list_del(&vd->node)) is not terminated or freed.

If a slave consumer configures peripheral_config via ma35d1_slave_config() and
later releases the channel, won't a subsequent DMA_MEMCPY user (or a slave
consumer that does not set peripheral_config) inherit the stale
ch->cfg.peripheral_config pointer in ma35d1_pdma_start_desc() ->
ma35d1_pdma_timeout() and erroneously enable peripheral timeout interrupts?

[ ... ]

> +static enum dma_status ma35d1_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
> +					struct dma_tx_state *txstate)
> +{
> +	enum dma_status ret;
> +	struct ma35d1_chan *ch = to_ma35d1_pdma_chan(chan);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&ch->vc.lock, flags);
> +
> +	ret = dma_cookie_status(chan, cookie, txstate);
> +	if (ret == DMA_COMPLETE) {
> +		if (ch->error)
> +			ret = DMA_ERROR;
> +		else
> +			dma_set_residue(txstate, ch->remain);
> +	}
> +
> +	spin_unlock_irqrestore(&ch->vc.lock, flags);
> +
> +	return ret;
> +}

[Severity: High]
There appear to be several issues with status and residue reporting in
ma35d1_tx_status() and ma35d1_pdma_interrupt():

1. ma35d1_tx_status() only calls dma_set_residue() when ret == DMA_COMPLETE.
For DMA_IN_PROGRESS transfers (including cyclic transfers, where cookie status
is always DMA_IN_PROGRESS), residue is never computed and remains 0.

2. Error and residue state are stored in per-channel fields (ch->error and
ch->remain) rather than per-descriptor, and ch->error is cleared in
ma35d1_desc_submit() whenever a new descriptor is prepared. Won't preparing a
new descriptor clear ch->error before a client queries the failed transfer's
cookie, or cause an abort to report DMA_ERROR for previously completed cookies
on that channel? Also, ch->desc->vd.tx_result is never updated on abort or
timeout, so callback_result clients will always see DMA_TRANS_NOERROR with 0
residue.

3. In ma35d1_pdma_interrupt(), timeout residue is computed as:

    ch->remain = PDMA_GET_TXCNT(ch->base + PDMA_CH_OFFSET(ch->id)) *
                 ch->desc->width;

Because PDMA_TXCNT(cnt) programs the hardware with (cnt) - 1, doesn't reading
PDMA_GET_TXCNT() without adding 1 under-report the remaining bytes in the
current segment by one beat (ch->desc->width bytes), and ignore any remaining
segments when sg_len > 1?

[ ... ]

> +static irqreturn_t ma35d1_pdma_interrupt(int irq, void *devid)
> +{
> +	int i;
> +	struct ma35d1_pdmadev *pdmadev = devid;
> +	struct ma35d1_chan *ch = &pdmadev->channels[pdmadev->nr_chans - 1];
> +	unsigned int timeout_sts, done_sts, abort_sts;
> +	unsigned long pending;
> +
> +	/*
> +	 * Read the interrupt flags. Normally, only one is set per channel.
> +	 * Check and clear each flag separately.
> +	 */
> +	done_sts = readl(ch->base + PDMA_TDSTS);
> +	timeout_sts = readl(ch->base + PDMA_INTSTS);
> +	abort_sts = readl(ch->base + PDMA_ABTSTS);
> +
> +	pending = (done_sts | abort_sts | (timeout_sts >> 8)) &
> +		  GENMASK(pdmadev->nr_chans - 1, 0);
> +
> +	for_each_set_bit(i, &pending, pdmadev->nr_chans) {
> +		unsigned long flags;
> +
> +		ch = &pdmadev->channels[i];
> +
> +		spin_lock_irqsave(&ch->vc.lock, flags);
> +
> +		if (done_sts & BIT(i)) {
> +			writel(BIT(i), ch->base + PDMA_TDSTS);
> +			if (ch->desc) {
> +				ch->remain = 0;
> +				if (ch->desc->cyclic)
> +					vchan_cyclic_callback(&ch->desc->vd);
> +				else
> +					ma35d1_pdma_complete(ch);
> +			}
> +		}
> +
> +		if (timeout_sts & BIT(i + 8)) {
> +			if (ch->desc)
> +				ch->remain = PDMA_GET_TXCNT(ch->base + PDMA_CH_OFFSET(ch->id)) *
> +					     ch->desc->width;
> +			ma35d1_pdma_reg_clear(ch, PDMA_TOUTEN, BIT(i));

[Severity: High]
PDMA_TOUTEN is a controller-wide register shared across all channels, and
ma35d1_pdma_timeout() serializes read-modify-write updates to PDMA_TOUTEN
using ch->pdmadev->lock.

Here in ma35d1_pdma_interrupt(), ma35d1_pdma_reg_clear(ch, PDMA_TOUTEN, BIT(i))
performs a read-modify-write while holding only the per-channel ch->vc.lock.
Can this race with another CPU starting a descriptor on a different channel in
ma35d1_issue_pending() -> ma35d1_pdma_start_desc() -> ma35d1_pdma_timeout()
and clobber PDMA_TOUTEN?

> +			writel(BIT(i + 8), ch->base + PDMA_INTSTS);
> +			ma35d1_pdma_complete(ch);
> +		}
> +
> +		if (abort_sts & BIT(i)) {
> +			writel(BIT(i), ch->base + PDMA_ABTSTS);
> +			ch->error = true;
> +			ma35d1_pdma_complete(ch);
> +		}
> +
> +		spin_unlock_irqrestore(&ch->vc.lock, flags);
> +	}
> +
> +	return IRQ_HANDLED;
> +}

[Severity: Critical]
In ma35d1_pdma_interrupt(), there are a few issues with how channel completion
and interrupt status are handled:

1. When a timeout interrupt fires (timeout_sts & BIT(i + 8)), the transfer has
not completed all beats and the hardware channel remains enabled in PDMA_CHCTL
with an active operating mode in PDMA_DSCT_CTL. Calling ma35d1_pdma_complete()
without disabling or resetting the hardware channel will complete and free
ch->desc (via vchan_complete() -> ma35d1_pdma_desc_free()) while the hardware
channel is still active. If another peripheral request arrives when no next
descriptor was queued, won't the controller DMA into the unmapped/freed client
buffer or fetch freed SG descriptors?

2. Because done_sts, timeout_sts, and abort_sts are handled in three sequential
if blocks that each call ma35d1_pdma_complete(ch) (which completes ch->desc and
immediately starts the next queued descriptor in ch->desc), if more than one
status bit is set for channel i in the same interrupt, won't the second block
immediately complete the newly started next descriptor (or free an active
cyclic descriptor)?

3. If pending is 0, should ma35d1_pdma_interrupt() return IRQ_NONE instead of
unconditionally returning IRQ_HANDLED?

> +
> +static int ma35d1_pdma_probe(struct platform_device *pdev)
> +{
> +	int i, irq, nr_chans, ret;
> +	struct clk *pdma_clk;
> +	struct device *dev = &pdev->dev;
> +	struct device_node *node = dev->of_node;
> +	struct ma35d1_chan *ch;
> +	struct ma35d1_pdmadev *pdmadev;
> +	void __iomem *reg_base;

[ ... ]

> +	ret = of_property_read_u32(node, "dma-channels", &nr_chans);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to get dma-channels\n");
> +
> +	if (nr_chans > PDMA_MAX_CHANS) {
> +		dev_warn(dev, "dma-channels %d exceeds max %d, clamping\n",
> +			 nr_chans, PDMA_MAX_CHANS);
> +		nr_chans = PDMA_MAX_CHANS;
> +	}

[Severity: High]
In ma35d1_pdma_probe(), nr_chans is declared as a signed int and passed as
&nr_chans to of_property_read_u32(), which expects a u32 * (triggering a
-Wpointer-sign warning), and the bounds check only tests nr_chans >
PDMA_MAX_CHANS without rejecting nr_chans <= 0.

If dma-channels in the Device Tree is 0 (or has bit 31 set, making nr_chans
negative), devm_kzalloc() will allocate 0 elements for pdmadev->channels[],
causing out-of-bounds accesses in ma35d1_pdma_interrupt() at
&pdmadev->channels[pdmadev->nr_chans - 1] and in ma35d1_pdma_remove() at
pdmadev->channels[0].base. Should nr_chans be a u32 and checked for
nr_chans == 0?

[ ... ]

> +	ret = devm_request_irq(dev, irq, ma35d1_pdma_interrupt, 0, pdev->name, pdmadev);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to request IRQ\n");
> +
> +	ret = dmaenginem_async_device_register(&pdmadev->ddev);
> +	if (ret)
> +		return ret;
> +
> +	ret = of_dma_controller_register(node, ma35d1_of_xlate, pdmadev);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "of_dma_controller_register failed\n");
> +
> +	platform_set_drvdata(pdev, pdmadev);
> +
> +	return 0;
> +}
> +
> +static void ma35d1_pdma_remove(struct platform_device *pdev)
> +{
> +	struct ma35d1_pdmadev *pdmadev = platform_get_drvdata(pdev);
> +	int i;
> +
> +	of_dma_controller_free(pdev->dev.of_node);
> +	writel(0, pdmadev->channels[0].base + PDMA_INTEN);
> +
> +	for (i = 0; i < pdmadev->nr_chans; i++)
> +		tasklet_kill(&pdmadev->channels[i].vc.task);
> +}

[Severity: High]
In ma35d1_pdma_probe(), the interrupt handler and dma_device are registered via
devm_request_irq() and dmaenginem_async_device_register(), so devres only
releases them after ma35d1_pdma_remove() returns.

During ma35d1_pdma_remove():
1. Active channels in PDMA_CHCTL and timeout interrupts in PDMA_TOUTIEN /
PDMA_TOUTEN are not disabled.
2. The IRQ is not freed or synchronized before tasklet_kill(), so an in-flight
ma35d1_pdma_interrupt() on another CPU (or a timeout interrupt) can call
vchan_cookie_complete() or vchan_cyclic_callback() and reschedule vc.task
after tasklet_kill() finishes.
3. Because pdmadev->ddev is still registered during ma35d1_pdma_remove(), a
concurrent client can still prepare descriptors and re-enable PDMA_INTEN in
ma35d1_pdma_setup_channel().

Could this allow vc.task to run after devres frees pdmadev? Also, if
of_dma_controller_register() fails in ma35d1_pdma_probe(), the channel
tasklets are not killed on error unwind.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103022202.2139674-1-shanchun1218@gmail.com?part=2

Re: [PATCH 1/2] dt-bindings: dma: nuvoton,ma35d1-pdma: Document MA35D1 PDMA controller

[Krzysztof Kozlowski]

On Tue, Nov 03, 2026 at 10:22:01AM +0800, Shan-Chun Hung wrote:
> Add binding for Nuvoton MA35D1 PDMA controller.
> 

Looks like valid Sashiko review, so only few nits, not a full review:

> Signed-off-by: Shan-Chun Hung <shanchun1218@gmail.com>
> ---
>  .../bindings/dma/nuvoton,ma35d1-pdma.yaml     |  72 +++++++++++
>  include/dt-bindings/dma/ma35d1-pdma.h         | 113 ++++++++++++++++++
>  2 files changed, 185 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/dma/nuvoton,ma35d1-pdma.yaml
>  create mode 100644 include/dt-bindings/dma/ma35d1-pdma.h
> 
> diff --git a/Documentation/devicetree/bindings/dma/nuvoton,ma35d1-pdma.yaml b/Documentation/devicetree/bindings/dma/nuvoton,ma35d1-pdma.yaml
> new file mode 100644
> index 000000000000..cd752571242c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/nuvoton,ma35d1-pdma.yaml
> @@ -0,0 +1,72 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/dma/nuvoton,ma35d1-pdma.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Nuvoton MA35D1 PDMA controller
> +
> +description:
> +  This describes the devicetree bindings for four Peripheral DMA engines to perform

Do not describe what the binding document is describe. Just describe
that directly, so explain the hardware. Look at other examples.

> +  memory-to-memory, memory-to-peripheral, or peripheral-to-memory data transfer
> +  capable of supporting 10 channels, implemented in Nuvoton MA35D1 SoCs.
> +
> +maintainers:
> +  - Shan-Chun Hung <shanchun1218@gmail.com>
> +
> +allOf:
> +  - $ref: dma-controller.yaml#
> +
> +properties:
> +  compatible:
> +    const: nuvoton,ma35d1-pdma
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  "#dma-cells":
> +    const: 2
> +    description:
> +      The first cell is the channel number. The second cell is the
> +      peripheral request source (REQSRC) selection, which defines which
> +      peripheral is connected to that PDMA channel.
> +
> +  dma-channels:
> +    const: 10

Why isn't this implied by the compatible? Drop.

> +
> +additionalProperties: false

This goes after required block.

> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - interrupts
> +  - "#dma-cells"
> +  - dma-channels
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        dma-controller@40080000 {
> +            compatible = "nuvoton,ma35d1-pdma";
> +            reg = <0x0 0x40080000 0x0 0x2000>;
> +            interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
> +            clocks = <&clk PDMA0_GATE>;
> +            #dma-cells = <2>;
> +            dma-channels = <10>;
> +        };
> +    };
> +
> +...
> diff --git a/include/dt-bindings/dma/ma35d1-pdma.h b/include/dt-bindings/dma/ma35d1-pdma.h
> new file mode 100644
> index 000000000000..0ddc99d29661
> --- /dev/null
> +++ b/include/dt-bindings/dma/ma35d1-pdma.h

Filename must match the binding filename.

> @@ -0,0 +1,113 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +/*
> + * Copyright (C) 2026 Nuvoton Technology Corp.
> + */
> +
> +#ifndef __DT_BINDINGS_DMA_MA35D1_PDMA_H__
> +#define __DT_BINDINGS_DMA_MA35D1_PDMA_H__
> +
> +#define MA35D1_PDMA_MEM			0


Hole

> +#define MA35D1_PDMA_UART0_TX		4
> +#define MA35D1_PDMA_UART0_RX		5
> +#define MA35D1_PDMA_UART1_TX		6
> +#define MA35D1_PDMA_UART1_RX		7
> +#define MA35D1_PDMA_UART2_TX		8
> +#define MA35D1_PDMA_UART2_RX		9
> +#define MA35D1_PDMA_UART3_TX		10
> +#define MA35D1_PDMA_UART3_RX		11
> +#define MA35D1_PDMA_UART4_TX		12
> +#define MA35D1_PDMA_UART4_RX		13
> +#define MA35D1_PDMA_UART5_TX		14
> +#define MA35D1_PDMA_UART5_RX		15
> +#define MA35D1_PDMA_UART6_TX		16
> +#define MA35D1_PDMA_UART6_RX		17
> +#define MA35D1_PDMA_UART7_TX		18
> +#define MA35D1_PDMA_UART7_RX		19
> +#define MA35D1_PDMA_UART8_TX		20
> +#define MA35D1_PDMA_UART8_RX		21
> +#define MA35D1_PDMA_UART9_TX		22
> +#define MA35D1_PDMA_UART9_RX		23
> +#define MA35D1_PDMA_UART10_TX		24
> +#define MA35D1_PDMA_UART10_RX		25
> +#define MA35D1_PDMA_UART11_TX		26
> +#define MA35D1_PDMA_UART11_RX		27
> +#define MA35D1_PDMA_UART12_TX		28
> +#define MA35D1_PDMA_UART12_RX		29
> +#define MA35D1_PDMA_UART13_TX		30
> +#define MA35D1_PDMA_UART13_RX		31
> +#define MA35D1_PDMA_UART14_TX		32
> +#define MA35D1_PDMA_UART14_RX		33
> +#define MA35D1_PDMA_UART15_TX		34
> +#define MA35D1_PDMA_UART15_RX		35
> +#define MA35D1_PDMA_UART16_TX		36
> +#define MA35D1_PDMA_UART16_RX		37
> +#define MA35D1_PDMA_QSPI0_TX		54

You should not have holes in the ABI or this is not DT ABI, so drop or
move out entire file from bindings.

Best regards,
Krzysztof

Re: [PATCH 2/2] dmaengine: ma35d1-pdma: Add MA35D1 PDMA driver

[Krzysztof Kozlowski]

On Tue, Nov 03, 2026 at 10:22:02AM +0800, Shan-Chun Hung wrote:
> +#include <linux/align.h>
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/compiler_types.h>
> +#include <linux/container_of.h>
> +#include <linux/dev_printk.h>
> +#include <linux/device.h>
> +#include <linux/dma-direction.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/dma/ma35d1-pdma.h>
> +#include <linux/dmaengine.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/gfp_types.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/irqreturn.h>
> +#include <linux/limits.h>
> +#include <linux/list.h>
> +#include <linux/math.h>
> +#include <linux/minmax.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_dma.h>
> +#include <linux/overflow.h>
> +#include <linux/platform_device.h>
> +#include <linux/scatterlist.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +#include <linux/spinlock_types.h>
> +#include <linux/stddef.h>
> +#include <linux/string.h>
> +#include <linux/types.h>

Do you use all these defines?

I don't see usage of header with bindings, though.

> +
> +#include "dmaengine.h"
> +#include "virt-dma.h"
> +

...

> +static int ma35d1_pdma_probe(struct platform_device *pdev)
> +{
> +	int i, irq, nr_chans, ret;
> +	struct clk *pdma_clk;
> +	struct device *dev = &pdev->dev;
> +	struct device_node *node = dev->of_node;
> +	struct ma35d1_chan *ch;
> +	struct ma35d1_pdmadev *pdmadev;
> +	void __iomem *reg_base;
> +
> +	pdma_clk = devm_clk_get_enabled(dev, NULL);
> +	if (IS_ERR(pdma_clk))
> +		return dev_err_probe(dev, PTR_ERR(pdma_clk), "Failed to get clk\n");
> +
> +	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> +	if (ret)
> +		return ret;
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0)
> +		return irq;
> +
> +	ret = of_property_read_u32(node, "dma-channels", &nr_chans);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to get dma-channels\n");
> +
> +	if (nr_chans > PDMA_MAX_CHANS) {
> +		dev_warn(dev, "dma-channels %d exceeds max %d, clamping\n",
> +			 nr_chans, PDMA_MAX_CHANS);
> +		nr_chans = PDMA_MAX_CHANS;
> +	}
> +
> +	pdmadev = devm_kzalloc(dev, struct_size(pdmadev, channels, nr_chans), GFP_KERNEL);
> +	if (!pdmadev)
> +		return -ENOMEM;
> +
> +	pdmadev->nr_chans = nr_chans;
> +
> +	reg_base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(reg_base))
> +		return dev_err_probe(dev, PTR_ERR(reg_base),
> +				     "Failed to map pdma global registers\n");
> +
> +	dma_cap_zero(pdmadev->ddev.cap_mask);
> +	dma_cap_set(DMA_SLAVE, pdmadev->ddev.cap_mask);
> +	dma_cap_set(DMA_PRIVATE, pdmadev->ddev.cap_mask);
> +	dma_cap_set(DMA_MEMCPY, pdmadev->ddev.cap_mask);
> +	dma_cap_set(DMA_CYCLIC, pdmadev->ddev.cap_mask);
> +
> +	pdmadev->ddev.dev = dev;
> +	pdmadev->ddev.src_addr_widths = PDMA_SLAVE_BUSWIDTHS;
> +	pdmadev->ddev.dst_addr_widths = PDMA_SLAVE_BUSWIDTHS;
> +	pdmadev->ddev.directions = BIT(DMA_MEM_TO_DEV) | BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_MEM);
> +	pdmadev->ddev.device_prep_dma_memcpy = ma35d1_prep_dma_memcpy;
> +	pdmadev->ddev.device_prep_slave_sg = ma35d1_prep_slave_sg;
> +	pdmadev->ddev.device_prep_dma_cyclic = ma35d1_prep_dma_cyclic;
> +	pdmadev->ddev.device_free_chan_resources = ma35d1_free_chan_resources;
> +	pdmadev->ddev.device_issue_pending = ma35d1_issue_pending;
> +	pdmadev->ddev.device_tx_status = ma35d1_tx_status;
> +	pdmadev->ddev.device_config = ma35d1_slave_config;
> +	pdmadev->ddev.device_terminate_all = ma35d1_terminate_all;
> +	pdmadev->ddev.device_synchronize = ma35d1_synchronize;
> +
> +	spin_lock_init(&pdmadev->lock);
> +	INIT_LIST_HEAD(&pdmadev->ddev.channels);
> +	for (i = 0; i < nr_chans; i++) {
> +		ch = &pdmadev->channels[i];
> +		ch->dev = dev;
> +		ch->pdmadev = pdmadev;
> +		ch->id = i;
> +		ch->base = reg_base;
> +		ch->vc.desc_free = ma35d1_pdma_desc_free;
> +		vchan_init(&ch->vc, &pdmadev->ddev);
> +	}
> +
> +	ret = devm_request_irq(dev, irq, ma35d1_pdma_interrupt, 0, pdev->name, pdmadev);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to request IRQ\n");
> +
> +	ret = dmaenginem_async_device_register(&pdmadev->ddev);
> +	if (ret)
> +		return ret;
> +
> +	ret = of_dma_controller_register(node, ma35d1_of_xlate, pdmadev);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "of_dma_controller_register failed\n");
> +
> +	platform_set_drvdata(pdev, pdmadev);
> +
> +	return 0;
> +}
> +
> +static void ma35d1_pdma_remove(struct platform_device *pdev)
> +{
> +	struct ma35d1_pdmadev *pdmadev = platform_get_drvdata(pdev);
> +	int i;
> +
> +	of_dma_controller_free(pdev->dev.of_node);
> +	writel(0, pdmadev->channels[0].base + PDMA_INTEN);
> +
> +	for (i = 0; i < pdmadev->nr_chans; i++)
> +		tasklet_kill(&pdmadev->channels[i].vc.task);
> +}
> +
> +static const struct of_device_id ma35d1_pdma_match[] = {
> +	{ .compatible = "nuvoton,ma35d1-pdma" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, ma35d1_pdma_match);
> +
> +static struct platform_driver ma35d1_pdma_driver = {
> +	.probe = ma35d1_pdma_probe,
> +	.remove = ma35d1_pdma_remove,
> +	.driver = {
> +		.name = "ma35d1-pdma",
> +		.of_match_table = ma35d1_pdma_match,
> +	},
> +};
> +module_platform_driver(ma35d1_pdma_driver);
> +
> +MODULE_DESCRIPTION("PDMA controller driver for Nuvoton MA35D1");
> +MODULE_AUTHOR("Shan-Chun Hung <shanchun1218@gmail.com>");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/dma/ma35d1-pdma.h b/include/linux/dma/ma35d1-pdma.h
> new file mode 100644
> index 000000000000..13f1410d74da
> --- /dev/null
> +++ b/include/linux/dma/ma35d1-pdma.h
> @@ -0,0 +1,24 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Header file for the MA35D1 PDMA Controller driver
> + *
> + * Copyright (C) 2026 Nuvoton Technology Corp.
> + */
> +#ifndef _MA35D1_PDMA_H
> +#define _MA35D1_PDMA_H
> +
> +/**
> + * struct ma35d1_peripheral - PDMA peripheral configuration
> + * @prescaler: timeout prescaler value
> + * @counter: timeout counter value
> + *
> + * Slave device drivers fill in this structure and pass it via
> + * dma_slave_config.peripheral_config/peripheral_size to configure
> + * the MA35D1 PDMA engine's per-channel timeout detection block.

Hm? Confusing, how is it supposed to be used? Do you just reimplement
board files?

> + */
> +struct ma35d1_peripheral {
> +	unsigned int prescaler;
> +	unsigned int counter;
> +};
> +
> +#endif /* _MA35D1_PDMA_H */
> --
> 2.25.1
>