i3c_controller: Remove CLK_MOD, set SDA low at Stop

Running the core at 50MHz, even though works, provide worst performance
and improper stall times.
The target carrier, DE10Nano, can be configured to provide a clk
near 100MHz without using a PLL.
Therefore, remove this option and require 100MHz input clock.
Also, simplify logic to hold SDA lane always to ground during the bit,
avoiding conditions where SDA rises before SCL, not yielding a proper
stop bit.

Signed-off-by: Jorge Marques <jorge.marques@analog.com>

Author: gastmaier

docs/library/i3c_controller/i3c_controller_core.rst

@@ -33,16 +33,11 @@ Configuration Parameters
 
 .. hdl-parameters::
 
-   * - CLK_MOD
-     - Clock cycles per bus bit at maximun speed (12.5MHz), set to:
-
-       * 0: 8 clock cycles at 100MHz input clock.
-       * 1: 4 clock cycles at 50MHz input clock.
    * - I2C_MOD
      - Further divide open drain speed by power of two,
        to support slow I2C devices.
 
-       For example, with input clock 100MHz and CLK_MOD=0:
+       For example, with input clock 100MHz:
 
        * 0: 1.5626MHz (no division).
        * 2 390.6kHz.

library/i3c_controller/i3c_controller_core/i3c_controller_bit_mod.v

@@ -37,10 +37,6 @@
  * SCL high time is fixed at 40ns in I3C mode:
  * * 4 clock cycles at 100MHz clk.
  * * 2 clock cycles at 50MHz clk.
- * Parameter CLK_MOD tunes the number of clock cycles to module one lane bus bit,
- * to achieve 12.5MHz at the maximum speed grade, set:
- * * CLK_MOD = 0, clk = 100MHz
- * * CLK_MOD = 1, clk = 50MHz
  * I3C Open drain is 1.5625MHz,while I2C speed depends on I2C_MOD, e.g.:
  * * I2C_MOD = 0 : 1.5626MHz
  * * I2C_MOD = 2:    390.6kHz
@@ -52,7 +48,6 @@
 `include "i3c_controller_bit_mod.vh"
 
 module i3c_controller_bit_mod #(
-  parameter CLK_MOD = 0,
   parameter I2C_MOD = 0
 ) (
   input reset_n,
@@ -91,7 +86,6 @@ module i3c_controller_bit_mod #(
   localparam [1:0] SM_SCL_LOW  = 2;
   localparam [1:0] SM_SCL_HIGH = 3;
 
-  localparam COUNT_INCR = CLK_MOD ? 2 : 1;
   localparam PRESCL = I2C_MOD+5;
   localparam PRESCALER_CLOG = $clog2(PRESCL);
 
@@ -105,14 +99,13 @@ module i3c_controller_bit_mod #(
   reg [1:0] sm;
 
   wire [`MOD_BIT_CMD_WIDTH:2] st;
-  wire [1:CLK_MOD] count_high;
+  wire [1:0] count_high;
   wire [PRESCL-2:0] scl_posedge_multi;
   wire       t_w;
   wire       t_w2;
   wire       sdo_w;
   wire       scl_posedge;
   wire [1:0] sr_sda;
-  wire [1:0] p_sda;
   wire [1:0] sr_scl;
   wire       i3c_scl_posedge;
   wire       i2c_scl;
@@ -169,7 +162,7 @@ module i3c_controller_bit_mod #(
       end
 
       if (!cmdb_ready) begin
-        count <= count + COUNT_INCR;
+        count <= count + 1;
       end
 
       if (sm == SM_SETUP || st == `MOD_BIT_CMD_STOP_) begin
@@ -180,18 +173,9 @@ module i3c_controller_bit_mod #(
     end
   end
 
-  generate if (CLK_MOD) begin
-    // Is short on one clock cycle at clk 50MHz, scl 12.5MHz,
-    // but due to the lower clk frequency, timing slack to propagate is better.
-    always @(*) begin
-      rx = sdi === 1'b0 ? 1'b0 : 1'b1;
-    end
-  end else begin
-    always @(posedge clk) begin
-      rx <= sdi === 1'b0 ? 1'b0 : 1'b1;
-    end
+  always @(posedge clk) begin
+    rx <= sdi === 1'b0 ? 1'b0 : 1'b1;
   end
-  endgenerate
 
   always @(posedge clk) begin
     // To guarantee thd_pp > 3ns.
@@ -202,12 +186,12 @@ module i3c_controller_bit_mod #(
   genvar i;
   generate
     for (i = 0; i < PRESCL-1; i = i+1) begin: gen_scl
-      assign scl_posedge_multi[i] = &count[i+2:CLK_MOD];
+      assign scl_posedge_multi[i] = &count[i+2:0];
     end
   endgenerate
 
   assign scl_posedge = scl_posedge_multi[pp_sg];
-  assign count_high = count[1:CLK_MOD];
+  assign count_high = count[1:0];
   assign cmdb_ready = (sm == SM_SETUP) ||
                       (sm == SM_STALL) ||
                       (sm == SM_SCL_HIGH & &count_high);
@@ -216,10 +200,8 @@ module i3c_controller_bit_mod #(
 
   // Used to generate Sr with generous timing (locked in open drain speed).
   assign sr_sda[0] = ((~count[4] & count[5]) | ~count[5]) & sm == SM_SCL_LOW;
-  assign p_sda [0] = ~sr_sda[0];
   assign sr_scl[0] = count[5] | sm == SM_SCL_HIGH;
   assign sr_sda[1] = ~i2c_scl;
-  assign p_sda [1] = 1'b0;
   assign sr_scl[1] = count[PRESCL] | sm == SM_SCL_HIGH;
   assign i2c_scl = count_delay[3];
 
@@ -230,10 +212,10 @@ module i3c_controller_bit_mod #(
   // * 4 clks (12.5MHz, half-bit ack, 100MHz clk)
   // * 2 clks (12.5MHz, half-bit ack, 50MHz clk)
   assign rx_valid = i2c_mode_reg ? i2c_scl_posedge :
-                    CLK_MOD ? scl_posedge : i3c_scl_posedge;
+                    i3c_scl_posedge;
 
   assign sdo_w = st == `MOD_BIT_CMD_START_   ? sr_sda[i2c_mode_reg] :
-                 st == `MOD_BIT_CMD_STOP_    ? p_sda[i2c_mode_reg]  :
+                 st == `MOD_BIT_CMD_STOP_    ? 1'b0  :
                  st == `MOD_BIT_CMD_WRITE_   ? ss[0] :
                  st == `MOD_BIT_CMD_ACK_SDR_ ?
                    (i2c_mode_reg ? 1'b1 : (sm == SM_SCL_HIGH ? rx : 1'b1)) :

library/i3c_controller/i3c_controller_core/i3c_controller_core.v

@@ -40,7 +40,6 @@
 
 module i3c_controller_core #(
   parameter MAX_DEVS = 16,
-  parameter CLK_MOD = 0,
   parameter I2C_MOD = 0
 ) (
   input         clk,
@@ -111,8 +110,7 @@ module i3c_controller_core #(
   wire i2c_mode;
 
   i3c_controller_framing #(
-    .MAX_DEVS(MAX_DEVS),
-    .CLK_MOD(CLK_MOD)
+    .MAX_DEVS(MAX_DEVS)
   ) i_i3c_controller_framing (
     .reset_n(reset_n),
     .clk(clk),
@@ -171,7 +169,6 @@ module i3c_controller_core #(
     .rmap_ibi_config(rmap_ibi_config));
 
   i3c_controller_bit_mod #(
-    .CLK_MOD(CLK_MOD),
     .I2C_MOD(I2C_MOD)
   ) i_i3c_controller_bit_mod (
     .reset_n(reset_n),

library/i3c_controller/i3c_controller_core/i3c_controller_core_hw.tcl

@@ -21,15 +21,13 @@ ad_ip_files i3c_controller_core [list \
 # parameters
 
 ad_ip_parameter MAX_DEVS STRING "MAX_DEVS"
-ad_ip_parameter CLK_MOD INTEGER 1
 ad_ip_parameter I2C_MOD INTEGER 0
 
 proc p_elaboration {} {
 
   # read parameters
 
   set max_devs [get_parameter_value "MAX_DEVS"]
-  set clk_mod [get_parameter_value "CLK_MOD"]
   set i2c_mod [get_parameter_value "I2C_MOD"]
 
   # clock and reset interface

library/i3c_controller/i3c_controller_core/i3c_controller_core_ip.tcl

@@ -89,13 +89,6 @@ set_property -dict [list \
  ] \
  [ipx::get_user_parameters MAX_DEVS -of_objects $cc]
 
-## CLK_MOD
-set_property -dict [list \
-	"value_validation_type" "pairs" \
-	"value_validation_pairs" {"8" 0 "4" 1} \
-] \
-[ipx::get_user_parameters CLK_MOD -of_objects $cc]
-
 ## I2C_MOD
 set_property -dict [list \
   "value_validation_type" "list" \
@@ -121,13 +114,6 @@ set_property -dict [list \
   "tooltip" "\[MAX_DEVS\] Maximum number of peripherals in the bus, counting the controller" \
 ] [ipgui::get_guiparamspec -name "MAX_DEVS" -component $cc]
 
-ipgui::add_param -name "CLK_MOD" -component $cc -parent $general_group
-set_property -dict [list \
-	"widget" "comboBox" \
-	"display_name" "Clock cycles per bit" \
-  "tooltip" "\[CLK_MOD\] Adjust clock cycles required to modulate the lane bus bits. Set 8 to achieve 12.5MHz at 100Mhz input clock, and 4 at 50MHz." \
-] [ipgui::get_guiparamspec -name "CLK_MOD" -component $cc]
-
 ipgui::add_param -name "I2C_MOD" -component $cc -parent $general_group
 set_property -dict [list \
 	"widget" "comboBox" \

library/i3c_controller/i3c_controller_core/i3c_controller_framing.v

@@ -64,8 +64,7 @@
 `include "i3c_controller_word.vh"
 
 module i3c_controller_framing #(
-  parameter MAX_DEVS = 16,
-  parameter CLK_MOD = 0
+  parameter MAX_DEVS = 16
 ) (
   input         clk,
   input         reset_n,
@@ -117,7 +116,7 @@ module i3c_controller_framing #(
 
   // Defines the bus available condition width (> 1us for target)
   // Setting around 0.64us for the controller.
-  localparam BUS_AVAIL = CLK_MOD ? 4 : 5;
+  localparam BUS_AVAIL = 5;
 
   localparam [2:0] SM_SETUP       = 0;
   localparam [2:0] SM_VALIDATE    = 1;

library/i3c_controller/scripts/i3c_controller_bd.tcl

@@ -3,7 +3,7 @@
 ### SPDX short identifier: ADIBSD
 ###############################################################################
 
-proc i3c_controller_create {{name "i3c_controller"} {async_clk 0} {clk_mod 1} {i2c_mod 0} {offload 1} {max_devs 16}} {
+proc i3c_controller_create {{name "i3c_controller"} {async_clk 0} {i2c_mod 0} {offload 1} {max_devs 16}} {
 
   create_bd_cell -type hier $name
   current_bd_instance /$name
@@ -26,7 +26,6 @@ proc i3c_controller_create {{name "i3c_controller"} {async_clk 0} {clk_mod 1} {i
 
   ad_ip_instance i3c_controller_core core
   ad_ip_parameter core CONFIG.MAX_DEVS $max_devs
-  ad_ip_parameter core CONFIG.CLK_MOD $clk_mod
   ad_ip_parameter core CONFIG.i2c_MOD $i2c_mod
 
   ad_connect clk host_interface/s_axi_aclk

library/i3c_controller/scripts/i3c_controller_qsys.tcl

@@ -3,7 +3,7 @@
 ### SPDX short identifier: ADIBSD
 ###############################################################################
 
-proc i3c_controller_create {{name "i3c_controller"} {async_clk 0} {clk_mod 1} {i2c_mod 0} {offload 1} {max_devs 16}} {
+proc i3c_controller_create {{name "i3c_controller"} {async_clk 0} {i2c_mod 0} {offload 1} {max_devs 16}} {
   add_instance ${name}_host_interface i3c_controller_host_interface
 
   set_instance_parameter_value ${name}_host_interface {ASYNC_CLK} $async_clk
@@ -12,7 +12,6 @@ proc i3c_controller_create {{name "i3c_controller"} {async_clk 0} {clk_mod 1} {i
   add_instance ${name}_core i3c_controller_core
 
   set_instance_parameter_value ${name}_core {MAX_DEVS} $max_devs
-  set_instance_parameter_value ${name}_core {CLK_MOD} $clk_mod
   set_instance_parameter_value ${name}_core {I2C_MOD} $i2c_mod
 
   add_connection ${name}_host_interface.sdo  ${name}_core.sdo