On Tue, 2011-07-26 at 18:31 +0900, Boojin Kim wrote:

Vinod Koul Wrote:

quoted

Sent: Monday, July 25, 2011 8:17 PM
To: Boojin Kim
Cc: vinod.koul at intel.com; linux-arm-kernel at lists.infradead.org; linux-
samsung-soc at vger.kernel.org; Kukjin Kim; Jassi Brar; Grant Likely;
Mark Brown; Dan Williams
Subject: Re: [PATCH V4 12/14] spi/s3c64xx: Add support DMA engine API

On Mon, 2011-07-25 at 10:28 +0900, Boojin Kim wrote:

quoted

This patch adds to support DMA generic API to transfer raw
SPI data. Basiclly the spi driver uses DMA generic API if
architecture supports it. Otherwise, uses Samsung specific
S3C-PL330 APIs.

Signed-off-by: Boojin Kim <redacted>
Cc: Grant Likely <redacted>
Signed-off-by: Kukjin Kim <redacted>
---
 drivers/spi/spi_s3c64xx.c |  141 ++++++++++++++++++++++-------------

----------

quoted

 1 files changed, 69 insertions(+), 72 deletions(-)

diff --git a/drivers/spi/spi_s3c64xx.c b/drivers/spi/spi_s3c64xx.c
index 8945e20..a4cf76a 100644
--- a/drivers/spi/spi_s3c64xx.c
+++ b/drivers/spi/spi_s3c64xx.c

@@ -172,6 +172,9 @@ struct s3c64xx_spi_driver_data {
 	unsigned                        state;
 	unsigned                        cur_mode, cur_bpw;
 	unsigned                        cur_speed;
+	unsigned			rx_ch;
+	unsigned			tx_ch;
+	struct samsung_dma_ops		*ops;
 };

 static struct s3c2410_dma_client s3c64xx_spi_dma_client = {

@@ -227,6 +230,38 @@ static void flush_fifo(struct

s3c64xx_spi_driver_data *sdd)

quoted

 	writel(val, regs + S3C64XX_SPI_CH_CFG);
 }

+static void s3c64xx_spi_dma_rxcb(void *data)
+{
+	struct s3c64xx_spi_driver_data *sdd
+		= (struct s3c64xx_spi_driver_data *)data;
+	unsigned long flags;
+
+	spin_lock_irqsave(&sdd->lock, flags);
+
+	sdd->state &= ~RXBUSY;
+	/* If the other done */
+	if (!(sdd->state & TXBUSY))
+		complete(&sdd->xfer_completion);
+
+	spin_unlock_irqrestore(&sdd->lock, flags);
+}
+
+static void s3c64xx_spi_dma_txcb(void *data)
+{
+	struct s3c64xx_spi_driver_data *sdd
+		= (struct s3c64xx_spi_driver_data *)data;
+	unsigned long flags;
+
+	spin_lock_irqsave(&sdd->lock, flags);
+
+	sdd->state &= ~TXBUSY;
+	/* If the other done */
+	if (!(sdd->state & RXBUSY))
+		complete(&sdd->xfer_completion);
+
+	spin_unlock_irqrestore(&sdd->lock, flags);
+}

I don't see much diff in these two functions and you should be able to
use a generic one which takes care of both TX and RX, does your
callback
data know if the channel is for TX or RX? If not then a helper to do
above should take care well and making code simpler

I'm very agree with you.
But, I think it isn't deeply related to this patch series. So, I wish to make 
and submit it after this patch series is finished.

Since you are reworking this driver would make sense to do now, it
doesn't sound to be a big change.

-- 
~Vinod Koul
Intel Corp.