[PATCH] usb: Do not send ZLP when we have filled the window

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/simtrace@lists.osmocom.org/.

Holger Hans Peter Freyther holger at freyther.de
Wed Nov 9 22:22:13 UTC 2011


From: Holger Hans Peter Freyther <zecke at selfish.org>

Only send the ZLP if we send less data than was required/asked for
by the host and it is a multiple of the bMaxPacketSize0 (which is
hardcoded to 8 right now).

This is completing the change done in fe88b83e80df8be0351ff38ee6 to
fix SIMtrace attached to OSX and not regress on windows.

Introduce another parameter to udp_ep0_send_data to specify the window
size (wLength) or if not available the default from USB 2.0 specification.
---
 firmware/src/dfu/dfu.c          |   46 ++++++++++++++++----------------
 firmware/src/dfu/dfu.h          |    2 +-
 firmware/src/os/pcd_enumerate.c |   54 +++++++++++++++++++++------------------
 3 files changed, 53 insertions(+), 49 deletions(-)

diff --git a/firmware/src/dfu/dfu.c b/firmware/src/dfu/dfu.c
index 5d6865c..9d1ad10 100644
--- a/firmware/src/dfu/dfu.c
+++ b/firmware/src/dfu/dfu.c
@@ -96,13 +96,15 @@ static void __dfufunc udp_init(void)
 static void __dfufunc udp_ep0_send_zlp(void);
 
 /* Send Data through the control endpoint */
-static void __dfufunc udp_ep0_send_data(const char *pData, u_int32_t length)
+static void __dfufunc udp_ep0_send_data(const char *pData, u_int32_t length,
+					u_int32_t window_length)
 {
 	AT91PS_UDP pUdp = AT91C_BASE_UDP;
-	u_int32_t cpt = 0, len_remain = length;
+	u_int32_t cpt = 0, len_remain;
 	AT91_REG csr;
 
-	DEBUGE("send_data: %u bytes ", length);
+	len_remain = MIN(length, window_length);
+	DEBUGE("send_data: %u/%u bytes ", length, window_length);
 
 	do {
 		cpt = MIN(len_remain, 8);
@@ -135,7 +137,7 @@ static void __dfufunc udp_ep0_send_data(const char *pData, u_int32_t length)
 		while (pUdp->UDP_CSR[0] & AT91C_UDP_TXCOMP) ;
 	}
 
-	if ((length % 8) == 0) {
+	if ((length % 8) == 0 && length < window_length) {
 		/* if the length is a multiple of the EP size, we need
 		 * to send another ZLP (zero-length packet) to tell the
 		 * host the transfer has completed.  */
@@ -366,7 +368,7 @@ static __dfufunc int handle_upload(u_int16_t __unused val, u_int16_t len)
 		first_download = 1;
 	}
 
-	udp_ep0_send_data((char *)ptr, len);
+	udp_ep0_send_data((char *)ptr, len, len);
 	ptr+= len;
 
 	return len;
@@ -409,7 +411,7 @@ static __dfufunc void handle_getstatus(void)
 	dstat.iString = 0;
 	/* FIXME: set dstat.bwPollTimeout */
 
-	udp_ep0_send_data((char *)&dstat, sizeof(dstat));
+	udp_ep0_send_data((char *)&dstat, sizeof(dstat), sizeof(dstat));
 }
 
 static void __dfufunc handle_getstate(void)
@@ -417,7 +419,7 @@ static void __dfufunc handle_getstate(void)
 	u_int8_t u8 = dfu_state;
 	DEBUGE("getstate ");
 
-	udp_ep0_send_data((char *)&u8, sizeof(u8));
+	udp_ep0_send_data((char *)&u8, sizeof(u8), sizeof(u8));
 }
 
 /* callback function for DFU requests */
@@ -806,15 +808,13 @@ static __dfufunc void dfu_udp_ep0_handler(void)
 			/* Return Device Descriptor */
 			udp_ep0_send_data((const char *) 
 					  &dfu_dev_descriptor,
-					  MIN(sizeof(dfu_dev_descriptor),
-					      wLength));
+					  sizeof(dfu_dev_descriptor), wLength);
 			break;
 		case USB_DT_CONFIG:
 			/* Return Configuration Descriptor */
 			udp_ep0_send_data((const char *)
 					  &dfu_cfg_descriptor,
-					  MIN(sizeof(dfu_cfg_descriptor),
-					      wLength));
+					  sizeof(dfu_cfg_descriptor), wLength);
 			break;
 		case USB_DT_STRING:
 			/* Return String Descriptor */
@@ -825,14 +825,12 @@ static __dfufunc void dfu_udp_ep0_handler(void)
 			DEBUGE("bLength=%u, wLength=%u ", 
 				usb_strings[desc_index]->bLength, wLength);
 			udp_ep0_send_data((const char *) usb_strings[desc_index],
-					  MIN(usb_strings[desc_index]->bLength, 
-					      wLength));
+					  usb_strings[desc_index]->bLength, wLength);
 			break;
 		case USB_DT_CS_DEVICE:
 			/* Return Function descriptor */
 			udp_ep0_send_data((const char *) &dfu_cfg_descriptor.func_dfu,
-					  MIN(sizeof(dfu_cfg_descriptor.func_dfu),
-					      wLength));
+					  sizeof(dfu_cfg_descriptor.func_dfu), wLength);
 			break;
 		default:
 			udp_ep0_send_stall();
@@ -867,18 +865,20 @@ static __dfufunc void dfu_udp_ep0_handler(void)
 		break;
 	case STD_GET_CONFIGURATION:
 		DEBUGE("GET_CONFIG ");
-		udp_ep0_send_data((char *)&(cur_config),
-				   sizeof(cur_config));
+		/* Table 9.4 wLength One */
+		udp_ep0_send_data((char *)&(cur_config), sizeof(cur_config), 1);
 		break;
 	case STD_GET_STATUS_ZERO:
 		DEBUGE("GET_STATUS_ZERO ");
 		wStatus = 0;
-		udp_ep0_send_data((char *)&wStatus, sizeof(wStatus));
+		/* Table 9.4 wLength Two */
+		udp_ep0_send_data((char *)&wStatus, sizeof(wStatus), 2);
 		break;
 	case STD_GET_STATUS_INTERFACE:
 		DEBUGE("GET_STATUS_INTERFACE ");
 		wStatus = 0;
-		udp_ep0_send_data((char *)&wStatus, sizeof(wStatus));
+		/* Table 9.4 wLength Two */
+		udp_ep0_send_data((char *)&wStatus, sizeof(wStatus), 2);
 		break;
 	case STD_GET_STATUS_ENDPOINT:
 		DEBUGE("GET_STATUS_ENDPOINT(EPidx=%u) ", wIndex&0x0f);
@@ -887,14 +887,14 @@ static __dfufunc void dfu_udp_ep0_handler(void)
 		if ((pUDP->UDP_GLBSTATE & AT91C_UDP_CONFG) && (wIndex == 0)) {
 			wStatus =
 			    (pUDP->UDP_CSR[wIndex] & AT91C_UDP_EPEDS) ? 0 : 1;
-			udp_ep0_send_data((char *)&wStatus,
-					   sizeof(wStatus));
+			/* Table 9.4 wLength Two */
+			udp_ep0_send_data((char *)&wStatus, sizeof(wStatus), 2);
 		} else if ((pUDP->UDP_GLBSTATE & AT91C_UDP_FADDEN)
 			   && (wIndex == 0)) {
 			wStatus =
 			    (pUDP->UDP_CSR[wIndex] & AT91C_UDP_EPEDS) ? 0 : 1;
-			udp_ep0_send_data((char *)&wStatus,
-					   sizeof(wStatus));
+			/* Table 9.4 wLength Two */
+			udp_ep0_send_data((char *)&wStatus, sizeof(wStatus), 2);
 		} else
 			udp_ep0_send_stall();
 		break;
diff --git a/firmware/src/dfu/dfu.h b/firmware/src/dfu/dfu.h
index c898197..27c63b5 100644
--- a/firmware/src/dfu/dfu.h
+++ b/firmware/src/dfu/dfu.h
@@ -124,7 +124,7 @@ struct _dfu_desc {
 
 struct dfuapi {
 	void (*udp_init)(void);
-	void (*ep0_send_data)(const char *data, u_int32_t len);
+	void (*ep0_send_data)(const char *data, u_int32_t len, u_int32_t wlen);
 	void (*ep0_send_zlp)(void);
 	void (*ep0_send_stall)(void);
 	int  (*dfu_ep0_handler)(u_int8_t req_type, u_int8_t req,
diff --git a/firmware/src/os/pcd_enumerate.c b/firmware/src/os/pcd_enumerate.c
index c360d37..3a7397f 100644
--- a/firmware/src/os/pcd_enumerate.c
+++ b/firmware/src/os/pcd_enumerate.c
@@ -531,12 +531,12 @@ static void udp_ep0_handler(void)
 			if (*dfu->dfu_state != DFU_STATE_appIDLE)
 				udp_ep0_send_data((const char *) 
 						  dfu->dfu_dev_descriptor,
-						  MIN(dfu->dfu_dev_descriptor->bLength,
-						      wLength));
+						  dfu->dfu_dev_descriptor->bLength,
+						  wLength);
 			else
 #endif
 			udp_ep0_send_data((const char *) &dev_descriptor,
-					   MIN(sizeof(dev_descriptor), wLength));
+					  sizeof(dev_descriptor), wLength);
 			break;
 		case USB_DT_CONFIG:
 			/* Return Configuration Descriptor */
@@ -544,12 +544,12 @@ static void udp_ep0_handler(void)
 			if (*dfu->dfu_state != DFU_STATE_appIDLE)
 				udp_ep0_send_data((const char *)
 						  dfu->dfu_cfg_descriptor,
-						  MIN(dfu->dfu_cfg_descriptor->ucfg.wTotalLength,
-						      wLength));
+						  dfu->dfu_cfg_descriptor->ucfg.wTotalLength,
+						  wLength);
 			else
 #endif
 			udp_ep0_send_data((const char *) &cfg_descriptor,
-					   MIN(sizeof(cfg_descriptor), wLength));
+					   sizeof(cfg_descriptor), wLength);
 			break;
 		case USB_DT_STRING:
 #ifdef CONFIG_USB_STRING
@@ -559,8 +559,8 @@ static void udp_ep0_handler(void)
 			DEBUGE("bLength=%u, wLength=%u\n", 
 				usb_strings[desc_index]->bLength, wLength);
 			udp_ep0_send_data((const char *) usb_strings[desc_index],
-					  MIN(usb_strings[desc_index]->bLength, 
-					      wLength));
+					  usb_strings[desc_index]->bLength,
+					  wLength);
 #else
 			goto out_stall;
 #endif
@@ -568,7 +568,7 @@ static void udp_ep0_handler(void)
 		case USB_DT_CS_DEVICE:
 			/* Return Function descriptor */
 			udp_ep0_send_data((const char *) &dfu->dfu_cfg_descriptor->func_dfu,
-					  MIN(sizeof(dfu->dfu_cfg_descriptor->func_dfu), wLength));
+					  sizeof(dfu->dfu_cfg_descriptor->func_dfu), wLength);
 			break;
 		case USB_DT_INTERFACE:
 			/* Return Interface descriptor */
@@ -578,27 +578,27 @@ static void udp_ep0_handler(void)
 			case 0:
 				udp_ep0_send_data((const char *)
 						&cfg_descriptor.uif,
-						MIN(sizeof(cfg_descriptor.uif),
-						      wLength));
+						sizeof(cfg_descriptor.uif),
+						wLength);
 				break;
 	#ifdef CONFIG_DFU
 			case 1:
 				udp_ep0_send_data((const char *)
 						&cfg_descriptor.uif_dfu[0],
-						MIN(sizeof(cfg_descriptor.uif_dfu[0]),
-						    wLength));
+						sizeof(cfg_descriptor.uif_dfu[0]),
+						wLength);
 				break;
 			case 2:
 				udp_ep0_send_data((const char *)
 						&cfg_descriptor.uif_dfu[1],
-						MIN(sizeof(cfg_descriptor.uif_dfu[1]),
-						    wLength));
+						sizeof(cfg_descriptor.uif_dfu[1]),
+						wLength);
 				break;
 			case 3:
 				udp_ep0_send_data((const char *)
 						&cfg_descriptor.uif_dfu[2],
-						MIN(sizeof(cfg_descriptor.uif_dfu[2]),
-						    wLength));
+						sizeof(cfg_descriptor.uif_dfu[2]),
+						wLength);
 				break;
 	#endif
 			default:
@@ -679,8 +679,9 @@ static void udp_ep0_handler(void)
 		switch (upcd.state) {
 		case USB_STATE_ADDRESS:
 		case USB_STATE_CONFIGURED:
+			/* Table 9.4 wLength One */
 			udp_ep0_send_data((char *)&(upcd.cur_config),
-					   sizeof(upcd.cur_config));
+					   sizeof(upcd.cur_config), 1);
 			break;
 		default:
 			goto out_stall;
@@ -691,13 +692,15 @@ static void udp_ep0_handler(void)
 		DEBUGE("GET_INTERFACE ");
 		if (upcd.state != USB_STATE_CONFIGURED)
 			goto out_stall;
+		/* Table 9.4 wLength One */
 		udp_ep0_send_data((char *)&(upcd.cur_altsett),
-				  sizeof(upcd.cur_altsett));
+				  sizeof(upcd.cur_altsett), 1);
 		break;
 	case STD_GET_STATUS_ZERO:
 		DEBUGE("GET_STATUS_ZERO ");
 		wStatus = 0;
-		udp_ep0_send_data((char *)&wStatus, sizeof(wStatus));
+		/* Table 9.4 wLength Two */
+		udp_ep0_send_data((char *)&wStatus, sizeof(wStatus), 2);
 		break;
 	case STD_GET_STATUS_INTERFACE:
 		DEBUGE("GET_STATUS_INTERFACE ");
@@ -705,7 +708,8 @@ static void udp_ep0_handler(void)
 		    (upcd.state == USB_STATE_ADDRESS && wIndex != 0))
 			goto out_stall;
 		wStatus = 0;
-		udp_ep0_send_data((char *)&wStatus, sizeof(wStatus));
+		/* Table 9.4 wLength Two */
+		udp_ep0_send_data((char *)&wStatus, sizeof(wStatus), 2);
 		break;
 	case STD_GET_STATUS_ENDPOINT:
 		DEBUGE("GET_STATUS_ENDPOINT(EPidx=%u) ", wIndex&0x0f);
@@ -717,14 +721,14 @@ static void udp_ep0_handler(void)
 		if ((pUDP->UDP_GLBSTATE & AT91C_UDP_CONFG) && (wIndex <= 3)) {
 			wStatus =
 			    (pUDP->UDP_CSR[wIndex] & AT91C_UDP_EPEDS) ? 0 : 1;
-			udp_ep0_send_data((char *)&wStatus,
-					   sizeof(wStatus));
+			/* Table 9.4 wLength Two */
+			udp_ep0_send_data((char *)&wStatus, sizeof(wStatus), 2);
 		} else if ((pUDP->UDP_GLBSTATE & AT91C_UDP_FADDEN)
 			   && (wIndex == 0)) {
 			wStatus =
 			    (pUDP->UDP_CSR[wIndex] & AT91C_UDP_EPEDS) ? 0 : 1;
-			udp_ep0_send_data((char *)&wStatus,
-					   sizeof(wStatus));
+			/* Table 9.4 wLength Two */
+			udp_ep0_send_data((char *)&wStatus, sizeof(wStatus), 2);
 		} else
 			goto out_stall;
 		break;
-- 
1.7.7.2





More information about the simtrace mailing list