[PATCH v11 0/3] Add Amlogic general DMA

[PATCH v11 0/3] Add Amlogic general DMA

[Xianwei Zhao via B4 Relay]

Add DMA driver and bindigns for the Amlogic SoCs.

Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
Changes in v11:
- MAX_DMA_LEN 4-byte align, and return real status when get, and lockless error print.
- Link to v10: https://lore.kernel.org/r/20260710-amlogic-dma-v10-0-ff4deae837e7@amlogic.com

Changes in v10:
- Free dma memory in workqueue.
- Deal with zero len sg (not consume sg_link),and reset status to DMA_COMPLETE when terminal channel.
- Add desc for #dma-cells in dt-bindings.
- Link to v9: https://lore.kernel.org/r/20260626-amlogic-dma-v9-0-558d672c4a95@amlogic.com

Changes in v9:
- Use each transmission request sg_link mem instead of the loop mem get.
- Fix some hidden issues which reviewed by ai robot.
- Link to v8: https://lore.kernel.org/r/20260521-amlogic-dma-v8-0-86cc2ce94142@amlogic.com

Changes in v8:
- Use kzalloc instead of kmalloc.
- Initialize the temporary variable and fix a spelling mistake.
- Link to v7: https://lore.kernel.org/r/20260324-amlogic-dma-v7-0-f8b91ee192c1@amlogic.com

Changes in v7:
- Take use vchan to support mltiple txns.
- Link to v6: https://lore.kernel.org/r/20260309-amlogic-dma-v6-0-63349d23bd4b@amlogic.com

Changes in v6:
- Some minor modifications according to Frank's suggestion.
- Link to v5: https://lore.kernel.org/r/20260304-amlogic-dma-v5-0-aa453d14fd43@amlogic.com

Changes in v5:
- Rename head file and rename macro definition.
- Rename the subject in [2/3] from "dma" to "dmaengine".
- Link to v4: https://lore.kernel.org/r/20260227-amlogic-dma-v4-0-f25e4614e9b7@amlogic.com

Changes in v4:
- Support split transfer when data len > MAX_LEN.
- When a module fails or exits, perform de-initialization.
- Some other minor modifications.
- Link to v3: https://lore.kernel.org/r/20260206-amlogic-dma-v3-0-56fb9f59ed22@amlogic.com

Changes in v3:
- Adjust the format of binding according to Frank's suggestion.
- Some code format modified according to Frank's suggestion.
- Support one prep_sg and one submit, drop multi prep_sg and one submit.
- Keep pre state when resume from pause status.
- Link to v2: https://lore.kernel.org/r/20260127-amlogic-dma-v2-0-4525d327d74d@amlogic.com

Changes in v2:
- Introduce what the DMA is used for in the A9 SoC.
- Some minor modifications were made according to Krzysztof's suggestions.
- Some modifications were made according to Neil's suggestions.
- Fix a build error.
- Link to v1: https://lore.kernel.org/r/20251216-amlogic-dma-v1-0-e289e57e96a7@amlogic.com

---
Xianwei Zhao (3):
      dt-bindings: dma: Add Amlogic A9 SoC DMA
      dmaengine: amlogic: Add general DMA driver for A9
      MAINTAINERS: Add an entry for Amlogic DMA driver

 .../devicetree/bindings/dma/amlogic,a9-dma.yaml    |  68 ++
 MAINTAINERS                                        |   7 +
 drivers/dma/Kconfig                                |  10 +
 drivers/dma/Makefile                               |   1 +
 drivers/dma/amlogic-dma.c                          | 726 +++++++++++++++++++++
 include/dt-bindings/dma/amlogic,a9-dma.h           |   8 +
 6 files changed, 820 insertions(+)
---
base-commit: f53b2c30a192b35064be2584df7a800a8e6ac710
change-id: 20251215-amlogic-dma-79477d5cd264

Best regards,
-- 
Xianwei Zhao <xianwei.zhao@amlogic.com>



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH v11 1/3] dt-bindings: dma: Add Amlogic A9 SoC DMA

[Xianwei Zhao via B4 Relay]

From: Xianwei Zhao <xianwei.zhao@amlogic.com>

Add documentation describing the Amlogic A9 SoC DMA. And add
the properties specific values defines into a new include file.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
 .../devicetree/bindings/dma/amlogic,a9-dma.yaml    | 68 ++++++++++++++++++++++
 include/dt-bindings/dma/amlogic,a9-dma.h           |  8 +++
 2 files changed, 76 insertions(+)

diff --git a/Documentation/devicetree/bindings/dma/amlogic,a9-dma.yaml b/Documentation/devicetree/bindings/dma/amlogic,a9-dma.yaml
new file mode 100644
index 000000000000..6cedd5fdfefd
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/amlogic,a9-dma.yaml
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/dma/amlogic,a9-dma.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Amlogic general DMA controller
+
+description:
+  This is a general-purpose peripheral DMA controller. It currently supports
+  major peripherals including I2C, I3C, PIO, and CAN-BUS. Transmit and receive
+  for the same peripheral use two separate channels, controlled by different
+  register sets. I2C and I3C transfer data in 1-byte units, while PIO and
+  CAN-BUS transfer data in 4-byte units. From the controller’s perspective,
+  there is no significant difference.
+
+maintainers:
+  - Xianwei Zhao <xianwei.zhao@amlogic.com>
+
+properties:
+  compatible:
+    const: amlogic,a9-dma
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: sys
+
+  '#dma-cells':
+    const: 2
+    description:
+      The first cell is the DMA channel type(DMA_TX or DMA_RX).
+      The second cell is the DMA channel index.
+
+  dma-channels:
+    maximum: 64
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - '#dma-cells'
+  - dma-channels
+
+allOf:
+  - $ref: dma-controller.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    dma-controller@fe400000 {
+        compatible = "amlogic,a9-dma";
+        reg = <0xfe400000 0x4000>;
+        interrupts = <GIC_SPI 35 IRQ_TYPE_EDGE_RISING>;
+        clocks = <&clkc 45>;
+        #dma-cells = <2>;
+        dma-channels = <28>;
+    };
diff --git a/include/dt-bindings/dma/amlogic,a9-dma.h b/include/dt-bindings/dma/amlogic,a9-dma.h
new file mode 100644
index 000000000000..c59c2fd4b956
--- /dev/null
+++ b/include/dt-bindings/dma/amlogic,a9-dma.h
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
+
+#ifndef __DT_BINDINGS_DMA_AMLOGIC_DMA_H__
+#define __DT_BINDINGS_DMA_AMLOGIC_DMA_H__
+
+#define DMA_TX			0
+#define DMA_RX			1
+#endif /* __DT_BINDINGS_DMA_AMLOGIC_DMA_H__ */

-- 
2.52.0



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH v11 2/3] dmaengine: amlogic: Add general DMA driver for A9

[Xianwei Zhao via B4 Relay]

From: Xianwei Zhao <xianwei.zhao@amlogic.com>

Amlogic A9 SoCs include a general-purpose DMA controller that can be used
by multiple peripherals, such as I2C PIO and I3C. Each peripheral group
is associated with a dedicated DMA channel in hardware.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
 drivers/dma/Kconfig       |  10 +
 drivers/dma/Makefile      |   1 +
 drivers/dma/amlogic-dma.c | 726 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 737 insertions(+)

diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index ae6a682c9f76..01f96a8257e5 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -85,6 +85,16 @@ config AMCC_PPC440SPE_ADMA
 	help
 	  Enable support for the AMCC PPC440SPe RAID engines.
 
+config AMLOGIC_DMA
+	tristate "Amlogic general DMA support"
+	depends on ARCH_MESON || COMPILE_TEST
+	select DMA_ENGINE
+	select DMA_VIRTUAL_CHANNELS
+	select REGMAP_MMIO
+	help
+	  Enable support for the Amlogic general DMA engines. THis DMA
+	  controller is used some Amlogic SoCs, such as A9.
+
 config APPLE_ADMAC
 	tristate "Apple ADMAC support"
 	depends on ARCH_APPLE || COMPILE_TEST
diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
index 14aa086629d5..f62d12b08e15 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_DMATEST) += dmatest.o
 obj-$(CONFIG_ALTERA_MSGDMA) += altera-msgdma.o
 obj-$(CONFIG_AMBA_PL08X) += amba-pl08x.o
 obj-$(CONFIG_AMCC_PPC440SPE_ADMA) += ppc4xx/
+obj-$(CONFIG_AMLOGIC_DMA) += amlogic-dma.o
 obj-$(CONFIG_APPLE_ADMAC) += apple-admac.o
 obj-$(CONFIG_ARM_DMA350) += arm-dma350.o
 obj-$(CONFIG_AT_HDMAC) += at_hdmac.o
diff --git a/drivers/dma/amlogic-dma.c b/drivers/dma/amlogic-dma.c
new file mode 100644
index 000000000000..9de650a79aba
--- /dev/null
+++ b/drivers/dma/amlogic-dma.c
@@ -0,0 +1,726 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR MIT)
+/*
+ * Copyright (C) 2025 Amlogic, Inc. All rights reserved
+ * Author: Xianwei Zhao <xianwei.zhao@amlogic.com>
+ */
+
+#include <dt-bindings/dma/amlogic,a9-dma.h>
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
+#include <linux/interrupt.h>
+#include <linux/init.h>
+#include <linux/list.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_dma.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+#include "virt-dma.h"
+
+#define RCH_REG_BASE		0x0
+#define WCH_REG_BASE		0x2000
+/*
+ * Each rch (read from memory) REG offset  Rch_offset 0x0 each channel total 0x40
+ * rch addr = DMA_base + Rch_offset+ chan_id * 0x40 + reg_offset
+ */
+#define RCH_READY		0x0
+#define RCH_STATUS		0x4
+#define RCH_CFG			0x8
+#define CFG_CLEAR		BIT(25)
+#define CFG_PAUSE		BIT(26)
+#define CFG_ENABLE		BIT(27)
+#define CFG_DONE		BIT(28)
+#define RCH_ADDR		0xc
+#define RCH_LEN			0x10
+#define RCH_RD_LEN		0x14
+#define RCH_PRT			0x18
+#define RCH_SYCN_STAT		0x1c
+#define RCH_ADDR_LOW		0x20
+#define RCH_ADDR_HIGH		0x24
+/* if work on 64, it work with RCH_PRT */
+#define RCH_PTR_HIGH		0x28
+
+/*
+ * Each wch (write to memory) REG offset  Wch_offset 0x2000 each channel total 0x40
+ * wch addr = DMA_base + Wch_offset+ chan_id * 0x40 + reg_offset
+ */
+#define WCH_READY		0x0
+#define WCH_TOTAL_LEN		0x4
+#define WCH_CFG			0x8
+#define WCH_ADDR		0xc
+#define WCH_LEN			0x10
+#define WCH_RD_LEN		0x14
+#define WCH_PRT			0x18
+#define WCH_CMD_CNT		0x1c
+#define WCH_ADDR_LOW		0x20
+#define WCH_ADDR_HIGH		0x24
+/* if work on 64, it work with RCH_PRT */
+#define WCH_PTR_HIGH		0x28
+
+/* DMA controller reg */
+#define RCH_INT_MASK		0x1000
+#define WCH_INT_MASK		0x1004
+#define CLEAR_W_BATCH		0x1014
+#define CLEAR_RCH		0x1024
+#define CLEAR_WCH		0x1028
+#define RCH_ACTIVE		0x1038
+#define WCH_ACTIVE		0x103c
+#define RCH_DONE		0x104c
+#define WCH_DONE		0x1050
+#define RCH_ERR			0x1060
+#define RCH_LEN_ERR		0x1064
+#define WCH_ERR			0x1068
+#define DMA_BATCH_END		0x1078
+#define WCH_EOC_DONE		0x1088
+#define WDMA_RESP_ERR		0x1098
+#define UPT_PKT_SYNC		0x10a8
+#define RCHN_CFG		0x10ac
+#define WCHN_CFG		0x10b0
+#define MEM_PD_CFG		0x10b4
+#define MEM_BUS_CFG		0x10b8
+#define DMA_GMV_CFG		0x10bc
+#define DMA_GMR_CFG		0x10c0
+
+#define MAX_CHAN_ID		32
+#define SG_MAX_LEN		(GENMASK(26, 0) & ~0x3)
+
+struct aml_dma_sg_link {
+#define LINK_LEN		GENMASK(26, 0)
+#define LINK_IRQ		BIT(27)
+#define LINK_EOC		BIT(28)
+#define LINK_LOOP		BIT(29)
+#define LINK_ERR		BIT(30)
+#define LINK_OWNER		BIT(31)
+	u32 ctl;
+	u32 addr_low;
+	u32 addr_high;
+	u32 revered;
+} __packed;
+
+/* 1 page for link 256*16 */
+#define DMA_MAX_LINK		256
+/* sizeof(struct aml_dma_sg_link) */
+#define DMA_LINK_SIZE		16
+#define DMA_LINK_MAX_SIZE	(DMA_LINK_SIZE * DMA_MAX_LINK)
+
+struct aml_dma_desc {
+	struct virt_dma_desc		vd;
+	struct aml_dma_sg_link		*sg_link;
+	struct dma_device		*dma_device;
+	dma_addr_t			sg_link_phys;
+	size_t				sg_link_size;
+	u32				data_len;
+	struct work_struct		free_work;
+};
+
+struct aml_dma_chan {
+	struct virt_dma_chan		vchan;
+	struct aml_dma_dev		*aml_dma;
+	struct aml_dma_desc		*cur_desc;
+	enum dma_status			pre_status;
+	enum dma_status			status;
+	enum dma_transfer_direction	direction;
+	int				chan_id;
+	/* reg_base (direction + chan_id) */
+	int				reg_offs;
+	/* When there are multiple consecutive transmission errors, this chanel halt */
+	int				err_num;
+};
+
+struct aml_dma_dev {
+	struct dma_device		dma_device;
+	void __iomem			*base;
+	struct regmap			*regmap;
+	struct clk			*clk;
+	int				irq;
+	struct platform_device		*pdev;
+	struct aml_dma_chan		*aml_rch[MAX_CHAN_ID];
+	struct aml_dma_chan		*aml_wch[MAX_CHAN_ID];
+	unsigned int			chan_nr;
+	unsigned int			chan_used;
+	struct aml_dma_chan		aml_chans[]__counted_by(chan_nr);
+};
+
+static inline struct aml_dma_chan *to_aml_dma_chan(struct dma_chan *chan)
+{
+	return container_of(chan, struct aml_dma_chan, vchan.chan);
+}
+
+static inline struct aml_dma_desc *to_aml_dma_desc(struct virt_dma_desc *vd)
+{
+	return container_of(vd, struct aml_dma_desc, vd);
+}
+
+static void aml_dma_free_desc_work(struct work_struct *work)
+{
+	struct aml_dma_desc *aml_desc =
+			container_of(work, struct aml_dma_desc, free_work);
+
+	dma_free_coherent(aml_desc->dma_device->dev,
+			  aml_desc->sg_link_size,
+			  aml_desc->sg_link,
+			  aml_desc->sg_link_phys);
+	kfree(aml_desc);
+}
+
+static void aml_dma_free_desc(struct virt_dma_desc *vd)
+{
+	struct aml_dma_desc *aml_desc = to_aml_dma_desc(vd);
+
+	schedule_work(&aml_desc->free_work);
+}
+
+static int aml_dma_alloc_chan_resources(struct dma_chan *chan)
+{
+	struct aml_dma_chan *aml_chan = to_aml_dma_chan(chan);
+	struct aml_dma_dev *aml_dma = aml_chan->aml_dma;
+
+	/* offset is the same RCH_CFG and WCH_CFG */
+	regmap_set_bits(aml_dma->regmap, aml_chan->reg_offs + RCH_CFG, CFG_CLEAR);
+	regmap_clear_bits(aml_dma->regmap, aml_chan->reg_offs + RCH_CFG, CFG_PAUSE);
+	regmap_clear_bits(aml_dma->regmap, aml_chan->reg_offs + RCH_CFG, CFG_CLEAR);
+
+	aml_chan->status = DMA_COMPLETE;
+	aml_chan->cur_desc = NULL;
+	aml_chan->err_num = 0;
+
+	return 0;
+}
+
+static void aml_dma_free_chan_resources(struct dma_chan *chan)
+{
+	struct aml_dma_chan *aml_chan = to_aml_dma_chan(chan);
+	struct aml_dma_dev *aml_dma = aml_chan->aml_dma;
+	struct virt_dma_desc *cur_vd = NULL;
+	unsigned long flags;
+
+	spin_lock_irqsave(&aml_chan->vchan.lock, flags);
+	regmap_set_bits(aml_dma->regmap, aml_chan->reg_offs + RCH_CFG, CFG_PAUSE);
+	regmap_set_bits(aml_dma->regmap, aml_chan->reg_offs + RCH_CFG, CFG_CLEAR);
+	if (aml_chan->cur_desc)
+		cur_vd = &aml_chan->cur_desc->vd;
+	aml_chan->cur_desc = NULL;
+	spin_unlock_irqrestore(&aml_chan->vchan.lock, flags);
+	if (cur_vd)
+		aml_dma_free_desc(cur_vd);
+
+	vchan_free_chan_resources(&aml_chan->vchan);
+}
+
+/* DMA transfer state  update how many data reside it */
+static enum dma_status aml_dma_tx_status(struct dma_chan *chan,
+					 dma_cookie_t cookie,
+					 struct dma_tx_state *txstate)
+{
+	struct aml_dma_chan *aml_chan = to_aml_dma_chan(chan);
+	struct aml_dma_dev *aml_dma = aml_chan->aml_dma;
+	struct aml_dma_desc *aml_desc = NULL;
+	struct virt_dma_desc *vd;
+	u32 residue = 0, done;
+	unsigned long flags;
+	enum dma_status ret;
+
+	ret = dma_cookie_status(chan, cookie, txstate);
+	if (ret == DMA_COMPLETE)
+		return ret;
+
+	ret = aml_chan->status;
+	if (!txstate)
+		return ret;
+
+	spin_lock_irqsave(&aml_chan->vchan.lock, flags);
+	vd = vchan_find_desc(&aml_chan->vchan, cookie);
+	spin_unlock_irqrestore(&aml_chan->vchan.lock, flags);
+	if (vd) {
+		aml_desc = to_aml_dma_desc(vd);
+		residue = aml_desc->data_len;
+	} else if (aml_chan->cur_desc && aml_chan->cur_desc->vd.tx.cookie == cookie) {
+		aml_desc = aml_chan->cur_desc;
+		regmap_read(aml_dma->regmap, aml_chan->reg_offs + RCH_RD_LEN, &done);
+		residue = aml_desc->data_len - done;
+	} else {
+		dev_err(aml_dma->dma_device.dev, "cookie error\n");
+	}
+	dma_set_residue(txstate, residue);
+
+	return ret;
+}
+
+static struct dma_async_tx_descriptor *aml_dma_prep_slave_sg
+		(struct dma_chan *chan, struct scatterlist *sgl,
+		unsigned int sg_len, enum dma_transfer_direction direction,
+		unsigned long flags, void *context)
+{
+	struct aml_dma_chan *aml_chan = to_aml_dma_chan(chan);
+	struct aml_dma_dev *aml_dma = aml_chan->aml_dma;
+	struct aml_dma_desc *aml_desc = NULL;
+	struct aml_dma_sg_link *sg_link = NULL;
+	struct scatterlist *sg = NULL;
+	u64 paddr;
+	u32 link_count, avail;
+	u32 i;
+
+	if (aml_chan->direction != direction) {
+		dev_err(aml_dma->dma_device.dev, "direction not support\n");
+		return NULL;
+	}
+
+	link_count = sg_nents_for_dma(sgl, sg_len, SG_MAX_LEN);
+	if (link_count == 0)
+		return NULL;
+
+	aml_desc = kzalloc_obj(*aml_desc, GFP_NOWAIT);
+	if (!aml_desc)
+		return NULL;
+
+	INIT_WORK(&aml_desc->free_work, aml_dma_free_desc_work);
+	aml_desc->sg_link_size = link_count * sizeof(*sg_link);
+	aml_desc->sg_link = dma_alloc_coherent(aml_dma->dma_device.dev, aml_desc->sg_link_size,
+					       &aml_desc->sg_link_phys, GFP_NOWAIT);
+	if (!aml_desc->sg_link) {
+		kfree(aml_desc);
+		return NULL;
+	}
+	aml_desc->dma_device = &aml_dma->dma_device;
+
+	sg_link = aml_desc->sg_link;
+	for_each_sg(sgl, sg, sg_len, i) {
+		avail = sg_dma_len(sg);
+		if (avail == 0)
+			continue;
+		paddr = sg->dma_address;
+		while (avail > SG_MAX_LEN) {
+			/* set dma address and len to sglink*/
+			sg_link->addr_low = lower_32_bits(paddr);
+			sg_link->addr_high = upper_32_bits(paddr);
+			sg_link->ctl = FIELD_PREP(LINK_LEN, SG_MAX_LEN);
+			paddr = paddr + SG_MAX_LEN;
+			avail = avail - SG_MAX_LEN;
+			sg_link++;
+		}
+		/* set dma address and len to sglink*/
+		sg_link->addr_low = lower_32_bits(paddr);
+		sg_link->addr_high = upper_32_bits(paddr);
+		sg_link->ctl = FIELD_PREP(LINK_LEN, avail);
+
+		aml_desc->data_len += sg_dma_len(sg);
+		sg_link++;
+	}
+
+	/* the last sg set eoc flag */
+	sg_link--;
+	sg_link->ctl |= LINK_EOC;
+
+	return vchan_tx_prep(&aml_chan->vchan, &aml_desc->vd, flags);
+}
+
+static int aml_dma_chan_pause(struct dma_chan *chan)
+{
+	struct aml_dma_chan *aml_chan = to_aml_dma_chan(chan);
+	struct aml_dma_dev *aml_dma = aml_chan->aml_dma;
+	unsigned long flags;
+
+	spin_lock_irqsave(&aml_chan->vchan.lock, flags);
+	regmap_set_bits(aml_dma->regmap, aml_chan->reg_offs + RCH_CFG, CFG_PAUSE);
+	aml_chan->pre_status = aml_chan->status;
+	aml_chan->status = DMA_PAUSED;
+	spin_unlock_irqrestore(&aml_chan->vchan.lock, flags);
+
+	return 0;
+}
+
+static int aml_dma_chan_resume(struct dma_chan *chan)
+{
+	struct aml_dma_chan *aml_chan = to_aml_dma_chan(chan);
+	struct aml_dma_dev *aml_dma = aml_chan->aml_dma;
+	unsigned long flags;
+
+	spin_lock_irqsave(&aml_chan->vchan.lock, flags);
+	regmap_clear_bits(aml_dma->regmap, aml_chan->reg_offs + RCH_CFG, CFG_PAUSE);
+	aml_chan->status = aml_chan->pre_status;
+	spin_unlock_irqrestore(&aml_chan->vchan.lock, flags);
+
+	return 0;
+}
+
+static int aml_dma_terminate_all(struct dma_chan *chan)
+{
+	struct aml_dma_chan *aml_chan = to_aml_dma_chan(chan);
+	struct aml_dma_dev *aml_dma = aml_chan->aml_dma;
+	int chan_id = aml_chan->chan_id;
+	struct virt_dma_desc *cur_vd;
+	unsigned long flags;
+	LIST_HEAD(head);
+
+	spin_lock_irqsave(&aml_chan->vchan.lock, flags);
+	regmap_set_bits(aml_dma->regmap, aml_chan->reg_offs + RCH_CFG, CFG_PAUSE);
+	regmap_set_bits(aml_dma->regmap, aml_chan->reg_offs + RCH_CFG, CFG_CLEAR);
+
+	if (aml_chan->direction == DMA_MEM_TO_DEV)
+		regmap_set_bits(aml_dma->regmap, RCH_INT_MASK, BIT(chan_id));
+	else if (aml_chan->direction == DMA_DEV_TO_MEM)
+		regmap_set_bits(aml_dma->regmap, WCH_INT_MASK, BIT(chan_id));
+
+	vchan_get_all_descriptors(&aml_chan->vchan, &head);
+	cur_vd = &aml_chan->cur_desc->vd;
+	aml_chan->cur_desc = NULL;
+	spin_unlock_irqrestore(&aml_chan->vchan.lock, flags);
+	if (cur_vd)
+		aml_dma_free_desc(cur_vd);
+
+	vchan_dma_desc_free_list(&aml_chan->vchan, &head);
+
+	regmap_clear_bits(aml_dma->regmap, aml_chan->reg_offs + RCH_CFG, CFG_PAUSE);
+	regmap_clear_bits(aml_dma->regmap, aml_chan->reg_offs + RCH_CFG, CFG_CLEAR);
+	aml_chan->status = DMA_COMPLETE;
+	aml_chan->err_num = 0;
+
+	return 0;
+}
+
+static void aml_dma_start(struct aml_dma_chan *aml_chan)
+{
+	struct virt_dma_desc *vd = vchan_next_desc(&aml_chan->vchan);
+	struct aml_dma_dev *aml_dma = aml_chan->aml_dma;
+	struct aml_dma_desc *aml_desc = NULL;
+	int chan_id = aml_chan->chan_id;
+
+	if (aml_chan->status == DMA_ERROR) {
+		if (aml_chan->err_num > 5) {
+			dev_err(aml_dma->dma_device.dev, "hw error\n");
+			return;
+		}
+		regmap_set_bits(aml_dma->regmap, aml_chan->reg_offs + RCH_CFG, CFG_PAUSE);
+		regmap_set_bits(aml_dma->regmap, aml_chan->reg_offs + RCH_CFG, CFG_CLEAR);
+		aml_chan->err_num++;
+		regmap_clear_bits(aml_dma->regmap, aml_chan->reg_offs + RCH_CFG, CFG_PAUSE);
+		regmap_clear_bits(aml_dma->regmap, aml_chan->reg_offs + RCH_CFG, CFG_CLEAR);
+		aml_chan->status = DMA_COMPLETE;
+	} else {
+		aml_chan->err_num = 0;
+	}
+
+	if (!vd)
+		return;
+
+	if (aml_chan->status != DMA_COMPLETE)
+		return;
+
+	list_del(&vd->node);
+	aml_desc = to_aml_dma_desc(vd);
+	aml_chan->cur_desc = aml_desc;
+
+	if (aml_chan->direction == DMA_MEM_TO_DEV) {
+		regmap_write(aml_dma->regmap, aml_chan->reg_offs + RCH_ADDR,
+			     lower_32_bits(aml_desc->sg_link_phys));
+		regmap_write(aml_dma->regmap, aml_chan->reg_offs + RCH_ADDR_HIGH,
+			     upper_32_bits(aml_desc->sg_link_phys));
+		regmap_write(aml_dma->regmap, aml_chan->reg_offs + RCH_LEN, aml_desc->data_len);
+		regmap_clear_bits(aml_dma->regmap, RCH_INT_MASK, BIT(chan_id));
+		/* for rch (tx) need set cfg 0 to trigger start */
+		regmap_write(aml_dma->regmap, aml_chan->reg_offs + RCH_CFG, 0);
+	} else if (aml_chan->direction == DMA_DEV_TO_MEM) {
+		regmap_write(aml_dma->regmap, aml_chan->reg_offs + WCH_ADDR,
+			     lower_32_bits(aml_desc->sg_link_phys));
+		regmap_write(aml_dma->regmap, aml_chan->reg_offs + WCH_ADDR_HIGH,
+			     upper_32_bits(aml_desc->sg_link_phys));
+		regmap_write(aml_dma->regmap, aml_chan->reg_offs + WCH_LEN, aml_desc->data_len);
+		regmap_clear_bits(aml_dma->regmap, WCH_INT_MASK, BIT(chan_id));
+	}
+}
+
+static void aml_dma_issue_pending(struct dma_chan *chan)
+{
+	struct aml_dma_chan *aml_chan = to_aml_dma_chan(chan);
+	unsigned long flags;
+
+	spin_lock_irqsave(&aml_chan->vchan.lock, flags);
+	if (vchan_issue_pending(&aml_chan->vchan) && !aml_chan->cur_desc)
+		aml_dma_start(aml_chan);
+	spin_unlock_irqrestore(&aml_chan->vchan.lock, flags);
+}
+
+static irqreturn_t aml_dma_interrupt_handler(int irq, void *dev_id)
+{
+	struct aml_dma_dev *aml_dma = dev_id;
+	struct aml_dma_chan *aml_chan;
+	struct aml_dma_desc *aml_desc;
+	u32 done, eoc_done, err, err_l, end;
+	u32 cpl_data;
+	int i = 0;
+
+	/* deal with rch normal complete and error */
+	regmap_read(aml_dma->regmap, RCH_DONE, &done);
+	regmap_read(aml_dma->regmap, RCH_ERR, &err);
+	regmap_read(aml_dma->regmap, RCH_LEN_ERR, &err_l);
+	err = err | err_l;
+
+	done = done | err;
+
+	while (done) {
+		i = ffs(done) - 1;
+		regmap_write(aml_dma->regmap, CLEAR_RCH, BIT(i));
+		done &= ~BIT(i);
+		aml_chan = aml_dma->aml_rch[i];
+		if (!aml_chan) {
+			dev_err(aml_dma->dma_device.dev, "idx %d rch not initialized\n", i);
+			continue;
+		}
+		spin_lock(&aml_chan->vchan.lock);
+		aml_chan->status = (err & BIT(i)) ? DMA_ERROR : DMA_COMPLETE;
+		aml_desc = aml_chan->cur_desc;
+		if (!aml_desc) {
+			spin_unlock(&aml_chan->vchan.lock);
+			continue;
+		}
+		if (aml_chan->status == DMA_ERROR) {
+			aml_desc->vd.tx_result.result = DMA_TRANS_READ_FAILED;
+			regmap_read(aml_dma->regmap, aml_chan->reg_offs + RCH_RD_LEN, &cpl_data);
+			aml_desc->vd.tx_result.residue = aml_desc->data_len - cpl_data;
+		}
+		vchan_cookie_complete(&aml_desc->vd);
+		aml_chan->cur_desc = NULL;
+		aml_dma_start(aml_chan);
+		spin_unlock(&aml_chan->vchan.lock);
+	}
+
+	/* deal with wch normal complete and error */
+	regmap_read(aml_dma->regmap, DMA_BATCH_END, &end);
+	if (end)
+		regmap_write(aml_dma->regmap, CLEAR_W_BATCH, end);
+
+	regmap_read(aml_dma->regmap, WCH_DONE, &done);
+	regmap_read(aml_dma->regmap, WCH_EOC_DONE, &eoc_done);
+	done = done | eoc_done;
+
+	regmap_read(aml_dma->regmap, WCH_ERR, &err);
+	regmap_read(aml_dma->regmap, WDMA_RESP_ERR, &err_l);
+	err = err | err_l;
+
+	done = done | err;
+	i = 0;
+	while (done) {
+		i = ffs(done) - 1;
+		done &= ~BIT(i);
+		regmap_write(aml_dma->regmap, CLEAR_WCH, BIT(i));
+		aml_chan = aml_dma->aml_wch[i];
+		if (!aml_chan) {
+			dev_err(aml_dma->dma_device.dev, "idx %d wch not initialized\n", i);
+			continue;
+		}
+		spin_lock(&aml_chan->vchan.lock);
+		aml_chan->status = (err & BIT(i)) ? DMA_ERROR : DMA_COMPLETE;
+		aml_desc = aml_chan->cur_desc;
+		if (!aml_desc) {
+			spin_unlock(&aml_chan->vchan.lock);
+			continue;
+		}
+		if (aml_chan->status == DMA_ERROR) {
+			aml_desc->vd.tx_result.result = DMA_TRANS_WRITE_FAILED;
+			regmap_read(aml_dma->regmap, aml_chan->reg_offs + RCH_RD_LEN, &cpl_data);
+			aml_desc->vd.tx_result.residue = aml_desc->data_len - cpl_data;
+		}
+		vchan_cookie_complete(&aml_desc->vd);
+		aml_chan->cur_desc = NULL;
+		aml_dma_start(aml_chan);
+		spin_unlock(&aml_chan->vchan.lock);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static struct dma_chan *aml_of_dma_xlate(struct of_phandle_args *dma_spec, struct of_dma *ofdma)
+{
+	struct aml_dma_dev *aml_dma = (struct aml_dma_dev *)ofdma->of_dma_data;
+	struct aml_dma_chan *aml_chan = NULL;
+	u32 type;
+	u32 phy_chan_id;
+
+	if (dma_spec->args_count != 2)
+		return NULL;
+
+	type = dma_spec->args[0];
+	phy_chan_id = dma_spec->args[1];
+
+	if (phy_chan_id >= MAX_CHAN_ID)
+		return NULL;
+
+	if (type == DMA_TX) {
+		aml_chan = aml_dma->aml_rch[phy_chan_id];
+		if (!aml_chan) {
+			if (aml_dma->chan_used >= aml_dma->chan_nr) {
+				dev_err(aml_dma->dma_device.dev, "some dma clients err used\n");
+				return NULL;
+			}
+			aml_chan = &aml_dma->aml_chans[aml_dma->chan_used];
+			aml_dma->chan_used++;
+			aml_chan->direction = DMA_MEM_TO_DEV;
+			aml_chan->chan_id = phy_chan_id;
+			aml_chan->reg_offs = RCH_REG_BASE + 0x40 * aml_chan->chan_id;
+			aml_dma->aml_rch[phy_chan_id] = aml_chan;
+		}
+	} else if (type == DMA_RX) {
+		aml_chan = aml_dma->aml_wch[phy_chan_id];
+		if (!aml_chan) {
+			if (aml_dma->chan_used >= aml_dma->chan_nr) {
+				dev_err(aml_dma->dma_device.dev, "some dma clients err used\n");
+				return NULL;
+			}
+			aml_chan = &aml_dma->aml_chans[aml_dma->chan_used];
+			aml_dma->chan_used++;
+			aml_chan->direction = DMA_DEV_TO_MEM;
+			aml_chan->chan_id = phy_chan_id;
+			aml_chan->reg_offs = WCH_REG_BASE + 0x40 * aml_chan->chan_id;
+			aml_dma->aml_wch[phy_chan_id] = aml_chan;
+		}
+	} else {
+		dev_err(aml_dma->dma_device.dev, "type %d not supported\n", type);
+		return NULL;
+	}
+
+	return dma_get_slave_channel(&aml_chan->vchan.chan);
+}
+
+static int aml_dma_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct dma_device *dma_dev;
+	struct aml_dma_dev *aml_dma;
+	int ret, i, len;
+	u32 chan_nr;
+
+	const struct regmap_config aml_regmap_config = {
+		.reg_bits = 32,
+		.val_bits = 32,
+		.reg_stride = 4,
+		.max_register = 0x3000,
+	};
+
+	ret = of_property_read_u32(np, "dma-channels", &chan_nr);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "failed to read dma-channels\n");
+	if (chan_nr > (MAX_CHAN_ID * 2))
+		return dev_err_probe(&pdev->dev, -EINVAL, "dma-channels unusual\n");
+
+	len = sizeof(struct aml_dma_dev) + sizeof(struct aml_dma_chan) * chan_nr;
+	aml_dma = devm_kzalloc(&pdev->dev, len, GFP_KERNEL);
+	if (!aml_dma)
+		return -ENOMEM;
+
+	aml_dma->chan_nr = chan_nr;
+
+	aml_dma->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(aml_dma->base))
+		return PTR_ERR(aml_dma->base);
+
+	aml_dma->regmap = devm_regmap_init_mmio(&pdev->dev, aml_dma->base,
+						&aml_regmap_config);
+	if (IS_ERR_OR_NULL(aml_dma->regmap))
+		return PTR_ERR(aml_dma->regmap);
+
+	aml_dma->clk = devm_clk_get_enabled(&pdev->dev, NULL);
+	if (IS_ERR(aml_dma->clk))
+		return PTR_ERR(aml_dma->clk);
+
+	aml_dma->irq = platform_get_irq(pdev, 0);
+
+	aml_dma->pdev = pdev;
+	aml_dma->dma_device.dev = &pdev->dev;
+
+	dma_dev = &aml_dma->dma_device;
+	INIT_LIST_HEAD(&dma_dev->channels);
+
+	/* Initialize channel parameters */
+	for (i = 0; i < chan_nr; i++) {
+		struct aml_dma_chan *aml_chan = &aml_dma->aml_chans[i];
+
+		aml_chan->aml_dma = aml_dma;
+		aml_chan->vchan.desc_free = aml_dma_free_desc;
+		vchan_init(&aml_chan->vchan, &aml_dma->dma_device);
+	}
+	aml_dma->chan_used = 0;
+
+	dma_set_max_seg_size(dma_dev->dev, SG_MAX_LEN);
+	dma_cap_set(DMA_SLAVE, dma_dev->cap_mask);
+	dma_dev->device_alloc_chan_resources = aml_dma_alloc_chan_resources;
+	dma_dev->device_free_chan_resources = aml_dma_free_chan_resources;
+	dma_dev->device_tx_status = aml_dma_tx_status;
+	dma_dev->device_prep_slave_sg = aml_dma_prep_slave_sg;
+	dma_dev->device_pause = aml_dma_chan_pause;
+	dma_dev->device_resume = aml_dma_chan_resume;
+	dma_dev->device_terminate_all = aml_dma_terminate_all;
+	dma_dev->device_issue_pending = aml_dma_issue_pending;
+	/* PIO 4 bytes and I2C 1 byte */
+	dma_dev->dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) | BIT(DMA_SLAVE_BUSWIDTH_1_BYTE);
+	dma_dev->directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
+	dma_dev->residue_granularity = DMA_RESIDUE_GRANULARITY_BURST;
+
+	regmap_write(aml_dma->regmap, RCH_INT_MASK, 0xffffffff);
+	regmap_write(aml_dma->regmap, WCH_INT_MASK, 0xffffffff);
+
+	ret = devm_request_irq(&pdev->dev, aml_dma->irq, aml_dma_interrupt_handler,
+			       0, dev_name(&pdev->dev), aml_dma);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "failed to register irq\n");
+
+	ret = dmaenginem_async_device_register(dma_dev);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "failed to register dmaenginem\n");
+
+	ret = of_dma_controller_register(np, aml_of_dma_xlate, aml_dma);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "failed to register xlate\n");
+
+	platform_set_drvdata(pdev, aml_dma);
+
+	return 0;
+}
+
+static void aml_dma_remove(struct platform_device *pdev)
+{
+	struct aml_dma_dev *aml_dma = platform_get_drvdata(pdev);
+	struct aml_dma_chan *aml_chan = NULL;
+	int i;
+
+	of_dma_controller_free((&pdev->dev)->of_node);
+
+	regmap_write(aml_dma->regmap, RCH_INT_MASK, 0xffffffff);
+	regmap_write(aml_dma->regmap, WCH_INT_MASK, 0xffffffff);
+
+	for (i = 0; i < MAX_CHAN_ID; i++) {
+		aml_chan = aml_dma->aml_rch[i];
+		if (aml_chan)
+			tasklet_kill(&aml_chan->vchan.task);
+		aml_chan = aml_dma->aml_wch[i];
+		if (aml_chan)
+			tasklet_kill(&aml_chan->vchan.task);
+	}
+}
+
+static const struct of_device_id aml_dma_ids[] = {
+	{ .compatible = "amlogic,a9-dma", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, aml_dma_ids);
+
+static struct platform_driver aml_dma_driver = {
+	.probe		= aml_dma_probe,
+	.remove		= aml_dma_remove,
+	.driver		= {
+		.name	= "aml-dma",
+		.of_match_table = aml_dma_ids,
+	},
+};
+
+module_platform_driver(aml_dma_driver);
+
+MODULE_DESCRIPTION("GENERAL DMA driver for Amlogic");
+MODULE_AUTHOR("Xianwei Zhao <xianwei.zhao@amlogic.com>");
+MODULE_LICENSE("GPL");

-- 
2.52.0



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH v11 3/3] MAINTAINERS: Add an entry for Amlogic DMA driver

[Xianwei Zhao via B4 Relay]

From: Xianwei Zhao <xianwei.zhao@amlogic.com>

Add Amlogic DMA controller entry to MAINTAINERS to clarify
the maintainers.

Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 3e7b2d9e9c24..b4ef8d3f52cb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1307,6 +1307,13 @@ F:	Documentation/devicetree/bindings/perf/amlogic,g12-ddr-pmu.yaml
 F:	drivers/perf/amlogic/
 F:	include/soc/amlogic/
 
+AMLOGIC DMA DRIVER
+M:	Xianwei Zhao <xianwei.zhao@amlogic.com>
+L:	linux-amlogic@lists.infradead.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/dma/amlogic,a9-dma.yaml
+F:	drivers/dma/amlogic-dma.c
+
 AMLOGIC ISP DRIVER
 M:	Keke Li <keke.li@amlogic.com>
 L:	linux-media@vger.kernel.org

-- 
2.52.0



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v11 1/3] dt-bindings: dma: Add Amlogic A9 SoC DMA

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The macros DMA_TX and DMA_RX in the global DT binding header lack an Amlogic/SoC specific prefix, leading to namespace pollution and potential build conflicts.
--

--- Patch [1]: [PATCH v11 1/3] dt-bindings: dma: Add Amlogic A9 SoC DMA ---
commit 093f368eaa67418008421d43d724ce2e98651039
Author: Xianwei Zhao <xianwei.zhao@amlogic.com>
dt-bindings: dma: Add Amlogic A9 SoC DMA

This commit adds device tree binding documentation for the Amlogic A9 SoC
general-purpose DMA controller. It introduces the YAML schema and a header
file defining the channel type macros.

> diff --git a/Documentation/devicetree/bindings/dma/amlogic,a9-dma.yaml b/Documentation/devicetree/bindings/dma/amlogic,a9-dma.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/amlogic,a9-dma.yaml
[ ... ]
> +  '#dma-cells':
> +    const: 2
> +    description:
> +      The first cell is the DMA channel type(DMA_TX or DMA_RX).
> +      The second cell is the DMA channel index.
[ ... ]
> diff --git a/include/dt-bindings/dma/amlogic,a9-dma.h b/include/dt-bindings/dma/amlogic,a9-dma.h
> new file mode 100644
> --- /dev/null
> +++ b/include/dt-bindings/dma/amlogic,a9-dma.h
> @@ -0,0 +1,8 @@
> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
> +
> +#ifndef __DT_BINDINGS_DMA_AMLOGIC_DMA_H__
> +#define __DT_BINDINGS_DMA_AMLOGIC_DMA_H__
> +
> +#define DMA_TX			0
> +#define DMA_RX			1

[Severity: Low]
Are these macro names too generic for a global dt-bindings header?

Since headers in include/dt-bindings/ are globally visible and frequently
included by C drivers, unprefixed names like DMA_TX and DMA_RX could lead
to namespace pollution and macro redefinition build failures if a driver
includes this alongside other headers that use the same common names.

Would it be safer to add a vendor or SoC prefix, such as AMLOGIC_A9_DMA_TX?

> +#endif /* __DT_BINDINGS_DMA_AMLOGIC_DMA_H__ */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714-amlogic-dma-v11-0-de79c2394282@amlogic.com?part=1

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v11 2/3] dmaengine: amlogic: Add general DMA driver for A9

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] aml_dma_tx_status falsely reports in-flight transfers as DMA_COMPLETE, causing data corruption.
- [High] Data race and TOCTOU NULL pointer dereference in aml_dma_tx_status due to early lock release.
- [High] Use-After-Free in devres-managed remove path due to tasklet scheduled after tasklet_kill.
- [High] Pending free_work items on the global workqueue cause a kernel panic on module unload.
- [High] aml_dma_terminate_all() masks interrupts but does not clear pending interrupt status, potentially corrupting subsequent transfers.
- [Medium] Missing src_addr_widths configuration prevents DMA_DEV_TO_MEM clients from validating capabilities.
- [Medium] DMA descriptors use host-endian fields, breaking hardware functionality on big-endian platforms.
--

--- Patch [2]: [PATCH v11 2/3] dmaengine: amlogic: Add general DMA driver for A9 ---
commit 91d4c9c09e9b3baa0931dae1e37c022fae2b7447
Author: Xianwei Zhao <xianwei.zhao@amlogic.com>

dmaengine: amlogic: Add general DMA driver for A9

This patch introduces a new DMA engine driver for Amlogic A9 SoCs. The
general-purpose DMA controller is used by multiple peripherals such as I2C,
PIO, and I3C.

> diff --git a/drivers/dma/amlogic-dma.c b/drivers/dma/amlogic-dma.c
> new file mode 100644
> index 0000000000000..9de650a79abac
> --- /dev/null
> +++ b/drivers/dma/amlogic-dma.c

[ ... ]

> +struct aml_dma_sg_link {
> +#define LINK_LEN		GENMASK(26, 0)
> +#define LINK_IRQ		BIT(27)
> +#define LINK_EOC		BIT(28)
> +#define LINK_LOOP		BIT(29)
> +#define LINK_ERR		BIT(30)
> +#define LINK_OWNER		BIT(31)
> +	u32 ctl;
> +	u32 addr_low;
> +	u32 addr_high;
> +	u32 revered;
> +} __packed;

[Severity: Medium]
Since this structure describes a hardware memory descriptor, should these
fields use __le32 instead of u32 to ensure the driver functions correctly on
big-endian architectures?

[ ... ]

> +static void aml_dma_free_desc(struct virt_dma_desc *vd)
> +{
> +	struct aml_dma_desc *aml_desc = to_aml_dma_desc(vd);
> +
> +	schedule_work(&aml_desc->free_work);
> +}

[Severity: High]
Does scheduling memory cleanup on the system workqueue here introduce a panic
on module unload?

If the driver module is unloaded while a work item is still pending, the
module's text segment will be unmapped. When the workqueue executes
aml_dma_free_desc_work(), it might jump to unmapped memory because
aml_dma_remove() does not appear to flush this work.

[ ... ]

> +static enum dma_status aml_dma_tx_status(struct dma_chan *chan,
> +					 dma_cookie_t cookie,
> +					 struct dma_tx_state *txstate)
> +{
> +	struct aml_dma_chan *aml_chan = to_aml_dma_chan(chan);
> +	struct aml_dma_dev *aml_dma = aml_chan->aml_dma;
> +	struct aml_dma_desc *aml_desc = NULL;
> +	struct virt_dma_desc *vd;
> +	u32 residue = 0, done;
> +	unsigned long flags;
> +	enum dma_status ret;
> +
> +	ret = dma_cookie_status(chan, cookie, txstate);
> +	if (ret == DMA_COMPLETE)
> +		return ret;
> +
> +	ret = aml_chan->status;

[Severity: High]
Could this overwrite the correct DMA_IN_PROGRESS return value from
dma_cookie_status() with aml_chan->status?

If aml_chan->status is usually set to DMA_COMPLETE during initialization,
polling clients might mistakenly believe the transfer is finished before it
actually is, leading to data corruption.

> +	if (!txstate)
> +		return ret;
> +
> +	spin_lock_irqsave(&aml_chan->vchan.lock, flags);
> +	vd = vchan_find_desc(&aml_chan->vchan, cookie);
> +	spin_unlock_irqrestore(&aml_chan->vchan.lock, flags);
> +	if (vd) {
> +		aml_desc = to_aml_dma_desc(vd);
> +		residue = aml_desc->data_len;
> +	} else if (aml_chan->cur_desc && aml_chan->cur_desc->vd.tx.cookie == cookie) {

[Severity: High]
Can releasing the lock before accessing aml_chan->cur_desc lead to a
NULL pointer dereference?

If a hardware interrupt completes the transfer concurrently, it sets cur_desc
to NULL. A thread executing this code could check aml_chan->cur_desc (finding
it non-NULL), but if the interrupt handler clears the pointer right after,
the subsequent aml_chan->cur_desc->vd.tx.cookie access would dereference NULL.

Alternatively, aml_desc could be assigned NULL, causing a dereference when
calculating the residue.

> +		aml_desc = aml_chan->cur_desc;
> +		regmap_read(aml_dma->regmap, aml_chan->reg_offs + RCH_RD_LEN, &done);
> +		residue = aml_desc->data_len - done;
> +	} else {
> +		dev_err(aml_dma->dma_device.dev, "cookie error\n");
> +	}
> +	dma_set_residue(txstate, residue);
> +
> +	return ret;
> +}

[ ... ]

> +static struct dma_async_tx_descriptor *aml_dma_prep_slave_sg
> +		(struct dma_chan *chan, struct scatterlist *sgl,
> +		unsigned int sg_len, enum dma_transfer_direction direction,
> +		unsigned long flags, void *context)
> +{

[ ... ]

> +		while (avail > SG_MAX_LEN) {
> +			/* set dma address and len to sglink*/
> +			sg_link->addr_low = lower_32_bits(paddr);
> +			sg_link->addr_high = upper_32_bits(paddr);
> +			sg_link->ctl = FIELD_PREP(LINK_LEN, SG_MAX_LEN);

[Severity: Medium]
Should these assignments use cpu_to_le32() to ensure proper conversion of
host-endian values before writing to coherent DMA memory? Without it, this
could break hardware functionality on big-endian platforms.

[ ... ]

> +static int aml_dma_terminate_all(struct dma_chan *chan)
> +{

[ ... ]

> +	spin_lock_irqsave(&aml_chan->vchan.lock, flags);
> +	regmap_set_bits(aml_dma->regmap, aml_chan->reg_offs + RCH_CFG, CFG_PAUSE);
> +	regmap_set_bits(aml_dma->regmap, aml_chan->reg_offs + RCH_CFG, CFG_CLEAR);
> +
> +	if (aml_chan->direction == DMA_MEM_TO_DEV)
> +		regmap_set_bits(aml_dma->regmap, RCH_INT_MASK, BIT(chan_id));
> +	else if (aml_chan->direction == DMA_DEV_TO_MEM)
> +		regmap_set_bits(aml_dma->regmap, WCH_INT_MASK, BIT(chan_id));

[Severity: High]
Does masking the interrupts here without also clearing the pending interrupt
status (via CLEAR_RCH or CLEAR_WCH) risk corrupting subsequent transfers?

If an interrupt is already pending, its status bit remains set. When
aml_dma_start() is called later and unmasks the interrupt, it will fire
immediately and falsely complete the new transfer.

[ ... ]

> +static int aml_dma_probe(struct platform_device *pdev)
> +{

[ ... ]

> +	dma_dev->device_issue_pending = aml_dma_issue_pending;
> +	/* PIO 4 bytes and I2C 1 byte */
> +	dma_dev->dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) | BIT(DMA_SLAVE_BUSWIDTH_1_BYTE);
> +	dma_dev->directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);

[Severity: Medium]
Since this driver advertises DMA_DEV_TO_MEM support, is there a missing
configuration for src_addr_widths?

If omitted, client drivers validating source widths using
dma_get_slave_caps() will see a width of 0 and might fail initialization.

[ ... ]

> +static void aml_dma_remove(struct platform_device *pdev)
> +{
> +	struct aml_dma_dev *aml_dma = platform_get_drvdata(pdev);
> +	struct aml_dma_chan *aml_chan = NULL;
> +	int i;
> +
> +	of_dma_controller_free((&pdev->dev)->of_node);
> +
> +	regmap_write(aml_dma->regmap, RCH_INT_MASK, 0xffffffff);
> +	regmap_write(aml_dma->regmap, WCH_INT_MASK, 0xffffffff);
> +
> +	for (i = 0; i < MAX_CHAN_ID; i++) {
> +		aml_chan = aml_dma->aml_rch[i];
> +		if (aml_chan)
> +			tasklet_kill(&aml_chan->vchan.task);
> +		aml_chan = aml_dma->aml_wch[i];
> +		if (aml_chan)
> +			tasklet_kill(&aml_chan->vchan.task);
> +	}
> +}

[Severity: High]
Can a use-after-free occur during driver unbind if an interrupt fires
concurrently?

Since the devres-managed IRQ is not freed or synchronized before calling
tasklet_kill(), an active interrupt handler on another CPU could schedule the
tasklet again right after tasklet_kill() returns. Once the remove function
finishes, devres frees the channel memory containing the tasklet, leading to
execution on freed memory.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714-amlogic-dma-v11-0-de79c2394282@amlogic.com?part=2

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v11 1/3] dt-bindings: dma: Add Amlogic A9 SoC DMA

[Vinod Koul]

On 14-07-26, 08:19, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] The macros DMA_TX and DMA_RX in the global DT binding header lack an Amlogic/SoC specific prefix, leading to namespace pollution and potential build conflicts.
> --
> 
> --- Patch [1]: [PATCH v11 1/3] dt-bindings: dma: Add Amlogic A9 SoC DMA ---
> commit 093f368eaa67418008421d43d724ce2e98651039
> Author: Xianwei Zhao <xianwei.zhao@amlogic.com>
> dt-bindings: dma: Add Amlogic A9 SoC DMA
> 
> This commit adds device tree binding documentation for the Amlogic A9 SoC
> general-purpose DMA controller. It introduces the YAML schema and a header
> file defining the channel type macros.
> 
> > diff --git a/Documentation/devicetree/bindings/dma/amlogic,a9-dma.yaml b/Documentation/devicetree/bindings/dma/amlogic,a9-dma.yaml
> > new file mode 100644
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/dma/amlogic,a9-dma.yaml
> [ ... ]
> > +  '#dma-cells':
> > +    const: 2
> > +    description:
> > +      The first cell is the DMA channel type(DMA_TX or DMA_RX).
> > +      The second cell is the DMA channel index.
> [ ... ]
> > diff --git a/include/dt-bindings/dma/amlogic,a9-dma.h b/include/dt-bindings/dma/amlogic,a9-dma.h
> > new file mode 100644
> > --- /dev/null
> > +++ b/include/dt-bindings/dma/amlogic,a9-dma.h
> > @@ -0,0 +1,8 @@
> > +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
> > +
> > +#ifndef __DT_BINDINGS_DMA_AMLOGIC_DMA_H__
> > +#define __DT_BINDINGS_DMA_AMLOGIC_DMA_H__
> > +
> > +#define DMA_TX			0
> > +#define DMA_RX			1
> 
> [Severity: Low]
> Are these macro names too generic for a global dt-bindings header?
> 
> Since headers in include/dt-bindings/ are globally visible and frequently
> included by C drivers, unprefixed names like DMA_TX and DMA_RX could lead
> to namespace pollution and macro redefinition build failures if a driver
> includes this alongside other headers that use the same common names.
> 
> Would it be safer to add a vendor or SoC prefix, such as AMLOGIC_A9_DMA_TX?

I think this makes sense to add here

-- 
~Vinod

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v11 2/3] dmaengine: amlogic: Add general DMA driver for A9

[Vinod Koul]

On 14-07-26, 08:08, Xianwei Zhao via B4 Relay wrote:
> From: Xianwei Zhao <xianwei.zhao@amlogic.com>
> 
> Amlogic A9 SoCs include a general-purpose DMA controller that can be used
> by multiple peripherals, such as I2C PIO and I3C. Each peripheral group
> is associated with a dedicated DMA channel in hardware.
> 
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
> ---
>  drivers/dma/Kconfig       |  10 +
>  drivers/dma/Makefile      |   1 +
>  drivers/dma/amlogic-dma.c | 726 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 737 insertions(+)
> 
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index ae6a682c9f76..01f96a8257e5 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -85,6 +85,16 @@ config AMCC_PPC440SPE_ADMA
>  	help
>  	  Enable support for the AMCC PPC440SPe RAID engines.
>  
> +config AMLOGIC_DMA
> +	tristate "Amlogic general DMA support"
> +	depends on ARCH_MESON || COMPILE_TEST
> +	select DMA_ENGINE
> +	select DMA_VIRTUAL_CHANNELS
> +	select REGMAP_MMIO
> +	help
> +	  Enable support for the Amlogic general DMA engines. THis DMA
> +	  controller is used some Amlogic SoCs, such as A9.
> +
>  config APPLE_ADMAC
>  	tristate "Apple ADMAC support"
>  	depends on ARCH_APPLE || COMPILE_TEST
> diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
> index 14aa086629d5..f62d12b08e15 100644
> --- a/drivers/dma/Makefile
> +++ b/drivers/dma/Makefile
> @@ -16,6 +16,7 @@ obj-$(CONFIG_DMATEST) += dmatest.o
>  obj-$(CONFIG_ALTERA_MSGDMA) += altera-msgdma.o
>  obj-$(CONFIG_AMBA_PL08X) += amba-pl08x.o
>  obj-$(CONFIG_AMCC_PPC440SPE_ADMA) += ppc4xx/
> +obj-$(CONFIG_AMLOGIC_DMA) += amlogic-dma.o
>  obj-$(CONFIG_APPLE_ADMAC) += apple-admac.o
>  obj-$(CONFIG_ARM_DMA350) += arm-dma350.o
>  obj-$(CONFIG_AT_HDMAC) += at_hdmac.o
> diff --git a/drivers/dma/amlogic-dma.c b/drivers/dma/amlogic-dma.c
> new file mode 100644
> index 000000000000..9de650a79aba
> --- /dev/null
> +++ b/drivers/dma/amlogic-dma.c
> @@ -0,0 +1,726 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR MIT)
> +/*
> + * Copyright (C) 2025 Amlogic, Inc. All rights reserved

2026 please

> +/* DMA controller reg */
> +#define RCH_INT_MASK		0x1000
> +#define WCH_INT_MASK		0x1004
> +#define CLEAR_W_BATCH		0x1014
> +#define CLEAR_RCH		0x1024
> +#define CLEAR_WCH		0x1028
> +#define RCH_ACTIVE		0x1038
> +#define WCH_ACTIVE		0x103c
> +#define RCH_DONE		0x104c
> +#define WCH_DONE		0x1050
> +#define RCH_ERR			0x1060
> +#define RCH_LEN_ERR		0x1064
> +#define WCH_ERR			0x1068
> +#define DMA_BATCH_END		0x1078
> +#define WCH_EOC_DONE		0x1088
> +#define WDMA_RESP_ERR		0x1098
> +#define UPT_PKT_SYNC		0x10a8
> +#define RCHN_CFG		0x10ac
> +#define WCHN_CFG		0x10b0
> +#define MEM_PD_CFG		0x10b4
> +#define MEM_BUS_CFG		0x10b8
> +#define DMA_GMV_CFG		0x10bc
> +#define DMA_GMR_CFG		0x10c0
> +
> +#define MAX_CHAN_ID		32
> +#define SG_MAX_LEN		(GENMASK(26, 0) & ~0x3)

So you define a mask for 0-26 and then clear everything expect last two
bits, why not define last two bits..? Something does not look right here

> +static int aml_dma_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct dma_device *dma_dev;
> +	struct aml_dma_dev *aml_dma;
> +	int ret, i, len;
> +	u32 chan_nr;
> +
> +	const struct regmap_config aml_regmap_config = {
> +		.reg_bits = 32,
> +		.val_bits = 32,
> +		.reg_stride = 4,
> +		.max_register = 0x3000,
> +	};
> +
> +	ret = of_property_read_u32(np, "dma-channels", &chan_nr);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret, "failed to read dma-channels\n");
> +	if (chan_nr > (MAX_CHAN_ID * 2))
> +		return dev_err_probe(&pdev->dev, -EINVAL, "dma-channels unusual\n");
> +
> +	len = sizeof(struct aml_dma_dev) + sizeof(struct aml_dma_chan) * chan_nr;
> +	aml_dma = devm_kzalloc(&pdev->dev, len, GFP_KERNEL);
> +	if (!aml_dma)
> +		return -ENOMEM;
> +
> +	aml_dma->chan_nr = chan_nr;
> +
> +	aml_dma->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(aml_dma->base))
> +		return PTR_ERR(aml_dma->base);
> +
> +	aml_dma->regmap = devm_regmap_init_mmio(&pdev->dev, aml_dma->base,
> +						&aml_regmap_config);
> +	if (IS_ERR_OR_NULL(aml_dma->regmap))
> +		return PTR_ERR(aml_dma->regmap);
> +
> +	aml_dma->clk = devm_clk_get_enabled(&pdev->dev, NULL);
> +	if (IS_ERR(aml_dma->clk))
> +		return PTR_ERR(aml_dma->clk);
> +
> +	aml_dma->irq = platform_get_irq(pdev, 0);
> +
> +	aml_dma->pdev = pdev;
> +	aml_dma->dma_device.dev = &pdev->dev;
> +
> +	dma_dev = &aml_dma->dma_device;
> +	INIT_LIST_HEAD(&dma_dev->channels);
> +
> +	/* Initialize channel parameters */
> +	for (i = 0; i < chan_nr; i++) {
> +		struct aml_dma_chan *aml_chan = &aml_dma->aml_chans[i];
> +
> +		aml_chan->aml_dma = aml_dma;
> +		aml_chan->vchan.desc_free = aml_dma_free_desc;
> +		vchan_init(&aml_chan->vchan, &aml_dma->dma_device);
> +	}
> +	aml_dma->chan_used = 0;
> +
> +	dma_set_max_seg_size(dma_dev->dev, SG_MAX_LEN);
> +	dma_cap_set(DMA_SLAVE, dma_dev->cap_mask);
> +	dma_dev->device_alloc_chan_resources = aml_dma_alloc_chan_resources;
> +	dma_dev->device_free_chan_resources = aml_dma_free_chan_resources;
> +	dma_dev->device_tx_status = aml_dma_tx_status;
> +	dma_dev->device_prep_slave_sg = aml_dma_prep_slave_sg;
> +	dma_dev->device_pause = aml_dma_chan_pause;
> +	dma_dev->device_resume = aml_dma_chan_resume;
> +	dma_dev->device_terminate_all = aml_dma_terminate_all;
> +	dma_dev->device_issue_pending = aml_dma_issue_pending;
> +	/* PIO 4 bytes and I2C 1 byte */
> +	dma_dev->dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) | BIT(DMA_SLAVE_BUSWIDTH_1_BYTE);
> +	dma_dev->directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
> +	dma_dev->residue_granularity = DMA_RESIDUE_GRANULARITY_BURST;
> +
> +	regmap_write(aml_dma->regmap, RCH_INT_MASK, 0xffffffff);
> +	regmap_write(aml_dma->regmap, WCH_INT_MASK, 0xffffffff);

I think we have macros for 32bit masks, please use that here and other
places
 

-- 
~Vinod

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic