From 9bc31a9edaa3a7683c123e6032b2067ba2df0fa6 Mon Sep 17 00:00:00 2001 From: Jorge Marques Date: Fri, 8 Nov 2024 10:07:50 -0300 Subject: [PATCH 1/4] i3c_controller: Fix Start bit after direct Stop There is no guarantee that the device driver will use the same xfer (to yield repeated start (Sr)), for multiple concurrent transfer. Instead, it may queue multiple xfers which generate (...Stop)+(Start...). This case would cause the bit modulation to yield a [Stop+RepeatedStart], generating an "extra" clock bit. Check for the bit-mod if the current state is Stop, if so, clear the autoset Sr state reg, ensuring a [Stop+Start]. Signed-off-by: Jorge Marques --- .../i3c_controller_core/i3c_controller_bit_mod.v | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/i3c_controller/i3c_controller_core/i3c_controller_bit_mod.v b/library/i3c_controller/i3c_controller_core/i3c_controller_bit_mod.v index 9b0ba9f32a..38d080bb05 100644 --- a/library/i3c_controller/i3c_controller_core/i3c_controller_bit_mod.v +++ b/library/i3c_controller/i3c_controller_core/i3c_controller_bit_mod.v @@ -163,7 +163,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; @@ -220,7 +220,7 @@ module i3c_controller_bit_mod #( CLK_MOD ? scl_posedge : i3c_scl_posedge; assign sdo_w = st == `MOD_BIT_CMD_START_ ? sr_sda : - st == `MOD_BIT_CMD_STOP_ ? 1'b0 : + st == `MOD_BIT_CMD_STOP_ ? ~sr_sda : 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)) : From 0691edf6b1ebd5aafba282fe61127bdd215dbcab Mon Sep 17 00:00:00 2001 From: Jorge Marques Date: Fri, 8 Nov 2024 10:16:08 -0300 Subject: [PATCH 2/4] i3c_controller: Fix I2C RX Stop I2C RX transfers should stop with last ACK-bit as a NACK and then a stop, previously, the controller would ACK all bytes and then stop. Signed-off-by: Jorge Marques --- .../i3c_controller_core/i3c_controller_word.v | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/library/i3c_controller/i3c_controller_core/i3c_controller_word.v b/library/i3c_controller/i3c_controller_core/i3c_controller_word.v index 3585818153..130e69b683 100644 --- a/library/i3c_controller/i3c_controller_core/i3c_controller_word.v +++ b/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 From 2a1f5a96d55fdf13329959da388e1926065511bf Mon Sep 17 00:00:00 2001 From: Jorge Marques Date: Fri, 8 Nov 2024 16:33:55 -0300 Subject: [PATCH 3/4] i3c_controller: Add I2C_MOD parameter For backwards compatibility with slow I2C devices (100kHz, 400kHz), add a parameter to further divide the open drain speed. I3C devices still operate at 1.5MHz in open drain mode, since the I3C clock cycles are filtered by the I2C 50ns Spike filter. Signed-off-by: Jorge Marques --- .../i3c_controller/i3c_controller_core.rst | 17 +++++-- .../i3c_controller_bit_mod.v | 51 ++++++++++++------- .../i3c_controller_core/i3c_controller_core.v | 6 ++- .../i3c_controller_core_hw.tcl | 2 + .../i3c_controller_core_ip.tcl | 14 +++++ .../scripts/i3c_controller_bd.tcl | 3 +- .../scripts/i3c_controller_qsys.tcl | 3 +- 7 files changed, 72 insertions(+), 24 deletions(-) diff --git a/docs/library/i3c_controller/i3c_controller_core.rst b/docs/library/i3c_controller/i3c_controller_core.rst index e1c18cc3d6..1b5cdb9c03 100644 --- a/docs/library/i3c_controller/i3c_controller_core.rst +++ b/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 -------------------------------------------------------------------------------- diff --git a/library/i3c_controller/i3c_controller_core/i3c_controller_bit_mod.v b/library/i3c_controller/i3c_controller_core/i3c_controller_bit_mod.v index 38d080bb05..de6f215351 100644 --- a/library/i3c_controller/i3c_controller_core/i3c_controller_bit_mod.v +++ b/library/i3c_controller/i3c_controller_core/i3c_controller_bit_mod.v @@ -41,6 +41,10 @@ * 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 @@ -48,7 +52,8 @@ `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}; @@ -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_ ? ~sr_sda : + 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 diff --git a/library/i3c_controller/i3c_controller_core/i3c_controller_core.v b/library/i3c_controller/i3c_controller_core/i3c_controller_core.v index b81f606b19..6916f26e97 100644 --- a/library/i3c_controller/i3c_controller_core/i3c_controller_core.v +++ b/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), diff --git a/library/i3c_controller/i3c_controller_core/i3c_controller_core_hw.tcl b/library/i3c_controller/i3c_controller_core/i3c_controller_core_hw.tcl index 82c59689a8..0ab3092c71 100644 --- a/library/i3c_controller/i3c_controller_core/i3c_controller_core_hw.tcl +++ b/library/i3c_controller/i3c_controller_core/i3c_controller_core_hw.tcl @@ -22,6 +22,7 @@ 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 {} { @@ -29,6 +30,7 @@ proc p_elaboration {} { 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 diff --git a/library/i3c_controller/i3c_controller_core/i3c_controller_core_ip.tcl b/library/i3c_controller/i3c_controller_core/i3c_controller_core_ip.tcl index 4b7c428af5..f87e2a83a5 100644 --- a/library/i3c_controller/i3c_controller_core/i3c_controller_core_ip.tcl +++ b/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 diff --git a/library/i3c_controller/scripts/i3c_controller_bd.tcl b/library/i3c_controller/scripts/i3c_controller_bd.tcl index 49e488d96a..41cd2ed22f 100644 --- a/library/i3c_controller/scripts/i3c_controller_bd.tcl +++ b/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} { diff --git a/library/i3c_controller/scripts/i3c_controller_qsys.tcl b/library/i3c_controller/scripts/i3c_controller_qsys.tcl index f11d5e9e5c..b1eedeee21 100644 --- a/library/i3c_controller/scripts/i3c_controller_qsys.tcl +++ b/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 From 2056906bb835c4e82a038aa233f543984c23525f Mon Sep 17 00:00:00 2001 From: Jorge Marques Date: Fri, 8 Nov 2024 10:07:26 -0300 Subject: [PATCH 4/4] docs: I3C controller fixes Signed-off-by: Jorge Marques --- docs/library/i3c_controller/interface.rst | 2 +- docs/regmap/adi_regmap_i3c_controller.txt | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/library/i3c_controller/interface.rst b/docs/library/i3c_controller/interface.rst index f008b0d8cf..8e40cbdaae 100644 --- a/docs/library/i3c_controller/interface.rst +++ b/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 diff --git a/docs/regmap/adi_regmap_i3c_controller.txt b/docs/regmap/adi_regmap_i3c_controller.txt index 6746abdb29..b944ac7307 100644 --- a/docs/regmap/adi_regmap_i3c_controller.txt +++ b/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