[PATCH RFC 0/2] Prepare smooth PXA transition to dmaengine

[PATCH RFC 0/2] Prepare smooth PXA transition to dmaengine

[Robert Jarzmik]

This is the best I could think of to smoothly transition the PXA drivers to
dmaengine, driver by driver. The goal is to :
 - be able to port several drivers, but not _all_ of them at the same time
 - have both dmaengine and legacy dma code working in the same kernel
 - have dma chanels correctly split between both APIs

This was only compiled for now, I'll make some test shortly.

If anybody thinks of something smarter, please let me know.

Cheers.

--
Robert

Robert Jarzmik (2):
  dma: mmp_dma: add support for legacy transition
  ARM: pxa: transition to dmaengine phase 1

 arch/arm/Kconfig                     |  2 ++
 arch/arm/plat-pxa/dma.c              |  4 +++-
 arch/arm/plat-pxa/include/plat/dma.h |  1 +
 drivers/dma/mmp_pdma.c               | 26 ++++++++++++++++++++++++++
 4 files changed, 32 insertions(+), 1 deletion(-)

-- 
2.1.0

[PATCH RFC 1/2] dma: mmp_dma: add support for legacy transition

[Robert Jarzmik]

In order to achieve smooth transition of pxa drivers from old legacy dma
handling to new dmaengine, introduce a function to "hide" dma physical
channels from dmaengine.

This is temporary situation where pxa dma will be handled in 2 places :
 - arch/arm/plat-pxa/dma.c
 - drivers/dma/mmp_pdma.c

The resources, ie. dma channels, will be controlled by mmp_dma. The
legacy code will request or release a channel with
mmp_pdma_toggle_reserved_channel().

This is not very pretty, but it ensures both legacy and dmaengine
consumers can live in the same kernel until the conversion is done.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/dma/mmp_pdma.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c
index 8926f27..d4efbe1 100644
--- a/drivers/dma/mmp_pdma.c
+++ b/drivers/dma/mmp_pdma.c
@@ -232,6 +232,15 @@ static irqreturn_t mmp_pdma_int_handler(int irq, void *dev_id)
 	return IRQ_NONE;
 }
 
+/*
+ * In the transition phase where legacy pxa handling is done at the same time as
+ * mmp_dma, the DMA physical channel split between the 2 DMA providers is done
+ * through legacy_reserved. Legacy code reserves DMA channels by settings
+ * corresponding bits in legacy_reserved.
+ */
+static u32 legacy_reserved;
+static u32 legacy_unavailable;
+
 /* lookup free phy channel as descending priority */
 static struct mmp_pdma_phy *lookup_phy(struct mmp_pdma_chan *pchan)
 {
@@ -253,10 +262,14 @@ static struct mmp_pdma_phy *lookup_phy(struct mmp_pdma_chan *pchan)
 		for (i = 0; i < pdev->dma_channels; i++) {
 			if (prio != (i & 0xf) >> 2)
 				continue;
+			if ((i < 32) && (legacy_reserved & (1 << i)))
+				continue;
 			phy = &pdev->phy[i];
 			if (!phy->vchan) {
 				phy->vchan = pchan;
 				found = phy;
+				if (i < 32)
+					legacy_unavailable |= 1 << i;
 				goto out_unlock;
 			}
 		}
@@ -272,6 +285,7 @@ static void mmp_pdma_free_phy(struct mmp_pdma_chan *pchan)
 	struct mmp_pdma_device *pdev = to_mmp_pdma_dev(pchan->chan.device);
 	unsigned long flags;
 	u32 reg;
+	int i;
 
 	if (!pchan->phy)
 		return;
@@ -281,6 +295,9 @@ static void mmp_pdma_free_phy(struct mmp_pdma_chan *pchan)
 	writel(0, pchan->phy->base + reg);
 
 	spin_lock_irqsave(&pdev->phy_lock, flags);
+	for (i = 0; i < 32; i++)
+		if (pchan->phy == &pdev->phy[i])
+			legacy_unavailable &= ~(1 << i);
 	pchan->phy->vchan = NULL;
 	pchan->phy = NULL;
 	spin_unlock_irqrestore(&pdev->phy_lock, flags);
@@ -1121,6 +1138,15 @@ bool mmp_pdma_filter_fn(struct dma_chan *chan, void *param)
 }
 EXPORT_SYMBOL_GPL(mmp_pdma_filter_fn);
 
+int mmp_pdma_toggle_reserved_channel(int legacy_channel)
+{
+	if (legacy_unavailable & (1 << legacy_channel))
+		return -EBUSY;
+	legacy_reserved ^= 1 << legacy_channel;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(mmp_pdma_toggle_reserved_channel);
+
 module_platform_driver(mmp_pdma_driver);
 
 MODULE_DESCRIPTION("MARVELL MMP Peripheral DMA Driver");
-- 
2.1.0

[PATCH RFC 2/2] ARM: pxa: transition to dmaengine phase 1

[Robert Jarzmik]

In order to slowly transition pxa to dmaengine, the legacy code will now
rely on dmaengine to request a channel.

This implies that PXA architecture selects DMADEVICES and MMP_PDMA,
which is not pretty. Yet it enables PXA drivers to be ported one by one,
with part of them using dmaengine, and the other part using the legacy
code.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 arch/arm/Kconfig                     | 2 ++
 arch/arm/plat-pxa/dma.c              | 4 +++-
 arch/arm/plat-pxa/include/plat/dma.h | 1 +
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index bb358d2..cc23f2b 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -617,10 +617,12 @@ config ARCH_PXA
 	select CLKDEV_LOOKUP
 	select CLKSRC_MMIO
 	select CLKSRC_OF
+	select DMADEVICES
 	select GENERIC_CLOCKEVENTS
 	select GPIO_PXA
 	select HAVE_IDE
 	select IRQ_DOMAIN
+	select MMP_PDMA
 	select MULTI_IRQ_HANDLER
 	select PLAT_PXA
 	select SPARSE_IRQ
diff --git a/arch/arm/plat-pxa/dma.c b/arch/arm/plat-pxa/dma.c
index 054fc5a..e5094a6 100644
--- a/arch/arm/plat-pxa/dma.c
+++ b/arch/arm/plat-pxa/dma.c
@@ -294,7 +294,8 @@ int pxa_request_dma (char *name, pxa_dma_prio prio,
 		/* try grabbing a DMA channel with the requested priority */
 		for (i = 0; i < num_dma_channels; i++) {
 			if ((dma_channels[i].prio == prio) &&
-			    !dma_channels[i].name) {
+			    !dma_channels[i].name &&
+			    !mmp_pdma_toggle_reserved_channel(i)) {
 				found = 1;
 				break;
 			}
@@ -331,6 +332,7 @@ void pxa_free_dma (int dma_ch)
 	local_irq_save(flags);
 	DCSR(dma_ch) = DCSR_STARTINTR|DCSR_ENDINTR|DCSR_BUSERR;
 	dma_channels[dma_ch].name = NULL;
+	mmp_pdma_toggle_reserved_channel(dma_ch);
 	local_irq_restore(flags);
 }
 EXPORT_SYMBOL(pxa_free_dma);
diff --git a/arch/arm/plat-pxa/include/plat/dma.h b/arch/arm/plat-pxa/include/plat/dma.h
index a7b91dc..f4c6339 100644
--- a/arch/arm/plat-pxa/include/plat/dma.h
+++ b/arch/arm/plat-pxa/include/plat/dma.h
@@ -81,5 +81,6 @@ int pxa_request_dma (char *name,
 			 void *data);
 
 void pxa_free_dma (int dma_ch);
+extern int mmp_pdma_toggle_reserved_channel(int legacy_channel);
 
 #endif /* __PLAT_DMA_H */
-- 
2.1.0

Re: [PATCH RFC 2/2] ARM: pxa: transition to dmaengine phase 1

[Arnd Bergmann]

On Saturday 14 February 2015 23:47:33 Robert Jarzmik wrote:
> @@ -294,7 +294,8 @@ int pxa_request_dma (char *name, pxa_dma_prio prio,
>                 /* try grabbing a DMA channel with the requested priority */
>                 for (i = 0; i < num_dma_channels; i++) {
>                         if ((dma_channels[i].prio == prio) &&
> -                           !dma_channels[i].name) {
> +                           !dma_channels[i].name &&
> +                           !mmp_pdma_toggle_reserved_channel(i)) {
>                                 found = 1;
>                                 break;
>                         }
> 

How is the order between the two enforced? I.e. can it be that the dmaengine
driver uses the same channel for a different slave before we get here?

If this is ensured to work, I'm fine with your approach.

	Arnd

Re: [PATCH RFC 2/2] ARM: pxa: transition to dmaengine phase 1

[robert.jarzmik]

----- Mail original -----
De: "Arnd Bergmann" <arnd@arndb.de>
À: linux-arm-kernel@lists.infradead.org
Cc: "Robert Jarzmik" <robert.jarzmik@free.fr>, "Vinod Koul" <vinod.koul@intel.com>, "Olof Johansson" <olof@lixom.net>, "Daniel Mack" <zonque@gmail.com>, "Haojian Zhuang" <haojian.zhuang@gmail.com>, dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org
Envoyé: Lundi 16 Février 2015 11:28:57
Objet: Re: [PATCH RFC 2/2] ARM: pxa: transition to dmaengine phase 1

On Saturday 14 February 2015 23:47:33 Robert Jarzmik wrote:
>> @@ -294,7 +294,8 @@ int pxa_request_dma (char *name, pxa_dma_prio prio,
>>                 /* try grabbing a DMA channel with the requested priority */
>>                 for (i = 0; i < num_dma_channels; i++) {
>>                         if ((dma_channels[i].prio == prio) &&
>> -                           !dma_channels[i].name) {
>> +                           !dma_channels[i].name &&
>> +                           !mmp_pdma_toggle_reserved_channel(i)) {
>>                                 found = 1;
>>                                 break;
>>                         }
>> 

> How is the order between the two enforced? I.e. can it be that the dmaengine
> driver uses the same channel for a different slave before we get here?

If a request is made for a "low prio channel", ie. channel 8, 9 or 10 if I remember
correctly :
 - suppose dmaengine has transactions underway, and channel 8 is busy
 - this loop, for i == 8 : mmp_pdma_toggle_reserved_channel(8) -> -EBUSY
 - loop continues, i == 9 : mmp_pdma_toggle_reserved_channel(8) -> 0
   => pxa_request_dma reserves channel 9

>From now on, mmp_pdma will "skip" channel 9 from its candidates to serve requests.

> If this is ensured to work, I'm fine with your approach.
Actually it does. Not exactly this version, as the mmp_pdma interrupt was a bit
amended to "skip" also reserved channels and not steal events from legacy code,
but that will be for official submission.

It's also designed to be race free, relying on the fact that there is only one
CPU on pxa{2,3}xx SocS (irq_save covers). 

I'm testing it right now with 2 drivers :
 - pxa3xx_nand, which is converted to dmaengine
 - pxamci, which is not converted to dmaengine
In 3 variants :
 - zylonite pxa3xx board booted with device-tree
 - zylonite pxa3xx board booted in legacy
 - mioa701 board booted in device-tree

So far so good. If the mmp_pdma patch is accepted for the transition, it will
open up my path towards dmaengine conversion of all pxa drivers (mmc, nand,
pxa_camera and pxa2xx-pcm-lib). As Daniel has already done most of the work,
and because I have kept it for 2 years, that part will be less a burden than
it would seem ... only the camera will give me a small headache.

Cheers.

--
Robert

Re: [PATCH RFC 2/2] ARM: pxa: transition to dmaengine phase 1

[Arnd Bergmann]

On Monday 16 February 2015 12:12:14 robert.jarzmik@free.fr wrote:
> ----- Mail original -----
> De: "Arnd Bergmann" <arnd@arndb.de>
> À: linux-arm-kernel@lists.infradead.org
> Cc: "Robert Jarzmik" <robert.jarzmik@free.fr>, "Vinod Koul" <vinod.koul@intel.com>, "Olof Johansson" <olof@lixom.net>, "Daniel Mack" <zonque@gmail.com>, "Haojian Zhuang" <haojian.zhuang@gmail.com>, dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org
> Envoyé: Lundi 16 Février 2015 11:28:57
> Objet: Re: [PATCH RFC 2/2] ARM: pxa: transition to dmaengine phase 1
> 
> On Saturday 14 February 2015 23:47:33 Robert Jarzmik wrote:
> >> @@ -294,7 +294,8 @@ int pxa_request_dma (char *name, pxa_dma_prio prio,
> >>                 /* try grabbing a DMA channel with the requested priority */
> >>                 for (i = 0; i < num_dma_channels; i++) {
> >>                         if ((dma_channels[i].prio == prio) &&
> >> -                           !dma_channels[i].name) {
> >> +                           !dma_channels[i].name &&
> >> +                           !mmp_pdma_toggle_reserved_channel(i)) {
> >>                                 found = 1;
> >>                                 break;
> >>                         }
> >> 
> 
> > How is the order between the two enforced? I.e. can it be that the dmaengine
> > driver uses the same channel for a different slave before we get here?
> 
> If a request is made for a "low prio channel", ie. channel 8, 9 or 10 if I remember
> correctly :
>  - suppose dmaengine has transactions underway, and channel 8 is busy
>  - this loop, for i == 8 : mmp_pdma_toggle_reserved_channel(8) -> -EBUSY
>  - loop continues, i == 9 : mmp_pdma_toggle_reserved_channel(8) -> 0
>    => pxa_request_dma reserves channel 9
> 
> From now on, mmp_pdma will "skip" channel 9 from its candidates to serve requests.

Ah, so the function asks for just one channel of the right priority,
not a particular channel. I missed that detail.

> > If this is ensured to work, I'm fine with your approach.
> Actually it does. Not exactly this version, as the mmp_pdma interrupt was a bit
> amended to "skip" also reserved channels and not steal events from legacy code,
> but that will be for official submission.
> 
> It's also designed to be race free, relying on the fact that there is only one
> CPU on pxa{2,3}xx SocS (irq_save covers). 

Ok.

> I'm testing it right now with 2 drivers :
>  - pxa3xx_nand, which is converted to dmaengine
>  - pxamci, which is not converted to dmaengine
> In 3 variants :
>  - zylonite pxa3xx board booted with device-tree
>  - zylonite pxa3xx board booted in legacy
>  - mioa701 board booted in device-tree
> 
> So far so good. If the mmp_pdma patch is accepted for the transition, it will
> open up my path towards dmaengine conversion of all pxa drivers (mmc, nand,
> pxa_camera and pxa2xx-pcm-lib). As Daniel has already done most of the work,
> and because I have kept it for 2 years, that part will be less a burden than
> it would seem ... 

Ok, great!

> only the camera will give me a small headache.

Let's worry about that when all the other ones are done then ;-)

	Arnd

Re: [PATCH RFC 2/2] ARM: pxa: transition to dmaengine phase 1

[Daniel Mack]

On 02/16/2015 12:12 PM, robert.jarzmik@free.fr wrote:
> So far so good. If the mmp_pdma patch is accepted for the transition, it will
> open up my path towards dmaengine conversion of all pxa drivers (mmc, nand,
> pxa_camera and pxa2xx-pcm-lib). As Daniel has already done most of the work,
> and because I have kept it for 2 years, that part will be less a burden than
> it would seem ... only the camera will give me a small headache.

Ah, sorry, missed this part of the thread. Good to know you're on it :)


Thanks,
Daniel

Re: [PATCH RFC 2/2] ARM: pxa: transition to dmaengine phase 1

[Vasily Khoruzhick]

On Sun, Feb 15, 2015 at 1:47 AM, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> In order to slowly transition pxa to dmaengine, the legacy code will now
> rely on dmaengine to request a channel.

Hi Robert,

What about dropping old PXA DMA code completely? Daniel Mack did port
for most of PXA drivers to dma engine,
I've rebased his patches against 3.17 several months ago and fixed
oopses in pxamci and asoc drivers, but I didn't resubmit whole series
due to lack of time.

My 3.17 tree is at [1], I've tested it on pxa270 machine (Zipit Z2),
and everything works fine so far. I guess it won't be too much work to
rebase it against linux-3.20.

Regards,
Vasily

[1] https://github.com/anarsoul/linux-2.6/tree/v3.17-pxa-dmaengine-wip

Re: [PATCH RFC 2/2] ARM: pxa: transition to dmaengine phase 1

[Daniel Mack]

On 02/16/2015 12:14 PM, Vasily Khoruzhick wrote:
> What about dropping old PXA DMA code completely? Daniel Mack did port
> for most of PXA drivers to dma engine,
> I've rebased his patches against 3.17 several months ago and fixed
> oopses in pxamci and asoc drivers, but I didn't resubmit whole series
> due to lack of time.

Yes, I did the same locally.

> My 3.17 tree is at [1], I've tested it on pxa270 machine (Zipit Z2),
> and everything works fine so far. I guess it won't be too much work to
> rebase it against linux-3.20.

Rebasing is easy, but there's more to that. We should at least try to
convert all drivers over to dmaengine, otherwise they will just stop
working.

The biggest problem that I currently see is the pxa camera driver, which
also uses an advanced way of descriptor hot-linking at runtime. Last
time I checked, this wasn't possible to achieve with the dma-slave API.
Hence, this driver might need to be completely rewritten, which only
someone with such hardware at hand can work on. Maybe the cyclic DMA
support in mmp-dma already suffices for that. Robert, weren't you
already working on that?


Thanks,
Daniel

Re: [PATCH RFC 2/2] ARM: pxa: transition to dmaengine phase 1

[Robert Jarzmik]

Vasily Khoruzhick <anarsoul@gmail.com> writes:

> On Sun, Feb 15, 2015 at 1:47 AM, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
>> In order to slowly transition pxa to dmaengine, the legacy code will now
>> rely on dmaengine to request a channel.
>
> Hi Robert,
>
> What about dropping old PXA DMA code completely? Daniel Mack did port
> for most of PXA drivers to dma engine,
> I've rebased his patches against 3.17 several months ago and fixed
> oopses in pxamci and asoc drivers, but I didn't resubmit whole series
> due to lack of time.

Well, it's the last step, yes.
But I want a "smooth transition" : if amongst the ported drivers one starts to
bug, I want to be able to revert _only_ that driver port to dmaengine, and not
_all_ the drivers.

That's the rationale of this patch :
 - phase 1 : enable peacefull coexistence of legacy pxa_request_dma() and
             dmaengine for pxa, for both devicetree and legacy platforms
 - phase 2 : port the drivers, and ensure the work correctly
             This might take a couple of cycles
             Note that phase 1 ensures that submissions can go through each
             maintainer's tree without need for strong consistence.
 - phase 3 : revert the mmp_pdma patch, and drop arch/arm/plat-pxa/dma.c

> My 3.17 tree is at [1], I've tested it on pxa270 machine (Zipit Z2),
> and everything works fine so far. I guess it won't be too much work to
> rebase it against linux-3.20.
Oh, do you volunteer ? That would indeed ease up my burden. I only rebased
pxa3xx_nand, so any help to submit and push is welcome. At least I can commit to
review, and I would concentrate on pxa_camera.c in the meantime.

Cheers.

-- 
Robert

Re: [PATCH RFC 2/2] ARM: pxa: transition to dmaengine phase 1

[Vasily Khoruzhick]

On Mon, Feb 16, 2015 at 7:54 PM, Robert Jarzmik <robert.jarzmik@free.fr> wrote:

> Oh, do you volunteer ? That would indeed ease up my burden. I only rebased
> pxa3xx_nand, so any help to submit and push is welcome. At least I can commit to
> review, and I would concentrate on pxa_camera.c in the meantime.

I can rebase those patches on-top of Linus' tree and make sure they
work on my machine.
I can test pxamci, pxa2xx-spi and pxa2xx-pcm drivers. But I don't
think that I have enough time now to submit it and
go through review process.

Btw, where I can find your git tree?

Regards,
Vasily

> Cheers.
>
> --
> Robert

Re: [PATCH RFC 2/2] ARM: pxa: transition to dmaengine phase 1

[Robert Jarzmik]

Vasily Khoruzhick <anarsoul@gmail.com> writes:

> On Mon, Feb 16, 2015 at 7:54 PM, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
>
>> Oh, do you volunteer ? That would indeed ease up my burden. I only rebased
>> pxa3xx_nand, so any help to submit and push is welcome. At least I can commit to
>> review, and I would concentrate on pxa_camera.c in the meantime.
>
> I can rebase those patches on-top of Linus' tree and make sure they
> work on my machine.
> I can test pxamci, pxa2xx-spi and pxa2xx-pcm drivers. But I don't
> think that I have enough time now to submit it and
> go through review process.
Do your best, I'll take over when you'll meet your load limit.

> Btw, where I can find your git tree?
Here :
git fetch https://github.com/rjarzmik/linux.git work/clocks-pxa

Cheers.

-- 
Robert

[PATCH v2 1/2] dma: mmp_dma: add support for legacy transition

[Robert Jarzmik]

In order to achieve smooth transition of pxa drivers from old legacy dma
handling to new dmaengine, introduce a function to "hide" dma physical
channels from dmaengine.

This is temporary situation where pxa dma will be handled in 2 places :
 - arch/arm/plat-pxa/dma.c
 - drivers/dma/mmp_pdma.c

The resources, ie. dma channels, will be controlled by mmp_dma. The
legacy code will request or release a channel with
mmp_pdma_toggle_reserved_channel().

This is not very pretty, but it ensures both legacy and dmaengine
consumers can live in the same kernel until the conversion is done.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
Since v1: fix irq handler to not touch legacy reserved dma interrupts
---
 drivers/dma/mmp_pdma.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c
index 8926f27..1b82bd6 100644
--- a/drivers/dma/mmp_pdma.c
+++ b/drivers/dma/mmp_pdma.c
@@ -198,6 +198,15 @@ static int clear_chan_irq(struct mmp_pdma_phy *phy)
 	return 0;
 }
 
+/*
+ * In the transition phase where legacy pxa handling is done at the same time as
+ * mmp_dma, the DMA physical channel split between the 2 DMA providers is done
+ * through legacy_reserved. Legacy code reserves DMA channels by settings
+ * corresponding bits in legacy_reserved.
+ */
+static u32 legacy_reserved;
+static u32 legacy_unavailable;
+
 static irqreturn_t mmp_pdma_chan_handler(int irq, void *dev_id)
 {
 	struct mmp_pdma_phy *phy = dev_id;
@@ -221,6 +230,8 @@ static irqreturn_t mmp_pdma_int_handler(int irq, void *dev_id)
 		i = __ffs(dint);
 		dint &= (dint - 1);
 		phy = &pdev->phy[i];
+		if ((i < 32) && (legacy_reserved & (1 << i)))
+			continue;
 		ret = mmp_pdma_chan_handler(irq, phy);
 		if (ret == IRQ_HANDLED)
 			irq_num++;
@@ -253,10 +264,14 @@ static struct mmp_pdma_phy *lookup_phy(struct mmp_pdma_chan *pchan)
 		for (i = 0; i < pdev->dma_channels; i++) {
 			if (prio != (i & 0xf) >> 2)
 				continue;
+			if ((i < 32) && (legacy_reserved & (1 << i)))
+				continue;
 			phy = &pdev->phy[i];
 			if (!phy->vchan) {
 				phy->vchan = pchan;
 				found = phy;
+				if (i < 32)
+					legacy_unavailable |= (1 << i);
 				goto out_unlock;
 			}
 		}
@@ -272,6 +287,7 @@ static void mmp_pdma_free_phy(struct mmp_pdma_chan *pchan)
 	struct mmp_pdma_device *pdev = to_mmp_pdma_dev(pchan->chan.device);
 	unsigned long flags;
 	u32 reg;
+	int i;
 
 	if (!pchan->phy)
 		return;
@@ -281,6 +297,9 @@ static void mmp_pdma_free_phy(struct mmp_pdma_chan *pchan)
 	writel(0, pchan->phy->base + reg);
 
 	spin_lock_irqsave(&pdev->phy_lock, flags);
+	for (i = 0; i < 32; i++)
+		if (pchan->phy == &pdev->phy[i])
+			legacy_unavailable &= ~(1 << i);
 	pchan->phy->vchan = NULL;
 	pchan->phy = NULL;
 	spin_unlock_irqrestore(&pdev->phy_lock, flags);
@@ -1121,6 +1140,15 @@ bool mmp_pdma_filter_fn(struct dma_chan *chan, void *param)
 }
 EXPORT_SYMBOL_GPL(mmp_pdma_filter_fn);
 
+int mmp_pdma_toggle_reserved_channel(int legacy_channel)
+{
+	if (legacy_unavailable & (1 << legacy_channel))
+		return -EBUSY;
+	legacy_reserved ^= 1 << legacy_channel;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(mmp_pdma_toggle_reserved_channel);
+
 module_platform_driver(mmp_pdma_driver);
 
 MODULE_DESCRIPTION("MARVELL MMP Peripheral DMA Driver");
-- 
2.1.0

[PATCH v2 2/2] ARM: pxa: transition to dmaengine phase 1

[Robert Jarzmik]

In order to slowly transition pxa to dmaengine, the legacy code will now
rely on dmaengine to request a channel.

This implies that PXA architecture selects DMADEVICES and MMP_PDMA,
which is not pretty. Yet it enables PXA drivers to be ported one by one,
with part of them using dmaengine, and the other part using the legacy
code.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>

---
Since v1: removed Kconfig selects
          added the mmp_pdma platform devices for legacy platforms
	  fixed the legacy dma interrupt handler to cooperate with mmp_pdma
---
 arch/arm/mach-pxa/devices.c          | 37 ++++++++++++++++++++++++++++++++++++
 arch/arm/mach-pxa/pxa25x.c           |  1 +
 arch/arm/mach-pxa/pxa27x.c           |  1 +
 arch/arm/mach-pxa/pxa3xx.c           |  1 +
 arch/arm/plat-pxa/dma.c              | 22 ++++++++++-----------
 arch/arm/plat-pxa/include/plat/dma.h | 15 +++++++++++++++
 6 files changed, 66 insertions(+), 11 deletions(-)

diff --git a/arch/arm/mach-pxa/devices.c b/arch/arm/mach-pxa/devices.c
index 3543466..7b7161a 100644
--- a/arch/arm/mach-pxa/devices.c
+++ b/arch/arm/mach-pxa/devices.c
@@ -17,6 +17,7 @@
 #include <linux/platform_data/camera-pxa.h>
 #include <mach/audio.h>
 #include <mach/hardware.h>
+#include <linux/platform_data/mmp_dma.h>
 #include <linux/platform_data/mtd-nand-pxa3xx.h>
 
 #include "devices.h"
@@ -1193,3 +1194,39 @@ void __init pxa2xx_set_spi_info(unsigned id, struct pxa2xx_spi_master *info)
 	pd->dev.platform_data = info;
 	platform_device_add(pd);
 }
+
+static struct mmp_dma_platdata mmp_pdma_pdata = {
+	.dma_channels	= 0,
+};
+
+static struct resource mmp_pdma_resource[] = {
+	[0] = {
+		.start	= 0x40000000,
+		.end	= 0x4000ffff,
+		.flags	= IORESOURCE_MEM,
+	},
+	[1] = {
+		.start	= IRQ_DMA,
+		.end	= IRQ_DMA,
+		.flags	= IORESOURCE_IRQ,
+	},
+};
+
+static u64 pxadma_dmamask = 0xffffffffUL;
+
+static struct platform_device pxa2xx_mmp_dma = {
+	.name		= "mmp-pdma",
+	.id		= 0,
+	.dev		= {
+		.dma_mask = &pxadma_dmamask,
+		.coherent_dma_mask = 0xffffffff,
+	},
+	.num_resources	= ARRAY_SIZE(mmp_pdma_resource),
+	.resource	= mmp_pdma_resource,
+};
+
+void __init pxa2xx_set_dmac_info(int nb_channels)
+{
+	mmp_pdma_pdata.dma_channels = nb_channels;
+	return pxa_register_device(&pxa2xx_mmp_dma, &mmp_pdma_pdata);
+}
diff --git a/arch/arm/mach-pxa/pxa25x.c b/arch/arm/mach-pxa/pxa25x.c
index 23a90c6..1dc85ff 100644
--- a/arch/arm/mach-pxa/pxa25x.c
+++ b/arch/arm/mach-pxa/pxa25x.c
@@ -206,6 +206,7 @@ static int __init pxa25x_init(void)
 		register_syscore_ops(&pxa_irq_syscore_ops);
 		register_syscore_ops(&pxa2xx_mfp_syscore_ops);
 
+		pxa2xx_set_dmac_info(16);
 		pxa_register_device(&pxa25x_device_gpio, &pxa25x_gpio_info);
 		ret = platform_add_devices(pxa25x_devices,
 					   ARRAY_SIZE(pxa25x_devices));
diff --git a/arch/arm/mach-pxa/pxa27x.c b/arch/arm/mach-pxa/pxa27x.c
index b5abdeb..e6aae9e 100644
--- a/arch/arm/mach-pxa/pxa27x.c
+++ b/arch/arm/mach-pxa/pxa27x.c
@@ -310,6 +310,7 @@ static int __init pxa27x_init(void)
 		if (!of_have_populated_dt()) {
 			pxa_register_device(&pxa27x_device_gpio,
 					    &pxa27x_gpio_info);
+			pxa2xx_set_dmac_info(32);
 			ret = platform_add_devices(devices,
 						   ARRAY_SIZE(devices));
 		}
diff --git a/arch/arm/mach-pxa/pxa3xx.c b/arch/arm/mach-pxa/pxa3xx.c
index bd4cbef..aa85ec1 100644
--- a/arch/arm/mach-pxa/pxa3xx.c
+++ b/arch/arm/mach-pxa/pxa3xx.c
@@ -431,6 +431,7 @@ static int __init pxa3xx_init(void)
 		if (of_have_populated_dt())
 			return 0;
 
+		pxa2xx_set_dmac_info(32);
 		ret = platform_add_devices(devices, ARRAY_SIZE(devices));
 		if (ret)
 			return ret;
diff --git a/arch/arm/plat-pxa/dma.c b/arch/arm/plat-pxa/dma.c
index 054fc5a..67a2b1f 100644
--- a/arch/arm/plat-pxa/dma.c
+++ b/arch/arm/plat-pxa/dma.c
@@ -294,7 +294,8 @@ int pxa_request_dma (char *name, pxa_dma_prio prio,
 		/* try grabbing a DMA channel with the requested priority */
 		for (i = 0; i < num_dma_channels; i++) {
 			if ((dma_channels[i].prio == prio) &&
-			    !dma_channels[i].name) {
+			    !dma_channels[i].name &&
+			    !mmp_pdma_toggle_reserved_channel(i)) {
 				found = 1;
 				break;
 			}
@@ -331,13 +332,14 @@ void pxa_free_dma (int dma_ch)
 	local_irq_save(flags);
 	DCSR(dma_ch) = DCSR_STARTINTR|DCSR_ENDINTR|DCSR_BUSERR;
 	dma_channels[dma_ch].name = NULL;
+	mmp_pdma_toggle_reserved_channel(dma_ch);
 	local_irq_restore(flags);
 }
 EXPORT_SYMBOL(pxa_free_dma);
 
 static irqreturn_t dma_irq_handler(int irq, void *dev_id)
 {
-	int i, dint = DINT;
+	int i, dint = DINT, done = 0;
 	struct dma_channel *channel;
 
 	while (dint) {
@@ -346,16 +348,13 @@ static irqreturn_t dma_irq_handler(int irq, void *dev_id)
 		channel = &dma_channels[i];
 		if (channel->name && channel->irq_handler) {
 			channel->irq_handler(i, channel->data);
-		} else {
-			/*
-			 * IRQ for an unregistered DMA channel:
-			 * let's clear the interrupts and disable it.
-			 */
-			printk (KERN_WARNING "spurious IRQ for DMA channel %d\n", i);
-			DCSR(i) = DCSR_STARTINTR|DCSR_ENDINTR|DCSR_BUSERR;
+			done++;
 		}
 	}
-	return IRQ_HANDLED;
+	if (done)
+		return IRQ_HANDLED;
+	else
+		return IRQ_NONE;
 }
 
 int __init pxa_init_dma(int irq, int num_ch)
@@ -377,7 +376,8 @@ int __init pxa_init_dma(int irq, int num_ch)
 		spin_lock_init(&dma_channels[i].lock);
 	}
 
-	ret = request_irq(irq, dma_irq_handler, 0, "DMA", NULL);
+	ret = request_irq(irq, dma_irq_handler, IRQF_SHARED, "DMA",
+			  dma_channels);
 	if (ret) {
 		printk (KERN_CRIT "Wow!  Can't register IRQ for DMA\n");
 		kfree(dma_channels);
diff --git a/arch/arm/plat-pxa/include/plat/dma.h b/arch/arm/plat-pxa/include/plat/dma.h
index a7b91dc..f6f1acb 100644
--- a/arch/arm/plat-pxa/include/plat/dma.h
+++ b/arch/arm/plat-pxa/include/plat/dma.h
@@ -82,4 +82,19 @@ int pxa_request_dma (char *name,
 
 void pxa_free_dma (int dma_ch);
 
+/*
+ * Cooperation with mmp_pdma + dmaengine while there remains at least one pxa
+ * driver not converted to dmaengine.
+ */
+#if defined(CONFIG_MMP_PDMA)
+extern int mmp_pdma_toggle_reserved_channel(int legacy_channel);
+#else
+static inline mmp_pdma_toggle_reserved_channel(int legacy_channel)
+{
+	return 0;
+}
+#endif
+
+extern void __init pxa2xx_set_dmac_info(int nb_channels);
+
 #endif /* __PLAT_DMA_H */
-- 
2.1.0

Re: [PATCH v2 1/2] dma: mmp_dma: add support for legacy transition

[Daniel Mack]

Hi Robert,

Thanks for pushing this topic :)

On 02/17/2015 09:39 PM, Robert Jarzmik wrote:
> In order to achieve smooth transition of pxa drivers from old legacy dma
> handling to new dmaengine, introduce a function to "hide" dma physical
> channels from dmaengine.
> 
> This is temporary situation where pxa dma will be handled in 2 places :
>  - arch/arm/plat-pxa/dma.c
>  - drivers/dma/mmp_pdma.c
> 
> The resources, ie. dma channels, will be controlled by mmp_dma. The
> legacy code will request or release a channel with
> mmp_pdma_toggle_reserved_channel().
> 
> This is not very pretty, but it ensures both legacy and dmaengine
> consumers can live in the same kernel until the conversion is done.

I like the approach. I actually thought the legacy DMA driver would
claim the io port range, which would have made the two drivers mutually
exclusive. But it doesn't, so this can in fact work, even though it's
really not pretty. Also, it's limited to one single instance of the
mmp-pdma driver, but that reflects reality, so I'm fine.

One minor nit:

> +int mmp_pdma_toggle_reserved_channel(int legacy_channel)
> +{
> +	if (legacy_unavailable & (1 << legacy_channel))
> +		return -EBUSY;
> +	legacy_reserved ^= 1 << legacy_channel;
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(mmp_pdma_toggle_reserved_channel);

My concern is that if pxa_request_dma() is called more than once for
whatever reason by a legacy implementation, the toggled bit mask might
get out of sync. As we know exactly on the caller site what we want to
achieve, let's make the API explicit with something like:

int mmp_pdma_set_reserved_channel(int legacy_channel, bool reserved)


Thanks,
Daniel

Re: [PATCH v2 1/2] dma: mmp_dma: add support for legacy transition

[Robert Jarzmik]

Daniel Mack <daniel@zonque.org> writes:

> Hi Robert,
>
> Thanks for pushing this topic :)
> One minor nit:
>
>> +int mmp_pdma_toggle_reserved_channel(int legacy_channel)
>> +{
>> +	if (legacy_unavailable & (1 << legacy_channel))
>> +		return -EBUSY;
>> +	legacy_reserved ^= 1 << legacy_channel;
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(mmp_pdma_toggle_reserved_channel);
>
> My concern is that if pxa_request_dma() is called more than once for
> whatever reason by a legacy implementation, the toggled bit mask might
> get out of sync.
This is not possible.
The first call to pxa_request_dma() sets dma_channels[i].name to a non-NULL
value.
The second call to pxa_request_dma() cannot take the same i as
!dma_channels[i].name is not fullfilled.

> As we know exactly on the caller site what we want to achieve, let's make the
> API explicit with something like:
>
> int mmp_pdma_set_reserved_channel(int legacy_channel, bool reserved)
Even if I have no strong opinion about it, I'll let the patch as it is, unless
you really want me to add the reserved parameter, in which case I'll release a
v3.

Cheers.

-- 
Robert