i3c_controller: Bug fixes for I2C devices and lower sclk speed

docs/library/i3c_controller/i3c_controller_core.rst

@@ -34,9 +34,20 @@ Configuration Parameters
 .. hdl-parameters::
 
    * - CLK_MOD
-     - | Clock cycles per bus bit at maximun speed (12.5MHz), set to:
-       | * 8 clock cycles at 100MHz input clock.
-       | * 4 clock cycles at 50MHz input clock.
+     - 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:
+
+       * 0: 1.5626MHz (no division).
+       * 2 390.6kHz.
+       * 4 97.6kHz.
+
 
 Signal and Interface Pins
 --------------------------------------------------------------------------------

docs/library/i3c_controller/interface.rst

@@ -418,7 +418,7 @@ sampled:
 
 .. warning::
 
-   To not sample the ``bit_mod:sdo`` signal, it will alter the SDA I/O logic and
+   Do not sample the ``bit_mod:sdo`` signal, it will alter the SDA I/O logic and
    the core won't work properly.
 
 The trigger at ``cmd_parser:cmdp_ccc_id`` allows to start the capture at the start

docs/regmap/adi_regmap_i3c_controller.txt

@@ -575,6 +575,7 @@ RW
 Indicate if the device is attached.
 ENDFIELD
 
+FIELD
 [0] 0x0
 DEV_CHAR_IS_I2C
 RW

library/i3c_controller/i3c_controller_core/i3c_controller_bit_mod.v

@@ -41,14 +41,19 @@
  * 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
+ * * I2C_MOD = 4:     97.6kHz
  */
 
 `timescale 1ns/100ps
 
 `include "i3c_controller_bit_mod.vh"
 
 module i3c_controller_bit_mod #(
-  parameter CLK_MOD = 0
+  parameter CLK_MOD = 0,
+  parameter I2C_MOD = 0
 ) (
   input reset_n,
   input clk,
@@ -87,38 +92,42 @@ module i3c_controller_bit_mod #(
   localparam [1:0] SM_SCL_HIGH = 3;
 
   localparam COUNT_INCR = CLK_MOD ? 2 : 1;
+  localparam PRESCL = I2C_MOD+5;
+  localparam PRESCALER_CLOG = $clog2(PRESCL);
 
   reg [`MOD_BIT_CMD_WIDTH:0] cmdb_r;
-  reg [1:0] pp_sg;
-  reg [5:0] count; // Worst-case: 1.56MHz, 32-bits per half-bit.
+  reg [1:0] pp_sg_reg;
+  reg [PRESCL:0] count;
   reg       sr;
   reg       i2c_mode_reg;
   reg       i2c_scl_reg;
-  reg [3:0] count_delay;
+  reg [PRESCL-2:0] count_delay;
   reg [1:0] sm;
 
   wire [`MOD_BIT_CMD_WIDTH:2] st;
   wire [1:CLK_MOD] count_high;
-  wire [3:0] scl_posedge_multi;
+  wire [PRESCL-2:0] scl_posedge_multi;
   wire       t_w;
   wire       t_w2;
   wire       sdo_w;
   wire       scl_posedge;
-  wire       sr_sda;
-  wire       sr_scl;
+  wire [1:0] sr_sda;
+  wire [1:0] p_sda;
+  wire [1:0] sr_scl;
   wire       i3c_scl_posedge;
   wire       i2c_scl;
   wire       i2c_scl_posedge;
   wire [1:0] ss;
+  wire [PRESCALER_CLOG-1:0] pp_sg;
 
   always @(posedge clk) begin
     count <= 4;
     i2c_scl_reg <= i2c_scl;
-    count_delay <= {count_delay[2:0], count[5]};
+    count_delay <= {count_delay[PRESCL-3:0], count[PRESCL]};
     if (!reset_n) begin
       sm <= SM_SETUP;
       cmdb_r <= {`MOD_BIT_CMD_NOP_, 2'b01};
-      pp_sg <= 2'b00;
+      pp_sg_reg <= 2'b00;
       sr <= 1'b0;
       i2c_mode_reg <= 1'b0;
     end else begin
@@ -152,7 +161,7 @@ module i3c_controller_bit_mod #(
         if (cmdb_valid) begin
           cmdb_r <= cmdb[4:2] != `MOD_BIT_CMD_START_ ? cmdb : {cmdb[4:2], 1'b0, cmdb[0]};
           // CMDW_MSG_RX is push-pull, but the Sr to stop from the controller side is open drain.
-          pp_sg <= cmdb[1] & cmdb[4:2] != `MOD_BIT_CMD_START_ ? scl_pp_sg : 2'b00;
+          pp_sg_reg <= cmdb[1] & cmdb[4:2] != `MOD_BIT_CMD_START_ ? scl_pp_sg : 2'b00;
           sm <= SM_SCL_LOW;
         end else begin
           cmdb_r <= {`MOD_BIT_CMD_NOP_, 2'b01};
@@ -163,7 +172,7 @@ module i3c_controller_bit_mod #(
         count <= count + COUNT_INCR;
       end
 
-      if (sm == SM_SETUP) begin
+      if (sm == SM_SETUP || st == `MOD_BIT_CMD_STOP_) begin
         sr <= 1'b0;
       end else if (cmdb_ready) begin
         sr <= 1'b1;
@@ -192,12 +201,12 @@ module i3c_controller_bit_mod #(
 
   genvar i;
   generate
-    for (i = 0; i < 4; i = i+1) begin: gen_scl
+    for (i = 0; i < PRESCL-1; i = i+1) begin: gen_scl
       assign scl_posedge_multi[i] = &count[i+2:CLK_MOD];
     end
   endgenerate
 
-  assign scl_posedge = scl_posedge_multi[3-pp_sg];
+  assign scl_posedge = scl_posedge_multi[pp_sg];
   assign count_high = count[1:CLK_MOD];
   assign cmdb_ready = (sm == SM_SETUP) ||
                       (sm == SM_STALL) ||
@@ -206,8 +215,12 @@ module i3c_controller_bit_mod #(
   assign st = cmdb_r[`MOD_BIT_CMD_WIDTH:2];
 
   // Used to generate Sr with generous timing (locked in open drain speed).
-  assign sr_sda = ((~count[4] & count[5]) | ~count[5]) & sm == SM_SCL_LOW;
-  assign sr_scl = count[5] | sm == SM_SCL_HIGH;
+  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];
 
   assign i2c_scl_posedge = i2c_scl & ~i2c_scl_reg;
@@ -219,8 +232,8 @@ module i3c_controller_bit_mod #(
   assign rx_valid = i2c_mode_reg ? i2c_scl_posedge :
                     CLK_MOD ? scl_posedge : i3c_scl_posedge;
 
-  assign sdo_w = st == `MOD_BIT_CMD_START_   ? sr_sda :
-                 st == `MOD_BIT_CMD_STOP_    ? 1'b0 :
+  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_WRITE_   ? ss[0] :
                  st == `MOD_BIT_CMD_ACK_SDR_ ?
                    (i2c_mode_reg ? 1'b1 : (sm == SM_SCL_HIGH ? rx : 1'b1)) :
@@ -238,10 +251,14 @@ module i3c_controller_bit_mod #(
   assign t_w  = st[4] ? 1'b0 : ss[1];
   assign t_w2 = ~t_w & sdo_w ? 1'b1 : 1'b0;
 
-  assign scl = st == `MOD_BIT_CMD_START_ ? (sr ? sr_scl : 1'b1) :
+  assign scl = st == `MOD_BIT_CMD_START_ ? (sr ? sr_scl[i2c_mode_reg] : 1'b1) :
                i2c_mode_reg ? (i2c_scl || sm == SM_SETUP) :
                (~(sm == SM_SCL_LOW || sm == SM_STALL));
 
   assign nop = st == `MOD_BIT_CMD_NOP_ & sm == SM_SETUP;
+  assign pp_sg = i2c_mode_reg ? PRESCL-2 :
+                 pp_sg_reg == 2'b00 ? 3 :
+                 pp_sg_reg == 2'b01 ? 2 :
+                 pp_sg_reg == 2'b10 ? 1 : 0;
 
 endmodule

library/i3c_controller/i3c_controller_core/i3c_controller_core.v

@@ -40,7 +40,8 @@
 
 module i3c_controller_core #(
   parameter MAX_DEVS = 16,
-  parameter CLK_MOD = 0
+  parameter CLK_MOD = 0,
+  parameter I2C_MOD = 0
 ) (
   input         clk,
   input         reset_n,
@@ -170,7 +171,8 @@ module i3c_controller_core #(
     .rmap_ibi_config(rmap_ibi_config));
 
   i3c_controller_bit_mod #(
-    .CLK_MOD(CLK_MOD)
+    .CLK_MOD(CLK_MOD),
+    .I2C_MOD(I2C_MOD)
   ) i_i3c_controller_bit_mod (
     .reset_n(reset_n),
     .clk(clk),

library/i3c_controller/i3c_controller_core/i3c_controller_core_hw.tcl

@@ -22,13 +22,15 @@ ad_ip_files i3c_controller_core [list \
 
 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

@@ -96,6 +96,13 @@ set_property -dict [list \
 ] \
 [ipx::get_user_parameters CLK_MOD -of_objects $cc]
 
+## I2C_MOD
+set_property -dict [list \
+  "value_validation_type" "list" \
+  "value_validation_list" "0 1 2 3 4" \
+] \
+[ipx::get_user_parameters I2C_MOD -of_objects $cc]
+
 ## Customize IP Layout
 ## Remove the automatically generated GUI page
 ipgui::remove_page -component $cc [ipgui::get_pagespec -name "Page 0" -component $cc]
@@ -121,6 +128,13 @@ set_property -dict [list \
   "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" \
+	"display_name" "I2C divider" \
+  "tooltip" "\[I2C_MOD\] Further divide (two power) open drain speed to achieve a lower I2C. Set 0 to achieve 1.56MHz at (clk=100MHz, Clock cycles per bit=8), 1 for 781kHz." \
+] [ipgui::get_guiparamspec -name "I2C_MOD" -component $cc]
+
 ## Create and save the XGUI file
 ipx::create_xgui_files $cc
 ipx::save_core $cc

library/i3c_controller/i3c_controller_core/i3c_controller_word.v

@@ -262,9 +262,13 @@ module i3c_controller_word (
                 // In I2C, the peripheral cannot stop the SM_TRANSFER.
                 cmd_r <= `MOD_BIT_CMD_WRITE_;
                 if (sdi_ready) begin
-                  cmd_wr <= 1'b0; // ACK
-                end else begin
-                  cmd_wr <= 1'b1; // NACK
+                  if (cmdw_header == `CMDW_STOP_OD) begin
+                    // Peek next command to see if the controller wishes to
+                    // end the SM_TRANSFER.
+                    cmd_wr <= 1'b1; // NACK
+                  end else begin
+                    cmd_wr <= 1'b0; // ACK
+                  end
                 end
               end else begin
                 // SDI

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} {offload 1} {max_devs 16}} {
+proc i3c_controller_create {{name "i3c_controller"} {async_clk 0} {clk_mod 1} {i2c_mod 0} {offload 1} {max_devs 16}} {
 
   create_bd_cell -type hier $name
   current_bd_instance /$name
@@ -27,6 +27,7 @@ proc i3c_controller_create {{name "i3c_controller"} {async_clk 0} {clk_mod 1} {o
   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
   if {$async_clk == 1} {

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} {offload 1} {max_devs 16}} {
+proc i3c_controller_create {{name "i3c_controller"} {async_clk 0} {clk_mod 1} {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
@@ -13,6 +13,7 @@ proc i3c_controller_create {{name "i3c_controller"} {async_clk 0} {clk_mod 1} {o
 
   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
   add_connection ${name}_host_interface.cmdp ${name}_core.cmdp