* [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; 8+ 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] 8+ 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-04 14:03 ` Krzysztof Kozlowski 2026-11-03 2:22 ` [PATCH 2/2] dmaengine: ma35d1-pdma: Add MA35D1 PDMA driver Shan-Chun Hung 1 sibling, 2 replies; 8+ 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] 8+ 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 2026-11-04 14:03 ` Krzysztof Kozlowski 1 sibling, 0 replies; 8+ 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] 8+ 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 @ 2026-11-04 14:03 ` Krzysztof Kozlowski 1 sibling, 0 replies; 8+ messages in thread From: Krzysztof Kozlowski @ 2026-11-04 14:03 UTC (permalink / raw) To: Shan-Chun Hung Cc: ychuang3, schung, vkoul, Frank.Li, robh, krzk+dt, conor+dt, kees, gustavoars, linux-arm-kernel, dmaengine, devicetree, linux-kernel, linux-hardening 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 ^ permalink raw reply [flat|nested] 8+ 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 2026-11-04 14:06 ` Krzysztof Kozlowski 1 sibling, 2 replies; 8+ 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] 8+ 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 2026-11-04 19:30 ` Frank Li 2026-11-04 14:06 ` Krzysztof Kozlowski 1 sibling, 1 reply; 8+ 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] 8+ messages in thread
* Re: [PATCH 2/2] dmaengine: ma35d1-pdma: Add MA35D1 PDMA driver 2026-11-03 2:43 ` sashiko-bot @ 2026-11-04 19:30 ` Frank Li 0 siblings, 0 replies; 8+ messages in thread From: Frank Li @ 2026-11-04 19:30 UTC (permalink / raw) To: sashiko-reviews Cc: Shan-Chun Hung, dmaengine, devicetree, robh, conor+dt, vkoul, Frank.Li On Tue, Nov 03, 2026 at 02:43:08AM +0000, sashiko-bot@kernel.org wrote: > 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() > -- Please fix these issues. Frank ^ permalink raw reply [flat|nested] 8+ 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 @ 2026-11-04 14:06 ` Krzysztof Kozlowski 1 sibling, 0 replies; 8+ messages in thread From: Krzysztof Kozlowski @ 2026-11-04 14:06 UTC (permalink / raw) To: Shan-Chun Hung Cc: ychuang3, schung, vkoul, Frank.Li, robh, krzk+dt, conor+dt, kees, gustavoars, linux-arm-kernel, dmaengine, devicetree, linux-kernel, linux-hardening 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 > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-11-04 19:30 UTC | newest] Thread overview: 8+ 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-04 14:03 ` Krzysztof Kozlowski 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 2026-11-04 19:30 ` Frank Li 2026-11-04 14:06 ` Krzysztof Kozlowski
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.