Are SPI APIs thread safe ?

Tip / Sign in to post questions, reply, level up, and achieve exciting badges. Know more

cross mob
Anonymous
Not applicable

Looking into wiced_spi_* and platform_spi_* APIs and they don't look like thread safe to me, is that the case ? Also, it looks like current SPI implementation would not support multiples devices with difference settings such as clock frequency or bus mode, is that the case as well ?

0 Likes
1 Solution
SeyhanA_31
Employee
Employee
250 replies posted 100 replies posted 50 replies posted

Hi,

Yes, the wiced_spi_* APIs are not thread safe. Locking/Unlocking is required if multiple threads are accessing to the same SPI peripheral.

Yes, currently the same speed, mode and data size devices could share the same SPI peripheral.


Seyhan

View solution in original post

22 Replies
SeyhanA_31
Employee
Employee
250 replies posted 100 replies posted 50 replies posted

Hi,

Yes, the wiced_spi_* APIs are not thread safe. Locking/Unlocking is required if multiple threads are accessing to the same SPI peripheral.

Yes, currently the same speed, mode and data size devices could share the same SPI peripheral.


Seyhan

Anonymous
Not applicable

Thanks for confirming, I have added locking/unlocking as well as per bus context to allow different device configuration. It seems to work well for me. Let me know if you would like to share that back.

Anonymous
Not applicable

Why is the same speed, mode and data size shared? It looks like the wiced_spi_device_t structure is passed into the init and the deinit (which looks more like a reinit to me).  Since you have to specify a different GPIO pin to select by, why can't the speed/mode/size be specified on each init/reinit when you select the desired slave?

0 Likes
Anonymous
Not applicable

SPI bus is initialized once, during spi_init. Previous settings are not retained. Currently, if you init port with settings A, then again for another device with setting B, settings A will be lost. Worst, a transfer for device A, after device B has been initialized will be performed with most recent settings.

Current SPI bus driver is pretty much broken for multi-client use. A concept of bus context needs to be introduced and bus re-configure for each client when necessary between transactions as it is not the case currently.

Also, a lock / port needs to be added so multi-threaded system can access ports safely.

0 Likes
Anonymous
Not applicable

So is this a problem with the SDK or the driver talking to the SDK?

On the side of the SDK, you could locally have a state that indicates which SPI slave you are talking to, you can easily switch states on the calling side by calling init/deinit. As long as you switch at request boundaries, which would be ensured by your locking code, the slave should not see the switch and the reconfigure should work fine.

0 Likes
Anonymous
Not applicable

I believe the is should be fixed on the MCU platform code, it is quite easy to achieve, what is the point of having SPI device config passed down to the SPI platform APIs if they are not used to reconfigure the bus prior to transfer ?

0 Likes
Anonymous
Not applicable

I am not disagreeing with you.  The simplest interface that could have been given to you would be one that just requested a transfer and passed a wiced_spi_device_t , which it does, and have that interface perform the init/deinit as is required.  That would be clean and would solve this part of your request.


I don't know what the API designer(s) had in mind.  It is possible they separated the init/deinit from the transfer to allow the hardware to be turned off to save power and pass the structure pointer just as a handle.  It is hard to tell without asking them.

0 Likes
Anonymous
Not applicable

The deinit does actually do anything, I think SPI wasn't quite thought through as a bus, with multiple client in mind. It was clearly design to handle the serial flash that comes with the ref design.

Since I/O are pretty limited on the module itself, I use a SPI GPIO expander, a SPI display, etc... and the driver in its current state does not quite cut it.

I open that thread before taking on the task of modifying the SDK code to my need, just to check that I was not missing anything.

Maybe my use case is singular, but maybe not 😉

0 Likes
Anonymous
Not applicable

The deinit in its current state, looks like you could pass a different wiced_spi_device_t structure and it would reinit, which was part of my first comment.

The API designer thought it out enough to pass a GPIO and didn't make the assumption that there was a hard wired slave select line.  So they did move in the right direction.  Making the change to drop init/deinit and just perform the transfer (configuring as required based on the passed wiced_spi_device_t) seems like it is a simple request and one that should be quite common, and not just a singular case.  Does anyone know who designed this interface?

0 Likes
Anonymous
Not applicable

You are indeed right about deinit calling init, I was looking at the MCU code itself.

Maybe they were expecting caller to take care of handling the lock, calling init before every single transfer... That would probably work, but it is not intuitive, not efficient either if you ask me.

0 Likes
Anonymous
Not applicable

The locking is best handled by the caller.  This could be handled by a semaphore or disabling interrupts, as is required.  The latter could be a performance hit and should not be inflicted on all for the cases that the request is being performed at an ISR level, I would assume this to be a minority.

If the MCU code kept the state of the current connection and compared against that before performing the inefficient configuration setup, then you would only take the hit when you change the device you are talking to. In most cases, you are going to have bursts of activity on one device at a time and this would solve that issue.

0 Likes
Anonymous
Not applicable

I am not sure I agree by the lock handled by caller, but we can agree to disagree on that 🙂

Regarding current port state, this is what I did, comparison between caller device settings and current set settings comes down to a few memcpm() on a few bytes, this is quite painless. If setting is different, port is reconfigured.

Anonymous
Not applicable

I will agree on the disagreeing 🙂

You chose the right answer and we just need to get this pushed into the MCU code and it will be handled for all.  I will ask around to see who designed that and if it can be changed.

Thanks, Denis

Anonymous
Not applicable

Thanks for the feedback !

0 Likes
Anonymous
Not applicable

I believe that the future direction for this interface is to keep the three interfaces.

  1. Init will register the SPI device.
  2. Deinit will unregister it and allow the interface to be shut off, assuming no other registered SPI devices are still outstanding.
  3. Transfer will check the passed wiced_spi_device_t and verify it is registered and will perform any required configuration to make sure that the SPI bus is ready to talk to the specific device.

The reasoning behind keeping the init/deinit is as I mentioned above that it might be there for power management reasons and that was a correct assumption.

0 Likes
Anonymous
Not applicable

I think this is fine, as long as the the transfer part get fixed and allow different configurations for different devices.

I still do not understand why the lock should be handled by the upper layer, but I feel like we could be discussing this all day 🙂

As for power management, in a SPI master configuration, interface could be shutdown at all times, except during transfer, powerUp -> transfer -> powerDown. This is assuming power up does not imply long timing delay, like waiting for PLL to lock for instance, but I don't think this is the case.

0 Likes
Anonymous
Not applicable

The transfer part will get fixed.

The lock that works for you may not work for me.  I may have only one device and I don't want to take any extra cycles performing useless code. I may interact with SPI in a task.  I may talk at the ISR level... The answers are all different and to make this generic would be difficult at best if efficiency is top priority.

You can take the entire interface down, but that is not very efficient.  You will gain the same feel to init and deinit for each transfer.  If I am talking to a SPI flash, then I may need to perform a number of operations to make data flow in one direction or the other.  In this case, I don't want to take the power to come up and restart my clocks for each SPI transaction.

0 Likes
Anonymous
Not applicable

Alright, fair enough, maybe a sample app for SPI showing API flow would be great, serial flash driver isn't really appropriate for that.

0 Likes
Anonymous
Not applicable

Hi

If I'm using SPI6 and SPI1(each has only one device connecetd to it) do I need to call the spi_init() function each time while using SPI6 or SPI1 ?

0 Likes
Anonymous
Not applicable

Hi,

As long as you have only one slave per master, I don't think you need to call spi_init() between each transfer.

Did you actually try ?

0 Likes
Anonymous
Not applicable

Thanks,

I haven't tried it yet, but will post the result once I test it....

0 Likes
Anonymous
Not applicable

Discussion is being locked. If you have any follow-up, please start a new discussion.

0 Likes