diff --git a/hw/dv/sv/i2c_agent/i2c_if.sv b/hw/dv/sv/i2c_agent/i2c_if.sv index 3983eddf455a1..f81c0a1b76cf8 100644 --- a/hw/dv/sv/i2c_agent/i2c_if.sv +++ b/hw/dv/sv/i2c_agent/i2c_if.sv @@ -128,9 +128,13 @@ interface i2c_if( join endtask: wait_for_host_stop_or_rstart + // TODO(#21887) Re-strengthen checks when detecting ACK/NACK on the bus + // Similar to S/Sr/P conditions above, these timing-referenced monitor + // routines are brittle. Remove some delays for now. + task automatic wait_for_host_ack(ref timing_cfg_t tc); `uvm_info(msg_id, "Wait for host ack::Begin", UVM_HIGH) - wait_for_dly(tc.tClockLow + tc.tSetupBit); + // wait_for_dly(tc.tClockLow + tc.tSetupBit); forever begin @(posedge scl_i); if (!sda_i) begin @@ -144,7 +148,7 @@ interface i2c_if( task automatic wait_for_host_nack(ref timing_cfg_t tc); `uvm_info(msg_id, "Wait for host nack::Begin", UVM_HIGH) - wait_for_dly(tc.tClockLow + tc.tSetupBit); + // wait_for_dly(tc.tClockLow + tc.tSetupBit); forever begin @(posedge scl_i); if (sda_i) begin diff --git a/hw/ip/i2c/doc/programmers_guide.md b/hw/ip/i2c/doc/programmers_guide.md index f88e58b67746a..51638e34c7724 100644 --- a/hw/ip/i2c/doc/programmers_guide.md +++ b/hw/ip/i2c/doc/programmers_guide.md @@ -32,11 +32,16 @@ The values of these parameters will depend primarily on three bus details: - By default the device should operate at the maximum frequency for that mode. However, If the system developer wishes to operate at slower than the mode-specific maximum, a larger than minimum period could be allowed as an additional functional parameter when calculating the timing parameters. +Additional Constraints +- To guarantee clock stretching works correctly in Controller-Mode, there is a requirement of `THIGH >= 4`. +This constraint derives from the fact that there is a latency between the Controller FSM driving the bus and observing the effect of driving the bus. +The implementation requires `THIGH` to be at least this large to guarantee that if the Target stretches the clock, we can observe it in time, and react accordingly. + Based on the inputs, the timing parameters may be chosen using the following algorithm: 1. The physical timing parameters tHD,STA, tSU,STA, tHD.DAT, tSU,DAT, tBUF, and tSTO, tHIGH, and tLOW all have minimum allowed values which depend on the choice of speed mode (Standard-mode, Fast-mode or Fast-mode Plus). Using the speed mode input, look up the appropriate minimum value (in ns) for each parameter (i.e. tHD,STA,min, tSU,STA,min, etc) 1. For each of these eight parameters, obtain an integer minimum by dividing the physical minimum parameter by the clock frequency and rounding up to the next highest integer: -$$ \textrm{THIGH_MIN}=\lceil{t\_{HIGH,min}/t\_{clk}}\rceil $$ +$$ \textrm{THIGH_MIN}=(\lceil{t\_{HIGH,min}/t\_{clk}}\rceil,4)\_{max} $$ $$ \textrm{TLOW_MIN}=\lceil{t\_{LOW,min}/t\_{clk}}\rceil $$ $$ \textrm{THD_STA_MIN}= \lceil{t\_{HD,STA,min}/t\_{clk}}\rceil $$ $$ \textrm{TSU_STA_MIN}= \lceil{t\_{SU,STA,min}/t\_{clk}}\rceil $$ diff --git a/hw/ip/i2c/dv/env/seq_lib/i2c_base_vseq.sv b/hw/ip/i2c/dv/env/seq_lib/i2c_base_vseq.sv index e8bd4643ebe9e..f96e7ce8d014a 100644 --- a/hw/ip/i2c/dv/env/seq_lib/i2c_base_vseq.sv +++ b/hw/ip/i2c/dv/env/seq_lib/i2c_base_vseq.sv @@ -177,7 +177,7 @@ class i2c_base_vseq extends cip_base_vseq #( } constraint timing_val_c { - thigh inside {[cfg.seq_cfg.i2c_min_timing : cfg.seq_cfg.i2c_max_timing]}; + thigh inside {[ 4 : cfg.seq_cfg.i2c_max_timing]}; t_r inside {[cfg.seq_cfg.i2c_min_timing : cfg.seq_cfg.i2c_max_timing]}; t_f inside {[cfg.seq_cfg.i2c_min_timing : cfg.seq_cfg.i2c_max_timing]}; thd_sta inside {[cfg.seq_cfg.i2c_min_timing : cfg.seq_cfg.i2c_max_timing]}; @@ -198,10 +198,13 @@ class i2c_base_vseq extends cip_base_vseq #( t_scl_interference == 0; } else { tsu_sta inside {[cfg.seq_cfg.i2c_min_timing : cfg.seq_cfg.i2c_max_timing]}; + // If we are generating a fixed_period SCL in the agent, we need the clock pulses + // to be at-least long enough to contain an RSTART condition to chain transfers + // together. + thigh >= tsu_sta + t_f + thd_sta; // RSTART constraint // force derived timing parameters to be positive (correct DUT config) // tlow must be at least 2 greater than the sum of t_r + tsu_dat + thd_dat // because the flopped clock (see #15003 below) reduces tClockLow by 1. - thigh == (thd_sta + tsu_sta + t_r); tlow inside {[(t_r + tsu_dat + thd_dat + 2) : (t_r + tsu_dat + thd_dat + 2) + cfg.seq_cfg.i2c_time_range]}; t_buf inside {[(tsu_sta - t_r + 1) : @@ -985,7 +988,7 @@ class i2c_base_vseq extends cip_base_vseq #( task write_tx_fifo(bit add_delay = 0); uvm_reg_data_t data; int read_size; - int rd_txfifo_timeout_ns = 100_000; + int rd_txfifo_timeout_ns = 10_000_000; // indefinite time int tx_empty_timeout_ns = 500_000_000; bit is_valid; diff --git a/hw/ip/i2c/dv/env/seq_lib/i2c_glitch_vseq.sv b/hw/ip/i2c/dv/env/seq_lib/i2c_glitch_vseq.sv index a63376393ee1a..83a7defca73d4 100644 --- a/hw/ip/i2c/dv/env/seq_lib/i2c_glitch_vseq.sv +++ b/hw/ip/i2c/dv/env/seq_lib/i2c_glitch_vseq.sv @@ -16,8 +16,6 @@ class i2c_glitch_vseq extends i2c_target_smoke_vseq; parameter uint WRITE_STATE_WAIT_TIMEOUT_CYCLES = 40; // Number of cycles sequence wait before all of the read states are executed parameter uint READ_STATE_WAIT_TIMEOUT_CYCLES = 40; - // ACQ FIFO size in bytes - import i2c_env_pkg::I2C_ACQ_FIFO_DEPTH; // Period of SCL clock depending on timing parameters uint scl_period; diff --git a/hw/ip/i2c/dv/env/seq_lib/i2c_target_smoke_vseq.sv b/hw/ip/i2c/dv/env/seq_lib/i2c_target_smoke_vseq.sv index 57585e75225eb..aac75616e5e8e 100644 --- a/hw/ip/i2c/dv/env/seq_lib/i2c_target_smoke_vseq.sv +++ b/hw/ip/i2c/dv/env/seq_lib/i2c_target_smoke_vseq.sv @@ -7,8 +7,11 @@ class i2c_target_smoke_vseq extends i2c_base_vseq; `uvm_object_utils(i2c_target_smoke_vseq) `uvm_object_new + typedef i2c_item item_q[$]; + item_q txn_stimulus[int]; + constraint timing_val_c { - thigh inside {[cfg.seq_cfg.i2c_min_timing : cfg.seq_cfg.i2c_max_timing]}; + thigh inside {[ 4 : cfg.seq_cfg.i2c_max_timing]}; t_r inside {[cfg.seq_cfg.i2c_min_timing : cfg.seq_cfg.i2c_max_timing]}; t_f inside {[cfg.seq_cfg.i2c_min_timing : cfg.seq_cfg.i2c_max_timing]}; thd_sta inside {[cfg.seq_cfg.i2c_min_timing : cfg.seq_cfg.i2c_max_timing]}; @@ -30,10 +33,13 @@ class i2c_target_smoke_vseq extends i2c_base_vseq; t_scl_interference == 0; } else { tsu_sta inside {[cfg.seq_cfg.i2c_min_timing : cfg.seq_cfg.i2c_max_timing]}; + // If we are generating a fixed_period SCL in the agent, we need the clock pulses + // to be at-least long enough to contain an RSTART condition to chain transfers + // together. + thigh >= tsu_sta + t_f + thd_sta; // RSTART constraint // force derived timing parameters to be positive (correct DUT config) // tlow must be at least 2 greater than the sum of t_r + tsu_dat + thd_dat // because the flopped clock (see #15003 below) reduces tClockLow by 1. - thigh == (thd_sta + tsu_sta + t_r); tlow inside {[(t_r + tsu_dat + thd_dat + 5) : (t_r + tsu_dat + thd_dat + 5) + cfg.seq_cfg.i2c_time_range]}; t_buf inside {[(tsu_sta - t_r + 1) : @@ -64,7 +70,6 @@ class i2c_target_smoke_vseq extends i2c_base_vseq; virtual task body(); i2c_target_base_seq m_i2c_host_seq; - i2c_item txn_q[$]; `uvm_info(`gfn, $sformatf("num_trans:%0d", num_trans), UVM_MEDIUM) // Intialize dut in device mode and agent in host mode @@ -75,6 +80,12 @@ class i2c_target_smoke_vseq extends i2c_base_vseq; fork begin + for (int i = 0; i < num_trans; i++) begin + item_q txn_q; + // Generate all the transactions up-front + create_txn(txn_q); + fetch_txn(txn_q, txn_stimulus[i]); + end for (int i = 0; i < num_trans; i++) begin get_timing_values(); if (i > 0) begin @@ -85,8 +96,7 @@ class i2c_target_smoke_vseq extends i2c_base_vseq; program_registers(); `uvm_create_obj(i2c_target_base_seq, m_i2c_host_seq) - create_txn(txn_q); - fetch_txn(txn_q, m_i2c_host_seq.req_q); + m_i2c_host_seq.req_q = txn_stimulus[i]; m_i2c_host_seq.start(p_sequencer.i2c_sequencer_h); sent_txn_cnt++; end diff --git a/hw/ip/i2c/rtl/i2c_fsm.sv b/hw/ip/i2c/rtl/i2c_fsm.sv index bcf46b5e808ab..ed774abd2374f 100644 --- a/hw/ip/i2c/rtl/i2c_fsm.sv +++ b/hw/ip/i2c/rtl/i2c_fsm.sv @@ -93,6 +93,9 @@ module i2c_fsm import i2c_pkg::*; // or clock idle by host. logic [30:0] stretch_active_cnt; // In target mode keep track of how long it has stretched for // the NACK timeout feature. + + // (IP in HOST-Mode) This bit is active when the FSM is in a state where a TARGET might + // be trying to stretch the clock, preventing the controller FSM from continuing. logic stretch_en; logic actively_stretching; // Only high when this target is holding SCL low to stretch. @@ -156,6 +159,7 @@ module i2c_fsm import i2c_pkg::*; tClockStart, tClockLow, tClockPulse, + tClockHigh, tHoldBit, tClockStop, tSetupStop, @@ -175,6 +179,7 @@ module i2c_fsm import i2c_pkg::*; tClockStart : tcount_d = 20'(thd_dat_i); tClockLow : tcount_d = 20'(tlow_i) - 20'(thd_dat_i); tClockPulse : tcount_d = 20'(t_r_i) + 20'(thigh_i); + tClockHigh : tcount_d = 20'(thigh_i); tHoldBit : tcount_d = 20'(t_f_i) + 20'(thd_dat_i); tClockStop : tcount_d = 20'(t_f_i) + 20'(tlow_i) - 20'(thd_dat_i); tSetupStop : tcount_d = 20'(t_r_i) + 20'(tsu_sto_i); @@ -182,10 +187,8 @@ module i2c_fsm import i2c_pkg::*; tNoDelay : tcount_d = 20'h00001; default : tcount_d = 20'h00001; endcase - end else if (stretch_idle_cnt == '0 || target_enable_i) begin + end else if (host_enable_i || target_enable_i) begin tcount_d = tcount_q - 1'b1; - end else begin - tcount_d = tcount_q; // pause timer if clock is stretched end end @@ -208,7 +211,8 @@ module i2c_fsm import i2c_pkg::*; always_ff @ (posedge clk_i or negedge rst_ni) begin : clk_stretch if (!rst_ni) begin stretch_idle_cnt <= '0; - end else if (stretch_en && scl_d && !scl_i) begin + end else if (stretch_en) begin + // HOST-mode count of clock stretching stretch_idle_cnt <= stretch_idle_cnt + 1'b1; end else if (!target_idle_o && event_host_timeout_o) begin // If the host has timed out, reset the counter and try again @@ -232,6 +236,34 @@ module i2c_fsm import i2c_pkg::*; end end + // The TARGET can stretch the clock during any time that the host drives SCL to 0. + // However, we (the HOST) cannot know it is being stretched until we release SCL, + // usually trying to create the next clock pulse. + // There is a minimum 3-cycle round trip (1-cycle output flop, 2-cycle input synchronizer), + // between releasing the clock and observing the effect of releasing the clock on + // the inputs. However, this is really '1 + t_r + 2' as the bus also needs to slew to '1 + // before can observe it. Even if the TARGET is not stretching the clock, we cannot + // confirm it until at-least this amount of time has elapsed. + // + // 'stretch_predict_cnt_expired' becomes active once we have observed (4 + t_r) cycles of + // delay, and if !scl_i at this point we know that the TARGET is stretching the clock. + // > This implementation requires 'thigh >= 4' to guarantee we don't miss stretching. + logic [31:0] stretch_cnt_threshold; + assign stretch_cnt_threshold = 32'd2 + 32'(t_r_i); + + logic stretch_predict_cnt_expired; + always_ff @ (posedge clk_i or negedge rst_ni) begin + if (!rst_ni) begin + stretch_predict_cnt_expired <= 1'b0; + end else begin + if (stretch_idle_cnt == stretch_cnt_threshold) begin + stretch_predict_cnt_expired <= 1'b1; + end else if (!stretch_en) begin + stretch_predict_cnt_expired <= 1'b0; + end + end + end + // Latch the nack next byte value when we receive an address to write to but // there is no space in the ACQ FIFO. The address is still ack'ed to be // compatible with the @@ -538,6 +570,11 @@ module i2c_fsm import i2c_pkg::*; scl_d = 1'b1; end end + + /////////////// + // HOST MODE // + /////////////// + // SetupStart: SDA and SCL are released SetupStart : begin host_idle_o = 1'b0; @@ -692,6 +729,11 @@ module i2c_fsm import i2c_pkg::*; else scl_d = 1'b0; fmt_fifo_rready_o = 1'b1; end + + ///////////////// + // TARGET MODE // + ///////////////// + // AcquireStart: hold start condition AcquireStart : begin target_idle_o = 1'b0; @@ -949,6 +991,11 @@ module i2c_fsm import i2c_pkg::*; end clear_nack_next_byte = 1'b1; end + + /////////////// + // HOST MODE // + /////////////// + // SetupStart: SDA and SCL are released SetupStart : begin if (tcount_q == 20'd1) begin @@ -991,7 +1038,11 @@ module i2c_fsm import i2c_pkg::*; // ClockPulse: SCL is released, SDA keeps the indexed bit value ClockPulse : begin en_sda_interf_det = 1'b1; - if (tcount_q == 20'd1) begin + if (!scl_i && stretch_predict_cnt_expired) begin + // Saw stretching. Remain in this state and don't count down until we see SCL high. + load_tcount = 1'b1; + tcount_sel = tClockHigh; + end else if (tcount_q == 20'd1) begin state_d = HoldBit; load_tcount = 1'b1; tcount_sel = tHoldBit; @@ -1024,7 +1075,11 @@ module i2c_fsm import i2c_pkg::*; end // ClockPulseAck: SCL is released ClockPulseAck : begin - if (tcount_q == 20'd1) begin + if (!scl_i && stretch_predict_cnt_expired) begin + // Saw stretching. Remain in this state and don't count down until we see SCL high. + load_tcount = 1'b1; + tcount_sel = tClockHigh; + end else if (tcount_q == 20'd1) begin state_d = HoldDevAck; load_tcount = 1'b1; tcount_sel = tHoldBit; @@ -1054,11 +1109,15 @@ module i2c_fsm import i2c_pkg::*; end // ReadClockPulse: SCL is released, the indexed bit value is read off SDA ReadClockPulse : begin - if (tcount_q == 20'd1) begin + if (!scl_i && stretch_predict_cnt_expired) begin + // Saw stretching. Remain in this state and don't count down until we see SCL high. + load_tcount = 1'b1; + tcount_sel = tClockHigh; + end else if (tcount_q == 20'd1) begin state_d = ReadHoldBit; load_tcount = 1'b1; tcount_sel = tHoldBit; - shift_data_en = 1'b1; + shift_data_en = 1'b1; // SDA is sampled on the final clk_i cycle of the SCL pulse. end end // ReadHoldBit: SCL is pulled low @@ -1089,7 +1148,11 @@ module i2c_fsm import i2c_pkg::*; // HostClockPulseAck: SCL is released HostClockPulseAck : begin en_sda_interf_det = 1'b1; - if (tcount_q == 20'd1) begin + if (!scl_i && stretch_predict_cnt_expired) begin + // Saw stretching. Remain in this state and don't count down until we see SCL high. + load_tcount = 1'b1; + tcount_sel = tClockHigh; + end else if (tcount_q == 20'd1) begin state_d = HostHoldBitAck; load_tcount = 1'b1; tcount_sel = tHoldBit; @@ -1185,6 +1248,12 @@ module i2c_fsm import i2c_pkg::*; tcount_sel = tNoDelay; end end + + + ///////////////// + // TARGET MODE // + ///////////////// + // AcquireStart: hold start condition AcquireStart : begin if (scl_i_q && !scl_i) begin