1
\$\begingroup\$

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
```
\$\endgroup\$
  • 2
    \$\begingroup\$ 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. \$\endgroup\$ – doynax Feb 23 at 12:05
  • 1
    \$\begingroup\$ 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. \$\endgroup\$ – Dave Tweed Feb 23 at 13:03
0
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ 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. \$\endgroup\$ – David Given Feb 23 at 17:15
  • \$\begingroup\$ 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. \$\endgroup\$ – David Given Feb 23 at 17:24
  • \$\begingroup\$ 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. \$\endgroup\$ – David Given Feb 23 at 17:29
  • \$\begingroup\$ @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. \$\endgroup\$ – doynax Feb 23 at 18:15
  • \$\begingroup\$ 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. \$\endgroup\$ – David Given Feb 23 at 18:19
0
\$\begingroup\$

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
\$\endgroup\$
  • \$\begingroup\$ 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. \$\endgroup\$ – David Given Feb 25 at 21:16
  • \$\begingroup\$ 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. \$\endgroup\$ – Dave Tweed Feb 26 at 0:17
  • \$\begingroup\$ I don't get notified when the register gets read, and there is no DMA acknowledge. \$\endgroup\$ – David Given Feb 26 at 10:46

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.