"does not match a standard flip-flop"

P

Paul Burke

Guest
Sorry for the naive question, but my synthesis (Actel Synplify) is
giving me this message:
"The logic for ADSerial.ADShift_3 does not match a standard flip-flop"

Googling hasn't so far helped me to find what I'm doing wrong. I wpould
much appreciate any hints.

Paul Burke
 
Paul Burke wrote:
Sorry for the naive question, but my synthesis (Actel Synplify) is
giving me this message:
"The logic for ADSerial.ADShift_3 does not match a standard flip-flop"
Click to expand...

Maybe it's not a synchronous process.
Post the code.

-- Mike Treseler
 
Mike Treseler wrote:
Paul Burke wrote:
Sorry for the naive question, but my synthesis (Actel Synplify) is
giving me this message:
"The logic for ADSerial.ADShift_3 does not match a standard flip-flop"


Maybe it's not a synchronous process.
Post the code.
Click to expand...

There's a lot of it I'm a fred. I've extracted what I hope might be enough:

-- Serialiser.vhd

library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use IEEE.STD_LOGIC_ARITH.ALL;
use IEEE.STD_LOGIC_SIGNED.ALL;
..........
-- ADNSxxx state machine

signal ADState: std_logic_vector(2 downto 0);
constant Idle: std_logic_vector(2 downto 0) := "000";
constant SendDataWrite: std_logic_vector(2 downto 0) := "001";
constant SendCmdRead: std_logic_vector(2 downto 0) := "010";
constant WaitReadyRead: std_logic_vector(2 downto 0) := "011";
constant GetDataRead: std_logic_vector(2 downto 0) := "111";

signal ADShift: std_logic;
......

ADStateMachine: process( SW, ADClkGen, ADState, ADOutReg, ADClkCount) begin

if rising_edge(ADClkPreScale(PreScaleSize)) then

case ADState is

when Idle =>
if rising_edge(SW(0)) then
ADState <= SendDataWrite;
ADClkCount <= "000000001000";
ADOutReg <= "1010101000110011";
ADShift <= '1';
ADOutputEnable <= '1';
elsif rising_edge(SW(1)) then
ADState <= SendCmdRead;
ADClkCount <= "000000001000";
ADOutReg <= "0101010100000000";
ADShift <= '1';
ADOutputEnable <= '1';
else
ADShift <= '0';
ADState <= Idle;
end if;

when SendDataWrite =>
ADOutputEnable <= '1';
ADShift <= '1';
if ADClkCount = "000000000000" then
ADState <= Idle;
else
ADState <= SendDataWrite;
end if;
if rising_edge( ADClkGen(1)) then
ADClkCount <= ADClkCount - 1;
ADOutReg(15 downto 1) <= ADOutReg(14 downto 0);
ADOutReg(0) <= '0';
else
ADClkCount <= ADClkCount;
ADOutReg <= ADOutReg;
end if;

when SendCmdRead =>
ADOutputEnable <= '1';
ADShift <= '1';
if ADClkCount = "000000000000" then
ADState <= WaitReadyRead;
ADClkCount <= "111111111111";
else
ADState <= SendCmdRead;
end if;
if rising_edge( ADClkGen(1)) then
ADClkCount <= ADClkCount - 1;
ADOutReg(15 downto 1) <= ADOutReg(14 downto 0);
ADOutReg(0) <= '0';
else
ADClkCount <= ADClkCount;
ADOutReg <= ADOutReg;
end if;

when WaitReadyRead =>
ADOutputEnable <= '0';
ADShift <= '0';
ADOutReg <= ADOutReg;
if ADClkCount = "000000000000" then
ADState <= GetDataRead;
ADClkCount <= "111111111111";
else
ADState <= WaitReadyRead;
end if;
if rising_edge( ADClkGen(1)) then
ADClkCount <= ADClkCount - 1;
end if;

when GetDataRead =>
ADOutputEnable <= '0';
ADShift <= '1';
if ADClkCount <= "000000000000" then
ADState <= Idle;
else
ADState <= GetDataRead;
end if;
if rising_edge( ADClkGen(1)) then
ADClkCount <= ADClkCount - 1;
ADOutReg(15 downto 1) <= ADOutReg(14 downto 0);
ADOutReg(0) <= '0';
ADInReg(7 downto 1) <= ADInReg(6 downto 0);
ADInReg(0) <= ADDAT;
else
ADClkCount <= ADClkCount;
end if;

end case;
end if;

end process ADStateMachine;
 
On Sep 25, 10:40 am, Paul Burke <p...@scazon.com> wrote:
Mike Treseler wrote:
Paul Burke wrote:
Sorry for the naive question, but my synthesis (Actel Synplify) is
giving me this message:
"The logic for ADSerial.ADShift_3 does not match a standard flip-flop"

Maybe it's not a synchronous process.
Post the code.

There's a lot of it I'm a fred. I've extracted what I hope might be enough:

-- Serialiser.vhd

library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use IEEE.STD_LOGIC_ARITH.ALL;
use IEEE.STD_LOGIC_SIGNED.ALL;
.........
-- ADNSxxx state machine

signal ADState: std_logic_vector(2 downto 0);
constant Idle: std_logic_vector(2 downto 0) := "000";
constant SendDataWrite: std_logic_vector(2 downto 0) := "001";
constant SendCmdRead: std_logic_vector(2 downto 0) := "010";
constant WaitReadyRead: std_logic_vector(2 downto 0) := "011";
constant GetDataRead: std_logic_vector(2 downto 0) := "111";

signal ADShift: std_logic;
.....

ADStateMachine: process( SW, ADClkGen, ADState, ADOutReg, ADClkCount) begin

if rising_edge(ADClkPreScale(PreScaleSize)) then

case ADState is

when Idle =
if rising_edge(SW(0)) then
ADState <= SendDataWrite;
ADClkCount <= "000000001000";
ADOutReg <= "1010101000110011";
ADShift <= '1';
ADOutputEnable <= '1';
elsif rising_edge(SW(1)) then
ADState <= SendCmdRead;
ADClkCount <= "000000001000";
ADOutReg <= "0101010100000000";
ADShift <= '1';
ADOutputEnable <= '1';
else
ADShift <= '0';
ADState <= Idle;
end if;

when SendDataWrite =
ADOutputEnable <= '1';
ADShift <= '1';
if ADClkCount = "000000000000" then
ADState <= Idle;
else
ADState <= SendDataWrite;
end if;
if rising_edge( ADClkGen(1)) then
ADClkCount <= ADClkCount - 1;
ADOutReg(15 downto 1) <= ADOutReg(14 downto 0);
ADOutReg(0) <= '0';
else
ADClkCount <= ADClkCount;
ADOutReg <= ADOutReg;
end if;

when SendCmdRead =
ADOutputEnable <= '1';
ADShift <= '1';
if ADClkCount = "000000000000" then
ADState <= WaitReadyRead;
ADClkCount <= "111111111111";
else
ADState <= SendCmdRead;
end if;
if rising_edge( ADClkGen(1)) then
ADClkCount <= ADClkCount - 1;
ADOutReg(15 downto 1) <= ADOutReg(14 downto 0);
ADOutReg(0) <= '0';
else
ADClkCount <= ADClkCount;
ADOutReg <= ADOutReg;
end if;

when WaitReadyRead =
ADOutputEnable <= '0';
ADShift <= '0';
ADOutReg <= ADOutReg;
if ADClkCount = "000000000000" then
ADState <= GetDataRead;
ADClkCount <= "111111111111";
else
ADState <= WaitReadyRead;
end if;
if rising_edge( ADClkGen(1)) then
ADClkCount <= ADClkCount - 1;
end if;

when GetDataRead =
ADOutputEnable <= '0';
ADShift <= '1';
if ADClkCount <= "000000000000" then
ADState <= Idle;
else
ADState <= GetDataRead;
end if;
if rising_edge( ADClkGen(1)) then
ADClkCount <= ADClkCount - 1;
ADOutReg(15 downto 1) <= ADOutReg(14 downto 0);
ADOutReg(0) <= '0';
ADInReg(7 downto 1) <= ADInReg(6 downto 0);
ADInReg(0) <= ADDAT;
else
ADClkCount <= ADClkCount;
end if;

end case;
end if;

end process ADStateMachine;
Click to expand...
You cannot nest rising_edge() calls in a process. If you are trying to
determine within the state machine if an input has risen (from
previous value), then you need to store previous input value and
compare with current value.

Rising_edge() implies a clock input (i.e. a clocked register).

Andy
 
rising_edge(SW(0))

sounds like a slow signal to me. I'm betting replacing this with:

IF SW(0) = '1' THEN....

would work. Of course you have to handle switch bounce somewhere.
And I would hope that routine would synchronize it with ADClkPreScale.

Shannon
 
Oh and I forgot to mention that:

What Andy said was right on. You need to think about what your code
implies. You are waiting for one clock edge and when that happens you
look for another clock EDGE. How often would THAT occur!?!?!

AND

ADClkCount could be a free running counter that is gated by your state
machine. I had to do something very similar in a serial ADC driver
recently.

Shannon
 
Andy wrote:

You cannot nest rising_edge() calls in a process. If you are trying to
determine within the state machine if an input has risen (from
previous value), then you need to store previous input value and
compare with current value.

Rising_edge() implies a clock input (i.e. a clocked register).
Click to expand...
OK, thanks a lot. Got it. I'm telling it that various flip flops need to
be clocked by two different clocks, and I can't expect the tool to work
out how to combine the clocks when I haven't even thought of how to it
myself. Typical computer programmer's error I suppose.

My own fault for being lazy- the input is only there because I want to
try the thing out with a switch rather than the "real" signal derived
from elsewhere.

In this context, is there any way of telling it that a signal is always
to be considered asynchronous, and that it shouldn't ever be a flipflop?

Paul Burke
 
On Wed, 26 Sep 2007 09:03:34 +0100,
Paul Burke <paul@scazon.com> wrote:

OK, thanks a lot. Got it. I'm telling it that various flip flops need to
be clocked by two different clocks, and I can't expect the tool to work
out how to combine the clocks when I haven't even thought of how to it
myself.
Click to expand...
It's actually worse than that. As someone else pointed out, your
code is effectively doing this...

process (clk1, clk2)
begin
if rising_edge(clk1) then
....
if rising_edge(clk2) then
...

Note that you're not WAITING for the rising edge of clk2,
but only TESTING it; so the second "if" can succeed only if
both rising edges occur exactly simultaneously. Exact
simultaneity does have a sensible meaning in discrete-event
simulation, but of course it cannot be realised in hardware.
In simulation-only code I could reasonably write...

process -- no sensitivity list
begin
wait until rising_edge(clk1);
.... -- do some stuff
wait until rising_edge(clk2);
.... -- do some more stuff
wait until falling_edge(clk2) or rising_edge(clk1);
..... -- and so on.
end process;

But of course there is no chance of synthesising
such a thing to hardware.

You might care to read my recent response to the thread
"johnson ring counter and how to simulate it" for an
extended rant about synchronous design, and how to
detect transitions synchronously.

In this context, is there any way of telling it that a signal is always
to be considered asynchronous, and that it shouldn't ever be a flipflop?
Click to expand...
Signals get implemented as flipflops in VHDL for one reason only:
because you write to them on the rising edge of a clock, and
at no other time.

To make a signal that will be synthesised to asynchronous
(combinational) logic, you must write to it from a combinational
process - one in which the outputs are calculated as a pure
function of the inputs, and are recalculated whenever any one
(or more) of the inputs changes:

process (list, your, inputs, here)
begin
output1 <= list and inputs;
if here = '1' then
output2 <= your;
else
output2 <= '0';
end if;
end process;

Note the rules for a combinational process:
- every input goes in the sensitivity list
- every output is given a new value whenever the
process executes
- no feedback (don't use outputs as inputs)

Reconsider how you could solve your problem using a single,
fast master clock and fully synchronous logic.
--
Jonathan Bromley, Consultant

DOULOS - Developing Design Know-how
VHDL * Verilog * SystemC * e * Perl * Tcl/Tk * Project Services

Doulos Ltd., 22 Market Place, Ringwood, BH24 1AW, UK
jonathan.bromley@MYCOMPANY.com
http://www.MYCOMPANY.com

The contents of this message may contain personal views which
are not the views of Doulos Ltd., unless specifically stated.
 
On Sep 26, 3:03 am, Paul Burke <p...@scazon.com> wrote:
Andy wrote:

You cannot nest rising_edge() calls in a process. If you are trying to
determine within the state machine if an input has risen (from
previous value), then you need to store previous input value and
compare with current value.

Rising_edge() implies a clock input (i.e. a clocked register).

OK, thanks a lot. Got it. I'm telling it that various flip flops need to
be clocked by two different clocks, and I can't expect the tool to work
out how to combine the clocks when I haven't even thought of how to it
myself. Typical computer programmer's error I suppose.

My own fault for being lazy- the input is only there because I want to
try the thing out with a switch rather than the "real" signal derived
from elsewhere.

In this context, is there any way of telling it that a signal is always
to be considered asynchronous, and that it shouldn't ever be a flipflop?

Paul Burke
Click to expand...
The only thing a synthesis tool can be told about an asynchronous
input is that it is a false path, so that timing constraints are not
enforced on it (it is going to violate setup/hold anyway).

Volumes have been written about proper synchronization techniques, but
in essence, you must register the asynchronous input once, and then
register that again to avoid metastability. This second output is safe
to use. Depending on how it is used, the second flop can sometimes be
eliminated, but it gets tricky. Note that this method only works for
single bit inputs. Synchronizing multiple bit inputs is different.

process (clk)
begin
if rising_edge(clk) then
-- look for change on input
if (inputr1 and not inputr2) = '1' then
do_something;
end if;
inputr2 := inputr1; -- 2nd reg
inputr1 := input; -- 1st reg
end if;
end process;

Depending on your clock speed (almost always faster than a switch
bounce), you also need to think about how to debounce the switch
input, so that your circuit is not responding to every bounce of the
switch after it is thrown only once.

Andy
 
You must log in or register to reply here.

Welcome to EDABoard.com

Sponsor

Back
Top