-
Notifications
You must be signed in to change notification settings - Fork 39
Description
Hi,
I might have found a bug in the debounce logic for asg/external trigger.
My understanding of the intended functionality is that once an external trigger arrives, a debounce counter is started. If the external trigger remains active after the debounce period is over, it propagates the trigger further to the scope. However, I've noticed that the delay that should be introduced by the debounce period is not present. After some digging I think I've identified the bug in rp_ext_trig.v (I suspect that this may not be a problem for the asg trigger due to hystersis, but the logic is the same):
always @(posedge adc_clk_i)
begin
asg_trig_in <= {asg_trig_in[1:0],trig_asg_i} ;
// look for input changes
if ((asg_trig_debp == 20'h0) && (asg_trig_in[1] && !asg_trig_in[2])) // CONDITION A: START DEBOUNCE COUNTER (OK)
asg_trig_debp <= set_deb_len_i ; // ~0.5ms
else if (asg_trig_debp != 20'h0)
asg_trig_debp <= asg_trig_debp - 20'd1 ;
// update output values
asg_trig_dp[1] <= asg_trig_dp[0] ;
if (asg_trig_debp == 20'h0) // <--- CONDITION B: EVALUATES TO TRUE AT THE SAME TIME AS CONDITION A (NOK)
asg_trig_dp[0] <= asg_trig_in[1] ;
end
assign ext_trig_p_o = (ext_trig_dp == 2'b01) ;
The problem is that wile condition B is supposed to be met only after the debounce counter finishes, it actually triggers right afterthe pulse arrives, together with condition A. This means that:
- glitchy trigger pulses are not filtered out
- multiple pulses can arrive within the debounce period
- longer trigger pulses are propagated without debounce delay
I believe that the fix is to change condition B to trigger when the trigger reaches one instead of zero: if (asg_trig_debp == 20'h1).
Can you please confirm that my understanding of the debounce semantics is corect? If so, let me know if you'll be able to fix it or whether you'd like me to create a pull request (seems like an overkill but can do). Attached is a testbench and screenshots of resulting waveforms

