Browse Source

esplink: fix startup time race condition on ESP32 side

Fix a startup race condition when ESP32 boots without the FPGA.

When ESP32 would boot but the FPGA was already online, sometimes the
ESP32 would lose the initialization interrupt. Change around the
notifications a bit to avoid that race.
H. Peter Anvin 2 years ago
parent
commit
065742e213

+ 25 - 16
esp32/max80/fpgasvc.c

@@ -57,7 +57,8 @@ static void ARDUINO_ISR_ATTR fpga_interrupt(void)
     if (!fpga_task)
 	return;
 
-    xTaskNotifyIndexedFromISR(fpga_task, NOTIFY_INDEX, 1, eSetBits, &do_wakeup);
+    xTaskNotifyIndexedFromISR(fpga_task, NOTIFY_INDEX, 1, eIncrement,
+			      &do_wakeup);
     if (do_wakeup)
 	portYIELD_FROM_ISR(do_wakeup);
 }
@@ -93,16 +94,16 @@ int fpga_service_start(void)
     xSemaphoreGive(spi_mutex);
 
     if (!fpga_task) {
+	/* The ordering here attempts to avoid race conditions... */
 	if (xTaskCreate(fpga_service_task, "fpga_svc", 4096, NULL,
 			FPGA_PRIORITY, &fpga_task) != pdPASS)
 	    goto failed;
-
 	attachInterrupt(PIN_FPGA_INT, fpga_interrupt, FALLING);
+	xTaskNotifyIndexed(fpga_task, NOTIFY_INDEX, 1, eIncrement);
 	esp_register_shutdown_handler(fpga_service_stop);
     }
 
-    /* All good! */
-    printf("[FPGA] FPGA services started\n");
+    /* All good (hopefully?) */
     return 0;
 
 failed:
@@ -144,18 +145,19 @@ void fpga_service_stop(void)
 #define FPGA_CMD_ACK3	(3 << 2)
 #define FPGA_CMD_WR	(0 << 4)
 #define FPGA_CMD_RD	(1 << 4)
+#define FPGA_CMD_CONTROL_MASK 0x0f
 
 #define FPGA_HDR_ADDR	0x40000000
 
-static esp_err_t fpga_io_write(uint8_t cmd, uint32_t addr,
+static esp_err_t fpga_io_write(unsigned int cmd, uint32_t addr,
 			       const void *data, size_t len)
 {
     spi_transaction_ext_t trans;
     esp_err_t err;
 
-    if (!len)
+    if (!len && !(cmd & ~FPGA_CMD_RD))
 	return ESP_OK;
-
+    
     memset(&trans, 0, sizeof trans);
     trans.base.flags   =
 	SPI_TRANS_MODE_DIO |
@@ -174,15 +176,15 @@ static esp_err_t fpga_io_write(uint8_t cmd, uint32_t addr,
     return err;
 }
 
-static esp_err_t fpga_io_read(uint8_t cmd, uint32_t addr,
+static esp_err_t fpga_io_read(unsigned int cmd, uint32_t addr,
 			      void *data, size_t len)
 {
     spi_transaction_ext_t trans;
     esp_err_t err;
 
-    if (!len)
+    if (!len && !(cmd & ~FPGA_CMD_RD))
 	return ESP_OK;
-
+    
     memset(&trans, 0, sizeof trans);
     trans.base.flags   =
 	SPI_TRANS_MODE_DIO |
@@ -204,7 +206,8 @@ static esp_err_t fpga_io_read(uint8_t cmd, uint32_t addr,
     return err;
 }
 
-static uint32_t fpga_io_status(void)
+/* CMD here can be interrupt flags, for example */
+static uint32_t fpga_io_status(unsigned int cmd)
 {
     spi_transaction_ext_t trans;
     esp_err_t err;
@@ -216,7 +219,7 @@ static uint32_t fpga_io_status(void)
 	SPI_TRANS_MULTILINE_ADDR |
 	SPI_TRANS_USE_RXDATA;
 
-    trans.base.cmd       = FPGA_CMD_RD;
+    trans.base.cmd       = cmd | FPGA_CMD_RD;
     trans.base.rxlength  = 32;
 
     xSemaphoreTake(spi_mutex, portMAX_DELAY);
@@ -230,15 +233,23 @@ static void fpga_service_task(void *dummy)
 {
     (void)dummy;
     struct dram_io_head head;
+    uint32_t status;
+
+    printf("[FPGA] Starting FPGA services\n");
 
     /* If the FPGA is already up, need to issue our own active handshake */
-    fpga_io_write(FPGA_CMD_IRQ(RV_IRQ_HELLO), 0, NULL, 0);
+    status = fpga_io_status(FPGA_CMD_IRQ(RV_IRQ_HELLO));
+    printf("[FPGA] Link status bits = 0x%08x, int = %u\n",
+	   status, digitalRead(PIN_FPGA_INT));
 
     while (1) {
+	/* Wait until an interrupt is received */
+	xTaskNotifyWaitIndexed(NOTIFY_INDEX, 0, -1U, NULL, portMAX_DELAY);
+	
 	while (!digitalRead(PIN_FPGA_INT)) {
 	    printf("[FPGA] FPGA signals ready\n");
 
-	    uint32_t status = fpga_io_status();
+	    uint32_t status = fpga_io_status(0);
 	    printf("[FPGA] Link status bits = 0x%08x\n", status);
 	    
 	    fpga_io_read(FPGA_CMD_ACK(ESP_IRQ_READY), FPGA_HDR_ADDR,
@@ -258,7 +269,5 @@ static void fpga_service_task(void *dummy)
 	    printf("[FPGA] \"%s\"\n", signature_string);
 	}
 
-	/* Wait until an interrupt is received */
-	xTaskNotifyWaitIndexed(NOTIFY_INDEX, 0, 1, NULL, portMAX_DELAY);
     }
 }

BIN
esp32/output/max80.ino.bin


+ 12 - 7
fpga/esp.sv

@@ -97,8 +97,9 @@ module esp #(
    reg [31:0] spi_shr;
    reg [ 3:0] spi_ctr;
    reg [ 3:0] cpu_irq;
+   reg [ 3:1] cpu_set_irq;	// CPU IRQs to set once idle
    reg [ 3:1] spi_irq;
-   reg [ 3:1] latched_spi_irq;
+   reg [ 3:1] latched_spi_irq;	// SPI IRQ as of transition start
    reg [ 1:0] spi_out;
    reg 	      spi_oe;
    reg [ 2:0] spi_wbe;		// Partial word write byte enables
@@ -119,7 +120,9 @@ module esp #(
 	  spi_cmd   <= 'b0;
 	  spi_ctr   <= 4'd3;	// 8 bits needed for this state
 	  cpu_irq   <= 'b0;
+	  cpu_set_irq <= 'b0;
 	  spi_irq   <= 'b0;
+	  latched_spi_irq <= 'b0;
 	  spi_oe    <= 1'b0;
 	  spi_wbe   <= 3'b0;
 	  mem_addr  <= 'b0;
@@ -137,10 +140,8 @@ module esp #(
 	       spi_oe     <= 1'b0;
 	       if (~mem_valid)
 		 begin
-		    spi_cmd <= 'b0;
-		    for (int i = 1; i < 4; i++)
-		      if (spi_cmd[1:0] == i)
-			cpu_irq[i] <= 1'b1;
+		    cpu_irq <= cpu_irq | { cpu_set_irq, 1'b0 };
+		    cpu_set_irq <= 'b0;
 		 end
 	    end
 	  else if (spi_clk_q == 2'b01)
@@ -196,8 +197,12 @@ module esp #(
 			    spi_state       <= st_addr;
 			    latched_spi_irq <= spi_irq;
 			    for (int i = 1; i < 4; i++)
-			      if (spi_indata[3:2] == i)
-				spi_irq[i] <= 1'b0;
+			      begin
+				 if (spi_indata[3:2] == i)
+				   spi_irq[i] <= 1'b0;
+				 if (spi_indata[1:0] == i)
+				   cpu_set_irq[i] <= 1'b1;
+			      end
 			 end
 			 st_addr: begin
 			    mem_addr  <= spi_indata & mem_addr_mask;

+ 3 - 3
fpga/max80.qpf

@@ -19,15 +19,15 @@
 #
 # Quartus Prime
 # Version 21.1.0 Build 842 10/21/2021 SJ Lite Edition
-# Date created = 17:39:37  May 02, 2022
+# Date created = 00:54:28  May 03, 2022
 #
 # -------------------------------------------------------------------------- #
 
 QUARTUS_VERSION = "21.1"
-DATE = "17:39:37  May 02, 2022"
+DATE = "00:54:28  May 03, 2022"
 
 # Revisions
 
-PROJECT_REVISION = "v2"
 PROJECT_REVISION = "v1"
+PROJECT_REVISION = "v2"
 PROJECT_REVISION = "bypass"

BIN
fpga/output/bypass.jic


BIN
fpga/output/v1.fw


BIN
fpga/output/v1.jic


BIN
fpga/output/v1.rbf.gz


BIN
fpga/output/v1.rpd.gz


BIN
fpga/output/v1.sof


BIN
fpga/output/v1.svf.gz


BIN
fpga/output/v1.xsvf.gz


BIN
fpga/output/v2.fw


BIN
fpga/output/v2.jic


BIN
fpga/output/v2.rbf.gz


BIN
fpga/output/v2.rpd.gz


BIN
fpga/output/v2.sof


BIN
fpga/output/v2.svf.gz


BIN
fpga/output/v2.xsvf.gz


+ 1 - 1
rv32/checksum.h

@@ -1,4 +1,4 @@
 #ifndef CHECKSUM_H
 #define CHECKSUM_H
-#define SDRAM_SUM 0x8381a3d3
+#define SDRAM_SUM 0x2591a983
 #endif

+ 1 - 2
rv32/esp.c

@@ -25,8 +25,7 @@ IRQHANDLER(esp,0)
 
 void esp_init(void)
 {
-    static const char __aligned(4) esp_signature[]
-	= "Hej tomtebuggar slå i glasen!";
+    static const char esp_signature[] = "Hej tomtebuggar slå i glasen!";
 
     dram_io_head.magic         = 0;
     dram_io_head.hlen          = sizeof dram_io_head;