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

[PATCH 1/2] 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 |   76 ++++++++++++++++++++++++++++++++++++---------
 1 files changed, 61 insertions(+), 15 deletions(-)

diff --git a/drivers/spi/spi_s3c64xx.c b/drivers/spi/spi_s3c64xx.c
index 39816bb..3ff335d 100644
--- a/drivers/spi/spi_s3c64xx.c
+++ b/drivers/spi/spi_s3c64xx.c
@@ -255,15 +255,35 @@ 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);
+			switch (sdd->cur_bpw) {
+			case 32:
+				s3c2410_dma_config(sdd->tx_dmach, 4);
+				break;
+			case 16:
+				s3c2410_dma_config(sdd->tx_dmach, 2);
+				break;
+			default:
+				s3c2410_dma_config(sdd->tx_dmach, 1);
+				break;
+			}
 			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 +300,17 @@ 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);
+			switch (sdd->cur_bpw) {
+			case 32:
+				s3c2410_dma_config(sdd->rx_dmach, 4);
+				break;
+			case 16:
+				s3c2410_dma_config(sdd->rx_dmach, 2);
+				break;
+			default:
+				s3c2410_dma_config(sdd->rx_dmach, 1);
+				break;
+			}
 			s3c2410_dma_enqueue(sdd->rx_dmach, (void *)sdd,
 						xfer->rx_dma, xfer->len);
 			s3c2410_dma_ctrl(sdd->rx_dmach, S3C2410_DMAOP_START);
@@ -360,20 +390,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 +459,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 +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)) {
+			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 1/2] SPI: S3C64XX: Correction for 16,32 bits bus width

[Grant Likely]

On Thu, Sep 09, 2010 at 04:18:57PM +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>

Looks pretty good to me.  A couple of minor comments below.

g.

> ---
>  drivers/spi/spi_s3c64xx.c |   76 ++++++++++++++++++++++++++++++++++++---------
>  1 files changed, 61 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/spi/spi_s3c64xx.c b/drivers/spi/spi_s3c64xx.c
> index 39816bb..3ff335d 100644
> --- a/drivers/spi/spi_s3c64xx.c
> +++ b/drivers/spi/spi_s3c64xx.c
> @@ -255,15 +255,35 @@ 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);
> +			switch (sdd->cur_bpw) {
> +			case 32:
> +				s3c2410_dma_config(sdd->tx_dmach, 4);
> +				break;
> +			case 16:
> +				s3c2410_dma_config(sdd->tx_dmach, 2);
> +				break;
> +			default:
> +				s3c2410_dma_config(sdd->tx_dmach, 1);
> +				break;
> +			}

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.

>  			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 +300,17 @@ 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);
> +			switch (sdd->cur_bpw) {
> +			case 32:
> +				s3c2410_dma_config(sdd->rx_dmach, 4);
> +				break;
> +			case 16:
> +				s3c2410_dma_config(sdd->rx_dmach, 2);
> +				break;
> +			default:
> +				s3c2410_dma_config(sdd->rx_dmach, 1);
> +				break;
> +			}

Ditto here

>  			s3c2410_dma_enqueue(sdd->rx_dmach, (void *)sdd,
>  						xfer->rx_dma, xfer->len);
>  			s3c2410_dma_ctrl(sdd->rx_dmach, S3C2410_DMAOP_START);
> @@ -360,20 +390,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 +459,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 +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.

> +			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 1/2] SPI: S3C64XX: Correction for 16,32 bits bus width

[Jassi Brar]

On Thu, Sep 9, 2010 at 11:51 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Thu, Sep 09, 2010 at 04:18:57PM +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@samsung.com>
>
> Looks pretty good to me.  A couple of minor comments below.
>
> g.
>
>> ---
>>  drivers/spi/spi_s3c64xx.c |   76 ++++++++++++++++++++++++++++++++++++---------
>>  1 files changed, 61 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/spi/spi_s3c64xx.c b/drivers/spi/spi_s3c64xx.c
>> index 39816bb..3ff335d 100644
>> --- a/drivers/spi/spi_s3c64xx.c
>> +++ b/drivers/spi/spi_s3c64xx.c
>> @@ -255,15 +255,35 @@ 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);
>> +                     switch (sdd->cur_bpw) {
>> +                     case 32:
>> +                             s3c2410_dma_config(sdd->tx_dmach, 4);
>> +                             break;
>> +                     case 16:
>> +                             s3c2410_dma_config(sdd->tx_dmach, 2);
>> +                             break;
>> +                     default:
>> +                             s3c2410_dma_config(sdd->tx_dmach, 1);
>> +                             break;
>> +                     }
>
> 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); ??


>
>>                       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 +300,17 @@ 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);
>> +                     switch (sdd->cur_bpw) {
>> +                     case 32:
>> +                             s3c2410_dma_config(sdd->rx_dmach, 4);
>> +                             break;
>> +                     case 16:
>> +                             s3c2410_dma_config(sdd->rx_dmach, 2);
>> +                             break;
>> +                     default:
>> +                             s3c2410_dma_config(sdd->rx_dmach, 1);
>> +                             break;
>> +                     }
>
> Ditto here
>
>>                       s3c2410_dma_enqueue(sdd->rx_dmach, (void *)sdd,
>>                                               xfer->rx_dma, xfer->len);
>>                       s3c2410_dma_ctrl(sdd->rx_dmach, S3C2410_DMAOP_START);
>> @@ -360,20 +390,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 +459,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 +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

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 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

[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