[#5903] Impossible to work properly with two or more SPI slave devices using spi_bfin5xx driver.

Document created by Aaronwu Employee on Sep 10, 2013
Version 1Show Document
  • View in full screen mode

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

Attachments

    Outcomes