CODEWITHSUNDEEP
verilog
T. Saeki
T. Saeki

Reputation: 1

How to write a verilog code in two-always-block style with multiple state regs?

I'm a begginer of Verilog. I read a several materials about recommended Verilog coding styles like this paper and stackoverflow's questions.
Now, I learned from them that "two always block style" is recommended; separate a code into two parts, one is a combinational block that modifies next, and the another is a sequential block that assigns it to state reg like this.

reg [1:0] state, next;

always @(posedge clk or negedge rst_n)
    if (!rst_n)
        state <= IDLE;
    else
        state <= next;

 always @(state or go or ws) begin
     next = 'bx;
     rd = 1'b0;
     ds = 1'b0;
    case (state)
        IDLE : if (go) next = READ;
               else next = IDLE;
 ...

And here is my question. All example codes I found have only one pair of registers named state and next.
However, if there are multiple regs that preserve some kinds of state, how should I write codes in this state-and-next style?
Preparing next regs corresponding to each of them looks a little redundant because all regs will be doubled.

For instance, please look at an UART sender code of RS232c I wrote below. It needed wait_count, state and send_buf as state regs. So, I wrote corresponding wait_count_next, state_next and send_buf_next as next for a combinational block. This looks a bit redundant and troublesome to me. Is there other proper way?

module uart_sender #(
    parameter clock = 50_000_000,
    parameter baudrate = 9600
) (
    input clk,
    input go,
    input [7:0] data,
    output tx,
    output ready
);

parameter wait_time = clock / baudrate;

parameter send_ready = 10'b0000000000,
        send_start = 10'b0000000001,
        send_stop  = 10'b1000000000;

reg [31:0] wait_count = wait_time,
        wait_count_next = wait_time;
reg [9:0] state = send_ready,
        state_next  = send_ready;
reg [8:0] send_buf = 9'b111111111,
        send_buf_next = 9'b111111111;

always @(posedge clk) begin
    state <= state_next;
    wait_count <= wait_count_next;
    send_buf <= send_buf_next;
end

always @(*) begin
    state_next = state;
    wait_count_next = wait_count;
    send_buf_next = send_buf;
    case (state)
        send_ready: begin
            if (go == 1) begin
                state_next = send_start;
                wait_count_next = wait_time;
                send_buf_next = {data, 1'b0};
            end
        end
        default: begin
            if (wait_count == 0) begin
                if (state == send_stop)
                    state_next = send_ready;
                else
                    state_next = {state[8:0], 1'b0};
                wait_count_next = wait_time;
                send_buf_next = {1'b1, send_buf[8:1]};
            end
            else begin
                wait_count_next = wait_count - 1;
            end
        end
    endcase
end

assign tx = send_buf[0];
assign ready = state == send_ready;

endmodule

Upvotes: 0

Views: 2245

Answers (1)

Serge
Serge

Reputation: 12344

I think you did a good job and correctly flopped the variables. The issue is that without flops you would have a loop. i.e. if you write something like the following, the simulation will loop and silicon will probably burn out:

 always_comb wait_count = wait_count - 1;

So, you need to stage this by inserting a flop:

 always_ff @(posedge clk)
    wait_count <= wait_count - 1;

Or in your case you you used an intermediate wait_count_next which is a good style:

 always_ff @(posedge clk)
    wait_count_next <= wait_count;
 always_comb
    wait_count = wait_count_next;

You might or might not have an issue with the last assignments. Which version of the signals you want to assign to tx and ready? the flopped one or not?

And yes, you can split theses blocks in multiple blocks, but in this case there seems to be no need.

And yes, the other style would be write everything in a single flop always block. But this will reduce readability, will be more prone to your errors and might have synthesis issues.

Upvotes: 1

Related Questions