matthijskooijman
commented
3 years ago Question 1: I think that when you put different values into a byte array and then send that using transfer, the responsibility for encoding your values into the byte array with the right byte order (endianness) etc is up to the caller. This is pretty much how every low-level transfer library works, I think.
The transfer16 function is just a convenience function to transfer 16-bit words, where the implementation has chosen to match the byte order with the bit order, so essentially each 16-bit word is transferred as a single word with just a bit order.
Of course, similar helpers could have been added for transfer32, transferfloat, or whatever, but that seems too much and impossible to make complete. In a sense, I think transfer16 is maybe already too much, just sending raw bytes and letting the caller figure out all upper layers seems sufficient to me.
So, indeed, callers should figure out any byte order and other encodings, since they're the only ones that really know what is needed. Does that answer your question?
Question 2: The nop ensures that there is a little delay between the write to SPDR and the first time SPSR is checked in the wait loop, so that at max SPI speed, the wait loop is not executed at all, leading to slightly better performance (at the expense of slightly worse performance at slightly lower speeds, I assume). However, in the transfer(void*, size_t) case, there are already other instructions between the write to SPDR and the SPSR check, so I assume no additional delay is needed (and also, because rather than a nop some actual work is done in this time, I suspect that this leads to even better performance). See https://github.com/arduino/ArduinoCore-avr/blob/60f0d0b125e06dbf57b800192c80e5f60d681438/libraries/SPI/src/SPI.h#L247-L254
RobTillaart
Note these questions were found during a code review of the SPI library, not a factual problem (yet).
Question 1 inline static uint16_t transfer16(uint16_t data) does check
SPCR & _BV(DORD)to do MSB or LSB first to keep all 16 bits within e.g. an int16 in the right order.This order check is not in inline static void transfer(void *buf, size_t count). So here the order of the SPI settings only affects the individual bits in the bytes, but not the bytes themselves.
This might cause errors when sending e.g an int32, a float or a struct.
Question 2 inline static void transfer(void *buf, size_t count) does not have the NOP statements that the other two transfer functions have. Is this not needed?