[i2c,rtl] Predict target clock stretching in HOST mode

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

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 t<sub>HD,STA</sub>, t<sub>SU,STA</sub>, t<sub>HD.DAT</sub>, t<sub>SU,DAT</sub>, t<sub>BUF</sub>, and t<sub>STO</sub>, t<sub>HIGH</sub>, and t<sub>LOW</sub> 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. t<sub>HD,STA,min</sub>, t<sub>SU,STA,min</sub>, 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 $$

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;

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;
 

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

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,17 +179,16 @@ 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);
         tHoldStop   : tcount_d = 20'(t_r_i) + 20'(t_buf_i) - 20'(tsu_sta_i);
         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