From fd6094ee9e867e17c7fe6a882e07bb13e5fa0695 Mon Sep 17 00:00:00 2001 From: Jason <83615043+JJassonn69@users.noreply.github.com> Date: Mon, 16 Mar 2026 22:24:06 +0200 Subject: [PATCH] Fix P0/P1 RTL bugs found during pre-hardware audit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P0-1: nco_400m_enhanced.v — DSP48E1 OPMODE corrected from PCIN to P feedback (was routing stale cascade data into accumulator) P0-2: radar_receiver_final.v — removed same-clock CDC that corrupted ADC data path between ad9484_interface and DDC P1-5: fir_lowpass.v — fixed zero replication count in coefficient symmetric extension ({0{1'b0}} is empty, now uses explicit 0) Also updates .gitignore to exclude debug/scratch artifacts. All 30+ testbenches pass (unit, co-sim, integration). --- .gitignore | 19 +++++++++++++++ 9_Firmware/9_2_FPGA/fir_lowpass.v | 7 ++++-- 9_Firmware/9_2_FPGA/nco_400m_enhanced.v | 2 +- 9_Firmware/9_2_FPGA/radar_receiver_final.v | 27 ++++++++-------------- 4 files changed, 34 insertions(+), 21 deletions(-) diff --git a/.gitignore b/.gitignore index 85b776b..83b7daa 100644 --- a/.gitignore +++ b/.gitignore @@ -10,6 +10,25 @@ rbd_mode10_avg.csv rbd_mode10_ramp.csv rmc_autoscan.csv +# Debug / scratch RTL (not part of the design) +9_Firmware/9_2_FPGA/debug_*.v +9_Firmware/9_2_FPGA/tb/tb_fft_debug*.v +9_Firmware/9_2_FPGA/tb/tb_fft_mini*.v +9_Firmware/9_2_FPGA/tb/tb_bram_debug.v + +# Stray CSV artifacts from unit testbenches +9_Firmware/9_2_FPGA/cic_*.csv +9_Firmware/9_2_FPGA/fir_*.csv +9_Firmware/9_2_FPGA/nco_*.csv +9_Firmware/9_2_FPGA/ddc_*.csv +9_Firmware/9_2_FPGA/mf_pipeline_output.csv +9_Firmware/9_2_FPGA/tb_usb_data_interface.csv + +# Co-sim intermediate CSVs (regenerated by scripts) +9_Firmware/9_2_FPGA/tb/cosim/rtl_doppler_*.csv +9_Firmware/9_2_FPGA/tb/cosim/compare_doppler_*.csv +9_Firmware/9_2_FPGA/tb/cosim/rtl_multiseg_*.csv + # macOS .DS_Store diff --git a/9_Firmware/9_2_FPGA/fir_lowpass.v b/9_Firmware/9_2_FPGA/fir_lowpass.v index 0eb3347..f237cd9 100644 --- a/9_Firmware/9_2_FPGA/fir_lowpass.v +++ b/9_Firmware/9_2_FPGA/fir_lowpass.v @@ -117,8 +117,11 @@ always @(posedge clk or negedge reset_n) begin end end else if (valid_pipe[0]) begin for (i = 0; i < 16; i = i + 1) begin - add_l0[i] <= {{(ACCUM_WIDTH-DATA_WIDTH-COEFF_WIDTH){mult_result[2*i][DATA_WIDTH+COEFF_WIDTH-1]}}, mult_result[2*i]} + - {{(ACCUM_WIDTH-DATA_WIDTH-COEFF_WIDTH){mult_result[2*i+1][DATA_WIDTH+COEFF_WIDTH-1]}}, mult_result[2*i+1]}; + // mult_result is (DATA_WIDTH + COEFF_WIDTH) = 36 bits = ACCUM_WIDTH, + // so no sign extension is needed. Direct assignment preserves the + // signed multiply result. (Fixes Vivado Synth 8-693 "zero replication + // count" warning from the original {0{sign_bit}} expression.) + add_l0[i] <= mult_result[2*i] + mult_result[2*i+1]; end end end diff --git a/9_Firmware/9_2_FPGA/nco_400m_enhanced.v b/9_Firmware/9_2_FPGA/nco_400m_enhanced.v index fa2a8a5..7707a9d 100644 --- a/9_Firmware/9_2_FPGA/nco_400m_enhanced.v +++ b/9_Firmware/9_2_FPGA/nco_400m_enhanced.v @@ -217,7 +217,7 @@ DSP48E1 #( .D(25'b0), .CARRYIN(1'b0), // Control ports - .OPMODE(7'b0010011), // Z=P (010), Y=0 (00), X=C_reg (11) → P = P + C + .OPMODE(7'b0101100), // Z=P (010), Y=C (11), X=0 (00) → P = P + C .ALUMODE(4'b0000), // Z + X + Y + CIN (standard add) .INMODE(5'b00000), .CARRYINSEL(3'b000), diff --git a/9_Firmware/9_2_FPGA/radar_receiver_final.v b/9_Firmware/9_2_FPGA/radar_receiver_final.v index d406641..0c1bce6 100644 --- a/9_Firmware/9_2_FPGA/radar_receiver_final.v +++ b/9_Firmware/9_2_FPGA/radar_receiver_final.v @@ -103,9 +103,6 @@ wire clk_400m; wire [7:0] adc_data_cmos; // 8-bit ADC data (CMOS, from ad9484_interface_400m) wire adc_valid; // Data valid signal -wire [7:0] cdc_data_cmos; // 8-bit ADC data (CMOS) -wire cdc_valid; // Data valid signal - // ADC power-down control (directly tie low = ADC always on) assign adc_pwdn = 1'b0; @@ -121,18 +118,12 @@ ad9484_interface_400m adc ( .adc_dco_bufg(clk_400m) ); -cdc_adc_to_processing #( - .WIDTH(8), - .STAGES(3) -)cdc( - .src_clk(clk_400m), - .dst_clk(clk_400m), - .reset_n(reset_n), - .src_data(adc_data_cmos), - .src_valid(adc_valid), - .dst_data(cdc_data_cmos), - .dst_valid(cdc_valid) -); +// NOTE: The cdc_adc_to_processing instance that was here used src_clk=dst_clk=clk_400m +// (same clock domain — no crossing). Gray-code CDC on same-clock with fast-changing +// ADC data corrupts samples because Gray coding only guarantees safe transfer of +// values that change by 1 LSB at a time. The real 400MHz→100MHz CDC crossing is +// handled inside ddc_400m_enhanced via CIC decimation + CDC_FIR instances. +// Removed: cdc_adc_to_processing instance. ADC data now goes directly to DDC. // 2. DDC Input Interface wire signed [17:0] ddc_out_i; @@ -145,9 +136,9 @@ ddc_400m_enhanced ddc( .clk_400m(clk_400m), // 400MHz clock from ADC DCO .clk_100m(clk), // 100MHz system clock //used by the 2 FIR .reset_n(reset_n), - .adc_data(cdc_data_cmos), // ADC data at 400MHz (unsigned 0-255) - .adc_data_valid_i(cdc_valid), // Valid at 400MHz - .adc_data_valid_q(cdc_valid), // Valid at 400MHz + .adc_data(adc_data_cmos), // ADC data at 400MHz (direct from ADC interface) + .adc_data_valid_i(adc_valid), // Valid at 400MHz + .adc_data_valid_q(adc_valid), // Valid at 400MHz .baseband_i(ddc_out_i), // I output at 100MHz .baseband_q(ddc_out_q), // Q output at 100MHz .baseband_valid_i(ddc_valid_i), // Valid at 100MHz