-
Notifications
You must be signed in to change notification settings - Fork 8.1k
STM32: Add SPI RTIO support #91847
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
STM32: Add SPI RTIO support #91847
Conversation
|
is there some benchmark showing any value add over the default implementation? |
The main benefit is that you can simply start asynchronous transactions, at any time, from any call context. E.g. a gpio/timer ISR, even direct or zli ISRs. For sensors in particular this saves needing to use a thread per trigger with the stack cost, or the work queue with a latency cost. Are you looking for some other particular benefit/benchmark? |
f19eac6 to
4c58db2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First round of review
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems OK
|
Last week I tried on sensortile_box_pro and it was NOT working. I'm waiting for new commits to try out. Thanks for this effort! |
|
I currently have failed SPI loopback tests on boards that haven't the "st,stm32h7-spi" compatible. I'm working on solving that before retesting on sensortile_box_pro. |
815925b to
54c1d6e
Compare
|
spi_loopback test is passing on all series, RTIO or not (except STM32H7, but it is not related to this PR). |
OK Guillaume, I'll retest then. Maybe I did something wrong. But tomorrow |
|
BTW, I tested it on a nucleo_h503rb plus a SPI-configured shield with a lsm6dsv320x on DIL24. And used the stream rtio configuration. @etienne-lms can you possibly tell me how did you test (what build command) on sensortile_box_pro? |
|
I used (edited) Last option is |
OK, thx. It is the right sample! |
Move some code into a new function to prepare for RTIO integration. Signed-off-by: Guillaume Gautier <[email protected]> Signed-off-by: Etienne Carriere <[email protected]>
Remove inclusion of zephyr_arch/cache.h header file from STM32 SPI driver. This header file is included by zephyr/cache.h if applicable (e.g. CONFIG_ARCH_CACHE is enabled) and should not be used when CONFIG_EXTERNAL_CACHE is enabled. Signed-off-by: Etienne Carriere <[email protected]>
Reorder inclusions of header files to clarify and simplify later changes. By the way, remove #ifdef CONFIG_SPI_STM32_DMA condition to include DMA header files as its not required. Keep zephyr/log.h with use of LOG_MODULE_REGISTER() first since included local spi_context.h depends on log resources to be defined. Signed-off-by: Etienne Carriere <[email protected]>
Remove useless LOG_LEVEL in STM32 SPI RTIO driver. Signed-off-by: Etienne Carriere <[email protected]>
Remove bits2byte() helper function than was not always used. Replace it with a division by BITS_PER_BYTE that is explicit enough. Signed-off-by: Etienne Carriere <[email protected]>
Use explicit boolean tests in STM32 SPI driver. Signed-off-by: Etienne Carriere <[email protected]>
Correct some indentation issues, a few useless line escapes, double space characters or parentheses pair in STM32 SPI driver. Signed-off-by: Etienne Carriere <[email protected]>
Fix a missing braces pair around a conditioned instruction in STM32 SPI driver. Fix that by aggregating the 2 if() instructions into a single ANDed one. Signed-off-by: Etienne Carriere <[email protected]>
Remove useless inline keywords in STM32 SPI driver. Signed-off-by: Etienne Carriere <[email protected]>
Replace a #ifndef directive with a if(!IS_ENABLED()) instrcution in transceive() function. This change makes later integration of RTIO support in this function smoother, polluting a bit less this function with #if based directives. Signed-off-by: Etienne Carriere <[email protected]>
6a8381d to
2724b27
Compare
|
Review comments and CI Compliance Checks complain addressed. (edited) By the way, I also restored authorship of commit "drivers: spi: stm32: add rtio support" to @gautierg-st that made all the work. I saw I mistakenly replaced it when handing over this P-R, 😕. |
Add SPI RTIO support for STM32. SPI RTIO required interrupts. DMA is not supported yet. Signed-off-by: Guillaume Gautier <[email protected]> Signed-off-by: Etienne Carriere <[email protected]>
Add a test case for STM32 SPI RTIO. Signed-off-by: Guillaume Gautier <[email protected]>
Adjust transfer duration scaling config for STM32 boards Signed-off-by: Guillaume Gautier <[email protected]>
|
In the last update, I introduced build warnings and a recursion that is not really need. It's addressed now. |
|
|
All good in our HW bench. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can tell, this PR looks good to me. Thanks for your effort!
|
Thanks @avisconti for ytour help and tests. |



Add SPI RTIO support for STM32. This requires interrupts (no DMA yet)