# Verilog to detect double rising edge

Status
Not open for further replies.

#### Rocketmagnet

##### Junior Member level 3
Hi all,

I am writing my first Verilog program, but I'm finding it quite hard. I just don't understand why it doesn't do what I expect.

Can anyone critique this for me, many thanks. I would really like to get to know Verilog better, so I can write my own peripherals for the Cypress PSoC.

The program should do this (See attached image too):
* If two rising edges are seen on the input (EDGES) within 15 clock cycles the output (CS) goes low.
* If a falling edge is seen on the input, CS goes high.

This is the code I have written so far:

Code:
module CS_Detect (
Counter,
CS,
E,
CLK,
EDGES
);
output [3:0] Counter;    // Used to time the edges
output  CS;              // The main output
output  E;               // A debugging output
// gives a pulse on rising edge of EDGES
input   CLK;
input   EDGES;

reg T;
reg E;
reg CS;
reg Counter;

always @(EDGES or T)
begin
E <= EDGES & !T;
end

always @(posedge CLK)
begin
if (EDGES & !T) begin         // CS Rising
if (Counter > 0) begin    // OMFG, the counter HASN'T timed out. This is the second rising edge!
CS = 0;
end else begin            // Counter has timed out. This is a possible first edge
Counter = 15;
end
end

if (!EDGES & T) begin         // CS Falling
CS = 1;
end

if (Counter > 0) Counter = Counter - 1;

end

always @(negedge CLK)
begin
T       = EDGES;
end

endmodule

#### Attachments

• Double_Rising_Edge.png
3.2 KB · Views: 60

#### lostinxlation

Quick ones.
1. it times out only when EDGE rises.
2. CS value is unknown till EDGE either rises or falls ? The counter has no initial value either.
3. Try to use blocking and non blocking statements properly.
4. your spec, "within 15 clocks" is not very clear. Within 15 clocks from when ?

Last edited:

#### j_andr

##### Full Member level 4
look at my version, I hope it gives an idea how to do the job;
did not test my version on a simulator, may be something needs to be fixed;
Code:
module CS_Detect
(
input      clk, edges,
output reg cs
);

reg [1:0] edges_reg;
reg [1:0] pos_edges;
reg [3:0] cnt;

always @(posedge clk)
edges_reg <= {edges[0],edges}; // just store input, shift-in reg

always @(posedge clk)
if ( edges_reg == 2'b01 )             // if rising edge
pos_edges <= {pos_edges[0], 1'b1}; //  shift in '1'

always @(posedge clk)
if ( pos_edges[0]  )     // if first pos. edge detected
cnt <= cnt + 1'b1;     // incr. the counter
else
cnt <= 4'h0;

always @(posedge clk)
if      ( edges_reg == 2'b10 )           // falling edge
cs <= 1'b1;
else if ( edges_reg == 2'b11 & !(&cnt) ) // 2 rising edges AND
// cnt < 15 (not all cnt bits are HIGH)
cs <= 1'b0;
endmodule

look at your code at the always block, where counter in assigned,

there are to assignments that can happen at the same time:
if (Counter > 0) Counter = Counter - 1;

and

if (EDGES & !T) /.../
Counter = 15;

you will get a synthesis error or the tool will decide how to implement it;

J.A
-------------------------------

a condition to clear 'edges_reg ' has to be added

Last edited:

#### Rocketmagnet

##### Junior Member level 3
1. it times out only when EDGE rises.
That's right. It's only on a rising edge that I'm interested in the time. On a falling edge, CS should always go high.

2. CS value is unknown till EDGE either rises or falls ? The counter has no initial value either.
As a beginner, I'm still not sure how to initialise the values.

3. Try to use blocking and non blocking statements properly.
That's another thing I don't fully understand. I get the general idea, but I'm not exactly sure which I should use, and when.

4. your spec, "within 15 clocks" is not very clear. Within 15 clocks from when ?

I need to detect that there have been two rising edges no more than 15 clocks apart from eachother. I.E. if it ever sees two consecutive rising edges, and the time between them is no more than 15 clocks, then CS goes low.

---------- Post added at 09:55 ---------- Previous post was at 09:53 ----------

look at my version, I hope it gives an idea how to do the job;
did not test my version on a simulator, may be something needs to be fixed.

Thanks. I'll try it out tonight.

#### Rocketmagnet

##### Junior Member level 3
On closer examination, this code cannot be correct.

Code:
 always @(posedge clk)
edges_reg <= {edges[0],edges}; // just store input, shift-in reg

I assume you meant:

Code:
 always @(posedge clk)
edges_reg <= {edges_reg[0],edges}; // just store input, shift-in reg

Furthermore, the Counter is only ever incremented on a rising edge, therefore it is unable to count the number of clocks between rising edges.

I think the problem is that I didn't describe the behaviour properly. Let me try again:

Rule 1: Whenever a falling edge is seen, the output goes high.
Rule 2: Whenever a rising edge is seen, if another rising edge was also seen no more than 15 clock cycles previously, then the output goes low.

I hope that is unambiguous. Please also see the waveform attached to the original post.

Many thanks

Hugo

#### FvM

##### Super Moderator
Staff member
Some additional comment on the original code:
- reg Counter must be changed to reg [3:0] Counter, otherwise, Counter is only synthesized with one bit
- the below comment is wrong and misunderstanding how sequential blocks are evaluated:
there are to assignments that can happen at the same time:
if (Counter > 0) Counter = Counter - 1;
and
if (EDGES & !T) /.../
Counter = 15;
you will get a synthesis error or the tool will decide how to implement it;
You don't get a synthesis error, the code has no problem in this regard
- The original edge detection is incorrect in so far, that it's only sensitive to transitions between the negative and the positive edge, which seems not to correspond to the specification
- The edge detection scheme suggested by j_andr (despite of the mentioned syntax error) is basically on the right track. I didn't however analyze his code completely, I think it has other errors, too. Particularly, it doesn't succeed in making a 15 clocks interval to accept two edges.

j_andr

### j_andr

Points: 2

#### Rocketmagnet

##### Junior Member level 3
You don't get a synthesis error, the code has no problem in this regard.

That is correct, my code synthesises, but does not function according to spec.

- The original edge detection is incorrect in so far, that it's only sensitive to transitions between the negative and the positive edge, which seems not to correspond to the specification.

No. There is code there to detect both types of edge. See the "// CS Falling" and "// CS Rising" comments.

- The edge detection scheme suggested by j_andr (despite of the mentioned syntax error) is basically on the right track.

To me is seems wrong because it only seems to be sensitive to the first two rising edges ever, not to all rising edges.

Please see the image attached to my original post.

Many thanks
Hugo

#### FvM

##### Super Moderator
Staff member
Please see the image attached to my original post.
You don't need to repeat this mantra-like statement. I think, I basically understood the intention of your specification, although the specification isn't very clear, as others already mentioned.
No. There is code there to detect both types of edge. See the "// CS Falling" and "// CS Rising" comments
I see, that my comment wasn't clear enough. But you also didn't take the effort to think about it, although you already experienced problems with your code.
I said your code is "only sensitive to transitions between the negative and the positive edge". Here I meant signal ("EDGE") transitions and clock ("CLK") edges. The wrong point is to use CLK negedge and posedge. If you consider thoroughly, what your code is doing, you'll get the point. If EDGE changes it's state between CLK rising and falling edge, the event is ignored.
To me is seems wrong because it only seems to be sensitive to the first two rising edges ever, not to all rising edges.
Yes, I mentioned that the code has errors. The basic edge detection scheme is correct, not the way how it's acting on first or second one. You'll find the basic concept, shifting the input signal into a chain of two, better three registers on one clock edge, in many applications.

#### j_andr

##### Full Member level 4
FvM said:
You don't get a synthesis error, the code has no problem in this regard
you are right, I was wrong, quartus compiled it smoothly;
have to write slightly slower, think a little bit longer;

the first example was written too fast,
but have a better version, it at least compiles without errors;
and is witty I think;
Code:
module CS_Detect
(
input      clk, edges,
output reg cs
);

reg  edges_reg = 1'b1;

always @(posedge clk)
edges_reg <= edges;

wire pos_pulse = edges & !edges_reg;

reg [14:0] train;

always @(posedge clk)
train <= {train[13:0], pos_pulse};

always @(posedge clk)
if      ( !edges )                      cs <= 1'b1;
else if ( train != 15'h0 && pos_pulse ) cs <= 1'b0;

endmodule

if 'edges' signal is asynch. to clk it has to be latched twice
and 'pos_pulse' should use latched input to detect L to H transition;

----
J.A

Last edited:
shyam4908

### shyam4908

Points: 2

#### shyam4908

##### Newbie level 6
module CS_Detect
(
input clk, edges,
output reg cs
);

reg d_edges;
wire pos_edge;
reg pos_db_edge := '0' ;

//Delay the edges signal
always@(posedge clk)
d_edges <= edges;

//postive edge detection
pos_edge = edges && !d_edges;
neg_edge = !edges && d_edges;

//number of posedge
always@(posedge clk)
if (pos_db_edge == '0' and pos_edge = '1')
pos_db_edge <= '1';
else (pos_db_edge == '1' and pos_edge = '1')
pos_db_edge <= '0'

always@(posedge clk)
d_pos_db_edge <= pos_db_edge;

//double detection
pos_db_edge_pulse = !pos_db_edge && d_pos_db_edge;

always@(clk)
if (neg_edge = '1' )
CS <= '1' ;
else if (pos_db_edge_pulse = '1')
CS <= '0'

Thanks
Shyam

endmodule

#### FvM

##### Super Moderator
Staff member
@j_andr: Your second solution is pretty straightforward, but it uses slightly more registers and logic cells than a counter approach. It can be accepted with a window length of 15, I think. The double registering of asynchronous inputs is very import, because nasty random errors are brought up, if you ignore it. It's often done by shifting the input into a 2-bit SR.

@shyam4908: Whatever your code does, it's missing a means to detect two events within a 15 clocks window, either a counter or a SR, as j_andr suggested. Thus it can't keep the specification.

#### Rocketmagnet

##### Junior Member level 3
Hi, thanks for the new code.

Code:
 always @(posedge clk)
train <= {train[13:0], pos_pulse};

always @(posedge clk)
if      ( !edges )                      cs <= 1'b1;
else if ( train != 15'h0 && pos_pulse ) cs <= 1'b0;

Isn't there a chance here that train will always be 0x01 during the last if statement?

The problem I see with this approach is that a valid train looks like:
* 0001111110000000

and an invalid train looks like:
* 1111111110000000

So to be valid:
* train must be non-zero
* train[14] must be zero
* train[0] must be set after train is tested for validity.

Perhaps it should be:
Code:
 always @(posedge clk)
if      ( !edges )                           cs <= 1'b1;
else if ( !train[14] && train && pos_pulse ) cs <= 1'b0;

always @(posedge clk)
train <= {train[13:0], pos_pulse};

I'll try this tonight when I get home from work.

Many thanks again
Hugo

#### FvM

##### Super Moderator
Staff member
The problem I see with this approach is that a valid train looks like:
* 0001111110000000

and an invalid train looks like:
* 1111111110000000
Sounds like you misunderstood the code. Both cases can't happen because a "one" in the train SR is marking a rising edge of the EDGE input rather than a high state. You'll have exactly a single "one" in the train register, when the logic triggers.

P.S.: Exact fall back condition and possible retriggering hasn't been specified yet.

#### Rocketmagnet

##### Junior Member level 3
Sounds like you misunderstood the code. Both cases can't happen because a "one" in the train SR is marking a rising edge of the EDGE input rather than a high state. You'll have exactly a single "one" in the train register, when the logic triggers.

Aah, yes. Sorry. I missed that.

P.S.: Exact fall back condition and possible retriggering hasn't been specified yet.

I'm sure it has:
Rule 1: Whenever a falling edge is seen, the output goes high.
Rule 2: Whenever a rising edge is seen, if another rising edge was also seen no more than 15 clock cycles previously, then the output goes low.

P.S. Did you look at the attached image?

Hugo

#### FvM

##### Super Moderator
Staff member
P.S. Did you look at the attached image?
Yes, as already mentioned. And although you possibly think, it doesn't clearly specify all cases. The latest text however does, as far as I see.
- The waveform neither shows the clock nor defines what's exactly considered a clock cycle. Your initial code demonstrates, why this matters.
- It suggests a fall-back condition, but doesn't claraify, if it's e.g. valid also during the 15 clocks window
So all implementations have been partly based on guesses.

#### Rocketmagnet

##### Junior Member level 3
- The waveform neither shows the clock nor defines what's exactly considered a clock cycle. Your initial code demonstrates, why this matters.
- It suggests a fall-back condition, but doesn't claraify, if it's e.g. valid also during the 15 clocks window
So all implementations have been partly based on guesses.

That's a good point. Fortunately, it's not super critical. Anything outside the spec can cause undefined behaviour (as long as it doesn't start WWIII). This function only really needs to distinguish between two types of event: a double rising edge, and a single rising edge. The double rising edges will always be much closer together than 15 clocks, and the single rising edges will always be much further apart than 15 clocks. There will never be any borderline cases, e.g. where the two rising edges are exactly 15 clocks apart.

The falling edges will always be outside of the 15 clock period.

Anyway, I'll try the "train" code tonight.

Hugo

#### j_andr

##### Full Member level 4
/.../
So to be valid:
* train must be non-zero
* train[14] must be zero
* train[0] must be set after train is tested for validity.

Perhaps it should be:
/.../

may be I'm wrong but sounds like you think the order of 'always' block
has any meaning in rtl description; it's false assumption;
J.A

#### Rocketmagnet

##### Junior Member level 3
but have a better version, it at least compiles without errors;

Haha! It works!

Thanks guys, you have been very helpful, and I have learned a lot with my first verilog project. There was only one problem, which is that the synthesiser
didn't like the initalisation of edges_reg, so I left it uninitialised.

I think I will try to modify it use a counter, rather than a shift register, because I may want the time limit to be much longer in the future.

If you're interested in what I'm going to use it for, this will be going into a PSoC 3, which will be sampling data for a micro sized 6-axis force sensor. Eventually, I hope to install them in the finger tips of our robot hand.

Many thanks again everyone for all your help.

Hugo

#### Rocketmagnet

##### Junior Member level 3
I think I managed to do it with a counter in the end. Here's the code:

Code:
module CS_Detect (
CS,
CLK,
EDGES
);
output  CS;
input   CLK;
input   EDGES;

reg  edges_reg;
reg [3:0]  Counter;
reg CS;

wire  pos_pulse =  EDGES & !edges_reg;
wire  neg_pulse = !EDGES &  edges_reg;

always @(posedge CLK)
begin
edges_reg <= EDGES;
if (pos_pulse == 1) begin
Counter <= 15;
end else begin
if (Counter > 0) Counter <= Counter - 1;
end
end

always @(posedge CLK)
begin
if ((pos_pulse == 1) && (Counter > 0)) begin
CS <= 0;
end else begin
if (neg_pulse == 1) begin
CS <= 1;
end
end
end

endmodule

Many thanks again for all you taught me.

Hugo

#### FvM

##### Super Moderator
Staff member
Looks good in general. If EDGE is asnychronous to clk, you should observe what's said about double registering. Otherwise, random errors can occur, either triggering of the output on first pulse or ignorance of input events.

Status
Not open for further replies.