Devicetree
 help / color / mirror / Atom feed
* [PATCH 0/2] dmaengine: ma35d1-pdma: Add MA35D1 PDMA driver
@ 2026-11-03  2:22 Shan-Chun Hung
  2026-11-03  2:22 ` [PATCH 1/2] dt-bindings: dma: nuvoton,ma35d1-pdma: Document MA35D1 PDMA controller Shan-Chun Hung
  2026-11-03  2:22 ` [PATCH 2/2] dmaengine: ma35d1-pdma: Add MA35D1 PDMA driver Shan-Chun Hung
  0 siblings, 2 replies; 5+ messages in thread
From: Shan-Chun Hung @ 2026-11-03  2:22 UTC (permalink / raw)
  To: ychuang3, schung, vkoul, Frank.Li, robh, krzk+dt, conor+dt, kees,
	gustavoars, linux-arm-kernel, dmaengine, devicetree, linux-kernel,
	linux-hardening
  Cc: 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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] dt-bindings: dma: nuvoton,ma35d1-pdma: Document MA35D1 PDMA controller
  2026-11-03  2:22 [PATCH 0/2] dmaengine: ma35d1-pdma: Add MA35D1 PDMA driver Shan-Chun Hung
@ 2026-11-03  2:22 ` Shan-Chun Hung
  2026-11-03  2:36   ` sashiko-bot
  2026-11-03  2:22 ` [PATCH 2/2] dmaengine: ma35d1-pdma: Add MA35D1 PDMA driver Shan-Chun Hung
  1 sibling, 1 reply; 5+ messages in thread
From: Shan-Chun Hung @ 2026-11-03  2:22 UTC (permalink / raw)
  To: ychuang3, schung, vkoul, Frank.Li, robh, krzk+dt, conor+dt, kees,
	gustavoars, linux-arm-kernel, dmaengine, devicetree, linux-kernel,
	linux-hardening
  Cc: 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] dmaengine: ma35d1-pdma: Add MA35D1 PDMA driver
  2026-11-03  2:22 [PATCH 0/2] dmaengine: ma35d1-pdma: Add MA35D1 PDMA driver Shan-Chun Hung
  2026-11-03  2:22 ` [PATCH 1/2] dt-bindings: dma: nuvoton,ma35d1-pdma: Document MA35D1 PDMA controller Shan-Chun Hung
@ 2026-11-03  2:22 ` Shan-Chun Hung
  2026-11-03  2:43   ` sashiko-bot
  1 sibling, 1 reply; 5+ messages in thread
From: Shan-Chun Hung @ 2026-11-03  2:22 UTC (permalink / raw)
  To: ychuang3, schung, vkoul, Frank.Li, robh, krzk+dt, conor+dt, kees,
	gustavoars, linux-arm-kernel, dmaengine, devicetree, linux-kernel,
	linux-hardening
  Cc: 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] dt-bindings: dma: nuvoton,ma35d1-pdma: Document MA35D1 PDMA controller
  2026-11-03  2:22 ` [PATCH 1/2] dt-bindings: dma: nuvoton,ma35d1-pdma: Document MA35D1 PDMA controller Shan-Chun Hung
@ 2026-11-03  2:36   ` sashiko-bot
  0 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-11-03  2:36 UTC (permalink / raw)
  To: Shan-Chun Hung, dmaengine, devicetree; +Cc: robh, conor+dt, vkoul, Frank.Li

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] dmaengine: ma35d1-pdma: Add MA35D1 PDMA driver
  2026-11-03  2:22 ` [PATCH 2/2] dmaengine: ma35d1-pdma: Add MA35D1 PDMA driver Shan-Chun Hung
@ 2026-11-03  2:43   ` sashiko-bot
  0 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-11-03  2:43 UTC (permalink / raw)
  To: Shan-Chun Hung, dmaengine, devicetree; +Cc: robh, conor+dt, vkoul, Frank.Li

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-11-03  2:43 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-03  2:22 [PATCH 0/2] dmaengine: ma35d1-pdma: Add MA35D1 PDMA driver Shan-Chun Hung
2026-11-03  2:22 ` [PATCH 1/2] dt-bindings: dma: nuvoton,ma35d1-pdma: Document MA35D1 PDMA controller Shan-Chun Hung
2026-11-03  2:36   ` sashiko-bot
2026-11-03  2:22 ` [PATCH 2/2] dmaengine: ma35d1-pdma: Add MA35D1 PDMA driver Shan-Chun Hung
2026-11-03  2:43   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox