From bd694244db7bc9699548ca276f992aa5ce9bbac0 Mon Sep 17 00:00:00 2001 From: Lukasz Majewski Date: Mon, 12 May 2014 10:43:36 +0200 Subject: [PATCH 01/13] dfu: Introduction of the "dfu_hash_algo" env variable for checksum method setting Up till now the CRC32 of received data was calculated unconditionally. The standard crc32 implementation causes long delay when large images were uploaded. The "dfu_hash_algo" environment variable gives the opportunity to disable on demand the hash (crc32) calculation. It can be done without the need to recompile the u-boot binary. By default the crc32 is calculated, which means that legacy behavior has been preserved. Tests results: 400 MiB ums.img file With crc32 calculation: 65 sec [avg 6.29 MB/s] Without crc32 calculation: 25 sec [avg 16.17 MB/s] Signed-off-by: Lukasz Majewski Cc: Marek Vasut --- drivers/dfu/dfu.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 47 insertions(+), 5 deletions(-) diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c index a938109..5878f99 100644 --- a/drivers/dfu/dfu.c +++ b/drivers/dfu/dfu.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -20,6 +21,7 @@ static bool dfu_reset_request; static LIST_HEAD(dfu_list); static int dfu_alt_num; static int alt_num_cnt; +static struct hash_algo *dfu_hash_algo; bool dfu_reset(void) { @@ -99,6 +101,29 @@ unsigned char *dfu_get_buf(void) return dfu_buf; } +static char *dfu_get_hash_algo(void) +{ + char *s; + + s = getenv("dfu_hash_algo"); + /* + * By default the legacy behaviour to calculate the crc32 hash + * value is preserved. + * + * To disable calculation of the hash algorithm for received data + * specify the "dfu_hash_algo = disabled" at your board envs. + */ + debug("%s: DFU hash method: %s\n", __func__, s ? s : "not specified"); + + if (!s || !strcmp(s, "crc32")) + return "crc32"; + + if (!strcmp(s, "disabled")) + return NULL; + + return NULL; +} + static int dfu_write_buffer_drain(struct dfu_entity *dfu) { long w_size; @@ -109,8 +134,9 @@ static int dfu_write_buffer_drain(struct dfu_entity *dfu) if (w_size == 0) return 0; - /* update CRC32 */ - dfu->crc = crc32(dfu->crc, dfu->i_buf_start, w_size); + if (dfu_hash_algo) + dfu_hash_algo->hash_update(dfu_hash_algo, &dfu->crc, + dfu->i_buf_start, w_size, 0); ret = dfu->write_medium(dfu, dfu->offset, dfu->i_buf_start, &w_size); if (ret) @@ -138,7 +164,9 @@ int dfu_flush(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num) if (dfu->flush_medium) ret = dfu->flush_medium(dfu); - printf("\nDFU complete CRC32: 0x%08x\n", dfu->crc); + if (dfu_hash_algo) + printf("\nDFU complete %s: 0x%08x\n", dfu_hash_algo->name, + dfu->crc); /* clear everything */ dfu_free_buf(); @@ -238,7 +266,11 @@ static int dfu_read_buffer_fill(struct dfu_entity *dfu, void *buf, int size) /* consume */ if (chunk > 0) { memcpy(buf, dfu->i_buf, chunk); - dfu->crc = crc32(dfu->crc, buf, chunk); + if (dfu_hash_algo) + dfu_hash_algo->hash_update(dfu_hash_algo, + &dfu->crc, buf, + chunk, 0); + dfu->i_buf += chunk; dfu->b_left -= chunk; dfu->r_left -= chunk; @@ -322,7 +354,9 @@ int dfu_read(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num) } if (ret < size) { - debug("%s: %s CRC32: 0x%x\n", __func__, dfu->name, dfu->crc); + if (dfu_hash_algo) + debug("%s: %s %s: 0x%x\n", __func__, dfu->name, + dfu_hash_algo->name, dfu->crc); puts("\nUPLOAD ... done\nCtrl+C to exit ...\n"); dfu_free_buf(); @@ -397,6 +431,14 @@ int dfu_config_entities(char *env, char *interface, int num) dfu_alt_num = dfu_find_alt_num(env); debug("%s: dfu_alt_num=%d\n", __func__, dfu_alt_num); + dfu_hash_algo = NULL; + s = dfu_get_hash_algo(); + if (s) { + ret = hash_lookup_algo(s, &dfu_hash_algo); + if (ret) + error("Hash algorithm %s not supported\n", s); + } + dfu = calloc(sizeof(*dfu), dfu_alt_num); if (!dfu) return -1; From 0d437bcaf9be36d7bb954cb261635678c790dff7 Mon Sep 17 00:00:00 2001 From: Stephen Warren Date: Mon, 19 May 2014 14:21:17 -0600 Subject: [PATCH 02/13] usb: hub: fix power good delay timing usb_hub_power_on() currently waits for the maximum of (a) the hub port's power output to become good, (b) the max time the USB specification allows a device to take to connect. However, these two operations must occur in series rather than in parallel. First, the power supply ramps up to the level required to power the USB device, and then the device may take a certain amount of time to connect (assert D+/D- pullups). Related, the maximum time that a device has to assert pullups is 1s not 100ms. This is explained in "Connect Timing ECN.pdf", itself part of usb_20_042814.zip from www.usb.org. Signed-off-by: Stephen Warren --- common/usb_hub.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/common/usb_hub.c b/common/usb_hub.c index ffac0e7..4875a51 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -36,7 +36,7 @@ #endif #ifndef CONFIG_USB_HUB_MIN_POWER_ON_DELAY -#define CONFIG_USB_HUB_MIN_POWER_ON_DELAY 100 +#define CONFIG_USB_HUB_MIN_POWER_ON_DELAY 1000 #endif #define USB_BUFSIZ 512 @@ -138,8 +138,11 @@ static void usb_hub_power_on(struct usb_hub_device *hub) debug("port %d returns %lX\n", i + 1, dev->status); } - /* Wait for power to become stable */ - mdelay(max(pgood_delay, CONFIG_USB_HUB_MIN_POWER_ON_DELAY)); + /* + * Wait for power to become stable, + * plus spec-defined max time for device to connect + */ + mdelay(pgood_delay + CONFIG_USB_HUB_MIN_POWER_ON_DELAY); } void usb_hub_reset(void) From 77b83e6d099cb2149e5b2c33a700003227d99297 Mon Sep 17 00:00:00 2001 From: Stephen Warren Date: Mon, 19 May 2014 14:21:18 -0600 Subject: [PATCH 03/13] usb: hub: remove CONFIG_USB_HUB_MIN_POWER_ON_DELAY Now that we wait the correct specification-mandated time at the end of usb_hub_power_on(), I suspect that CONFIG_USB_HUB_MIN_POWER_ON_DELAY has no purpose. For cm_t35.h, we already wait longer than the original MIN_POWER_ON_DELAY, so this change is safe. For gw_ventana.h, we will wait as long as the original MIN_POWER_ON_DELAY iff pgood_delay was at least 200ms. I'm not sure if this is the case or not, hence I've CC'd relevant people to test this change. Cc: Igor Grinberg Cc: Tim Harvey Signed-off-by: Stephen Warren --- README | 3 --- common/usb_hub.c | 6 +----- include/configs/cm_t35.h | 2 -- include/configs/gw_ventana.h | 1 - 4 files changed, 1 insertion(+), 11 deletions(-) diff --git a/README b/README index a280435..a085625 100644 --- a/README +++ b/README @@ -1432,9 +1432,6 @@ The following options need to be configured: CONFIG_USB_EHCI_TXFIFO_THRESH enables setting of the txfilltuning field in the EHCI controller on reset. - CONFIG_USB_HUB_MIN_POWER_ON_DELAY defines the minimum - interval for usb hub power-on delay.(minimum 100msec) - - USB Device: Define the below if you wish to use the USB console. Once firmware is rebuilt from a serial console issue the diff --git a/common/usb_hub.c b/common/usb_hub.c index 4875a51..2add4b9 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -35,10 +35,6 @@ #include #endif -#ifndef CONFIG_USB_HUB_MIN_POWER_ON_DELAY -#define CONFIG_USB_HUB_MIN_POWER_ON_DELAY 1000 -#endif - #define USB_BUFSIZ 512 static struct usb_hub_device hub_dev[USB_MAX_HUB]; @@ -142,7 +138,7 @@ static void usb_hub_power_on(struct usb_hub_device *hub) * Wait for power to become stable, * plus spec-defined max time for device to connect */ - mdelay(pgood_delay + CONFIG_USB_HUB_MIN_POWER_ON_DELAY); + mdelay(pgood_delay + 1000); } void usb_hub_reset(void) diff --git a/include/configs/cm_t35.h b/include/configs/cm_t35.h index aae05e0..f6acf69 100644 --- a/include/configs/cm_t35.h +++ b/include/configs/cm_t35.h @@ -104,8 +104,6 @@ #define CONFIG_USB_DEVICE #define CONFIG_USB_TTY #define CONFIG_SYS_CONSOLE_IS_IN_ENV -/* This delay is really for slow-to-power-on USB sticks, not the hub */ -#define CONFIG_USB_HUB_MIN_POWER_ON_DELAY 500 /* commands to include */ #include diff --git a/include/configs/gw_ventana.h b/include/configs/gw_ventana.h index cd55495..f41c96e 100644 --- a/include/configs/gw_ventana.h +++ b/include/configs/gw_ventana.h @@ -192,7 +192,6 @@ #define CONFIG_USB_ETH_CDC #define CONFIG_NETCONSOLE #define CONFIG_SYS_USB_EVENT_POLL_VIA_CONTROL_EP -#define CONFIG_USB_HUB_MIN_POWER_ON_DELAY 1200 /* Framebuffer and LCD */ #define CONFIG_VIDEO From 7484d84cbb58e01ff1d2d458d899ea1fba012724 Mon Sep 17 00:00:00 2001 From: Stephen Warren Date: Thu, 29 May 2014 14:53:00 -0600 Subject: [PATCH 04/13] usb: ci_udc: detect queued requests on ep0 The flipping of ep0 between IN and OUT relies on ci_ep_queue() consuming the current IN/OUT setting immediately. If this is deferred to a later point when the req is pulled out of ci_req->queue, then the IN/OUT setting may have been changed since the req was queued, and state will get out of sync. This condition doesn't occur today, but could if bugs were introduced later, and this error-check will save a lot of debugging time. Signed-off-by: Stephen Warren --- drivers/usb/gadget/ci_udc.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c index 9cd0036..a68a85f 100644 --- a/drivers/usb/gadget/ci_udc.c +++ b/drivers/usb/gadget/ci_udc.c @@ -397,6 +397,21 @@ static int ci_ep_queue(struct usb_ep *ep, num = ci_ep->desc->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK; in = (ci_ep->desc->bEndpointAddress & USB_DIR_IN) != 0; + if (!num && ci_ep->req_primed) { + /* + * The flipping of ep0 between IN and OUT relies on + * ci_ep_queue consuming the current IN/OUT setting + * immediately. If this is deferred to a later point when the + * req is pulled out of ci_req->queue, then the IN/OUT setting + * may have been changed since the req was queued, and state + * will get out of sync. This condition doesn't occur today, + * but could if bugs were introduced later, and this error + * check will save a lot of debugging time. + */ + printf("%s: ep0 transaction already in progress\n", __func__); + return -EPROTO; + } + ret = ci_bounce(ci_req, in); if (ret) return ret; From 054731b09e1944cdb130b755ac0f6c188920e98a Mon Sep 17 00:00:00 2001 From: Stephen Warren Date: Thu, 29 May 2014 14:53:01 -0600 Subject: [PATCH 05/13] usb: ci_udc: use a single descriptor for ep0 ci_udc currently points ep->desc at separate descriptors for IN and OUT. These descriptors only differ in the ep address IN/OUT field. Modify the code to use a single descriptor, and change that descriptor's ep address to indicate IN/OUT as required. This removes some data duplication. Signed-off-by: Stephen Warren --- drivers/usb/gadget/ci_udc.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c index a68a85f..77f8c9e 100644 --- a/drivers/usb/gadget/ci_udc.c +++ b/drivers/usb/gadget/ci_udc.c @@ -56,14 +56,7 @@ static const char *reqname(unsigned r) } #endif -static struct usb_endpoint_descriptor ep0_out_desc = { - .bLength = sizeof(struct usb_endpoint_descriptor), - .bDescriptorType = USB_DT_ENDPOINT, - .bEndpointAddress = 0, - .bmAttributes = USB_ENDPOINT_XFER_CONTROL, -}; - -static struct usb_endpoint_descriptor ep0_in_desc = { +static struct usb_endpoint_descriptor ep0_desc = { .bLength = sizeof(struct usb_endpoint_descriptor), .bDescriptorType = USB_DT_ENDPOINT, .bEndpointAddress = USB_DIR_IN, @@ -435,7 +428,7 @@ static void handle_ep_complete(struct ci_ep *ep) num = ep->desc->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK; in = (ep->desc->bEndpointAddress & USB_DIR_IN) != 0; if (num == 0) - ep->desc = &ep0_out_desc; + ep0_desc.bEndpointAddress = 0; item = ci_get_qtd(num, in); ci_invalidate_qtd(num); @@ -460,7 +453,7 @@ static void handle_ep_complete(struct ci_ep *ep) if (num == 0) { ci_req->req.length = 0; usb_ep_queue(&ep->ep, &ci_req->req, 0); - ep->desc = &ep0_in_desc; + ep0_desc.bEndpointAddress = USB_DIR_IN; } } @@ -771,7 +764,7 @@ static int ci_udc_probe(void) /* Init EP 0 */ memcpy(&controller.ep[0].ep, &ci_ep_init[0], sizeof(*ci_ep_init)); - controller.ep[0].desc = &ep0_in_desc; + controller.ep[0].desc = &ep0_desc; INIT_LIST_HEAD(&controller.ep[0].queue); controller.ep[0].req_primed = false; controller.gadget.ep0 = &controller.ep[0].ep; From a2d8f929857b7bc50528114c29e48a99cbcee1f1 Mon Sep 17 00:00:00 2001 From: Stephen Warren Date: Thu, 29 May 2014 14:53:02 -0600 Subject: [PATCH 06/13] usb: ci_udc: pre-allocate ep0 req Allocate ep0's USB request object when the UDC driver is probed. This solves a couple of issues in the current code: a) A request object always exists for ep0. Prior to this patch, if setup transactions arrived in an unexpected order, handle_setup() would need to reply to a setup transaction before any ep0 usb_req was created. This issue was introduced in commit 2813006fecda "usb: ci_udc: allow multiple buffer allocs per ep." b) handle_ep_complete no longer /has/ to queue the ep0 request again after every single request completion. This is currently required, since handle_setup() assumes it can find some request object in ep0's request queue. This patch doesn't actually stop handle_ep_complete() from always requeueing the request, but the next patch will. Signed-off-by: Stephen Warren --- drivers/usb/gadget/ci_udc.c | 17 ++++++++++++++++- drivers/usb/gadget/ci_udc.h | 1 + 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c index 77f8c9e..03ad231 100644 --- a/drivers/usb/gadget/ci_udc.c +++ b/drivers/usb/gadget/ci_udc.c @@ -198,8 +198,14 @@ static void ci_invalidate_qtd(int ep_num) static struct usb_request * ci_ep_alloc_request(struct usb_ep *ep, unsigned int gfp_flags) { + struct ci_ep *ci_ep = container_of(ep, struct ci_ep, ep); + int num; struct ci_req *ci_req; + num = ci_ep->desc->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK; + if (num == 0 && controller.ep0_req) + return &controller.ep0_req->req; + ci_req = memalign(ARCH_DMA_MINALIGN, sizeof(*ci_req)); if (!ci_req) return NULL; @@ -207,6 +213,9 @@ ci_ep_alloc_request(struct usb_ep *ep, unsigned int gfp_flags) INIT_LIST_HEAD(&ci_req->queue); ci_req->b_buf = 0; + if (num == 0) + controller.ep0_req = ci_req; + return &ci_req->req; } @@ -471,7 +480,7 @@ static void handle_setup(void) int num, in, _num, _in, i; char *buf; - ci_req = list_first_entry(&ci_ep->queue, struct ci_req, queue); + ci_req = controller.ep0_req; req = &ci_req->req; head = ci_get_qh(0, 0); /* EP0 OUT */ @@ -780,6 +789,12 @@ static int ci_udc_probe(void) &controller.gadget.ep_list); } + ci_ep_alloc_request(&controller.ep[0].ep, 0); + if (!controller.ep0_req) { + free(controller.epts); + return -ENOMEM; + } + return 0; } diff --git a/drivers/usb/gadget/ci_udc.h b/drivers/usb/gadget/ci_udc.h index 23cff56..7d76af8 100644 --- a/drivers/usb/gadget/ci_udc.h +++ b/drivers/usb/gadget/ci_udc.h @@ -97,6 +97,7 @@ struct ci_ep { struct ci_drv { struct usb_gadget gadget; + struct ci_req *ep0_req; struct usb_gadget_driver *driver; struct ehci_ctrl *ctrl; struct ept_queue_head *epts; From 006c7026882011ba49c9a39d27c4aff3ace07847 Mon Sep 17 00:00:00 2001 From: Stephen Warren Date: Thu, 29 May 2014 14:53:03 -0600 Subject: [PATCH 07/13] usb: ci_udc: complete ep0 direction handling handle_setup() currently assumes that the response to a Setup transaction will be an OUT transaction, and any subsequent packet (if any) will be an IN transaction. This appears to be valid in many cases; both USB enumeration and Mass Storage work OK with this restriction. However, DFU uses ep0 to transfer data in both directions. This renders the assumption invalid; when sending data from device to host, the Data Stage is an IN transaction, and the Status Stage is an OUT transaction. Enhance handle_setup() to deduce the correct direction for the USB transactions based on Setup transaction data. ep0's request object only needs to be automatically re-queued when the Data Stage completes, in order to implement the Status Stage. Once the Status Stage transaction is complete, there is no need to re-queue the USB request, so don't do that. Don't sent USB request completion callbacks for Status Stage transactions. These were queued by ci_udc itself, and only serve to confuse the USB function code. For example, f_dfu attempts to interpret the 0-length data buffers for Status Stage transactions as DFU packets. These buffers contain stale data from the previous transaction. This causes f_dfu to complain about a sequence number mismatch. Signed-off-by: Stephen Warren --- drivers/usb/gadget/ci_udc.c | 48 ++++++++++++++++++++++++++++++++++++++------- drivers/usb/gadget/ci_udc.h | 1 + 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c index 03ad231..6dc20c6 100644 --- a/drivers/usb/gadget/ci_udc.c +++ b/drivers/usb/gadget/ci_udc.c @@ -428,6 +428,17 @@ static int ci_ep_queue(struct usb_ep *ep, return 0; } +static void flip_ep0_direction(void) +{ + if (ep0_desc.bEndpointAddress == USB_DIR_IN) { + DBG("%s: Flipping ep0 ot OUT\n", __func__); + ep0_desc.bEndpointAddress = 0; + } else { + DBG("%s: Flipping ep0 ot IN\n", __func__); + ep0_desc.bEndpointAddress = USB_DIR_IN; + } +} + static void handle_ep_complete(struct ci_ep *ep) { struct ept_queue_item *item; @@ -436,8 +447,6 @@ static void handle_ep_complete(struct ci_ep *ep) num = ep->desc->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK; in = (ep->desc->bEndpointAddress & USB_DIR_IN) != 0; - if (num == 0) - ep0_desc.bEndpointAddress = 0; item = ci_get_qtd(num, in); ci_invalidate_qtd(num); @@ -458,11 +467,18 @@ static void handle_ep_complete(struct ci_ep *ep) DBG("ept%d %s req %p, complete %x\n", num, in ? "in" : "out", ci_req, len); - ci_req->req.complete(&ep->ep, &ci_req->req); - if (num == 0) { + if (num != 0 || controller.ep0_data_phase) + ci_req->req.complete(&ep->ep, &ci_req->req); + if (num == 0 && controller.ep0_data_phase) { + /* + * Data Stage is complete, so flip ep0 dir for Status Stage, + * which always transfers a packet in the opposite direction. + */ + DBG("%s: flip ep0 dir for Status Stage\n", __func__); + flip_ep0_direction(); + controller.ep0_data_phase = false; ci_req->req.length = 0; usb_ep_queue(&ep->ep, &ci_req->req, 0); - ep0_desc.bEndpointAddress = USB_DIR_IN; } } @@ -491,8 +507,26 @@ static void handle_setup(void) #else writel(EPT_RX(0), &udc->epstat); #endif - DBG("handle setup %s, %x, %x index %x value %x\n", reqname(r.bRequest), - r.bRequestType, r.bRequest, r.wIndex, r.wValue); + DBG("handle setup %s, %x, %x index %x value %x length %x\n", + reqname(r.bRequest), r.bRequestType, r.bRequest, r.wIndex, + r.wValue, r.wLength); + + /* Set EP0 dir for Data Stage based on Setup Stage data */ + if (r.bRequestType & USB_DIR_IN) { + DBG("%s: Set ep0 to IN for Data Stage\n", __func__); + ep0_desc.bEndpointAddress = USB_DIR_IN; + } else { + DBG("%s: Set ep0 to OUT for Data Stage\n", __func__); + ep0_desc.bEndpointAddress = 0; + } + if (r.wLength) { + controller.ep0_data_phase = true; + } else { + /* 0 length -> no Data Stage. Flip dir for Status Stage */ + DBG("%s: 0 length: flip ep0 dir for Status Stage\n", __func__); + flip_ep0_direction(); + controller.ep0_data_phase = false; + } list_del_init(&ci_req->queue); ci_ep->req_primed = false; diff --git a/drivers/usb/gadget/ci_udc.h b/drivers/usb/gadget/ci_udc.h index 7d76af8..c214402 100644 --- a/drivers/usb/gadget/ci_udc.h +++ b/drivers/usb/gadget/ci_udc.h @@ -98,6 +98,7 @@ struct ci_ep { struct ci_drv { struct usb_gadget gadget; struct ci_req *ep0_req; + bool ep0_data_phase; struct usb_gadget_driver *driver; struct ehci_ctrl *ctrl; struct ept_queue_head *epts; From 43a8f25b6ca77894ddfd46c2b1196c7bd487561f Mon Sep 17 00:00:00 2001 From: Stephen Warren Date: Tue, 10 Jun 2014 11:02:35 -0600 Subject: [PATCH 08/13] usb: ci_udc: call udc_disconnect() from ci_pullup() ci_pullup()'s !is_on path contains a cut/paste copy of udc_disconnect(). Remove the duplication by simply calling udc_disconnect() instead. Signed-off-by: Stephen Warren --- drivers/usb/gadget/ci_udc.c | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c index 6dc20c6..5f30856 100644 --- a/drivers/usb/gadget/ci_udc.c +++ b/drivers/usb/gadget/ci_udc.c @@ -697,6 +697,17 @@ int usb_gadget_handle_interrupts(void) return value; } +void udc_disconnect(void) +{ + struct ci_udc *udc = (struct ci_udc *)controller.ctrl->hcor; + /* disable pullup */ + stop_activity(); + writel(USBCMD_FS2, &udc->usbcmd); + udelay(800); + if (controller.driver) + controller.driver->disconnect(&controller.gadget); +} + static int ci_pullup(struct usb_gadget *gadget, int is_on) { struct ci_udc *udc = (struct ci_udc *)controller.ctrl->hcor; @@ -715,27 +726,12 @@ static int ci_pullup(struct usb_gadget *gadget, int is_on) /* Turn on the USB connection by enabling the pullup resistor */ writel(USBCMD_ITC(MICRO_8FRAME) | USBCMD_RUN, &udc->usbcmd); } else { - stop_activity(); - writel(USBCMD_FS2, &udc->usbcmd); - udelay(800); - if (controller.driver) - controller.driver->disconnect(gadget); + udc_disconnect(); } return 0; } -void udc_disconnect(void) -{ - struct ci_udc *udc = (struct ci_udc *)controller.ctrl->hcor; - /* disable pullup */ - stop_activity(); - writel(USBCMD_FS2, &udc->usbcmd); - udelay(800); - if (controller.driver) - controller.driver->disconnect(&controller.gadget); -} - static int ci_udc_probe(void) { struct ept_queue_head *head; From bdf81611e444e8aef21cb05eeae69f694c0c7a39 Mon Sep 17 00:00:00 2001 From: Stephen Warren Date: Tue, 10 Jun 2014 11:02:36 -0600 Subject: [PATCH 09/13] usb: ci_udc: fix freeing of ep0 req ci_ep_alloc_request() avoids allocating multiple request objects for ep0 by keeping a record of the first req allocated for ep0, and always returning that instead of allocating a new req. However, if this req is ever freed, the record of the previous allocation is not cleared, so ci_ep_alloc_request() will keep returning this stale pointer. Fix ci_ep_free_request() to clear the record of the previous allocation. Signed-off-by: Stephen Warren --- drivers/usb/gadget/ci_udc.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c index 5f30856..7a6563f 100644 --- a/drivers/usb/gadget/ci_udc.c +++ b/drivers/usb/gadget/ci_udc.c @@ -221,9 +221,14 @@ ci_ep_alloc_request(struct usb_ep *ep, unsigned int gfp_flags) static void ci_ep_free_request(struct usb_ep *ep, struct usb_request *req) { - struct ci_req *ci_req; + struct ci_ep *ci_ep = container_of(ep, struct ci_ep, ep); + struct ci_req *ci_req = container_of(req, struct ci_req, req); + int num; + + num = ci_ep->desc->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK; + if (num == 0) + controller.ep0_req = 0; - ci_req = container_of(req, struct ci_req, req); if (ci_req->b_buf) free(ci_req->b_buf); free(ci_req); From 9a7d34be13a6934e0fddfaf12236d8784343f902 Mon Sep 17 00:00:00 2001 From: Stephen Warren Date: Tue, 10 Jun 2014 11:02:37 -0600 Subject: [PATCH 10/13] usb: ci_udc: fix probe error cleanup If allocation of the ep0 req fails, clean up all the allocations that were made in ci_udc_probe(). Signed-off-by: Stephen Warren --- drivers/usb/gadget/ci_udc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c index 7a6563f..1428af8 100644 --- a/drivers/usb/gadget/ci_udc.c +++ b/drivers/usb/gadget/ci_udc.c @@ -826,6 +826,7 @@ static int ci_udc_probe(void) ci_ep_alloc_request(&controller.ep[0].ep, 0); if (!controller.ep0_req) { + free(controller.items_mem); free(controller.epts); return -ENOMEM; } From b7c0051687f42173e15b151a74e524587e8b59db Mon Sep 17 00:00:00 2001 From: Stephen Warren Date: Tue, 10 Jun 2014 11:02:38 -0600 Subject: [PATCH 11/13] usb: ci_udc: clean up all allocations in unregister usb_gadget_unregister_driver() is called to tear down the USB device mode stack. Fix the driver to stop the USB HW (which causes any attached host to notice the disappearance of the device), and free all allocations (which obviously prevents memory leaks). Signed-off-by: Stephen Warren --- drivers/usb/gadget/ci_udc.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c index 1428af8..435a272 100644 --- a/drivers/usb/gadget/ci_udc.c +++ b/drivers/usb/gadget/ci_udc.c @@ -875,5 +875,11 @@ int usb_gadget_register_driver(struct usb_gadget_driver *driver) int usb_gadget_unregister_driver(struct usb_gadget_driver *driver) { + udc_disconnect(); + + ci_ep_free_request(&controller.ep[0].ep, &controller.ep0_req->req); + free(controller.items_mem); + free(controller.epts); + return 0; } From e0672b3c3ae4fdd48a66af9f7932d2c8e839e459 Mon Sep 17 00:00:00 2001 From: Stephen Warren Date: Tue, 10 Jun 2014 15:27:39 -0600 Subject: [PATCH 12/13] usb: ci_udc: terminate ep0 INs with a zlp when required Sometimes, a zero-length packet is required at the end of an IN transaction so that the host knows the device is done sending data. Enhance ci_udc to send a zlp when necessary. See the comments for more details. Signed-off-by: Stephen Warren --- drivers/usb/gadget/ci_udc.c | 38 +++++++++++++++++++++++++++++++++++--- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c index 435a272..b18bee4 100644 --- a/drivers/usb/gadget/ci_udc.c +++ b/drivers/usb/gadget/ci_udc.c @@ -369,18 +369,50 @@ static void ci_ep_submit_next_request(struct ci_ep *ci_ep) ci_req = list_first_entry(&ci_ep->queue, struct ci_req, queue); len = ci_req->req.length; - item->next = TERMINATE; - item->info = INFO_BYTES(len) | INFO_IOC | INFO_ACTIVE; + item->info = INFO_BYTES(len) | INFO_ACTIVE; item->page0 = (uint32_t)ci_req->hw_buf; item->page1 = ((uint32_t)ci_req->hw_buf & 0xfffff000) + 0x1000; item->page2 = ((uint32_t)ci_req->hw_buf & 0xfffff000) + 0x2000; item->page3 = ((uint32_t)ci_req->hw_buf & 0xfffff000) + 0x3000; item->page4 = ((uint32_t)ci_req->hw_buf & 0xfffff000) + 0x4000; - ci_flush_qtd(num); head->next = (unsigned) item; head->info = 0; + /* + * When sending the data for an IN transaction, the attached host + * knows that all data for the IN is sent when one of the following + * occurs: + * a) A zero-length packet is transmitted. + * b) A packet with length that isn't an exact multiple of the ep's + * maxpacket is transmitted. + * c) Enough data is sent to exactly fill the host's maximum expected + * IN transaction size. + * + * One of these conditions MUST apply at the end of an IN transaction, + * or the transaction will not be considered complete by the host. If + * none of (a)..(c) already applies, then we must force (a) to apply + * by explicitly sending an extra zero-length packet. + */ + /* IN !a !b !c */ + if (in && len && !(len % ci_ep->ep.maxpacket) && ci_req->req.zero) { + /* + * Each endpoint has 2 items allocated, even though typically + * only 1 is used at a time since either an IN or an OUT but + * not both is queued. For an IN transaction, item currently + * points at the second of these items, so we know that we + * can use (item - 1) to transmit the extra zero-length packet + */ + item->next = (unsigned)(item - 1); + item--; + item->info = INFO_ACTIVE; + } + + item->next = TERMINATE; + item->info |= INFO_IOC; + + ci_flush_qtd(num); + DBG("ept%d %s queue len %x, req %p, buffer %p\n", num, in ? "in" : "out", len, ci_req, ci_req->hw_buf); ci_flush_qh(num); From 3d83e6752d5595351a47309aab0749984c5909b7 Mon Sep 17 00:00:00 2001 From: Lukasz Majewski Date: Tue, 10 Jun 2014 12:25:59 +0200 Subject: [PATCH 13/13] dfu: Disable default calculation of CRC32 Patch (SHA1: bd694244db7bc969954) dfu: Introduction of the "dfu_hash_algo" env variable for checksum method setting already introduced more generic handling of the crc32 calculation. Up till now the CRC32 of received data was calculated unconditionally. This patch changes this and from now - by default the crc32 is NOT calculated anymore. Signed-off-by: Lukasz Majewski Cc: Marek Vasut --- drivers/dfu/dfu.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c index 5878f99..dc09ff6 100644 --- a/drivers/dfu/dfu.c +++ b/drivers/dfu/dfu.c @@ -106,21 +106,15 @@ static char *dfu_get_hash_algo(void) char *s; s = getenv("dfu_hash_algo"); - /* - * By default the legacy behaviour to calculate the crc32 hash - * value is preserved. - * - * To disable calculation of the hash algorithm for received data - * specify the "dfu_hash_algo = disabled" at your board envs. - */ - debug("%s: DFU hash method: %s\n", __func__, s ? s : "not specified"); - - if (!s || !strcmp(s, "crc32")) - return "crc32"; - - if (!strcmp(s, "disabled")) + if (!s) return NULL; + if (!strcmp(s, "crc32")) { + debug("%s: DFU hash method: %s\n", __func__, s); + return s; + } + + error("DFU hash method: %s not supported!\n", s); return NULL; }