On 14-07-26, 08:08, Xianwei Zhao via B4 Relay wrote:
> From: Xianwei Zhao <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Xianwei Zhao <[email protected]>
> ---
> 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
