Unable to understand a LCD module controller code :(

[vGoodtimes]

3. In the 'send' state of the, why has been busy assigned '1' before the first if statement. Since the controller has to wait for atleast in the send state for 50 usec before going back to ready state, in the first if statement, the busy port has been already assigned logic 1.

The code has many extra assignments that are not needed. The important assignments to "busy" are the ones that occur when state is assigned to a different value. These are the transitions of the state machines.

My guess is that the author wanted to have the simplicity of placing the assignments in state cases, but then realized that he needed them on the transitions between states. This wasn't done in all places -- for example busy will be '1' on the first cycle of ready. There are a few other small issues like that. For example clk_count doesn't reset with reset_n, so reset_n can only be a power-on-reset.

 

[hobbyiclearner]

First of all, thanks for taking time out to answer the queries. Pls. note the following:

The code has many extra assignments that are not needed. The important assignments to "busy" are the ones that occur when state is assigned to a different value. These are the transitions of the state machines.

This may not be entirely true. For eg in the power_up state needs to be executed in busy=1 condition so that interfacing circuit does not send any signal to the lcd_controller (which is stabilizing its voltage level to 4.5-5v). Similarly, the entire initialize state has to be executed under busy=1 state as the lcd_controller is preparing the lcd display regarding the no. of lines, dots per cell etc. Basically, the lcd display is not ready to receive any data/ external display related command signals up to this point and thereby the lcd_controller is not ready to receive any data/ command signal up to this point. One exception is in the else condition of initialize state where busy is de asserted. This may/ may not be required (something on which I am seeking second opinion) as busy needs to remain asserted so as to inform the interfacing circuit that the lcd display is still not ready.

            e <= '0';
            state <= initialize;
          ELSE                                   --initialization complete
            clk_count := 0;
            [B]busy <= '0';[/B]
            state <= ready;
          END IF;

In the ready state, when the lcd_enable signal is high, the data bus and instruction signals (rs and rw) are read and busy is again asserted. Hence should busy not be asserted after these signals are read (because as I know the instruction inside an if statement are executed in the order they are mentioned). I am again seeking second opinion on this.

WHEN ready =>
          IF(lcd_enable = '1') THEN
            [B]busy <= '1';[/B] -- should this not come after the 3rd line form this line
            rs <= lcd_bus(9);
            rw <= lcd_bus(8);
            lcd_data <= lcd_bus(7 DOWNTO 0);
            clk_count := 0;            
            state <= send;

Next, coming to my original question, is busy assertion required separately before the if clause of the send state. I am saying so as the lcd _controller needs to be busy for 50 usec only during which it will strobe the enable of lcd display and not accept any more input signal and after the 50 usec, it will go back to ready state for fresh data/ instruction. Again I was/still am seeking second opinion here.

------------------

Next, I do not understand what was meant when you mention that busy will be 1 on first cycle of state ready.

for example busy will be '1' on the first cycle of ready.

-------------------

Yes, I also understand that when reset_n is active, the clk_count does not reset to 0, which may be required as the control switches to power_up state and all its conditions have to be re satisfied. So do you suppose that adding the following line will work:

      IF(reset_n = '0') THEN
          state <= power_up;
          [B]clk_count:=0;[/B]
      END IF;

Thanks,
Hobbyiclearner.

- - - Updated - - -

Forum members are not free consultants, we are all volunteers, as such having to work on some random code (perhaps rubbish code) and modify it to do something that we've suggested (that you should implement) is not something you should expect from another forum member. Occasionally you'll find someone with too much time on their hands that might do the work for you, but don't count on it!

I understand your point and respect it as well. But unfortunately, I do not know anyone nearby to help me in this matter. Hence I am forced to request at the forum for support. Since the posts remain on the forum, you may see it as an explanation to the other people who may come up with lcd interfacing issues later on .

Regards,
Hobbyiclearner.

 

[andre_luis]

I do not know anyone nearby to help me in this matter. Hence I am forced to request at the forum for support.

You asked the reason for someone have no motivation to fix your code.
It was expected that you have made changes according to given recommendations.

 

[FvM]

You have been asking for the purpose of a specific busy <= '1' assignment and the answer is that it's redundant, or more generally that the code isn't perfect.

As you mentioned, asserting busy during power-up is required. There's surely more than one way to implement the function in HDL, thus it's of limited worth to discuss the pro and con of each single line of code. It's much more important that you understand the required functionality and are able to verify the correct operation in a simulation.

It's generally preferred to implement an asynchronous reset and reset/preset all signals. In the original code, clk_count has an initial value which is implemented as power-up initialization by most FPGA compilers. But it's not as reliable as an explicit reset, it can e.g. fail with a dangling clock during power-up.

 

[hobbyiclearner]

Thanks for the feedback. As per it, there were 2 problems in the code: first the reset was not async. and clk_count was not resetting with it. Secondly busy <='1' was redundant and it needs to be asserted till the lcd display is ready to take instructions/ data from the user. I have modified the code as below:

LIBRARY ieee;
USE ieee.std_logic_1164.ALL;

ENTITY lcd_controller_modified IS
  PORT(
    clk        : IN    STD_LOGIC;  --system clock
    reset_n    : IN    STD_LOGIC;  --active low reinitializes lcd
    lcd_enable : IN    STD_LOGIC;  --latches data into lcd controller
    lcd_bus    : IN    STD_LOGIC_VECTOR(9 DOWNTO 0);  --data and control signals
    busy       : OUT   STD_LOGIC := '1';  --lcd controller busy/idle feedback
    rw, rs, e  : OUT   STD_LOGIC;  --read/write, setup/data, and enable for lcd
    lcd_data   : OUT   STD_LOGIC_VECTOR(7 DOWNTO 0)); --data signals for lcd
END lcd_controller_modified;

ARCHITECTURE controller OF lcd_controller_modified IS
  TYPE CONTROL IS(power_up, initialize, ready, send);
  SIGNAL    state      : CONTROL;
  CONSTANT  freq       : INTEGER := 25; --system clock frequency in MHz
BEGIN
  PROCESS(clk, reset_n)
    VARIABLE clk_count : INTEGER := 0; --event counter for timing
  BEGIN
  IF(clk'EVENT and clk = '1') THEN
    
      CASE state IS        
--wait 50 ms to ensure Vdd has risen and required LCD wait is met
        WHEN power_up =>
           busy <= '1';
	     IF (reset_n = '0') then
           state <= power_up;
           clk_count := 0;		
          ELSIF(clk_count < (50000 * freq)) THEN    --wait 50 ms
            clk_count := clk_count + 1;
            state <= power_up;
          ELSE                          --power-up complete
            clk_count := 0;
            rs <= '0';
            rw <= '0';
            lcd_data <= "00110000";
            state <= initialize;
          END IF;
          
        --cycle through initialization sequence  
        WHEN initialize =>
          busy <= '1';  
          clk_count := clk_count + 1;			 
          IF (reset_n = '0') then
           state <= power_up;
           clk_count := 0;
          ELSIF(clk_count < (10 * freq)) THEN   --function set
            lcd_data <= "00111100";
            e <= '1';
            state <= initialize;
          ELSIF(clk_count < (60 * freq)) THEN    --wait 50 us
            lcd_data <= "00000000";
            e <= '0';
            state <= initialize;
          ELSIF(clk_count < (70 * freq)) THEN --display on/off 										--control
            lcd_data <= "00001100";
            e <= '1';
            state <= initialize;
          ELSIF(clk_count < (120 * freq)) THEN   --wait 50 us
            lcd_data <= "00000000";
            e <= '0';
            state <= initialize;
          ELSIF(clk_count < (130 * freq)) THEN   --display clear
            lcd_data <= "00000001";
            e <= '1';
            state <= initialize;
          ELSIF(clk_count < (2130 * freq)) THEN  --wait 2 ms
            lcd_data <= "00000000";
            e <= '0';
            state <= initialize;
          ELSIF(clk_count < (2140 * freq)) THEN  --entry mode set
            lcd_data <= "00000110";      
            e <= '1';
            state <= initialize;
          ELSIF(clk_count < (2200 * freq)) THEN  --wait 60 us
            lcd_data <= "00000000";
            e <= '0';
            state <= initialize;
          ELSE                    --initialization complete
            clk_count := 0;
            busy <= '0';
            state <= ready;
          END IF;    
       
        --wait for the enable signal and then latch in the instruction
        WHEN ready =>
          IF (reset_n = '0') then
           state <= power_up;
           clk_count := 0;
          ELSIF(lcd_enable = '1') THEN
            busy <= '1';
            rs <= lcd_bus(9);
            rw <= lcd_bus(8);
            lcd_data <= lcd_bus(7 DOWNTO 0);
            clk_count := 0;            
            state <= send;
          ELSE
            busy <= '0';
            rs <= '0';
            rw <= '0';
            lcd_data <= "00000000";
            clk_count := 0;
            state <= ready;
          END IF;
        
        --send instruction to lcd        
        WHEN send =>
        IF (reset_n = '0') then
           state <= power_up;
           clk_count := 0;
        ELSIF(clk_count < (50 * freq)) THEN --doNt exit for 50us
           busy <= '1';
           IF(clk_count < freq) THEN      --negative enable
            e <= '0';
           ELSIF(clk_count < (14 * freq)) THEN --+IVE enable 1/2 cycle
            e <= '1';
           ELSIF(clk_count < (27 * freq)) THEN  ---IVE enable half-cycle
            e <= '0';
           END IF;
           clk_count := clk_count + 1;
           state <= send;
        ELSE
          clk_count := 0;
          state <= ready;
        END IF;

      END CASE;    
    
   END IF;
  END PROCESS;
END controller;

Though the code is working on the kit, pls. let me know if the problems as above are rectified and if there is any other short coming. Also, since I am from a non english speaking place, it would be nice of the contributors o state the problem in crisp clear words viz

busy <= '1' assignment and the answer is that it's redundant,

:grin:

Hobbyiclearner

 

[vGoodtimes]

This is one of those designs where things happen slowly and requirements are very loose. Success can be an accident.

The original design placed reset at the end of the process. This is my preferred location, as it make it clear which signals need to have resets. For ASIC designs, this tends to be "everything", for FPGA designs, this tends to be "only control/state".

It isn't clear if you understood my comments about the mixed styles, or if you've gotten an easy design to work and just want to feel good about it. The latter is actually fine -- these languages take a lot to master.

Specifically, you should be aware of signals that are updated on state transitions, and ones updated within a state. This is something that this design probably doesn't hit, but is a key realization. Nearly as important as cycle vs sample delays. This is not just because it is weird to mix styles, but because developers who mix styles also create misleading signal names.

For example, if the state machine is in the "ready" state with "busy = '1'", that would seem a bit odd. But that does happen. When in the send state, if the else statement is reached state will change to ready. busy is not changed. On the next cycle, the state will be ready and busy will remain '1'. There is an implicit assumption that lcd_enable will be '0'. "i'm ready but also busy" seems to mean the signals were not named (or used) correctly.

 

[hobbyiclearner]

Thanks for your input.

For example, if the state machine is in the "ready" state with "busy = '1'", that would seem a bit odd. But that does happen. When in the send state, if the else statement is reached state will change to ready. busy is not changed. On the next cycle, the state will be ready and busy will remain '1'. There is an implicit assumption that lcd_enable will be '0'. "i'm ready but also busy" seems to mean the signals were not named (or used) correctly.

I am not quite able to understand your point here. IN the else part of send state, weather or not busy is changed, after state change to ready state, it is assigned an appropriate value depending on the condition of lcd_enable signal and reset_n (if reset_n is low, the state changes to power_up where the busy is assigned '1' and if lcd_enable is high busy is '1' else '0'). Does it make any difference if I explicitly dont change the value of busy signal before the state changes from send state to ready state? Yes there is an implicit assumption that lcd_enable can be '0' (in the ready state). Should I explicitly mention this possible input to avoid other possible input conditions viz. 'Z'. Is that a common design practice? Also where did you find

"i'm ready but also busy" seems to mean the signals were not named (or used) correctly.

Thanks again,
Hobbyiclearner.

 

[vGoodtimes]

--+-----+-------+------+--------------------------+
  --| cnt | state | busy |                          |
  --+-----+-------+------+--------------------------+
  --| N-2 | send  | '1'  | <- elsif branch reached  |
  --| N-1 | send  | '1'  | <- elsif branch reached  |
  --| N   | send  | '1'  | <- else branch reached   |
  --| 0   | ready | '1'  | <- busy + ready          |
  --| 0   | ready | '0'  | <- changes 1 cycle later |
  --+-----+-------+------+--------------------------+

  PROCESS(clk)
    VARIABLE clk_count : INTEGER := 0; --event counter for timing
  BEGIN
  IF(clk'EVENT and clk = '1') THEN
    
      CASE state IS
        
        WHEN power_up =>
          busy <= '1';                        -- (1) should not be needed, but is needed
          IF(clk_count < (50000 * freq)) THEN    
            clk_count := clk_count + 1;       -- post-increment.
            state <= power_up;                -- (2) not needed.
          ELSE                                   
            clk_count := 0;
            rs <= '0';
            rw <= '0';
            lcd_data <= "00110000";           -- (3) this should be left out, or should be the same value as line (5)
            state <= initialize;
          END IF;
          
        WHEN initialize =>
          busy <= '1';                        -- (4) not needed
          clk_count := clk_count + 1;         -- pre-increment this time
          IF(clk_count < (10 * freq)) THEN       
            lcd_data <= "00111100";           -- (5) one cycle after setting lcd_data 00110000, it changes to 00111100.
            e <= '1';
            state <= initialize;              -- (6) not needed
          ELSIF(clk_count < (60 * freq)) THEN    
            lcd_data <= "00000000";
            e <= '0';
            state <= initialize;              -- (7) not needed
          ELSIF(clk_count < (70 * freq)) THEN    
            lcd_data <= "00001100";      
            e <= '1';
            state <= initialize;              -- (8) not needed
          ELSIF(clk_count < (120 * freq)) THEN   
            lcd_data <= "00000000";
            e <= '0';
            state <= initialize;              -- (9) not needed
          ELSIF(clk_count < (130 * freq)) THEN   
            lcd_data <= "00000001";
            e <= '1';
            state <= initialize;              -- (10) not needed
          ELSIF(clk_count < (2130 * freq)) THEN  
            lcd_data <= "00000000";
            e <= '0';
            state <= initialize;              -- (11) not needed
          ELSIF(clk_count < (2140 * freq)) THEN  
            lcd_data <= "00000110";      
            e <= '1';
            state <= initialize;              -- (12) not needed
          ELSIF(clk_count < (2200 * freq)) THEN  
            lcd_data <= "00000000";
            e <= '0';
            state <= initialize;              -- (13) not needed
          ELSE                                   
            clk_count := 0;
            busy <= '0';
            state <= ready;
          END IF;    
        
        WHEN ready =>
          IF(lcd_enable = '1') THEN
            busy <= '1';                      -- (14) busy will be set 1 cycle after entering "ready"
            rs <= lcd_bus(9);
            rw <= lcd_bus(8);
            lcd_data <= lcd_bus(7 DOWNTO 0);
            clk_count := 0;            
            state <= send;
          ELSE                                -- (15) should not be needed.
            busy <= '0';
            rs <= '0';
            rw <= '0';
            lcd_data <= "00000000";
            clk_count := 0;
            state <= ready;
          END IF;
        
        WHEN send =>
        busy <= '1';                          -- (16) not needed
        IF(clk_count < (50 * freq)) THEN  
          busy <= '1';                        -- (17) not needed
          IF(clk_count < freq) THEN        
            e <= '0';
          ELSIF(clk_count < (14 * freq)) THEN  
            e <= '1';
          ELSIF(clk_count < (27 * freq)) THEN  
            e <= '0';
          END IF;
          clk_count := clk_count + 1;         -- post increment.
          state <= send;                      -- (18) not needed
        ELSE                                  -- (19) busy <= '0' needed.  
                                              -- busy will change at the same edge as state, 
                                              -- placeing busy = '0' and state = ready
          clk_count := 0;
          state <= ready;
        END IF;

      END CASE;    
    
      IF(reset_n = '0') THEN
          state <= power_up;                   -- (20) count should be reset, busy should be set. other io should be set.
      END IF;
    
    END IF;
  END PROCESS;

I don't think the original author, or you, fully understand how "<=" works, nor how it differs from ":=". Even if the technical differences were known, it looks like the code was written without considering how "<=" worked.

 

[hobbyiclearner]

Thanks for the comments. I am going ahead and referring to each comment and indicating the changes made in the code :

1.Comment 1: busy =1 may be needed during power up as this is to inform the module driving the lcd controller that any data transmitted during this duration will not be processed. Hence I understand it to be required. Not making any changes in it as of now.

2.Comments 2, 6, 7, 8, 9, 10, 11, 12, 13, 18. These code lines explicitly mention that no state change is required when the if/ elsif/ else branch instruction are carried out. Other than that, do these lines cause any error / increase in hardware during synthesis. Presently not removing these lines.


3.Comment 3: Yes, this line seemed to have a problem and I had enquired previously as well (post 15). FvM then informed me to refer to the HD 44780 instruction data set. I could not find anything relevant which should be done during power on besides wait for 50 msecs. Hence deleting the line.

4.Comment 4: Busy =1 may be required for the entire initialize process. Hence keeping it for now.


5.Comment 5: May be resolved after action on comment 3

6.Comment 14: Yes, since signal assignment is not instantaneous, I have modified the else branch of initialize state with busy =1. When the state changes to ready state, during the same time the busy will change to 1 and not in the next clock cycle (if that is what you meant by ‘1 cycle after entering ready’).


7.Comment 15: I am not able to understand why the else portion should not be required. If lcd_enable =0, the busy =0 have to be asserted to inform the module driving lcd controller that it is available for data/ instruction. Also lcd_data bus can be given all zeros to (maybe) avoid garbage value and control signals rs and rw to lcd display unit given 1 and 0 respectively to inform the display that it is to be prepared for write operation. Made the changes to rs and rw in the code accordingly.

8.Comment 16: Yes, the line does not seem to be needed. Hence deleted.


9.Comment 17: I don’t know why this line is not required. As per the FSM of this code here once the send cycle starts, the FSM should not change state for 50 usec and thereby any other data/ instruction during this period sent from the module driving the lcd controller is to be disregarded. So busy =1 will inform the lcd controller driving module the same. Presently not deleting this line.

10.Comment 19: Yes, busy =0 is required here. Change is done.


11.Comment 20: As per the FSM, if reset is asserted during any state, the control is to go back to power up state. Also, as per FvM (post 24) reset is better async. Hence, I am checking reset condition at the beginning of each state and have added it to process sensitivity list. If reset is asserted during any state, clk_count gets reset and state changes to power_up where busy also gets asserted. and rs and rw are given zero.

Also all increments have been changed to pre increment mode. Do note, I have not had a chance to run this code. Will be able to do that on the other side of the weekend.

LIBRARY ieee;
USE ieee.std_logic_1164.ALL;

ENTITY lcd_controller_modified_1 IS
  PORT(
    clk        : IN    STD_LOGIC;  --system clock
    reset_n    : IN    STD_LOGIC;  --active low reinitializes lcd
    lcd_enable : IN    STD_LOGIC;  --latches data into lcd controller
    lcd_bus    : IN    STD_LOGIC_VECTOR(9 DOWNTO 0);  --data and control signals
    busy       : OUT   STD_LOGIC := '1';  --lcd controller busy/idle feedback
    rw, rs, e  : OUT   STD_LOGIC;  --read/write, setup/data, and enable for lcd
    lcd_data   : OUT   STD_LOGIC_VECTOR(7 DOWNTO 0)); --data signals for lcd
END lcd_controller_modified_1;

ARCHITECTURE controller OF lcd_controller_modified_1 IS
  TYPE CONTROL IS(power_up, initialize, ready, send);
  SIGNAL    state      : CONTROL;
  CONSTANT  freq       : INTEGER := 25; --system clock frequency in MHz
BEGIN
PROCESS(clk, reset_n)
    VARIABLE clk_count : INTEGER := 0; --event counter for timing
  BEGIN
  IF(clk'EVENT and clk = '1') THEN
    
      CASE state IS        
--wait 50 ms to ensure Vdd has risen and required LCD wait is met
        WHEN power_up =>
           clk_count := clk_count + 1;
           busy <= '1';
	     IF (reset_n = '0') then
           state <= power_up;
           clk_count := 0;
           rs <= '0';
            rw <= '0';
         ELSIF(clk_count < (50000 * freq)) THEN    --wait 50 ms
            state <= power_up;
          ELSE                          --power-up complete
            clk_count := 0;
            rs <= '0';
            rw <= '0';
           state <= initialize;
          END IF;
          
        --cycle through initialization sequence  
        WHEN initialize =>
          busy <= '1';  
          clk_count := clk_count + 1;			 
          IF (reset_n = '0') then
           state <= power_up;
           clk_count := 0;
          ELSIF(clk_count < (10 * freq)) THEN   --function set
            lcd_data <= "00111100";
            e <= '1';
            state <= initialize;
          ELSIF(clk_count < (60 * freq)) THEN    --wait 50 us
            lcd_data <= "00000000";
            e <= '0';
            state <= initialize;
          ELSIF(clk_count < (70 * freq)) THEN --display on/off control
            lcd_data <= "00001100";
            e <= '1';
            state <= initialize;
          ELSIF(clk_count < (120 * freq)) THEN   --wait 50 us
            lcd_data <= "00000000";
            e <= '0';
            state <= initialize;
          ELSIF(clk_count < (130 * freq)) THEN   --display clear
            lcd_data <= "00000001";
            e <= '1';
            state <= initialize;
          ELSIF(clk_count < (2130 * freq)) THEN  --wait 2 ms
            lcd_data <= "00000000";
            e <= '0';
            state <= initialize;
          ELSIF(clk_count < (2140 * freq)) THEN  --entry mode set
            lcd_data <= "00000110";      
            e <= '1';
            state <= initialize;
          ELSIF(clk_count < (2200 * freq)) THEN  --wait 60 us
            lcd_data <= "00000000";
            e <= '0';
            state <= initialize;
          ELSE                    --initialization complete
            clk_count := 0;
            busy <= '1';
            state <= ready;
          END IF;    
       
        --wait for the enable signal and then latch in the instruction
        WHEN ready =>
          IF (reset_n = '0') then
           state <= power_up;
           clk_count := 0;
          ELSIF(lcd_enable = '1') THEN
            rs <= lcd_bus(9);
            rw <= lcd_bus(8);
            lcd_data <= lcd_bus(7 DOWNTO 0);
            clk_count := 0;            
            state <= send;
          ELSE
            busy <= '0';
            rs <= '1';
            rw <= '0';
            lcd_data <= "00000000";
            clk_count := 0;
            state <= ready;
          END IF;
        
        --send instruction to lcd        
        WHEN send =>
       clk_count := clk_count + 1;
        IF (reset_n = '0') then
           state <= power_up;
           clk_count := 0;
        ELSIF(clk_count < (50 * freq)) THEN --dont exit for 50us
           busy <= '1';
           IF(clk_count < freq) THEN      --negative enable
            e <= '0';
           ELSIF(clk_count < (14 * freq)) THEN --+IVE enable 1/2 cycle
            e <= '1';
           ELSIF(clk_count < (27 * freq)) THEN  ---IVE enable half-cycle
            e <= '0';
           END IF;
           state <= send;
        ELSE
          clk_count := 0;
          busy <= '0';
          state <= ready;
        END IF;

      END CASE;    
    
   END IF;
  END PROCESS;
END controller;

Pls. refer to the points above and do let me know if my understanding is correct. Also, going ahead, if you have any suggestions, kindly refer to the modified code as above. It has all the modifications which I could understand to be correct.

Hobbyiclearner

- - - Updated - - -

+++UPDATE+++

Pls. ignore point 7 / reply to comment 15 for now.

Thanks,
Hobbyiclearner.

 

[vGoodtimes]

My commentary was more for an expected design without any odd behavior. This means fully functional resets, no glitches, and no misleading signals. A design can work if it can tolerate these, and may even be a tiny amount smaller. This difference is ok if there is documentation that you know about it.

There are other threads about the non-blocking "signal" assign and the blocking "variable" assign. busy should be assigned at, and only at, places where state is also assigned. If that is done, there would be no need to assign busy within a state, like in my #4. There would be no way to reach that state with busy = 0.

For the lines where you don't understand why the assignment is not needed, like #17, it is because there is no way to reach that point with busy = 0. busy will already be 1 because all places where state changes to send also change busy to '1'. busy then remains '1' while in the send state.

 

[hobbyiclearner]

I was away for some time and could not work on the code. As per your comments in post 28 why are lines as per comment no 2, 4, 6, 7 etc not needed?

Thanks,
Hobbyiclearner

 

[vGoodtimes]

current state is the same as the next state, or similar.

VHDL/Verilog have a default of:

process (blah) is
begin
  X <= X;  // a signal remains the same if not changed in a process, even if that is not desired.  (eg, non clocked processes infer latches vs erroring if a signal isn't assigned in any code branch.)
  Y := Y;  // a variable remains the same if not changed within a process.  (eg, you can infer registers or combinatorial logic or both with variables.  There is no way that could get confusing...)
  -- your actual code here.
end process

As a result there is no reason to write the code for: "when the current state is X, on the next cycle set the current state to (the same) X".

It isn't harmful to makes these re-assignments. It just isn't needed.

For comment #4, busy is set to '1' for all transitions to this state. It is not set to '0' for any transition from this state. As a result, the assignment is not needed as there would never be a way to reach the state with busy=0. You could add an assertion for busy = '1' instead. This would cause a simulation to show an error if busy /= '1' when in that state.


With this new understanding, you might try adding assert statements to your fsm's. This would help show when you are in a state and a state-related signal doesn't agree. Often it doesn't matter, but such designs are also often sloppy and miss corner cases. They can require much more test time than the 1-5 minutes to add asserts.

 

[hobbyiclearner]

As a result there is no reason to write the code for: "when the current state is X, on the next cycle set the current state to (the same) X".

It isn't harmful to makes these re-assignments. It just isn't needed.

.

Thanks. That helped. Removing the extra lines.

For comment #4, busy is set to '1' for all transitions to this state. It is not set to '0' for any transition from this state. As a result, the assignment is not needed as there would never be a way to reach the state with busy=0. You could add an assertion for busy = '1' instead. This would cause a simulation to show an error if busy /= '1' when in that state.

With this new understanding, you might try adding assert statements to your fsm's. This would help show when you are in a state and a state-related signal doesn't agree. Often it doesn't matter, but such designs are also often sloppy and miss corner cases. They can require much more test time than the 1-5 minutes to add asserts.

I would like to discuss this further before settling on an answer . Yes, there are no conditions with busy = 0. But this busy flag is for any block (lets say main block X)which is to send data to the lcd display via the lcd controller. There can be two possibilities: one in which X does not want to transmit any data to lcd controller which is perfect as the lcd display is in any case not ready. In the other scenario, the block X has data to send, in which case it has to wait till the lcd display is ready. So how does X come to know if display is ready ? It checks the status of busy flag. That is why I am keeping busy =1 for the entire initialize state.

BTW, from your last post it seems that assert is mainly for checking simulation results and maybe is not synthesizable. Am I true?

Thanks,
Hobbyiclearner.

 

[ads-ee]

What vGoodtimes was referring to is that assigning busy in a state which busy <= '1' is never going to be 0 when entering that state is redundant.

e.g.
You have 20 states s1 through s20. In s3 you set flag <= '1', you only clear flag when you reach state s17. you can either set flag to '1' in s3,s4,s5,s6,...,s15,s16 resulting in a huge OR gate or you could just write code that sets flag on s3 and clears it on s17...

---

Code VHDL - [expand]
1
2
3
4
5
6
7
if (reset = '1') then
  flag <= '0';
elsif (state = s3) then -- synchronous set
  flag <= '1';
elsif (state = s17) then -- synchronous clear
  flag <= '0';
end if;

---

flag only has to be explicitly set in state s3 as it will hold it's value until cleared in s17.

I typically write all my FSMs with no output signals from the FSM itself. I only have the state transitions in the FSM and perhaps any counters that are used to control the FSM (e.g. for counting how many times the FSM stays in a specific state). I put all the outputs of the FSM in a separate process, which only has the FSM outputs. Others may find this hard to read, but I prefer being able to see the flow of the FSM without all the output assignments, so I know what the transitions are in the FSM without having to draw a bubble diagram, which is what I usually have to do to FSMs with 15-20 outputs embedded that require multiple 60 line pages of text in my editor.

 

[vGoodtimes]

@ads-ee, you might also like tasks/procedures for fsm's. This isn't perfect, but you can also define a task for each transition-to-state. The output signals are only assigned in the tasks. The logic inside the core of the process is then very short and very understandable. There is no manual duplication of logic for the outputs.

There is also an interesting "idle trick". Often, the idle state could be skipped. The logic for the idle state is often long, making it a bad idea to manually copy/paste it. But a task for the idle state logic (with a default of at least moving to the idle state) means you can reliably/readably transition-to-or-skip the idle state. This has the same implications as copy-pasting the logic for the idle state, so it could increase the longest path. (the logic already exists for the idle state, so area probably won't be affected too much.)

I don't expect you will do this, as it is a large change to how designs are done. It has some negatives in that it makes it harder to analyze complexity of the outputs easily. However, I found the idea interesting and useful, if used consistently by someone who understands/expects this type of fsm construction.

 

[hobbyiclearner]

So maybe what vGoodTimes meant was since the busy =1 was set in the previous state and in the complete next state also it remains high, there is no point mentioning it again. Thanks, that also helped.

 

[vGoodtimes]

@hobbyiclearner: There are multiple ways to look at state machines.

In one method, you consider logic as it occurs _within_ a state. (This is best represented with a 2+ process state machine)

In one method, you consider logic as you _transition-to_ a state. (This is best represented with a 1 process state machine)

And sometimes you actually do both.

Confusion and design errors occur when you describe/expect a signal as "within this state", but fail to ensure all "transitions-to" that state actually work and use a language construct that only allows "transition-to" logic.

 

[hobbyiclearner]

@hobbyiclearner: There are multiple ways to look at state machines.

In one method, you consider logic as it occurs _within_ a state. (This is best represented with a 2+ process state machine)

In one method, you consider logic as you _transition-to_ a state. (This is best represented with a 1 process state machine)

.

OK... can you point to some literature for detailed understanding of the point. I might well as go through it and get a better grip on fsm while I am working on this code.

Hobbyiclearner

 

[hobbyiclearner]

I have one more query pls. As per your post 28, comment 15, why is that block of code not required.

Thanks,
Hobbyiclearner