[PATCH] s3cmci: port DMA code to dmaengine API

[PATCH] s3cmci: port DMA code to dmaengine API

[Vasily Khoruzhick]

Utilise new s3c24xx-dma dmaengine driver for DMA ops.

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 drivers/mmc/host/Kconfig  |   9 ---
 drivers/mmc/host/s3cmci.c | 183 +++++++++++++++-------------------------------
 drivers/mmc/host/s3cmci.h |   6 +-
 3 files changed, 61 insertions(+), 137 deletions(-)

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 1384f67..45c791c3 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -457,15 +457,6 @@ config MMC_S3C_DMA
 	  working properly and needs to be debugged before this
 	  option is useful.
 
-config MMC_S3C_PIODMA
-	bool "Support for both PIO and DMA"
-	help
-	  Compile both the PIO and DMA transfer routines into the
-	  driver and let the platform select at run-time which one
-	  is best.
-
-	  See notes for the DMA option.
-
 endchoice
 
 config MMC_SDRICOH_CS
diff --git a/drivers/mmc/host/s3cmci.c b/drivers/mmc/host/s3cmci.c
index f237826..9f32a30 100644
--- a/drivers/mmc/host/s3cmci.c
+++ b/drivers/mmc/host/s3cmci.c
@@ -12,6 +12,7 @@
  */
 
 #include <linux/module.h>
+#include <linux/dmaengine.h>
 #include <linux/dma-mapping.h>
 #include <linux/clk.h>
 #include <linux/mmc/host.h>
@@ -24,9 +25,9 @@
 #include <linux/io.h>
 
 #include <plat/gpio-cfg.h>
-#include <mach/dma.h>
 #include <mach/gpio-samsung.h>
 
+#include <linux/platform_data/dma-s3c24xx.h>
 #include <linux/platform_data/mmc-s3cmci.h>
 
 #include "s3cmci.h"
@@ -140,10 +141,6 @@ static const int dbgmap_debug = dbg_err | dbg_debug;
 		dev_dbg(&host->pdev->dev, args);  \
 	} while (0)
 
-static struct s3c2410_dma_client s3cmci_dma_client = {
-	.name		= "s3c-mci",
-};
-
 static void finalize_request(struct s3cmci_host *host);
 static void s3cmci_send_request(struct mmc_host *mmc);
 static void s3cmci_reset(struct s3cmci_host *host);
@@ -256,25 +253,8 @@ static inline bool s3cmci_host_usedma(struct s3cmci_host *host)
 {
 #ifdef CONFIG_MMC_S3C_PIO
 	return false;
-#elif defined(CONFIG_MMC_S3C_DMA)
-	return true;
-#else
-	return host->dodma;
-#endif
-}
-
-/**
- * s3cmci_host_canpio - return true if host has pio code available
- *
- * Return true if the driver has been compiled with the PIO support code
- * available.
- */
-static inline bool s3cmci_host_canpio(void)
-{
-#ifdef CONFIG_MMC_S3C_PIO
+#else /* CONFIG_MMC_S3C_DMA */
 	return true;
-#else
-	return false;
 #endif
 }
 
@@ -841,11 +821,9 @@ static irqreturn_t s3cmci_irq_cd(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static void s3cmci_dma_done_callback(struct s3c2410_dma_chan *dma_ch,
-				     void *buf_id, int size,
-				     enum s3c2410_dma_buffresult result)
+static void s3cmci_dma_done_callback(void *arg)
 {
-	struct s3cmci_host *host = buf_id;
+	struct s3cmci_host *host = arg;
 	unsigned long iflags;
 	u32 mci_csta, mci_dsta, mci_fsta, mci_dcnt;
 
@@ -856,45 +834,17 @@ static void s3cmci_dma_done_callback(struct s3c2410_dma_chan *dma_ch,
 
 	BUG_ON(!host->mrq);
 	BUG_ON(!host->mrq->data);
-	BUG_ON(!host->dmatogo);
 
 	spin_lock_irqsave(&host->complete_lock, iflags);
 
-	if (result != S3C2410_RES_OK) {
-		dbg(host, dbg_fail, "DMA FAILED: csta=0x%08x dsta=0x%08x "
-			"fsta=0x%08x dcnt:0x%08x result:0x%08x toGo:%u\n",
-			mci_csta, mci_dsta, mci_fsta,
-			mci_dcnt, result, host->dmatogo);
-
-		goto fail_request;
-	}
-
-	host->dmatogo--;
-	if (host->dmatogo) {
-		dbg(host, dbg_dma, "DMA DONE  Size:%i DSTA:[%08x] "
-			"DCNT:[%08x] toGo:%u\n",
-			size, mci_dsta, mci_dcnt, host->dmatogo);
-
-		goto out;
-	}
-
-	dbg(host, dbg_dma, "DMA FINISHED Size:%i DSTA:%08x DCNT:%08x\n",
-		size, mci_dsta, mci_dcnt);
+	dbg(host, dbg_dma, "DMA FINISHED\n");
 
 	host->dma_complete = 1;
 	host->complete_what = COMPLETION_FINALIZE;
 
-out:
 	tasklet_schedule(&host->pio_tasklet);
 	spin_unlock_irqrestore(&host->complete_lock, iflags);
-	return;
 
-fail_request:
-	host->mrq->data->error = -EINVAL;
-	host->complete_what = COMPLETION_FINALIZE;
-	clear_imask(host);
-
-	goto out;
 }
 
 static void finalize_request(struct s3cmci_host *host)
@@ -966,7 +916,7 @@ static void finalize_request(struct s3cmci_host *host)
 	 * DMA channel and the fifo to clear out any garbage. */
 	if (mrq->data->error != 0) {
 		if (s3cmci_host_usedma(host))
-			s3c2410_dma_ctrl(host->dma, S3C2410_DMAOP_FLUSH);
+			dmaengine_terminate_all(host->dma);
 
 		if (host->is2440) {
 			/* Clear failure register and reset fifo. */
@@ -992,29 +942,6 @@ request_done:
 	mmc_request_done(host->mmc, mrq);
 }
 
-static void s3cmci_dma_setup(struct s3cmci_host *host,
-			     enum dma_data_direction source)
-{
-	static enum dma_data_direction last_source = -1;
-	static int setup_ok;
-
-	if (last_source == source)
-		return;
-
-	last_source = source;
-
-	s3c2410_dma_devconfig(host->dma, source,
-			      host->mem->start + host->sdidata);
-
-	if (!setup_ok) {
-		s3c2410_dma_config(host->dma, 4);
-		s3c2410_dma_set_buffdone_fn(host->dma,
-					    s3cmci_dma_done_callback);
-		s3c2410_dma_setflags(host->dma, S3C2410_DMAF_AUTOSTART);
-		setup_ok = 1;
-	}
-}
-
 static void s3cmci_send_command(struct s3cmci_host *host,
 					struct mmc_command *cmd)
 {
@@ -1162,43 +1089,44 @@ static int s3cmci_prepare_pio(struct s3cmci_host *host, struct mmc_data *data)
 
 static int s3cmci_prepare_dma(struct s3cmci_host *host, struct mmc_data *data)
 {
-	int dma_len, i;
 	int rw = data->flags & MMC_DATA_WRITE;
+	struct dma_slave_config conf = {
+		.src_addr = host->mem->start + host->sdidata,
+		.dst_addr = host->mem->start + host->sdidata,
+		.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES,
+		.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES,
+	};
 
 	BUG_ON((data->flags & BOTH_DIR) == BOTH_DIR);
 
-	s3cmci_dma_setup(host, rw ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
-	s3c2410_dma_ctrl(host->dma, S3C2410_DMAOP_FLUSH);
-
-	dma_len = dma_map_sg(mmc_dev(host->mmc), data->sg, data->sg_len,
-			     rw ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
-
-	if (dma_len == 0)
-		return -ENOMEM;
-
-	host->dma_complete = 0;
-	host->dmatogo = dma_len;
-
-	for (i = 0; i < dma_len; i++) {
-		int res;
-
-		dbg(host, dbg_dma, "enqueue %i: %08x@%u\n", i,
-		    sg_dma_address(&data->sg[i]),
-		    sg_dma_len(&data->sg[i]));
+	/* Restore prescaler value */
+	writel(host->prescaler, host->base + S3C2410_SDIPRE);
 
-		res = s3c2410_dma_enqueue(host->dma, host,
-					  sg_dma_address(&data->sg[i]),
-					  sg_dma_len(&data->sg[i]));
+	if (!rw)
+		conf.direction = DMA_DEV_TO_MEM;
+	else
+		conf.direction = DMA_MEM_TO_DEV;
 
-		if (res) {
-			s3c2410_dma_ctrl(host->dma, S3C2410_DMAOP_FLUSH);
-			return -EBUSY;
-		}
-	}
+	dma_map_sg(mmc_dev(host->mmc), data->sg, data->sg_len,
+			     rw ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
 
-	s3c2410_dma_ctrl(host->dma, S3C2410_DMAOP_START);
+	dmaengine_slave_config(host->dma, &conf);
+	host->desc = dmaengine_prep_slave_sg(host->dma, data->sg, data->sg_len,
+		conf.direction,
+		DMA_CTRL_ACK | DMA_PREP_INTERRUPT);
+	if (!host->desc)
+		goto unmap_exit;
+	host->desc->callback = s3cmci_dma_done_callback;
+	host->desc->callback_param = host;
+	dmaengine_submit(host->desc);
+	dma_async_issue_pending(host->dma);
 
 	return 0;
+
+unmap_exit:
+	dma_unmap_sg(mmc_dev(host->mmc), data->sg, data->sg_len,
+			     rw ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
+	return -ENOMEM;
 }
 
 static void s3cmci_send_request(struct mmc_host *mmc)
@@ -1676,10 +1604,6 @@ static int s3cmci_probe(struct platform_device *pdev)
 	host->complete_what 	= COMPLETION_NONE;
 	host->pio_active 	= XFER_NONE;
 
-#ifdef CONFIG_MMC_S3C_PIODMA
-	host->dodma		= host->pdata->use_dma;
-#endif
-
 	host->mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!host->mem) {
 		dev_err(&pdev->dev,
@@ -1765,17 +1689,26 @@ static int s3cmci_probe(struct platform_device *pdev)
 	/* depending on the dma state, get a dma channel to use. */
 
 	if (s3cmci_host_usedma(host)) {
-		host->dma = s3c2410_dma_request(DMACH_SDI, &s3cmci_dma_client,
-						host);
-		if (host->dma < 0) {
+		struct resource *res;
+		dma_cap_mask_t mask;
+
+		res = platform_get_resource(pdev, IORESOURCE_DMA, 0);
+		if (!res) {
+			dev_err(&pdev->dev,
+				"failed to get dma resource.\n");
+			ret = -ENOENT;
+			goto probe_free_gpio_wp;
+		}
+
+		dma_cap_zero(mask);
+		dma_cap_set(DMA_SLAVE, mask);
+
+		host->dma = dma_request_channel(mask,
+			s3c24xx_dma_filter, (void *)res->start);
+		if (!host->dma) {
 			dev_err(&pdev->dev, "cannot get DMA channel.\n");
-			if (!s3cmci_host_canpio()) {
-				ret = -EBUSY;
-				goto probe_free_gpio_wp;
-			} else {
-				dev_warn(&pdev->dev, "falling back to PIO.\n");
-				host->dodma = 0;
-			}
+			ret = -EBUSY;
+			goto probe_free_gpio_wp;
 		}
 	}
 
@@ -1816,7 +1749,7 @@ static int s3cmci_probe(struct platform_device *pdev)
 	mmc->max_segs		= 128;
 
 	dbg(host, dbg_debug,
-	    "probe: mode:%s mapped mci_base:%p irq:%u irq_cd:%u dma:%u.\n",
+	    "probe: mode:%s mapped mci_base:%p irq:%u irq_cd:%u dma:%p.\n",
 	    (host->is2440?"2440":""),
 	    host->base, host->irq, host->irq_cd, host->dma);
 
@@ -1852,7 +1785,7 @@ static int s3cmci_probe(struct platform_device *pdev)
 
  probe_free_dma:
 	if (s3cmci_host_usedma(host))
-		s3c2410_dma_free(host->dma, &s3cmci_dma_client);
+		dma_release_channel(host->dma);
 
  probe_free_gpio_wp:
 	if (!host->pdata->no_wprotect)
@@ -1914,7 +1847,7 @@ static int s3cmci_remove(struct platform_device *pdev)
 	tasklet_disable(&host->pio_tasklet);
 
 	if (s3cmci_host_usedma(host))
-		s3c2410_dma_free(host->dma, &s3cmci_dma_client);
+		dma_release_channel(host->dma);
 
 	free_irq(host->irq, host);
 
diff --git a/drivers/mmc/host/s3cmci.h b/drivers/mmc/host/s3cmci.h
index c76b53d..09dbbcd 100644
--- a/drivers/mmc/host/s3cmci.h
+++ b/drivers/mmc/host/s3cmci.h
@@ -26,7 +26,9 @@ struct s3cmci_host {
 	void __iomem		*base;
 	int			irq;
 	int			irq_cd;
-	int			dma;
+	struct dma_chan		*dma;
+	struct dma_async_tx_descriptor
+				*desc;
 
 	unsigned long		clk_rate;
 	unsigned long		clk_div;
@@ -36,8 +38,6 @@ struct s3cmci_host {
 	int			is2440;
 	unsigned		sdiimsk;
 	unsigned		sdidata;
-	int			dodma;
-	int			dmatogo;
 
 	bool			irq_disabled;
 	bool			irq_enabled;
-- 
1.9.3

[PATCH] s3cmci: port DMA code to dmaengine API

[Arnd Bergmann]

On Monday 19 May 2014 21:03:13 Vasily Khoruzhick wrote:
> Utilise new s3c24xx-dma dmaengine driver for DMA ops.
> 
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>

Very nice!

I had done a similar conversion earlier but not tested and only
posted in a reply to another thread. I'd assume that your
version is better and that you have actually tested it, but just
for reference, here is what I had. Maybe it helps you to take
a look at my version to see if there is something you missed.

Here are the differences I found:

- I don't save the 'desc' pointer as you do, but I don't see it used
  anywhere.
- I did not remove MMC_S3C_PIODMA, but I don't see anything wrong
  with yours there
- the slave config setup is slightly different
- I hardcode the DMA request number, while you pass it as a resource.
  Actually both are wrong I guess, it should be platform data instead.
 
	Arnd

diff --git a/drivers/mmc/host/s3cmci.c b/drivers/mmc/host/s3cmci.c
index f237826..02847d3 100644
--- a/drivers/mmc/host/s3cmci.c
+++ b/drivers/mmc/host/s3cmci.c
@@ -14,6 +14,7 @@
 #include <linux/module.h>
 #include <linux/dma-mapping.h>
 #include <linux/clk.h>
+#include <linux/dmaengine.h>
 #include <linux/mmc/host.h>
 #include <linux/platform_device.h>
 #include <linux/cpufreq.h>
@@ -28,6 +29,7 @@
 #include <mach/gpio-samsung.h>
 
 #include <linux/platform_data/mmc-s3cmci.h>
+#include <linux/platform_data/dma-s3c24xx.h>
 
 #include "s3cmci.h"
 
@@ -140,10 +142,6 @@ static const int dbgmap_debug = dbg_err | dbg_debug;
 		dev_dbg(&host->pdev->dev, args);  \
 	} while (0)
 
-static struct s3c2410_dma_client s3cmci_dma_client = {
-	.name		= "s3c-mci",
-};
-
 static void finalize_request(struct s3cmci_host *host);
 static void s3cmci_send_request(struct mmc_host *mmc);
 static void s3cmci_reset(struct s3cmci_host *host);
@@ -841,9 +839,7 @@ static irqreturn_t s3cmci_irq_cd(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static void s3cmci_dma_done_callback(struct s3c2410_dma_chan *dma_ch,
-				     void *buf_id, int size,
-				     enum s3c2410_dma_buffresult result)
+static void s3cmci_dma_done_callback(void *buf_id)
 {
 	struct s3cmci_host *host = buf_id;
 	unsigned long iflags;
@@ -856,45 +852,18 @@ static void s3cmci_dma_done_callback(struct s3c2410_dma_chan *dma_ch,
 
 	BUG_ON(!host->mrq);
 	BUG_ON(!host->mrq->data);
-	BUG_ON(!host->dmatogo);
 
 	spin_lock_irqsave(&host->complete_lock, iflags);
 
-	if (result != S3C2410_RES_OK) {
-		dbg(host, dbg_fail, "DMA FAILED: csta=0x%08x dsta=0x%08x "
-			"fsta=0x%08x dcnt:0x%08x result:0x%08x toGo:%u\n",
-			mci_csta, mci_dsta, mci_fsta,
-			mci_dcnt, result, host->dmatogo);
-
-		goto fail_request;
-	}
-
-	host->dmatogo--;
-	if (host->dmatogo) {
-		dbg(host, dbg_dma, "DMA DONE  Size:%i DSTA:[%08x] "
-			"DCNT:[%08x] toGo:%u\n",
-			size, mci_dsta, mci_dcnt, host->dmatogo);
-
-		goto out;
-	}
-
-	dbg(host, dbg_dma, "DMA FINISHED Size:%i DSTA:%08x DCNT:%08x\n",
-		size, mci_dsta, mci_dcnt);
+	dbg(host, dbg_dma, "DMA FINISHED DSTA:%08x DCNT:%08x\n",
+		mci_dsta, mci_dcnt);
 
 	host->dma_complete = 1;
 	host->complete_what = COMPLETION_FINALIZE;
 
-out:
 	tasklet_schedule(&host->pio_tasklet);
 	spin_unlock_irqrestore(&host->complete_lock, iflags);
 	return;
-
-fail_request:
-	host->mrq->data->error = -EINVAL;
-	host->complete_what = COMPLETION_FINALIZE;
-	clear_imask(host);
-
-	goto out;
 }
 
 static void finalize_request(struct s3cmci_host *host)
@@ -966,7 +935,7 @@ static void finalize_request(struct s3cmci_host *host)
 	 * DMA channel and the fifo to clear out any garbage. */
 	if (mrq->data->error != 0) {
 		if (s3cmci_host_usedma(host))
-			s3c2410_dma_ctrl(host->dma, S3C2410_DMAOP_FLUSH);
+			dmaengine_terminate_all(host->dma);
 
 		if (host->is2440) {
 			/* Clear failure register and reset fifo. */
@@ -993,26 +962,27 @@ request_done:
 }
 
 static void s3cmci_dma_setup(struct s3cmci_host *host,
-			     enum dma_data_direction source)
+			     enum dma_transfer_direction source)
 {
-	static enum dma_data_direction last_source = -1;
-	static int setup_ok;
+	static enum dma_transfer_direction last_source = -1;
+	struct dma_slave_config slave_config;
 
 	if (last_source == source)
 		return;
 
-	last_source = source;
-
-	s3c2410_dma_devconfig(host->dma, source,
-			      host->mem->start + host->sdidata);
-
-	if (!setup_ok) {
-		s3c2410_dma_config(host->dma, 4);
-		s3c2410_dma_set_buffdone_fn(host->dma,
-					    s3cmci_dma_done_callback);
-		s3c2410_dma_setflags(host->dma, S3C2410_DMAF_AUTOSTART);
-		setup_ok = 1;
+	memset(&slave_config, 0, sizeof(struct dma_slave_config));
+	slave_config.direction = source;
+	if (source == DMA_DEV_TO_MEM) {
+		slave_config.src_addr = host->mem->start + host->sdidata;
+		slave_config.src_addr_width = 4;
+		slave_config.src_maxburst = 1;
+	} else {
+		slave_config.dst_addr = host->mem->start + host->sdidata;
+		slave_config.dst_addr_width = 4;
+		slave_config.dst_maxburst = 1;
 	}
+	last_source = source;
+	dmaengine_slave_config(host->dma, &slave_config);
 }
 
 static void s3cmci_send_command(struct s3cmci_host *host,
@@ -1162,41 +1132,33 @@ static int s3cmci_prepare_pio(struct s3cmci_host *host, struct mmc_data *data)
 
 static int s3cmci_prepare_dma(struct s3cmci_host *host, struct mmc_data *data)
 {
-	int dma_len, i;
-	int rw = data->flags & MMC_DATA_WRITE;
+	int dma_len;
+	int dir = data->flags & MMC_DATA_WRITE ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
+	struct dma_async_tx_descriptor *desc;
 
 	BUG_ON((data->flags & BOTH_DIR) == BOTH_DIR);
 
-	s3cmci_dma_setup(host, rw ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
-	s3c2410_dma_ctrl(host->dma, S3C2410_DMAOP_FLUSH);
-
-	dma_len = dma_map_sg(mmc_dev(host->mmc), data->sg, data->sg_len,
-			     rw ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
+	s3cmci_dma_setup(host, dir);
 
+	dma_len = dma_map_sg(mmc_dev(host->mmc), data->sg, data->sg_len, dir);
 	if (dma_len == 0)
 		return -ENOMEM;
 
-	host->dma_complete = 0;
-	host->dmatogo = dma_len;
-
-	for (i = 0; i < dma_len; i++) {
-		int res;
+	desc = dmaengine_prep_slave_sg(host->dma, data->sg, dma_len,
+				       dir, DMA_CTRL_ACK);
 
-		dbg(host, dbg_dma, "enqueue %i: %08x@%u\n", i,
-		    sg_dma_address(&data->sg[i]),
-		    sg_dma_len(&data->sg[i]));
+	if (!desc) {
+		dma_unmap_sg(mmc_dev(host->mmc), data->sg, dma_len, dir);
+		return -EIO;
+	}
 
-		res = s3c2410_dma_enqueue(host->dma, host,
-					  sg_dma_address(&data->sg[i]),
-					  sg_dma_len(&data->sg[i]));
+	host->dma_complete = 0;
 
-		if (res) {
-			s3c2410_dma_ctrl(host->dma, S3C2410_DMAOP_FLUSH);
-			return -EBUSY;
-		}
-	}
+	desc->callback = s3cmci_dma_done_callback;
+	desc->callback_param = host;
 
-	s3c2410_dma_ctrl(host->dma, S3C2410_DMAOP_START);
+	dmaengine_submit(desc);
+	dma_async_issue_pending(host->dma);
 
 	return 0;
 }
@@ -1765,9 +1727,14 @@ static int s3cmci_probe(struct platform_device *pdev)
 	/* depending on the dma state, get a dma channel to use. */
 
 	if (s3cmci_host_usedma(host)) {
-		host->dma = s3c2410_dma_request(DMACH_SDI, &s3cmci_dma_client,
-						host);
-		if (host->dma < 0) {
+		dma_cap_mask_t mask;
+        
+		dma_cap_zero(mask);
+		dma_cap_set(DMA_SLAVE, mask);
+
+		host->dma = dma_request_channel(mask, s3c24xx_dma_filter, (void *)DMACH_SDI);
+
+		if (!host->dma) {
 			dev_err(&pdev->dev, "cannot get DMA channel.\n");
 			if (!s3cmci_host_canpio()) {
 				ret = -EBUSY;
@@ -1816,9 +1783,9 @@ static int s3cmci_probe(struct platform_device *pdev)
 	mmc->max_segs		= 128;
 
 	dbg(host, dbg_debug,
-	    "probe: mode:%s mapped mci_base:%p irq:%u irq_cd:%u dma:%u.\n",
+	    "probe: mode:%s mapped mci_base:%p irq:%u irq_cd:%u.\n",
 	    (host->is2440?"2440":""),
-	    host->base, host->irq, host->irq_cd, host->dma);
+	    host->base, host->irq, host->irq_cd);
 
 	ret = s3cmci_cpufreq_register(host);
 	if (ret) {
@@ -1852,7 +1819,7 @@ static int s3cmci_probe(struct platform_device *pdev)
 
  probe_free_dma:
 	if (s3cmci_host_usedma(host))
-		s3c2410_dma_free(host->dma, &s3cmci_dma_client);
+		dma_release_channel(host->dma);
 
  probe_free_gpio_wp:
 	if (!host->pdata->no_wprotect)
@@ -1914,7 +1881,7 @@ static int s3cmci_remove(struct platform_device *pdev)
 	tasklet_disable(&host->pio_tasklet);
 
 	if (s3cmci_host_usedma(host))
-		s3c2410_dma_free(host->dma, &s3cmci_dma_client);
+		dma_release_channel(host->dma);
 
 	free_irq(host->irq, host);
 
diff --git a/drivers/mmc/host/s3cmci.h b/drivers/mmc/host/s3cmci.h
index c76b53d..514a887 100644
--- a/drivers/mmc/host/s3cmci.h
+++ b/drivers/mmc/host/s3cmci.h
@@ -26,7 +26,7 @@ struct s3cmci_host {
 	void __iomem		*base;
 	int			irq;
 	int			irq_cd;
-	int			dma;
+	struct dma_chan		*dma;
 
 	unsigned long		clk_rate;
 	unsigned long		clk_div;

[PATCH] s3cmci: port DMA code to dmaengine API

[Vasily Khoruzhick]

On Mon, May 19, 2014 at 9:18 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 19 May 2014 21:03:13 Vasily Khoruzhick wrote:
>> Utilise new s3c24xx-dma dmaengine driver for DMA ops.
>>
>> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>

Hi Arnd,

> Very nice!

Thanks! :)

> I had done a similar conversion earlier but not tested and only
> posted in a reply to another thread. I'd assume that your
> version is better and that you have actually tested it, but just
> for reference, here is what I had. Maybe it helps you to take
> a look at my version to see if there is something you missed.

Hm, I did search for s3c24xx-i2s conversion, but didn't for s3cmci, my fault.
Your version looks pretty close to mine, however I don't think that
much variations
are possible here. I've tested mine on s3c2410 machine.

> Here are the differences I found:
>
> - I don't save the 'desc' pointer as you do, but I don't see it used
>   anywhere.

Yeah, I should remove it. Will send v2 shortly.

> - I did not remove MMC_S3C_PIODMA, but I don't see anything wrong
>   with yours there

It didn't work even with legacy code for me. I don't want to fix it,
so it's just dropped.
Anyway, I don't see much sense in PIODMA mode.

> - the slave config setup is slightly different

Yep.

> - I hardcode the DMA request number, while you pass it as a resource.
>   Actually both are wrong I guess, it should be platform data instead.

Well, at least s3c64xx uses resources for passing dma channel number,
and so did I.
Btw, It'll make transition to device tree a bit easier.

There's one more difference: original s3cmci doesn't restore prescaler
value when necessary on s3c2410,
my version has a fix for it.

>         Arnd

Regards
Vasily

[PATCH] s3cmci: port DMA code to dmaengine API

[Arnd Bergmann]

On Monday 19 May 2014 21:54:28 Vasily Khoruzhick wrote:
> On Mon, May 19, 2014 at 9:18 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Monday 19 May 2014 21:03:13 Vasily Khoruzhick wrote:
> >> Utilise new s3c24xx-dma dmaengine driver for DMA ops.
> >>
> >> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> 
> > - I did not remove MMC_S3C_PIODMA, but I don't see anything wrong
> >   with yours there
> 
> It didn't work even with legacy code for me. I don't want to fix it,
> so it's just dropped.
> Anyway, I don't see much sense in PIODMA mode.

Ah, that makes sense.

> > - the slave config setup is slightly different
> 
> Yep.
> 
> > - I hardcode the DMA request number, while you pass it as a resource.
> >   Actually both are wrong I guess, it should be platform data instead.
> 
> Well, at least s3c64xx uses resources for passing dma channel number,
> and so did I.
> Btw, It'll make transition to device tree a bit easier.

Actually it doesn't help with the transition to DT at all, because DT
does not use resources for DMA requests. Instead there is a
dma_request_slave_channel() interface that gets passed a string to
match the DT descriptor of the DMA channel. If you want to enable
DT probing while keeping the traditional way alive, you can
use dma_request_slave_channel_compat(), although I find it not
much easier than open-coding it.

There is one issue with dma_request_slave_channel_compat and
dma_request_channel, which is that you need a pointer to the
filter function. In portable drivers that pointer should get passed
in platform_data for the non-DT case, along with the data it
needs, because there are dma engines that need more than just
an integer to identify a slave.

For this driver, you don't have to go that far, as long as it's
ensure that the pointer to the filter function is available to
the driver, i.e. you can't have a built-in s3mci driver when the
dmaengine driver is a loadable module.

> There's one more difference: original s3cmci doesn't restore prescaler
> value when necessary on s3c2410,
> my version has a fix for it.

Ok.

	Arnd

[PATCH] s3cmci: port DMA code to dmaengine API

[Vasily Khoruzhick]

On Mon, May 19, 2014 at 10:02 PM, Arnd Bergmann <arnd@arndb.de> wrote:

>> Well, at least s3c64xx uses resources for passing dma channel number,
>> and so did I.
>> Btw, It'll make transition to device tree a bit easier.
>
> Actually it doesn't help with the transition to DT at all, because DT
> does not use resources for DMA requests. Instead there is a
> dma_request_slave_channel() interface that gets passed a string to
> match the DT descriptor of the DMA channel. If you want to enable
> DT probing while keeping the traditional way alive, you can
> use dma_request_slave_channel_compat(), although I find it not
> much easier than open-coding it.
>
> There is one issue with dma_request_slave_channel_compat and
> dma_request_channel, which is that you need a pointer to the
> filter function. In portable drivers that pointer should get passed
> in platform_data for the non-DT case, along with the data it
> needs, because there are dma engines that need more than just
> an integer to identify a slave.
>
> For this driver, you don't have to go that far, as long as it's
> ensure that the pointer to the filter function is available to
> the driver, i.e. you can't have a built-in s3mci driver when the
> dmaengine driver is a loadable module.

I'll take a look at dma_request_slave_channel_compat(), thanks for a hint!

Regards,
Vasily

[PATCH] s3cmci: port DMA code to dmaengine API

[Vasily Khoruzhick]

Hi Arnd,

On Mon, May 19, 2014 at 10:02 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> For this driver, you don't have to go that far, as long as it's
> ensure that the pointer to the filter function is available to
> the driver, i.e. you can't have a built-in s3mci driver when the
> dmaengine driver is a loadable module.

OK, so I guess I need to add explicit dependency on
CONFIG_S3C24XX_DMAC for s3cmci driver.

Btw, I didn't understand if it's acceptable to pass DMA channel number
through DMA resource for non-DT case or not?

Regards
Vasily

[PATCH] s3cmci: port DMA code to dmaengine API

[Arnd Bergmann]

On Tuesday 20 May 2014 13:22:35 Vasily Khoruzhick wrote:
> Hi Arnd,
> 
> On Mon, May 19, 2014 at 10:02 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > For this driver, you don't have to go that far, as long as it's
> > ensure that the pointer to the filter function is available to
> > the driver, i.e. you can't have a built-in s3mci driver when the
> > dmaengine driver is a loadable module.
> 
> OK, so I guess I need to add explicit dependency on
> CONFIG_S3C24XX_DMAC for s3cmci driver.
> 
> Btw, I didn't understand if it's acceptable to pass DMA channel number
> through DMA resource for non-DT case or not?

It's commonly done on certain SoCs, but I'd prefer to not start doing
it on those that don't do it today.

At the moment, we do it only on s3c64xx, s5p, davinci, omap1, and pxa
on ARM, as well as arch/blackfin and one MIPS machine.
I suppose we have to introduce it on s3c24xx in order to keep
supporting sound, unless we put the audio dma channels into
s3c_audio_pdata.

I think that would be a better approach, given that you also need
to put the filter function pointer for the audio stuff somewhere.

Maybe Mark Brown has a strong preference to how he wants this done
in the audio drivers, then you can do it the same way for s3cmci.


	Arnd

[PATCH] s3cmci: port DMA code to dmaengine API

[Vasily Khoruzhick]

On Tue, May 20, 2014 at 1:39 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 20 May 2014 13:22:35 Vasily Khoruzhick wrote:
>> Hi Arnd,
>>
>> On Mon, May 19, 2014 at 10:02 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > For this driver, you don't have to go that far, as long as it's
>> > ensure that the pointer to the filter function is available to
>> > the driver, i.e. you can't have a built-in s3mci driver when the
>> > dmaengine driver is a loadable module.
>>
>> OK, so I guess I need to add explicit dependency on
>> CONFIG_S3C24XX_DMAC for s3cmci driver.
>>
>> Btw, I didn't understand if it's acceptable to pass DMA channel number
>> through DMA resource for non-DT case or not?
>
> It's commonly done on certain SoCs, but I'd prefer to not start doing
> it on those that don't do it today.
>
> At the moment, we do it only on s3c64xx, s5p, davinci, omap1, and pxa
> on ARM, as well as arch/blackfin and one MIPS machine.
> I suppose we have to introduce it on s3c24xx in order to keep
> supporting sound, unless we put the audio dma channels into
> s3c_audio_pdata.
>
> I think that would be a better approach, given that you also need
> to put the filter function pointer for the audio stuff somewhere.
>
> Maybe Mark Brown has a strong preference to how he wants this done
> in the audio drivers, then you can do it the same way for s3cmci.

Let's just leave channel number hardcoded, how it was before conversion.
It doesn't seem to be a good idea to clean non-DT machine code now, since
it's better to put some effort into converting s3c2410 and s3c244{0,2} to DT.

Regards
Vasily

>
>         Arnd

[PATCH] s3cmci: port DMA code to dmaengine API

[Arnd Bergmann]

On Tuesday 20 May 2014 14:36:49 Vasily Khoruzhick wrote:
> On Tue, May 20, 2014 at 1:39 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Tuesday 20 May 2014 13:22:35 Vasily Khoruzhick wrote:
> >> Hi Arnd,
> >>
> >> On Mon, May 19, 2014 at 10:02 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> > For this driver, you don't have to go that far, as long as it's
> >> > ensure that the pointer to the filter function is available to
> >> > the driver, i.e. you can't have a built-in s3mci driver when the
> >> > dmaengine driver is a loadable module.
> >>
> >> OK, so I guess I need to add explicit dependency on
> >> CONFIG_S3C24XX_DMAC for s3cmci driver.
> >>
> >> Btw, I didn't understand if it's acceptable to pass DMA channel number
> >> through DMA resource for non-DT case or not?
> >
> > It's commonly done on certain SoCs, but I'd prefer to not start doing
> > it on those that don't do it today.
> >
> > At the moment, we do it only on s3c64xx, s5p, davinci, omap1, and pxa
> > on ARM, as well as arch/blackfin and one MIPS machine.
> > I suppose we have to introduce it on s3c24xx in order to keep
> > supporting sound, unless we put the audio dma channels into
> > s3c_audio_pdata.
> >
> > I think that would be a better approach, given that you also need
> > to put the filter function pointer for the audio stuff somewhere.
> >
> > Maybe Mark Brown has a strong preference to how he wants this done
> > in the audio drivers, then you can do it the same way for s3cmci.
> 
> Let's just leave channel number hardcoded, how it was before conversion.
> It doesn't seem to be a good idea to clean non-DT machine code now, since
> it's better to put some effort into converting s3c2410 and s3c244{0,2} to DT.

Ok, fair enough.

I'm actually more interested in making s3c24xx multiplatform capable than
moving it to DT, but we that requires a few other patches as well, and
we can fix this one along with those.

	Arnd

[PATCH] s3cmci: port DMA code to dmaengine API

[Vasily Khoruzhick]

On Tue, May 20, 2014 at 2:51 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> Let's just leave channel number hardcoded, how it was before conversion.
>> It doesn't seem to be a good idea to clean non-DT machine code now, since
>> it's better to put some effort into converting s3c2410 and s3c244{0,2} to DT.
>
> Ok, fair enough.
>
> I'm actually more interested in making s3c24xx multiplatform capable than
> moving it to DT, but we that requires a few other patches as well, and
> we can fix this one along with those.

Hm, but s3c2410, s3c2440 and s3c2442 are armv4t. Is it possible at all
to make them multiplatform?

Regards
Vasily

>         Arnd

[PATCH] s3cmci: port DMA code to dmaengine API

[Arnd Bergmann]

On Tuesday 20 May 2014 14:55:40 Vasily Khoruzhick wrote:
> On Tue, May 20, 2014 at 2:51 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> Let's just leave channel number hardcoded, how it was before conversion.
> >> It doesn't seem to be a good idea to clean non-DT machine code now, since
> >> it's better to put some effort into converting s3c2410 and s3c244{0,2} to DT.
> >
> > Ok, fair enough.
> >
> > I'm actually more interested in making s3c24xx multiplatform capable than
> > moving it to DT, but we that requires a few other patches as well, and
> > we can fix this one along with those.
> 
> Hm, but s3c2410, s3c2440 and s3c2442 are armv4t. Is it possible at all
> to make them multiplatform?

Yes, of course. We can run any combination of armv4, armv4t and armv5
CPUs in a multiplatform, we just can't combine them with armv6, armv6k
or armv7.

There is an interesting dependency here: for armv6/v7, we have multiple
Samsung platforms that are mutually exclusive as long as they use ATAGS
based boot: s3c64xx, s5p64xx, s5pc100, s5pv210 and exynos, because the
plat-samsung directory uses mutually exclusive options to pick which set
of mach/*.h headers it uses. The plan here is to move s5pv210 to DT
(same as Exynos already is) and delete s5p64xx and s5pc100, which don't
seem to be used by anybody really. Then all that is left is s3c64xx
using board files and all the non-DT code from plat-samsung becomes
s3c64xx-only.

For s3c24xx, we don't have that problem at all, we can just build
plat-samsung with the s3c24xx headers into the same kernel as things
like mxs, imx1/2, kirkwood, orion, or versatile.

	Arnd