Is there anything fundamentally wrong with this code?

entity my_entity is
port (
clock : in std_logic;
reset : in std_logic;
input_signal : in std_logic;
output_signal : out std_logic;
output_complete_signal : in std_logic
);
end my_entity;

architecture RTL of my_entity is


signal local_signal : std_logic;
signal local_flag : std_logic;


begin


process ( clock, reset )
begin
if ( reset = '0') then
output_signal <= '0';
local_signal <= '0';
local_flag <= '0';
elsif ( clock'event and clock = '1') then


if ( input_signal = '1' ) then
if ( local_signal = '0' ) then
local_signal <= '1';
end if;
end if;


if ( local_signal = '1' ) then
if ( local_flag = '0' ) then
local_flag <= '1';
output_signal <= '1';
else
if ( output_complete_signal = '1' ) then
output_signal <= '0';
local_flag <= '0';
local_signal <= '0';
end if;
end if;
end if;


end if;
end process;

 

[James Unterburger]

1) No LIBRARY and USE clauses to make "std_logic" and the
operators and enumeration literals of its base type visible.
2) No "end;" for the architecture body.
3) No comments.

Items 1 and 2 are necessary to have it compile.
Item 3 would give the (human) reader some idea of
what the code is for.

simon.stockton@baesystems.com wrote:

entity my_entity is
port (
clock : in std_logic;
reset : in std_logic;
input_signal : in std_logic;
output_signal : out std_logic;
output_complete_signal : in std_logic
);
end my_entity;

architecture RTL of my_entity is


signal local_signal : std_logic;
signal local_flag : std_logic;


begin


process ( clock, reset )
begin
if ( reset = '0') then
output_signal <= '0';
local_signal <= '0';
local_flag <= '0';
elsif ( clock'event and clock = '1') then


if ( input_signal = '1' ) then
if ( local_signal = '0' ) then
local_signal <= '1';
end if;
end if;


if ( local_signal = '1' ) then
if ( local_flag = '0' ) then
local_flag <= '1';
output_signal <= '1';
else
if ( output_complete_signal = '1' ) then
output_signal <= '0';
local_flag <= '0';
local_signal <= '0';
end if;
end if;
end if;


end if;
end process;

 

Thanks for your response,

1) This was lost in transcribing the code from its source to the
internet.
2) See 1)
3) Understood - but the codes function outside of this entity (i.e. in
the grand scheme of things) is of little importance.

This code snippet is actually a structural representation of my actual
code (which is much more complex). I am experiencing some strange
behaviour where although 'input_signal' is only held high for one clock
cycle (by another process, but using the same clock) the
'output_signal' is asserted twice, or so it appears. I was wondering if
the structure of my code (and my coding style) would cause this
situation to occur. It only appears to present itself on
'real-hardware' in the 'simulator' I cannot repeat the strange
behaviour.

Regards,

Simon

 

[JustDave]

The simulator & synthesis tools must have interpreted the code
differently. The more I look at your snippet, the less I like the fact
that you have two separate 'if' statements each performing some control
on "local_signal". I would have made sure all control of each variable
is completely contained in its own if/else, like this:

if ( input_signal = '1' and output_complete_signal = '1' )
then
local_signal <= '1';
elsif ( output_complete_signal = '1' ) then
local_signal <= '0';
end if;

if ( local_signal = '1' and output_complete_signal = '0' )
then
local_flag <= '1';
output_signal <= '1';
elsif ( output_complete_signal = '1' ) then
output_signal <= '0';
local_flag <= '0';
end if;

I've had similar problems in the past and have learned to do this as a
matter of habit.

Dave

 

[Mike Treseler]

simon.stockton@baesystems.com wrote:

It only appears to present itself on
'real-hardware' in the 'simulator' I cannot repeat the strange
behaviour.

Ok, lets put it on the Quartus RTL viewer:
http://home.comcast.net/~mike_treseler/my_entity.pdf
Hmmm.
Maybe the output_complete input is not sychronized.

-- Mike Treseler

 

[KJ]

When reality and simulation differ in the manner that you describe
usually the root cause is a timing problem.

Double check with the output of your fitter that it agrees with you
that all of the inputs (reset, input_signal and output_complete_signal)
really are synchronous to clock.

Also, even you are using reset as an asynchronous reset of the outputs,
reset still needs to be synchronized with clock. When reset switches
from '1' to '0' and violates the setup time relative to the clock then
all bets are off on what state the outputs will be in now. Since what
you've coded is basically a form of state machine then you'll be hosed.

KJ