* [PATCH 1/2] SPI: S3C64XX: Correction for 16,32 bits bus width
@ 2010-09-09 7:18 Jassi Brar
[not found] ` <1284016737-21620-1-git-send-email-jassi.brar-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Jassi Brar @ 2010-09-09 7:18 UTC (permalink / raw)
To: grant.likely-s3s/WqlpOiPyB63q8FvJNQ,
dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f
Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] SPI: S3C64XX: Correction for 16,32 bits bus width
[not found] ` <1284016737-21620-1-git-send-email-jassi.brar-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
@ 2010-09-09 14:51 ` Grant Likely
[not found] ` <20100909145115.GA5652-MrY2KI0G/OVr83L8+7iqerDks+cytr/Z@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Grant Likely @ 2010-09-09 14:51 UTC (permalink / raw)
To: Jassi Brar
Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f,
broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] SPI: S3C64XX: Correction for 16,32 bits bus width
[not found] ` <20100909145115.GA5652-MrY2KI0G/OVr83L8+7iqerDks+cytr/Z@public.gmane.org>
@ 2010-09-09 15:07 ` Jassi Brar
[not found] ` <AANLkTim-gpnfsvWrFnJBcE9DcWFsh8soJwq4gL_bMpMv-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Jassi Brar @ 2010-09-09 15:07 UTC (permalink / raw)
To: Grant Likely
Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f,
broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] SPI: S3C64XX: Correction for 16,32 bits bus width
[not found] ` <AANLkTim-gpnfsvWrFnJBcE9DcWFsh8soJwq4gL_bMpMv-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2010-09-09 16:00 ` Grant Likely
[not found] ` <20100909160051.GA6273-MrY2KI0G/OVr83L8+7iqerDks+cytr/Z@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Grant Likely @ 2010-09-09 16:00 UTC (permalink / raw)
To: Jassi Brar
Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f,
broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 4/6] SPI: S3C64XX: Correction for 16,32 bits bus width
[not found] ` <20100909160051.GA6273-MrY2KI0G/OVr83L8+7iqerDks+cytr/Z@public.gmane.org>
@ 2010-09-09 23:55 ` Jassi Brar
[not found] ` <1284076549-17715-1-git-send-email-jassi.brar-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Jassi Brar @ 2010-09-09 23:55 UTC (permalink / raw)
To: grant.likely-s3s/WqlpOiPyB63q8FvJNQ,
dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f
Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 4/6] SPI: S3C64XX: Correction for 16,32 bits bus width
[not found] ` <1284076549-17715-1-git-send-email-jassi.brar-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
@ 2010-09-10 18:00 ` Grant Likely
0 siblings, 0 replies; 6+ messages in thread
From: Grant Likely @ 2010-09-10 18:00 UTC (permalink / raw)
To: Jassi Brar
Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f,
broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-09-10 18:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-09 7:18 [PATCH 1/2] SPI: S3C64XX: Correction for 16,32 bits bus width Jassi Brar
[not found] ` <1284016737-21620-1-git-send-email-jassi.brar-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2010-09-09 14:51 ` Grant Likely
[not found] ` <20100909145115.GA5652-MrY2KI0G/OVr83L8+7iqerDks+cytr/Z@public.gmane.org>
2010-09-09 15:07 ` Jassi Brar
[not found] ` <AANLkTim-gpnfsvWrFnJBcE9DcWFsh8soJwq4gL_bMpMv-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-09-09 16:00 ` Grant Likely
[not found] ` <20100909160051.GA6273-MrY2KI0G/OVr83L8+7iqerDks+cytr/Z@public.gmane.org>
2010-09-09 23:55 ` [PATCH 4/6] " Jassi Brar
[not found] ` <1284076549-17715-1-git-send-email-jassi.brar-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2010-09-10 18:00 ` Grant Likely
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).