Re: [PATCH 2/2] dma: Add Ingenic PDMA driver support

From: Krzysztof Kozlowski
Date: Wed Feb 28 2024 - 03:11:52 EST


On 28/02/2024 02:24, bin.yao wrote:
> From: byao <bin.yao@xxxxxxxxxxx>
>
> Add Ingenic PDMA controller support.
> This module can be found on ingenic victory soc.
>
> Signed-off-by: byao <bin.yao@xxxxxxxxxxx>
> Signed-off-by: rick <rick.tyliu@xxxxxxxxxxx>

Full names


Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching.

> ---
> drivers/dma/Kconfig | 14 +
> drivers/dma/Makefile | 1 +
> drivers/dma/ingenic/Makefile | 1 +
> drivers/dma/ingenic/ingenic_dma.c | 1053 +++++++++++++++++++++++++++++
> drivers/dma/ingenic/ingenic_dma.h | 250 +++++++
> 5 files changed, 1319 insertions(+)
> create mode 100644 drivers/dma/ingenic/Makefile
> create mode 100644 drivers/dma/ingenic/ingenic_dma.c
> create mode 100644 drivers/dma/ingenic/ingenic_dma.h
>
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index e928f2ca0f1e..3214c72aef31 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -179,6 +179,20 @@ config DMA_SUN6I
> help
> Support for the DMA engine first found in Allwinner A31 SoCs.
>
> +config DMA_JZ4780
> + tristate "Ingenic JZ SoC DMA support"
> + select DMA_ENGINE
> + select DMA_VIRTUAL_CHANNELS
> + help
> + Support the DMA engine found on Ingenic Jz SoCs.
> +
> +config DMA_INGENIC_SOC
> + tristate "Ingenic SoC DMA support"
> + select DMA_ENGINE
> + select DMA_VIRTUAL_CHANNELS
> + help
> + Support the DMA engine found on Ingenic T41 SoCs.
> +
> config DW_AXI_DMAC
> tristate "Synopsys DesignWare AXI DMA support"
> depends on OF
> diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
> index dfd40d14e408..8a56175bbfbb 100644
> --- a/drivers/dma/Makefile
> +++ b/drivers/dma/Makefile
> @@ -83,6 +83,7 @@ obj-$(CONFIG_XGENE_DMA) += xgene-dma.o
> obj-$(CONFIG_ST_FDMA) += st_fdma.o
> obj-$(CONFIG_FSL_DPAA2_QDMA) += fsl-dpaa2-qdma/
> obj-$(CONFIG_INTEL_LDMA) += lgm/
> +obj-$(CONFIG_DMA_INGENIC_SOC) += ingenic/
>
> obj-y += mediatek/
> obj-y += qcom/
> diff --git a/drivers/dma/ingenic/Makefile b/drivers/dma/ingenic/Makefile
> new file mode 100644
> index 000000000000..2028a6f0b0c8
> --- /dev/null
> +++ b/drivers/dma/ingenic/Makefile
> @@ -0,0 +1 @@
> +obj-y += ingenic_dma.o
> diff --git a/drivers/dma/ingenic/ingenic_dma.c b/drivers/dma/ingenic/ingenic_dma.c
> new file mode 100644
> index 000000000000..066325e35a92
> --- /dev/null
> +++ b/drivers/dma/ingenic/ingenic_dma.c
> @@ -0,0 +1,1053 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2016 Ingenic Semiconductor Co., Ltd.
> + * Author: cli <chen.li@xxxxxxxxxxx>
> + *
> + * Programmable DMA Controller Driver For Ingenic's SOC,
> + * such as X1000, and so on. (kernel.4.4)
> + *
> + * Author: cli <chen.li@xxxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/dmaengine.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +#include <linux/irq.h>
> +#include <linux/device.h>
> +#include <linux/of.h>
> +#include <linux/of_dma.h>
> +#include <linux/dmapool.h>
> +#include "ingenic_dma.h"
> +
> +static void dump_dma(struct ingenic_dma_chan *master)
> +{
> + pr_debug("CH_DSA = 0x%08x\n", readl(master->iomem + CH_DSA));
> + pr_debug("CH_DTA = 0x%08x\n", readl(master->iomem + CH_DTA));
> + pr_debug("CH_DTC = 0x%08x\n", readl(master->iomem + CH_DTC));
> + pr_debug("CH_DRT = 0x%08x\n", readl(master->iomem + CH_DRT));
> + pr_debug("CH_DCS = 0x%08x\n", readl(master->iomem + CH_DCS));
> + pr_debug("CH_DCM = 0x%08x\n", readl(master->iomem + CH_DCM));
> + pr_debug("CH_DDA = 0x%08x\n", readl(master->iomem + CH_DDA));
> + pr_debug("CH_DSD = 0x%08x\n", readl(master->iomem + CH_DSD));
> +}
> +
> +static int dump_dma_hdesc(struct hdma_desc *desc, const char *d)
> +{
> + int i;
> + unsigned long *p = (unsigned long *)desc;
> +
> + pr_debug("%s(): %s\n", __func__, d);
> + for (i = 0; i < 8; i++)
> + pr_debug("\t%08lx\n", (unsigned long)*p++);
> +
> + return 0;
> +}
> +
> +void jzdma_dump(struct dma_chan *chan)
> +{
> + struct ingenic_dma_chan *dmac = to_ingenic_dma_chan(chan);
> +
> + dump_dma(dmac);
> +}
> +EXPORT_SYMBOL_GPL(jzdma_dump);

Drop, not used.

> +
> +/*tsz for 1,2,4,8,16,32,64 128bytes */
> +static const char dcm_tsz[8] = { 1, 2, 0, 0, 3, 4, 5, 6};

You have total mess here. Definitions of such data go before functions.
Don't mix them.

> +static inline unsigned int get_current_tsz(unsigned long dcmp)

Drop all these inlines.

> +{
> + int i;
> + int val = (dcmp & DCM_TSZ_MSK) >> DCM_TSZ_SFT;
> +
> + if (val == DCM_TSZ_AUTO)
> + return 0;
> +
> + for (i = 0; i < ARRAY_SIZE(dcm_tsz); i++) {
> + if (val == dcm_tsz[i])
> + break;
> + }
> +
> + return i;
> +}
> +
> +static inline unsigned char get_max_tsz(unsigned long val, unsigned int *shift)

Drop inline.

> +{
> + int ord = ffs(val) - 1;
> +
> + /*
> + * 8 byte transfer sizes unsupported so fall back on 4. If it's larger
> + * than the maximum, just limit it. It is perfectly safe to fall back
> + * in this way since we won't exceed the maximum burst size supported
> + * by the device, the only effect is reduced efficiency. This is better
> + * than refusing to perform the request at all.
> + */
> + if (ord == 3)
> + ord = 2;
> + else if (ord > 7)
> + ord = 7;
> +
> + if (shift)
> + *shift = ord;
> +
> + return dcm_tsz[ord];
> +}
> +
> +static const struct of_device_id ingenic_dma_dt_match[];
> +
> +static struct ingenic_dma_engine *ingenic_dma_parse_dt(struct platform_device *pdev)

This should be just before probe function.

> +{
> + const struct of_device_id *match;
> + struct ingenic_dma_engine *ingenic_dma;
> + u32 nr_chs;
> +
> + if (!pdev->dev.of_node)
> + return ERR_PTR(-ENODEV);
> +
> + match = of_match_node(ingenic_dma_dt_match, pdev->dev.of_node);
> + if (!match)
> + return ERR_PTR(-ENODEV);
> +
> + if (of_property_read_u32(pdev->dev.of_node, "#dma-channels", &nr_chs))
> + nr_chs = 32;
> +
> + ingenic_dma = devm_kzalloc(&pdev->dev, sizeof(*ingenic_dma) +
> + sizeof(struct ingenic_dma_chan *) * nr_chs, GFP_KERNEL);

You should use struct_size

> + if (!ingenic_dma)
> + return ERR_PTR(-ENOMEM);
> +
> + ingenic_dma->dev = &pdev->dev;
> + ingenic_dma->nr_chs = nr_chs;
> + ingenic_dma->hwattr = (unsigned long)match->data;
> +
> + /* Property is optional, if it doesn't exist the value will remain 0. */
> + of_property_read_u32(pdev->dev.of_node, "ingenic,reserved-chs",

NAK, there is no such property.

> + &ingenic_dma->chan_reserved);
> +
> + if (!of_property_read_u32(pdev->dev.of_node, "ingenic,programed-chs",

NAK, there is no such property.

Cleanup your driver before sending it upstream.



> + &ingenic_dma->chan_programed))
> + ingenic_dma->chan_reserved |= ingenic_dma->chan_programed;
> +
> + if (HWATTR_SPECIAL_CH01_SUP(ingenic_dma->hwattr) &&
> + of_property_read_bool(pdev->dev.of_node,
> + "ingenic,special-chs")) {
> + ingenic_dma->chan_reserved |= DMA_SPECAIL_CHS;
> + ingenic_dma->chan_programed |= DMA_SPECAIL_CHS;
> + ingenic_dma->special_ch = true;
> + }
> +
> + ingenic_dma->intc_ch = -1;
> + if (HWATTR_INTC_IRQ_SUP(ingenic_dma->hwattr) &&
> + !of_property_read_u32(pdev->dev.of_node,
> + "ingenic,intc-ch", (u32 *)&ingenic_dma->intc_ch)) {

NAK, cleanup your driver from such vendor stuff before sending upstream.


> +static unsigned int build_one_slave_desc(struct ingenic_dma_chan *dmac, dma_addr_t addr,
> + unsigned int length,
> + enum dma_transfer_direction direction,
> + struct hdma_desc *desc)
> +{
> + enum dma_transfer_direction dir;
> + unsigned int rdil;
> +// unsigned int tsz, transfer_shift;

???

Clean your driver before submitting it to upstream.


...


> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + /* must be delete it */

Or really?

> + //if (config->slave_id & INGENIC_DMA_TYPE_REQ_MSK)
> + // dmac->slave_id = config->slave_id & INGENIC_DMA_TYPE_REQ_MSK;

Then please delete it.

> +
> + return 0;
> +}
> +
> +static void pdma_handle_chan_irq(struct ingenic_dma_engine *ingenic_dma, int ch_id)
> +{
> + struct ingenic_dma_chan *dmac = ingenic_dma->chan[ch_id];
> + struct ingenic_dma_sdesc *sdesc;
> + unsigned int dcs;
> +
> + spin_lock(&dmac->vc.lock);
> +
> + dcs = readl(dmac->iomem + CH_DCS);
> + writel(0, dmac->iomem + CH_DCS);
> +
> + if (dcs & DCS_AR)
> + dev_warn(&dmac->vc.chan.dev->device,
> + "address error (DCS=0x%x)\n", dcs);
> +
> + if (dcs & DCS_HLT)
> + dev_warn(&dmac->vc.chan.dev->device,
> + "channel halt (DCS=0x%x)\n", dcs);
> + sdesc = dmac->sdesc;
> + if (sdesc) {
> + if (sdesc->status == STAT_STOPPED) {
> + dma_cookie_complete(&sdesc->vd.tx);
> + ingenic_dma_free_swdesc(&sdesc->vd);
> + complete(&dmac->completion);
> + } else if (dmac->fake_cyclic && sdesc->cyclic) {
> + vchan_cyclic_callback(&sdesc->vd);
> + } else {
> + vchan_cookie_complete(&sdesc->vd);
> + }
> + dmac->sdesc = NULL;
> + ingenic_dma_start_trans(dmac);
> + } else {
> + dev_warn(&dmac->vc.chan.dev->device,
> + "channel irq with no active transfer, channel stop\n");
> + }
> +
> + spin_unlock(&dmac->vc.lock);
> +}
> +
> +static irqreturn_t pdma_int_handler(int irq, void *dev)
> +{
> + struct ingenic_dma_engine *ingenic_dma = (struct ingenic_dma_engine *)dev;
> + unsigned long pending, dmac;
> + int i;
> +
> + pending = readl(ingenic_dma->iomem + DIRQP);
> + writel(~pending, ingenic_dma->iomem + DIRQP);
> +
> + for (i = 0; i < ingenic_dma->nr_chs ; i++) {
> + if (!(pending & (1 << i)))
> + continue;
> + pdma_handle_chan_irq(ingenic_dma, i);
> + }
> +
> + dmac = readl(ingenic_dma->iomem + DMAC);
> + dmac &= ~(DMAC_HLT | DMAC_AR);
> + writel(dmac, ingenic_dma->iomem + DMAC);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t pdmam_int_handler(int irq, void *dev)
> +{
> + /*TODO*/
> + return IRQ_HANDLED;

???


> +}
> +
> +static irqreturn_t pdmad_int_handler(int irq, void *dev)
> +{
> + struct ingenic_dma_engine *ingenic_dma = (struct ingenic_dma_engine *)dev;
> + unsigned long pending;
> + int i;
> +
> + pending = readl(ingenic_dma->iomem + DIP);
> + writel(readl(ingenic_dma->iomem + DIP) & (~pending), ingenic_dma->iomem + DIC);
> +
> + for (i = 0; i < ingenic_dma->nr_chs; i++) {
> + struct ingenic_dma_chan *dmac = ingenic_dma->chan[i];
> + struct ingenic_dma_sdesc *sdesc;
> +
> + if (!(pending & (1 << i)))
> + continue;
> + sdesc = dmac->sdesc;
> + if (sdesc && sdesc->cyclic) {
> + spin_lock(&dmac->vc.lock);
> + vchan_cyclic_callback(&sdesc->vd);
> + spin_unlock(&dmac->vc.lock);
> + }
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int ingenic_dma_alloc_chan_resources(struct dma_chan *chan)
> +{
> + struct ingenic_dma_chan *dmac = to_ingenic_dma_chan(chan);
> +
> + dmac->hdesc_pool = dma_pool_create(dev_name(&chan->dev->device),
> + chan->device->dev, sizeof(struct hdma_desc), 0, PAGE_SIZE);
> + if (!dmac->hdesc_pool) {
> + dev_err(&chan->dev->device,
> + "failed to allocate descriptor pool\n");
> + return -ENOMEM;
> + }
> +
> + dmac->hdesc_max = PAGE_SIZE / sizeof(struct hdma_desc);
> + dmac->hdesc_num = 0;
> +
> + return 0;
> +}
> +
> +static void ingenic_dma_synchronize(struct dma_chan *chan)
> +{
> + struct ingenic_dma_chan *dmac = to_ingenic_dma_chan(chan);
> +
> + vchan_synchronize(&dmac->vc);
> +}
> +
> +static void ingenic_dma_free_chan_resources(struct dma_chan *chan)
> +{
> + struct ingenic_dma_chan *dmac = to_ingenic_dma_chan(chan);
> + unsigned long flags;
> +
> + ingenic_dma_terminate_all(chan);
> +
> + ingenic_dma_wait_terminate_complete(chan);
> +
> + dma_pool_destroy(dmac->hdesc_pool);
> + spin_lock_irqsave(&dmac->hdesc_lock, flags);
> + dmac->hdesc_pool = NULL;
> + dmac->hdesc_max = 0;
> + dmac->hdesc_num = 0;
> + spin_unlock_irqrestore(&dmac->hdesc_lock, flags);
> +}
> +
> +static bool ingenic_dma_filter_fn(struct dma_chan *chan, void *param)
> +{
> + struct ingenic_dma_chan *dmac = to_ingenic_dma_chan(chan);
> + unsigned int private = *(unsigned int *)param;
> + unsigned long type = (unsigned long)chan->private;

Fix your indentation.



..

> +
> +static int __init ingenic_dma_probe(struct platform_device *pdev)

Since when probe is an __init?

> +{
> + struct ingenic_dma_engine *dma = NULL;
> + struct resource *iores;
> + u32 reg_dmac = DMAC_DMAE;
> + int i, ret = 0;
> +
> + /* check of first. if of failed, use platform */
> +
> + dma = ingenic_dma_parse_dt(pdev);
> + if (IS_ERR(dma))
> + return PTR_ERR(dma);
> +
> + iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + dma->iomem = devm_ioremap_resource(&pdev->dev, iores);

Use helper combining these two.

> + if (IS_ERR(dma->iomem))
> + return PTR_ERR(dma->iomem);
> +
> + /* PDMA interrupt*/
> + dma->irq_pdma = platform_get_irq_byname(pdev, "pdma");
> + if (dma->irq_pdma < 0)
> + return dma->irq_pdma;
> +
> + ret = devm_request_irq(&pdev->dev, dma->irq_pdma, pdma_int_handler,
> + 0, "pdma", dma);
> + if (ret)
> + return ret;
> +
> + /* PDMA mcu interrupt*/
> + dma->irq_pdmam = platform_get_irq_byname(pdev, "pdmam");
> + if (dma->irq_pdmam >= 0) {
> + ret = devm_request_irq(&pdev->dev, dma->irq_pdmam, pdmam_int_handler,
> + 0, "pdmam", dma);
> + if (ret)
> + return ret;
> + }
> +
> + /* PDMA descriptor interrupt */
> + if (HWATTR_DESC_INTER_SUP(dma->hwattr)) {
> + dma->irq_pdmad = platform_get_irq_byname(pdev, "pdmad");

Not really, you said you have only one interrupt.

> + if (dma->irq_pdmad < 0) {
> + dev_err(&pdev->dev, "unable to get pdmad irq\n");
> + return dma->irq_pdmad;
> + }
> + ret = devm_request_irq(&pdev->dev, dma->irq_pdmad, pdmad_int_handler,
> + 0, "pdmad", dma);
> + if (ret)
> + return ret;
> + }
> +
> + /* Initialize dma engine */
> + INIT_LIST_HEAD(&dma->dma_device.channels);
> + for (i = 0; i < dma->nr_chs; i++) {
> + /*reserved one channel for intc interrupt*/
> + if (dma->intc_ch == i)
> + continue;
> + ingenic_dma_chan_init(dma, i);
> + }
> +
> + dma_cap_set(DMA_MEMCPY, dma->dma_device.cap_mask);
> + dma_cap_set(DMA_SLAVE, dma->dma_device.cap_mask);
> + dma_cap_set(DMA_CYCLIC, dma->dma_device.cap_mask);
> +
> + dma->dma_device.dev = &pdev->dev;
> + dma->dma_device.device_alloc_chan_resources = ingenic_dma_alloc_chan_resources;
> + dma->dma_device.device_free_chan_resources = ingenic_dma_free_chan_resources;
> + dma->dma_device.device_tx_status = ingenic_dma_tx_status;
> + dma->dma_device.device_prep_slave_sg = ingenic_dma_prep_slave_sg;
> + dma->dma_device.device_prep_dma_cyclic = ingenic_dma_prep_dma_cyclic;
> + dma->dma_device.device_prep_dma_memcpy = ingenic_dma_prep_dma_memcpy;
> + dma->dma_device.device_config = ingenic_dma_config;
> + dma->dma_device.device_terminate_all = ingenic_dma_terminate_all;
> + dma->dma_device.device_issue_pending = ingenic_dma_issue_pending;
> + dma->dma_device.device_synchronize = ingenic_dma_synchronize;
> + dma->dma_device.copy_align = DMAENGINE_ALIGN_4_BYTES;
> + dma->dma_device.src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
> + BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
> + BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
> + dma->dma_device.dst_addr_widths = dma->dma_device.src_addr_widths;
> + dma->dma_device.directions = BIT(DMA_DEV_TO_MEM) |
> + BIT(DMA_MEM_TO_DEV) |
> + BIT(DMA_MEM_TO_MEM);
> + dma->dma_device.residue_granularity = DMA_RESIDUE_GRANULARITY_BURST;
> + dma->dma_device.dev->dma_parms = &dma->dma_parms;
> +
> + dma_set_max_seg_size(dma->dma_device.dev, DTC_TC_MSK); /*At least*/
> +
> + dma->gate_clk = devm_clk_get(&pdev->dev, "gate_pdma");
> + if (IS_ERR(dma->gate_clk))
> + return PTR_ERR(dma->gate_clk);
> +
> + ret = dma_async_device_register(&dma->dma_device);
> + if (ret) {
> + dev_err(&pdev->dev, "unable to register\n");
> + clk_disable(dma->gate_clk);
> + return ret;
> + }
> +
> + of_ingenic_dma_info.dma_cap = dma->dma_device.cap_mask;
> + ret = of_dma_controller_register(pdev->dev.of_node,
> + of_dma_simple_xlate, &of_ingenic_dma_info);
> + if (ret) {
> + dev_err(&pdev->dev, "unable to register dma to device tree\n");
> + dma_async_device_unregister(&dma->dma_device);
> + clk_disable(dma->gate_clk);
> + return ret;
> + }
> +
> +
> + platform_set_drvdata(pdev, dma);
> +
> + /*enable pdma controller*/
> + clk_prepare_enable(dma->gate_clk);
> +
> + if (dma->chan_programed)
> + writel(dma->chan_programed, dma->iomem + DMACP);
> + if (dma->intc_ch >= 0)
> + reg_dmac |= DMAC_INTCE | ((dma->intc_ch << DMAC_INTCC_SFT) & DMAC_INTCC_MSK);
> + if (dma->special_ch)
> + reg_dmac |= DMAC_CH01;
> + writel(reg_dmac, dma->iomem + DMAC);
> +
> + dev_info(dma->dev, "INGENIC SoC DMA initialized\n");

Drop simple success prints, useless.

> +
> + return 0;
> +}
> +
> +static int ingenic_dma_suspend(struct platform_device *pdev, pm_message_t state)
> +{
> + struct ingenic_dma_engine *ingenic_dma = platform_get_drvdata(pdev);
> + struct dma_chan *chan;
> +
> + list_for_each_entry(chan, &ingenic_dma->dma_device.channels, device_node) {
> + struct ingenic_dma_chan *dmac = to_ingenic_dma_chan(chan);
> +
> + if (dmac->sdesc)
> + return -EBUSY;
> + }
> + clk_disable_unprepare(ingenic_dma->gate_clk);
> +
> + return 0;
> +}
> +
> +static int ingenic_dma_resume(struct platform_device *pdev)
> +{
> + struct ingenic_dma_engine *ingenic_dma = platform_get_drvdata(pdev);
> +
> + clk_prepare_enable(ingenic_dma->gate_clk);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id ingenic_dma_dt_match[] = {
> + { .compatible = "ingenic,m200-pdma", .data = (void *)(HWATTR_SPECIAL_CH01 |
> + HWATTR_INTC_IRQ)},
> + { .compatible = "ingenic,x1000-pdma", .data = (void *)HWATTR_DESC_INTER },
> + { .compatible = "ingenic,t40-pdma", .data = (void *)HWATTR_INTC_IRQ },
> + /*{ .compatible = "ingenic,t41-pdma", .data = (void *)HWATTR_INTC_IRQ },*/

This is a mess...

> + { .compatible = "ingenic,t33-pdma", .data = (void *)NULL },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, ingenic_dma_dt_match);
> +
> +static struct platform_driver ingenic_dma_driver = {
> + .driver = {
> + .name = "ingenic-dma",
> + .owner = THIS_MODULE,

Come on... that's like 7 year or 10 year old code. Please don't dump on
us such old stuff, but clean it up before posting.

Run coccinelle/coccicheck and fix all the issues. Then run smatch. Then
run sparse.

> + .of_match_table = of_match_ptr(ingenic_dma_dt_match),

Drop of_match_ptr

> + },
> + .suspend = ingenic_dma_suspend,
> + .resume = ingenic_dma_resume,
> +};
> +
> +static int __init ingenic_dma_module_init(void)
> +{
> + return platform_driver_probe(&ingenic_dma_driver, ingenic_dma_probe);
> +}
> +
> +subsys_initcall(ingenic_dma_module_init);
> +MODULE_AUTHOR("Chen.li <chen.li@xxxxxxxxxx>");
> +MODULE_DESCRIPTION("Ingenic dma driver");


..

> +
> +static inline struct ingenic_dma_chan *to_ingenic_dma_chan(struct dma_chan *chan)
> +{
> + return container_of(chan, struct ingenic_dma_chan, vc.chan);
> +}
> +
> +static inline struct ingenic_dma_sdesc *to_ingenic_dma_sdesc(struct virt_dma_desc *vd)
> +{
> + return container_of(vd, struct ingenic_dma_sdesc, vd);
> +}
> +
> +#define INGENIC_DMA_REQ_AUTO 0xff
> +#define INGENIC_DMA_CHAN_CNT 32
> +unsigned long pdma_maps[INGENIC_DMA_CHAN_CNT] = {

This cannot be in the header.

> + INGENIC_DMA_REQ_AUTO,
> + INGENIC_DMA_REQ_AUTO,
> + INGENIC_DMA_REQ_AIC_LOOP_RX,
> + INGENIC_DMA_REQ_AIC_TX,
> + INGENIC_DMA_REQ_AIC_F_RX,
> + INGENIC_DMA_REQ_AUTO_TX,
> + INGENIC_DMA_REQ_SADC_RX,
> + INGENIC_DMA_REQ_UART5_TX,
> + INGENIC_DMA_REQ_UART5_RX,
> + INGENIC_DMA_REQ_UART4_TX,
> + INGENIC_DMA_REQ_UART4_RX,
> + INGENIC_DMA_REQ_UART3_TX,
> + INGENIC_DMA_REQ_UART3_RX,
> + INGENIC_DMA_REQ_UART2_TX,
> + INGENIC_DMA_REQ_UART2_RX,
> + INGENIC_DMA_REQ_UART1_TX,
> + INGENIC_DMA_REQ_UART1_RX,
> + INGENIC_DMA_REQ_UART0_TX,
> + INGENIC_DMA_REQ_UART0_RX,
> + INGENIC_DMA_REQ_SSI0_TX,
> + INGENIC_DMA_REQ_SSI0_RX,
> + INGENIC_DMA_REQ_SSI1_TX,
> + INGENIC_DMA_REQ_SSI1_RX,
> + INGENIC_DMA_REQ_SLV_TX,
> + INGENIC_DMA_REQ_SLV_RX,
> + INGENIC_DMA_REQ_I2C0_TX,
> + INGENIC_DMA_REQ_I2C0_RX,
> + INGENIC_DMA_REQ_I2C1_TX,
> + INGENIC_DMA_REQ_I2C1_RX,
> + INGENIC_DMA_REQ_DES_TX,
> + INGENIC_DMA_REQ_DES_RX,
> +};
> +
> +void jzdma_dump(struct dma_chan *chan);

Drop

> +#endif /*__INGENIC_DMA_H__*/

Best regards,
Krzysztof