[RFC][PATCH 0/3] fixes and MPC8308 support for the mpc512x_dma driver

[RFC][PATCH 0/3] fixes and MPC8308 support for the mpc512x_dma driver

[Ilya Yanok]

Hello everybody,

I've found that mpc512x_dma doesn't work reliable in my tests (dmatest
module and NetPipe with CONFIG_NET_DMA enabled).
These patches fixes two issues I've found: inproper handling of
scatter/gather transfers and missing interrupts.
The third patch adds support for MPC8308 which has pretty much the same
DMA controller as MPC5121.

The first patch has an issue: I'm accessing IO space with direct memory
access not via accessor functions. I don't know how to use accessor
functions with bitfield structures.

Any comments would be appreciated.

With these patches applied my tests succeed both on MPC5121 and MPC8308.

Regards, Ilya.

Signed-off-by: Ilya Yanok <yanok@emcraft.com>
Cc: Piotr Ziecik <kosmo@semihalf.com>

[PATCH 1/3] mpc512x_dma: scatter/gather fix

[Ilya Yanok]

While testing mpc512x-dma driver with dmatest module I've found that
I can hang the mpc512x-dma issueing request from multiple threads to
the single channel.
(insmod dmatest.ko max_channels=1 threads_per_chan=16)
After investingating this case I've managed to find that this happens
if and only if we have more than one quequed requests.
In this case the driver tries to make use of hardware scatter/gather
functionality. I've found two problems with scatter/gather:
 1. When TCD is copied form RAM to the TCD register space with memcpy_io()
e_sg bit eventually gets cleared. This results in only first TCD being
executed. I've added setting of e_sg bit excplicitly in the TCD registers.
BTW, what is the correct way to do this? (How can I use setbits with bitfield
structure?) After that hardware loads consecutive TCDs and we hit the
second issue.
 2. Existing code clears int_maj bit in the last TCD so we never get
an interrupt on transfefr completion.

With these fixes my tests with many threads of single channel succeed but
tests that use many channels simultaneously still don't work reliable.

Signed-off-by: Ilya Yanok <yanok@emcraft.com>
Cc: Piotr Ziecik <kosmo@semihalf.com>
---
 drivers/dma/mpc512x_dma.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
index 4e9cbf3..1bc04aa 100644
--- a/drivers/dma/mpc512x_dma.c
+++ b/drivers/dma/mpc512x_dma.c
@@ -252,11 +252,13 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan)
 		prev = mdesc;
 	}
 
-	prev->tcd->start = 0;
 	prev->tcd->int_maj = 1;
 
 	/* Send first descriptor in chain into hardware */
 	memcpy_toio(&mdma->tcd[cid], first->tcd, sizeof(struct mpc_dma_tcd));
+
+	if (first != prev)
+		mdma->tcd[cid].e_sg = 1;
 	out_8(&mdma->regs->dmassrt, cid);
 }
 
-- 
1.7.2.3

[PATCH 2/3] mpc512x_dma: fix the hanged transfer issue

[Ilya Yanok]

Current code clears interrupt active status _after_ submiting new
transfers. This leaves a possibility of clearing the interrupt for this
new transfer (if it is triggered fast enough) and thus lose this
interrupt. We want to clear interrupt active status _before_ new
transfers is submited and for current channel only.

Signed-off-by: Ilya Yanok <yanok@emcraft.com>
Cc: Piotr Ziecik <kosmo@semihalf.com>
---
 drivers/dma/mpc512x_dma.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
index 1bc04aa..0717527 100644
--- a/drivers/dma/mpc512x_dma.c
+++ b/drivers/dma/mpc512x_dma.c
@@ -276,6 +276,9 @@ static void mpc_dma_irq_process(struct mpc_dma *mdma, u32 is, u32 es, int off)
 
 		spin_lock(&mchan->lock);
 
+		out_8(&mdma->regs->dmacint, ch + off);
+		out_8(&mdma->regs->dmacerr, ch + off);
+
 		/* Check error status */
 		if (es & (1 << ch))
 			list_for_each_entry(mdesc, &mchan->active, node)
@@ -309,12 +312,6 @@ static irqreturn_t mpc_dma_irq(int irq, void *data)
 	mpc_dma_irq_process(mdma, in_be32(&mdma->regs->dmaintl),
 					in_be32(&mdma->regs->dmaerrl), 0);
 
-	/* Ack interrupt on all channels */
-	out_be32(&mdma->regs->dmainth, 0xFFFFFFFF);
-	out_be32(&mdma->regs->dmaintl, 0xFFFFFFFF);
-	out_be32(&mdma->regs->dmaerrh, 0xFFFFFFFF);
-	out_be32(&mdma->regs->dmaerrl, 0xFFFFFFFF);
-
 	/* Schedule tasklet */
 	tasklet_schedule(&mdma->tasklet);
 
-- 
1.7.2.3

[PATCH 3/3] mpc512x_dma: add MPC8308 support

[Ilya Yanok]

MPC8308 has pretty much the same DMA controller as MPC5121 and
this patch adds support for MPC8308 to the mpc512x_dma driver.

Signed-off-by: Ilya Yanok <yanok@emcraft.com>
Cc: Piotr Ziecik <kosmo@semihalf.com>
---
 drivers/dma/Kconfig       |    2 +-
 drivers/dma/mpc512x_dma.c |   95 +++++++++++++++++++++++++++++++++-----------
 2 files changed, 72 insertions(+), 25 deletions(-)

diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 9520cf0..5c5e95b 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -100,7 +100,7 @@ config FSL_DMA
 
 config MPC512X_DMA
 	tristate "Freescale MPC512x built-in DMA engine support"
-	depends on PPC_MPC512x
+	depends on PPC_MPC512x || PPC_MPC831x
 	select DMA_ENGINE
 	---help---
 	  Enable support for the Freescale MPC512x built-in DMA engine.
diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
index 0717527..97b92ec 100644
--- a/drivers/dma/mpc512x_dma.c
+++ b/drivers/dma/mpc512x_dma.c
@@ -1,6 +1,7 @@
 /*
  * Copyright (C) Freescale Semicondutor, Inc. 2007, 2008.
  * Copyright (C) Semihalf 2009
+ * Copyright (C) Ilya Yanok, Emcraft Systems 2010
  *
  * Written by Piotr Ziecik <kosmo@semihalf.com>. Hardware description
  * (defines, structures and comments) was taken from MPC5121 DMA driver
@@ -70,6 +71,8 @@
 #define MPC_DMA_DMAES_SBE	(1 << 1)
 #define MPC_DMA_DMAES_DBE	(1 << 0)
 
+#define MPC_DMA_DMAGPOR_SNOOP_ENABLE	(1 << 6)
+
 #define MPC_DMA_TSIZE_1		0x00
 #define MPC_DMA_TSIZE_2		0x01
 #define MPC_DMA_TSIZE_4		0x02
@@ -104,7 +107,10 @@ struct __attribute__ ((__packed__)) mpc_dma_regs {
 	/* 0x30 */
 	u32 dmahrsh;		/* DMA hw request status high(ch63~32) */
 	u32 dmahrsl;		/* DMA hardware request status low(ch31~0) */
-	u32 dmaihsa;		/* DMA interrupt high select AXE(ch63~32) */
+	union {
+		u32 dmaihsa;	/* DMA interrupt high select AXE(ch63~32) */
+		u32 dmagpor;	/* (General purpose register on MPC8308) */
+	};
 	u32 dmailsa;		/* DMA interrupt low select AXE(ch31~0) */
 	/* 0x40 ~ 0xff */
 	u32 reserve0[48];	/* Reserved */
@@ -195,7 +201,9 @@ struct mpc_dma {
 	struct mpc_dma_regs __iomem	*regs;
 	struct mpc_dma_tcd __iomem	*tcd;
 	int				irq;
+	int				irq2;
 	uint				error_status;
+	int				is_mpc8308;
 
 	/* Lock for error_status field in this structure */
 	spinlock_t			error_status_lock;
@@ -307,8 +315,10 @@ static irqreturn_t mpc_dma_irq(int irq, void *data)
 	spin_unlock(&mdma->error_status_lock);
 
 	/* Handle interrupt on each channel */
-	mpc_dma_irq_process(mdma, in_be32(&mdma->regs->dmainth),
+	if (mdma->dma.chancnt > 32) {
+		mpc_dma_irq_process(mdma, in_be32(&mdma->regs->dmainth),
 					in_be32(&mdma->regs->dmaerrh), 32);
+	}
 	mpc_dma_irq_process(mdma, in_be32(&mdma->regs->dmaintl),
 					in_be32(&mdma->regs->dmaerrl), 0);
 
@@ -562,6 +572,7 @@ static struct dma_async_tx_descriptor *
 mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t dst, dma_addr_t src,
 					size_t len, unsigned long flags)
 {
+	struct mpc_dma *mdma = dma_chan_to_mpc_dma(chan);
 	struct mpc_dma_chan *mchan = dma_chan_to_mpc_dma_chan(chan);
 	struct mpc_dma_desc *mdesc = NULL;
 	struct mpc_dma_tcd *tcd;
@@ -590,7 +601,8 @@ mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t dst, dma_addr_t src,
 		tcd->dsize = MPC_DMA_TSIZE_32;
 		tcd->soff = 32;
 		tcd->doff = 32;
-	} else if (IS_ALIGNED(src | dst | len, 16)) {
+	} else if (!mdma->is_mpc8308 && IS_ALIGNED(src | dst | len, 16)) {
+		/* MPC8308 doesn't support 16 byte transfers */
 		tcd->ssize = MPC_DMA_TSIZE_16;
 		tcd->dsize = MPC_DMA_TSIZE_16;
 		tcd->soff = 16;
@@ -650,6 +662,15 @@ static int __devinit mpc_dma_probe(struct platform_device *op,
 		return -EINVAL;
 	}
 
+	if (of_device_is_compatible(dn, "fsl,mpc8308-dma")) {
+		mdma->is_mpc8308 = 1;
+		mdma->irq2 = irq_of_parse_and_map(dn, 1);
+		if (mdma->irq2 == NO_IRQ) {
+			dev_err(dev, "Error mapping IRQ!\n");
+			return -EINVAL;
+		}
+	}
+
 	retval = of_address_to_resource(dn, 0, &res);
 	if (retval) {
 		dev_err(dev, "Error parsing memory region!\n");
@@ -680,11 +701,23 @@ static int __devinit mpc_dma_probe(struct platform_device *op,
 		return -EINVAL;
 	}
 
+	if (mdma->is_mpc8308) {
+		retval = devm_request_irq(dev, mdma->irq2, &mpc_dma_irq, 0,
+				DRV_NAME, mdma);
+		if (retval) {
+			dev_err(dev, "Error requesting IRQ2!\n");
+			return -EINVAL;
+		}
+	}
+
 	spin_lock_init(&mdma->error_status_lock);
 
 	dma = &mdma->dma;
 	dma->dev = dev;
-	dma->chancnt = MPC_DMA_CHANNELS;
+	if (!mdma->is_mpc8308)
+		dma->chancnt = MPC_DMA_CHANNELS;
+	else
+		dma->chancnt = 16; /* MPC8308 DMA has only 16 channels */
 	dma->device_alloc_chan_resources = mpc_dma_alloc_chan_resources;
 	dma->device_free_chan_resources = mpc_dma_free_chan_resources;
 	dma->device_issue_pending = mpc_dma_issue_pending;
@@ -720,26 +753,40 @@ static int __devinit mpc_dma_probe(struct platform_device *op,
 	 * - Round-robin group arbitration,
 	 * - Round-robin channel arbitration.
 	 */
-	out_be32(&mdma->regs->dmacr, MPC_DMA_DMACR_EDCG |
-				MPC_DMA_DMACR_ERGA | MPC_DMA_DMACR_ERCA);
-
-	/* Disable hardware DMA requests */
-	out_be32(&mdma->regs->dmaerqh, 0);
-	out_be32(&mdma->regs->dmaerql, 0);
-
-	/* Disable error interrupts */
-	out_be32(&mdma->regs->dmaeeih, 0);
-	out_be32(&mdma->regs->dmaeeil, 0);
-
-	/* Clear interrupts status */
-	out_be32(&mdma->regs->dmainth, 0xFFFFFFFF);
-	out_be32(&mdma->regs->dmaintl, 0xFFFFFFFF);
-	out_be32(&mdma->regs->dmaerrh, 0xFFFFFFFF);
-	out_be32(&mdma->regs->dmaerrl, 0xFFFFFFFF);
-
-	/* Route interrupts to IPIC */
-	out_be32(&mdma->regs->dmaihsa, 0);
-	out_be32(&mdma->regs->dmailsa, 0);
+	if (!mdma->is_mpc8308) {
+		out_be32(&mdma->regs->dmacr, MPC_DMA_DMACR_EDCG |
+					MPC_DMA_DMACR_ERGA | MPC_DMA_DMACR_ERCA);
+
+		/* Disable hardware DMA requests */
+		out_be32(&mdma->regs->dmaerqh, 0);
+		out_be32(&mdma->regs->dmaerql, 0);
+
+		/* Disable error interrupts */
+		out_be32(&mdma->regs->dmaeeih, 0);
+		out_be32(&mdma->regs->dmaeeil, 0);
+
+		/* Clear interrupts status */
+		out_be32(&mdma->regs->dmainth, 0xFFFFFFFF);
+		out_be32(&mdma->regs->dmaintl, 0xFFFFFFFF);
+		out_be32(&mdma->regs->dmaerrh, 0xFFFFFFFF);
+		out_be32(&mdma->regs->dmaerrl, 0xFFFFFFFF);
+
+		/* Route interrupts to IPIC */
+		out_be32(&mdma->regs->dmaihsa, 0);
+		out_be32(&mdma->regs->dmailsa, 0);
+	} else {
+		/* MPC8308 has 16 channels and lacks some registers */
+		out_be32(&mdma->regs->dmacr, MPC_DMA_DMACR_ERCA);
+
+		/* enable snooping */
+		out_be32(&mdma->regs->dmagpor, MPC_DMA_DMAGPOR_SNOOP_ENABLE);
+		/* Disable error interrupts */
+		out_be32(&mdma->regs->dmaeeil, 0);
+
+		/* Clear interrupts status */
+		out_be32(&mdma->regs->dmaintl, 0xFFFF);
+		out_be32(&mdma->regs->dmaerrl, 0xFFFF);
+	}
 
 	/* Register DMA engine */
 	dev_set_drvdata(dev, mdma);
-- 
1.7.2.3

Re: [PATCH 3/3] mpc512x_dma: add MPC8308 support

[Wolfgang Denk]

Dear Ilya Yanok,

In message <1285676696-5358-4-git-send-email-yanok@emcraft.com> you wrote:
> MPC8308 has pretty much the same DMA controller as MPC5121 and
> this patch adds support for MPC8308 to the mpc512x_dma driver.
> 
> Signed-off-by: Ilya Yanok <yanok@emcraft.com>
> Cc: Piotr Ziecik <kosmo@semihalf.com>
> ---
>  drivers/dma/Kconfig       |    2 +-
>  drivers/dma/mpc512x_dma.c |   95 +++++++++++++++++++++++++++++++++-----------
>  2 files changed, 72 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index 9520cf0..5c5e95b 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -100,7 +100,7 @@ config FSL_DMA
>  
>  config MPC512X_DMA
>  	tristate "Freescale MPC512x built-in DMA engine support"
> -	depends on PPC_MPC512x
> +	depends on PPC_MPC512x || PPC_MPC831x

Is MPC831x correct here? My understanding is that MPC831x processors
have yet other DMA cotnrollers, and we're on a MPC8308 here?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
"A little knowledge is a dangerous thing."                - Doug Gwyn

Re: [PATCH 3/3] mpc512x_dma: add MPC8308 support

[Ilya Yanok]

Dear Wolfgang,

28.09.2010 17:09, Wolfgang Denk wrote:
>>   config MPC512X_DMA
>>   	tristate "Freescale MPC512x built-in DMA engine support"
>> -	depends on PPC_MPC512x
>> +	depends on PPC_MPC512x || PPC_MPC831x
>
> Is MPC831x correct here? My understanding is that MPC831x processors
> have yet other DMA cotnrollers, and we're on a MPC8308 here?

Well, PPC_MPC831x is not correct here in the strict sense, but there are 
some reasons for it:
  1. We don't really have PPC_MPC8308 config option for MPC8308 
processor. Well, maybe that was my fault that I didn't add it when I 
initially introduced support for MPC8308. But I don't actually see the 
point for it. All the differencies from MPC831x are handled run-time 
based on device-tree.
  2. Some of MPC831x (I believe it's MPC8315) really has the compatible 
DMA controller (it's called something like DMA controller of the TDM 
module). Well it will probably need some additional work in the driver 
to support this controller but hardware is mostly the same.
  3. Well, it's only compilation option you need a proper device-tree 
node for the driver to start. Ok, you can make your kernel bigger by 
compiling in the driver which is useless for your CPU but you can't 
break it provided you have a correct device-tree.

Regards, Ilya.

[REPOST] [PATCH 0/6] fixes and MPC8308 support for the mpc512x_dma driver

[Ilya Yanok]

Hello everybody,

meanwhile I've fixed one more issue in mpc512x_dma driver.

Any comments? Anybody interrested in this driver? Piotr?

Still unsure how to deal with bitfield structures in IO space...

Regards, Ilya.

Signed-off-by: Ilya Yanok <yanok@emcraft.com>
Cc: Piotr Ziecik <kosmo@semihalf.com>

Re: [REPOST] [PATCH 0/6] fixes and MPC8308 support for the mpc512x_dma driver

[Piotr Zięcik]

On Wednesday 27 October 2010 01:52:54 Ilya Yanok wrote:
> Hello everybody,
> 
> meanwhile I've fixed one more issue in mpc512x_dma driver.
> 
> Any comments? Anybody interrested in this driver? Piotr?
> 
> Still unsure how to deal with bitfield structures in IO space...
> 

Currently I am not able to deal with this as I am much involved in other development.

-- 
Best Regards,
Piotr Zięcik.

Re: [REPOST] [PATCH 0/6] fixes and MPC8308 support for the mpc512x_dma driver

[Ilya Yanok]

Hi Piotr,

On 27.10.2010 11:24, Piotr Zięcik wrote:
> Currently I am not able to deal with this as I am much involved in 
> other development.

I see. Excuse me for disturbing you then.

Guys, anybody can review/test/pull these patches?

Regards, Ilya.

Re: [REPOST] [PATCH 0/6] fixes and MPC8308 support for the mpc512x_dma driver

[Wolfgang Denk]

Dear Ilya Yanok,

In message <1288137180-3220-1-git-send-email-yanok@emcraft.com> you wrote:
> Hello everybody,
> 
> meanwhile I've fixed one more issue in mpc512x_dma driver.
> 
> Any comments? Anybody interrested in this driver? Piotr?
> 
> Still unsure how to deal with bitfield structures in IO space...
> 
> Regards, Ilya.
> 
> Signed-off-by: Ilya Yanok <yanok@emcraft.com>
> Cc: Piotr Ziecik <kosmo@semihalf.com>

Tested-by: Wolfgang Denk <wd@denx.de>
Acked-by: Wolfgang Denk <wd@denx.de>

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
I have often regretted my speech, never my silence.

[PATCH 1/6] mpc512x_dma: scatter/gather fix

[Ilya Yanok]

While testing mpc512x-dma driver with dmatest module I've found that
I can hang the mpc512x-dma issueing request from multiple threads to
the single channel.
(insmod dmatest.ko max_channels=1 threads_per_chan=16)
After investingating this case I've managed to find that this happens
if and only if we have more than one quequed requests.
In this case the driver tries to make use of hardware scatter/gather
functionality. I've found two problems with scatter/gather:
 1. When TCD is copied form RAM to the TCD register space with memcpy_io()
e_sg bit eventually gets cleared. This results in only first TCD being
executed. I've added setting of e_sg bit excplicitly in the TCD registers.
BTW, what is the correct way to do this? (How can I use setbits with bitfield
structure?) After that hardware loads consecutive TCDs and we hit the
second issue.
 2. Existing code clears int_maj bit in the last TCD so we never get
an interrupt on transfefr completion.

With these fixes my tests with many threads of single channel succeed but
tests that use many channels simultaneously still don't work reliable.

Signed-off-by: Ilya Yanok <yanok@emcraft.com>
Cc: Piotr Ziecik <kosmo@semihalf.com>
---
 drivers/dma/mpc512x_dma.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
index 4e9cbf3..1bc04aa 100644
--- a/drivers/dma/mpc512x_dma.c
+++ b/drivers/dma/mpc512x_dma.c
@@ -252,11 +252,13 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan)
 		prev = mdesc;
 	}
 
-	prev->tcd->start = 0;
 	prev->tcd->int_maj = 1;
 
 	/* Send first descriptor in chain into hardware */
 	memcpy_toio(&mdma->tcd[cid], first->tcd, sizeof(struct mpc_dma_tcd));
+
+	if (first != prev)
+		mdma->tcd[cid].e_sg = 1;
 	out_8(&mdma->regs->dmassrt, cid);
 }
 
-- 
1.7.2.3

Re: [PATCH 1/6] mpc512x_dma: scatter/gather fix

[Grant Likely]

On Wed, Oct 27, 2010 at 01:52:55AM +0200, Ilya Yanok wrote:
> While testing mpc512x-dma driver with dmatest module I've found that
> I can hang the mpc512x-dma issueing request from multiple threads to
> the single channel.
> (insmod dmatest.ko max_channels=1 threads_per_chan=16)
> After investingating this case I've managed to find that this happens
> if and only if we have more than one quequed requests.
> In this case the driver tries to make use of hardware scatter/gather
> functionality. I've found two problems with scatter/gather:
>  1. When TCD is copied form RAM to the TCD register space with memcpy_io()
> e_sg bit eventually gets cleared. This results in only first TCD being
> executed. I've added setting of e_sg bit excplicitly in the TCD registers.
> BTW, what is the correct way to do this? (How can I use setbits with bitfield
> structure?) After that hardware loads consecutive TCDs and we hit the
> second issue.
>  2. Existing code clears int_maj bit in the last TCD so we never get
> an interrupt on transfefr completion.
> 
> With these fixes my tests with many threads of single channel succeed but
> tests that use many channels simultaneously still don't work reliable.
> 
> Signed-off-by: Ilya Yanok <yanok@emcraft.com>
> Cc: Piotr Ziecik <kosmo@semihalf.com>

Merged for -next, thanks.

g.

> ---
>  drivers/dma/mpc512x_dma.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
> index 4e9cbf3..1bc04aa 100644
> --- a/drivers/dma/mpc512x_dma.c
> +++ b/drivers/dma/mpc512x_dma.c
> @@ -252,11 +252,13 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan)
>  		prev = mdesc;
>  	}
>  
> -	prev->tcd->start = 0;
>  	prev->tcd->int_maj = 1;
>  
>  	/* Send first descriptor in chain into hardware */
>  	memcpy_toio(&mdma->tcd[cid], first->tcd, sizeof(struct mpc_dma_tcd));
> +
> +	if (first != prev)
> +		mdma->tcd[cid].e_sg = 1;
>  	out_8(&mdma->regs->dmassrt, cid);
>  }
>  
> -- 
> 1.7.2.3
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 2/6] mpc512x_dma: fix the hanged transfer issue

[Ilya Yanok]

Current code clears interrupt active status _after_ submiting new
transfers. This leaves a possibility of clearing the interrupt for this
new transfer (if it is triggered fast enough) and thus lose this
interrupt. We want to clear interrupt active status _before_ new
transfers is submited and for current channel only.

Signed-off-by: Ilya Yanok <yanok@emcraft.com>
Cc: Piotr Ziecik <kosmo@semihalf.com>
---
 drivers/dma/mpc512x_dma.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
index 1bc04aa..0717527 100644
--- a/drivers/dma/mpc512x_dma.c
+++ b/drivers/dma/mpc512x_dma.c
@@ -276,6 +276,9 @@ static void mpc_dma_irq_process(struct mpc_dma *mdma, u32 is, u32 es, int off)
 
 		spin_lock(&mchan->lock);
 
+		out_8(&mdma->regs->dmacint, ch + off);
+		out_8(&mdma->regs->dmacerr, ch + off);
+
 		/* Check error status */
 		if (es & (1 << ch))
 			list_for_each_entry(mdesc, &mchan->active, node)
@@ -309,12 +312,6 @@ static irqreturn_t mpc_dma_irq(int irq, void *data)
 	mpc_dma_irq_process(mdma, in_be32(&mdma->regs->dmaintl),
 					in_be32(&mdma->regs->dmaerrl), 0);
 
-	/* Ack interrupt on all channels */
-	out_be32(&mdma->regs->dmainth, 0xFFFFFFFF);
-	out_be32(&mdma->regs->dmaintl, 0xFFFFFFFF);
-	out_be32(&mdma->regs->dmaerrh, 0xFFFFFFFF);
-	out_be32(&mdma->regs->dmaerrl, 0xFFFFFFFF);
-
 	/* Schedule tasklet */
 	tasklet_schedule(&mdma->tasklet);
 
-- 
1.7.2.3

Re: [PATCH 2/6] mpc512x_dma: fix the hanged transfer issue

[Grant Likely]

On Wed, Oct 27, 2010 at 01:52:56AM +0200, Ilya Yanok wrote:
> Current code clears interrupt active status _after_ submiting new
> transfers. This leaves a possibility of clearing the interrupt for this
> new transfer (if it is triggered fast enough) and thus lose this
> interrupt. We want to clear interrupt active status _before_ new
> transfers is submited and for current channel only.
> 
> Signed-off-by: Ilya Yanok <yanok@emcraft.com>
> Cc: Piotr Ziecik <kosmo@semihalf.com>

Merged for -next, thanks.

g.

> ---
>  drivers/dma/mpc512x_dma.c |    9 +++------
>  1 files changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
> index 1bc04aa..0717527 100644
> --- a/drivers/dma/mpc512x_dma.c
> +++ b/drivers/dma/mpc512x_dma.c
> @@ -276,6 +276,9 @@ static void mpc_dma_irq_process(struct mpc_dma *mdma, u32 is, u32 es, int off)
>  
>  		spin_lock(&mchan->lock);
>  
> +		out_8(&mdma->regs->dmacint, ch + off);
> +		out_8(&mdma->regs->dmacerr, ch + off);
> +
>  		/* Check error status */
>  		if (es & (1 << ch))
>  			list_for_each_entry(mdesc, &mchan->active, node)
> @@ -309,12 +312,6 @@ static irqreturn_t mpc_dma_irq(int irq, void *data)
>  	mpc_dma_irq_process(mdma, in_be32(&mdma->regs->dmaintl),
>  					in_be32(&mdma->regs->dmaerrl), 0);
>  
> -	/* Ack interrupt on all channels */
> -	out_be32(&mdma->regs->dmainth, 0xFFFFFFFF);
> -	out_be32(&mdma->regs->dmaintl, 0xFFFFFFFF);
> -	out_be32(&mdma->regs->dmaerrh, 0xFFFFFFFF);
> -	out_be32(&mdma->regs->dmaerrl, 0xFFFFFFFF);
> -
>  	/* Schedule tasklet */
>  	tasklet_schedule(&mdma->tasklet);
>  
> -- 
> 1.7.2.3
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 3/6] mpc512x_dma: add MPC8308 support

[Ilya Yanok]

MPC8308 has pretty much the same DMA controller as MPC5121 and
this patch adds support for MPC8308 to the mpc512x_dma driver.

Signed-off-by: Ilya Yanok <yanok@emcraft.com>
Cc: Piotr Ziecik <kosmo@semihalf.com>
---
 drivers/dma/Kconfig       |    2 +-
 drivers/dma/mpc512x_dma.c |   95 +++++++++++++++++++++++++++++++++-----------
 2 files changed, 72 insertions(+), 25 deletions(-)

diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 9520cf0..5c5e95b 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -100,7 +100,7 @@ config FSL_DMA
 
 config MPC512X_DMA
 	tristate "Freescale MPC512x built-in DMA engine support"
-	depends on PPC_MPC512x
+	depends on PPC_MPC512x || PPC_MPC831x
 	select DMA_ENGINE
 	---help---
 	  Enable support for the Freescale MPC512x built-in DMA engine.
diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
index 0717527..97b92ec 100644
--- a/drivers/dma/mpc512x_dma.c
+++ b/drivers/dma/mpc512x_dma.c
@@ -1,6 +1,7 @@
 /*
  * Copyright (C) Freescale Semicondutor, Inc. 2007, 2008.
  * Copyright (C) Semihalf 2009
+ * Copyright (C) Ilya Yanok, Emcraft Systems 2010
  *
  * Written by Piotr Ziecik <kosmo@semihalf.com>. Hardware description
  * (defines, structures and comments) was taken from MPC5121 DMA driver
@@ -70,6 +71,8 @@
 #define MPC_DMA_DMAES_SBE	(1 << 1)
 #define MPC_DMA_DMAES_DBE	(1 << 0)
 
+#define MPC_DMA_DMAGPOR_SNOOP_ENABLE	(1 << 6)
+
 #define MPC_DMA_TSIZE_1		0x00
 #define MPC_DMA_TSIZE_2		0x01
 #define MPC_DMA_TSIZE_4		0x02
@@ -104,7 +107,10 @@ struct __attribute__ ((__packed__)) mpc_dma_regs {
 	/* 0x30 */
 	u32 dmahrsh;		/* DMA hw request status high(ch63~32) */
 	u32 dmahrsl;		/* DMA hardware request status low(ch31~0) */
-	u32 dmaihsa;		/* DMA interrupt high select AXE(ch63~32) */
+	union {
+		u32 dmaihsa;	/* DMA interrupt high select AXE(ch63~32) */
+		u32 dmagpor;	/* (General purpose register on MPC8308) */
+	};
 	u32 dmailsa;		/* DMA interrupt low select AXE(ch31~0) */
 	/* 0x40 ~ 0xff */
 	u32 reserve0[48];	/* Reserved */
@@ -195,7 +201,9 @@ struct mpc_dma {
 	struct mpc_dma_regs __iomem	*regs;
 	struct mpc_dma_tcd __iomem	*tcd;
 	int				irq;
+	int				irq2;
 	uint				error_status;
+	int				is_mpc8308;
 
 	/* Lock for error_status field in this structure */
 	spinlock_t			error_status_lock;
@@ -307,8 +315,10 @@ static irqreturn_t mpc_dma_irq(int irq, void *data)
 	spin_unlock(&mdma->error_status_lock);
 
 	/* Handle interrupt on each channel */
-	mpc_dma_irq_process(mdma, in_be32(&mdma->regs->dmainth),
+	if (mdma->dma.chancnt > 32) {
+		mpc_dma_irq_process(mdma, in_be32(&mdma->regs->dmainth),
 					in_be32(&mdma->regs->dmaerrh), 32);
+	}
 	mpc_dma_irq_process(mdma, in_be32(&mdma->regs->dmaintl),
 					in_be32(&mdma->regs->dmaerrl), 0);
 
@@ -562,6 +572,7 @@ static struct dma_async_tx_descriptor *
 mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t dst, dma_addr_t src,
 					size_t len, unsigned long flags)
 {
+	struct mpc_dma *mdma = dma_chan_to_mpc_dma(chan);
 	struct mpc_dma_chan *mchan = dma_chan_to_mpc_dma_chan(chan);
 	struct mpc_dma_desc *mdesc = NULL;
 	struct mpc_dma_tcd *tcd;
@@ -590,7 +601,8 @@ mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t dst, dma_addr_t src,
 		tcd->dsize = MPC_DMA_TSIZE_32;
 		tcd->soff = 32;
 		tcd->doff = 32;
-	} else if (IS_ALIGNED(src | dst | len, 16)) {
+	} else if (!mdma->is_mpc8308 && IS_ALIGNED(src | dst | len, 16)) {
+		/* MPC8308 doesn't support 16 byte transfers */
 		tcd->ssize = MPC_DMA_TSIZE_16;
 		tcd->dsize = MPC_DMA_TSIZE_16;
 		tcd->soff = 16;
@@ -650,6 +662,15 @@ static int __devinit mpc_dma_probe(struct platform_device *op,
 		return -EINVAL;
 	}
 
+	if (of_device_is_compatible(dn, "fsl,mpc8308-dma")) {
+		mdma->is_mpc8308 = 1;
+		mdma->irq2 = irq_of_parse_and_map(dn, 1);
+		if (mdma->irq2 == NO_IRQ) {
+			dev_err(dev, "Error mapping IRQ!\n");
+			return -EINVAL;
+		}
+	}
+
 	retval = of_address_to_resource(dn, 0, &res);
 	if (retval) {
 		dev_err(dev, "Error parsing memory region!\n");
@@ -680,11 +701,23 @@ static int __devinit mpc_dma_probe(struct platform_device *op,
 		return -EINVAL;
 	}
 
+	if (mdma->is_mpc8308) {
+		retval = devm_request_irq(dev, mdma->irq2, &mpc_dma_irq, 0,
+				DRV_NAME, mdma);
+		if (retval) {
+			dev_err(dev, "Error requesting IRQ2!\n");
+			return -EINVAL;
+		}
+	}
+
 	spin_lock_init(&mdma->error_status_lock);
 
 	dma = &mdma->dma;
 	dma->dev = dev;
-	dma->chancnt = MPC_DMA_CHANNELS;
+	if (!mdma->is_mpc8308)
+		dma->chancnt = MPC_DMA_CHANNELS;
+	else
+		dma->chancnt = 16; /* MPC8308 DMA has only 16 channels */
 	dma->device_alloc_chan_resources = mpc_dma_alloc_chan_resources;
 	dma->device_free_chan_resources = mpc_dma_free_chan_resources;
 	dma->device_issue_pending = mpc_dma_issue_pending;
@@ -720,26 +753,40 @@ static int __devinit mpc_dma_probe(struct platform_device *op,
 	 * - Round-robin group arbitration,
 	 * - Round-robin channel arbitration.
 	 */
-	out_be32(&mdma->regs->dmacr, MPC_DMA_DMACR_EDCG |
-				MPC_DMA_DMACR_ERGA | MPC_DMA_DMACR_ERCA);
-
-	/* Disable hardware DMA requests */
-	out_be32(&mdma->regs->dmaerqh, 0);
-	out_be32(&mdma->regs->dmaerql, 0);
-
-	/* Disable error interrupts */
-	out_be32(&mdma->regs->dmaeeih, 0);
-	out_be32(&mdma->regs->dmaeeil, 0);
-
-	/* Clear interrupts status */
-	out_be32(&mdma->regs->dmainth, 0xFFFFFFFF);
-	out_be32(&mdma->regs->dmaintl, 0xFFFFFFFF);
-	out_be32(&mdma->regs->dmaerrh, 0xFFFFFFFF);
-	out_be32(&mdma->regs->dmaerrl, 0xFFFFFFFF);
-
-	/* Route interrupts to IPIC */
-	out_be32(&mdma->regs->dmaihsa, 0);
-	out_be32(&mdma->regs->dmailsa, 0);
+	if (!mdma->is_mpc8308) {
+		out_be32(&mdma->regs->dmacr, MPC_DMA_DMACR_EDCG |
+					MPC_DMA_DMACR_ERGA | MPC_DMA_DMACR_ERCA);
+
+		/* Disable hardware DMA requests */
+		out_be32(&mdma->regs->dmaerqh, 0);
+		out_be32(&mdma->regs->dmaerql, 0);
+
+		/* Disable error interrupts */
+		out_be32(&mdma->regs->dmaeeih, 0);
+		out_be32(&mdma->regs->dmaeeil, 0);
+
+		/* Clear interrupts status */
+		out_be32(&mdma->regs->dmainth, 0xFFFFFFFF);
+		out_be32(&mdma->regs->dmaintl, 0xFFFFFFFF);
+		out_be32(&mdma->regs->dmaerrh, 0xFFFFFFFF);
+		out_be32(&mdma->regs->dmaerrl, 0xFFFFFFFF);
+
+		/* Route interrupts to IPIC */
+		out_be32(&mdma->regs->dmaihsa, 0);
+		out_be32(&mdma->regs->dmailsa, 0);
+	} else {
+		/* MPC8308 has 16 channels and lacks some registers */
+		out_be32(&mdma->regs->dmacr, MPC_DMA_DMACR_ERCA);
+
+		/* enable snooping */
+		out_be32(&mdma->regs->dmagpor, MPC_DMA_DMAGPOR_SNOOP_ENABLE);
+		/* Disable error interrupts */
+		out_be32(&mdma->regs->dmaeeil, 0);
+
+		/* Clear interrupts status */
+		out_be32(&mdma->regs->dmaintl, 0xFFFF);
+		out_be32(&mdma->regs->dmaerrl, 0xFFFF);
+	}
 
 	/* Register DMA engine */
 	dev_set_drvdata(dev, mdma);
-- 
1.7.2.3

Re: [PATCH 3/6] mpc512x_dma: add MPC8308 support

[Grant Likely]

On Wed, Oct 27, 2010 at 01:52:57AM +0200, Ilya Yanok wrote:
> MPC8308 has pretty much the same DMA controller as MPC5121 and
> this patch adds support for MPC8308 to the mpc512x_dma driver.
> 
> Signed-off-by: Ilya Yanok <yanok@emcraft.com>
> Cc: Piotr Ziecik <kosmo@semihalf.com>

Merged for -next, thanks.

g.

> ---
>  drivers/dma/Kconfig       |    2 +-
>  drivers/dma/mpc512x_dma.c |   95 +++++++++++++++++++++++++++++++++-----------
>  2 files changed, 72 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index 9520cf0..5c5e95b 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -100,7 +100,7 @@ config FSL_DMA
>  
>  config MPC512X_DMA
>  	tristate "Freescale MPC512x built-in DMA engine support"
> -	depends on PPC_MPC512x
> +	depends on PPC_MPC512x || PPC_MPC831x
>  	select DMA_ENGINE
>  	---help---
>  	  Enable support for the Freescale MPC512x built-in DMA engine.
> diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
> index 0717527..97b92ec 100644
> --- a/drivers/dma/mpc512x_dma.c
> +++ b/drivers/dma/mpc512x_dma.c
> @@ -1,6 +1,7 @@
>  /*
>   * Copyright (C) Freescale Semicondutor, Inc. 2007, 2008.
>   * Copyright (C) Semihalf 2009
> + * Copyright (C) Ilya Yanok, Emcraft Systems 2010
>   *
>   * Written by Piotr Ziecik <kosmo@semihalf.com>. Hardware description
>   * (defines, structures and comments) was taken from MPC5121 DMA driver
> @@ -70,6 +71,8 @@
>  #define MPC_DMA_DMAES_SBE	(1 << 1)
>  #define MPC_DMA_DMAES_DBE	(1 << 0)
>  
> +#define MPC_DMA_DMAGPOR_SNOOP_ENABLE	(1 << 6)
> +
>  #define MPC_DMA_TSIZE_1		0x00
>  #define MPC_DMA_TSIZE_2		0x01
>  #define MPC_DMA_TSIZE_4		0x02
> @@ -104,7 +107,10 @@ struct __attribute__ ((__packed__)) mpc_dma_regs {
>  	/* 0x30 */
>  	u32 dmahrsh;		/* DMA hw request status high(ch63~32) */
>  	u32 dmahrsl;		/* DMA hardware request status low(ch31~0) */
> -	u32 dmaihsa;		/* DMA interrupt high select AXE(ch63~32) */
> +	union {
> +		u32 dmaihsa;	/* DMA interrupt high select AXE(ch63~32) */
> +		u32 dmagpor;	/* (General purpose register on MPC8308) */
> +	};
>  	u32 dmailsa;		/* DMA interrupt low select AXE(ch31~0) */
>  	/* 0x40 ~ 0xff */
>  	u32 reserve0[48];	/* Reserved */
> @@ -195,7 +201,9 @@ struct mpc_dma {
>  	struct mpc_dma_regs __iomem	*regs;
>  	struct mpc_dma_tcd __iomem	*tcd;
>  	int				irq;
> +	int				irq2;
>  	uint				error_status;
> +	int				is_mpc8308;
>  
>  	/* Lock for error_status field in this structure */
>  	spinlock_t			error_status_lock;
> @@ -307,8 +315,10 @@ static irqreturn_t mpc_dma_irq(int irq, void *data)
>  	spin_unlock(&mdma->error_status_lock);
>  
>  	/* Handle interrupt on each channel */
> -	mpc_dma_irq_process(mdma, in_be32(&mdma->regs->dmainth),
> +	if (mdma->dma.chancnt > 32) {
> +		mpc_dma_irq_process(mdma, in_be32(&mdma->regs->dmainth),
>  					in_be32(&mdma->regs->dmaerrh), 32);
> +	}
>  	mpc_dma_irq_process(mdma, in_be32(&mdma->regs->dmaintl),
>  					in_be32(&mdma->regs->dmaerrl), 0);
>  
> @@ -562,6 +572,7 @@ static struct dma_async_tx_descriptor *
>  mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t dst, dma_addr_t src,
>  					size_t len, unsigned long flags)
>  {
> +	struct mpc_dma *mdma = dma_chan_to_mpc_dma(chan);
>  	struct mpc_dma_chan *mchan = dma_chan_to_mpc_dma_chan(chan);
>  	struct mpc_dma_desc *mdesc = NULL;
>  	struct mpc_dma_tcd *tcd;
> @@ -590,7 +601,8 @@ mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t dst, dma_addr_t src,
>  		tcd->dsize = MPC_DMA_TSIZE_32;
>  		tcd->soff = 32;
>  		tcd->doff = 32;
> -	} else if (IS_ALIGNED(src | dst | len, 16)) {
> +	} else if (!mdma->is_mpc8308 && IS_ALIGNED(src | dst | len, 16)) {
> +		/* MPC8308 doesn't support 16 byte transfers */
>  		tcd->ssize = MPC_DMA_TSIZE_16;
>  		tcd->dsize = MPC_DMA_TSIZE_16;
>  		tcd->soff = 16;
> @@ -650,6 +662,15 @@ static int __devinit mpc_dma_probe(struct platform_device *op,
>  		return -EINVAL;
>  	}
>  
> +	if (of_device_is_compatible(dn, "fsl,mpc8308-dma")) {
> +		mdma->is_mpc8308 = 1;
> +		mdma->irq2 = irq_of_parse_and_map(dn, 1);
> +		if (mdma->irq2 == NO_IRQ) {
> +			dev_err(dev, "Error mapping IRQ!\n");
> +			return -EINVAL;
> +		}
> +	}
> +
>  	retval = of_address_to_resource(dn, 0, &res);
>  	if (retval) {
>  		dev_err(dev, "Error parsing memory region!\n");
> @@ -680,11 +701,23 @@ static int __devinit mpc_dma_probe(struct platform_device *op,
>  		return -EINVAL;
>  	}
>  
> +	if (mdma->is_mpc8308) {
> +		retval = devm_request_irq(dev, mdma->irq2, &mpc_dma_irq, 0,
> +				DRV_NAME, mdma);
> +		if (retval) {
> +			dev_err(dev, "Error requesting IRQ2!\n");
> +			return -EINVAL;
> +		}
> +	}
> +
>  	spin_lock_init(&mdma->error_status_lock);
>  
>  	dma = &mdma->dma;
>  	dma->dev = dev;
> -	dma->chancnt = MPC_DMA_CHANNELS;
> +	if (!mdma->is_mpc8308)
> +		dma->chancnt = MPC_DMA_CHANNELS;
> +	else
> +		dma->chancnt = 16; /* MPC8308 DMA has only 16 channels */
>  	dma->device_alloc_chan_resources = mpc_dma_alloc_chan_resources;
>  	dma->device_free_chan_resources = mpc_dma_free_chan_resources;
>  	dma->device_issue_pending = mpc_dma_issue_pending;
> @@ -720,26 +753,40 @@ static int __devinit mpc_dma_probe(struct platform_device *op,
>  	 * - Round-robin group arbitration,
>  	 * - Round-robin channel arbitration.
>  	 */
> -	out_be32(&mdma->regs->dmacr, MPC_DMA_DMACR_EDCG |
> -				MPC_DMA_DMACR_ERGA | MPC_DMA_DMACR_ERCA);
> -
> -	/* Disable hardware DMA requests */
> -	out_be32(&mdma->regs->dmaerqh, 0);
> -	out_be32(&mdma->regs->dmaerql, 0);
> -
> -	/* Disable error interrupts */
> -	out_be32(&mdma->regs->dmaeeih, 0);
> -	out_be32(&mdma->regs->dmaeeil, 0);
> -
> -	/* Clear interrupts status */
> -	out_be32(&mdma->regs->dmainth, 0xFFFFFFFF);
> -	out_be32(&mdma->regs->dmaintl, 0xFFFFFFFF);
> -	out_be32(&mdma->regs->dmaerrh, 0xFFFFFFFF);
> -	out_be32(&mdma->regs->dmaerrl, 0xFFFFFFFF);
> -
> -	/* Route interrupts to IPIC */
> -	out_be32(&mdma->regs->dmaihsa, 0);
> -	out_be32(&mdma->regs->dmailsa, 0);
> +	if (!mdma->is_mpc8308) {
> +		out_be32(&mdma->regs->dmacr, MPC_DMA_DMACR_EDCG |
> +					MPC_DMA_DMACR_ERGA | MPC_DMA_DMACR_ERCA);
> +
> +		/* Disable hardware DMA requests */
> +		out_be32(&mdma->regs->dmaerqh, 0);
> +		out_be32(&mdma->regs->dmaerql, 0);
> +
> +		/* Disable error interrupts */
> +		out_be32(&mdma->regs->dmaeeih, 0);
> +		out_be32(&mdma->regs->dmaeeil, 0);
> +
> +		/* Clear interrupts status */
> +		out_be32(&mdma->regs->dmainth, 0xFFFFFFFF);
> +		out_be32(&mdma->regs->dmaintl, 0xFFFFFFFF);
> +		out_be32(&mdma->regs->dmaerrh, 0xFFFFFFFF);
> +		out_be32(&mdma->regs->dmaerrl, 0xFFFFFFFF);
> +
> +		/* Route interrupts to IPIC */
> +		out_be32(&mdma->regs->dmaihsa, 0);
> +		out_be32(&mdma->regs->dmailsa, 0);
> +	} else {
> +		/* MPC8308 has 16 channels and lacks some registers */
> +		out_be32(&mdma->regs->dmacr, MPC_DMA_DMACR_ERCA);
> +
> +		/* enable snooping */
> +		out_be32(&mdma->regs->dmagpor, MPC_DMA_DMAGPOR_SNOOP_ENABLE);
> +		/* Disable error interrupts */
> +		out_be32(&mdma->regs->dmaeeil, 0);
> +
> +		/* Clear interrupts status */
> +		out_be32(&mdma->regs->dmaintl, 0xFFFF);
> +		out_be32(&mdma->regs->dmaerrl, 0xFFFF);
> +	}
>  
>  	/* Register DMA engine */
>  	dev_set_drvdata(dev, mdma);
> -- 
> 1.7.2.3
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 4/6] mpc512x_dma: try to free descriptors in case of allocation failure

[Ilya Yanok]

Currently completed descriptors are processed in the tasklet. This can
lead to dead lock in case of CONFIG_NET_DMA enabled (new requests are
submitted from softirq context and dma_memcpy_to_iovec() busy loops until
the requests is submitted). To prevent this we should process completed
descriptors from the allocation failure path in prepare_memcpy too.

Signed-off-by: Ilya Yanok <yanok@emcraft.com>
Cc: Piotr Ziecik <kosmo@semihalf.com>
---
 drivers/dma/mpc512x_dma.c |   79 +++++++++++++++++++++++++-------------------
 1 files changed, 45 insertions(+), 34 deletions(-)

diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
index 97b92ec..59c2701 100644
--- a/drivers/dma/mpc512x_dma.c
+++ b/drivers/dma/mpc512x_dma.c
@@ -328,19 +328,55 @@ static irqreturn_t mpc_dma_irq(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
-/* DMA Tasklet */
-static void mpc_dma_tasklet(unsigned long data)
+/* proccess completed descriptors */
+static void mpc_dma_process_completed(struct mpc_dma *mdma)
 {
-	struct mpc_dma *mdma = (void *)data;
 	dma_cookie_t last_cookie = 0;
 	struct mpc_dma_chan *mchan;
 	struct mpc_dma_desc *mdesc;
 	struct dma_async_tx_descriptor *desc;
 	unsigned long flags;
 	LIST_HEAD(list);
-	uint es;
 	int i;
 
+	for (i = 0; i < mdma->dma.chancnt; i++) {
+		mchan = &mdma->channels[i];
+
+		/* Get all completed descriptors */
+		spin_lock_irqsave(&mchan->lock, flags);
+		if (!list_empty(&mchan->completed))
+			list_splice_tail_init(&mchan->completed, &list);
+		spin_unlock_irqrestore(&mchan->lock, flags);
+
+		if (list_empty(&list))
+			continue;
+
+		/* Execute callbacks and run dependencies */
+		list_for_each_entry(mdesc, &list, node) {
+			desc = &mdesc->desc;
+
+			if (desc->callback)
+				desc->callback(desc->callback_param);
+
+			last_cookie = desc->cookie;
+			dma_run_dependencies(desc);
+		}
+
+		/* Free descriptors */
+		spin_lock_irqsave(&mchan->lock, flags);
+		list_splice_tail_init(&list, &mchan->free);
+		mchan->completed_cookie = last_cookie;
+		spin_unlock_irqrestore(&mchan->lock, flags);
+	}
+}
+
+/* DMA Tasklet */
+static void mpc_dma_tasklet(unsigned long data)
+{
+	struct mpc_dma *mdma = (void *)data;
+	unsigned long flags;
+	uint es;
+
 	spin_lock_irqsave(&mdma->error_status_lock, flags);
 	es = mdma->error_status;
 	mdma->error_status = 0;
@@ -379,35 +415,7 @@ static void mpc_dma_tasklet(unsigned long data)
 			dev_err(mdma->dma.dev, "- Destination Bus Error\n");
 	}
 
-	for (i = 0; i < mdma->dma.chancnt; i++) {
-		mchan = &mdma->channels[i];
-
-		/* Get all completed descriptors */
-		spin_lock_irqsave(&mchan->lock, flags);
-		if (!list_empty(&mchan->completed))
-			list_splice_tail_init(&mchan->completed, &list);
-		spin_unlock_irqrestore(&mchan->lock, flags);
-
-		if (list_empty(&list))
-			continue;
-
-		/* Execute callbacks and run dependencies */
-		list_for_each_entry(mdesc, &list, node) {
-			desc = &mdesc->desc;
-
-			if (desc->callback)
-				desc->callback(desc->callback_param);
-
-			last_cookie = desc->cookie;
-			dma_run_dependencies(desc);
-		}
-
-		/* Free descriptors */
-		spin_lock_irqsave(&mchan->lock, flags);
-		list_splice_tail_init(&list, &mchan->free);
-		mchan->completed_cookie = last_cookie;
-		spin_unlock_irqrestore(&mchan->lock, flags);
-	}
+	mpc_dma_process_completed(mdma);
 }
 
 /* Submit descriptor to hardware */
@@ -587,8 +595,11 @@ mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t dst, dma_addr_t src,
 	}
 	spin_unlock_irqrestore(&mchan->lock, iflags);
 
-	if (!mdesc)
+	if (!mdesc) {
+		/* try to free completed descriptors */
+		mpc_dma_process_completed(mdma);
 		return NULL;
+	}
 
 	mdesc->error = 0;
 	tcd = mdesc->tcd;
-- 
1.7.2.3

Re: [PATCH 4/6] mpc512x_dma: try to free descriptors in case of allocation failure

[Grant Likely]

On Wed, Oct 27, 2010 at 01:52:58AM +0200, Ilya Yanok wrote:
> Currently completed descriptors are processed in the tasklet. This can
> lead to dead lock in case of CONFIG_NET_DMA enabled (new requests are
> submitted from softirq context and dma_memcpy_to_iovec() busy loops until
> the requests is submitted). To prevent this we should process completed
> descriptors from the allocation failure path in prepare_memcpy too.
> 
> Signed-off-by: Ilya Yanok <yanok@emcraft.com>
> Cc: Piotr Ziecik <kosmo@semihalf.com>

Merged for -next, thanks.

g.

> ---
>  drivers/dma/mpc512x_dma.c |   79 +++++++++++++++++++++++++-------------------
>  1 files changed, 45 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
> index 97b92ec..59c2701 100644
> --- a/drivers/dma/mpc512x_dma.c
> +++ b/drivers/dma/mpc512x_dma.c
> @@ -328,19 +328,55 @@ static irqreturn_t mpc_dma_irq(int irq, void *data)
>  	return IRQ_HANDLED;
>  }
>  
> -/* DMA Tasklet */
> -static void mpc_dma_tasklet(unsigned long data)
> +/* proccess completed descriptors */
> +static void mpc_dma_process_completed(struct mpc_dma *mdma)
>  {
> -	struct mpc_dma *mdma = (void *)data;
>  	dma_cookie_t last_cookie = 0;
>  	struct mpc_dma_chan *mchan;
>  	struct mpc_dma_desc *mdesc;
>  	struct dma_async_tx_descriptor *desc;
>  	unsigned long flags;
>  	LIST_HEAD(list);
> -	uint es;
>  	int i;
>  
> +	for (i = 0; i < mdma->dma.chancnt; i++) {
> +		mchan = &mdma->channels[i];
> +
> +		/* Get all completed descriptors */
> +		spin_lock_irqsave(&mchan->lock, flags);
> +		if (!list_empty(&mchan->completed))
> +			list_splice_tail_init(&mchan->completed, &list);
> +		spin_unlock_irqrestore(&mchan->lock, flags);
> +
> +		if (list_empty(&list))
> +			continue;
> +
> +		/* Execute callbacks and run dependencies */
> +		list_for_each_entry(mdesc, &list, node) {
> +			desc = &mdesc->desc;
> +
> +			if (desc->callback)
> +				desc->callback(desc->callback_param);
> +
> +			last_cookie = desc->cookie;
> +			dma_run_dependencies(desc);
> +		}
> +
> +		/* Free descriptors */
> +		spin_lock_irqsave(&mchan->lock, flags);
> +		list_splice_tail_init(&list, &mchan->free);
> +		mchan->completed_cookie = last_cookie;
> +		spin_unlock_irqrestore(&mchan->lock, flags);
> +	}
> +}
> +
> +/* DMA Tasklet */
> +static void mpc_dma_tasklet(unsigned long data)
> +{
> +	struct mpc_dma *mdma = (void *)data;
> +	unsigned long flags;
> +	uint es;
> +
>  	spin_lock_irqsave(&mdma->error_status_lock, flags);
>  	es = mdma->error_status;
>  	mdma->error_status = 0;
> @@ -379,35 +415,7 @@ static void mpc_dma_tasklet(unsigned long data)
>  			dev_err(mdma->dma.dev, "- Destination Bus Error\n");
>  	}
>  
> -	for (i = 0; i < mdma->dma.chancnt; i++) {
> -		mchan = &mdma->channels[i];
> -
> -		/* Get all completed descriptors */
> -		spin_lock_irqsave(&mchan->lock, flags);
> -		if (!list_empty(&mchan->completed))
> -			list_splice_tail_init(&mchan->completed, &list);
> -		spin_unlock_irqrestore(&mchan->lock, flags);
> -
> -		if (list_empty(&list))
> -			continue;
> -
> -		/* Execute callbacks and run dependencies */
> -		list_for_each_entry(mdesc, &list, node) {
> -			desc = &mdesc->desc;
> -
> -			if (desc->callback)
> -				desc->callback(desc->callback_param);
> -
> -			last_cookie = desc->cookie;
> -			dma_run_dependencies(desc);
> -		}
> -
> -		/* Free descriptors */
> -		spin_lock_irqsave(&mchan->lock, flags);
> -		list_splice_tail_init(&list, &mchan->free);
> -		mchan->completed_cookie = last_cookie;
> -		spin_unlock_irqrestore(&mchan->lock, flags);
> -	}
> +	mpc_dma_process_completed(mdma);
>  }
>  
>  /* Submit descriptor to hardware */
> @@ -587,8 +595,11 @@ mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t dst, dma_addr_t src,
>  	}
>  	spin_unlock_irqrestore(&mchan->lock, iflags);
>  
> -	if (!mdesc)
> +	if (!mdesc) {
> +		/* try to free completed descriptors */
> +		mpc_dma_process_completed(mdma);
>  		return NULL;
> +	}
>  
>  	mdesc->error = 0;
>  	tcd = mdesc->tcd;
> -- 
> 1.7.2.3
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 5/6] MPC8308RDB: add DMA controller device-tree node

[Ilya Yanok]

MPC8308 has DMA controller compatible with mpc512x_dma driver. This
patch adds device-tree node to support DMA controller on MPC8308RDB
board.

Signed-off-by: Ilya Yanok <yanok@emcraft.com>
---
 arch/powerpc/boot/dts/mpc8308rdb.dts |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8308rdb.dts b/arch/powerpc/boot/dts/mpc8308rdb.dts
index 1e2b888..a0bd188 100644
--- a/arch/powerpc/boot/dts/mpc8308rdb.dts
+++ b/arch/powerpc/boot/dts/mpc8308rdb.dts
@@ -265,6 +265,14 @@
 			interrupt-parent = < &ipic >;
 		};
 
+		dma@2c000 {
+			compatible = "fsl,mpc8308-dma", "fsl,mpc5121-dma";
+			reg = <0x2c000 0x1800>;
+			interrupts = <3 0x8
+					94 0x8>;
+			interrupt-parent = < &ipic >;
+		};
+
 	};
 
 	pci0: pcie@e0009000 {
-- 
1.7.2.3

Re: [PATCH 5/6] MPC8308RDB: add DMA controller device-tree node

[Grant Likely]

On Wed, Oct 27, 2010 at 01:52:59AM +0200, Ilya Yanok wrote:
> MPC8308 has DMA controller compatible with mpc512x_dma driver. This
> patch adds device-tree node to support DMA controller on MPC8308RDB
> board.
> 
> Signed-off-by: Ilya Yanok <yanok@emcraft.com>

Merged for -next, thanks.

g.

> ---
>  arch/powerpc/boot/dts/mpc8308rdb.dts |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/boot/dts/mpc8308rdb.dts b/arch/powerpc/boot/dts/mpc8308rdb.dts
> index 1e2b888..a0bd188 100644
> --- a/arch/powerpc/boot/dts/mpc8308rdb.dts
> +++ b/arch/powerpc/boot/dts/mpc8308rdb.dts
> @@ -265,6 +265,14 @@
>  			interrupt-parent = < &ipic >;
>  		};
>  
> +		dma@2c000 {
> +			compatible = "fsl,mpc8308-dma", "fsl,mpc5121-dma";
> +			reg = <0x2c000 0x1800>;
> +			interrupts = <3 0x8
> +					94 0x8>;
> +			interrupt-parent = < &ipic >;
> +		};
> +
>  	};
>  
>  	pci0: pcie@e0009000 {
> -- 
> 1.7.2.3
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 6/6] mpc8308_p1m: add DMA controller device-tree node

[Ilya Yanok]

MPC8308 has DMA controller compatible with mpc512x_dma driver. This
patch adds device-tree node to support DMA controller on MPC8308 P1M
board.

Signed-off-by: Ilya Yanok <yanok@emcraft.com>
---
 arch/powerpc/boot/dts/mpc8308_p1m.dts |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8308_p1m.dts b/arch/powerpc/boot/dts/mpc8308_p1m.dts
index 05a76cc..697b3f6 100644
--- a/arch/powerpc/boot/dts/mpc8308_p1m.dts
+++ b/arch/powerpc/boot/dts/mpc8308_p1m.dts
@@ -297,6 +297,14 @@
 			interrupt-parent = < &ipic >;
 		};
 
+		dma@2c000 {
+			compatible = "fsl,mpc8308-dma", "fsl,mpc5121-dma";
+			reg = <0x2c000 0x1800>;
+			interrupts = <3 0x8
+					94 0x8>;
+			interrupt-parent = < &ipic >;
+		};
+
 	};
 
 	pci0: pcie@e0009000 {
-- 
1.7.2.3

Re: [PATCH 6/6] mpc8308_p1m: add DMA controller device-tree node

[Grant Likely]

On Wed, Oct 27, 2010 at 01:53:00AM +0200, Ilya Yanok wrote:
> MPC8308 has DMA controller compatible with mpc512x_dma driver. This
> patch adds device-tree node to support DMA controller on MPC8308 P1M
> board.
> 
> Signed-off-by: Ilya Yanok <yanok@emcraft.com>

Merged for -next, thanks.

g.

> ---
>  arch/powerpc/boot/dts/mpc8308_p1m.dts |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/boot/dts/mpc8308_p1m.dts b/arch/powerpc/boot/dts/mpc8308_p1m.dts
> index 05a76cc..697b3f6 100644
> --- a/arch/powerpc/boot/dts/mpc8308_p1m.dts
> +++ b/arch/powerpc/boot/dts/mpc8308_p1m.dts
> @@ -297,6 +297,14 @@
>  			interrupt-parent = < &ipic >;
>  		};
>  
> +		dma@2c000 {
> +			compatible = "fsl,mpc8308-dma", "fsl,mpc5121-dma";
> +			reg = <0x2c000 0x1800>;
> +			interrupts = <3 0x8
> +					94 0x8>;
> +			interrupt-parent = < &ipic >;
> +		};
> +
>  	};
>  
>  	pci0: pcie@e0009000 {
> -- 
> 1.7.2.3
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev