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 --------------------
-
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\$JHBonarius– JHBonarius2019年11月05日 14:05:34 +00:00Commented Nov 5, 2019 at 14:05
1 Answer 1
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.