5
\$\begingroup\$

I designed a (Q)SPI controller which can be configured over generics in QSYS (Quartus) and acts as an Avalon slave.

The documentation started in German, so please bear with the partial translation.

I want to know if I can improve my signal handling or naming of my variables (or let's just say: what can be done better and how?). We are two in our team, and there is no code review, and most of the time we define signal naming, or how to write VHDL code for ourselves.

The following code is a snippet:

 -- altera vhdl_input_version vhdl_2008
-- (Q)SPI Controller for communiction with an Avalon Master
-- Parametizeable over the GUI of QSYS - see *_hw.tcl
-- Calculation of ports is done in the *_hw.tcl file
-- Has to be done manually for simulation
-- Function:
-- zim_qspi_controller :
-- Top level - Communication with AVM and all components
-- flash_comp_cmd_V* :
-- Gets r/w and cmd codes signals from zim_qspi_controller
-- communicates directly with the SPI lines
-- clk_domain_sync and clk_domain_port_sync:
-- DFFs for synchronization between domains
-- dcfifo:
-- Intel IP for a FIFO
-- s1_* signals that the signal is in clk_domain 1 (avalon clock)
-- s2_* signals that the signal is in clk_domain 2 (spi_clock)
-- V01: 
/* Basic functions implemented
 The AVS_CSR(32 upto 63) = Unique ID and so on
 max Burstbytes = Number Bytes from the Fifo
 */
-- CSR can be read back to check if the settings were taken
-- From QSPI_DEF_PACK:
/* CSR Interface - Bit Mapping */
/* Wird hier was geaendert sind in der Architektur die Konstanten anzupassen */
 /*constant FR_BIT : natural := 0;
 constant PP_BIT : natural := 1;
 constant FRQIO_BIT : natural := 2; -- Normalerweise Dummy Cycle 
 constant PPQ_BIT : natural := 3;
 
 constant FRDIO_BIT : natural := 4; -- Normalerweise Dummy Cycle
 constant FRDO_BIT : natural := 5; 
 constant RD_BIT : natural := 6;
 constant FRQO_BIT : natural := 7; --0x80
 
 constant PERRSM_BIT : natural := 8; 
 constant RDID_BIT : natural := 9;
 constant RDUID_BIT : natural := 10;
 constant RDJDID_BIT : natural := 11; --x800
 
 constant RDMDID_BIT : natural := 12; 
 constant RDSFDP_BIT : natural := 13;
 constant RSTEN_BIT : natural := 14;
 constant RST_BIT : natural := 15; --x8000
 
 constant IRRD_BIT : natural := 16; 
 constant SER_BIT : natural := 17;
 constant BER32K_BIT : natural := 18;
 constant BER64K_BIT : natural := 19; --x8 0000
 
 constant CER_BIT : natural := 20; 
 constant WER_BIT : natural := 21; 
 constant WRSR_BIT : natural := 22;
 constant WRFR_BIT : natural := 23; --x80 0000
 
 constant RDSR_BIT : natural := 24;
 constant RDFR_BIT : natural := 25; 
*/
library altera_mf;
use altera_mf.all;
library ieee;
use ieee.std_logic_1164.all;
use ieee.numeric_std.all;
use ieee.math_real.all;
use WORK.avm_behav_pack.all;
use WORK.QSPI_DEF_PACK.all; -- Functions and constants
use IEEE.math_real.all; -- Only for pre synthesis 
entity zim_qspi_controller is
generic (
 constant FPGA_TYPE : string := "MAX 10";
 constant DEVICE_FAMILY : string := "IS25LQ";
 constant FIFO_WORDS : natural := 32;
 constant DEF_MODE : string := "QUAD MODE";
 constant SYNC_STAGES : natural := 2;
 constant ENDIAN : string := "BIG";
 constant AVALON_BUS_DATA_BYTES : natural := 4;
 constant ADDRESS_BITS : natural := 10;
 constant SLAVE_LATENCY : natural := 1;
 constant PERM_WR_ENA : boolean := TRUE;
 constant RESET_CMD_AFTER_EXEC : boolean := TRUE; -- sets the FLASH in R/W condition after each command
 constant CHECK_STATUS_REG_BEFORE_WRITE : boolean := TRUE -- checks if the FLASH is in IDLE condition before each write - Important for interrupted burst operations
 );
port (
 -- Sys
 -- clock interface
 csi_clock_clk : in std_logic;
 csi_clock_reset_n : in std_logic; -- Koennte auch rsi_reset_n sein
 csi_pll_locked : in std_logic;
 csi_qspi_clk_in : in std_logic;
 csi_qspi_inv_clk_in : in std_logic;
 
 --CSR-Interface
 avs_csr_write :in std_logic;
 avs_csr_read :in std_logic;
 avs_csr_address :in std_logic_vector(0 downto 0);
 avs_csr_writedata :in std_logic_vector(31 downto 0);
 avs_csr_readdata :out std_logic_vector(31 downto 0);
 avs_csr_waitrequest :out std_logic;
 
 
 --Speicher-Interface
 avs_mem_write :in std_logic; 
 avs_mem_waitrequest :out std_logic;
 avs_mem_read :in std_logic; 
 avs_mem_writedata :in std_logic_vector(AVALON_BUS_DATA_BYTES*8-1 downto 0);
 avs_mem_readdata :out std_logic_vector(AVALON_BUS_DATA_BYTES*8-1 downto 0);
 avs_mem_readdatavalid :out std_logic;
 avs_mem_byteenable :in std_logic_vector(AVALON_BUS_DATA_BYTES-1 downto 0);
 --avs_mem_irq :out std_logic; 
 avs_mem_burstcount :in std_logic_vector (4 downto 0);
 avs_mem_address :in std_logic_vector (ADDRESS_BITS-1 downto 0); 
 -- Flash Verbindung - bridged through flash_comp_cmd_v02 component
 coe_qspi_clk : out std_logic;
 coe_qspi_cs_n : out std_logic;
 coe_qspi_data : inout std_logic_vector(3 downto 0);
 coe_pll_locked : out std_logic
 );
end entity zim_qspi_controller;
architecture rtl of zim_qspi_controller is
-------- Helper constants --------
constant c_LITTLE_ENDIAN : std_logic := '0';
constant c_BIG_ENDIAN : std_logic := '1';
constant c_CSR_BITS : natural := 32;
constant c_NIOS_BYTES_PER_ADDR : natural := 4;
constant c_FLASH_BYTES_PER_ADDR : natural := get_byte_per_addr(DEVICE_FAMILY);
constant c_BURST_BITS : natural := 5;
constant c_ADDR_MULTIPLIER : natural := c_NIOS_BYTES_PER_ADDR / c_FLASH_BYTES_PER_ADDR; -- TODO: Pruefen auf gerade Zahlen und Vielfaches von 2 assert usw.
constant c_EXT_MODE : std_logic_vector (c_CSR_BITS-1 downto 0) := (others => '0'); 
constant c_FAST_DUAL_MODE : std_logic_vector (c_CSR_BITS-1 downto 0) := (0=>'1', others => '0'); 
constant c_FAST_QUAD_MODE : std_logic_vector (c_CSR_BITS-1 downto 0) := (1 =>'1', others => '0'); 
constant c_NORMAL_MODE : std_logic_vector (c_CSR_BITS-1 downto 0) := (2=> '1', others => '0'); 
/* CMD mapping
 * 4-7 => Dummy
 * ---- Ab hier sind Kommandos ----
 * 8 => Read Status Register
 * 9 => Read Function Register 
 * 10 => Resume Program/Erase
 * 11 => read manufactor and product id
 * 12 => read unique ID number
 * 13 => read manufacturer and product id by jedec id command
 * 14 => read manufacturer and device id
 * 15 => sfdp read
 * 16 => software reset enable
 * 17 => reset (only along with x66)
 * 18 => read information row 
*/
-------------------------- Components ------------------------
-- clk Domain Sync
component clk_domain_sync
generic ( num_stages : natural := 2);
port (
 clk_out : in std_logic;
 a_reset_n : in std_logic;
 
 signal_in : in std_logic;
 signal_out : out std_logic
 );
end component clk_domain_sync;
 
-- clk Domain Port SYNC
component clk_domain_port_sync is
generic ( NUM_STAGES : natural := 2;
 BUS_BITS : natural := 8
 );
port (
 clk_out : in std_logic;
 a_reset_n : in std_logic;
 
 signal_in : in std_logic_vector (BUS_BITS-1 downto 0);
 signal_out : out std_logic_vector (BUS_BITS-1 downto 0)
 );
end component clk_domain_port_sync;
-- FIFO - Intel IP
COMPONENT dcfifo
 GENERIC (
 intended_DEVICE_FAMILY : STRING;
 lpm_numwords : NATURAL;
 lpm_showahead : STRING;
 lpm_type : STRING;
 lpm_width : NATURAL;
 lpm_widthu : NATURAL;
 overflow_checking : STRING;
 rdsync_delaypipe : NATURAL;
 read_aclr_synch : STRING;
 underflow_checking : STRING;
 use_eab : STRING;
 write_aclr_synch : STRING;
 wrsync_delaypipe : NATURAL
 );
 PORT (
 aclr : IN STD_LOGIC ;
 data : IN STD_LOGIC_VECTOR (lpm_width-1 DOWNTO 0);
 rdclk : IN STD_LOGIC ;
 rdreq : IN STD_LOGIC ;
 wrclk : IN STD_LOGIC ;
 wrreq : IN STD_LOGIC ;
 q : OUT STD_LOGIC_VECTOR (lpm_width-1 DOWNTO 0);
 rdempty : OUT STD_LOGIC ;
 rdfull : OUT STD_LOGIC ;
 rdusedw : OUT STD_LOGIC_VECTOR (lpm_widthu-1 DOWNTO 0);
 wrfull : OUT STD_LOGIC ;
 wrempty : OUT STD_LOGIC ;
 wrusedw : OUT STD_LOGIC_VECTOR (lpm_widthu-1 DOWNTO 0)
 
 );
END COMPONENT;
-- Flash Comp CMD V02
 
 component flash_comp_cmd_v02 is
 generic (
 constant DEVICE_FAMILY : string := "IS25LQ";
 constant ENDIAN : string := "BIG"
 );
 port (
 csi_clk_clk : in std_logic; -- SPI clock
 csi_clk_reset_n : in std_logic;
 csi_pll_locked : in std_logic;
 csi_comp_ready : out std_logic;
 
 coe_cmd : in std_logic_vector(get_CMD_BITS(DEVICE_FAMILY)-1 downto 0);
 coe_spi_addr : in std_logic_vector(get_command_length(DEVICE_FAMILY)*get_addr_multi(DEVICE_FAMILY)-1 downto 0); 
 coe_spi_comp_busy : out std_logic;
 coe_cmd_data : in std_logic_vector (23 downto 0);
 
 coe_write : in std_logic; -- as long as write is active, data will be written on the SPI bus
 coe_wrfifo_wait : out std_logic; -- Daten is still being processed
 coe_wrfifo_empty : in std_logic;
 coe_wrfifo_data : in std_logic_vector (get_command_length(DEVICE_FAMILY)-1 downto 0);
 
 coe_read : in std_logic; -- As long as read is active, there is an read operation to the flash
 coe_rdfifo_wait : out std_logic;
 coe_rdfifo_data : out std_logic_vector (get_command_length(DEVICE_FAMILY)-1 downto 0); -- Shiftregister output
 
 -- Flash connection
 coe_qspi_cs_n :out std_logic;
 coe_qspi_data :inout std_logic_vector(3 downto 0)
 
 
 );
end component;
------------------------ TYPES --------------------------
 type avs_csr_register is array (1 downto 0) of std_logic_vector (31 downto 0);
 
 -- Quartus unterstuetzt keine unconstrained records ...(Q18)
 
---------------------- RECORDS --------------------------
---------------------- Functions ------------------------
--- GET DEFAULT MODE
function get_default_mode (constant DEF_MODE : in string) return std_logic_vector is
variable v_return_vector : std_logic_vector (31 downto 0);
begin
 v_return_vector := (others => '0');
 IF DEF_MODE = "NORMAL MODE" THEN
 v_return_vector := c_NORMAL_MODE; 
 ELSIF DEF_MODE = "EXTENDED MODE" THEN
 v_return_vector := c_EXT_MODE;
 ELSIF DEF_MODE = "FAST DUAL MODE" THEN
 v_return_vector := c_FAST_DUAL_MODE;
 ELSIF DEF_MODE = "QUAD MODE" THEN
 v_return_vector := c_FAST_QUAD_MODE;
 ELSE
 assert FALSE REPORT "Unbekannter Modus: " & DEF_MODE severity ERROR;
 END IF;
 return v_return_vector;
end get_default_mode;
----------------- Constants -----------------
constant c_ENDIAN : std_logic := get_endian (ENDIAN);
constant c_DEF_CSR : std_logic_vector (31 downto 0) := get_default_mode(DEF_MODE);
------------------- Signals -----------------
--SIGNAL s1_avm_mem_byteenable : std_logic_vector(AVALON_BUS_DATA_BYTES downto 0); -- wir nehmen ein Bit mehr fuer den RD_DATA_ALIGN_PROC
--alias a_byteena_zero_bit : std_logic is s1_avm_mem_byteenable(AVALON_BUS_DATA_BYTES);
---------- Process FSM
-- MAIN PROZESS <-> SPI Controller
SIGNAL s1_spi_read, s1_spi_write : std_logic;
SIGNAL s2_spi_read, s2_spi_write : std_logic;
SIGNAL s1_spi_cmd, s2_spi_cmd : std_logic_vector (get_cmd_bits(DEVICE_FAMILY)-1 downto 0);
SIGNAL s1_spi_addr, s2_spi_addr : std_logic_vector (get_command_length(DEVICE_FAMILY)*get_addr_multi(DEVICE_FAMILY)-1 downto 0);
SIGNAl s1_coe_cmd_data,s2_coe_cmd_data : std_logic_vector (23 downto 0);
--- FIFO
SIGNAL s2_wrfifo_out, s2_rdfifo_data : std_logic_vector (get_command_length(DEVICE_FAMILY)-1 downto 0);
SIGNAL s2_wrfifo_wait, s2_rdfifo_wait,s2_rdfifo_wrreq : std_logic; -- Signals from the flash_comp_cmd_v* component
SIGNAL s1_rdfifo_empty : std_logic;
SIGNAL s1_rdfifo_read, s1_rdfifo_read_avm : std_logic; -- For reading from RDfifo for CSR cmds
SIGNAL s1_clear_rdfifo, s1_aclr_rdfifo : std_logic;
alias spi_clk : std_logic is csi_qspi_clk_in;
SIGNAL s1_rd_ready : std_logic; -- Marker if RDfifo is ready for data
SIGNAL s1_wrfifo_write : std_logic;
SIGNAL s1_wr_data_buffer : unsigned (AVALON_BUS_DATA_BYTES*8-1 downto 0 );
SIGNAL s1_wrfifo_usedw : std_logic_vector (integer(ceil(log2(real(fifo_words))))-1 downto 0);
SIGNAl s1_byte_cnt : unsigned(AVALON_BUS_DATA_BYTES-1 downto 0);
-- WAITREQUEST 
SIGNAL s1_avs_readdata_valid, s1_avs_writedata_ack : std_logic;
-- CSR Register
SIGNAL s1_avs_csr : avs_csr_register;
alias a_csr : std_logic_vector (31 downto 0) is s1_avs_csr(0); 
alias a_addr : std_logic_vector (23 downto 0) is s1_avs_csr(1)(23 downto 0);
alias a_csr_cmd : std_logic_vector (7 downto 0) is s1_avs_csr(1)(7 downto 0);
-- WrFifo
SIGNAL s1_wrfifo_full, s2_wrfifo_rd_empty : std_logic;
SIGNAL s1_wrfifo_wr_requ : std_logic;
SIGNAL s1_wrfifo_data : std_logic_vector(7 downto 0);
SIGNAL s1_wrfifo_empty : std_logic;
------ Sonstiges ----
alias clk : std_logic is csi_clock_clk;
alias reset_n : std_logic is csi_clock_reset_n;
--SIGNAL s1_csr_busy : std_logic; -- Validation ob CSR Prozess beim Arbeiten ist, ACHTUNG! Kombinatorisch
SIGNAL s1_flash_comp_busy, s2_spi_busy : std_logic;
alias qspi_clk_inv : std_logic is csi_qspi_inv_clk_in; -- We are working with the falling edge of the spi_clk 
SIGNAL s2_flash_comp_rdy, s1_flash_comp_rdy : std_logic;
-- RD Data Align Proc
SIGNAL s1_rdfifo_ack : std_logic;
SIGNAL s1_rdfifo_data : std_logic_vector (7 downto 0);
SIGNAL s1_rdfifo_read_req : std_logic;
SIGNAL s1_rd_pre_ack : std_logic; -- For burstcount, waitrequest is set one cycle before readdata_valid
---------------------- BEGIN -------------------------
begin
coe_qspi_clk <= csi_qspi_clk_in;
-------------------- MAIN FSM ------------------------
MAIN_FSM : process (ALL) 
 type fsm_type is (
 IDLE , -- Waiting for cmd
 SPI_PREP_CMD , -- Commands ned to be synced
 MEM_WRITE , -- WRFIFO is being written - flash_comp_cmd_V* has a direct communication with the FIFO
 MEM_READ , -- Read RDFIFO and put data on avalon bus
 WAIT_FINISH , -- Wait for flash_comp_cmd_V*
 READ_CSR ,
 WRITE_CSR ,
 EXEC_CMD 
 );
variable fsm_cur : fsm_type;
--variable v_mode_changed : std_logic; -- Zum schauen ob sich Read/Write geloest hat
variable v_csr_read, v_csr_write : std_logic; -- marker
variable v_mem_read, v_mem_write : std_logic; -- marker;
variable v_wait_for_flash_busy : std_logic;
variable v_cmd_running : std_logic; -- Marker
variable v_wr_requ : std_logic;
variable v_mode_changed : std_logic; -- 1 Pulse, checking if the AVM wants to do something new, while an operation is running
variable v_csr_addr : std_logic_vector (0 downto 0);
--variable v_mem_addr : std_logic_vector (ADDRESS_BITS-1 downto 0);
variable v_avs_csr_wr_valid : std_logic;
variable v_avs_csr_rd_valid : std_logic;
variable v_mem_access : std_logic;
variable v_read_csr_ack : std_logic;
variable v_conv_flash_addr : std_logic_vector (get_command_length(DEVICE_FAMILY)*get_addr_multi(DEVICE_FAMILY)-1 downto 0);
variable v_set_write_enable : std_logic; -- Marker, if set we enable Write Enable of the flash
variable v_status_reg_idle : std_logic; -- When CHECK_FOR_BUSY is set, => 1 if status reg is idle
variable v_csr_write_ack : std_logic; -- Waitrequest Ack
begin
 coe_pll_locked <= csi_pll_locked;
 -- Waitrequest Kombinatorische Zuweisungen
 
 IF reset_n /= '1' or s1_flash_comp_rdy = '0' THEN 
 
 fsm_cur := IDLE; -- Vorbereiten mit den Default Werten
 
 s1_spi_read <= '0';
 s1_spi_write <= '0';
 v_wait_for_flash_busy := '1';
 v_wr_requ := '0';
 s1_clear_rdfifo <= '1';
 v_read_csr_ack := '0';
 s1_rdfifo_read <= '0';
 s1_rdfifo_read_avm <= '0';
 v_mem_access := '0';
 avs_csr_waitrequest <= '1';
 avs_mem_waitrequest <= '1';
 v_cmd_running := '0';
 v_csr_read := '0';
 v_csr_write := '0';
 v_mem_read := '0';
 v_mem_write := '0';
 s1_wrfifo_write <= '0';
 v_set_write_enable := '0';
 avs_mem_readdatavalid <= '0';
 s1_avs_readdata_valid <= '0';
 v_avs_csr_rd_valid := '0';
 v_avs_csr_wr_valid := '0';
 v_status_reg_idle := '0';
 v_csr_write_ack := '0';
 
 v_conv_flash_addr := (others => '0');
 s1_coe_cmd_data <= (others => '0');
 
 
 s1_spi_addr <= (others => '0');
 s1_spi_cmd <= (others => '0');
 s1_avs_csr <= (others => (others => '0'));
 avs_csr_readdata <= (others => '0');
 a_csr <= c_DEF_CSR; -- Reset to the pre configured CSR 
 
 ELSIF RISING_EDGE(clk) THEN
 v_avs_csr_wr_valid := v_csr_write_ack and not (v_mode_changed or v_cmd_running);
 v_avs_csr_rd_valid := (avs_csr_read and v_read_csr_ack); -- Read needs one cycle => s1_csr_busy for one cycle to HIGH
 
 avs_csr_waitrequest <= not ((v_avs_csr_wr_valid or v_avs_csr_rd_valid));
 avs_mem_waitrequest <= not (((s1_avs_writedata_ack or s1_avs_readdata_valid)) ) ;
 avs_mem_readdatavalid <= s1_rdfifo_ack and v_mem_access;
 s1_avs_readdata_valid <= s1_rd_pre_ack and s1_flash_comp_busy and avs_mem_read; --avs_mem_read and not s1_rdfifo_empty and not v_mode_changed;
 
 
 
 if v_csr_read then
 v_mode_changed := avs_mem_read or avs_mem_write or avs_csr_write;
 elsif v_csr_write then
 v_mode_changed := avs_mem_read or avs_mem_write or avs_csr_read;
 elsif v_mem_read then
 v_mode_changed := avs_mem_write or avs_csr_read or avs_csr_write;
 elsif v_mem_write then
 v_mode_changed := avs_mem_read or avs_csr_read or avs_csr_write;
 else
 v_mode_changed := '0';
 end if;
 
 
 v_wait_for_flash_busy := not s1_flash_comp_busy and v_cmd_running and not avs_csr_read;--'1' WHEN not s1_flash_comp_busy and v_cmd_running ELSE '0'; -- Durchs synchronisieren ist die flash_comp nicht sofort beim Arbeiten
 
 --------------- FSM --------------
 CASE fsm_cur is
 ------------ IDLE
 -- Only in the IDLE state are commandos valid
 -- Else: Waitrequest until we finished the operation
 WHEN IDLE => 
 if s1_flash_comp_rdy then
 v_csr_addr := avs_csr_address; -- We ack write commands immediately
 avs_csr_readdata <= s1_avs_csr(to_integer(UNSIGNED(v_csr_addr)));
 v_wr_requ := avs_mem_write;
 s1_clear_rdfifo <= '0';
 s1_rdfifo_read <= '0';
 s1_rdfifo_read_avm <= '0';
 v_mem_access := '0';
 v_read_csr_ack := '0';
 v_cmd_running := '0';
 v_csr_read := '0';
 v_csr_write := '0';
 v_mem_read := '0';
 v_mem_write := '0';
 s1_wrfifo_write <= '0';
 v_csr_write_ack := '0';
 -- Mem Read
 IF avs_mem_read then
 fsm_cur := SPI_PREP_CMD;
 v_mem_access := '1';
 s1_rdfifo_read_avm <= '1';
 v_cmd_running := '1';
 v_mem_read := '1';
 
 -- Mem Write 
 elsif avs_mem_write then
 v_mem_write := '1';
 if CHECK_STATUS_REG_BEFORE_WRITE then
 if not v_status_reg_idle then -- Zuerst Status Pollen
 a_csr <= (RDSR_BIT => '1', others => '0');
 s1_spi_cmd <= (RDSR_BIT => '1', others => '0');
 v_cmd_running := '1';
 v_csr_write := '1';
 fsm_cur := EXEC_CMD;
 else
 if PERM_WR_ENA then
 if v_set_write_enable then
 s1_spi_cmd <= (WER_BIT => '1', others => '0');
 s1_spi_write <= '1';
 fsm_cur := MEM_WRITE;
 v_cmd_running := '1';
 else
 fsm_cur := SPI_PREP_CMD;
 v_mem_access := '1';
 v_cmd_running := '1';
 
 s1_wrfifo_write <= '1';
 end if;
 else -- no PERM_WR_ENA
 fsm_cur := SPI_PREP_CMD;
 v_mem_access := '1';
 v_cmd_running := '1';
 v_mem_write := '1';
 s1_wrfifo_write <= '1';
 end if;
 end if;
 
 else 
 if PERM_WR_ENA then
 if v_set_write_enable then
 s1_spi_cmd <= (WER_BIT => '1', others => '0');
 s1_spi_write <= '1';
 fsm_cur := MEM_WRITE;
 v_cmd_running := '1';
 else
 fsm_cur := SPI_PREP_CMD;
 v_mem_access := '1';
 v_cmd_running := '1';
 v_mem_write := '1';
 s1_wrfifo_write <= '1';
 end if;
 else -- kein PERM_WR_ENA
 fsm_cur := SPI_PREP_CMD;
 v_mem_access := '1';
 v_cmd_running := '1';
 v_mem_write := '1';
 s1_wrfifo_write <= '1';
 end if;
 end if;
 
 -- CSR read
 elsif avs_csr_read then
 fsm_cur := READ_CSR;
 v_cmd_running := '1';
 v_csr_read := '1';
 
 -- CSR write
 elsif avs_csr_write then
 v_cmd_running := '1';
 if PERM_WR_ENA then
 if v_set_write_enable and (avs_csr_writedata(WRSR_BIT) or avs_csr_writedata(WRFR_BIT) or avs_csr_writedata(CER_BIT) or avs_csr_writedata(SER_BIT) or avs_csr_writedata(BER32K_BIT) or avs_csr_writedata(BER64K_BIT)) then
 s1_spi_cmd <= (WER_BIT => '1', others => '0');
 s1_spi_write <= '1';
 fsm_cur := MEM_WRITE;
 
 else
 fsm_cur := WRITE_CSR;
 v_csr_write := '1';
 end if;
 else
 fsm_cur := WRITE_CSR;
 v_csr_write := '1';
 end if;
 
 -- We do nothing
 else
 v_set_write_enable := '1';
 fsm_cur := IDLE; 
 end if;
 
 -- End check if command pending --
 
 
 --v_conv_flash_addr(avs_mem_address'HIGH downto 0) := avs_mem_address; -- Nios kann QUAD Alignt sein, also z.B. Adresse 1 wäre avs_mem => (others=>'0') mit byte_ena b"0010"
 -- Check byteenable bits
 for I in AVALON_BUS_DATA_BYTES-1 downto 0 loop
 if avs_mem_byteenable(I) then
 --v_conv_flash_addr := STD_LOGIC_VECTOR(TO_UNSIGNED(TO_INTEGER(UNSIGNED(avs_mem_address)) + I,v_conv_flash_addr'HIGH+1));
 v_conv_flash_addr := STD_LOGIC_VECTOR(TO_UNSIGNED(TO_INTEGER(UNSIGNED(avs_mem_address))*c_ADDR_MULTIPLIER-I,v_conv_flash_addr'HIGH+1));
 end if;
 end loop; 
 
 end if; 
 
 
 ----------- MEM READ
 -- Command is already sent to flash_comp_cmd_V*
 -- Signals are routed to the sync components and from there to the flash_comp_cmd_v*
 WHEN MEM_READ =>
 IF avs_mem_read THEN -- We read as long as the master is asserting read
 s1_spi_read <= '1'; 
 ELSE
 s1_rdfifo_read_avm <= '0';
 fsm_cur := WAIT_FINISH;
 END IF;
 
 s1_spi_addr <= v_conv_flash_addr;
 --------------- MEM_WRITE 
 WHEN MEM_WRITE =>
 v_wr_requ := '0';
 s1_spi_addr <= v_conv_flash_addr;
 
 s1_spi_write <= '1';
 IF not v_wait_for_flash_busy THEN
 fsm_cur := WAIT_FINISH;
 s1_spi_write <= '0';
 END IF;
 
 --------------- WAIT_FINISH
 WHEN WAIT_FINISH =>
 if s1_rd_ready then
 s1_spi_read <= '0';
 end if;
 s1_clear_rdfifo <= '1';
 if RESET_CMD_AFTER_EXEC = TRUE then -- Is eavluated at pre synthesis
 a_csr <= c_DEF_CSR;
 s1_spi_cmd <= c_DEF_CSR(s1_spi_cmd'HIGH downto 0);
 end if;
 
 IF s1_rd_ready and s1_wrfifo_empty and not s1_flash_comp_busy THEN -- All data was sent/read and the flash_comp_cmd_V* component is IDLE
 if CHECK_STATUS_REG_BEFORE_WRITE then
 if not v_status_reg_idle and v_mem_write then 
 v_status_reg_idle := not s1_avs_csr(1)(24); 
 fsm_cur := IDLE;
 if PERM_WR_ENA then
 v_set_write_enable := '1';
 end if;
 else
 v_cmd_running := '0';
 fsm_cur := IDLE; -- We go to IDLE in each path
 
 if PERM_WR_ENA then 
 if v_set_write_enable then -- Write enable was already set
 v_set_write_enable := '0';
 else
 v_status_reg_idle := '0';
 end if; 
 end if;
 end if;
 
 -------------------------------
 elsif PERM_WR_ENA then 
 v_set_write_enable := not v_set_write_enable; 
 fsm_cur := IDLE;
 v_cmd_running := '0'; 
 else
 fsm_cur := IDLE;
 v_cmd_running := '0';
 end if;
 
 END IF;
 ------------- CSR READ
 WHEN READ_CSR => 
 v_read_csr_ack := '1';
 fsm_cur := IDLE;
 --v_cmd_running := '0';
 
 ----------- CSR Write 
 WHEN WRITE_CSR => -- Kommen hier nur von IDLE aus her, somit sparen wir uns die state Kontrollen
 v_csr_write_ack := '1';
 IF UNSIGNED(v_csr_addr) = 0 THEN 
 a_csr <= avs_csr_writedata;
 fsm_cur := SPI_PREP_CMD;
 else
 a_addr <= avs_csr_writedata(23 downto 0);
 fsm_cur := IDLE;
 v_cmd_running := '0';
 END IF; 
 
 ---------------------
 -- Hier wird der STATUS der CSR in den Kommando code fuer das SPI Interface umgewandelt
 WHEN SPI_PREP_CMD => 
 --v_csr_write_ack := '0';
 s1_wrfifo_write <= '0';
 s1_spi_cmd <= (others=>'0'); 
 -- Kommando auswerten
 -- Nach Synthese wird nur der Teil verwendet der ausgewaehlt wurde
 if v_wr_requ then
 fsm_cur := MEM_WRITE;
 elsif avs_mem_read then
 fsm_cur := MEM_READ;
 else
 fsm_cur := EXEC_CMD;
 end if;
 
 IF DEVICE_FAMILY = "IS25LQ" THEN
 CASE a_csr is
 
 
 -- FAST Mode
 WHEN c_EXT_MODE => 
 IF v_wr_requ THEN
 s1_spi_cmd(PP_BIT) <= '1';
 ELSE
 s1_spi_cmd(FR_BIT) <= '1';
 END IF; 
 
 
 -- FAST_QUAD_MODE
 WHEN c_FAST_QUAD_MODE =>
 IF v_wr_requ THEN
 s1_spi_cmd (PPQ_BIT) <= '1';
 else
 s1_spi_cmd(FRQIO_BIT) <= '1';
 END IF;
 
 
 --- Others
 -- Ein Kommando soll ausgefuehrt werden
 WHEN others =>
 s1_coe_cmd_data <= a_addr;
 s1_spi_cmd <= STD_LOGIC_VECTOR(TO_UNSIGNED((to_integer(UNSIGNED(a_csr))),s1_spi_cmd'HIGH+1)); 
 end CASE; 
 
 ELSE -- Nicht unterstuetzer FLASH
 assert FALSE report "Flash Type wird nicht unterstuetzt" severity ERROR;
 END IF;
 
 ---- EXEC_CMD
 when EXEC_CMD =>
 -- Validiere welches Kommando 
 
 if a_csr(RDSR_BIT) or a_csr(RDFR_BIT) then
 if s1_rd_ready then
 s1_spi_read <= '1';
 s1_rdfifo_read <= '1';
 else
 s1_rdfifo_read <= '0';
 end if;
 else
 s1_spi_write <= '1';
 s1_rdfifo_read <= '0';
 end if;
 
 IF not v_wait_for_flash_busy and s1_spi_write THEN
 fsm_cur := WAIT_FINISH;
 s1_spi_write <= '0';
 ELSIF not v_wait_for_flash_busy and s1_rdfifo_read_req then
 s1_spi_read <= '0';
 fsm_cur := WAIT_FINISH;
 s1_avs_csr(1)(31 downto 24) <= s1_rdfifo_data;
 s1_clear_rdfifo <= '1';
 END IF;
 
 ----- WHEN OTHERS
 WHEN others =>
 fsm_cur := IDLE;
 v_cmd_running := '0';
 end CASE;
 END IF;
end process MAIN_FSM;
------------------ ENDE MAIN FSM --------------------
toolic
14.6k5 gold badges29 silver badges204 bronze badges
asked Oct 10, 2019 at 12:36
\$\endgroup\$
1
  • 1
    \$\begingroup\$ To prevent opinionated answers: what do you want to achieve? I mean, I think this is very hard to read as it is so large. So I would split it up over some hierarchy, with fsms in multiple processes. But that's a personal preference. \$\endgroup\$ Commented Nov 5, 2019 at 14:05

1 Answer 1

1
\$\begingroup\$

Comments

The comments at the top of the code use an inconsistent mix of single-line and block comments. I recommend a block comment there.

The constants in header comment block seem to be unrelated to the code. If that is the case, remove these comments to avoid clutter.

Remove all other commented-out code lines, like:

 --avs_mem_irq :out std_logic; 

Use English in all the comments, instead of a mixture of German and English.

Delete obvious comments like these:

---------------------- BEGIN -------------------------
begin
-------------------- MAIN FSM ------------------------
MAIN_FSM : process (ALL) 
 ----- WHEN OTHERS
 WHEN others =>

The comments convey no useful information since they are redundant with the code the are meant to describe.

Indentation

Indentation usage is inconsistent. For example:

clk_out : in std_logic;
a_reset_n : in std_logic;
signal_in : in std_logic;
signal_out : out std_logic

Also, it seems you are mixing Tab characters with single-spaces. This can make the code render poorly because not all people and code displaying/editing software use the same Tab width. This will cause inconsistent indentation, making the code hard to understand.

I recommend using 4-space indentation levels and no Tab characters.

Keyword case

There is inconsistent usage of upper and lower case for keywords. For example:

component clk_domain_port_sync is
COMPONENT dcfifo

You use both for the component keyword. In the following, you have a strange combination of upper and lower for a single word:

SIGNAl

I recommend all lower-case for all keywords.

Naming

Names like this are too generic:

MAIN_FSM

While it is good to denote that some logic represents an FSM, many digital designs contain an FSM. When building a larger design from smaller components, you could encounter naming collisions. The name should be changed to be more unique, like:

SPI_AVALON_FSM

Partitioning

The FSM code uses a single process block, but there is a lot of that code in one process. Consider splitting the code up into more than one process, as is commonly done. It could help make the code easier to understand and maintain.

answered Nov 11, 2024 at 17:57
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.