code review harris

sim/sim-wally-batch

@@ -1 +1 @@
-vsim -c -do "do wally-batch.do rv64gc wally64priv"
+vsim -c -do "do wally-batch.do rv64gc wally64periph"

src/uncore/spi_apb.sv

@@ -26,8 +26,16 @@
 ////////////////////////////////////////////////////////////////////////////////////////////////
 
 // Current limitations: Flash read sequencer mode not implemented, dual and quad modes untestable with current test plan.
-// Hardware interlock cs_mode hold interaction 
+// Hardware interlock change to busy signal
 // relook at fifo empty full logic; might be that watermark level is low when full
+// ChipSelectInternal boolean logic simplification (Harris suggestion)
+// document timing on loopback testing
+// change SCLKenable comparison to equals if possible
+// Explain how sck divider gets to correct value
+// HoldModeDeassert verilater lint
+// Comment on FIFOs: readenable, watermark calculations
+/* high level explanation of architecture
+*/
 
 
 
@@ -141,6 +149,8 @@ module spi_apb import cvw::*; #(parameter cvw_t P) (
     logic TransmitInactive;
     logic SCLKenableEarly;
     logic ReceiveShiftFullDelayPCLK;
+    logic [2:0] LeftShiftAmount;
+    logic [7:0] ASR; // AlignedReceiveShiftReg
 
     
     assign Entry = {PADDR[7:2],2'b00};  // 32-bit word-aligned accesses
@@ -253,7 +263,7 @@ module spi_apb import cvw::*; #(parameter cvw_t P) (
 
     assign SCLKenable = (DivCounter >= {1'b0,SckDiv});
     assign SCLKenableEarly = ((DivCounter + 13'b1) >= {1'b0, SckDiv});
-
+    // 
     always_ff @(posedge PCLK, negedge PRESETn)
         if (~PRESETn) DivCounter <= #1 0;
         else if (SCLKenable) DivCounter <= 0;
@@ -481,6 +491,8 @@ module spi_apb import cvw::*; #(parameter cvw_t P) (
             end
         end
     always_comb
+
+    //Transmit shift register
     if (Active | Delay0Compare | ~TransmitShiftEmpty) begin
             case(Format[1:0])
                 2'b00: SPIOut = {3'b0,TransmitShiftReg[7]}; 
@@ -492,6 +504,8 @@ module spi_apb import cvw::*; #(parameter cvw_t P) (
         end else SPIOut = 4'b0;
     
     assign shiftin = P.SPI_LOOPBACK_TEST ? SPIOut : SPIIn;
+
+    // Receive shift register
     always_ff @(posedge PCLK, negedge PRESETn)
         if(~PRESETn)  ReceiveShiftReg <= 8'b0;
         else if (SampleEdge & SCLKenable) begin
@@ -506,48 +520,22 @@ module spi_apb import cvw::*; #(parameter cvw_t P) (
             end
         end
 
-    assign ReceiveShiftRegInvert = (Format[2]) ? {ReceiveShiftReg[0], ReceiveShiftReg[1], ReceiveShiftReg[2], ReceiveShiftReg[3], ReceiveShiftReg[4], ReceiveShiftReg[5], ReceiveShiftReg[6], ReceiveShiftReg[7]} : ReceiveShiftReg[7:0];
+    // Aligns received data and reverses if little-endian
-
+    assign LeftShiftAmount = 8 - Format[7:4];
-    always_comb
+    assign ASR = ReceiveShiftReg << LeftShiftAmount;
-        if (Format[2]) begin 
+    assign ReceiveShiftRegEndian = Format[2] ? {ASR[0], ASR[1], ASR[2], ASR[3], ASR[4], ASR[5], ASR[6], ASR[7]} : ASR[7:0];
-            case(Format[7:4])
-                4'b0001: ReceiveShiftRegEndian = {7'b0, ReceiveShiftRegInvert[7]};
-                4'b0010: ReceiveShiftRegEndian = {6'b0, ReceiveShiftRegInvert[7:6]};
-                4'b0011: ReceiveShiftRegEndian = {5'b0, ReceiveShiftRegInvert[7:5]};
-                4'b0100: ReceiveShiftRegEndian = {4'b0, ReceiveShiftRegInvert[7:4]};
-                4'b0101: ReceiveShiftRegEndian = {3'b0, ReceiveShiftRegInvert[7:3]};
-                4'b0110: ReceiveShiftRegEndian = {2'b0, ReceiveShiftRegInvert[7:2]};
-                4'b0111: ReceiveShiftRegEndian = {1'b0, ReceiveShiftRegInvert[7:1]};
-                4'b1000: ReceiveShiftRegEndian = ReceiveShiftRegInvert;
-                default: ReceiveShiftRegEndian = ReceiveShiftRegInvert;
-            endcase
-        end else begin
-            case(Format[7:4])
-                4'b0001: ReceiveShiftRegEndian = {ReceiveShiftRegInvert[0], 7'b0};
-                4'b0010: ReceiveShiftRegEndian = {ReceiveShiftRegInvert[1:0], 6'b0};
-                4'b0011: ReceiveShiftRegEndian = {ReceiveShiftRegInvert[2:0], 5'b0};
-                4'b0100: ReceiveShiftRegEndian = {ReceiveShiftRegInvert[3:0], 4'b0};
-                4'b0101: ReceiveShiftRegEndian = {ReceiveShiftRegInvert[4:0], 3'b0};
-                4'b0110: ReceiveShiftRegEndian = {ReceiveShiftRegInvert[5:0], 2'b0};
-                4'b0111: ReceiveShiftRegEndian = {ReceiveShiftRegInvert[6:0], 1'b0};
-                4'b1000: ReceiveShiftRegEndian = ReceiveShiftRegInvert;
-                default: ReceiveShiftRegEndian = ReceiveShiftRegInvert;
-            endcase
-        end
 
+    // Interrupt logic: raise interrupt if any enabled interrupts are pending
     assign SPIIntr = |(InterruptPending & InterruptEnable);
 
+    // Chip select logic
     always_comb
         case(ChipSelectID[1:0])
             2'b00: ChipSelectAuto = {ChipSelectDef[3], ChipSelectDef[2], ChipSelectDef[1], ChipSelectInternal[0]};
-
             2'b01: ChipSelectAuto = {ChipSelectDef[3],ChipSelectDef[2], ChipSelectInternal[1], ChipSelectDef[0]};
-
             2'b10: ChipSelectAuto = {ChipSelectDef[3],ChipSelectInternal[2], ChipSelectDef[1], ChipSelectDef[0]};
-
             2'b11: ChipSelectAuto = {ChipSelectInternal[3],ChipSelectDef[2], ChipSelectDef[1], ChipSelectDef[0]};
         endcase
-
     assign SPICS = ChipSelectMode[0] ? ChipSelectDef : ChipSelectAuto;
 
 
@@ -582,8 +570,6 @@ module TransmitSynchFIFO #(parameter M =3 , N= 8)(
     assign raddr = rptr[M-1:0];
     assign rptrnext = rptr + {3'b0, (rinc & ~rempty)};      
 
-
-    
     always_ff @(posedge PCLK, negedge PRESETn)
         if (~PRESETn) wptr <= 0;
         else wptr <= wptrnext;