lowRISC / opentitan

OpenTitan: Open source silicon root of trust
https://www.opentitan.org
Apache License 2.0
2.55k stars 759 forks source link

[i2c/dv] issues with i2c_agent #14825

Open tjaychen opened 2 years ago

tjaychen commented 2 years ago

This issue attempts to describe all the known issues with the existing i2c_agent and how they should be fixed.

The i2c_monitor directly interprets the line content and creates response items (acknowledgement, read data) instead of just passively observing the line. The i2c_driver, instead of just receiving data and driving it onto the pins, has a method of directly creating randomized data

To address these issues, the following should be done:

tjaychen commented 2 years ago

@sriyerg @jdonjdon

let me know if there are others you noticed. I admittedly didn't really look at the scoreboard and focused mostly on the driver / monitor / base_seq

tjaychen commented 1 year ago

@jdonjdon i think we can postpone this to M3. Because this issue is really about all the things in the i2c agent that need to be cleaned up, but perhaps it cannot be done in time for M2.

Let me know what you think.

jdonjdon commented 1 year ago

@jdonjdon i think we can postpone this to M3. Because this issue is really about all the things in the i2c agent that need to be cleaned up, but perhaps it cannot be done in time for M2.

Let me know what you think.

Agree

tjaychen commented 1 year ago

actually @jdonjdon were some of these items already addressed? do you want to update the description to be more accurate?

jdonjdon commented 1 year ago

actually @jdonjdon were some of these items already addressed? do you want to update the description to be more accurate?

This issues are only applied to "host mode' rtl test bench. Unfortunately none of these are addressed during recent "target mode" rtl verification.

tjaychen commented 1 year ago

do you think this is even worth fixing in V3?

jdonjdon commented 1 year ago

I am neutral at this point. Can we revisit after v2?

hcallahan-lowrisc commented 3 months ago

This is a V3/icebox item really, hence moved to P4 and removed the PROD Candidate label.