Question about SPI ASYNCH transfer API (documentation need?)

[LMESTM]

Hello

I've started to deliver STM32 support for SPI ASYNCH. While doing this, I discussed with colleagues and this triggered a question on the trasnfer asynch API (documented here and header file) https://developer.mbed.org/users/mbed_official/code/mbed/docs/6c34061e7c34/group__AsynchSPI.html

The SPI transfer API is very similar to I2C one.
In I2C, even if this is not clearly documented (AFAIK), the transfer consist in sending a Tx command (with Tx_lenght) and then reading the Rx response from slave (with Rx_lenght that might be different from Tx_lenght). Very useful for using many I2C devices indeed.
Note that this could help new comers to explicitely explain that Tx happens first, then Rx happens.

Now coming to my SPI question: do we expect a similar steps approach as in I2C, that is, sending a command, then reading the corresponding response. That could make it easy to use some SPI devices ... or do we consider that because SPI is full duplex (Rx and Tx can happen at the same time) so transfer of Tx and Rx shall happen simultenaously. This takes benefit of the full duplex link, but this is not clear to us which SPI devices would benefit of such interface. For full duplex communication, one could easily consider that having the same Rx and Tx lenght makes things simpler (and allow full usage of the bandwidth).

Thanks in advance for clarifiying this point,
Br
Laurent

[LMESTM]

@Sissors hi - do you have a view or even final answer to the above questions ?

[Sissors]

Are you sure you meant me? Because I have an opinion. I have an opinion about many things. But few people here really care about it :P.

I agree with your statement that there are very few SPI devices that actually really use full duplex. At the same time I feel that SPI is a full duplex protocol, and I wouldn't limit it to half duplex in the API. Also for example if you need to send 2 bytes to request 500 bytes, you can just send those 2 synchronous, and then start an asynchronous operation for those 500 bytes.

Now what I do dislike about the (SPI) async API, is that in my opinion it is needlessly complicated: As you mentioned why would you have different read and write lengths if you do both at the same time? And if your read is shorter than your write, do you not read at the beginning or at the end of the transaction?. And I really miss the possibility to easily send constant values, which is extensively used in for example LCD TFT displays, but also has some other use cases.

[LMESTM]

Yes I was right to ask for your opinion because now there is a discussion :-) - and I feel like we're on the same line here. So that may pop up higher in the pile of issues ...
Now I still would be happy to get MBED staff' word on this topic

[0xc0170]

Now what I do dislike about the (SPI) async API, is that in my opinion it is needlessly complicated: As you mentioned why would you have different read and write lengths if you do both at the same time?

And if your read is shorter than your write, do you not read at the beginning or at the end of the transaction?

This was highlighted previously, we agreed that this could be done. We can still add a new transfer method with just one length argument, and deprecate the rx/tx lengths.

And I really miss the possibility to easily send constant values, which is extensively used in for example LCD TFT displays, but also has some other use cases.

Any proposals for this. We discussed earlier to reuse default SPI write value or introduce a new transfer with just 2 arguments?

// constant value transferred
spi.transfer(value, count, callback, event_type);

[0xc0170]

Besides adding new API or deprecating, how the docs can be improved @LMESTM ? Please send a pull request or pinpoint here what's missing. Thanks

[LMESTM]

@0xc0170 : the doc here https://developer.mbed.org/users/mbed_official/code/mbed/docs/6c34061e7c34/group__AsynchSPI.html says :

Begin the SPI transfer.

This needs to be clarified to specify whether Rx and Tx will happen at the same time or if Rx happens first and then Tx will go on in a second step. Once decided, we can conclude on how to actually send a PR for updating the description (and maybe APIs)

there is also a note below which seems outdated I think as we now have a pointer and a length as paramters

Buffer pointers and lengths are specified in tx_buff and rx_buff

[LMESTM]

@0xc0170 reading again your comment

This was highlighted previously, we agreed that this could be done. We can still add a new transfer method with just one length argument, and deprecate the rx/tx lengths.

does it mean that the answer to my question is : SPI transfer API is meant for sending both Tx and Rx at the same time (full duplex) ?

[Sissors]

Any proposals for this. We discussed earlier to reuse default SPI write value or introduce a new transfer with just 2 arguments?

One option I see would be just get the current one:
int transfer(const Type *tx_buffer, int tx_length, Type *rx_buffer, int rx_length, const event_callback_t& callback, int event = SPI_EVENT), and add the function:

int transfer(const Type tx_value, int tx_length, Type *rx_buffer, int rx_length, const event_callback_t& callback, int event = SPI_EVENT)

(and then the lengths merged also into one length argument). I don't know if that will be save with the template that is used, and I can imagine it could cause confusion maybe sometimes. But my initial idea would be to just re-use the same function, only instead of a pointer to a buffer, give a value. Alternative would be calling it 'transfer_value', or 'transfer_constant'.

How that is being handled then in the API code would be another question (quite easy to implement, but it must be implemented).

[LMESTM]

Hi all

I know understand that yes the answer to my question is :
SPI transfer API is meant for sending both Tx and Rx at the same time (full duplex) ?

And I now understand that transfer API is meant to be used with;
Tx only with Tx size (and Rx null / rx size 0)
Rx only (and Tx null / Tx size 0)
Rx and Tx with same size
Rx and Tx with different doesn't seem to make much sense and makes things much more complex in the target layers ... so I'll consider to not support this case in my ongoing developments.

Is this a common understanding and shall we make such comments in the API description ?

If ok, the issue can be closed on my side.
Another one could be created to follow-up on Sissors proposal for sending constants

[0xc0170]

Another one could be created to follow-up on Sissors proposal for sending constants

+1, shall be separete issue

Is this a common understanding and shall we make such comments in the API description ?

Please send a patch for the documentation of those functions.

[AnotherButler]

@LMESTM Do you think we still need to add comments to the API description, or is it OK to close this issue?

[LMESTM]

@AnotherButler
yes I still think this could help new comers.
I don't know how to update this page
https://developer.mbed.org/users/mbed_official/code/mbed/docs/6c34061e7c34/group__AsynchSPI.html
Would someone at ARM take care of the update ?

[Sissors]

That should be generated from the Doxygen automatically. I think for update to documentation also this issue is relevant to take into account: #3261

[AnotherButler]

@0xc0170 Are you the best person to help with this?