From fd20e740794eef90e2d52eca175bc6fbcbe21707 Mon Sep 17 00:00:00 2001
From: Reinier van der Walle <walle@astron.nl>
Date: Thu, 6 Apr 2023 12:28:22 +0200
Subject: [PATCH] processed review comments

---
 .../axi4/src/vhdl/axi4_lite_mm_bridge.vhd     | 24 +++++++-----
 .../axi4/tb/vhdl/tb_axi4_lite_mm_bridge.vhd   | 39 +++++++++++++------
 2 files changed, 42 insertions(+), 21 deletions(-)

diff --git a/libraries/base/axi4/src/vhdl/axi4_lite_mm_bridge.vhd b/libraries/base/axi4/src/vhdl/axi4_lite_mm_bridge.vhd
index 443f690a74..73799be8cb 100644
--- a/libraries/base/axi4/src/vhdl/axi4_lite_mm_bridge.vhd
+++ b/libraries/base/axi4/src/vhdl/axi4_lite_mm_bridge.vhd
@@ -28,8 +28,9 @@
 --     AXI4 interface often comes with an active-low reset while DP comes with an active-high 
 --     reset.
 -- . Remark:
---   . The read latency is not adapted. Ensure that the Controller an Peripheral use the same 
+--   . The read latency is not adapted. Ensure that the Controller and Peripheral use the same 
 --     read-latency.
+--   . Both AXI4-lite and MM use ready latency = 0 for waitrequest/ready.
 
 LIBRARY IEEE, common_lib;
 USE IEEE.STD_LOGIC_1164.ALL;
@@ -87,25 +88,30 @@ BEGIN
   -- Translate AXI4 to MM
   -------------------------------------------
   -- AXI4 Lite to MM
-  i_mm_out_copi <= func_axi4_lite_to_mm_copi(axi4_in_copi);
-  axi4_in_cipo  <= func_axi4_lite_from_mm_cipo(mm_out_cipo, q_bvalid);
+  mm_out_copi  <= func_axi4_lite_to_mm_copi(axi4_in_copi);
+  axi4_in_cipo <= func_axi4_lite_from_mm_cipo(mm_out_cipo, q_bvalid);
 
   -- Generate bvalid
   q_bvalid <= d_bvalid WHEN rising_edge(in_clk); 
-
-  p_bvalid : PROCESS(i_rst, q_bvalid, mm_out_cipo, i_mm_out_copi, axi4_in_copi)
+  
+  -- According to AXI4 interface definition, BVALID must be asserted after 
+  -- WVALID and WREADY are both asserted. Then BVALID is acknowledged by 
+  -- BREADY. Note that in this process we use WREADY = NOT mm_out_cipo.wairequest.
+  p_bvalid : PROCESS(i_rst, q_bvalid, mm_out_cipo, axi4_in_copi)
   BEGIN
     d_bvalid <= q_bvalid;
-    IF mm_out_cipo.waitrequest = '0' AND i_mm_out_copi.wr = '1' THEN
+
+    -- WREADY = '1' AND WVALID = '1', so assert BVALID.
+    IF mm_out_cipo.waitrequest = '0' AND axi4_in_copi.wvalid = '1' THEN 
       d_bvalid <= '1';
-    ELSIF axi4_in_copi.bready = '1' THEN
+
+    -- BVALID is acknowledged by BREADY, so deassert BVALID.
+    ELSIF axi4_in_copi.bready = '1' THEN 
       d_bvalid <= '0';
     END IF;
     IF i_rst = '1' THEN
      d_bvalid <= '0';
     END IF;
   END PROCESS;
-
-  mm_out_copi   <= i_mm_out_copi;
 END str;
 
diff --git a/libraries/base/axi4/tb/vhdl/tb_axi4_lite_mm_bridge.vhd b/libraries/base/axi4/tb/vhdl/tb_axi4_lite_mm_bridge.vhd
index 7ae57fa96c..a85d556688 100644
--- a/libraries/base/axi4/tb/vhdl/tb_axi4_lite_mm_bridge.vhd
+++ b/libraries/base/axi4/tb/vhdl/tb_axi4_lite_mm_bridge.vhd
@@ -23,6 +23,23 @@
 -- Purpose:  
 --   TB for testing axi4_lite_mm_bridge using common_ram_r_w
 -- Description:
+--   Stimuli/verification is done by a MM interface generated using proc_mem_mm_bus 
+--   procedures. The DUT translates the MM stimuli to AXI4-Lite which is looped
+--   back into the DUT to translate it again to MM. The resulting MM is connected
+--   to RAM to provide some address space for verification. Additionaly the 
+--   mm_waitrequest_model is used to provide backpressure to model a peripheral
+--   with flow control.
+--
+--               +---------------------+    +------------+   +-------+
+-- Stimuli/Verify|         DUT         |    | Waitrequest|   |       |
+--       <------->mm_in          mm_out<---->   Model    <--->  RAM  |
+--               | copi/cipo  copi/cipo|    |            |   |       |
+--               |                     |    +------------+   +-------+
+--            +-->axi_in        axi_out<--+
+--            |  | copi/cipo  copi/cipo|  |
+--            |  +---------------------+  |
+--            |                           |
+--            +---------------------------+
 -------------------------------------------------------------------------------
 
 LIBRARY IEEE, common_lib, mm_lib;
@@ -46,15 +63,14 @@ ARCHITECTURE tb OF tb_axi4_lite_mm_bridge IS
   CONSTANT c_reset_len       : NATURAL := 4;
 
   CONSTANT c_mm_usr_ram      : t_c_mem :=   (latency    => 1,
-                                             adr_w	    => 5,
-                                             dat_w	    => 8,
-                                             nof_dat	  => 32,
+                                             adr_w      => 5,
+                                             dat_w      => 8,
+                                             nof_dat    => 32,
                                              init_sl    => '0');
   CONSTANT c_offset          : NATURAL := 57; --Some value to offset the counter data written to ram.
 
   SIGNAL mm_rst              : STD_LOGIC;
   SIGNAL mm_clk              : STD_LOGIC := '0';
-  SIGNAL sim_finished        : STD_LOGIC := '0';
   SIGNAL tb_end              : STD_LOGIC := '0';
 
   SIGNAL mm_in_copi          : t_mem_copi := c_mem_copi_rst;
@@ -69,9 +85,10 @@ ARCHITECTURE tb OF tb_axi4_lite_mm_bridge IS
 
 BEGIN
 
-  mm_clk <= NOT mm_clk OR sim_finished  AFTER c_mm_clk_period/2;
-  mm_rst <= '1', '0'    AFTER c_mm_clk_period*c_reset_len;
+  mm_clk <= NOT mm_clk OR tb_end AFTER c_mm_clk_period/2;
+  mm_rst <= '1', '0' AFTER c_mm_clk_period*c_reset_len;
 
+  -- DUT
   u_axi4_lite_mm_bridge : ENTITY work.axi4_lite_mm_bridge
   PORT MAP (
      in_clk        => mm_clk,
@@ -87,6 +104,7 @@ BEGIN
      axi4_out_cipo => axi_cipo
   );
 
+  -- Provide waitrequest stimuli to model a peripheral with MM flow control.
   u_waitrequest_model : ENTITY mm_lib.mm_waitrequest_model
   GENERIC MAP (
     g_waitrequest => TRUE,
@@ -99,7 +117,7 @@ BEGIN
     slave_mosi => ram_copi,
     slave_miso => ram_cipo
   );
-
+  -- Use common ram as a MM peripherpal.
   u_ram : ENTITY common_lib.common_ram_r_w
   GENERIC MAP (
     g_ram     => c_mm_usr_ram
@@ -117,8 +135,8 @@ BEGIN
     rd_val    => ram_cipo.rdval
   );
 
-  -- Testbench writes a number of words to memory and then reads them back trough the AXI interface.
-  -- It uses the axi_lite_transaction to write, read and verify the data.
+  -- Testbench writes/reads a number of words to/from memory through the axi4_lite_mm_bridge.
+  -- This tests the interface MM <-> AXI4-Lite <-> MM.
   tb : PROCESS
   BEGIN
     proc_common_wait_until_low(mm_clk, mm_rst);
@@ -138,10 +156,7 @@ BEGIN
           & " but received " & int_to_str(TO_UINT(mm_in_cipo.rddata(c_mm_usr_ram.dat_w-1 DOWNTO 0))) SEVERITY ERROR;
     END LOOP;
   
-    sim_finished <= '1';
     tb_end <= '1';
-    WAIT FOR 1 us;
-    REPORT "Finished Simulation" SEVERITY FAILURE;
     WAIT;
   END PROCESS tb;
 END tb;
-- 
GitLab