beagleboard / kernel

Kernel for the beagleboard.org boards
172 stars 159 forks source link

SPI: cs_change not working properly #85

Open corrosion opened 10 years ago

corrosion commented 10 years ago

In file drivers/spi/spi-omap2-mcspi.c funcion omap2_mcspi_work()

/* ignore the "leave it on after last xfer" hint */
if (t->cs_change) {
    omap2_mcspi_force_cs(spi, 0);
    cs_active = 0;
}

Setting cs_change is supposed to keep the chip select pin active after returning from the function. This code does not allow to keep the transaction alive.

ohporter commented 10 years ago

It definitely a bug...or incomplete implementation of the API. In order to add some more background on how cs_change is supposed to behave, please note this documentation from include/linux/spi/spi.h:

* All SPI transfers start with the relevant chipselect active.  Normally
* it stays selected until after the last transfer in a message.  Drivers
* can affect the chipselect signal using cs_change.
*
* (i) If the transfer isn't the last one in the message, this flag is
* used to make the chipselect briefly go inactive in the middle of the
* message.  Toggling chipselect in this way may be needed to terminate
* a chip command, letting a single spi_message perform all of group of
* chip transactions together.
*                                              
* (ii) When the transfer is the last one in the message, the chip may
* stay selected until the next transfer.  On multi-device SPI busses
* with nothing blocking messages going to other devices, this is just
* a performance hint; starting a message to another device deselects
* this one.  But in other cases, this can be used to ensure correctness.
* Some devices need protocol transactions to be built from a series of
* spi_message submissions, where the content of one message is determined
* by the results of previous messages and where the whole transaction
* ends when the chipselect goes inactive.

Case 1 is supported as seen in the driver code. It will deactivate the chip select after each transfer in a message when cs_change is flagged. Case 2 is only relevant on the last transfer in a message. In case 2, if cs_change is flagged and the transfer is the last in a message, then the chip select shouldl be left active.

plan44 commented 8 years ago

Thanks for this comment. Years later, it helped me to understand why RaspberryPi spi-bcm2735 driver did not work with my code. It seems to me that the description of _cschange in spidev.h is misleading, as it just says:

  • @cs_change: True to deselect device before starting the next transfer.

This suggests that in order to make CS go inactive after a transfer (including the last in a message) cs_change should be set. The more thorough description in spi.h @ohporter quoted makes clear that it's exactly the opposite for the last transfer in a message.

Rpi's spi-bcm2708 driver apparently did not check the cs_change flag at all, so code using _cschange as suggested by spidev.h (user land /dev/spidev access) worked on spi-bcm2708 but not on spi-bcm2735.

A good example of someone running into this is this commit on github - where the original author set cs_change intending CS to go inactive after the transfer, see his comment. The commit just comments out touching cs_change, but with no explanation why.

Luckily, I eventually found this post!

jadonk commented 8 years ago

:-)