help with simple code in vhdl

bennzia · Oct 11, 2011

hi, i wrote a code in vhdl that: get 6 bits data the program subtract 1 until "000000"
when not "000000" the output(out1) is= 'Z'.
when "000000" the Ouput(out1) is= '0'.
the code:
library ieee;
use ieee.std_logic_1164.all;
use ieee.std_logic_arith.all;
use ieee.std_logic_unsigned.all;

entity delay_time is

port (t_in1 : in integer range 0 to 255;
u_clk: in std_logic;
reset: in std_logic;
out1: out std_logic
);

end delay_time;

architecture arc_dt of delay_time is

signal temp: integer range 0 to 255;

begin

process (reset,u_clk)

begin

if (reset='0') then

if (u_clk'event and u_clk='1') then
temp <= temp + 1;

end if;
end if;
end process;

process (temp)

begin

if(t_in=temp) then
out1 <= '0';
else
out1<= 'Z';

end if;
end process;

end arc_dt;

here is the simulation:

so according to the program after 7 clk (t_in=000111) the output get '0'.
but you can see that the output is 'Z' all the time, anyone can tell me what the problem? thanks.

barry · Oct 11, 2011

Your problem is that you've got your clock statement inside your reset statement. You should have something like:

if reset='0' then
do something;
elsif (u_clk'event and u_clk='1') then
do something else;
end if;

lucbra · Oct 11, 2011

please use rising_edge(u_clk)

Code:

process (u_clk, rst)
begin
   if rst = '1' then
      temp <= (others => '0')
   elsif rising_edge (u_clk) then
      temp <= temp + 1;
   end if;
end process;

tyy38 · Oct 12, 2011

Usually, don't use more than one process in one entity.

Below is for your reference.

library ieee;
use ieee.std_logic_1164.all;
use ieee.std_logic_arith.all;
use ieee.std_logic_unsigned.all;

entity test is

port (t_in : in integer range 0 to 255;
u_clk: in std_logic;
reset: in std_logic;
out1: out std_logic
);

end test;

architecture arc_dt of test is

signal temp: integer range 0 to 255;

begin

process (reset,u_clk)

begin

if (reset='0') then

if (u_clk'event and u_clk='1') then

if(t_in=temp) then
out1 <= '0';

else
out1<= 'Z';
temp <= temp + 1;

end if;
end if;
end if;
end process;

end arc_dt;

lucbra · Oct 12, 2011

Usually, don't use more than one process in one entity.

Below is for your reference.

That's completely false!!

several processes can be described in one entity. You're making the same error as OP in the process description.

if (reset...) then
if (clk'event ...) then

This means that you're waiting for a clock event while you have a reset condition (all registers are tied to their reset condition).

Just take a good book to learn the basics of VHDL, and you'll understand that what you wrote is not going to work.

tyy38 · Oct 12, 2011

lucbra said:
That's completely false!!

several processes can be described in one entity. You're making the same error as OP in the process description.

if (reset...) then
if (clk'event ...) then

This means that you're waiting for a clock event while you have a reset condition (all registers are tied to their reset condition).

Just take a good book to learn the basics of VHDL, and you'll understand that what you wrote is not going to work.

Can use but not suggest! The entity will work if reset= '0'!!!

My code could get a success simulation as top guy's purpose in Quartus. Output '0' not 'Z' after 7 clk (t_in=000111).

Please synthesize the code in Quartus or other software. You will find your conclusion is "completely false".

stevendong · Oct 12, 2011

I think your code is right. Maybe the problem is your simulation process time. Try to add the time, or change the value of t_in to a smaller number (1 or 3...).
Hope this helps.

Stevendong ^_^

lucbra · Oct 12, 2011

Code:

process (reset,u_clk)

begin
   if (reset='0') then
      if (u_clk'event and u_clk='1') then
         if(t_in=temp) then
            out1 <= '0';
         else
            out1<= 'Z';
            temp <= temp + 1;
         end if;
      end if;
   end if;
end process;

if it's your intention to add a reset condition, then the clk'event runs in the reset condition, otherwise the "if (reset ...)" condition is redundant.

FvM · Oct 12, 2011

It seems like all contributors have been fooled by the original post. It gives the appearance, that the shown code doesn't work because of a hidden problem. But the simulation waveform apparently doesn't belong to this code. As is, the code doesn't compile because t_in is not the port signal t_in1. After correcting this problem, the code compiles and gives the expected output signal. To discuss a possible problem showing in the waveform, we would need to see the real code first.

Nevertheless, the suggestions are reasonable (in part). Referring to a standard async/sync template as suggested by barry is in fact strongly recommended. At second sight, you realize that the present code still confirms to it (omitting a reset action), but it's likely to be changed into something non-synthesizable, if you don't keep the principle.

Although there's a subtile difference between (u_clk'event and u_clk='1') and rising_edge(u_clk), most VHDL users aren't aware of it and consider the latter as convenient shortcut, which at least isn't wrong. But you rarely would need to change it in an existing code.

Apart from using one process, tyy38 also changed the functionality of the original code.

tyy38 · Oct 12, 2011

lucbra: in fact, the "if (reset ...)" condition is redundant. it's only follow the original post. My intention is only to make the entity work as top guy describe.
"so according to the program after 7 clk (t_in=000111) the output get '0'."

FvM · Oct 12, 2011

My intention is only to make the entity work as top guy describe.

But you changed the functionality. In the original code, out1 will be low for one clock cycle.

I just noticed another weirdness of the original post. It assumes a clock of 500 MHz, so the design will possibly fail in a timing simulation depending on the used FPGA family.

---------- Post added at 09:29 ---------- Previous post was at 09:26 ----------

P.S.: Here's the waveform generated by the original design (Quartus timing simulation with Cyclone III):

stevendong · Oct 12, 2011

FvM said:
But you changed the functionality. In the original code, out1 will be low for one clock cycle.

I just noticed another weirdness of the original post. It assumes a clock of 500 MHz, so the design will possibly fail in a timing simulation depending on the used FPGA family.

---------- Post added at 09:29 ---------- Previous post was at 09:26 ----------

P.S.: Here's the waveform generated by the original design (Quartus timing simulation with Cyclone III):

The essence of the problem!

tyy38 · Oct 12, 2011

FvM said:
But you changed the functionality. In the original code, out1 will be low for one clock cycle.

I just noticed another weirdness of the original post. It assumes a clock of 500 MHz, so the design will possibly fail in a timing simulation depending on the used FPGA family.

---------- Post added at 09:29 ---------- Previous post was at 09:26 ----------

P.S.: Here's the waveform generated by the original design (Quartus timing simulation with Cyclone III):

You are careful and maybe right. But top post is a wrong code. No one knows top guy's true intention. Mostly, low for one clock cycle is an incidental.

bennzia · Oct 12, 2011

thanks for help everyone, tyy38 and fvm, can you post me the code that you simulate? i tried servel times to change things without success..
and if simulation process time is wrong, how to fix it? because in my projcet the clock will be 1Hz.

and final question: how i put temp(signal) to simulation screen? thanks again for help!

barry · Oct 12, 2011

I still question the nested if construction. MAYBE it will simulate properly, but it doesn't completely define the output. What happens when reset=1? Is this an unintended latch? Also, there may be simulation problems because the initial state of temp is undefined so temp+1=??

As far as the rising_edge vs. clk'event controversy, the difference is that rising_edge requires the previous value of clk to be '0'; clk'event does not. In other words, if clk where to go from 'z' to '1', rising_edge would not detect a clock edge. In my designs I (hopefully) never have a clock with a value other than '0' or '1'.

FvM · Oct 12, 2011

What happens when reset=1? Is this an unintended latch?

It's the same as an empty reset condition. It synthesizes as a clock enable in this case. But I already explained why I would refer to the standard synchronous design structure.

Also, there may be simulation problems because the initial state of temp is undefined so temp+1=??

In simulation, temp is required by VHDL rules to have an initial value of 0 respectively temp'low. The behaviour of uninitialized signals in a synthesized design depends on synthesis settings. An explicite initial condition achieves correspondense of simulation and synthesis with most tools.

Code:

signal temp: integer range 0 to 255 := 0;

the difference is that rising_edge requires the previous value of clk to be '0'; clk'event does not

Yes, that's the subtile difference. You see, why it's meaningless for synthesized VHDL.

barry · Oct 13, 2011

FvM said:
In simulation, temp is required by VHDL rules to have an initial value of 0

My simulator (ActiveHDL) does not assign any initial values, I don't know about others; I'm not aware of any VHDL 'rule' about this. I usually use a construct like the following which explicitly assigns an initial value for simulation:

signal temp:std_logic:='0';

TrickyDicky · Oct 13, 2011

It does assign initial values, or VHDL is broken. Uninitialised signals default to the left most value.

So:
std_logic will initialise to 'U'
integers will initialise to integer'low
boolean initialises to false
natural initialises to 0
integer range 24 to 444 initialises to 24
(cat, dog, wolf, horse) initialises to cat.

So yes, your similator IS assigning initial values.

---------- Post added at 14:53 ---------- Previous post was at 14:52 ----------

barry said:
My simulator (ActiveHDL) does not assign any initial values, I don't know about others; I'm not aware of any VHDL 'rule' about this. I usually use a construct like the following which explicitly assigns an initial value for simulation:

signal temp:std_logic:='0';

You're probably getting confused. 'U' is a legal std_logic state called "uninitialised". So activeHDL IS initialising the signal.

barry · Oct 13, 2011

TrickyDicky said:
You're probably getting confused. 'U' is a legal std_logic state called "uninitialised". So activeHDL IS initialising the signal.

So uninitialized is initialized?

The point is, if a signal is not explicitly given a '1' or '0' value on initialization, the simulation will look broken, as that signal will have an undefined value for all time. And there is no physical realization of 'uninitialized'; the actual hardware will give that signal a '1' or '0' on power-up, and the point of simulation is to simulate the real world.

TrickyDicky · Oct 13, 2011

barry said:
So uninitialized is initialized?

Yes. Std_logic is define as:

type std_ulogic is ('U', 'X', '0', '1', 'Z'. 'W', 'L', 'H', '-');
type std_logic is resolved std_ulogic;

On your second point, all 'U' shows is a signal that has not been reset or has not been assigned a value. giving ALL signals an initial value is pointless. It does serve a purpose to see 'U' values, as it shows you where you have not assigned a value, which would cause a problem in a real system because something is probably unconnected. It will not stay 'U' forever!

Welcome to EDAboard.com

help with simple code in vhdl

Newbie level 2

Advanced Member level 7

Advanced Member level 2

Newbie level 5

Advanced Member level 2

Newbie level 5

Newbie level 3

Advanced Member level 2

Super Moderator

Newbie level 5

Super Moderator

Newbie level 3

Newbie level 5

Newbie level 2

Advanced Member level 7

Super Moderator

Advanced Member level 7

Advanced Member level 7

Advanced Member level 7

Advanced Member level 7

Similar threads

Part and Inventory Search

Welcome to EDABoard.com

Sponsor