Skip to contentSkip to conversation

DMA engine conversion for omap_hsmmc

RFC New Russell King - ARM Linux 10 participants 60 messages Apr 18, 2012 – Apr 27, 2012

Subsystems (9)
lore
Share & tools
lore
#0 lore raw

cover DMA engine conversion for omap_hsmmc

This series is the preliminary conversion of OMAPs private DMA API to DMAengine. It focuses on the OMAP HSMMC driver, as I believe that is the only DMA-capable driver which is used on the platforms I have available (3430LDP and 4430SDP).

For the OMAP DMAengine driver, there's a few short-comings:

1. pause/resume support is not implemented; it's not clear whether the OMAP hardware is capable of supporting this sanely.

2. status function does not return DMA residue.

3. DMAengine holds on to physical DMA channels while the driver maintains a reference to the virtual DMA engine channel. This is not an issue with drivers; drivers should be able to hold a DMA channel for their lifetime. DMAengine code could be improved to dynamically allocate physical channels, but this should happen when more drivers have been converted.

I took the approach with omap_hsmmc to add DMAengine support to the driver along side the existing DMA support, so that I had a stage where I could switch between the two implementations for comparisons - and also to allow me to run with read using DMAengine support, and write using the private API. This allowed a progressive switch over to using DMAengine, and allowed debugging without destroying the contents of the MMC card.

A point about DMAengine and DMA API for anyone who contemplates converting other drivers: the struct device to be used for mapping/unmapping/coherent memory is the DMAengine's device structure (channel->device->dev) not the peripheral (eg, MMC) device structure. It's the DMAengine which is performing the DMA to memory, not the peripheral device. The peripheral device is merely receiving the data from the DMAengine, and does not care where that data has come from. So, dma masks for peripheral devices are meaningless (and wrong.)

#1 lore raw

PATCH 1/8 ARM: OMAP: fix DMA vs memory ordering

Using coherent DMA memory with the OMAP DMA engine results in unpredictable behaviour due to memory ordering issues; as things stand, there is no guarantee that data written to coherent DMA memory will be visible to the DMA hardware.

This is because the OMAP dma_write() accessor contains no barriers, necessary on ARMv6 and above. The effect of this can be seen in comments in the OMAP serial driver, which incorrectly talks about cache flushing for the coherent DMA stuff.

Rather than adding barriers to the accessors, add it in the DMA support code just before we enable DMA, and just after we disable DMA. This avoids having barriers for every DMA register access.

---
 arch/arm/plat-omap/dma.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)
arch/arm/plat-omap/dma.c +14 -0
index ecdb3da..c58d896 100644
@@ -916,6 +916,13 @@ void omap_start_dma(int lch)
 			l |= OMAP_DMA_CCR_BUFFERING_DISABLE;
 	l |= OMAP_DMA_CCR_EN;
 
+	/*
+	 * As dma_write() uses IO accessors which are weakly ordered, there
+	 * is no guarantee that data in coherent DMA memory will be visible
+	 * to the DMA device.  Add a memory barrier here to ensure that any
+	 * such data is visible prior to enabling DMA.
+	 */
+	mb();
 	p->dma_write(l, CCR, lch);
 
 	dma_chan[lch].flags |= OMAP_DMA_ACTIVE;
@@ -965,6 +972,13 @@ void omap_stop_dma(int lch)
 		p->dma_write(l, CCR, lch);
 	}
 
+	/*
+	 * Ensure that data transferred by DMA is visible to any access
+	 * after DMA has been disabled.  This is important for coherent
+	 * DMA regions.
+	 */
+	mb();
+
 	if (!omap_dma_in_1510_mode() && dma_chan[lch].next_lch != -1) {
 		int next_lch, cur_lch = lch;
 		char dma_chan_link_map[dma_lch_count];
Signature · 1 line
1.7.4.4
#2 lore raw

PATCH 1/8 ARM: OMAP: fix DMA vs memory ordering

Quoted 10 lines · Russell King

Using coherent DMA memory with the OMAP DMA engine results in unpredictable behaviour due to memory ordering issues; as things stand, there is no guarantee that data written to coherent DMA memory will be visible to the DMA hardware.

This is because the OMAP dma_write() accessor contains no barriers, necessary on ARMv6 and above. The effect of this can be seen in comments in the OMAP serial driver, which incorrectly talks about cache flushing for the coherent DMA stuff.

Quoted 5 lines

Rather than adding barriers to the accessors, add it in the DMA support code just before we enable DMA, and just after we disable DMA. This avoids having barriers for every DMA register access.

cool, should this go to stable too ?

Signature · 8 lines
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120418/e0c8a6c0/attachment.sig>
#3 lore raw

PATCH 1/8 ARM: OMAP: fix DMA vs memory ordering

Quoted 11 lines · Felipe Balbi
Quoted 10 lines · Russell King

Using coherent DMA memory with the OMAP DMA engine results in unpredictable behaviour due to memory ordering issues; as things stand, there is no guarantee that data written to coherent DMA memory will be visible to the DMA hardware.

This is because the OMAP dma_write() accessor contains no barriers, necessary on ARMv6 and above. The effect of this can be seen in comments in the OMAP serial driver, which incorrectly talks about cache flushing for the coherent DMA stuff.

Quoted 7 lines
Quoted 5 lines

Rather than adding barriers to the accessors, add it in the DMA support code just before we enable DMA, and just after we disable DMA. This avoids having barriers for every DMA register access.

cool, should this go to stable too ?

Has anyone seen a problem (other than me when trying to get DMA engine working with omap-serial transmit paths) ?

#4 lore raw

PATCH 1/8 ARM: OMAP: fix DMA vs memory ordering

Quoted 16 lines · Russell King - ARM Linux
Quoted 15 lines · Felipe Balbi
Quoted 14 lines · Russell King

Using coherent DMA memory with the OMAP DMA engine results in unpredictable behaviour due to memory ordering issues; as things stand, there is no guarantee that data written to coherent DMA memory will be visible to the DMA hardware.

This is because the OMAP dma_write() accessor contains no barriers, necessary on ARMv6 and above. The effect of this can be seen in comments in the OMAP serial driver, which incorrectly talks about cache flushing for the coherent DMA stuff.

Rather than adding barriers to the accessors, add it in the DMA support code just before we enable DMA, and just after we disable DMA. This avoids having barriers for every DMA register access.

Quoted 6 lines
Quoted 3 lines
Quoted 1 line

cool, should this go to stable too ?

Has anyone seen a problem (other than me when trying to get DMA engine working with omap-serial transmit paths) ?

probably not since omap-serial isn't using DMA anyway. Nevermind

Signature · 8 lines
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120418/1da5c157/attachment.sig>
#5 lore raw

PATCH 1/8 ARM: OMAP: fix DMA vs memory ordering

Quoted 5 lines · Felipe Balbi
Quoted 2 lines · Russell King - ARM Linux

Has anyone seen a problem (other than me when trying to get DMA engine working with omap-serial transmit paths) ?

probably not since omap-serial isn't using DMA anyway. Nevermind

... which is a good thing because the driver is rather broken for DMA support (take a look at what the set_termios() method does, and consider what happens if that function is called when there's DMA activity outstanding.)

I've come to the conclusion that it's not worth the effort to try to convert omap-serial to DMAengine support. Firstly, DMA support requires hardware assisted flow control to work - which it currently doesn't for many many reasons. Not only because of wrong defines, but also because the programming examples in the OMAP4430 TRM is wrong. You'd do much better going to the TI16C752B data sheet and seeing how the original IP was described. Why TI didn't include a copy of that in the TRM...

Secondly, the problem I mention above wrt set_termios(), with the flow control fixed, results in a nice race condition causing the TX DMA to write into registers other than the FIFO data register, corrupting the UART configuration and halting system boot.

(I have a series of patches to fix the flow control issues, and this requires some work outside of the omap-serial driver to achieve that. I'll post those patches when they're fully ready. At the moment, it's probably better to rip out all the hardware assisted flow control code from omap-serial so that we fall back on the standard software driven flow control.)

maintainer
#6 lore raw

PATCH 1/8 ARM: OMAP: fix DMA vs memory ordering

In reply to Russell King
Quoted 10 lines · Russell King

Using coherent DMA memory with the OMAP DMA engine results in unpredictable behaviour due to memory ordering issues; as things stand, there is no guarantee that data written to coherent DMA memory will be visible to the DMA hardware.

This is because the OMAP dma_write() accessor contains no barriers, necessary on ARMv6 and above. The effect of this can be seen in comments in the OMAP serial driver, which incorrectly talks about cache flushing for the coherent DMA stuff.

Quoted 5 lines

Rather than adding barriers to the accessors, add it in the DMA support code just before we enable DMA, and just after we disable DMA. This avoids having barriers for every DMA register access.

#7 lore raw

PATCH 1/8 ARM: OMAP: fix DMA vs memory ordering

Quoted 11 lines · Tony Lindgren
Quoted 10 lines · Russell King

Using coherent DMA memory with the OMAP DMA engine results in unpredictable behaviour due to memory ordering issues; as things stand, there is no guarantee that data written to coherent DMA memory will be visible to the DMA hardware.

This is because the OMAP dma_write() accessor contains no barriers, necessary on ARMv6 and above. The effect of this can be seen in comments in the OMAP serial driver, which incorrectly talks about cache flushing for the coherent DMA stuff.

Quoted 7 lines
Quoted 5 lines

Rather than adding barriers to the accessors, add it in the DMA support code just before we enable DMA, and just after we disable DMA. This avoids having barriers for every DMA register access.

Would you like me to push this upstream on my next -rc push (with a stable tag?)

maintainer
#8 lore raw

PATCH 1/8 ARM: OMAP: fix DMA vs memory ordering

Quoted 16 lines · Russell King - ARM Linux
Quoted 15 lines · Tony Lindgren
Quoted 14 lines · Russell King

Using coherent DMA memory with the OMAP DMA engine results in unpredictable behaviour due to memory ordering issues; as things stand, there is no guarantee that data written to coherent DMA memory will be visible to the DMA hardware.

This is because the OMAP dma_write() accessor contains no barriers, necessary on ARMv6 and above. The effect of this can be seen in comments in the OMAP serial driver, which incorrectly talks about cache flushing for the coherent DMA stuff.

Rather than adding barriers to the accessors, add it in the DMA support code just before we enable DMA, and just after we disable DMA. This avoids having barriers for every DMA register access.

Quoted 6 lines
Quoted 3 lines
Quoted 1 line

Would you like me to push this upstream on my next -rc push (with a stable tag?)

Yes sounds good to me.

Regards,

Tony

#9 lore raw

PATCH 1/8 ARM: OMAP: fix DMA vs memory ordering

Quoted 17 lines · Tony Lindgren
Quoted 16 lines · Russell King - ARM Linux
Quoted 15 lines · Tony Lindgren
Quoted 14 lines · Russell King

Using coherent DMA memory with the OMAP DMA engine results in unpredictable behaviour due to memory ordering issues; as things stand, there is no guarantee that data written to coherent DMA memory will be visible to the DMA hardware.

This is because the OMAP dma_write() accessor contains no barriers, necessary on ARMv6 and above. ?The effect of this can be seen in comments in the OMAP serial driver, which incorrectly talks about cache flushing for the coherent DMA stuff.

Rather than adding barriers to the accessors, add it in the DMA support code just before we enable DMA, and just after we disable DMA. ?This avoids having barriers for every DMA register access.

Quoted 9 lines
Quoted 6 lines
Quoted 3 lines
Quoted 1 line

Would you like me to push this upstream on my next -rc push (with a stable tag?)

Yes sounds good to me.

FWIW,

#10 lore raw

PATCH 2/8 dmaengine: amba-pl08x: ensure physical channels are properly held

In reply to Russell King - ARM Linux

Ensure that physical channels are held while there are descriptors for them to process. This is needed when we split the pending queue into separate pending and issued queues.

---
 drivers/dma/amba-pl08x.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)
drivers/dma/amba-pl08x.c +6 -4
index c301a8e..2e74cca 100644
@@ -855,6 +855,8 @@ static int prep_phy_channel(struct pl08x_dma_chan *plchan,
 	struct pl08x_phy_chan *ch;
 	int ret;
 
+	plchan->phychan_hold++;
+
 	/* Check if we already have a channel */
 	if (plchan->phychan) {
 		ch = plchan->phychan;
@@ -863,6 +865,7 @@ static int prep_phy_channel(struct pl08x_dma_chan *plchan,
 
 	ch = pl08x_get_phy_channel(pl08x, plchan);
 	if (!ch) {
+		plchan->phychan_hold--;
 		/* No physical channel available, cope with it */
 		dev_dbg(&pl08x->adev->dev, "no physical channel available for xfer on %s\n", plchan->name);
 		return -EBUSY;
@@ -876,6 +879,7 @@ static int prep_phy_channel(struct pl08x_dma_chan *plchan,
 	if (plchan->slave && pl08x->pd->get_signal) {
 		ret = pl08x->pd->get_signal(plchan);
 		if (ret < 0) {
+			plchan->phychan_hold--;
 			dev_dbg(&pl08x->adev->dev,
 				"unable to use physical channel %d for transfer on %s due to platform restrictions\n",
 				ch->id, plchan->name);
@@ -899,8 +903,6 @@ static int prep_phy_channel(struct pl08x_dma_chan *plchan,
 	else if (txd->direction == DMA_DEV_TO_MEM)
 		txd->ccfg |= ch->signal << PL080_CONFIG_SRC_SEL_SHIFT;
 
-	plchan->phychan_hold++;
-
 	return 0;
 }
 
@@ -938,8 +940,6 @@ static dma_cookie_t pl08x_tx_submit(struct dma_async_tx_descriptor *tx)
 		/* Do this memcpy whenever there is a channel ready */
 		plchan->state = PL08X_CHAN_WAITING;
 		plchan->waiting = txd;
-	} else {
-		plchan->phychan_hold--;
 	}
 
 	spin_unlock_irqrestore(&plchan->lock, flags);
@@ -1428,6 +1428,7 @@ static int pl08x_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
 			 * Mark physical channel as free and free any slave
 			 * signal
 			 */
+			plchan->phychan_hold = 0;
 			release_phy_channel(plchan);
 		}
 		/* Dequeue jobs and free LLIs */
@@ -1529,6 +1530,7 @@ static void pl08x_tasklet(unsigned long data)
 	if (txd) {
 		/* Update last completed */
 		dma_cookie_complete(&txd->tx);
+		plchan->phychan_hold--;
 	}
 
 	/* If a new descriptor is queued, set it up plchan->at is NULL here */
Signature · 1 line
1.7.4.4
#11 lore raw

PATCH 2/8 dmaengine: amba-pl08x: ensure physical channels are properly held

Oops, this patch wasn't supposed to be part of this set...

Quoted 80 lines · Russell King

Ensure that physical channels are held while there are descriptors for them to process. This is needed when we split the pending queue into separate pending and issued queues.

---
 drivers/dma/amba-pl08x.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)
drivers/dma/amba-pl08x.c +6 -4
index c301a8e..2e74cca 100644
@@ -855,6 +855,8 @@ static int prep_phy_channel(struct pl08x_dma_chan *plchan,
 	struct pl08x_phy_chan *ch;
 	int ret;
 
+	plchan->phychan_hold++;
+
 	/* Check if we already have a channel */
 	if (plchan->phychan) {
 		ch = plchan->phychan;
@@ -863,6 +865,7 @@ static int prep_phy_channel(struct pl08x_dma_chan *plchan,
 
 	ch = pl08x_get_phy_channel(pl08x, plchan);
 	if (!ch) {
+		plchan->phychan_hold--;
 		/* No physical channel available, cope with it */
 		dev_dbg(&pl08x->adev->dev, "no physical channel available for xfer on %s\n", plchan->name);
 		return -EBUSY;
@@ -876,6 +879,7 @@ static int prep_phy_channel(struct pl08x_dma_chan *plchan,
 	if (plchan->slave && pl08x->pd->get_signal) {
 		ret = pl08x->pd->get_signal(plchan);
 		if (ret < 0) {
+			plchan->phychan_hold--;
 			dev_dbg(&pl08x->adev->dev,
 				"unable to use physical channel %d for transfer on %s due to platform restrictions\n",
 				ch->id, plchan->name);
@@ -899,8 +903,6 @@ static int prep_phy_channel(struct pl08x_dma_chan *plchan,
 	else if (txd->direction == DMA_DEV_TO_MEM)
 		txd->ccfg |= ch->signal << PL080_CONFIG_SRC_SEL_SHIFT;
 
-	plchan->phychan_hold++;
-
 	return 0;
 }
 
@@ -938,8 +940,6 @@ static dma_cookie_t pl08x_tx_submit(struct dma_async_tx_descriptor *tx)
 		/* Do this memcpy whenever there is a channel ready */
 		plchan->state = PL08X_CHAN_WAITING;
 		plchan->waiting = txd;
-	} else {
-		plchan->phychan_hold--;
 	}
 
 	spin_unlock_irqrestore(&plchan->lock, flags);
@@ -1428,6 +1428,7 @@ static int pl08x_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
 			 * Mark physical channel as free and free any slave
 			 * signal
 			 */
+			plchan->phychan_hold = 0;
 			release_phy_channel(plchan);
 		}
 		/* Dequeue jobs and free LLIs */
@@ -1529,6 +1530,7 @@ static void pl08x_tasklet(unsigned long data)
 	if (txd) {
 		/* Update last completed */
 		dma_cookie_complete(&txd->tx);
+		plchan->phychan_hold--;
 	}
 
 	/* If a new descriptor is queued, set it up plchan->at is NULL here */
Signature · 7 lines
1.7.4.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
#12 lore raw

PATCH 2/8 dmaengine: amba-pl08x: ensure physical channels are properly held

Quoted 1 line · Russell King - ARM Linux

Oops, this patch wasn't supposed to be part of this set...

...however it gives me the impression that you have a patch set cooking to also switch PL08x over to the virtual channel mechanism which is really appetizing to see :-)

Yours,
Linus Walleij

#13 lore raw

PATCH 2/8 dmaengine: amba-pl08x: ensure physical channels are properly held

Quoted 8 lines · Linus Walleij
Quoted 1 line · Russell King - ARM Linux

Oops, this patch wasn't supposed to be part of this set...

...however it gives me the impression that you have a patch set cooking to also switch PL08x over to the virtual channel mechanism which is really appetizing to see :-)

Actually, that doesn't exist yet.

This was a patch I'd had floating around, but I don't think its needed in any way - I mis-analysed the code and created the patch...

#14 lore raw

PATCH 3/8 dmaengine: split out virtual channel DMA support from sa11x0 driver

In reply to Russell King - ARM Linux

Split the virtual slave channel DMA support from the sa11x0 driver so this code can be shared with other slave DMA engine drivers.

---
 drivers/dma/Kconfig      |    4 +
 drivers/dma/Makefile     |    1 +
 drivers/dma/sa11x0-dma.c |  249 ++++++++++++++-------------------------------
 drivers/dma/virt-dma.c   |   99 ++++++++++++++++++
 drivers/dma/virt-dma.h   |  138 +++++++++++++++++++++++++
 5 files changed, 320 insertions(+), 171 deletions(-)
 create mode 100644 drivers/dma/virt-dma.c
 create mode 100644 drivers/dma/virt-dma.h
drivers/dma/Kconfig +4 -0
index cf9da36..5828ac4 100644
@@ -255,6 +255,7 @@ config DMA_SA11X0
 	tristate "SA-11x0 DMA support"
 	depends on ARCH_SA1100
 	select DMA_ENGINE
+	select DMA_VIRTUAL_CHANNELS
 	help
 	  Support the DMA engine found on Intel StrongARM SA-1100 and
 	  SA-1110 SoCs.  This DMA engine can only be used with on-chip
@@ -263,6 +264,9 @@ config DMA_SA11X0
 config DMA_ENGINE
 	bool
 
+config DMA_VIRTUAL_CHANNELS
+	tristate
+
 comment "DMA Clients"
 	depends on DMA_ENGINE
 
drivers/dma/Makefile +1 -0
index 86b795b..fc05f7d 100644
@@ -2,6 +2,7 @@ ccflags-$(CONFIG_DMADEVICES_DEBUG)  := -DDEBUG
 ccflags-$(CONFIG_DMADEVICES_VDEBUG) += -DVERBOSE_DEBUG
 
 obj-$(CONFIG_DMA_ENGINE) += dmaengine.o
+obj-$(CONFIG_DMA_VIRTUAL_CHANNELS) += virt-dma.o
 obj-$(CONFIG_NET_DMA) += iovlock.o
 obj-$(CONFIG_INTEL_MID_DMAC) += intel_mid_dma.o
 obj-$(CONFIG_DMATEST) += dmatest.o
drivers/dma/sa11x0-dma.c +78 -171

517 lines not on this page · raw message

drivers/dma/virt-dma.c new file +99 -0

99 lines not on this page · raw message

drivers/dma/virt-dma.h new file +138 -0

138 lines not on this page · raw message

Signature · 1 line
1.7.4.4
#15 lore raw

PATCH 3/8 dmaengine: split out virtual channel DMA support from sa11x0 driver

Quoted 7 lines · Russell King
drivers/dma/virt-dma.h new file +7 -0
+/**
+ * vchan_cookie_complete - report completion of a descriptor
+ * vd: virtual descriptor to update
+ *
+ * vc.lock must be held by caller
+ */
+static inline void vchan_cookie_complete(struct virt_dma_desc *vd)

drivers/dma/virt-dma.h line 92

Quoted 10 lines
drivers/dma/virt-dma.h new file +10 -0
+{
+       struct virt_dma_chan *vc = to_virt_chan(vd->tx.chan);
+
+       dma_cookie_complete(&vd->tx);
+       dev_vdbg(vc->chan.device->dev, "txd %p[%x]: marked complete\n",
+               vd, vd->tx.cookie);
+       list_add_tail(&vd->node,&vc->desc_completed);
+
+       tasklet_schedule(&vc->task);
+}

For cyclic case, we will not like to call the dma_cookie_complete() but want to put the desc in callback list.
So can we have one more arg on this function which byspass the call of dma_cookie_complete()

#16 lore raw

PATCH 3/8 dmaengine: split out virtual channel DMA support from sa11x0 driver

Quoted 15 lines · Laxman Dewangan
Quoted 14 lines · Russell King
drivers/dma/virt-dma.h new file +14 -0
+/**
+ * vchan_cookie_complete - report completion of a descriptor
+ * vd: virtual descriptor to update
+ *
+ * vc.lock must be held by caller
+ */
+static inline void vchan_cookie_complete(struct virt_dma_desc *vd)
+{
+       struct virt_dma_chan *vc = to_virt_chan(vd->tx.chan);
+
+       dma_cookie_complete(&vd->tx);
+       dev_vdbg(vc->chan.device->dev, "txd %p[%x]: marked complete\n",
+               vd, vd->tx.cookie);
+       list_add_tail(&vd->node,&vc->desc_completed);
Quoted 8 lines

drivers/dma/virt-dma.h line 92

Quoted 3 lines
drivers/dma/virt-dma.h new file +3 -0
+
+       tasklet_schedule(&vc->task);
+}

For cyclic case, we will not like to call the dma_cookie_complete() but want to put the desc in callback list.
So can we have one more arg on this function which byspass the call of dma_cookie_complete()

See the discussion on what's supposed to happen with cyclic transfers. Cyclic transfers don't complete, so adding them to the completed list and marking them complete is the wrong thing to be doing. So arguably calling this function is also the wrong thing to be doing because you're not completing the transfer.

I'll be addressing the issue of cyclic transfers when I eventually get to sorting out the OMAP ASoC driver.

#17 lore raw

PATCH 3/8 dmaengine: split out virtual channel DMA support from sa11x0 driver

Quoted 10 lines · Russell King - ARM Linux
Quoted 5 lines

For cyclic case, we will not like to call the dma_cookie_complete() but want to put the desc in callback list.
So can we have one more arg on this function which byspass the call of dma_cookie_complete()

See the discussion on what's supposed to happen with cyclic transfers. Cyclic transfers don't complete, so adding them to the completed list and marking them complete is the wrong thing to be doing. So arguably calling this function is also the wrong thing to be doing because you're not completing the transfer.

OK, we will not call this function but still need to call the callback. So do you suggest to call callback directly from dma driver rather than the virt_chan?

Quoted 2 lines

I'll be addressing the issue of cyclic transfers when I eventually get to sorting out the OMAP ASoC driver.

Here I am developing the dma driver for Tegra in cyclic and normal mode and what is your suggestion here?
Should I use your virt_chan now or I can go ahead with my first patch without virt_chan and once you are done with your virt_chan with all cyclic support then port the tegra_dma to use virt chan and next enhanced patch?

In this way, my Tegra dma will be there in tree, all client will be move to dma engine based driver and then I will comeback to tegra_dma for using the virt_channel.

#18 lore raw

PATCH 4/8 dmaengine: add OMAP DMA engine driver

In reply to Russell King - ARM Linux
---
 drivers/dma/Kconfig    |    6 +
 drivers/dma/Makefile   |    1 +
 drivers/dma/omap-dma.c |  519 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 526 insertions(+), 0 deletions(-)
 create mode 100644 drivers/dma/omap-dma.c
drivers/dma/Kconfig +6 -0
index 5828ac4..792b486 100644
@@ -261,6 +261,12 @@ config DMA_SA11X0
 	  SA-1110 SoCs.  This DMA engine can only be used with on-chip
 	  devices.
 
+config DMA_OMAP
+	tristate "OMAP DMA support"
+	depends on ARCH_OMAP
+	select DMA_ENGINE
+	select DMA_VIRTUAL_CHANNELS
+
 config DMA_ENGINE
 	bool
 
drivers/dma/Makefile +1 -0
index fc05f7d..ddc291a 100644
@@ -29,3 +29,4 @@ obj-$(CONFIG_PCH_DMA) += pch_dma.o
 obj-$(CONFIG_AMBA_PL08X) += amba-pl08x.o
 obj-$(CONFIG_EP93XX_DMA) += ep93xx_dma.o
 obj-$(CONFIG_DMA_SA11X0) += sa11x0-dma.o
+obj-$(CONFIG_DMA_OMAP) += omap-dma.o
drivers/dma/omap-dma.c new file +519 -0

519 lines not on this page · raw message

Signature · 1 line
1.7.4.4
#19 lore raw

PATCH 5/8 mmc: omap_hsmmc: release correct resource

In reply to Russell King - ARM Linux

res can be one of several resources, as this variable is re-used several times during probe. This can cause the wrong resource parameters to be passed to release_mem_region().

Get the original memory resource before calling release_mem_region().

---
 drivers/mmc/host/omap_hsmmc.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
drivers/mmc/host/omap_hsmmc.c +3 -1
index 5c2b1c1..ac26f81a 100644
@@ -2030,7 +2030,9 @@ static int __devinit omap_hsmmc_probe(struct platform_device *pdev)
 err_alloc:
 	omap_hsmmc_gpio_free(pdata);
 err:
-	release_mem_region(res->start, resource_size(res));
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (res)
+		release_mem_region(res->start, resource_size(res));
 	return ret;
 }
 
Signature · 1 line
1.7.4.4
maintainer
#20 lore raw

PATCH 5/8 mmc: omap_hsmmc: release correct resource

Quoted 7 lines · Russell King

res can be one of several resources, as this variable is re-used several times during probe. This can cause the wrong resource parameters to be passed to release_mem_region().

Get the original memory resource before calling release_mem_region().

#21 lore raw

PATCH 5/8 mmc: omap_hsmmc: release correct resource

Hi,

Quoted 10 lines · Tony Lindgren
Quoted 7 lines · Russell King

res can be one of several resources, as this variable is re-used several times during probe. This can cause the wrong resource parameters to be passed to release_mem_region().

Get the original memory resource before calling release_mem_region().

Thanks, pushed to mmc-next for 3.4.

- Chris.

Signature · 2 lines
Chris Ball   <[email protected]>   <http://printf.net/>
One Laptop Per Child
#22 lore raw

PATCH 5/8 mmc: omap_hsmmc: release correct resource

Hi,

Quoted 6 lines · Chris Ball
Quoted 5 lines · Tony Lindgren
Quoted 4 lines · Russell King

res can be one of several resources, as this variable is re-used several times during probe. This can cause the wrong resource parameters to be passed to release_mem_region().

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines

Get the original memory resource before calling release_mem_region().

Thanks, pushed to mmc-next for 3.4.

(Actually, that's premature of me, sorry -- just noticed that this is part of an RFC patchset. I'll wait for the rest of the patches.)

- Chris.

Signature · 2 lines
Chris Ball   <[email protected]>   <http://printf.net/>
One Laptop Per Child
#23 lore raw

PATCH 5/8 mmc: omap_hsmmc: release correct resource

Quoted 11 lines · Chris Ball

Hi,

Quoted 8 lines · Chris Ball
Quoted 7 lines · Tony Lindgren
Quoted 6 lines · Russell King

res can be one of several resources, as this variable is re-used several times during probe. This can cause the wrong resource parameters to be passed to release_mem_region().

Get the original memory resource before calling release_mem_region().

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line

Thanks, pushed to mmc-next for 3.4.

(Actually, that's premature of me, sorry -- just noticed that this is part of an RFC patchset. I'll wait for the rest of the patches.)

It was previously posted as an entirely separate patch - the reason its part of this RFC is because I needed the patch while developing this series. So please go ahead and take _this_ patch.

#24 lore raw

PATCH 5/8 mmc: omap_hsmmc: release correct resource

Hi,

Quoted 8 lines · Russell King - ARM Linux
Quoted 4 lines
Quoted 1 line

Thanks, pushed to mmc-next for 3.4.

(Actually, that's premature of me, sorry -- just noticed that this is part of an RFC patchset. I'll wait for the rest of the patches.)

It was previously posted as an entirely separate patch - the reason its part of this RFC is because I needed the patch while developing this series. So please go ahead and take _this_ patch.

Okay, thanks. It's back in mmc-next.

- Chris.

Signature · 2 lines
Chris Ball   <[email protected]>   <http://printf.net/>
One Laptop Per Child
#25 lore raw

PATCH 6/8 mmc: omap_hsmmc: add DMA engine support

In reply to Russell King - ARM Linux

Add DMA engine support to the OMAP HSMMC driver. This supplements the private DMA API implementation contained within this driver, and the driver can be switched at build time between using DMA engine and the private DMA API.

---
 drivers/mmc/host/omap_hsmmc.c |  192 +++++++++++++++++++++++++++++++++++------
 1 files changed, 165 insertions(+), 27 deletions(-)
drivers/mmc/host/omap_hsmmc.c +165 -27
index ac26f81a..6c09a80 100644
@@ -19,6 +19,7 @@
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/debugfs.h>
+#include <linux/dmaengine.h>
 #include <linux/seq_file.h>
 #include <linux/interrupt.h>
 #include <linux/delay.h>
@@ -164,7 +165,9 @@ struct omap_hsmmc_host {
 	u32			bytesleft;
 	int			suspended;
 	int			irq;
-	int			use_dma, dma_ch;
+	int			use_dma, dma_ch, dma2;
+	struct dma_chan		*tx_chan;
+	struct dma_chan		*rx_chan;
 	int			dma_line_tx, dma_line_rx;
 	int			slot_id;
 	int			got_dbclk;
@@ -793,18 +796,25 @@ omap_hsmmc_get_dma_dir(struct omap_hsmmc_host *host, struct mmc_data *data)
 		return DMA_FROM_DEVICE;
 }
 
+static struct dma_chan *omap_hsmmc_get_dma_chan(struct omap_hsmmc_host *host,
+	struct mmc_data *data)
+{
+	return data->flags & MMC_DATA_WRITE ? host->tx_chan : host->rx_chan;
+}
+
 static void omap_hsmmc_request_done(struct omap_hsmmc_host *host, struct mmc_request *mrq)
 {
-	int dma_ch;
+	int dma_ch, dma2;
 
 	spin_lock(&host->irq_lock);
 	host->req_in_progress = 0;
 	dma_ch = host->dma_ch;
+	dma2 = host->dma2;
 	spin_unlock(&host->irq_lock);
 
 	omap_hsmmc_disable_irq(host);
 	/* Do not complete the request if DMA is still in progress */
-	if (mrq->data && host->use_dma && dma_ch != -1)
+	if (mrq->data && host->use_dma && (dma_ch != -1 || dma2 != -1))
 		return;
 	host->mrq = NULL;
 	mmc_request_done(host->mmc, mrq);
@@ -873,15 +883,27 @@ omap_hsmmc_cmd_done(struct omap_hsmmc_host *host, struct mmc_command *cmd)
  */
 static void omap_hsmmc_dma_cleanup(struct omap_hsmmc_host *host, int errno)
 {
-	int dma_ch;
+	int dma_ch, dma2;
 
 	host->data->error = errno;
 
 	spin_lock(&host->irq_lock);
 	dma_ch = host->dma_ch;
 	host->dma_ch = -1;
+	dma2 = host->dma2;
+	host->dma2 = -1;
 	spin_unlock(&host->irq_lock);
 
+	if (host->use_dma && dma2 != -1) {
+		struct dma_chan *chan = omap_hsmmc_get_dma_chan(host, host->data);
+
+		dmaengine_terminate_all(chan);
+		dma_unmap_sg(chan->device->dev,
+			host->data->sg, host->data->sg_len,
+			omap_hsmmc_get_dma_dir(host, host->data));
+
+		host->data->host_cookie = 0;
+	}
 	if (host->use_dma && dma_ch != -1) {
 		dma_unmap_sg(mmc_dev(host->mmc), host->data->sg,
 			host->data->sg_len,
@@ -1277,9 +1299,43 @@ static void omap_hsmmc_dma_cb(int lch, u16 ch_status, void *cb_data)
 	}
 }
 
+static void omap_hsmmc_dma_callback(void *param)
+{
+	struct omap_hsmmc_host *host = param;
+	struct dma_chan *chan;
+	struct mmc_data *data;
+	int req_in_progress;
+
+	spin_lock_irq(&host->irq_lock);
+	if (host->dma2 < 0) {
+		spin_unlock_irq(&host->irq_lock);
+		return;
+	}
+
+	data = host->mrq->data;
+	chan = omap_hsmmc_get_dma_chan(host, data);
+	if (!data->host_cookie)
+		dma_unmap_sg(chan->device->dev,
+			     data->sg, data->sg_len,
+			     omap_hsmmc_get_dma_dir(host, data));
+
+	req_in_progress = host->req_in_progress;
+	host->dma2 = -1;
+	spin_unlock_irq(&host->irq_lock);
+
+	/* If DMA has finished after TC, complete the request */
+	if (!req_in_progress) {
+		struct mmc_request *mrq = host->mrq;
+
+		host->mrq = NULL;
+		mmc_request_done(host->mmc, mrq);
+	}
+}
+
 static int omap_hsmmc_pre_dma_transfer(struct omap_hsmmc_host *host,
 				       struct mmc_data *data,
-				       struct omap_hsmmc_next *next)
+				       struct omap_hsmmc_next *next,
+				       struct device *dev)
 {
 	int dma_len;
 
@@ -1294,8 +1350,7 @@ static int omap_hsmmc_pre_dma_transfer(struct omap_hsmmc_host *host,
 	/* Check if next job is already prepared */
 	if (next ||
 	    (!next && data->host_cookie != host->next_data.cookie)) {
-		dma_len = dma_map_sg(mmc_dev(host->mmc), data->sg,
-				     data->sg_len,
+		dma_len = dma_map_sg(dev, data->sg, data->sg_len,
 				     omap_hsmmc_get_dma_dir(host, data));
 
 	} else {
@@ -1324,6 +1379,7 @@ static int omap_hsmmc_start_dma_transfer(struct omap_hsmmc_host *host,
 {
 	int dma_ch = 0, ret = 0, i;
 	struct mmc_data *data = req->data;
+	struct dma_chan *chan;
 
 	/* Sanity check: all the SG entries must be aligned by block size. */
 	for (i = 0; i < data->sg_len; i++) {
@@ -1339,24 +1395,66 @@ static int omap_hsmmc_start_dma_transfer(struct omap_hsmmc_host *host,
 		 */
 		return -EINVAL;
 
-	BUG_ON(host->dma_ch != -1);
+	BUG_ON(host->dma_ch != -1 || host->dma2 != -1);
 
-	ret = omap_request_dma(omap_hsmmc_get_dma_sync_dev(host, data),
-			       "MMC/SD", omap_hsmmc_dma_cb, host, &dma_ch);
-	if (ret != 0) {
-		dev_err(mmc_dev(host->mmc),
-			"%s: omap_request_dma() failed with %d\n",
-			mmc_hostname(host->mmc), ret);
-		return ret;
-	}
-	ret = omap_hsmmc_pre_dma_transfer(host, data, NULL);
-	if (ret)
-		return ret;
+	chan = omap_hsmmc_get_dma_chan(host, data);
+	if (!chan) {
+		ret = omap_request_dma(omap_hsmmc_get_dma_sync_dev(host, data),
+				       "MMC/SD", omap_hsmmc_dma_cb, host, &dma_ch);
+		if (ret != 0) {
+			dev_err(mmc_dev(host->mmc),
+				"%s: omap_request_dma() failed with %d\n",
+				mmc_hostname(host->mmc), ret);
+			return ret;
+		}
+		ret = omap_hsmmc_pre_dma_transfer(host, data, NULL,
+						  mmc_dev(host->mmc));
+		if (ret)
+			return ret;
+
+		host->dma_ch = dma_ch;
+		host->dma_sg_idx = 0;
+
+		omap_hsmmc_config_dma_params(host, data, data->sg);
+	} else {
+		struct dma_slave_config cfg;
+		struct dma_async_tx_descriptor *tx;
+
+		cfg.src_addr = host->mapbase + OMAP_HSMMC_DATA;
+		cfg.dst_addr = host->mapbase + OMAP_HSMMC_DATA;
+		cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+		cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+		cfg.src_maxburst = data->blksz / 4;
+		cfg.dst_maxburst = data->blksz / 4;
+
+		ret = dmaengine_slave_config(chan, &cfg);
+		if (ret)
+			return ret;
+
+		ret = omap_hsmmc_pre_dma_transfer(host, data, NULL,
+						  chan->device->dev);
+		if (ret)
+			return ret;
+
+		tx = dmaengine_prep_slave_sg(chan, data->sg, data->sg_len,
+			data->flags & MMC_DATA_WRITE ? DMA_MEM_TO_DEV : DMA_DEV_TO_MEM,
+			DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+		if (!tx) {
+			dev_err(mmc_dev(host->mmc), "prep_slave_sg() failed\n");
+			/* FIXME: cleanup */
+			return -1;
+		}
 
-	host->dma_ch = dma_ch;
-	host->dma_sg_idx = 0;
+		tx->callback = omap_hsmmc_dma_callback;
+		tx->callback_param = host;
 
-	omap_hsmmc_config_dma_params(host, data, data->sg);
+		/* Does not fail */
+		dmaengine_submit(tx);
+
+		host->dma2 = 1;
+
+		dma_async_issue_pending(chan);
+	}
 
 	return 0;
 }
@@ -1439,9 +1537,12 @@ static void omap_hsmmc_post_req(struct mmc_host *mmc, struct mmc_request *mrq,
 	struct mmc_data *data = mrq->data;
 
 	if (host->use_dma) {
+		struct dma_chan *c = omap_hsmmc_get_dma_chan(host, data);
+		struct device *dev = c ? c->device->dev : mmc_dev(mmc);
+
 		if (data->host_cookie)
-			dma_unmap_sg(mmc_dev(host->mmc), data->sg,
-				     data->sg_len,
+			dma_unmap_sg(dev,
+				     data->sg, data->sg_len,
 				     omap_hsmmc_get_dma_dir(host, data));
 		data->host_cookie = 0;
 	}
@@ -1457,10 +1558,14 @@ static void omap_hsmmc_pre_req(struct mmc_host *mmc, struct mmc_request *mrq,
 		return ;
 	}
 
-	if (host->use_dma)
+	if (host->use_dma) {
+		struct dma_chan *c = omap_hsmmc_get_dma_chan(host, mrq->data);
+		struct device *dev = c ? c->device->dev : mmc_dev(mmc);
+
 		if (omap_hsmmc_pre_dma_transfer(host, mrq->data,
-						&host->next_data))
+						&host->next_data, dev))
 			mrq->data->host_cookie = 0;
+	}
 }
 
 /*
@@ -1472,7 +1577,7 @@ static void omap_hsmmc_request(struct mmc_host *mmc, struct mmc_request *req)
 	int err;
 
 	BUG_ON(host->req_in_progress);
-	BUG_ON(host->dma_ch != -1);
+	BUG_ON(host->dma_ch != -1 || host->dma2 != -1);
 	if (host->protect_card) {
 		if (host->reqs_blocked < 3) {
 			/*
@@ -1839,6 +1944,7 @@ static int __devinit omap_hsmmc_probe(struct platform_device *pdev)
 	host->use_dma	= 1;
 	host->dev->dma_mask = &pdata->dma_mask;
 	host->dma_ch	= -1;
+	host->dma2	= -1;
 	host->irq	= irq;
 	host->slot_id	= 0;
 	host->mapbase	= res->start + pdata->reg_offset;
@@ -1939,6 +2045,29 @@ static int __devinit omap_hsmmc_probe(struct platform_device *pdev)
 	}
 	host->dma_line_rx = res->start;
 
+	{
+		dma_cap_mask_t mask;
+		unsigned sig;
+		extern bool omap_dma_filter_fn(struct dma_chan *chan, void *param);
+
+		dma_cap_zero(mask);
+		dma_cap_set(DMA_SLAVE, mask);
+#if 1
+		sig = host->dma_line_rx;
+		host->rx_chan = dma_request_channel(mask, omap_dma_filter_fn, &sig);
+		if (!host->rx_chan) {
+			dev_warn(mmc_dev(host->mmc), "unable to obtain RX DMA engine channel %u\n", sig);
+		}
+#endif
+#if 1
+		sig = host->dma_line_tx;
+		host->tx_chan = dma_request_channel(mask, omap_dma_filter_fn, &sig);
+		if (!host->tx_chan) {
+			dev_warn(mmc_dev(host->mmc), "unable to obtain TX DMA engine channel %u\n", sig);
+		}
+#endif
+	}
+
 	/* Request IRQ for MMC operations */
 	ret = request_irq(host->irq, omap_hsmmc_irq, 0,
 			mmc_hostname(mmc), host);
@@ -2016,6 +2145,10 @@ static int __devinit omap_hsmmc_probe(struct platform_device *pdev)
 err_irq_cd_init:
 	free_irq(host->irq, host);
 err_irq:
+	if (host->tx_chan)
+		dma_release_channel(host->tx_chan);
+	if (host->rx_chan)
+		dma_release_channel(host->rx_chan);
 	pm_runtime_put_sync(host->dev);
 	pm_runtime_disable(host->dev);
 	clk_put(host->fclk);
@@ -2051,6 +2184,11 @@ static int __devexit omap_hsmmc_remove(struct platform_device *pdev)
 	if (mmc_slot(host).card_detect_irq)
 		free_irq(mmc_slot(host).card_detect_irq, host);
 
+	if (host->tx_chan)
+		dma_release_channel(host->tx_chan);
+	if (host->rx_chan)
+		dma_release_channel(host->rx_chan);
+
 	pm_runtime_put_sync(host->dev);
 	pm_runtime_disable(host->dev);
 	clk_put(host->fclk);
Signature · 1 line
1.7.4.4
maintainer
#26 lore raw

PATCH 6/8 mmc: omap_hsmmc: add DMA engine support

Quoted 4 lines · Russell King

Add DMA engine support to the OMAP HSMMC driver. This supplements the private DMA API implementation contained within this driver, and the driver can be switched at build time between using DMA engine and the private DMA API.

Glad to see this happening!

No luck here though, after a quick test I'm getting the following with MMC not working:

zoom3

[    2.277160] omap_hsmmc omap_hsmmc.1: unable to obtain RX DMA engine channel 48
[    2.284729] omap_hsmmc omap_hsmmc.2: unable to obtain RX DMA engine channel 78
[    2.292999] omap_hsmmc omap_hsmmc.0: unable to obtain RX DMA engine channel 62
blaze
[    1.681610] omap_hsmmc omap_hsmmc.1: unable to obtain RX DMA engine channel 48
[    1.689331] omap_hsmmc omap_hsmmc.0: unable to obtain RX DMA engine channel 62
[    1.697174] omap_hsmmc omap_hsmmc.4: unable to obtain RX DMA engine channel 60
n900
[    2.492248] omap_hsmmc omap_hsmmc.0: unable to obtain RX DMA engine channel 62
[    2.500488] omap_hsmmc omap_hsmmc.1: unable to obtain RX DMA engine channel 48

And leaving out patch 7/8 produces similar errors for both rx and tx, but MMC works without DMA:

n900

[    2.478363] omap_hsmmc omap_hsmmc.0: unable to obtain RX DMA engine channel 62
[    2.486267] omap_hsmmc omap_hsmmc.0: unable to obtain TX DMA engine channel 61
[    2.718322] omap_hsmmc omap_hsmmc.1: unable to obtain RX DMA engine channel 48
[    2.726135] omap_hsmmc omap_hsmmc.1: unable to obtain TX DMA engine channel 47

Regards,

Tony

#27 lore raw

PATCH 6/8 mmc: omap_hsmmc: add DMA engine support

Quoted 10 lines · Tony Lindgren
Quoted 4 lines · Russell King

Add DMA engine support to the OMAP HSMMC driver. This supplements the private DMA API implementation contained within this driver, and the driver can be switched at build time between using DMA engine and the private DMA API.

Glad to see this happening!

No luck here though, after a quick test I'm getting the following with MMC not working:

Did you enable OMAP DMA engine support?

maintainer
#28 lore raw

PATCH 6/8 mmc: omap_hsmmc: add DMA engine support

Quoted 7 lines · Russell King - ARM Linux
Quoted 6 lines · Tony Lindgren
Quoted 4 lines · Russell King

Add DMA engine support to the OMAP HSMMC driver. This supplements the private DMA API implementation contained within this driver, and the driver can be switched at build time between using DMA engine and the private DMA API.

Quoted 6 lines
Quoted 4 lines

Glad to see this happening!

No luck here though, after a quick test I'm getting the following with MMC not working:

Did you enable OMAP DMA engine support?

Heh that's it :) Yup it works now after adding:

CONFIG_DMADEVICES=y
CONFIG_DMA_OMAP=y

How about just continue on PIO if DMA reques fails:

drivers/mmc/host/omap_hsmmc.c +4 -8
@@ -1921,16 +1921,12 @@ static int __devinit omap_hsmmc_probe(struct platform_device *pdev)
 	dma_cap_set(DMA_SLAVE, mask);
 
 	host->rx_chan = dma_request_channel(mask, omap_dma_filter_fn, &rx_req);
-	if (!host->rx_chan) {
-		dev_err(mmc_dev(host->mmc), "unable to obtain RX DMA engine channel %u\n", rx_req);
-		goto err_irq;
-	}
+	if (!host->rx_chan)
+		dev_info(mmc_dev(host->mmc), "unable to obtain RX DMA engine channel %u\n", rx_req);
 
 	host->tx_chan = dma_request_channel(mask, omap_dma_filter_fn, &tx_req);
-	if (!host->tx_chan) {
-		dev_err(mmc_dev(host->mmc), "unable to obtain TX DMA engine channel %u\n", tx_req);
-		goto err_irq;
-	}
+	if (!host->tx_chan)
+		dev_info(mmc_dev(host->mmc), "unable to obtain TX DMA engine channel %u\n", tx_req);
 
 	/* Request IRQ for MMC operations */
 	ret = request_irq(host->irq, omap_hsmmc_irq, 0,
#29 lore raw

PATCH 7/8 mmc: omap_hsmmc: remove private DMA API implementation

In reply to Russell King - ARM Linux

Remove the private DMA API implementation from omap_hsmmc, making it use entirely the DMA engine API.

---
 drivers/mmc/host/omap_hsmmc.c |  264 ++++++++++-------------------------------
 1 files changed, 64 insertions(+), 200 deletions(-)
drivers/mmc/host/omap_hsmmc.c +63 -199
index 6c09a80..fa85efe 100644
@@ -38,7 +38,6 @@
 #include <linux/gpio.h>
 #include <linux/regulator/consumer.h>
 #include <linux/pm_runtime.h>
-#include <plat/dma.h>
 #include <mach/hardware.h>
 #include <plat/board.h>
 #include <plat/mmc.h>
@@ -165,10 +164,9 @@ struct omap_hsmmc_host {
 	u32			bytesleft;
 	int			suspended;
 	int			irq;
-	int			use_dma, dma_ch, dma2;
+	int			use_dma, dma_ch;
 	struct dma_chan		*tx_chan;
 	struct dma_chan		*rx_chan;
-	int			dma_line_tx, dma_line_rx;
 	int			slot_id;
 	int			got_dbclk;
 	int			response_busy;
@@ -804,17 +802,16 @@ static struct dma_chan *omap_hsmmc_get_dma_chan(struct omap_hsmmc_host *host,
 
 static void omap_hsmmc_request_done(struct omap_hsmmc_host *host, struct mmc_request *mrq)
 {
-	int dma_ch, dma2;
+	int dma_ch;
 
 	spin_lock(&host->irq_lock);
 	host->req_in_progress = 0;
 	dma_ch = host->dma_ch;
-	dma2 = host->dma2;
 	spin_unlock(&host->irq_lock);
 
 	omap_hsmmc_disable_irq(host);
 	/* Do not complete the request if DMA is still in progress */
-	if (mrq->data && host->use_dma && (dma_ch != -1 || dma2 != -1))
+	if (mrq->data && host->use_dma && dma_ch != -1)
 		return;
 	host->mrq = NULL;
 	mmc_request_done(host->mmc, mrq);
@@ -883,18 +880,16 @@ omap_hsmmc_cmd_done(struct omap_hsmmc_host *host, struct mmc_command *cmd)
  */
 static void omap_hsmmc_dma_cleanup(struct omap_hsmmc_host *host, int errno)
 {
-	int dma_ch, dma2;
+	int dma_ch;
 
 	host->data->error = errno;
 
 	spin_lock(&host->irq_lock);
 	dma_ch = host->dma_ch;
 	host->dma_ch = -1;
-	dma2 = host->dma2;
-	host->dma2 = -1;
 	spin_unlock(&host->irq_lock);
 
-	if (host->use_dma && dma2 != -1) {
+	if (host->use_dma && dma_ch != -1) {
 		struct dma_chan *chan = omap_hsmmc_get_dma_chan(host, host->data);
 
 		dmaengine_terminate_all(chan);
@@ -904,13 +899,6 @@ static void omap_hsmmc_dma_cleanup(struct omap_hsmmc_host *host, int errno)
 
 		host->data->host_cookie = 0;
 	}
-	if (host->use_dma && dma_ch != -1) {
-		dma_unmap_sg(mmc_dev(host->mmc), host->data->sg,
-			host->data->sg_len,
-			omap_hsmmc_get_dma_dir(host, host->data));
-		omap_free_dma(dma_ch);
-		host->data->host_cookie = 0;
-	}
 	host->data = NULL;
 }
 
@@ -1206,99 +1194,6 @@ static irqreturn_t omap_hsmmc_detect(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static int omap_hsmmc_get_dma_sync_dev(struct omap_hsmmc_host *host,
-				     struct mmc_data *data)
-{
-	int sync_dev;
-
-	if (data->flags & MMC_DATA_WRITE)
-		sync_dev = host->dma_line_tx;
-	else
-		sync_dev = host->dma_line_rx;
-	return sync_dev;
-}
-
-static void omap_hsmmc_config_dma_params(struct omap_hsmmc_host *host,
-				       struct mmc_data *data,
-				       struct scatterlist *sgl)
-{
-	int blksz, nblk, dma_ch;
-
-	dma_ch = host->dma_ch;
-	if (data->flags & MMC_DATA_WRITE) {
-		omap_set_dma_dest_params(dma_ch, 0, OMAP_DMA_AMODE_CONSTANT,
-			(host->mapbase + OMAP_HSMMC_DATA), 0, 0);
-		omap_set_dma_src_params(dma_ch, 0, OMAP_DMA_AMODE_POST_INC,
-			sg_dma_address(sgl), 0, 0);
-	} else {
-		omap_set_dma_src_params(dma_ch, 0, OMAP_DMA_AMODE_CONSTANT,
-			(host->mapbase + OMAP_HSMMC_DATA), 0, 0);
-		omap_set_dma_dest_params(dma_ch, 0, OMAP_DMA_AMODE_POST_INC,
-			sg_dma_address(sgl), 0, 0);
-	}
-
-	blksz = host->data->blksz;
-	nblk = sg_dma_len(sgl) / blksz;
-
-	omap_set_dma_transfer_params(dma_ch, OMAP_DMA_DATA_TYPE_S32,
-			blksz / 4, nblk, OMAP_DMA_SYNC_FRAME,
-			omap_hsmmc_get_dma_sync_dev(host, data),
-			!(data->flags & MMC_DATA_WRITE));
-
-	omap_start_dma(dma_ch);
-}
-
-/*
- * DMA call back function
- */
-static void omap_hsmmc_dma_cb(int lch, u16 ch_status, void *cb_data)
-{
-	struct omap_hsmmc_host *host = cb_data;
-	struct mmc_data *data;
-	int dma_ch, req_in_progress;
-
-	if (!(ch_status & OMAP_DMA_BLOCK_IRQ)) {
-		dev_warn(mmc_dev(host->mmc), "unexpected dma status %x\n",
-			ch_status);
-		return;
-	}
-
-	spin_lock(&host->irq_lock);
-	if (host->dma_ch < 0) {
-		spin_unlock(&host->irq_lock);
-		return;
-	}
-
-	data = host->mrq->data;
-	host->dma_sg_idx++;
-	if (host->dma_sg_idx < host->dma_len) {
-		/* Fire up the next transfer. */
-		omap_hsmmc_config_dma_params(host, data,
-					   data->sg + host->dma_sg_idx);
-		spin_unlock(&host->irq_lock);
-		return;
-	}
-
-	if (!data->host_cookie)
-		dma_unmap_sg(mmc_dev(host->mmc), data->sg, data->sg_len,
-			     omap_hsmmc_get_dma_dir(host, data));
-
-	req_in_progress = host->req_in_progress;
-	dma_ch = host->dma_ch;
-	host->dma_ch = -1;
-	spin_unlock(&host->irq_lock);
-
-	omap_free_dma(dma_ch);
-
-	/* If DMA has finished after TC, complete the request */
-	if (!req_in_progress) {
-		struct mmc_request *mrq = host->mrq;
-
-		host->mrq = NULL;
-		mmc_request_done(host->mmc, mrq);
-	}
-}
-
 static void omap_hsmmc_dma_callback(void *param)
 {
 	struct omap_hsmmc_host *host = param;
@@ -1307,7 +1202,7 @@ static void omap_hsmmc_dma_callback(void *param)
 	int req_in_progress;
 
 	spin_lock_irq(&host->irq_lock);
-	if (host->dma2 < 0) {
+	if (host->dma_ch < 0) {
 		spin_unlock_irq(&host->irq_lock);
 		return;
 	}
@@ -1320,7 +1215,7 @@ static void omap_hsmmc_dma_callback(void *param)
 			     omap_hsmmc_get_dma_dir(host, data));
 
 	req_in_progress = host->req_in_progress;
-	host->dma2 = -1;
+	host->dma_ch = -1;
 	spin_unlock_irq(&host->irq_lock);
 
 	/* If DMA has finished after TC, complete the request */
@@ -1335,7 +1230,7 @@ static void omap_hsmmc_dma_callback(void *param)
 static int omap_hsmmc_pre_dma_transfer(struct omap_hsmmc_host *host,
 				       struct mmc_data *data,
 				       struct omap_hsmmc_next *next,
-				       struct device *dev)
+				       struct dma_chan *chan)
 {
 	int dma_len;
 
@@ -1350,7 +1245,7 @@ static int omap_hsmmc_pre_dma_transfer(struct omap_hsmmc_host *host,
 	/* Check if next job is already prepared */
 	if (next ||
 	    (!next && data->host_cookie != host->next_data.cookie)) {
-		dma_len = dma_map_sg(dev, data->sg, data->sg_len,
+		dma_len = dma_map_sg(chan->device->dev, data->sg, data->sg_len,
 				     omap_hsmmc_get_dma_dir(host, data));
 
 	} else {
@@ -1377,7 +1272,9 @@ static int omap_hsmmc_pre_dma_transfer(struct omap_hsmmc_host *host,
 static int omap_hsmmc_start_dma_transfer(struct omap_hsmmc_host *host,
 					struct mmc_request *req)
 {
-	int dma_ch = 0, ret = 0, i;
+	struct dma_slave_config cfg;
+	struct dma_async_tx_descriptor *tx;
+	int ret = 0, i;
 	struct mmc_data *data = req->data;
 	struct dma_chan *chan;
 
@@ -1395,66 +1292,43 @@ static int omap_hsmmc_start_dma_transfer(struct omap_hsmmc_host *host,
 		 */
 		return -EINVAL;
 
-	BUG_ON(host->dma_ch != -1 || host->dma2 != -1);
+	BUG_ON(host->dma_ch != -1);
 
 	chan = omap_hsmmc_get_dma_chan(host, data);
-	if (!chan) {
-		ret = omap_request_dma(omap_hsmmc_get_dma_sync_dev(host, data),
-				       "MMC/SD", omap_hsmmc_dma_cb, host, &dma_ch);
-		if (ret != 0) {
-			dev_err(mmc_dev(host->mmc),
-				"%s: omap_request_dma() failed with %d\n",
-				mmc_hostname(host->mmc), ret);
-			return ret;
-		}
-		ret = omap_hsmmc_pre_dma_transfer(host, data, NULL,
-						  mmc_dev(host->mmc));
-		if (ret)
-			return ret;
-
-		host->dma_ch = dma_ch;
-		host->dma_sg_idx = 0;
 
-		omap_hsmmc_config_dma_params(host, data, data->sg);
-	} else {
-		struct dma_slave_config cfg;
-		struct dma_async_tx_descriptor *tx;
+	cfg.src_addr = host->mapbase + OMAP_HSMMC_DATA;
+	cfg.dst_addr = host->mapbase + OMAP_HSMMC_DATA;
+	cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+	cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+	cfg.src_maxburst = data->blksz / 4;
+	cfg.dst_maxburst = data->blksz / 4;
 
-		cfg.src_addr = host->mapbase + OMAP_HSMMC_DATA;
-		cfg.dst_addr = host->mapbase + OMAP_HSMMC_DATA;
-		cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
-		cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
-		cfg.src_maxburst = data->blksz / 4;
-		cfg.dst_maxburst = data->blksz / 4;
-
-		ret = dmaengine_slave_config(chan, &cfg);
-		if (ret)
-			return ret;
+	ret = dmaengine_slave_config(chan, &cfg);
+	if (ret)
+		return ret;
 
-		ret = omap_hsmmc_pre_dma_transfer(host, data, NULL,
-						  chan->device->dev);
-		if (ret)
-			return ret;
+	ret = omap_hsmmc_pre_dma_transfer(host, data, NULL, chan);
+	if (ret)
+		return ret;
 
-		tx = dmaengine_prep_slave_sg(chan, data->sg, data->sg_len,
-			data->flags & MMC_DATA_WRITE ? DMA_MEM_TO_DEV : DMA_DEV_TO_MEM,
-			DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
-		if (!tx) {
-			dev_err(mmc_dev(host->mmc), "prep_slave_sg() failed\n");
-			/* FIXME: cleanup */
-			return -1;
-		}
+	tx = dmaengine_prep_slave_sg(chan, data->sg, data->sg_len,
+		data->flags & MMC_DATA_WRITE ? DMA_MEM_TO_DEV : DMA_DEV_TO_MEM,
+		DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+	if (!tx) {
+		dev_err(mmc_dev(host->mmc), "prep_slave_sg() failed\n");
+		/* FIXME: cleanup */
+		return -1;
+	}
 
-		tx->callback = omap_hsmmc_dma_callback;
-		tx->callback_param = host;
+	tx->callback = omap_hsmmc_dma_callback;
+	tx->callback_param = host;
 
-		/* Does not fail */
-		dmaengine_submit(tx);
+	/* Does not fail */
+	dmaengine_submit(tx);
 
-		host->dma2 = 1;
+	host->dma_ch = 1;
 
-		dma_async_issue_pending(chan);
-	}
+	dma_async_issue_pending(chan);
 
 	return 0;
 }
@@ -1536,14 +1410,11 @@ static void omap_hsmmc_post_req(struct mmc_host *mmc, struct mmc_request *mrq,
 	struct omap_hsmmc_host *host = mmc_priv(mmc);
 	struct mmc_data *data = mrq->data;
 
-	if (host->use_dma) {
+	if (host->use_dma && data->host_cookie) {
 		struct dma_chan *c = omap_hsmmc_get_dma_chan(host, data);
-		struct device *dev = c ? c->device->dev : mmc_dev(mmc);
 
-		if (data->host_cookie)
-			dma_unmap_sg(dev,
-				     data->sg, data->sg_len,
-				     omap_hsmmc_get_dma_dir(host, data));
+		dma_unmap_sg(c->device->dev, data->sg, data->sg_len,
+			     omap_hsmmc_get_dma_dir(host, data));
 		data->host_cookie = 0;
 	}
 }
@@ -1560,10 +1431,9 @@ static void omap_hsmmc_pre_req(struct mmc_host *mmc, struct mmc_request *mrq,
 
 	if (host->use_dma) {
 		struct dma_chan *c = omap_hsmmc_get_dma_chan(host, mrq->data);
-		struct device *dev = c ? c->device->dev : mmc_dev(mmc);
 
 		if (omap_hsmmc_pre_dma_transfer(host, mrq->data,
-						&host->next_data, dev))
+						&host->next_data, c))
 			mrq->data->host_cookie = 0;
 	}
 }
@@ -1577,7 +1447,7 @@ static void omap_hsmmc_request(struct mmc_host *mmc, struct mmc_request *req)
 	int err;
 
 	BUG_ON(host->req_in_progress);
-	BUG_ON(host->dma_ch != -1 || host->dma2 != -1);
+	BUG_ON(host->dma_ch != -1);
 	if (host->protect_card) {
 		if (host->reqs_blocked < 3) {
 			/*
@@ -1890,6 +1760,8 @@ static inline struct omap_mmc_platform_data
 }
 #endif
 
+extern bool omap_dma_filter_fn(struct dma_chan *chan, void *param);
+
 static int __devinit omap_hsmmc_probe(struct platform_device *pdev)
 {
 	struct omap_mmc_platform_data *pdata = pdev->dev.platform_data;
@@ -1898,6 +1770,8 @@ static int __devinit omap_hsmmc_probe(struct platform_device *pdev)
 	struct resource *res;
 	int ret, irq;
 	const struct of_device_id *match;
+	dma_cap_mask_t mask;
+	unsigned tx_req, rx_req;
 
 	match = of_match_device(of_match_ptr(omap_mmc_of_match), &pdev->dev);
 	if (match) {
@@ -1942,9 +1816,7 @@ static int __devinit omap_hsmmc_probe(struct platform_device *pdev)
 	host->pdata	= pdata;
 	host->dev	= &pdev->dev;
 	host->use_dma	= 1;
-	host->dev->dma_mask = &pdata->dma_mask;
 	host->dma_ch	= -1;
-	host->dma2	= -1;
 	host->irq	= irq;
 	host->slot_id	= 0;
 	host->mapbase	= res->start + pdata->reg_offset;
@@ -2036,36 +1908,28 @@ static int __devinit omap_hsmmc_probe(struct platform_device *pdev)
 		dev_err(mmc_dev(host->mmc), "cannot get DMA TX channel\n");
 		goto err_irq;
 	}
-	host->dma_line_tx = res->start;
+	tx_req = res->start;
 
 	res = platform_get_resource_byname(pdev, IORESOURCE_DMA, "rx");
 	if (!res) {
 		dev_err(mmc_dev(host->mmc), "cannot get DMA RX channel\n");
 		goto err_irq;
 	}
-	host->dma_line_rx = res->start;
+	rx_req = res->start;
 
-	{
-		dma_cap_mask_t mask;
-		unsigned sig;
-		extern bool omap_dma_filter_fn(struct dma_chan *chan, void *param);
-
-		dma_cap_zero(mask);
-		dma_cap_set(DMA_SLAVE, mask);
-#if 1
-		sig = host->dma_line_rx;
-		host->rx_chan = dma_request_channel(mask, omap_dma_filter_fn, &sig);
-		if (!host->rx_chan) {
-			dev_warn(mmc_dev(host->mmc), "unable to obtain RX DMA engine channel %u\n", sig);
-		}
-#endif
-#if 1
-		sig = host->dma_line_tx;
-		host->tx_chan = dma_request_channel(mask, omap_dma_filter_fn, &sig);
-		if (!host->tx_chan) {
-			dev_warn(mmc_dev(host->mmc), "unable to obtain TX DMA engine channel %u\n", sig);
-		}
-#endif
+	dma_cap_zero(mask);
+	dma_cap_set(DMA_SLAVE, mask);
+
+	host->rx_chan = dma_request_channel(mask, omap_dma_filter_fn, &rx_req);
+	if (!host->rx_chan) {
+		dev_err(mmc_dev(host->mmc), "unable to obtain RX DMA engine channel %u\n", rx_req);
+		goto err_irq;
+	}
+
+	host->tx_chan = dma_request_channel(mask, omap_dma_filter_fn, &tx_req);
+	if (!host->tx_chan) {
+		dev_err(mmc_dev(host->mmc), "unable to obtain TX DMA engine channel %u\n", tx_req);
+		goto err_irq;
 	}
 
 	/* Request IRQ for MMC operations */
Signature · 1 line
1.7.4.4
#30 lore raw

PATCH 8/8 ARM: omap_hsmmc: remove platform data dma_mask and initialization

In reply to Russell King - ARM Linux

DMAengine uses the DMA engine device structure when mapping/unmapping memory for DMA, so the MMC devices do not need their DMA masks initialized (this reflects hardware: the MMC device is not the device doing DMA.)

---
 arch/arm/mach-omap1/board-h2-mmc.c    |    1 -
 arch/arm/mach-omap1/board-h3-mmc.c    |    1 -
 arch/arm/mach-omap1/board-nokia770.c  |    1 -
 arch/arm/mach-omap2/board-n8x0.c      |    1 -
 arch/arm/mach-omap2/hsmmc.c           |    1 -
 arch/arm/plat-omap/include/plat/mmc.h |    2 --
 6 files changed, 0 insertions(+), 7 deletions(-)
arch/arm/mach-omap1/board-h2-mmc.c +0 -1
index da0e37d..e1362ce 100644
@@ -54,7 +54,6 @@ static struct omap_mmc_platform_data mmc1_data = {
 	.nr_slots                       = 1,
 	.init				= mmc_late_init,
 	.cleanup			= mmc_cleanup,
-	.dma_mask			= 0xffffffff,
 	.slots[0]       = {
 		.set_power              = mmc_set_power,
 		.ocr_mask               = MMC_VDD_32_33 | MMC_VDD_33_34,
arch/arm/mach-omap1/board-h3-mmc.c +0 -1
index f8242aa..c74daac 100644
@@ -36,7 +36,6 @@ static int mmc_set_power(struct device *dev, int slot, int power_on,
  */
 static struct omap_mmc_platform_data mmc1_data = {
 	.nr_slots                       = 1,
-	.dma_mask			= 0xffffffff,
 	.slots[0]       = {
 		.set_power              = mmc_set_power,
 		.ocr_mask               = MMC_VDD_32_33 | MMC_VDD_33_34,
arch/arm/mach-omap1/board-nokia770.c +0 -1
index d21dcc2..cac9954 100644
@@ -185,7 +185,6 @@ static int nokia770_mmc_get_cover_state(struct device *dev, int slot)
 
 static struct omap_mmc_platform_data nokia770_mmc2_data = {
 	.nr_slots                       = 1,
-	.dma_mask			= 0xffffffff,
 	.max_freq                       = 12000000,
 	.slots[0]       = {
 		.set_power		= nokia770_mmc_set_power,
arch/arm/mach-omap2/board-n8x0.c +0 -1
index 518091c..1003748 100644
@@ -470,7 +470,6 @@ static struct omap_mmc_platform_data mmc1_data = {
 	.cleanup			= n8x0_mmc_cleanup,
 	.shutdown			= n8x0_mmc_shutdown,
 	.max_freq			= 24000000,
-	.dma_mask			= 0xffffffff,
 	.slots[0] = {
 		.wires			= 4,
 		.set_power		= n8x0_mmc_set_power,
arch/arm/mach-omap2/hsmmc.c +0 -1
index b0268ea..41e5277 100644
@@ -315,7 +315,6 @@ static int __init omap_hsmmc_pdata_init(struct omap2_hsmmc_info *c,
 	mmc->slots[0].caps = c->caps;
 	mmc->slots[0].pm_caps = c->pm_caps;
 	mmc->slots[0].internal_clock = !c->ext_clock;
-	mmc->dma_mask = 0xffffffff;
 	mmc->max_freq = c->max_freq;
 	if (cpu_is_omap44xx())
 		mmc->reg_offset = OMAP4_MMC_REG_OFFSET;
arch/arm/plat-omap/include/plat/mmc.h +0 -2
index 7a38750..9fe87a7 100644
@@ -80,8 +80,6 @@ struct omap_mmc_platform_data {
 	/* Return context loss count due to PM states changing */
 	int (*get_context_loss_count)(struct device *dev);
 
-	u64 dma_mask;
-
 	/* Integrating attributes from the omap_hwmod layer */
 	u8 controller_flags;
 
Signature · 1 line
1.7.4.4
#31 lore raw

PATCH 8/8 ARM: omap_hsmmc: remove platform data dma_mask and initialization

Quoted 5 lines · Russell King

DMAengine uses the DMA engine device structure when mapping/unmapping memory for DMA, so the MMC devices do not need their DMA masks initialized (this reflects hardware: the MMC device is not the device doing DMA.)

Quoted 8 lines

---

?arch/arm/mach-omap1/board-h2-mmc.c ? ?| ? ?1 -
?arch/arm/mach-omap1/board-h3-mmc.c ? ?| ? ?1 -
?arch/arm/mach-omap1/board-nokia770.c ?| ? ?1 -
?arch/arm/mach-omap2/board-n8x0.c ? ? ?| ? ?1 -
?arch/arm/mach-omap2/hsmmc.c ? ? ? ? ? | ? ?1 -
?arch/arm/plat-omap/include/plat/mmc.h | ? ?2 --

Hi,

drivers/mmc/host/omap.c is also using dma_mask should that also be removed

Quoted 82 lines

?6 files changed, 0 insertions(+), 7 deletions(-)

arch/arm/mach-omap1/board-h2-mmc.c +0 -1
index da0e37d..e1362ce 100644
@@ -54,7 +54,6 @@ static struct omap_mmc_platform_data mmc1_data = {
 ? ? ? ?.nr_slots ? ? ? ? ? ? ? ? ? ? ? = 1,
 ? ? ? ?.init ? ? ? ? ? ? ? ? ? ? ? ? ? = mmc_late_init,
 ? ? ? ?.cleanup ? ? ? ? ? ? ? ? ? ? ? ?= mmc_cleanup,
- ? ? ? .dma_mask ? ? ? ? ? ? ? ? ? ? ? = 0xffffffff,
 ? ? ? ?.slots[0] ? ? ? = {
 ? ? ? ? ? ? ? ?.set_power ? ? ? ? ? ? ?= mmc_set_power,
 ? ? ? ? ? ? ? ?.ocr_mask ? ? ? ? ? ? ? = MMC_VDD_32_33 | MMC_VDD_33_34,
arch/arm/mach-omap1/board-h3-mmc.c +0 -0
index f8242aa..c74daac 100644
@@ -36,7 +36,6 @@ static int mmc_set_power(struct device *dev, int slot, int power_on,

?*/
?static struct omap_mmc_platform_data mmc1_data = {
? ? ? ?.nr_slots ? ? ? ? ? ? ? ? ? ? ? = 1,
- ? ? ? .dma_mask ? ? ? ? ? ? ? ? ? ? ? = 0xffffffff,
? ? ? ?.slots[0] ? ? ? = {
? ? ? ? ? ? ? ?.set_power ? ? ? ? ? ? ?= mmc_set_power,
? ? ? ? ? ? ? ?.ocr_mask ? ? ? ? ? ? ? = MMC_VDD_32_33 | MMC_VDD_33_34,

arch/arm/mach-omap1/board-nokia770.c +0 -1
index d21dcc2..cac9954 100644
@@ -185,7 +185,6 @@ static int nokia770_mmc_get_cover_state(struct device *dev, int slot)
 
 ?static struct omap_mmc_platform_data nokia770_mmc2_data = {
 ? ? ? ?.nr_slots ? ? ? ? ? ? ? ? ? ? ? = 1,
- ? ? ? .dma_mask ? ? ? ? ? ? ? ? ? ? ? = 0xffffffff,
 ? ? ? ?.max_freq ? ? ? ? ? ? ? ? ? ? ? = 12000000,
 ? ? ? ?.slots[0] ? ? ? = {
 ? ? ? ? ? ? ? ?.set_power ? ? ? ? ? ? ?= nokia770_mmc_set_power,
arch/arm/mach-omap2/board-n8x0.c +0 -1
index 518091c..1003748 100644
@@ -470,7 +470,6 @@ static struct omap_mmc_platform_data mmc1_data = {
 ? ? ? ?.cleanup ? ? ? ? ? ? ? ? ? ? ? ?= n8x0_mmc_cleanup,
 ? ? ? ?.shutdown ? ? ? ? ? ? ? ? ? ? ? = n8x0_mmc_shutdown,
 ? ? ? ?.max_freq ? ? ? ? ? ? ? ? ? ? ? = 24000000,
- ? ? ? .dma_mask ? ? ? ? ? ? ? ? ? ? ? = 0xffffffff,
 ? ? ? ?.slots[0] = {
 ? ? ? ? ? ? ? ?.wires ? ? ? ? ? ? ? ? ?= 4,
 ? ? ? ? ? ? ? ?.set_power ? ? ? ? ? ? ?= n8x0_mmc_set_power,
arch/arm/mach-omap2/hsmmc.c +0 -1
index b0268ea..41e5277 100644
@@ -315,7 +315,6 @@ static int __init omap_hsmmc_pdata_init(struct omap2_hsmmc_info *c,
 ? ? ? ?mmc->slots[0].caps = c->caps;
 ? ? ? ?mmc->slots[0].pm_caps = c->pm_caps;
 ? ? ? ?mmc->slots[0].internal_clock = !c->ext_clock;
- ? ? ? mmc->dma_mask = 0xffffffff;
 ? ? ? ?mmc->max_freq = c->max_freq;
 ? ? ? ?if (cpu_is_omap44xx())
 ? ? ? ? ? ? ? ?mmc->reg_offset = OMAP4_MMC_REG_OFFSET;
arch/arm/plat-omap/include/plat/mmc.h +0 -0
index 7a38750..9fe87a7 100644
@@ -80,8 +80,6 @@ struct omap_mmc_platform_data {

? ? ? ?/* Return context loss count due to PM states changing */ ? ? ? ?int (*get_context_loss_count)(struct device *dev);

- ? ? ? u64 dma_mask;
-
? ? ? ?/* Integrating attributes from the omap_hwmod layer */ ? ? ? ?u8 controller_flags;

--
1.7.4.4

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo at vger.kernel.org
More majordomo info at ?http://vger.kernel.org/majordomo-info.html

#32 lore raw

PATCH 8/8 ARM: omap_hsmmc: remove platform data dma_mask and initialization

Quoted 3 lines · Balaji

Hi,

drivers/mmc/host/omap.c is also using dma_mask should that also be removed

Does this driver make use of this platform data?

If so, it needs converting to DMA engine _before_ this patch (which is one reason why its a good idea to do these changes as separate patches... as I've done.) It means stuff like this can be slotted in as necessary in the patch order.

I'm told that driver is OMAP2 only - I don't have any OMAP2 platforms to be able to test it on (I only have OMAP3 and OMAP4.)

#33 lore raw

PATCH 8/8 ARM: omap_hsmmc: remove platform data dma_mask and initialization

Quoted 7 lines · Russell King - ARM Linux
Quoted 3 lines · Balaji

Hi,

drivers/mmc/host/omap.c is also using dma_mask should that also be removed

Does this driver make use of this platform data?

Quoted 7 lines

If so, it needs converting to DMA engine _before_ this patch (which is one reason why its a good idea to do these changes as separate patches... as I've done.) ?It means stuff like this can be slotted in as necessary in the patch order.

I'm told that driver is OMAP2 only - I don't have any OMAP2 platforms to be able to test it on (I only have OMAP3 and OMAP4.)

Yes, this driver is for omap2420 and omap1 devices.

maintainer
#34 lore raw

PATCH 8/8 ARM: omap_hsmmc: remove platform data dma_mask and initialization

Quoted 5 lines · T Krishnamoorthy, Balaji
Quoted 3 lines · Russell King - ARM Linux
Quoted 2 lines · Balaji

Hi,

Quoted 8 lines
Quoted 8 lines
Quoted 1 line

drivers/mmc/host/omap.c is also using dma_mask should that also be removed

Does this driver make use of this platform data?

If so, it needs converting to DMA engine _before_ this patch (which is one reason why its a good idea to do these changes as separate patches... as I've done.) ?It means stuff like this can be slotted in as necessary in the patch order.

Can't we can just do this in drivers/mmc/host/omap.c:

- host->dev->dma_mask = &pdata->dma_mask;
+ host->dev->dma_mask = 0xffffffff;

And that way drop it from the platform data. Looks like some omap1 omap_mmc_platform_data don't have it set though, but we do have host->use_dma = 1 set anyways in drivers/mmc/host/omap.c.

Regards,

Tony

#35 lore raw

PATCH 8/8 ARM: omap_hsmmc: remove platform data dma_mask and initialization

Quoted 15 lines · Tony Lindgren
Quoted 5 lines · T Krishnamoorthy, Balaji
Quoted 3 lines · Russell King - ARM Linux
Quoted 2 lines · Balaji

Hi,

Quoted 8 lines
Quoted 8 lines
Quoted 1 line

drivers/mmc/host/omap.c is also using dma_mask should that also be removed

Does this driver make use of this platform data?

If so, it needs converting to DMA engine _before_ this patch (which is one reason why its a good idea to do these changes as separate patches... as I've done.) ?It means stuff like this can be slotted in as necessary in the patch order.

Quoted 8 lines

Can't we can just do this in drivers/mmc/host/omap.c:

- host->dev->dma_mask = &pdata->dma_mask;
+ host->dev->dma_mask = 0xffffffff;

And that way drop it from the platform data. Looks like some omap1 omap_mmc_platform_data don't have it set though, but we do have host->use_dma = 1 set anyways in drivers/mmc/host/omap.c.

No. dma_mask is a pointer to u64. You need it stored in some variable.

But, really, we shouldn't be initializing dma masks in drivers at all. That's supposed to come from the point where devices are created - and in the case of DMA engine stuff, its not the peripheral device structures which are doing DMA (they're DMA-incapable) - they rely on the services of a separate IP to perform DMA on their behalf.

maintainer
#36 lore raw

PATCH 8/8 ARM: omap_hsmmc: remove platform data dma_mask and initialization

Quoted 18 lines · Russell King - ARM Linux
Quoted 17 lines · Tony Lindgren
Quoted 5 lines · T Krishnamoorthy, Balaji
Quoted 3 lines · Russell King - ARM Linux
Quoted 2 lines · Balaji

Hi,

Quoted 8 lines
Quoted 8 lines
Quoted 1 line

drivers/mmc/host/omap.c is also using dma_mask should that also be removed

Does this driver make use of this platform data?

If so, it needs converting to DMA engine _before_ this patch (which is one reason why its a good idea to do these changes as separate patches... as I've done.) ?It means stuff like this can be slotted in as necessary in the patch order.

Can't we can just do this in drivers/mmc/host/omap.c:

Quoted 8 lines
Quoted 6 lines

- host->dev->dma_mask = &pdata->dma_mask;
+ host->dev->dma_mask = 0xffffffff;

And that way drop it from the platform data. Looks like some omap1 omap_mmc_platform_data don't have it set though, but we do have host->use_dma = 1 set anyways in drivers/mmc/host/omap.c.

No. dma_mask is a pointer to u64. You need it stored in some variable.

Oops yeah.

Quoted 5 lines

But, really, we shouldn't be initializing dma masks in drivers at all. That's supposed to come from the point where devices are created - and in the case of DMA engine stuff, its not the peripheral device structures which are doing DMA (they're DMA-incapable) - they rely on the services of a separate IP to perform DMA on their behalf.

OK

Tony

#37 lore raw

PATCH 8/8 ARM: omap_hsmmc: remove platform data dma_mask and initialization

In reply to T Krishnamoorthy, Balaji
Quoted 14 lines · Balaji
Quoted 12 lines · Russell King - ARM Linux
Quoted 3 lines · Balaji

Hi,

drivers/mmc/host/omap.c is also using dma_mask should that also be removed

Does this driver make use of this platform data?

If so, it needs converting to DMA engine _before_ this patch (which is one reason why its a good idea to do these changes as separate patches... as I've done.) ?It means stuff like this can be slotted in as necessary in the patch order.

Quoted 4 lines
Quoted 2 lines

I'm told that driver is OMAP2 only - I don't have any OMAP2 platforms to be able to test it on (I only have OMAP3 and OMAP4.)

Yes, this driver is for omap2420 and omap1 devices.

Looking at this driver, there seems to be some interaction between how the DMA is programmed for each part of the scatterlist, and how the MMC interface is programmed, specifically:

        frame = data->blksz;
        if (cpu_is_omap15xx() && frame > 32)
                frame = 32;
        else if (frame > 64)
                frame = 64;
        frame >>= 1;
        if (!(data->flags & MMC_DATA_WRITE)) {
                buf = 0x800f | ((frame - 1) << 8);
        } else {
                buf = 0x0f80 | ((frame - 1) << 0);
        }
        OMAP_MMC_WRITE(host, BUF, buf);

Does this register have to be written on every scatterlist entry, or can it be written once@the start of request processing?

maintainer
#38 lore raw

PATCH 8/8 ARM: omap_hsmmc: remove platform data dma_mask and initialization

Quoted 37 lines · Russell King - ARM Linux
Quoted 14 lines · Balaji
Quoted 12 lines · Russell King - ARM Linux
Quoted 3 lines · Balaji

Hi,

drivers/mmc/host/omap.c is also using dma_mask should that also be removed

Does this driver make use of this platform data?

If so, it needs converting to DMA engine _before_ this patch (which is one reason why its a good idea to do these changes as separate patches... as I've done.) ?It means stuff like this can be slotted in as necessary in the patch order.

Quoted 4 lines
Quoted 2 lines

I'm told that driver is OMAP2 only - I don't have any OMAP2 platforms to be able to test it on (I only have OMAP3 and OMAP4.)

Yes, this driver is for omap2420 and omap1 devices.

Looking at this driver, there seems to be some interaction between how the DMA is programmed for each part of the scatterlist, and how the MMC interface is programmed, specifically:

        frame = data->blksz;
        if (cpu_is_omap15xx() && frame > 32)
                frame = 32;
        else if (frame > 64)
                frame = 64;
        frame >>= 1;
        if (!(data->flags & MMC_DATA_WRITE)) {
                buf = 0x800f | ((frame - 1) << 8);
        } else {
                buf = 0x0f80 | ((frame - 1) << 0);
        }
        OMAP_MMC_WRITE(host, BUF, buf);
Quoted 2 lines

Does this register have to be written on every scatterlist entry, or can it be written once at the start of request processing?

The scatterlist entries are separate DMA requests on omap1, there's no chaining support in the DMA hardware on omap1. So yes, it's for every scatterlist entry on omap1.

Regards,

Tony

#39 lore raw

PATCH 8/8 ARM: omap_hsmmc: remove platform data dma_mask and initialization

Quoted 3 lines · Tony Lindgren

The scatterlist entries are separate DMA requests on omap1, there's no chaining support in the DMA hardware on omap1. So yes, it's for every scatterlist entry on omap1.

There's no direct scatterlist support on OMAP2 either (omap_hsmmc _used_ to do each scatterlist entry separately too). The DMA engine driver does the same, but will walk the scatterlist internally rather than the driver having to do it.

I'd like to have the same thing happen on OMAP1 as well (it's actually quite simple to do) and it means that this DMA engine implementation detail is (correctly) hidden from the drivers - provided there's no dependencies on each individual scatterlist entry back to the peripheral driver.

maintainer
#40 lore raw

PATCH 8/8 ARM: omap_hsmmc: remove platform data dma_mask and initialization

Quoted 9 lines · Russell King - ARM Linux
Quoted 3 lines · Tony Lindgren

The scatterlist entries are separate DMA requests on omap1, there's no chaining support in the DMA hardware on omap1. So yes, it's for every scatterlist entry on omap1.

There's no direct scatterlist support on OMAP2 either (omap_hsmmc _used_ to do each scatterlist entry separately too). The DMA engine driver does the same, but will walk the scatterlist internally rather than the driver having to do it.

OK

Quoted 5 lines

I'd like to have the same thing happen on OMAP1 as well (it's actually quite simple to do) and it means that this DMA engine implementation detail is (correctly) hidden from the drivers - provided there's no dependencies on each individual scatterlist entry back to the peripheral driver.

That would be nice. If the DMA engine driver can't dynamically adjust the frame size for each SG entry, then maybe we can work around that by not doing DMA for entries that are below the frame sizes?

Tony

#41 lore raw

PATCH 8/8 ARM: omap_hsmmc: remove platform data dma_mask and initialization

Quoted 10 lines · Tony Lindgren
Quoted 5 lines · Russell King - ARM Linux

I'd like to have the same thing happen on OMAP1 as well (it's actually quite simple to do) and it means that this DMA engine implementation detail is (correctly) hidden from the drivers - provided there's no dependencies on each individual scatterlist entry back to the peripheral driver.

That would be nice. If the DMA engine driver can't dynamically adjust the frame size for each SG entry, then maybe we can work around that by not doing DMA for entries that are below the frame sizes?

I don't think it has to in this case, though I am thinking that we may have to adjust the frame size for other peripherals.

In the case of the OMAP1 MMC driver, let me pull out that chunk of code again:

        frame = data->blksz;
        if (cpu_is_omap15xx() && frame > 32)
                frame = 32;
        else if (frame > 64)
                frame = 64;
        frame >>= 1;
        if (!(data->flags & MMC_DATA_WRITE)) {
                buf = 0x800f | ((frame - 1) << 8);
        } else {
                buf = 0x0f80 | ((frame - 1) << 0);
        }
        OMAP_MMC_WRITE(host, BUF, buf);

Nothing there depends on the size of an individual scatterlist entry - the dependencies for the frame size are the smaller of the FIFO size and the data block size. I believe we can cope with that quite well on a per-transfer basis, and I _think_ we can get away with writing this BUF register just once per transfer (similar to what happens for PIO transfers.)

However, this is going to be rather hit and miss for me - all I can do is create a patch and check that it builds. I have no way (that I know of) to test this change as I don't have any OMAP1 nor OMAP2 hardware.

maintainer
#42 lore raw

PATCH 8/8 ARM: omap_hsmmc: remove platform data dma_mask and initialization

Quoted 31 lines · Russell King - ARM Linux
Quoted 10 lines · Tony Lindgren
Quoted 5 lines · Russell King - ARM Linux

I'd like to have the same thing happen on OMAP1 as well (it's actually quite simple to do) and it means that this DMA engine implementation detail is (correctly) hidden from the drivers - provided there's no dependencies on each individual scatterlist entry back to the peripheral driver.

That would be nice. If the DMA engine driver can't dynamically adjust the frame size for each SG entry, then maybe we can work around that by not doing DMA for entries that are below the frame sizes?

I don't think it has to in this case, though I am thinking that we may have to adjust the frame size for other peripherals.

In the case of the OMAP1 MMC driver, let me pull out that chunk of code again:

        frame = data->blksz;
        if (cpu_is_omap15xx() && frame > 32)
                frame = 32;
        else if (frame > 64)
                frame = 64;
        frame >>= 1;
        if (!(data->flags & MMC_DATA_WRITE)) {
                buf = 0x800f | ((frame - 1) << 8);
        } else {
                buf = 0x0f80 | ((frame - 1) << 0);
        }
        OMAP_MMC_WRITE(host, BUF, buf);
Quoted 6 lines

Nothing there depends on the size of an individual scatterlist entry - the dependencies for the frame size are the smaller of the FIFO size and the data block size. I believe we can cope with that quite well on a per-transfer basis, and I _think_ we can get away with writing this BUF register just once per transfer (similar to what happens for PIO transfers.)

OK, let's see if that works :)

Quoted 3 lines

However, this is going to be rather hit and miss for me - all I can do is create a patch and check that it builds. I have no way (that I know of) to test this change as I don't have any OMAP1 nor OMAP2 hardware.

I can certainly test it on at least for 1710 on 770 and 2420 on N800.

Regards,

Tony

#43 lore raw

PATCH 8/8 ARM: omap_hsmmc: remove platform data dma_mask and initialization

In reply to Russell King - ARM Linux
Quoted 26 lines · Russell King - ARM Linux

I don't think it has to in this case, though I am thinking that we may have to adjust the frame size for other peripherals.

In the case of the OMAP1 MMC driver, let me pull out that chunk of code again:

        frame = data->blksz;
        if (cpu_is_omap15xx() && frame > 32)
                frame = 32;
        else if (frame > 64)
                frame = 64;
        frame >>= 1;
        if (!(data->flags & MMC_DATA_WRITE)) {
                buf = 0x800f | ((frame - 1) << 8);
        } else {
                buf = 0x0f80 | ((frame - 1) << 0);
        }
        OMAP_MMC_WRITE(host, BUF, buf);

Nothing there depends on the size of an individual scatterlist entry - the dependencies for the frame size are the smaller of the FIFO size and the data block size. I believe we can cope with that quite well on a per-transfer basis, and I _think_ we can get away with writing this BUF register just once per transfer (similar to what happens for PIO transfers.)

Quoted 3 lines

However, this is going to be rather hit and miss for me - all I can do is create a patch and check that it builds. I have no way (that I know of) to test this change as I don't have any OMAP1 nor OMAP2 hardware.

Right, so from a purely theoretical standpoint, here's the OMAP1 driver converted, and build-tested only. Modulo a few warnings and patch 8, you should be able to convert between DMA engine and the original DMA engine implementation by changing the definition at the top.

I have to stress: this is untested, it may eat your MMC/SD cards. Test it carefully and test it well.

drivers/mmc/host/omap.c +192 -10
index 887c0e5..a77c72e 100644
@@ -10,6 +10,7 @@
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
+#define USE_DMA_ENGINE
 
 #include <linux/module.h>
 #include <linux/moduleparam.h>
@@ -17,6 +18,7 @@
 #include <linux/ioport.h>
 #include <linux/platform_device.h>
 #include <linux/interrupt.h>
+#include <linux/dmaengine.h>
 #include <linux/dma-mapping.h>
 #include <linux/delay.h>
 #include <linux/spinlock.h>
@@ -128,6 +130,10 @@ struct mmc_omap_host {
 	unsigned char		id; /* 16xx chips have 2 MMC blocks */
 	struct clk *		iclk;
 	struct clk *		fclk;
+	struct dma_chan		*dma_rx;
+	u32			dma_rx_burst;
+	struct dma_chan		*dma_tx;
+	u32			dma_tx_burst;
 	struct resource		*mem_res;
 	void __iomem		*virt_base;
 	unsigned int		phys_base;
@@ -153,12 +159,14 @@ struct mmc_omap_host {
 
 	unsigned		use_dma:1;
 	unsigned		brs_received:1, dma_done:1;
-	unsigned		dma_is_read:1;
 	unsigned		dma_in_use:1;
+#ifdef USE_DMA_PRIVATE
+	unsigned		dma_is_read:1;
 	int			dma_ch;
-	spinlock_t		dma_lock;
 	struct timer_list	dma_timer;
 	unsigned		dma_len;
+#endif
+	spinlock_t		dma_lock;
 
 	struct mmc_omap_slot    *slots[OMAP_MMC_MAX_SLOTS];
 	struct mmc_omap_slot    *current_slot;
@@ -406,18 +414,34 @@ mmc_omap_release_dma(struct mmc_omap_host *host, struct mmc_data *data,
 		     int abort)
 {
 	enum dma_data_direction dma_data_dir;
+	struct device *dev = mmc_dev(host->mmc);
+	struct dma_chan *c;
 
+#ifdef USE_DMA_PRIVATE
 	BUG_ON(host->dma_ch < 0);
 	if (data->error)
 		omap_stop_dma(host->dma_ch);
 	/* Release DMA channel lazily */
 	mod_timer(&host->dma_timer, jiffies + HZ);
-	if (data->flags & MMC_DATA_WRITE)
+#endif
+	if (data->flags & MMC_DATA_WRITE) {
 		dma_data_dir = DMA_TO_DEVICE;
-	else
+		c = host->dma_tx;
+	} else {
 		dma_data_dir = DMA_FROM_DEVICE;
-	dma_unmap_sg(mmc_dev(host->mmc), data->sg, host->sg_len,
-		     dma_data_dir);
+		c = host->dma_rx;
+	}
+#ifdef USE_DMA_ENGINE
+	if (c) {
+		if (data->error) {
+			dmaengine_terminate_all(c);
+			/* Claim nothing transferred on error... */
+			data->bytes_xfered = 0;
+		}
+		dev = c->device->dev;
+	}
+#endif
+	dma_unmap_sg(dev, data->sg, host->sg_len, dma_data_dir);
 }
 
 static void mmc_omap_send_stop_work(struct work_struct *work)
@@ -524,6 +548,7 @@ mmc_omap_end_of_data(struct mmc_omap_host *host, struct mmc_data *data)
 		mmc_omap_xfer_done(host, data);
 }
 
+#ifdef USE_DMA_PRIVATE
 static void
 mmc_omap_dma_timer(unsigned long data)
 {
@@ -533,6 +558,7 @@ mmc_omap_dma_timer(unsigned long data)
 	omap_free_dma(host->dma_ch);
 	host->dma_ch = -1;
 }
+#endif
 
 static void
 mmc_omap_dma_done(struct mmc_omap_host *host, struct mmc_data *data)
@@ -891,6 +917,19 @@ static void mmc_omap_cover_handler(unsigned long param)
 		  jiffies + msecs_to_jiffies(OMAP_MMC_COVER_POLL_DELAY));
 }
 
+#ifdef USE_DMA_ENGINE
+static void mmc_omap_dma_callback(void *priv)
+{
+	struct mmc_omap_host *host = priv;
+	struct mmc_data *data = host->data;
+
+	/* If we got to the end of DMA, assume everything went well */
+	data->bytes_xfered += data->blocks * data->blksz;
+
+	mmc_omap_dma_done(host, data);
+}
+#endif
+#ifdef USE_DMA_PRIVATE
 /* Prepare to transfer the next segment of a scatterlist */
 static void
 mmc_omap_prepare_dma(struct mmc_omap_host *host, struct mmc_data *data)
@@ -1045,6 +1084,7 @@ static int mmc_omap_get_dma_channel(struct mmc_omap_host *host, struct mmc_data
 
 	return 0;
 }
+#endif
 
 static inline void set_cmd_timeout(struct mmc_omap_host *host, struct mmc_request *req)
 {
@@ -1117,6 +1157,82 @@ mmc_omap_prepare_data(struct mmc_omap_host *host, struct mmc_request *req)
 	}
 
 	host->sg_idx = 0;
+#ifdef USE_DMA_ENGINE
+	if (use_dma) {
+		enum dma_data_direction dma_data_dir;
+		struct dma_async_tx_descriptor *tx;
+		struct dma_chan *c;
+		u32 burst, *bp;
+		u16 buf;
+
+		/*
+		 * FIFO is 16x2 bytes on 15xx, and 32x2 bytes on 16xx
+		 * and 24xx. Use 16 or 32 word frames when the
+		 * blocksize is at least that large. Blocksize is
+		 * usually 512 bytes; but not for some SD reads.
+		 */
+		burst = cpu_is_omap15xx() ? 32 : 64;
+		if (burst > data->blksz)
+			burst = data->blksz;
+
+		burst >>= 1;
+
+		if (data->flags & MMC_DATA_WRITE) {
+			c = host->dma_tx;
+			bp = &host->dma_tx_burst;
+			buf = 0x0f80 | (burst - 1) << 0;
+			dma_data_dir = DMA_TO_DEVICE;
+		} else {
+			c = host->dma_rx;
+			bp = &host->dma_rx_burst;
+			buf = 0x800f | (burst - 1) << 8;
+			dma_data_dir = DMA_FROM_DEVICE;
+		}
+
+		if (!c)
+			goto use_pio;
+
+		/* Only reconfigure if we have a different burst size */
+		if (*bp != burst) {
+			struct dma_slave_config cfg;
+
+			cfg.src_addr = host->phys_base + OMAP_MMC_REG(host, DATA);
+			cfg.dst_addr = host->phys_base + OMAP_MMC_REG(host, DATA);
+			cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+			cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+			cfg.src_maxburst = burst;
+			cfg.dst_maxburst = burst;
+
+			if (dmaengine_slave_config(c, &cfg))
+				goto use_pio;
+
+			*bp = burst;
+		}
+
+		host->sg_len = dma_map_sg(c->device->dev, data->sg, sg_len,
+					  dma_data_dir);
+		if (host->sg_len == 0)
+			goto use_pio;
+
+		tx = dmaengine_prep_slave_sg(c, data->sg, host->sg_len,
+			data->flags & MMC_DATA_WRITE ? DMA_MEM_TO_DEV : DMA_DEV_TO_MEM,
+			DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+		if (!tx)
+			goto use_pio;
+
+		OMAP_MMC_WRITE(host, BUF, buf);
+
+		tx->callback = mmc_omap_dma_callback;
+		tx->callback_param = host;
+		dmaengine_submit(tx);
+		host->brs_received = 0;
+		host->dma_done = 0;
+		host->dma_in_use = 1;
+		return;
+	}
+ use_pio:
+#endif
+#ifdef USE_DMA_PRIVATE
 	if (use_dma) {
 		if (mmc_omap_get_dma_channel(host, data) == 0) {
 			enum dma_data_direction dma_data_dir;
@@ -1136,6 +1252,9 @@ mmc_omap_prepare_data(struct mmc_omap_host *host, struct mmc_request *req)
 		} else
 			use_dma = 0;
 	}
+#else
+	use_dma = 0;
+#endif
 
 	/* Revert to PIO? */
 	if (!use_dma) {
@@ -1157,8 +1276,17 @@ static void mmc_omap_start_request(struct mmc_omap_host *host,
 	/* only touch fifo AFTER the controller readies it */
 	mmc_omap_prepare_data(host, req);
 	mmc_omap_start_command(host, req->cmd);
-	if (host->dma_in_use)
-		omap_start_dma(host->dma_ch);
+	if (host->dma_in_use) {
+		struct dma_chan *c = host->data->flags & MMC_DATA_WRITE ?
+				host->dma_tx : host->dma_rx;
+
+		if (c)
+			dma_async_issue_pending(c);
+#ifdef USE_DMA_PRIVATE
+		else
+			omap_start_dma(host->dma_ch);
+#endif
+	}
 }
 
 static void mmc_omap_request(struct mmc_host *mmc, struct mmc_request *req)
@@ -1395,11 +1523,15 @@ static void mmc_omap_remove_slot(struct mmc_omap_slot *slot)
 	mmc_free_host(mmc);
 }
 
+extern bool omap_dma_filter_fn(struct dma_chan *chan, void *param);
+
 static int __init mmc_omap_probe(struct platform_device *pdev)
 {
 	struct omap_mmc_platform_data *pdata = pdev->dev.platform_data;
 	struct mmc_omap_host *host = NULL;
 	struct resource *res;
+	dma_cap_mask_t mask;
+	unsigned sig;
 	int i, ret = 0;
 	int irq;
 
@@ -1439,7 +1571,9 @@ static int __init mmc_omap_probe(struct platform_device *pdev)
 	setup_timer(&host->clk_timer, mmc_omap_clk_timer, (unsigned long) host);
 
 	spin_lock_init(&host->dma_lock);
+#ifdef USE_DMA_PRIVATE
 	setup_timer(&host->dma_timer, mmc_omap_dma_timer, (unsigned long) host);
+#endif
 	spin_lock_init(&host->slot_lock);
 	init_waitqueue_head(&host->slot_wq);
 
@@ -1452,8 +1586,10 @@ static int __init mmc_omap_probe(struct platform_device *pdev)
 	host->irq = irq;
 
 	host->use_dma = 1;
+#ifdef USE_DMA_PRIVATE
 	host->dev->dma_mask = &pdata->dma_mask;
 	host->dma_ch = -1;
+#endif
 
 	host->irq = irq;
 	host->phys_base = host->mem_res->start;
@@ -1474,9 +1610,46 @@ static int __init mmc_omap_probe(struct platform_device *pdev)
 		goto err_free_iclk;
 	}
 
+	dma_cap_zero(mask);
+	dma_cap_set(DMA_SLAVE, mask);
+
+#ifdef USE_DMA_ENGINE
+	host->dma_tx_burst = -1;
+	host->dma_rx_burst = -1;
+
+	sig = host->id == 0 ? OMAP_DMA_MMC_TX : OMAP_DMA_MMC2_TX;
+	host->dma_tx = dma_request_channel(mask, omap_dma_filter_fn, &sig);
+#if 0
+	if (!host->dma_tx) {
+		def_err(host->dev, "unable to obtain TX DMA engine channel %u\n",
+			sig);
+		goto err_dma;
+	}
+#else
+	if (!host->dma_tx)
+		dev_warn(host->dev, "unable to obtain TX DMA engine channel %u\n",
+			sig);
+#endif
+#endif
+#ifdef USE_DMA_ENGINE
+	sig = host->id == 0 ? OMAP_DMA_MMC_RX : OMAP_DMA_MMC2_RX;
+	host->dma_rx = dma_request_channel(mask, omap_dma_filter_fn, &sig);
+#if 0
+	if (!host->dma_rx) {
+		dev_err(host->dev, "unable to obtain RX DMA engine channel %u\n",
+			sig);
+		goto err_dma;
+	}
+#else
+	if (!host->dma_rx)
+		dev_warn(host->dev, "unable to obtain RX DMA engine channel %u\n",
+			sig);
+#endif
+#endif
+
 	ret = request_irq(host->irq, mmc_omap_irq, 0, DRIVER_NAME, host);
 	if (ret)
-		goto err_free_fclk;
+		goto err_free_dma;
 
 	if (pdata->init != NULL) {
 		ret = pdata->init(&pdev->dev);
@@ -1504,7 +1677,11 @@ static int __init mmc_omap_probe(struct platform_device *pdev)
 		pdata->cleanup(&pdev->dev);
 err_free_irq:
 	free_irq(host->irq, host);
-err_free_fclk:
+err_free_dma:
+	if (host->dma_tx)
+		dma_release_channel(host->dma_tx);
+	if (host->dma_rx)
+		dma_release_channel(host->dma_rx);
 	clk_put(host->fclk);
 err_free_iclk:
 	clk_disable(host->iclk);
@@ -1539,6 +1716,11 @@ static int mmc_omap_remove(struct platform_device *pdev)
 	clk_disable(host->iclk);
 	clk_put(host->iclk);
 
+	if (host->dma_tx)
+		dma_release_channel(host->dma_tx);
+	if (host->dma_rx)
+		dma_release_channel(host->dma_rx);
+
 	iounmap(host->virt_base);
 	release_mem_region(pdev->resource[0].start,
 			   pdev->resource[0].end - pdev->resource[0].start + 1);
maintainer
#44 lore raw

PATCH 8/8 ARM: omap_hsmmc: remove platform data dma_mask and initialization

Quoted 31 lines · Russell King - ARM Linux
Quoted 26 lines · Russell King - ARM Linux

I don't think it has to in this case, though I am thinking that we may have to adjust the frame size for other peripherals.

In the case of the OMAP1 MMC driver, let me pull out that chunk of code again:

        frame = data->blksz;
        if (cpu_is_omap15xx() && frame > 32)
                frame = 32;
        else if (frame > 64)
                frame = 64;
        frame >>= 1;
        if (!(data->flags & MMC_DATA_WRITE)) {
                buf = 0x800f | ((frame - 1) << 8);
        } else {
                buf = 0x0f80 | ((frame - 1) << 0);
        }
        OMAP_MMC_WRITE(host, BUF, buf);

Nothing there depends on the size of an individual scatterlist entry - the dependencies for the frame size are the smaller of the FIFO size and the data block size. I believe we can cope with that quite well on a per-transfer basis, and I _think_ we can get away with writing this BUF register just once per transfer (similar to what happens for PIO transfers.)

Quoted 3 lines

However, this is going to be rather hit and miss for me - all I can do is create a patch and check that it builds. I have no way (that I know of) to test this change as I don't have any OMAP1 nor OMAP2 hardware.

Quoted 7 lines

Right, so from a purely theoretical standpoint, here's the OMAP1 driver converted, and build-tested only. Modulo a few warnings and patch 8, you should be able to convert between DMA engine and the original DMA engine implementation by changing the definition at the top.

I have to stress: this is untested, it may eat your MMC/SD cards. Test it carefully and test it well.

Cool, you almost got it. Got it working for n800 and 770 with the following patch. Only extremely light testing done now so careful with this patch too..

Had to hack in support for the src_port and dst_port that's needed for omap_set_dma_src/dest_params on omap1. For that I just used existing struct omap_dma_channel_params to pass src_port and dst_port to omap_dma_filter_fn. Got any better ideas for doing that?

Regards,

Tony

arch/arm/plat-omap/include/plat/dma.h +2 -0
index dc562a5..1f65d8e 100644
@@ -401,6 +401,8 @@ struct omap_dma_channel_params {
 	unsigned char read_prio;/* read priority */
 	unsigned char write_prio;/* write priority */
 
+	unsigned dma_sig;
+
 #ifndef CONFIG_ARCH_OMAP1
 	enum omap_dma_burst_mode burst_mode; /* Burst mode 4/8/16 words */
 #endif
drivers/dma/omap-dma.c +15 -6
index 5b12f74..b2f8c62 100644
@@ -36,6 +36,9 @@ struct omap_chan {
 	int dma_ch;
 	struct omap_desc *desc;
 	unsigned sgidx;
+
+	uint8_t src_port;	/* OMAP_DMA_PORT_xxx, omap1 only */
+	uint8_t dst_port;	/* OMAP_DMA_PORT_xxx, omap1 only */
 };
 
 struct omap_sg {
@@ -83,10 +86,10 @@ static void omap_dma_start_sg(struct omap_chan *c, struct omap_desc *d,
 	struct omap_sg *sg = d->sg + idx;
 
 	if (d->dir == DMA_DEV_TO_MEM)
-		omap_set_dma_dest_params(c->dma_ch, 0, OMAP_DMA_AMODE_POST_INC,
+		omap_set_dma_dest_params(c->dma_ch, c->dst_port, OMAP_DMA_AMODE_POST_INC,
 				sg->addr, 0, 0);
 	else
-		omap_set_dma_src_params(c->dma_ch, 0, OMAP_DMA_AMODE_POST_INC,
+		omap_set_dma_src_params(c->dma_ch, c->src_port, OMAP_DMA_AMODE_POST_INC,
 				sg->addr, 0, 0);
 
 	omap_set_dma_transfer_params(c->dma_ch, d->es, sg->en, sg->fn,
@@ -111,10 +114,10 @@ static void omap_dma_start_desc(struct omap_chan *c)
 	c->sgidx = 0;
 
 	if (d->dir == DMA_DEV_TO_MEM)
-		omap_set_dma_src_params(c->dma_ch, 0, OMAP_DMA_AMODE_CONSTANT,
+		omap_set_dma_src_params(c->dma_ch, c->src_port, OMAP_DMA_AMODE_CONSTANT,
 			d->dev_addr, 0, 0);
 	else
-		omap_set_dma_dest_params(c->dma_ch, 0, OMAP_DMA_AMODE_CONSTANT,
+		omap_set_dma_dest_params(c->dma_ch, c->dst_port, OMAP_DMA_AMODE_CONSTANT,
 			d->dev_addr, 0, 0);
 
 	omap_dma_start_sg(c, d, 0);
@@ -475,11 +478,17 @@ static struct platform_driver omap_dma_driver = {
 
 bool omap_dma_filter_fn(struct dma_chan *chan, void *param)
 {
+	struct omap_dma_channel_params *p = param;
+
 	if (chan->device->dev->driver == &omap_dma_driver.driver) {
 		struct omap_chan *c = to_omap_dma_chan(chan);
-		unsigned req = *(unsigned *)param;
+		unsigned req = p->dma_sig;
 
-		return req == c->dma_sig;
+		if (req == c->dma_sig) {
+			c->src_port = p->src_port;
+			c->dst_port = p->dst_port;
+			return true;
+		}
 	}
 	return false;
 }
drivers/mmc/host/omap.c +25 -10
index a77c72e..284e602 100644
@@ -1531,7 +1531,7 @@ static int __init mmc_omap_probe(struct platform_device *pdev)
 	struct mmc_omap_host *host = NULL;
 	struct resource *res;
 	dma_cap_mask_t mask;
-	unsigned sig;
+	struct omap_dma_channel_params p;
 	int i, ret = 0;
 	int irq;
 
@@ -1616,34 +1616,49 @@ static int __init mmc_omap_probe(struct platform_device *pdev)
 #ifdef USE_DMA_ENGINE
 	host->dma_tx_burst = -1;
 	host->dma_rx_burst = -1;
-
-	sig = host->id == 0 ? OMAP_DMA_MMC_TX : OMAP_DMA_MMC2_TX;
-	host->dma_tx = dma_request_channel(mask, omap_dma_filter_fn, &sig);
+	if (cpu_is_omap24xx()) {
+		p.dma_sig = host->id == 0 ? OMAP24XX_DMA_MMC1_TX : OMAP24XX_DMA_MMC2_TX;
+		p.src_port = 0;
+		p.dst_port = 0;
+	} else {
+		p.dma_sig = host->id == 0 ? OMAP_DMA_MMC_TX : OMAP_DMA_MMC2_TX;
+		p.src_port = OMAP_DMA_PORT_EMIFF;
+		p.dst_port = OMAP_DMA_PORT_TIPB;
+	}
+	host->dma_tx = dma_request_channel(mask, omap_dma_filter_fn, &p);
 #if 0
 	if (!host->dma_tx) {
-		def_err(host->dev, "unable to obtain TX DMA engine channel %u\n",
+		dev_err(host->dev, "unable to obtain TX DMA engine channel %u\n",
 			sig);
 		goto err_dma;
 	}
 #else
 	if (!host->dma_tx)
 		dev_warn(host->dev, "unable to obtain TX DMA engine channel %u\n",
-			sig);
+			p.dma_sig);
 #endif
 #endif
 #ifdef USE_DMA_ENGINE
-	sig = host->id == 0 ? OMAP_DMA_MMC_RX : OMAP_DMA_MMC2_RX;
-	host->dma_rx = dma_request_channel(mask, omap_dma_filter_fn, &sig);
+	if (cpu_is_omap24xx()) {
+		p.dma_sig = host->id == 0 ? OMAP24XX_DMA_MMC1_RX :  OMAP24XX_DMA_MMC2_RX;
+		p.src_port = 0;
+		p.dst_port = 0;
+	} else {
+		p.dma_sig = host->id == 0 ? OMAP_DMA_MMC_RX : OMAP_DMA_MMC2_RX;
+		p.src_port = OMAP_DMA_PORT_TIPB;
+		p.dst_port = OMAP_DMA_PORT_EMIFF;
+	}
+	host->dma_rx = dma_request_channel(mask, omap_dma_filter_fn, &p);
 #if 0
 	if (!host->dma_rx) {
 		dev_err(host->dev, "unable to obtain RX DMA engine channel %u\n",
-			sig);
+			p.dma_sig);
 		goto err_dma;
 	}
 #else
 	if (!host->dma_rx)
 		dev_warn(host->dev, "unable to obtain RX DMA engine channel %u\n",
-			sig);
+			p.dma_sig);
 #endif
 #endif
 
drivers/mmc/host/omap_hsmmc.c +8 -2
index fa85efe..f373791 100644
@@ -42,6 +42,7 @@
 #include <plat/board.h>
 #include <plat/mmc.h>
 #include <plat/cpu.h>
+#include <plat/dma.h>
 
 /* OMAP HSMMC Host Controller Registers */
 #define OMAP_HSMMC_SYSCONFIG	0x0010
@@ -1772,6 +1773,7 @@ static int __devinit omap_hsmmc_probe(struct platform_device *pdev)
 	const struct of_device_id *match;
 	dma_cap_mask_t mask;
 	unsigned tx_req, rx_req;
+	struct omap_dma_channel_params p;
 
 	match = of_match_device(of_match_ptr(omap_mmc_of_match), &pdev->dev);
 	if (match) {
@@ -1920,13 +1922,17 @@ static int __devinit omap_hsmmc_probe(struct platform_device *pdev)
 	dma_cap_zero(mask);
 	dma_cap_set(DMA_SLAVE, mask);
 
-	host->rx_chan = dma_request_channel(mask, omap_dma_filter_fn, &rx_req);
+	p.dma_sig = rx_req;
+	p.src_port = 0;
+	p.dst_port = 0;
+	host->rx_chan = dma_request_channel(mask, omap_dma_filter_fn, &p);
 	if (!host->rx_chan) {
 		dev_err(mmc_dev(host->mmc), "unable to obtain RX DMA engine channel %u\n", rx_req);
 		goto err_irq;
 	}
 
-	host->tx_chan = dma_request_channel(mask, omap_dma_filter_fn, &tx_req);
+	p.dma_sig = tx_req;
+	host->tx_chan = dma_request_channel(mask, omap_dma_filter_fn, &p);
 	if (!host->tx_chan) {
 		dev_err(mmc_dev(host->mmc), "unable to obtain TX DMA engine channel %u\n", tx_req);
 		goto err_irq;
#45 lore raw

PATCH 8/8 ARM: omap_hsmmc: remove platform data dma_mask and initialization

Quoted 5 lines · Tony Lindgren

Cool, you almost got it. Got it working for n800 and 770 with the following patch. Only extremely light testing done now so careful with this patch too..

Had to hack in support for the src_port and dst_port that's needed for omap_set_dma_src/dest_params on omap1.

What's the relationship between these ports and the DMA request signal? Is there a document which describes this?

maintainer
#46 lore raw

PATCH 8/8 ARM: omap_hsmmc: remove platform data dma_mask and initialization

Quoted 9 lines · Russell King - ARM Linux
Quoted 5 lines · Tony Lindgren

Cool, you almost got it. Got it working for n800 and 770 with the following patch. Only extremely light testing done now so careful with this patch too..

Had to hack in support for the src_port and dst_port that's needed for omap_set_dma_src/dest_params on omap1.

What's the relationship between these ports and the DMA request signal? Is there a document which describes this?

It's the source and destination addresses bus port, so it's more related to the addresses than request signal. I guess routing of the DMA request signal is what it really does.

I'm now wondering if it might be possible to set it automatically based on source and destination address.

There's some information in the omap5912 trm on pages 717 and 718:

http://www.ti.com/litv/pdf/spru742

It says for CSDP source port "This field identifies the port originator of the transfer."

Regards,

Tony

#47 lore raw

PATCH 8/8 ARM: omap_hsmmc: remove platform data dma_mask and initialization

Quoted 18 lines · Tony Lindgren
Quoted 9 lines · Russell King - ARM Linux
Quoted 5 lines · Tony Lindgren

Cool, you almost got it. Got it working for n800 and 770 with the following patch. Only extremely light testing done now so careful with this patch too..

Had to hack in support for the src_port and dst_port that's needed for omap_set_dma_src/dest_params on omap1.

What's the relationship between these ports and the DMA request signal? Is there a document which describes this?

It's the source and destination addresses bus port, so it's more related to the addresses than request signal. I guess routing of the DMA request signal is what it really does.

I'm now wondering if it might be possible to set it automatically based on source and destination address.

Quoted 6 lines

There's some information in the omap5912 trm on pages 717 and 718:

http://www.ti.com/litv/pdf/spru742

It says for CSDP source port "This field identifies the port originator of the transfer."

Is there a reason not to use EMIFF as for the memory port for any DMA activity between a peripheral and system (kernel) memory?

It looks to me like TIPB is the right port to use for anything except camera and LCD?

If that's true, we can solve the memory side of the problem very easily - we know that prep_slave_sg() will always souce from system memory, so the port for that will be known.

For the peripheral side of the transfer, I think we should case this on the request signal as the combinations seem to be very limited - and it looks like this knowledge is specific to the DMA controller rather than the peripheral.

maintainer
#48 lore raw

PATCH 8/8 ARM: omap_hsmmc: remove platform data dma_mask and initialization

Quoted 12 lines · Russell King - ARM Linux
Quoted 11 lines · Tony Lindgren
Quoted 9 lines · Russell King - ARM Linux
Quoted 5 lines · Tony Lindgren

Cool, you almost got it. Got it working for n800 and 770 with the following patch. Only extremely light testing done now so careful with this patch too..

Had to hack in support for the src_port and dst_port that's needed for omap_set_dma_src/dest_params on omap1.

What's the relationship between these ports and the DMA request signal? Is there a document which describes this?

Quoted 6 lines
Quoted 6 lines

It's the source and destination addresses bus port, so it's more related to the addresses than request signal. I guess routing of the DMA request signal is what it really does.

I'm now wondering if it might be possible to set it automatically based on source and destination address.

There's probably some latency involved that requires setting the src and dst port earlier, so I doubt that setting it automatically based on the src and dst address would work correctly.

Quoted 9 lines
Quoted 6 lines

There's some information in the omap5912 trm on pages 717 and 718:

http://www.ti.com/litv/pdf/spru742

It says for CSDP source port "This field identifies the port originator of the transfer."

Is there a reason not to use EMIFF as for the memory port for any DMA activity between a peripheral and system (kernel) memory?

I don't think we have any device to device cases, and it's unlikely that we'll get them either.

Quoted 6 lines

It looks to me like TIPB is the right port to use for anything except camera and LCD?

If that's true, we can solve the memory side of the problem very easily - we know that prep_slave_sg() will always souce from system memory, so the port for that will be known.

OK

Quoted 4 lines

For the peripheral side of the transfer, I think we should case this on the request signal as the combinations seem to be very limited - and it looks like this knowledge is specific to the DMA controller rather than the peripheral.

OK sounds good to me.

Regards,

Tony

#49 lore raw

PATCH 8/8 ARM: omap_hsmmc: remove platform data dma_mask and initialization

Quoted 22 lines · Tony Lindgren
Quoted 10 lines · Russell King - ARM Linux
Quoted 6 lines · Tony Lindgren

There's some information in the omap5912 trm on pages 717 and 718:

http://www.ti.com/litv/pdf/spru742

It says for CSDP source port "This field identifies the port originator of the transfer."

Is there a reason not to use EMIFF as for the memory port for any DMA activity between a peripheral and system (kernel) memory?

I don't think we have any device to device cases, and it's unlikely that we'll get them either.

Quoted 6 lines

It looks to me like TIPB is the right port to use for anything except camera and LCD?

If that's true, we can solve the memory side of the problem very easily - we know that prep_slave_sg() will always souce from system memory, so the port for that will be known.

Quoted 8 lines

OK

Quoted 4 lines

For the peripheral side of the transfer, I think we should case this on the request signal as the combinations seem to be very limited - and it looks like this knowledge is specific to the DMA controller rather than the peripheral.

OK sounds good to me.

Okay, so this turns out to be really quite simple as the first step - as below. This makes us always use EMIFF for the memory side, and TIPB for the peripherals, which won't be a problem until we start thinking about camera and LCD.

I'm not going to conditionalise their use because they're merely ignored by arch/arm/plat-omap/dma.c on non-OMAP1 targets, so having this driver set them to zero for non-OMAP1 makes little sense (other than just making things unnecessarily more complicated.)

So, with this patch plus my original patch to omap's mmc host driver, this should result in something which works without all the overhead of drivers supplying the port information. Please confirm, and I'll merge this into the original omap-dma.c commit.

Thanks.

 drivers/dma/omap-dma.c |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)
drivers/dma/omap-dma.c +10 -8
index 5b12f74..ed1b8f5 100644
@@ -52,6 +52,7 @@ struct omap_desc {
 	uint8_t es;		/* element size */
 	uint8_t sync_mode;	/* OMAP_DMA_SYNC_xxx */
 	uint8_t sync_type;	/* OMAP_DMA_xxx_SYNC* */
+	uint8_t periph_port;	/* Peripheral port */
 
 	unsigned sglen;
 	struct omap_sg sg[0];
@@ -83,11 +84,11 @@ static void omap_dma_start_sg(struct omap_chan *c, struct omap_desc *d,
 	struct omap_sg *sg = d->sg + idx;
 
 	if (d->dir == DMA_DEV_TO_MEM)
-		omap_set_dma_dest_params(c->dma_ch, 0, OMAP_DMA_AMODE_POST_INC,
-				sg->addr, 0, 0);
+		omap_set_dma_dest_params(c->dma_ch, d->periph_port,
+			OMAP_DMA_AMODE_POST_INC, sg->addr, 0, 0);
 	else
-		omap_set_dma_src_params(c->dma_ch, 0, OMAP_DMA_AMODE_POST_INC,
-				sg->addr, 0, 0);
+		omap_set_dma_src_params(c->dma_ch, d->periph_port,
+			OMAP_DMA_AMODE_POST_INC, sg->addr, 0, 0);
 
 	omap_set_dma_transfer_params(c->dma_ch, d->es, sg->en, sg->fn,
 		d->sync_mode, c->dma_sig, d->sync_type);
@@ -111,11 +112,11 @@ static void omap_dma_start_desc(struct omap_chan *c)
 	c->sgidx = 0;
 
 	if (d->dir == DMA_DEV_TO_MEM)
-		omap_set_dma_src_params(c->dma_ch, 0, OMAP_DMA_AMODE_CONSTANT,
-			d->dev_addr, 0, 0);
+		omap_set_dma_src_params(c->dma_ch, OMAP_DMA_PORT_EMIFF,
+			OMAP_DMA_AMODE_CONSTANT, d->dev_addr, 0, 0);
 	else
-		omap_set_dma_dest_params(c->dma_ch, 0, OMAP_DMA_AMODE_CONSTANT,
-			d->dev_addr, 0, 0);
+		omap_set_dma_dest_params(c->dma_ch, OMAP_DMA_PORT_EMIFF,
+			OMAP_DMA_AMODE_CONSTANT, d->dev_addr, 0, 0);
 
 	omap_dma_start_sg(c, d, 0);
 }
@@ -269,6 +270,7 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg(
 	d->es = es;
 	d->sync_mode = OMAP_DMA_SYNC_FRAME;
 	d->sync_type = sync_type;
+	d->periph_port = OMAP_DMA_PORT_TIPB;
 
 	/*
 	 * Build our scatterlist entries: each contains the address,
maintainer
#50 lore raw

PATCH 8/8 ARM: omap_hsmmc: remove platform data dma_mask and initialization

Quoted 8 lines · Russell King - ARM Linux
drivers/dma/omap-dma.c +0 -2
@@ -83,11 +84,11 @@ static void omap_dma_start_sg(struct omap_chan *c, struct omap_desc *d,
 	struct omap_sg *sg = d->sg + idx;
 
 	if (d->dir == DMA_DEV_TO_MEM)
-		omap_set_dma_dest_params(c->dma_ch, 0, OMAP_DMA_AMODE_POST_INC,
-				sg->addr, 0, 0);

drivers/dma/omap-dma.c static void omap_dma_start_sg(struct omap_chan *c, struct omap_desc *d, · line 94

Quoted 10 lines
drivers/dma/omap-dma.c +4 -2
+		omap_set_dma_dest_params(c->dma_ch, d->periph_port,
+			OMAP_DMA_AMODE_POST_INC, sg->addr, 0, 0);
 	else
-		omap_set_dma_src_params(c->dma_ch, 0, OMAP_DMA_AMODE_POST_INC,
-				sg->addr, 0, 0);
+		omap_set_dma_src_params(c->dma_ch, d->periph_port,
+			OMAP_DMA_AMODE_POST_INC, sg->addr, 0, 0);
 
 	omap_set_dma_transfer_params(c->dma_ch, d->es, sg->en, sg->fn,
 		d->sync_mode, c->dma_sig, d->sync_type);

These are now wrong way around, should use OMAP_DMA_PORT_EMIFF here..

Quoted 6 lines
drivers/dma/omap-dma.c +0 -2
@@ -111,11 +112,11 @@ static void omap_dma_start_desc(struct omap_chan *c)
 	c->sgidx = 0;
 
 	if (d->dir == DMA_DEV_TO_MEM)
-		omap_set_dma_src_params(c->dma_ch, 0, OMAP_DMA_AMODE_CONSTANT,
-			d->dev_addr, 0, 0);

drivers/dma/omap-dma.c static void omap_dma_start_desc(struct omap_chan *c) · line 122

Quoted 10 lines
drivers/dma/omap-dma.c +4 -2
+		omap_set_dma_src_params(c->dma_ch, OMAP_DMA_PORT_EMIFF,
+			OMAP_DMA_AMODE_CONSTANT, d->dev_addr, 0, 0);
 	else
-		omap_set_dma_dest_params(c->dma_ch, 0, OMAP_DMA_AMODE_CONSTANT,
-			d->dev_addr, 0, 0);
+		omap_set_dma_dest_params(c->dma_ch, OMAP_DMA_PORT_EMIFF,
+			OMAP_DMA_AMODE_CONSTANT, d->dev_addr, 0, 0);
 
 	omap_dma_start_sg(c, d, 0);
 }

..and then use d->periph_port here. So the following is also needed on top of this:

drivers/dma/omap-dma.c +4 -4
@@ -84,10 +84,10 @@ static void omap_dma_start_sg(struct omap_chan *c, struct omap_desc *d,
 	struct omap_sg *sg = d->sg + idx;
 
 	if (d->dir == DMA_DEV_TO_MEM)
-		omap_set_dma_dest_params(c->dma_ch, d->periph_port,
+		omap_set_dma_dest_params(c->dma_ch, OMAP_DMA_PORT_EMIFF,
 			OMAP_DMA_AMODE_POST_INC, sg->addr, 0, 0);
 	else
-		omap_set_dma_src_params(c->dma_ch, d->periph_port,
+		omap_set_dma_src_params(c->dma_ch, OMAP_DMA_PORT_EMIFF,
 			OMAP_DMA_AMODE_POST_INC, sg->addr, 0, 0);
 
 	omap_set_dma_transfer_params(c->dma_ch, d->es, sg->en, sg->fn,
@@ -112,10 +112,10 @@ static void omap_dma_start_desc(struct omap_chan *c)
 	c->sgidx = 0;
 
 	if (d->dir == DMA_DEV_TO_MEM)
-		omap_set_dma_src_params(c->dma_ch, OMAP_DMA_PORT_EMIFF,
+		omap_set_dma_src_params(c->dma_ch, d->periph_port,
 			OMAP_DMA_AMODE_CONSTANT, d->dev_addr, 0, 0);
 	else
-		omap_set_dma_dest_params(c->dma_ch, OMAP_DMA_PORT_EMIFF,
+		omap_set_dma_dest_params(c->dma_ch, d->periph_port,
 			OMAP_DMA_AMODE_CONSTANT, d->dev_addr, 0, 0);
 
 	omap_dma_start_sg(c, d, 0);
#51 lore raw

PATCH 8/8 ARM: omap_hsmmc: remove platform data dma_mask and initialization

Quoted 17 lines · Tony Lindgren
Quoted 16 lines · Russell King - ARM Linux
drivers/dma/omap-dma.c +4 -4
@@ -83,11 +84,11 @@ static void omap_dma_start_sg(struct omap_chan *c, struct omap_desc *d,
 	struct omap_sg *sg = d->sg + idx;
 
 	if (d->dir == DMA_DEV_TO_MEM)
-		omap_set_dma_dest_params(c->dma_ch, 0, OMAP_DMA_AMODE_POST_INC,
-				sg->addr, 0, 0);
+		omap_set_dma_dest_params(c->dma_ch, d->periph_port,
+			OMAP_DMA_AMODE_POST_INC, sg->addr, 0, 0);
 	else
-		omap_set_dma_src_params(c->dma_ch, 0, OMAP_DMA_AMODE_POST_INC,
-				sg->addr, 0, 0);
+		omap_set_dma_src_params(c->dma_ch, d->periph_port,
+			OMAP_DMA_AMODE_POST_INC, sg->addr, 0, 0);
 
Quoted 4 lines

drivers/dma/omap-dma.c static void omap_dma_start_sg(struct omap_chan *c, struct omap_desc *d, · line 94

Quoted 2 lines
drivers/dma/omap-dma.c +0 -0
 	omap_set_dma_transfer_params(c->dma_ch, d->es, sg->en, sg->fn,
 		d->sync_mode, c->dma_sig, d->sync_type);

These are now wrong way around, should use OMAP_DMA_PORT_EMIFF here..

Right, sorry about that.

Okay, I'll combine all these patches into the original adding the OMAP DMA support tonight. So, the new OMAP DMA driver now looks like this:

 drivers/dma/Kconfig    |    6 +
 drivers/dma/Makefile   |    1 +
 drivers/dma/omap-dma.c |  521 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 528 insertions(+), 0 deletions(-)
drivers/dma/Kconfig +6 -0
index 5828ac4..792b486 100644
@@ -261,6 +261,12 @@ config DMA_SA11X0
 	  SA-1110 SoCs.  This DMA engine can only be used with on-chip
 	  devices.
 
+config DMA_OMAP
+	tristate "OMAP DMA support"
+	depends on ARCH_OMAP
+	select DMA_ENGINE
+	select DMA_VIRTUAL_CHANNELS
+
 config DMA_ENGINE
 	bool
 
drivers/dma/Makefile +1 -0
index fc05f7d..ddc291a 100644
@@ -29,3 +29,4 @@ obj-$(CONFIG_PCH_DMA) += pch_dma.o
 obj-$(CONFIG_AMBA_PL08X) += amba-pl08x.o
 obj-$(CONFIG_EP93XX_DMA) += ep93xx_dma.o
 obj-$(CONFIG_DMA_SA11X0) += sa11x0-dma.o
+obj-$(CONFIG_DMA_OMAP) += omap-dma.o
drivers/dma/omap-dma.c new file +521 -0

521 lines not on this page · raw message

maintainer
#52 lore raw

PATCH 8/8 ARM: omap_hsmmc: remove platform data dma_mask and initialization

Quoted 23 lines · Russell King - ARM Linux
Quoted 17 lines · Tony Lindgren
Quoted 16 lines · Russell King - ARM Linux
drivers/dma/omap-dma.c +4 -4
@@ -83,11 +84,11 @@ static void omap_dma_start_sg(struct omap_chan *c, struct omap_desc *d,
 	struct omap_sg *sg = d->sg + idx;
 
 	if (d->dir == DMA_DEV_TO_MEM)
-		omap_set_dma_dest_params(c->dma_ch, 0, OMAP_DMA_AMODE_POST_INC,
-				sg->addr, 0, 0);
+		omap_set_dma_dest_params(c->dma_ch, d->periph_port,
+			OMAP_DMA_AMODE_POST_INC, sg->addr, 0, 0);
 	else
-		omap_set_dma_src_params(c->dma_ch, 0, OMAP_DMA_AMODE_POST_INC,
-				sg->addr, 0, 0);
+		omap_set_dma_src_params(c->dma_ch, d->periph_port,
+			OMAP_DMA_AMODE_POST_INC, sg->addr, 0, 0);
 
Quoted 4 lines

drivers/dma/omap-dma.c static void omap_dma_start_sg(struct omap_chan *c, struct omap_desc *d, · line 94

Quoted 2 lines
drivers/dma/omap-dma.c +0 -0
 	omap_set_dma_transfer_params(c->dma_ch, d->es, sg->en, sg->fn,
 		d->sync_mode, c->dma_sig, d->sync_type);

These are now wrong way around, should use OMAP_DMA_PORT_EMIFF here..

Quoted 5 lines

Right, sorry about that.

Okay, I'll combine all these patches into the original adding the OMAP DMA support tonight. So, the new OMAP DMA driver now looks like this:

Thanks that works. Tried it on 700, n800, n900, 2430sdp, zoom3 and blaze:

maintainer
#53 lore raw

PATCH 8/8 ARM: omap_hsmmc: remove platform data dma_mask and initialization

In reply to Russell King - ARM Linux
Quoted 5 lines · Russell King - ARM Linux

So, with this patch plus my original patch to omap's mmc host driver, this should result in something which works without all the overhead of drivers supplying the port information. Please confirm, and I'll merge this into the original omap-dma.c commit.

Then something like this is still needed on top of your original mmc driver patch for omap2420:

drivers/mmc/host/omap.c +9 -3
@@ -1617,11 +1617,14 @@ static int __init mmc_omap_probe(struct platform_device *pdev)
 	host->dma_tx_burst = -1;
 	host->dma_rx_burst = -1;
 
-	sig = host->id == 0 ? OMAP_DMA_MMC_TX : OMAP_DMA_MMC2_TX;
+	if (cpu_is_omap24xx())
+		sig = host->id == 0 ? OMAP24XX_DMA_MMC1_TX :  OMAP24XX_DMA_MMC2_TX;
+	else
+		sig = host->id == 0 ? OMAP_DMA_MMC_TX : OMAP_DMA_MMC2_TX;
 	host->dma_tx = dma_request_channel(mask, omap_dma_filter_fn, &sig);
 #if 0
 	if (!host->dma_tx) {
-		def_err(host->dev, "unable to obtain TX DMA engine channel %u\n",
+		dev_err(host->dev, "unable to obtain TX DMA engine channel %u\n",
 			sig);
 		goto err_dma;
 	}
@@ -1632,7 +1635,10 @@ static int __init mmc_omap_probe(struct platform_device *pdev)
 #endif
 #endif
 #ifdef USE_DMA_ENGINE
-	sig = host->id == 0 ? OMAP_DMA_MMC_RX : OMAP_DMA_MMC2_RX;
+	if (cpu_is_omap24xx())
+		sig = host->id == 0 ? OMAP24XX_DMA_MMC1_RX :  OMAP24XX_DMA_MMC2_RX;
+	else
+		sig = host->id == 0 ? OMAP_DMA_MMC_RX : OMAP_DMA_MMC2_RX;
 	host->dma_rx = dma_request_channel(mask, omap_dma_filter_fn, &sig);
 #if 0
 	if (!host->dma_rx) {
#54 lore raw

PATCH 8/8 ARM: omap_hsmmc: remove platform data dma_mask and initialization

Quoted 24 lines · Tony Lindgren

Then something like this is still needed on top of your original mmc driver patch for omap2420:

drivers/mmc/host/omap.c +5 -2
@@ -1617,11 +1617,14 @@ static int __init mmc_omap_probe(struct platform_device *pdev)
 	host->dma_tx_burst = -1;
 	host->dma_rx_burst = -1;
 
-	sig = host->id == 0 ? OMAP_DMA_MMC_TX : OMAP_DMA_MMC2_TX;
+	if (cpu_is_omap24xx())
+		sig = host->id == 0 ? OMAP24XX_DMA_MMC1_TX :  OMAP24XX_DMA_MMC2_TX;
+	else
+		sig = host->id == 0 ? OMAP_DMA_MMC_TX : OMAP_DMA_MMC2_TX;
 	host->dma_tx = dma_request_channel(mask, omap_dma_filter_fn, &sig);
 #if 0
 	if (!host->dma_tx) {
-		def_err(host->dev, "unable to obtain TX DMA engine channel %u\n",
+		dev_err(host->dev, "unable to obtain TX DMA engine channel %u\n",
 			sig);
 		goto err_dma;
 	}
@@ -1632,7 +1635,10 @@ static int __init mmc_omap_probe(struct platform_device *pdev)
 #endif

drivers/mmc/host/omap.c static int __init mmc_omap_probe(struct platform_device *pdev) · line 1644

Quoted 10 lines
drivers/mmc/host/omap.c +4 -1
 #endif
 #ifdef USE_DMA_ENGINE
-	sig = host->id == 0 ? OMAP_DMA_MMC_RX : OMAP_DMA_MMC2_RX;
+	if (cpu_is_omap24xx())
+		sig = host->id == 0 ? OMAP24XX_DMA_MMC1_RX :  OMAP24XX_DMA_MMC2_RX;
+	else
+		sig = host->id == 0 ? OMAP_DMA_MMC_RX : OMAP_DMA_MMC2_RX;
 	host->dma_rx = dma_request_channel(mask, omap_dma_filter_fn, &sig);
 #if 0
 	if (!host->dma_rx) {

Is there any chance this DMA signal information could be moved into platform resources, like omap_hsmmc has done? I could then get rid of the plat/dma.h dependence from this driver.

maintainer
#55 lore raw

PATCH 8/8 ARM: omap_hsmmc: remove platform data dma_mask and initialization

Quoted 31 lines · Russell King - ARM Linux
Quoted 30 lines · Tony Lindgren

Then something like this is still needed on top of your original mmc driver patch for omap2420:

drivers/mmc/host/omap.c +8 -3
@@ -1617,11 +1617,14 @@ static int __init mmc_omap_probe(struct platform_device *pdev)
 	host->dma_tx_burst = -1;
 	host->dma_rx_burst = -1;
 
-	sig = host->id == 0 ? OMAP_DMA_MMC_TX : OMAP_DMA_MMC2_TX;
+	if (cpu_is_omap24xx())
+		sig = host->id == 0 ? OMAP24XX_DMA_MMC1_TX :  OMAP24XX_DMA_MMC2_TX;
+	else
+		sig = host->id == 0 ? OMAP_DMA_MMC_TX : OMAP_DMA_MMC2_TX;
 	host->dma_tx = dma_request_channel(mask, omap_dma_filter_fn, &sig);
 #if 0
 	if (!host->dma_tx) {
-		def_err(host->dev, "unable to obtain TX DMA engine channel %u\n",
+		dev_err(host->dev, "unable to obtain TX DMA engine channel %u\n",
 			sig);
 		goto err_dma;
 	}
@@ -1632,7 +1635,10 @@ static int __init mmc_omap_probe(struct platform_device *pdev)
 #endif
 #endif
 #ifdef USE_DMA_ENGINE
-	sig = host->id == 0 ? OMAP_DMA_MMC_RX : OMAP_DMA_MMC2_RX;
+	if (cpu_is_omap24xx())
+		sig = host->id == 0 ? OMAP24XX_DMA_MMC1_RX :  OMAP24XX_DMA_MMC2_RX;
+	else
Quoted 8 lines

drivers/mmc/host/omap.c static int __init mmc_omap_probe(struct platform_device *pdev) · line 1644

Quoted 4 lines
drivers/mmc/host/omap.c +1 -0
+		sig = host->id == 0 ? OMAP_DMA_MMC_RX : OMAP_DMA_MMC2_RX;
 	host->dma_rx = dma_request_channel(mask, omap_dma_filter_fn, &sig);
 #if 0
 	if (!host->dma_rx) {

Is there any chance this DMA signal information could be moved into platform resources, like omap_hsmmc has done? I could then get rid of the plat/dma.h dependence from this driver.

Sure, I'll take a look at that later on today.

Tony

#56 lore raw

PATCH 8/8 ARM: omap_hsmmc: remove platform data dma_mask and initialization

Quoted 41 lines · Tony Lindgren
Quoted 31 lines · Russell King - ARM Linux
Quoted 30 lines · Tony Lindgren

Then something like this is still needed on top of your original mmc driver patch for omap2420:

drivers/mmc/host/omap.c +8 -3
@@ -1617,11 +1617,14 @@ static int __init mmc_omap_probe(struct platform_device *pdev)
 	host->dma_tx_burst = -1;
 	host->dma_rx_burst = -1;
 
-	sig = host->id == 0 ? OMAP_DMA_MMC_TX : OMAP_DMA_MMC2_TX;
+	if (cpu_is_omap24xx())
+		sig = host->id == 0 ? OMAP24XX_DMA_MMC1_TX :  OMAP24XX_DMA_MMC2_TX;
+	else
+		sig = host->id == 0 ? OMAP_DMA_MMC_TX : OMAP_DMA_MMC2_TX;
 	host->dma_tx = dma_request_channel(mask, omap_dma_filter_fn, &sig);
 #if 0
 	if (!host->dma_tx) {
-		def_err(host->dev, "unable to obtain TX DMA engine channel %u\n",
+		dev_err(host->dev, "unable to obtain TX DMA engine channel %u\n",
 			sig);
 		goto err_dma;
 	}
@@ -1632,7 +1635,10 @@ static int __init mmc_omap_probe(struct platform_device *pdev)
 #endif
 #endif
 #ifdef USE_DMA_ENGINE
-	sig = host->id == 0 ? OMAP_DMA_MMC_RX : OMAP_DMA_MMC2_RX;
+	if (cpu_is_omap24xx())
+		sig = host->id == 0 ? OMAP24XX_DMA_MMC1_RX :  OMAP24XX_DMA_MMC2_RX;
+	else
Quoted 8 lines

drivers/mmc/host/omap.c static int __init mmc_omap_probe(struct platform_device *pdev) · line 1644

Quoted 4 lines
drivers/mmc/host/omap.c +1 -0
+		sig = host->id == 0 ? OMAP_DMA_MMC_RX : OMAP_DMA_MMC2_RX;
 	host->dma_rx = dma_request_channel(mask, omap_dma_filter_fn, &sig);
 #if 0
 	if (!host->dma_rx) {

Is there any chance this DMA signal information could be moved into platform resources, like omap_hsmmc has done? I could then get rid of the plat/dma.h dependence from this driver.

Quoted 1 line

Sure, I'll take a look at that later on today.

Let me re-post the series first so you can produce patches against the latest version of the series. It's 11 patches, which contains the SPI conversion.

#57 lore raw

cover DMA engine conversion

In reply to Russell King - ARM Linux

Here's a patch which converts the SPI driver to use dma engine support. It's more or less a mechanical replacement in this driver. Tested on OMAP4430SDP ethernet (which basically means that udhcpc works.)

One thing to note: the old DMA code programmed the DMA hardware for an element count of N and frame size of 1, with element sync. The DMA engine code will program the DMA hardware for an element count of 1 and a frame size of N, with frame sync. This appears to work - but at the moment I've not looked to see if there's any implications of this.

You can enable/disable the DMA engine code for the TX or RX side by tweaking the two definitions at the top of the file if you want to do comparisons.

Comments? If none, I'll see about committing this into the patch set, adding the removal of the private DMA code, and sending it out more "officially" with the next round of RFC for this set.

Modulo the element vs frame sync issue, I think folk can see that the conversion process is fairly straight forward - it's more or less:
1. add struct dma_chan * pointer to the private structure.
2. add allocation/freeing functions for the DMA engine channels.
3. convert the callbacks to match the DMA engine callback prototype.
4. add code to set the DMA engine channel configuration appropriately (avoiding any unnecessary calls) via dmaengine_slave_config().
5. add code to queue the transfer via dmaengine_prep_slave_sg() and dmaengine_submit().
6. omap_start_dma() becomes dma_async_issue_pending().

 drivers/spi/spi-omap2-mcspi.c |  186 +++++++++++++++++++++++++++++++++-------
 1 files changed, 153 insertions(+), 33 deletions(-)
drivers/spi/spi-omap2-mcspi.c +151 -31
index bb9274c..a77af0b 100644
@@ -20,6 +20,8 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
  *
  */
+#define USE_DMA_ENGINE_RX
+#define USE_DMA_ENGINE_TX
 
 #include <linux/kernel.h>
 #include <linux/init.h>
@@ -28,6 +30,7 @@
 #include <linux/device.h>
 #include <linux/delay.h>
 #include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
 #include <linux/platform_device.h>
 #include <linux/err.h>
 #include <linux/clk.h>
@@ -95,6 +98,8 @@
 
 /* We have 2 DMA channels per CS, one for RX and one for TX */
 struct omap2_mcspi_dma {
+	struct dma_chan *dma_tx;
+	struct dma_chan *dma_rx;
 	int dma_tx_channel;
 	int dma_rx_channel;
 
@@ -290,6 +295,30 @@ static int mcspi_wait_for_reg_bit(void __iomem *reg, unsigned long bit)
 	return 0;
 }
 
+static void omap2_mcspi_rx_callback(void *data)
+{
+	struct spi_device *spi = data;
+	struct omap2_mcspi *mcspi = spi_master_get_devdata(spi->master);
+	struct omap2_mcspi_dma *mcspi_dma = &mcspi->dma_channels[spi->chip_select];
+
+	complete(&mcspi_dma->dma_rx_completion);
+
+	/* We must disable the DMA RX request */
+	omap2_mcspi_set_dma_req(spi, 1, 0);
+}
+
+static void omap2_mcspi_tx_callback(void *data)
+{
+	struct spi_device *spi = data;
+	struct omap2_mcspi *mcspi = spi_master_get_devdata(spi->master);
+	struct omap2_mcspi_dma *mcspi_dma = &mcspi->dma_channels[spi->chip_select];
+
+	complete(&mcspi_dma->dma_tx_completion);
+
+	/* We must disable the DMA TX request */
+	omap2_mcspi_set_dma_req(spi, 0, 0);
+}
+
 static unsigned
 omap2_mcspi_txrx_dma(struct spi_device *spi, struct spi_transfer *xfer)
 {
@@ -304,6 +333,9 @@ omap2_mcspi_txrx_dma(struct spi_device *spi, struct spi_transfer *xfer)
 	u8			* rx;
 	const u8		* tx;
 	void __iomem		*chstat_reg;
+	struct dma_slave_config	cfg;
+	enum dma_slave_buswidth width;
+	unsigned es;
 
 	mcspi = spi_master_get_devdata(spi->master);
 	mcspi_dma = &mcspi->dma_channels[spi->chip_select];
@@ -311,6 +343,71 @@ omap2_mcspi_txrx_dma(struct spi_device *spi, struct spi_transfer *xfer)
 
 	chstat_reg = cs->base + OMAP2_MCSPI_CHSTAT0;
 
+	if (cs->word_len <= 8) {
+		width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+		es = 1;
+	} else if (cs->word_len <= 16) {
+		width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+		es = 2;
+	} else {
+		width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+		es = 4;
+	}
+
+	memset(&cfg, 0, sizeof(cfg));
+	cfg.src_addr = cs->phys + OMAP2_MCSPI_RX0;
+	cfg.dst_addr = cs->phys + OMAP2_MCSPI_TX0;
+	cfg.src_addr_width = width;
+	cfg.dst_addr_width = width;
+	cfg.src_maxburst = 1;
+	cfg.dst_maxburst = 1;
+
+	if (xfer->tx_buf && mcspi_dma->dma_tx) {
+		struct dma_async_tx_descriptor *tx;
+		struct scatterlist sg;
+
+		dmaengine_slave_config(mcspi_dma->dma_tx, &cfg);
+
+		sg_init_table(&sg, 1);
+		sg_dma_address(&sg) = xfer->tx_dma;
+		sg_dma_len(&sg) = xfer->len;
+
+		tx = dmaengine_prep_slave_sg(mcspi_dma->dma_tx, &sg, 1,
+			DMA_MEM_TO_DEV, DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+		if (tx) {
+			tx->callback = omap2_mcspi_tx_callback;
+			tx->callback_param = spi;
+			dmaengine_submit(tx);
+		} else {
+			/* FIXME: fall back to PIO? */
+		}
+	}
+
+	if (xfer->rx_buf && mcspi_dma->dma_rx) {
+		struct dma_async_tx_descriptor *tx;
+		struct scatterlist sg;
+		size_t len = xfer->len - es;
+
+		dmaengine_slave_config(mcspi_dma->dma_rx, &cfg);
+
+		if (l & OMAP2_MCSPI_CHCONF_TURBO)
+			len -= es;
+
+		sg_init_table(&sg, 1);
+		sg_dma_address(&sg) = xfer->rx_dma;
+		sg_dma_len(&sg) = len;
+
+		tx = dmaengine_prep_slave_sg(mcspi_dma->dma_rx, &sg, 1,
+			DMA_DEV_TO_MEM, DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+		if (tx) {
+			tx->callback = omap2_mcspi_rx_callback;
+			tx->callback_param = spi;
+			dmaengine_submit(tx);
+		} else {
+			/* FIXME: fall back to PIO? */
+		}
+	}
+
 	count = xfer->len;
 	c = count;
 	word_len = cs->word_len;
@@ -332,7 +429,7 @@ omap2_mcspi_txrx_dma(struct spi_device *spi, struct spi_transfer *xfer)
 		element_count = count >> 2;
 	}
 
-	if (tx != NULL) {
+	if (tx != NULL && mcspi_dma->dma_tx_channel != -1) {
 		omap_set_dma_transfer_params(mcspi_dma->dma_tx_channel,
 				data_type, element_count, 1,
 				OMAP_DMA_SYNC_ELEMENT,
@@ -347,7 +444,7 @@ omap2_mcspi_txrx_dma(struct spi_device *spi, struct spi_transfer *xfer)
 				xfer->tx_dma, 0, 0);
 	}
 
-	if (rx != NULL) {
+	if (rx != NULL && mcspi_dma->dma_rx_channel != -1) {
 		elements = element_count - 1;
 		if (l & OMAP2_MCSPI_CHCONF_TURBO)
 			elements--;
@@ -367,12 +464,18 @@ omap2_mcspi_txrx_dma(struct spi_device *spi, struct spi_transfer *xfer)
 	}
 
 	if (tx != NULL) {
-		omap_start_dma(mcspi_dma->dma_tx_channel);
+		if (mcspi_dma->dma_tx)
+			dma_async_issue_pending(mcspi_dma->dma_tx);
+		else
+			omap_start_dma(mcspi_dma->dma_tx_channel);
 		omap2_mcspi_set_dma_req(spi, 0, 1);
 	}
 
 	if (rx != NULL) {
-		omap_start_dma(mcspi_dma->dma_rx_channel);
+		if (mcspi_dma->dma_rx)
+			dma_async_issue_pending(mcspi_dma->dma_rx);
+		else
+			omap_start_dma(mcspi_dma->dma_rx_channel);
 		omap2_mcspi_set_dma_req(spi, 1, 1);
 	}
 
@@ -396,7 +499,10 @@ omap2_mcspi_txrx_dma(struct spi_device *spi, struct spi_transfer *xfer)
 		dma_unmap_single(&spi->dev, xfer->rx_dma, count, DMA_FROM_DEVICE);
 		omap2_mcspi_set_enable(spi, 0);
 
+		elements = element_count - 1;
+
 		if (l & OMAP2_MCSPI_CHCONF_TURBO) {
+			elements--;
 
 			if (likely(mcspi_read_cs_reg(spi, OMAP2_MCSPI_CHSTAT0)
 				   & OMAP2_MCSPI_CHSTAT_RXS)) {
@@ -715,50 +821,58 @@ static int omap2_mcspi_setup_transfer(struct spi_device *spi,
 
 static void omap2_mcspi_dma_rx_callback(int lch, u16 ch_status, void *data)
 {
-	struct spi_device	*spi = data;
-	struct omap2_mcspi	*mcspi;
-	struct omap2_mcspi_dma	*mcspi_dma;
-
-	mcspi = spi_master_get_devdata(spi->master);
-	mcspi_dma = &(mcspi->dma_channels[spi->chip_select]);
-
-	complete(&mcspi_dma->dma_rx_completion);
-
-	/* We must disable the DMA RX request */
-	omap2_mcspi_set_dma_req(spi, 1, 0);
+	omap2_mcspi_rx_callback(data);
 }
 
 static void omap2_mcspi_dma_tx_callback(int lch, u16 ch_status, void *data)
 {
-	struct spi_device	*spi = data;
-	struct omap2_mcspi	*mcspi;
-	struct omap2_mcspi_dma	*mcspi_dma;
-
-	mcspi = spi_master_get_devdata(spi->master);
-	mcspi_dma = &(mcspi->dma_channels[spi->chip_select]);
-
-	complete(&mcspi_dma->dma_tx_completion);
-
-	/* We must disable the DMA TX request */
-	omap2_mcspi_set_dma_req(spi, 0, 0);
+	omap2_mcspi_tx_callback(data);
 }
 
+extern bool omap_dma_filter_fn(struct dma_chan *chan, void *param);
+
 static int omap2_mcspi_request_dma(struct spi_device *spi)
 {
 	struct spi_master	*master = spi->master;
 	struct omap2_mcspi	*mcspi;
 	struct omap2_mcspi_dma	*mcspi_dma;
+	dma_cap_mask_t mask;
+	unsigned sig;
 
 	mcspi = spi_master_get_devdata(master);
 	mcspi_dma = mcspi->dma_channels + spi->chip_select;
 
+	init_completion(&mcspi_dma->dma_rx_completion);
+	init_completion(&mcspi_dma->dma_tx_completion);
+
+	dma_cap_zero(mask);
+	dma_cap_set(DMA_SLAVE, mask);
+#ifdef USE_DMA_ENGINE_RX
+	sig = mcspi_dma->dma_rx_sync_dev;
+	mcspi_dma->dma_rx = dma_request_channel(mask, omap_dma_filter_fn, &sig);
+	if (!mcspi_dma->dma_rx) {
+		dev_err(&spi->dev, "no RX DMA engine channel for McSPI\n");
+		return -EAGAIN;
+	}
+#else
 	if (omap_request_dma(mcspi_dma->dma_rx_sync_dev, "McSPI RX",
 			omap2_mcspi_dma_rx_callback, spi,
 			&mcspi_dma->dma_rx_channel)) {
 		dev_err(&spi->dev, "no RX DMA channel for McSPI\n");
 		return -EAGAIN;
 	}
+#endif
 
+#ifdef USE_DMA_ENGINE_TX
+	sig = mcspi_dma->dma_tx_sync_dev;
+	mcspi_dma->dma_tx = dma_request_channel(mask, omap_dma_filter_fn, &sig);
+	if (!mcspi_dma->dma_tx) {
+		dev_err(&spi->dev, "no TX DMA engine channel for McSPI\n");
+		dma_release_channel(mcspi_dma->dma_rx);
+		mcspi_dma->dma_rx = NULL;
+		return -EAGAIN;
+	}
+#else
 	if (omap_request_dma(mcspi_dma->dma_tx_sync_dev, "McSPI TX",
 			omap2_mcspi_dma_tx_callback, spi,
 			&mcspi_dma->dma_tx_channel)) {
@@ -767,9 +881,7 @@ static int omap2_mcspi_request_dma(struct spi_device *spi)
 		dev_err(&spi->dev, "no TX DMA channel for McSPI\n");
 		return -EAGAIN;
 	}
-
-	init_completion(&mcspi_dma->dma_rx_completion);
-	init_completion(&mcspi_dma->dma_tx_completion);
+#endif
 
 	return 0;
 }
@@ -803,8 +915,8 @@ static int omap2_mcspi_setup(struct spi_device *spi)
 			&omap2_mcspi_ctx[mcspi->master->bus_num - 1].cs);
 	}
 
-	if (mcspi_dma->dma_rx_channel == -1
-			|| mcspi_dma->dma_tx_channel == -1) {
+	if ((!mcspi_dma->dma_rx && mcspi_dma->dma_rx_channel == -1) ||
+	    (!mcspi_dma->dma_tx && mcspi_dma->dma_tx_channel == -1)) {
 		ret = omap2_mcspi_request_dma(spi);
 		if (ret < 0)
 			return ret;
@@ -839,6 +951,14 @@ static void omap2_mcspi_cleanup(struct spi_device *spi)
 	if (spi->chip_select < spi->master->num_chipselect) {
 		mcspi_dma = &mcspi->dma_channels[spi->chip_select];
 
+		if (mcspi_dma->dma_rx) {
+			dma_release_channel(mcspi_dma->dma_rx);
+			mcspi_dma->dma_rx = NULL;
+		}
+		if (mcspi_dma->dma_tx) {
+			dma_release_channel(mcspi_dma->dma_tx);
+			mcspi_dma->dma_tx = NULL;
+		}
 		if (mcspi_dma->dma_rx_channel != -1) {
 			omap_free_dma(mcspi_dma->dma_rx_channel);
 			mcspi_dma->dma_rx_channel = -1;
#58 lore raw

cover DMA engine conversion

Quoted 10 lines · Russell King - ARM Linux

Here's a patch which converts the SPI driver to use dma engine support. It's more or less a mechanical replacement in this driver. ?Tested on OMAP4430SDP ethernet (which basically means that udhcpc works.)

One thing to note: the old DMA code programmed the DMA hardware for an element count of N and frame size of 1, with element sync. ?The DMA engine code will program the DMA hardware for an element count of 1 and a frame size of N, with frame sync. ?This appears to work - but at the moment I've not looked to see if there's any implications of this.

Quoted 8 lines

You can enable/disable the DMA engine code for the TX or RX side by tweaking the two definitions at the top of the file if you want to do comparisons.

Comments? ?If none, I'll see about committing this into the patch set, adding the removal of the private DMA code, and sending it out more "officially" with the next round of RFC for this set.

Element sync vs Frame sync change seems to be fine thought it's bit puzzling that why it was not that way first place.

Quoted 11 lines

Modulo the element vs frame sync issue, I think folk can see that the conversion process is fairly straight forward - it's more or less:
1. add struct dma_chan * pointer to the private structure.
2. add allocation/freeing functions for the DMA engine channels.
3. convert the callbacks to match the DMA engine callback prototype.
4. add code to set the DMA engine channel configuration appropriately ? (avoiding any unnecessary calls) via dmaengine_slave_config().
5. add code to queue the transfer via dmaengine_prep_slave_sg() and ? dmaengine_submit().
6. omap_start_dma() becomes dma_async_issue_pending().

Quoted 2 lines

?drivers/spi/spi-omap2-mcspi.c | ?186 +++++++++++++++++++++++++++++++++------- ?1 files changed, 153 insertions(+), 33 deletions(-)

The patch seems to be fine to me.

Am looping Shubro in case he has any comment on "why element sync was used ?" and also to test the patch with more peripherals connected over SPI.

Regards
Santosh

#59 lore raw

cover DMA engine conversion

Quoted 12 lines · Santosh
Quoted 12 lines

conversion process is fairly straight forward - it's more or less:

Quoted 11 lines

1. add struct dma_chan * pointer to the private structure.
2. add allocation/freeing functions for the DMA engine channels.
3. convert the callbacks to match the DMA engine callback prototype.
4. add code to set the DMA engine channel configuration appropriately (avoiding any unnecessary calls) via dmaengine_slave_config().
5. add code to queue the transfer via dmaengine_prep_slave_sg() and dmaengine_submit().
6. omap_start_dma() becomes dma_async_issue_pending().

 drivers/spi/spi-omap2-mcspi.c |  186 +++++++++++++++++++++++++++++++++-------
 1 files changed, 153 insertions(+), 33 deletions(-)
Quoted 7 lines

The patch seems to be fine to me.

Am looping Shubro in case he has any comment on "why element sync was used ?" and also to test the patch with more peripherals connected over SPI.

Regards
Santosh

Tested on omap4 ethernet and omap3 touchscreen
also did a spidev test.

Tested-by Shubhrajyoti <shubhrajyoti at ti.com <mailto:[email protected]>>