Skip to content
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

[i2c,dv] I2C Controller-Mode NACK modelling, mapping TP:host_mode_halt_on_nak #23940

Merged

Conversation

hcallahan-lowrisc
Copy link
Contributor

@hcallahan-lowrisc hcallahan-lowrisc commented Jul 5, 2024

The first two commits in this PR are squashes of other in-flight PRs, which we depend on here. Ignore for reviewing purposes.

Build on the changes in #23911 to start modelling and correctly-predicting transfers with a Target that may NACK our bytes.
The modelling in place in this PR is very rudimentary, and cannot yet make predictions when mid-transfer stimulus is applied, such as software clearing fifo state during NACK events which cause the DUT to halt. This modelling will be further fleshed-out in the future.

Map the test "i2c_host_may_nack" to the TP:host_mode_halt_on_nak.

The description and documentation of the "i2c_reference_model.sv" should be significantly fleshed-out in the future, as it will greatly increase in apparent complexity as we amend it to model and predict more and more DUT behaviours. In addition, we should strive to document under what limitations the model can predict, such as bounds for the validity of certain predictions. It would be greatly desirable to be explicit in capturing these model details.

@hcallahan-lowrisc hcallahan-lowrisc added Component:DV DV issue: testbench, test case, etc. IP:i2c labels Jul 5, 2024
@hcallahan-lowrisc hcallahan-lowrisc requested a review from a team as a code owner July 5, 2024 15:13
@hcallahan-lowrisc hcallahan-lowrisc mentioned this pull request Jul 5, 2024
13 tasks
@hcallahan-lowrisc hcallahan-lowrisc force-pushed the i2c_host_may_nack_testing branch from 79cecdc to 7cd0118 Compare July 5, 2024 15:16
Copy link
Contributor

@rswarbrick rswarbrick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've looked at the final few commits and they look sensible. One question about processes.

hw/ip/i2c/dv/env/i2c_reference_model.sv Show resolved Hide resolved
@hcallahan-lowrisc hcallahan-lowrisc marked this pull request as draft July 9, 2024 14:28
@hcallahan-lowrisc
Copy link
Contributor Author

Some parts of this PR have been relocated to other PRs, and I want to implement some other things in here differently. Therefore, I'm going to move it to draft until the concerns have been separated properly.

@hcallahan-lowrisc hcallahan-lowrisc force-pushed the i2c_host_may_nack_testing branch from 7cd0118 to 3ee2765 Compare July 10, 2024 23:43
@hcallahan-lowrisc hcallahan-lowrisc marked this pull request as ready for review July 10, 2024 23:44
@hcallahan-lowrisc hcallahan-lowrisc force-pushed the i2c_host_may_nack_testing branch from 3ee2765 to 33db515 Compare July 10, 2024 23:49
This is part of a larger push to improve the i2c_reference_model to start making
better and more granular predictions.

This commit achieves the following:
- Connects the i2c_monitor's target-mode transfer ports and in-progress ports
through to the reference model.
- Creates a local queue of predicted controller-mode transfers based on CSR
access stimulus that defines the transfers. Rather than pushing these items
directly to the scoreboard, we need to keep an in-order list to fuse
with bus observations from the i2c monitor.
  - Create a separate queue of both read and write transfers (prev_item[$])
  together, as determination of the next transfer always depends on the
  ending conditions of the previous one, no matter if it was a read or a write.
- Fuses observed ACK/NACK bits from the bus with the predictions from the CSR
accesses. This is not yet particularly interesting, but will be crucial for
better modelling mid-transfer inputs in the future.

Signed-off-by: Harry Callahan <[email protected]>
Copy link
Contributor

@rswarbrick rswarbrick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me (if rather complicated!). Thanks for the cleanups.

@hcallahan-lowrisc
Copy link
Contributor Author

Thanks @rswarbrick for the review.

@hcallahan-lowrisc hcallahan-lowrisc merged commit 84bdaf1 into lowRISC:master Jul 11, 2024
32 checks passed
@hcallahan-lowrisc hcallahan-lowrisc deleted the i2c_host_may_nack_testing branch July 11, 2024 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component:DV DV issue: testbench, test case, etc. IP:i2c
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants