More elegant code for synchronous square wave generator in Verilog

Question

I'm self studying with Chu's FPGA prototyping book. Exercise 4.7.1 asks for a programmable square wave generator:

A programmable square-wave generator is a circuit that can generate a square wave with variable on (i.e., logic 1) and off (i.e., logic 0) intervals. The durations of the intervals are specified by two 4-bit control signals, m and n, which are interpreted as unsigned integers. The on and off intervals are m* 100 ns and n* 100 ns, respectively. Design a programmable square-wave generator circuit. The circuit should be completely synchronous.

I have a completely syncrhonous working solution, but I couldn't find a nice way for it to be cycle accurate, in the sense that the code I like the most uses two more clock cycles to change the logic level of the square wave. I'm pasting the whole code at the end of my question, but what I don't like is the part that does if(clk_tick_count == interval_quant_ns - 1), specifically having to substract 1 from the number of intervals I want to count. I came to this solution after verifying that the solution without the -1 was taking two more clock cycles, and fixed it with the -1. I understand this is because since the update is synchronous I need another full clock cycle for the changes to be reflected in my ouput wave.

I just have a feeling that there ir a nicer way to do this but I can't discuss it with anyone nor have I been lucky searching for nicer code online.

Thanks!

This is my current code:

module sqwaveGen
#( 
parameter interval_quant_ns = 5
)
(
input wire clk,
input wire rst,
output wire wave,
input wire [3:0] on_interval, 
input wire [3:0] off_interval 
);
reg wave_next, wave_reg;
reg [3:0] clk_tick_count, clk_tick_count_next;
reg [3:0] count, count_next;
always @(posedge clk, negedge rst) begin
 if(~rst) begin
 wave_reg <= 0;
 clk_tick_count <= 0;
 count <= 0;
 end else begin
 wave_reg <= wave_next;
 clk_tick_count <= clk_tick_count_next;
 count <= count_next; 
 end
end
always @(*) begin
 wave_next = wave_reg;
 count_next = count;
 clk_tick_count_next = clk_tick_count + 1;
 if(wave_reg == 0 && count == off_interval) begin
 wave_next = 1;
 count_next = 0;
 end
 if(wave_reg == 1 && count == on_interval) begin
 wave_next = 0;
 count_next = 0;
 end 
 if(clk_tick_count == interval_quant_ns - 1) begin
 count_next = count + 1;
 clk_tick_count_next = 0;
 end
end
assign wave = wave_reg;
endmodule

Output wave of a test for the sake of completeness, showing that this works:

waveform

Answer 1

If you want to implement something that requires a number of count N (4 for example), you can either count 0, 1, ..., N-1 (example, 0, 1, 2, 3, 0, 1...). Or you can count 1, 2, ..., N (example, 1, 2, 3, 4, 1, 2...). So if you want to end the count at the number N, start the count at 1.

Often, coding styles are just personal preferences. The offset by one condition never bothers me. While I almost always use down counting by habit because there are couple of potential benefits -- it may synthesize to be faster, and, it only requires the count parameter input at the beginning clock (usually not an beneficial feature in a continuous loop like here).

Answer 2

Well for starters you are using two always blocks where one would be fine. Merging everything into one block gets rid of all those ugly "next" signals.

We can also make the output a reg getting rid of the wave_reg signal.

module sqwaveGen
#( 
parameter interval_quant_ns = 5
)
(
input wire clk,
input wire rst,
output reg wave,
input wire [3:0] on_interval, 
input wire [3:0] off_interval 
);
reg [3:0] clk_tick_count, clk_tick_count;
reg [3:0] count;
always @(posedge clk, negedge rst) begin
 if(~rst) begin
 wave_reg <= 0;
 clk_tick_count <= 0;
 count <= 0;
 end else begin 
 clk_tick_count <= clk_tick_count + 1;
 if(wave == 0 && count == off_interval) begin
 wave <= 1;
 count <= 0;
 end
 if(wave == 1 && count == on_interval) begin
 wave <= 0;
 count <= 0;
 end 
 if(clk_tick_count == interval_quant_ns - 1) begin
 count <= count + 1;
 clk_tick_count <= 0;
 end
 end
end
endmodule

Answer 3

You could take the "interval_quant_ns - 1" out of your logic and hide it away at the top of the file where you won't often be looking.

Counting your wave counter down (rioraxe's tip) + a modification to Peter Green's code enables you to trim off a few lines too:

module sqwaveGen #(parameter interval_quant_ns = 5)
 (
 input wire clk,
 input wire rst,
 output reg wave,
 input wire [3:0] on_interval, 
 input wire [3:0] off_interval 
 );
 localparam interval_quant_count_to = interval_quant_ns - 1; /* Shh don't tell anyone I'm here */
 reg [3:0] clk_tick_count;
 reg [3:0] count;
 always @(posedge clk, negedge rst) begin
 if(~rst) begin
 wave <= 0;
 clk_tick_count <= 0;
 count <= 0;
 end
 else begin 
 clk_tick_count <= clk_tick_count + 1;
 if(clk_tick_count == interval_quant_count_to) begin
 count <= count - 1;
 clk_tick_count <= 0;
 end
 if(count == 0) begin // now all you need to keep track of is when the counter hits 0; then you toggle the wave and set the counter up for the next interval
 count <= (wave == 1) ? off_interval : on_interval;
 wave <= ~wave;
 end
 end
 end
endmodule