All of lore.kernel.org
 help / color / mirror / Atom feed
* [v5,2/2] dmaengine: fsl-edma: add ColdFire mcf5441x edma support
@ 2018-07-01 13:11 Vinod Koul
  0 siblings, 0 replies; 11+ messages in thread
From: Vinod Koul @ 2018-07-01 13:11 UTC (permalink / raw)
  To: Angelo Dureghello; +Cc: dmaengine, linux-m68k

On 30-06-18, 15:42, Angelo Dureghello wrote:
> Hi Vinod,
> 
> fixed mostly all, but sorry, i have still two questions before
> proceeding,
> 
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +// Copyright (c) 2013-2014 Freescale Semiconductor, Inc
> > > +// Copyright (c) 2017 Sysam, Angelo Dureghello  <angelo@sysam.it>
> > > +/*
> > > + * drivers/dma/mcf-edma.c
> > > + *
> > > + * Driver for the Freescale ColdFire 64-ch eDMA implementation,
> > > + * derived from drivers/dma/fsl-edma.c.
> > > + *
> > > + * This program is free software; you can redistribute  it and/or modify it
> > > + * under  the terms of  the GNU General  Public License as published by the
> > > + * Free Software Foundation;  either version 2 of the  License, or (at your
> > > + * option) any later version.
> > > + */
> > 
> > again, no need for text
> > 
> 
> It is not clear to me now how the initial header should be (i guess for 
> all the 3 c files at this point).
> 
> Do you want just something as :
> 
> // SPDX-License-Identifier: GPL-2.0
> // Copyright (c) 2013-2014 Freescale Semiconductor, Inc
> // Copyright (c) 2017 Sysam, Angelo Dureghello  <angelo@sysam.it>

That would be nice. Text is no longer required as SPDX represents that.
Copyright needs to be retained.
> 
> And nothing else ?
> 
> Majority of the files in the dma folder has also generally a line with 
> the file name and path, a brief driver explaination and the reduced GPL
> licence text, and, as imx-sdma.c often copyrights at the end. So what is 
> the current rule ?

Above and if you would like add a line explaining the role of driver.
File name and path are not required and gets stale as people update
stuff in future.

> > > +static int __init mcf_edma_init(void)
> > > +{
> > > +	return platform_driver_register(&mcf_edma_driver);
> > > +}
> > > +subsys_initcall(mcf_edma_init);
> > 
> > why subsys_initcall?
> > 
> 
> I find subsys_initcall in several dma drivers, my understanding is that
> it initializes the driver before other drivers can use it.
> It also sets the driver as built in only.
> This seems ok for my case.

Yes that is the case, but in your case I would like to know why you
would want this. Others doing is not a good enough justification and you
need to find your reason :)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [v5,2/2] dmaengine: fsl-edma: add ColdFire mcf5441x edma support
@ 2018-06-30 21:06 Geert Uytterhoeven
  0 siblings, 0 replies; 11+ messages in thread
From: Geert Uytterhoeven @ 2018-06-30 21:06 UTC (permalink / raw)
  To: Angelo Dureghello; +Cc: vkoul, dmaengine, Linux/m68k

Hi Angelo,

On Sat, Jun 30, 2018 at 3:42 PM Angelo Dureghello <angelo@sysam.it> wrote:
> fixed mostly all, but sorry, i have still two questions before
> proceeding,
>
> On Thu, Jun 28, 2018 at 11:53:41AM +0530, Vinod wrote:
> It is not clear to me now how the initial header should be (i guess for
> all the 3 c files at this point).
>
> Do you want just something as :
>
> // SPDX-License-Identifier: GPL-2.0
> // Copyright (c) 2013-2014 Freescale Semiconductor, Inc
> // Copyright (c) 2017 Sysam, Angelo Dureghello  <angelo@sysam.it>
>
> And nothing else ?

Looks good to me.

> Majority of the files in the dma folder has also generally a line with
> the file name and path, a brief driver explaination and the reduced GPL

File names and path are superfluous, and annoying, considering move/rename
later.

> licence text, and, as imx-sdma.c often copyrights at the end. So what is

Reduced license text is no longer needed with the right SPDX header.

> the current rule ?

Copyright should be retained.

> > > +static int __init mcf_edma_init(void)
> > > +{
> > > +   return platform_driver_register(&mcf_edma_driver);
> > > +}
> > > +subsys_initcall(mcf_edma_init);
> >
> > why subsys_initcall?
> >
>
> I find subsys_initcall in several dma drivers, my understanding is that
> it initializes the driver before other drivers can use it.
> It also sets the driver as built in only.
> This seems ok for my case.

Indeed. If DMAC drivers are initialized before normal device drivers (which
can be DMA slaves), probe deferral can be avoided.

Gr{oetje,eeting}s,

                        Geert

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [v5,2/2] dmaengine: fsl-edma: add ColdFire mcf5441x edma support
@ 2018-06-30 13:42 Angelo Dureghello
  0 siblings, 0 replies; 11+ messages in thread
From: Angelo Dureghello @ 2018-06-30 13:42 UTC (permalink / raw)
  To: Vinod; +Cc: dmaengine, linux-m68k

Hi Vinod,

fixed mostly all, but sorry, i have still two questions before
proceeding,

On Thu, Jun 28, 2018 at 11:53:41AM +0530, Vinod wrote:
> On 22-06-18, 11:44, Angelo Dureghello wrote:
> >  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
> 
> that makes kernel have two copies of common.o one in thsi driver and one
> in previous one why not do:
> 
> CONFIG_FSL_COMMON += fsl-edma-common.o
> CONFIG_FSL_EDMA += fsl-edma.o
> CONFIG_MCF_EDMA += mcf-edma.o
> 
> and you select CONFIG_FSL_COMMON in both FSL and MCF Kconfig?
> 
> > +// SPDX-License-Identifier: GPL-2.0
> > +// Copyright (c) 2013-2014 Freescale Semiconductor, Inc
> > +// Copyright (c) 2017 Sysam, Angelo Dureghello  <angelo@sysam.it>
> > +/*
> > + * drivers/dma/mcf-edma.c
> > + *
> > + * Driver for the Freescale ColdFire 64-ch eDMA implementation,
> > + * derived from drivers/dma/fsl-edma.c.
> > + *
> > + * This program is free software; you can redistribute  it and/or modify it
> > + * under  the terms of  the GNU General  Public License as published by the
> > + * Free Software Foundation;  either version 2 of the  License, or (at your
> > + * option) any later version.
> > + */
> 
> again, no need for text
> 

It is not clear to me now how the initial header should be (i guess for 
all the 3 c files at this point).

Do you want just something as :

// SPDX-License-Identifier: GPL-2.0
// Copyright (c) 2013-2014 Freescale Semiconductor, Inc
// Copyright (c) 2017 Sysam, Angelo Dureghello  <angelo@sysam.it>

And nothing else ?

Majority of the files in the dma folder has also generally a line with 
the file name and path, a brief driver explaination and the reduced GPL
licence text, and, as imx-sdma.c often copyrights at the end. So what is 
the current rule ?


> > +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 & (0x1 << ch)) {
> 
>                 intmap & BIT(ch)
> 
> > +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 & (0x1 << ch)) {
> 
> here as well
> 
> > +static int mcf_edma_remove(struct platform_device *pdev)
> > +{
> > +	struct fsl_edma_engine *mcf_edma = platform_get_drvdata(pdev);
> > +
> > +	fsl_edma_cleanup_vchan(&mcf_edma->dma_dev);
> > +	dma_async_device_unregister(&mcf_edma->dma_dev);
> 
> at this point your irqs are still registered and running. You vchan
> tasklet maybe still pending to be eecuted and can be scheduled again
> 
> > +static int __init mcf_edma_init(void)
> > +{
> > +	return platform_driver_register(&mcf_edma_driver);
> > +}
> > +subsys_initcall(mcf_edma_init);
> 
> why subsys_initcall?
> 

I find subsys_initcall in several dma drivers, my understanding is that
it initializes the driver before other drivers can use it.
It also sets the driver as built in only.
This seems ok for my case.

Regards,
Angelo

> -- 
> ~Vinod
---
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	[flat|nested] 11+ messages in thread

* [v5,2/2] dmaengine: fsl-edma: add ColdFire mcf5441x edma support
@ 2018-06-29  5:15 Vinod Koul
  0 siblings, 0 replies; 11+ messages in thread
From: Vinod Koul @ 2018-06-29  5:15 UTC (permalink / raw)
  To: Angelo Dureghello; +Cc: Geert Uytterhoeven, dmaengine, Linux/m68k

On 28-06-18, 18:56, Angelo Dureghello wrote:

> Vinod,
> what do you think, am i near to a possible "accept" in a v6 or v7 ? 
> Or do you see any additional great job to do or other important 
> blocking points ?

I review to merge :) If I dont find any issues I will merge straight
away, there is nothing like v6/v7

HTH

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [v5,2/2] dmaengine: fsl-edma: add ColdFire mcf5441x edma support
@ 2018-06-28 16:56 Angelo Dureghello
  0 siblings, 0 replies; 11+ messages in thread
From: Angelo Dureghello @ 2018-06-28 16:56 UTC (permalink / raw)
  To: Vinod; +Cc: Geert Uytterhoeven, dmaengine, Linux/m68k

Hi Vinod and Geert,

On Thu, Jun 28, 2018 at 04:39:13PM +0530, Vinod wrote:
> On 28-06-18, 09:43, Geert Uytterhoeven wrote:
> > Hi Vinod,
> > 
> > On Thu, Jun 28, 2018 at 9:29 AM Vinod <vkoul@kernel.org> wrote:
> > > On 28-06-18, 08:50, Geert Uytterhoeven wrote:
> > > > On Thu, Jun 28, 2018 at 8:29 AM Vinod <vkoul@kernel.org> wrote:
> > > > > On 22-06-18, 11:44, Angelo Dureghello wrote:
> > > > > >  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
> > > > >
> > > > > that makes kernel have two copies of common.o one in thsi driver and one
> > > >
> > > > Does it? It's a common pattern in several Makefiles (e.g.
> > > > drivers/net/ethernet/8390/Makefile and drivers/scsi/Makefile)
> > >
> > > won't each static symbol be part each one?
> > 
> > Remember, obj-y is a list, and IIRC it's filtered for duplicates.
> > 
> > > What about when they are modules?
> > 
> > Same thing, you'll have fsl-edma-common.ko, and fsl-edma.ko and/or mcf-edma.ko.
> 
> Yeah that is right, I missed the list part
>

Ok, so if i understand, i'll fix all the Vinod points except the
Kconfig/makefile part that seems ok as is.

Vinod,
what do you think, am i near to a possible "accept" in a v6 or v7 ? 
Or do you see any additional great job to do or other important 
blocking points ?

Thanks,
regards,
Angelo Dureghello

 
> -- 
> ~Vinod
> --
> 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
---
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	[flat|nested] 11+ messages in thread

* [v5,2/2] dmaengine: fsl-edma: add ColdFire mcf5441x edma support
@ 2018-06-28 11:09 Vinod Koul
  0 siblings, 0 replies; 11+ messages in thread
From: Vinod Koul @ 2018-06-28 11:09 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Angelo Dureghello, dmaengine, Linux/m68k

On 28-06-18, 09:43, Geert Uytterhoeven wrote:
> Hi Vinod,
> 
> On Thu, Jun 28, 2018 at 9:29 AM Vinod <vkoul@kernel.org> wrote:
> > On 28-06-18, 08:50, Geert Uytterhoeven wrote:
> > > On Thu, Jun 28, 2018 at 8:29 AM Vinod <vkoul@kernel.org> wrote:
> > > > On 22-06-18, 11:44, Angelo Dureghello wrote:
> > > > >  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
> > > >
> > > > that makes kernel have two copies of common.o one in thsi driver and one
> > >
> > > Does it? It's a common pattern in several Makefiles (e.g.
> > > drivers/net/ethernet/8390/Makefile and drivers/scsi/Makefile)
> >
> > won't each static symbol be part each one?
> 
> Remember, obj-y is a list, and IIRC it's filtered for duplicates.
> 
> > What about when they are modules?
> 
> Same thing, you'll have fsl-edma-common.ko, and fsl-edma.ko and/or mcf-edma.ko.

Yeah that is right, I missed the list part

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [v5,2/2] dmaengine: fsl-edma: add ColdFire mcf5441x edma support
@ 2018-06-28  7:43 Geert Uytterhoeven
  0 siblings, 0 replies; 11+ messages in thread
From: Geert Uytterhoeven @ 2018-06-28  7:43 UTC (permalink / raw)
  To: vkoul; +Cc: Angelo Dureghello, dmaengine, Linux/m68k

Hi Vinod,

On Thu, Jun 28, 2018 at 9:29 AM Vinod <vkoul@kernel.org> wrote:
> On 28-06-18, 08:50, Geert Uytterhoeven wrote:
> > On Thu, Jun 28, 2018 at 8:29 AM Vinod <vkoul@kernel.org> wrote:
> > > On 22-06-18, 11:44, Angelo Dureghello wrote:
> > > >  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
> > >
> > > that makes kernel have two copies of common.o one in thsi driver and one
> >
> > Does it? It's a common pattern in several Makefiles (e.g.
> > drivers/net/ethernet/8390/Makefile and drivers/scsi/Makefile)
>
> won't each static symbol be part each one?

Remember, obj-y is a list, and IIRC it's filtered for duplicates.

> What about when they are modules?

Same thing, you'll have fsl-edma-common.ko, and fsl-edma.ko and/or mcf-edma.ko.

> > > in previous one why not do:
> > >
> > > CONFIG_FSL_COMMON += fsl-edma-common.o
> > > CONFIG_FSL_EDMA += fsl-edma.o
> > > CONFIG_MCF_EDMA += mcf-edma.o
> > >
> > > and you select CONFIG_FSL_COMMON in both FSL and MCF Kconfig?
> >
> > That's what he had originally, until I suggested to do the above, as
> > nothing else
> > needed the CONFIG_FSL_COMMON symbol.
>
> Hmmm, okay what are we gaining from this approach?

Less Kconfig symbols, less lines of Kconfig and Makefile?

Gr{oetje,eeting}s,

                        Geert

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [v5,2/2] dmaengine: fsl-edma: add ColdFire mcf5441x edma support
@ 2018-06-28  7:29 Vinod Koul
  0 siblings, 0 replies; 11+ messages in thread
From: Vinod Koul @ 2018-06-28  7:29 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Angelo Dureghello, dmaengine, Linux/m68k

On 28-06-18, 08:50, Geert Uytterhoeven wrote:
> Hi Vinod,
> 
> On Thu, Jun 28, 2018 at 8:29 AM Vinod <vkoul@kernel.org> wrote:
> > On 22-06-18, 11:44, Angelo Dureghello wrote:
> > >  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
> >
> > that makes kernel have two copies of common.o one in thsi driver and one
> 
> Does it? It's a common pattern in several Makefiles (e.g.
> drivers/net/ethernet/8390/Makefile and drivers/scsi/Makefile)

won't each static symbol be part each one? What about when they are
modules?

> > in previous one why not do:
> >
> > CONFIG_FSL_COMMON += fsl-edma-common.o
> > CONFIG_FSL_EDMA += fsl-edma.o
> > CONFIG_MCF_EDMA += mcf-edma.o
> >
> > and you select CONFIG_FSL_COMMON in both FSL and MCF Kconfig?
> 
> That's what he had originally, until I suggested to do the above, as
> nothing else
> needed the CONFIG_FSL_COMMON symbol.

Hmmm, okay what are we gaining from this approach?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [v5,2/2] dmaengine: fsl-edma: add ColdFire mcf5441x edma support
@ 2018-06-28  6:50 Geert Uytterhoeven
  0 siblings, 0 replies; 11+ messages in thread
From: Geert Uytterhoeven @ 2018-06-28  6:50 UTC (permalink / raw)
  To: vkoul; +Cc: Angelo Dureghello, dmaengine, Linux/m68k

Hi Vinod,

On Thu, Jun 28, 2018 at 8:29 AM Vinod <vkoul@kernel.org> wrote:
> On 22-06-18, 11:44, Angelo Dureghello wrote:
> >  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
>
> that makes kernel have two copies of common.o one in thsi driver and one

Does it? It's a common pattern in several Makefiles (e.g.
drivers/net/ethernet/8390/Makefile and drivers/scsi/Makefile)

> in previous one why not do:
>
> CONFIG_FSL_COMMON += fsl-edma-common.o
> CONFIG_FSL_EDMA += fsl-edma.o
> CONFIG_MCF_EDMA += mcf-edma.o
>
> and you select CONFIG_FSL_COMMON in both FSL and MCF Kconfig?

That's what he had originally, until I suggested to do the above, as
nothing else
needed the CONFIG_FSL_COMMON symbol.

Gr{oetje,eeting}s,

                        Geert

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [v5,2/2] dmaengine: fsl-edma: add ColdFire mcf5441x edma support
@ 2018-06-28  6:23 Vinod Koul
  0 siblings, 0 replies; 11+ messages in thread
From: Vinod Koul @ 2018-06-28  6:23 UTC (permalink / raw)
  To: Angelo Dureghello; +Cc: dmaengine, linux-m68k

On 22-06-18, 11:44, Angelo Dureghello wrote:
>  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

that makes kernel have two copies of common.o one in thsi driver and one
in previous one why not do:

CONFIG_FSL_COMMON += fsl-edma-common.o
CONFIG_FSL_EDMA += fsl-edma.o
CONFIG_MCF_EDMA += mcf-edma.o

and you select CONFIG_FSL_COMMON in both FSL and MCF Kconfig?

> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2013-2014 Freescale Semiconductor, Inc
> +// Copyright (c) 2017 Sysam, Angelo Dureghello  <angelo@sysam.it>
> +/*
> + * drivers/dma/mcf-edma.c
> + *
> + * Driver for the Freescale ColdFire 64-ch eDMA implementation,
> + * derived from drivers/dma/fsl-edma.c.
> + *
> + * This program is free software; you can redistribute  it and/or modify it
> + * under  the terms of  the GNU General  Public License as published by the
> + * Free Software Foundation;  either version 2 of the  License, or (at your
> + * option) any later version.
> + */

again, no need for text

> +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 & (0x1 << ch)) {

                intmap & BIT(ch)

> +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 & (0x1 << ch)) {

here as well

> +static int mcf_edma_remove(struct platform_device *pdev)
> +{
> +	struct fsl_edma_engine *mcf_edma = platform_get_drvdata(pdev);
> +
> +	fsl_edma_cleanup_vchan(&mcf_edma->dma_dev);
> +	dma_async_device_unregister(&mcf_edma->dma_dev);

at this point your irqs are still registered and running. You vchan
tasklet maybe still pending to be eecuted and can be scheduled again

> +static int __init mcf_edma_init(void)
> +{
> +	return platform_driver_register(&mcf_edma_driver);
> +}
> +subsys_initcall(mcf_edma_init);

why subsys_initcall?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [v5,2/2] dmaengine: fsl-edma: add ColdFire mcf5441x edma support
@ 2018-06-22  9:44 Angelo Dureghello
  0 siblings, 0 replies; 11+ messages in thread
From: Angelo Dureghello @ 2018-06-22  9:44 UTC (permalink / raw)
  To: vinod.koul; +Cc: dmaengine, linux-m68k, Angelo Dureghello

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/2).

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
---
 drivers/dma/Kconfig                        |  11 +
 drivers/dma/Makefile                       |   1 +
 drivers/dma/mcf-edma.c                     | 299 +++++++++++++++++++++
 include/linux/platform_data/dma-mcf-edma.h |  38 +++
 4 files changed, 349 insertions(+)
 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..23f444608514 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/mcf-edma.c b/drivers/dma/mcf-edma.c
new file mode 100644
index 000000000000..9e1d55a5cc90
--- /dev/null
+++ b/drivers/dma/mcf-edma.c
@@ -0,0 +1,299 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2013-2014 Freescale Semiconductor, Inc
+// Copyright (c) 2017 Sysam, Angelo Dureghello  <angelo@sysam.it>
+/*
+ * drivers/dma/mcf-edma.c
+ *
+ * Driver for the Freescale ColdFire 64-ch eDMA implementation,
+ * derived from drivers/dma/fsl-edma.c.
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+
+#include <linux/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 & (0x1 << 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 & (0x1 << 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 & (0x1 << (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 |= devm_request_irq(&pdev->dev, 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 |= devm_request_irq(&pdev->dev, 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 = devm_request_irq(&pdev->dev, 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 = devm_request_irq(&pdev->dev, ret,
+			mcf_edma_err_handler, 0, "eDMA", mcf_edma);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+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, &regs->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);
+
+	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..4f45d0d40aa7
--- /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@xxxxxxxx>
+ *
+ * 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__
+
+struct dma_slave_map;
+
+bool mcf_edma_filter_fn(struct dma_chan *chan, void *param);
+
+#define MCF_EDMA_FILTER_PARAM(ch)	((void *)ch)
+
+/**
+ * struct mcf_edma_platform_data - platform specific data for eDMA engine
+ *
+ * @ver			The eDMA module version.
+ * @dma_channels	The number of eDMA channels.
+ */
+struct mcf_edma_platform_data {
+	int dma_channels;
+	const struct dma_slave_map *slave_map;
+	int slavecnt;
+};
+
+#endif /* __MACH_MCF_EDMA_H__ */

^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-07-01 13:11 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-01 13:11 [v5,2/2] dmaengine: fsl-edma: add ColdFire mcf5441x edma support Vinod Koul
  -- strict thread matches above, loose matches on Subject: below --
2018-06-30 21:06 Geert Uytterhoeven
2018-06-30 13:42 Angelo Dureghello
2018-06-29  5:15 Vinod Koul
2018-06-28 16:56 Angelo Dureghello
2018-06-28 11:09 Vinod Koul
2018-06-28  7:43 Geert Uytterhoeven
2018-06-28  7:29 Vinod Koul
2018-06-28  6:50 Geert Uytterhoeven
2018-06-28  6:23 Vinod Koul
2018-06-22  9:44 Angelo Dureghello

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.