Setting cs_change breaks MFRC522 RFID reader

[pelwell]

I've been investigating a problem using the Mifare RFID reader module on Raspberry Pi. The problem started when we made the switch to the upstream SPI driver, spi-bcm2835.

One of the features of the upstream SPI driver is that it forces software CS due to a deficiency in the hardware. I noticed that with that driver the CS line was being held active between messages, but the MFRC522 spec says that it must become inactive between messages. Hacking the downstream driver to allow hardware CS to be used if requested, and requesting it, fixed the problem. I put this down to some incompatibility in the SW CS implementation in the driver.

I now think the driver is behaving correctly, and the bug is in this library. The kernel documentation says:

  - An spi_message is a sequence of protocol operations, executed
    as one atomic sequence.  SPI driver controls include:
[...]
      + whether the chipselect becomes inactive after a transfer and
        any delay ... by using the spi_transfer.cs_change flag;

      + hinting whether the next message is likely to go to this same
        device ... using the spi_transfer.cs_change flag on the last
        transfer in that atomic group, and potentially saving costs
        for chip deselect and select operations.

I think this is a bit cryptic, but another doc I found online spells it out:

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 intactive.

When the transfer() function sets cs_change to 1 on the one-and-only message it sends, it is asking the SPI subsystem to try to keep CS active between it and the next message. I don't know if this was your intention, but it breaks the RFC reader. I suspect it only used to work on the old SPI driver because the hardware was incapable of keep CS active between transfers.

You can read the original bug report and subsequent discussion here.

[pelwell]

Closing, since my pull request has been accepted.