Problem with Verilog module (Basys3 Board)

[jrosengarden]

Hi All:

I'm fairly new to both Verilog and the Basys3 board.  I'm working thru a 'self education' course.  I'm having a problem with the following module.  I could use any answers/advice I can get.
The module, as posted (below) is processed by Vivado properly, loads into the Basys3 board and runs as expected with the LED blinking at a frequency of .7451 Hz.  No problems.

The example I'm working thru then threw out a 'challenge' of changing the frequency of the blinking LED based on the positions of two switches [1:0].
     1.  I added in the additional input - input [1:0] sw
     2.  I added in the case statement (which is currently commented out)
     3.  I commented out the original assignment of led - //assign led = clkdiv[26];

The problem crops up immediately with a Verilog error showing on the following line: case(sw)
     - The error states: Error: 'sw' is not a constant

I've spent hours trying various changes...all to no avail.  

HELP....PLEASE!!!!     Thanks Tons!

VERILOG MODULE:

module clk_divider(
    input clk,
    input rst,
    input [1:0] sw,
    output led
    );
      
wire [26:0] din;
wire [26:0] clkdiv;
 
dff dff_inst (
    .clk(clk),
    .rst(rst),
    .D(din[0]),
    .Q(clkdiv[0])
);
 
genvar i;
generate
for (i = 1; i < 27; i=i+1) 
begin : dff_gen_label
    dff dff_inst (
        .clk(clkdiv[i-1]),
        .rst(rst),
        .D(din),
        .Q(clkdiv)
    );
    end
endgenerate
 
assign din = ~clkdiv;

/*
begin                                                                 
case(sw)  
    2'b00: assign led = clkdiv[26];  
    2'b01: assign led = clkdiv[25];
    2'b10: assign led = clkdiv[24];
    2'b11: assign led = clkdiv[23];                                  
    default: assign led = clkdiv[26];
endcase
end
*/

 assign led = clkdiv[26];
 
endmodule
 

[jpeyron]

Hi @jrosengarden,

The case statement should be within an always statement.  Here is a thread that discusses this.  Here is an example on using case statements.

best regards,

Jon

 

 

[jrosengarden]

jpeyron:

Thanks tons for responding.  Believe it, or not, that was one of the changes I tried (see below)but when I did that I then got an error of "Procedural assign not supported" on the elements within the case statement.  This is what has been frustrating me....when I change what seems to be the obvious "fix"...it breaks something else.  My weak depth of knowledge of Verilog leaves me stymied.

 

always @ * begin                                                                 
case(sw)  
    2'b00: assign led = clkdiv[26];  
    2'b01: assign led = clkdiv[25];
    2'b10: assign led = clkdiv[24];
    2'b11: assign led = clkdiv[23];                                  
    default: assign led = clkdiv[26];
endcase
end

[jrosengarden]

OK...I think I fixed it.  I won't be able to load it into the Basys3 board until tonight but Vivado processed it completely with no problems (and I'm not up-to-speed yet on how to run a simulation).

1.     I added the following at the top of the module--> reg switched_clkdiv;
2.    I then changed the case statement as shown below
3.    I then changed the assign led statement as shown below

always @ (*) 
begin                                                                 
    case(sw)  
        2'b00:   switched_clkdiv = clkdiv[26];  
        2'b01:   switched_clkdiv = clkdiv[25];
        2'b10:   switched_clkdiv = clkdiv[24];
        2'b11:   switched_clkdiv = clkdiv[23];                                  
        default: switched_clkdiv = clkdiv[26];
    endcase
end

assign led = switched_clkdiv;
 

Thanks for the help.  It is greatly appreciated!

[jpeyron]

Hi @jrosengarden,

Glad to hear that you were able to get your project further in vivado. Thank you for sharing what you did. Here is a verilog tutorial that should help with simulation.

best regards,

Jon

[xc6lx45]

Hi,

just a thought looking at the code: What I read between the lines is that you're starting out with the wrong textbook. Most of the "legacy" material you find on the web etc is not optimal for FPGA. Your code examples will work but they set you out on a path that is steeper than necessary.

Some general guidelines

• avoid combinational expressions "always @(*)" but assign results to registers using delayed assignment <=.
  The obvious objection "but isn't this slow?" is usually countered with "not if it lets me increase clock frequency by 10x or more".
  Using the register hardware on the chip costs nothing, they will just sit idle if you don't but it'll save you a lot of trouble.
• Let the language do the job for you, e.g. use a 32-bit register and increase by 1 instead of cascading instantiated flipflops.
• Usually there are much better alternatives to "generate". Save it e.g. for parameterized, reusable code

The code below will probably pass timing up to 300 MHz on an Artix (haven't tried this specific example):

reg [23:0] counter = 24'd0;
reg [1:0] swReg = 2'd0;
reg LED = 1'd0; // register the output (don't drive chip outputs from a combinational expression)

always @(posedge clk) begin
swReg <= sw; // registered copy of asynchronous inputs, one cycle late
case (swReg)
2'b00: LED <= counter[23];
2'b01: LED <= counter[22];
2'b10: LED <= counter[21];
2'b11: LED <= counter[20];
endcase
end

[jrosengarden]

xc6lx45:

Thanks for the response/advise.  Can you recommend your opinion of the 'right' textbook for Verilog?  I'm all ears (and eyes) at this early juncture......

 

Thanks

[D@n]

@jrosengarden,

Ooohh, I've got one: Check out this tutorial, and the articles on this blog.

</shameless plug>

Dan

[jrosengarden]

To All:

1st off...thank-you to everybody that's been so kind to respond and help out.  This newbie greatly appreciates the assistance.  That being said;

I've resolved the main problem(s) and now Vivado properly processes the project.  However the simulation is showing me that the frequency that the LED is blinking at is NOT changing based on the switch value [1:-0]sw changing.  I've posted both the updated module, below and a screen grab of the simulation.  You can see, from the simulation that as the switch value changes (from 3 to 2 to 1 to 0) the LED continues to blink at the same frequency.

that's the bad news.  The good news is I am WAY further along than I was this morning....especially being able to properly get a simulation going for analysis!  I still haven't loaded the .BIN file onto the board...yet I KNOW there is a problem from the simulation!  EUREKA!   ?

Any idea why the led is not properly being assigned the value being determined in the case statements?

Thanks again:

 

MODULE:

module clk_divider(
    input clk,
    input rst,
    input [1:0] sw,
    output led
    );
     
wire [26:0] din;
wire [26:0] clkdiv;
reg switched_clkdiv;
 
dff dff_inst (
    .clk(clk),
    .rst(rst),
    .D(din[0]),
    .Q(clkdiv[0])
);
 
genvar i;
generate
for (i = 1; i < 27; i=i+1) 
begin : dff_gen_label
    dff dff_inst (
        .clk(clkdiv[i-1]),
        .rst(rst),
        .D(din),
        .Q(clkdiv)
    );
    end
endgenerate
 
assign din = ~clkdiv;

always @ (*) 
begin                                                                 
    case(sw)  
        2'b00:   switched_clkdiv <= clkdiv[0];  
        2'b01:   switched_clkdiv <= clkdiv[1];
        2'b10:   switched_clkdiv <= clkdiv[2];
        2'b11:   switched_clkdiv <= clkdiv[3];                                  
        default: switched_clkdiv <= clkdiv[0];
    endcase
end

assign led = switched_clkdiv;
 
endmodule

[xc6lx45]

It may be the non-blocking assignment <= in combinational logic e.g.
2'b00:   switched_clkdiv <= clkdiv[0]; 

try
2'b00:   switched_clkdiv = clkdiv[0];  

See
https://stackoverflow.com/questions/26727727/assignment-operator-in-verilog
"It is Recommended to use non-blocking assignment for sequential logic and blocking assignment for combinational logic, only then it infers correct hardware logic during synthesis."

and http://www.sunburst-design.com/papers/CummingsSNUG2002Boston_NBAwithDelays.pdf
It is illegal to use a nonblocking assignment in a continuous assignment statement or in a net declaration.

Actually I'm not 100 % convinced this really is the issue (check warnings).

Are the outputs from your divider chain really what you expect them to? This looks a bit odd

  .clk(clkdiv[i-1]),
        .rst(rst),
        .D(din),
        .Q(clkdiv)

BTW I think you are violating one guideline I didn't even mention: Don't mix information signals (as from .Q) and clock signals (as going into .clk). Many books teach this, with either simulation or ASICs in mind. It also works on a breadboard circuit. But on an FPGA, those are physically different structures, and connecting them makes a mess of things for the tools.

If you try my example, note I forgot to increase the counter :-) that is, counter <= counter + 24'd1.

And, reading other peoples' code is probably a good idea.
Check Vivado "Tools / language templates", Verilog/Synthesis constructs" for Xilinx official examples, e.g. "Example modules / DSP / Complex multiplier". Note, some of this is quite advanced or specialized.

[jrosengarden]

HI xc6lx45:

Well, to my surprise, when I got home and loaded the .BIT file onto the board...it works perfectly.  [1:0]sw is changing the frequency the the led is blinking at properly.  

So this tells me that I don't quite have my testbed code done properly.  I tried to attach it into this text but it kept getting reformatted so I've simply attached the actual file.

If somebody could look at it and tell me what (if anything) I've done wrong I'd greatly appreciate it.  THANKS!

NOTE:  In the actual module code, above, I had changed the CASE choices to the 0, 1st, 2nd and 3rd flip-flops in order to better see the led changing value on the wave panel.  However I've changed the code back to the actual flip-flops I wanted; the 26th, 25th, 24th and 23rd flip-flops.  As I said...the board is working perfectly now and the switch setting are appropriately changing the led blinking frequency.  It HAS to be something wrong with the TestBench code...or me not using the simulator properly.

 

THANKS MUCH!

 

clock_divider.tb