Various hardware improvements and fixed

Bender.yml

@@ -37,7 +37,6 @@ sources:
   - hw/rsp_read/crc7_read.sv
   - hw/sd_clk_generator.sv
   - hw/sdhci_debounce.sv
-  - hw/ser_par_shift_reg.sv
   - hw/sram_shift_reg.sv # tc_sram_impl
 
 
@@ -46,7 +45,7 @@ sources:
   - hw/dat_buffer.sv # sram_shift_reg
   - hw/dat_read/dat_read.sv # crc16_read
   - hw/dat_write/dat_write.sv # crc16_write
-  - hw/rsp_read/rsp_read.sv # ser_par_shift_reg, crc7_read, counter
+  - hw/rsp_read/rsp_read.sv # crc7_read, counter
 
   # Level 3
   - hw/cmd_logic.sv # cmd_write, rsp_read, counter
@@ -59,12 +58,15 @@ sources:
   - hw/sdhci_top.sv # sdhci_reg_obi, sdhci_reg_logic, sd_clk_generator, cmd_wrap, dat_wrap
 
   # Level 6
+  - hw/sdhci_obi_to_reg.sv # register_interface to OBI bridge
   - hw/sdhci_top_obi.sv # sdhci_top, external obi_pkg
 
   - target: any(simulation, test)
     files:
       # Level 1
       - target/sim/src/tb_dat.sv # sd_clk_generator, dat_write, dat_read
+      - target/sim/src/tb_dat_buffer_sizes.sv # dat_buffer
+      - target/sim/src/tb_reg_reset.sv # sdhci_top register reset behavior
       - target/sim/src/tb_driver_crc.sv
       - target/sim/model/sd_crc_7.v
       - target/sim/model/sd_crc_16.v

Makefile

@@ -23,7 +23,7 @@ deepclean: sdhci-deepclean
 help:
 	@echo "Some available commands"
 	@echo "all:       Fetch & compile everything"
-	@echo "hw:        Regenerate generated hardware"
+	@echo "hw:        Hardware source hook; no generated register files"
 	@echo "sw:        Compile the software libraries"
 	@echo "sim:       Generate simulation scripts and download models."
 	@echo "           Note: Some of these models are under other licenses."

README.md

@@ -0,0 +1,142 @@
+# SDHCI Controller
+
+This repository contains a synthesizable SD Host Controller Interface (SDHCI)
+hardware block with a register interface, SD command/data-line logic, an OBI
+wrapper, simulation testbenches, and simple software support files.
+
+## Current IP Behavior
+
+The top-level SDHCI controller is `hw/sdhci_top.sv`. It exposes a generic
+register request/response interface, SD card pads (`sd_clk`, `sd_cmd`, 4-bit
+`sd_dat`, card detect), and one interrupt output. `hw/sdhci_top_obi.sv` wraps
+the same controller for OBI integrations.
+
+The controller currently implements:
+
+- SD command issue and response capture, including CRC7 handling.
+- DAT read/write transfers on the 4-bit SD data bus, including CRC16 handling.
+- Single-block and multi-block transfer flow, including block-count updates.
+- Auto CMD12 sequencing and Auto CMD12 error status reporting.
+- Command and data timeout reporting.
+- Card-detect debounce and present-state reporting.
+- SD clock generation from `clk_i` with `ClkPreDiv` and the SDHCI clock-control
+  divider.
+- A semi-handwritten SDHCI register file in `hw/reg/sdhci_reg_top.sv`, with
+  register data structures in `hw/reg/sdhci_reg_pkg.sv` and register-side
+  behavioral glue in `hw/reg/sdhci_reg_logic.sv`.
+- Software reset of the full controller, command path, and data path.
+- Write-one-to-clear interrupt/status behavior and interrupt signal generation.
+
+The DAT buffer size is controlled by `BufferNumWords`, with one 32-bit word per
+entry. The default configuration is SDHCI-compliant:
+
+```systemverilog
+parameter int unsigned BufferNumWords = 256;
+parameter bit AllowNoncompliantBufferSizes = 1'b0;
+```
+
+With `AllowNoncompliantBufferSizes == 0`, the buffer must be at least 512 bytes
+and data-port ready/status behavior is presented in full-block chunks. With
+`AllowNoncompliantBufferSizes == 1`, smaller buffers are allowed as an explicit
+integration escape hatch; software can discover the data-port chunk size through
+the vendor capabilities register at offset `0x44`. In that mode, bit 31 marks
+the noncompliant configuration and bits `[15:0]` report the buffer chunk size in
+bytes.
+
+The implementation intentionally does not include DMA yet. Programmed I/O
+through the Buffer Data Port is the current data path.
+
+## Tool Flow
+
+Dependencies are managed through Bender. The root `Makefile` includes
+`sdhci.mk`, which in turn includes the hardware, software, and simulation
+fragments.
+
+Common entry points:
+
+```sh
+make hw
+make sw
+make sim
+make all
+make clean
+make deepclean
+```
+
+`make sim` downloads the external SD card simulation model files and generates
+the Questa/ModelSim compile script at `target/sim/vsim/compile.sdhci.tcl`:
+
+```sh
+make sim
+cd target/sim/vsim
+vsim -c -do "do compile.sdhci.tcl; quit -f"
+```
+
+The regression testbenches used for this change were:
+
+```sh
+for tb in \
+  tb_dat \
+  tb_dat_buffer_sizes \
+  tb_reg_reset \
+  tb_driver_crc \
+  tb_acmd12_errorhandling \
+  tb_acmd12_interrupts \
+  tb_cmd_timeout \
+  tb_dat_timeout \
+  tb_block_read
+do
+  timeout 300 vsim -c "$tb" -do "run -all; quit -f"
+done
+```
+
+All of the above compiled and ran with zero simulator errors in the latest
+local verification run. The previously reported `tb_acmd12_interrupts` hang was
+not reproduced after the register/interrupt cleanup; that test completed under
+the same timeout as the rest of the suite.
+
+## Synthesis Snapshot
+
+The latest synthesis check used Synopsys Design Compiler with the IHP13 target
+setup under `target/ihp13/synopsys` and a 10 ns `clk_i` constraint. Two wrapper
+variants were synthesized:
+
+- Minimum-size DAT buffer: `BufferNumWords = 8`,
+  `AllowNoncompliantBufferSizes = 1'b1`.
+- Full compliant DAT buffer: `BufferNumWords = 256`,
+  `AllowNoncompliantBufferSizes = 1'b0`.
+
+The generated reports were written under `/tmp/sdhci_syn/{minimal,compliant}`.
+The local run used temporary wrapper sources and Tcl scripts in `/tmp/sdhci_syn`
+to instantiate the two parameter sets without changing the checked-in RTL. The
+commands were run from `target/ihp13/synopsys`:
+
+```sh
+timeout 1800 env LD_LIBRARY_PATH=/usr/lib64 \
+  /usr/sepp/bin/synopsys dc_shell \
+  -f /tmp/sdhci_syn/run_variant.tcl \
+  -x "set argv minimal"
+
+timeout 1800 env LD_LIBRARY_PATH=/usr/lib64 \
+  /usr/sepp/bin/synopsys dc_shell \
+  -f /tmp/sdhci_syn/run_variant.tcl \
+  -x "set argv compliant"
+```
+
+The area results were:
+
+| Variant | Buffer size | Cell area | Comb area | Noncomb area | Seq cells |
+| --- | ---: | ---: | ---: | ---: | ---: |
+| Minimum | 8 words / 32 B | 73,647.1024 | 38,914.6469 | 34,732.4555 | 1,288 |
+| Full compliant | 256 words / 1024 B | 455,444.6491 | 241,100.2964 | 214,344.3527 | 9,239 |
+
+Timing met in both synthesis runs:
+
+| Variant | WNS | TNS | Violating paths | Worst reported path slack |
+| --- | ---: | ---: | ---: | ---: |
+| Minimum | 0.0000 | 0.0000 | 0 | 1.4512 ns |
+| Full compliant | 0.0000 | 0.0000 | 0 | 1.3624 ns |
+
+For the full compliant variant, the DAT buffer dominates the area. The
+`i_top/i_dat_wrap/i_dat_buffer` hierarchy accounts for 396,383.9118 area units,
+with 392,445.0509 area units under the SRAM implementation.

hw/autocmd_wrap.sv

@@ -14,6 +14,7 @@ import sdhci_reg_pkg::*;
 module autocmd_wrap (
   input  logic clk_i,
   input  logic rst_ni,
+  input  logic clear_i,
   input  logic clk_en_p_i, // high before next sd_clk posedge
   input  logic clk_en_n_i, // high before next sd_clk negedge
   input  logic div_1_i,
@@ -55,13 +56,13 @@ module autocmd_wrap (
   ////////////////
 
   logic driver_cmd_queued_q, driver_cmd_queued_d;
-  `FF(driver_cmd_queued_q, driver_cmd_queued_d, '0, clk_i, rst_ni);
+  `FFARNC(driver_cmd_queued_q, driver_cmd_queued_d, clear_i, '0, clk_i, rst_ni);
 
   logic autocmd12_queued_q, autocmd12_queued_d;
-  `FF(autocmd12_queued_q, autocmd12_queued_d, '0, clk_i, rst_ni);
+  `FFARNC(autocmd12_queued_q, autocmd12_queued_d, clear_i, '0, clk_i, rst_ni);
 
   logic running_autocmd12_q, running_autocmd12_d;
-  `FF(running_autocmd12_q, running_autocmd12_d, '0, clk_i, rst_ni);
+  `FFARNC(running_autocmd12_q, running_autocmd12_d, clear_i, '0, clk_i, rst_ni);
 
   logic command_queued;
   assign command_queued = driver_cmd_queued_q || autocmd12_queued_q;
@@ -88,21 +89,32 @@ module autocmd_wrap (
   logic cmd_errors_occured;
   assign cmd_errors_occured = end_bit_error || crc_error || index_error || timeout_error;
 
+  logic active_transfer_direction_q, active_transfer_direction_d;
+  `FFLARNC(active_transfer_direction_q, active_transfer_direction_d,
+           command_started && !autocmd12_queued_q && cmd_data_present_o,
+           clear_i, '0, clk_i, rst_ni);
+
+  // Use running_autocmd12_q (not autocmd12_queued_q) so the mux output
+  // stays stable throughout command execution, allowing cmd_logic to read
+  // these directly without re-latching them.
+  logic is_autocmd12;
+  assign is_autocmd12 = autocmd12_queued_q | running_autocmd12_q;
+
   sdhci_pkg::cmd_t current_cmd;
-  assign current_cmd = autocmd12_queued_q ? 6'd12 :
+  assign current_cmd = is_autocmd12 ? 6'd12 :
                        reg2hw.command.command_index.q;
 
   sdhci_pkg::cmd_arg_t current_arg;
-  assign current_arg = autocmd12_queued_q ? '0 : reg2hw.argument.q;
+  assign current_arg = is_autocmd12 ? '0 : reg2hw.argument.q;
 
   sdhci_pkg::response_type_e current_rsp_type;
 
   always_comb begin : rsp_type
     current_rsp_type = sdhci_pkg::response_type_e'(reg2hw.command.response_type_select.q);
 
     // according to electrical spec 7.8.4, CMD12 is R1 on reads and R1b on writes
-    if (autocmd12_queued_q) begin
-      if (reg2hw.transfer_mode.data_transfer_direction_select.q == 1'b0) begin
+    if (is_autocmd12) begin
+      if (active_transfer_direction_q == 1'b0) begin
         // write -> R1b
         current_rsp_type = sdhci_pkg::RESPONSE_LENGTH_48_CHECK_BUSY;
       end else begin
@@ -115,12 +127,25 @@ module autocmd_wrap (
   assign cmd_needs_busy_o = current_rsp_type == sdhci_pkg::RESPONSE_LENGTH_48_CHECK_BUSY;
   assign cmd_transfer_direction_o = reg2hw.transfer_mode.data_transfer_direction_select.q;
 
+  sdhci_pkg::cmd_t accepted_cmd_q, accepted_cmd_d;
+  `FFLARNC(accepted_cmd_q, accepted_cmd_d, command_started, clear_i, '0, clk_i, rst_ni);
+
+  sdhci_pkg::cmd_arg_t accepted_arg_q, accepted_arg_d;
+  `FFLARNC(accepted_arg_q, accepted_arg_d, command_started, clear_i, '0, clk_i, rst_ni);
+
+  sdhci_pkg::response_type_e accepted_rsp_type_q, accepted_rsp_type_d;
+  `FFLARNC(accepted_rsp_type_q, accepted_rsp_type_d, command_started, clear_i, sdhci_pkg::NO_RESPONSE, clk_i, rst_ni);
+
   always_comb begin : request_commands
     driver_cmd_queued_d = driver_cmd_queued_q;
     autocmd12_queued_d = autocmd12_queued_q;
     auto_cmd12_errors_o.command_not_issued_by_auto_cmd12_error.de = 1'b0;
     auto_cmd12_errors_o.auto_cmd12_not_executed.de = 1'b0;
     running_autocmd12_d = running_autocmd12_q;
+    active_transfer_direction_d = reg2hw.transfer_mode.data_transfer_direction_select.q;
+    accepted_cmd_d = current_cmd;
+    accepted_arg_d = current_arg;
+    accepted_rsp_type_d = current_rsp_type;
 
     if (reg2hw.command.command_index.qe) begin
       driver_cmd_queued_d = 1'b1;
@@ -172,48 +197,29 @@ module autocmd_wrap (
   // autocmd12 execution should not inhibit the driver
   assign command_inhibit_cmd_o.d  = driver_cmd_queued_q | (cmd_inhibit_logic && ~running_autocmd12_q);
 
-  logic [31:0] rsp0, rsp1, rsp2, rsp3;
-  logic [119:0] rsp;
+  logic [31:0] cmd_response0_d, cmd_response1_d, cmd_response2_d, cmd_response3_d;
+  logic cmd_response0_de, cmd_response1_de, cmd_response2_de, cmd_response3_de;
 
   always_comb begin : rsp_assignment
-    rsp0 = reg2hw.response0.q;
-    rsp1 = reg2hw.response1.q;
-    rsp2 = reg2hw.response2.q;
-    rsp3 = reg2hw.response3.q;
-
-    if (running_autocmd12_q) begin
-      // auto cmd 12 response goes to upper word of rsp register
-      rsp3 = rsp [31:0];
-    end else begin
-      unique case (current_rsp_type)
-        sdhci_pkg::NO_RESPONSE:;
-
-        sdhci_pkg::RESPONSE_LENGTH_136: begin
-          // long response
-          rsp0 = rsp[31:0];
-          rsp1 = rsp[63:32];
-          rsp2 = rsp[95:64];
-          rsp3[23:0] = rsp[119:96]; // save bits 31:24 of rsp3
-        end
-
-        sdhci_pkg::RESPONSE_LENGTH_48, sdhci_pkg::RESPONSE_LENGTH_48_CHECK_BUSY: begin
-          rsp0 = rsp[31:0];
-        end
-
-        default:;
-      endcase
+    response0_d_o  = cmd_response0_d;
+    response1_d_o  = cmd_response1_d;
+    response2_d_o  = cmd_response2_d;
+    response3_d_o  = cmd_response3_d;
+    response0_de_o = cmd_response0_de && !running_autocmd12_q;
+    response1_de_o = cmd_response1_de && !running_autocmd12_q;
+    response2_de_o = cmd_response2_de && !running_autocmd12_q;
+    response3_de_o = cmd_response3_de && !running_autocmd12_q;
+
+    if (cmd_response3_de && accepted_rsp_type_q == sdhci_pkg::RESPONSE_LENGTH_136) begin
+      response3_d_o = {reg2hw.response3.q[31:24], cmd_response3_d[23:0]};
     end
-  end : rsp_assignment
-
-  assign response0_d_o  = rsp0;
-  assign response1_d_o  = rsp1;
-  assign response2_d_o  = rsp2;
-  assign response3_d_o  = rsp3;
 
-  assign response0_de_o = cmd_result_valid;
-  assign response1_de_o = cmd_result_valid;
-  assign response2_de_o = cmd_result_valid;
-  assign response3_de_o = cmd_result_valid;
+    if (running_autocmd12_q && cmd_response0_de) begin
+      // Auto CMD12 response is stored in RESPONSE3.
+      response3_d_o  = cmd_response0_d;
+      response3_de_o = 1'b1;
+    end
+  end : rsp_assignment
 
   ////////////////////
   // Error Checking //
@@ -272,6 +278,7 @@ module autocmd_wrap (
   cmd_logic i_cmd_logic (
     .clk_i             (clk_i),
     .rst_ni            (rst_ni),
+    .clear_i           (clear_i),
     .clk_en_p_i        (clk_en_p_i),
     .clk_en_n_i        (clk_en_n_i),
     .div_1_i           (div_1_i),
@@ -283,14 +290,21 @@ module autocmd_wrap (
     .rsp_done_o        (sd_rsp_done_o),
     .cmd_inhibit_cmd_o (cmd_inhibit_logic),
 
-    .cmd_i             (current_cmd),
-    .cmd_arg_i         (current_arg),
-    .response_type_i   (current_rsp_type),
+    .cmd_i             (accepted_cmd_q),
+    .cmd_arg_i         (accepted_arg_q),
+    .response_type_i   (accepted_rsp_type_q),
     .cmd_valid_i       (command_queued),
     .cmd_ready_o       (command_ready),
 
     .cmd_result_valid_o(cmd_result_valid),
-    .rsp_o             (rsp),
+    .response0_d_o     (cmd_response0_d),
+    .response1_d_o     (cmd_response1_d),
+    .response2_d_o     (cmd_response2_d),
+    .response3_d_o     (cmd_response3_d),
+    .response0_de_o    (cmd_response0_de),
+    .response1_de_o    (cmd_response1_de),
+    .response2_de_o    (cmd_response2_de),
+    .response3_de_o    (cmd_response3_de),
     .end_bit_error_o   (end_bit_error),
     .crc_error_o       (crc_error),
     .index_error_o     (index_error),