PaulStoffregen
Well-known member
Yes, it is exactly ATOMIC_BLOCK, that is the entire idea. Yes, it is brute force, and that is actually the only way to make it ISR safe.
Quite a lot has happened on this SPI sharing topic over the last few weeks. The conversation has mostly been on the Arduino Developers mail list.
https://groups.google.com/a/arduino.cc/forum/#!topic/developers/TuZLfjeZjDI
Adding a fix only in Teensyduino and then patching several dozen libraries would probably be quicker and easier than trying to work with the Arduino Team. It's always a long and slow process to get them to accept anything other than simple bug fixes, though the situation is vastly improved now that Cristian Maglie is the technical lead.
But I really do believe this needs to be fixed upstream and become part of the official SPI library. Long term, that'll make for less maintenance than asking lots of library authors to accept a Teensy-only patch.
Here's the proposal:
I'd like to propose and work on an extended API for the SPI library. Of
course, before beginning, I'm asking here for comments and feedback on
the API, and hopefully a tentative yeah/nay from Cristian.
My goal is improving compatibility between SPI-based libraries. Some
libraries work well together, like Ethernet and SD, but many others
conflict, because of different SPI settings or usage within interrupt
routines.
My hope is to define an API that allows the actual implementation inside
the SPI library to work across different platforms, which may involve
different implementations. But the API should be the same, so libraries
and sketches using SPI can use the functions with the same parameters.
So far, my plan involves 3 new functions:
SPI.beginTransaction(clockRate, bitOrder, dataMode);
SPI.endTransaction();
SPI.usingInterrupt(interruptNumber);
The beginTransaction() function is meant to be called before asserting a
chip select signal and performing a group of SPI transfers. It will
obsolete setClockDivider(), setBitOrder() and setDataMode(). The SPI
library will "know" if any SPI usage involves interrupts and take
appropriate steps to ensure this SPI transaction is atomic. Of course,
endTransaction() ends the transaction, usually right after deasserting
the chip select.
The usingInterrupt() function is how the SPI library will become
interrupt aware. If no calls have been made to SPI.usingInterrupt(),
then SPI.beginTransaction() will leave interrupts fully enabled. But if
any usage of SPI is from interrupts, SPI.beginTransaction() will disable
at least those interrupts. The specific mechanism will likely vary
across platforms, perhaps using interrupt priority levels on ARM and
simple masking or global disable on AVR. The specific implementation
inside the SPI library can be very simple or very sophisticated and can
be changed while keeping the API stable.
The interruptNumber arg is an integer, the same as used with
attachInterrupt(). SPI.usingInterrupt() might also support other
platform-specific numbers, to represent the other interrupts. If an
unknown interrupt number is given, SPI.beginTransaction() should fall
back to global interrupt disable during transactions. This behavior
will guarantee SPI-based libraries always work together. The
implementation within SPI can be improved, without any API change, to
support more selective interrupt masking.
I'd also like to add constants to SPI.h for the clockRate parameter,
based on actual frequencies. These would be automatically adapted by
#ifdefs based on F_CPU. The "DIV" parameters should be deprecated. New
SPI code should use names with actual frequencies, where the library
will use the closest available frequency that does not exceed the spec.
I realize this is a pretty substantial change to a very widely used
library. My goal is to maintain backwards compatibility. The many
libraries using SPI will gradually adopt the new API, perhaps with some
symbol like SPI_HAS_TRANSACTION to test for the new API at compile time.