0
\$\begingroup\$

I'm writing a finite sequence encoder in Verilog. Basically, an output Z will be activated if the input W is on for at least four clock cycles, or if its off for at least 4 clock cycles. See the following timing diagram

enter image description here

Also, to get some more context, take a look at the state diagram for the sequence encoder.

enter image description here

I've implemented a simple behavioral design and it works, but for some reason, it takes two clock cycles for the state to change when I'm trying to go to the RESET state. For example, when I'm in state C and put on the Reset, I have to press the Clock button twice for it to finally move into reset mode. Take a look at my code. Do you have any idea what is wrong?

module Part2(

    input Reset_L,
    input W,             
    input Clock_L,
    output reg [8:0] Y, //group of 9 flip flops
    output Z

);

wire CLK;

assign CLK = ~Clock_L;

reg [3:0] SCURRENT= A; //initializing to RESET state

reg [3:0] SNEXT;

parameter A = 4'b0000;
parameter B = 4'b0001;
parameter C = 4'b0010;
parameter D = 4'b0011;
parameter E = 4'b0100;
parameter F = 4'b0101;
parameter G = 4'b0110;
parameter H = 4'b0111;
parameter I = 4'b1000;

always @(W or SCURRENT) //next state logic
begin

        case (SCURRENT)
            A: if (W==0) SNEXT <= B;
                else if (W==1) SNEXT <= F;

            B: if (W==0) SNEXT <= C;
                else if (W==1) SNEXT <= F;

            C: if (W==0) SNEXT <= D;
                else if (W==1) SNEXT <= F;

            D: if (W==0) SNEXT <= E;
                else if (W==1) SNEXT <= F;

            E: if (W==0) SNEXT <= E;
                else if (W==1) SNEXT <= F;

            F: if (W==1) SNEXT <= G;
                else if (W==0) SNEXT <= B;

            G: if (W==1) SNEXT <= H;
                else if (W==0) SNEXT <= B;

            H: if (W==1) SNEXT <= I;
                else if (W==0) SNEXT <= B;

            I: if (W==1) SNEXT <= I;
                else if (W==0) SNEXT <= B;

            default: SNEXT <= 4'bxxxx;

        endcase
end

always @(posedge CLK)
begin
    if (Reset_L==0) SCURRENT <= A;
    else SCURRENT <= SNEXT;

    case (SCURRENT)
        A: Y<=9'b000000001;
        B: Y<=9'b000000010;
        C: Y<=9'b000000100;
        D: Y<=9'b000001000;
        E: Y<=9'b000010000;
        F: Y<=9'b000100000;
        G: Y<=9'b001000000;
        H: Y<=9'b010000000;
        I: Y<=9'b100000000;
        default: Y<= 9'b111111111;
    endcase
end

assign Z = Y[4] | Y[8];

endmodule 
\$\endgroup\$
  • \$\begingroup\$ The button is the CLK. It doesn't have a state. It's just on or off. Should be off to start with, until its pressed. The button on my FPGA is active low, so I made a simple assign statement to convert it to active high. \$\endgroup\$ – user3308807 Jan 5 at 5:25
  • \$\begingroup\$ I'd also like to clarify that it only takes 2 clock cycles when I'm going from any of the states back to the Reset State (Also known as A). Sorry if I didn't make that clear. Like after it resets, then it proceeds through its sequence one Clock cycle at a time. \$\endgroup\$ – user3308807 Jan 5 at 6:08
1
\$\begingroup\$

I think the problem lies in this part

always @(posedge CLK)
begin
    if (Reset_L==0) SCURRENT <= A;
    else SCURRENT <= SNEXT;

    case (SCURRENT)
        A: Y<=9'b000000001;
        B: Y<=9'b000000010;
        C: Y<=9'b000000100;
        D: Y<=9'b000001000;
        E: Y<=9'b000010000;
        F: Y<=9'b000100000;
        G: Y<=9'b001000000;
        H: Y<=9'b010000000;
        I: Y<=9'b100000000;
        default: Y<= 9'b111111111;
    endcase
end

From my understanding of Verilog and the testbench I created to test your code, I think that what is being executed in parallel is:

posedged(k) (if || case) ----> posedged(k+1) (if || case)

where the first posedged sets

SCURRENT(k+1) <= A, and 
Y <= X. (where X depends on the original value of SCURRENT(k))

and the second one does

SCURRENT(k+2) <= A, and 
Y<=9'b000000001. (since  SCURRENT(k+1) is A)

What you probably intended to do is

always @(posedge CLK)
begin
    if (Reset_L==0) SCURRENT = A;
    else SCURRENT = SNEXT;

    case (SCURRENT)
        A: Y<=9'b000000001;
        B: Y<=9'b000000010;
        C: Y<=9'b000000100;
        D: Y<=9'b000001000;
        E: Y<=9'b000010000;
        F: Y<=9'b000100000;
        G: Y<=9'b001000000;
        H: Y<=9'b010000000;
        I: Y<=9'b100000000;
        default: Y<= 9'b111111111;
    endcase
end

Where the case will be evaluated after the if being "executed", and since mixing blocking and non-blocking statements can lead to problems synthesizing, use this to sort it all out,

always @(posedge CLK)
begin
    if (Reset_L==0) SCURRENT = A;
    else SCURRENT = SNEXT;

    case (SCURRENT)
        A: Y=9'b000000001;
        B: Y=9'b000000010;
        C: Y=9'b000000100;
        D: Y=9'b000001000;
        E: Y=9'b000010000;
        F: Y=9'b000100000;
        G: Y=9'b001000000;
        H: Y=9'b010000000;
        I: Y=9'b100000000;
        default: Y= 9'b111111111;
    endcase
end
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Holy cow! Wow you're a hero! Thank you for taking the time to look at this. I would like to understand more why the non blocking assignment caused this problem. I thought that if I used non blocking, then my statements would execute sequentially, but clearly that did not result. Can you tell me more? \$\endgroup\$ – user3308807 Jan 5 at 9:44
  • 1
    \$\begingroup\$ If they are non-blocking they all happen at the same time, if you want them to happen one then the other, the first one must be blocking. This is related to how blocking and non-blocking statements are "executed" by Verilog. \$\endgroup\$ – jDAQ Jan 5 at 9:48
1
\$\begingroup\$

The issue is due to the line number 67, as u are using synchronous mode of asserting the reset it takes 2 cycles to show its affect on the outputs. if u write it in async assertion

always@(posedge Clock_L or negedge Reset_L)

then it would have taken single cycle .. please use synchronizers if u are using the push button to reset ur code as it might lead to metastability which will be nightmare to debug the issue.

can u use the below code just modified ur code to a standard 4-always block FSM code which might be useful for readability,changeability,debugging. please use #TCQ in NBA in sequential logics so that u can see the toggling same as in hardware, and standard mode of FSM coding which will be helpful for the designers to understand and debug the code easily.

ref for my suggestions:

https://forums.xilinx.com/t5/Simulation-and-Verification/why-there-is-a-TCQ-delay-in-many-Xilinx-official-example-designs/td-p/682670

http://www.sunburst-design.com/papers/CummingsSNUG2019SV_FSM1.pdf

module Part2(
    input            Reset_L,
    input            W,
    input            Clock_L,
    output reg [8:0] Y, //group of 9 flip flops
    output           Z
);
reg [8:0] n_y;
reg       Z;

localparam A = 4'b0000;
localparam B = 4'b0001;
localparam C = 4'b0010;
localparam D = 4'b0011;
localparam E = 4'b0100;
localparam F = 4'b0101;
localparam G = 4'b0110;
localparam H = 4'b0111;
localparam I = 4'b1000;

reg[3:0] state,next;

always@(posedge Clock_L)
    if(~Reset_L) state <= #10 A;
    else         state <= #10 next;

always@* begin
            next = 'x;
  casez(state)
        A : next = W ? F : B ;
        B : next = W ? F : C ;
        C : next = W ? F : D ;
        D : next = W ? F : E ;
        E : next = W ? F : E ;
        F : next = W ? G : B ;
        G : next = W ? H : B ;
        H : next = W ? I : B ;
        I : next = W ? I : B ;
   default: next = 'x;
  endcase
end

always@* case(state)
        A :  n_y = 9'b0_0000_0001;
        B :  n_y = 9'b0_0000_0010;
        C :  n_y = 9'b0_0000_0100;
        D :  n_y = 9'b0_0000_1000;
        E :  n_y = 9'b0_0001_0000;
        F :  n_y = 9'b0_0010_0000;
        G :  n_y = 9'b0_0100_0000;
        H :  n_y = 9'b0_1000_0000;
        I :  n_y = 9'b1_0000_0000;
   default:  n_y = 9'b1_1111_1111;
 endcase

always@(posedge Clock_L)begin
        Y <= #10 n_y;
        Z <= #10 n_y[4] || n_y[8];
end

endmodule
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Feel free to use you instead of u. It is even encouraged here. \$\endgroup\$ – Huisman Mar 1 at 20:55
  • \$\begingroup\$ Sure @huisman, I will follow this in my future answers. Cheers!! \$\endgroup\$ – RAMA KRISHNA MEDA Mar 2 at 2:50
  • \$\begingroup\$ Sorry but this wasn't the problem. The first answer solved my issue. And no metastability is not an issue with a simple school project where the clock is literally just a button that is pushed. Timing is certainly not a concern here. \$\endgroup\$ – user3308807 Apr 19 at 0:39

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.