From: Vinod Koul <vkoul@kernel.org>
To: xianwei.zhao@amlogic.com
Cc: Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>, Kees Cook <kees@kernel.org>,
"Gustavo A. R. Silva" <gustavoars@kernel.org>,
Frank Li <Frank.Li@kernel.org>,
linux-amlogic@lists.infradead.org, dmaengine@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-hardening@vger.kernel.org, Frank Li <Frank.Li@nxp.com>
Subject: Re: [PATCH v11 2/3] dmaengine: amlogic: Add general DMA driver for A9
Date: Tue, 14 Jul 2026 17:33:18 +0530 [thread overview]
Message-ID: <alYlhm9vav59wKq9@vaman> (raw)
In-Reply-To: <20260714-amlogic-dma-v11-2-de79c2394282@amlogic.com>
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
WARNING: multiple messages have this Message-ID (diff)
From: Vinod Koul <vkoul@kernel.org>
To: xianwei.zhao@amlogic.com
Cc: Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>, Kees Cook <kees@kernel.org>,
"Gustavo A. R. Silva" <gustavoars@kernel.org>,
Frank Li <Frank.Li@kernel.org>,
linux-amlogic@lists.infradead.org, dmaengine@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-hardening@vger.kernel.org, Frank Li <Frank.Li@nxp.com>
Subject: Re: [PATCH v11 2/3] dmaengine: amlogic: Add general DMA driver for A9
Date: Tue, 14 Jul 2026 17:33:18 +0530 [thread overview]
Message-ID: <alYlhm9vav59wKq9@vaman> (raw)
In-Reply-To: <20260714-amlogic-dma-v11-2-de79c2394282@amlogic.com>
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
next prev parent reply other threads:[~2026-07-14 12:03 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-14 8:08 [PATCH v11 0/3] Add Amlogic general DMA Xianwei Zhao via B4 Relay
2026-07-14 8:08 ` Xianwei Zhao via B4 Relay
2026-07-14 8:08 ` Xianwei Zhao
2026-07-14 8:08 ` [PATCH v11 1/3] dt-bindings: dma: Add Amlogic A9 SoC DMA Xianwei Zhao via B4 Relay
2026-07-14 8:08 ` Xianwei Zhao via B4 Relay
2026-07-14 8:08 ` Xianwei Zhao
2026-07-14 8:19 ` sashiko-bot
2026-07-14 8:19 ` sashiko-bot
2026-07-14 11:58 ` Vinod Koul
2026-07-14 11:58 ` Vinod Koul
2026-07-15 2:23 ` Xianwei Zhao
2026-07-15 2:23 ` Xianwei Zhao
2026-07-14 8:08 ` [PATCH v11 2/3] dmaengine: amlogic: Add general DMA driver for A9 Xianwei Zhao via B4 Relay
2026-07-14 8:08 ` Xianwei Zhao via B4 Relay
2026-07-14 8:08 ` Xianwei Zhao
2026-07-14 8:22 ` sashiko-bot
2026-07-14 8:22 ` sashiko-bot
2026-07-14 12:03 ` Vinod Koul [this message]
2026-07-14 12:03 ` Vinod Koul
2026-07-15 2:25 ` Xianwei Zhao
2026-07-15 2:25 ` Xianwei Zhao
2026-07-14 8:08 ` [PATCH v11 3/3] MAINTAINERS: Add an entry for Amlogic DMA driver Xianwei Zhao via B4 Relay
2026-07-14 8:08 ` Xianwei Zhao via B4 Relay
2026-07-14 8:08 ` Xianwei Zhao
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alYlhm9vav59wKq9@vaman \
--to=vkoul@kernel.org \
--cc=Frank.Li@kernel.org \
--cc=Frank.Li@nxp.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=dmaengine@vger.kernel.org \
--cc=gustavoars@kernel.org \
--cc=kees@kernel.org \
--cc=krzk+dt@kernel.org \
--cc=linux-amlogic@lists.infradead.org \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=robh@kernel.org \
--cc=xianwei.zhao@amlogic.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.