0
\$\begingroup\$

I'm experimenting with FPGA (I'm using the Mojo board but I don't think is essential), and I wrote some code to add a counter each time a button is pressed (the counter is assigned to the 8 leds in the board)

The code is below (button_conditioner is simply a debouncer and shouldn't matter, BTW the code of this project is here)

module mojo_top(
    // 50MHz clock input
    input clk,
    // Input from reset button (active low)
    input rst_n,
    // cclk input from AVR, high when AVR is ready
    input cclk,
    // Outputs to the 8 onboard LEDs
    output[7:0]led,
    // AVR SPI connections
    output spi_miso,
    input spi_ss,
    input spi_mosi,
    input spi_sck,
    // AVR ADC channel select
    output [3:0] spi_channel,
    // Serial connections
    input avr_tx, // AVR Tx => FPGA Rx
    output avr_rx, // AVR Rx => FPGA Tx
    input avr_rx_busy, // AVR Rx buffer full
    input button
);


wire rst = ~rst_n; // make reset active high

// these signals should be high-z when not used
assign spi_miso = 1'bz;
assign avr_rx = 1'bz;
assign spi_channel = 4'bzzzz;

wire btn_out;
reg [7:0] led_r;

assign led = led_r;

button_conditioner btn(
  .clk(clk),
  .btn(button),
  .out(btn_out)
);

always @(posedge rst or posedge btn_out) begin
  if (rst)
    led_r <= 0;
  else if (btn_out == 1'b0) // <=== THIS IS THE WTF LINE
    led_r <= led_r + 1;  
end

What puzzles me is that the counter is increased only checking for the signal equal to zero but the condition is on posedge i.e. a transition from 0 to 1: so I would expect a check with 1'b1, what I'm missing?

\$\endgroup\$
2
  • \$\begingroup\$ You may tag with FPGA in addition to attract more readers. \$\endgroup\$
    – Andreas
    Oct 28, 2016 at 9:37
  • \$\begingroup\$ @Andreas good idea \$\endgroup\$
    – gipi
    Oct 28, 2016 at 10:23

3 Answers 3

1
\$\begingroup\$

Thinking about it in terms of hardware, what value is there at the input to the led_r inferred D-flip-flop at the moment when there is a rising edge that clocks it? Basically you don't know because its input is a rising edge signal (btn_out). It will be somewhere between 0 and 1, though it will probably still be 0 once you factor in propagation delays.

It's a very badly written piece of code (using logic as a clock, probably causing metastability, etc.), and I'm not sure what it is supposed to do. Having said that, if I were to make a guess I'd say it should be simply else, not else if(). You don't need to check it with anything because you know it is a rising edge by virtue of the fact that it will be clocked by the rising edge you are looking for.

\$\endgroup\$
0
\$\begingroup\$

Your button_conditioner may well be the problem. You are using its output as a clock, and clocks need to be free from glitches. I cannot see if thats the case for your design.

Clocks need special treatment: Never gate them with logic, always connect that logic to clock enable. In a good design, all your blocks read always @(posedge rst or posedge clk). All clks (you may well have more than one) should come from designated clock inputs or internal PLLs, so the design software routes them on designated clock networks and minimizes (and analyzes) skew.

In a synchronous design, signals are steady and well-defined at the (rising) clock edge. In between, all kinds of stuff can happen. Signals may change multiple times if their inputs have different propagation delays ("glitches"). That does no harm as long as everything has settled when the clock comes.

The clock must not have additional rising edges during that settle time, as that will trigger registers to record an unsettled (undefined) state. Unwanted rising edges are easily introduced by combining or gating the clock with other logic signals.

For your design, you should introduce a new register btn_out_delayed and do btn_out_delayed <= btn_out in every clock cycle. Then you can do if (btn_out & ~btn_out_delayed) { led_r <= led_r + 1 } to only get one increase for each press. This will typically be inferred to clock enable.

Your software should also include some viewer to check the synthesis result. Its always a good idea to look into that for every small module, many errors can be detected there (typical case: whole module optimized away and replaced with constant driver).

\$\endgroup\$
0
\$\begingroup\$

I'm sure if you read the verilog language spec this is well defined (but I failed to find it). If you had a more complex design where several signals were changing at once and being used in a clocked process, you would certainly want to act on the signal states existing before the clock edge in order to chose your actions. This is similar to asking why you need to use non-blocking assignments when you infer a flop.

When the clock transitions is an ambiguous statement in English. It can read as when the clock is zero and needs to go high, or as once the clock is high after being changed.

Your logic is unchanged if you remove the if (btn_out). By definition, this signal is low unless the reset has been asserted. To make it more obvious, consider if you had written if (led_r == 8'h00) and needed to see the first cycle in your evaluation.

\$\endgroup\$
6
  • 1
    \$\begingroup\$ I would expect that posedge triggers the block when changes from 0 to 1 and is now 1, otherwise the rst check shouldn't work as expected. \$\endgroup\$
    – gipi
    Oct 28, 2016 at 8:00
  • 1
    \$\begingroup\$ Hmmm...Maybe there is something more subtle going on in the spec. which makes the reset term special. \$\endgroup\$ Oct 28, 2016 at 8:06
  • \$\begingroup\$ @gipi, are you observing this behaviour in simulation, or synthesis? FPGA is a slightly constrained synthesis target, so what you are observing here might be the result of propogation delays rather than the interpretation of the HDL. \$\endgroup\$ Oct 28, 2016 at 9:32
  • \$\begingroup\$ Formal Simulation seems to agree with your observation, so I'm confused... \$\endgroup\$ Oct 28, 2016 at 10:52
  • \$\begingroup\$ I'm observing this in synthesis (I mean, I load the binary on the FPGA and press the button :)) @sean-houlihane in your formal simulation are you including also the button_conditioner module? \$\endgroup\$
    – gipi
    Oct 28, 2016 at 11:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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