[test] Add spi_passthrough support to the power virus test #17747
Conversation
bilgiday
requested review from
jdonjdon,
cindychip and
timothytrippel
and removed request for
a team
| // The sequence of opcodes used for the test. | ||
| spi_flash_cmd_e test_opcodes[$]; | ||
| // Frequencies to use for testing the sequences. | ||
| time sck_periods_ps[] = '{ |
There was a problem hiding this comment.
we probably only want to use the highest frequency for this test no?
There was a problem hiding this comment.
Yep, right, it makes sense :) Will use 30MHz 👍
| virtual function void generate_spi_flash_sequence(); | ||
| // Reference: chip_sw_spi_passthrough_vseq.sv | ||
| // Keep the structure in case we need to send different commands later. | ||
| test_opcodes = '{ |
There was a problem hiding this comment.
does this need to be in a function? could it just be global? (will defer to DV experts here 😃 )
There was a problem hiding this comment.
I think like the comments in line 94, if this test plans to change the test_opcode sequence in an extended test, then we can make it a function and override it later.
If we do not intend to create any tests based on this one, I think it is easier to just make a global variable :)
There was a problem hiding this comment.
Hehe, thanks!
To make the code cleaner, I went over the whole spi_passthrough part and removed the parts which are not strictly necessary for the power virus test: Now, the code just sends a SpiFlashReadQuad command to read 2048 bytes. Please take a look at it and let me know if it makes sense :)
| forever begin | ||
| `uvm_create_on(m_spi_device_seq, p_sequencer.spi_device_sequencer_hs[0]); | ||
| `uvm_send(m_spi_device_seq); | ||
| device_rsp_q.push_back(m_spi_device_seq.rsp); |
There was a problem hiding this comment.
I assume the response packets are comprised of random bits? was there a way to make them all 01010101 for maximum switching? i.e., send 0xaa repeatedly
There was a problem hiding this comment.
I will check this one 👍
There was a problem hiding this comment.
I think one way to do it is to assign the rsp value before sending the sequence, for example something like this:
https://cs.opensource.google/opentitan/opentitan/+/master:hw/top_earlgrey/dv/env/seq_lib/chip_sw_spi_passthrough_collision_vseq.sv;l=82?q=m_spi_device_seq&ss=opentitan%2Fopentitan
There was a problem hiding this comment.
Or like you did in line 130, that could work too I think
There was a problem hiding this comment.
Thanks for the guidance! I updated the code based on your suggestion.
Now, the response packages are an alternating pattern of 0xaa and 0x55: All of the data lines are toggled at every SCK cycle.
| spi_item device_rsp_q[$]; | ||
| spi_agent_cfg agent_cfg = cfg.m_spi_host_agent_cfg; | ||
|
|
||
| fork begin : isolation_fork |
There was a problem hiding this comment.
Under this isolation fork, do you think this structure is more clear?
fork begin : isolation_fork
forever begin : send_spi_device_seq_forever
end
begin: spi_host_seq
end
join_any
disable fork;
end join
There was a problem hiding this comment.
Thanks Cindy :) I applied this change in the new version.
Add spi_device_agent_0 and spi_host_agent code to generate command-response sequence for the SPI passthrough part of the test. Signed-off-by: Bilgiday Yuce <[email protected]>
bilgiday
force-pushed
the
power_virus_passthru_pr
branch
from
0336c7a
to
e2b3141
Compare
Add spi_device_agent_0 and spi_host_agent code to generate command-response sequence for the SPI passthrough part of the test.
The sequence:
1- spi_host_agent sends a SpiFlashReadQuad command
2- The command goes through the spi_device and spi_host_0 IPs that are configured in the passthrough mode
3- spi_device_agent_0 will respond with 2048 bytes of data.
Tested in DVSim.