Hardware Clock Divider

Question

I have created a clock divider that works as follows:

• If the division factor is 0, pass the clock through unchanged.
• Otherwise, flip the output clock signal after the specified number of input clock cycles has passed.

I am relatively new to hardware design, so I would like advice on general best practices that I may have violated, as well as whether there would be any robustness issues. (I know that the output clock will be delayed since it passes through gates, but I don't think this is a problem as long as downstream logic does not attempt to use both clocks simultaneously.)

library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use IEEE.NUMERIC_STD.ALL;

entity config_clk_div is
    Port ( clk : in STD_LOGIC;
           divfactor : in STD_LOGIC_VECTOR (15 downto 0);
           clkout : out STD_LOGIC);
end config_clk_div;

architecture Behavioral of config_clk_div is
    signal clk_counter : UNSIGNED(15 downto 0);
    signal current_clk : STD_LOGIC;
begin
    process (clk, divfactor) is
    begin
        if divfactor = x"0000" then
            clkout <= clk;
            current_clk <= clk;
        else
            if rising_edge(clk) then
                clk_counter <= clk_counter + 1;
                if clk_counter >= unsigned(divfactor) then
                    clk_counter <= to_unsigned(1,16);
                    current_clk <= not current_clk;
                    -- Not here as well to avoid lag
                    clkout <= not current_clk;
                else
                    clkout <= current_clk;
                end if;
            end if;
        end if;
    end process;

end Behavioral;

Answer 1

Yes, there are some issues.

With process (clk, divfactor) you're making it difficult for the synthesis software to generate a nice clocked process. So you should separate the clocked process from the clock multiplexer.

A bigger problem is the clock multiplexer. divfactor is — even if it is synchronous — a 16-bit value, for which all bits have to be checked for 0. This introduces quite some logic, which will always be glitchy... and you're putting it directly into your clock output. danger! danger! A glitchy clock results in undefined behavior... it can even lock your FSMs to an unresolvable state.

Instead check the condition in a clocked process and set a single synchronous output based on its value.

This is how I would write something like this:

library ieee;
use ieee.std_logic_1164.all;

entity config_clk_div is
    port (
        clk : in std_logic;
        divfactor : in std_logic_vector (15 downto 0); -- Note this is actually not a factor!
        clkout : out std_logic);
end entity;

architecture rtl of config_clk_div is
    use ieee.numeric_std.all;
    signal clk_counter : unsigned(15 downto 0) := (others => '0');
    signal div_clk : std_logic := '1';

    signal divider_disabled : std_logic := '0';
begin
    process (clk)
    begin
        if rising_edge(clk) then
            if divfactor = x"0000" then
                divider_disabled <= '1';
            else
                divider_disabled <= '0';
                if clk_counter = 0 then
                    clk_counter <= unsigned(divfactor) - 1;
                    div_clk <= not div_clk;
                else
                    clk_counter <= clk_counter - 1;
                end if;
            end if;
        end if;
    end process;

    clkout <= clk when divider_disabled = '1' else div_clk;
end architecture;

Note: counting down on a counter makes the compare (to 0) a bit easier. Although this makes more of a difference when designing ASICs.