i2c: Bug fixes for default RTIO handler #79890
Merged
Conversation
This file contains 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
teburd
force-pushed
the
i2c_rtio_drop_txrx
branch
from
036211e
to
940f2ba
Compare
ubieda
previously approved these changes
Oct 18, 2024
|
Not sure why this PR didn't get review-requests. |
teburd
changed the title
TXRX is meant specifically to handle a full duplex bus like SPI, I2C is half duplex meaning only read or write can be performed at once. Drop TXRX as a supported operation code for the default I2C submission path. Signed-off-by: Tom Burdick <thomas.burdick@intel.com>
teburd
force-pushed
the
i2c_rtio_drop_txrx
branch
3 times, most recently
from
30417c4
to
eff46a7
Compare
|
looks like some tests may need to be updated... but I ported over this code to my default i3c rtio handler branch that I'm working on and it's looking as expected with 'restarts' in between transactions. Thanks for this!! |
ubieda
previously approved these changes
Oct 19, 2024
There was a problem hiding this comment.
Code looks good! Approving assuming we take care of https://github.com/zephyrproject-rtos/zephyr/tree/main/tests/subsys/rtio/rtio_i2c.
teburd
force-pushed
the
i2c_rtio_drop_txrx
branch
2 times, most recently
from
db32ac8
to
5c8217d
Compare
teburd
force-pushed
the
i2c_rtio_drop_txrx
branch
2 times, most recently
from
77186ea
to
d84d383
Compare
Transactions from RTIO should result in single calls to i2c_transfer. This corrects the default handler to first count the number of submissions in the transaction, allocate on the stack, and then copy over each submission to an equivalent i2c_msg. It also cleans up the helper functions to be infallible, taking only the submission and msg to copy to. Signed-off-by: Tom Burdick <thomas.burdick@intel.com>
teburd
force-pushed
the
i2c_rtio_drop_txrx
branch
from
d84d383
to
fafa353
Compare
Transactions should result in a single transfer call not multiple transfer calls. Transceive isn't supported by i2c and so the TXRX op isn't validated for success anymore. Signed-off-by: Tom Burdick <thomas.burdick@intel.com>
ubieda
approved these changes
Oct 22, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
TXRX is meant specifically to handle a full duplex bus like SPI, I2C is half duplex meaning only read or write can be performed at once. Drop TXRX from the default handler.
Each transaction should result in a single call to i2c_transfer. This corrects the default handler to do just that. Also simplifies the various helpers here.