SPI HAL

[will sanfilippo <wi...@runtime.io>]

Hello:

Since we are on the subject of HALs and blocking/non-blocking interfaces…

1) Looking at the SPI HAL for the slave I am thinking that the API is a bit off. Here are the changes I am considering:

a) The per-byte interface is not really practical and can be implemented using the txrx API anyway. I feel that this should be removed. This means removal of hal_spi_start_rx() and removal of the per-value callback (hal_spi_rx_val). There will only be one callback that will be executed when the transfer is done (see more below).

b) A slave may not be able to know how many bytes it will receive from the master for a given transaction. If the master de-asserts the chip select to the slave, the slave should get notified. This would necessitate a change to the callback API to pass the length. Given that there will be only one callback I will change the name as well:
typedef void (*hal_spi_txrx_cb)(void *arg, int len). Just to re-iterate: this will be called when either the number of values in the hal_spi_txrx() call were received or the master de-asserts chip select.

c) The API to set the character to send to the master (hal_spi_tx_val) should not be dual-purposed. The HAL needs to provide a way to set the “default” character that the slave will clock to the master. This could be done in the configuration but I prefer a separate API for two reasons: I think it is cleaner when having a system that is only a master, and it also allows using this API without having to specify the rest of the configuration. This api would be called: hal_spi_slave_set_def_tx_val(int spi_num, uint16_t val);

2) Regarding the blocking master transmit API hal_spi_tx_val. I certainly think there should be a blocking SPI API for the master. Furthermore, and this is sort of a key point, you should not have to “re-initialize” the SPI interface when you want to do blocking transfers and non-blocking transfers; there are times when you might want to send some values and get immediate responses and other times when you just want to hand something to the spi interface and get called back when it is done.

The issue that I am running into which bothers me is having just a “per-value" master blocking API. Given that the SPI interface could be set up for a txrx callback during configuration (for future hal_spi_txrx calls), the API has to disable interrupts or disable the callback feature in some way. This makes the per-value API somewhat ineffecient. Not a huge deal, but things like this bother me. Given that there is no separate SPI API to disable SPI interrupts, I think we should consider the following:

a) Just leave it the way it is and hal_spi_tx_val() will make sure no interrupts occur or the callback is called.
b) Use the API to set the callback to tell the HAL that it is in blocking or non-blocking mode. In other words (and using the new terminology stated above), if the txrx callback is NULL, the device is in blocking mode and thus there is no need to disable interrupts during every call to hal_spi_tx_val. And btw, this does not mean we are replacing disabling interrupts with a check of a function pointer for NULL. The HAL assumes that the user has called hal_spi_set_txrx_cb with NULL as the function pointer or that the SPI was configured without a callback.
c) Add an API to enable/disable HAL SPI interrupts. It would be up to the user to make sure interrupts are disabled prior to calling hal_spi_tx_val().

NOTE: I prefer b) as this would also allow the hal_spi_txrx api to be used in blocking fashion to send an entire buffer.

Thanks everyone! Sorry for the long email...