arduino / ArduinoCore-renesas

MIT License
111 stars 76 forks source link

Need true 16 bit SPI transfer #360

Open drmcnelson opened 2 months ago

drmcnelson commented 2 months ago

Hi,

I hope to use your library to read an ADC, I need to be able to do the transfers with best possible speed. One application for example is the readout for a linear CCD (4k of 16 bit samples). In my lab we read long records at 500KSPS, to study slow charge mobility at microampere currents in organic electronics.

We are trying to adapt this now to the UNO R4, (an RA4M1).

I found some issues with the transfer16().

Here is code and a scope images for transfer16() on the UNO R4. The upper trace is the SPI clock. The lower trace is CS pin, which I am using to assert the CNVST of the ADC. Data is available 700 nsec after the rising edge of CNVST and falling edge enables SPI transfer.

Notice that there is a 2.5usec setup time for the transfer, and then the transfer occurs as two 8 byte transfers, separated by 1.2usec rather than as a single 16 bit transfer. I checked in your code, and indeed it is done as two 8 bit transfer.

So, I think what is need is a true 16 bit transfer16(). You already do something like that in the buffered transfer (the next function in the source code file after tansfer16()).

Also, for reading a long record from the ADC, it would be great is some of that setup time could be moved outside the loop.

I hesitate to do this myself. Looking at your code , you did a great job and wou are already quite expert in the part,

Thank you

digitalWrite(CNVSTPIN,HIGH;
DELAYNANOSECONDS(700)
digitalWrite(CNVSTPIN,LOW);

dataword = SPI.transfer16(0xFFFF);

UNOR4SPI01

drmcnelson commented 2 months ago

@per1234 I think the 16 bit transfer is a fix (not an enhancement).

The second part, separate calls for setup and transfer, would be an important improvement in the API, and one that is very strongly needed. There are a lot of parts that require a logic pulse between transfers, and this is the only way to do it efficiently and preserve the speed of the interface.

Why do I feel the first part is a fix (not an enhancement)?

A) Contiguous 16 bits is the way it is supposed to work on a 16 or 32 bit platform.

B) And, as you can see in the scope traces, doing this as two 8 bit transfers downgrades performance by more than a factor of two, just for the actual transfer.

(And, besides that, notice that overall it takes a solid 8 usec for each 16 bit read!!! )

And finally, an appeal to good engineering:

The R4 is a big step above the R3. It opens more applications, and you might find it used in more labs and so forth. It suggests you want that Arduino be taken seriously.

But then leaving this as two 8 bit transfers, means Arduino is committed to the R4 being a toy, not a serious platform.

I sincerely hope this will get fixed. The R4 deserves, at a minimum, to have a correctly functioning SPI library, and we all want the R4 to be successful.

Thank you

drmcnelson commented 2 months ago

Here is a scope trace for a 16 bit transfer on the Teensy 4. Notice it is an honest 16 bit transfer.

Both your chip the RA4M1 and the T4's IMXRT1062 have this setting available. And in fact you use it in transfer(buffer,length).

TeensySPI02