HAL SPI

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

Hello:

We are considering a new HAL for the SPI. Following is the set of API we are considering for this HAL. Comments are appreciated. NOTE: the term “value” is used instead of “byte” as SPI interfaces may use more than 8 bits per value transferred.

NOTE: the spi_config structure is not shown here. It will contain the receive callback and the transmit done callback along with other generic SPI configuration. 

typedef int (*hal_spi_rx_val)(void *arg, uint8_t val);
This is the prototype for the receive callback. Called for each received spi value at interrupt context when using the hal_spi_start_rx() API.

typedef void (*hal_spi_tx_done)(void *arg);
Callback when buffer transferred using the hal_spi_txrx API.

int hal_spi_init(int spi_num, void *cfg)
Initializes the spi with processor specific configuration. This does not turn on the SPI nor does it enable it from a power perspective.

int hal_spi_config(int spi_num, struct spi_config *cfg);
Used to configure generic SPI information after initialization. This will contain the receive callback as well as generic information like # data bits, baudrate, mode, etc. Also will include the callbacks.

int hal_spi_enable(int spi_num);
Turns the SPI on from a power perspective

int hal_spi_disable(int spi_num);
Turns the SPI off from a power perspective

uint16_t hal_spi_tx_val(int spi_num, uint16_t val);
Blocking call to send a value on the SPI. Returns the value received from the SPI.
MASTER: Sends the value and returns the received value from the slave.
SLAVE: Sets the value to send to the master when the master transfers a value. This value will be sent until another call to hal_spi_tx_val() is made. The return code is ignored for a slave. 

int hal_spi_txrx(int spi_num, void *txbuf, void *rxbuf, int len);
Non-blocking call to send a buffer and also store the received values. For both the master and slave, the transmit done callback is executed at interrupt context when the buffer is sent. If rxbuf is NULL, no received values are stored. There is no callback per value when using this API
MASTER: master sends all the values in the buffer and stores the values in the receive buffer if rxbuf is not NULL.
SLAVE: Slave preloads the data to be sent to the master (values stored in txbuf) and places received data from master in rxbuf (if not NULL). No callback per received value; transmit done callback when len values are transferred.

int hal_spi_start_rx(int spi_num);
Enables the SPI receiver. Only valid for slaves. Returns error if the receive is not able to be started. There will be a receive callback per value.

int hal_spi_abort(int spi_num);
This aborts the current transfer but keeps the spi enabled.

void hal_spi_set_rx_val_cb(int spi_num, hal_spi_rx_val rx_val_cb, void *arg);
Sets the callback executed when a slave receives a value. Allowed to be called when the spi is enabled but not when a transfer is in progress.

void hal_spi_set_tx_done_cb(int spi_num, hal_spi_tx_done tx_done_cb, void *arg);
Sets the callback executed the buffer is transferred by the master or slave using the hal_spi_txrx API. Allowed to be called when the SPI is enabled but not during a transfer.

Re: HAL SPI

[Kevin Townsend <ke...@adafruit.com>]

> Regarding slaves: I thought that I mentioned which interfaces would be used by a slave? The slave interfaces are fairly basic, admittedly, and there might be other interfaces that one might want to have, but they should be usable by a slave. I will give my mail another read through to see if I left something out but I certainly intended to provide (at least basic) slave functionality.
Oh, ugg, my bad I see it now. :)

I don't often use slave, but every time I do need it for SPI or I2C it 
always feels like an after thought by the silicon or IP vendors. Glad to 
see that's included as well then.

K.

Re: HAL SPI

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

Kevin:

Thanks for taking the time to read it over. Much appreciated!

I will make the spi config structure public in a bit (was still messing around with part of it) but certainly the baudrate is in there. That is an interesting comment you made regarding the modification of the baudrate when the spi is enabled and running. I had not considered that even though I have done something similar in the past: you dont know what you are talking to so you use a slow rate at first, and once you know, you jump up the speed. I certainly have had cases where I clocked down the spi to save power although that was more due to the CPU speed changing and thus changing the rate at which the spi was being clocked…

I see that Sterling chimed in on the bit-bang interface. I have certainly written bit-bang SPI interfaces in previous lives. Fun stuff! :-) However, I am with Sterling on this one: we would implement a driver for that interface.

Regarding slaves: I thought that I mentioned which interfaces would be used by a slave? The slave interfaces are fairly basic, admittedly, and there might be other interfaces that one might want to have, but they should be usable by a slave. I will give my mail another read through to see if I left something out but I certainly intended to provide (at least basic) slave functionality.

Thanks again.

> On Aug 23, 2016, at 4:21 PM, Kevin Townsend <ke...@adafruit.com> wrote:
> 
> Hi Will,
> 
> You've probably already included this in the spi_config struct, but just in case I wanted to make sure you can dynamically change the SPI speed even after the spi bus has been initialized once?
> 
> The use case I have in mind is SD cards communicating over the SPI bus where you need to start the communication at a fairly slow clock speed, and once everything is setup you can jump to a much faster clock. I'm sure it's already in there though.
> 
> The callbacks below seem sensible to me based on the drivers I've written in the past.
> 
> Being able to bit-bang SPI at a lower level with any GPIO pins would be a nice plus, but hardly a deal breaker and that is a layer lower anyway.
> 
> It might be worth doing a quick review of the requirements around SPI slave as well, just to see if you can accommodate both in one HAL update? Slave isn't commonly used, but for end users who are making commercial modules or components that fit into a larger system, such as say a wireless IMU widget, you'll probably want to enable SPI slave to communicate with another MCU. It isn't worth delaying the current HAL update for this, but maybe just something to think about quickly to see if you can carve room out for that in a future update without introducing a BC?
> 
> I don't see any fundamental issues that would prevent me from using this for most SPI devices I've worked with in the past, though. That's just my opinion, mind you, and I'm sure other users will comment in more detail if they spot something.
> 
> K.
> 
> 
> On 23/08/16 23:23, will sanfilippo wrote:
>> Hello:
>> 
>> We are considering a new HAL for the SPI. Following is the set of API we are considering for this HAL. Comments are appreciated. NOTE: the term “value” is used instead of “byte” as SPI interfaces may use more than 8 bits per value transferred.
>> 
>> NOTE: the spi_config structure is not shown here. It will contain the receive callback and the transmit done callback along with other generic SPI configuration.
>> 
>> typedef int (*hal_spi_rx_val)(void *arg, uint8_t val);
>> This is the prototype for the receive callback. Called for each received spi value at interrupt context when using the hal_spi_start_rx() API.
>> 
>> typedef void (*hal_spi_tx_done)(void *arg);
>> Callback when buffer transferred using the hal_spi_txrx API.
>> 
>> int hal_spi_init(int spi_num, void *cfg)
>> Initializes the spi with processor specific configuration. This does not turn on the SPI nor does it enable it from a power perspective.
>> 
>> int hal_spi_config(int spi_num, struct spi_config *cfg);
>> Used to configure generic SPI information after initialization. This will contain the receive callback as well as generic information like # data bits, baudrate, mode, etc. Also will include the callbacks.
>> 
>> int hal_spi_enable(int spi_num);
>> Turns the SPI on from a power perspective
>> 
>> int hal_spi_disable(int spi_num);
>> Turns the SPI off from a power perspective
>> 
>> uint16_t hal_spi_tx_val(int spi_num, uint16_t val);
>> Blocking call to send a value on the SPI. Returns the value received from the SPI.
>> MASTER: Sends the value and returns the received value from the slave.
>> SLAVE: Sets the value to send to the master when the master transfers a value. This value will be sent until another call to hal_spi_tx_val() is made. The return code is ignored for a slave.
>> 
>> int hal_spi_txrx(int spi_num, void *txbuf, void *rxbuf, int len);
>> Non-blocking call to send a buffer and also store the received values. For both the master and slave, the transmit done callback is executed at interrupt context when the buffer is sent. If rxbuf is NULL, no received values are stored. There is no callback per value when using this API
>> MASTER: master sends all the values in the buffer and stores the values in the receive buffer if rxbuf is not NULL.
>> SLAVE: Slave preloads the data to be sent to the master (values stored in txbuf) and places received data from master in rxbuf (if not NULL). No callback per received value; transmit done callback when len values are transferred.
>> 
>> int hal_spi_start_rx(int spi_num);
>> Enables the SPI receiver. Only valid for slaves. Returns error if the receive is not able to be started. There will be a receive callback per value.
>> 
>> int hal_spi_abort(int spi_num);
>> This aborts the current transfer but keeps the spi enabled.
>> 
>> void hal_spi_set_rx_val_cb(int spi_num, hal_spi_rx_val rx_val_cb, void *arg);
>> Sets the callback executed when a slave receives a value. Allowed to be called when the spi is enabled but not when a transfer is in progress.
>> 
>> void hal_spi_set_tx_done_cb(int spi_num, hal_spi_tx_done tx_done_cb, void *arg);
>> Sets the callback executed the buffer is transferred by the master or slave using the hal_spi_txrx API. Allowed to be called when the SPI is enabled but not during a transfer.
>> 
>> 
> 

Re: HAL SPI

[Sterling Hughes <st...@apache.org>]

Hi Kevin,

On 23 Aug 2016, at 16:21, Kevin Townsend wrote:

> Hi Will,
>
> You've probably already included this in the spi_config struct, but 
> just in case I wanted to make sure you can dynamically change the SPI 
> speed even after the spi bus has been initialized once?
>
> The use case I have in mind is SD cards communicating over the SPI bus 
> where you need to start the communication at a fairly slow clock 
> speed, and once everything is setup you can jump to a much faster 
> clock. I'm sure it's already in there though.
>
> The callbacks below seem sensible to me based on the drivers I've 
> written in the past.
>
> Being able to bit-bang SPI at a lower level with any GPIO pins would 
> be a nice plus, but hardly a deal breaker and that is a layer lower 
> anyway.
>

Agree with all the other comments, just wanted to +1 this last part.

I think this belongs in a higher layer SPI driver, that can either 
bitbang (using hal_gpio*) or use hal_spi.  Marko did something similar 
with UART, generating 9600 baud, because the Arduino Primo boards use 
Nordic\u2019s (1!) UART to talk with the ESP8266.

Cheers,

Sterling

Re: HAL SPI

[Kevin Townsend <ke...@adafruit.com>]

Hi Will,

You've probably already included this in the spi_config struct, but just 
in case I wanted to make sure you can dynamically change the SPI speed 
even after the spi bus has been initialized once?

The use case I have in mind is SD cards communicating over the SPI bus 
where you need to start the communication at a fairly slow clock speed, 
and once everything is setup you can jump to a much faster clock. I'm 
sure it's already in there though.

The callbacks below seem sensible to me based on the drivers I've 
written in the past.

Being able to bit-bang SPI at a lower level with any GPIO pins would be 
a nice plus, but hardly a deal breaker and that is a layer lower anyway.

It might be worth doing a quick review of the requirements around SPI 
slave as well, just to see if you can accommodate both in one HAL 
update? Slave isn't commonly used, but for end users who are making 
commercial modules or components that fit into a larger system, such as 
say a wireless IMU widget, you'll probably want to enable SPI slave to 
communicate with another MCU. It isn't worth delaying the current HAL 
update for this, but maybe just something to think about quickly to see 
if you can carve room out for that in a future update without 
introducing a BC?

I don't see any fundamental issues that would prevent me from using this 
for most SPI devices I've worked with in the past, though. That's just 
my opinion, mind you, and I'm sure other users will comment in more 
detail if they spot something.

K.


On 23/08/16 23:23, will sanfilippo wrote:
> Hello:
>
> We are considering a new HAL for the SPI. Following is the set of API we are considering for this HAL. Comments are appreciated. NOTE: the term \u201cvalue\u201d is used instead of \u201cbyte\u201d as SPI interfaces may use more than 8 bits per value transferred.
>
> NOTE: the spi_config structure is not shown here. It will contain the receive callback and the transmit done callback along with other generic SPI configuration.
>
> typedef int (*hal_spi_rx_val)(void *arg, uint8_t val);
> This is the prototype for the receive callback. Called for each received spi value at interrupt context when using the hal_spi_start_rx() API.
>
> typedef void (*hal_spi_tx_done)(void *arg);
> Callback when buffer transferred using the hal_spi_txrx API.
>
> int hal_spi_init(int spi_num, void *cfg)
> Initializes the spi with processor specific configuration. This does not turn on the SPI nor does it enable it from a power perspective.
>
> int hal_spi_config(int spi_num, struct spi_config *cfg);
> Used to configure generic SPI information after initialization. This will contain the receive callback as well as generic information like # data bits, baudrate, mode, etc. Also will include the callbacks.
>
> int hal_spi_enable(int spi_num);
> Turns the SPI on from a power perspective
>
> int hal_spi_disable(int spi_num);
> Turns the SPI off from a power perspective
>
> uint16_t hal_spi_tx_val(int spi_num, uint16_t val);
> Blocking call to send a value on the SPI. Returns the value received from the SPI.
> MASTER: Sends the value and returns the received value from the slave.
> SLAVE: Sets the value to send to the master when the master transfers a value. This value will be sent until another call to hal_spi_tx_val() is made. The return code is ignored for a slave.
>
> int hal_spi_txrx(int spi_num, void *txbuf, void *rxbuf, int len);
> Non-blocking call to send a buffer and also store the received values. For both the master and slave, the transmit done callback is executed at interrupt context when the buffer is sent. If rxbuf is NULL, no received values are stored. There is no callback per value when using this API
> MASTER: master sends all the values in the buffer and stores the values in the receive buffer if rxbuf is not NULL.
> SLAVE: Slave preloads the data to be sent to the master (values stored in txbuf) and places received data from master in rxbuf (if not NULL). No callback per received value; transmit done callback when len values are transferred.
>
> int hal_spi_start_rx(int spi_num);
> Enables the SPI receiver. Only valid for slaves. Returns error if the receive is not able to be started. There will be a receive callback per value.
>
> int hal_spi_abort(int spi_num);
> This aborts the current transfer but keeps the spi enabled.
>
> void hal_spi_set_rx_val_cb(int spi_num, hal_spi_rx_val rx_val_cb, void *arg);
> Sets the callback executed when a slave receives a value. Allowed to be called when the spi is enabled but not when a transfer is in progress.
>
> void hal_spi_set_tx_done_cb(int spi_num, hal_spi_tx_done tx_done_cb, void *arg);
> Sets the callback executed the buffer is transferred by the master or slave using the hal_spi_txrx API. Allowed to be called when the SPI is enabled but not during a transfer.
>
>