I3C peripheral updates and integration tests #41
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
swenson
requested review from
parvathib,
mlvisaya,
mhatrevi,
vsonims and
helloxiling
swenson
force-pushed
the
i3c-peripheral
branch
from
5ba3aa7 to
f5e1c48
Compare
helloxiling
reviewed
Dec 2, 2024
emulator/periph/src/i3c.rs
Outdated
| use emulator_cpu::Irq; | ||
| use emulator_registers_generated::i3c::I3cPeripheral; | ||
| use emulator_types::{RvData, RvSize}; | ||
| use registers_generated::i3c::bits::InterruptEnable; | ||
| use registers_generated::i3c::bits::InterruptStatus; | ||
| use registers_generated::i3c::bits::StbyCrDeviceAddr; |
There was a problem hiding this comment.
minor: is it better to combine them into a single use statement?
helloxiling
reviewed
Dec 2, 2024
| /// TX Command in u32 | ||
| tti_tx_desc_queue_raw: VecDeque<u32>, | ||
| /// TX DATA in u32 | ||
| tti_tx_data_raw: VecDeque<u8>, | ||
| tti_tx_data_raw: VecDeque<Vec<u8>>, |
There was a problem hiding this comment.
Minor: comment should be updated. /// TX DATA in u8
helloxiling
reviewed
Dec 2, 2024
| @@ -273,18 +273,10 @@ impl<'a, A: Alarm<'a>> I3CCore<'a, A> { | |||
| } | |||
There was a problem hiding this comment.
Is interrupt-status register a regular type or write1toclear? If it is write1toclear, I think we should change this piece of code accordingly.
There was a problem hiding this comment.
Ah, you're correct, the docs say "woclr".
swenson
force-pushed
the
i3c-peripheral
branch
from
d8a0cf7 to
db4bb6c
Compare
This progresses more on the I3C peripheral, correctly queueing writes how the hardware does. We also start on writing a socket protocol so that another thread or program can send reads and writes to the I3C bus. This hasn't been teste yet though. I noticed two bugs that I will address in separate PRs: * The I3C interrupts are not quite working yet. When enabled, they are routed correctly to the kernel handler, but keep getting retriggered and not handled properly. I've disabled the interrupts for now, so the read/write methods need to be called manually. * Alarms are not firing properly. I tried to work around the interrupts not handling correctly by scheduling an alarm to call the read/write methods periodically, but the alarms never fire. I've noticed something similar to this before. I will debug this separately. The biggest piece though is adding new integration tests for the kernel: * A test can be added to `tests/integration/src/lib.rs` * This will build the emulator and firmware with the feature equal to the test name, and then run them. * The test can exit with a non-zero exit code to signal failure. The tests need to built sequentially so that they don't conflict when writing the `layout.ld` linker script (this has to have the same name each time to avoid modifying `RUSTFLAGS` with every invocation; otherwise dependencies are recompiled every single time).
swenson
force-pushed
the
i3c-peripheral
branch
from
8d30f8c to
277dc87
Compare
helloxiling
approved these changes
Dec 3, 2024
|
Thanks! |
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 progresses more on the I3C peripheral, correctly queueing writes how the hardware does.
We also start on writing a socket protocol so that another thread or program can send reads and writes to the I3C bus. This hasn't been teste yet though.
I noticed two bugs that I will address in separate PRs:
The biggest piece though is adding new integration tests for the kernel:
tests/integration/src/lib.rsThe tests need to built sequentially so that they don't conflict when writing the
layout.ldlinker script (this has to have the same name each time to avoid modifyingRUSTFLAGSwith every invocation; otherwise dependencies are recompiled every single time).