[PATCH 1/6] spi: rename config macro name for bfin5xx spi controller driver

[PATCH 1/6] spi: rename config macro name for bfin5xx spi controller driver

[Scott Jiang]

This controller is only for blackfin 5xx soc, so rename it to BFIN5XX

Signed-off-by: Scott Jiang <scott.jiang.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/spi/Kconfig  |    2 +-
 drivers/spi/Makefile |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 3f9a47e..7a07b02 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -74,7 +74,7 @@ config SPI_ATMEL
 	  This selects a driver for the Atmel SPI Controller, present on
 	  many AT32 (AVR32) and AT91 (ARM) chips.
 
-config SPI_BFIN
+config SPI_BFIN5XX
 	tristate "SPI controller driver for ADI Blackfin5xx"
 	depends on BLACKFIN
 	help
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 61c3261..e391be8 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -14,7 +14,7 @@ obj-$(CONFIG_SPI_ALTERA)		+= spi-altera.o
 obj-$(CONFIG_SPI_ATMEL)			+= spi-atmel.o
 obj-$(CONFIG_SPI_ATH79)			+= spi-ath79.o
 obj-$(CONFIG_SPI_AU1550)		+= spi-au1550.o
-obj-$(CONFIG_SPI_BFIN)			+= spi-bfin5xx.o
+obj-$(CONFIG_SPI_BFIN5XX)		+= spi-bfin5xx.o
 obj-$(CONFIG_SPI_BFIN_SPORT)		+= spi-bfin-sport.o
 obj-$(CONFIG_SPI_BITBANG)		+= spi-bitbang.o
 obj-$(CONFIG_SPI_BUTTERFLY)		+= spi-butterfly.o
-- 
1.7.0.4

[PATCH 2/6] spi: spi-bfin-sport: move word length setup to transfer handler

[Scott Jiang]

Each transfer may have its own bits per word.

Signed-off-by: Scott Jiang <scott.jiang.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/spi/spi-bfin-sport.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-bfin-sport.c b/drivers/spi/spi-bfin-sport.c
index 248a2cc..bfd9047 100644
--- a/drivers/spi/spi-bfin-sport.c
+++ b/drivers/spi/spi-bfin-sport.c
@@ -252,19 +252,15 @@ static void
 bfin_sport_spi_restore_state(struct bfin_sport_spi_master_data *drv_data)
 {
 	struct bfin_sport_spi_slave_data *chip = drv_data->cur_chip;
-	unsigned int bits = (drv_data->ops == &bfin_sport_transfer_ops_u8 ? 7 : 15);
 
 	bfin_sport_spi_disable(drv_data);
 	dev_dbg(drv_data->dev, "restoring spi ctl state\n");
 
 	bfin_write(&drv_data->regs->tcr1, chip->ctl_reg);
-	bfin_write(&drv_data->regs->tcr2, bits);
 	bfin_write(&drv_data->regs->tclkdiv, chip->baud);
-	bfin_write(&drv_data->regs->tfsdiv, bits);
 	SSYNC();
 
 	bfin_write(&drv_data->regs->rcr1, chip->ctl_reg & ~(ITCLK | ITFS));
-	bfin_write(&drv_data->regs->rcr2, bits);
 	SSYNC();
 
 	bfin_sport_spi_cs_active(chip);
@@ -425,6 +421,9 @@ bfin_sport_spi_pump_transfers(unsigned long data)
 		drv_data->ops = &bfin_sport_transfer_ops_u8;
 	else
 		drv_data->ops = &bfin_sport_transfer_ops_u16;
+	bfin_write(&drv_data->regs->tcr2, bits_per_word - 1);
+	bfin_write(&drv_data->regs->tfsdiv, bits_per_word - 1);
+	bfin_write(&drv_data->regs->rcr2, bits_per_word - 1);
 
 	drv_data->state = RUNNING_STATE;
 
-- 
1.7.0.4



------------------------------------------------------------------------------
For Developers, A Lot Can Happen In A Second.
Boundary is the first to Know...and Tell You.
Monitor Your Applications in Ultra-Fine Resolution. Try it FREE!
http://p.sf.net/sfu/Boundary-d2dvs2

Re: [PATCH 2/6] spi: spi-bfin-sport: move word length setup to transfer handler

[Grant Likely]

On Mon, 23 Apr 2012 18:18:09 -0400, Scott Jiang <scott.jiang.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Each transfer may have its own bits per word.
> 
> Signed-off-by: Scott Jiang <scott.jiang.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Applied, thanks.

g.

> ---
>  drivers/spi/spi-bfin-sport.c |    7 +++----
>  1 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/spi/spi-bfin-sport.c b/drivers/spi/spi-bfin-sport.c
> index 248a2cc..bfd9047 100644
> --- a/drivers/spi/spi-bfin-sport.c
> +++ b/drivers/spi/spi-bfin-sport.c
> @@ -252,19 +252,15 @@ static void
>  bfin_sport_spi_restore_state(struct bfin_sport_spi_master_data *drv_data)
>  {
>  	struct bfin_sport_spi_slave_data *chip = drv_data->cur_chip;
> -	unsigned int bits = (drv_data->ops == &bfin_sport_transfer_ops_u8 ? 7 : 15);
>  
>  	bfin_sport_spi_disable(drv_data);
>  	dev_dbg(drv_data->dev, "restoring spi ctl state\n");
>  
>  	bfin_write(&drv_data->regs->tcr1, chip->ctl_reg);
> -	bfin_write(&drv_data->regs->tcr2, bits);
>  	bfin_write(&drv_data->regs->tclkdiv, chip->baud);
> -	bfin_write(&drv_data->regs->tfsdiv, bits);
>  	SSYNC();
>  
>  	bfin_write(&drv_data->regs->rcr1, chip->ctl_reg & ~(ITCLK | ITFS));
> -	bfin_write(&drv_data->regs->rcr2, bits);
>  	SSYNC();
>  
>  	bfin_sport_spi_cs_active(chip);
> @@ -425,6 +421,9 @@ bfin_sport_spi_pump_transfers(unsigned long data)
>  		drv_data->ops = &bfin_sport_transfer_ops_u8;
>  	else
>  		drv_data->ops = &bfin_sport_transfer_ops_u16;
> +	bfin_write(&drv_data->regs->tcr2, bits_per_word - 1);
> +	bfin_write(&drv_data->regs->tfsdiv, bits_per_word - 1);
> +	bfin_write(&drv_data->regs->rcr2, bits_per_word - 1);
>  
>  	drv_data->state = RUNNING_STATE;
>  
> -- 
> 1.7.0.4
> 
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/

Re: [PATCH 2/6] spi: spi-bfin-sport: move word length setup to transfer handler

[Grant Likely]

On Mon, 23 Apr 2012 18:18:09 -0400, Scott Jiang <scott.jiang.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Each transfer may have its own bits per word.
> 
> Signed-off-by: Scott Jiang <scott.jiang.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Applied, thanks.

g.

> ---
>  drivers/spi/spi-bfin-sport.c |    7 +++----
>  1 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/spi/spi-bfin-sport.c b/drivers/spi/spi-bfin-sport.c
> index 248a2cc..bfd9047 100644
> --- a/drivers/spi/spi-bfin-sport.c
> +++ b/drivers/spi/spi-bfin-sport.c
> @@ -252,19 +252,15 @@ static void
>  bfin_sport_spi_restore_state(struct bfin_sport_spi_master_data *drv_data)
>  {
>  	struct bfin_sport_spi_slave_data *chip = drv_data->cur_chip;
> -	unsigned int bits = (drv_data->ops == &bfin_sport_transfer_ops_u8 ? 7 : 15);
>  
>  	bfin_sport_spi_disable(drv_data);
>  	dev_dbg(drv_data->dev, "restoring spi ctl state\n");
>  
>  	bfin_write(&drv_data->regs->tcr1, chip->ctl_reg);
> -	bfin_write(&drv_data->regs->tcr2, bits);
>  	bfin_write(&drv_data->regs->tclkdiv, chip->baud);
> -	bfin_write(&drv_data->regs->tfsdiv, bits);
>  	SSYNC();
>  
>  	bfin_write(&drv_data->regs->rcr1, chip->ctl_reg & ~(ITCLK | ITFS));
> -	bfin_write(&drv_data->regs->rcr2, bits);
>  	SSYNC();
>  
>  	bfin_sport_spi_cs_active(chip);
> @@ -425,6 +421,9 @@ bfin_sport_spi_pump_transfers(unsigned long data)
>  		drv_data->ops = &bfin_sport_transfer_ops_u8;
>  	else
>  		drv_data->ops = &bfin_sport_transfer_ops_u16;
> +	bfin_write(&drv_data->regs->tcr2, bits_per_word - 1);
> +	bfin_write(&drv_data->regs->tfsdiv, bits_per_word - 1);
> +	bfin_write(&drv_data->regs->rcr2, bits_per_word - 1);
>  
>  	drv_data->state = RUNNING_STATE;
>  
> -- 
> 1.7.0.4
> 
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/

[PATCH 3/6] spi/bfin_spi: drop bits_per_word from client data

[Scott Jiang]

No other SPI controller has this field, and SPI clients should be setting
this up in their own drivers.  So drop it from the Blackfin controller to
keep people from using it.

Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
Signed-off-by: Scott Jiang <scott.jiang.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/spi/spi-bfin5xx.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/spi/spi-bfin5xx.c b/drivers/spi/spi-bfin5xx.c
index 3b83ff8..c0cdcb7 100644
--- a/drivers/spi/spi-bfin5xx.c
+++ b/drivers/spi/spi-bfin5xx.c
@@ -1026,7 +1026,6 @@ static int bfin_spi_setup(struct spi_device *spi)
 		chip->cs_chg_udelay = chip_info->cs_chg_udelay;
 		chip->idle_tx_val = chip_info->idle_tx_val;
 		chip->pio_interrupt = chip_info->pio_interrupt;
-		spi->bits_per_word = chip_info->bits_per_word;
 	} else {
 		/* force a default base state */
 		chip->ctl_reg &= bfin_ctl_reg;
-- 
1.7.0.4



------------------------------------------------------------------------------
For Developers, A Lot Can Happen In A Second.
Boundary is the first to Know...and Tell You.
Monitor Your Applications in Ultra-Fine Resolution. Try it FREE!
http://p.sf.net/sfu/Boundary-d2dvs2

Re: [PATCH 3/6] spi/bfin_spi: drop bits_per_word from client data

[Grant Likely]

On Mon, 23 Apr 2012 18:18:10 -0400, Scott Jiang <scott.jiang.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> No other SPI controller has this field, and SPI clients should be setting
> this up in their own drivers.  So drop it from the Blackfin controller to
> keep people from using it.
> 
> Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Scott Jiang <scott.jiang.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Applied, thanks.

g.

> ---
>  drivers/spi/spi-bfin5xx.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/spi/spi-bfin5xx.c b/drivers/spi/spi-bfin5xx.c
> index 3b83ff8..c0cdcb7 100644
> --- a/drivers/spi/spi-bfin5xx.c
> +++ b/drivers/spi/spi-bfin5xx.c
> @@ -1026,7 +1026,6 @@ static int bfin_spi_setup(struct spi_device *spi)
>  		chip->cs_chg_udelay = chip_info->cs_chg_udelay;
>  		chip->idle_tx_val = chip_info->idle_tx_val;
>  		chip->pio_interrupt = chip_info->pio_interrupt;
> -		spi->bits_per_word = chip_info->bits_per_word;
>  	} else {
>  		/* force a default base state */
>  		chip->ctl_reg &= bfin_ctl_reg;
> -- 
> 1.7.0.4
> 
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/

[PATCH 4/6] spi/spi_bfin_sport: drop bits_per_word from client data

[Scott Jiang]

Since the member was dropped from the common Blackfin header, we need
to stop using it in the SPORT driver too.

Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
Signed-off-by: Scott Jiang <scott.jiang.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/spi/spi-bfin-sport.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/spi/spi-bfin-sport.c b/drivers/spi/spi-bfin-sport.c
index bfd9047..1fe5119 100644
--- a/drivers/spi/spi-bfin-sport.c
+++ b/drivers/spi/spi-bfin-sport.c
@@ -416,11 +416,12 @@ bfin_sport_spi_pump_transfers(unsigned long data)
 	drv_data->cs_change = transfer->cs_change;
 
 	/* Bits per word setup */
-	bits_per_word = transfer->bits_per_word ? : message->spi->bits_per_word;
-	if (bits_per_word == 8)
-		drv_data->ops = &bfin_sport_transfer_ops_u8;
-	else
+	bits_per_word = transfer->bits_per_word ? :
+		message->spi->bits_per_word ? : 8;
+	if (bits_per_word % 16 == 0)
 		drv_data->ops = &bfin_sport_transfer_ops_u16;
+	else
+		drv_data->ops = &bfin_sport_transfer_ops_u8;
 	bfin_write(&drv_data->regs->tcr2, bits_per_word - 1);
 	bfin_write(&drv_data->regs->tfsdiv, bits_per_word - 1);
 	bfin_write(&drv_data->regs->rcr2, bits_per_word - 1);
@@ -597,11 +598,12 @@ bfin_sport_spi_setup(struct spi_device *spi)
 			}
 			chip->cs_chg_udelay = chip_info->cs_chg_udelay;
 			chip->idle_tx_val = chip_info->idle_tx_val;
-			spi->bits_per_word = chip_info->bits_per_word;
 		}
 	}
 
-	if (spi->bits_per_word != 8 && spi->bits_per_word != 16) {
+	if (spi->bits_per_word % 8) {
+		dev_err(&spi->dev, "%d bits_per_word is not supported\n",
+				spi->bits_per_word);
 		ret = -EINVAL;
 		goto error;
 	}
-- 
1.7.0.4



------------------------------------------------------------------------------
For Developers, A Lot Can Happen In A Second.
Boundary is the first to Know...and Tell You.
Monitor Your Applications in Ultra-Fine Resolution. Try it FREE!
http://p.sf.net/sfu/Boundary-d2dvs2

Re: [PATCH 4/6] spi/spi_bfin_sport: drop bits_per_word from client data

[Grant Likely]

On Mon, 23 Apr 2012 18:18:11 -0400, Scott Jiang <scott.jiang.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Since the member was dropped from the common Blackfin header, we need
> to stop using it in the SPORT driver too.
> 
> Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Scott Jiang <scott.jiang.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Applied, thanks

g.

> ---
>  drivers/spi/spi-bfin-sport.c |   14 ++++++++------
>  1 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/spi/spi-bfin-sport.c b/drivers/spi/spi-bfin-sport.c
> index bfd9047..1fe5119 100644
> --- a/drivers/spi/spi-bfin-sport.c
> +++ b/drivers/spi/spi-bfin-sport.c
> @@ -416,11 +416,12 @@ bfin_sport_spi_pump_transfers(unsigned long data)
>  	drv_data->cs_change = transfer->cs_change;
>  
>  	/* Bits per word setup */
> -	bits_per_word = transfer->bits_per_word ? : message->spi->bits_per_word;
> -	if (bits_per_word == 8)
> -		drv_data->ops = &bfin_sport_transfer_ops_u8;
> -	else
> +	bits_per_word = transfer->bits_per_word ? :
> +		message->spi->bits_per_word ? : 8;
> +	if (bits_per_word % 16 == 0)
>  		drv_data->ops = &bfin_sport_transfer_ops_u16;
> +	else
> +		drv_data->ops = &bfin_sport_transfer_ops_u8;
>  	bfin_write(&drv_data->regs->tcr2, bits_per_word - 1);
>  	bfin_write(&drv_data->regs->tfsdiv, bits_per_word - 1);
>  	bfin_write(&drv_data->regs->rcr2, bits_per_word - 1);
> @@ -597,11 +598,12 @@ bfin_sport_spi_setup(struct spi_device *spi)
>  			}
>  			chip->cs_chg_udelay = chip_info->cs_chg_udelay;
>  			chip->idle_tx_val = chip_info->idle_tx_val;
> -			spi->bits_per_word = chip_info->bits_per_word;
>  		}
>  	}
>  
> -	if (spi->bits_per_word != 8 && spi->bits_per_word != 16) {
> +	if (spi->bits_per_word % 8) {
> +		dev_err(&spi->dev, "%d bits_per_word is not supported\n",
> +				spi->bits_per_word);
>  		ret = -EINVAL;
>  		goto error;
>  	}
> -- 
> 1.7.0.4
> 
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/

[PATCH 5/6] spi: spi-bfin5xx: reverse if condition in interrupt mode

[Scott Jiang]

This condition is used to determine 8 bits or 16 and 32 bits transfer. Obviously it is reversed.

Signed-off-by: Scott Jiang <scott.jiang.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/spi/spi-bfin5xx.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-bfin5xx.c b/drivers/spi/spi-bfin5xx.c
index c0cdcb7..432d019 100644
--- a/drivers/spi/spi-bfin5xx.c
+++ b/drivers/spi/spi-bfin5xx.c
@@ -396,7 +396,7 @@ static irqreturn_t bfin_spi_pio_irq_handler(int irq, void *dev_id)
 		/* last read */
 		if (drv_data->rx) {
 			dev_dbg(&drv_data->pdev->dev, "last read\n");
-			if (n_bytes % 2) {
+			if (!(n_bytes % 2)) {
 				u16 *buf = (u16 *)drv_data->rx;
 				for (loop = 0; loop < n_bytes / 2; loop++)
 					*buf++ = bfin_read(&drv_data->regs->rdbr);
@@ -424,7 +424,7 @@ static irqreturn_t bfin_spi_pio_irq_handler(int irq, void *dev_id)
 	if (drv_data->rx && drv_data->tx) {
 		/* duplex */
 		dev_dbg(&drv_data->pdev->dev, "duplex: write_TDBR\n");
-		if (n_bytes % 2) {
+		if (!(n_bytes % 2)) {
 			u16 *buf = (u16 *)drv_data->rx;
 			u16 *buf2 = (u16 *)drv_data->tx;
 			for (loop = 0; loop < n_bytes / 2; loop++) {
@@ -442,7 +442,7 @@ static irqreturn_t bfin_spi_pio_irq_handler(int irq, void *dev_id)
 	} else if (drv_data->rx) {
 		/* read */
 		dev_dbg(&drv_data->pdev->dev, "read: write_TDBR\n");
-		if (n_bytes % 2) {
+		if (!(n_bytes % 2)) {
 			u16 *buf = (u16 *)drv_data->rx;
 			for (loop = 0; loop < n_bytes / 2; loop++) {
 				*buf++ = bfin_read(&drv_data->regs->rdbr);
@@ -458,7 +458,7 @@ static irqreturn_t bfin_spi_pio_irq_handler(int irq, void *dev_id)
 	} else if (drv_data->tx) {
 		/* write */
 		dev_dbg(&drv_data->pdev->dev, "write: write_TDBR\n");
-		if (n_bytes % 2) {
+		if (!(n_bytes % 2)) {
 			u16 *buf = (u16 *)drv_data->tx;
 			for (loop = 0; loop < n_bytes / 2; loop++) {
 				bfin_read(&drv_data->regs->rdbr);
-- 
1.7.0.4

Re: [PATCH 5/6] spi: spi-bfin5xx: reverse if condition in interrupt mode

[Grant Likely]

On Mon, 23 Apr 2012 18:18:12 -0400, Scott Jiang <scott.jiang.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> This condition is used to determine 8 bits or 16 and 32 bits transfer. Obviously it is reversed.
> 
> Signed-off-by: Scott Jiang <scott.jiang.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Applied, thanks.

g.

> ---
>  drivers/spi/spi-bfin5xx.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/spi/spi-bfin5xx.c b/drivers/spi/spi-bfin5xx.c
> index c0cdcb7..432d019 100644
> --- a/drivers/spi/spi-bfin5xx.c
> +++ b/drivers/spi/spi-bfin5xx.c
> @@ -396,7 +396,7 @@ static irqreturn_t bfin_spi_pio_irq_handler(int irq, void *dev_id)
>  		/* last read */
>  		if (drv_data->rx) {
>  			dev_dbg(&drv_data->pdev->dev, "last read\n");
> -			if (n_bytes % 2) {
> +			if (!(n_bytes % 2)) {
>  				u16 *buf = (u16 *)drv_data->rx;
>  				for (loop = 0; loop < n_bytes / 2; loop++)
>  					*buf++ = bfin_read(&drv_data->regs->rdbr);
> @@ -424,7 +424,7 @@ static irqreturn_t bfin_spi_pio_irq_handler(int irq, void *dev_id)
>  	if (drv_data->rx && drv_data->tx) {
>  		/* duplex */
>  		dev_dbg(&drv_data->pdev->dev, "duplex: write_TDBR\n");
> -		if (n_bytes % 2) {
> +		if (!(n_bytes % 2)) {
>  			u16 *buf = (u16 *)drv_data->rx;
>  			u16 *buf2 = (u16 *)drv_data->tx;
>  			for (loop = 0; loop < n_bytes / 2; loop++) {
> @@ -442,7 +442,7 @@ static irqreturn_t bfin_spi_pio_irq_handler(int irq, void *dev_id)
>  	} else if (drv_data->rx) {
>  		/* read */
>  		dev_dbg(&drv_data->pdev->dev, "read: write_TDBR\n");
> -		if (n_bytes % 2) {
> +		if (!(n_bytes % 2)) {
>  			u16 *buf = (u16 *)drv_data->rx;
>  			for (loop = 0; loop < n_bytes / 2; loop++) {
>  				*buf++ = bfin_read(&drv_data->regs->rdbr);
> @@ -458,7 +458,7 @@ static irqreturn_t bfin_spi_pio_irq_handler(int irq, void *dev_id)
>  	} else if (drv_data->tx) {
>  		/* write */
>  		dev_dbg(&drv_data->pdev->dev, "write: write_TDBR\n");
> -		if (n_bytes % 2) {
> +		if (!(n_bytes % 2)) {
>  			u16 *buf = (u16 *)drv_data->tx;
>  			for (loop = 0; loop < n_bytes / 2; loop++) {
>  				bfin_read(&drv_data->regs->rdbr);
> -- 
> 1.7.0.4
> 
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/

[PATCH 6/6] spi: spi-bfin5xx: flush spi after each transfer

[Scott Jiang]

This can make sure last bit has been shifted out of register.

Signed-off-by: Scott Jiang <scott.jiang.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/spi/spi-bfin5xx.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/spi/spi-bfin5xx.c b/drivers/spi/spi-bfin5xx.c
index 432d019..9bb4d4a 100644
--- a/drivers/spi/spi-bfin5xx.c
+++ b/drivers/spi/spi-bfin5xx.c
@@ -587,6 +587,7 @@ static void bfin_spi_pump_transfers(unsigned long data)
 	if (message->state == DONE_STATE) {
 		dev_dbg(&drv_data->pdev->dev, "transfer: all done!\n");
 		message->status = 0;
+		bfin_spi_flush(drv_data);
 		bfin_spi_giveback(drv_data);
 		return;
 	}
@@ -870,8 +871,10 @@ static void bfin_spi_pump_transfers(unsigned long data)
 		message->actual_length += drv_data->len_in_bytes;
 		/* Move to next transfer of this msg */
 		message->state = bfin_spi_next_transfer(drv_data);
-		if (drv_data->cs_change)
+		if (drv_data->cs_change && message->state != DONE_STATE) {
+			bfin_spi_flush(drv_data);
 			bfin_spi_cs_deactive(drv_data, chip);
+		}
 	}
 
 	/* Schedule next transfer tasklet */
-- 
1.7.0.4



------------------------------------------------------------------------------
For Developers, A Lot Can Happen In A Second.
Boundary is the first to Know...and Tell You.
Monitor Your Applications in Ultra-Fine Resolution. Try it FREE!
http://p.sf.net/sfu/Boundary-d2dvs2

Re: [PATCH 6/6] spi: spi-bfin5xx: flush spi after each transfer

[Grant Likely]

On Mon, 23 Apr 2012 18:18:13 -0400, Scott Jiang <scott.jiang.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> This can make sure last bit has been shifted out of register.
> 
> Signed-off-by: Scott Jiang <scott.jiang.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Okay, so I have a few issues about the way this series was
submitted, but I've gone ahead and applied them anyway to get them off
my plate since they look like bug fixes.

The problem is that the commit text is far from complete.  Take for
example the commit text from this patch:

> This can make sure last bit has been shifted out of register.

So, what does that tell me?  Not a whole lot.  It tells me that a bit
needs to be shifted out, but nothing about when or where that is
needed.  I can *guess* that this is a bug fix, but there is nowhere
that says that is what it does.

The commit text needs to give a lot more information about why a patch
is needed.  It should answer the following questions:
1) What is the problem
2) how does the problem occur; what are the symptoms
3) A description of the fix and/or how this patch fixes the problem
4) some details about how the patch was tested.

So, for example, I might write something like this for the commit
text (I'm guessing at details here for the purpose of an example):

	The bfin5xx spi driver has a bug where the last bit of a
	transfer does not get properly shifted out onto the bus until
	another transfer gets scheduled.  This means that the last
	transfer sent will stall and never complete if there aren't
	any other transfers ready to be sent.

	This patch fixes the problem by calling the bfin_spi_flush()
	routine at the end of each transfer which ensures that all
	bits get properly shifted out when a transfer completes.

	Tested on the ACME Magic 8-ball blackfin board attached to a
	spi LED strip.

See?  That tells me immediately that this is a bug fix and specific
problem it solves which helps me decide whether or not I need to apply
it for -next or -merge.  It also helps anyone in the future if they
need to bisect changes on this driver.

For future changes, I expect the commit text to be more complete.

Thanks,
g.

> ---
>  drivers/spi/spi-bfin5xx.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/spi/spi-bfin5xx.c b/drivers/spi/spi-bfin5xx.c
> index 432d019..9bb4d4a 100644
> --- a/drivers/spi/spi-bfin5xx.c
> +++ b/drivers/spi/spi-bfin5xx.c
> @@ -587,6 +587,7 @@ static void bfin_spi_pump_transfers(unsigned long data)
>  	if (message->state == DONE_STATE) {
>  		dev_dbg(&drv_data->pdev->dev, "transfer: all done!\n");
>  		message->status = 0;
> +		bfin_spi_flush(drv_data);
>  		bfin_spi_giveback(drv_data);
>  		return;
>  	}
> @@ -870,8 +871,10 @@ static void bfin_spi_pump_transfers(unsigned long data)
>  		message->actual_length += drv_data->len_in_bytes;
>  		/* Move to next transfer of this msg */
>  		message->state = bfin_spi_next_transfer(drv_data);
> -		if (drv_data->cs_change)
> +		if (drv_data->cs_change && message->state != DONE_STATE) {
> +			bfin_spi_flush(drv_data);
>  			bfin_spi_cs_deactive(drv_data, chip);
> +		}
>  	}
>  
>  	/* Schedule next transfer tasklet */
> -- 
> 1.7.0.4
> 
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/

Re: [PATCH 6/6] spi: spi-bfin5xx: flush spi after each transfer

[Mike Frysinger]

[-- Attachment #1.1: Type: Text/Plain, Size: 1728 bytes --]

On Friday 27 April 2012 14:35:01 Grant Likely wrote:
> On Mon, 23 Apr 2012 18:18:13 -0400, Scott Jiang 
<scott.jiang.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > This can make sure last bit has been shifted out of register.
> > 
> > Signed-off-by: Scott Jiang <scott.jiang.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> Okay, so I have a few issues about the way this series was
> submitted, but I've gone ahead and applied them anyway to get them off
> my plate since they look like bug fixes.
> 
> The problem is that the commit text is far from complete.

here's the original commit messages:

commit b489d522f6445de7261f4e57a52b6abe24717450
Author: Sonic Zhang <sonic.zhang-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
Date:   Fri Aug 12 10:44:43 2011 +0800

    bug[#6683] spi:spi_bfin5xx: SPI SSEL deasserted too early in soft irq mode.
    
    Should poll FIFO in last dummy pump_transfer.
    
    Signed-off-by: Sonic Zhang <sonic.zhang-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>

commit 6201ee95d78b555b656cda6fadb91c031f90408c
Author: Sonic Zhang <sonic.zhang-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
Date:   Thu Aug 11 18:24:16 2011 +0800

    bug[#6683] spi:spi_bfin5xx: SPI SSEL deasserted too early in soft irq mode.
    
    Poll the FIFO till it is empty before deassert SSEL in pump_transfers in
    soft irq mode. No polling is necessary in interrupt mode and error 
handling.
    
    Signed-off-by: Sonic Zhang <sonic.zhang-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>

and here's the referenced bug:
	https://blackfin.uclinux.org/gf/tracker/6683

and here's a logic analyzer showing the problem :)
	http://blackfin.uclinux.org/gf/forumthread/44481
-mike

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 213 bytes --]

_______________________________________________
Uclinux-dist-devel mailing list
Uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b@public.gmane.org
https://blackfin.uclinux.org/mailman/listinfo/uclinux-dist-devel

Re: [PATCH 1/6] spi: rename config macro name for bfin5xx spi controller driver

[Grant Likely]

On Mon, 23 Apr 2012 18:18:08 -0400, Scott Jiang <scott.jiang.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> This controller is only for blackfin 5xx soc, so rename it to BFIN5XX
> 
> Signed-off-by: Scott Jiang <scott.jiang.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Is this series bug fixes that need to get into v3.4?

g.

> ---
>  drivers/spi/Kconfig  |    2 +-
>  drivers/spi/Makefile |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index 3f9a47e..7a07b02 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -74,7 +74,7 @@ config SPI_ATMEL
>  	  This selects a driver for the Atmel SPI Controller, present on
>  	  many AT32 (AVR32) and AT91 (ARM) chips.
>  
> -config SPI_BFIN
> +config SPI_BFIN5XX
>  	tristate "SPI controller driver for ADI Blackfin5xx"
>  	depends on BLACKFIN
>  	help
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index 61c3261..e391be8 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -14,7 +14,7 @@ obj-$(CONFIG_SPI_ALTERA)		+= spi-altera.o
>  obj-$(CONFIG_SPI_ATMEL)			+= spi-atmel.o
>  obj-$(CONFIG_SPI_ATH79)			+= spi-ath79.o
>  obj-$(CONFIG_SPI_AU1550)		+= spi-au1550.o
> -obj-$(CONFIG_SPI_BFIN)			+= spi-bfin5xx.o
> +obj-$(CONFIG_SPI_BFIN5XX)		+= spi-bfin5xx.o
>  obj-$(CONFIG_SPI_BFIN_SPORT)		+= spi-bfin-sport.o
>  obj-$(CONFIG_SPI_BITBANG)		+= spi-bitbang.o
>  obj-$(CONFIG_SPI_BUTTERFLY)		+= spi-butterfly.o
> -- 
> 1.7.0.4
> 
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/

Re: [PATCH 1/6] spi: rename config macro name for bfin5xx spi controller driver

[Mike Frysinger]

[-- Attachment #1.1: Type: Text/Plain, Size: 464 bytes --]

On Friday 27 April 2012 13:21:51 Grant Likely wrote:
> On Mon, 23 Apr 2012 18:18:08 -0400, Scott Jiang wrote:
> > This controller is only for blackfin 5xx soc, so rename it to BFIN5XX
> > 
> > Signed-off-by: Scott Jiang <scott.jiang.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> Is this series bug fixes that need to get into v3.4?

i think so ... i asked Scott to mark the ones that needed to be in 3.4, but i 
guess that msg got missed :/
-mike

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 213 bytes --]

_______________________________________________
Uclinux-dist-devel mailing list
Uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b@public.gmane.org
https://blackfin.uclinux.org/mailman/listinfo/uclinux-dist-devel

Re: [PATCH 1/6] spi: rename config macro name for bfin5xx spi controller driver

[Grant Likely]

On Mon, 23 Apr 2012 18:18:08 -0400, Scott Jiang <scott.jiang.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> This controller is only for blackfin 5xx soc, so rename it to BFIN5XX
> 
> Signed-off-by: Scott Jiang <scott.jiang.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Applied, thanks.

g.

> ---
>  drivers/spi/Kconfig  |    2 +-
>  drivers/spi/Makefile |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index 3f9a47e..7a07b02 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -74,7 +74,7 @@ config SPI_ATMEL
>  	  This selects a driver for the Atmel SPI Controller, present on
>  	  many AT32 (AVR32) and AT91 (ARM) chips.
>  
> -config SPI_BFIN
> +config SPI_BFIN5XX
>  	tristate "SPI controller driver for ADI Blackfin5xx"
>  	depends on BLACKFIN
>  	help
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index 61c3261..e391be8 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -14,7 +14,7 @@ obj-$(CONFIG_SPI_ALTERA)		+= spi-altera.o
>  obj-$(CONFIG_SPI_ATMEL)			+= spi-atmel.o
>  obj-$(CONFIG_SPI_ATH79)			+= spi-ath79.o
>  obj-$(CONFIG_SPI_AU1550)		+= spi-au1550.o
> -obj-$(CONFIG_SPI_BFIN)			+= spi-bfin5xx.o
> +obj-$(CONFIG_SPI_BFIN5XX)		+= spi-bfin5xx.o
>  obj-$(CONFIG_SPI_BFIN_SPORT)		+= spi-bfin-sport.o
>  obj-$(CONFIG_SPI_BITBANG)		+= spi-bitbang.o
>  obj-$(CONFIG_SPI_BUTTERFLY)		+= spi-butterfly.o
> -- 
> 1.7.0.4
> 
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/