Improve target reset pattern detection

Signed-off-by: Robert Szczepanski <[email protected]>

src/ctrl/bus_monitor.sv

@@ -29,9 +29,9 @@ module bus_monitor
     output logic start_detect_o,  // Module detected START or REPEATED START condition
     output logic stop_detect_o,   // Module detected STOP condition
 
-    input logic is_in_hdr_mode_i,       // Module is in HDR mode
+    input logic is_in_hdr_mode_i,  // Module is in HDR mode
     output logic hdr_exit_detect_o,     // Detected HDR exit condition (see: 5.2.1.1.1 of the base spec)
-    output logic target_reset_detect_o  // Deteced Target Reset condtition
+    output logic target_reset_detect_o  // Detected Target Reset condtition
 );
   logic enable, enable_q;
 
@@ -229,17 +229,17 @@ module bus_monitor
   assign stop_detect_o = stop_det;
   assign hdr_exit_detect_o = hdr_exit_det;
 
-  target_reset_detector target_reset_detector(
-    .clk_i,
-    .rst_ni,
-    .enable_i,
-    .scl_high(scl_stable_high),
-    .scl_low(scl_stable_low),
-    .scl_negedge,
-    .sda_posedge,
-    .sda_negedge,
-    .start_detected_i(start_det),
-    .stop_detected_i(stop_det),
-    .target_reset_detect_o
+  target_reset_detector target_reset_detector (
+      .clk_i,
+      .rst_ni,
+      .enable_i,
+      .scl_high(scl_stable_high),
+      .scl_low(scl_stable_low),
+      .scl_negedge,
+      .sda_posedge,
+      .sda_negedge,
+      .start_detected_i(start_det),
+      .stop_detected_i(stop_det),
+      .target_reset_detect_o
   );
 endmodule

src/ctrl/target_reset_detector.sv

@@ -11,106 +11,85 @@ module target_reset_detector
     input logic clk_i,
     input logic rst_ni,
 
-    input logic enable_i,  // Enable
+    input logic enable_i,
 
     input logic scl_low,
     input logic scl_high,
     input logic scl_negedge,
     input logic sda_posedge,
     input logic sda_negedge,
 
-    input start_detected_i, stop_detected_i,
+    input start_detected_i,
+    stop_detected_i,
 
     output logic target_reset_detect_o
 );
-  logic count_sda_transition;
+  logic count_sda_transition_en;
 
-  logic [3:0] transition_count_q;
-  logic [3:0] transition_count_d;
-  logic [1:0] suspicious_transition_count_q;
-  logic [1:0] suspicious_transition_count_d;
-  logic invalidate_sr;
+  logic [3:0] sda_transition_count_q;
+  logic [3:0] sda_transition_count_d;
 
   target_reset_detector_state_e state_q, state_d;
 
   always_comb begin
-    if (scl_high)
-      transition_count_d = 4'h0;
+    if (scl_high) sda_transition_count_d = 4'h0;
     else
-      transition_count_d =
-        count_sda_transition ? transition_count_q + 4'h1 : transition_count_q;
+      sda_transition_count_d = count_sda_transition_en ?
+                                  sda_transition_count_q + 4'h1 :
+                                  sda_transition_count_q;
 
-    if ((state_q inside {AwaitP, AwaitSr}) & (sda_posedge | sda_negedge)) begin
-      suspicious_transition_count_d = suspicious_transition_count_q + 2'h1;
-    end else if (state_q == AwaitPattern) begin
-      suspicious_transition_count_d = 2'h0;
-    end
-
-    count_sda_transition = 0;
-    if ((state_q == AwaitPattern) & (transition_count_q < 4'he))
-      count_sda_transition = (sda_posedge & (transition_count_q != 0)) | sda_negedge;
-
-    invalidate_sr = suspicious_transition_count_d > 1;
+    if ((state_q == AwaitPattern) & (sda_transition_count_q < 4'he))
+      count_sda_transition_en = (sda_posedge & (sda_transition_count_q != 0)) | sda_negedge;
+    else count_sda_transition_en = 0;
   end
 
-  always_ff @(posedge clk_i or negedge rst_ni) begin : target_reset_transition_counter
-    if (!rst_ni) begin
-      transition_count_q <= 4'h0;
-    end else if (clk_i) begin
-      transition_count_q <= transition_count_d;
-    end
-  end : target_reset_transition_counter
-
-  // If suspicious_transition_count == 1, it's either start or stop signal,
-  // but if it's more then it means something weird has happened or start/stop timing
-  // requirements were not met
-  always_ff @(posedge clk_i or negedge rst_ni) begin : suspicious_transition_counter
+  always_ff @(posedge clk_i or negedge rst_ni) begin : target_reset_sda_transition_counter
     if (!rst_ni) begin
-      suspicious_transition_count_q <= 2'h0;
-    end else if (clk_i) begin
-      if (state_d == state_q)
-        suspicious_transition_count_q <= suspicious_transition_count_d;
-      else
-        suspicious_transition_count_q <= 2'h0; // Reset on state transition
+      sda_transition_count_q <= 4'h0;
+    end else begin
+      sda_transition_count_q <= sda_transition_count_d;
     end
-  end: suspicious_transition_counter
+  end : target_reset_sda_transition_counter
 
   always_ff @(posedge clk_i or negedge rst_ni) begin : target_reset_detection_state
     if (!rst_ni) begin
       state_q <= AwaitPattern;
-    end else if (clk_i) begin
+    end else begin
       state_q <= state_d;
     end
   end : target_reset_detection_state
 
   always_comb begin : target_reset_detection_fsm
     case (state_q)
       AwaitPattern: begin
-        state_d = (transition_count_q == 4'he) ? AwaitSr : AwaitPattern;
+        state_d = (sda_transition_count_q == 4'he) ? AwaitSr : AwaitPattern;
       end
       AwaitSr: begin
         state_d = AwaitSr;
-        if (start_detected_i & scl_high) begin
-          state_d = AwaitP;
-        end else if (scl_low | suspicious_transition_count_q > 1) begin
+        // If SDA posedge is detected, it means that it toggled from LOW to HIGH before fulfilling
+        // the START condition hold timing, otherwise `start_detected_i` would be asserted
+        if (scl_low | sda_posedge) begin
           state_d = AwaitPattern;
+        end else if (start_detected_i) begin
+          state_d = AwaitP;
         end
       end
       AwaitP: begin
         state_d = AwaitP;
-        if (stop_detected_i & scl_high) begin
+        // If SDA negedge is detected, it means that it toggled from HIGH to LOW before fulfilling
+        // the STOP condition hold timing, otherwise `stop_detected_i` would be asserted
+        if (scl_low | sda_negedge) begin
+          state_d = AwaitPattern;
+        end else if (stop_detected_i) begin
           state_d = ResetDetected;
-        end else begin
-          if (invalidate_sr)
-            state_d = AwaitPattern;
         end
       end
       ResetDetected: begin
         state_d = AwaitPattern;
       end
       default: begin
         state_d = AwaitPattern;
-      end // Unreachable
+      end
     endcase
   end : target_reset_detection_fsm
 

verification/cocotb/block/ctrl_bus_monitor/test_bus_monitor.py

@@ -34,6 +34,7 @@ async def count_high_cycles(clk, sig, e_terminate):
         await RisingEdge(clk)
     return num_det
 
+
 def create_default_controller(dut: SimHandleBase) -> I3cController:
     return I3cController(
         sda_i=None,
@@ -162,11 +163,9 @@ async def test_target_reset_detection(dut: SimHandleBase):
     cocotb.log.info("Performing basic target reset test with no configuration")
 
     e_terminate = cocotb.triggers.Event()
-    t_detect_target_reset = cocotb.start_soon(count_high_cycles(
-        dut.clk_i,
-        dut.target_reset_detect_o,
-        e_terminate
-    ))
+    t_detect_target_reset = cocotb.start_soon(
+        count_high_cycles(dut.clk_i, dut.target_reset_detect_o, e_terminate)
+    )
     await i3c_controller.target_reset()
 
     await ClockCycles(dut.clk_i, 32)

violations.waiver

@@ -1,5 +1,6 @@
 waive --rule=line-length --location="src/ctrl/i2c_controller_fsm.sv"
 waive --rule=line-length --location="src/ctrl/i2c_target_fsm.sv"
+waive --rule=line-length --location="src/ctrl/bus_monitor.sv"
 waive --rule=line-length --location="src/i2c_phy_integration.sv"
 waive --rule=line-length --location="verification/block/i2c_phy_integration/i2c_phy_integration_wrapper.sv"
 waive --rule=line-length --location="src/csr/I3CCSR_covergroups.svh"