VHDL: UART State Machine

[Hugo P. (portisch)]

Hallo,

ich bin VHDL Anfänger und hab nun so einige Probleme eine UART State 
Machine zum Laufen zu bekommen. Das Empfangen/Senden von 8 Bits (1 Byte) 
funktioniert ohne Probleme - mit z.B. einem Echo Process.

Nun will ich aber ein Protokoll im FPGA implementieren. Ein Command vom 
PC zum FPGA sieht z.B. so aus:
AA 55 00 03 00 00
AA: Sync1
55: Sync2
00: FPGA Adresse (RS422)
03: Command (Set DAC)
00: 15..8 data of 16 Bit Unsigned
00: 7..0 data of 16 Bit Unsigned

Ich habe die UART von hier in Verwendung:
https://github.com/jakubcabal/uart-for-fpga

(Habe aber das Parity rausgeschmissen)

Ich habe das Cmod S6 Board das eine 8MHz und eine 1Hz Clock für den FPGA 
hat.
Aus dem 8MHz mache ich mir mit einem IP Core 100Mhz mit dieser der UART 
betrieben wird. Der Process(CLK_LFC) überprüft ob es einen Timeout gibt 
mit ~1-2s. Dann springt die UART State Machine zurück auf idle.

Nun funktioniert das ganze in der Simulation aber beim Erstellen des 
Bitstreams für den FPGA gibt es diese Warnung und deswegen funktioniert 
das auf dem FPGA nicht. Verwendet wird Xilinx ISE 14.7:

WARNING:PhysDesignRules:372 - Gated clock. Clock net state[3]_PWR_14_o_Mux_31_o
   is sourced by a combinatorial pin. This is not good design practice. Use the
   CE pin to control the loading of data into the flip-flop.
WARNING:PhysDesignRules:372 - Gated clock. Clock net state[3]_PWR_20_o_Mux_43_o
   is sourced by a combinatorial pin. This is not good design practice. Use the
   CE pin to control the loading of data into the flip-flop.

----------------------------------------------------------------------------------
-- Company: 
-- Engineer: 
-- 
-- Create Date:    13:20:04 06/06/2018 
-- Design Name: 
-- Module Name:    Spartan6_Uart - Behavioral 
-- Project Name: 
-- Target Devices: 
-- Tool versions: 
-- Description: 
--
-- Dependencies: 
--
-- Revision: 
-- Revision 0.01 - File Created
-- Additional Comments: 
--
----------------------------------------------------------------------------------
library IEEE;
use IEEE.STD_LOGIC_1164.ALL;

-- Uncomment the following library declaration if using
-- arithmetic functions with Signed or Unsigned values
use IEEE.NUMERIC_STD.ALL;

-- Uncomment the following library declaration if instantiating
-- any Xilinx primitives in this code.
--library UNISIM;
--use UNISIM.VComponents.all;

entity Spartan6_Uart is
    Generic (
        CLK_FREQ   : integer := 100e6;  -- set system clock frequency in Hz
        BAUD_RATE  : integer := 9600    -- baud rate value
    );
  Port ( 
    -- FPGA_GCLK, 8MHz
    CLK      : in STD_LOGIC;

    -- FPGA_LFC, 1 Hz
    CLK_LFC    : in STD_LOGIC;

    -- LED outputs
    LED_0      : out STD_LOGIC;
    LED_1      : out STD_LOGIC;
    LED_2      : out STD_LOGIC;
    LED_3      : out STD_LOGIC;

    -- BTN inputs
    BTN_0      : in STD_LOGIC;
    BTN_1      : in STD_LOGIC;    

    -- connected to physical I/O pad
    PORTA     : inout  STD_LOGIC_VECTOR (7 downto 0);
    PORTB     : inout  STD_LOGIC_VECTOR (7 downto 0);
    PORTC     : inout  STD_LOGIC_VECTOR (6 downto 0);
    PORTD     : inout  STD_LOGIC_VECTOR (7 downto 0);
    PORTE     : inout  STD_LOGIC_VECTOR (7 downto 0);  
    PORTF     : inout  STD_LOGIC_VECTOR (6 downto 0)
  );
end Spartan6_Uart;

architecture Behavioral of Spartan6_Uart is
-------------------------------------------------------
-- 100 MHz clock
-------------------------------------------------------
  component Clock_100Mhz
    port
    (  
      -- Clock in ports
      CLK_IN1           : in     std_logic;
      -- Clock out ports
      CLK_OUT1          : out    std_logic
    );
  end component;
  
  signal clk_100 : STD_LOGIC;
  
-------------------------------------------------------
-- UART
-------------------------------------------------------  

  signal data_in    : std_logic_vector(7 downto 0) := ( others => '0' );
  signal data_out   : std_logic_vector(7 downto 0) := ( others => '0' );

  signal valid_in: std_logic := '0';
  signal valid_out: std_logic := '0';
  
  signal BUSY   : std_logic;
  
  -- FSM states
  type statetype is (idle, sync_1, sync_2, address_match, cmd3_r, send_address, AKN, s_tx);
  signal state, next_state, next_tx_state : statetype := idle;  
  
  signal timeout_counter : integer range 0 to 1 := 0;
  signal timeout : std_logic := '0';
  
begin

  reset <= BTN_0;
  
  my_100MHz_clock_instance : Clock_100Mhz
  port map
  (  
    -- Clock in ports
    CLK_IN1 => CLK,
    -- Clock out ports
    CLK_OUT1 => clk_100
  );
  
  uart_i: entity work.UART
    generic map (
        CLK_FREQ    => CLK_FREQ,
        BAUD_RATE   => BAUD_RATE
    )
    port map (
        CLK         => clk_100,
        RST         => reset,
        -- UART INTERFACE
        UART_TXD    => PORTD(4),
        UART_RXD    => PORTD(5),
        -- USER DATA OUTPUT INTERFACE
        DATA_OUT    => data_in,
        DATA_VLD    => valid_in,
        -- USER DATA INPUT INTERFACE
        DATA_IN     => data_out,
        DATA_SEND   => valid_out,
        BUSY        => BUSY      
    );
   
   process (clk_100) is
   begin
    if rising_edge(clk_100) then
      if timeout = '0' then
        state <= next_state;
      else
        state <= idle;
      end if;
    end if;   
   end process;
   
   process (CLK_LFC) is
   begin
    if rising_edge(CLK_LFC) then    
      if state /= idle then
        timeout_counter <= timeout_counter + 1;
        
        if timeout_counter = 1 then
          timeout <= '1';
        else
          timeout <= '0';
        end if;
      else
        timeout_counter <= 0;
        timeout <= '0';
      end if;
    end if;
   end process;
   
  process(state, valid_in, BUSY)
    variable bits : integer range 0 to 48 := 0;
  begin  
      next_state <= state;    
      valid_out <= '0';
      
      case state is
        when idle =>
          next_state <= idle;    
          if valid_in = '1' then
            if data_in = x"AA" then
              next_state <= sync_1;              
            end if;
          end if;
          
        when sync_1 =>
          next_state <= sync_1;    
          if valid_in = '1' then
            if data_in = x"55" then
              next_state <= sync_2;              
            else
              next_state <= idle;
            end if;
          end if;  

        when sync_2 =>
          next_state <= sync_2;  
          if valid_in = '1' then
            if unsigned(data_in) = unsigned(PORTF) then
              next_state <= address_match;              
            else
              next_state <= idle;
            end if;
          end if;
          
        when address_match =>
          next_state <= address_match;
          if valid_in = '1' then
            if unsigned(data_in) = x"03" then
              bits := 16;
              next_state <= cmd3_r;
            end if;          
          end if;
          
        when cmd3_r =>
          next_state <= cmd3_r;
          if valid_in = '1' then
            
            bits := bits - 8;
            --load 16 unsigned value
                      
            if bits = 0 then        
              next_tx_state <= AKN;
              next_state <= send_address;              
            end if;
          end if;
          
        when s_rx_wait_done =>
          if valid_in = '0' then
            next_state <= next_rx_state;
          end if;    
            
        when send_address =>
          data_out <= "0" & PORTF;
          next_state <= s_tx;
          
        when AKN =>
          data_out <= x"55";
          next_tx_state <= idle;
          next_state <= s_tx;        
          
        when s_tx =>
          next_state <= s_tx;
          if BUSY = '0' then            
            valid_out <= '1';
            next_state <= next_tx_state;
          end if;    
      end case;
  end process;
     
  

  -------------------------------------------------------
  -- LED/BTN Testing
  -------------------------------------------------------
  LED_3 <= CLK_LFC;
  LED_2 <= not CLK_LFC;

  LED_0 <= BTN_0;
  LED_1 <= timeout;

end Behavioral;

Das Warning über die Gated clock bekomme ich weg, wenn ich die State 
Machine zusätzlich um die clk_100 erweitere und ein "if 
rising_edge(clk_100) then" einbaue.

Gibt es irgendwo ein Beispiel wo so eine State Machine dargestellt ist?
Im Internet finde ich nur die UART State Machine selbst um die 8 bits zu 
empfangen/senden. Jedoch nichts dazu wenn mehrere Bytes empfangen werden 
sollen...

Oder wie löst man eine solche Protokoll Auswertung im FPGA besser!?

[Lothar M. (Firma: Titel) (lkmiller) (Moderator)]

Hugo P. schrieb:
> Das Warning über die Gated clock bekomme ich weg, wenn ich die State
> Machine zusätzlich um die clk_100 erweitere und ein "if
> rising_edge(clk_100) then" einbaue.
Der beliebte Anfängerfehler aus der Zwei-Prozess-Fraktion: du hast mit 
"bits := bits - 8;" in einem kombinatorischen Prozess einen Zähler 
eingebaut.
Der Synthesizer führt dich in die Irre, denn eigentlich ist das eine 
kombinatorische Schleife, die aber über den state und valid_in 
unterbrochen (gegated) werden kann.
Hier die Grundlagen dazu:
http://www.lothar-miller.de/s9y/archives/42-Kombinatorische-Schleifen.html

Mit ein wenig Stöbern findest du dort auch noch eine RS232 
Schnittstelle. Die kannst du dir ja mal als Denkanstoß anschauen...

> Nun will ich aber ein Protokoll im FPGA implementieren. Ein Command vom
> PC zum FPGA sieht z.B. so aus:
> AA 55 00 03 00 00
> AA: Sync1
> 55: Sync2
> 00: FPGA Adresse (RS422)
> 03: Command (Set DAC)
> 00: 15..8 data of 16 Bit Unsigned
> 00: 7..0 data of 16 Bit Unsigned
Das ist ein recht wagemutiges Protokoll. Was, wenn da Daten AA 55 
übertragen werden sollen?

Aber prinzipiell läuft es schon so ab, dass du ein Byte nach dem anderen 
empfängst und dann entsprechend der Kommandos verzweigst. eben so, wie 
du es da schon implementiert hast. Nur eben ohne die kombinatorische 
Schleife...

> Ich habe die UART von hier in Verwendung:
Unschöne Sache, diese Taktgenerierung. Das sollte die Toolchain auch 
anmeckern...

[Hugo P. (portisch)]

Danke! Das Hilft doch gleich mal weiter!
Ein Problem ist das "bits" und ein weiteres das "data_out".

Werde mir die Methode "Ein-Prozess-Darstellung" für die FSM zu Herzen 
nehmen und einmal versuchen diese so umzubauen. (Ist mir wegen der 
Übersicht auch lieber...)

[Hugo P. (portisch)]

Das geht doch gleich viel besser!!

Habe hier mal nur den Empfang der Daten AA 55 00 03 00 00 eingebaut:

   process (CLK_LFC) is
   begin
    if rising_edge(CLK_LFC) then    
      if state /= idle then
        timeout_counter <= timeout_counter + 1;
        
        if timeout_counter = 1 then
          timeout <= '1';
        else
          timeout <= '0';
        end if;
      else
        timeout_counter <= 0;
        timeout <= '0';
      end if;
    end if;
   end process;   
   
   process(clk_100) is
    variable bits : integer range 0 to 48 := 0;
   begin
    if rising_edge(clk_100) then  
      
      if timeout = '1' then
        state <= idle;
      end if;
      
      case state is
        when idle =>
          if valid_in = '1' then
            if data_in = x"AA" then
              state <= sync_1;
            else
              state <= idle;
            end if;
          end if;
          
        when sync_1 =>
          if valid_in = '1' then
            if data_in = x"55" then
              state <= sync_2;
            else
              state <= idle;
            end if;
          end if;  

        when sync_2 =>
          if valid_in = '1' then
            if unsigned(data_in) = unsigned(PORTF) then
              state <= address_match;
            else
              state <= idle;
            end if;
          end if;
          
        when address_match =>
          if valid_in = '1' then
            if unsigned(data_in) = x"03" then
              bits := 16;
              state <= cmd_03;
            else
              state <= idle;
            end if;          
          end if;
          
        when cmd_03 =>
          if valid_in = '1' then
            bits := bits - 8;
            
            if bits = 0 then
              next_tx_state <= AKN;
              state <= send_address;              
            end if;
          end if;
          
        when send_address =>
          if BUSY = '0' then
            data_out <= "0" & PORTF;
            valid_out <= '1';
            state <= s_tx_done;
          end if;          
          
        when AKN =>
          if BUSY = '0' then
            data_out <= x"55";
            valid_out <= '1';
            next_tx_state <= idle;
            state <= s_tx_done;
          end if;  

        when s_tx_done =>
          if BUSY = '0' then
            valid_out <= '0';
            state <= next_tx_state;
          end if;
          
        when others =>
          state <= idle;
      end case;
    end if;
   end process;

Lothar M. schrieb:
> Das ist ein recht wagemutiges Protokoll. Was, wenn da Daten AA 55
> übertragen werden sollen?

Wie sollte man das sicherer Lösen? Einer Checksum über alle Daten 
zusätzlich?

[Lothar M. (Firma: Titel) (lkmiller) (Moderator)]

Hugo P. schrieb:
> Wie sollte man das sicherer Lösen?
Entweder mit der Prüfsumme, die aber im Grunde auch nur das Verwerfen 
solch versetzter Telegramme erleichtert.
Mit einem langen Timeout zwischen den Telegrammen ginge das auch. Den 
Counter dafür hast du ja schon.

Ich übertrage die Nutzdaten nicht binär sondern als ASCII-Zeichen z.B. 
als Hex-Zahlen. Dann habe ich zwar mehr Traffic, kann aber mit den 
üblichen seriellen Steuerzeichen wie STX, ETX, CR, LF usw. arbeiten. Und 
das Zeug mit jedem beliebigen Terminal zeilenweise mitlesen.