[SOLVED] [Verilog] Clock enable causes glitch on ouptut - Best resolution

pigtwo · Jan 21, 2018

Hello all,

I'm writing some verilog to get data from an ADC for an upcoming project of mine and I've run into a little problem with using a clock enable. I've come up with a solution but I want to make sure it makes sense before I move forward.

For reference below is my code:

Code Verilog - [expand]
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
`timescale 1ns / 1ps
module AD7091RBRMZ(
        input wire clk,
        input wire reset,
        
        // ADC signals
        output reg ADC_conv,
        output reg ADC_cs,
        output reg ADC_sclk,
        
        input wire ADC_sdo,
        
        // Control signals
        output reg ready,
        output reg[11:0] data_out,
        input wire convert
    );
    
    // Local parameters
    localparam use_busy_sig = 0;     // This tells the module to use the busy signal feature or not. 
    
    localparam[3:0]
        start = 0,
        idle = 1,
        convert_start = 2,
        convert_start_busy = 3,
        convert_cs_busy = 4,
        convert_cs = 5,
        convert_wait_busy = 6,
        convert_get_data = 7,
        convert_wait = 8,
        convert_done_busy = 9,
        convert_done = 10,
        convert_get_data_delay = 11;
    
    // Define local registers and wires
    reg ADC_conv_nxt, ADC_cs_nxt, ready_nxt, ADC_ce, ADC_ce_nxt, busy_init, busy_init_nxt;
    reg[3:0] state, state_nxt;
    reg[5:0] counter, counter_nxt;
    reg[11:0] data_out_nxt;
    
    //assign ADC_sclk = (ADC_ce) ? clk & ADC_ce : busy_init;
    
    always@(posedge clk or negedge clk) begin
        ADC_sclk <= (ADC_ce) ? clk : busy_init;
    end
    
    // Define sequential logic
    always@(posedge clk) begin
        if(!reset) begin
            ADC_conv <= 1;
            ADC_cs <= 1;
            ADC_ce <= 0;
            ready <= 0;
            counter <= 0;
            data_out <= 0;
            busy_init <= 0;
            state <= start;
        end else begin
            ADC_conv <= ADC_conv_nxt;
            ADC_cs <= ADC_cs_nxt;
            ADC_ce <= ADC_ce_nxt;
            ready <= ready_nxt;
            counter <= counter_nxt;
            data_out <= data_out_nxt;
            busy_init <= busy_init_nxt;
            state <= state_nxt;
        end 
    end
    
    // Define combinational logic
    always @* begin
        // Define default transistions
        ADC_conv_nxt = ADC_conv;
        ADC_cs_nxt = ADC_cs;
        ADC_ce_nxt = ADC_ce;
        ready_nxt = ready;
        counter_nxt = counter;
        data_out_nxt = data_out;
        busy_init_nxt = busy_init;
        state_nxt = state;
        
        case(state)
        
            start: begin
                ADC_conv_nxt = 1;
                ADC_cs_nxt = 1;
                ADC_ce_nxt = 0;
                ready_nxt = 0;
                counter_nxt = 0;
                data_out_nxt = 0;
                busy_init_nxt = 0;
                state_nxt = idle;
            end
            
            idle: begin
                ADC_conv_nxt = 1;
                ADC_cs_nxt = 1;
                ADC_ce_nxt = 0;
                ready_nxt = 1;
                counter_nxt = 0;
                data_out_nxt = data_out;
                if(convert) begin        // A conversion request is received
                    ready_nxt = 0;
                    if(use_busy_sig)      // Determine whether to use busy signal or not.
                        state_nxt = convert_start_busy;
                    else
                        state_nxt = convert_start;
                end else begin
                    state_nxt = idle;
                end
            end
            
            convert_start_busy: begin
                ADC_conv_nxt = 0;
                state_nxt = convert_cs_busy;
            end
            
            convert_cs_busy: begin
                ADC_conv_nxt = 1;
                ADC_cs_nxt = 0;
                if(!busy_init_nxt) begin  // If using the busy signal has not been initialized, then initalize it.  
                    state_nxt =  convert_cs;  // The rest of the initializtion sequence is to follow non-busy signal sequence. 
                end else begin
                    state_nxt = convert_wait_busy;
                end
            end
    
            convert_wait_busy: begin
                if(!ADC_sdo) begin   // The ADC finished the conversion.  
                    ADC_ce_nxt = 1;
                    counter_nxt = 0;
                    state_nxt = convert_get_data;
                end 
            end
            
            convert_start: begin  // This begins the conversion process without using the busy signal. 
                ADC_conv_nxt = 0;
                state_nxt = convert_cs;
            end
            
            convert_cs: begin
                ADC_conv_nxt = 1;
                ADC_cs_nxt = 1;
                state_nxt = convert_wait;
            end
            
            convert_wait: begin
                counter_nxt = counter + 1;
                if(counter == 33) begin
                    counter_nxt = 0;
                    ADC_ce_nxt = 1;
                    ADC_cs_nxt = 0;
                    state_nxt = convert_get_data;//convert_get_data_delay;
                end
            end
            
/*          convert_get_data_delay: begin
                ADC_ce_nxt = 1;
                data_out_nxt = {data_out[10:0], ADC_sdo};
                state_nxt = convert_get_data;
            end
*/          
            convert_get_data: begin
                data_out_nxt = {data_out[10:0], ADC_sdo};
                counter_nxt = counter + 1;
                if(counter == 11) begin  // We have all 12 bits(11 iterations plus the one bit sampled as we entered this state).
                    if(use_busy_sig && busy_init) begin    // We have to hold CE high for an extra cycle if using busy signal.
                        state_nxt = convert_done_busy;
                    end else begin
                        if(use_busy_sig)
                            busy_init_nxt = 1;     // After this the busy signal will be initialized.
                        ADC_ce_nxt = 0;
                        state_nxt = convert_done;
                    end
                end 
            end
            
            convert_done_busy: begin
                ADC_ce_nxt = 0;
                ADC_cs_nxt = 1;
                ready_nxt = 1;
                state_nxt = convert_done;  // We need a delay so another conversion doesn't start too soon.  
            end
            
            convert_done: begin  // This is a delay so another conversion doesn't start too soon.
                ADC_cs_nxt = 1;
                ready_nxt = 1;
                state_nxt = idle;
            end
            
            default: state_nxt = start; 
        endcase
    end
    
    
endmodule

Most of that is unimportant. The only real part of concern is the parts near the top dealing with the clock enable. Originally I did something like this:

Code Verilog - [expand]
1
assign ADC_sclk = (ADC_ce) ? clk : busy_init;

Where ADC_sclk was a wire and ADC_ce was clocked by 'clk'. So clearly there is a problem here because ADC_ce is clocked by 'clk' but clk is also being gated by ADC_ce. I noticed a glitch on the output of ADC_sclk when ADC_ce transitioned low.

I think the typical solution to this would be to change whether ADC_ce was positive or negative edge sensitive. But this wouldn't work for me because the 'default' value of the clock(in this case 'busy_init') could be one or zero. So changing which edge it is sensitive to would just move the glitch to another condition.

So I tried what is in the code above which is making ADC_sclk a register and making it positive and negative edge sensitive. For reference:

Code Verilog - [expand]
1
2
3
always@(posedge clk or negedge clk) begin
        ADC_sclk <= (ADC_ce) ? clk : busy_init;
    end

This seems to work but I'm a little nervous because I've never tried making a register sensitive to both edges. Is there any problems or complications that happen because of this? Or is my solution not very typical and there is a much more common way to solve this?

Thank you!

- - - Updated - - -

Well I found the problem with my solution. I was only testing in ModelSim which didn't have a problem with it. But now that I've tried synthesizing it in ISE I see that ISE says it's unsynthesizable.

- - - Updated - - -

It turns out making ADC_ce negative edge sensitive actually works perfectly. I'm not sure why I was thinking this wouldn't work. I guess I thought it would be symmetrical where transitioning on rising edges would cause problems on high to low transitions where transitioning on falling edges would would cause problems for low to high transitions. With my system transitioning on rising edges doesn't work for either and transitioning on falling edges works for both. Maybe this will be helpful for someone else with this confusion.

FvM · Jan 21, 2018

My general impression, the module looks overly complicated for a simple SPI interface.

Not synthesizable for almost any programmable logic device:

Code:

always@(posedge clk or negedge clk)

The question is what you want to achieve. Is it actually necessary to run adc_sclk at the full system clock and thus utilize clock gating? If so, a possible solution is to control the combinational gate logic by a FF clocked at negedge clk, should work without glitches.

Probably it's better to run the module at double adc_sclk rate and generate registered output signals.

KlausST · Jan 21, 2018

Hi,

Probably it's better to run the module at double adc_sclk rate and generate registered output signals.

I agree. We always use a system clock frequency that is an integer (if possible) multiple of 2 x the max used output signal frequency.
This makes live and coding easy.

But i miss informations about system clock frequency and SPI clock frequency. What frequency range are we talking about?

Klaus

pigtwo · Jan 21, 2018

My general impression, the module looks overly complicated for a simple SPI interface.

I probably made it more complicated than necessary. A lot of complexity comes from the adc having two different modes that you can switch between. Basically one mode is normal SPI while the other mode the ADC will generate a interrupt signal on the sdo line.

The question is what you want to achieve. Is it actually necessary to run adc_sclk at the full system clock and thus utilize clock gating? If so, a possible solution is to control the combinational gate logic by a FF clocked at negedge clk, should work without glitches.

It's not completely necessary to run adc_sclk at the full system clock. The main reason I choose this is I need to run the adc at 50MHz to get the throughput I'm looking for. I could run my system clock at 100MHz but the reason I didn't at first is because my logic analyzer doesn't go much over 50MHz. But I'll probably switch to 100MHz anyway and just trouble shoot on my scope instead.

For my own information, what is the down side of using a gated clock like I'm doing?

I agree. We always use a system clock frequency that is an integer (if possible) multiple of 2 x the max used output signal frequency.
This makes live and coding easy.

But i miss informations about system clock frequency and SPI clock frequency. What frequency range are we talking about?

That's good to know, I generally try to do things the more traditional way first. Initially I wasn't very sure what would be the normal way to handle this. In the code above the clk and adc_sclk are both 50MHz. ADC_sclk needs to be 50MHz but the system clock could be higher. Since it seems better I'll probably move to 100MHz for the system clock.

ads-ee · Jan 22, 2018

pigtwo said:
II could run my system clock at 100MHz but the reason I didn't at first is because my logic analyzer doesn't go much over 50MHz. But I'll probably switch to 100MHz anyway and just trouble shoot on my scope instead.

Xilinx has a tool called ChipScope that inserts a logic analyzer in the FPGA fabric, which is accessed via the JTAG programming pod. As you are using ISE, you may have to see about getting a temporary license (I'm assuming you are using ISE because you are stuck using an older part, i.e. non-7Series or newer).

FvM · Jan 22, 2018

For my own information, what is the down side of using a gated clock like I'm doing?

It's generally avoided when driving internal logic due to timing problems. I believe it's OK for external clock output with the suggested negedge triggered gating FF. Nevertheless I prefer a straightforward single edge driven logic scheme.

Welcome to EDAboard.com

[SOLVED] [Verilog] Clock enable causes glitch on ouptut - Best resolution

pigtwo

Member level 4

FvM

Super Moderator

KlausST

Advanced Member level 7

pigtwo

Member level 4

ads-ee

Super Moderator

FvM

Super Moderator

Similar threads

Part and Inventory Search

Welcome to EDABoard.com

Sponsor

Connect with us

Online statistics

Forum statistics