[PATCH 1/6] SPI: S3C64XX: Fix compilation warning

[PATCH 1/6] SPI: S3C64XX: Fix compilation warning

[Jassi Brar]

Fix compilation warning by typecasting the tx_buf pointer.

Signed-off-by: Jassi Brar <jassi.brar-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/spi/spi_s3c64xx.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi_s3c64xx.c b/drivers/spi/spi_s3c64xx.c
index 9736581..774e861 100644
--- a/drivers/spi/spi_s3c64xx.c
+++ b/drivers/spi/spi_s3c64xx.c
@@ -508,8 +508,9 @@ static int s3c64xx_spi_map_mssg(struct s3c64xx_spi_driver_data *sdd,
 	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
 
 		if (xfer->tx_buf != NULL) {
-			xfer->tx_dma = dma_map_single(dev, xfer->tx_buf,
-						xfer->len, DMA_TO_DEVICE);
+			xfer->tx_dma = dma_map_single(dev,
+					(void *)xfer->tx_buf, xfer->len,
+					DMA_TO_DEVICE);
 			if (dma_mapping_error(dev, xfer->tx_dma)) {
 				dev_err(dev, "dma_map_single Tx failed\n");
 				xfer->tx_dma = XFER_DMAADDR_INVALID;
-- 
1.6.2.5


------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:

Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd

Re: [PATCH 1/6] SPI: S3C64XX: Fix compilation warning

[Grant Likely]

On Fri, Sep 03, 2010 at 10:36:26AM +0900, Jassi Brar wrote:
> Fix compilation warning by typecasting the tx_buf pointer.
> 
> Signed-off-by: Jassi Brar <jassi.brar-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

Applied to spi-merge, thanks.

g.

> ---
>  drivers/spi/spi_s3c64xx.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/spi/spi_s3c64xx.c b/drivers/spi/spi_s3c64xx.c
> index 9736581..774e861 100644
> --- a/drivers/spi/spi_s3c64xx.c
> +++ b/drivers/spi/spi_s3c64xx.c
> @@ -508,8 +508,9 @@ static int s3c64xx_spi_map_mssg(struct s3c64xx_spi_driver_data *sdd,
>  	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
>  
>  		if (xfer->tx_buf != NULL) {
> -			xfer->tx_dma = dma_map_single(dev, xfer->tx_buf,
> -						xfer->len, DMA_TO_DEVICE);
> +			xfer->tx_dma = dma_map_single(dev,
> +					(void *)xfer->tx_buf, xfer->len,
> +					DMA_TO_DEVICE);
>  			if (dma_mapping_error(dev, xfer->tx_dma)) {
>  				dev_err(dev, "dma_map_single Tx failed\n");
>  				xfer->tx_dma = XFER_DMAADDR_INVALID;
> -- 
> 1.6.2.5
> 

------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:

Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd

[PATCH 2/6] SPI: S3C64XX: Prevent unnecessary map-unmap

[Jassi Brar]

Since we use DMA mode only for xfers bigger than FIFO size,
do not map/unmap buffers for polling mode transfers.

Signed-off-by: Jassi Brar <jassi.brar-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/spi/spi_s3c64xx.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/spi/spi_s3c64xx.c b/drivers/spi/spi_s3c64xx.c
index 774e861..12e7915 100644
--- a/drivers/spi/spi_s3c64xx.c
+++ b/drivers/spi/spi_s3c64xx.c
@@ -492,6 +492,7 @@ void s3c64xx_spi_dma_txcb(struct s3c2410_dma_chan *chan, void *buf_id,
 static int s3c64xx_spi_map_mssg(struct s3c64xx_spi_driver_data *sdd,
 						struct spi_message *msg)
 {
+	struct s3c64xx_spi_info *sci = sdd->cntrlr_info;
 	struct device *dev = &sdd->pdev->dev;
 	struct spi_transfer *xfer;
 
@@ -507,6 +508,9 @@ static int s3c64xx_spi_map_mssg(struct s3c64xx_spi_driver_data *sdd,
 	/* Map until end or first fail */
 	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
 
+		if (xfer->len <= ((sci->fifo_lvl_mask >> 1) + 1))
+			continue;
+
 		if (xfer->tx_buf != NULL) {
 			xfer->tx_dma = dma_map_single(dev,
 					(void *)xfer->tx_buf, xfer->len,
@@ -538,6 +542,7 @@ static int s3c64xx_spi_map_mssg(struct s3c64xx_spi_driver_data *sdd,
 static void s3c64xx_spi_unmap_mssg(struct s3c64xx_spi_driver_data *sdd,
 						struct spi_message *msg)
 {
+	struct s3c64xx_spi_info *sci = sdd->cntrlr_info;
 	struct device *dev = &sdd->pdev->dev;
 	struct spi_transfer *xfer;
 
@@ -546,6 +551,9 @@ static void s3c64xx_spi_unmap_mssg(struct s3c64xx_spi_driver_data *sdd,
 
 	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
 
+		if (xfer->len <= ((sci->fifo_lvl_mask >> 1) + 1))
+			continue;
+
 		if (xfer->rx_buf != NULL
 				&& xfer->rx_dma != XFER_DMAADDR_INVALID)
 			dma_unmap_single(dev, xfer->rx_dma,
-- 
1.6.2.5


------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:

Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd

Re: [PATCH 2/6] SPI: S3C64XX: Prevent unnecessary map-unmap

[Grant Likely]

On Fri, Sep 03, 2010 at 10:36:37AM +0900, Jassi Brar wrote:
> Since we use DMA mode only for xfers bigger than FIFO size,
> do not map/unmap buffers for polling mode transfers.
> 
> Signed-off-by: Jassi Brar <jassi.brar-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

I'll pick this one up for the 2.6.37 merge window.

g.

> ---
>  drivers/spi/spi_s3c64xx.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/spi/spi_s3c64xx.c b/drivers/spi/spi_s3c64xx.c
> index 774e861..12e7915 100644
> --- a/drivers/spi/spi_s3c64xx.c
> +++ b/drivers/spi/spi_s3c64xx.c
> @@ -492,6 +492,7 @@ void s3c64xx_spi_dma_txcb(struct s3c2410_dma_chan *chan, void *buf_id,
>  static int s3c64xx_spi_map_mssg(struct s3c64xx_spi_driver_data *sdd,
>  						struct spi_message *msg)
>  {
> +	struct s3c64xx_spi_info *sci = sdd->cntrlr_info;
>  	struct device *dev = &sdd->pdev->dev;
>  	struct spi_transfer *xfer;
>  
> @@ -507,6 +508,9 @@ static int s3c64xx_spi_map_mssg(struct s3c64xx_spi_driver_data *sdd,
>  	/* Map until end or first fail */
>  	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
>  
> +		if (xfer->len <= ((sci->fifo_lvl_mask >> 1) + 1))
> +			continue;
> +
>  		if (xfer->tx_buf != NULL) {
>  			xfer->tx_dma = dma_map_single(dev,
>  					(void *)xfer->tx_buf, xfer->len,
> @@ -538,6 +542,7 @@ static int s3c64xx_spi_map_mssg(struct s3c64xx_spi_driver_data *sdd,
>  static void s3c64xx_spi_unmap_mssg(struct s3c64xx_spi_driver_data *sdd,
>  						struct spi_message *msg)
>  {
> +	struct s3c64xx_spi_info *sci = sdd->cntrlr_info;
>  	struct device *dev = &sdd->pdev->dev;
>  	struct spi_transfer *xfer;
>  
> @@ -546,6 +551,9 @@ static void s3c64xx_spi_unmap_mssg(struct s3c64xx_spi_driver_data *sdd,
>  
>  	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
>  
> +		if (xfer->len <= ((sci->fifo_lvl_mask >> 1) + 1))
> +			continue;
> +
>  		if (xfer->rx_buf != NULL
>  				&& xfer->rx_dma != XFER_DMAADDR_INVALID)
>  			dma_unmap_single(dev, xfer->rx_dma,
> -- 
> 1.6.2.5
> 

------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:

Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd

[PATCH 3/6] SPI: S3C64XX: Debug status read

[Jassi Brar]

Instead of, wrongly, reusing the 'val' variable, use a dedicated
one for reading the status register.

Signed-off-by: Jassi Brar <jassi.brar-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/spi/spi_s3c64xx.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi_s3c64xx.c b/drivers/spi/spi_s3c64xx.c
index 12e7915..39816bb 100644
--- a/drivers/spi/spi_s3c64xx.c
+++ b/drivers/spi/spi_s3c64xx.c
@@ -326,10 +326,11 @@ static int wait_for_xfer(struct s3c64xx_spi_driver_data *sdd,
 		val = msecs_to_jiffies(ms) + 10;
 		val = wait_for_completion_timeout(&sdd->xfer_completion, val);
 	} else {
+		u32 status;
 		val = msecs_to_loops(ms);
 		do {
-			val = readl(regs + S3C64XX_SPI_STATUS);
-		} while (RX_FIFO_LVL(val, sci) < xfer->len && --val);
+			status = readl(regs + S3C64XX_SPI_STATUS);
+		} while (RX_FIFO_LVL(status, sci) < xfer->len && --val);
 	}
 
 	if (!val)
-- 
1.6.2.5


------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:

Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd

Re: [PATCH 3/6] SPI: S3C64XX: Debug status read

[Grant Likely]

On Fri, Sep 03, 2010 at 10:36:46AM +0900, Jassi Brar wrote:
> Instead of, wrongly, reusing the 'val' variable, use a dedicated
> one for reading the status register.
> 
> Signed-off-by: Jassi Brar <jassi.brar-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

Applied to spi-merge, thanks.

g.

> ---
>  drivers/spi/spi_s3c64xx.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/spi/spi_s3c64xx.c b/drivers/spi/spi_s3c64xx.c
> index 12e7915..39816bb 100644
> --- a/drivers/spi/spi_s3c64xx.c
> +++ b/drivers/spi/spi_s3c64xx.c
> @@ -326,10 +326,11 @@ static int wait_for_xfer(struct s3c64xx_spi_driver_data *sdd,
>  		val = msecs_to_jiffies(ms) + 10;
>  		val = wait_for_completion_timeout(&sdd->xfer_completion, val);
>  	} else {
> +		u32 status;
>  		val = msecs_to_loops(ms);
>  		do {
> -			val = readl(regs + S3C64XX_SPI_STATUS);
> -		} while (RX_FIFO_LVL(val, sci) < xfer->len && --val);
> +			status = readl(regs + S3C64XX_SPI_STATUS);
> +		} while (RX_FIFO_LVL(status, sci) < xfer->len && --val);
>  	}
>  
>  	if (!val)
> -- 
> 1.6.2.5
> 

------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:

Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd

[PATCH 4/6] SPI: S3C64XX: Correction for 16,32 bits bus width

[Jassi Brar]

We can't do without setting channel and bus width to
same size.
Inorder to do that, define a new callback to be used
to do read/write of appropriate widths.

Signed-off-by: Jassi Brar <jassi.brar-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/spi/spi_s3c64xx.c |   78 +++++++++++++++++++++++++++++++++++++-------
 1 files changed, 65 insertions(+), 13 deletions(-)

diff --git a/drivers/spi/spi_s3c64xx.c b/drivers/spi/spi_s3c64xx.c
index 39816bb..8aa9f85 100644
--- a/drivers/spi/spi_s3c64xx.c
+++ b/drivers/spi/spi_s3c64xx.c
@@ -174,12 +174,59 @@ struct s3c64xx_spi_driver_data {
 	unsigned                        state;
 	unsigned                        cur_mode, cur_bpw;
 	unsigned                        cur_speed;
+	void (*do_xfer)(void *ptr, void *fifo, unsigned sz, bool rd);
 };
 
 static struct s3c2410_dma_client s3c64xx_spi_dma_client = {
 	.name = "samsung-spi-dma",
 };
 
+static void s3c64xx_spi_xfer8(void *ptr, void __iomem *fifo,
+		unsigned len, bool read)
+{
+	u8 *buf = (u8 *)ptr;
+	int i = 0;
+
+	if (read)
+		while (i < len)
+			buf[i++] = readb(fifo);
+	else
+		while (i < len)
+			writeb(buf[i++], fifo);
+}
+
+static void s3c64xx_spi_xfer16(void *ptr, void __iomem *fifo,
+		unsigned len, bool read)
+{
+	u16 *buf = (u16 *)ptr;
+	int i = 0;
+
+	len /= 2;
+
+	if (read)
+		while (i < len)
+			buf[i++] = readw(fifo);
+	else
+		while (i < len)
+			writew(buf[i++], fifo);
+}
+
+static void s3c64xx_spi_xfer32(void *ptr, void __iomem *fifo,
+		unsigned len, bool read)
+{
+	u32 *buf = (u32 *)ptr;
+	int i = 0;
+
+	len /= 4;
+
+	if (read)
+		while (i < len)
+			buf[i++] = readl(fifo);
+	else
+		while (i < len)
+			writel(buf[i++], fifo);
+}
+
 static void flush_fifo(struct s3c64xx_spi_driver_data *sdd)
 {
 	struct s3c64xx_spi_info *sci = sdd->cntrlr_info;
@@ -260,10 +307,8 @@ static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,
 						xfer->tx_dma, xfer->len);
 			s3c2410_dma_ctrl(sdd->tx_dmach, S3C2410_DMAOP_START);
 		} else {
-			unsigned char *buf = (unsigned char *) xfer->tx_buf;
-			int i = 0;
-			while (i < xfer->len)
-				writeb(buf[i++], regs + S3C64XX_SPI_TX_DATA);
+			sdd->do_xfer((void *)xfer->tx_buf,
+				regs + S3C64XX_SPI_TX_DATA, xfer->len, false);
 		}
 	}
 
@@ -360,20 +405,14 @@ static int wait_for_xfer(struct s3c64xx_spi_driver_data *sdd,
 				return -EIO;
 		}
 	} else {
-		unsigned char *buf;
-		int i;
-
 		/* If it was only Tx */
 		if (xfer->rx_buf == NULL) {
 			sdd->state &= ~TXBUSY;
 			return 0;
 		}
 
-		i = 0;
-		buf = xfer->rx_buf;
-		while (i < xfer->len)
-			buf[i++] = readb(regs + S3C64XX_SPI_RX_DATA);
-
+		sdd->do_xfer(xfer->rx_buf,
+			regs + S3C64XX_SPI_RX_DATA, xfer->len, true);
 		sdd->state &= ~RXBUSY;
 	}
 
@@ -423,15 +462,20 @@ static void s3c64xx_spi_config(struct s3c64xx_spi_driver_data *sdd)
 	switch (sdd->cur_bpw) {
 	case 32:
 		val |= S3C64XX_SPI_MODE_BUS_TSZ_WORD;
+		val |= S3C64XX_SPI_MODE_CH_TSZ_WORD;
+		sdd->do_xfer = s3c64xx_spi_xfer32;
 		break;
 	case 16:
 		val |= S3C64XX_SPI_MODE_BUS_TSZ_HALFWORD;
+		val |= S3C64XX_SPI_MODE_CH_TSZ_HALFWORD;
+		sdd->do_xfer = s3c64xx_spi_xfer16;
 		break;
 	default:
 		val |= S3C64XX_SPI_MODE_BUS_TSZ_BYTE;
+		val |= S3C64XX_SPI_MODE_CH_TSZ_BYTE;
+		sdd->do_xfer = s3c64xx_spi_xfer8;
 		break;
 	}
-	val |= S3C64XX_SPI_MODE_CH_TSZ_BYTE; /* Always 8bits wide */
 
 	writel(val, regs + S3C64XX_SPI_MODE_CFG);
 
@@ -610,6 +654,14 @@ static void handle_msg(struct s3c64xx_spi_driver_data *sdd,
 		bpw = xfer->bits_per_word ? : spi->bits_per_word;
 		speed = xfer->speed_hz ? : spi->max_speed_hz;
 
+		if (bpw != 8 && xfer->len % (bpw / 8)) {
+			dev_err(&spi->dev,
+				"Xfer length(%u) not a multiple of word size(%u)\n",
+				xfer->len, bpw / 8);
+			status = -EIO;
+			goto out;
+		}
+
 		if (bpw != sdd->cur_bpw || speed != sdd->cur_speed) {
 			sdd->cur_bpw = bpw;
 			sdd->cur_speed = speed;
-- 
1.6.2.5


------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:

Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd

Re: [PATCH 4/6] SPI: S3C64XX: Correction for 16,32 bits bus width

[Grant Likely]

On Fri, Sep 03, 2010 at 10:36:54AM +0900, Jassi Brar wrote:
> We can't do without setting channel and bus width to
> same size.
> Inorder to do that, define a new callback to be used
> to do read/write of appropriate widths.
> 
> Signed-off-by: Jassi Brar <jassi.brar-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/spi/spi_s3c64xx.c |   78 +++++++++++++++++++++++++++++++++++++-------
>  1 files changed, 65 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/spi/spi_s3c64xx.c b/drivers/spi/spi_s3c64xx.c
> index 39816bb..8aa9f85 100644
> --- a/drivers/spi/spi_s3c64xx.c
> +++ b/drivers/spi/spi_s3c64xx.c
> @@ -174,12 +174,59 @@ struct s3c64xx_spi_driver_data {
>  	unsigned                        state;
>  	unsigned                        cur_mode, cur_bpw;
>  	unsigned                        cur_speed;
> +	void (*do_xfer)(void *ptr, void *fifo, unsigned sz, bool rd);

Looking at the code; it appears that it would be better to have
separate read and write transfer hooks.

>  };
>  
>  static struct s3c2410_dma_client s3c64xx_spi_dma_client = {
>  	.name = "samsung-spi-dma",
>  };
>  
> +static void s3c64xx_spi_xfer8(void *ptr, void __iomem *fifo,
> +		unsigned len, bool read)
> +{
> +	u8 *buf = (u8 *)ptr;

ptr is already a void* which makes this an unnecessary cast.

> +	int i = 0;
> +
> +	if (read)
> +		while (i < len)
> +			buf[i++] = readb(fifo);

perhaps ioread8_rep() instead of open-coding?  If so, then
ioread*_rep() and iowrite*_rep() can be used in all these cases.

In fact, the 8, 16 and 32 versions have the same signature and so
could possibly be used directly without these hooks.

g.

> +	else
> +		while (i < len)
> +			writeb(buf[i++], fifo);
> +}
> +
> +static void s3c64xx_spi_xfer16(void *ptr, void __iomem *fifo,
> +		unsigned len, bool read)
> +{
> +	u16 *buf = (u16 *)ptr;
> +	int i = 0;
> +
> +	len /= 2;
> +
> +	if (read)
> +		while (i < len)
> +			buf[i++] = readw(fifo);
> +	else
> +		while (i < len)
> +			writew(buf[i++], fifo);
> +}
> +
> +static void s3c64xx_spi_xfer32(void *ptr, void __iomem *fifo,
> +		unsigned len, bool read)
> +{
> +	u32 *buf = (u32 *)ptr;
> +	int i = 0;
> +
> +	len /= 4;
> +
> +	if (read)
> +		while (i < len)
> +			buf[i++] = readl(fifo);
> +	else
> +		while (i < len)
> +			writel(buf[i++], fifo);
> +}
> +
>  static void flush_fifo(struct s3c64xx_spi_driver_data *sdd)
>  {
>  	struct s3c64xx_spi_info *sci = sdd->cntrlr_info;
> @@ -260,10 +307,8 @@ static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,
>  						xfer->tx_dma, xfer->len);
>  			s3c2410_dma_ctrl(sdd->tx_dmach, S3C2410_DMAOP_START);
>  		} else {
> -			unsigned char *buf = (unsigned char *) xfer->tx_buf;
> -			int i = 0;
> -			while (i < xfer->len)
> -				writeb(buf[i++], regs + S3C64XX_SPI_TX_DATA);
> +			sdd->do_xfer((void *)xfer->tx_buf,
> +				regs + S3C64XX_SPI_TX_DATA, xfer->len, false);
>  		}
>  	}
>  
> @@ -360,20 +405,14 @@ static int wait_for_xfer(struct s3c64xx_spi_driver_data *sdd,
>  				return -EIO;
>  		}
>  	} else {
> -		unsigned char *buf;
> -		int i;
> -
>  		/* If it was only Tx */
>  		if (xfer->rx_buf == NULL) {
>  			sdd->state &= ~TXBUSY;
>  			return 0;
>  		}
>  
> -		i = 0;
> -		buf = xfer->rx_buf;
> -		while (i < xfer->len)
> -			buf[i++] = readb(regs + S3C64XX_SPI_RX_DATA);
> -
> +		sdd->do_xfer(xfer->rx_buf,
> +			regs + S3C64XX_SPI_RX_DATA, xfer->len, true);
>  		sdd->state &= ~RXBUSY;
>  	}
>  
> @@ -423,15 +462,20 @@ static void s3c64xx_spi_config(struct s3c64xx_spi_driver_data *sdd)
>  	switch (sdd->cur_bpw) {
>  	case 32:
>  		val |= S3C64XX_SPI_MODE_BUS_TSZ_WORD;
> +		val |= S3C64XX_SPI_MODE_CH_TSZ_WORD;
> +		sdd->do_xfer = s3c64xx_spi_xfer32;
>  		break;
>  	case 16:
>  		val |= S3C64XX_SPI_MODE_BUS_TSZ_HALFWORD;
> +		val |= S3C64XX_SPI_MODE_CH_TSZ_HALFWORD;
> +		sdd->do_xfer = s3c64xx_spi_xfer16;
>  		break;
>  	default:
>  		val |= S3C64XX_SPI_MODE_BUS_TSZ_BYTE;
> +		val |= S3C64XX_SPI_MODE_CH_TSZ_BYTE;
> +		sdd->do_xfer = s3c64xx_spi_xfer8;
>  		break;
>  	}
> -	val |= S3C64XX_SPI_MODE_CH_TSZ_BYTE; /* Always 8bits wide */
>  
>  	writel(val, regs + S3C64XX_SPI_MODE_CFG);
>  
> @@ -610,6 +654,14 @@ static void handle_msg(struct s3c64xx_spi_driver_data *sdd,
>  		bpw = xfer->bits_per_word ? : spi->bits_per_word;
>  		speed = xfer->speed_hz ? : spi->max_speed_hz;
>  
> +		if (bpw != 8 && xfer->len % (bpw / 8)) {
> +			dev_err(&spi->dev,
> +				"Xfer length(%u) not a multiple of word size(%u)\n",
> +				xfer->len, bpw / 8);
> +			status = -EIO;
> +			goto out;
> +		}
> +
>  		if (bpw != sdd->cur_bpw || speed != sdd->cur_speed) {
>  			sdd->cur_bpw = bpw;
>  			sdd->cur_speed = speed;
> -- 
> 1.6.2.5
> 

------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:

Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd

Re: [PATCH 4/6] SPI: S3C64XX: Correction for 16,32 bits bus width

[Jassi Brar]

On Thu, Sep 9, 2010 at 2:07 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Fri, Sep 03, 2010 at 10:36:54AM +0900, Jassi Brar wrote:
>> We can't do without setting channel and bus width to
>> same size.
>> Inorder to do that, define a new callback to be used
>> to do read/write of appropriate widths.
>>
>> Signed-off-by: Jassi Brar <jassi.brar@samsung.com>
>> ---
>>  drivers/spi/spi_s3c64xx.c |   78 +++++++++++++++++++++++++++++++++++++-------
>>  1 files changed, 65 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/spi/spi_s3c64xx.c b/drivers/spi/spi_s3c64xx.c
>> index 39816bb..8aa9f85 100644
>> --- a/drivers/spi/spi_s3c64xx.c
>> +++ b/drivers/spi/spi_s3c64xx.c
>> @@ -174,12 +174,59 @@ struct s3c64xx_spi_driver_data {
>>       unsigned                        state;
>>       unsigned                        cur_mode, cur_bpw;
>>       unsigned                        cur_speed;
>> +     void (*do_xfer)(void *ptr, void *fifo, unsigned sz, bool rd);
>
> Looking at the code; it appears that it would be better to have
> separate read and write transfer hooks.
>
>>  };
>>
>>  static struct s3c2410_dma_client s3c64xx_spi_dma_client = {
>>       .name = "samsung-spi-dma",
>>  };
>>
>> +static void s3c64xx_spi_xfer8(void *ptr, void __iomem *fifo,
>> +             unsigned len, bool read)
>> +{
>> +     u8 *buf = (u8 *)ptr;
>
> ptr is already a void* which makes this an unnecessary cast.
>
>> +     int i = 0;
>> +
>> +     if (read)
>> +             while (i < len)
>> +                     buf[i++] = readb(fifo);
>
> perhaps ioread8_rep() instead of open-coding?  If so, then
> ioread*_rep() and iowrite*_rep() can be used in all these cases.
>
> In fact, the 8, 16 and 32 versions have the same signature and so
> could possibly be used directly without these hooks.

Great idea. I have rather avoided new callbacks and directly used the
ioread*_rep
calls. Also, I had missed setting appropriate bus width while in DMA mode.
Resending this again.
Thanks.

------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:

Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd
_______________________________________________
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general

[PATCH 5/6] SPI: S3C64XX: Define new clk src indicator

[Jassi Brar]

Since the driver is meant to manage SPI of newer SoCs as well, we
need to define a new structure member for latest SoC that has the
SPI clock control/scaling block moved from the controller to the
platform's clock management unit.
The flag should be set to 'true' by any SoC that has SPI controller
clock managed by CMU.

Signed-off-by: Jassi Brar <jassi.brar-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 arch/arm/plat-samsung/include/plat/s3c64xx-spi.h |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-samsung/include/plat/s3c64xx-spi.h b/arch/arm/plat-samsung/include/plat/s3c64xx-spi.h
index e5aba8f..b226f74 100644
--- a/arch/arm/plat-samsung/include/plat/s3c64xx-spi.h
+++ b/arch/arm/plat-samsung/include/plat/s3c64xx-spi.h
@@ -32,6 +32,8 @@ struct s3c64xx_spi_csinfo {
  * struct s3c64xx_spi_info - SPI Controller defining structure
  * @src_clk_nr: Clock source index for the CLK_CFG[SPI_CLKSEL] field.
  * @src_clk_name: Platform name of the corresponding clock.
+ * @clk_from_cmu: If the SPI clock/prescalar control block is present
+ *     by the platform's clock-management-unit and not in SPI controller.
  * @num_cs: Number of CS this controller emulates.
  * @cfg_gpio: Configure pins for this SPI controller.
  * @fifo_lvl_mask: All tx fifo_lvl fields start at offset-6
@@ -41,6 +43,7 @@ struct s3c64xx_spi_csinfo {
 struct s3c64xx_spi_info {
 	int src_clk_nr;
 	char *src_clk_name;
+	bool clk_from_cmu;
 
 	int num_cs;
 
-- 
1.6.2.5


------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:

Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd

Re: [PATCH 5/6] SPI: S3C64XX: Define new clk src indicator

[Grant Likely]

On Fri, Sep 03, 2010 at 10:37:02AM +0900, Jassi Brar wrote:
> Since the driver is meant to manage SPI of newer SoCs as well, we
> need to define a new structure member for latest SoC that has the
> SPI clock control/scaling block moved from the controller to the
> platform's clock management unit.
> The flag should be set to 'true' by any SoC that has SPI controller
> clock managed by CMU.
> 
> Signed-off-by: Jassi Brar <jassi.brar-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

Squash this patch into the patch that actually implements this new behaviour.

g.

> ---
>  arch/arm/plat-samsung/include/plat/s3c64xx-spi.h |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/plat-samsung/include/plat/s3c64xx-spi.h b/arch/arm/plat-samsung/include/plat/s3c64xx-spi.h
> index e5aba8f..b226f74 100644
> --- a/arch/arm/plat-samsung/include/plat/s3c64xx-spi.h
> +++ b/arch/arm/plat-samsung/include/plat/s3c64xx-spi.h
> @@ -32,6 +32,8 @@ struct s3c64xx_spi_csinfo {
>   * struct s3c64xx_spi_info - SPI Controller defining structure
>   * @src_clk_nr: Clock source index for the CLK_CFG[SPI_CLKSEL] field.
>   * @src_clk_name: Platform name of the corresponding clock.
> + * @clk_from_cmu: If the SPI clock/prescalar control block is present
> + *     by the platform's clock-management-unit and not in SPI controller.
>   * @num_cs: Number of CS this controller emulates.
>   * @cfg_gpio: Configure pins for this SPI controller.
>   * @fifo_lvl_mask: All tx fifo_lvl fields start at offset-6
> @@ -41,6 +43,7 @@ struct s3c64xx_spi_csinfo {
>  struct s3c64xx_spi_info {
>  	int src_clk_nr;
>  	char *src_clk_name;
> +	bool clk_from_cmu;
>  
>  	int num_cs;
>  
> -- 
> 1.6.2.5
> 

------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:

Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd

[PATCH 6/6] SPI: S3C64XX: Consider the clk_from_cmu flag

[Jassi Brar]

Newer SoCs have the SPI clock scaling control in platform's
clock management unit. Inorder for such SoCs to work, we need
to check the flag clk_from_cmu before making any clock changes.

Signed-off-by: Jassi Brar <jassi.brar-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/spi/spi_s3c64xx.c |   94 +++++++++++++++++++++++++++------------------
 1 files changed, 56 insertions(+), 38 deletions(-)

diff --git a/drivers/spi/spi_s3c64xx.c b/drivers/spi/spi_s3c64xx.c
index 8aa9f85..46f42b8 100644
--- a/drivers/spi/spi_s3c64xx.c
+++ b/drivers/spi/spi_s3c64xx.c
@@ -432,13 +432,18 @@ static inline void disable_cs(struct s3c64xx_spi_driver_data *sdd,
 
 static void s3c64xx_spi_config(struct s3c64xx_spi_driver_data *sdd)
 {
+	struct s3c64xx_spi_info *sci = sdd->cntrlr_info;
 	void __iomem *regs = sdd->regs;
 	u32 val;
 
 	/* Disable Clock */
-	val = readl(regs + S3C64XX_SPI_CLK_CFG);
-	val &= ~S3C64XX_SPI_ENCLK_ENABLE;
-	writel(val, regs + S3C64XX_SPI_CLK_CFG);
+	if (sci->clk_from_cmu) {
+		clk_disable(sdd->src_clk);
+	} else {
+		val = readl(regs + S3C64XX_SPI_CLK_CFG);
+		val &= ~S3C64XX_SPI_ENCLK_ENABLE;
+		writel(val, regs + S3C64XX_SPI_CLK_CFG);
+	}
 
 	/* Set Polarity and Phase */
 	val = readl(regs + S3C64XX_SPI_CH_CFG);
@@ -479,17 +484,25 @@ static void s3c64xx_spi_config(struct s3c64xx_spi_driver_data *sdd)
 
 	writel(val, regs + S3C64XX_SPI_MODE_CFG);
 
-	/* Configure Clock */
-	val = readl(regs + S3C64XX_SPI_CLK_CFG);
-	val &= ~S3C64XX_SPI_PSR_MASK;
-	val |= ((clk_get_rate(sdd->src_clk) / sdd->cur_speed / 2 - 1)
-			& S3C64XX_SPI_PSR_MASK);
-	writel(val, regs + S3C64XX_SPI_CLK_CFG);
-
-	/* Enable Clock */
-	val = readl(regs + S3C64XX_SPI_CLK_CFG);
-	val |= S3C64XX_SPI_ENCLK_ENABLE;
-	writel(val, regs + S3C64XX_SPI_CLK_CFG);
+	if (sci->clk_from_cmu) {
+		/* Configure Clock */
+		/* There is half-multiplier before the SPI */
+		clk_set_rate(sdd->src_clk, sdd->cur_speed * 2);
+		/* Enable Clock */
+		clk_enable(sdd->src_clk);
+	} else {
+		/* Configure Clock */
+		val = readl(regs + S3C64XX_SPI_CLK_CFG);
+		val &= ~S3C64XX_SPI_PSR_MASK;
+		val |= ((clk_get_rate(sdd->src_clk) / sdd->cur_speed / 2 - 1)
+				& S3C64XX_SPI_PSR_MASK);
+		writel(val, regs + S3C64XX_SPI_CLK_CFG);
+
+		/* Enable Clock */
+		val = readl(regs + S3C64XX_SPI_CLK_CFG);
+		val |= S3C64XX_SPI_ENCLK_ENABLE;
+		writel(val, regs + S3C64XX_SPI_CLK_CFG);
+	}
 }
 
 void s3c64xx_spi_dma_rxcb(struct s3c2410_dma_chan *chan, void *buf_id,
@@ -852,7 +865,6 @@ static int s3c64xx_spi_setup(struct spi_device *spi)
 	struct s3c64xx_spi_driver_data *sdd;
 	struct s3c64xx_spi_info *sci;
 	struct spi_message *msg;
-	u32 psr, speed;
 	unsigned long flags;
 	int err = 0;
 
@@ -895,32 +907,37 @@ static int s3c64xx_spi_setup(struct spi_device *spi)
 	}
 
 	/* Check if we can provide the requested rate */
-	speed = clk_get_rate(sdd->src_clk) / 2 / (0 + 1); /* Max possible */
-
-	if (spi->max_speed_hz > speed)
-		spi->max_speed_hz = speed;
-
-	psr = clk_get_rate(sdd->src_clk) / 2 / spi->max_speed_hz - 1;
-	psr &= S3C64XX_SPI_PSR_MASK;
-	if (psr == S3C64XX_SPI_PSR_MASK)
-		psr--;
+	if (!sci->clk_from_cmu) {
+		u32 psr, speed;
+
+		/* Max possible */
+		speed = clk_get_rate(sdd->src_clk) / 2 / (0 + 1);
+
+		if (spi->max_speed_hz > speed)
+			spi->max_speed_hz = speed;
+
+		psr = clk_get_rate(sdd->src_clk) / 2 / spi->max_speed_hz - 1;
+		psr &= S3C64XX_SPI_PSR_MASK;
+		if (psr == S3C64XX_SPI_PSR_MASK)
+			psr--;
+
+		speed = clk_get_rate(sdd->src_clk) / 2 / (psr + 1);
+		if (spi->max_speed_hz < speed) {
+			if (psr+1 < S3C64XX_SPI_PSR_MASK) {
+				psr++;
+			} else {
+				err = -EINVAL;
+				goto setup_exit;
+			}
+		}
 
-	speed = clk_get_rate(sdd->src_clk) / 2 / (psr + 1);
-	if (spi->max_speed_hz < speed) {
-		if (psr+1 < S3C64XX_SPI_PSR_MASK) {
-			psr++;
-		} else {
+		speed = clk_get_rate(sdd->src_clk) / 2 / (psr + 1);
+		if (spi->max_speed_hz >= speed)
+			spi->max_speed_hz = speed;
+		else
 			err = -EINVAL;
-			goto setup_exit;
-		}
 	}
 
-	speed = clk_get_rate(sdd->src_clk) / 2 / (psr + 1);
-	if (spi->max_speed_hz >= speed)
-		spi->max_speed_hz = speed;
-	else
-		err = -EINVAL;
-
 setup_exit:
 
 	/* setup() returns with device de-selected */
@@ -942,7 +959,8 @@ static void s3c64xx_spi_hwinit(struct s3c64xx_spi_driver_data *sdd, int channel)
 	/* Disable Interrupts - we use Polling if not DMA mode */
 	writel(0, regs + S3C64XX_SPI_INT_EN);
 
-	writel(sci->src_clk_nr << S3C64XX_SPI_CLKSEL_SRCSHFT,
+	if (!sci->clk_from_cmu)
+		writel(sci->src_clk_nr << S3C64XX_SPI_CLKSEL_SRCSHFT,
 				regs + S3C64XX_SPI_CLK_CFG);
 	writel(0, regs + S3C64XX_SPI_MODE_CFG);
 	writel(0, regs + S3C64XX_SPI_PACKET_CNT);
-- 
1.6.2.5


------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:

Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd

Re: [PATCH 6/6] SPI: S3C64XX: Consider the clk_from_cmu flag

[Grant Likely]

On Fri, Sep 03, 2010 at 10:37:10AM +0900, Jassi Brar wrote:
> Newer SoCs have the SPI clock scaling control in platform's
> clock management unit. Inorder for such SoCs to work, we need
> to check the flag clk_from_cmu before making any clock changes.
> 
> Signed-off-by: Jassi Brar <jassi.brar-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

Would it not make sense (or be possible) to roll the current clock behaviour into the current clock struct so that this driver can always use the same API for dealing with its clocking?  Or am I missing something?

g.

> ---
>  drivers/spi/spi_s3c64xx.c |   94 +++++++++++++++++++++++++++------------------
>  1 files changed, 56 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/spi/spi_s3c64xx.c b/drivers/spi/spi_s3c64xx.c
> index 8aa9f85..46f42b8 100644
> --- a/drivers/spi/spi_s3c64xx.c
> +++ b/drivers/spi/spi_s3c64xx.c
> @@ -432,13 +432,18 @@ static inline void disable_cs(struct s3c64xx_spi_driver_data *sdd,
>  
>  static void s3c64xx_spi_config(struct s3c64xx_spi_driver_data *sdd)
>  {
> +	struct s3c64xx_spi_info *sci = sdd->cntrlr_info;
>  	void __iomem *regs = sdd->regs;
>  	u32 val;
>  
>  	/* Disable Clock */
> -	val = readl(regs + S3C64XX_SPI_CLK_CFG);
> -	val &= ~S3C64XX_SPI_ENCLK_ENABLE;
> -	writel(val, regs + S3C64XX_SPI_CLK_CFG);
> +	if (sci->clk_from_cmu) {
> +		clk_disable(sdd->src_clk);
> +	} else {
> +		val = readl(regs + S3C64XX_SPI_CLK_CFG);
> +		val &= ~S3C64XX_SPI_ENCLK_ENABLE;
> +		writel(val, regs + S3C64XX_SPI_CLK_CFG);
> +	}
>  
>  	/* Set Polarity and Phase */
>  	val = readl(regs + S3C64XX_SPI_CH_CFG);
> @@ -479,17 +484,25 @@ static void s3c64xx_spi_config(struct s3c64xx_spi_driver_data *sdd)
>  
>  	writel(val, regs + S3C64XX_SPI_MODE_CFG);
>  
> -	/* Configure Clock */
> -	val = readl(regs + S3C64XX_SPI_CLK_CFG);
> -	val &= ~S3C64XX_SPI_PSR_MASK;
> -	val |= ((clk_get_rate(sdd->src_clk) / sdd->cur_speed / 2 - 1)
> -			& S3C64XX_SPI_PSR_MASK);
> -	writel(val, regs + S3C64XX_SPI_CLK_CFG);
> -
> -	/* Enable Clock */
> -	val = readl(regs + S3C64XX_SPI_CLK_CFG);
> -	val |= S3C64XX_SPI_ENCLK_ENABLE;
> -	writel(val, regs + S3C64XX_SPI_CLK_CFG);
> +	if (sci->clk_from_cmu) {
> +		/* Configure Clock */
> +		/* There is half-multiplier before the SPI */
> +		clk_set_rate(sdd->src_clk, sdd->cur_speed * 2);
> +		/* Enable Clock */
> +		clk_enable(sdd->src_clk);
> +	} else {
> +		/* Configure Clock */
> +		val = readl(regs + S3C64XX_SPI_CLK_CFG);
> +		val &= ~S3C64XX_SPI_PSR_MASK;
> +		val |= ((clk_get_rate(sdd->src_clk) / sdd->cur_speed / 2 - 1)
> +				& S3C64XX_SPI_PSR_MASK);
> +		writel(val, regs + S3C64XX_SPI_CLK_CFG);
> +
> +		/* Enable Clock */
> +		val = readl(regs + S3C64XX_SPI_CLK_CFG);
> +		val |= S3C64XX_SPI_ENCLK_ENABLE;
> +		writel(val, regs + S3C64XX_SPI_CLK_CFG);
> +	}
>  }
>  
>  void s3c64xx_spi_dma_rxcb(struct s3c2410_dma_chan *chan, void *buf_id,
> @@ -852,7 +865,6 @@ static int s3c64xx_spi_setup(struct spi_device *spi)
>  	struct s3c64xx_spi_driver_data *sdd;
>  	struct s3c64xx_spi_info *sci;
>  	struct spi_message *msg;
> -	u32 psr, speed;
>  	unsigned long flags;
>  	int err = 0;
>  
> @@ -895,32 +907,37 @@ static int s3c64xx_spi_setup(struct spi_device *spi)
>  	}
>  
>  	/* Check if we can provide the requested rate */
> -	speed = clk_get_rate(sdd->src_clk) / 2 / (0 + 1); /* Max possible */
> -
> -	if (spi->max_speed_hz > speed)
> -		spi->max_speed_hz = speed;
> -
> -	psr = clk_get_rate(sdd->src_clk) / 2 / spi->max_speed_hz - 1;
> -	psr &= S3C64XX_SPI_PSR_MASK;
> -	if (psr == S3C64XX_SPI_PSR_MASK)
> -		psr--;
> +	if (!sci->clk_from_cmu) {
> +		u32 psr, speed;
> +
> +		/* Max possible */
> +		speed = clk_get_rate(sdd->src_clk) / 2 / (0 + 1);
> +
> +		if (spi->max_speed_hz > speed)
> +			spi->max_speed_hz = speed;
> +
> +		psr = clk_get_rate(sdd->src_clk) / 2 / spi->max_speed_hz - 1;
> +		psr &= S3C64XX_SPI_PSR_MASK;
> +		if (psr == S3C64XX_SPI_PSR_MASK)
> +			psr--;
> +
> +		speed = clk_get_rate(sdd->src_clk) / 2 / (psr + 1);
> +		if (spi->max_speed_hz < speed) {
> +			if (psr+1 < S3C64XX_SPI_PSR_MASK) {
> +				psr++;
> +			} else {
> +				err = -EINVAL;
> +				goto setup_exit;
> +			}
> +		}
>  
> -	speed = clk_get_rate(sdd->src_clk) / 2 / (psr + 1);
> -	if (spi->max_speed_hz < speed) {
> -		if (psr+1 < S3C64XX_SPI_PSR_MASK) {
> -			psr++;
> -		} else {
> +		speed = clk_get_rate(sdd->src_clk) / 2 / (psr + 1);
> +		if (spi->max_speed_hz >= speed)
> +			spi->max_speed_hz = speed;
> +		else
>  			err = -EINVAL;
> -			goto setup_exit;
> -		}
>  	}
>  
> -	speed = clk_get_rate(sdd->src_clk) / 2 / (psr + 1);
> -	if (spi->max_speed_hz >= speed)
> -		spi->max_speed_hz = speed;
> -	else
> -		err = -EINVAL;
> -
>  setup_exit:
>  
>  	/* setup() returns with device de-selected */
> @@ -942,7 +959,8 @@ static void s3c64xx_spi_hwinit(struct s3c64xx_spi_driver_data *sdd, int channel)
>  	/* Disable Interrupts - we use Polling if not DMA mode */
>  	writel(0, regs + S3C64XX_SPI_INT_EN);
>  
> -	writel(sci->src_clk_nr << S3C64XX_SPI_CLKSEL_SRCSHFT,
> +	if (!sci->clk_from_cmu)
> +		writel(sci->src_clk_nr << S3C64XX_SPI_CLKSEL_SRCSHFT,
>  				regs + S3C64XX_SPI_CLK_CFG);
>  	writel(0, regs + S3C64XX_SPI_MODE_CFG);
>  	writel(0, regs + S3C64XX_SPI_PACKET_CNT);
> -- 
> 1.6.2.5
> 

------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:

Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd

Re: [PATCH 6/6] SPI: S3C64XX: Consider the clk_from_cmu flag

[Jassi Brar]

On Thu, Sep 9, 2010 at 2:11 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Fri, Sep 03, 2010 at 10:37:10AM +0900, Jassi Brar wrote:
>> Newer SoCs have the SPI clock scaling control in platform's
>> clock management unit. Inorder for such SoCs to work, we need
>> to check the flag clk_from_cmu before making any clock changes.
>>
>> Signed-off-by: Jassi Brar <jassi.brar@samsung.com>
>
> Would it not make sense (or be possible) to roll the current clock behaviour into the current clock struct so that this driver can always use the same API for dealing with its clocking?  Or am I missing something?
>

The problem is, earlier SPI had clock control/scaling in it's own IP,
whereas latest(and in future)
SPIs have them moved out into the Clock-Management-Unit of the SoC -
just how it should be.
In order to have common API, we need to hack clock-api to manipulate
SPI controller
registers outside of the SPI driver for older SPIs.... which wouldn't
be very nice to see.
any suggestion is welcome, though.

thanks.

------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:

Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd
_______________________________________________
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general

Re: [PATCH 6/6] SPI: S3C64XX: Consider the clk_from_cmu flag

[Grant Likely]

On Thu, Sep 09, 2010 at 02:29:22PM +0900, Jassi Brar wrote:
> On Thu, Sep 9, 2010 at 2:11 PM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> > On Fri, Sep 03, 2010 at 10:37:10AM +0900, Jassi Brar wrote:
> >> Newer SoCs have the SPI clock scaling control in platform's
> >> clock management unit. Inorder for such SoCs to work, we need
> >> to check the flag clk_from_cmu before making any clock changes.
> >>
> >> Signed-off-by: Jassi Brar <jassi.brar-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> >
> > Would it not make sense (or be possible) to roll the current clock behaviour into the current clock struct so that this driver can always use the same API for dealing with its clocking?  Or am I missing something?
> >
> 
> The problem is, earlier SPI had clock control/scaling in it's own IP,
> whereas latest(and in future)
> SPIs have them moved out into the Clock-Management-Unit of the SoC -
> just how it should be.
> In order to have common API, we need to hack clock-api to manipulate
> SPI controller
> registers outside of the SPI driver for older SPIs.... which wouldn't
> be very nice to see.
> any suggestion is welcome, though.

Fair enough, not an ideal situation regardless of the approach.  Your approach is probably better then.

g.


------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:

Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd

Re: [PATCH 6/6] SPI: S3C64XX: Consider the clk_from_cmu flag

[Jassi Brar]

On Thu, Sep 9, 2010 at 2:55 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Thu, Sep 09, 2010 at 02:29:22PM +0900, Jassi Brar wrote:
>> On Thu, Sep 9, 2010 at 2:11 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
>> > On Fri, Sep 03, 2010 at 10:37:10AM +0900, Jassi Brar wrote:
>> >> Newer SoCs have the SPI clock scaling control in platform's
>> >> clock management unit. Inorder for such SoCs to work, we need
>> >> to check the flag clk_from_cmu before making any clock changes.
>> >>
>> >> Signed-off-by: Jassi Brar <jassi.brar@samsung.com>
>> >
>> > Would it not make sense (or be possible) to roll the current clock behaviour into the current clock struct so that this driver can always use the same API for dealing with its clocking?  Or am I missing something?
>> >
>>
>> The problem is, earlier SPI had clock control/scaling in it's own IP,
>> whereas latest(and in future)
>> SPIs have them moved out into the Clock-Management-Unit of the SoC -
>> just how it should be.
>> In order to have common API, we need to hack clock-api to manipulate
>> SPI controller
>> registers outside of the SPI driver for older SPIs.... which wouldn't
>> be very nice to see.
>> any suggestion is welcome, though.
>
> Fair enough, not an ideal situation regardless of the approach.  Your approach is probably better then.

Ok, I am resending after squashing it with 5/6

------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:

Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd
_______________________________________________
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general

Re: [PATCH 1/2] SPI: S3C64XX: Correction for 16,32 bits bus width

[Grant Likely]

On Fri, Sep 10, 2010 at 12:07:50AM +0900, Jassi Brar wrote:
> On Thu, Sep 9, 2010 at 11:51 PM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> > On Thu, Sep 09, 2010 at 04:18:57PM +0900, Jassi Brar wrote:
[...]
> > Nit: switch statements like this tend to obscure the fact that the same
> > function is called each time with a different value.  How about the
> > following:
> >
> >        width = 1;
> >        if (sdd->cur_bpw == 32)
> >                width = 4;
> >        if (sdd->cur_bpw == 16)
> >                width = 2;
> >        s3c2410_dma_config(sdd->tx_dmach, width);
> >
> > It's also more concise.
> 
> yuck! why didn't i do
>           s3c2410_dma_config(sdd->tx_dmach, sdd->cur_bpw / 8); ??

Heh, very true!

> >> @@ -610,6 +648,14 @@ static void handle_msg(struct s3c64xx_spi_driver_data *sdd,
> >>               bpw = xfer->bits_per_word ? : spi->bits_per_word;
> >>               speed = xfer->speed_hz ? : spi->max_speed_hz;
> >>
> >> +             if (bpw != 8 && xfer->len % (bpw / 8)) {
> >
> > The (bpw != 8) test is superfluous.
> 
>  xfer->len % (bpw / 8)    always evaluate to true(error hence) when bpw==8
> so we wanna avoid that case

When bpw == 8, the equation reduces to (len % 1), which will always
evaluate to 0 (false) for integer len.

g.


------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:

Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd

[PATCH 4/6] SPI: S3C64XX: Correction for 16,32 bits bus width

[Jassi Brar]

We can't do without setting channel and bus width to
same size. Inorder to do that, use loop read/writes in
polling mode and appropriate burst size in DMA mode.

Signed-off-by: Jassi Brar <jassi.brar-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/spi/spi_s3c64xx.c |   56 +++++++++++++++++++++++++++++++++------------
 1 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/drivers/spi/spi_s3c64xx.c b/drivers/spi/spi_s3c64xx.c
index 39816bb..cf45e01 100644
--- a/drivers/spi/spi_s3c64xx.c
+++ b/drivers/spi/spi_s3c64xx.c
@@ -255,15 +255,25 @@ static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,
 		chcfg |= S3C64XX_SPI_CH_TXCH_ON;
 		if (dma_mode) {
 			modecfg |= S3C64XX_SPI_MODE_TXDMA_ON;
-			s3c2410_dma_config(sdd->tx_dmach, 1);
+			s3c2410_dma_config(sdd->tx_dmach, sdd->cur_bpw / 8);
 			s3c2410_dma_enqueue(sdd->tx_dmach, (void *)sdd,
 						xfer->tx_dma, xfer->len);
 			s3c2410_dma_ctrl(sdd->tx_dmach, S3C2410_DMAOP_START);
 		} else {
-			unsigned char *buf = (unsigned char *) xfer->tx_buf;
-			int i = 0;
-			while (i < xfer->len)
-				writeb(buf[i++], regs + S3C64XX_SPI_TX_DATA);
+			switch (sdd->cur_bpw) {
+			case 32:
+				iowrite32_rep(regs + S3C64XX_SPI_TX_DATA,
+					xfer->tx_buf, xfer->len / 4);
+				break;
+			case 16:
+				iowrite16_rep(regs + S3C64XX_SPI_TX_DATA,
+					xfer->tx_buf, xfer->len / 2);
+				break;
+			default:
+				iowrite8_rep(regs + S3C64XX_SPI_TX_DATA,
+					xfer->tx_buf, xfer->len);
+				break;
+			}
 		}
 	}
 
@@ -280,7 +290,7 @@ static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,
 			writel(((xfer->len * 8 / sdd->cur_bpw) & 0xffff)
 					| S3C64XX_SPI_PACKET_CNT_EN,
 					regs + S3C64XX_SPI_PACKET_CNT);
-			s3c2410_dma_config(sdd->rx_dmach, 1);
+			s3c2410_dma_config(sdd->rx_dmach, sdd->cur_bpw / 8);
 			s3c2410_dma_enqueue(sdd->rx_dmach, (void *)sdd,
 						xfer->rx_dma, xfer->len);
 			s3c2410_dma_ctrl(sdd->rx_dmach, S3C2410_DMAOP_START);
@@ -360,20 +370,26 @@ static int wait_for_xfer(struct s3c64xx_spi_driver_data *sdd,
 				return -EIO;
 		}
 	} else {
-		unsigned char *buf;
-		int i;
-
 		/* If it was only Tx */
 		if (xfer->rx_buf == NULL) {
 			sdd->state &= ~TXBUSY;
 			return 0;
 		}
 
-		i = 0;
-		buf = xfer->rx_buf;
-		while (i < xfer->len)
-			buf[i++] = readb(regs + S3C64XX_SPI_RX_DATA);
-
+		switch (sdd->cur_bpw) {
+		case 32:
+			ioread32_rep(regs + S3C64XX_SPI_RX_DATA,
+				xfer->rx_buf, xfer->len / 4);
+			break;
+		case 16:
+			ioread16_rep(regs + S3C64XX_SPI_RX_DATA,
+				xfer->rx_buf, xfer->len / 2);
+			break;
+		default:
+			ioread8_rep(regs + S3C64XX_SPI_RX_DATA,
+				xfer->rx_buf, xfer->len);
+			break;
+		}
 		sdd->state &= ~RXBUSY;
 	}
 
@@ -423,15 +439,17 @@ static void s3c64xx_spi_config(struct s3c64xx_spi_driver_data *sdd)
 	switch (sdd->cur_bpw) {
 	case 32:
 		val |= S3C64XX_SPI_MODE_BUS_TSZ_WORD;
+		val |= S3C64XX_SPI_MODE_CH_TSZ_WORD;
 		break;
 	case 16:
 		val |= S3C64XX_SPI_MODE_BUS_TSZ_HALFWORD;
+		val |= S3C64XX_SPI_MODE_CH_TSZ_HALFWORD;
 		break;
 	default:
 		val |= S3C64XX_SPI_MODE_BUS_TSZ_BYTE;
+		val |= S3C64XX_SPI_MODE_CH_TSZ_BYTE;
 		break;
 	}
-	val |= S3C64XX_SPI_MODE_CH_TSZ_BYTE; /* Always 8bits wide */
 
 	writel(val, regs + S3C64XX_SPI_MODE_CFG);
 
@@ -610,6 +628,14 @@ static void handle_msg(struct s3c64xx_spi_driver_data *sdd,
 		bpw = xfer->bits_per_word ? : spi->bits_per_word;
 		speed = xfer->speed_hz ? : spi->max_speed_hz;
 
+		if (xfer->len % (bpw / 8)) {
+			dev_err(&spi->dev,
+				"Xfer length(%u) not a multiple of word size(%u)\n",
+				xfer->len, bpw / 8);
+			status = -EIO;
+			goto out;
+		}
+
 		if (bpw != sdd->cur_bpw || speed != sdd->cur_speed) {
 			sdd->cur_bpw = bpw;
 			sdd->cur_speed = speed;
-- 
1.6.2.5


------------------------------------------------------------------------------
Automate Storage Tiering Simply
Optimize IT performance and efficiency through flexible, powerful, 
automated storage tiering capabilities. View this brief to learn how
you can reduce costs and improve performance. 
http://p.sf.net/sfu/dell-sfdev2dev

Re: [PATCH 4/6] SPI: S3C64XX: Correction for 16,32 bits bus width

[Grant Likely]

On Fri, Sep 10, 2010 at 08:55:49AM +0900, Jassi Brar wrote:
> We can't do without setting channel and bus width to
> same size. Inorder to do that, use loop read/writes in
> polling mode and appropriate burst size in DMA mode.
> 
> Signed-off-by: Jassi Brar <jassi.brar-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

applied to -next branch.  Thanks.

g.

> ---
>  drivers/spi/spi_s3c64xx.c |   56 +++++++++++++++++++++++++++++++++------------
>  1 files changed, 41 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/spi/spi_s3c64xx.c b/drivers/spi/spi_s3c64xx.c
> index 39816bb..cf45e01 100644
> --- a/drivers/spi/spi_s3c64xx.c
> +++ b/drivers/spi/spi_s3c64xx.c
> @@ -255,15 +255,25 @@ static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,
>  		chcfg |= S3C64XX_SPI_CH_TXCH_ON;
>  		if (dma_mode) {
>  			modecfg |= S3C64XX_SPI_MODE_TXDMA_ON;
> -			s3c2410_dma_config(sdd->tx_dmach, 1);
> +			s3c2410_dma_config(sdd->tx_dmach, sdd->cur_bpw / 8);
>  			s3c2410_dma_enqueue(sdd->tx_dmach, (void *)sdd,
>  						xfer->tx_dma, xfer->len);
>  			s3c2410_dma_ctrl(sdd->tx_dmach, S3C2410_DMAOP_START);
>  		} else {
> -			unsigned char *buf = (unsigned char *) xfer->tx_buf;
> -			int i = 0;
> -			while (i < xfer->len)
> -				writeb(buf[i++], regs + S3C64XX_SPI_TX_DATA);
> +			switch (sdd->cur_bpw) {
> +			case 32:
> +				iowrite32_rep(regs + S3C64XX_SPI_TX_DATA,
> +					xfer->tx_buf, xfer->len / 4);
> +				break;
> +			case 16:
> +				iowrite16_rep(regs + S3C64XX_SPI_TX_DATA,
> +					xfer->tx_buf, xfer->len / 2);
> +				break;
> +			default:
> +				iowrite8_rep(regs + S3C64XX_SPI_TX_DATA,
> +					xfer->tx_buf, xfer->len);
> +				break;
> +			}
>  		}
>  	}
>  
> @@ -280,7 +290,7 @@ static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,
>  			writel(((xfer->len * 8 / sdd->cur_bpw) & 0xffff)
>  					| S3C64XX_SPI_PACKET_CNT_EN,
>  					regs + S3C64XX_SPI_PACKET_CNT);
> -			s3c2410_dma_config(sdd->rx_dmach, 1);
> +			s3c2410_dma_config(sdd->rx_dmach, sdd->cur_bpw / 8);
>  			s3c2410_dma_enqueue(sdd->rx_dmach, (void *)sdd,
>  						xfer->rx_dma, xfer->len);
>  			s3c2410_dma_ctrl(sdd->rx_dmach, S3C2410_DMAOP_START);
> @@ -360,20 +370,26 @@ static int wait_for_xfer(struct s3c64xx_spi_driver_data *sdd,
>  				return -EIO;
>  		}
>  	} else {
> -		unsigned char *buf;
> -		int i;
> -
>  		/* If it was only Tx */
>  		if (xfer->rx_buf == NULL) {
>  			sdd->state &= ~TXBUSY;
>  			return 0;
>  		}
>  
> -		i = 0;
> -		buf = xfer->rx_buf;
> -		while (i < xfer->len)
> -			buf[i++] = readb(regs + S3C64XX_SPI_RX_DATA);
> -
> +		switch (sdd->cur_bpw) {
> +		case 32:
> +			ioread32_rep(regs + S3C64XX_SPI_RX_DATA,
> +				xfer->rx_buf, xfer->len / 4);
> +			break;
> +		case 16:
> +			ioread16_rep(regs + S3C64XX_SPI_RX_DATA,
> +				xfer->rx_buf, xfer->len / 2);
> +			break;
> +		default:
> +			ioread8_rep(regs + S3C64XX_SPI_RX_DATA,
> +				xfer->rx_buf, xfer->len);
> +			break;
> +		}
>  		sdd->state &= ~RXBUSY;
>  	}
>  
> @@ -423,15 +439,17 @@ static void s3c64xx_spi_config(struct s3c64xx_spi_driver_data *sdd)
>  	switch (sdd->cur_bpw) {
>  	case 32:
>  		val |= S3C64XX_SPI_MODE_BUS_TSZ_WORD;
> +		val |= S3C64XX_SPI_MODE_CH_TSZ_WORD;
>  		break;
>  	case 16:
>  		val |= S3C64XX_SPI_MODE_BUS_TSZ_HALFWORD;
> +		val |= S3C64XX_SPI_MODE_CH_TSZ_HALFWORD;
>  		break;
>  	default:
>  		val |= S3C64XX_SPI_MODE_BUS_TSZ_BYTE;
> +		val |= S3C64XX_SPI_MODE_CH_TSZ_BYTE;
>  		break;
>  	}
> -	val |= S3C64XX_SPI_MODE_CH_TSZ_BYTE; /* Always 8bits wide */
>  
>  	writel(val, regs + S3C64XX_SPI_MODE_CFG);
>  
> @@ -610,6 +628,14 @@ static void handle_msg(struct s3c64xx_spi_driver_data *sdd,
>  		bpw = xfer->bits_per_word ? : spi->bits_per_word;
>  		speed = xfer->speed_hz ? : spi->max_speed_hz;
>  
> +		if (xfer->len % (bpw / 8)) {
> +			dev_err(&spi->dev,
> +				"Xfer length(%u) not a multiple of word size(%u)\n",
> +				xfer->len, bpw / 8);
> +			status = -EIO;
> +			goto out;
> +		}
> +
>  		if (bpw != sdd->cur_bpw || speed != sdd->cur_speed) {
>  			sdd->cur_bpw = bpw;
>  			sdd->cur_speed = speed;
> -- 
> 1.6.2.5
> 

------------------------------------------------------------------------------
Start uncovering the many advantages of virtual appliances
and start using them to simplify application deployment and
accelerate your shift to cloud computing
http://p.sf.net/sfu/novell-sfdev2dev