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

From: Holger Hans Peter Freyther &lt;zecke(a)selfish.org&gt; 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