# Interval measurement in Verilog

I am writing my Very First Verilog Program Ever: I'm trying to capture intervals between incoming pulses. I have a sample clock, an asynchronous pulse source, and the output is a registered 8-bit byte and an interrupt line; every time a pulse arrives, I assert the interval in the range 0x00-0x7f on the output and twiddle the interrupt, and a microcontroller DMAs it into memory. (The context.)

That bit works fine. The complication is that if a pulse doesn't arrive by the time my internal counter rolls over, I emit a 0x80 byte. The intention is that long intervals are to be represented as multiple bytes.

What I am finding is that intervals in the 0x00-0x7f range work fine, but longer intervals emit complete garbage. The logic looks very simple to me, so there's obviously something I'm not getting in the way Verilog works. Anything here look obviously wrong?

My constraints are:

• the input signal is not aligned with the sample clock, and each signal may span multiple clock ticks. I only care about the rising edge.

• interrupts are edge-triggered but there's a delay between the interrupt and the DMA read. I do appear to need the latch on result to ensure the result remains asserted.

• I know there's no reset, I'll do that later.

• this is on a Cypress PSoC5LP development kit, which uses a cut-down version of Warp Verilog. I did try to use EDA Playground to simulate it, and it all seems to work there, but it doesn't work in real life.

• in real life, my sample clock is 12MHz and an input pulse can be anything from 150ns to 500ns.

• there may actually be nothing wrong and I'm hitting hardware constraints in my pulse generator. Which isn't confusing at all.

• Update: my hardware can't generate pulses closer together than about 750ns, which is about nine sample clocks, so I don't think I need to worry about pulses coming in in successive clocks. Update update: no, that's rubbish. Pulses 0x81 ticks apart will need to generate two bytes, 0x80 and 0x01, one sample clock apart.

module Sampler (
output reg interrupt,    // a pulse has been detected
output reg [7:0] result, // output data byte
input clock,             // sample clock
input sample             // incoming signal
);

reg [6:0] counter;   // monotonically increasing counter
reg last_sample;     // for sample edge detection

always @(posedge clock)
begin
if (counter == 0)
begin
// Rollover.
result = 8'h80;
interrupt = 1;
counter = 1;
end
else if (sample && !last_sample)
begin
// A sample happened since the last clock.
result[6:0] = counter;
result[7] = 0;
interrupt = 1;
counter = 1;
end
else
begin
counter = counter + 1;
interrupt = 0;
end
last_sample = sample;
end
$$$$

• Can you elaborate a little on how you have determined that the code fails in practice? May the problem lie in DMA/buffer integration code omitted from the sample above? As you've noted the DMA latency is significant so I would suggest a datapath FIFO to latch the result on the same cycle. Oh, and you seem to have a race between the roll-over and a flank on the same cycle. Feb 23 '19 at 12:05
• Define "complete garbage". There's nothing obviously wrong with your code, but there are a couple of caveats: First, did you run sample through a synchronizer first? Any state machine needs to have its inputs synchronized to its own clock. Second, if you have intervals that are just a little longer than 0x80 counts, you'll get two interrupts essentially back-to-back. Can your CPU deal with this correctly? You might need a small FIFO to buffer these events. Feb 23 '19 at 13:03

There are a few "obvious" problems here, try these first.

1. You are using blocking assignments (=) in sequential logic, where you should be using non-blocking (<=) assignment. Have a look at this question here.
2. You have no reset mechanism as you've pointed out. This means that variables are undefined (at times), so your if-else structure may not work the way you think it will. For example on the very first edge, what will the result of counter == 0 be?
3. You do not resynchronise your pulses. You should convert your input "pulse", which may actually look like a level as it's asynchronous, into a pulse in the local clock domain. For example, if your driving domain is clocked at 10 MHz, but your receiving domain is clocked at 100 MHz, a single cycle pulse in one domain is a 10-cycle long pulse in the other.
4. Your sampling frequency is too low. A 150 ns pulse is 6.66 MHz, which means you need a clock of at least (6.66 * 2) MHz to reliably capture your inputs. EDIT: This is to reconstruct, rather than capture, so you should be ok.
5. As mentioned in the comments, there's a problem if the overflow happens, then the next cycle the pulse comes in. You should ideally store your outputs in a FIFO that is drained by the system when it can.
6. This sort of system is perfect for a state machine. This way, you can drive your outputs based on the state of the system - it's a form that's less liable to error**. For example, see below. Note, the sample_sync_i input comes from another module, will leave you to implement that :)

typedef enum bits [1:0] {
IDLE  = 2'b00,
COUNT = 2'b01,
OVF   = 2'b10,
LATCH = 2'b11,
} fsm_t;

module counter (
input  logic       clk          ,
input  logic       reset_n      ,
input  logic       sample_sync_i,
output logic [7:0] result_o     ,
output logic       interrupt_o
);

fsm_t fsm, fsm_nxt;

logic [6:0] counter      ;
logic [6:0] counter_nxt  ;
logic [7:0] result_nxt   ;
logic       overflow     ;
logic       interrupt_nxt;

// Move to next FSM state based on inputs
always_comb begin
case (fsm)
IDLE  : fsm_nxt = sample_sync_i ? COUNT : IDLE;
COUNT : fsm_nxt = sample_sync_i ? LATCH : (overflow ? OVF : COUNT);
OVF   : fsm_nxt = sample_sync_i ? LATCH : COUNT;
LATCH : fsm_nxt = COUNT;
endcase
end

// Update FSM flops
always_ff @(posedge clk or negedge reset_n) begin
if (~reset_n) begin
fsm <= IDLE;
end else begin
fsm <= fsm_nxt;
end
end

// outputs depend on FSM
always_comb begin
interrupt_nxt = 1'b0;
counter_nxt = counter;
case (fsm_nxt)
COUNT : begin
counter_nxt = counter + 1'b1;
overflow = (counter_nxt == 0);
end
OVF : begin
counter_nxt = counter + 1'b1;
interrupt_nxt = 1'b1;
end
LATCH : begin
counter_nxt = 7'b000_0001;
interrupt_nxt = 1'b1;
end
endcase
end

assign result_nxt[7]   = overflow;
assign result_nxt[6:0] = counter;

// drive FFs from FSM
always_ff @(posedge clk or negedge reset_n) begin
if (~reset_n) begin
result_o    <= {8{1'b0}};
interrupt_o <= 1'b0;
counter     <= 7'b000_0001;
end else begin
result_o    <= result_nxt;
interrupt_o <= interrupt_nxt;
counter     <= counter_nxt;
end
end

endmodule : counter


** - so, I said it's less prone to error, but I haven't simulated this! If anyone spots anything, I won't be offended.

• Re (1): yep --- I had actually read the guidelines but managed to get combinatorial and sequential the wrong way round. Re (2): I expect all variables to be fully defined once the second sample arrives (causing the state machine to reset). Is this wrong? Re (3): isn't this taken care of by the sample/last_sample mechanism? Every clock positive edge I, well, sample sample and trigger the logic only if it rose since the last positive edge. Why would I need additional synchronisation? Re (4), (5): yup, fair enough. Feb 23 '19 at 17:15
• Re (6): The PSoC Verilog dialect is apparently limited compared to 'real' Verilog, and I don't think it's got always_comb and always_ff. I can see the advantages, though --- in fact, I think the PSoC has built-in hardware for implementating state machines. Feb 23 '19 at 17:24
• Re FIFOs: I don't actually think I've got one (unless I want to write it myself in Verilog). Which is strange. But there's a limitation in the hardware in that pulses can't happen closer together than about 750ns, which is about nine ticks, so I don't think I need to worry about races there. Feb 23 '19 at 17:29
• @David Given: The datapath fixed function blocks each have two 4-level 8-bit FIFOs, and allows you to use wider DMA transactions. The DMA engine has rather high worst-case latency even at maximum priority, with each transaction requiring many internal sub-cycles to process descriptors and perform arbitration, and is unlikely to keep up with bursts of individual 9-cycle byte-wide transfers. Feb 23 '19 at 18:15
• Re FIFOs: ...eeeeexcept I do have to worry about 0x81 tick intervals, which will show up as two consecutive bytes one tick apart. So I very much do need a FIFO. Curses. Also, bear in mind that my sampler clock runs at 12MHz, but the actual MCU is running considerably faster. I'll try cranking it higher and see if that helps. Feb 23 '19 at 18:19

Pulses 0x81 ticks apart will need to generate two bytes, 0x80 and 0x01, one sample clock apart.

Indeed. In fact, pulses 0x80 ticks apart ALSO need to generate two bytes, 0x80 and 0x00.

And given that there's some latency between generating an interrupt and reading the data byte, it would be a good idea to include an explicit interrupt-acknowledge signal into the design.

I have a fondness for retro projects — thanks for including the context link! — so I put some effort into this. Here's the design that I came up with. You do need a state machine, with a minimum of four states:

• Counting - just waiting for either an input edge or a counter overflow.
• Overflow - the counter has overflowed and we've generated an interrupt. Now we're waiting for either the interrupt acknowledge or an actual input edge.
• Both - an input edge came along either simultaneous with or just after a counter overflow. We need to generate a second interrupt after the first one gets acknowledged.
• Interrupt - an input edge caused an interrupt, and we're waiting for the acknowledge.

Here's the Verilog implementation. Note that counter_hold and result constitute a simple 2-stage FIFO that allows back-to-back interrupts to be generated correctly.

/* pulse_interval.v */

/* This module uses a 12 MHz clock to measure the intervals between rising
* edges of an asynchronous input. The intervals are reported to a CPU though
* a series of one-byte messages: values 0x00 through 0x7F represent pulse
* measurements, while the value 0x80 represents an overflow (no pulse for 128
* clock periods.
*
* The CPU has a limited interrupt bandwidth, so we require it to acknowledge
* each interrupt. The minimum pulse interval is approximately 9 clocks. We
* assume that the CPU can handle an interrupt within 9 clocks, but this still
* means that any intervals in the range 0x80 to 0x89 need to generate two
* interrupts that both need to be handled correctly.
*
* Note that both int_req and int_ack are edge-based -- a rising edge on
* int_req represents a new interrupt to the CPU, and a rising edge on int_ack
* represents an acknowledgement of the previous interrupt.
*/
module pulse_interval (
input                 pulse_in,
output reg      [7:0] result,
output reg            int_req,
input                 int_ack,
input                 clock,
input                 reset
);

/* Synchronize the inputs and detect rising edges
*/
reg           pulse_a;
reg           pulse_b;

reg           int_ack_a;
reg           int_ack_b;

always @(posedge clock) begin
pulse_a <= pulse_in;
pulse_b <= pulse_a;

int_ack_a <= int_ack;
int_ack_b <= int_ack_a;
end

wire          pulse_edge = pulse_a & !pulse_b;
wire          int_ack_edge = int_ack_a & !int_ack_b;

/* State machine
*/
reg     [7:0] counter;
reg     [7:0] counter_hold;
reg     [1:0] state;

wire          overflow = counter[7];

parameter
ST_COUNT = 0,
ST_OVER  = 1,
ST_BOTH  = 2,
ST_INT   = 3;

always @(posedge clock) begin
if (reset) begin
counter <= 0;
int_req <= 0;
state <= ST_COUNT;
end else begin
case (state)

/* Counting, waiting for either an input edge or an overflow.
* Note that if an overflow occurs, we reset the counter to 1,
* because we've already counted the "0" state of the next cycle.
*/
ST_COUNT: begin
if (pulse_edge && !overflow) begin
counter <= 0;
result <= counter;
int_req <= 1;
state <= ST_INT;
end else if (pulse_edge && overflow) begin
counter <= 1;
counter_hold <= 0;
result <= 8'h80;
int_req <= 1;
state <= ST_BOTH;
end else if (!pulse_edge && overflow) begin
counter <= 1;
result <= 8'h80;
int_req <= 1;
state <= ST_OVER;
end else begin
counter <= counter + 1;
end
end

/* Overflow, waiting for int_ack and/or pulse_edge
* We assume another overflow can't occur before the int_ack.
*/
ST_OVER: begin
if (pulse_edge && !int_ack_edge) begin
counter <= 0;
counter_hold <= counter;
state <= ST_BOTH;
end else if (pulse_edge && int_ack_edge) begin
counter <= 0;
result <= counter;
int_req <= 0;
state <= ST_INT;
end else if (!pulse_edge && int_ack_edge) begin
counter <= counter + 1;
int_req <= 0;
state <= ST_COUNT;
end else begin
counter <= counter + 1;
end
end

/* Back-to-back events, waiting for int_ack on overflow before sending
* edge event. We assume no new events can occur during this time.
*/
ST_BOTH: begin
counter <= counter + 1;
if (int_ack_edge) begin
int_req <= 0;
result <= counter_hold;
state <= ST_INT;
end
end

/* Edge event, waiting for int_ack.
* We assume overflow can't occur before the int_ack.
*/
ST_INT: begin
counter <= counter + 1;
if (int_ack_edge) begin
int_req <= 0;
state <= ST_COUNT;
end else begin
/* We set this here to create new edge in case we just came from
* ST_OVER or ST_BOTH.
*/
int_req <= 1;
end
end

endcase
end
end
endmodule
`
• Yes, that's basically what I'm heading towards... unfortunately, there's a problem; I don't have a way to acknowledge interrupts! The module output interrupt line is in fact connected to a DMA controller drq line; this causes a read from a register attached to the data output. It doesn't generate acknowledgments, and neither does the status register. So, I don't think this problem is actually soluble inside the Verilog domain at all and I'll have to go with a proprietary Cypress UDB solution. Feb 25 '19 at 21:16
• You could use the read of the register itself as the interrupt acknowledge (or DMA acknowledge, which is essentially the same thing). Or you could just assume that every interrupt gets handled within 9 clock periods and dispense with an explicit acknowledge. You'd just need another 4-bit counter to implement that. Feb 26 '19 at 0:17
• I don't get notified when the register gets read, and there is no DMA acknowledge. Feb 26 '19 at 10:46