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

[Xianwei Zhao <xianwei.zhao@amlogic.com>]

Hi Vinod Koul,
    Thanks for your review.

On 2026/7/14 20:03, Vinod Koul wrote:
> 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
> 

Will do, define  GENMASK(26, 2)

>> +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

Will do.

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