[#3821] SPI driver does not respect cs_change

Submitted By: Larry Samuels

Open Date

2008-01-08 08:09:06 Close Date

2008-01-10 20:08:57

Priority:

High Assignee:

Bryan Wu

Status:

Closed Fixed In Release:

N/A

Found In Release:

N/A Release:

2.6.x

Category:

Drivers Board:

EZKIT Lite

Processor:

BF537 Silicon Revision:

0.3

Is this bug repeatable?:

Yes Resolution:

Fixed

Uboot version or rev.:

Toolchain version or rev.:

App binary format:

N/A

Summary: SPI driver does not respect cs_change

Details:

I am reporting a bug when using SPI port on BF537 Stamp Board.

I am connecting to a SPI slave device, namely Microchip MCP23S08 SPI I/O Expander.

The chip select (CS) signal goes high after every byte of the transaction,

where I need it to stay low for all 3 bytes of the transaction.

My setup uses the generic kernel driver spidev.c for user land control of Spi,

which calls the spi_bfin5xx.c blackfin driver file.

Below is the changes I made to stamp.c (This way I select AD9960 in make menuconfig

and the spidev driver is then used)

------------------------------stamp.c-----------------------------------------------

#if defined(CONFIG_AD9960) || defined(CONFIG_AD9960_MODULE)

static struct bfin5xx_spi_chip ad9960_spi_chip_info = {

.enable_dma = 0,

.bits_per_word = 8,

.cs_change_per_word = 0,

};

#endif

...........

#if defined(CONFIG_AD9960) || defined(CONFIG_AD9960_MODULE)

{

// .modalias = "ad9960-spi",

.modalias = "spidev",

.max_speed_hz = 6250000, /* max spi clock (SCK) speed in HZ */

.bus_num = 0,

.chip_select = 2,

.controller_data = &ad9960_spi_chip_info,

#endif

------------------------------------stamp.c----------------------------------

This creates /dev/spidev0.2 in the target.

My user land application is as follows:

--------------------------------------spi_bug.c-----------------------------

#include <sys/types.h>

#include <sys/stat.h>

#include <sys/poll.h>

#include <stdio.h>

#include <fcntl.h>

#include <getopt.h>

#include <unistd.h>

#include <stdlib.h>

#include <string.h>

#include <sys/ioctl.h>

#include <sys/types.h>

#include <sys/stat.h>

#include <linux/types.h>

#include <linux/spi/spidev.h>

#include <strings.h>

#include "adsp-spiadc.h"

#include "pflags.h"

#define VERSION "0.8"

#define FUNC_SET_DIR 0

#define FUNC_SET_LED 1

#define FUNC_READ_INPUTS 2

static void do_msg(int fd)

{

//full duplex transfer - for Microcip MP23S08

//send 2 bytes, clock in 1 byte

struct spi_ioc_transfer xfer[2];

unsigned char buf[6], *bp;

int status;

memset(xfer, 0, sizeof xfer);

memset(buf, 0, sizeof buf);

buf[0] = 0x41; // read

buf[1] = 0x09; // GPIO register

xfer[0].tx_buf = (__u64) buf;

xfer[0].len = 2;

xfer[0].cs_change = 0;

xfer[1].rx_buf = (__u64) buf;

xfer[1].len = 1;

xfer[1].cs_change = 0;

ioctl (fd,CMD_SPI_SET_CSENABLE,CFG_SPI_CHIPSEL2);

ioctl (fd,CMD_SPI_SET_CSLOW,CFG_SPI_CS2VALUE);

status = ioctl(fd, SPI_IOC_MESSAGE(2), xfer);

if (status < 0) {

printf("Error SPI_IOC_MESSAGE\n");

return;

}

ioctl(fd, CMD_SPI_SET_CSDISABLE, CFG_SPI_CHIPSEL2);

printf ("read: %02x\n", buf[0]); // display received byte

}

int main(int argc, char ** argv)

{

int fd;

printf ("spi_bug v %s\n",VERSION);

fd = open("/dev/spidev0.2",O_RDWR);

if(fd < 0)

{

printf("Can't open \/dev\/spi.\n");

return -1;

}

printf ("spi dev opened\n");

ioctl(fd, CMD_SPI_OUT_ENABLE, 1);

ioctl(fd, CMD_SPI_MISO_ENABLE, 1);

ioctl(fd, CMD_SPI_SET_BAUDRATE, 0x10);

ioctl(fd, CMD_SPI_SET_MASTER, 1);

printf ("spi dev configured\n");

do_msg(fd);

return 0;

}

--------------------------------------spi_bug.c----------------------------

I compile,link and copy spi_bug to /bin of the target

I run using spi_bug

The output waveforms looks like this:

SCLK __--__--__--__--__--__--__--__--____--__--__--__--__--__--__--__--________

MOSI --___--______________________-----_________________----________------------

CS2 --________________________________--_______________________________------

cotinued from above:

SLK ________--__--__--__--__--__--__--__--___________________

MOSI -------------------------------------------------------- (high)

CS2 -------_________________________________------------------

CS2 is PJ11 on the BF537, and on the BF537 EZKIT it is connector SPORT0/pin 21.

Note that CS2 went high after every 8 clock bits. I need it stay low for the

entire 3 byte transaction.

With the CS going high evey byte, the slave device does not respond with any MISO message.

When I use PFlags GPIO in place of the CS signal, my slave device responds correctly.

Unfortunatley using PFLags creates a 100uSec delay so it is not an acceptable alternative.

Please correct this bug in the driver, or let me know how to correct it.

Regards,

Larry Samuels

Follow-ups

--- Bryan Wu 2008-01-08 23:35:06

Thanks Larry,

I think I fixed the bug in the svn trunk, please update it.

Or just change one line to fix this issue which was reported as [#3427] bug

before.

---

Index: drivers/spi/spi_bfin5xx.c

===================================================================

--- drivers/spi/spi_bfin5xx.c (revision 4081)

+++ drivers/spi/spi_bfin5xx.c (working copy)

@@ -543,7 +543,7 @@ static void giveback(struct driver_data

bfin_spi_disable(drv_data);

}

- if (!drv_data->cs_change)

+ if (drv_data->cs_change)

cs_deactive(drv_data, chip);

if (msg->complete)

----

-Bryan Wu

---

--- Larry Samuels 2008-01-09 10:35:20

Bryan

I made the change as you suggested. I also inserted a printk message to

confirm the change is in the kernel.

Still have the same problem.

If you copy my code from spi_bug.c and run it, you will see the CS signal going

high on an oscilloscope.

I feel that the bug still exists. If there is something wrong with my user

code spi_bug.c, then let me know.

Larry

--- Bryan Wu 2008-01-09 13:19:03

Larry:

Firstly, there are something wrong in the spi_bug.c

<adsp-spiadc.h> is only for "Blackfin BF53x ADSP SPI ADC

support" (drivers/char/bfin_spi_adc.c). So some ioctl similar to following

code are useless.

ioctl (fd,CMD_SPI_SET_CSENABLE,CFG_SPI_CHIPSEL2);

ioctl (fd,CMD_SPI_SET_CSLOW,CFG_SPI_CS2VALUE);

The same ioctl API you can found in include/linux/spi/spidev.h

Please replace the adsp-spiadc code to spidev API.

Secondly, if software intend to control cs_change, the CPHA should be set in

SPI_CTL register. So you have 2 method to do this:

1) set spi_mode to SPI_MODE_1 (SPI_CPHA | 0) in

arch/blackfin/mach-bf537/boards/stamp.c

---

#if defined(CONFIG_SPI_SPIDEV) || defined(CONFIG_SPI_SPIDEV_MODULE)

static struct bfin5xx_spi_chip spidev_chip_info = {

.enable_dma = 0,

.bits_per_word = 8,

.cs_change_per_word = 0,

};

#endif

...........

#if defined(CONFIG_SPI_SPIDEV) || defined(CONFIG_SPI_SPIDEV_MODULE)

{

.modalias = "spidev",

.max_speed_hz = 6250000, /* max spi clock (SCK) speed in HZ

.bus_num = 0,

.chip_select = 2,

.controller_data = &spidev_chip_info,

.mode = SPI_MODE_1,

#endif

----

2) set CPHA in the spi_bug.c

---

ioctl(fd, SPI_IOC_WR_MODE, SPI_MODE_1);

---

Please revert my previous patch and do some test.

I will do the same test tomorrow morning in the office.

-Bryan Wu

--- Larry Samuels 2008-01-09 15:33:10

Byan,

I made the changes to stamp.c as you suggested.

I did not do 2) ioctl(fd, SPI_IOC_WR_MODE, SPI_MODE_1)

I then added a pullup resistor to CS2 line.

Thie signal now goes low before the 3 byte transmission, and then goes high

after the 3 bytes as it should. So that is working now.

The problem now is if I send consecutive messages, each message takes 83 usec

to transmit.

For each message:

After CS goes low, the 1st SCLK pulse appears 29 uSec later.

After the last SCLK pulse, CS goes high in 8.8 uSec

The next message CS goes low 38 uSec after that.

The portion of the message where SCLK runs is 10 uSec

So I'm losing 73 uSec of throughput every message.

Can you find a way to minimize the delay in CS to SCLK, and message to

message?

--- Bryan Wu 2008-01-09 21:33:42

Thanks. Did you revert my previous patch:

Index: drivers/spi/spi_bfin5xx.c

===================================================================

--- drivers/spi/spi_bfin5xx.c (revision 4081)

+++ drivers/spi/spi_bfin5xx.c (working copy)

@@ -543,7 +543,7 @@ static void giveback(struct driver_data

bfin_spi_disable(drv_data);

}

- if (!drv_data->cs_change)

+ if (drv_data->cs_change)

cs_deactive(drv_data, chip);

if (msg->complete)

---

For the second problem, can you open another bug tracker?

I wanna close this bug with other 2 same issues in my bug list.

Thanks

-Bryan Wu

--- Larry Samuels 2008-01-10 08:57:16

I found out from the BF537 Hardware Reference Manual, that only when CPHA is 1,

and therefore SPI_MODE_ and SPI_MODE_3 will CS stay enabled for the whole

transaction.

From the HRM:

When CPHA = 0, the slave select line, SPISS, must be inactive (high)

between each serial transfer. This is controlled automatically by the SPI

hardware logic. When CPHA = 1, SPISS may either remain active (low)

between successive transfers or be inactive (high). This must be controlled

by the software via manipulation of SPI_FLG.

So it is the Blackfin hardware that causes CS to go high after every byte or

word. Only with a custom driver controlling the CS line in software could you

get SPI_MODE_0 or SPI_MODE_2 to allow CS to stay active (low) for the entire

transaction.

I will close out this bug. Still would like to correct the timing issue. I

will open a bug for that.

--- Larry Samuels 2008-01-10 09:02:02

Bryan - I need you to close out this bug. Thanks

--- Bryan Wu 2008-01-10 22:08:23

OK, Larry.

I will close this, could you tell me do you reverted my previous patch before

you did the test?

Thanks

-Bryan Wu

--- Larry Samuels 2008-01-11 08:50:28

Bryan,

I did revert to the original patch in spi_bfin5xx.c. No need to patch that.

The only issue is timing, which is covered in bug 3823.

So: if (!drv_data->cs_change) should remain in the code.

Larry

--- Bryan Wu 2008-01-11 12:31:45

OK, Larry.

I will remove it.

-Bryan

Files

Changes

Commits

Dependencies

Duplicates

Associations