What is the correct implementation of handling asynchronous signals in an FSM?

Question

We are implementing an Ethernet MAC controller in VHDL..

To start of, here is a code snippet of my code..

-- next state
PROCESS(p_state, phy_start, phy_ctr, phy_clk)
BEGIN
    CASE p_state IS
        WHEN sIDLE =>
            IF(phy_start = '1' or rising_edge(phy_start)) THEN
                n_state <= sPRE;
            ELSIF(phy_start'event AND phy_start='0') THEN
                n_state <= n_state;
            ELSE
                n_state <= sIDLE;
            END IF;
     ............

The problem is that my professor told me I associated phy_start as the clock signal where in the rising_edge() must be only associated to only one clock which is phy_clk. What I want to happen is when phy_start asserts, it would go to sPRE state at the next clock cycle. The assertion is done in the rising edge of the clock. I have tried

    PROCESS(p_state, phy_start, phy_ctr, phy_clk)
BEGIN
    CASE p_state IS
        WHEN sIDLE =>
            IF(phy_start = '1') THEN
                n_state <= sPRE;
            ELSIF(phy_start'event AND phy_start='0') THEN
                n_state <= n_state;
            ELSE
                n_state <= sIDLE;
     .............

but it does not enter the phy_start = '1' because it happened in the transition.. (there is what we call the setup time in which the data must be stable in that duration in order to be read correctly). What is the correct implementation then? Or I have no choice but to assert the phy_start for 2 clock cycles if the assertion happens in the rising edge, or phy_start must be asserted in the falling edge of the clock. ALso, what is the correct set of sensitivity list for the next state logic?

Answer 1

If everything is clocked under phy_clk, you should never use rising_edge() or 'event on other signals. These are associated to the clocks, not the signals. If you want to detect when a signal clocked on phy_clk rises, you should proceed like this:

process(phy_clk,nreset)
begin
  if nreset = '0' then
    phy_start_d <= '0';
  elsif phy_clk = '1' and phy_clk'event then
    phy_start_d <= phy_start;
  end if;
end process;
phy_start_p <= phy_start and not phy_start_d;

the phy_start_p signal is set to 1 only when phy_start rises, and it's totally synchronous with phy_clk;

Answer 2

If phy_start is created synchronously by another process, then you have no problem.

Just read it in your other synchronous process, and compare against a value you stored from the last clock cycle, to detect when the signal has changed from 0 to 1:

process (phy_clk)
  variable last_start : std_logic := '0';
begin
  if rising_edge(phy_clk) then
     if phy_start = '1' and last_start = '0' then
        -- do something in here
     end if;
     last_start := phy_start;
  end if;
end process;

Answer 3

There has been a related question with answers recently, which you can find at: how many processes i need to monitor two signals?

It is useful to keep in mind that VHDL is for describing hardware (design), so the synthesis tool can convert the description to fit the available hardware, which is typically flip flops (sequential design) and gates (combinatorial design). The synthesis tools does typically have some recommendations for writing VHDL, so the translation to hardware will work smoothly.

For flip flops with asynchronous reset (rst) and rising edge clock (clk), with next value generated by optional gates, the VHDL is typically:

-- Template for flip flops
process (clk, rst) is
begin
  if rising_edge(clk) then
    -- Flip flops outputs updated on rising edge of clk
  end if;
  if rst = '1' then
    -- Flip flops outputs assigned with constant value when rst is '1'
  end if;
end process;

Only rst and clk should be in the sensitivity list, so other signals used in expressions in the process should not be included. Any expressions used to generate the value for the flip flop will be converted to gates by the tool.

Only rising edge clock should be used for flip flop, unless there is a good reason to use falling edge clock, or even both edges, since using only a single edge will make it easier to do the timing constraining.

If asynchronous reset is not used, then leave out the rst from the sensitivity list and remove the related if statement.

For pure gates the VHDL is typically, assuming use of VHDL-2008:

-- Template for gates
process (all) is
begin
  -- Gates outputs updated based
end process;

or for simple expression just drop the process and write:

my_signal <= my_expression;

So back to the specific code, then it is possible to write this as a single process with phy_clk as clock:

PROCESS (phy_clk)
BEGIN
    IF RISING_EDGE(phy_clk) THEN
        CASE p_state IS
            WHEN sIDLE =>
                p_state <= ...  -- New value for state based on signals

When it is required to react on changes in signals, like phy_start going from '0' to '1', then a signal with a single cycle delayed version of phy_start can be made, for example phy_start_ff, and the an expression can be written in the code like:

if (phy_start_ff = '0') and (phy_start = '1') then  -- phy_start from 0 to 1
  ..