Bug reported by Jean-Christian de Rivaz <[email protected]> and he also gave a patch.
http://blackfin.uclinux.org/gf/project/uclinux-dist/tracker/?action=TrackerItemEdit&tracker_item_id=3678
So revert spi_bfin5xx.c duplex operation.
Signed-off-by: Jean-Christian de Rivaz <[email protected]>
Signed-off-by: Bryan Wu <[email protected]>
---
drivers/spi/spi_bfin5xx.c | 18 +++---------------
1 files changed, 3 insertions(+), 15 deletions(-)
diff --git a/drivers/spi/spi_bfin5xx.c b/drivers/spi/spi_bfin5xx.c
index ceb5b53..c5d4d77 100644
--- a/drivers/spi/spi_bfin5xx.c
+++ b/drivers/spi/spi_bfin5xx.c
@@ -358,14 +358,10 @@ static void u8_cs_chg_reader(struct driver_data *drv_data)
static void u8_duplex(struct driver_data *drv_data)
{
- /* poll for SPI completion before start */
- while (!(read_STAT(drv_data) & BIT_STAT_SPIF))
- cpu_relax();
-
/* in duplex mode, clk is triggered by writing of TDBR */
while (drv_data->rx < drv_data->rx_end) {
write_TDBR(drv_data, (*(u8 *) (drv_data->tx)));
- while (read_STAT(drv_data) & BIT_STAT_TXS)
+ while (!(read_STAT(drv_data) & BIT_STAT_SPIF))
cpu_relax();
while (!(read_STAT(drv_data) & BIT_STAT_RXS))
cpu_relax();
@@ -495,14 +491,10 @@ static void u16_cs_chg_reader(struct driver_data *drv_data)
static void u16_duplex(struct driver_data *drv_data)
{
- /* poll for SPI completion before start */
- while (!(read_STAT(drv_data) & BIT_STAT_SPIF))
- cpu_relax();
-
/* in duplex mode, clk is triggered by writing of TDBR */
while (drv_data->tx < drv_data->tx_end) {
write_TDBR(drv_data, (*(u16 *) (drv_data->tx)));
- while (read_STAT(drv_data) & BIT_STAT_TXS)
+ while (!(read_STAT(drv_data) & BIT_STAT_SPIF))
cpu_relax();
while (!(read_STAT(drv_data) & BIT_STAT_RXS))
cpu_relax();
@@ -516,15 +508,11 @@ static void u16_cs_chg_duplex(struct driver_data *drv_data)
{
struct chip_data *chip = drv_data->cur_chip;
- /* poll for SPI completion before start */
- while (!(read_STAT(drv_data) & BIT_STAT_SPIF))
- cpu_relax();
-
while (drv_data->tx < drv_data->tx_end) {
cs_active(drv_data, chip);
write_TDBR(drv_data, (*(u16 *) (drv_data->tx)));
- while (read_STAT(drv_data) & BIT_STAT_TXS)
+ while (!(read_STAT(drv_data) & BIT_STAT_SPIF))
cpu_relax();
while (!(read_STAT(drv_data) & BIT_STAT_RXS))
cpu_relax();
--
1.5.3.4
Cc: Michael Hennerich <[email protected]>
Signed-off-by: Bryan Wu <[email protected]>
---
drivers/spi/spi_bfin5xx.c | 15 ++-------------
1 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/drivers/spi/spi_bfin5xx.c b/drivers/spi/spi_bfin5xx.c
index 7ef39a6..8fd1365 100644
--- a/drivers/spi/spi_bfin5xx.c
+++ b/drivers/spi/spi_bfin5xx.c
@@ -223,10 +223,9 @@ static void cs_deactive(struct driver_data *drv_data, struct chip_data *chip)
#define MAX_SPI_SSEL 7
/* stop controller and re-config current chip*/
-static int restore_state(struct driver_data *drv_data)
+static void restore_state(struct driver_data *drv_data)
{
struct chip_data *chip = drv_data->cur_chip;
- int ret = 0;
/* Clear status and disable clock */
write_STAT(drv_data, BIT_STAT_CLR);
@@ -239,13 +238,6 @@ static int restore_state(struct driver_data *drv_data)
bfin_spi_enable(drv_data);
cs_active(drv_data, chip);
-
- if (ret)
- dev_dbg(&drv_data->pdev->dev,
- ": request chip select number %d failed\n",
- chip->chip_select_num);
-
- return ret;
}
/* used to kick off transfer in rx mode */
@@ -978,10 +970,7 @@ static void pump_messages(struct work_struct *work)
/* Setup the SSP using the per chip configuration */
drv_data->cur_chip = spi_get_ctldata(drv_data->cur_msg->spi);
- if (restore_state(drv_data)) {
- spin_unlock_irqrestore(&drv_data->lock, flags);
- return;
- };
+ restore_state(drv_data);
list_del_init(&drv_data->cur_msg->queue);
--
1.5.3.4
PBX 2 SPI device need cs change per word and this regression is due to commit
Signed-off-by: Bryan Wu <[email protected]>
---
drivers/spi/spi_bfin5xx.c | 40 ++++++++++++----------------------------
1 files changed, 12 insertions(+), 28 deletions(-)
diff --git a/drivers/spi/spi_bfin5xx.c b/drivers/spi/spi_bfin5xx.c
index c3f9ed4..ceb5b53 100644
--- a/drivers/spi/spi_bfin5xx.c
+++ b/drivers/spi/spi_bfin5xx.c
@@ -294,16 +294,14 @@ static void u8_cs_chg_writer(struct driver_data *drv_data)
{
struct chip_data *chip = drv_data->cur_chip;
- /* poll for SPI completion before start */
- while (!(read_STAT(drv_data) & BIT_STAT_SPIF))
- cpu_relax();
-
while (drv_data->tx < drv_data->tx_end) {
cs_active(drv_data, chip);
write_TDBR(drv_data, (*(u8 *) (drv_data->tx)));
while (read_STAT(drv_data) & BIT_STAT_TXS)
cpu_relax();
+ while (!(read_STAT(drv_data) & BIT_STAT_SPIF))
+ cpu_relax();
cs_deactive(drv_data, chip);
@@ -342,31 +340,20 @@ static void u8_cs_chg_reader(struct driver_data *drv_data)
{
struct chip_data *chip = drv_data->cur_chip;
- /* poll for SPI completion before start */
- while (!(read_STAT(drv_data) & BIT_STAT_SPIF))
- cpu_relax();
-
- /* clear TDBR buffer before read(else it will be shifted out) */
- write_TDBR(drv_data, 0xFFFF);
+ while (drv_data->rx < drv_data->rx_end) {
+ cs_active(drv_data, chip);
+ read_RDBR(drv_data); /* kick off */
- cs_active(drv_data, chip);
- dummy_read(drv_data);
+ while (!(read_STAT(drv_data) & BIT_STAT_RXS))
+ cpu_relax();
+ while (!(read_STAT(drv_data) & BIT_STAT_SPIF))
+ cpu_relax();
- while (drv_data->rx < drv_data->rx_end - 1) {
+ *(u8 *) (drv_data->rx) = read_SHAW(drv_data);
cs_deactive(drv_data, chip);
- while (!(read_STAT(drv_data) & BIT_STAT_RXS))
- cpu_relax();
- cs_active(drv_data, chip);
- *(u8 *) (drv_data->rx) = read_RDBR(drv_data);
++drv_data->rx;
}
- cs_deactive(drv_data, chip);
-
- while (!(read_STAT(drv_data) & BIT_STAT_RXS))
- cpu_relax();
- *(u8 *) (drv_data->rx) = read_SHAW(drv_data);
- ++drv_data->rx;
}
static void u8_duplex(struct driver_data *drv_data)
@@ -392,15 +379,12 @@ static void u8_cs_chg_duplex(struct driver_data *drv_data)
{
struct chip_data *chip = drv_data->cur_chip;
- /* poll for SPI completion before start */
- while (!(read_STAT(drv_data) & BIT_STAT_SPIF))
- cpu_relax();
-
while (drv_data->rx < drv_data->rx_end) {
cs_active(drv_data, chip);
write_TDBR(drv_data, (*(u8 *) (drv_data->tx)));
- while (read_STAT(drv_data) & BIT_STAT_TXS)
+
+ while (!(read_STAT(drv_data) & BIT_STAT_SPIF))
cpu_relax();
while (!(read_STAT(drv_data) & BIT_STAT_RXS))
cpu_relax();
--
1.5.3.4
From: Sonic Zhang <[email protected]>
Signed-off-by: Sonic Zhang <[email protected]>
Signed-off-by: Bryan Wu <[email protected]>
---
drivers/spi/spi_bfin5xx.c | 22 ++++++++++------------
1 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/spi/spi_bfin5xx.c b/drivers/spi/spi_bfin5xx.c
index c5d4d77..9f12639 100644
--- a/drivers/spi/spi_bfin5xx.c
+++ b/drivers/spi/spi_bfin5xx.c
@@ -278,16 +278,16 @@ static void u8_writer(struct driver_data *drv_data)
dev_dbg(&drv_data->pdev->dev,
"cr8-s is 0x%x\n", read_STAT(drv_data));
- /* poll for SPI completion before start */
- while (!(read_STAT(drv_data) & BIT_STAT_SPIF))
- cpu_relax();
-
while (drv_data->tx < drv_data->tx_end) {
write_TDBR(drv_data, (*(u8 *) (drv_data->tx)));
while (read_STAT(drv_data) & BIT_STAT_TXS)
cpu_relax();
++drv_data->tx;
}
+
+ /* poll for SPI completion before return */
+ while (!(read_STAT(drv_data) & BIT_STAT_SPIF))
+ cpu_relax();
}
static void u8_cs_chg_writer(struct driver_data *drv_data)
@@ -398,32 +398,30 @@ static void u16_writer(struct driver_data *drv_data)
dev_dbg(&drv_data->pdev->dev,
"cr16 is 0x%x\n", read_STAT(drv_data));
- /* poll for SPI completion before start */
- while (!(read_STAT(drv_data) & BIT_STAT_SPIF))
- cpu_relax();
-
while (drv_data->tx < drv_data->tx_end) {
write_TDBR(drv_data, (*(u16 *) (drv_data->tx)));
while ((read_STAT(drv_data) & BIT_STAT_TXS))
cpu_relax();
drv_data->tx += 2;
}
+
+ /* poll for SPI completion before return */
+ while (!(read_STAT(drv_data) & BIT_STAT_SPIF))
+ cpu_relax();
}
static void u16_cs_chg_writer(struct driver_data *drv_data)
{
struct chip_data *chip = drv_data->cur_chip;
- /* poll for SPI completion before start */
- while (!(read_STAT(drv_data) & BIT_STAT_SPIF))
- cpu_relax();
-
while (drv_data->tx < drv_data->tx_end) {
cs_active(drv_data, chip);
write_TDBR(drv_data, (*(u16 *) (drv_data->tx)));
while ((read_STAT(drv_data) & BIT_STAT_TXS))
cpu_relax();
+ while (!(read_STAT(drv_data) & BIT_STAT_SPIF))
+ cpu_relax();
cs_deactive(drv_data, chip);
--
1.5.3.4
From: Mike Frysinger <[email protected]>
Signed-off-by: Mike Frysinger <[email protected]>
Signed-off-by: Bryan Wu <[email protected]>
---
drivers/spi/spi_bfin5xx.c | 32 +++-----------------------------
1 files changed, 3 insertions(+), 29 deletions(-)
diff --git a/drivers/spi/spi_bfin5xx.c b/drivers/spi/spi_bfin5xx.c
index 9f12639..6338235 100644
--- a/drivers/spi/spi_bfin5xx.c
+++ b/drivers/spi/spi_bfin5xx.c
@@ -1,37 +1,11 @@
/*
- * File: drivers/spi/bfin5xx_spi.c
- * Maintainer:
- * Bryan Wu <[email protected]>
- * Original Author:
- * Luke Yang (Analog Devices Inc.)
- *
- * Created: March. 10th 2006
- * Description: SPI controller driver for Blackfin BF5xx
- * Bugs: Enter bugs at http://blackfin.uclinux.org/
- *
- * Modified:
- * March 10, 2006 bfin5xx_spi.c Created. (Luke Yang)
- * August 7, 2006 added full duplex mode (Axel Weiss & Luke Yang)
- * July 17, 2007 add support for BF54x SPI0 controller (Bryan Wu)
- * July 30, 2007 add platfrom_resource interface to support multi-port
- * SPI controller (Bryan Wu)
+ * Blackfin On-Chip SPI Driver
*
* Copyright 2004-2007 Analog Devices Inc.
*
- * This program is free software ; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation ; either version 2, or (at your option)
- * any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY ; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
+ * Enter bugs at http://blackfin.uclinux.org/
*
- * You should have received a copy of the GNU General Public License
- * along with this program ; see the file COPYING.
- * If not, write to the Free Software Foundation,
- * 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ * Licensed under the GPL-2 or later.
*/
#include <linux/init.h>
--
1.5.3.4
When got some gpio conflict, it is easy to know which SPI device driver not just got "bfin-spi"
Signed-off-by: Bryan Wu <[email protected]>
---
drivers/spi/spi_bfin5xx.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/spi/spi_bfin5xx.c b/drivers/spi/spi_bfin5xx.c
index 8fd1365..c3f9ed4 100644
--- a/drivers/spi/spi_bfin5xx.c
+++ b/drivers/spi/spi_bfin5xx.c
@@ -1176,7 +1176,7 @@ static int setup(struct spi_device *spi)
if ((chip->chip_select_num > 0)
&& (chip->chip_select_num <= spi->master->num_chipselect))
peripheral_request(ssel[spi->master->bus_num]
- [chip->chip_select_num-1], DRV_NAME);
+ [chip->chip_select_num-1], spi->modalias);
cs_deactive(drv_data, chip);
--
1.5.3.4
On Wednesday 30 January 2008, Bryan Wu wrote:
> When got some gpio conflict, it is easy to know which SPI device driver not just got "bfin-spi"
>
> Signed-off-by: Bryan Wu <[email protected]>
> ---
> drivers/spi/spi_bfin5xx.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/spi/spi_bfin5xx.c b/drivers/spi/spi_bfin5xx.c
> index 8fd1365..c3f9ed4 100644
> --- a/drivers/spi/spi_bfin5xx.c
> +++ b/drivers/spi/spi_bfin5xx.c
> @@ -1176,7 +1176,7 @@ static int setup(struct spi_device *spi)
> if ((chip->chip_select_num > 0)
> && (chip->chip_select_num <= spi->master->num_chipselect))
> peripheral_request(ssel[spi->master->bus_num]
> - [chip->chip_select_num-1], DRV_NAME);
> + [chip->chip_select_num-1], spi->modalias);
Better: spi->dev.bus_id (always unique) not spi->modalias (isn't) ...
>
> cs_deactive(drv_data, chip);
>
>
On Jan 31, 2008 5:48 AM, David Brownell <[email protected]> wrote:
> On Wednesday 30 January 2008, Bryan Wu wrote:
> > When got some gpio conflict, it is easy to know which SPI device driver not just got "bfin-spi"
> >
> > Signed-off-by: Bryan Wu <[email protected]>
> > ---
> > drivers/spi/spi_bfin5xx.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/spi/spi_bfin5xx.c b/drivers/spi/spi_bfin5xx.c
> > index 8fd1365..c3f9ed4 100644
> > --- a/drivers/spi/spi_bfin5xx.c
> > +++ b/drivers/spi/spi_bfin5xx.c
> > @@ -1176,7 +1176,7 @@ static int setup(struct spi_device *spi)
> > if ((chip->chip_select_num > 0)
> > && (chip->chip_select_num <= spi->master->num_chipselect))
> > peripheral_request(ssel[spi->master->bus_num]
> > - [chip->chip_select_num-1], DRV_NAME);
> > + [chip->chip_select_num-1], spi->modalias);
>
> Better: spi->dev.bus_id (always unique) not spi->modalias (isn't) ...
>
spi->dev.bus_id is just number, while spi->modalias usually is a
string name which it is more meaningful.
In most time, modalias is easier for us to locate the gpio conflict.
Thanks
-Bryan Wu
I've forwarded these to Andrew, with my signoff and updated comments.
You still need to work on having your patch descriptions match up
to what the patches actually do...
Patches 3-5 in this series seem to have a common thread: waiting
until BIT_STAT_SPIF is set before moving to the next step of the
transfer. Next time something similar happens, I'd rather see just
one patch addressing the issue on all code paths ... not three small
patches that only fix it for a few of the code paths.
Also, two of those three patches describe their updates as fixing
a "regression", or "reverting" the code. Was this a bug that came
in those patches you wanted to merge to 2.6.24? If so, shouldn't
those regression fixes go into the stable series?
- Dave
On Feb 1, 2008 4:50 AM, David Brownell <[email protected]> wrote:
> I've forwarded these to Andrew, with my signoff and updated comments.
> You still need to work on having your patch descriptions match up
> to what the patches actually do...
>
Thanks a lot, I will try to make it more clearer next time.
> Patches 3-5 in this series seem to have a common thread: waiting
> until BIT_STAT_SPIF is set before moving to the next step of the
> transfer. Next time something similar happens, I'd rather see just
> one patch addressing the issue on all code paths ... not three small
> patches that only fix it for a few of the code paths.
>
OK, I will try to merge these same bug fixing into one patch.
> Also, two of those three patches describe their updates as fixing
> a "regression", or "reverting" the code. Was this a bug that came
> in those patches you wanted to merge to 2.6.24? If so, shouldn't
> those regression fixes go into the stable series?
>
Yes, I agree with you. Actually, I intend to send out these bug fixing
patch ASAP, but we want to make sure our tester verify this bug was
fixing first.
Thanks
-Bryan