# Forum: FPGA, VHDL & Verilog Making a frequency reducer

Rate this post
 • ▲ useful ▼ not useful
I am working with an online learning tool for VHDL, trying to make a
frequency reducer using generics.
The basic idea is: Set 'DIVIDE_BY' to a value, then get 'clk_en_out' = 1
on every 'DIVIDE_BY'th rising edge of 'clk_in' for one pulse, before
setting it back to 0.

E.g.: DIVIDE_BY = 8: wait for 7 rising edges, on the 8th, set clk_en_out
= 1, on the 9th, set it back to 0, repeat.

If 'reset' = 1, reset the cycle and start at 0.

Unfortunately, I do not know what values the tool tests, so I cannot
check why it doesn't work/on what pulse it wants me which test instance
to be 1.
I do know though that 'DIVIDE_BY' is less than 16, so an overflow of my
'counter' signal should not be the problem.

In the attached screenshot, the 'valid' line stops being 1 on rising
edge 4, yet when I twisted the code so as to have 'clk_en_a' (test
instance one; the second one is 'clk_en_b') be 1 there, it didn't
change. I assumed that trying to get 'b' to be 1 there would be
pointless, as that would make 'a' be 1 much earlier, which in theory
should set the valid line to 0 much earlier.

Can someone help me figure out why it does not work?

 1 library ieee;  2 use ieee.std_logic_1164.all;  3 use ieee.numeric_std.all;  4 5 entity ClockEnableGenerator is  6 7 generic (  8  DIVIDE_BY : integer  9 );  10 11 port (  12  clk_in : in std_logic;  13  clk_en_out : out std_logic;  14  reset : in std_logic  15 );  16 end ClockEnableGenerator;  17 18 architecture rtl of ClockEnableGenerator is  19 20 signal counter : unsigned(3 downto 0) := (others => '0');  21 signal clk_out2 : std_logic := '0';  22 23 begin  24 process(clk_in)  25  begin  26  if(rising_edge(clk_in)) then  27  counter <= counter + 1;  28  if (to_integer(counter) = DIVIDE_BY) then --reset the counter at 'DIVIDE_BY'  29  counter <= (others => '0');  30  clk_out2 <= '1'; --set the out2 signal  31  else  32  clk_out2 <= '0';  33  end if;  34  end if;  35  if reset = '1' then --reset on HIGH reset input  36  counter <= (others => '0');  37  clk_out2 <= '0';  38  end if;  39 end process;  40 clk_en_out <= clk_out2; --update out to out2  41 end architecture rtl; 

Rate this post
 • ▲ useful ▼ not useful
It looks fairly fine, all in all.

One problem is the well known one-by-one error: you are counting one
step to far. With DIVIDE_BY-1 set to 8 you will see:

0,1,2,3,4,5,6,7,8

Try it this way:

if (to_integer(counter) = DIVIDE_BY-1) then...

An additional hint: why don't you simply use an integer as counter?

Eric J. wrote:
> In the attached screenshot
What's the testbench code for that waveform?

: Edited by Moderator

Rate this post
 • ▲ useful ▼ not useful
Hi, thanks for the reply.
I tried the -1 in the if... didn't work either, unfortunately.

Uh, good question with the integer. Most of the code was copied from my
solution for a previous task, that's why it is a binary vector.

For the screenshot, I do not know what the testbench code is... that's
the problem with the online tool.

Rate this post
 • ▲ useful ▼ not useful
Eric J. wrote:
> I do not know what the testbench code is...
The testbench code is useless for counters more than 7..8 clocks,
because then the external reset pops up.

Rate this post
 • ▲ useful ▼ not useful
I had a chance to put your code on a testbench. What I found: you have a
weird async reset structure:
 1 process(clk_in)  2  begin  3  if rising_edge(clk_in) then  4  :  5  end if;  6  if reset = '1' then  7  :  8  end if;  9 end process; 
That means: your sensitivity list is incomplete, reset is missing. Due
to that you see a strange behaviour of your design: although reset pop
up at a absolutely random time, the counter value is reset with the next
change of the clk_in and therefore at the falling(!!) edge of this
clk_in.

Two solutions:
1. Add the reset to the sensitivity list
 1 process(clk_in, reset)  2  begin  3  if rising_edge(clk_in) then  4  :  5  end if;  6  if reset = '1' then  7  :  8  end if;  9 end process; 
2. Change the structure to a synchronous reset:
 1 process(clk_in)  2  begin  3  if rising_edge(clk_in) then  4  :  5  if reset = '1' then  6  :  7  end if;  8  end if;  9 end process; 
I prefer the unmentioned third one, so I get a synchronous design with
no hazzle about a sensitivity list at all:
 1 process begin  2  wait until rising_edge(clk_in);  3  :  4  :  5  if reset = '1' then  6  :  7  end if;  8 end process; 

See my solutions attached. I wrote a simple testbench generatin an
100MHz clock and a reset signal every 331ns. Both of them behave the
same and like I expect them to.

: Edited by Moderator

Rate this post
 • ▲ useful ▼ not useful
Thank you for the detailed explanation. Made my code prettier with the
reset.
It didn't fix the problem, though; after some trial and error, I landed
at:

 1 library ieee;  2 use ieee.std_logic_1164.all;  3 use ieee.numeric_std.all;  4 5 entity ClockEnableGenerator is  6 7 generic (  8  DIVIDE_BY : integer := 1  9 );  10 11 port (  12  clk_in : in std_logic;  13  clk_en_out : out std_logic;  14  reset : in std_logic  15 );  16 end ClockEnableGenerator;  17 18 architecture rtl of ClockEnableGenerator is  19 20 signal counter : integer range -1 to 15 := 0;  21 22 begin  23 process(clk_in, reset)  24  begin  25  if(rising_edge(clk_in)) then  26  counter <= counter + 1;  27  if (counter = DIVIDE_BY-2) then --reset the counter at 'DIVIDE_BY'  28  counter <= -1;  29  clk_en_out <= '1'; --set the out2 signal  30  else  31  clk_en_out <= '0';  32  end if;  33  end if;  34  if reset = '1' then --reset on HIGH reset input  35  counter <= 0;  36  clk_en_out <= '0';  37  end if;  38 end process;  39 end architecture rtl; 

Note that counter is set to -1 once the DIVIDE_BYth rising edge is
reached - because the interval clk_out_en is 1 apparently should not be
counted. By giving the counter one more rising edge to count, the
problem is fixed.

Thank you for the help!

Rate this post
 • ▲ useful ▼ not useful
Eric J. wrote:
> the problem is fixed.
What "problem"? As far as I see the problem is your understanding of
what should happen.

> because the interval clk_out_en is 1 apparently should not be counted.
Look on any µC or any other computing device with a counter inside all
over the world: each of them continues counting when reporting an
overflow or a match or something else.

> because the interval clk_out_en is 1 apparently should not be counted.
Based on what "appearance" do you come to this conclusion? What should
happen, when DIVIDE_BY is 1? If you have a clock of 10 MHz and DIVIDE it
BY  1, then the clockenable must be '1' all the time. Because dividing
by 1 means "run with the clocks frequency". And exactly thats what my
code does (see the screenshot)...

Running your code against my testbench leads to
 1 # RUNTIME: Fatal Error: RUNTIME_0067 design.vhd (24): Value 16 out of range (-1 to 15).  2 # KERNEL: Time: 165 ns, Iteration: 0, Instance: /tb_cegenerator/uut, Process: line__20. 
And its obviously why: you initialize counter with 0. Then you set
DIVIDE_BY to 1. Then you assign a clock and do this comparison "if
(counter = DIVIDE_BY-2) then..." which is the same as "if (counter = -1)
then...". And as counter value is "0 and increasing" that compare
against -1 is always wrong and the counter counts on and on until it
reaches the end of its range generating an error message.

Check it out yourself: https://www.edaplayground.com/x/DZuL

: Edited by Moderator