Counter Glitches Question

Hello, I am using my FPGA to write 1024 bytes to another device. I am
noticing "sometimes" it writes 1025 bytes. I am wondering it this
could be a timing glitch. Here is basically the code I have


**************************************

data_en <= data_en_w;
data_out <= data_out_w;

process(clk1)
begin
if(clk1'event and clk1 = '1') then
stop <= '0';
if (senddata = '1') then
if(counter1 < 1024) then -- potentially write more than one
byte here by timing glitch???
counter1<= counter1 + 1;
data_en_w <= '1';
data_out_w <= buffer(counter1);
else
counter <= 0;
data_en_w <= '0';
stop <= '1'; -- possibly a timing glitch here?
end if;
end if;
end proceess;

trigger <= '1' when (counter2 >= 1024) else '0'; -- this is to wait
for other clock domain (clk2) to start process for clk1

process(trigger,stop)
begin
if (trigger = '1') then
senddata <= '1';
elsif(stop = '1') then
senddata <= '0';
end if;
end process

process(clk2)
begin
if (clk2'event and clk2 = '1') then
counter2 <= counter2 +1;
-- other logic
end if;
end process;

This is on a Spartan 3 with clk1 operating at 48MHz and clk2 operating
at a frequency lower than 48MHz. I feel as if counter1 could be an
issue on the check, or stop has a glitch causing me to write one byte
over.

If anyone has the patience to read this please let me know what they
think.

Since the data is sent every clock and the clock speed is low I don't
know why I would have problems with this.

Thanks,
Lloyd

nfirtaps

nfirtaps schrieb:
> noticing "sometimes" it writes 1025 bytes. I am wondering it this
> could be a timing glitch. Here is basically the code I have

Yes, if clk1 and clk2 are not phase alligned, you have trouble using a
signal generated from clk2
with clk1 without any synchronising mechanism.

bye Thomas

Thomas thanks for your reply.

I am a little unclear that clk2 could have the problem here since it
only starts the clk1 process. The clk1 process then stops itself after
writing 1024 bytes. Could you please explain a little more what you
mean? It seems to me as clk2 can start the clk1 process anytime an no
synchronizing problems would occur, because the clk1 process stops
itself.

Thanks,
Lloyd

nfirtaps wrote:
> Hello, I am using my FPGA to write 1024 bytes to another device. I am
> noticing "sometimes" it writes 1025 bytes. I am wondering it this
> could be a timing glitch.
Appears to be.

A few comments:
1. I don't see what is generating the signal 'clk2' but I'm assuming
that it is asynchronous to clk1.
2. The process that generates 'senddata' will infer a latch since it
does not assign senddata anything if 'trigger' and 'stop' are both '0'.
I understand that logically this is probably what you want to do, but
transparent latches are always a problem in an FPGA environment; make
it a clocked process. Since the output senddata is needed in the clk1
domain you should use clk1 as the clock for this process.
3. As it is, 'senddata' is a function of both clock domains since it is
a function of 'trigger' which comes out of the clk2 domain (since it
depends on counter2) and is also a function of 'stop' (which comes out
of the clk1 domain). The usage of senddata in the clk1 process will
require that it meet a setup time relative to clk1 which, depending on
how the synthesizer actually implements the logic it may not. This is
probably at the root of why you occasionally get extra writes, senddata
is probably failing timing.

The fix for #3 is mostly the same as with #2, make sure that senddata
comes from a single clock domain (i.e. clk1). The next thing to clean
up is 'trigger' so that you can use it properly. Change trigger to be
a new signal trigger_clk2 by putting the logic for what you currently
have for trigger inside a clocked process that is clocked by clk2.
Now, you'll need to move trigger_clk2 over into the clk1 domain so
inside the clocked process that generates senddata, add a line that is
trigger <= trigger_clk2. The reason for the two step approach is that
all 10 bits of counter2 are synched to clk2 so it is basically hopeless
to look for counter2 >= 1024 within the clk1 domain, this must be done
within the clk2 domain. The result of that comparison is the new
signal trigger_clk2 which is functionally what you want, but not
synchronized to clk1 as it will need to be...hence the second step.

You might also be able to get away without the second step of trigger
<= trigger_clk2 since it appears that the logic you have for senddata
is the only thing that depends on the trigger signal. If that's the
case, then the resynchronization to the clk1 domain will happen with
senddata now being generated in the clk1 domain.

KJ

KJ,
I really appreciate your reply. I have done all the things you have
instructed but have not had any better luck. It maybe another issue I
am not aware of. Is the design you propose robust to glitches in the
counters, I have seen many people implment grey code counters? I am
mostly concerned with counter1, that it may take an extra clock cycle
to set data_en to go low. After all by it not going low at the check
(counter1 < 1024) another rising edge of the clock latches one extra
byte of data.

One point I did not make was that clk1 is an input clock, it comes in
through the FPGA and goes into a DLL. The device that reads this data
will sample data_out and data_en at the rising edges of this clock. I
have the FPGA just generating handshaking signals in sync with this
input clock. The DLL does no phase shifting or clock multiplication.
Is there some sort of issue buried by this method?

Thanks,
Lloyd

nfirtaps wrote:
> KJ,
> I really appreciate your reply. I have done all the things you have
> instructed but have not had any better luck. It maybe another issue I
> am not aware of. Is the design you propose robust to glitches in the
> counters,
If you implemented as I suggested then 'yes' it should be robust.
Where you run into problems is where signals get generated in one clock
domain and then 'used' in another clock domain. Generating counter2
off of clk2 and then checking to see if counter2 is equal to some
number while inside an 'if rising_edge(clk1)' statement is an example
of such a problem case.

> I have seen many people implment grey code counters?
Gray code counters won't be of any help here. On the surface it seems
you have some relatively straightforward function to implement. If
it's 'flaky' then this is a timing problem which needs to be solved.
Trying to use gray code will not uncover what the timing issue really
is and may end up masking it for a bit of time....which makes things
worse, since then you'll be in a situation where things might appear to
be working even though you have no idea what fixed it.

> I am
> mostly concerned with counter1, that it may take an extra clock cycle
> to set data_en to go low. After all by it not going low at the check
> (counter1 < 1024) another rising edge of the clock latches one extra
> byte of data.
>
> One point I did not make was that clk1 is an input clock, it comes in
> through the FPGA and goes into a DLL. The device that reads this data
> will sample data_out and data_en at the rising edges of this clock. I
> have the FPGA just generating handshaking signals in sync with this
> input clock. The DLL does no phase shifting or clock multiplication.
> Is there some sort of issue buried by this method?
Maybe. Can you...
1. Post your updated code including whatever is generating clk1 and
clk2?
2. Have you checked with a simulator that the design is doing what you
want it to do?
3. Post the code for your simulation testbench if you have one.

KJ

In this code clk1 = usb_ifclk, and clk2 = ddc_clk. Also, rptr =
counter1, and wptr = counter2. Trigger will now be startchunk. The
trigger is started by startchunk_clk2, and stopped by stopchunk_clk1.


Thanks again !!!

*********************************** vhdl file
*****************************
library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use IEEE.STD_LOGIC_ARITH.ALL;
use IEEE.STD_LOGIC_UNSIGNED.ALL;
use IEEE.numeric_std.all;
---- Uncomment the following library declaration if instantiating
---- any Xilinx primitives in this code.
library UNISIM;
use UNISIM.VComponents.all;

entity usb_backend is
Port (
-- Inputs/Outputs for FX2
usb_ifclk : in STD_LOGIC;
usb_slwr : out STD_LOGIC;
usb_fd : out STD_LOGIC_VECTOR(15 downto 0);
usb_ep6ff : in STD_LOGIC;
usb_ep6ef : in STD_LOGIC;

-- Inputs from Down Converter Logic
ddc_clk : in std_logic;
ddc_data : in std_logic_vector(15 downto 0);

reset : in std_logic
);
end usb_backend;

architecture Behavioral of usb_backend is

constant NBITS : integer := 14;
constant READCHUNKSIZE : integer := 511;

subtype POINTER is integer range 0 to (2**NBITS)-1;
subtype BYTECOUNT is integer range 0 to (2**NBITS)-1;

type MEMORY is array(0 to 16383) of std_logic_vector(15 downto 0);

signal rptr : POINTER;
signal wptr : POINTER;
signal rbytec : BYTECOUNT;
signal wbytec : BYTECOUNT;
signal circ : MEMORY;

signal data_out : std_logic_vector(15 downto 0);
signal readchunk : std_logic;
signal startchunk : std_logic;
signal startchunk_clk2 : std_logic;
signal stopchunk_clk1 : std_logic;

begin
usb_fd <= data_out after 1ns;

reader : process(usb_ifclk,reset) begin
if (reset = '1') then
rptr <= 0;
rbytec <= 0;
stopchunk_clk1 <= '0';
-- usb_slwr <= '0';
elsif(usb_ifclk'event and usb_ifclk = '1') then
if (readchunk = '1') then
stopchunk_clk1 <= '0';
if(rbytec = READCHUNKSIZE-1) then
stopchunk_clk1 <= '1'; -- takes one extra clock cycle to stop
end if;
if(rbytec < READCHUNKSIZE) then
data_out <= circ(rptr);
rptr <= (rptr + 1) mod 16384;
rbytec <= (rbytec + 1) mod 16384;
usb_slwr <= '1';
else
usb_slwr <= '0';
rbytec <= 0;
stopchunk_clk1 <= '1';
end if;
else
stopchunk_clk1 <= '0';
end if;
end if;
end process;

startchunk <= startchunk_clk2;

process(usb_ifclk,startchunk,stopchunk_clk1,reset)
begin
if (reset = '1') then
readchunk <= '0';
elsif(usb_ifclk'event and usb_ifclk = '1') then
if (stopchunk_clk1 = '1') then
readchunk <= '0';
elsif (startchunk = '1' and usb_ep6ef = '1') then
readchunk <= '1';
end if;
end if;
end process;

writer : process(ddc_clk,reset) begin
if(reset = '1') then
wptr <= 0;
wbytec <= 0;
elsif(ddc_clk'event and ddc_clk = '1') then
circ(wptr) <= ddc_data;
wptr <= (wptr + 1) mod 16384;
wbytec <= (wbytec + 1) mod 16384;

if (wbytec >= READCHUNKSIZE) then
wbytec <= 0;
startchunk_clk2 <= '1';
else
startchunk_clk2 <= '0';
end if;
end if;
end process;
end Behavioral;


********************** Testbench file run for ~ 3 msec
**********************
library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use IEEE.STD_LOGIC_ARITH.ALL;
use IEEE.STD_LOGIC_UNSIGNED.ALL;
use IEEE.numeric_std.all;
---- Uncomment the following library declaration if instantiating
---- any Xilinx primitives in this code.
library UNISIM;
use UNISIM.VComponents.all;

entity usb_backend is
Port (
-- Inputs/Outputs for FX2
usb_ifclk : in STD_LOGIC;
usb_slwr : out STD_LOGIC;
usb_fd : out STD_LOGIC_VECTOR(15 downto 0);
usb_ep6ff : in STD_LOGIC;
usb_ep6ef : in STD_LOGIC;

-- Inputs from Down Converter Logic
ddc_clk : in std_logic;
ddc_data : in std_logic_vector(15 downto 0);

reset : in std_logic
);
end usb_backend;

architecture Behavioral of usb_backend is

constant NBITS : integer := 14;
constant READCHUNKSIZE : integer := 511;

subtype POINTER is integer range 0 to (2**NBITS)-1;
subtype BYTECOUNT is integer range 0 to (2**NBITS)-1;

type MEMORY is array(0 to 16383) of std_logic_vector(15 downto 0);

signal rptr : POINTER;
signal wptr : POINTER;
signal rbytec : BYTECOUNT;
signal wbytec : BYTECOUNT;
signal circ : MEMORY;

signal data_out : std_logic_vector(15 downto 0);
signal readchunk : std_logic;
signal startchunk : std_logic;
signal startchunk_clk2 : std_logic;
signal stopchunk_clk1 : std_logic;

begin
usb_fd <= data_out after 1ns;

reader : process(usb_ifclk,reset) begin
if (reset = '1') then
rptr <= 0;
rbytec <= 0;
stopchunk_clk1 <= '0';
-- usb_slwr <= '0';
elsif(usb_ifclk'event and usb_ifclk = '1') then
if (readchunk = '1') then
stopchunk_clk1 <= '0';
if(rbytec = READCHUNKSIZE-1) then
stopchunk_clk1 <= '1'; -- takes one extra clock cycle to stop
end if;
if(rbytec < READCHUNKSIZE) then
data_out <= circ(rptr);
rptr <= (rptr + 1) mod 16384;
rbytec <= (rbytec + 1) mod 16384;
usb_slwr <= '1';
else
usb_slwr <= '0';
rbytec <= 0;
stopchunk_clk1 <= '1';
end if;
else
stopchunk_clk1 <= '0';
end if;
end if;
end process;

startchunk <= startchunk_clk2;

process(usb_ifclk,startchunk,stopchunk_clk1,reset)
begin
if (reset = '1') then
readchunk <= '0';
elsif(usb_ifclk'event and usb_ifclk = '1') then
if (stopchunk_clk1 = '1') then
readchunk <= '0';
elsif (startchunk = '1' and usb_ep6ef = '1') then
readchunk <= '1';
end if;
end if;
end process;

writer : process(ddc_clk,reset) begin
if(reset = '1') then
wptr <= 0;
wbytec <= 0;
elsif(ddc_clk'event and ddc_clk = '1') then
circ(wptr) <= ddc_data;
wptr <= (wptr + 1) mod 16384;
wbytec <= (wbytec + 1) mod 16384;

if (wbytec >= READCHUNKSIZE) then
wbytec <= 0;
startchunk_clk2 <= '1';
else
startchunk_clk2 <= '0';
end if;
end if;
end process;
end Behavioral;

nfirtaps schrieb:
> I am a little unclear that clk2 could have the problem here since it
> only starts the clk1 process. The clk1 process then stops itself after
> writing 1024 bytes. Could you please explain a little more what you
> mean? It seems to me as clk2 can start the clk1 process anytime an no
> synchronizing problems would occur, because the clk1 process stops
> itself.

The sendbit changes its value depending on the rising edge of clk2.
Inside your fpga, you need more than one FF to build your counter.
Delay differences between the clk2 and all FF in Clk1 will lead to
situations, where some FF of your counter see your statbit from clk2 is
'0' and other see it is '1'.

bye Thomas

Just realized I posted my vhdl code twice, and forgot to put my
testbench. Here it is:

LIBRARY ieee;
USE ieee.std_logic_1164.ALL;
use ieee.std_logic_unsigned.all;
use ieee.std_logic_arith.all;
USE ieee.numeric_std.ALL;
use std.textio.all;

ENTITY usbbackend_testbench IS
END usbbackend_testbench;

ARCHITECTURE behavior OF usbbackend_testbench IS

-- Component Declaration
COMPONENT usb_backend
Port (
-- Inputs/Outputs for FX2
usb_ifclk : in STD_LOGIC;
usb_slwr : out STD_LOGIC;
usb_fd : out STD_LOGIC_VECTOR(15 downto 0);
usb_ep6ff : in STD_LOGIC;
usb_ep6ef : in std_logic;
-- Inputs from Down Converter Logic
ddc_clk : in std_logic;
ddc_data : in std_logic_vector(15 downto 0);
reset : in std_logic
);
end component;

signal usb_ifclk_w : std_logic;
signal usb_ep6ff_w : std_logic := '0';
signal usb_ep6ef_w : std_logic := '1';
signal ddc_clk_w : std_logic;
signal ddc_data_w : std_logic_vector(15 downto 0) :=
"0000000000000000";
signal usb_slwr_w : std_logic := '0';
signal usb_fd_w : std_logic_vector(15 downto 0);
signal reset_w : std_logic := '1';

file InFile : TEXT is in "simdatain.txt";
file OutFile : TEXT is out "simdataout.txt";
BEGIN

uut: usb_backend PORT MAP(
usb_ifclk => usb_ifclk_w,
usb_slwr => usb_slwr_w,
usb_fd => usb_fd_w,
usb_ep6ff => usb_ep6ff_w,
usb_ep6ef => usb_ep6ef_w,
ddc_clk => ddc_clk_w,
ddc_data => ddc_data_w,
reset => reset_w
);

process
begin
usb_ifclk_w <= '1';
wait for 10.5 ns;
usb_ifclk_w <= '0';
wait for 10.5 ns;
end process;

process
begin
ddc_clk_w <= '1';
wait for 50 ns;
ddc_clk_w <= '0';
wait for 50 ns;
end process;

process(usb_slwr_w)
begin
if (usb_slwr_w'event and usb_slwr_w = '1') then
usb_ep6ef_w <= '0' after 23 ns;
end if;
if (usb_slwr_w'event and usb_slwr_w = '0') then
usb_ep6ef_w <= '1' after 100ns;
end if;
end process;

-- Test Bench Statements
tb : PROCESS
BEGIN

wait for 100 ns; -- wait until global set/reset completes

reset_w <= '1';

wait for 100 ns;
reset_w <= '0';

wait; -- will wait forever
END PROCESS tb;
-- End Test Bench

END;