Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion hw/top_chip/dv/env/seq_lib/top_chip_dv_base_vseq.sv
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class top_chip_dv_base_vseq extends uvm_sequence;

// Class specific methods
extern function void set_handles();
extern task dut_init(string reset_kind = "HARD");
extern virtual task dut_init(string reset_kind = "HARD");
Comment thread
elliotb-lowrisc marked this conversation as resolved.
extern task apply_reset(string kind = "HARD");
extern task wait_for_sw_test_done();
// Backdoor-read or override a const symbol in SW to modify the behavior of the test.
Expand Down
55 changes: 55 additions & 0 deletions hw/top_chip/dv/env/seq_lib/top_chip_dv_i2c_host_tx_rx_vseq.sv
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// 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");
super.dut_init(reset_kind);

// Read the timing parameters through SW backdoor load
sw_symbol_backdoor_read("sys_clk_period_ns", sw_sys_clk_period_ns);
sw_symbol_backdoor_read("scl_low_time_ns", sw_scl_low_time_ns);
sw_symbol_backdoor_read("hold_data_time_ns", sw_data_hold_time_ns);

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]);

endtask
Comment thread
KinzaQamar marked this conversation as resolved.

task top_chip_dv_i2c_host_tx_rx_vseq::body();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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:
super.body();

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
84 changes: 84 additions & 0 deletions hw/top_chip/dv/env/seq_lib/top_chip_dv_i2c_tx_rx_vseq.sv
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
Comment thread
KinzaQamar marked this conversation as resolved.
2 changes: 2 additions & 0 deletions hw/top_chip/dv/env/seq_lib/top_chip_dv_vseq_list.sv
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,5 @@
`include "top_chip_dv_base_vseq.sv"
`include "top_chip_dv_uart_base_vseq.sv"
`include "top_chip_dv_gpio_smoke_vseq.sv"
`include "top_chip_dv_i2c_tx_rx_vseq.sv"
`include "top_chip_dv_i2c_host_tx_rx_vseq.sv"
3 changes: 3 additions & 0 deletions hw/top_chip/dv/env/top_chip_dv_env.core
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ filesets:
- lowrisc:dv:uart_agent
- lowrisc:dv:common_ifs
- lowrisc:mocha_dv:gpio_env
- lowrisc:dv:i2c_env
files:
- top_chip_dv_env_pkg.sv
- mem_clear_util.sv: {is_include_file: true}
Expand All @@ -24,6 +25,8 @@ filesets:
- seq_lib/top_chip_dv_base_vseq.sv: {is_include_file: true}
- seq_lib/top_chip_dv_uart_base_vseq.sv: {is_include_file: true}
- seq_lib/top_chip_dv_gpio_smoke_vseq.sv: {is_include_file: true}
- seq_lib/top_chip_dv_i2c_tx_rx_vseq.sv: {is_include_file: true}
- seq_lib/top_chip_dv_i2c_host_tx_rx_vseq.sv: {is_include_file: true}
file_type: systemVerilogSource

targets:
Expand Down
8 changes: 8 additions & 0 deletions hw/top_chip/dv/env/top_chip_dv_env.sv
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit - for consistency:

Suggested change
// 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);
// 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);
// Instantiate I2C agent
m_i2c_agent = i2c_agent::type_id::create("m_i2c_agent", this);
uvm_config_db#(i2c_agent_cfg)::set(this, "m_i2c_agent", "cfg", cfg.m_i2c_agent_cfg);

Expand All @@ -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.
Expand Down
4 changes: 4 additions & 0 deletions hw/top_chip/dv/env/top_chip_dv_env_cfg.sv
Original file line number Diff line number Diff line change
Expand Up @@ -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;
rand i2c_agent_cfg m_i2c_agent_cfg;

`uvm_object_utils_begin(top_chip_dv_env_cfg)
`uvm_object_utils_end
Expand Down Expand Up @@ -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();
Expand Down
1 change: 1 addition & 0 deletions hw/top_chip/dv/env/top_chip_dv_virtual_sequencer.sv
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class top_chip_dv_virtual_sequencer extends uvm_sequencer;
// Handles to specific interface agent sequencers. Used by some virtual
// sequences to drive RX (to-chip) items.
uart_sequencer uart_sqr;
i2c_sequencer i2c_sqr;

// FIFOs for monitor output. Used by some virtual sequences to check
// TX (from-chip) items.
Expand Down
25 changes: 25 additions & 0 deletions hw/top_chip/dv/tb/tb.sv
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 ( ),
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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
uvm_config_db#(virtual i2c_if)::set(null, "*.env.m_i2c_agent", "vif", i2c_if);
uvm_config_db#(virtual i2c_if)::set(null, "*.env.m_i2c_agent*", "vif", i2c_if);


// SW logger and test status interfaces.
uvm_config_db#(virtual sw_test_status_if)::set(
Expand Down
11 changes: 10 additions & 1 deletion hw/top_chip/dv/top_chip_sim_cfg.hjson
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,14 @@
sw_images: ["i2c_host_tx_rx_test_vanilla_sram:5" "bootrom:5"]
run_opts: ["+ChipMemSRAM_image_file={run_dir}/i2c_host_tx_rx_test_vanilla_sram.vmem",
"+ChipMemROM_image_file={run_dir}/bootrom.vmem"]
}
}
{
name: i2c_host_tx_rx_cheri
uvm_test_seq: top_chip_dv_i2c_host_tx_rx_vseq
sw_images: ["i2c_host_tx_rx_test_cheri_sram:5" "bootrom:5"]
run_opts: ["+ChipMemSRAM_image_file={run_dir}/i2c_host_tx_rx_test_cheri_sram.vmem",
"+ChipMemROM_image_file={run_dir}/bootrom.vmem"]
}
{
name: i2c_device_tx_rx
uvm_test_seq: top_chip_dv_i2c_device_tx_rx_vseq
Expand Down Expand Up @@ -345,6 +352,8 @@
"spi_device_smoke_cheri",
"spi_host_smoke",
"spi_host_smoke_cheri",
"i2c_host_tx_rx",
"i2c_host_tx_rx_cheri",
"gpio_smoke",
"gpio_smoke_cheri",
"mailbox_smoke",
Expand Down
4 changes: 2 additions & 2 deletions sw/device/lib/hal/i2c.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ uint16_t i2c_calc_scl_high_cycles(uint16_t rise_cycles, uint16_t fall_cycles,
uint16_t scl_period_cycles, uint16_t scl_low_cycles,
uint16_t scl_high_cycles_min)
{
// scl_high_time should be atleast 4 cycles to aid correct clock streching
// scl_high_time should be at least 4 cycles to aid correct clock stretching
scl_high_cycles_min = (scl_high_cycles_min < 4u) ? 4u : scl_high_cycles_min;

// An SCL period duration is divided into 4 segments:
Expand All @@ -51,7 +51,7 @@ uint16_t i2c_calc_scl_high_cycles(uint16_t rise_cycles, uint16_t fall_cycles,
// specification "UM10204" Table 10 (rev. 6) / Table 11 (rev. 7).
//
// The values for Rise and Fall times for Fast mode are taken as spec minimum. For Fast plus mode,
// the values are taken from OT's i2c_host_tx_rx_test.c test.
// the values are taken from OpenTitan's i2c_host_tx_rx_test.c test.
i2c_timing_params_t compute_minimum_timing_paramaters(i2c_speed_mode_t speed)
{
switch (speed) {
Expand Down
1 change: 1 addition & 0 deletions sw/device/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ mocha_add_test(NAME gpio_reg_access_test SOURCES gpio/reg_access_test.c LIBRARIE
# driven externally
mocha_add_test(NAME gpio_smoketest SOURCES gpio/smoketest.c LIBRARIES ${LIBS} SKIP_VERILATOR SKIP_FPGA)
mocha_add_test(NAME i2c_smoketest SOURCES i2c/smoketest.c LIBRARIES ${LIBS})
mocha_add_test(NAME i2c_host_tx_rx_test SOURCES i2c/host_test.c LIBRARIES ${LIBS} SKIP_VERILATOR SKIP_FPGA)
mocha_add_test(NAME mailbox_smoketest SOURCES mailbox/smoketest.c LIBRARIES ${LIBS})
mocha_add_test(NAME plic_smoketest SOURCES plic/smoketest.c LIBRARIES ${LIBS})
mocha_add_test(NAME pwrmgr_smoketest SOURCES pwrmgr/smoketest.c LIBRARIES ${LIBS})
Expand Down
Loading