Assuming motorruns
is intended to be a signal rather than an asynchronous reset, then for synchronous logic you shouldn't be using the signal in the sensitivity list.
Edge-sensitivity should only be used for the clock signal (posedge clock
), and optionally an asynchronous reset signal (to clear or set registers immediately regardless of the clock signal).
Instead you would use a synchronous edge-detector circuit to detect the rising edge of your input signal, and check the output of the edge detector in your counter logic.
So how would you code it?
If your input signal (motorruns) is not synchronous to the clock domain (e.g. it comes from the outside world), then you should first add a synchroniser chain to prevent metastability and/or concurrency issues:
// This is a 2-flop synchroniser chain. Two is usually enough, but you could add a third flop if desired.
reg sigInSync;
reg sigIn;
always @ (posedge clk) begin
sigInSync <= motorruns;
sigIn <= sigInSync;
end
The first flop in the above circuit samples the input signal on a rising clock edge. In an ideal world this would be enough. However in reality the input signal could change just as the clock edge occurs, causing the output of the flop to go metastable (literally ends up in a state somewhere between high and low, eventually picking a state after a short time period). The second flop prevents this metastable state from continuing on into the rest of the logic, by not sampling again until the next clock edge (giving time for the first to have stabilised).
Now that we have a synchronous signal, we can detect a rising edge by simply comparing the current state of the signal with its state in the previous clock cycle. If it is currently high, but used to be low, then we have a rising edge (for falling edge detection you'd check if it is currently low but used to be high):
wire risingEdge;
reg sigInDly;
always @ (posedge clk) begin
sigInDly <= sigIn;
end
assign risingEdge = sigIn && !sigInDly;
This will produce a signal synchronous to clock, which will be high for precisely one clock cycle whenever there is a rising edge on the input signal.
Your counter code then no longer needs to do edge detection, it can simply check on each clock cycle to see if the output of the edge detector is high. So your code would become:
reg [<COUNTER_WIDTH>-1:0] count;
always @(posedge clk) begin
if (risingEdge) begin
// Motorruns just went high
count <= 200;
end else /*if(count > 0)*/ begin // Uncomment if you want to prevent the counter wrapping around.
// Otherwise
count <= count - 1;
end
// Could be in a different always block as @toolic suggests:
if(count = 0) begin
"do motor is not running stuff here"
end
end
(Note that your counter needs a width setting, otherwise it's only 1-bit wide!)
You would usually also want to add a reset signal to your code. This could either be synchronous, or asynchronous. For async you would add it into the sensitivity list along with the clock.
always @(posedge clk /*or posedge reset*/) begin //Uncomment the posedge reset check if asynchronous.
if (reset) begin
~ do reset here ~
end else begin
~ not in reset here ~
end
end
When using asynhronous resets, you usually actually want "async assert, sync deassert" to ensure that all blocks which use the same reset signal come out of reset on the same clock edge. Otherwise you risk concurrency and metastability issues occurring. This is acheived using a reset synchroniser chain:
localparam CHAIN_LEN = 3; //Min 2.
reg [CHAIN_LEN-1:0] resetChain;
wire resetOut;
always @ (posedge clk or posedge resetSource) begin
if (resetSource) begin
resetChain <= {(CHAIN_LEN){1'b1}};
end else begin
resetChain <= {1'b0,resetChain[CHAIN_LEN-1:1]};
end
end
// This reset signal will go high immediately when resetSource goes high (async assert), but will
// go low synchronously to the clock signal approximately CHAIN_LEN clock cycles after resetSource goes low.
assign reset = resetChain[0];