Strange simulator behavior for code in Verilog

[Gizmotoy]

I've been using VHDL for awhile now, and am relatively new to Verilog. I feel like I'm probably making an obvious mistake, but I'm not having much luck finding it.

I'm working on creating a serial interface to a motor controller that uses an 8N1 serial protocol. I'm targeting an Altera Cyclone V SoC. I have some code that generates the start, data, and stop bits. The problem is that there is a particular transition that appears to be occurring at an impossible time.

Here's the controller:

---

Code Verilog - [expand]
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
// motor_handler.sv
//   Controls the Sabertooth 2 channel x 12A motor controller
// -------------------------------------------------------------------------------------------------
 
module motor_handler
#(  parameter CLK_FREQ_HZ = 50000000,
    parameter BAUD = 9600
)
(
    output reg          S1,                 // Serial TTL output
    input  wire         Reset,              // Active-high reset
    input  wire         Clk,                // 50MHz clock
    input  wire         Enable,             // Enable the sensor
    input  wire [6:0]   Left_speed,         // Speed of the left wheels (signed)
    input  wire [6:0]   Right_speed         // Speed of the right wheels (signed)
);
 
// Convert Hz to 10 us worth of clock ticks
localparam CLKS_PER_BIT = CLK_FREQ_HZ / BAUD;  // 5208 @ 50 MHz and 9600 baud
 
// Bit-times to wait between transmissions
localparam INTERWORD_BITS = 5;
 
// Bit to select between left and right
logic left_right_sel;
 
// -------------------------------------------------------------------------------------------------
 
// State logic
enum logic [2:0] {
    SM_RESET,
    SM_SEND_START_BIT,
    SM_SEND_DATA_BIT,
    SM_SEND_STOP_BIT,
    SM_WAIT
} state, next_state;
 
// Number of clocks in the current bit
logic [$clog2(CLKS_PER_BIT + 1):0] clks_in_bit;
 
// Counter for number of bits during wait period
logic [$clog2(INTERWORD_BITS + 1):0] wait_bit;
 
// Counter for number of bits during the data word
logic [3:0] data_bit;
 
// -------------------------------------------------------------------------------------------------
 
// Manage state transitions
always_ff @(posedge Clk) begin
    case (state)
        SM_RESET:
            state <= SM_WAIT;
 
        SM_WAIT:
            if (wait_bit >= INTERWORD_BITS) state <= SM_SEND_START_BIT;
            else state <= SM_WAIT;
 
        SM_SEND_START_BIT:
            if (clks_in_bit >= CLKS_PER_BIT) state <= SM_SEND_DATA_BIT;
            else state <= SM_SEND_START_BIT;
 
        SM_SEND_DATA_BIT:
            if (clks_in_bit >= CLKS_PER_BIT && data_bit >= 7) state <= SM_SEND_STOP_BIT;
            else state <= SM_SEND_DATA_BIT;
 
        SM_SEND_STOP_BIT:
            if (clks_in_bit >= CLKS_PER_BIT) state <= SM_WAIT;
            else state <= SM_SEND_STOP_BIT;
 
        default:
            state <= SM_RESET;
    endcase
end
 
// -------------------------------------------------------------------------------------------------
 
// Count the clocks to achieve the correct baud
always_ff @(posedge Clk)
    if (state == SM_WAIT) begin
        data_bit <= 0;
        if (clks_in_bit >= CLKS_PER_BIT) begin
            clks_in_bit <= 0;
            if (wait_bit >= INTERWORD_BITS)
                wait_bit <= 0;
            else
                wait_bit++;
        end else
            clks_in_bit++;
    end else if (state == SM_SEND_START_BIT || state == SM_SEND_STOP_BIT) begin
        if (clks_in_bit >= CLKS_PER_BIT)
            clks_in_bit <= 0;
        else
            clks_in_bit++;
        data_bit <= 0;
        wait_bit <= 0;
    end else if (state == SM_SEND_DATA_BIT) begin
        if (clks_in_bit >= CLKS_PER_BIT) begin
            clks_in_bit <= 0;
            if (data_bit >= 7)
                data_bit <= 0;
            else
                data_bit++;
        end else
            clks_in_bit++;
        wait_bit <= 0;
    end else begin
        data_bit <= 0;
        clks_in_bit <= 0;
        wait_bit <= 0;
    end
 
// Create the serial output
logic [7:0] tmp_byte;
always_ff @(posedge Clk)
    if (state == SM_RESET || state == SM_WAIT || state == SM_SEND_STOP_BIT) begin
        S1 <= 1;
    end else if (state == SM_SEND_START_BIT)
        S1 <= 0;
    else if (state == SM_SEND_DATA_BIT) begin
        if (left_right_sel == 0) begin
            tmp_byte = {left_right_sel, Left_speed};
            S1 <= tmp_byte[data_bit];
        end else begin
            tmp_byte = {left_right_sel, Right_speed};
            S1 <= tmp_byte[data_bit];
        end
    end else
        // Error case: Shouldn't be possible to get here
        S1 <= 1;
 
// Determine what to send next
always_ff @(posedge Clk) begin
    if (state == SM_RESET)
        left_right_sel <= 0;
    else if (state == SM_SEND_DATA_BIT && clks_in_bit >= 5208 && data_bit >= 7)
        left_right_sel <= ~left_right_sel;
end
 
endmodule

---

And the testbench:

---

Code Verilog - [expand]
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
// Validate the motor_handler
 
// `timescale 1ns/1ns
 
module test_motor_handler();
    logic clock;
    logic reset;
 
    logic s1;
    logic enable;
    logic [6:0] left_speed;
    logic [6:0] right_speed;
 
    // Generate the clock
    always #2 clock <= ~clock;
 
    // Instantiate module
    motor_handler dut(s1, reset, clock, enable, left_speed, right_speed);
 
    initial begin
        // Create reset
        clock <= 0;
        reset <= 1;
        enable <= 1;
        #6 reset <= 0;
 
        left_speed <= 75;
        right_speed <= 75;
    end
endmodule

---

So the particular issue involves this line:

---

Code Verilog - [expand]
1
else if (state == SM_SEND_DATA_BIT && clks_in_bit >= 5208 && data_bit >= 7)

---

See here:

You can see that the transition of left_right_sel from 0 to 1 occurs at the cursor in the image. It's in a (posedge Clk) block, which seems to mean that it happened when clks_in_bit was 5208 and data_bit was 6. data_bit is transitioning to 7 at around that time, but it doesn't happen until the next clock, and on that clock clks_in_bit is 0. Therefore, both conditions were not satisfied here. The actual time when left_right_sel transitions to 1 shouldn't happen until a bit later once clks_in_bit has ticked up to 5208 again and data_bit has remained at 7.

What have I done wrong here? My presumption would be that to transition clk_in_bit would have to be 5208 and data_bit would have to be 7 on the same clock cycle. The simulator seems to indicate this hadn't happened, and yet the change in left_right_sel happened anyway.

Thanks for the help. Pretty frustrated by this.

- - - Updated - - -

I can't modify the original post, but two additional points of interest:
* Simulator is Modelsim 10.5b Altera Edition
* If building to FPGA image, it seems like the code might work as expected on hardware. Not sure how to determine if the problem is simulator setup or bad code practices.

Any help or suggestions appreciated.

 

[ads-ee]

I don't see anything wrong with your code, it looks like the simulator is lying to you? I've never tried 10.5b Altera Edition with SV so I'm wondering if there is a problem in that version with SV.

Though I do think you may be counting wrong as you count from 0 to 5208, which is actually 5209 counts instead of 5208. 5208 is closer to the exact value of 5208.333333...

 

[Gizmotoy]

Seriously good eye on the extra clock in the counter. It seemed like the least of my problems so I had left it in for the time being. I've got it fixed up now, though.

I do kind of think the simulator might be lying to me. I spent a few hours tonight in SignalTap, and the behavior seemed to be as one would expect without any of the same issues present in ModelSim. I hooked up a mixed digital/analog logic analyzer to the pin, and saw the correct behavior there as well. I thought maybe I'd see some of the glitches in that ModelSim S1 output when viewed in analog mode, but they looked perfect.

ModelSim must be doing something weird, I guess. Maybe I can track down a different version.

 

[TrickyDicky]

This is because data_bit++ is treated as a blocking assignment. That would give you the issue you see as you could be creating a race condition between the two always blocks as to which gets evaluated first.

try:

data_bit <= data_bit + 1;

instead.

PS. It will likely work on hardware as synth tools are often a bit less strict on the rules, and probably treated it like a non-blocking assignemnt, or it created a combinatorial path with enough delay that it "works"

 

[ads-ee]

This is because data_bit++ is treated as a blocking assignment. That would give you the issue you see as you could be creating a race condition between the two always blocks as to which gets evaluated first.

Interesting I don't code using SV much and have only used the ++ operator in for loops. But it makes sense that it's blocking.

 

[TrickyDicky]

Interesting I don't code using SV much and have only used the ++ operator in for loops. But it makes sense that it's blocking.

It should be the same issue with Verilog 2001/95 etc as this hasnt changed.

 

[dpaul]

Interesting I don't code using SV much and have only used the ++ operator in for loops. But it makes sense that it's blocking.

Nice to know about this!

Can the OP please confirm on this after making the changes?

 

[ads-ee]

It should be the same issue with Verilog 2001/95 etc as this hasnt changed.

Uh, ++ is a SystemVerilog operator it was never in Verilog 2001/95 and yes it is blocking.

From IEEE Std 1800-2012

11.4.2 Increment and decrement operators
SystemVerilog includes the C increment and decrement assignment operators ++i, --i, i++, and i--.
These do not need parentheses when used in expressions. These increment and decrement assignment
operators behave as blocking assignments.

I should mention I used to use ++ in my for loops but kept running into -sv options that didn't "take" when compiling code for Xilinx/Actel/Altera/Lattice, someone always barfs on using SV and they don't all barf on the same portions of SV code. So all my current code uses i=i+1 in the for loop.

 

[Gizmotoy]

TrickyDicky is definitely correct. I tried swapping the "x++;" assignments for "x <= x + 1;" equivalents. Once completed, simulator performed exactly as intended.

It's interesting that Altera's synthesis tool didn't seem as bothered by it as ModelSim, but am happy to have found the cause. I carefully combed through my code looking for a blocking assignment that could have been messing things up, and never even suspected it could be the ++ operator. Thanks so much for the help!

 

[TrickyDicky]

TrickyDicky is definitely correct. I tried swapping the "x++;" assignments for "x <= x + 1;" equivalents. Once completed, simulator performed exactly as intended.

It's interesting that Altera's synthesis tool didn't seem as bothered by it as ModelSim, but am happy to have found the cause. I carefully combed through my code looking for a blocking assignment that could have been messing things up, and never even suspected it could be the ++ operator. Thanks so much for the help!

Neither was bothered by it - it worked exactly as intended in modelsim. Technically, the error is with quartus.

- - - Updated - - -

TrickyDicky is definitely correct. I tried swapping the "x++;" assignments for "x <= x + 1;" equivalents. Once completed, simulator performed exactly as intended.

It's interesting that Altera's synthesis tool didn't seem as bothered by it as ModelSim, but am happy to have found the cause. I carefully combed through my code looking for a blocking assignment that could have been messing things up, and never even suspected it could be the ++ operator. Thanks so much for the help!

Neither was bothered by it - it worked exactly as intended in modelsim. Technically, the error is with quartus.