Open drmcnelson opened 2 months ago
Hi @drmcnelson , that code is community contributed and we merged since our tests didn't find any device misbehaving. However, you are right about this not being the expected SPI behaviour, so if you could submit a patch we'll retest and eventually merge the change.
My version of the library is at https://github.com/drmcnelson/Arduino_UNO_R4_SPI_Speedup
There is another very important issue that is also addressed there. There are two interrelated parts to the issue; (a) transfer16() is far too slow, and (b) MOSI (after the patch) still drops momentarily before the SPI clock starts.
For my ADC, dropping MOSI even momentarily, clears the data from the ADC before the clock starts. This happens in the beginning of transfer16() when it sets up the 16 bit transfer. See the MCP33131D datasheet.
(Normally the procedure is set CNVST high for 700 nsecs, then set CNVST low, and then call transfer16(). But MOSI drops at the beginning of transfer16() and clears the data.)
So, to solve this, and also speed up reading the ADC inside of a loop, I added two calls to the library, read16_setup() and read16_transfer(). There is still a lot of overhead in the transfer, but at least it works.
Regarding submitting the patch:
I tried to submit a pull request, but as I recall, I was stopped by the CLA manager. It made an unacceptable demand for access to private data.
I was looking at the MCP33131D datasheet and in one of the figures SDI is just connected to DVio (the supply voltage) instead of the MOSI pin, that should solve this part of your problems. IMHO, SDI does not look to me like a SPI input pin, as no data is accepted by the MCP33131D and it affects operation even when there is no clock pulse.
Yes, you are right, that is the way I did it on all of the designs until now. And will go back to it for the next fab.
(I think the idea was to experiment with using it as an enable, but on further thought, I am not sure what the benefit would be.)
Anyway, the 16bit setup does not need to be inside the loop, and that is where it happens. So, that solves it for the moment also.
Here is the board, in case you are interested. Custom InAmp with the ADC lower left, +/-5V supply upper middle, Controller with T4 lower right.
I am developing a set of instrumentation boards and firmware, and testing each with Teensy 4, UNO R4 and Nucleo-144.
SPI.cpp sets MOSI idle state to low.
This is an error and it makes it incompatible with accepted convention and incompatible with many peripheral chips.
Please comment out the following lines.
https://github.com/arduino/ArduinoCore-renesas/blob/5748045afbe0cabb1fc54bcb60ca6d73f6bcdd84/libraries/SPI/SPI.cpp#L435-L437
If you insist on leaving it like that then at least provide an API so it can be corrected by users.