spirom: restructure handshake signals

The handshake signals seems to have been unreliable, resuling in
invalid SDRAM contents. Restructure and fix. Create an explicit RAM
write control flag. Do not start a RAM transaction if the data size is
0.

Minor changes to the romcopy routines for reliability.

Fix to the linker script to error out on unknown sections.

Disable the startup delay by default.

fpga/spirom.sv

@@ -39,18 +39,15 @@ module spirom (
    reg [31:0] romcmd;
    reg [23:2] datalen;
    reg [2:0]  cmdlen;
-   reg	      go_zero;
    reg	      go_spi;
+   reg	      go_ram;
+   reg	      is_ram;
    reg	      spi_dual;
    reg	      spi_more;		// Do not raise CS# after command done
    reg	      ram_done;
    reg	      ram_done_q;
-   reg [1:0]  done_q;
-   reg	      busy;
    reg [31:0] spi_in_shr;	// Input shift register for one-bit input
-
-   reg        spi_active;
-   reg	      spi_active_q;
+   wire       spi_active_s;
 
    always @(negedge rst_n or posedge sys_clk)
      if (~rst_n)
@@ -59,23 +56,25 @@ module spirom (
 	  romcmd       <= 32'b0;
 	  datalen      <= 22'b0;
 	  cmdlen       <= 3'bx;
-	  go_zero      <= 1'b0;
 	  go_spi       <= 1'b0;
+	  go_ram       <= 1'b0;
+	  is_ram       <= 1'b0;
 	  ram_done_q   <= 1'b1;
-	  done_q       <= 2'b11;
-	  busy         <= 1'b0;
-	  spi_active_q <= 1'b0;
 	  irq          <= 1'b1;
 	  spi_dual     <= 1'b0;
 	  spi_more     <= 1'b0;
        end
      else
        begin
-	  ram_done_q   <= ram_done;
-	  spi_active_q <= spi_active;
-	  done_q   <= { done_q[0], ram_done_q & ~spi_active_q };
+	  ram_done_q <= ram_done;
+
+	  if (~ram_done_q)
+	    go_ram <= 1'b0;
+	  if (spi_active_s)
+	    go_spi <= 1'b0;
 
-	  irq <= ~(busy | spi_active_q);
+	  if (ram_done_q & ~go_ram & ~spi_active_s & ~go_spi)
+	    irq     <= 1'b1;
 
 	  if (cpu_valid & cpu_wstrb[0])
 	    begin
@@ -88,33 +87,27 @@ module spirom (
 		    romcmd   <= cpu_wdata[31:0];
 		 end
 		 2'b10: begin
-		    datalen  <= cpu_wdata[23:2];
-		    cmdlen   <= cpu_wdata[26:24];
-		    go_spi   <= cpu_wdata[26:24] != 3'd0;
-		    go_zero  <= cpu_wdata[26:24] == 3'd0;
-		    spi_dual <= cpu_wdata[27];
-		    spi_more <= cpu_wdata[28];
-		    busy     <= 1'b1;
-		    irq      <= 1'b0;
+		    datalen     <= cpu_wdata[23:2];
+		    cmdlen      <= cpu_wdata[26:24];
+		    go_spi      <= cpu_wdata[26:24] != 3'd0;
+		    spi_dual    <= cpu_wdata[27];
+		    spi_more    <= cpu_wdata[28];
+		    is_ram      <= cpu_wdata[29];
+		    go_ram      <= cpu_wdata[29] & |cpu_wdata[23:2];
 		 end
 		 default: begin
 		    // Do nothing
 		 end
 	       endcase // case (cpu_addr)
 	    end // if (cpu_valid & cpu_wstrb[0])
-	  else if (done_q == 2'b01)
-	    begin
-	       go_zero <= 1'b0;
-	       go_spi  <= 1'b0;
-	       busy    <= 1'b0;
-	    end
        end // else: !if(~rst_n)
 
    always_comb
      case (cpu_addr)
        3'b000: cpu_rdata = { 7'b0, ramstart, 2'b0 };
        3'b001: cpu_rdata = romcmd;
-       3'b010: cpu_rdata = { 3'b0, spi_more, spi_dual, cmdlen, datalen,  2'b0 };
+       3'b010: cpu_rdata = { 2'b0, is_ram, spi_more, spi_dual,
+			     cmdlen, datalen,  2'b0 };
        3'b011: cpu_rdata = { 31'b0, irq };
        3'b100: cpu_rdata = spi_in_shr;
        default: cpu_rdata = 32'bx;
@@ -164,7 +157,6 @@ module spirom (
    reg [24:1] waddr_q;
    reg [23:1] ram_data_ctr;
    reg	      wacc_q;
-   reg [1:0]  go_ram_q;
 
    assign waddr = waddr_q;
 
@@ -172,48 +164,45 @@ module spirom (
      if (~rst_n)
        begin
 	  waddr_q      <= 24'bx;
-	  ram_data_ctr <= 23'bx;
+	  ram_data_ctr <= 23'b0;
 	  wacc_q       <= 1'b0;
-	  ram_done     <= 1'b1;
-	  go_ram_q     <= 2'b00;
 	  wrq          <= 2'b00;
+	  ram_done     <= 1'b1;
        end
      else
        begin
-	  wrq <= 2'b00;
-
-	  if (ram_done)
+	  if (|ram_data_ctr)
 	    begin
-	       wrq <= 2'b00;
-	    end
-	  else if (from_spi)
-	    begin
-	       // Reading from SPI ROM
-	       wrq[0] <= rdusedw >=  9'd4; // 4*2 =  8 bytes min available
-	       wrq[1] <= rdusedw >=  9'd8; // 8*2 = 16 bytes min available
-	    end
-	  else
-	    begin
-	       // Zeroing memory
-	       wrq[0] <= |ram_data_ctr[23:3];
-	       wrq[1] <= |ram_data_ctr[23:4];
-	    end
+	       ram_done <= 1'b0;
 
-	  wacc_q    <= wacc;
-	  go_ram_q  <= { go_ram_q[0], go_spi|go_zero };
+	       if (from_spi)
+		 begin
+		    // Reading from SPI ROM
+		    wrq[0] <= rdusedw >=  9'd4; // 4*2 =  8 bytes min available
+		    wrq[1] <= rdusedw >=  9'd8; // 8*2 = 16 bytes min available
+		 end
+	       else
+		 begin
+		    // Zeroing memory
+		    wrq[0] <= |ram_data_ctr[23:3];
+		    wrq[1] <= |ram_data_ctr[23:4];
+		 end
 
-	  if (go_ram_q == 2'b01)
-	    begin
-	       waddr_q      <= { ramstart, 1'b0 };
-	       ram_data_ctr <= { datalen,  1'b0 };
-	       ram_done     <= !datalen;
-	       from_spi     <= go_spi;
-	    end
-	  else if (~ram_done)
-	    begin
+	       wacc_q       <= wacc;
 	       waddr_q      <= waddr_q + wacc_q;
 	       ram_data_ctr <= ram_data_ctr - wacc_q;
-	       ram_done     <= !(ram_data_ctr - wacc_q);
+	    end // if (|ram_data_ctr)
+	  else
+	    begin
+	       wrq      <= 2'b00;
+	       ram_done <= 1'b1;
+
+	       if (go_ram)
+		 begin
+		    waddr_q      <= { ramstart, 1'b0 };
+		    ram_data_ctr <= { datalen,  1'b0 };
+		    from_spi     <= |cmdlen;
+		 end
 	    end
        end // else: !if(~rst_n)
 
@@ -226,7 +215,7 @@ module spirom (
    wire        go_spi_s;
    reg	       spi_more_q;
 
-   // Explicit synchronizer for go_spi
+   // Explicit synchronizers for handshake signals
    synchronizer #(.width(1)) go_spi_synchro
      (
       .rst_n ( rst_n ),
@@ -234,6 +223,13 @@ module spirom (
       .d ( go_spi ),
       .q ( go_spi_s )
       );
+   synchronizer #(.width(1)) spi_active_synchro
+     (
+      .rst_n ( rst_n ),
+      .clk ( ram_clk ),
+      .d ( spi_active ),
+      .q ( spi_active_s )
+      );
 
    always @(negedge rst_n or posedge rom_clk)
      if (~rst_n)
@@ -247,30 +243,29 @@ module spirom (
 	  spi_mosi_en  <= 1'b1;
 	  spi_in_q     <= 2'bx;
 	  spi_in_shr   <= 32'b0;
-	  go_spi_q     <= 2'b00;
 	  spi_active   <= 1'b0;
 	  spi_more_q   <= 1'b0;
        end
      else
        begin
-	  go_spi_q     <= { go_spi_q[0], go_spi_s };
 	  spi_in_q     <= spi_io;
 	  spi_in_req   <= 1'b0;
 	  spi_in_req_q <= spi_in_req;
 	  spi_clk_en   <= 1'b0;
 
 	  // Note: datalen <- spi_data_ctr is a 2-cycle multipath
-	  if (go_spi_q == 2'b01)
+	  if (go_spi_s & ~spi_active)
 	    begin
+	       // Starting new transaction
 	       spi_cmd_ctr  <= { cmdlen,  3'b0 };
 	       spi_data_ctr <= { datalen, 4'b0 };
 	       spi_active   <= 1'b1;
 	       spi_cs_n     <= 1'b0;
 	       spi_more_q   <= spi_more;
 	    end
-
-	  if ( ~|{spi_data_ctr, spi_cmd_ctr} )
+	  else if ( ~|{spi_data_ctr, spi_cmd_ctr} )
 	    begin
+	       // Transaction completed
 	       spi_clk_en  <= 1'b0;
 	       spi_mosi_en <= 1'b1;
 	       spi_active  <= 1'b0;

rv32/Makefile

@@ -15,6 +15,7 @@ CFLAGS    = $(CPPFLAGS) -W -Wextra
 SFLAGS    = $(CPPFLAGS) -D__ASSEMBLY__
 LDSCRIPT  = max80.ild
 LDFLAGS   = $(CFLAGS) \
+	    -Wl,-Map=$*.map \
 	    -Wl,--gc-sections \
 	    -Wl,--sort-section=alignment \
 	    -Wl,-T,$(LDSCRIPT) \
@@ -130,7 +131,7 @@ irqtable.h: ../iodevs.conf ../tools/iodevs.pl
 clean:
 	for d in . $(SUBDIRS); do \
 		rm -f $$d/*.o $$d/*.i $$d/*.s $$d/*.elf $$d/*.bin \
-		$$d/.*.d $$d/*.ild; \
+		$$d/.*.d $$d/*.ild $$d/*.map; \
 	done
 	rm -f $(genhdrs) $(gensrcs)
 

rv32/ioregs.h

@@ -70,6 +70,7 @@
 #define ROMCOPY_ZERO_BUFFER	ROMCOPY_SPI_CMDLEN(0)
 #define ROMCOPY_SPI_DUAL	(1 << 27)
 #define ROMCOPY_SPI_MORE	(1 << 28)
+#define ROMCOPY_WRITE_RAM	(1 << 29)
 #define ROMCOPY_STATUS		IODEVRL(ROMCOPY,3)
 #define ROMCOPY_INPUT		IODEVRL(ROMCOPY,4)
 #define ROMCOPY_STATUS_DONE	1

rv32/max80.ld

@@ -69,6 +69,7 @@ SECTIONS
   .debug_str_offsets 0 : { *(.debug_str_offsets) }
   .debug_sup      0 : { *(.debug_sup) }
   .gnu.attributes 0 : { KEEP (*(.gnu.attributes)) }
+  .riscv.attributes 0 : { KEEP(*(.riscv.attributes)) }
   /DISCARD/ : { *(.note.GNU-stack) *(.gnu_debuglink) *(.gnu.lto_*) }
 
 	/*
@@ -96,7 +97,8 @@ SECTIONS
 		*crt0.o(*)
 	}
 
-	PROVIDE (__executable_start = 0);
+	. = 0;
+	PROVIDE (__executable_start = .);
 
 	/*
 	 * Make sure the output binary starts at address 0
@@ -247,11 +249,15 @@ SECTIONS
 	} >DRAM
 
 	.dram.str : ALIGN(4) {
+		__dram_str_start = .;
 		*(.rodata*.str* .dram.rodata*.str*)
+		__dram_str_end = .;
 	} >DRAM
 
 	.dram.rodata : ALIGN(4) {
+		__dram_rodata_start = .;
 		*(.rodata* .dram.rodata*)
+		__dram_rodata_end = .;
 	} >DRAM
 
 	.dram.init_array : ALIGN(4) {
@@ -303,6 +309,11 @@ SECTIONS
 	__dram_end = .;
 
 	/* Catch missing sections */
+	__junk_start = .;
 	.junk : {
+		*(*)
 	}
+	__junk_end = .;
+
+	HIDDEN($assert_no_junk = ASSERT(__junk_end == __junk_start, "unknown sections present"));
 }

rv32/romcopy.c

@@ -2,10 +2,8 @@
 #include "console.h"
 #include "io.h"
 
-extern char __dram_init_start[];
-abssymval(_dram_init_len)
-extern char __dram_bss_start[];
-abssymval(_dram_bss_len)
+extern char __dram_init_start[], __dram_init_end[];
+extern char __dram_bss_start[],  __dram_bss_end[];
 
 enum romcmd {
     ROM_WRITE_ENABLE			= 0x06,
@@ -45,38 +43,51 @@ enum romcmd {
     ROM_FAST_READ_DUAL			= 0x3b
 };
 
+size_t __sbss romcopy_log[12];
+static size_t __sdata *romcopy_log_ptr;
+
 void __hot romcopy_download(void *dst, size_t offset, size_t len)
 {
-    ROMCOPY_RAMADDR = (size_t)dst;
-    ROMCOPY_ROMCMD  = __rom_offset + offset + (ROM_FAST_READ_DUAL << 24);
-    ROMCOPY_DATALEN = len | ROMCOPY_SPI_CMDLEN(5) | ROMCOPY_SPI_DUAL;
+    *romcopy_log_ptr++ = rdtime();
+    *romcopy_log_ptr++ = ROMCOPY_RAMADDR = (size_t)dst;
+    *romcopy_log_ptr++ = ROMCOPY_ROMCMD  =
+	__rom_offset + offset + (ROM_FAST_READ_DUAL << 24);
+    *romcopy_log_ptr++ = ROMCOPY_DATALEN =
+	len | ROMCOPY_SPI_CMDLEN(5) | ROMCOPY_SPI_DUAL | ROMCOPY_WRITE_RAM;
 }
 
 void __hot romcopy_bzero(void *dst, size_t len)
 {
-    ROMCOPY_RAMADDR = (size_t)dst;
-    ROMCOPY_DATALEN = len | ROMCOPY_ZERO_BUFFER;
+    *romcopy_log_ptr++ = rdtime();
+    *romcopy_log_ptr++ = ROMCOPY_RAMADDR = (size_t)dst;
+    *romcopy_log_ptr++ = ROMCOPY_ROMCMD = 0;
+    *romcopy_log_ptr++ = ROMCOPY_DATALEN =
+	len | ROMCOPY_ZERO_BUFFER | ROMCOPY_WRITE_RAM;
 }
 
-uint32_t __bss_hot romcopy_time[2];
+uint32_t __sbss romcopy_time[2];
 IRQHANDLER(romcopy)
 {
     static unsigned int romcopy_state;
+    size_t len;
 
     switch (romcopy_state++) {
     case 0:
+	romcopy_log_ptr = romcopy_log;
+
 	/* Copy DRAM data */
-	romcopy_download(__dram_init_start, 0, _dram_init_len());
+	len = __dram_init_end - __dram_init_start;
+	romcopy_download(__dram_init_start, 0, len);
 	break;
     case 1:
 	/* Zero .dram.bss */
-	romcopy_time[0] = rdtime() - time_zero;
-	romcopy_bzero(__dram_bss_start, _dram_bss_len());
+	len = __dram_bss_end - __dram_bss_start;
+	romcopy_bzero(__dram_bss_start, len);
 	break;
     default:
-	romcopy_time[1] = rdtime() - romcopy_time[0];
+	*romcopy_log_ptr++ = rdtime();
 	mask_irq(ROMCOPY_IRQ);
-	return;
+	break;
     }
 }
 

rv32/system.c

@@ -7,6 +7,7 @@
 
 #define DEBUG     0
 #define MINITESTS 1
+#define DELAY     0
 
 volatile __sbss uint32_t timer_irq_count;
 IRQHANDLER(sysclock)
@@ -99,7 +100,7 @@ void __hot init(void)
     con_flush();
 
     set_leds(5);
-#if 1 // DEBUG
+#if DELAY
     con_puts("Waiting 5 s for testing...");
     udelay(5000000);
     con_putc('\n');