Merge pull request 'comment_clean_up' (#4) from comment_clean_up into master

Reviewed-on: #4

design/audio/audio_buffer.sv

@@ -6,6 +6,9 @@
  * @date: 6/12/2025
  *
  * */
+
+//define used here because verilator needs it but vivado is smart enough
+//not to. 
 `ifdef VERILATOR
     `include "sdvd_defs.sv"
 `endif
@@ -75,8 +78,6 @@ always_ff @(posedge clk) begin
             // This will overflow to the correct value always
             address <= address + speed;
             enb <= 1;
-            // NOTE: I really don't know a good way to generate the load
-            // signal. It maybe could be an inverted 48khz clock?
             if (delay == 0) begin
                 sample[15-:SIZE] <= doutb;
             end
@@ -89,59 +90,56 @@ always_ff @(posedge clk) begin
     end
 end
 
-//xpm_memory_sdpram #(
-//    .WRITE_DATA_WIDTH_A(16)
-//) buffer ();
+//vivado block ram xpm macro
 xpm_memory_sdpram #(
-  .ADDR_WIDTH_A(11),               // DECIMAL
-  .ADDR_WIDTH_B(12-(SIZE/8)),       // DECIMAL
-  .AUTO_SLEEP_TIME(0),            // DECIMAL
-  .BYTE_WRITE_WIDTH_A(8),        // DECIMAL
-  .CASCADE_HEIGHT(0),             // DECIMAL
-  .CLOCKING_MODE("independent_clock"), // String
-  .ECC_BIT_RANGE("7:0"),          // String
-  .ECC_MODE("no_ecc"),            // String
-  .ECC_TYPE("none"),              // String
-  .IGNORE_INIT_SYNTH(0),          // DECIMAL
-  .MEMORY_INIT_FILE("none"),      // String
-  .MEMORY_INIT_PARAM("0"),        // String
-  .MEMORY_OPTIMIZATION("true"),   // String
-  .MEMORY_PRIMITIVE("auto"),      // String
-  .MEMORY_SIZE(16*1024),             // DECIMAL
-  .MESSAGE_CONTROL(0),            // DECIMAL
-  .RAM_DECOMP("auto"),            // String
-  .READ_DATA_WIDTH_B(SIZE),         // DECIMAL
-  .READ_LATENCY_B(1),             // DECIMAL
-  .READ_RESET_VALUE_B("0"),       // String
-  .RST_MODE_A("SYNC"),            // String
-  .RST_MODE_B("SYNC"),            // String
-  .SIM_ASSERT_CHK(0),             // DECIMAL; 0=disable simulation messages, 1=enable simulation messages
-  .USE_EMBEDDED_CONSTRAINT(0),    // DECIMAL
-  .USE_MEM_INIT(1),               // DECIMAL
-  .USE_MEM_INIT_MMI(0),           // DECIMAL
-  .WAKEUP_TIME("disable_sleep"),  // String
-  .WRITE_DATA_WIDTH_A(8),        // DECIMAL
-  .WRITE_MODE_B("no_change"),     // String
-  .WRITE_PROTECT(1)               // DECIMAL
+  .ADDR_WIDTH_A(11),                    // DECIMAL
+  .ADDR_WIDTH_B(12-(SIZE/8)),           // DECIMAL
+  .AUTO_SLEEP_TIME(0),                  // DECIMAL
+  .BYTE_WRITE_WIDTH_A(8),               // DECIMAL
+  .CASCADE_HEIGHT(0),                   // DECIMAL
+  .CLOCKING_MODE("independent_clock"),  // String
+  .ECC_BIT_RANGE("7:0"),                // String
+  .ECC_MODE("no_ecc"),                  // String
+  .ECC_TYPE("none"),                    // String
+  .IGNORE_INIT_SYNTH(0),                // DECIMAL
+  .MEMORY_INIT_FILE("none"),            // String
+  .MEMORY_INIT_PARAM("0"),              // String
+  .MEMORY_OPTIMIZATION("true"),         // String
+  .MEMORY_PRIMITIVE("auto"),            // String
+  .MEMORY_SIZE(16*1024),                // DECIMAL
+  .MESSAGE_CONTROL(0),                  // DECIMAL
+  .RAM_DECOMP("auto"),                  // String
+  .READ_DATA_WIDTH_B(SIZE),             // DECIMAL
+  .READ_LATENCY_B(1),                   // DECIMAL
+  .READ_RESET_VALUE_B("0"),             // String
+  .RST_MODE_A("SYNC"),                  // String
+  .RST_MODE_B("SYNC"),                  // String
+  .SIM_ASSERT_CHK(0),                   // DECIMAL; 0=disable simulation messages, 1=enable simulation messages
+  .USE_EMBEDDED_CONSTRAINT(0),          // DECIMAL
+  .USE_MEM_INIT(1),                     // DECIMAL
+  .USE_MEM_INIT_MMI(0),                 // DECIMAL
+  .WAKEUP_TIME("disable_sleep"),        // String
+  .WRITE_DATA_WIDTH_A(8),               // DECIMAL
+  .WRITE_MODE_B("no_change"),           // String
+  .WRITE_PROTECT(1)                     // DECIMAL
 )
 buffer (
   .doutb(doutb),          // READ_DATA_WIDTH_B-bit output: Data output for port B read operations.
-  .addra(driver.addra),          // ADDR_WIDTH_A-bit input: Address for port A write operations.
+  .addra(driver.addra),   // ADDR_WIDTH_A-bit input: Address for port A write operations.
   .addrb(address),        // ADDR_WIDTH_B-bit input: Address for port B read operations.
-  .clka(driver.clka),            // 1-bit input: Clock signal for port A. Also clocks port B when
+  .clka(driver.clka),     // 1-bit input: Clock signal for port A. Also clocks port B when
                           // parameter CLOCKING_MODE is "common_clock".
 
   .clkb(clk),             // 1-bit input: Clock signal for port B when parameter CLOCKING_MODE is
                           // "independent_clock". Unused when parameter CLOCKING_MODE is
                           // "common_clock".
-  .dina(driver.dina),            // WRITE_DATA_WIDTH_A-bit input: Data input for port A write operations.
-  .ena(driver.ena),              // 1-bit input: Memory enable signal for port A. Must be high on clock
+  .dina(driver.dina),     // WRITE_DATA_WIDTH_A-bit input: Data input for port A write operations.
+  .ena(driver.ena),       // 1-bit input: Memory enable signal for port A. Must be high on clock
                           // cycles when write operations are initiated. Pipelined internally.
 
   .enb(enb),              // 1-bit input: Memory enable signal for port B. Must be high on clock
                           // cycles when read operations are initiated. Pipelined internally.
-//active high reset
-  .rstb(reset),            // 1-bit input: Reset signal for the final port B output register stage.
+  .rstb(reset),           // 1-bit input: Reset signal for the final port B output register stage.
                           // Synchronously resets output port doutb to the value specified by
                           // parameter READ_RESET_VALUE_B.
   .wea(driver.ena),       // WRITE_DATA_WIDTH_A/BYTE_WRITE_WIDTH_A-bit input: Write enable vector
@@ -150,11 +148,10 @@ buffer (
                           // writing one byte of dina to address addra. For example, to
                           // synchronously write only bits [15-8] of dina when WRITE_DATA_WIDTH_A
                           // is 32, wea would be 4'b0010.
-  // Extra inputs that I guess I need
-  .sleep(0),
-  .injectsbiterra(0),
+  
+  .sleep(0),              // The XPM macros say these can be removed, but when I did this the
+  .injectsbiterra(0),     // entire block memory got optimized out. These were readded in later
   .injectdbiterra(0),
-  // With a latency of 1 this surely does not matter
   .regceb(enb)
 );
 

design/low_freq_clock_gen.sv

@@ -6,7 +6,7 @@
  * @author: Waylon Cude, Dilanthi Prentice
  * @date: 6/12/2025
  * */
-
+//Define used because verilator needs it but vivado is samrt enough not to
 `ifdef VERILATOR
     `include "sdvd_defs.sv"
 `endif
@@ -19,7 +19,7 @@ module low_freq_clock_gen(
     output logic clk1k, clk10h, seconds_pulse
 );
 
-// Hardcoded to be 1,000,000/4,000
+// Hardcoded to be 1,000,000/4,000 (25,000)
 // Relying on constant maths makes it the wrong size
 localparam num_cycles = 25_000;
 

design/modular_clock_gen.sv

@@ -1,18 +1,21 @@
+/****
+ * modular_clock_gen.sv - parameterizable clock divider.
+ *
+ * @author: Waylon Cude, Dilanthi Prentice
+ * @date: 6/12/2025
+ *
+ * */
 module modular_clock_gen(
     input clk, reset,
     output logic oclk
 );
+//parameter has no default because the user should always set it
 parameter DIVISOR;
 
 logic [$clog2(DIVISOR)-1:0] counter;
 
 logic set;
 
-// clock will be high for about half of the cycle, depending on integer
-// rounding
-// OOPS this makes it combinational
-//assign oclk = counter < (DIVISOR/2);
-
 always_ff @(posedge clk) begin
     // modular clock has to keep ticking through reset
     // so everything with a synchronous reset actually works

design/nexys_a7_top.sv

@@ -117,12 +117,15 @@ playback_controller playbackController (clk_10hz,reset,BTNC, BTNR, speed);
     // We don't use more than one dat line
     logic clk_100khz;
     logic clk_25mhz;
-    // Actually 200khz now
+    // We can clock this at around 200khz before things start breaking
+    // Even at 200khz sometimes things break
     modular_clock_gen #(500) slowSdClock(CLK100MHZ, reset, clk_100khz);
     // Try clocking this slower than max speed
-    // To see if that makes it actually work ...
-    // If these are different speeds bad things happen :) oops
+    // To see if that makes it actually work 
+    //
+    // NOTE: It did not make it work
     modular_clock_gen #(400) fastSdClock(CLK100MHZ, reset, clk_25mhz);
+
     sd_controller realSdPlayer(
         clk_100khz,
         clk_25mhz,

design/playback_controller.sv

@@ -9,6 +9,8 @@
  *
  * */
 
+//VERILATOR define used here because verilator needs it to find the package
+//while vivado is smart enough not to.
 `ifdef VERILATOR
     `include "sdvd_defs.sv"
 `endif
@@ -35,7 +37,6 @@ state_t current, next;
 
 wire play_pulse,ff_pulse;
 
-// NOTE: These might need to be hooked to an even lower clock? Not sure
 debouncer playDebouncer (clk,reset,play,play_pulse);
 debouncer ffDebouncer (clk,reset,ff,ff_pulse);
 

design/sd/crc_gen.sv

@@ -1,5 +1,11 @@
-// parameterizable sequential crc generator
-// This probably could be combinational logic too, though it would be way slow
+/***
+ * crc_gen.sv - paramerizable sequential crc generator. This could probably be
+ *              combinational logic too (but it would be way slower)
+ *
+ * @author: Waylon Cude, Dilanthi Prentice
+ * @date: 6/12/2025
+ *
+ * */
 module crc_gen #(
     parameter CRCBITS=7,
     parameter COMMANDLEN=40,

design/sd/read_command.sv

@@ -1,3 +1,11 @@
+/****
+ * read_command.sv - reads the sd command line and listens for sd command
+ *                   responses.
+ * 
+ * @author: Waylon Cude, Dilanthi Prentice
+ * @date: 6/12/2025
+ *
+ * **/
 module read_command(
     input logic clk,
     input logic reset,
@@ -19,8 +27,8 @@ logic received_reg;
 
 logic [2:0] response_type_reg;
 
-// oops this was set a cycle before the data was actually ready
-// and if it's just a reg it holds for too long
+// recevied was set a cycle before the data was actually ready
+// and if it's just a register it holds for too long
 // use both for maximum reactivity
 assign received = received_reg && !listen;
 

design/sd/read_data.sv

@@ -1,6 +1,12 @@
-// Whenever we receive a data block, start spitting bytes out into the audio
-// buffer bram. This could always be clocked at the 25MHz default speed clock,
-// or it could switch over like the writer has to
+/***
+ * read_data.sv - whenever we receive a data block, start spitting bytes out
+ *                into the audio buffer block ram. This could be clocked at
+ *                25MHz default speed clock, or it could swtich over like the
+ *                writer has to.
+ * @author: Waylon Cude, Dilanthi Prentice
+ * @date: 6/12/2025
+ *
+ ***/
 module read_data(
     input clk,
     input reset,

design/sd/send_command.sv

@@ -1,3 +1,11 @@
+/***
+ * send_command.sv - sends sd card commands. It takes in a command and an
+ *                   argument and will figure out the necessary crc. 
+ *
+ * @author: Waylon Cude, Dilanthi Prentice
+ * @date: 6/12/2025
+ *
+ * */
 module send_command(
     input clk, 
     // The crc should be clocked way faster than the sender

design/segment_display/sixty_display.sv

@@ -1,5 +1,7 @@
 /***
-* sixty_display.sv - converts a five bit seconds counter to its seven segement display equivalent.
+* sixty_display.sv - converts a binary digit from zero to fifty-nine to its
+*                    ones and tens digits and then gets the seven segment
+*                    display equivalents of them.
 *
 * @author: Dilanthi Prentice, Waylon Cude
 * @date: 6/12/25
@@ -16,14 +18,14 @@ module sixty_display
 logic [4:0] ones_digit; 
 logic [4:0] tens_digit;
 
+//convert digit to ones and tens places
 always_comb
 begin
     ones_digit = number % 10;
     tens_digit = number / 10;
 end
 
-//instantiate the display_converter to convert the counter
-//to a seven segment display number
+//convert both ones and tens place digits to their seven segement equivalent
 display_converter ones (ones_digit, display_ones);
 display_converter tens (tens_digit, display_tens);
 

doc/bugs.md

@@ -38,7 +38,7 @@
   entirely. Detected in testbench
 - off-by-one error detected in the counter
 
-## read_data
+### read_data
 - audio buffer address to write to was never changing, caught in simulation
 - some off-by-one errors in the byte shifting were found and corrected