[#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
Tags
File Name File Type File Size Posted By
No Files Were Found