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\$
1
  • 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\$ Commented Jul 7, 2015 at 23:14

3 Answers 3

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\$
1
  • \$\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\$ Commented Jul 17, 2015 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\$
4
  • \$\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\$ Commented Jul 8, 2015 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\$ Commented Jul 8, 2015 at 1:42
  • \$\begingroup\$ Constant assignment to a wire should work everywhere as the synthesizer will optimize it out with constant pushing. \$\endgroup\$ Commented Jul 8, 2015 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\$ Commented Jul 8, 2015 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\$

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.