Interval measurement in Verilog

Question

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
```

Answer 1

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.

Answer 2

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