[SOLVED] Signal value goes to 'X' in between,want to drive it to either '0' or '1'

[rahdirs]

Hi,

I have attached a snapshot of my testbench waveform.

In the snapshot,u can see that clr signal is usually 'X' & in between it goes to '1'.

This maybe because i'm driving clr signal at multiple places to different values.But my question is because it remains at only at one state at a time,shouldn't only one source be driving it ?

I've attached code,but it is lengthy

NEXT_STATE_DECODE: process (state,clk,rst,init_calib_complete)
   begin
      --declare default state for next_state to avoid latches
      next_state <= state;  --default is to stay in current state
      --insert statements to decode next_state
      --below is a simple example
      case (state) is
         when st1_reset =>
            if rst = '1' then
                next_state <= st1_reset;
                clr        <= '1';
			else
			    next_state <= st2_idle;
			    clr <= '1';
            end if;
         when st2_idle =>
            if init_calib_complete = '1' then
                next_state <= st3_write;
                clr        <= '1';
			else 
			    next_state <= st2_idle;
            end if;
         when st3_write =>
			if(count = 1500) then
			    next_state <= st4_wait;
				
				clr <= '1';
				ui_c0_app_addr_i <= "111111111111111111111111111";
				ui_c0_app_cmd_i <= "111";
				ui_c0_app_en_i <= '0';
				ui_c0_app_wdf_wren_i <= '0';
				ui_c0_app_wdf_end_i <= '0';
				wr_en_i           <= '0';
				rd_en_i           <= '0';
				
			else
			    next_state <= st3_write;
				
				if(app_rdy_1 = '1') then
                                        clr            <= '0';
                                        ui_c0_app_addr_i <= addr_count;
                                        ui_c0_app_cmd_i <= "000";
                                        ui_c0_app_en_i <= '1';
                                        ui_c0_app_wdf_wren_i <= '1';
                                        ui_c0_app_wdf_end_i <= '1';
                                        wr_en_i           <= '1';
                                         rd_en_i           <= '1';
				else
				    clr            <= '0';
                                    ui_c0_app_addr_i <= addr_count;
                                    ui_c0_app_cmd_i <= "000";
                                    ui_c0_app_en_i <= '1';
                                    ui_c0_app_wdf_wren_i <= '1';
                                    ui_c0_app_wdf_end_i <= '1';
                                    wr_en_i           <= '1';
                                    rd_en_i           <= '0';
				end if;
			end if;
		 when st4_wait =>
			if(count = 157) then
			    next_state <= st5_read;
			    clr        <= '1';
			else
			    clr        <= '0';
				next_state <= st4_wait;
			end if;
		 when st5_read =>
            if(count = 1500) then
				next_state <= st1_reset;
				
				clr        <= '1';
                                ui_c0_app_en_i <= '0';
                                ui_c0_app_cmd_i <= "111";
		    else
				next_state <= st5_read;
				
				if(app_rdy_1 = '1') then
				  clr        <= '0';
				  ui_c0_app_en_i <= '1';
                                  ui_c0_app_cmd_i <= "001";
                                  ui_c0_app_addr_i <= addr_count;
                end if;
			end if;
         when others =>
            next_state <= st1_reset;
      end case;      
   end process;
   end fsm_logic;

 

[std_match]

I don't see an attached waveform.
Your code only has one process, so it is only one driver. Maybe you have connected clr to another driver, outside of the attached code?

You generate clr with combinatorial logic, so the 'X' can also be caused by an input signal that temporarily goes to 'X'.

Your state machine coding style may be OK for a test bench, but it would not be good for FPGA synthesis,
Your code will have the logic after the registers (for the output signals), which is not good for current FPGA's. The logic should be before the registers. If you do the state machine in one clocked process, you will automatically get that right.

 

[rahdirs]

I don't see an attached waveform.

Your state machine coding style may be OK for a test bench, but it would not be good for FPGA synthesis,

Sorry,forgot to attach waveform.
I noticed that as well,that i was driving clr signal in 2 processes.Now that i have rectified it,the clr signal is only either '0' or '1'.

But what do you mean by "If you do the state machine in one clocked process, you will automatically get that right"?
Are you referring to my outputs which get updated one clock cycle later ?

 

[ads-ee]

Your state machine coding style may be OK for a test bench, but it would not be good for FPGA synthesis,
Your code will have the logic after the registers (for the output signals), which is not good for current FPGA's.

But what do you mean by "If you do the state machine in one clocked process, you will automatically get that right"?
Are you referring to my outputs which get updated one clock cycle later ?

He's referring to the combonatorial logic that is after the state registers, which is inferred by coding the outputs in a combinational process.

One issue I see with your process is the inclusion of signals in the sensitivity list that are not read in the process and missing ones which are read within the process.

 

[K-J]

This maybe because i'm driving clr signal at multiple places to different values.But my question is because it remains at only at one state at a time,shouldn't only one source be driving it ?

Although you've solved your particular problem here, here are a couple of suggestions to help you avoid having to debug this same problem again in the future:

- Use std_ulogic rather than std_logic whenever possible. That way, either when you compile or at worst when you go to start the simulation, you'll immediately receive an error message saying that there are multiple drivers for an unresolved signal. The tool will also tell you exactly where the multiple drivers are in your source code, no debug on your part will be necessary.
- If you do use std_logic signals, your simulator may have a way to query who is driving it. In Modelsim, you would say "drivers xyz" and it will spit out a response indicating where there are drivers on a signal and what they are each trying to drive the signal to.

Kevin Jennings

 

[rahdirs]

One issue I see with your process is the inclusion of signals in the sensitivity list that are not read in the process and missing ones which are read within the process.

NEXT_STATE_DECODE: process (state,clk,rst,init_calib_complete)

You are saying that i should remove clk from my sensitivity list as it is not being read & add count,app_rdy_1,addr_count into my sensitivity list ?

Yes,i should add app_rdy_1 into my sensitivity list but count & addr_count are signals which change at every rising edge of clk,so instead of adding them both into the list,i simply added clk.It is just that it is not a good practice or i'm making a mistake over here ?

Your code will have the logic after the registers (for the output signals), which is not good for current FPGA's. The logic should be before the registers. If you do the state machine in one clocked process, you will automatically get that right.

So you're saying,i should be doing this ? Only a snippet of above posted code is posted below

case (state) is
         when st3_write =>
			if(count = 1500) then
				clr <= '1';
				ui_c0_app_addr_i <= (OTHERS => '1');
				ui_c0_app_cmd_i <= "111";
				ui_c0_app_en_i <= '0';
				ui_c0_app_wdf_wren_i <= '0';
				ui_c0_app_wdf_end_i <= '0';
				wr_en_i           <= '0';
				rd_en_i           <= '0';

			        next_state <= st4_wait;

What major difference would make except for the fraction of time that my outputs are assigned/changed first before the state type changes ?

 

[ads-ee]

You must not understand the difference between combination logic and sequential logic. What you have written is combinatorial logic (i.e. a bunch of logic gates). When signals in comb logic switch the entire process needs to be execute to determine if the input signal has changed any of the outputs. Therefore adding clk in lieu of the signals count and addr_count is just plain wrong.

You also don't seem to understand that if you have a registered signal (i.e. state) then using that signal in a combinatorial process will infer gates after the registers, so your outputs are all from combinational logic cones, not from the output of registers. This will mean that you've just reduce the amount of available clock period to the next place those outputs are used.

You should start running synthesis on code snippets to get an understanding how your coding is being implemented in hardware. Your logic produce the following kind of circuit:

 

[rahdirs]

When signals in comb logic switch the entire process needs to be execute to determine if the input signal has changed any of the outputs. Therefore adding clk in lieu of the signals count and addr_count is just plain wrong.

That is what i was saying.When count & addr_count signals change,the entire process needs to execute again,but when do they change ? At every rising edge of clock.

You also don't seem to understand that if you have a registered signal (i.e. state) then using that signal in a combinatorial process will infer gates after the registers, so your outputs are all from combinational logic cones, not from the output of registers

I wasn't saying that if i should follow what @std_match said.I was asking whether the last code snippet that i posted in #6 is the way he was expecting the code to be written.Or to write two different processes,one in which next_state is decided & other in which outputs are assigned based on state & input.

 

[ads-ee]

Then you need to read the LRM as you don't understand the scheduling mechanism in VHDL. There are delta time differences between looking at the edges (both edges) of the clock and looking at the signals count and addr_count that are generated by the clock signal. Believe me or don't believe me (and have problems down the road on projects due to your misunderstanding of scheduling)

No std_match was saying you should always use registers to generate outputs from a FSM, (ads-ee, unless you can justify to a group of peers that you absolutely have to generate an output from combinational logic). Your new code still has logic after the registers that generate your outputs, which isn't a good synchronous design practice.

It's always a good practice to have all the output from a FSM registered, by writing your FSM in a single process style instead of the two process style you will automatically have everything registered.

 

[rahdirs]

Hi,

@ads-ee:

So you are saying the delta time difference between count & addr_count changing with respect to clock & eventually the process getting rerun delta time earlier than supposed to may lead to errors ?

Oh so you were just saying about writing my fsm in single process & ditch my present two process fsm,on which there have been many threads as well on Xilinx forum,the advantages of one-process over two process like latches etc...

 

[ads-ee]

So you are saying the delta time difference between count & addr_count changing with respect to clock & eventually the process getting rerun delta time earlier than supposed to may lead to errors ?

exactly that, and those types of errors can be really difficult to find and usually end up as simulation synthesis mismatches. Not a good thing.

Oh so you were just saying about writing my fsm in single process & ditch my present two process fsm,on which there have been many threads as well on Xilinx forum,the advantages of one-process over two process like latches etc...

Yeah ditch the two process, use one process. You'll get a lot of agreement from a lot of the advanced forum members with the exception of a few individuals, who prefer the two process style.

 

[std_match]

In addition to being a good design practice in general, avoiding combinatorial logic after registers is also motivated by the modern FPGA architecture. A logic block consists of combinatorial logic followed by a register. The code in the first post will synthesize to logic as in the figure by ads-ee in post #7. In that case the register and the logic can't be placed in the same block. That design style will probably consume more hardware resources.

Another important problem with the two-process state machines isn't shown by the figure in post #7. It is very easy to create asynchronous paths paths from inputs to outputs, and that can lead to timing problems when you build a larger system.

By writing state machines in a single clocked process you will automatically get a design that will map nicely to the FPGA architecture. All outputs will come directly from a register, so timing problems will be minimized. There will always be a one clock cycle delay from input to output, but that is normally not a problem.

 

[vGoodtimes]

It really is a pre-requisite that you be able to convert between 1 and 2 process designs conceptually. You should be able to look at a single process design and be able to determine the complexity and latency of the logic. This is independent of any coding style choices.

You should try to have registered outputs as much as possible. While some modules allow/require combinatorial outputs, or even a combinatorial in-out path, they are rare.

In any case, you should only drive bits of a signals from one (and the same*) process. For sim, the resolution function will result in '0' or '1' if all drivers agree, so the signal will often toggle to 'X'.

* driving different bits of a signal from different processes can work on some tools, and can fail on others.

 

[TrickyDicky]

* driving different bits of a signal from different processes can work on some tools, and can fail on others.

If this is the case, you need to complain to the tool vendor, as its braking the lrm. Arrays and records are all counted as separate objects and should be drivable from separate processes.

 

[K-J]

In any case, you should only drive bits of a signals from one (and the same*) process. For sim, the resolution function will result in '0' or '1' if all drivers agree, so the signal will often toggle to 'X'.

As I mentioned earlier in the thread in #5, use std_ulogic rather than std_logic whenever possible and you can avoid ever having to debug this problem, you either will fail on the compile or at the start of the simulation and won't be able to run.

Kevin Jennings

- - - Updated - - -

If this is the case, you need to complain to the tool vendor, as its braking the lrm.

Having a design error in your code does not mean the tool is not compliant to the LRM. You still should complain to the vendor about the behavior, but not because it does not conform to the LRM.

Kevin Jennings