processed review comments

applications/lofar2/libraries/sdp/src/vhdl/sdp_crosslets_subband_select.vhd

@@ -163,7 +163,9 @@ BEGIN
 
   p_comb_crosslets_control : PROCESS(r, start_trigger, crosslets_info_reg, in_sosi_arr, col_select_miso)
     VARIABLE v : t_crosslets_control_reg;
-    VARIABLE v_offsets : t_natural_arr(g_N_crosslets-1 DOWNTO 0); -- Use extra variable to simplify col_select_mosi address selection
+    -- Use extra variable to simplify col_select_mosi address selection.
+    -- Also using v_offsets instead of v.offsets to clearly indicate we do not only use this variable on the left side but also on the right side of assignments.
+    VARIABLE v_offsets : t_natural_arr(g_N_crosslets-1 DOWNTO 0); 
   BEGIN
     v := r;
     v.col_select_mosi := c_mem_mosi_rst;
@@ -171,28 +173,34 @@ BEGIN
 
     -- start/restart
     IF start_trigger = '1' THEN
-      v.started       := '1';
+      v.started       := '1'; -- Once started r.started remains active. This is to prevent read/write actions before the initial start_trigger.
       v.offset_index  := 0;
       v.row_index     := 0;
       v.col_index     := 0;
-      v.sync_detected := '0';
-
+      v.sync_detected := '0'; -- set sync_detected to 0 in the case that a sync has been detected before the initial start_trigger.
+      -- start_trigger is active on the eop so we can immediatly reset the offsets/step such that they are used in the next packet.
+      -- It is up to the user to schedule the start trigger on a BSN that coincides with a sync interval if that is desired.
       v.step := TO_UINT(crosslets_info_reg(c_sdp_crosslets_info_reg_w-1 DOWNTO c_sdp_crosslets_info_reg_w - c_sdp_crosslets_index_w));
       FOR I IN 0 TO g_N_crosslets-1 LOOP
         v_offsets(I) := TO_UINT(crosslets_info_reg((I+1)*c_sdp_crosslets_index_w-1 DOWNTO I*c_sdp_crosslets_index_w));
       END LOOP;
     END IF;
 
-    IF in_sosi_arr(0).sync = '1' AND start_trigger = '0' THEN
+    IF in_sosi_arr(0).sync = '1' THEN 
       v.sync_detected := '1';
     END IF;
 
-    IF r.started = '1' THEN 
+    IF r.started = '1' THEN -- Once started r.started remains active.
       -- add step to offsets
-      IF in_sosi_arr(0).eop = '1' AND r.sync_detected = '1' THEN -- change offsets 1 packet after the sync due to the buffered packet in reorder_col_wide_select
+      IF in_sosi_arr(0).eop = '1' AND r.sync_detected = '1' THEN -- using r.sync_detected to change offsets 1 packet after the sync due to the buffered packet in reorder_col_wide_select
         v.sync_detected := '0';
         FOR I IN 0 TO g_N_crosslets-1 LOOP
+          IF start_trigger = '1' THEN
+            -- Using the crosslets_info_reg directly instead of r.step when start trigger coincides with the current eop as step can have a new value. 
             v_offsets(I) := r.offsets(I) + TO_UINT(crosslets_info_reg(c_sdp_crosslets_info_reg_w-1 DOWNTO c_sdp_crosslets_info_reg_w - c_sdp_crosslets_index_w));
+          ELSE
+            v_offsets(I) := r.offsets(I) + r.step;
+          END IF;
         END LOOP;
       END IF;
 
@@ -238,11 +246,6 @@ BEGIN
     out_dat => row_select_slv
   );
 
-  active_crosslets_info(c_sdp_crosslets_info_reg_w-1 DOWNTO c_sdp_crosslets_info_reg_w - c_sdp_crosslets_index_w) <= TO_UVEC(r.step, c_sdp_crosslets_index_w);
-  gen_crosslets_info : FOR I IN 0 TO g_N_crosslets-1 GENERATE
-    active_crosslets_info((I+1)*c_sdp_crosslets_index_w-1 DOWNTO I*c_sdp_crosslets_index_w) <= TO_UVEC(r.offsets(I), c_sdp_crosslets_index_w);
-  END GENERATE;
-
   ---------------------------------------------------------------
   -- Crosslet Select 
   ---------------------------------------------------------------
@@ -292,6 +295,10 @@ BEGIN
   ---------------------------------------------------------------
   -- Out Crosslet info pipeline
   ---------------------------------------------------------------
+  active_crosslets_info(c_sdp_crosslets_info_reg_w-1 DOWNTO c_sdp_crosslets_info_reg_w - c_sdp_crosslets_index_w) <= TO_UVEC(r.step, c_sdp_crosslets_index_w);
+  gen_crosslets_info : FOR I IN 0 TO g_N_crosslets-1 GENERATE
+    active_crosslets_info((I+1)*c_sdp_crosslets_index_w-1 DOWNTO I*c_sdp_crosslets_index_w) <= TO_UVEC(r.offsets(I), c_sdp_crosslets_index_w);
+  END GENERATE;
   -- pipeline for alignment with sync
   u_common_pipeline : ENTITY common_lib.common_pipeline
   GENERIC MAP(

applications/lofar2/libraries/sdp/src/vhdl/sdp_statistics_offload.vhd

@@ -46,7 +46,7 @@ ENTITY sdp_statistics_offload IS
     g_statistics_type : STRING  := "SST";
     g_offload_time    : NATURAL := c_sdp_offload_time;
     g_beamset_id      : NATURAL := 0;
-    g_P_sq            : NATURAL := c_sdp_P_sq
+    g_P_sq            : NATURAL := c_sdp_P_sq -- use generic to support P_sq = 1 for one node and P_sq = c_sdp_P_sq for multiple nodes (with ring)
   );
   PORT (
     -- Clocks and reset