From: Stefan Brüns <redacted>
Date: Wed, 3 Jan 2018 22:59:41 +0000 (+0100)
Subject: fx2lafw: Delay start of GPIF until first BULK IN request is received
X-Git-Tag: sigrok-firmware-fx2lafw-0.1.6~7
X-Git-Url: https://sigrok.org/gitweb/?a=commitdiff_plain;ds=sidebyside;h=cfeb1a3602f587eb981c4ce1f9ff368997f4e0e0;p=sigrok-firmware-fx2lafw.git

fx2lafw: Delay start of GPIF until first BULK IN request is received

Apparently the host controller may issue the CONTROL transfer with
the START command earlier than any of the BULK IN requests. The result
is a filled up EP2 packet FIFO, which aborts the acquisition.

Waiting for the first BULK IN request matches the sample code in the
TRM, see e.g. Figure 10-30.

This fixes bug #655.
---

diff --git a/fx2lafw.c b/fx2lafw.c
index a1a486cd..b99d77f4 100644
--- a/fx2lafw.c
+++ b/fx2lafw.c
@@ -51,9 +51,7 @@
 volatile __bit got_sud;
 BYTE vendor_command;
 
-volatile WORD ledcounter = 1000;
-
-extern __bit gpif_acquiring;
+volatile WORD ledcounter = 0;
 
 static void setup_endpoints(void)
 {
@@ -127,6 +125,7 @@ BOOL handle_vendorcommand(BYTE cmd)
 	/* Protocol implementation */
 	switch (cmd) {
 	case CMD_START:
+		/* Tell hardware we are ready to receive data. */
 		vendor_command = cmd;
 		EP0BCL = 0;
 		return TRUE;
@@ -193,6 +192,38 @@ void sudav_isr(void) __interrupt SUDAV_ISR
 	CLEAR_SUDAV();
 }
 
+/* IN BULK NAK - the host started requesting data. */
+void ibn_isr(void) __interrupt IBN_ISR
+{
+	/*
+	 * If the IBN interrupt is not disabled, clearing
+	 * does not work. See AN78446, 6.2.
+	 */
+	BYTE ibnsave = IBNIE;
+	IBNIE = 0;
+	CLEAR_USBINT();
+
+	/*
+	 * If the host sent the START command, start the GPIF
+	 * engine. The host will repeat the BULK IN in the next
+	 * microframe.
+	 */
+	if ((IBNIRQ & bmEP2IBN) && (gpif_acquiring == PREPARED)) {
+		ledcounter = 1;
+		PA1 = 0;
+		gpif_acquisition_start();
+	}
+
+	/* Clear IBN flags for all EPs. */
+	IBNIRQ = 0xff;
+
+	NAKIRQ = bmIBN;
+	SYNCDELAY();
+
+	IBNIE = ibnsave;
+	SYNCDELAY();
+}
+
 void usbreset_isr(void) __interrupt USBRESET_ISR
 {
 	handle_hispeed(FALSE);
@@ -208,12 +239,12 @@ void hispeed_isr(void) __interrupt HISPEED_ISR
 void timer2_isr(void) __interrupt TF2_ISR
 {
 	/* Blink LED during acquisition, keep it on otherwise. */
-	if (gpif_acquiring) {
+	if (gpif_acquiring == RUNNING) {
 		if (--ledcounter == 0) {
 			PA1 = !PA1;
 			ledcounter = 1000;
 		}
-	} else {
+	} else if (gpif_acquiring == STOPPED) {
 		PA1 = 1; /* LED on. */
 	}
 	TF2 = 0;
@@ -236,6 +267,7 @@ void fx2lafw_init(void)
 
 	/* TODO: Does the order of the following lines matter? */
 	ENABLE_SUDAV();
+	ENABLE_EP2IBN();
 	ENABLE_HISPEED();
 	ENABLE_USBRESET();
 
@@ -275,7 +307,7 @@ void fx2lafw_poll(void)
 				break;
 
 			if (EP0BCL == sizeof(struct cmd_start_acquisition)) {
-				gpif_acquisition_start(
+				gpif_acquisition_prepare(
 				 (const struct cmd_start_acquisition *)EP0BUF);
 			}
 
diff --git a/gpif-acquisition.c b/gpif-acquisition.c
index 7d3dcb6d..e98cbc54 100644
--- a/gpif-acquisition.c
+++ b/gpif-acquisition.c
@@ -26,7 +26,7 @@
 #include <fx2lafw.h>
 #include <gpif-acquisition.h>
 
-__bit gpif_acquiring;
+enum gpif_status gpif_acquiring = STOPPED;
 
 static void gpif_reset_waveforms(void)
 {
@@ -125,7 +125,7 @@ void gpif_init_la(void)
 	gpif_init_flowstates();
 
 	/* Reset the status. */
-	gpif_acquiring = FALSE;
+	gpif_acquiring = STOPPED;
 }
 
 static void gpif_make_delay_state(volatile BYTE *pSTATE, uint8_t delay, uint8_t output)
@@ -183,7 +183,7 @@ static void gpif_make_data_dp_state(volatile BYTE *pSTATE)
 	pSTATE[24] = (6 << 3) | (6 << 0);
 }
 
-bool gpif_acquisition_start(const struct cmd_start_acquisition *cmd)
+bool gpif_acquisition_prepare(const struct cmd_start_acquisition *cmd)
 {
 	int i;
 	volatile BYTE *pSTATE = &GPIF_WAVE_DATA;
@@ -238,6 +238,14 @@ bool gpif_acquisition_start(const struct cmd_start_acquisition *cmd)
 	/* Populate S1 - the decision point. */
 	gpif_make_data_dp_state(pSTATE++);
 
+	/* Update the status. */
+	gpif_acquiring = PREPARED;
+
+	return true;
+}
+
+void gpif_acquisition_start(void)
+{
 	/* Execute the whole GPIF waveform once. */
 	gpif_set_tc16(1);
 
@@ -245,15 +253,13 @@ bool gpif_acquisition_start(const struct cmd_start_acquisition *cmd)
 	gpif_fifo_read(GPIF_EP2);
 
 	/* Update the status. */
-	gpif_acquiring = TRUE;
-
-	return true;
+	gpif_acquiring = RUNNING;
 }
 
 void gpif_poll(void)
 {
 	/* Detect if acquisition has completed. */
-	if (gpif_acquiring && (GPIFTRIG & 0x80)) {
+	if ((gpif_acquiring == RUNNING) && (GPIFTRIG & 0x80)) {
 		/* Activate NAK-ALL to avoid race conditions. */
 		FIFORESET = 0x80;
 		SYNCDELAY();
@@ -274,6 +280,6 @@ void gpif_poll(void)
 		FIFORESET = 0x00;
 		SYNCDELAY();
 
-		gpif_acquiring = FALSE;
+		gpif_acquiring = STOPPED;
 	}
 }
diff --git a/include/gpif-acquisition.h b/include/gpif-acquisition.h
index 7b55b83b..a0c7af93 100644
--- a/include/gpif-acquisition.h
+++ b/include/gpif-acquisition.h
@@ -23,8 +23,16 @@
 #include <stdbool.h>
 #include <command.h>
 
+enum gpif_status {
+	STOPPED = 0,
+	PREPARED,
+	RUNNING,
+};
+extern enum gpif_status gpif_acquiring;
+
 void gpif_init_la(void);
-bool gpif_acquisition_start(const struct cmd_start_acquisition *cmd);
+bool gpif_acquisition_prepare(const struct cmd_start_acquisition *cmd);
+void gpif_acquisition_start(void);
 void gpif_poll(void);
 
 #endif