2010-02-01 05:37:45     changes in spidev driver

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

2010-02-01 05:37:45     changes in spidev driver

Rob Maris (GERMANY)

Message: 85413   

 

I'm investigating the spidev driver and, especially, the spi_bfin5xx chip driver.

 

Regarding cs_gpio removement, I'm not sure about the context. According to the SVN log "/trunk/arch/blackfin/include/asm/bfin5xx_spi.h" it is removed, but no reason is given. Remark: with spi_bfin5xx.c cs_gpio is still defined.

 

At actual SVN trunk state, chip_select = 0 (board file) does not generate a device /dev/spidev0.0 (as used in conjunction with cs_gpio according to the docs page "SPI"). Any information about this would be helpful.

 

It is my goal to have CS go low immediately after my full duplex transmission is going to start (after CLK is preset to the appropriate level for the chip), while it should be pulled high due to a user program call, and not upon termination of the ioctl call that initiated the transmission.

QuoteReplyEditDelete

 

 

2010-02-01 11:29:19     Re: changes in spidev driver

Robin Getz (UNITED STATES)

Message: 85419   

 

Rob:

 

Just to confirm - you are trying to use a SPI chip select that is not a hardware controlled chip select, but a normal general purpose I/O as a chip select?

 

-Robin

QuoteReplyEditDelete

 

 

2010-02-01 11:40:44     Re: changes in spidev driver

Michael Hennerich (GERMANY)

Message: 85423    Revisit the documentation page

https://docs.blackfin.uclinux.org/doku.php?id=spi#device_resources

 

I just added some updates: GPIO-based chip select (new way)

QuoteReplyEditDelete

 

 

2010-02-01 12:47:57     Re: changes in spidev driver

Rob Maris (GERMANY)

Message: 85425   

 

Robin: yes, that's right. A few days ago I emulated this behaviour by assigning another SSEL pin that isn't used, and then program the target CS-portpin via simple-gpio (was possible with my current production Linux 2.6.28). The problem however was that CS is pulled low, then the full-duplex transfer is started, which pulls CLK low (CPOL=0 is a requirement). At this time, a clock is already recognized by the slave device. Hence the requirement: either CLK is low steady while idle (with "manual" CS control), or CS is issued after CLK goes low (as is the standard case - and requires CS set low by the driver, and later deactivated by userspace command). Note: sharing with other devices is currently not an issue.

 

The requirement of pulling CS high *manually* after transmission is completed is given from the fact that the slave device provides a somewhat curious behaviour: When received byte n equals a certain value, then more bytes must be transmitted while CS still active, hence: two different message lenghts. In userspace I prepared this with a second ioctl call when the condition is met.

 

Of course this pitfall can be solved with a real device driver, but that is not a serious option, because the slave device may later offer other message formats (actually the slave is a small MCU).

 

BTW: I hoped that 2.6.32 offered more capabilities since a SPI_NO_CS is defined. But it is not supported by the driver. I'll check if I add something to the driver code - this fact alone is a logical drive to move to current trunk state.

 

-Rob

QuoteReplyEditDelete

 

 

2010-02-01 15:52:14     Re: changes in spidev driver

Rob Maris (GERMANY)

Message: 85426   

 

Michael:

 

when I say

 

        .chip_select = GPIO_PF6 + MAX_CTRL_CS,

 

Iget

 

    bfin-spi bfin-spi.0: cs14 >= max 8 

    bfin-spi bfin-spi.0: Blackfin on-chip SPI Controller Driver, Version 1.0, regs_base@ffc00500, dma channel@7

 

When I say (standard way)

 

        .chip_select = 4,

 

I get

 

    bfin-spi bfin-spi.0: Blackfin on-chip SPI Controller Driver, Version 1.0, regs_base@ffc00500, dma channel@7 

 

and a device spidev0.4.

 

 

 

BTW: When I have got advances regarding my current SPI demands, I'll also contribute to docs, e.g. the struct element cs_change_per_word has been removed.

 

-Rob

 

QuoteReplyEditDelete

 

 

2010-02-01 23:34:03     Re: changes in spidev driver

Mike Frysinger (UNITED STATES)

Message: 85439   

 

adding support for SPI_NO_CS to the Blackfin SPI bus driver looks pretty trivial

QuoteReplyEditDelete

 

 

2010-02-02 02:49:17     Re: changes in spidev driver

Rob Maris (GERMANY)

Message: 85447   

 

Mike: Meanwhile I know: that's right. There is quite good documentation in spi.h (as I found out yesterday), and it makes practically no sense.

 

Generally: Well, I'm going to get the parameter cs_change operate correct for the case ii as documented in spi.c, line 407 (see also my application code snippet below).

 

// prepare n byte message, with bytewise delay as requested by the slave

for (n=0; n < len; n++) {

xfer[n].tx_buf = (unsigned long)bp1;

xfer[n].rx_buf = (unsigned long)bp2;

xfer[n].len = 1;

xfer[n].cs_change = 0;

xfer[n].delay_usecs = 500;

bp1++;

bp2++;

}

 

 

According to mentioned case ii, the chip select should be left unmodified after the last byte as specified above. Until recently, this was no problem, but with the new requirement I'm coped with, it is a problem, and I'm going to fix it - accompanied by a gpio set hi from userspace. I'm doing this using my current 2.6.28 trunk-based system. Why? I explain this in next posting.

 

-Rob

QuoteReplyEditDelete

 

 

2010-02-02 03:47:42     Re: changes in spidev driver

Rob Maris (GERMANY)

Message: 85464   

 

Follow-up for Michael et al...

 

- first of all: result of checkings after having written most part of following text: the "new" gpio-based selection should work OK when the boardfile info is correct. I have counterchecked it with the stamp boardfile.:

 

        .num_chipselect = MAX_CTRL_CS + MAX_BLACKFIN_GPIOS,

 

 

while my tcm-bf537 only contains a fixed assignment (8). A general problem. I have to keep forever in mind that any source code changes that affect board files must be counterchecked with the state of non-analog-devices boardfiles...

 

 

 

I'm a bit unhappy with the new method to establish gpio-based chip select. Let me explain why. The former method was not elegant, because it makes not much sense to define a chip select in a master struct, I agree. The new method is not elegant because it relies on arithmetic. I'd prefer to use e.g. negative numbers to accomplish the desired effect (e.g. -GPIO_PF6). I know that the underlying struct element in spi.h is unsigned, but it makes no error to specify a negative number, while negative numbers may be detected at bf5xx driver level using casts. Far more better be the definition of SSEL1...7 macro names which would subsitute by any numbers in a range far beyond normal gpio port enumerations. But here the problem arises that the spi.c driver code checks whether the cs_line index is equal or lower than the runtime specified number of chip select lines, as specified in the board file, master struct.

 

But regardless how it is done, the problem is that spi.c contains code that checks plausibility using num_chipselect. There negative numbers will cause problems and the SSEL macro name proposel makes trouble too, in this respect. I think that changes in spi.c would be necessary, or in spi.h - define chip_select as s16.

 

The actual bf5xx-spi code introduces MAX_CTRL_CS, and uses this extensively for this select stuff gpio-based CS resp. native CS. It is prefixed with the value 8 (header file). But another define already exists - with the same purpose: MAX_SPI_SSEL (source code, value 7). To my opinion, the master struct element in the board file. When the new method is kept in the trunk, I'd suggest to move MAX_SPI_SSEL into header file.

 

I'm very interested in any thoughts about this stuff.

 

-Rob

QuoteReplyEditDelete

 

 

2010-02-02 12:26:57     Re: changes in spidev driver (requirement is met now)

Rob Maris (GERMANY)

Message: 85475   

 

After quite a lot of fiddling in the bf5xx-spi.c code I succeeded in getting SCLK having low in idle (the native state in SPI_MODE 0, which is required in my app. As a reminder: this condition allows me to control CS completely independent of driver processing.

 

My first goal - letting CS in active state *after* the message is sent out (according to the last byte's cs_change flag value) was not realized. I simply did not find the code portion responsible for that, even not in the spi_giveback function, conditional execution of gpio_free in bfin_spi_cleanup didn't do it, too. Nevertheless, having such an option corresponds to intended behaviour according to spi.h notes. I checked it in atmel_spi.c. There, a variable stay takes care of this. Any active CS is automatically reset when the subsequent transfer is with another SPI slave.

 

Attached is a diff file with my personal changes (based on current SVN trunk) - note that this diff includes a bug related change, separately reported in bug tracker.

 

As a result of the work done, I have some more suggestions for the driver code/behaviour - separate posting.

 

-Rob

 

diff

QuoteReplyEditDelete

 

 

2010-02-02 15:00:17     Re: changes in spidev driver - proposals cont'd

Rob Maris (GERMANY)

Message: 85480   

 

Some more proposal ideas:

 

    let specifiyng SPI_NO_CS not result in an error return. Instead, specifying this may help write userspace code that checks the value via ioctl call, and generate its own cs signals if necessary.

    let have specification of chip_select in board file = 0 the same effect as above (or makes that suggestion superfluous) and generate a spidev0.0 device. Makes sense: spidevs without intrinsic ce control can share multiple devices.

    have the spi-bus clock signal never high between transmissions, when a CPOL=0 is given in last executed transmission context, i.e. as long as any one device on a given bus is open (file descriptor) - of course as far as the low level driver can recognize this.

 

QuoteReplyEditDelete

 

 

2010-02-02 19:20:26     Re: changes in spidev driver - proposals cont'd

Mike Frysinger (UNITED STATES)

Message: 85491   

 

it'd make more sense to just add support for SPI_NO_CS then test random hacks.

 

we cant special case any CS value as the common SPI code does checking on it.  i also dont think it's a good idea, so if you wanted to change the common code, i'd leave it up to you to propose it to the SPI maintainers.

 

the Blackfin SPI clock is not handled in software so there is no way to make it do weird things.  if you want that kind of control, use the GPIO-bitbanging SPI bus.

QuoteReplyEditDelete

 

 

2010-02-03 04:53:13     Re: changes in spidev driver - proposals cont'd

Rob Maris (GERMANY)

Message: 85530   

 

Indeed it is my intention to have support for SPI_NO_CS. It is a subproject deadline that forced me to proceed as documented above (random hack....). To be precise: when a spidev0.0 will be "reintroduced" again, in this case as a device without CS control, it needs code for this: Not only a differentiation between SSEL-based operation and GPIO-based operation, but extra as a "do nothing cs related". I can take care of adding this code to the blackfin spi driver - if no one has complaints about this. At this time, I have configured my boardfile to associate the driver's CS to an unused port.

 

After posting my last post I realized that spidev0.0 AND having set this flag (must be automatically when device 0.0 is probed) makes sense: I have set up my application to check out via ioctl read whether SPI_NO_CS is set or not. This is the only way to ensure my app's backward compatibility with kernels used until recently: a future 2.6.32 kernel with spidev0.0 support uses the same device as the old 2.6.28 spidev0.0 support for GPIO-based control, which I needed for the sake of CS low between bytes in SPI_MODE 0 (I have patched my 2.6.28 production code to allow SPI_NO_CS value read runtime/set boardfile - 2.6.32 is not yed used for this).

 

Regarding SCLK: I personally prefer it to have this as specific in my spi_bf5xx.c code instead of using bitbanging SPI. Of course this wouln't be a topic when the "keep CS active after last byte" capability would operate according to the intention of spi.h comments, as referenced in a former posting.

 

When I don't overlook anything there is no need to do a proposal to the SPI maintainers, regarding above issues - unless a change of chip select struct element to s16 be an issue. I'll contact them - let alone because of the fact that the mode word is fully utilized now, and should probably be updated to u16 - could e.g. contain a new bit to distinguish between SSEL-based CS and GPIO-based CS, making unelegant arithmetic in boardfile and driver code superfluous.

QuoteReplyEditDelete

 

 

2010-02-03 05:56:37     Re: changes in spidev driver - proposals cont'd

Rob Maris (GERMANY)

Message: 85532   

 

Application example (timing diagram as screenshot - attached):

 

left side of blue cursor shows first message (5 bytes) where app decides if more bytes must be xmitted, right side of cursor shows the additional transfer. In this exampe 100 us delay between bytes was specified.

 

extended-spi.png

QuoteReplyEditDelete

 

 

2010-09-22 07:55:15     Re: changes in spidev driver - proposals cont'd

Rob Maris (GERMANY)

Message: 93740   

 

......(big jump).......   Follow-up:  I have got the opportunity to extensively check my special requirements using trunk code.

 

As a reminder: CS must not only be low during transfers, but also a message section's read result must be evaluated while CS is kept active, and eventually more bytes are transferred. This potential behaviour is described in spi.h (the reference is contained in attached patch file). According to the top of this thread, two factors forced me to use a sort of SPI_NO_CS flag:

 

- The 2.6.28 state of the driver had no clean CS low *after* CLK low;

- No possibility to keep CS low after a part of a message is sent.

 

Both issues are solved, the first apparently by emerging trunk code; the second through attached patch file, which even allows me to use standard gpio control by the driver. I think that the patch contains useful additions for the trunk base (note: temporary comments are contained).

 

In order to clarify things, I quote a part of my userspace code here (which does full duplex transfer). The "official" trunk code always deactivates CS after the transmission is done, regardless the last cs_change value in the example below. My patch changes this: the red line below is necessary to deactivate CS automatically upon message end, when this is requested (in most cases). This altered behaviour allows message evaluation and continuation:

 

if ((ioctl(fd, SPI_IOC_RD_MODE, &iostatus)) < 0)

    perror("SPI_IOC_RD_MODE");

if (iostatus&0x40) {     // my 2.6.28 check if SPI_NO_CS set (now only for bw-compatibility)

    cs = gpio_export(6);

    gpio_dir_out(6);

}

len = 5;

do {

    usleep(500);

    // prepare n byte message, with bytewise delay as requested by the slave

    for (n=0; n < len; n++) {

        xfer[n].tx_buf = (unsigned long)bp1;

        xfer[n].rx_buf = (unsigned long)bp2;

        xfer[n].len = 1;       // odd requirement of one byte with delay...

        xfer[n].cs_change = 0; // this means change after any packet with len as specified here

        xfer[n].delay_usecs = 500;

        bp1++;

        bp2++;

    }

    //xfer[n-1].cs_change = 1; //< MUST be used as final CS return if message terminated

    /* now perform the SPI I/O */

    if ((ioctl(fd, SPI_IOC_MESSAGE(len), xfer)) < 0) {

        perror("SPI_IOC_MESSAGE");

        return;

    }

    len = 8;

} while (n != 8 && bp2[4] == 0x12);  // continue depends on data evaluation!

if (iostatus&0x40)

    gpio_value(cs, 1);   // CS hi, former method

else {

    /* send a dummy message with 0 length, sets CS hi (only when red line not active) */

    xfer[0].len = 0;

    xfer[0].cs_change = 1;

    if ((ioctl(fd, SPI_IOC_MESSAGE(1), xfer)) < 0) {

        perror("SPI_IOC_MESSAGE");

        return;   

    }

}

 

Remarks:

 

    As said: CS/SCK-timing with message begin has become OK (see SS-afterCLK.png).

    Upon message termination (and cs_change be set  - the red line above) CS may get high just prior to clocking out the last clock slope. To solve this, I have added a udelay(1) at appropriate places. BTW: I used a clock of 500 kHz.

    (quite unrelevant). Overall timing has been improved, because I no longer need sysfs gpio accesses. The attachment spi-extended.png could be compared with previous posting attachment. The blue cursor represents the evaluation timestamp of the first message part.

    

 

 

spi_bfin5xx.c.patch

SS-after-CLK.png

spi-extended.png

Outcomes