1
\$\begingroup\$

I currently have a Spartan-6 FPGA in a Digilent Nexus 3 board. I am using Xilinx 14.6 Project Navigator to write the code and program the FPGA.

My code for the top (and only) module is the following:

21 module blinker(
22 input clk,
23 output reg led_state
24 );
25   
26 reg [31:0] count;
27 wire count_max = 32'd50000000;
28
29 assign count_next = (count_next > count_max) ? 32'd00000000 : count + 32'd00000001;
30 assign led_next = (count == count_max) ? ~led_state : led_state;
31
32 always @(posedge clk)
33 begin
34    count <= count_next;
35    led_state <= led_next;
36 end
37
38 endmodule

According to the Nexus 3 reference manual, the board has a 100MHz Oscillator at pin V10, and so in my User Constraints File, I mapped the V10 pin to the clk input signal of my module.

Since the oscillator is at 100MHz, I set count_max to be 50,000,000, so that the led_state should invert every 50 x 10^6 clk edges, or every half-second assuming 100MHz.

When I attempted Synthesis using XST in the Project Navigator, I get these warning messages:

WARNING:HDLCompiler:413 - "C:\Users\Public\Documents\Xilinx Projects\blinker\blinker.v" Line 29: Result of 32-bit expression is truncated to fit in 1-bit target.
WARNING:Xst:2404 -  FFs/Latches <count<31:1>> (without init value) have a constant value of 0 in block <blinker>.
WARNING:Xst:2170 - Unit blinker : the following signal(s) form a combinatorial loop: n0011<0>.

However, I am still able to go ahead and build the .bit file and program the Nexus 3. The result, however, is that the LED, which I designated to by the output, simply stayed on the entire time (unless it is blinking so rapidly that I do not notice it blinking).

I thought that maybe it has to do with the Warning where the Synthesizer truncated the expression in Line 29, so I tried declaring count_next as a 32-bit vector [31:0], both as a reg and a wire, and also initializing its value. Tried the Synthesizer and I get errors instead.

Declaring and initializing count, led_state, count_next, led_next didn't work either.

I was wondering if someone can look at this and tell me what's wrong with the code. There's also a combinatorial loop for the signal "n0011<0>" which I don't recognize.

\$\endgroup\$
  • 2
    \$\begingroup\$ I don't know Verilog, but you shouldn't be using count_next in the expression to generate count_next. That is, exactly as the error message says, a combinatorial loop. I'm pretty sure you meant to use counthere. \$\endgroup\$ – Brian Drummond Jul 7 '15 at 23:14
3
\$\begingroup\$

The problem I see is you have declared count_max as a 1 bit wide wire and then assign a 32bit constant to it - this will essentially get truncated to 1 bit.

This will then mean you are doing additions and comparisons between 32 bit and 1 bit values on line 29, thus you get the issue of it saying truncated 32bit down to 1bit.

Then by extension your calculations optimise away to a single 1bit value (essentially it boils down to if >0, set to 0, else add 1) which will only ever be 0 or 1 and hence why your count[31:1] (notice not the LSB, bit 0) are unchanged in the always construct - because you assign them to always 0.

Also declaring count_next specifically would help - you assign to a non-existent wire, so it creates one for you. In some tools e.g. modelsim, this would be an error, in others at least a warning. The simplest way to do it would be to replace assign count_next with wire [31:0] count_next.

Finally as Brian points out quite rightly in the comments, your conditional is wrong. You use count_next as part of it's own assign statement which is plain bad. It is fine to do that with clocked registers, as there is no risk of creating a design which would oscillate at very high frequencies potentially causing damage to the FPGA. I believe what you meant is (count >= count_max). Notice I also changed to be >=, this is because your count includes zero - say count_max = 5, you would want your counter to go 0,1,2,3,4,0 - notice how there are 5 clock cycles taken to count from 0 to 4 and then go back to 0?

\$\endgroup\$
  • \$\begingroup\$ Thanks. I've very new to Verilog and you've clarified why my code doesn't work. I simply implemented everything in a behavioral fashion in the always @(posedge clk) block using conditional statements and counters, while using dataflow assignments to ultimately assign a register to an output. I'm more familiar with C++ so doing this is easier for me. \$\endgroup\$ – user279043 Jul 17 '15 at 0:13
2
\$\begingroup\$

Your count_next wire is implicitly declared. That means it is one bit wide therefor count[31:1] never get assigned and you never reach your max count which also needs to be declared as a 32bits rather than 1 bit as Tom pointed out.

wire [31:0] count_next;
localparam count_max = 32'd50000000;  //you can use a wire but a parameter is a better description for a run time constant.

There are several other issues as mentioned by others.

  • count_max should be a parameter (or better in this case a localparam).
  • use count not count_next in your assignment.
  • add a reset or for fpga synthesis only initialize your registers.
  • Simulate Simulate Simulate. These problems become far easier to debug and far more apparent in simulation. See http://www.edaplayground.com/ if you need one but Xilinx also includes isim for free in webpack I believe.
\$\endgroup\$
  • \$\begingroup\$ Assigning a constant to a wire is fine. No reason to replace it with a parameter besides convention. Actually, the argument for leaving it as a wire is that you could connect it up to another piece of logic so you could change the frequency dynamically. If you just assign a constant value, the synthesizer will push the constants and remove the wire. \$\endgroup\$ – alex.forencich Jul 8 '15 at 1:15
  • \$\begingroup\$ @alex.forencich As I removed my answer, I've came over this thread discussing initialization. Among the other stuff it's pointed that this type of initialization will work on FPGAs but not ASICs or CPLDs. \$\endgroup\$ – Eugene Sh. Jul 8 '15 at 1:42
  • \$\begingroup\$ Constant assignment to a wire should work everywhere as the synthesizer will optimize it out with constant pushing. \$\endgroup\$ – alex.forencich Jul 8 '15 at 2:35
  • \$\begingroup\$ I agree, you can assign a constant to a wire. As your code base gets large it makes it harder to read the more ambiguous of intent your code is. A parameter is a direct and unambiguous description of a run time constant. That said it is up to the user(s) to decide how much effort to put into maintainability or if this is a placeholder for future dynamic assignment. \$\endgroup\$ – davidd Jul 8 '15 at 3:38
0
\$\begingroup\$

Init your count reg to zero and specify the correct width for the count_next wire. Should work just fine once you make those changes.

\$\endgroup\$

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.