-
Notifications
You must be signed in to change notification settings - Fork 18
[chip, I2C, DV] Top level I2C host test #473
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
base: main
Are you sure you want to change the base?
Changes from all commits
5e1d075
d630bb1
9d075b5
48917e0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| // Copyright lowRISC contributors (COSMIC project). | ||
| // Licensed under the Apache License, Version 2.0, see LICENSE for details. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| // This vseq is going to be starting a reactive sequence. | ||
| // | ||
| // i2c_monitor shares an analysis port with i2c_sequencer. It sends an i2c_item which contains | ||
| // a member "state". i2c_monitor watches the i2c_if and as soon as it sees the communication started | ||
| // on the bus, it change the state accordingly. Based on the state received on the | ||
| // analysis port of sequencer, i2c_base_seq initializes the start_item method to send i2c_item to | ||
| // i2c_driver so that it can drive ack, nack or rdata to the controller. | ||
| class top_chip_dv_i2c_host_tx_rx_vseq extends top_chip_dv_i2c_tx_rx_vseq; | ||
| `uvm_object_utils(top_chip_dv_i2c_host_tx_rx_vseq) | ||
|
|
||
| extern function new(string name=""); | ||
| extern task body(); | ||
| extern virtual task dut_init(string reset_kind = "HARD"); | ||
| endclass : top_chip_dv_i2c_host_tx_rx_vseq | ||
|
|
||
| function top_chip_dv_i2c_host_tx_rx_vseq::new(string name = ""); | ||
| super.new(name); | ||
| endfunction | ||
|
|
||
| task top_chip_dv_i2c_host_tx_rx_vseq::dut_init(string reset_kind = "HARD"); | ||
| // Read the timing parameters through SW backdoor load | ||
| sw_symbol_backdoor_read("SysClkPeriodNS", sw_sys_clk_period_ns); | ||
| sw_symbol_backdoor_read("SCLLowTimeNs", sw_scl_low_time_ns); | ||
| sw_symbol_backdoor_read("HoldDataTimeNs", sw_data_hold_time_ns); | ||
|
KinzaQamar marked this conversation as resolved.
|
||
|
|
||
| scl_low_cycles = round_up_divide({sw_scl_low_time_ns[1], sw_scl_low_time_ns[0]}, | ||
| sw_sys_clk_period_ns[0]); | ||
| sda_hold_cycles = round_up_divide({sw_data_hold_time_ns[1], sw_data_hold_time_ns[0]}, | ||
| sw_sys_clk_period_ns[0]); | ||
|
|
||
| super.dut_init(reset_kind); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we call the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No reason but just a preference to override the function first and then call the parent class definition later
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, for the moment this works as the memories are loaded at compile time, but I think this call should be moved before as it calls In my experience, in general it's better to call the |
||
| endtask | ||
|
KinzaQamar marked this conversation as resolved.
|
||
|
|
||
| task top_chip_dv_i2c_host_tx_rx_vseq::body(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should add a call to the super in case there's something added there in the future: Also, for consistency, you can have these lines at the very beginning like we do for other vseqs: `DV_WAIT(cfg.sw_test_status_vif.sw_test_status == SwTestStatusInTest);
`uvm_info(`gfn, "Starting I2C Host TX-RX test", UVM_LOW) |
||
| i2c_device_response_seq seq = i2c_device_response_seq::type_id::create("seq"); | ||
|
|
||
| configure_agent_timing(); | ||
| print_i2c_timing_cfg(); | ||
|
|
||
| // Configure the agent to be reactive | ||
| cfg.m_i2c_agent_cfg.if_mode = Device; | ||
|
|
||
| `DV_WAIT(cfg.sw_test_status_vif.sw_test_status == SwTestStatusInTest); | ||
|
|
||
| `uvm_info(`gfn, "Starting I2C Host TX-RX test", UVM_LOW) | ||
|
|
||
| fork | ||
| seq.start(p_sequencer.i2c_sqr); | ||
| join_none | ||
| endtask | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| // Copyright lowRISC contributors (COSMIC project). | ||
| // Licensed under the Apache License, Version 2.0, see LICENSE for details. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| class top_chip_dv_i2c_tx_rx_vseq extends top_chip_dv_base_vseq; | ||
| `uvm_object_utils(top_chip_dv_i2c_tx_rx_vseq) | ||
|
|
||
| // Below variables will get assign through SW backdoor load. They are defined as byte size array | ||
| // because "sw_symbol_backdoor_read/overwrite" takes an array as an argument to write or read the | ||
| // SW symbol. | ||
| protected bit [7:0] sw_sys_clk_period_ns[1]; | ||
| protected bit [7:0] sw_scl_low_time_ns[2]; | ||
| protected bit [7:0] sw_data_hold_time_ns[2]; | ||
|
|
||
| // The timing parameter in cycles used by the agent to add relevant delays before driving the | ||
| // transfer | ||
| protected bit [15:0] scl_low_cycles; | ||
| protected bit [15:0] sda_hold_cycles; | ||
|
|
||
| extern function new(string name=""); | ||
|
|
||
| // Obtain an integer minimum of timing parameter "a" (which is expected to be a spec minimum | ||
| // value) and round up to the next highest integer. | ||
| extern protected function int unsigned round_up_divide(int unsigned a, int unsigned b); | ||
|
|
||
| // Compute timing parameters utilized by the agent to add delays to the transfer | ||
| extern protected function void configure_agent_timing(); | ||
| extern protected function void print_i2c_timing_cfg(); | ||
| endclass : top_chip_dv_i2c_tx_rx_vseq | ||
|
|
||
| function top_chip_dv_i2c_tx_rx_vseq::new(string name = ""); | ||
| super.new(name); | ||
| endfunction | ||
|
|
||
| function int unsigned top_chip_dv_i2c_tx_rx_vseq::round_up_divide(int unsigned a, int unsigned b); | ||
| return (((a - 1) / b) + 1); | ||
| endfunction | ||
|
|
||
| function void top_chip_dv_i2c_tx_rx_vseq::configure_agent_timing(); | ||
| // tSetupBit are the cycles before SCL goes high to drive SDA. Agent should drive at least two | ||
| // cycles before SCL goes high. | ||
| int unsigned tSetupBit = 2; | ||
| cfg.m_i2c_agent_cfg.timing_cfg.tSetupBit = tSetupBit; | ||
|
|
||
| // tHoldBit are the cycles to hold SDA after SCL goes low. | ||
| cfg.m_i2c_agent_cfg.timing_cfg.tHoldBit = sda_hold_cycles; | ||
|
|
||
| // Used by i2c_if to stretch SCL by tClockpulse before driving SDA. If tClockPulse is greater | ||
| // than scl_low_cycles then i2c_monitor acknowledge the Ack later and then drives Rdata when SCL | ||
| // is high. | ||
| cfg.m_i2c_agent_cfg.timing_cfg.tClockPulse = scl_low_cycles; | ||
|
|
||
| // tClockLow are the SCL low cycles that the i2c_driver use before driving SDA after stretching | ||
| // SCL by tClockPulse cycles. Drive SDA at least tSetBit cycles earlier to avoid the chances of | ||
| // SDA interference. | ||
| cfg.m_i2c_agent_cfg.timing_cfg.tClockLow = scl_low_cycles - tSetupBit; | ||
| endfunction | ||
|
|
||
| function void top_chip_dv_i2c_tx_rx_vseq::print_i2c_timing_cfg(); | ||
| timing_cfg_t timing_cfg = cfg.m_i2c_agent_cfg.timing_cfg; | ||
| string str = ""; | ||
|
|
||
| // Print the timing parameters in a tabular form | ||
| str = {str, "\n+----------------------+---------+"}; | ||
| str = {str, $sformatf("\n| %-20s | %-7s |", "Timing Parameter", "Value")}; | ||
| str = {str, "\n+----------------------+---------+"}; | ||
| str = {str, $sformatf("\n| %-20s | %7d |", "tSetupStart", timing_cfg.tSetupStart)}; | ||
| str = {str, $sformatf("\n| %-20s | %7d |", "tHoldStart", timing_cfg.tHoldStart)}; | ||
| str = {str, $sformatf("\n| %-20s | %7d |", "tClockStart", timing_cfg.tClockStart)}; | ||
| str = {str, $sformatf("\n| %-20s | %7d |", "tClockLow", timing_cfg.tClockLow)}; | ||
| str = {str, $sformatf("\n| %-20s | %7d |", "tSetupBit", timing_cfg.tSetupBit)}; | ||
| str = {str, $sformatf("\n| %-20s | %7d |", "tClockPulse", timing_cfg.tClockPulse)}; | ||
| str = {str, $sformatf("\n| %-20s | %7d |", "tHoldBit", timing_cfg.tHoldBit)}; | ||
| str = {str, $sformatf("\n| %-20s | %7d |", "tClockStop", timing_cfg.tClockStop)}; | ||
| str = {str, $sformatf("\n| %-20s | %7d |", "tSetupStop", timing_cfg.tSetupStop)}; | ||
| str = {str, $sformatf("\n| %-20s | %7d |", "tHoldStop", timing_cfg.tHoldStop)}; | ||
| str = {str, $sformatf("\n| %-20s | %7d |", "tTimeOut", timing_cfg.tTimeOut)}; | ||
| str = {str, $sformatf("\n| %-20s | %7d |", "enbTimeOut", timing_cfg.enbTimeOut)}; | ||
| str = {str, $sformatf("\n| %-20s | %7d |", "tStretchHostClock",timing_cfg.tStretchHostClock)}; | ||
| str = {str, $sformatf("\n| %-20s | %7d |", "tSdaUnstable", timing_cfg.tSdaUnstable)}; | ||
| str = {str, $sformatf("\n| %-20s | %7d |", "tSdaInterference", timing_cfg.tSdaInterference)}; | ||
| str = {str, $sformatf("\n| %-20s | %7d |", "tSclInterference", timing_cfg.tSclInterference)}; | ||
| `uvm_info(`gfn, $sformatf("%s", str), UVM_MEDIUM); | ||
| endfunction | ||
|
KinzaQamar marked this conversation as resolved.
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -13,6 +13,7 @@ class top_chip_dv_env extends uvm_env; | |||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Agents | ||||||||||||||||||||||||||||||||||
| uart_agent m_uart_agent; | ||||||||||||||||||||||||||||||||||
| i2c_agent m_i2c_agent; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Standard SV/UVM methods | ||||||||||||||||||||||||||||||||||
| extern function new(string name = "", uvm_component parent = null); | ||||||||||||||||||||||||||||||||||
|
|
@@ -67,6 +68,12 @@ function void top_chip_dv_env::build_phase(uvm_phase phase); | |||||||||||||||||||||||||||||||||
| `uvm_fatal(`gfn, "Cannot get sys_clk_vif") | ||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Set I2C agent config object for I2C agent | ||||||||||||||||||||||||||||||||||
| uvm_config_db#(i2c_agent_cfg)::set(this, "m_i2c_agent", "cfg", cfg.m_i2c_agent_cfg); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Create I2C agent | ||||||||||||||||||||||||||||||||||
| m_i2c_agent = i2c_agent::type_id::create("m_i2c_agent", this); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Instantiate UART agent | ||||||||||||||||||||||||||||||||||
| m_uart_agent = uart_agent::type_id::create("m_uart_agent", this); | ||||||||||||||||||||||||||||||||||
| uvm_config_db#(uart_agent_cfg)::set(this, "m_uart_agent*", "cfg", cfg.m_uart_agent_cfg); | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+71
to
79
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit - for consistency:
Suggested change
|
||||||||||||||||||||||||||||||||||
|
|
@@ -83,6 +90,7 @@ function void top_chip_dv_env::connect_phase(uvm_phase phase); | |||||||||||||||||||||||||||||||||
| // Track specific agent sequencers in the virtual sequencer. | ||||||||||||||||||||||||||||||||||
| // Allows virtual sequences to use the agents to drive RX items. | ||||||||||||||||||||||||||||||||||
| top_vsqr.uart_sqr = m_uart_agent.sequencer; | ||||||||||||||||||||||||||||||||||
| top_vsqr.i2c_sqr = m_i2c_agent.sequencer; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Connect monitor output to matching FIFO in the virtual sequencer. | ||||||||||||||||||||||||||||||||||
| // Allows virtual sequences to check TX items. | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -18,6 +18,7 @@ class top_chip_dv_env_cfg extends uvm_object; | |||||
|
|
||||||
| // External interface agent configs | ||||||
| rand uart_agent_cfg m_uart_agent_cfg; | ||||||
| i2c_agent_cfg m_i2c_agent_cfg; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no member in
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I understand correctly, the point of adding
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My understanding is that if you have a class member declared with
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right, I just tried printing the value of
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is a common (and a good) practice to declare this config class as Often config classes have randomized variables in order to explore more unexpected configurations and discover more bugs. |
||||||
|
|
||||||
| `uvm_object_utils_begin(top_chip_dv_env_cfg) | ||||||
| `uvm_object_utils_end | ||||||
|
|
@@ -45,6 +46,9 @@ function void top_chip_dv_env_cfg::initialize(); | |||||
| // Configuration is required to perform meaningful monitoring | ||||||
| m_uart_agent_cfg.en_tx_monitor = 0; | ||||||
| m_uart_agent_cfg.en_rx_monitor = 0; | ||||||
|
|
||||||
| // Create I2C agent config object | ||||||
| m_i2c_agent_cfg = i2c_agent_cfg::type_id::create("m_i2c_agent_cfg"); | ||||||
| endfunction : initialize | ||||||
|
|
||||||
| function void top_chip_dv_env_cfg::get_mem_image_files_from_plusargs(); | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -38,10 +38,23 @@ module tb; | |||||
| logic rng_valid; | ||||||
| logic [top_pkg::EntropySrcRngBusWidth-1:0] rng_bits; | ||||||
|
|
||||||
| // I2C connections | ||||||
| wire scl; | ||||||
| wire sda; | ||||||
| logic scl_en_o; | ||||||
| logic sda_en_o; | ||||||
| logic scl_o; | ||||||
| logic sda_o; | ||||||
|
|
||||||
| // ------ Interfaces ------ | ||||||
| clk_rst_if sys_clk_if(.clk(clk), .rst_n(rst_n)); | ||||||
| uart_if uart_if(); | ||||||
| pins_if #(NUM_GPIOS) gpio_pins_if (.pins(gpio_pads)); | ||||||
| i2c_if i2c_if (.clk_i(clk ), | ||||||
| .rst_ni(rst_n ), | ||||||
| .scl_io(scl ), | ||||||
| .sda_io(sda ) | ||||||
| ); | ||||||
|
|
||||||
| // ------ Mock DRAM ------ | ||||||
| top_pkg::axi_dram_req_t dram_req; | ||||||
|
|
@@ -82,6 +95,13 @@ module tb; | |||||
| // UART receive and transmit. | ||||||
| .uart_rx_i (uart_if.uart_rx ), | ||||||
| .uart_tx_o (uart_if.uart_tx ), | ||||||
| // I2C controller/target bidirectional interface. | ||||||
| .i2c_scl_i (scl ), | ||||||
| .i2c_scl_o (scl_o ), | ||||||
| .i2c_scl_en_o (scl_en_o ), | ||||||
| .i2c_sda_i (sda ), | ||||||
| .i2c_sda_o (sda_o ), | ||||||
| .i2c_sda_en_o (sda_en_o ), | ||||||
| // External Mailbox port | ||||||
| .axi_mailbox_req_i ('0 ), | ||||||
| .axi_mailbox_resp_o ( ), | ||||||
|
|
@@ -126,6 +146,10 @@ module tb; | |||||
| assign gpio_pads[i] = dut_gpio_en_o[i] ? dut_gpio_o[i] : 1'bz; | ||||||
| end | ||||||
|
|
||||||
| // Modelling the open-drain circuit | ||||||
| assign (strong0, weak1) scl = (scl_en_o) ? scl_o : 1'b1; | ||||||
| assign (strong0, weak1) sda = (sda_en_o) ? sda_o : 1'b1; | ||||||
|
|
||||||
| // Signals to connect the sink | ||||||
| logic sim_sram_clk; | ||||||
| logic sim_sram_rst; | ||||||
|
|
@@ -260,6 +284,7 @@ module tb; | |||||
| uvm_config_db#(virtual clk_rst_if)::set(null, "*", "sys_clk_if", sys_clk_if); | ||||||
| uvm_config_db#(virtual uart_if)::set(null, "*.env.m_uart_agent*", "vif", uart_if); | ||||||
| uvm_config_db#(virtual pins_if #(NUM_GPIOS))::set(null, "*.env", "gpio_vif", gpio_pins_if); | ||||||
| uvm_config_db#(virtual i2c_if)::set(null, "*.env.m_i2c_agent", "vif", i2c_if); | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably best to add a wildcard as if a sub-component of i2c agent tries to get this handle it will fail. But it doesn't hurt us to give this possibility in case:
Suggested change
|
||||||
|
|
||||||
| // SW logger and test status interfaces. | ||||||
| uvm_config_db#(virtual sw_test_status_if)::set( | ||||||
|
|
||||||
Uh oh!
There was an error while loading. Please reload this page.