Is the use of rising_edge on non-clock signal bad practice? Are there alternatives?

Question

I am working on a VHDL design and I have it working, but the code is pretty ugly and the fact that it seems that I am trying to work around the language's design to accomplish my goal makes me feel like something is wrong. I'm pretty new to VHDL, but I have been working on smaller chunks of the project for nearly a month so I have the general idea. However, This part is a bit more complex.

I need a process that will create a one clock period long pulse (LOAD_PULSE) after the rising edge of a signal (END_ADC), but not until 4 clocks has passed from the latest rising edge of that signal (END_ADC) OR the falling edge of a second signal (LVAL).

To accomplish the wait period, I have built a timer module that counts microseconds and periods, here:

entity uS_generator is
    generic(
        Frequency       : integer := 66                                     -- Frequency in MHz
    );
    Port ( 
        CLK     : in STD_LOGIC;
        RESET   : in STD_LOGIC;
        T_CNT   : out integer range Frequency downto 1 := 1;
        uS_CNT  : out integer range 65535 downto 0 := 0
    );
end uS_generator;

architecture behavior of uS_generator is

    signal T_CNT_INT        : integer range Frequency downto 1 := 1;        -- Counter for 1 uS
    signal uS_CNT_INT       : integer range 65535 downto 0 := 0;

begin

    COUNT: process(CLK, RESET)
    begin
        if RESET = '1' then
            T_CNT_INT   <= 1;
            uS_CNT_INT  <= 0;
        elsif rising_edge(CLK) then
            if T_CNT_INT = (Frequency - 1) then                             -- Increment one clock early so last rising edge sees one uS elapsed.
                uS_CNT_INT <= uS_CNT_INT + 1;
                T_CNT_INT <= T_CNT_INT + 1;
                if uS_CNT_INT = 65535 then
                    uS_CNT_INT <= 0;
                end if;
            elsif T_CNT_INT = Frequency then
                T_CNT_INT <= 1;
            else
                T_CNT_INT <= T_CNT_INT + 1;
            end if;
        end if;
    end process COUNT;

    T_CNT   <= T_CNT_INT;
    uS_CNT  <= uS_CNT_INT;

end behavior;

The processes I'm using for the pulse generation portion of the design are as follows:

loadPulseProc: process(PIXEL_CLK, END_ADC, RESET)
begin

    if RESET = '1' then
        PULSE_FLAG <= '0';
        LOAD_PULSE <= '0';
    elsif rising_edge(END_ADC) then
        PULSE_FLAG <= '1';
    end if;

    if rising_edge(PIXEL_CLK) then
        if PULSE_FLAG = '1' and END_ADC = '1' and LVAL <= '0' and ADC_TMR_T >= 4 and LVAL_TMR_T >= 4 then
            LOAD_PULSE <= '1', '0' after PIXEL_CLK_T;
            PULSE_FLAG <= '0';
        end if;
    end if;

end process loadPulseProc;

ADCTimerProc: process(END_ADC, RESET)
begin

    if RESET = '1' then
        ADC_TMR_RST <= '1', '0' after PIXEL_CLK_T/10;
    end if;

    if rising_edge(END_ADC) then
        ADC_TMR_RST <= '1', '0' after PIXEL_CLK_T/10;
    end if;

    if falling_edge(END_ADC) then
        ADC_TMR_RST <= '1', '0' after PIXEL_CLK_T/10;
    end if;

end process ADCTimerProc;

LVALTimerProc: process(LVAL, RESET)
begin

    if RESET = '1' then
        LVAL_TMR_RST <= '1', '0' after PIXEL_CLK_T/10;
    end if;

    if rising_edge(LVAL) then
        LVAL_TMR_RST <= '1', '0' after PIXEL_CLK_T/10;
    end if;

    if falling_edge(LVAL) then
        LVAL_TMR_RST <= '1', '0' after PIXEL_CLK_T/10;
    end if;         

end process LVALTimerProc;

PIXEL_CLK_T is the period of the clock, 15.152 ns.

This design works, simulation shows that it does as I require, but only after significant hassle avoiding errors due to using multiple rising_edge of falling_edge calls by separating them into separate if statements that really should be together. As far as I've read using rising_edge and falling_edge seems to be reserved for clocks only, so is this just bad practice? How can avoid this behavior but still create the same output?

Thanks!

Your code is not synthesis eligible it does not describe hardware in a Hardware Description Language. The delay imposed by after PIXEL_CLK_T/10 is not acted on nor is more than one waveform element in a signal assignment. Using both edges to control the same signal is not possible (double data rate registers are specific I/O cells in FPGAs). See the now withdrawn IEEE Std 1076.6-2004 (RTL synthesis) or your favorite vendor's synthesis documentation. — user1155120
If you use rising_edge (or the equivalent 'event and =1 form) you have created a clock signal. Sometimes this is appropriate, but often it is bad practice (and you would be better re-clocking the signal, and comparing the new and old values). — user_1818839

Paebbels Paebbels · Accepted Answer · 2016-05-04T19:15:31

Yes, rising_edge()/falling_edge() should only be used for clock signal. While it works in simulation it can cause problems and unintended hardware in synthesis.

Synthesis tools infer a clock from the function's argument and place such signals/wires on special tracks in the FPGAs (assuming you are targeting an FPGA for your design). The tool will further infer special clock buffers and warn if your input clock pin is not a clock capable pin.

Introducing several clocks can lead to asynchronous designs and make it vulnerable for cross clock faults.

Detecting a rising or falling edge on a signal is done by an edge detection circuit like the following which compares the signal in the previous clock cycle to the current value.

Needed signals:

signal mySignal_d : std_logic := '0'; 
signal mySignal_re : std_logic;

Needed logic:

mySignal_d <= mySignal when rising_edge(Clock); 
mySignal_re <= not mySignal_d and mySignal;

This first line translates to an 1-bit D-flipflop (You could also use a process). The second lines generates a one cycle strobe signal when mySignal changes from low to high. I'm using *_d to indicate a delayed signal of the original input and *_re for rising edge.

The generated signal is still synchronous to Clock.

Is the use of rising_edge on non-clock signal bad practice? Are there alternatives?

1 Answers