Backwards compatible eh1 SPI support #163
Open
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
When we change the transmit / receive masks within a continuous transaction, we might demonstrate an LPSPI hardware defect. In the example shown here, the LPSPI SCK stalls when transmitting the final bit of the final byte, preventing the read from executing. Right now, I'm not sure if this is the LPSPI continuous transaction hardware defect, or if there's an issue in this new driver. I'll shoot for a deeper review later.
mciantyre
reviewed
Jun 10, 2024
There was a problem hiding this comment.
The backwards compatibility LGTM. Thanks for sharing this idea!
This looks like a good starting point for an EH1 SPI bus and device implementation. I agree that we can break / extend it later.
I pushed my first test case to this branch. All existing transactions in the loopback test are still working on my 1176 MCU. However, the SpiDevice transaction sequence
spi.transaction(&mut [
TransferInPlace(&mut xfer),
Read(&mut read),
]).unwrap();isn't finishing the TransferInPlace operation. My logic analyzer output is below. It looks like SCK is stuck high, and it's not clocking out the final bit of the final byte.
SCK stuck high before end of transfer
Nothing about the continuous transactions looked obviously questionable, so I'm still digging into the issue.
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.
This adds impls of eh1's
SpiBusandSpiDevicetraits toLpspiin a backwards compatible manner.At least I think it is backwards compatible. There are some added and changes trait bounds, but they ought to be equivalent to or slightly more permissive than the preexisting ones.
The
DelayNsoperation is not supported and documented as such, since without breaking changes we have no delay provider available to us. Users that needs it can useembedded-hal-busin conjunction with theSpiBusimpl.This is probably not where we want to go long term. The
Lpspitype needs to change to support multiple chip selects, which will probably create a more natural split between the Bus and the Device trait. However, I think it is beneficial to provide a basic impl to users in the meantime.Caveat: I've not yet extensively tested this.