RFC: rtio: SPI and a Fast Sensor #55383
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tbursztyka
reviewed
Mar 3, 2023
teburd
force-pushed
the
tdk_icm42688p_rtio_spi2
branch
8 times, most recently
from
1071d7d to
35d637e
Compare
Contributor
Author
|
Patch set is mostly cleaned up now and worth perusing for criticisms and such. |
teburd
force-pushed
the
tdk_icm42688p_rtio_spi2
branch
7 times, most recently
from
162fa6f to
1f48b88
Compare
Merged
MaureenHelm
reviewed
Mar 22, 2023
Member
MaureenHelm
left a comment
MaureenHelm
left a comment
There was a problem hiding this comment.
some nitpicky comments, but overall this looks very cool!
| const struct sensor_driver_api *api = | ||
| (const struct sensor_driver_api *)dev->api; | ||
|
|
||
| return api->fifo_iodev(dev, iodev); |
Member
There was a problem hiding this comment.
Might want to check if fifo_iodev is NULL
| # | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| DT_COMPAT_ICM42688 := invensense,icm42688 |
Member
There was a problem hiding this comment.
Most (all?) of these have been removed. Is this a rebasing mistake?
Provides the necessary APIs to stream a sensors FIFO using RTIO Adds an API call to enable explicitly reading the fifo from the sensor at any time. The thinking is that while the sensors gpio trigger might be one source of triggering a fifo read, external sources should be possible. Signed-off-by: Tom Burdick <[email protected]>
teburd
force-pushed
the
tdk_icm42688p_rtio_spi2
branch
from
1f48b88 to
2a87239
Compare
An experimental idea to enable streaming fifo data from the sensor using RTIO with the existing sensor API. A sensor reader could easily be built on top of the buffer to produce meaningful values. There are many ways this can be built currently. Using either the existing trigger work context (system workqueue or own thread) when SPI_RTIO=n or using a sequence. With or without SPI_SAM_DMA (where only polling SPI transfers are used). This is a really nice setup to explore and experiment with RTIO, SPI, and a really fast sensor. Signed-off-by: Tom Burdick <[email protected]>
Provides a sample demonstrating the fast sensor on a tdk robokit1 used with and without RTIO. With RTIO the sample may be built with SPI_RTIO enabled or disabled to get a sense of the cost of a filler thread when SPI does not support RTIO as well. Signed-off-by: Tom Burdick <[email protected]>
While an asynchronous transfer is on going for the 6 axis imu, concurrently read the magnetometer with its blocking fetch call. In the future it'd be nice to have the magnetometer also work with RTIO driven by a timer. In fact having all the sensors read on a timer would be incredibly nice to showcase. Signed-off-by: Tom Burdick <[email protected]>
teburd
force-pushed
the
tdk_icm42688p_rtio_spi2
branch
from
2a87239 to
a82d297
Compare
|
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Many changes in this RFC PR are very unlikely to make it upstream as-is or perhaps ever but as a practical demonstration for how RTIO could be used with a sensor, on a spi bus it does just that.
The sample app itself (samples/boards/tdk_robokit1) shows how delays may result in buffer underruns/overruns (depending on the perspective!), and how they may be resolved by having more buffers to feed the demand of the sensor. It would be interesting to also consider a throughput/latency comparison... lots of small buffers vs a few large ones.
It also lets you build with and without CONFIG_SPI_RTIO to show how thread stacks might be saved by using RTIO with SPI. Once #55178 is merged I'll rebase again and we can do probably even more compare/contrasts to get a sense of things.
This builds on top of #55382