[#5903] Impossible to work properly with two or more SPI slave devices using spi_bfin5xx driver.
Submitted By: Vadim Galitsyn
Open Date
2010-02-18 06:47:45 Close Date
2010-06-11 04:18:01
Priority:
Medium Assignee:
Barry Song
Status:
Closed Fixed In Release:
N/A
Found In Release:
2009R1.1-RC4 Release:
trunk
Category:
Drivers Board:
STAMP
Processor:
BF523 Silicon Revision:
0.3
Is this bug repeatable?:
Yes Resolution:
Rejected
Uboot version or rev.:
Toolchain version or rev.:
trunk
App binary format:
FLAT
Summary: Impossible to work properly with two or more SPI slave devices using spi_bfin5xx driver.
Details:
Probably, spi_bfin5xx driver incorrectly manages SPI_FLG register. It enables all the Slave Select bits for all registered in kernel SPI devices at the same time.
My case is below.
I have two SPI slave devices which are use FLS1 and FLS2 accordingly. In order to create these devices, I have modified arch/blackfin/mach-bf533/boards/stamp.c:
--- linux-2.6.32.2.orig/arch/blackfin/mach-bf533/boards/stamp.c 2009-07-22 10:37:02.000000000 +0400
+++ linux-2.6.32.2/arch/blackfin/mach-bf533/boards/stamp.c 2009-10-27 16:35:55.000000000 +0300
@@ -224,6 +224,14 @@
};
#endif
+#if defined(CONFIG_SPI_CCDIO)
+static struct bfin5xx_spi_chip spi_ccdio_chip_info = {
+ .enable_dma = 1,
+ .bits_per_word = 16,
+};
+#endif
+
+
#if defined(CONFIG_MMC_SPI) || defined(CONFIG_MMC_SPI_MODULE)
#define MMC_SPI_CARD_DETECT_INT IRQ_PF5
static int bfin_mmc_spi_init(struct device *dev,
@@ -314,6 +322,25 @@
.controller_data = &spidev_chip_info,
},
#endif
+
+#if defined(CONFIG_SPI_CCDIO)
+ {
+ .modalias = "ccdio_spi0",
+ .max_speed_hz = 2000000, /* max spi clock (SCK) speed in HZ */
+ .bus_num = 0,
+ .chip_select = 2,
+ .controller_data = &spi_ccdio_chip_info,
+ },
+
+ {
+ .modalias = "ccdio_spi1",
+ .max_speed_hz = 2000000, /* max spi clock (SCK) speed in HZ */
+ .bus_num = 0,
+ .chip_select = 1,
+ .controller_data = &spi_ccdio_chip_info,
+ },
+#endif
+
#if defined(CONFIG_MMC_SPI) || defined(CONFIG_MMC_SPI_MODULE)
{
.modalias = "mmc_spi",
A driver code:
static struct spi_driver mod_ccdio_spi0_driver = {
.driver = {
.name = "ccdio_spi0",
.bus = &spi_bus_type,
.owner = THIS_MODULE,
},
.probe = mod_ccdio_spi_probe,
.remove = __devexit_p(mod_ccdio_spi_remove),
};
static struct spi_driver mod_ccdio_spi1_driver = {
.driver = {
.name = "ccdio_spi1",
.bus = &spi_bus_type,
.owner = THIS_MODULE,
},
.probe = mod_ccdio_spi_probe,
.remove = __devexit_p(mod_ccdio_spi_remove),
};
Then,
spi_register_driver(&mod_ccdio_spi0_driver);
spi_register_driver(&mod_ccdio_spi1_driver);
To start SPI write transfer,
spi_sync(mod_ccdio_spi0_dev, &m); // <- Both devices recive my message
spi_sync(mod_ccdio_spi1_dev, &m); // <- Both devices recive my message again
I tried to read SPI_FLG register after spi_register_driver()'s were done and I saw both FLS1 and FLS2 were enabled at the same time.
So, there is no way to manage each device separately without affecting another one. Or may be I am following by a wrong way in my setup?
Thanks!
Follow-ups
--- Mike Frysinger 2010-02-18 06:53:55
linux 2.6.32.2 is not the latest trunk. you need to update to the very latest.
--- Vadim Galitsyn 2010-02-18 07:09:49
I have verified all the changes for spi_bfin5xx driver between my version and
the latest one from trunk. No the major changes I've got. I've also tried the
trunk version of driver - result is the same.
--- Michael Hennerich 2010-02-18 09:27:19
enable_dma = 1,
Can you try to disable DMA to isee if it makes a difference?
--- Vadim Galitsyn 2010-02-18 14:16:56
I've just tried. That does not helps.
--- Vadim Galitsyn 2010-02-18 14:49:14
I tried to review the driver code and I have some questions.
1. Driver do modify SPI_FLG reg generally in four functions:
bfin_spi_cs_active(), bfin_spi_cs_deactive(), bfin_spi_cs_enable() and
bfin_spi_cs_disable(). The last two functions are related to SPI master code, so
we skip it. The function bfin_spi_cs_active() should activate corresponding bit
in SPI_FLG that corresponds to a chip select value of a __current__ SPI driver.
But it disables it! Why so? (The same is about bfin_spi_cs_deactive())
2. In function bfin_spi_setup() driver do initialize of chip->flag:
chip->flag = (1 << spi->chip_select) << 8;
Why do we need left shift to 8 bits here? Even in case we will apply this value
to SPI FLG reg directly without right shifting to 8 bits again like in
bfin_spi_cs_enable/disable()?
--- Mike Frysinger 2010-02-18 15:11:41
the FLS and FLG handling need to be split. the enable/disable handle the FLS
bit while the active/deactive handle the FLG bit.
--- Vadim Galitsyn 2010-02-18 16:25:27
Mike, in case we want to communicate with FLS(i) we should disable bit FLG(i),
is that correct? Or otherwise, we should disable all the FLGs except of FLG(i)
or it depends of some other settings?
--- Vadim Galitsyn 2010-02-20 14:05:06
I've found solution. The one way to disable SPI "broadcasting" is to
play with FLG bits in SPI_FLG reg. In this case you should enable CPHA bit in
SPI_CTL reg, then enable all the FLS bits you need in SPI_FLG reg. During SPI
r/w operaion all that you need is to disable corresponding FLG bits. To enable
CPHA you should play with a 'mode' field in struct spi_device -- SPI_MODE_1 or
SPI_MODE_3 should be selected.
The other way is to disable CPHA and forget about FLG bits. In this case you
should enable the only one FLS you need.
spi_bfin5xx driver supports the first way only. In all other cases you will
always get SPI "broadcasting".
Mike, could be you or someone else can update SPI wiki page to add some words
about "broadcasting"?
--- Barry Song 2010-03-29 01:28:34
This problem comes from a strange hardware design in blackfin SPI:
If CPHA = 0, the SPI hardware sets the output value and the FLGx
bits are ignored. The SPI protocol requires that the slave select be
deasserted between transferred words. In this case, the SPI hardware
controls the pins. For example, to use PJ10 as a slave select
pin, it is only necessary to set the FLS3 bit in SPI_FLG. It is not necessary
to write to the FLG3 bit, because the SPI hardware
automatically drives the PJ10 pin.
When CPHA = 0, hardware will control the cs if the slave select is enabled.Then
it causes the so-called spi "boardcasting". The solution is:
for spi mode with CPHA = 0, pls use gpio_cs to get right timing, for example,
#if defined(CONFIG_ENC28J60) || defined(CONFIG_ENC28J60_MODULE)
{
.modalias = "enc28j60",
.max_speed_hz = 20000000, /* max spi clock (SCK) speed in
HZ */
.irq = IRQ_PF6,
.bus_num = 0,
.chip_select = GPIO_PF10 + MAX_CTRL_CS, /* GPIO controlled SSEL
*/
.controller_data = &enc28j60_spi_chip_info,
.mode = SPI_MODE_0,
},
#endif
-barry
--- Mike Frysinger 2010-04-06 02:14:02
i dont think the change (require GPIO CS when CPHA=0) is correct. we're
manually driving the CS already, so fix that part. maybe revert the splitting
of FLS/FLG that was added not too long ago ?
--- Barry Song 2010-04-06 02:36:44
reverting the splitting of FLS/FLG can only avoid spi broadcast, but can't fix
spi timing while CPHA=0. CS controlled by hardware while CPHA=0 has wrong
timing. SPI bus assumes cs is low in the whole spi transfer but not in every
word. So I think gpio cs is a better choice to clear chaos.
-barry
--- Vadim Galitsyn 2010-07-25 10:18:57
It's no good approach to use GPIO. That way is very often causes timings leak.
That's my opinion.
--- Barry Song 2010-07-26 10:45:38
I don't think so. Here you don't need to see it as a gpio. You just think it as
CS controlled by SW. You know, many SPI controllers let users to control CS by
writing 1/0 to a register bit.
GPIO to simulate CS for SPI, NAND is something very common. Just look it as
hardware level 1 and 0.
-barry
Files
Changes
Commits
Dependencies
Duplicates
Associations
Tags
File Name File Type File Size Posted By
No Files Were Found