FlintyLemming/PLFM_RADAR
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(pre-bringup): resolve P0 + quick-win P1 findings from 2026-04-19 audit

Browse Source 
Addresses findings from docs/DEVELOP_AUDIT_2026-04-19.md:

P0 source-level:
- F-4.3 ADAR1000_Manager::adarSetTxPhase now writes REG_LOAD_WORKING
  with LD_WRK_REGS_LDTX_OVERRIDE (0x02) instead of 0x01. Previous value
  toggled the LDRX latch on a TX-phase write, so host TX phase updates
  never reached the working registers.
- F-6.1 DDC mixer_saturation / filter_overflow / diagnostics were deleted
  at the receiver boundary. Now plumbed to new outputs on
  radar_receiver_final (ddc_overflow_any, ddc_saturation_count) and
  aggregated into gpio_dig5 in radar_system_top. Added mark_debug
  attributes for ILA visibility. Test/debug inputs tied low explicitly.
- F-0.8 adc_clk_mmcm.xdc set_clock_uncertainty: removed invalid -add
  flag (Vivado silently rejected it, applying zero guardband). Now uses
  absolute 0.150 ns which covers 53 ps jitter + ~100 ps PVT margin.

P1:
- F-4.2 adarSetBit / adarResetBit reject broadcast=ON — the RMW sampled
  a single device but wrote to all four, clobbering the other three's
  state.
- F-4.4 initializeSingleDevice returns false and leaves initialized=false
  when scratchpad verification fails; previously marked the device
  initialized anyway so downstream PA enable could drive a dead bus.
- F-6.2 FIR I/Q filter_overflow ports, previously unconnected, now OR'd
  into the module-level filter_overflow output.
- F-6.3 mti_canceller exposes 8-bit saturation counter. Saturation was
  previously invisible and produces spurious Doppler harmonics.

Verification:
- 27/27 iverilog testbenches pass
- 228/228 pytest pass (cross-layer contract + cosim)
- MCU unit tests 51/51 + 24/24 pass
- Remote Vivado 2025.2 build: bitstream writes; 400 MHz mixer pipeline
  now shows WNS -0.109 ns which MATCHES the audit's F-0.9 prediction
  that the design only closed because F-0.8's guardband was silently
  dropped. ft_clkout F-0.9 remains a show-stopper (requires MRCC pin
  move), tracked separately.

Not addressed in this PR (larger scope, follow-up tickets):
F-0.4, F-0.5, F-0.6, F-0.7, F-0.9, F-1.1, F-1.2, F-2.2, F-3.2, F-4.1,
F-4.7, F-6.4, F-6.5.
This commit is contained in:
Jason
2026-04-20 13:48:36 +05:45
parent c82b25f7a0
commit 3f47d1ef71
6 changed files with 190 additions and 92 deletions
Download Patch File Download Diff File Expand all files Collapse all files
9_Firmware/9_2_FPGA/constraints/adc_clk_mmcm.xdc
+6 -5
View File
@@ -88,8 +88,9 @@ set_false_path -hold -from [get_ports {adc_d_p[*]}] -to [get_clocks adc_dco_p]
# Timing margin for 400 MHz critical paths
# --------------------------------------------------------------------------
# Extra setup uncertainty forces Vivado to leave margin for temperature/voltage/
# aging variation. Reduced from 200 ps to 100 ps after NCO→mixer pipeline
# register fix eliminated the dominant timing bottleneck (WNS went from +0.002ns
# to comfortable margin). 100 ps still provides ~4% guardband on the 2.5ns period.
# This is additive to the existing jitter-based uncertainty (~53 ps).
set_clock_uncertainty -setup -add 0.100 [get_clocks clk_mmcm_out0]
# aging variation. 150 ps absolute covers the built-in jitter-based value
# (~53 ps) plus ~100 ps temperature/voltage/aging guardband.
# NOTE: Vivado's set_clock_uncertainty does NOT accept -add; prior use of
# -add 0.100 was silently rejected as a CRITICAL WARNING, so no guardband
# was applied. Use an absolute value. (audit finding F-0.8)
set_clock_uncertainty -setup 0.150 [get_clocks clk_mmcm_out0]
9_Firmware/9_2_FPGA/ddc_400m.v
+9 -3
View File
@@ -634,6 +634,11 @@ cdc_adc_to_processing #(
// FIR Filter Instances
// ============================================================================
// FIR overflow flags (audit F-6.2 — previously dangling, now OR'd into
// module-level filter_overflow so the receiver can see FIR arithmetic overflow)
wire fir_i_overflow;
wire fir_q_overflow;
// FIR I channel
fir_lowpass_parallel_enhanced fir_i_inst (
.clk(clk_100m),
@@ -643,10 +648,10 @@ fir_lowpass_parallel_enhanced fir_i_inst (
.data_out(fir_i_out),
.data_out_valid(fir_valid_i),
.fir_ready(fir_i_ready),
.filter_overflow()
.filter_overflow(fir_i_overflow)
);
// FIR Q channel
// FIR Q channel
fir_lowpass_parallel_enhanced fir_q_inst (
.clk(clk_100m),
.reset_n(reset_n),
@@ -655,10 +660,11 @@ fir_lowpass_parallel_enhanced fir_q_inst (
.data_out(fir_q_out),
.data_out_valid(fir_valid_q),
.fir_ready(fir_q_ready),
.filter_overflow()
.filter_overflow(fir_q_overflow)
);
assign fir_valid = fir_valid_i & fir_valid_q;
assign filter_overflow = fir_i_overflow | fir_q_overflow;
// ============================================================================
// Enhanced Output Stage
9_Firmware/9_2_FPGA/mti_canceller.v
+24 -7
View File
@@ -58,7 +58,12 @@ module mti_canceller #(
input wire mti_enable, // 1=MTI active, 0=pass-through
// ========== STATUS ==========
output reg mti_first_chirp // 1 during first chirp (output muted)
output reg mti_first_chirp, // 1 during first chirp (output muted)
// Audit F-6.3: count of saturated samples since last reset. Saturation
// here produces spurious Doppler harmonics (phantom targets at ±fs/2)
// and was previously invisible to the MCU. Saturates at 0xFF.
output reg [7:0] mti_saturation_count
);
// ============================================================================
@@ -104,18 +109,30 @@ assign diff_q_sat = (diff_q_full > $signed({{2{1'b0}}, {(DATA_WIDTH-1){1'b1}}}))
? $signed({1'b1, {(DATA_WIDTH-1){1'b0}}})
: diff_q_full[DATA_WIDTH-1:0];
// Saturation detection (F-6.3): the top two bits of the DATA_WIDTH+1 signed
// difference disagree iff the value exceeds the DATA_WIDTH signed range.
wire diff_i_overflow = (diff_i_full[DATA_WIDTH] != diff_i_full[DATA_WIDTH-1]);
wire diff_q_overflow = (diff_q_full[DATA_WIDTH] != diff_q_full[DATA_WIDTH-1]);
// ============================================================================
// MAIN LOGIC
// ============================================================================
always @(posedge clk or negedge reset_n) begin
if (!reset_n) begin
range_i_out <= {DATA_WIDTH{1'b0}};
range_q_out <= {DATA_WIDTH{1'b0}};
range_valid_out <= 1'b0;
range_bin_out <= 6'd0;
has_previous <= 1'b0;
mti_first_chirp <= 1'b1;
range_i_out <= {DATA_WIDTH{1'b0}};
range_q_out <= {DATA_WIDTH{1'b0}};
range_valid_out <= 1'b0;
range_bin_out <= 6'd0;
has_previous <= 1'b0;
mti_first_chirp <= 1'b1;
mti_saturation_count <= 8'd0;
end else begin
// Count saturated MTI-active samples (F-6.3). Clamp at 0xFF.
if (range_valid_in && mti_enable && has_previous
&& (diff_i_overflow || diff_q_overflow)
&& (mti_saturation_count != 8'hFF)) begin
mti_saturation_count <= mti_saturation_count + 8'd1;
end
// Default: no valid output
range_valid_out <= 1'b0;
9_Firmware/9_2_FPGA/radar_receiver_final.v
+48 -12
View File
@@ -74,7 +74,16 @@ module radar_receiver_final (
// AGC status outputs (for status readback / STM32 outer loop)
output wire [7:0] agc_saturation_count, // Per-frame clipped sample count
output wire [7:0] agc_peak_magnitude, // Per-frame peak (upper 8 bits)
output wire [3:0] agc_current_gain // Effective gain_shift encoding
output wire [3:0] agc_current_gain, // Effective gain_shift encoding
// DDC overflow diagnostics (audit F-6.1 — previously deleted at boundary).
// Not yet plumbed into the USB status packet (protocol contract is frozen);
// exposed here for gpio aggregation and ILA mark_debug visibility.
output wire ddc_overflow_any,
output wire [2:0] ddc_saturation_count,
// MTI 2-pulse canceller saturation count (audit F-6.3).
output wire [7:0] mti_saturation_count_out
);
// ========== INTERNAL SIGNALS ==========
@@ -211,6 +220,16 @@ wire signed [17:0] ddc_out_q;
wire ddc_valid_i;
wire ddc_valid_q;
// DDC diagnostic signals (audit F-6.1 — all outputs previously unconnected)
wire [1:0] ddc_status_w;
wire [7:0] ddc_diagnostics_w;
wire ddc_mixer_saturation;
wire ddc_filter_overflow;
(* mark_debug = "true" *) wire ddc_mixer_saturation_dbg = ddc_mixer_saturation;
(* mark_debug = "true" *) wire ddc_filter_overflow_dbg = ddc_filter_overflow;
(* mark_debug = "true" *) wire [7:0] ddc_diagnostics_dbg = ddc_diagnostics_w;
ddc_400m_enhanced ddc(
.clk_400m(clk_400m), // 400MHz clock from ADC DCO
.clk_100m(clk), // 100MHz system clock //used by the 2 FIR
@@ -219,12 +238,28 @@ ddc_400m_enhanced ddc(
.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_q(ddc_out_q), // Q output at 100MHz
.baseband_valid_i(ddc_valid_i), // Valid at 100MHz
.baseband_valid_q(ddc_valid_q),
.mixers_enable(1'b1)
.baseband_valid_q(ddc_valid_q),
.mixers_enable(1'b1),
// Diagnostics (audit F-6.1) — previously all unconnected
.ddc_status(ddc_status_w),
.ddc_diagnostics(ddc_diagnostics_w),
.mixer_saturation(ddc_mixer_saturation),
.filter_overflow(ddc_filter_overflow),
// Test/debug inputs — explicit tie-low (were floating)
.test_mode(2'b00),
.test_phase_inc(16'h0000),
.force_saturation(1'b0),
.reset_monitors(1'b0),
.debug_sample_count(),
.debug_internal_i(),
.debug_internal_q()
);
assign ddc_overflow_any = ddc_mixer_saturation | ddc_filter_overflow;
assign ddc_saturation_count = ddc_diagnostics_w[7:5];
ddc_input_interface ddc_if (
.clk(clk),
.reset_n(reset_n),
@@ -391,7 +426,8 @@ mti_canceller #(
.range_valid_out(mti_range_valid),
.range_bin_out(mti_range_bin),
.mti_enable(host_mti_enable),
.mti_first_chirp(mti_first_chirp)
.mti_first_chirp(mti_first_chirp),
.mti_saturation_count(mti_saturation_count_out)
);
// ========== FRAME SYNC FROM TRANSMITTER ==========
@@ -430,12 +466,12 @@ assign range_data_32bit = {mti_range_q, mti_range_i};
assign range_data_valid = mti_range_valid;
// ========== DOPPLER PROCESSOR ==========
doppler_processor_optimized #(
.DOPPLER_FFT_SIZE(16),
.RANGE_BINS(64),
.CHIRPS_PER_FRAME(32),
.CHIRPS_PER_SUBFRAME(16)
) doppler_proc (
doppler_processor_optimized #(
.DOPPLER_FFT_SIZE(16),
.RANGE_BINS(64),
.CHIRPS_PER_FRAME(32),
.CHIRPS_PER_SUBFRAME(16)
) doppler_proc (
.clk(clk),
.reset_n(reset_n),
.range_data(range_data_32bit),
@@ -498,4 +534,4 @@ assign agc_saturation_count = gc_saturation_count;
assign agc_peak_magnitude = gc_peak_magnitude;
assign agc_current_gain = gc_current_gain;
endmodule
endmodule
9_Firmware/9_2_FPGA/radar_system_top.v
+56 -37
View File
@@ -198,6 +198,14 @@ wire [7:0] rx_agc_saturation_count;
wire [7:0] rx_agc_peak_magnitude;
wire [3:0] rx_agc_current_gain;
// DDC overflow diagnostics (audit F-6.1) — plumbed out of receiver so the
// DDC mixer_saturation / filter_overflow ports are no longer deleted at
// the boundary. Aggregated into gpio_dig5 alongside AGC saturation.
wire rx_ddc_overflow_any;
wire [2:0] rx_ddc_saturation_count;
// MTI saturation count (audit F-6.3). OR'd into gpio_dig5 for MCU visibility.
wire [7:0] rx_mti_saturation_count;
// Data packing for USB
wire [31:0] usb_range_profile;
wire usb_range_valid;
@@ -243,12 +251,12 @@ reg [5:0] host_chirps_per_elev; // Opcode 0x15 (default 32)
reg host_status_request; // Opcode 0xFF (self-clearing pulse)
// Fix 4: Doppler/chirps mismatch protection
// DOPPLER_FRAME_CHIRPS is the fixed chirp count expected by the staggered-PRI
// Doppler path (16 long + 16 short). If host sets chirps_per_elev to a
// different value, Doppler accumulation is corrupted. Clamp at command decode
// and flag the mismatch so the host knows.
localparam DOPPLER_FRAME_CHIRPS = 32; // Total chirps per Doppler frame
reg chirps_mismatch_error; // Set if host tried to set chirps != FFT size
// DOPPLER_FRAME_CHIRPS is the fixed chirp count expected by the staggered-PRI
// Doppler path (16 long + 16 short). If host sets chirps_per_elev to a
// different value, Doppler accumulation is corrupted. Clamp at command decode
// and flag the mismatch so the host knows.
localparam DOPPLER_FRAME_CHIRPS = 32; // Total chirps per Doppler frame
reg chirps_mismatch_error; // Set if host tried to set chirps != FFT size
// Fix 7: Range-mode register (opcode 0x20)
// Future-proofing for 3km/10km antenna switching.
@@ -562,7 +570,12 @@ radar_receiver_final rx_inst (
// AGC status outputs
.agc_saturation_count(rx_agc_saturation_count),
.agc_peak_magnitude(rx_agc_peak_magnitude),
.agc_current_gain(rx_agc_current_gain)
.agc_current_gain(rx_agc_current_gain),
// DDC overflow diagnostics (audit F-6.1)
.ddc_overflow_any(rx_ddc_overflow_any),
.ddc_saturation_count(rx_ddc_saturation_count),
// MTI saturation count (audit F-6.3)
.mti_saturation_count_out(rx_mti_saturation_count)
);
// ============================================================================
@@ -578,21 +591,21 @@ assign rx_doppler_data_valid = rx_doppler_valid;
// ============================================================================
// DC NOTCH FILTER (post-Doppler-FFT, pre-CFAR)
// ============================================================================
// Zeros out Doppler bins within ±host_dc_notch_width of DC for BOTH
// sub-frames in the dual 16-pt FFT architecture.
// doppler_bin[4:0] = {sub_frame, bin[3:0]}:
// Sub-frame 0: bins 0-15, DC = bin 0, wrap = bin 15
// Sub-frame 1: bins 16-31, DC = bin 16, wrap = bin 31
// notch_width=1 → zero bins {0,16}. notch_width=2 → zero bins
// {0,1,15,16,17,31}. etc.
// When host_dc_notch_width=0: pass-through (no zeroing).
wire dc_notch_active;
wire [4:0] dop_bin_unsigned = rx_doppler_bin;
wire [3:0] bin_within_sf = dop_bin_unsigned[3:0];
assign dc_notch_active = (host_dc_notch_width != 3'd0) &&
(bin_within_sf < {1'b0, host_dc_notch_width} ||
bin_within_sf > (4'd15 - {1'b0, host_dc_notch_width} + 4'd1));
// Zeros out Doppler bins within ±host_dc_notch_width of DC for BOTH
// sub-frames in the dual 16-pt FFT architecture.
// doppler_bin[4:0] = {sub_frame, bin[3:0]}:
// Sub-frame 0: bins 0-15, DC = bin 0, wrap = bin 15
// Sub-frame 1: bins 16-31, DC = bin 16, wrap = bin 31
// notch_width=1 → zero bins {0,16}. notch_width=2 → zero bins
// {0,1,15,16,17,31}. etc.
// When host_dc_notch_width=0: pass-through (no zeroing).
wire dc_notch_active;
wire [4:0] dop_bin_unsigned = rx_doppler_bin;
wire [3:0] bin_within_sf = dop_bin_unsigned[3:0];
assign dc_notch_active = (host_dc_notch_width != 3'd0) &&
(bin_within_sf < {1'b0, host_dc_notch_width} ||
bin_within_sf > (4'd15 - {1'b0, host_dc_notch_width} + 4'd1));
// Notched Doppler data: zero I/Q when in notch zone, pass through otherwise
wire [31:0] notched_doppler_data = dc_notch_active ? 32'd0 : rx_doppler_output;
@@ -959,19 +972,19 @@ always @(posedge clk_100m_buf or negedge sys_reset_n) begin
8'h13: host_short_chirp_cycles <= usb_cmd_value;
8'h14: host_short_listen_cycles <= usb_cmd_value;
8'h15: begin
// Fix 4: Clamp chirps_per_elev to the fixed Doppler frame size.
// If host requests a different value, clamp and set error flag.
if (usb_cmd_value[5:0] > DOPPLER_FRAME_CHIRPS[5:0]) begin
host_chirps_per_elev <= DOPPLER_FRAME_CHIRPS[5:0];
chirps_mismatch_error <= 1'b1;
end else if (usb_cmd_value[5:0] == 6'd0) begin
host_chirps_per_elev <= DOPPLER_FRAME_CHIRPS[5:0];
chirps_mismatch_error <= 1'b1;
end else begin
host_chirps_per_elev <= usb_cmd_value[5:0];
// Clear error only if value matches FFT size exactly
chirps_mismatch_error <= (usb_cmd_value[5:0] != DOPPLER_FRAME_CHIRPS[5:0]);
end
// Fix 4: Clamp chirps_per_elev to the fixed Doppler frame size.
// If host requests a different value, clamp and set error flag.
if (usb_cmd_value[5:0] > DOPPLER_FRAME_CHIRPS[5:0]) begin
host_chirps_per_elev <= DOPPLER_FRAME_CHIRPS[5:0];
chirps_mismatch_error <= 1'b1;
end else if (usb_cmd_value[5:0] == 6'd0) begin
host_chirps_per_elev <= DOPPLER_FRAME_CHIRPS[5:0];
chirps_mismatch_error <= 1'b1;
end else begin
host_chirps_per_elev <= usb_cmd_value[5:0];
// Clear error only if value matches FFT size exactly
chirps_mismatch_error <= (usb_cmd_value[5:0] != DOPPLER_FRAME_CHIRPS[5:0]);
end
end
8'h16: host_gain_shift <= usb_cmd_value[3:0]; // Fix 3: digital gain
8'h20: host_range_mode <= usb_cmd_value[1:0]; // Fix 7: range mode
@@ -1040,7 +1053,13 @@ assign system_status = status_reg;
// DIG_6: AGC enable flag — mirrors host_agc_enable so STM32 outer-loop AGC
// tracks the FPGA register as single source of truth.
// DIG_7: Reserved (tied low for future use).
assign gpio_dig5 = (rx_agc_saturation_count != 8'd0);
// gpio_dig5: "signal-chain clipped" — asserts on AGC saturation, DDC mixer/FIR
// overflow, or MTI 2-pulse saturation. Audit F-6.1/F-6.3: these were all
// previously invisible to the MCU.
assign gpio_dig5 = (rx_agc_saturation_count != 8'd0)
| rx_ddc_overflow_any
| (rx_ddc_saturation_count != 3'd0)
| (rx_mti_saturation_count != 8'd0);
assign gpio_dig6 = host_agc_enable;
assign gpio_dig7 = 1'b0;
@@ -1075,4 +1094,4 @@ always @(posedge clk_100m_buf) begin
end
`endif
endmodule
endmodule