Welcome to EDAboard.com

Welcome to our site! EDAboard.com is an international Electronic Discussion Forum focused on EDA software, circuits, schematics, books, theory, papers, asic, pld, 8051, DSP, Network, RF, Analog Design, PCB, Service Manuals... and a whole lot more! To participate you need to register. Registration is free. Click here to register now.

Register Log in

Verilog to detect double rising edge

Status
Not open for further replies.

Rocketmagnet

Junior Member level 3
Joined
Aug 9, 2005
Messages
31
Helped
1
Reputation
2
Reaction score
0
Trophy points
1,286
Activity points
1,597
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


lostinxlation

Advanced Member level 3
Joined
Aug 19, 2010
Messages
701
Helped
197
Reputation
394
Reaction score
184
Trophy points
1,323
Location
San Jose area
Activity points
5,051
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
Joined
Mar 30, 2008
Messages
208
Helped
59
Reputation
118
Reaction score
37
Trophy points
1,308
Location
europe
Activity points
2,491
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
Joined
Aug 9, 2005
Messages
31
Helped
1
Reputation
2
Reaction score
0
Trophy points
1,286
Activity points
1,597
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
Joined
Aug 9, 2005
Messages
31
Helped
1
Reputation
2
Reaction score
0
Trophy points
1,286
Activity points
1,597
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
Joined
Jan 22, 2008
Messages
47,652
Helped
14,075
Reputation
28,407
Reaction score
12,742
Trophy points
1,393
Location
Bochum, Germany
Activity points
276,859
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.
 
  • Like
Reactions: j_andr

    j_andr

    points: 2
    Helpful Answer Positive Rating

Rocketmagnet

Junior Member level 3
Joined
Aug 9, 2005
Messages
31
Helped
1
Reputation
2
Reaction score
0
Trophy points
1,286
Activity points
1,597
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
Joined
Jan 22, 2008
Messages
47,652
Helped
14,075
Reputation
28,407
Reaction score
12,742
Trophy points
1,393
Location
Bochum, Germany
Activity points
276,859
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
Joined
Mar 30, 2008
Messages
208
Helped
59
Reputation
118
Reaction score
37
Trophy points
1,308
Location
europe
Activity points
2,491
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

Newbie level 6
Joined
Dec 1, 2009
Messages
14
Helped
0
Reputation
0
Reaction score
0
Trophy points
1,281
Activity points
1,402
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
Joined
Jan 22, 2008
Messages
47,652
Helped
14,075
Reputation
28,407
Reaction score
12,742
Trophy points
1,393
Location
Bochum, Germany
Activity points
276,859
@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
Joined
Aug 9, 2005
Messages
31
Helped
1
Reputation
2
Reaction score
0
Trophy points
1,286
Activity points
1,597
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
Joined
Jan 22, 2008
Messages
47,652
Helped
14,075
Reputation
28,407
Reaction score
12,742
Trophy points
1,393
Location
Bochum, Germany
Activity points
276,859
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
Joined
Aug 9, 2005
Messages
31
Helped
1
Reputation
2
Reaction score
0
Trophy points
1,286
Activity points
1,597
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
Joined
Jan 22, 2008
Messages
47,652
Helped
14,075
Reputation
28,407
Reaction score
12,742
Trophy points
1,393
Location
Bochum, Germany
Activity points
276,859
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
Joined
Aug 9, 2005
Messages
31
Helped
1
Reputation
2
Reaction score
0
Trophy points
1,286
Activity points
1,597
- 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
Joined
Mar 30, 2008
Messages
208
Helped
59
Reputation
118
Reaction score
37
Trophy points
1,308
Location
europe
Activity points
2,491
/.../
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
Joined
Aug 9, 2005
Messages
31
Helped
1
Reputation
2
Reaction score
0
Trophy points
1,286
Activity points
1,597
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
Joined
Aug 9, 2005
Messages
31
Helped
1
Reputation
2
Reaction score
0
Trophy points
1,286
Activity points
1,597
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
Joined
Jan 22, 2008
Messages
47,652
Helped
14,075
Reputation
28,407
Reaction score
12,742
Trophy points
1,393
Location
Bochum, Germany
Activity points
276,859
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.
Toggle Sidebar

Part and Inventory Search

Welcome to EDABoard.com

Sponsor

Top