fix(pre-bringup): second-batch P1/P2/P3 audit findings

Addresses the remaining actionable items from
docs/DEVELOP_AUDIT_2026-04-19.md after commit 3f47d1e.

XDC (dead waivers — F-0.4, F-0.5, F-0.6, F-0.7):
- ft_clkout_IBUF CLOCK_DEDICATED_ROUTE now uses hierarchical filter;
  flat net name did not exist post-synth.
- reset_sync_reg[*] false-path rewritten to walk hierarchy and filter
  on CLR/PRE pins.
- adc_clk_mmcm.xdc ft601_clk_in references replaced with foreach-loop
  over real USB clock names, gated on -quiet existence.
- MMCM LOCKED waiver uses REF_PIN_NAME filter instead of the
  previously-missing u_core/ literal path.

CDC (F-1.1, F-1.2, F-1.3):
- Documented the quasi-static-bus stability invariant above the
  FT601 cmd_valid toggle block.
- cdc_adc_to_processing gains an `overrun` output; the two CIC->FIR
  instances feed a sticky cdc_cic_fir_overrun flag surfaced on
  gpio_dig5 so silent sample drops become visible to the MCU.
- Removed the dead mixers_enable synchronizer in ddc_400m.v; the _sync
  output was unused and every caller ties the port to 1'b1.

Diagnostics (F-6.4):
- range_bin_decimator watchdog_timeout plumbed through receiver
  and top-level, OR'd into gpio_dig5.

ADAR (F-4.7):
- delayUs() replaced with DWT cycle counter; self-initialising
  TRCENA/CYCCNTENA, overflow-safe unsigned subtraction.

Regression: tb_cdc_modules.v 57/57 passes under iverilog after
the cdc_modules.v change. Remote Vivado verification in progress.
This commit is contained in:
Jason
2026-04-20 14:28:22 +05:45
parent 3f47d1ef71
commit 675b1c0015
7 changed files with 153 additions and 26 deletions
@@ -735,10 +735,21 @@ void ADAR1000Manager::setLNABias(bool enable) {
}
void ADAR1000Manager::delayUs(uint32_t microseconds) {
// Simple implementation - for F7 @ 216MHz, each loop ~7 cycles ≈ 0.032us
volatile uint32_t cycles = microseconds * 10; // Adjust this multiplier for your clock
while (cycles--) {
__NOP();
// Audit F-4.7: the prior implementation was a calibrated __NOP() busy-loop
// that silently drifted with compiler optimization, cache state, and flash
// wait-states. The ADAR1000 PLL/TX settling times require a real clock, so
// we poll the DWT cycle counter instead. One-time TRCENA/CYCCNTENA enable
// is idempotent; subsequent calls skip the init branch via DWT->CTRL read.
if ((DWT->CTRL & DWT_CTRL_CYCCNTENA_Msk) == 0U) {
CoreDebug->DEMCR |= CoreDebug_DEMCR_TRCENA_Msk;
DWT->CYCCNT = 0U;
DWT->CTRL |= DWT_CTRL_CYCCNTENA_Msk;
}
const uint32_t cycles_per_us = SystemCoreClock / 1000000U;
const uint32_t start = DWT->CYCCNT;
const uint32_t target = microseconds * cycles_per_us;
while ((DWT->CYCCNT - start) < target) {
/* CYCCNT wraps cleanly modulo 2^32 — subtraction stays correct. */
}
}
+36 -1
View File
@@ -17,7 +17,12 @@ module cdc_adc_to_processing #(
input wire [WIDTH-1:0] src_data,
input wire src_valid,
output wire [WIDTH-1:0] dst_data,
output wire dst_valid
output wire dst_valid,
// Audit F-1.2: overrun pulse in src_clk domain. Asserts for 1 src cycle
// whenever src_valid fires while the previous sample has not yet been
// acknowledged by the destination edge-detector (i.e., the transaction
// the CDC is silently dropping). Hold/count externally.
output wire overrun
`ifdef FORMAL
,output wire [WIDTH-1:0] fv_src_data_reg,
output wire [1:0] fv_src_toggle
@@ -130,6 +135,36 @@ module cdc_adc_to_processing #(
assign dst_data = dst_data_reg;
assign dst_valid = dst_valid_reg;
// ------------------------------------------------------------------
// Audit F-1.2: overrun detection
//
// The src-side `src_toggle` counter flips on each latched src_valid.
// We feed back a 1-bit "ack" toggle from the dst domain (flipped each
// time dst_valid fires) through a STAGES-deep synchronizer into the
// src domain. If a new src_valid arrives while src_toggle[0] already
// differs from the acked value, the previous sample is still in flight
// and this new latch drops it. Emit a 1-cycle overrun pulse.
// ------------------------------------------------------------------
reg dst_ack_toggle;
always @(posedge dst_clk) begin
if (!dst_reset_n) dst_ack_toggle <= 1'b0;
else if (dst_valid_reg) dst_ack_toggle <= ~dst_ack_toggle;
end
(* ASYNC_REG = "TRUE" *) reg [STAGES-1:0] ack_sync_chain;
always @(posedge src_clk) begin
if (!src_reset_n) ack_sync_chain <= {STAGES{1'b0}};
else ack_sync_chain <= {ack_sync_chain[STAGES-2:0], dst_ack_toggle};
end
wire ack_in_src = ack_sync_chain[STAGES-1];
reg overrun_r;
always @(posedge src_clk) begin
if (!src_reset_n) overrun_r <= 1'b0;
else overrun_r <= src_valid && (src_toggle[0] != ack_in_src);
end
assign overrun = overrun_r;
`ifdef FORMAL
assign fv_src_data_reg = src_data_reg;
assign fv_src_toggle = src_toggle;
@@ -47,8 +47,16 @@ set_max_delay -datapath_only -from [get_clocks clk_mmcm_out0] \
set_false_path -from [get_clocks clk_100m] -to [get_clocks clk_mmcm_out0]
set_false_path -from [get_clocks clk_mmcm_out0] -to [get_clocks clk_100m]
set_false_path -from [get_clocks clk_mmcm_out0] -to [get_clocks ft601_clk_in]
set_false_path -from [get_clocks ft601_clk_in] -to [get_clocks clk_mmcm_out0]
# Audit F-0.6: the clock name `ft601_clk_in` does not exist on either 50T
# (FT2232H, clock is `ft_clkout`) or 200T builds in the current RTL. Waive
# against whichever USB-domain clock is actually defined in the build.
foreach _usb_clk {ft_clkout ft601_clk ft601_clk_in} {
if {[llength [get_clocks -quiet $_usb_clk]] > 0} {
set_false_path -from [get_clocks clk_mmcm_out0] -to [get_clocks $_usb_clk]
set_false_path -from [get_clocks $_usb_clk] -to [get_clocks clk_mmcm_out0]
}
}
unset _usb_clk
set_false_path -from [get_clocks clk_mmcm_out0] -to [get_clocks clk_120m_dac]
set_false_path -from [get_clocks clk_120m_dac] -to [get_clocks clk_mmcm_out0]
@@ -59,7 +67,10 @@ set_false_path -from [get_clocks clk_120m_dac] -to [get_clocks clk_mmcm_out0]
# LOCKED is not a valid timing startpoint (it's a combinational output of the
# MMCM primitive). Use -through instead of -from to waive all paths that pass
# through the LOCKED net. This avoids the CRITICAL WARNING from Build 19/20.
set_false_path -through [get_pins rx_inst/adc/mmcm_inst/mmcm_adc_400m/LOCKED]
# Audit F-0.7: the literal hierarchical path was missing the `u_core/`
# prefix and silently matched no pins. Use a hierarchical wildcard to
# catch the MMCM LOCKED pin regardless of wrapper hierarchy.
set_false_path -through [get_pins -hierarchical -filter {REF_PIN_NAME == LOCKED}]
# --------------------------------------------------------------------------
# Hold waiver for source-synchronous ADC capture (BUFIO-clocked IDDR)
@@ -107,8 +107,15 @@ set_property PACKAGE_PIN C4 [get_ports {ft_clkout}]
set_property IOSTANDARD LVCMOS33 [get_ports {ft_clkout}]
create_clock -name ft_clkout -period 16.667 [get_ports {ft_clkout}]
set_input_jitter [get_clocks ft_clkout] 0.2
# N-type MRCC pin requires dedicated route override (Place 30-876)
set_property CLOCK_DEDICATED_ROUTE FALSE [get_nets {ft_clkout_IBUF}]
# N-type MRCC pin requires dedicated route override (Place 30-876).
# Audit F-0.4: the literal net name `ft_clkout_IBUF` exists post-synth but
# the XDC scan happens before synthesis, when the IBUF net does not yet
# exist — Vivado reported `No nets matched 'ft_clkout_IBUF'` + CRITICAL
# WARNING. Use -hierarchical -filter + -quiet so the constraint matches
# post-synth without warning during pre-synth XDC scan. The TCL duplicate
# at scripts/50t/build_50t.tcl:119 remains as belt-and-suspenders.
set_property -quiet CLOCK_DEDICATED_ROUTE FALSE \
[get_nets -quiet -hierarchical -filter {NAME =~ *ft_clkout_IBUF}]
# ============================================================================
# RESET (Active-Low)
@@ -408,7 +415,17 @@ set_false_path -from [get_ports {stm32_mixers_enable}]
# - Reset deassertion order is not functionally critical — all registers
# come out of reset within a few cycles of each other
# --------------------------------------------------------------------------
set_false_path -from [get_cells reset_sync_reg[*]] -to [get_pins -filter {REF_PIN_NAME == CLR} -of_objects [get_cells -hierarchical -filter {PRIMITIVE_TYPE =~ REGISTER.*.*}]]
# Audit F-0.5: the literal cell name `reset_sync_reg[*]` does not match any
# cell in the post-synth netlist. The actual sync regs are
# `u_core/reset_sync_reg[0..1]`, `u_core/rx_inst/ddc/reset_sync_400m_reg[*]`,
# `u_core/gen_ft2232h.usb_inst/ft_reset_sync_reg[*]`, and peers under
# `u_core/reset_sync_120m_reg[*]`, `u_core/reset_sync_ft601_reg[*]`,
# `u_core/rx_inst/adc/reset_sync_400m_reg[*]`. The waiver below covers all
# of them by matching any register whose name contains `reset_sync`.
# Without this, STA runs recovery/removal on the fanout of each sync-chain
# output register (up to ~1000 loads pre-PR#113 replication).
set_false_path -from [get_cells -hierarchical -filter {NAME =~ *reset_sync*_reg*}] \
-to [get_pins -hierarchical -filter {REF_PIN_NAME == CLR || REF_PIN_NAME == PRE}]
# --------------------------------------------------------------------------
# Clock Domain Crossing false paths
+27 -10
View File
@@ -25,7 +25,10 @@ module ddc_400m_enhanced (
input wire reset_monitors,
output wire [31:0] debug_sample_count,
output wire [17:0] debug_internal_i,
output wire [17:0] debug_internal_q
output wire [17:0] debug_internal_q,
// Audit F-1.2: sticky CICFIR CDC overrun flag (clk_400m domain). Goes
// high on the first dropped sample and stays high until reset_monitors.
output wire cdc_cic_fir_overrun
);
// Parameters for numerical precision
@@ -148,22 +151,20 @@ always @(posedge clk_400m or negedge reset_n) begin
end
end
// CDC synchronization for control signals (2-stage synchronizers)
(* ASYNC_REG = "TRUE" *) reg [1:0] mixers_enable_sync_chain;
// CDC synchronization for control signals (2-stage synchronizers).
// Audit F-1.3: the mixers_enable synchronizer was dead its _sync output
// was never consumed (the NCO phase_valid uses the raw port), and the only
// caller (radar_receiver_final.v) ties the port to 1'b1. Removed.
(* ASYNC_REG = "TRUE" *) reg [1:0] force_saturation_sync_chain;
wire mixers_enable_sync;
wire force_saturation_sync;
assign mixers_enable_sync = mixers_enable_sync_chain[1];
assign force_saturation_sync = force_saturation_sync_chain[1];
// Sync reset via reset_400m (replicated, max_fanout=50). Was async on
// reset_n_400m see "400 MHz RESET DISTRIBUTION" comment above.
always @(posedge clk_400m) begin
if (reset_400m) begin
mixers_enable_sync_chain <= 2'b00;
force_saturation_sync_chain <= 2'b00;
end else begin
mixers_enable_sync_chain <= {mixers_enable_sync_chain[0], mixers_enable};
force_saturation_sync_chain <= {force_saturation_sync_chain[0], force_saturation};
end
end
@@ -600,7 +601,10 @@ assign cic_valid = cic_valid_i & cic_valid_q;
wire fir_in_valid_i, fir_in_valid_q;
wire fir_valid_i, fir_valid_q;
wire fir_i_ready, fir_q_ready;
wire [17:0] fir_d_in_i, fir_d_in_q;
wire [17:0] fir_d_in_i, fir_d_in_q;
// Audit F-1.2: per-lane CICFIR CDC overrun pulses (clk_400m domain)
wire cdc_fir_i_overrun;
wire cdc_fir_q_overrun;
cdc_adc_to_processing #(
.WIDTH(18),
@@ -613,7 +617,8 @@ cdc_adc_to_processing #(
.src_data(cic_i_out),
.src_valid(cic_valid_i),
.dst_data(fir_d_in_i),
.dst_valid(fir_in_valid_i)
.dst_valid(fir_in_valid_i),
.overrun(cdc_fir_i_overrun)
);
cdc_adc_to_processing #(
@@ -627,9 +632,21 @@ cdc_adc_to_processing #(
.src_data(cic_q_out),
.src_valid(cic_valid_q),
.dst_data(fir_d_in_q),
.dst_valid(fir_in_valid_q)
.dst_valid(fir_in_valid_q),
.overrun(cdc_fir_q_overrun)
);
// Audit F-1.2: sticky-latch the two per-lane overrun pulses in the 400 MHz
// domain and expose a single module-level flag. Cleared only by
// reset_monitors (or reset_n via reset_400m), matching the other DDC
// diagnostic latches (overflow/saturation).
reg cdc_cic_fir_overrun_sticky;
always @(posedge clk_400m) begin
if (reset_400m || reset_monitors) cdc_cic_fir_overrun_sticky <= 1'b0;
else if (cdc_fir_i_overrun || cdc_fir_q_overrun) cdc_cic_fir_overrun_sticky <= 1'b1;
end
assign cdc_cic_fir_overrun = cdc_cic_fir_overrun_sticky;
// ============================================================================
// FIR Filter Instances
// ============================================================================
+16 -3
View File
@@ -83,7 +83,19 @@ module radar_receiver_final (
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
output wire [7:0] mti_saturation_count_out,
// Range-bin decimator watchdog (audit F-6.4 previously tied off
// with an ILA-only note). A high pulse here means the decimator
// FSM has not seen the expected number of input samples within
// its timeout window, i.e. the upstream FIR/CDC has stalled.
output wire range_decim_watchdog,
// Audit F-1.2: sticky CICFIR CDC overrun flag. Asserts on the first
// silent sample drop between the 400 MHz CIC output and the 100 MHz
// FIR input; stays high until the next reset. OR'd into the GPIO
// diagnostic bit at the top level.
output wire ddc_cic_fir_overrun
);
// ========== INTERNAL SIGNALS ==========
@@ -254,7 +266,8 @@ ddc_400m_enhanced ddc(
.reset_monitors(1'b0),
.debug_sample_count(),
.debug_internal_i(),
.debug_internal_q()
.debug_internal_q(),
.cdc_cic_fir_overrun(ddc_cic_fir_overrun)
);
assign ddc_overflow_any = ddc_mixer_saturation | ddc_filter_overflow;
@@ -404,7 +417,7 @@ range_bin_decimator #(
.range_bin_index(decimated_range_bin),
.decimation_mode(2'b01), // Peak detection mode
.start_bin(10'd0),
.watchdog_timeout() // Diagnostic unconnected (monitored via ILA if needed)
.watchdog_timeout(range_decim_watchdog) // Audit F-6.4 plumbed out
);
// ========== MTI CANCELLER (Ground Clutter Removal) ==========
+25 -2
View File
@@ -205,6 +205,11 @@ 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;
// Range-bin decimator watchdog (audit F-6.4). High = decimator stalled.
wire rx_range_decim_watchdog;
// CICFIR CDC overrun sticky (audit F-1.2). High = at least one baseband
// sample has been silently dropped between the 400 MHz CIC and 100 MHz FIR.
wire rx_ddc_cic_fir_overrun;
// Data packing for USB
wire [31:0] usb_range_profile;
@@ -575,7 +580,10 @@ radar_receiver_final rx_inst (
.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)
.mti_saturation_count_out(rx_mti_saturation_count),
// Range-bin decimator watchdog (audit F-6.4)
.range_decim_watchdog(rx_range_decim_watchdog),
.ddc_cic_fir_overrun(rx_ddc_cic_fir_overrun)
);
// ============================================================================
@@ -884,6 +892,19 @@ endgenerate
// we simply sample them in clk_100m when the CDC'd pulse arrives.
// Step 1: Toggle on cmd_valid pulse (ft601_clk domain)
//
// CDC INVARIANT (audit F-1.1): usb_cmd_opcode / usb_cmd_addr / usb_cmd_value
// / usb_cmd_data MUST be driven to their final values BEFORE usb_cmd_valid
// asserts, and held stable for at least (STAGES + 1) clk_100m cycles after
// (i.e., until cmd_valid_100m has pulsed in the destination domain). These
// buses cross from ft601_clk to clk_100m as quasi-static data, NOT through
// a synchronizer — only the toggle bit above is CDC'd. If a future edit
// moves the cmd_* register write to the SAME cycle as the toggle flip, or
// drops the stability hold, the clk_100m sampler at the command decoder
// will latch metastable bits and dispatch on a garbage opcode.
// The source-side FSM in usb_data_interface_ft2232h.v / usb_data_interface.v
// currently satisfies this by assigning the cmd_* buses several cycles
// before pulsing cmd_valid and leaving them stable until the next command.
reg cmd_valid_toggle_ft601;
always @(posedge ft601_clk_buf or negedge sys_reset_ft601_n) begin
if (!sys_reset_ft601_n)
@@ -1059,7 +1080,9 @@ assign system_status = status_reg;
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);
| (rx_mti_saturation_count != 8'd0)
| rx_range_decim_watchdog // audit F-6.4
| rx_ddc_cic_fir_overrun; // audit F-1.2
assign gpio_dig6 = host_agc_enable;
assign gpio_dig7 = 1'b0;