# How to send a packet every n clock cycles in verilog?

I am fairly new to Verilog and in general Digital Design. I am working on a project which has a state machine. The module, in a particular state, receives a read request packet from some other module and I have to decode the required read response data size and send back as many data bytes in the payload. Now I have to do this every n clock cycles.

I am finding it difficult to understand where should my count be updated and where should it be assigned the next count value? Here is the general structure of my code.

    always@(posedge clk_i or negedge resetn_i) begin
if (resetn_i == 1'b0) begin
reg_write_en        <= 1'b0;
count               <= 3'b111;
next_count          <= 3'b111;
state               <= S_INITIAL;

end else begin
state               <= next_state;
count               <= next_count;
end


This is the sequential block.Now in my combinational block

always@(*) begin

next_state  = state;
next_count  = count;
.......
........
M_PASSIVE:begin
if(count == 3'b000)
begin
//Update some registers with new data values
end else begin
next_count = count - 3'b001;
end


I am unable to understand does this wait the required count no of cycles and then update the reg values or I am missing something here.

P.S. Pardon me if I am missing some fundamental aspects of behavioural design regarding difference between sequential and combinational blocks.

Here is a complete code of what I am trying to do. Essentially I want to read a packet, turn of reading of packets, wait 7 or 8 clock cycles or N clk cycles and then send a read response with the data in the registers which are assigned to output ports continuosly.

    always@(posedge clk_i or negedge resetn_i) begin
if (resetn_i == 1'b0) begin
reg_write_en        <= 1'b0;
count           <= 3'b111;
state           <= S_INITIAL;
end else begin
state           <= next_state;
count           <= next_count;
end
end

always@(*) begin
next_state  = state;
next_count  = count;
case(state)
S_INITIAL: begin //not enabled
if(enable_i == 1'b1) begin
next_state = S_ENABLED;
end
reg_write_en        <= 1'b0;
end

S_ENABLED: begin
if(enable_i == 1'b0) begin
next_state = S_INITIAL;
end

case(mode_i)
M_PASSIVE:begin
begin
if(count == 3'b000)
begin
reg_write_en    =   1'b1;
//count = 3'b111;
reg_write_en    =   1'b0;
end
next_count = count - 3'b001;
end
end

• Splitting the state machine into two like this is necessary in some cases, but it is overcomplicating things here, especially since you're new. Take a look at this answer I wrote, it includes an example of a full state machine with counter: electronics.stackexchange.com/questions/57035/… I highly recommend avoiding combinational always blocks (the ones with the (*) in the sensitivity list) until you get used to coding with just sequential blocks and get a feel for "this code -> this behaviour" – stanri Feb 15 '18 at 14:16
• @stanri Thank you very much for the reply. I will look through the answer you mentioned. I will write back here if I need more clarifications. – frisco_1989 Feb 15 '18 at 14:27
• @stanri when you say a signal must be driven from only one proper process, do you mean reg or wire? – frisco_1989 Feb 15 '18 at 14:41
• I realise the link I gave you was VHDL, but the principle still applies. regs are used in sequential process blocks, so I'm talking about a reg in Verilog. (By the way, is SystemVerilog an option for you to learn? SystemVerilog does away with the reg/wire distinction, amongst other things) – stanri Feb 15 '18 at 14:56

There are a couple of things to point out in your code.

1) you're driving the reg_read_en and reg_write_en signals in both always blocks.

always@(posedge clk_i or negedge resetn_i) begin
if (resetn_i == 1'b0) begin
reg_write_en        <= 1'b0; <=
count               <= 3'b111;
next_count          <= 3'b111;
state               <= S_INITIAL;

end else begin
state               <= next_state;
count               <= next_count;
end


and

always@(*) begin
next_state  = state;
next_count  = count;
case(state)
S_INITIAL: begin //not enabled
if(enable_i == 1'b1) begin
next_state = S_ENABLED;
end
reg_write_en        <= 1'b0; <==
end


This isn't synthesizable and probably wouldn't be allowed by some simulation tools either.

2) You're inferring latches in your combinitorial blocks:

always@(*) begin
next_state  = state;
next_count  = count;
case(state)
S_INITIAL: begin //not enabled
if(enable_i == 1'b1) begin         <==
next_state = S_ENABLED;        <==
end                                <==
reg_write_en        <= 1'b0;
end


The marked if statement needs a corresponding else in every case. You need to cover all the different possible options.

This is why in my comment I suggested you use a single sequential block, it'll fix both of these issues, and make everything a lot easier to keep track of.

So here's an example of combining them:

always@(posedge clk_i or negedge resetn_i) begin
if (resetn_i == 1'b0) begin
reg_write_en             <= 1'b0;
count                    <= 3'b111;
state                    <= S_INITIAL;

// I was trained to give everything a reset value
// But that's not really necessary and is more a matter of style
// as long as it gets a value when it needs one.
// including this here for completeness

end
else begin

case(state)
S_INITIAL: begin //not enabled
if(enable_i == 1'b1) begin
state <= S_ENABLED;

// set count to the initial value here
// this way, you re-set it every time you switch to enabled
count      <= 3'b111;

// If there's anything else you want to setup, here's a good place to do it...

end
else begin
state <= S_INITIAL;

end

reg_write_en <= 1'b0;

end

S_ENABLED: begin
if(enable_i == 1'b0) begin
state <= S_INITIAL;

// here's where you would clear any signals that you want to clear before going back to the initial state

end
else begin
state <= S_ENABLED;

end

case(mode_i)
M_PASSIVE:begin

if(count == 3'b000) begin
reg_write_en             <= 1'b1;

// you can reset count here to 7 if you want it to start again
count                    <= 3'b111;

end
else begin

// this will make reg_write_en valid only when count is 0
// and it will be disabled the rest of the time.
// if you want it valid all the time, you can set this to 1 and clear it elsewhere.
reg_write_en <= 1'b0;

// this stops the counter from underflowing
// otherwise, when it's 0 it will underflow to 111.
// Unless that's what you want, of course.
count        <= count - 3'b001;

end

end
end
// other cases...
endcase

endcase
end
end

• There are inferred latches (such as all theread_resp_tx_*), but next_state is not one of them. Note the next_state = state; which provides a default value, thereby satisfying the conditions for the synthesizer to see it as a combinational logic. A latch is inferred where there is at least one path the variable will not be assigned withing the entire always @* block. – Greg Feb 15 '18 at 17:11
• Thank you @Greg for the elaborate answer. I will try this in my simulation and see how it works. I have another question. Lets say I keep this running and I am sending data every N clock cycles. If I change the tx_pld address and data in the if statement where the count falls through, would it reflect in a single clock cycle? Is that what posedge clk means, at a given clokc edge? – frisco_1989 Feb 15 '18 at 17:40
• @Greg good point, I missed that line. – stanri Feb 15 '18 at 18:18
• Thank you @Greg for the help. It works for me. More importantly thank you so much for the beautiful explanation of the concept of inferred latches and how to avoid them. – frisco_1989 Feb 16 '18 at 12:40
• Just to add to the discussion, I would want a delay of one clock cycle when I enable my write enable signal to clear it. How can I do this in the combinational block? I did some research and this can be done by introducing a dummy state where I can jump to and then come back to my original state. Would this introduce a one clock cycle delay? Like in the M_PASSIVE case if i receive a packet, I want to write it back to my FIFO and so the write enable needs to high for exactly one clock cycle S_ONECLK_DELAY: next_state <= S_ENABLED;. But write enb is always high.. Am I missing something? – frisco_1989 Mar 13 '18 at 10:23

You are very close. The error is the 'next_count' is only updated in the combinatorial always @(*) block and 'count' is only update in the register/clocked part always@(posedge clk_i....

Thus remove next_count <= 3'b111;

You did not say how many cycle you have to wait: 7 or 8? I'll be honest. Most of the time I write code which gets me 99% of the way. If the data is stored after 7 or 8 cycles are details which are so easily solved that I often run the simulation first and then make a +1 or -1 correction.

Post edit after new code:
I can't debug your state machine for you, but I can make some remarks. You set reg_write_en and some lines lower later clear it again.

I assume this is supposed to really work in hardware not just in simulation. If so your code produces lots of latches. You will not be notified of those until you synthesis the code. For example these:

read_resp_tx_qpe_dest
reg_write_en


In your combinatorial section always @(*) you have to assign a value to every variable in every state. If not you get latches. One way to do that is what you do with next_state and next_count: set a default value at the top. But then it is also good practice to document that it is a default value which will be changed later.

// Defaults:
next_state  = state;
next_count  = count;
reg_write_en = 1'b0;

• Thank you for the answer. I removed the next_count <= 3'b111 from the sequential block. I have edited my question to include the full code of what I am trying to do. So you will be able to better understand the way regs are being updated. – frisco_1989 Feb 15 '18 at 13:42
• I have found that the the register/combinitorial state machine split is useful if you want to detect and work off state transitions if ((state != next_state) && (next_state = MY_STATE)). This means that instead of detecting the condition that triggers the state change, you detect the state change itself, and this allows you to change the state-change conditions in one place only. Register-only state machines make it a lot harder to do this. In the register-only case, you need to register the state and do state != state_z, which is delayed by 1cc from the transition. – stanri Feb 15 '18 at 15:45