* [09/46] dmaengine: cppi41: use dmaenginem_async_device_register to simplify the code
From: Huang Shijie @ 2018-08-06 3:28 UTC (permalink / raw)
To: Peter Ujfalusi
Cc: vkoul, dmaengine, linux-kernel, dave.jiang, radhey.shyam.pandey,
appana.durga.rao, jmkrzyszt, gomonovych, keescook, horms+renesas,
geert+renesas, shawnguo, baoyou.xie, michal.simek, baohua,
ludovic.desroches, linus.walleij, david.brown
On Fri, Aug 03, 2018 at 10:55:25AM +0300, Peter Ujfalusi wrote:
>
>
> On 2018-08-03 10:19, Huang Shijie wrote:
> > Use dmaenginem_async_device_register to simplify the code:
> > remove dma_async_device_unregister
> >
> > Signed-off-by: Huang Shijie <sjhuang@iluvatar.ai>
> > ---
> > drivers/dma/ti/cppi41.c | 7 ++-----
> > 1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/dma/ti/cppi41.c b/drivers/dma/ti/cppi41.c
> > index 1497da367710..d2998a19ed2e 100644
> > --- a/drivers/dma/ti/cppi41.c
> > +++ b/drivers/dma/ti/cppi41.c
> > @@ -1096,21 +1096,19 @@ static int cppi41_dma_probe(struct platform_device *pdev)
> > goto err_chans;
> > cdd->irq = irq;
> >
> > - ret = dma_async_device_register(&cdd->ddev);
> > + ret = dmaenginem_async_device_register(&cdd->ddev);
> > if (ret)
> > goto err_chans;
> >
> > ret = of_dma_controller_register(dev->of_node,
> > cppi41_dma_xlate, &cpp41_dma_info);
> > if (ret)
> > - goto err_of;
> > + goto err_chans;
> >
> > pm_runtime_mark_last_busy(dev);
> > pm_runtime_put_autosuspend(dev);
> >
> > return 0;
> > -err_of:
> > - dma_async_device_unregister(&cdd->ddev);
> > err_chans:
> > deinit_cppi41(dev, cdd);
> > err_init_cppi:
> > @@ -1132,7 +1130,6 @@ static int cppi41_dma_remove(struct platform_device *pdev)
> > dev_err(&pdev->dev, "%s could not pm_runtime_get: %i\n",
> > __func__, error);
> > of_dma_controller_free(pdev->dev.of_node);
> > - dma_async_device_unregister(&cdd->ddev);
>
> If I read the code right then this is not safe.
I read the code again, and find it is okay.
> We would have deinitalized cppi41 driver which is not functional, but we
> will still have the dma device registered and if a channel is requested
> we will have kernel crash.
We cannot succeed to request a channel when the drv->remove() is called.
Please see __device_release_driver:
---------------------------------------------------------------------
if (dev->bus && dev->bus->remove)
dev->bus->remove(dev);
else if (drv->remove)
drv->remove(dev);
device_links_driver_cleanup(dev);
dma_deconfigure(dev);
devres_release_all(dev); ============> Devres release
---------------------------------------------------------------------
For the DMA engine driver, there is only one case which will calls drv->remove():
Use the rmmod(or modprobe -r).
We do not use the device_link_add API for DMA engines.
And we not manually call the device_release_driver() for DMA engines.
But when we use the rmmod, the module state will be MODULE_STATE_GOING.
In the find_candidate(), dma_chan_get() will fail.
And we cannot get a channel.
Please correct me if I am wrong :)
Thanks
Huang Shijie
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [v8,1/3] dmaengine: fsl-edma: extract common fsl-edma code (no changes in behavior intended)
From: Krzysztof Kozlowski @ 2018-08-06 7:43 UTC (permalink / raw)
To: Angelo Dureghello
Cc: vinod.koul, dmaengine, linux-arm-kernel, linux-m68k, Stefan Agner
On 3 August 2018 at 21:32, Angelo Dureghello <angelo@sysam.it> wrote:
> This patch adds a new fsl-edma-common module to allow new
> mcf-edma module code to use most of the fsl-edma code.
>
> Signed-off-by: Angelo Dureghello <angelo@sysam.it>
> ---
> Changes for v2:
> - patch splitted into 4
> - add mcf-edma as minimal different parts from fsl-edma
>
> Changes for v3:
> none
>
> Changes for v4:
> - patch simplified from 4/4 into 2/2
> - collecting all the mcf-edma-related changes
>
> Changes for v5:
> none
>
> Changes for v6:
> - adjusted comment header
> - fixed bit shift with BIT()
> - we need to free the interrupts at remove(), so removed all devm_
> interrupt related calls
>
> Changes for v7:
> none
>
> Changes for v8:
> - patch rewritten from scratch, splitted into 3, common code isolated,
> minimal changes from the original Freescale code have been done.
> The patch has been tested with both Iris + Colibri Vybrid VF50 and
> stmark2/mcf54415 Coldfire boards.
> ---
> drivers/dma/Makefile | 2 +-
> drivers/dma/fsl-edma-common.c | 576 ++++++++++++++++++++++++++++
> drivers/dma/fsl-edma-common.h | 196 ++++++++++
> drivers/dma/fsl-edma.c | 697 +---------------------------------
> 4 files changed, 774 insertions(+), 697 deletions(-)
> create mode 100644 drivers/dma/fsl-edma-common.c
> create mode 100644 drivers/dma/fsl-edma-common.h
>
> diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
> index 203a99d68315..66022f59fca4 100644
> --- a/drivers/dma/Makefile
> +++ b/drivers/dma/Makefile
> @@ -31,7 +31,7 @@ obj-$(CONFIG_DW_AXI_DMAC) += dw-axi-dmac/
> obj-$(CONFIG_DW_DMAC_CORE) += dw/
> obj-$(CONFIG_EP93XX_DMA) += ep93xx_dma.o
> obj-$(CONFIG_FSL_DMA) += fsldma.o
> -obj-$(CONFIG_FSL_EDMA) += fsl-edma.o
> +obj-$(CONFIG_FSL_EDMA) += fsl-edma.o fsl-edma-common.o
> obj-$(CONFIG_FSL_RAID) += fsl_raid.o
> obj-$(CONFIG_HSU_DMA) += hsu/
> obj-$(CONFIG_IMG_MDC_DMA) += img-mdc-dma.o
> diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
> new file mode 100644
> index 000000000000..0ae7094f477a
> --- /dev/null
> +++ b/drivers/dma/fsl-edma-common.c
> @@ -0,0 +1,576 @@
> +// SPDX-License-Identifier: GPL-2.0
You moved code from fsl-edma.c which is licensed on GPL-2.0+. I think
that's not allowed as you effectively relicense the work on new
license.
Tested-by: Krzysztof Kozlowski <krzk@kernel.org>
Best regards,
Krzysztof
> +//
> +// Copyright (c) 2013-2014 Freescale Semiconductor, Inc
> +// Copyright (c) 2017 Sysam, Angelo Dureghello <angelo@sysam.it>
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [v8,2/3] dmaengine: fsl-edma: add edma version and configurable registers
From: Krzysztof Kozlowski @ 2018-08-06 7:59 UTC (permalink / raw)
To: Angelo Dureghello
Cc: vinod.koul, dmaengine, linux-arm-kernel, linux-m68k, Stefan Agner
On 3 August 2018 at 21:32, Angelo Dureghello <angelo@sysam.it> wrote:
> This patch adds configurable registers (using __iomem addresses)
> to allow the use of fsl-edma-common code with slightly different
> edma module versions, as Vybrid (v1) and ColdFire (v2) are.
>
> Removal of old membase-referenced registers, amd some fixes on
> macroes are included.
>
> Signed-off-by: Angelo Dureghello <angelo@sysam.it>
> ---
> Changes from v7:
> - patch rewritten from scratch, this patch (2/3) has just been added.
> ---
> drivers/dma/fsl-edma-common.c | 138 ++++++++++++++++++++++++++--------
> drivers/dma/fsl-edma-common.h | 115 ++++++++++++++--------------
> drivers/dma/fsl-edma.c | 32 ++++----
> 3 files changed, 182 insertions(+), 103 deletions(-)
>
> diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
> index 0ae7094f477a..948a3ee51bbb 100644
> --- a/drivers/dma/fsl-edma-common.c
> +++ b/drivers/dma/fsl-edma-common.c
> @@ -9,6 +9,38 @@
>
> #include "fsl-edma-common.h"
>
> +#define EDMA_CR 0x00
> +#define EDMA_ES 0x04
> +#define EDMA_ERQ 0x0C
> +#define EDMA_EEI 0x14
> +#define EDMA_SERQ 0x1B
> +#define EDMA_CERQ 0x1A
> +#define EDMA_SEEI 0x19
> +#define EDMA_CEEI 0x18
> +#define EDMA_CINT 0x1F
> +#define EDMA_CERR 0x1E
> +#define EDMA_SSRT 0x1D
> +#define EDMA_CDNE 0x1C
> +#define EDMA_INTR 0x24
> +#define EDMA_ERR 0x2C
> +
> +#define EDMA64_ERQH 0x08
> +#define EDMA64_EEIH 0x10
> +#define EDMA64_SERQ 0x18
> +#define EDMA64_CERQ 0x19
> +#define EDMA64_SEEI 0x1a
> +#define EDMA64_CEEI 0x1b
> +#define EDMA64_CINT 0x1c
> +#define EDMA64_CERR 0x1d
> +#define EDMA64_SSRT 0x1e
> +#define EDMA64_CDNE 0x1f
> +#define EDMA64_INTH 0x20
> +#define EDMA64_INTL 0x24
> +#define EDMA64_ERRH 0x28
> +#define EDMA64_ERRL 0x2c
> +
> +#define EDMA_TCD 0x1000
> +
> /*
> * R/W functions for big- or little-endian registers:
> * The eDMA controller's endian is independent of the CPU core's endian.
> @@ -67,20 +99,20 @@ EXPORT_SYMBOL_GPL(to_fsl_edma_desc);
>
> static void fsl_edma_enable_request(struct fsl_edma_chan *fsl_chan)
> {
> - void __iomem *addr = fsl_chan->edma->membase;
> + struct edma_regs *regs = &fsl_chan->edma->regs;
> u32 ch = fsl_chan->vchan.chan.chan_id;
>
> - edma_writeb(fsl_chan->edma, EDMA_SEEI_SEEI(ch), addr + EDMA_SEEI);
> - edma_writeb(fsl_chan->edma, ch, addr + EDMA_SERQ);
> + edma_writeb(fsl_chan->edma, EDMA_SEEI_SEEI(ch), regs->seei);
> + edma_writeb(fsl_chan->edma, ch, regs->serq);
> }
>
> void fsl_edma_disable_request(struct fsl_edma_chan *fsl_chan)
> {
> - void __iomem *addr = fsl_chan->edma->membase;
> + struct edma_regs *regs = &fsl_chan->edma->regs;
> u32 ch = fsl_chan->vchan.chan.chan_id;
>
> - edma_writeb(fsl_chan->edma, ch, addr + EDMA_CERQ);
> - edma_writeb(fsl_chan->edma, EDMA_CEEI_CEEI(ch), addr + EDMA_CEEI);
> + edma_writeb(fsl_chan->edma, ch, regs->cerq);
> + edma_writeb(fsl_chan->edma, EDMA_CEEI_CEEI(ch), regs->ceei);
> }
> EXPORT_SYMBOL_GPL(fsl_edma_disable_request);
>
> @@ -208,7 +240,7 @@ static size_t fsl_edma_desc_residue(struct fsl_edma_chan *fsl_chan,
> struct virt_dma_desc *vdesc, bool in_progress)
> {
> struct fsl_edma_desc *edesc = fsl_chan->edesc;
> - void __iomem *addr = fsl_chan->edma->membase;
> + struct edma_regs *regs = &fsl_chan->edma->regs;
> u32 ch = fsl_chan->vchan.chan.chan_id;
> enum dma_transfer_direction dir = fsl_chan->fsc.dir;
> dma_addr_t cur_addr, dma_addr;
> @@ -224,11 +256,9 @@ static size_t fsl_edma_desc_residue(struct fsl_edma_chan *fsl_chan,
> return len;
>
> if (dir == DMA_MEM_TO_DEV)
> - cur_addr = edma_readl(
> - fsl_chan->edma, addr + EDMA_TCD_SADDR(ch));
> + cur_addr = edma_readl(fsl_chan->edma, ®s->tcd[ch].saddr);
> else
> - cur_addr = edma_readl(
> - fsl_chan->edma, addr + EDMA_TCD_DADDR(ch));
> + cur_addr = edma_readl(fsl_chan->edma, ®s->tcd[ch].daddr);
>
> /* figure out the finished and calculate the residue */
> for (i = 0; i < fsl_chan->edesc->n_tcds; i++) {
> @@ -285,7 +315,7 @@ static void fsl_edma_set_tcd_regs(struct fsl_edma_chan *fsl_chan,
> struct fsl_edma_hw_tcd *tcd)
> {
> struct fsl_edma_engine *edma = fsl_chan->edma;
> - void __iomem *addr = fsl_chan->edma->membase;
> + struct edma_regs *regs = &fsl_chan->edma->regs;
> u32 ch = fsl_chan->vchan.chan.chan_id;
>
> /*
> @@ -293,24 +323,24 @@ static void fsl_edma_set_tcd_regs(struct fsl_edma_chan *fsl_chan,
> * endian format. However, we need to load the TCD registers in
> * big- or little-endian obeying the eDMA engine model endian.
> */
> - edma_writew(edma, 0, addr + EDMA_TCD_CSR(ch));
> - edma_writel(edma, le32_to_cpu(tcd->saddr), addr + EDMA_TCD_SADDR(ch));
> - edma_writel(edma, le32_to_cpu(tcd->daddr), addr + EDMA_TCD_DADDR(ch));
> + edma_writew(edma, 0, ®s->tcd[ch].csr);
> + edma_writel(edma, le32_to_cpu(tcd->saddr), ®s->tcd[ch].saddr);
> + edma_writel(edma, le32_to_cpu(tcd->daddr), ®s->tcd[ch].daddr);
>
> - edma_writew(edma, le16_to_cpu(tcd->attr), addr + EDMA_TCD_ATTR(ch));
> - edma_writew(edma, le16_to_cpu(tcd->soff), addr + EDMA_TCD_SOFF(ch));
> + edma_writew(edma, le16_to_cpu(tcd->attr), ®s->tcd[ch].attr);
> + edma_writew(edma, le16_to_cpu(tcd->soff), ®s->tcd[ch].soff);
>
> - edma_writel(edma, le32_to_cpu(tcd->nbytes), addr + EDMA_TCD_NBYTES(ch));
> - edma_writel(edma, le32_to_cpu(tcd->slast), addr + EDMA_TCD_SLAST(ch));
> + edma_writel(edma, le32_to_cpu(tcd->nbytes), ®s->tcd[ch].nbytes);
> + edma_writel(edma, le32_to_cpu(tcd->slast), ®s->tcd[ch].slast);
>
> - edma_writew(edma, le16_to_cpu(tcd->citer), addr + EDMA_TCD_CITER(ch));
> - edma_writew(edma, le16_to_cpu(tcd->biter), addr + EDMA_TCD_BITER(ch));
> - edma_writew(edma, le16_to_cpu(tcd->doff), addr + EDMA_TCD_DOFF(ch));
> + edma_writew(edma, le16_to_cpu(tcd->citer), ®s->tcd[ch].citer);
> + edma_writew(edma, le16_to_cpu(tcd->biter), ®s->tcd[ch].biter);
> + edma_writew(edma, le16_to_cpu(tcd->doff), ®s->tcd[ch].doff);
>
> - edma_writel(edma,
> - le32_to_cpu(tcd->dlast_sga), addr + EDMA_TCD_DLAST_SGA(ch));
> + edma_writel(edma, le32_to_cpu(tcd->dlast_sga),
> + ®s->tcd[ch].dlast_sga);
>
> - edma_writew(edma, le16_to_cpu(tcd->csr), addr + EDMA_TCD_CSR(ch));
> + edma_writew(edma, le16_to_cpu(tcd->csr), ®s->tcd[ch].csr);
> }
>
> static inline
> @@ -332,15 +362,15 @@ void fsl_edma_fill_tcd(struct fsl_edma_hw_tcd *tcd, u32 src, u32 dst,
>
> tcd->attr = cpu_to_le16(attr);
>
> - tcd->soff = cpu_to_le16(EDMA_TCD_SOFF_SOFF(soff));
> + tcd->soff = cpu_to_le16(soff);
>
> - tcd->nbytes = cpu_to_le32(EDMA_TCD_NBYTES_NBYTES(nbytes));
> - tcd->slast = cpu_to_le32(EDMA_TCD_SLAST_SLAST(slast));
> + tcd->nbytes = cpu_to_le32(nbytes);
> + tcd->slast = cpu_to_le32(slast);
>
> tcd->citer = cpu_to_le16(EDMA_TCD_CITER_CITER(citer));
> - tcd->doff = cpu_to_le16(EDMA_TCD_DOFF_DOFF(doff));
> + tcd->doff = cpu_to_le16(doff);
>
> - tcd->dlast_sga = cpu_to_le32(EDMA_TCD_DLAST_SGA_DLAST_SGA(dlast_sga));
> + tcd->dlast_sga = cpu_to_le32(dlast_sga);
>
> tcd->biter = cpu_to_le16(EDMA_TCD_BITER_BITER(biter));
> if (major_int)
> @@ -573,4 +603,52 @@ void fsl_edma_cleanup_vchan(struct dma_device *dmadev)
> }
> EXPORT_SYMBOL_GPL(fsl_edma_cleanup_vchan);
>
> +/*
> + * On the 32 channels Vybrid/mpc577x edma version (here called "v1"),
> + * register offsets are different compared to ColdFire mcf5441x 64 channels
> + * edma (here called "v2").
> + *
> + * This function sets up register offsets as per proper declared version
> + * so must be called in xxx_edma_probe() just after setting the
> + * edma "version" and "membase" appropriately.
> + */
> +void fsl_edma_setup_regs(struct fsl_edma_engine *edma)
> +{
> + edma->regs.cr = edma->membase + EDMA_CR;
> + edma->regs.es = edma->membase + EDMA_ES;
> + edma->regs.erql = edma->membase + EDMA_ERQ;
> + edma->regs.eeil = edma->membase + EDMA_EEI;
> +
> + edma->regs.serq = edma->membase + ((edma->version == v1) ?
> + EDMA_SERQ : EDMA64_SERQ);
> + edma->regs.cerq = edma->membase + ((edma->version == v1) ?
> + EDMA_CERQ : EDMA64_CERQ);
> + edma->regs.seei = edma->membase + ((edma->version == v1) ?
> + EDMA_SEEI : EDMA64_SEEI);
> + edma->regs.ceei = edma->membase + ((edma->version == v1) ?
> + EDMA_CEEI : EDMA64_CEEI);
> + edma->regs.cint = edma->membase + ((edma->version == v1) ?
> + EDMA_CINT : EDMA64_CINT);
> + edma->regs.cerr = edma->membase + ((edma->version == v1) ?
> + EDMA_CERR : EDMA64_CERR);
> + edma->regs.ssrt = edma->membase + ((edma->version == v1) ?
> + EDMA_SSRT : EDMA64_SSRT);
> + edma->regs.cdne = edma->membase + ((edma->version == v1) ?
> + EDMA_CDNE : EDMA64_CDNE);
> + edma->regs.intl = edma->membase + ((edma->version == v1) ?
> + EDMA_INTR : EDMA64_INTL);
> + edma->regs.errl = edma->membase + ((edma->version == v1) ?
> + EDMA_ERR : EDMA64_ERRL);
> +
> + if (edma->version == v2) {
> + edma->regs.erqh = edma->membase + EDMA64_ERQH;
> + edma->regs.eeih = edma->membase + EDMA64_EEIH;
> + edma->regs.errh = edma->membase + EDMA64_ERRH;
> + edma->regs.inth = edma->membase + EDMA64_INTH;
> + }
> +
> + edma->regs.tcd = edma->membase + EDMA_TCD;
> +}
> +EXPORT_SYMBOL_GPL(fsl_edma_setup_regs);
> +
> MODULE_LICENSE("GPL v2");
> diff --git a/drivers/dma/fsl-edma-common.h b/drivers/dma/fsl-edma-common.h
> index f3ea68e15b23..7a9231e7639f 100644
> --- a/drivers/dma/fsl-edma-common.h
> +++ b/drivers/dma/fsl-edma-common.h
> @@ -8,35 +8,6 @@
>
> #include "virt-dma.h"
>
> -#define EDMA_CR 0x00
> -#define EDMA_ES 0x04
> -#define EDMA_ERQ 0x0C
> -#define EDMA_EEI 0x14
> -#define EDMA_SERQ 0x1B
> -#define EDMA_CERQ 0x1A
> -#define EDMA_SEEI 0x19
> -#define EDMA_CEEI 0x18
> -#define EDMA_CINT 0x1F
> -#define EDMA_CERR 0x1E
> -#define EDMA_SSRT 0x1D
> -#define EDMA_CDNE 0x1C
> -#define EDMA_INTR 0x24
> -#define EDMA_ERR 0x2C
> -
> -#define EDMA_TCD_SADDR(x) (0x1000 + 32 * (x))
> -#define EDMA_TCD_SOFF(x) (0x1004 + 32 * (x))
> -#define EDMA_TCD_ATTR(x) (0x1006 + 32 * (x))
> -#define EDMA_TCD_NBYTES(x) (0x1008 + 32 * (x))
> -#define EDMA_TCD_SLAST(x) (0x100C + 32 * (x))
> -#define EDMA_TCD_DADDR(x) (0x1010 + 32 * (x))
> -#define EDMA_TCD_DOFF(x) (0x1014 + 32 * (x))
> -#define EDMA_TCD_CITER_ELINK(x) (0x1016 + 32 * (x))
> -#define EDMA_TCD_CITER(x) (0x1016 + 32 * (x))
> -#define EDMA_TCD_DLAST_SGA(x) (0x1018 + 32 * (x))
> -#define EDMA_TCD_CSR(x) (0x101C + 32 * (x))
> -#define EDMA_TCD_BITER_ELINK(x) (0x101E + 32 * (x))
> -#define EDMA_TCD_BITER(x) (0x101E + 32 * (x))
> -
> #define EDMA_CR_EDBG BIT(1)
> #define EDMA_CR_ERCA BIT(2)
> #define EDMA_CR_ERGA BIT(3)
> @@ -47,34 +18,29 @@
> #define EDMA_CR_ECX BIT(16)
> #define EDMA_CR_CX BIT(17)
>
> -#define EDMA_SEEI_SEEI(x) ((x) & 0x1F)
> -#define EDMA_CEEI_CEEI(x) ((x) & 0x1F)
> -#define EDMA_CINT_CINT(x) ((x) & 0x1F)
> -#define EDMA_CERR_CERR(x) ((x) & 0x1F)
> -
> -#define EDMA_TCD_ATTR_DSIZE(x) (((x) & 0x0007))
> -#define EDMA_TCD_ATTR_DMOD(x) (((x) & 0x001F) << 3)
> -#define EDMA_TCD_ATTR_SSIZE(x) (((x) & 0x0007) << 8)
> -#define EDMA_TCD_ATTR_SMOD(x) (((x) & 0x001F) << 11)
> -#define EDMA_TCD_ATTR_SSIZE_8BIT (0x0000)
> -#define EDMA_TCD_ATTR_SSIZE_16BIT (0x0100)
> -#define EDMA_TCD_ATTR_SSIZE_32BIT (0x0200)
> -#define EDMA_TCD_ATTR_SSIZE_64BIT (0x0300)
> -#define EDMA_TCD_ATTR_SSIZE_32BYTE (0x0500)
> -#define EDMA_TCD_ATTR_DSIZE_8BIT (0x0000)
> -#define EDMA_TCD_ATTR_DSIZE_16BIT (0x0001)
> -#define EDMA_TCD_ATTR_DSIZE_32BIT (0x0002)
> -#define EDMA_TCD_ATTR_DSIZE_64BIT (0x0003)
> -#define EDMA_TCD_ATTR_DSIZE_32BYTE (0x0005)
> -
> -#define EDMA_TCD_SOFF_SOFF(x) (x)
> -#define EDMA_TCD_NBYTES_NBYTES(x) (x)
> -#define EDMA_TCD_SLAST_SLAST(x) (x)
> -#define EDMA_TCD_DADDR_DADDR(x) (x)
> -#define EDMA_TCD_CITER_CITER(x) ((x) & 0x7FFF)
> -#define EDMA_TCD_DOFF_DOFF(x) (x)
> -#define EDMA_TCD_DLAST_SGA_DLAST_SGA(x) (x)
> -#define EDMA_TCD_BITER_BITER(x) ((x) & 0x7FFF)
> +#define EDMA_SEEI_SEEI(x) ((x) & GENMASK(4, 0))
> +#define EDMA_CEEI_CEEI(x) ((x) & GENMASK(4, 0))
> +#define EDMA_CINT_CINT(x) ((x) & GENMASK(4, 0))
> +#define EDMA_CERR_CERR(x) ((x) & GENMASK(4, 0))
> +
> +#define EDMA_TCD_ATTR_DSIZE(x) (((x) & GENMASK(2, 0)))
> +#define EDMA_TCD_ATTR_DMOD(x) (((x) & GENMASK(4, 0)) << 3)
> +#define EDMA_TCD_ATTR_SSIZE(x) (((x) & GENMASK(2, 0)) << 8)
> +#define EDMA_TCD_ATTR_SMOD(x) (((x) & GENMASK(4, 0)) << 11)
This looks like a change for its own commit.
> +
> +#define EDMA_TCD_ATTR_DSIZE_8BIT 0
> +#define EDMA_TCD_ATTR_DSIZE_16BIT BIT(0)
> +#define EDMA_TCD_ATTR_DSIZE_32BIT BIT(1)
> +#define EDMA_TCD_ATTR_DSIZE_64BIT (BIT(0) | BIT(1))
> +#define EDMA_TCD_ATTR_DSIZE_32BYTE (BIT(3) | BIT(0))
> +#define EDMA_TCD_ATTR_SSIZE_8BIT 0
> +#define EDMA_TCD_ATTR_SSIZE_16BIT (EDMA_TCD_ATTR_DSIZE_16BIT << 8)
> +#define EDMA_TCD_ATTR_SSIZE_32BIT (EDMA_TCD_ATTR_DSIZE_32BIT << 8)
> +#define EDMA_TCD_ATTR_SSIZE_64BIT (EDMA_TCD_ATTR_DSIZE_64BIT << 8)
> +#define EDMA_TCD_ATTR_SSIZE_32BYTE (EDMA_TCD_ATTR_DSIZE_32BYTE << 8)
> +
> +#define EDMA_TCD_CITER_CITER(x) ((x) & GENMASK(14, 0))
> +#define EDMA_TCD_BITER_BITER(x) ((x) & GENMASK(14, 0))
>
> #define EDMA_TCD_CSR_START BIT(0)
> #define EDMA_TCD_CSR_INT_MAJOR BIT(1)
> @@ -87,7 +53,7 @@
>
> #define EDMAMUX_CHCFG_DIS 0x0
> #define EDMAMUX_CHCFG_ENBL 0x80
> -#define EDMAMUX_CHCFG_SOURCE(n) ((n) & 0x3F)
> +#define EDMAMUX_CHCFG_SOURCE(n) ((n) & GENMASK(6, 0))
>
> #define DMAMUX_NR 2
>
> @@ -114,6 +80,31 @@ struct fsl_edma_hw_tcd {
> __le16 biter;
> };
>
> +/*
> + * This are iomem pointers, for both v32 and v64.
s/This/These/
Tested-by: Krzysztof Kozlowski <krzk@kernel.org>
Best regards,
Krzysztof
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [v3,3/3] dmaengine: imx-sdma: allocate max 20 bds for one transfer
From: Robin Gong @ 2018-08-06 8:04 UTC (permalink / raw)
To: Lucas Stach, vkoul@kernel.org, dan.j.williams@intel.com,
s.hauer@pengutronix.de, linux@armlinux.org.uk
Cc: dmaengine@vger.kernel.org, dl-linux-imx, kernel@pengutronix.de,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Hello Lucas,
Any comment for my reply?
> -----Original Message-----
> From: Robin Gong
> Sent: 2018年7月25日 9:25
> To: 'Lucas Stach' <l.stach@pengutronix.de>; vkoul@kernel.org;
> dan.j.williams@intel.com; s.hauer@pengutronix.de; linux@armlinux.org.uk
> Cc: dmaengine@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> kernel@pengutronix.de; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org
> Subject: RE: [PATCH v3 3/3] dmaengine: imx-sdma: allocate max 20 bds for one
> transfer
>
> > -----Original Message-----
> > From: Lucas Stach [mailto:l.stach@pengutronix.de]
> > Sent: 2018年7月24日 17:22
> > To: Robin Gong <yibin.gong@nxp.com>; vkoul@kernel.org;
> > dan.j.williams@intel.com; s.hauer@pengutronix.de;
> > linux@armlinux.org.uk
> > Cc: dmaengine@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> > kernel@pengutronix.de; linux-arm-kernel@lists.infradead.org;
> > linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH v3 3/3] dmaengine: imx-sdma: allocate max 20 bds
> > for one transfer
> >
> > Am Montag, den 23.07.2018, 13:55 +0000 schrieb Robin Gong:
> > > > -----Original Message-----
> > > > From: Lucas Stach [mailto:l.stach@pengutronix.de]
> > > > Sent: 2018年7月23日 18:54
> > > > To: Robin Gong <yibin.gong@nxp.com>; vkoul@kernel.org;
> > > > dan.j.williams@intel.com; s.hauer@pengutronix.de;
> > > > linux@armlinux.or g.uk
> > > > Cc: dmaengine@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> > > > kernel@pengutronix.de; linux-arm-kernel@lists.infradead.org;
> > > > linux-kernel@vger.kernel.org
> > > > Subject: Re: [PATCH v3 3/3] dmaengine: imx-sdma: allocate max 20
> > > > bds for one transfer
> > > >
> > > > Am Dienstag, den 24.07.2018, 01:46 +0800 schrieb Robin Gong:
> > > > > If multi-bds used in one transfer, all bds should be consisten
> > > > > memory.To easily follow it, enlarge the dma pool size into 20
> > > > > bds, and it will report error if the number of bds is over than
> > > > > 20. For dmatest, the max count for single transfer is NUM_BD *
> > > >
> > > > SDMA_BD_MAX_CNT
> > > > > = 20 * 65535 = ~1.28MB.
> > > >
> > > > Both the commit message and the comment need a lot more care to
> > > > actually tell what this commit is trying to achieve. Currently I
> > > > don't follow at all. What does "consisten" mean? Do you mean BDs
> > > > should be contiguous in memory?
> > >
> > > Yes, BDs should be contiguous one by one in memory.
> >
> > Okay, but this isn't what the code change does. By increasing the size
> > parameter of the dma pool you just allocate 20 times as much memory as
> > needed for each BD. So actually the BDs end up being very non-
> > contiguous in memory as there are now holes of 19 BD sizes between the
> start of each BD.
> Please notice only allocate bds memory from dma pool one time even in multi
> bds.
> That's different with the common use case that allocate memory from dma
> pool everytime for every bd. Why do this is to make sure all bd memory is
> contiguous for single transfer whatever single bd or multi-bds, since two call
> dma_pool_alloc() can't promise the address is contiguous especially for multi
> thread case such as dmatest 'threads_per_chan = 5'. You can change to '
> norandom=true ' and ' test_buf_size = 163840' in dmatest.c to look what issue
> coming without this patch.
> >
> > So something isn't right with this change.
> I think this patch is the easy way to resolve the bd contiguous issue, but the
> cost is to allocate more dma pool memory which may not used.
> >
> > Regards,
> > Lucas
> >
> > > >
> > > > What do you gain by over-allocating each BD by a factor of 20?
> > >
> > > I guess dma_pool_alloc will return error in such case, and then
> > > cause dma setup transfer failure.
> > > >
> > > > Regards,
> > > > Lucas
> > > >
> > > > > Signed-off-by: Robin Gong <yibin.gong@nxp.com>
> > > > > ---
> > > > > drivers/dma/imx-sdma.c | 17 ++++++++++++++++-
> > > > > 1 file changed, 16 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> > > > > index
> > > > > b4ec2d2..5973489 100644
> > > > > --- a/drivers/dma/imx-sdma.c
> > > > > +++ b/drivers/dma/imx-sdma.c
> > > > > @@ -298,6 +298,15 @@ struct sdma_context_data {
> > > > > > u32 scratch7;
> > > > >
> > > > > } __attribute__ ((packed));
> > > > >
> > > > > +/*
> > > > > + * All bds in one transfer should be consitent on SDMA. To
> > > > > easily
> > > > > +follow it,just
> > > > > + * set the dma pool size as the enough bds. For example, in
> > > > > dmatest
> > > > > +case, the
> > > > > + * max 20 bds means the max for single transfer is NUM_BD *
> > > > > +SDMA_BD_MAX_CNT = 20
> > > > > + * * 65535 = ~1.28MB. 20 bds supposed to be enough basically.If
> > > > > it's
> > > > > +still not
> > > > > + * enough in some specific cases, enlarge it here.Warning
> > > > > message
> > > > > +would also
> > > > > + * appear if the bd numbers is over than 20.
> > > > > + */
> > > > > +#define NUM_BD 20
> > > > >
> > > > > struct sdma_engine;
> > > > >
> > > > > @@ -1273,7 +1282,7 @@ static int
> > > > > sdma_alloc_chan_resources(struct dma_chan *chan)
> > > > > > goto disable_clk_ahb;
> > > > > > sdmac->bd_pool = dma_pool_create("bd_pool", chan-
> > > > > > >device->dev,
> > > > > > - sizeof(struct
> > > > > > sdma_buffer_descriptor),
> > > > > > + NUM_BD * sizeof(struct
> > > > > > sdma_buffer_descriptor),
> > > > > > 32, 0);
> > > > > > return 0;
> > > > >
> > > > > @@ -1314,6 +1323,12 @@ static struct sdma_desc
> > > > > *sdma_transfer_init(struct sdma_channel *sdmac,
> > > > > {
> > > > > > struct sdma_desc *desc;
> > > > > > + if (bds > NUM_BD) {
> > > > > > + dev_err(sdmac->sdma->dev, "%d bds exceed the
> > > > > > max %d\n",
> > > > > > + bds, NUM_BD);
> > > > > > + goto err_out;
> > > > > > + }
> > > > >
> > > > > +
> > > > > > desc = kzalloc((sizeof(*desc)), GFP_NOWAIT);
> > > > > > if (!desc)
> > > > > > goto err_out;
^ permalink raw reply
* [v8,3/3] dmaengine: fsl-edma: add ColdFire mcf5441x edma support
From: Krzysztof Kozlowski @ 2018-08-06 8:09 UTC (permalink / raw)
To: Angelo Dureghello
Cc: vinod.koul, dmaengine, linux-arm-kernel, linux-m68k, Stefan Agner
On 3 August 2018 at 21:32, Angelo Dureghello <angelo@sysam.it> wrote:
> This patch adds support for ColdFire mcf5441x-family edma
> module.
>
> The ColdFire edma module is slightly different from fsl-edma,
> so a new driver is added. But most of the code is common
> between fsl-edma and mcf-edma so it has been collected into a
> separate common module fsl-edma-common (patch 1/3).
>
> Signed-off-by: Angelo Dureghello <angelo@sysam.it>
> ---
> Changes for v7:
> - patch rewritten from scratch, this patch (3/3) has just been added.
> ---
> drivers/dma/Kconfig | 11 +
> drivers/dma/Makefile | 1 +
> drivers/dma/fsl-edma-common.c | 24 +-
> drivers/dma/mcf-edma.c | 315 +++++++++++++++++++++
> include/linux/platform_data/dma-mcf-edma.h | 38 +++
> 5 files changed, 385 insertions(+), 4 deletions(-)
> create mode 100644 drivers/dma/mcf-edma.c
> create mode 100644 include/linux/platform_data/dma-mcf-edma.h
>
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index ca1680afa20a..b45008e9c7e9 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -320,6 +320,17 @@ config LPC18XX_DMAMUX
> Enable support for DMA on NXP LPC18xx/43xx platforms
> with PL080 and multiplexed DMA request lines.
>
> +config MCF_EDMA
> + tristate "Freescale eDMA engine support, ColdFire mcf5441x SoCs"
> + depends on M5441x
> + select DMA_ENGINE
> + select DMA_VIRTUAL_CHANNELS
> + help
> + Support the Freescale ColdFire eDMA engine, 64-channel
> + implementation that performs complex data transfers with
> + minimal intervention from a host processor.
> + This module can be found on Freescale ColdFire mcf5441x SoCs.
> +
> config MMP_PDMA
> bool "MMP PDMA support"
> depends on ARCH_MMP || ARCH_PXA || COMPILE_TEST
> diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
> index 66022f59fca4..d97f317f4b34 100644
> --- a/drivers/dma/Makefile
> +++ b/drivers/dma/Makefile
> @@ -32,6 +32,7 @@ obj-$(CONFIG_DW_DMAC_CORE) += dw/
> obj-$(CONFIG_EP93XX_DMA) += ep93xx_dma.o
> obj-$(CONFIG_FSL_DMA) += fsldma.o
> obj-$(CONFIG_FSL_EDMA) += fsl-edma.o fsl-edma-common.o
> +obj-$(CONFIG_MCF_EDMA) += mcf-edma.o fsl-edma-common.o
> obj-$(CONFIG_FSL_RAID) += fsl_raid.o
> obj-$(CONFIG_HSU_DMA) += hsu/
> obj-$(CONFIG_IMG_MDC_DMA) += img-mdc-dma.o
> diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
> index 948a3ee51bbb..5a830a238a0c 100644
> --- a/drivers/dma/fsl-edma-common.c
> +++ b/drivers/dma/fsl-edma-common.c
> @@ -102,8 +102,16 @@ static void fsl_edma_enable_request(struct fsl_edma_chan *fsl_chan)
> struct edma_regs *regs = &fsl_chan->edma->regs;
> u32 ch = fsl_chan->vchan.chan.chan_id;
>
> - edma_writeb(fsl_chan->edma, EDMA_SEEI_SEEI(ch), regs->seei);
> - edma_writeb(fsl_chan->edma, ch, regs->serq);
> + if (fsl_chan->edma->version == v1) {
> + edma_writeb(fsl_chan->edma, EDMA_SEEI_SEEI(ch), regs->seei);
> + edma_writeb(fsl_chan->edma, ch, regs->serq);
> + } else {
> + /* ColdFire is big endian, and accesses natively
> + * big endian I/O peripherals
> + */
> + iowrite8(EDMA_SEEI_SEEI(ch), regs->seei);
> + iowrite8(ch, regs->serq);
> + }
> }
>
> void fsl_edma_disable_request(struct fsl_edma_chan *fsl_chan)
> @@ -111,8 +119,16 @@ void fsl_edma_disable_request(struct fsl_edma_chan *fsl_chan)
> struct edma_regs *regs = &fsl_chan->edma->regs;
> u32 ch = fsl_chan->vchan.chan.chan_id;
>
> - edma_writeb(fsl_chan->edma, ch, regs->cerq);
> - edma_writeb(fsl_chan->edma, EDMA_CEEI_CEEI(ch), regs->ceei);
> + if (fsl_chan->edma->version == v1) {
> + edma_writeb(fsl_chan->edma, ch, regs->cerq);
> + edma_writeb(fsl_chan->edma, EDMA_CEEI_CEEI(ch), regs->ceei);
> + } else {
> + /* ColdFire is big endian, and accesses natively
> + * big endian I/O peripherals
> + */
> + iowrite8(ch, regs->cerq);
> + iowrite8(EDMA_CEEI_CEEI(ch), regs->ceei);
> + }
> }
> EXPORT_SYMBOL_GPL(fsl_edma_disable_request);
>
> diff --git a/drivers/dma/mcf-edma.c b/drivers/dma/mcf-edma.c
> new file mode 100644
> index 000000000000..31e5317a8f90
> --- /dev/null
> +++ b/drivers/dma/mcf-edma.c
> @@ -0,0 +1,315 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// Copyright (c) 2013-2014 Freescale Semiconductor, Inc
Same comment as to 1/3 - if this is derivative work...
> +// Copyright (c) 2017 Sysam, Angelo Dureghello <angelo@sysam.it>
> +
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/dmaengine.h>
> +#include <linux/platform_device.h>
> +#include <linux/platform_data/dma-mcf-edma.h>
> +
> +#include "fsl-edma-common.h"
> +
> +#define EDMA_CHANNELS 64
> +#define EDMA_MASK_CH(x) ((x) & GENMASK(5, 0))
> +
> +static irqreturn_t mcf_edma_tx_handler(int irq, void *dev_id)
> +{
> + struct fsl_edma_engine *mcf_edma = dev_id;
> + struct edma_regs *regs = &mcf_edma->regs;
> + unsigned int ch;
> + struct fsl_edma_chan *mcf_chan;
> + u64 intmap;
> +
> + intmap = ioread32(regs->inth);
> + intmap <<= 32;
> + intmap |= ioread32(regs->intl);
> + if (!intmap)
> + return IRQ_NONE;
> +
> + for (ch = 0; ch < mcf_edma->n_chans; ch++) {
> + if (intmap & BIT(ch)) {
> + iowrite8(EDMA_MASK_CH(ch), regs->cint);
> +
> + mcf_chan = &mcf_edma->chans[ch];
> +
> + spin_lock(&mcf_chan->vchan.lock);
> + if (!mcf_chan->edesc->iscyclic) {
> + list_del(&mcf_chan->edesc->vdesc.node);
> + vchan_cookie_complete(&mcf_chan->edesc->vdesc);
> + mcf_chan->edesc = NULL;
> + mcf_chan->status = DMA_COMPLETE;
> + mcf_chan->idle = true;
> + } else {
> + vchan_cyclic_callback(&mcf_chan->edesc->vdesc);
> + }
> +
> + if (!mcf_chan->edesc)
> + fsl_edma_xfer_desc(mcf_chan);
> +
> + spin_unlock(&mcf_chan->vchan.lock);
> + }
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t mcf_edma_err_handler(int irq, void *dev_id)
> +{
> + struct fsl_edma_engine *mcf_edma = dev_id;
> + struct edma_regs *regs = &mcf_edma->regs;
> + unsigned int err, ch;
> +
> + err = ioread32(regs->errl);
> + if (!err)
> + return IRQ_NONE;
> +
> + for (ch = 0; ch < (EDMA_CHANNELS / 2); ch++) {
> + if (err & BIT(ch)) {
> + fsl_edma_disable_request(&mcf_edma->chans[ch]);
> + iowrite8(EDMA_CERR_CERR(ch), regs->cerr);
> + mcf_edma->chans[ch].status = DMA_ERROR;
> + mcf_edma->chans[ch].idle = true;
> + }
> + }
> +
> + err = ioread32(regs->errh);
> + if (!err)
> + return IRQ_NONE;
> +
> + for (ch = (EDMA_CHANNELS / 2); ch < EDMA_CHANNELS; ch++) {
> + if (err & (BIT(ch - (EDMA_CHANNELS / 2)))) {
> + fsl_edma_disable_request(&mcf_edma->chans[ch]);
> + iowrite8(EDMA_CERR_CERR(ch), regs->cerr);
> + mcf_edma->chans[ch].status = DMA_ERROR;
> + mcf_edma->chans[ch].idle = true;
> + }
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int mcf_edma_irq_init(struct platform_device *pdev,
> + struct fsl_edma_engine *mcf_edma)
> +{
> + int ret = 0, i;
> + struct resource *res;
> +
> + res = platform_get_resource_byname(pdev,
> + IORESOURCE_IRQ, "edma-tx-00-15");
> + if (!res)
> + return -1;
> +
> + for (ret = 0, i = res->start; i <= res->end; ++i)
> + ret |= request_irq(i, mcf_edma_tx_handler, 0, "eDMA", mcf_edma);
> + if (ret)
> + return ret;
> +
> + res = platform_get_resource_byname(pdev,
> + IORESOURCE_IRQ, "edma-tx-16-55");
> + if (!res)
> + return -1;
> +
> + for (ret = 0, i = res->start; i <= res->end; ++i)
> + ret |= request_irq(i, mcf_edma_tx_handler, 0, "eDMA", mcf_edma);
> + if (ret)
> + return ret;
> +
> + ret = platform_get_irq_byname(pdev, "edma-tx-56-63");
> + if (ret != -ENXIO) {
> + ret = request_irq(ret, mcf_edma_tx_handler,
> + 0, "eDMA", mcf_edma);
> + if (ret)
> + return ret;
> + }
> +
> + ret = platform_get_irq_byname(pdev, "edma-err");
> + if (ret != -ENXIO) {
> + ret = request_irq(ret, mcf_edma_err_handler,
> + 0, "eDMA", mcf_edma);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static void mcf_edma_irq_free(struct platform_device *pdev,
> + struct fsl_edma_engine *mcf_edma)
> +{
> + int irq;
> + struct resource *res;
> +
> + res = platform_get_resource_byname(pdev,
> + IORESOURCE_IRQ, "edma-tx-00-15");
> + if (res) {
> + for (irq = res->start; irq <= res->end; irq++)
> + free_irq(irq, mcf_edma);
> + }
> +
> + res = platform_get_resource_byname(pdev,
> + IORESOURCE_IRQ, "edma-tx-16-55");
> + if (res) {
> + for (irq = res->start; irq <= res->end; irq++)
> + free_irq(irq, mcf_edma);
> + }
> +
> + irq = platform_get_irq_byname(pdev, "edma-tx-56-63");
> + if (irq != -ENXIO)
> + free_irq(irq, mcf_edma);
> +
> + irq = platform_get_irq_byname(pdev, "edma-err");
> + if (irq != -ENXIO)
> + free_irq(irq, mcf_edma);
> +}
> +
> +static int mcf_edma_probe(struct platform_device *pdev)
> +{
> + struct mcf_edma_platform_data *pdata;
> + struct fsl_edma_engine *mcf_edma;
> + struct fsl_edma_chan *mcf_chan;
> + struct edma_regs *regs;
> + struct resource *res;
> + int ret, i, len, chans;
> +
> + pdata = dev_get_platdata(&pdev->dev);
> + if (!pdata)
> + return PTR_ERR(pdata);
> +
> + chans = pdata->dma_channels;
> + len = sizeof(*mcf_edma) + sizeof(*mcf_chan) * chans;
> + mcf_edma = devm_kzalloc(&pdev->dev, len, GFP_KERNEL);
> + if (!mcf_edma)
> + return -ENOMEM;
> +
> + mcf_edma->n_chans = chans;
> +
> + /* Set up version for ColdFire edma */
> + mcf_edma->version = v2;
> + mcf_edma->big_endian = 1;
> +
> + if (!mcf_edma->n_chans) {
> + dev_info(&pdev->dev, "setting default channel number to 64");
> + mcf_edma->n_chans = 64;
> + }
> +
> + mutex_init(&mcf_edma->fsl_edma_mutex);
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> + mcf_edma->membase = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(mcf_edma->membase))
> + return PTR_ERR(mcf_edma->membase);
> +
> + fsl_edma_setup_regs(mcf_edma);
> + regs = &mcf_edma->regs;
> +
> + INIT_LIST_HEAD(&mcf_edma->dma_dev.channels);
> + for (i = 0; i < mcf_edma->n_chans; i++) {
> + struct fsl_edma_chan *mcf_chan = &mcf_edma->chans[i];
> +
> + mcf_chan->edma = mcf_edma;
> + mcf_chan->slave_id = i;
> + mcf_chan->idle = true;
> + mcf_chan->vchan.desc_free = fsl_edma_free_desc;
> + vchan_init(&mcf_chan->vchan, &mcf_edma->dma_dev);
> + iowrite32(0x0, ®s->tcd[i].csr);
> + }
> +
> + iowrite32(~0, regs->inth);
> + iowrite32(~0, regs->intl);
> +
> + ret = mcf_edma_irq_init(pdev, mcf_edma);
> + if (ret)
> + return ret;
> +
> + dma_cap_set(DMA_PRIVATE, mcf_edma->dma_dev.cap_mask);
> + dma_cap_set(DMA_SLAVE, mcf_edma->dma_dev.cap_mask);
> + dma_cap_set(DMA_CYCLIC, mcf_edma->dma_dev.cap_mask);
> +
> + mcf_edma->dma_dev.dev = &pdev->dev;
> + mcf_edma->dma_dev.device_alloc_chan_resources =
> + fsl_edma_alloc_chan_resources;
> + mcf_edma->dma_dev.device_free_chan_resources =
> + fsl_edma_free_chan_resources;
> + mcf_edma->dma_dev.device_config = fsl_edma_slave_config;
> + mcf_edma->dma_dev.device_prep_dma_cyclic =
> + fsl_edma_prep_dma_cyclic;
> + mcf_edma->dma_dev.device_prep_slave_sg = fsl_edma_prep_slave_sg;
> + mcf_edma->dma_dev.device_tx_status = fsl_edma_tx_status;
> + mcf_edma->dma_dev.device_pause = fsl_edma_pause;
> + mcf_edma->dma_dev.device_resume = fsl_edma_resume;
> + mcf_edma->dma_dev.device_terminate_all = fsl_edma_terminate_all;
> + mcf_edma->dma_dev.device_issue_pending = fsl_edma_issue_pending;
> +
> + mcf_edma->dma_dev.src_addr_widths = FSL_EDMA_BUSWIDTHS;
> + mcf_edma->dma_dev.dst_addr_widths = FSL_EDMA_BUSWIDTHS;
> + mcf_edma->dma_dev.directions =
> + BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
> +
> + mcf_edma->dma_dev.filter.fn = mcf_edma_filter_fn;
> + mcf_edma->dma_dev.filter.map = pdata->slave_map;
> + mcf_edma->dma_dev.filter.mapcnt = pdata->slavecnt;
> +
> + platform_set_drvdata(pdev, mcf_edma);
> +
> + ret = dma_async_device_register(&mcf_edma->dma_dev);
> + if (ret) {
> + dev_err(&pdev->dev,
> + "Can't register Freescale eDMA engine. (%d)\n", ret);
> + return ret;
> + }
> +
> + /* Enable round robin arbitration */
> + iowrite32(EDMA_CR_ERGA | EDMA_CR_ERCA, regs->cr);
> +
> + return 0;
> +}
> +
> +static int mcf_edma_remove(struct platform_device *pdev)
> +{
> + struct fsl_edma_engine *mcf_edma = platform_get_drvdata(pdev);
> +
> + mcf_edma_irq_free(pdev, mcf_edma);
> + fsl_edma_cleanup_vchan(&mcf_edma->dma_dev);
> + dma_async_device_unregister(&mcf_edma->dma_dev);
> +
> + return 0;
> +}
> +
> +static struct platform_driver mcf_edma_driver = {
> + .driver = {
> + .name = "mcf-edma",
> + },
> + .probe = mcf_edma_probe,
> + .remove = mcf_edma_remove,
> +};
> +
> +bool mcf_edma_filter_fn(struct dma_chan *chan, void *param)
> +{
> + if (chan->device->dev->driver == &mcf_edma_driver.driver) {
> + struct fsl_edma_chan *mcf_chan = to_fsl_edma_chan(chan);
> +
> + return (mcf_chan->slave_id == (int)param);
> + }
> +
> + return false;
> +}
> +EXPORT_SYMBOL(mcf_edma_filter_fn);
> +
> +static int __init mcf_edma_init(void)
> +{
> + return platform_driver_register(&mcf_edma_driver);
> +}
> +subsys_initcall(mcf_edma_init);
> +
> +static void __exit mcf_edma_exit(void)
> +{
> + platform_driver_unregister(&mcf_edma_driver);
> +}
> +module_exit(mcf_edma_exit);
> +
> +MODULE_ALIAS("platform:mcf-edma");
> +MODULE_DESCRIPTION("Freescale eDMA engine driver, ColdFire family");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/platform_data/dma-mcf-edma.h b/include/linux/platform_data/dma-mcf-edma.h
> new file mode 100644
> index 000000000000..9a1819acb28f
> --- /dev/null
> +++ b/include/linux/platform_data/dma-mcf-edma.h
> @@ -0,0 +1,38 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Freescale eDMA platform data, ColdFire SoC's family.
> + *
> + * Copyright (c) 2017 Angelo Dureghello <angelo@sysam.it>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef __MACH_MCF_EDMA_H__
> +#define __MACH_MCF_EDMA_H__
The guard should rather match current location, e.g.
__LINUX_PLATFORM_DATA_MCF_EDMA_H__
Tested-by: Krzysztof Kozlowski <krzk@kernel.org>
Best regards,
Krzysztof
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [v2,1/9] dmaengine: dw-axi-dmac: use dmaenginem_async_device_register to simplify the code
From: Huang Shijie @ 2018-08-06 8:52 UTC (permalink / raw)
To: vkoul
Cc: dan.j.williams, Zubair.Kakakhel, daniel, haojian.zhuang,
robert.jarzmik, patrice.chotard, linus.walleij, mcoquelin.stm32,
alexandre.torgue, Eugeniy.Paltsev, colin.king, dmaengine,
linux-arm-kernel, Huang Shijie
Use dmaenginem_async_device_register to simplify the code:
remove dma_async_device_unregister.
Signed-off-by: Huang Shijie <sjhuang@iluvatar.ai>
---
drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
index c4eb55e3011c..b2ac1d2c5b86 100644
--- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
+++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
@@ -934,7 +934,7 @@ static int dw_probe(struct platform_device *pdev)
pm_runtime_put(chip->dev);
- ret = dma_async_device_register(&dw->dma);
+ ret = dmaenginem_async_device_register(&dw->dma);
if (ret)
goto err_pm_disable;
@@ -977,8 +977,6 @@ static int dw_remove(struct platform_device *pdev)
tasklet_kill(&chan->vc.task);
}
- dma_async_device_unregister(&dw->dma);
-
return 0;
}
^ permalink raw reply related
* [v2,2/9] dmaengine: stm32-mdma: use dmaenginem_async_device_register to simplify the code
From: Huang Shijie @ 2018-08-06 8:52 UTC (permalink / raw)
To: vkoul
Cc: dan.j.williams, Zubair.Kakakhel, daniel, haojian.zhuang,
robert.jarzmik, patrice.chotard, linus.walleij, mcoquelin.stm32,
alexandre.torgue, Eugeniy.Paltsev, colin.king, dmaengine,
linux-arm-kernel, Huang Shijie
Use dmaenginem_async_device_register to simplify the code:
remove dma_async_device_unregister.
Acked-by: Pierre-Yves MORDRET <pierre-yves.mordret@st.com>
Signed-off-by: Huang Shijie <sjhuang@iluvatar.ai>
---
drivers/dma/stm32-mdma.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/dma/stm32-mdma.c b/drivers/dma/stm32-mdma.c
index 06dd1725375e..390e4cae0e1a 100644
--- a/drivers/dma/stm32-mdma.c
+++ b/drivers/dma/stm32-mdma.c
@@ -1656,7 +1656,7 @@ static int stm32_mdma_probe(struct platform_device *pdev)
return ret;
}
- ret = dma_async_device_register(dd);
+ ret = dmaenginem_async_device_register(dd);
if (ret)
return ret;
@@ -1674,8 +1674,6 @@ static int stm32_mdma_probe(struct platform_device *pdev)
return 0;
err_unregister:
- dma_async_device_unregister(dd);
-
return ret;
}
^ permalink raw reply related
* [v2,3/9] dmaengine: st_fdma: use dmaenginem_async_device_register to simplify the code
From: Huang Shijie @ 2018-08-06 8:52 UTC (permalink / raw)
To: vkoul
Cc: dan.j.williams, Zubair.Kakakhel, daniel, haojian.zhuang,
robert.jarzmik, patrice.chotard, linus.walleij, mcoquelin.stm32,
alexandre.torgue, Eugeniy.Paltsev, colin.king, dmaengine,
linux-arm-kernel, Huang Shijie
Use dmaenginem_async_device_register to simplify the code:
remove dma_async_device_unregister.
Signed-off-by: Huang Shijie <sjhuang@iluvatar.ai>
---
drivers/dma/st_fdma.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/dma/st_fdma.c b/drivers/dma/st_fdma.c
index bfb79bd0c6de..7eafccd4f7f5 100644
--- a/drivers/dma/st_fdma.c
+++ b/drivers/dma/st_fdma.c
@@ -833,7 +833,7 @@ static int st_fdma_probe(struct platform_device *pdev)
fdev->dma_device.directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
fdev->dma_device.residue_granularity = DMA_RESIDUE_GRANULARITY_BURST;
- ret = dma_async_device_register(&fdev->dma_device);
+ ret = dmaenginem_async_device_register(&fdev->dma_device);
if (ret) {
dev_err(&pdev->dev,
"Failed to register DMA device (%d)\n", ret);
@@ -852,7 +852,6 @@ static int st_fdma_probe(struct platform_device *pdev)
return 0;
err_dma_dev:
- dma_async_device_unregister(&fdev->dma_device);
err_rproc:
st_fdma_free(fdev);
st_slim_rproc_put(fdev->slim_rproc);
@@ -867,7 +866,6 @@ static int st_fdma_remove(struct platform_device *pdev)
devm_free_irq(&pdev->dev, fdev->irq, fdev);
st_slim_rproc_put(fdev->slim_rproc);
of_dma_controller_free(pdev->dev.of_node);
- dma_async_device_unregister(&fdev->dma_device);
return 0;
}
^ permalink raw reply related
* [v2,4/9] dmaengine: mmp_tdma: use dmaenginem_async_device_register to simplify the code
From: Huang Shijie @ 2018-08-06 8:52 UTC (permalink / raw)
To: vkoul
Cc: dan.j.williams, Zubair.Kakakhel, daniel, haojian.zhuang,
robert.jarzmik, patrice.chotard, linus.walleij, mcoquelin.stm32,
alexandre.torgue, Eugeniy.Paltsev, colin.king, dmaengine,
linux-arm-kernel, Huang Shijie
Use dmaenginem_async_device_register to simplify the code:
remove dma_async_device_unregister.
return error if it fails.
Signed-off-by: Huang Shijie <sjhuang@iluvatar.ai>
---
drivers/dma/mmp_tdma.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/dma/mmp_tdma.c b/drivers/dma/mmp_tdma.c
index 13c68b6434ce..9f5869e1a017 100644
--- a/drivers/dma/mmp_tdma.c
+++ b/drivers/dma/mmp_tdma.c
@@ -530,9 +530,6 @@ static void mmp_tdma_issue_pending(struct dma_chan *chan)
static int mmp_tdma_remove(struct platform_device *pdev)
{
- struct mmp_tdma_device *tdev = platform_get_drvdata(pdev);
-
- dma_async_device_unregister(&tdev->device);
return 0;
}
@@ -696,7 +693,7 @@ static int mmp_tdma_probe(struct platform_device *pdev)
dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
platform_set_drvdata(pdev, tdev);
- ret = dma_async_device_register(&tdev->device);
+ ret = dmaenginem_async_device_register(&tdev->device);
if (ret) {
dev_err(tdev->device.dev, "unable to register\n");
return ret;
@@ -708,7 +705,7 @@ static int mmp_tdma_probe(struct platform_device *pdev)
if (ret) {
dev_err(tdev->device.dev,
"failed to register controller\n");
- dma_async_device_unregister(&tdev->device);
+ return ret;
}
}
^ permalink raw reply related
* [v2,5/9] dmaengine: ste_dma40: use dmaenginem_async_device_register to simplify the code
From: Huang Shijie @ 2018-08-06 8:52 UTC (permalink / raw)
To: vkoul
Cc: dan.j.williams, Zubair.Kakakhel, daniel, haojian.zhuang,
robert.jarzmik, patrice.chotard, linus.walleij, mcoquelin.stm32,
alexandre.torgue, Eugeniy.Paltsev, colin.king, dmaengine,
linux-arm-kernel, Huang Shijie
Use dmaenginem_async_device_register to simplify the code:
remove dma_async_device_unregister.
remove label unregister_slave, unregister_memcpy
Signed-off-by: Huang Shijie <sjhuang@iluvatar.ai>
---
drivers/dma/ste_dma40.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index f4edfc56f34e..5e328bd10c27 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -2839,7 +2839,7 @@ static int __init d40_dmaengine_init(struct d40_base *base,
d40_ops_init(base, &base->dma_slave);
- err = dma_async_device_register(&base->dma_slave);
+ err = dmaenginem_async_device_register(&base->dma_slave);
if (err) {
d40_err(base->dev, "Failed to register slave channels\n");
@@ -2854,12 +2854,12 @@ static int __init d40_dmaengine_init(struct d40_base *base,
d40_ops_init(base, &base->dma_memcpy);
- err = dma_async_device_register(&base->dma_memcpy);
+ err = dmaenginem_async_device_register(&base->dma_memcpy);
if (err) {
d40_err(base->dev,
"Failed to register memcpy only channels\n");
- goto unregister_slave;
+ goto exit;
}
d40_chan_init(base, &base->dma_both, base->phy_chans,
@@ -2871,18 +2871,14 @@ static int __init d40_dmaengine_init(struct d40_base *base,
dma_cap_set(DMA_CYCLIC, base->dma_slave.cap_mask);
d40_ops_init(base, &base->dma_both);
- err = dma_async_device_register(&base->dma_both);
+ err = dmaenginem_async_device_register(&base->dma_both);
if (err) {
d40_err(base->dev,
"Failed to register logical and physical capable channels\n");
- goto unregister_memcpy;
+ goto exit;
}
return 0;
- unregister_memcpy:
- dma_async_device_unregister(&base->dma_memcpy);
- unregister_slave:
- dma_async_device_unregister(&base->dma_slave);
exit:
return err;
}
^ permalink raw reply related
* [v2,6/9] dmaengine: dma-jz4780: use dmaenginem_async_device_register to simplify the code
From: Huang Shijie @ 2018-08-06 8:52 UTC (permalink / raw)
To: vkoul
Cc: dan.j.williams, Zubair.Kakakhel, daniel, haojian.zhuang,
robert.jarzmik, patrice.chotard, linus.walleij, mcoquelin.stm32,
alexandre.torgue, Eugeniy.Paltsev, colin.king, dmaengine,
linux-arm-kernel, Huang Shijie
Use dmaenginem_async_device_register to simplify the code:
remove dma_async_device_unregister.
remove label err_unregister_dev
Signed-off-by: Huang Shijie <sjhuang@iluvatar.ai>
---
drivers/dma/dma-jz4780.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
index 85820a2d69d4..b2a244902bc5 100644
--- a/drivers/dma/dma-jz4780.c
+++ b/drivers/dma/dma-jz4780.c
@@ -847,7 +847,7 @@ static int jz4780_dma_probe(struct platform_device *pdev)
jzchan->vchan.desc_free = jz4780_dma_desc_free;
}
- ret = dma_async_device_register(dd);
+ ret = dmaenginem_async_device_register(dd);
if (ret) {
dev_err(dev, "failed to register device\n");
goto err_disable_clk;
@@ -858,15 +858,12 @@ static int jz4780_dma_probe(struct platform_device *pdev)
jzdma);
if (ret) {
dev_err(dev, "failed to register OF DMA controller\n");
- goto err_unregister_dev;
+ goto err_disable_clk;
}
dev_info(dev, "JZ4780 DMA controller initialised\n");
return 0;
-err_unregister_dev:
- dma_async_device_unregister(dd);
-
err_disable_clk:
clk_disable_unprepare(jzdma->clk);
@@ -887,7 +884,6 @@ static int jz4780_dma_remove(struct platform_device *pdev)
for (i = 0; i < JZ_DMA_NR_CHANNELS; i++)
tasklet_kill(&jzdma->chan[i].vchan.task);
- dma_async_device_unregister(&jzdma->dma_device);
return 0;
}
^ permalink raw reply related
* [v2,7/9] dmaengine: pxa_dma: use dmaenginem_async_device_register to simplify the code
From: Huang Shijie @ 2018-08-06 8:52 UTC (permalink / raw)
To: vkoul
Cc: dan.j.williams, Zubair.Kakakhel, daniel, haojian.zhuang,
robert.jarzmik, patrice.chotard, linus.walleij, mcoquelin.stm32,
alexandre.torgue, Eugeniy.Paltsev, colin.king, dmaengine,
linux-arm-kernel, Huang Shijie
Use dmaenginem_async_device_register to simplify the code:
remove dma_async_device_unregister.
Signed-off-by: Huang Shijie <sjhuang@iluvatar.ai>
---
drivers/dma/pxa_dma.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/dma/pxa_dma.c b/drivers/dma/pxa_dma.c
index b31c28b67ad3..21001ea078fb 100644
--- a/drivers/dma/pxa_dma.c
+++ b/drivers/dma/pxa_dma.c
@@ -1285,7 +1285,6 @@ static int pxad_remove(struct platform_device *op)
pxad_cleanup_debugfs(pdev);
pxad_free_channels(&pdev->slave);
- dma_async_device_unregister(&pdev->slave);
return 0;
}
@@ -1396,7 +1395,7 @@ static int pxad_init_dmadev(struct platform_device *op,
init_waitqueue_head(&c->wq_state);
}
- return dma_async_device_register(&pdev->slave);
+ return dmaenginem_async_device_register(&pdev->slave);
}
static int pxad_probe(struct platform_device *op)
^ permalink raw reply related
* [v2,8/9] dmaengine: mxs-dma: use dmaenginem_async_device_register to simplify the code
From: Huang Shijie @ 2018-08-06 8:52 UTC (permalink / raw)
To: vkoul
Cc: dan.j.williams, Zubair.Kakakhel, daniel, haojian.zhuang,
robert.jarzmik, patrice.chotard, linus.walleij, mcoquelin.stm32,
alexandre.torgue, Eugeniy.Paltsev, colin.king, dmaengine,
linux-arm-kernel, Huang Shijie
Use dmaenginem_async_device_register to simplify the code:
remove dma_async_device_unregister.
Signed-off-by: Huang Shijie <sjhuang@iluvatar.ai>
---
drivers/dma/mxs-dma.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
index ae5182ff0128..35193b31a9e0 100644
--- a/drivers/dma/mxs-dma.c
+++ b/drivers/dma/mxs-dma.c
@@ -847,7 +847,7 @@ static int __init mxs_dma_probe(struct platform_device *pdev)
mxs_dma->dma_device.residue_granularity = DMA_RESIDUE_GRANULARITY_BURST;
mxs_dma->dma_device.device_issue_pending = mxs_dma_enable_chan;
- ret = dma_async_device_register(&mxs_dma->dma_device);
+ ret = dmaenginem_async_device_register(&mxs_dma->dma_device);
if (ret) {
dev_err(mxs_dma->dma_device.dev, "unable to register\n");
return ret;
@@ -857,7 +857,6 @@ static int __init mxs_dma_probe(struct platform_device *pdev)
if (ret) {
dev_err(mxs_dma->dma_device.dev,
"failed to register controller\n");
- dma_async_device_unregister(&mxs_dma->dma_device);
}
dev_info(mxs_dma->dma_device.dev, "initialized\n");
^ permalink raw reply related
* [v2,9/9] dmaengine: add COMPILE_TEST for the drivers
From: Huang Shijie @ 2018-08-06 8:52 UTC (permalink / raw)
To: vkoul
Cc: dan.j.williams, Zubair.Kakakhel, daniel, haojian.zhuang,
robert.jarzmik, patrice.chotard, linus.walleij, mcoquelin.stm32,
alexandre.torgue, Eugeniy.Paltsev, colin.king, dmaengine,
linux-arm-kernel, Huang Shijie
We can do the compiling test with COMPILE_TEST.
This patch adds the COMPILE_TEST for the drivers.
Signed-off-by: Huang Shijie <sjhuang@iluvatar.ai>
---
drivers/dma/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index d4a4230a7942..1f0dceb055d8 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -439,7 +439,7 @@ config PL330_DMA
config PXA_DMA
bool "PXA DMA support"
- depends on (ARCH_MMP || ARCH_PXA)
+ depends on (ARCH_MMP || ARCH_PXA || COMPILE_TEST)
select DMA_ENGINE
select DMA_VIRTUAL_CHANNELS
help
@@ -457,7 +457,7 @@ config SIRF_DMA
config STE_DMA40
bool "ST-Ericsson DMA40 support"
- depends on ARCH_U8500
+ depends on ARCH_U8500 || COMPILE_TEST
select DMA_ENGINE
help
Support for ST-Ericsson DMA40 controller
^ permalink raw reply related
* [v2,2/3] dmaengine: imx-sdma: add memcpy interface
From: Sascha Hauer @ 2018-08-06 8:53 UTC (permalink / raw)
To: Robin Gong
Cc: linux@armlinux.org.uk, linux-kernel@vger.kernel.org,
dmaengine@vger.kernel.org, vkoul@kernel.org, dl-linux-imx,
kernel@pengutronix.de, Fabio Estevam, dan.j.williams@intel.com,
shawnguo@kernel.org, linux-arm-kernel@lists.infradead.org
On Fri, Jul 20, 2018 at 09:40:51AM +0000, Robin Gong wrote:
> > -----Original Message-----
> > From: Sascha Hauer [mailto:s.hauer@pengutronix.de]
> > Sent: 2018年7月13日 14:16
> > To: Robin Gong <yibin.gong@nxp.com>
> > Cc: vkoul@kernel.org; dan.j.williams@intel.com; shawnguo@kernel.org; Fabio
> > Estevam <fabio.estevam@nxp.com>; linux@armlinux.org.uk;
> > linux-arm-kernel@lists.infradead.org; kernel@pengutronix.de;
> > dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx
> > <linux-imx@nxp.com>
> > Subject: Re: [PATCH v2 2/3] dmaengine: imx-sdma: add memcpy interface
> >
> > On Fri, Jul 13, 2018 at 09:08:46PM +0800, Robin Gong wrote:
> > > Add MEMCPY capability for imx-sdma driver.
> > >
> > > Signed-off-by: Robin Gong <yibin.gong@nxp.com>
> > > ---
> > > drivers/dma/imx-sdma.c | 95
> > > ++++++++++++++++++++++++++++++++++++++++++++++++--
> > > 1 file changed, 92 insertions(+), 3 deletions(-)
> > >
> > > @@ -1318,6 +1347,63 @@ static struct sdma_desc
> > *sdma_transfer_init(struct sdma_channel *sdmac,
> > > return NULL;
> > > }
> > >
> > > +static struct dma_async_tx_descriptor *sdma_prep_memcpy(
> > > + struct dma_chan *chan, dma_addr_t dma_dst,
> > > + dma_addr_t dma_src, size_t len, unsigned long flags) {
> > > + struct sdma_channel *sdmac = to_sdma_chan(chan);
> > > + struct sdma_engine *sdma = sdmac->sdma;
> > > + int channel = sdmac->channel;
> > > + size_t count;
> > > + int i = 0, param;
> > > + struct sdma_buffer_descriptor *bd;
> > > + struct sdma_desc *desc;
> > > +
> > > + if (!chan || !len)
> > > + return NULL;
> > > +
> > > + dev_dbg(sdma->dev, "memcpy: %pad->%pad, len=%zu, channel=%d.\n",
> > > + &dma_src, &dma_dst, len, channel);
> > > +
> > > + desc = sdma_transfer_init(sdmac, DMA_MEM_TO_MEM,
> > > + len / SDMA_BD_MAX_CNT + 1);
> > > + if (!desc)
> > > + return NULL;
> > > +
> > > + do {
> > > + count = min_t(size_t, len, SDMA_BD_MAX_CNT);
> > > + bd = &desc->bd[i];
> > > + bd->buffer_addr = dma_src;
> > > + bd->ext_buffer_addr = dma_dst;
> > > + bd->mode.count = count;
> > > + desc->chn_count += count;
> > > + /* align with sdma->dma_device.copy_align: 4bytes */
> > > + bd->mode.command = 0;
> > > +
> > > + dma_src += count;
> > > + dma_dst += count;
> > > + len -= count;
> > > + i++;
> >
> > NACK.
> >
> > Please actually look at your code and find out where you do unaligned DMA
> > accesses. Hint: What happens when this loop body is executed more than
> > once?
> >
> > Sascha
> Actually internal sdma script has already take such 'align' matter like below:
> Burst with words if source/dest address and data length are all aligned with words, burst with
> Byte if not.
If that's the case why did you add the DMAENGINE_ALIGN_4_BYTES flag in the
first place?
Also, if the SDMA script falls back to byte copy when necessary, doing
so probably has performance impacts, so I wonder why you don't just cut
'count' to something that is four byte aligned.
Sascha
^ permalink raw reply
* [v2,3/9] dmaengine: st_fdma: use dmaenginem_async_device_register to simplify the code
From: Daniel Mack @ 2018-08-06 9:03 UTC (permalink / raw)
To: Huang Shijie, vkoul
Cc: dan.j.williams, Zubair.Kakakhel, haojian.zhuang, robert.jarzmik,
patrice.chotard, linus.walleij, mcoquelin.stm32, alexandre.torgue,
Eugeniy.Paltsev, colin.king, dmaengine, linux-arm-kernel
On Monday, August 06, 2018 10:52 AM, Huang Shijie wrote:
> Use dmaenginem_async_device_register to simplify the code:
> remove dma_async_device_unregister.
>
> Signed-off-by: Huang Shijie <sjhuang@iluvatar.ai>
> ---
> drivers/dma/st_fdma.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/dma/st_fdma.c b/drivers/dma/st_fdma.c
> index bfb79bd0c6de..7eafccd4f7f5 100644
> --- a/drivers/dma/st_fdma.c
> +++ b/drivers/dma/st_fdma.c
> @@ -833,7 +833,7 @@ static int st_fdma_probe(struct platform_device *pdev)
> fdev->dma_device.directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
> fdev->dma_device.residue_granularity = DMA_RESIDUE_GRANULARITY_BURST;
>
> - ret = dma_async_device_register(&fdev->dma_device);
> + ret = dmaenginem_async_device_register(&fdev->dma_device);
> if (ret) {
> dev_err(&pdev->dev,
> "Failed to register DMA device (%d)\n", ret);
> @@ -852,7 +852,6 @@ static int st_fdma_probe(struct platform_device *pdev)
> return 0;
>
> err_dma_dev:
> - dma_async_device_unregister(&fdev->dma_device);
I guess the jump label can go away too?
Thanks,
Daniel
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [v2,3/9] dmaengine: st_fdma: use dmaenginem_async_device_register to simplify the code
From: Huang Shijie @ 2018-08-06 9:22 UTC (permalink / raw)
To: Daniel Mack
Cc: vkoul, dan.j.williams, Zubair.Kakakhel, haojian.zhuang,
robert.jarzmik, patrice.chotard, linus.walleij, mcoquelin.stm32,
alexandre.torgue, Eugeniy.Paltsev, colin.king, dmaengine,
linux-arm-kernel
On Mon, Aug 06, 2018 at 11:03:51AM +0200, Daniel Mack wrote:
> > - ret = dma_async_device_register(&fdev->dma_device);
> > + ret = dmaenginem_async_device_register(&fdev->dma_device);
> > if (ret) {
> > dev_err(&pdev->dev,
> > "Failed to register DMA device (%d)\n", ret);
> > @@ -852,7 +852,6 @@ static int st_fdma_probe(struct platform_device *pdev)
> > return 0;
> > err_dma_dev:
> > - dma_async_device_unregister(&fdev->dma_device);
>
> I guess the jump label can go away too?
Yes.
We can remove the err_dma_dev too.
Thanks
Huang Shijie
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [V2,fix] dmaengine: st_fdma: use dmaenginem_async_device_register to simplify the code
From: Huang Shijie @ 2018-08-06 9:43 UTC (permalink / raw)
To: vkoul
Cc: dan.j.williams, Zubair.Kakakhel, daniel, haojian.zhuang,
robert.jarzmik, patrice.chotard, linus.walleij, mcoquelin.stm32,
alexandre.torgue, Eugeniy.Paltsev, colin.king, dmaengine,
linux-arm-kernel, Huang Shijie
Use dmaenginem_async_device_register to simplify the code:
remove dma_async_device_unregister.
remove label err_dma_dev
Signed-off-by: Huang Shijie <sjhuang@iluvatar.ai>
---
Remove the label err_dma_dev
---
drivers/dma/st_fdma.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/dma/st_fdma.c b/drivers/dma/st_fdma.c
index bfb79bd0c6de..07c20aa2e955 100644
--- a/drivers/dma/st_fdma.c
+++ b/drivers/dma/st_fdma.c
@@ -833,7 +833,7 @@ static int st_fdma_probe(struct platform_device *pdev)
fdev->dma_device.directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
fdev->dma_device.residue_granularity = DMA_RESIDUE_GRANULARITY_BURST;
- ret = dma_async_device_register(&fdev->dma_device);
+ ret = dmaenginem_async_device_register(&fdev->dma_device);
if (ret) {
dev_err(&pdev->dev,
"Failed to register DMA device (%d)\n", ret);
@@ -844,15 +844,13 @@ static int st_fdma_probe(struct platform_device *pdev)
if (ret) {
dev_err(&pdev->dev,
"Failed to register controller (%d)\n", ret);
- goto err_dma_dev;
+ goto err_rproc;
}
dev_info(&pdev->dev, "ST FDMA engine driver, irq:%d\n", fdev->irq);
return 0;
-err_dma_dev:
- dma_async_device_unregister(&fdev->dma_device);
err_rproc:
st_fdma_free(fdev);
st_slim_rproc_put(fdev->slim_rproc);
@@ -867,7 +865,6 @@ static int st_fdma_remove(struct platform_device *pdev)
devm_free_irq(&pdev->dev, fdev->irq, fdev);
st_slim_rproc_put(fdev->slim_rproc);
of_dma_controller_free(pdev->dev.of_node);
- dma_async_device_unregister(&fdev->dma_device);
return 0;
}
^ permalink raw reply related
* [v2,5/9] dmaengine: ste_dma40: use dmaenginem_async_device_register to simplify the code
From: Linus Walleij @ 2018-08-06 10:34 UTC (permalink / raw)
To: sjhuang
Cc: vkoul, Dan Williams, Zubair Lutfullah Kakakhel, Daniel Mack,
Haojian Zhuang, Robert Jarzmik, Patrice CHOTARD, Maxime Coquelin,
Alexandre TORGUE, Eugeniy.Paltsev, Colin King, dmaengine,
Linux ARM
On Mon, Aug 6, 2018 at 10:53 AM Huang Shijie <sjhuang@iluvatar.ai> wrote:
> Use dmaenginem_async_device_register to simplify the code:
> remove dma_async_device_unregister.
> remove label unregister_slave, unregister_memcpy
>
> Signed-off-by: Huang Shijie <sjhuang@iluvatar.ai>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [v2,9/9] dmaengine: add COMPILE_TEST for the drivers
From: Linus Walleij @ 2018-08-06 10:35 UTC (permalink / raw)
To: sjhuang
Cc: vkoul, Dan Williams, Zubair Lutfullah Kakakhel, Daniel Mack,
Haojian Zhuang, Robert Jarzmik, Patrice CHOTARD, Maxime Coquelin,
Alexandre TORGUE, Eugeniy.Paltsev, Colin King, dmaengine,
Linux ARM
On Mon, Aug 6, 2018 at 10:53 AM Huang Shijie <sjhuang@iluvatar.ai> wrote:
> We can do the compiling test with COMPILE_TEST.
> This patch adds the COMPILE_TEST for the drivers.
>
> Signed-off-by: Huang Shijie <sjhuang@iluvatar.ai>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [v8,3/3] dmaengine: fsl-edma: add ColdFire mcf5441x edma support
From: Stefan Agner @ 2018-08-06 12:21 UTC (permalink / raw)
To: Krzysztof Kozlowski, Angelo Dureghello
Cc: vinod.koul, dmaengine, linux-arm-kernel, linux-m68k
On 06.08.2018 10:09, Krzysztof Kozlowski wrote:
> On 3 August 2018 at 21:32, Angelo Dureghello <angelo@sysam.it> wrote:
>> This patch adds support for ColdFire mcf5441x-family edma
>> module.
>>
>> The ColdFire edma module is slightly different from fsl-edma,
>> so a new driver is added. But most of the code is common
>> between fsl-edma and mcf-edma so it has been collected into a
>> separate common module fsl-edma-common (patch 1/3).
>>
>> Signed-off-by: Angelo Dureghello <angelo@sysam.it>
[...]
>> diff --git a/include/linux/platform_data/dma-mcf-edma.h b/include/linux/platform_data/dma-mcf-edma.h
>> new file mode 100644
>> index 000000000000..9a1819acb28f
>> --- /dev/null
>> +++ b/include/linux/platform_data/dma-mcf-edma.h
>> @@ -0,0 +1,38 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Freescale eDMA platform data, ColdFire SoC's family.
>> + *
>> + * Copyright (c) 2017 Angelo Dureghello <angelo@sysam.it>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#ifndef __MACH_MCF_EDMA_H__
>> +#define __MACH_MCF_EDMA_H__
>
> The guard should rather match current location, e.g.
> __LINUX_PLATFORM_DATA_MCF_EDMA_H__
>
> Tested-by: Krzysztof Kozlowski <krzk@kernel.org>
>
So this revision fixed the issue we had seen with the previous version?
Do we know what it was exactly?
---
Stefan
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [v3,3/3] dmaengine: imx-sdma: allocate max 20 bds for one transfer
From: Lucas Stach @ 2018-08-06 12:29 UTC (permalink / raw)
To: Robin Gong, vkoul@kernel.org, dan.j.williams@intel.com,
s.hauer@pengutronix.de, linux@armlinux.org.uk
Cc: dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
dl-linux-imx, kernel@pengutronix.de,
linux-arm-kernel@lists.infradead.org
Hi Robin,
Am Montag, den 06.08.2018, 08:04 +0000 schrieb Robin Gong:
> Hello Lucas,
> Any comment for my reply?
So I've looked at this again and sadly I need to NACK this patch. It is
a total API abuse of the dma_pool API and even the patch introducing
the dma_pool usage in this driver is wrong and should be reverted.
The SDMA need contiguous buffer descriptors for each channel, something
the dma_pool abstraction isn't able to provide. So either the dma_pool
implementation needs to be extended to support this use-case, or you
can't use this at all in the sdma driver. Adding hacks, which are
abusing the API, to cram a dma_pool into the sdma driver is not a valid
way to implement things for upstream.
Regards,
Lucas
> > -----Original Message-----
> > From: Robin Gong
> > Sent: 2018年7月25日 9:25
> > To: 'Lucas Stach' <l.stach@pengutronix.de>; vkoul@kernel.org;
> > dan.j.williams@intel.com; s.hauer@pengutronix.de; linux@armlinux.or
> > g.uk
> > Cc: dmaengine@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> > kernel@pengutronix.de; linux-arm-kernel@lists.infradead.org;
> > linux-kernel@vger.kernel.org
> > Subject: RE: [PATCH v3 3/3] dmaengine: imx-sdma: allocate max 20
> > bds for one
> > transfer
> >
> > > -----Original Message-----
> > > From: Lucas Stach [mailto:l.stach@pengutronix.de]
> > > Sent: 2018年7月24日 17:22
> > > To: Robin Gong <yibin.gong@nxp.com>; vkoul@kernel.org;
> > > dan.j.williams@intel.com; s.hauer@pengutronix.de;
> > > linux@armlinux.org.uk
> > > Cc: dmaengine@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> > > kernel@pengutronix.de; linux-arm-kernel@lists.infradead.org;
> > > linux-kernel@vger.kernel.org
> > > Subject: Re: [PATCH v3 3/3] dmaengine: imx-sdma: allocate max 20
> > > bds
> > > for one transfer
> > >
> > > Am Montag, den 23.07.2018, 13:55 +0000 schrieb Robin Gong:
> > > > > -----Original Message-----
> > > > > From: Lucas Stach [mailto:l.stach@pengutronix.de]
> > > > > Sent: 2018年7月23日 18:54
> > > > > To: Robin Gong <yibin.gong@nxp.com>; vkoul@kernel.org;
> > > > > dan.j.williams@intel.com; s.hauer@pengutronix.de;
> > > > > linux@armlinux.or g.uk
> > > > > Cc: dmaengine@vger.kernel.org; dl-linux-imx <linux-imx@nxp.co
> > > > > m>;
> > > > > kernel@pengutronix.de; linux-arm-kernel@lists.infradead.org;
> > > > > linux-kernel@vger.kernel.org
> > > > > Subject: Re: [PATCH v3 3/3] dmaengine: imx-sdma: allocate max
> > > > > 20
> > > > > bds for one transfer
> > > > >
> > > > > Am Dienstag, den 24.07.2018, 01:46 +0800 schrieb Robin Gong:
> > > > > > If multi-bds used in one transfer, all bds should be
> > > > > > consisten
> > > > > > memory.To easily follow it, enlarge the dma pool size into
> > > > > > 20
> > > > > > bds, and it will report error if the number of bds is over
> > > > > > than
> > > > > > 20. For dmatest, the max count for single transfer is
> > > > > > NUM_BD *
> > > > >
> > > > > SDMA_BD_MAX_CNT
> > > > > > = 20 * 65535 = ~1.28MB.
> > > > >
> > > > > Both the commit message and the comment need a lot more care
> > > > > to
> > > > > actually tell what this commit is trying to achieve.
> > > > > Currently I
> > > > > don't follow at all. What does "consisten" mean? Do you mean
> > > > > BDs
> > > > > should be contiguous in memory?
> > > >
> > > > Yes, BDs should be contiguous one by one in memory.
> > >
> > > Okay, but this isn't what the code change does. By increasing the
> > > size
> > > parameter of the dma pool you just allocate 20 times as much
> > > memory as
> > > needed for each BD. So actually the BDs end up being very non-
> > > contiguous in memory as there are now holes of 19 BD sizes
> > > between the
> >
> > start of each BD.
> > Please notice only allocate bds memory from dma pool one time even
> > in multi
> > bds.
> > That's different with the common use case that allocate memory from
> > dma
> > pool everytime for every bd. Why do this is to make sure all bd
> > memory is
> > contiguous for single transfer whatever single bd or multi-bds,
> > since two call
> > dma_pool_alloc() can't promise the address is contiguous especially
> > for multi
> > thread case such as dmatest 'threads_per_chan = 5'. You can change
> > to '
> > norandom=true ' and ' test_buf_size = 163840' in dmatest.c to look
> > what issue
> > coming without this patch.
> > >
> > > So something isn't right with this change.
> >
> > I think this patch is the easy way to resolve the bd contiguous
> > issue, but the
> > cost is to allocate more dma pool memory which may not used.
> > >
> > > Regards,
> > > Lucas
> > >
> > > > >
> > > > > What do you gain by over-allocating each BD by a factor of
> > > > > 20?
> > > >
> > > > I guess dma_pool_alloc will return error in such case, and then
> > > > cause dma setup transfer failure.
> > > > >
> > > > > Regards,
> > > > > Lucas
> > > > >
> > > > > > Signed-off-by: Robin Gong <yibin.gong@nxp.com>
> > > > > > ---
> > > > > > drivers/dma/imx-sdma.c | 17 ++++++++++++++++-
> > > > > > 1 file changed, 16 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-
> > > > > > sdma.c
> > > > > > index
> > > > > > b4ec2d2..5973489 100644
> > > > > > --- a/drivers/dma/imx-sdma.c
> > > > > > +++ b/drivers/dma/imx-sdma.c
> > > > > > @@ -298,6 +298,15 @@ struct sdma_context_data {
> > > > > > > u32 scratch7;
> > > > > >
> > > > > > } __attribute__ ((packed));
> > > > > >
> > > > > > +/*
> > > > > > + * All bds in one transfer should be consitent on SDMA. To
> > > > > > easily
> > > > > > +follow it,just
> > > > > > + * set the dma pool size as the enough bds. For example,
> > > > > > in
> > > > > > dmatest
> > > > > > +case, the
> > > > > > + * max 20 bds means the max for single transfer is NUM_BD
> > > > > > *
> > > > > > +SDMA_BD_MAX_CNT = 20
> > > > > > + * * 65535 = ~1.28MB. 20 bds supposed to be enough
> > > > > > basically.If
> > > > > > it's
> > > > > > +still not
> > > > > > + * enough in some specific cases, enlarge it here.Warning
> > > > > > message
> > > > > > +would also
> > > > > > + * appear if the bd numbers is over than 20.
> > > > > > + */
> > > > > > +#define NUM_BD 20
> > > > > >
> > > > > > struct sdma_engine;
> > > > > >
> > > > > > @@ -1273,7 +1282,7 @@ static int
> > > > > > sdma_alloc_chan_resources(struct dma_chan *chan)
> > > > > > > goto disable_clk_ahb;
> > > > > > > sdmac->bd_pool = dma_pool_create("bd_pool",
> > > > > > > chan-
> > > > > > > > device->dev,
> > > > > > >
> > > > > > > - sizeof(struct
> > > > > > > sdma_buffer_descriptor),
> > > > > > > + NUM_BD * sizeof(struct
> > > > > > > sdma_buffer_descriptor),
> > > > > > > 32, 0);
> > > > > > > return 0;
> > > > > >
> > > > > > @@ -1314,6 +1323,12 @@ static struct sdma_desc
> > > > > > *sdma_transfer_init(struct sdma_channel *sdmac,
> > > > > > {
> > > > > > > struct sdma_desc *desc;
> > > > > > > + if (bds > NUM_BD) {
> > > > > > > + dev_err(sdmac->sdma->dev, "%d bds exceed
> > > > > > > the
> > > > > > > max %d\n",
> > > > > > > + bds, NUM_BD);
> > > > > > > + goto err_out;
> > > > > > > + }
> > > > > >
> > > > > > +
> > > > > > > desc = kzalloc((sizeof(*desc)), GFP_NOWAIT);
> > > > > > > if (!desc)
> > > > > > > goto err_out;
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [v5,7/7] dmaengine: imx-sdma: alloclate bd memory from dma pool
From: Lucas Stach @ 2018-08-06 12:44 UTC (permalink / raw)
To: Robin Gong, vkoul, s.hauer, dan.j.williams, gregkh, jslaby
Cc: linux-serial, dmaengine, linux-kernel, linux-arm-kernel,
linux-imx
Hi Vinod, hi Robin,
this patch is already in your slave-dma tree, but upon closer
inspection this is totally doing the wrong thing and should be dropped,
see inline comments. The patchset in your tree will regress without
this patch, though. So I think we need to delay getting this set
upstream until this issue is properly fixed.
Am Mittwoch, den 20.06.2018, 00:57 +0800 schrieb Robin Gong:
> dma_terminate_all maybe called in interrupt context which means
> WARN_ON() will be triggered as below when bd memory freed. Allocat
> bd memory from dma pool instead.
While the dma_pool usage works around this WARN_ON, it doesn't fix the
underlying issue. Freeing of the descriptor memory should only happen
after the SDMA is not accessing it anymore. The comment in
sdma_disable_channel_with_delay() suggests that we need to wait some
time for this to be safe.
So rather than using a dma_pool for this memory, setting up a delayed
worker to clean up the channel would work much better and even allow to
drop that nasty mdelay() from the terminate callback. Then
device_synchronize function needs to be implemented properly, but
that's much better as than the blocking wait in atomic context.
> [ 29.161079] WARNING: CPU: 1 PID: 533 at ./include/linux/dma-mapping.h:541 sdma_free_bd+0xa4/0xb4
> [ 29.169883] Modules linked in:
> [ 29.172990] CPU: 1 PID: 533 Comm: mpegaudioparse0 Not tainted 4.18.0-rc1-next-20180618-00009-gf79f22c #20
> [ 29.182597] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> [ 29.189163] Backtrace:
> [ 29.191685] [<c010d1e0>] (dump_backtrace) from [<c010d4a0>] (show_stack+0x18/0x1c)
> [ 29.199306] r7:00000000 r6:600f0093 r5:00000000 r4:c107db7c
> [ 29.205029] [<c010d488>] (show_stack) from [<c0a5bba0>] (dump_stack+0xb4/0xe8)
> [ 29.212312] [<c0a5baec>] (dump_stack) from [<c012703c>] (__warn+0x104/0x130)
> [ 29.219411] r9:ec3e817c r8:0000021d r7:00000009 r6:c0d1d440 r5:00000000 r4:00000000
> [ 29.227204] [<c0126f38>] (__warn) from [<c0127180>] (warn_slowpath_null+0x44/0x50)
> [ 29.234821] r8:ed129dc4 r7:c0b01978 r6:c04d4e90 r5:0000021d r4:c0d1d440
> [ 29.241574] [<c012713c>] (warn_slowpath_null) from [<c04d4e90>] (sdma_free_bd+0xa4/0xb4)
> [ 29.249706] r6:4c001000 r5:f082e000 r4:00000024
> [ 29.254376] [<c04d4dec>] (sdma_free_bd) from [<c04d4eb4>] (sdma_desc_free+0x14/0x20)
> [ 29.262163] r7:ec3e8110 r6:00000100 r5:00000200 r4:ecf89a00
> [ 29.267873] [<c04d4ea0>] (sdma_desc_free) from [<c04d229c>] (vchan_dma_desc_free_list+0xa4/0xac)
> [ 29.276697] r5:00000200 r4:ed129d9c
> [ 29.280326] [<c04d21f8>] (vchan_dma_desc_free_list) from [<c04d482c>] (sdma_disable_channel_with_delay+0x14c/0x188)
> [ 29.290808] r9:ecae560c r8:ec3e815c r7:00000000 r6:c1008908 r5:ed129dc4 r4:ec3e8110
> [ 29.298605] [<c04d46e0>] (sdma_disable_channel_with_delay) from [<c07c5c84>] (snd_dmaengine_pcm_trigger+0x90/0x1b0)
> [ 29.309087] r8:ecae5000 r7:ec940800 r6:ed31bd80 r5:ecadb200 r4:ec26a700
> [ 29.315855] [<c07c5bf4>] (snd_dmaengine_pcm_trigger) from [<c07dd800>] (soc_pcm_trigger+0xb4/0x130)
> [ 29.324953] r8:ecae5000 r7:ec940800 r6:00000000 r5:ecadb200 r4:ec26a700
> [ 29.331716] [<c07dd74c>] (soc_pcm_trigger) from [<c07bc008>] (snd_pcm_do_stop+0x58/0x5c)
> [ 29.339859] r9:ecaed5a8 r8:ed31bdc0 r7:00000000 r6:00000001 r5:ecadb200 r4:c0b9c4d0
> [ 29.347652] [<c07bbfb0>] (snd_pcm_do_stop) from [<c07bbde8>] (snd_pcm_action_single+0x40/0x80)
> [ 29.356315] [<c07bbda8>] (snd_pcm_action_single) from [<c07bbf1c>] (snd_pcm_action+0xf4/0xfc)
> [ 29.364883] r7:00000001 r6:c0b9c4d0 r5:ecadb2d4 r4:ecadb200
> [ 29.370593] [<c07bbe28>] (snd_pcm_action) from [<c07bc8dc>] (snd_pcm_drop+0x58/0x9c)
>
> > Signed-off-by: Robin Gong <yibin.gong@nxp.com>
> ---
> drivers/dma/imx-sdma.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index f8becaf..7dab7e9 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -24,6 +24,7 @@
> #include <linux/spinlock.h>
> #include <linux/device.h>
> #include <linux/dma-mapping.h>
> +#include <linux/dmapool.h>
> #include <linux/firmware.h>
> #include <linux/slab.h>
> #include <linux/platform_device.h>
> @@ -343,6 +344,7 @@ struct sdma_channel {
> > > u32 shp_addr, per_addr;
> > > enum dma_status status;
> > > struct imx_dma_data data;
> > > + struct dma_pool *bd_pool;
> };
>
> > #define IMX_DMA_SG_LOOP BIT(0)
> @@ -1153,11 +1155,10 @@ static int sdma_request_channel0(struct sdma_engine *sdma)
>
> static int sdma_alloc_bd(struct sdma_desc *desc)
> {
> > - u32 bd_size = desc->num_bd * sizeof(struct sdma_buffer_descriptor);
> > int ret = 0;
>
> > - desc->bd = dma_zalloc_coherent(NULL, bd_size, &desc->bd_phys,
> > - GFP_ATOMIC);
> > + desc->bd = dma_pool_alloc(desc->sdmac->bd_pool, GFP_ATOMIC,
> + &desc->bd_phys);
This is replacing a allocation of num_bd * sizeof(descriptor) by a
single call to dma_pool_alloc, which provides enough memory for a
single descriptor, so this will lead to invalid usage of descriptor
memory for all transfers with more than a single descriptor.
Regards,
Lucas
> if (!desc->bd) {
> > ret = -ENOMEM;
> > goto out;
> @@ -1168,9 +1169,7 @@ static int sdma_alloc_bd(struct sdma_desc *desc)
>
> static void sdma_free_bd(struct sdma_desc *desc)
> {
> > - u32 bd_size = desc->num_bd * sizeof(struct sdma_buffer_descriptor);
> -
> > - dma_free_coherent(NULL, bd_size, desc->bd, desc->bd_phys);
> > + dma_pool_free(desc->sdmac->bd_pool, desc->bd, desc->bd_phys);
> }
>
> static void sdma_desc_free(struct virt_dma_desc *vd)
> @@ -1218,6 +1217,10 @@ static int sdma_alloc_chan_resources(struct dma_chan *chan)
> > if (ret)
> > goto disable_clk_ahb;
>
> > + sdmac->bd_pool = dma_pool_create("bd_pool", chan->device->dev,
> > + sizeof(struct sdma_buffer_descriptor),
> > + 32, 0);
> +
> > return 0;
>
> disable_clk_ahb:
> @@ -1246,6 +1249,9 @@ static void sdma_free_chan_resources(struct dma_chan *chan)
>
> > clk_disable(sdma->clk_ipg);
> > clk_disable(sdma->clk_ahb);
> +
> > + dma_pool_destroy(sdmac->bd_pool);
> > + sdmac->bd_pool = NULL;
> }
>
> static struct sdma_desc *sdma_transfer_init(struct sdma_channel *sdmac,
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [v8,3/3] dmaengine: fsl-edma: add ColdFire mcf5441x edma support
From: Angelo Dureghello @ 2018-08-06 20:23 UTC (permalink / raw)
To: Stefan Agner
Cc: Krzysztof Kozlowski, vinod.koul, dmaengine, linux-arm-kernel,
linux-m68k
Hi Stefan,
On Mon, Aug 06, 2018 at 02:21:52PM +0200, Stefan Agner wrote:
> On 06.08.2018 10:09, Krzysztof Kozlowski wrote:
> > On 3 August 2018 at 21:32, Angelo Dureghello <angelo@sysam.it> wrote:
> >> This patch adds support for ColdFire mcf5441x-family edma
> >> module.
> >>
> >> The ColdFire edma module is slightly different from fsl-edma,
> >> so a new driver is added. But most of the code is common
> >> between fsl-edma and mcf-edma so it has been collected into a
> >> separate common module fsl-edma-common (patch 1/3).
> >>
> >> Signed-off-by: Angelo Dureghello <angelo@sysam.it>
> [...]
> >> diff --git a/include/linux/platform_data/dma-mcf-edma.h b/include/linux/platform_data/dma-mcf-edma.h
> >> new file mode 100644
> >> index 000000000000..9a1819acb28f
> >> --- /dev/null
> >> +++ b/include/linux/platform_data/dma-mcf-edma.h
> >> @@ -0,0 +1,38 @@
> >> +/* SPDX-License-Identifier: GPL-2.0 */
> >> +/*
> >> + * Freescale eDMA platform data, ColdFire SoC's family.
> >> + *
> >> + * Copyright (c) 2017 Angelo Dureghello <angelo@sysam.it>
> >> + *
> >> + * This program is free software; you can redistribute it and/or modify
> >> + * it under the terms of the GNU General Public License version 2 as
> >> + * published by the Free Software Foundation.
> >> + *
> >> + * This program is distributed in the hope that it will be useful,
> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >> + * GNU General Public License for more details.
> >> + */
> >> +
> >> +#ifndef __MACH_MCF_EDMA_H__
> >> +#define __MACH_MCF_EDMA_H__
> >
> > The guard should rather match current location, e.g.
> > __LINUX_PLATFORM_DATA_MCF_EDMA_H__
> >
> > Tested-by: Krzysztof Kozlowski <krzk@kernel.org>
> >
>
> So this revision fixed the issue we had seen with the previous version?
> Do we know what it was exactly?
>
Well, no. I restarted completely the patch from scratch moving/changing
the code carefully. I avoided changes to Freescale code as much
as possible.
> --
> Stefan
> --
> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Regards,
Angelo
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [v8,2/3] dmaengine: fsl-edma: add edma version and configurable registers
From: Angelo Dureghello @ 2018-08-06 20:31 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: vinod.koul, dmaengine, linux-arm-kernel, linux-m68k, Stefan Agner
Hi Krzysztof,
many thanks for testing.
On Mon, Aug 06, 2018 at 09:59:35AM +0200, Krzysztof Kozlowski wrote:
> On 3 August 2018 at 21:32, Angelo Dureghello <angelo@sysam.it> wrote:
> > This patch adds configurable registers (using __iomem addresses)
> > to allow the use of fsl-edma-common code with slightly different
> > edma module versions, as Vybrid (v1) and ColdFire (v2) are.
> >
> > Removal of old membase-referenced registers, amd some fixes on
> > macroes are included.
> >
> > Signed-off-by: Angelo Dureghello <angelo@sysam.it>
> > ---
> > Changes from v7:
> > - patch rewritten from scratch, this patch (2/3) has just been added.
> > ---
> > drivers/dma/fsl-edma-common.c | 138 ++++++++++++++++++++++++++--------
> > drivers/dma/fsl-edma-common.h | 115 ++++++++++++++--------------
> > drivers/dma/fsl-edma.c | 32 ++++----
> > 3 files changed, 182 insertions(+), 103 deletions(-)
> >
> > diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
> > index 0ae7094f477a..948a3ee51bbb 100644
> > --- a/drivers/dma/fsl-edma-common.c
> > +++ b/drivers/dma/fsl-edma-common.c
> > @@ -9,6 +9,38 @@
> >
> > #include "fsl-edma-common.h"
> >
> > +#define EDMA_CR 0x00
> > +#define EDMA_ES 0x04
> > +#define EDMA_ERQ 0x0C
> > +#define EDMA_EEI 0x14
> > +#define EDMA_SERQ 0x1B
> > +#define EDMA_CERQ 0x1A
> > +#define EDMA_SEEI 0x19
> > +#define EDMA_CEEI 0x18
> > +#define EDMA_CINT 0x1F
> > +#define EDMA_CERR 0x1E
> > +#define EDMA_SSRT 0x1D
> > +#define EDMA_CDNE 0x1C
> > +#define EDMA_INTR 0x24
> > +#define EDMA_ERR 0x2C
> > +
> > +#define EDMA64_ERQH 0x08
> > +#define EDMA64_EEIH 0x10
> > +#define EDMA64_SERQ 0x18
> > +#define EDMA64_CERQ 0x19
> > +#define EDMA64_SEEI 0x1a
> > +#define EDMA64_CEEI 0x1b
> > +#define EDMA64_CINT 0x1c
> > +#define EDMA64_CERR 0x1d
> > +#define EDMA64_SSRT 0x1e
> > +#define EDMA64_CDNE 0x1f
> > +#define EDMA64_INTH 0x20
> > +#define EDMA64_INTL 0x24
> > +#define EDMA64_ERRH 0x28
> > +#define EDMA64_ERRL 0x2c
> > +
> > +#define EDMA_TCD 0x1000
> > +
> > /*
> > * R/W functions for big- or little-endian registers:
> > * The eDMA controller's endian is independent of the CPU core's endian.
> > @@ -67,20 +99,20 @@ EXPORT_SYMBOL_GPL(to_fsl_edma_desc);
> >
> > static void fsl_edma_enable_request(struct fsl_edma_chan *fsl_chan)
> > {
> > - void __iomem *addr = fsl_chan->edma->membase;
> > + struct edma_regs *regs = &fsl_chan->edma->regs;
> > u32 ch = fsl_chan->vchan.chan.chan_id;
> >
> > - edma_writeb(fsl_chan->edma, EDMA_SEEI_SEEI(ch), addr + EDMA_SEEI);
> > - edma_writeb(fsl_chan->edma, ch, addr + EDMA_SERQ);
> > + edma_writeb(fsl_chan->edma, EDMA_SEEI_SEEI(ch), regs->seei);
> > + edma_writeb(fsl_chan->edma, ch, regs->serq);
> > }
> >
> > void fsl_edma_disable_request(struct fsl_edma_chan *fsl_chan)
> > {
> > - void __iomem *addr = fsl_chan->edma->membase;
> > + struct edma_regs *regs = &fsl_chan->edma->regs;
> > u32 ch = fsl_chan->vchan.chan.chan_id;
> >
> > - edma_writeb(fsl_chan->edma, ch, addr + EDMA_CERQ);
> > - edma_writeb(fsl_chan->edma, EDMA_CEEI_CEEI(ch), addr + EDMA_CEEI);
> > + edma_writeb(fsl_chan->edma, ch, regs->cerq);
> > + edma_writeb(fsl_chan->edma, EDMA_CEEI_CEEI(ch), regs->ceei);
> > }
> > EXPORT_SYMBOL_GPL(fsl_edma_disable_request);
> >
> > @@ -208,7 +240,7 @@ static size_t fsl_edma_desc_residue(struct fsl_edma_chan *fsl_chan,
> > struct virt_dma_desc *vdesc, bool in_progress)
> > {
> > struct fsl_edma_desc *edesc = fsl_chan->edesc;
> > - void __iomem *addr = fsl_chan->edma->membase;
> > + struct edma_regs *regs = &fsl_chan->edma->regs;
> > u32 ch = fsl_chan->vchan.chan.chan_id;
> > enum dma_transfer_direction dir = fsl_chan->fsc.dir;
> > dma_addr_t cur_addr, dma_addr;
> > @@ -224,11 +256,9 @@ static size_t fsl_edma_desc_residue(struct fsl_edma_chan *fsl_chan,
> > return len;
> >
> > if (dir == DMA_MEM_TO_DEV)
> > - cur_addr = edma_readl(
> > - fsl_chan->edma, addr + EDMA_TCD_SADDR(ch));
> > + cur_addr = edma_readl(fsl_chan->edma, ®s->tcd[ch].saddr);
> > else
> > - cur_addr = edma_readl(
> > - fsl_chan->edma, addr + EDMA_TCD_DADDR(ch));
> > + cur_addr = edma_readl(fsl_chan->edma, ®s->tcd[ch].daddr);
> >
> > /* figure out the finished and calculate the residue */
> > for (i = 0; i < fsl_chan->edesc->n_tcds; i++) {
> > @@ -285,7 +315,7 @@ static void fsl_edma_set_tcd_regs(struct fsl_edma_chan *fsl_chan,
> > struct fsl_edma_hw_tcd *tcd)
> > {
> > struct fsl_edma_engine *edma = fsl_chan->edma;
> > - void __iomem *addr = fsl_chan->edma->membase;
> > + struct edma_regs *regs = &fsl_chan->edma->regs;
> > u32 ch = fsl_chan->vchan.chan.chan_id;
> >
> > /*
> > @@ -293,24 +323,24 @@ static void fsl_edma_set_tcd_regs(struct fsl_edma_chan *fsl_chan,
> > * endian format. However, we need to load the TCD registers in
> > * big- or little-endian obeying the eDMA engine model endian.
> > */
> > - edma_writew(edma, 0, addr + EDMA_TCD_CSR(ch));
> > - edma_writel(edma, le32_to_cpu(tcd->saddr), addr + EDMA_TCD_SADDR(ch));
> > - edma_writel(edma, le32_to_cpu(tcd->daddr), addr + EDMA_TCD_DADDR(ch));
> > + edma_writew(edma, 0, ®s->tcd[ch].csr);
> > + edma_writel(edma, le32_to_cpu(tcd->saddr), ®s->tcd[ch].saddr);
> > + edma_writel(edma, le32_to_cpu(tcd->daddr), ®s->tcd[ch].daddr);
> >
> > - edma_writew(edma, le16_to_cpu(tcd->attr), addr + EDMA_TCD_ATTR(ch));
> > - edma_writew(edma, le16_to_cpu(tcd->soff), addr + EDMA_TCD_SOFF(ch));
> > + edma_writew(edma, le16_to_cpu(tcd->attr), ®s->tcd[ch].attr);
> > + edma_writew(edma, le16_to_cpu(tcd->soff), ®s->tcd[ch].soff);
> >
> > - edma_writel(edma, le32_to_cpu(tcd->nbytes), addr + EDMA_TCD_NBYTES(ch));
> > - edma_writel(edma, le32_to_cpu(tcd->slast), addr + EDMA_TCD_SLAST(ch));
> > + edma_writel(edma, le32_to_cpu(tcd->nbytes), ®s->tcd[ch].nbytes);
> > + edma_writel(edma, le32_to_cpu(tcd->slast), ®s->tcd[ch].slast);
> >
> > - edma_writew(edma, le16_to_cpu(tcd->citer), addr + EDMA_TCD_CITER(ch));
> > - edma_writew(edma, le16_to_cpu(tcd->biter), addr + EDMA_TCD_BITER(ch));
> > - edma_writew(edma, le16_to_cpu(tcd->doff), addr + EDMA_TCD_DOFF(ch));
> > + edma_writew(edma, le16_to_cpu(tcd->citer), ®s->tcd[ch].citer);
> > + edma_writew(edma, le16_to_cpu(tcd->biter), ®s->tcd[ch].biter);
> > + edma_writew(edma, le16_to_cpu(tcd->doff), ®s->tcd[ch].doff);
> >
> > - edma_writel(edma,
> > - le32_to_cpu(tcd->dlast_sga), addr + EDMA_TCD_DLAST_SGA(ch));
> > + edma_writel(edma, le32_to_cpu(tcd->dlast_sga),
> > + ®s->tcd[ch].dlast_sga);
> >
> > - edma_writew(edma, le16_to_cpu(tcd->csr), addr + EDMA_TCD_CSR(ch));
> > + edma_writew(edma, le16_to_cpu(tcd->csr), ®s->tcd[ch].csr);
> > }
> >
> > static inline
> > @@ -332,15 +362,15 @@ void fsl_edma_fill_tcd(struct fsl_edma_hw_tcd *tcd, u32 src, u32 dst,
> >
> > tcd->attr = cpu_to_le16(attr);
> >
> > - tcd->soff = cpu_to_le16(EDMA_TCD_SOFF_SOFF(soff));
> > + tcd->soff = cpu_to_le16(soff);
> >
> > - tcd->nbytes = cpu_to_le32(EDMA_TCD_NBYTES_NBYTES(nbytes));
> > - tcd->slast = cpu_to_le32(EDMA_TCD_SLAST_SLAST(slast));
> > + tcd->nbytes = cpu_to_le32(nbytes);
> > + tcd->slast = cpu_to_le32(slast);
> >
> > tcd->citer = cpu_to_le16(EDMA_TCD_CITER_CITER(citer));
> > - tcd->doff = cpu_to_le16(EDMA_TCD_DOFF_DOFF(doff));
> > + tcd->doff = cpu_to_le16(doff);
> >
> > - tcd->dlast_sga = cpu_to_le32(EDMA_TCD_DLAST_SGA_DLAST_SGA(dlast_sga));
> > + tcd->dlast_sga = cpu_to_le32(dlast_sga);
> >
> > tcd->biter = cpu_to_le16(EDMA_TCD_BITER_BITER(biter));
> > if (major_int)
> > @@ -573,4 +603,52 @@ void fsl_edma_cleanup_vchan(struct dma_device *dmadev)
> > }
> > EXPORT_SYMBOL_GPL(fsl_edma_cleanup_vchan);
> >
> > +/*
> > + * On the 32 channels Vybrid/mpc577x edma version (here called "v1"),
> > + * register offsets are different compared to ColdFire mcf5441x 64 channels
> > + * edma (here called "v2").
> > + *
> > + * This function sets up register offsets as per proper declared version
> > + * so must be called in xxx_edma_probe() just after setting the
> > + * edma "version" and "membase" appropriately.
> > + */
> > +void fsl_edma_setup_regs(struct fsl_edma_engine *edma)
> > +{
> > + edma->regs.cr = edma->membase + EDMA_CR;
> > + edma->regs.es = edma->membase + EDMA_ES;
> > + edma->regs.erql = edma->membase + EDMA_ERQ;
> > + edma->regs.eeil = edma->membase + EDMA_EEI;
> > +
> > + edma->regs.serq = edma->membase + ((edma->version == v1) ?
> > + EDMA_SERQ : EDMA64_SERQ);
> > + edma->regs.cerq = edma->membase + ((edma->version == v1) ?
> > + EDMA_CERQ : EDMA64_CERQ);
> > + edma->regs.seei = edma->membase + ((edma->version == v1) ?
> > + EDMA_SEEI : EDMA64_SEEI);
> > + edma->regs.ceei = edma->membase + ((edma->version == v1) ?
> > + EDMA_CEEI : EDMA64_CEEI);
> > + edma->regs.cint = edma->membase + ((edma->version == v1) ?
> > + EDMA_CINT : EDMA64_CINT);
> > + edma->regs.cerr = edma->membase + ((edma->version == v1) ?
> > + EDMA_CERR : EDMA64_CERR);
> > + edma->regs.ssrt = edma->membase + ((edma->version == v1) ?
> > + EDMA_SSRT : EDMA64_SSRT);
> > + edma->regs.cdne = edma->membase + ((edma->version == v1) ?
> > + EDMA_CDNE : EDMA64_CDNE);
> > + edma->regs.intl = edma->membase + ((edma->version == v1) ?
> > + EDMA_INTR : EDMA64_INTL);
> > + edma->regs.errl = edma->membase + ((edma->version == v1) ?
> > + EDMA_ERR : EDMA64_ERRL);
> > +
> > + if (edma->version == v2) {
> > + edma->regs.erqh = edma->membase + EDMA64_ERQH;
> > + edma->regs.eeih = edma->membase + EDMA64_EEIH;
> > + edma->regs.errh = edma->membase + EDMA64_ERRH;
> > + edma->regs.inth = edma->membase + EDMA64_INTH;
> > + }
> > +
> > + edma->regs.tcd = edma->membase + EDMA_TCD;
> > +}
> > +EXPORT_SYMBOL_GPL(fsl_edma_setup_regs);
> > +
> > MODULE_LICENSE("GPL v2");
> > diff --git a/drivers/dma/fsl-edma-common.h b/drivers/dma/fsl-edma-common.h
> > index f3ea68e15b23..7a9231e7639f 100644
> > --- a/drivers/dma/fsl-edma-common.h
> > +++ b/drivers/dma/fsl-edma-common.h
> > @@ -8,35 +8,6 @@
> >
> > #include "virt-dma.h"
> >
> > -#define EDMA_CR 0x00
> > -#define EDMA_ES 0x04
> > -#define EDMA_ERQ 0x0C
> > -#define EDMA_EEI 0x14
> > -#define EDMA_SERQ 0x1B
> > -#define EDMA_CERQ 0x1A
> > -#define EDMA_SEEI 0x19
> > -#define EDMA_CEEI 0x18
> > -#define EDMA_CINT 0x1F
> > -#define EDMA_CERR 0x1E
> > -#define EDMA_SSRT 0x1D
> > -#define EDMA_CDNE 0x1C
> > -#define EDMA_INTR 0x24
> > -#define EDMA_ERR 0x2C
> > -
> > -#define EDMA_TCD_SADDR(x) (0x1000 + 32 * (x))
> > -#define EDMA_TCD_SOFF(x) (0x1004 + 32 * (x))
> > -#define EDMA_TCD_ATTR(x) (0x1006 + 32 * (x))
> > -#define EDMA_TCD_NBYTES(x) (0x1008 + 32 * (x))
> > -#define EDMA_TCD_SLAST(x) (0x100C + 32 * (x))
> > -#define EDMA_TCD_DADDR(x) (0x1010 + 32 * (x))
> > -#define EDMA_TCD_DOFF(x) (0x1014 + 32 * (x))
> > -#define EDMA_TCD_CITER_ELINK(x) (0x1016 + 32 * (x))
> > -#define EDMA_TCD_CITER(x) (0x1016 + 32 * (x))
> > -#define EDMA_TCD_DLAST_SGA(x) (0x1018 + 32 * (x))
> > -#define EDMA_TCD_CSR(x) (0x101C + 32 * (x))
> > -#define EDMA_TCD_BITER_ELINK(x) (0x101E + 32 * (x))
> > -#define EDMA_TCD_BITER(x) (0x101E + 32 * (x))
> > -
> > #define EDMA_CR_EDBG BIT(1)
> > #define EDMA_CR_ERCA BIT(2)
> > #define EDMA_CR_ERGA BIT(3)
> > @@ -47,34 +18,29 @@
> > #define EDMA_CR_ECX BIT(16)
> > #define EDMA_CR_CX BIT(17)
> >
> > -#define EDMA_SEEI_SEEI(x) ((x) & 0x1F)
> > -#define EDMA_CEEI_CEEI(x) ((x) & 0x1F)
> > -#define EDMA_CINT_CINT(x) ((x) & 0x1F)
> > -#define EDMA_CERR_CERR(x) ((x) & 0x1F)
> > -
> > -#define EDMA_TCD_ATTR_DSIZE(x) (((x) & 0x0007))
> > -#define EDMA_TCD_ATTR_DMOD(x) (((x) & 0x001F) << 3)
> > -#define EDMA_TCD_ATTR_SSIZE(x) (((x) & 0x0007) << 8)
> > -#define EDMA_TCD_ATTR_SMOD(x) (((x) & 0x001F) << 11)
> > -#define EDMA_TCD_ATTR_SSIZE_8BIT (0x0000)
> > -#define EDMA_TCD_ATTR_SSIZE_16BIT (0x0100)
> > -#define EDMA_TCD_ATTR_SSIZE_32BIT (0x0200)
> > -#define EDMA_TCD_ATTR_SSIZE_64BIT (0x0300)
> > -#define EDMA_TCD_ATTR_SSIZE_32BYTE (0x0500)
> > -#define EDMA_TCD_ATTR_DSIZE_8BIT (0x0000)
> > -#define EDMA_TCD_ATTR_DSIZE_16BIT (0x0001)
> > -#define EDMA_TCD_ATTR_DSIZE_32BIT (0x0002)
> > -#define EDMA_TCD_ATTR_DSIZE_64BIT (0x0003)
> > -#define EDMA_TCD_ATTR_DSIZE_32BYTE (0x0005)
> > -
> > -#define EDMA_TCD_SOFF_SOFF(x) (x)
> > -#define EDMA_TCD_NBYTES_NBYTES(x) (x)
> > -#define EDMA_TCD_SLAST_SLAST(x) (x)
> > -#define EDMA_TCD_DADDR_DADDR(x) (x)
> > -#define EDMA_TCD_CITER_CITER(x) ((x) & 0x7FFF)
> > -#define EDMA_TCD_DOFF_DOFF(x) (x)
> > -#define EDMA_TCD_DLAST_SGA_DLAST_SGA(x) (x)
> > -#define EDMA_TCD_BITER_BITER(x) ((x) & 0x7FFF)
> > +#define EDMA_SEEI_SEEI(x) ((x) & GENMASK(4, 0))
> > +#define EDMA_CEEI_CEEI(x) ((x) & GENMASK(4, 0))
> > +#define EDMA_CINT_CINT(x) ((x) & GENMASK(4, 0))
> > +#define EDMA_CERR_CERR(x) ((x) & GENMASK(4, 0))
> > +
> > +#define EDMA_TCD_ATTR_DSIZE(x) (((x) & GENMASK(2, 0)))
> > +#define EDMA_TCD_ATTR_DMOD(x) (((x) & GENMASK(4, 0)) << 3)
> > +#define EDMA_TCD_ATTR_SSIZE(x) (((x) & GENMASK(2, 0)) << 8)
> > +#define EDMA_TCD_ATTR_SMOD(x) (((x) & GENMASK(4, 0)) << 11)
>
> This looks like a change for its own commit.
>
Yes, was previously required the usage of BIT() and GENMASK().
> > +
> > +#define EDMA_TCD_ATTR_DSIZE_8BIT 0
> > +#define EDMA_TCD_ATTR_DSIZE_16BIT BIT(0)
> > +#define EDMA_TCD_ATTR_DSIZE_32BIT BIT(1)
> > +#define EDMA_TCD_ATTR_DSIZE_64BIT (BIT(0) | BIT(1))
> > +#define EDMA_TCD_ATTR_DSIZE_32BYTE (BIT(3) | BIT(0))
> > +#define EDMA_TCD_ATTR_SSIZE_8BIT 0
> > +#define EDMA_TCD_ATTR_SSIZE_16BIT (EDMA_TCD_ATTR_DSIZE_16BIT << 8)
> > +#define EDMA_TCD_ATTR_SSIZE_32BIT (EDMA_TCD_ATTR_DSIZE_32BIT << 8)
> > +#define EDMA_TCD_ATTR_SSIZE_64BIT (EDMA_TCD_ATTR_DSIZE_64BIT << 8)
> > +#define EDMA_TCD_ATTR_SSIZE_32BYTE (EDMA_TCD_ATTR_DSIZE_32BYTE << 8)
> > +
> > +#define EDMA_TCD_CITER_CITER(x) ((x) & GENMASK(14, 0))
> > +#define EDMA_TCD_BITER_BITER(x) ((x) & GENMASK(14, 0))
> >
> > #define EDMA_TCD_CSR_START BIT(0)
> > #define EDMA_TCD_CSR_INT_MAJOR BIT(1)
> > @@ -87,7 +53,7 @@
> >
> > #define EDMAMUX_CHCFG_DIS 0x0
> > #define EDMAMUX_CHCFG_ENBL 0x80
> > -#define EDMAMUX_CHCFG_SOURCE(n) ((n) & 0x3F)
> > +#define EDMAMUX_CHCFG_SOURCE(n) ((n) & GENMASK(6, 0))
> >
> > #define DMAMUX_NR 2
> >
> > @@ -114,6 +80,31 @@ struct fsl_edma_hw_tcd {
> > __le16 biter;
> > };
> >
> > +/*
> > + * This are iomem pointers, for both v32 and v64.
>
> s/This/These/
>
Ack. Will eventually fix all in a v9. Collecting feedbacks.
> Tested-by: Krzysztof Kozlowski <krzk@kernel.org>
>
> Best regards,
> Krzysztof
> --
> To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Best Regards,
Angelo
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox