From ca22726f00f43fba1600376f41f00b449f23444d Mon Sep 17 00:00:00 2001 From: Michal Czyz Date: Thu, 10 Oct 2024 19:34:09 +0200 Subject: [PATCH] Fixes to recovery read Signed-off-by: Michal Czyz --- Makefile | 33 ++-- i3c_core_configs.yaml | 2 +- src/ctrl/bus_monitor.sv | 18 +- src/ctrl/ccc.sv | 12 +- src/ctrl/controller_standby.sv | 2 +- src/ctrl/controller_standby_i2c.sv | 4 +- src/ctrl/edge_detector.sv | 2 +- src/ctrl/i3c_target_fsm.sv | 124 +++++++------- src/ctrl/stable_high_detector.sv | 10 +- src/ctrl/width_converter_8toN.sv | 2 +- src/ctrl/width_converter_Nto8.sv | 2 +- src/hci/axi_adapter.sv | 8 + src/hci/hci.sv | 26 +-- src/i3c.sv | 32 ++-- src/i3c_defines.svh | 8 +- src/i3c_wrapper.sv | 4 +- src/phy/bufs.sv | 3 +- src/recovery/recovery_executor.sv | 156 ++++++++---------- src/recovery/recovery_handler.sv | 62 +++---- src/recovery/recovery_pec.sv | 4 +- src/recovery/recovery_transmitter.sv | 111 ++++++------- .../cocotb/block/hci_queues_ahb/Makefile | 1 + .../cocotb/block/hci_queues_axi/Makefile | 1 + .../cocotb/top/lib_i3c_top/test_recovery.py | 27 ++- violations.waiver | 1 + 25 files changed, 319 insertions(+), 336 deletions(-) diff --git a/Makefile b/Makefile index b51b24b..9ce05e5 100644 --- a/Makefile +++ b/Makefile @@ -51,15 +51,16 @@ CFG_GEN = $(TOOL_DIR)/i3c_config/i3c_core_config.py config: config-rtl config-rdl ## Generate RDL and RTL configuration files +PYTHON ?= python3 config-rtl: config-print ## Generate top I3C definitions svh file - python $(CFG_GEN) $(CFG_NAME) $(CFG_FILE) svh_file --output-file $(SRC_DIR)/i3c_defines.svh + $(PYTHON) $(CFG_GEN) $(CFG_NAME) $(CFG_FILE) svh_file --output-file $(SRC_DIR)/i3c_defines.svh RDL_REGS := $(SRC_DIR)/rdl/registers.rdl RDL_GEN_DIR := $(SRC_DIR)/csr/ -RDL_ARGS := $(shell python $(CFG_GEN) $(CFG_NAME) $(CFG_FILE) reg_gen_opts) +RDL_ARGS := $(shell $(PYTHON) $(CFG_GEN) $(CFG_NAME) $(CFG_FILE) reg_gen_opts) config-rdl: config-print - python $(TOOL_DIR)/reg_gen/reg_gen.py --input-file=$(RDL_REGS) --output-dir=$(RDL_GEN_DIR) $(RDL_ARGS) $(EXTRA_REG_GEN_ARGS) + $(PYTHON) $(TOOL_DIR)/reg_gen/reg_gen.py --input-file=$(RDL_REGS) --output-dir=$(RDL_GEN_DIR) $(RDL_ARGS) $(EXTRA_REG_GEN_ARGS) config-print: ## Print configuration name, filename and RDL arguments @echo Using \'$(CFG_NAME)\' I3C configuration from \'$(CFG_FILE)\'. @@ -71,57 +72,57 @@ config-print: ## Print configuration name, filename and RDL arguments lint: lint-rtl lint-tests ## Run RTL and tests lint lint-check: lint-rtl ## Run RTL lint and check lint on tests source code without fixing errors - cd $(COCOTB_VERIF_DIR) && python -m nox -R -s test_lint + cd $(COCOTB_VERIF_DIR) && $(PYTHON) -m nox -R -s test_lint lint-rtl: ## Run lint on RTL source code $(SHELL) $(TOOL_DIR)/verible-scripts/run.sh lint-tests: ## Run lint on tests source code - cd $(COCOTB_VERIF_DIR) && python -m nox -R -s lint + cd $(COCOTB_VERIF_DIR) && $(PYTHON) -m nox -R -s lint # # Tests # test: config ## Run single module test (use `TEST=` flag) - cd $(COCOTB_VERIF_DIR) && python -m nox -R -s $(TEST)_verify + cd $(COCOTB_VERIF_DIR) && $(PYTHON) -m nox -R -s $(TEST)_verify tests-axi: ## Run all verification/cocotb/* RTL tests for AXI bus configuration without coverage $(MAKE) config CFG_NAME=axi - cd $(COCOTB_VERIF_DIR) && python -m nox -R -t "axi" + cd $(COCOTB_VERIF_DIR) && $(PYTHON) -m nox -R -t "axi" tests-ahb: ## Run all verification/cocotb/* RTL tests for AHB bus configuration without coverage $(MAKE) config CFG_NAME=ahb - cd $(COCOTB_VERIF_DIR) && python -m nox -R -t "ahb" + cd $(COCOTB_VERIF_DIR) && $(PYTHON) -m nox -R -t "ahb" tests: tests-axi tests-ahb ## Run all verification/cocotb/* RTL tests fro AHB and AXI bus configurations without coverage # TODO: Enable full coverage flow tests-coverage: ## Run all verification/block/* RTL tests with coverage - cd $(COCOTB_VERIF_DIR) && BLOCK_COVERAGE_ENABLE=1 python -m nox -R -k "verify" + cd $(COCOTB_VERIF_DIR) && BLOCK_COVERAGE_ENABLE=1 $(PYTHON) -m nox -R -k "verify" test-i3c-vip-uvm: config ## Run single I3C VIP UVM test with nox (use 'TEST=' flag) - cd $(UVM_VERIF_DIR) && python -m nox -R -s $(TEST) + cd $(UVM_VERIF_DIR) && $(PYTHON) -m nox -R -s $(TEST) tests-i3c-vip-uvm: config ## Run all I3C VIP UVM tests with nox - cd $(UVM_VERIF_DIR) && python -m nox -R -s "i3c_verify_uvm" + cd $(UVM_VERIF_DIR) && $(PYTHON) -m nox -R -s "i3c_verify_uvm" tests-i3c-vip-uvm-debug: config ## Run debugging I3C VIP UVM tests with nox - cd $(UVM_VERIF_DIR) && python -m nox -R -t "uvm_debug_tests" + cd $(UVM_VERIF_DIR) && $(PYTHON) -m nox -R -t "uvm_debug_tests" tests-uvm: config ## Run all I3C Core UVM tests with nox - cd $(UVM_VERIF_DIR) && python -m nox -R -s "i3c_core_verify_uvm" + cd $(UVM_VERIF_DIR) && $(PYTHON) -m nox -R -s "i3c_core_verify_uvm" tests-uvm-debug: config ## Run debugging I3C Core UVM tests with nox - cd $(UVM_VERIF_DIR) && python -m nox -R -s "i3c_core_uvm_debug_tests" + cd $(UVM_VERIF_DIR) && $(PYTHON) -m nox -R -s "i3c_core_uvm_debug_tests" tests-tool: ## Run all tool tests - cd $(TOOL_VERIF_DIR) && python -m nox -k "verify" + cd $(TOOL_VERIF_DIR) && $(PYTHON) -m nox -k "verify" # # Utilities # timings: ## Generate values for I2C/I3C timings - python $(TOOL_DIR)/timing/timing.py + $(PYTHON) $(TOOL_DIR)/timing/timing.py deps: ## Install python dependencies pip install -r $(I3C_ROOT_DIR)/requirements.txt diff --git a/i3c_core_configs.yaml b/i3c_core_configs.yaml index 4f18a21..44be96b 100644 --- a/i3c_core_configs.yaml +++ b/i3c_core_configs.yaml @@ -30,4 +30,4 @@ axi: FrontendBusAddrWidth: 12 FrontendBusDataWidth: 32 FrontendBusUserWidth: 32 - FrontendBusIdWidth: 2 + FrontendBusIdWidth: 8 diff --git a/src/ctrl/bus_monitor.sv b/src/ctrl/bus_monitor.sv index edf80da..a4a350f 100644 --- a/src/ctrl/bus_monitor.sv +++ b/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 is_in_hdr_mode_i, // Module is in HDR mode - output hdr_exit_detect_o // Detected HDR exit condition (see: 5.2.1.1.1 of the base spec) - ); + input is_in_hdr_mode_i, // Module is in HDR mode + output hdr_exit_detect_o // Detected HDR exit condition (see: 5.2.1.1.1 of the base spec) +); logic enable, enable_q; logic scl_negedge_i; @@ -65,8 +65,9 @@ module bus_monitor assign enable = enable_i; - edge_detector #(.DETECT_NEGEDGE(1'b1)) - edge_detector_scl_negedge ( + edge_detector #( + .DETECT_NEGEDGE(1'b1) + ) edge_detector_scl_negedge ( .clk_i(clk_i), .rst_ni(rst_ni), .trigger(scl_negedge_i), @@ -84,8 +85,9 @@ module bus_monitor .detect(scl_posedge) ); - edge_detector #(.DETECT_NEGEDGE(1'b1)) - edge_detector_sda_negedge ( + edge_detector #( + .DETECT_NEGEDGE(1'b1) + ) edge_detector_sda_negedge ( .clk_i(clk_i), .rst_ni(rst_ni), .trigger(sda_negedge_i), @@ -204,7 +206,7 @@ module bus_monitor end else if (hdr_exit_det_trigger) begin hdr_exit_det_pending <= 1'b1; end else if (!enable || stop_det) begin - hdr_exit_det_count <= 5'b10000; + hdr_exit_det_count <= 5'b10000; hdr_exit_det_pending <= 1'b0; end else if (enable && hdr_exit_det_pending && sda_negedge) begin hdr_exit_det_count <= {1'b0, hdr_exit_det_count[4:1]}; diff --git a/src/ctrl/ccc.sv b/src/ctrl/ccc.sv index def056c..3a4f98b 100644 --- a/src/ctrl/ccc.sv +++ b/src/ctrl/ccc.sv @@ -37,13 +37,14 @@ module ccc logic [7:0] defining_byte; logic defining_byte_valid; logic [7:0] command_data; + logic clear_command_code; always_ff @(posedge clk_i or negedge rst_ni) begin : proc_latch_inputs if (~rst_ni) begin - command_code <= '0; + command_code <= '0; defining_byte <= '0; defining_byte_valid <= '0; - command_data <= '0; + command_data <= '0; end else begin if (clear_command_code) begin command_code <= '0; @@ -52,7 +53,7 @@ module ccc end defining_byte <= defining_byte_valid_i ? defining_byte_i : defining_byte; defining_byte_valid <= defining_byte_valid_i; - command_data <= command_data_valid_i ? command_data_i : command_data; + command_data <= command_data_valid_i ? command_data_i : command_data; end end @@ -70,12 +71,11 @@ module ccc // Decode CCC logic is_direct_cmd = command_code[7]; // 0 - BCast, 1 - Direct - // CCC handling logic - logic clear_command_code; + always_comb begin response_valid_o = '0; - response_byte_o = '0; + response_byte_o = '0; enter_hdr_ccc_o = '0; rst_action_o = '0; command_min_bytes_o = '0; diff --git a/src/ctrl/controller_standby.sv b/src/ctrl/controller_standby.sv index 8feb7bb..1b0f04a 100644 --- a/src/ctrl/controller_standby.sv +++ b/src/ctrl/controller_standby.sv @@ -200,7 +200,7 @@ module controller_standby .rx_queue_wvalid_o(i2c_rx_queue_wvalid_o), .rx_queue_wready_i(rx_queue_wready_i), .rx_queue_wdata_o(i2c_rx_queue_wdata_o), -// .rx_queue_wflush_o(i2c_rx_queue_wflush_o), // TODO: Add flush support for I2C + // .rx_queue_wflush_o(i2c_rx_queue_wflush_o), // TODO: Add flush support for I2C .tx_queue_full_i(tx_queue_full_i), .tx_queue_start_thld_i(tx_queue_start_thld_i), .tx_queue_start_thld_trig_i(tx_queue_start_thld_trig_i), diff --git a/src/ctrl/controller_standby_i2c.sv b/src/ctrl/controller_standby_i2c.sv index f343af1..d902970 100644 --- a/src/ctrl/controller_standby_i2c.sv +++ b/src/ctrl/controller_standby_i2c.sv @@ -191,8 +191,8 @@ module controller_standby_i2c // TODO: Make the I2C FSM report start/stop condition detection assign bus_start_o = '0; - assign bus_rstart_o= '0; - assign bus_stop_o = '0; + assign bus_rstart_o = '0; + assign bus_stop_o = '0; // TODO: Make the I2C FSM output its received address + RnW bit and connect // them here. diff --git a/src/ctrl/edge_detector.sv b/src/ctrl/edge_detector.sv index caa541d..65c6851 100644 --- a/src/ctrl/edge_detector.sv +++ b/src/ctrl/edge_detector.sv @@ -38,7 +38,7 @@ module edge_detector end else if (trigger) begin check_in_progress <= 1'b1; count <= '0; - end else if(check_in_progress && detect_line) begin + end else if (check_in_progress && detect_line) begin count <= count + 1'b1; if (count >= delay_count) begin check_in_progress <= 1'b0; diff --git a/src/ctrl/i3c_target_fsm.sv b/src/ctrl/i3c_target_fsm.sv index 6aa0911..0a70d11 100644 --- a/src/ctrl/i3c_target_fsm.sv +++ b/src/ctrl/i3c_target_fsm.sv @@ -72,7 +72,7 @@ module i3c_target_fsm input logic tx_fifo_rvalid_i, // indicates there is valid data in tx_fifo output logic tx_fifo_rready_o, // pop entry from tx_fifo input logic [TxFifoWidth-1:0] tx_fifo_rdata_i, // byte in tx_fifo to be sent to host - output logic tx_host_nack_o, // a NACK has been received during transmission + output logic tx_host_nack_o, // NACK has been received during transmission // RX FIFO used for Target Write output logic rx_fifo_wvalid_o, // high if there is valid data in rx_fifo @@ -137,36 +137,36 @@ module i3c_target_fsm logic restart_det_d, restart_det_q; assign restart_det_q = '0; // TODO: Handle - logic xact_for_us_q; // Target was addressed in this transaction - logic xact_for_us_d; // - We only record Stop if the Target was addressed. - logic xfer_for_us_q; // Target was addressed in this transfer - logic xfer_for_us_d; // - event_cmd_complete_o is only for our transfers + logic xact_for_us_q; // Target was addressed in this transaction + logic xact_for_us_d; // - We only record Stop if the Target was addressed. + logic xfer_for_us_q; // Target was addressed in this transfer + logic xfer_for_us_d; // - event_cmd_complete_o is only for our transfers - logic input_strobe; + logic input_strobe; logic [7:0] input_byte; // register for reads from host - logic input_byte_clr; // clear input_byte contents + logic input_byte_clr; // clear input_byte contents // logic nack_timeout; - logic expect_stop; + logic expect_stop; // Target bit counter variables logic [3:0] bit_idx; // bit index including ack/nack - logic bit_ack; // indicates ACK bit been sent or received - logic rw_bit; // indicates host wants to read (1) or write (0) - logic host_ack; // indicates host acknowledged transmitted byte - logic host_tbit_ok; // indicates that T bit matches the expected (odd) parity + logic bit_ack; // indicates ACK bit been sent or received + logic rw_bit; // indicates host wants to read (1) or write (0) + logic host_ack; // indicates host acknowledged transmitted byte + logic host_tbit_ok; // indicates that T bit matches the expected (odd) parity - logic [7:0] command_code; // CCC byte - logic command_code_valid; + logic [7:0] command_code; // CCC byte + logic command_code_valid; - logic [7:0] defining_byte; // optional defining byte of the CCC code - logic defining_byte_valid; + logic [7:0] defining_byte; // optional defining byte of the CCC code + logic defining_byte_valid; - logic enter_hdr_ccc; - logic enter_hdr_after_stop; - logic enter_hdr_after_stop_clr; + logic enter_hdr_ccc; + logic enter_hdr_after_stop; + logic enter_hdr_after_stop_clr; - logic [1:0] command_min_bytes; // minimum number of bytes expected after a CCC read/write - logic [1:0] command_max_bytes; // maximum number of bytes expected after a CCC read/write + logic [1:0] command_min_bytes; // minimum number of bytes expected after a CCC read/write + logic [1:0] command_max_bytes; // maximum number of bytes expected after a CCC read/write logic tbit_after_byte_q; // whether to expect a T bit after acquiring data byte (we're doing i3c // flow) or old-style ACK (we're doing i2c) flow. Whether the flow is @@ -176,9 +176,6 @@ module i3c_target_fsm // by a later address read logic tbit_after_byte_d; - // TODO: Set transfer type based on the discovered state - assign transfer_type_o = 0; - // IBI logic ibi_handling; // Asserted when an IBI is transmitter logic ibi_payload; // Asserted when data from IBI queue is transmitter @@ -192,31 +189,31 @@ module i3c_target_fsm tcount_sel_e tcount_sel; - ccc ccc( - .clk_i(clk_i), - .rst_ni(rst_ni), + ccc ccc ( + .clk_i (clk_i), + .rst_ni(rst_ni), - // Latch CCC data - .command_code_i(command_code), - .command_code_valid_i(command_code_valid), + // Latch CCC data + .command_code_i(command_code), + .command_code_valid_i(command_code_valid), - .defining_byte_i(defining_byte), - .defining_byte_valid_i(defining_byte_valid), + .defining_byte_i(defining_byte), + .defining_byte_valid_i(defining_byte_valid), - //TODO: Establish correct size - .command_data_i(0), - .command_data_valid_i(0), + //TODO: Establish correct size + .command_data_i(0), + .command_data_valid_i(0), - .queue_size_reg_i(0), - .response_byte_o(), - .response_valid_o(), + .queue_size_reg_i(0), + .response_byte_o (), + .response_valid_o(), - .enter_hdr_ccc_o(enter_hdr_ccc), + .enter_hdr_ccc_o(enter_hdr_ccc), - .rst_action_o(rst_action_o), + .rst_action_o(rst_action_o), - .command_min_bytes_o(command_min_bytes), - .command_max_bytes_o(command_max_bytes) + .command_min_bytes_o(command_min_bytes), + .command_max_bytes_o(command_max_bytes) ); always_comb begin : counter_functions @@ -364,6 +361,9 @@ module i3c_target_fsm if (!rst_ni) bus_rnw <= '0; else if (input_strobe & (bit_idx == 4'd7)) bus_rnw <= input_byte[0]; + // FIXME: Add CCC type transfer + assign transfer_type_o = {1'b0, bus_rnw}; + // An artificial acq_fifo_wready is used here to ensure we always have // space to asborb a stop / repeat start format byte. Without guaranteeing // space for this entry, the target module would need to stretch the @@ -410,10 +410,10 @@ module i3c_target_fsm AcquireAckPulse = 'h52, AcquireAckHold = 'h53, // If in AddrRead we read I3C Reserved Byte, we go to ACK here - RsvdByteAckWait = 'h60, + RsvdByteAckWait = 'h60, RsvdByteAckSetup = 'h61, RsvdByteAckPulse = 'h62, - RsvdByteAckHold = 'h63, + RsvdByteAckHold = 'h63, // Do we get SR or CCC next? PostAckTBitSymbolDetect = 'h70, PostAckTBitSymbolDetect2 = 'h71, @@ -662,6 +662,7 @@ module i3c_target_fsm target_idle_o = 1'b0; target_transmitting_o = 1'b1; sda_d = sda_r; + // sda_d = 1'b0; end TransmitSetup: begin target_idle_o = 1'b0; @@ -685,10 +686,8 @@ module i3c_target_fsm target_transmitting_o = 1'b1; sda_d = sda_r; if (!scl_i) begin - if (ibi_handling) - ibi_fifo_rready_o = 1'b1; - else - tx_fifo_rready_o = 1'b1; + if (ibi_handling) ibi_fifo_rready_o = 1'b1; + else tx_fifo_rready_o = 1'b1; end end TbitSetup: begin @@ -862,17 +861,20 @@ module i3c_target_fsm always_ff @(posedge clk_i or negedge rst_ni) if (!rst_ni) ibi_payload <= '0; - else unique case(state_q) - Idle: ibi_payload <= '0; - TransmitWait: ibi_payload <= ibi_handling; - TransmitWaitOd: ibi_payload <= ibi_handling; - TbitWait: ibi_payload <= ibi_handling; - default: ibi_payload <= ibi_payload; - endcase + else + unique case (state_q) + Idle: ibi_payload <= '0; + TransmitWait: ibi_payload <= ibi_handling; + TransmitWaitOd: ibi_payload <= ibi_handling; + TbitWait: ibi_payload <= ibi_handling; + default: ibi_payload <= ibi_payload; + endcase always_ff @(posedge clk_i or negedge rst_ni) if (!rst_ni) sda_r <= 1'b1; - else if (state_q == IbiAddrSetup || state_q == TransmitSetup) + // || state_q == TransmitWait || state_q == TransmitWaitOd + // + else if (state_q == IbiAddrSetup || state_q == TransmitSetup || state_q == AddrAckHold) sda_r <= output_byte[3'(7-bit_idx)]; else if (state_q == TbitSetup) sda_r <= (ibi_handling & ibi_fifo_rvalid_i) | (~ibi_handling & tx_fifo_rvalid_i); @@ -922,7 +924,7 @@ module i3c_target_fsm if (is_any_addr_match) begin load_tcount = 1'b1; // Wait for hold time to avoid interfering with the controller. - tcount_sel = tHoldData; + tcount_sel = tHoldData; // TODO: Check that dynamic address really takes precedence // over static address in determining communication flow (I2C/I3C) // @@ -994,7 +996,9 @@ module i3c_target_fsm end // AddrAckHold: target pulls SDA low while SCL is pulled low AddrAckHold: begin - if (tcount_q == 20'd1) begin + // FIXME: WIP: Needed to increase this timing to respect setup of next push pull + // if (tcount_q == 20'd1) begin + if (tcount_q == 20'd5) begin if (nack_transaction_q) begin // If the Target is set to NACK already, release SDA and wait // for a Stop. This isn't an ideal response for SMBus reads, since @@ -1085,7 +1089,7 @@ module i3c_target_fsm end load_tcount = 1'b1; - tcount_sel = tSetupData; + tcount_sel = tSetupData; end end @@ -1154,7 +1158,7 @@ module i3c_target_fsm AcquireByte: begin if (bit_ack) begin load_tcount = 1'b1; - tcount_sel = tHoldData; + tcount_sel = tHoldData; if (tbit_after_byte_q) begin state_d = AcquireTBitWait; defining_byte = input_byte; diff --git a/src/ctrl/stable_high_detector.sv b/src/ctrl/stable_high_detector.sv index d176951..c353361 100644 --- a/src/ctrl/stable_high_detector.sv +++ b/src/ctrl/stable_high_detector.sv @@ -5,11 +5,11 @@ module stable_high_detector #( parameter int CNTR_W = 20 ) ( - input logic clk_i, - input logic rst_ni, - input logic line_i, - input logic [CNTR_W-1:0] delay_count_i, - output logic stable_o + input logic clk_i, + input logic rst_ni, + input logic line_i, + input logic [CNTR_W-1:0] delay_count_i, + output logic stable_o ); logic [CNTR_W-1:0] count; logic do_count; diff --git a/src/ctrl/width_converter_8toN.sv b/src/ctrl/width_converter_8toN.sv index 422d884..231fcb7 100644 --- a/src/ctrl/width_converter_8toN.sv +++ b/src/ctrl/width_converter_8toN.sv @@ -46,7 +46,7 @@ module width_converter_8toN #( end // Valid / ready - assign sink_ready_o = (bcnt != Bytes); + assign sink_ready_o = (bcnt != Bytes); assign source_valid_o = (bcnt == Bytes); // Data register diff --git a/src/ctrl/width_converter_Nto8.sv b/src/ctrl/width_converter_Nto8.sv index beced7d..502eb3b 100644 --- a/src/ctrl/width_converter_Nto8.sv +++ b/src/ctrl/width_converter_Nto8.sv @@ -42,7 +42,7 @@ module width_converter_Nto8 #( end // Valid / ready - assign sink_ready_o = (bcnt == '0); + assign sink_ready_o = (bcnt == '0); assign source_valid_o = (bcnt != '0); // Data register diff --git a/src/hci/axi_adapter.sv b/src/hci/axi_adapter.sv index a24a7c5..442dcf0 100644 --- a/src/hci/axi_adapter.sv +++ b/src/hci/axi_adapter.sv @@ -94,6 +94,10 @@ module axi_adapter #( axi.arid = arid_i; axi.araddr = araddr_i; axi.arsize = arsize_i; + axi.arlen = arlen_i; + axi.arburst = arburst_i; + axi.aruser = aruser_i; + axi.arlock = arlock_i; rvalid_o = axi.rvalid; axi.rready = rready_i; @@ -110,6 +114,10 @@ module axi_adapter #( axi.awid = awid_i; axi.awaddr = awaddr_i; axi.awsize = awsize_i; + axi.awlen = awlen_i; + axi.awburst = awburst_i; + axi.awuser = awuser_i; + axi.awlock = awlock_i; axi.wvalid = wvalid_i; wready_o = axi.wready; diff --git a/src/hci/hci.sv b/src/hci/hci.sv index 8c2f393..24aac8a 100644 --- a/src/hci/hci.sv +++ b/src/hci/hci.sv @@ -171,12 +171,15 @@ module hci I3CCSR_pkg::I3CCSR__DCT__out_t dct_o; I3CCSR_pkg::I3CCSR__DCT__in_t dct_i; + // TTI CSR interface assign hwif_tti_o = hwif_out.I3C_EC.TTI; assign hwif_in.I3C_EC.TTI = hwif_tti_i; // Recovery CSR interface assign hwif_rec_o = hwif_out.I3C_EC.SecFwRecoveryIf; + + // TODO: Use this if assign hwif_in.I3C_EC.SecFwRecoveryIf = hwif_rec_i; // Reset control @@ -303,29 +306,6 @@ module hci end : wire_hwif - // TTI - logic tti_rx_desc_queue_rd_ack; - logic [TtiRxDataWidth-1:0] tti_rx_desc_queue_rd_data; - logic tti_tx_desc_queue_wr_ack; - logic tti_rx_queue_rd_ack; - logic [TtiRxDataWidth-1:0] tti_rx_queue_rd_data; - logic tti_tx_queue_wr_ack; - logic tti_ibi_queue_wr_ack; - - always_comb begin : wire_hwif_tti - hwif_in.I3C_EC.TTI.RX_DESC_QUEUE_PORT.rd_ack = tti_rx_desc_queue_rd_ack; - hwif_in.I3C_EC.TTI.RX_DESC_QUEUE_PORT.rd_data = tti_rx_desc_queue_rd_data; - - hwif_in.I3C_EC.TTI.RX_DATA_PORT.rd_ack = tti_rx_queue_rd_ack; - hwif_in.I3C_EC.TTI.RX_DATA_PORT.rd_data = tti_rx_queue_rd_data; - - hwif_in.I3C_EC.TTI.TX_DESC_QUEUE_PORT.wr_ack = tti_tx_desc_queue_wr_ack; - - hwif_in.I3C_EC.TTI.TX_DATA_PORT.wr_ack = tti_tx_queue_wr_ack; - - hwif_in.I3C_EC.TTI.IBI_PORT.wr_ack = tti_ibi_queue_wr_ack; - end : wire_hwif_tti - always_comb begin : wire_hwif_ccc hwif_in.I3C_EC.StdbyCtrlMode.STBY_CR_CCC_CONFIG_RSTACT_PARAMS.RST_ACTION.next = rst_action_i; end : wire_hwif_ccc diff --git a/src/i3c.sv b/src/i3c.sv index 963fa8b..45b836e 100644 --- a/src/i3c.sv +++ b/src/i3c.sv @@ -143,11 +143,11 @@ module i3c input logic awvalid_i, output logic awready_o, - input logic [AxiDataWidth-1:0] wdata_i, - input logic [ 7:0] wstrb_i, - input logic wlast_i, - input logic wvalid_i, - output logic wready_o, + input logic [ AxiDataWidth-1:0] wdata_i, + input logic [AxiDataWidth/8-1:0] wstrb_i, + input logic wlast_i, + input logic wvalid_i, + output logic wready_o, output logic [ 1:0] bresp_o, output logic [AxiIdWidth-1:0] bid_o, @@ -166,7 +166,7 @@ module i3c input logic i3c_sda_i, // serial data input from i3c bus output logic i3c_sda_o, // serial data output to i3c bus - output logic sel_od_pp_o, // 0 - Open Drain, 1 - Push Pull + output logic sel_od_pp_o, // 0 - Open Drain, 1 - Push Pull // DAT memory export interface input dat_mem_src_t dat_mem_src_i, @@ -524,7 +524,7 @@ module i3c // TTI: TX Descriptor .tti_tx_desc_queue_full_i(tti_tx_desc_queue_full), .tti_tx_desc_queue_ready_thld_i(tti_tx_desc_queue_ready_thld), - .tti_tx_desc_queue_ready_thld_trig_i(tti_tx_desc_queue_ready_thld), + .tti_tx_desc_queue_ready_thld_trig_i(tti_tx_desc_queue_ready_thld_trig), .tti_tx_desc_queue_empty_i(tti_tx_desc_queue_empty), .tti_tx_desc_queue_rvalid_i(tti_tx_desc_queue_rvalid), .tti_tx_desc_queue_rready_o(tti_tx_desc_queue_rready), @@ -552,11 +552,11 @@ module i3c .ibi_queue_rdata_i(tti_ibi_queue_rdata), // I2C/I3C bus condition detection - .bus_start_o(bus_start), + .bus_start_o (bus_start), .bus_rstart_o(bus_rstart), - .bus_stop_o(bus_stop), + .bus_stop_o (bus_stop), - // I2C/I3C received address (with RnW# bit) for the recovery handler + // I2C/I3C received address (with RnW# bit) for the recovery handler .bus_addr_o(rx_bus_addr), .bus_addr_valid_o(rx_bus_addr_valid), @@ -575,7 +575,7 @@ module i3c // TODO: TTI interface //TODO: Rename - .i3c_fsm_en_i(i3c_fsm_en_i), + .i3c_fsm_en_i (i3c_fsm_en_i), .i3c_fsm_idle_o(i3c_fsm_idle_o), .err(), // TODO: Handle errors @@ -662,10 +662,10 @@ module i3c .dct_wdata_hw_i(dct_wdata_hw), .dct_rdata_hw_o(dct_rdata_hw), - .dat_mem_src_i(dat_mem_src_i), + .dat_mem_src_i (dat_mem_src_i), .dat_mem_sink_o(dat_mem_sink_o), - .dct_mem_src_i(dct_mem_src_i), + .dct_mem_src_i (dct_mem_src_i), .dct_mem_sink_o(dct_mem_sink_o), .hwif_tti_o(hwif_tti_out), @@ -960,11 +960,11 @@ module i3c .ctl_tti_ibi_queue_ready_thld_o(tti_ibi_queue_ready_thld), .ctl_tti_ibi_queue_ready_thld_trig_o(tti_ibi_queue_ready_thld_trig), - .irq_o(), // TODO: Connect me + .irq_o(), // TODO: Connect me // I2C/I3C bus condition detection - .ctl_bus_start_i(bus_start | bus_rstart), // S/Sr are both used to reset PEC - .ctl_bus_stop_i(bus_stop), + .ctl_bus_start_i(bus_start | bus_rstart), // S/Sr are both used to reset PEC + .ctl_bus_stop_i (bus_stop), // Received I2C/I3C address along with RnW# bit .ctl_bus_addr_i(rx_bus_addr), diff --git a/src/i3c_defines.svh b/src/i3c_defines.svh index b5330d3..a56a19a 100644 --- a/src/i3c_defines.svh +++ b/src/i3c_defines.svh @@ -13,8 +13,10 @@ `define IBI_FIFO_EXT_SIZE 0 `define DAT_DEPTH 128 `define DCT_DEPTH 128 -`define I3C_USE_AHB 1 -`define AHB_ADDR_WIDTH 12 -`define AHB_DATA_WIDTH 64 +`define I3C_USE_AXI 1 +`define AXI_ADDR_WIDTH 12 +`define AXI_DATA_WIDTH 32 +`define AXI_USER_WIDTH 32 +`define AXI_ID_WIDTH 8 `endif // I3C_CONFIG diff --git a/src/i3c_wrapper.sv b/src/i3c_wrapper.sv index 1391931..a9af4b2 100644 --- a/src/i3c_wrapper.sv +++ b/src/i3c_wrapper.sv @@ -102,8 +102,8 @@ module i3c_wrapper #( output logic sel_od_pp_o `else // I3C bus IO - inout wire i3c_scl_io, - inout wire i3c_sda_io + inout wire i3c_scl_io, + inout wire i3c_sda_io `endif // TODO: Add interrupts diff --git a/src/phy/bufs.sv b/src/phy/bufs.sv index a3185ca..c5e4c30 100644 --- a/src/phy/bufs.sv +++ b/src/phy/bufs.sv @@ -21,7 +21,8 @@ module bufs ( assign phy_data_i_z = ~phy_data_i; - logic buf_pp_o, buf_od_o; + logic buf_pp_o; + wire buf_od_o; // Model of a Push-Pull driver buf_pp xbuf_pp ( diff --git a/src/recovery/recovery_executor.sv b/src/recovery/recovery_executor.sv index 73992f3..5361fd8 100644 --- a/src/recovery/recovery_executor.sv +++ b/src/recovery/recovery_executor.sv @@ -6,6 +6,7 @@ TTI data queues. FIXME: Check if cmd_len_i is valid w.r.t. cmd_cmd_i + FIXME: Rework latches and defaults of cases */ module recovery_executor import i3c_pkg::*; @@ -27,10 +28,10 @@ module recovery_executor output logic [15:0] res_len_o, // Response data interface - output logic res_dvalid_o, - input logic res_dready_i, - output logic [ 7:0] res_data_o, - output logic res_dlast_o, + output logic res_dvalid_o, + input logic res_dready_i, + output logic [7:0] res_data_o, + output logic res_dlast_o, // TTI RX FIFO data interface output logic rx_req_o, @@ -40,7 +41,7 @@ module recovery_executor output logic rx_queue_sel_o, output logic rx_queue_clr_o, - input logic host_abort_i, + input logic host_abort_i, // Recovery CSR interface input I3CCSR_pkg::I3CCSR__I3C_EC__SecFwRecoveryIf__out_t hwif_rec_i, @@ -148,34 +149,26 @@ module recovery_executor CsrWrite: begin state_d = CsrWrite; - if (rx_ack_i & (dcnt == 1)) - state_d = Done; + if (rx_ack_i & (dcnt == 1)) state_d = Done; end CsrRead: begin state_d = CsrReadLen; - if (res_ready_i) - state_d = CsrReadLen; + if (res_ready_i) state_d = CsrReadLen; end - CsrReadLen: - state_d = CsrReadData; + CsrReadLen: state_d = CsrReadData; CsrReadData: begin state_d = CsrReadData; - if (host_abort_i) - state_d = Error; // FIXME: Should we make this an error ? - else if (res_dvalid_o & res_dready_i & (dcnt == 0)) - state_d = Done; + if (host_abort_i) state_d = Error; // FIXME: Should we make this an error ? + else if (res_dvalid_o & res_dready_i & (dcnt == 0)) state_d = Done; end - Error: - state_d = Done; - Done: - state_d = Idle; + Error: state_d = Done; + Done: state_d = Idle; - default: - state_d = Idle; + default: state_d = Idle; endcase // .................................................... @@ -184,27 +177,19 @@ module recovery_executor always_ff @(posedge clk_i) unique case (state_q) Idle: - if (cmd_valid_i) - dcnt <= (|cmd_len_i[1:0]) ? (cmd_len_i / 4 + 1) : (cmd_len_i / 4); // Round up - CsrWrite: - if (rx_ack_i) - dcnt <= dcnt - 1; - CsrReadLen: - dcnt <= csr_length; - CsrReadData: - if (res_dvalid_o & res_dready_i) - dcnt <= dcnt - 1; + if (cmd_valid_i) + dcnt <= (|cmd_len_i[1:0]) ? (cmd_len_i / 4 + 1) : (cmd_len_i / 4); // Round up + CsrWrite: if (rx_ack_i) dcnt <= dcnt - 1; + CsrReadLen: dcnt <= csr_length; + CsrReadData: if (res_dvalid_o & res_dready_i) dcnt <= dcnt - 1; default: dcnt <= dcnt; endcase // Byte counter always_ff @(posedge clk_i) unique case (state_q) - Idle: - bcnt <= '0; - CsrReadData: - if (res_dvalid_o & res_dready_i) - bcnt <= bcnt + 1; + Idle: bcnt <= '0; + CsrReadData: if (res_dvalid_o & res_dready_i) bcnt <= bcnt + 1; default: bcnt <= bcnt; endcase @@ -231,7 +216,7 @@ module recovery_executor // a malicious packet with length > CSR length is received CsrWrite: if (rx_ack_i) csr_sel <= csr_e'(csr_sel + 1); CsrReadData: if (res_dvalid_o & res_dready_i & (bcnt == 3)) csr_sel <= csr_e'(csr_sel + 1); - default: csr_sel <= csr_sel; + default: csr_sel <= csr_sel; endcase // CSR writeable flag @@ -250,47 +235,50 @@ module recovery_executor Idle: if (cmd_valid_i) unique case (cmd_cmd_i) - CMD_PROT_CAP: csr_length <= 'd15; - CMD_DEVICE_ID: csr_length <= 'd24; - CMD_DEVICE_STATUS: csr_length <= 'd8; - CMD_DEVICE_RESET: csr_length <= 'd3; - CMD_RECOVERY_CTRL: csr_length <= 'd3; - CMD_RECOVERY_STATUS: csr_length <= 'd2; - CMD_HW_STATUS: csr_length <= 'd4; - CMD_INDIRECT_FIFO_CTRL: csr_length <= 'd6; - CMD_INDIRECT_FIFO_STATUS: csr_length <= 'd20; + CMD_PROT_CAP: csr_length = 'd15; + CMD_DEVICE_ID: csr_length = 'd24; + CMD_DEVICE_STATUS: csr_length = 'd8; + CMD_DEVICE_RESET: csr_length = 'd3; + CMD_RECOVERY_CTRL: csr_length = 'd3; + CMD_RECOVERY_STATUS: csr_length = 'd2; + CMD_HW_STATUS: csr_length = 'd4; + CMD_INDIRECT_FIFO_CTRL: csr_length = 'd6; + CMD_INDIRECT_FIFO_STATUS: csr_length = 'd20; + default: csr_length = '0; endcase endcase // CSR read data mux - always_ff @(posedge clk_i) unique case(csr_sel) - CSR_PROT_CAP_0: csr_data <= hwif_rec_i.PROT_CAP_0.PLACEHOLDER.value; - CSR_PROT_CAP_1: csr_data <= hwif_rec_i.PROT_CAP_1.PLACEHOLDER.value; - CSR_PROT_CAP_2: csr_data <= hwif_rec_i.PROT_CAP_2.PLACEHOLDER.value; - CSR_PROT_CAP_3: csr_data <= hwif_rec_i.PROT_CAP_3.PLACEHOLDER.value; - CSR_DEVICE_ID_0: csr_data <= hwif_rec_i.DEVICE_ID_0.PLACEHOLDER.value; - CSR_DEVICE_ID_1: csr_data <= hwif_rec_i.DEVICE_ID_1.PLACEHOLDER.value; - CSR_DEVICE_ID_2: csr_data <= hwif_rec_i.DEVICE_ID_2.PLACEHOLDER.value; - CSR_DEVICE_ID_3: csr_data <= hwif_rec_i.DEVICE_ID_3.PLACEHOLDER.value; - CSR_DEVICE_ID_4: csr_data <= hwif_rec_i.DEVICE_ID_4.PLACEHOLDER.value; - CSR_DEVICE_ID_5: csr_data <= hwif_rec_i.DEVICE_ID_5.PLACEHOLDER.value; - CSR_DEVICE_ID_6: csr_data <= hwif_rec_i.DEVICE_ID_6.PLACEHOLDER.value; - CSR_DEVICE_STATUS_0: csr_data <= hwif_rec_i.DEVICE_STATUS_0.PLACEHOLDER.value; - CSR_DEVICE_STATUS_1: csr_data <= hwif_rec_i.DEVICE_STATUS_1.PLACEHOLDER.value; - CSR_DEVICE_RESET: csr_data <= hwif_rec_i.DEVICE_RESET.PLACEHOLDER.value; - CSR_RECOVERY_CTRL: csr_data <= hwif_rec_i.RECOVERY_CTRL.PLACEHOLDER.value; - CSR_RECOVERY_STATUS: csr_data <= hwif_rec_i.RECOVERY_STATUS.PLACEHOLDER.value; - CSR_HW_STATUS: csr_data <= hwif_rec_i.HW_STATUS.PLACEHOLDER.value; - - CSR_INDIRECT_FIFO_CTRL_0: csr_data <= hwif_rec_i.INDIRECT_FIFO_CTRL_0.PLACEHOLDER.value; - CSR_INDIRECT_FIFO_CTRL_1: csr_data <= hwif_rec_i.INDIRECT_FIFO_CTRL_1.PLACEHOLDER.value; - CSR_INDIRECT_FIFO_STATUS_0: csr_data <= hwif_rec_i.INDIRECT_FIFO_STATUS_0.PLACEHOLDER.value; - CSR_INDIRECT_FIFO_STATUS_1: csr_data <= hwif_rec_i.INDIRECT_FIFO_STATUS_1.PLACEHOLDER.value; - CSR_INDIRECT_FIFO_STATUS_2: csr_data <= hwif_rec_i.INDIRECT_FIFO_STATUS_2.PLACEHOLDER.value; - CSR_INDIRECT_FIFO_STATUS_3: csr_data <= hwif_rec_i.INDIRECT_FIFO_STATUS_3.PLACEHOLDER.value; - CSR_INDIRECT_FIFO_STATUS_4: csr_data <= hwif_rec_i.INDIRECT_FIFO_STATUS_4.PLACEHOLDER.value; - CSR_INDIRECT_FIFO_STATUS_5: csr_data <= hwif_rec_i.INDIRECT_FIFO_STATUS_5.PLACEHOLDER.value; - endcase + always_ff @(posedge clk_i) + unique case (csr_sel) + CSR_PROT_CAP_0: csr_data <= hwif_rec_i.PROT_CAP_0.PLACEHOLDER.value; + CSR_PROT_CAP_1: csr_data <= hwif_rec_i.PROT_CAP_1.PLACEHOLDER.value; + CSR_PROT_CAP_2: csr_data <= hwif_rec_i.PROT_CAP_2.PLACEHOLDER.value; + CSR_PROT_CAP_3: csr_data <= hwif_rec_i.PROT_CAP_3.PLACEHOLDER.value; + CSR_DEVICE_ID_0: csr_data <= hwif_rec_i.DEVICE_ID_0.PLACEHOLDER.value; + CSR_DEVICE_ID_1: csr_data <= hwif_rec_i.DEVICE_ID_1.PLACEHOLDER.value; + CSR_DEVICE_ID_2: csr_data <= hwif_rec_i.DEVICE_ID_2.PLACEHOLDER.value; + CSR_DEVICE_ID_3: csr_data <= hwif_rec_i.DEVICE_ID_3.PLACEHOLDER.value; + CSR_DEVICE_ID_4: csr_data <= hwif_rec_i.DEVICE_ID_4.PLACEHOLDER.value; + CSR_DEVICE_ID_5: csr_data <= hwif_rec_i.DEVICE_ID_5.PLACEHOLDER.value; + CSR_DEVICE_ID_6: csr_data <= hwif_rec_i.DEVICE_ID_6.PLACEHOLDER.value; + CSR_DEVICE_STATUS_0: csr_data <= hwif_rec_i.DEVICE_STATUS_0.PLACEHOLDER.value; + CSR_DEVICE_STATUS_1: csr_data <= hwif_rec_i.DEVICE_STATUS_1.PLACEHOLDER.value; + CSR_DEVICE_RESET: csr_data <= hwif_rec_i.DEVICE_RESET.PLACEHOLDER.value; + CSR_RECOVERY_CTRL: csr_data <= hwif_rec_i.RECOVERY_CTRL.PLACEHOLDER.value; + CSR_RECOVERY_STATUS: csr_data <= hwif_rec_i.RECOVERY_STATUS.PLACEHOLDER.value; + CSR_HW_STATUS: csr_data <= hwif_rec_i.HW_STATUS.PLACEHOLDER.value; + + CSR_INDIRECT_FIFO_CTRL_0: csr_data <= hwif_rec_i.INDIRECT_FIFO_CTRL_0.PLACEHOLDER.value; + CSR_INDIRECT_FIFO_CTRL_1: csr_data <= hwif_rec_i.INDIRECT_FIFO_CTRL_1.PLACEHOLDER.value; + CSR_INDIRECT_FIFO_STATUS_0: csr_data <= hwif_rec_i.INDIRECT_FIFO_STATUS_0.PLACEHOLDER.value; + CSR_INDIRECT_FIFO_STATUS_1: csr_data <= hwif_rec_i.INDIRECT_FIFO_STATUS_1.PLACEHOLDER.value; + CSR_INDIRECT_FIFO_STATUS_2: csr_data <= hwif_rec_i.INDIRECT_FIFO_STATUS_2.PLACEHOLDER.value; + CSR_INDIRECT_FIFO_STATUS_3: csr_data <= hwif_rec_i.INDIRECT_FIFO_STATUS_3.PLACEHOLDER.value; + CSR_INDIRECT_FIFO_STATUS_4: csr_data <= hwif_rec_i.INDIRECT_FIFO_STATUS_4.PLACEHOLDER.value; + CSR_INDIRECT_FIFO_STATUS_5: csr_data <= hwif_rec_i.INDIRECT_FIFO_STATUS_5.PLACEHOLDER.value; + default: csr_data <= '0; + endcase // .................................................... @@ -342,8 +330,8 @@ module recovery_executor // CSR write. Only applicable for writable CSRs as per the OCP // recovery spec. always_comb begin - hwif_rec_o.DEVICE_RESET.PLACEHOLDER.we = rx_ack_i & (csr_sel == CSR_DEVICE_RESET); - hwif_rec_o.RECOVERY_CTRL.PLACEHOLDER.we = rx_ack_i & (csr_sel == CSR_RECOVERY_CTRL); + hwif_rec_o.DEVICE_RESET.PLACEHOLDER.we = rx_ack_i & (csr_sel == CSR_DEVICE_RESET); + hwif_rec_o.RECOVERY_CTRL.PLACEHOLDER.we = rx_ack_i & (csr_sel == CSR_RECOVERY_CTRL); hwif_rec_o.INDIRECT_FIFO_CTRL_0.PLACEHOLDER.we = rx_ack_i & (csr_sel == CSR_INDIRECT_FIFO_CTRL_0); hwif_rec_o.INDIRECT_FIFO_CTRL_1.PLACEHOLDER.we = rx_ack_i & (csr_sel == CSR_INDIRECT_FIFO_CTRL_1); end @@ -365,18 +353,14 @@ module recovery_executor // Transmitt valid always_ff @(posedge clk_i) unique case (state_q) - CsrReadLen: - if (res_ready_i) - res_valid_o <= 1'd1; + CsrReadLen: if (res_ready_i) res_valid_o <= 1'd1; default: res_valid_o <= '0; endcase // Transmitt length always_ff @(posedge clk_i) unique case (state_q) - CsrReadLen: - if (res_ready_i) - res_len_o <= csr_length; + CsrReadLen: if (res_ready_i) res_len_o <= csr_length; endcase // Transmitt data valid @@ -385,10 +369,10 @@ module recovery_executor // Transmitt data always_comb unique case (bcnt) - 'd0: res_data_o <= csr_data[ 7: 0]; - 'd1: res_data_o <= csr_data[15: 8]; - 'd2: res_data_o <= csr_data[23:16]; - 'd3: res_data_o <= csr_data[31:24]; + 'd0: res_data_o = csr_data[7:0]; + 'd1: res_data_o = csr_data[15:8]; + 'd2: res_data_o = csr_data[23:16]; + 'd3: res_data_o = csr_data[31:24]; endcase // Transmitt data last diff --git a/src/recovery/recovery_handler.sv b/src/recovery/recovery_handler.sv index 0eedf00..098c5a3 100644 --- a/src/recovery/recovery_handler.sv +++ b/src/recovery/recovery_handler.sv @@ -691,21 +691,21 @@ module recovery_handler // .................................................... - logic cmd_valid; - logic cmd_is_rd; - logic [ 7:0] cmd_cmd; + logic cmd_valid; + logic cmd_is_rd; + logic [7:0] cmd_cmd; logic [15:0] cmd_len; - logic cmd_error; + logic cmd_error; logic cmd_done; // TODO: Consider replacing this with rx_cmd_ready and making the executor not ready rather than pulsing done. // RX PEC calculator - logic rx_pec_clear; - logic rx_pec_valid; - logic rx_pec_init; - logic [ 7:0] rx_pec_data; + logic rx_pec_clear; + logic rx_pec_valid; + logic rx_pec_init; + logic [7:0] rx_pec_data; - logic [ 7:0] recv_pec_crc; - logic recv_pec_enable; + logic [7:0] recv_pec_crc; + logic recv_pec_enable; recovery_pec xrecovery_rx_pec ( .clk_i, @@ -719,9 +719,9 @@ module recovery_handler // RX PEC mux for initializing it with I2C/I3C address byte always_comb begin - rx_pec_data = ctl_bus_addr_valid_i ? ctl_bus_addr_i : tti_rx_data_queue_wdata; - rx_pec_valid = ctl_bus_addr_valid_i ? 1'b1 : recv_pec_enable; - rx_pec_init = ctl_bus_addr_valid_i ? 1'b1 : 1'b0; + rx_pec_data = ctl_bus_addr_valid_i ? ctl_bus_addr_i : tti_rx_data_queue_wdata; + rx_pec_valid = ctl_bus_addr_valid_i ? 1'b1 : recv_pec_enable; + rx_pec_init = ctl_bus_addr_valid_i ? 1'b1 : 1'b0; end // Clear PEC on start @@ -789,9 +789,9 @@ module recovery_handler // TX PEC mux for initializing it with I2C/I3C address byte always_comb begin - tx_pec_data = ctl_bus_addr_valid_i ? ctl_bus_addr_i : ctl_tti_tx_data_queue_rdata_o; - tx_pec_valid = ctl_bus_addr_valid_i ? 1'b1 : xmit_pec_enable; - tx_pec_init = ctl_bus_addr_valid_i ? 1'b1 : 1'b0; + tx_pec_data = ctl_bus_addr_valid_i ? ctl_bus_addr_i : ctl_tti_tx_data_queue_rdata_o; + tx_pec_valid = ctl_bus_addr_valid_i ? 1'b1 : xmit_pec_enable; + tx_pec_init = ctl_bus_addr_valid_i ? 1'b1 : 1'b0; end // Clear PEC on start @@ -818,14 +818,14 @@ module recovery_handler .pec_crc_i (xmit_pec_crc), .pec_enable_o(xmit_pec_enable), - .res_valid_i (res_valid), - .res_ready_o (res_ready), - .res_len_i (res_len), + .res_valid_i(res_valid), + .res_ready_o(res_ready), + .res_len_i (res_len), - .res_dvalid_i (res_dvalid), - .res_dready_o (res_dready), - .res_data_i (res_data), - .res_dlast_i (res_dlast) + .res_dvalid_i(res_dvalid), + .res_dready_o(res_dready), + .res_data_i (res_data), + .res_dlast_i (res_dlast) ); // .................................................... @@ -842,14 +842,14 @@ module recovery_handler .cmd_error_i(cmd_error), .cmd_done_o (cmd_done), - .res_valid_o (res_valid), - .res_ready_i (res_ready), - .res_len_o (res_len), + .res_valid_o(res_valid), + .res_ready_i(res_ready), + .res_len_o (res_len), - .res_dvalid_o (res_dvalid), - .res_dready_i (res_dready), - .res_data_o (res_data), - .res_dlast_o (res_dlast), + .res_dvalid_o(res_dvalid), + .res_dready_i(res_dready), + .res_data_o (res_data), + .res_dlast_o (res_dlast), .rx_req_o (exec_tti_rx_data_req), .rx_ack_i (exec_tti_rx_data_ack), @@ -857,7 +857,7 @@ module recovery_handler .rx_queue_sel_o(exec_tti_rx_queue_sel), .rx_queue_clr_o(exec_tti_rx_queue_clr), - .host_abort_i (ctl_tti_tx_host_nack_i | ctl_bus_stop_i), + .host_abort_i(ctl_tti_tx_host_nack_i | ctl_bus_stop_i), .hwif_rec_i(hwif_rec_i), .hwif_rec_o(hwif_rec_o) diff --git a/src/recovery/recovery_pec.sv b/src/recovery/recovery_pec.sv index f533a3b..f269672 100644 --- a/src/recovery/recovery_pec.sv +++ b/src/recovery/recovery_pec.sv @@ -11,8 +11,8 @@ module recovery_pec ( input logic clk_i, input logic rst_ni, - input logic valid_i, // Data valid - input logic init_i, // When 1 assume that previous CRC was 0 + input logic valid_i, // Data valid + input logic init_i, // When 1 assume that previous CRC was 0 input logic [7:0] dat_i, output logic [7:0] crc_o ); diff --git a/src/recovery/recovery_transmitter.sv b/src/recovery/recovery_transmitter.sv index 6777a11..e4b78c2 100644 --- a/src/recovery/recovery_transmitter.sv +++ b/src/recovery/recovery_transmitter.sv @@ -1,10 +1,10 @@ module recovery_transmitter import i3c_pkg::*; -# ( +#( parameter int unsigned TtiTxDescDataWidth = 32 ) ( - input logic clk_i, // Clock - input logic rst_ni, // Reset (active low) + input logic clk_i, // Clock + input logic rst_ni, // Reset (active low) // TTI TX descriptor output logic desc_valid_o, @@ -17,9 +17,9 @@ module recovery_transmitter output logic [7:0] data_data_o, // TTX TX control - output logic data_queue_select_o, - output logic start_trig_o, - input logic host_abort_i, + output logic data_queue_select_o, + output logic start_trig_o, + input logic host_abort_i, // PEC computation control input logic [7:0] pec_crc_i, @@ -31,15 +31,15 @@ module recovery_transmitter input logic [15:0] res_len_i, // Response data interface - input logic res_dvalid_i, - output logic res_dready_o, - input logic [ 7:0] res_data_i, - input logic res_dlast_i + input logic res_dvalid_i, + output logic res_dready_o, + input logic [7:0] res_data_i, + input logic res_dlast_i ); // TODO - assign data_queue_select_o = 1'b1; - assign start_trig_o = 1'b0; + assign data_queue_select_o = 1'b1; + assign start_trig_o = 1'b0; // Internal signals logic [15:0] len_q; @@ -48,12 +48,12 @@ module recovery_transmitter // FSM typedef enum logic [7:0] { - Idle = 'h00, - TxLenL = 'h10, - TxLenH = 'h11, - TxData = 'h20, - TxPEC = 'h30, - Flush = 'hF0 + Idle = 'h00, + TxLenL = 'h10, + TxLenH = 'h11, + TxData = 'h20, + TxPEC = 'h30, + Flush = 'hF0 } state_e; state_e state_d, state_q; @@ -68,94 +68,81 @@ module recovery_transmitter unique case (state_q) Idle: begin state_d = Idle; - if (res_valid_i) - state_d = TxLenL; + if (res_valid_i) state_d = TxLenL; end TxLenL: begin state_d = TxLenL; - if(host_abort_i) - state_d = Idle; - else if(data_ready_i) - state_d = TxLenH; + if (host_abort_i) state_d = Idle; + else if (data_ready_i) state_d = TxLenH; end TxLenH: begin state_d = TxLenH; - if(host_abort_i) - state_d = Idle; - else if(data_ready_i) - state_d = TxData; + if (host_abort_i) state_d = Idle; + else if (data_ready_i) state_d = TxData; end TxData: begin state_d = TxData; - if(host_abort_i) - state_d = Flush; - else if(data_ready_i && data_valid_o) - if(res_dlast_i) - state_d = TxPEC; + if (host_abort_i) state_d = Flush; + else if (data_ready_i && data_valid_o) if (res_dlast_i) state_d = TxPEC; end TxPEC: begin state_d = TxPEC; - if(data_ready_i && data_valid_o) - state_d = Idle; + if (data_ready_i && data_valid_o) state_d = Idle; end Flush: begin state_d = Flush; - if(!res_dvalid_i) - state_d = Idle; + if (!res_dvalid_i) state_d = Idle; end - default: - state_d = Idle; + default: state_d = Idle; endcase // .................................................... // Latch length - always_ff @(posedge clk_i) - if (state_q == Idle & res_ready_o & res_valid_i) - len_q <= res_len_i; + always_ff @(posedge clk_i) if (state_q == Idle & res_ready_o & res_valid_i) len_q <= res_len_i; // Response ready assign res_ready_o = (state_q == Idle); // Response data ready //sign res_dready_o = (state_q == TxData) & data_ready_i; always_comb - unique case(state_q) - TxData: res_dready_o = data_ready_i; - Flush: res_dready_o = 1'b1; - default: res_dready_o = 1'b0; + unique case (state_q) + TxData: res_dready_o = data_ready_i; + Flush: res_dready_o = 1'b1; + default: res_dready_o = 1'b0; endcase // Data output always_comb - unique case(state_q) - TxLenL: data_data_o = len_q[ 7:0]; - TxLenH: data_data_o = len_q[15:8]; - TxPEC: data_data_o = pec_crc_i; - default: data_data_o = res_data_i; + unique case (state_q) + TxLenL: data_data_o = len_q[7:0]; + TxLenH: data_data_o = len_q[15:8]; + TxPEC: data_data_o = pec_crc_i; + default: data_data_o = res_data_i; endcase always_comb - unique case(state_q) - TxLenL: data_valid_o = 1'b1; - TxLenH: data_valid_o = 1'b1; - TxData: data_valid_o = res_dvalid_i; - TxPEC: data_valid_o = 1'b1; - default: data_valid_o = 1'b0; + unique case (state_q) + TxLenL: data_valid_o = 1'b1; + TxLenH: data_valid_o = 1'b1; + TxData: data_valid_o = res_dvalid_i; + TxPEC: data_valid_o = 1'b1; + default: data_valid_o = 1'b0; endcase // PEC enable always_comb - unique case(state_q) - TxLenL: pec_enable_o = data_valid_o & data_ready_i; - TxLenH: pec_enable_o = data_valid_o & data_ready_i; - TxData: pec_enable_o = data_valid_o & data_ready_i; - default: pec_enable_o = 1'b0; + unique case (state_q) + TxLenL: pec_enable_o = data_valid_o & data_ready_i; + TxLenH: pec_enable_o = data_valid_o & data_ready_i; + TxData: pec_enable_o = data_valid_o & data_ready_i; + default: pec_enable_o = 1'b0; endcase endmodule diff --git a/verification/cocotb/block/hci_queues_ahb/Makefile b/verification/cocotb/block/hci_queues_ahb/Makefile index 2862ac2..056f624 100644 --- a/verification/cocotb/block/hci_queues_ahb/Makefile +++ b/verification/cocotb/block/hci_queues_ahb/Makefile @@ -33,6 +33,7 @@ VERILOG_SOURCES = \ $(SRC_DIR)/ctrl/width_converter_Nto8.sv \ $(SRC_DIR)/recovery/recovery_pec.sv \ $(SRC_DIR)/recovery/recovery_receiver.sv \ + $(SRC_DIR)/recovery/recovery_transmitter.sv \ $(SRC_DIR)/recovery/recovery_executor.sv \ $(SRC_DIR)/recovery/recovery_handler.sv \ $(SRC_DIR)/libs/mem/prim_ram_1p_pkg.sv \ diff --git a/verification/cocotb/block/hci_queues_axi/Makefile b/verification/cocotb/block/hci_queues_axi/Makefile index 930febb..8485a24 100644 --- a/verification/cocotb/block/hci_queues_axi/Makefile +++ b/verification/cocotb/block/hci_queues_axi/Makefile @@ -34,6 +34,7 @@ VERILOG_SOURCES = \ $(SRC_DIR)/ctrl/width_converter_Nto8.sv \ $(SRC_DIR)/recovery/recovery_pec.sv \ $(SRC_DIR)/recovery/recovery_receiver.sv \ + $(SRC_DIR)/recovery/recovery_transmitter.sv \ $(SRC_DIR)/recovery/recovery_executor.sv \ $(SRC_DIR)/recovery/recovery_handler.sv \ $(SRC_DIR)/libs/mem/prim_ram_1p_pkg.sv \ diff --git a/verification/cocotb/top/lib_i3c_top/test_recovery.py b/verification/cocotb/top/lib_i3c_top/test_recovery.py index 08edc10..71d7950 100644 --- a/verification/cocotb/top/lib_i3c_top/test_recovery.py +++ b/verification/cocotb/top/lib_i3c_top/test_recovery.py @@ -5,8 +5,8 @@ from boot import boot_init from bus2csr import dword2int, int2dword from cocotbext_i3c.i3c_controller import I3cController -from cocotbext_i3c.i3c_target import I3CTarget from cocotbext_i3c.i3c_recovery_interface import I3cRecoveryInterface +from cocotbext_i3c.i3c_target import I3CTarget from interface import I3CTopTestInterface import cocotb @@ -45,6 +45,7 @@ async def initialize(dut): scl_o=dut.scl_sim_target_i, debug_state_o=None, speed=12.5e6, + address=0x23, ) tb = I3CTopTestInterface(dut) @@ -164,7 +165,7 @@ async def test_recovery_write_pec(dut): assert data == 0xDEADBEEF # From previous write -@cocotb.test() +@cocotb.test(skip=False) async def test_recovery_read(dut): """ Tests CSR read(s) using the recovery protocol @@ -174,17 +175,27 @@ async def test_recovery_read(dut): i3c_controller, i3c_target, tb, recovery = await initialize(dut) # Write data to PROT_CAP CSR - await tb.write_csr(tb.reg_map.I3C_EC.SECFWRECOVERYIF.PROT_CAP_0.base_addr, int2dword(0x04030201), 4) - await tb.write_csr(tb.reg_map.I3C_EC.SECFWRECOVERYIF.PROT_CAP_1.base_addr, int2dword(0x08070605), 4) - await tb.write_csr(tb.reg_map.I3C_EC.SECFWRECOVERYIF.PROT_CAP_2.base_addr, int2dword(0x0C0B0A09), 4) - await tb.write_csr(tb.reg_map.I3C_EC.SECFWRECOVERYIF.PROT_CAP_3.base_addr, int2dword(0xFF0F0E0D), 4) + await tb.write_csr( + tb.reg_map.I3C_EC.SECFWRECOVERYIF.PROT_CAP_0.base_addr, int2dword(0x04030201), 4 + ) + await tb.write_csr( + tb.reg_map.I3C_EC.SECFWRECOVERYIF.PROT_CAP_1.base_addr, int2dword(0x08070605), 4 + ) + await tb.write_csr( + tb.reg_map.I3C_EC.SECFWRECOVERYIF.PROT_CAP_2.base_addr, int2dword(0x0C0B0A09), 4 + ) + await tb.write_csr( + tb.reg_map.I3C_EC.SECFWRECOVERYIF.PROT_CAP_3.base_addr, int2dword(0xFF0F0E0D), 4 + ) # Wait await Timer(1, "us") # Read the PROT_CAP register - await recovery.command_read(0x5A, I3cRecoveryInterface.Command.PROT_CAP) + recovery_data, pec_ok = await recovery.command_read(0x5A, I3cRecoveryInterface.Command.PROT_CAP) + + # PROT_CAP read always returns 15 bytes + assert len(recovery_data) == 15 # Wait await Timer(2, "us") - diff --git a/violations.waiver b/violations.waiver index 6f49518..36cc7c5 100644 --- a/violations.waiver +++ b/violations.waiver @@ -11,3 +11,4 @@ waive --rule=line-length --location="src/hci/tti.sv" waive --rule=line-length --location="src/recovery/recovery_handler.sv" waive --rule=line-length --location="src/recovery/recovery_pec.sv" waive --rule=line-length --location="src/recovery/recovery_executor.sv" +waive --rule=case-missing-default --location="src/recovery/recovery_executor.sv"