[U-Boot] [PATCH v3 0/8] usb: ss: Some fixes and cleanup for USB super-speed support
Based on 'u-boot-usb' master branch.
This patch-series includes majorly some clean-up, few fixes and then some basic super-speed usb infrastructure addition, to help put support for XHCI in near future.
Changes from v2: - Added a patch "usb: common: Weed out USB_**_PRINTFs from usb framework" to replace USB_***_PRINTF with debug() so as to get rid of unnecessary DEBUG macros in usb common framework and thereby rebasing other patches on top of this so that no USB_PRINTF appears further. - Added a patch to reset only USB 2.0 ports, since USB 3.0 protocol ports don't require a reset to happen to move to 'enabled' state. - Added a patch to move definition of 'min3()' out of ehci-hcd and putting the same as macro definition in common header. - Using a 'switch-case' in portspeed() in cmd_usb.c
Changes from v1: - Fixing the issue with 'ifno' as well as added 'if_desc'. - Instead of turning-on only powered-off hub ports, power-cycle all the hub ports (aka: turning off and then turning on again power to all the ports. - Fixing commenting style problem in 'usb: Update device class in usb device's descriptor' - Fixing typo in commit message for 'usb: hub: Fix enumration timeout' - Removing separate definition for 'struct usb_ep_desc'; thereby adding 'struct usb_ss_ep_comp_descriptor' to 'struct usb_interface' only. As a result modifying the patch accordingly also dropping the patch 'usb: eth: Fix for updated usb interface descriptor structure' - Dropping the patch 'usb: hub: Increase device enumeration timeout for broken drives' for now (will come back with a solution at later point of time).
Vivek Gautam (8): usb: common: Weed out USB_**_PRINTFs from usb framework USB: Some cleanup prior to USB 3.0 interface addition usb: hub: Power-cycle on root-hub ports usb: Update device class in usb device's descriptor usb: hub: Fix enumration timeout USB: SS: Add support for Super Speed USB interface usb: hub: Reset only usb 2.0 ports usb: common: Use a global macro for 'min3'
common/cmd_usb.c | 24 +++- common/usb.c | 116 +++++++++--------- common/usb_hub.c | 228 ++++++++++++++++++++--------------- common/usb_kbd.c | 24 ++--- common/usb_storage.c | 283 +++++++++++++++++++++---------------------- drivers/usb/host/ehci-hcd.c | 10 -- include/common.h | 2 + include/usb.h | 6 + include/usb_defs.h | 26 ++++- 9 files changed, 389 insertions(+), 330 deletions(-)
USB_PRINTF, USB_HUB_PRINTF, USB_STOR_PRINTF, USB_KBD_PRINTF are nothing but conditional debug prints, depending on DEBUG. So better remove them and use debug() simply.
Signed-off-by: Vivek Gautam gautam.vivek@samsung.com ---
This patch added in V3(current-version) of this patch-series.
common/usb.c | 85 ++++++++---------- common/usb_hub.c | 162 +++++++++++++++----------------- common/usb_kbd.c | 24 ++--- common/usb_storage.c | 253 ++++++++++++++++++++++++-------------------------- 4 files changed, 241 insertions(+), 283 deletions(-)
diff --git a/common/usb.c b/common/usb.c index 6fc0fc1..50fa466 100644 --- a/common/usb.c +++ b/common/usb.c @@ -57,17 +57,6 @@ #include <asm/4xx_pci.h> #endif
-#ifdef DEBUG -#define USB_DEBUG 1 -#define USB_HUB_DEBUG 1 -#else -#define USB_DEBUG 0 -#define USB_HUB_DEBUG 0 -#endif - -#define USB_PRINTF(fmt, args...) debug_cond(USB_DEBUG, fmt, ##args) -#define USB_HUB_PRINTF(fmt, args...) debug_cond(USB_HUB_DEBUG, fmt, ##args) - #define USB_BUFSIZ 512
static struct usb_device usb_dev[USB_MAX_DEVICE]; @@ -130,7 +119,7 @@ int usb_init(void) usb_started = 1; }
- USB_PRINTF("scan end\n"); + debug("scan end\n"); /* if we were not able to find at least one working bus, bail out */ if (!usb_started) { puts("USB error: all controllers failed lowlevel init\n"); @@ -216,9 +205,9 @@ int usb_control_msg(struct usb_device *dev, unsigned int pipe, setup_packet->value = cpu_to_le16(value); setup_packet->index = cpu_to_le16(index); setup_packet->length = cpu_to_le16(size); - USB_PRINTF("usb_control_msg: request: 0x%X, requesttype: 0x%X, " \ - "value 0x%X index 0x%X length 0x%X\n", - request, requesttype, value, index, size); + debug("usb_control_msg: request: 0x%X, requesttype: 0x%X, " \ + "value 0x%X index 0x%X length 0x%X\n", + request, requesttype, value, index, size); dev->status = USB_ST_NOT_PROC; /*not yet processed */
if (submit_control_msg(dev, pipe, data, size, setup_packet) < 0) @@ -314,22 +303,22 @@ usb_set_maxpacket_ep(struct usb_device *dev, int if_idx, int ep_idx) /* Control => bidirectional */ dev->epmaxpacketout[b] = ep_wMaxPacketSize; dev->epmaxpacketin[b] = ep_wMaxPacketSize; - USB_PRINTF("##Control EP epmaxpacketout/in[%d] = %d\n", - b, dev->epmaxpacketin[b]); + debug("##Control EP epmaxpacketout/in[%d] = %d\n", + b, dev->epmaxpacketin[b]); } else { if ((ep->bEndpointAddress & 0x80) == 0) { /* OUT Endpoint */ if (ep_wMaxPacketSize > dev->epmaxpacketout[b]) { dev->epmaxpacketout[b] = ep_wMaxPacketSize; - USB_PRINTF("##EP epmaxpacketout[%d] = %d\n", - b, dev->epmaxpacketout[b]); + debug("##EP epmaxpacketout[%d] = %d\n", + b, dev->epmaxpacketout[b]); } } else { /* IN Endpoint */ if (ep_wMaxPacketSize > dev->epmaxpacketin[b]) { dev->epmaxpacketin[b] = ep_wMaxPacketSize; - USB_PRINTF("##EP epmaxpacketin[%d] = %d\n", - b, dev->epmaxpacketin[b]); + debug("##EP epmaxpacketin[%d] = %d\n", + b, dev->epmaxpacketin[b]); } } /* if out */ } /* if control */ @@ -358,7 +347,6 @@ static int usb_parse_config(struct usb_device *dev, { struct usb_descriptor_header *head; int index, ifno, epno, curr_if_num; - int i; u16 ep_wMaxPacketSize;
ifno = -1; @@ -414,23 +402,25 @@ static int usb_parse_config(struct usb_device *dev, if_desc[ifno].\ ep_desc[epno].\ wMaxPacketSize); - USB_PRINTF("if %d, ep %d\n", ifno, epno); + debug("if %d, ep %d\n", ifno, epno); break; default: if (head->bLength == 0) return 1;
- USB_PRINTF("unknown Description Type : %x\n", - head->bDescriptorType); + debug("unknown Description Type : %x\n", + head->bDescriptorType);
+#ifdef DEBUG { -#ifdef USB_DEBUG unsigned char *ch = (unsigned char *)head; -#endif + int i; + for (i = 0; i < head->bLength; i++) - USB_PRINTF("%02X ", *ch++); - USB_PRINTF("\n\n\n"); + debug("%02X ", *ch++); + debug("\n\n\n"); } +#endif break; } index += head->bLength; @@ -514,8 +504,7 @@ int usb_get_configuration_no(struct usb_device *dev, }
result = usb_get_descriptor(dev, USB_DT_CONFIG, cfgno, buffer, tmp); - USB_PRINTF("get_conf_no %d Result %d, wLength %d\n", - cfgno, result, tmp); + debug("get_conf_no %d Result %d, wLength %d\n", cfgno, result, tmp); return result; }
@@ -527,7 +516,7 @@ static int usb_set_address(struct usb_device *dev) { int res;
- USB_PRINTF("set address %d\n", dev->devnum); + debug("set address %d\n", dev->devnum); res = usb_control_msg(dev, usb_snddefctrl(dev), USB_REQ_SET_ADDRESS, 0, (dev->devnum), 0, @@ -579,7 +568,7 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate) static int usb_set_configuration(struct usb_device *dev, int configuration) { int res; - USB_PRINTF("set configuration %d\n", configuration); + debug("set configuration %d\n", configuration); /* set setup command */ res = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), USB_REQ_SET_CONFIGURATION, 0, @@ -731,19 +720,19 @@ int usb_string(struct usb_device *dev, int index, char *buf, size_t size) if (!dev->have_langid) { err = usb_string_sub(dev, 0, 0, tbuf); if (err < 0) { - USB_PRINTF("error getting string descriptor 0 " \ - "(error=%lx)\n", dev->status); + debug("error getting string descriptor 0 " \ + "(error=%lx)\n", dev->status); return -1; } else if (tbuf[0] < 4) { - USB_PRINTF("string descriptor 0 too short\n"); + debug("string descriptor 0 too short\n"); return -1; } else { dev->have_langid = -1; dev->string_langid = tbuf[2] | (tbuf[3] << 8); /* always use the first langid listed */ - USB_PRINTF("USB device number %d default " \ - "language ID 0x%x\n", - dev->devnum, dev->string_langid); + debug("USB device number %d default " \ + "language ID 0x%x\n", + dev->devnum, dev->string_langid); } }
@@ -789,7 +778,7 @@ struct usb_device *usb_get_dev_index(int index) struct usb_device *usb_alloc_new_device(void *controller) { int i; - USB_PRINTF("New Device %d\n", dev_index); + debug("New Device %d\n", dev_index); if (dev_index == USB_MAX_DEVICE) { printf("ERROR, too many USB Devices, max=%d\n", USB_MAX_DEVICE); return NULL; @@ -813,7 +802,7 @@ struct usb_device *usb_alloc_new_device(void *controller) void usb_free_device(void) { dev_index--; - USB_PRINTF("Freeing device node: %d\n", dev_index); + debug("Freeing device node: %d\n", dev_index); memset(&usb_dev[dev_index], 0, sizeof(struct usb_device)); usb_dev[dev_index].devnum = -1; } @@ -880,7 +869,7 @@ int usb_new_device(struct usb_device *dev)
err = usb_get_descriptor(dev, USB_DT_DEVICE, 0, desc, 64); if (err < 0) { - USB_PRINTF("usb_new_device: usb_get_descriptor() failed\n"); + debug("usb_new_device: usb_get_descriptor() failed\n"); return 1; }
@@ -973,9 +962,9 @@ int usb_new_device(struct usb_device *dev) "len %d, status %lX\n", dev->act_len, dev->status); return -1; } - USB_PRINTF("new device strings: Mfr=%d, Product=%d, SerialNumber=%d\n", - dev->descriptor.iManufacturer, dev->descriptor.iProduct, - dev->descriptor.iSerialNumber); + debug("new device strings: Mfr=%d, Product=%d, SerialNumber=%d\n", + dev->descriptor.iManufacturer, dev->descriptor.iProduct, + dev->descriptor.iSerialNumber); memset(dev->mf, 0, sizeof(dev->mf)); memset(dev->prod, 0, sizeof(dev->prod)); memset(dev->serial, 0, sizeof(dev->serial)); @@ -988,9 +977,9 @@ int usb_new_device(struct usb_device *dev) if (dev->descriptor.iSerialNumber) usb_string(dev, dev->descriptor.iSerialNumber, dev->serial, sizeof(dev->serial)); - USB_PRINTF("Manufacturer %s\n", dev->mf); - USB_PRINTF("Product %s\n", dev->prod); - USB_PRINTF("SerialNumber %s\n", dev->serial); + debug("Manufacturer %s\n", dev->mf); + debug("Product %s\n", dev->prod); + debug("SerialNumber %s\n", dev->serial); /* now prode if the device is a hub */ usb_hub_probe(dev, 0); return 0; diff --git a/common/usb_hub.c b/common/usb_hub.c index b5eeb62..f2a0285 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -53,17 +53,6 @@ #include <asm/4xx_pci.h> #endif
-#ifdef DEBUG -#define USB_DEBUG 1 -#define USB_HUB_DEBUG 1 -#else -#define USB_DEBUG 0 -#define USB_HUB_DEBUG 0 -#endif - -#define USB_PRINTF(fmt, args...) debug_cond(USB_DEBUG, fmt, ##args) -#define USB_HUB_PRINTF(fmt, args...) debug_cond(USB_HUB_DEBUG, fmt, ##args) - #define USB_BUFSIZ 512
static struct usb_hub_device hub_dev[USB_MAX_HUB]; @@ -114,10 +103,10 @@ static void usb_hub_power_on(struct usb_hub_device *hub)
dev = hub->pusb_dev; /* Enable power to the ports */ - USB_HUB_PRINTF("enabling power on all ports\n"); + debug("enabling power on all ports\n"); for (i = 0; i < dev->maxchild; i++) { usb_set_port_feature(dev, i + 1, USB_PORT_FEAT_POWER); - USB_HUB_PRINTF("port %d returns %lX\n", i + 1, dev->status); + debug("port %d returns %lX\n", i + 1, dev->status); }
/* Wait at least 100 msec for power to become stable */ @@ -157,29 +146,28 @@ int hub_port_reset(struct usb_device *dev, int port, ALLOC_CACHE_ALIGN_BUFFER(struct usb_port_status, portsts, 1); unsigned short portstatus, portchange;
- USB_HUB_PRINTF("hub_port_reset: resetting port %d...\n", port); + debug("hub_port_reset: resetting port %d...\n", port); for (tries = 0; tries < MAX_TRIES; tries++) {
usb_set_port_feature(dev, port + 1, USB_PORT_FEAT_RESET); mdelay(200);
if (usb_get_port_status(dev, port + 1, portsts) < 0) { - USB_HUB_PRINTF("get_port_status failed status %lX\n", - dev->status); + debug("get_port_status failed status %lX\n", + dev->status); return -1; } portstatus = le16_to_cpu(portsts->wPortStatus); portchange = le16_to_cpu(portsts->wPortChange);
- USB_HUB_PRINTF("portstatus %x, change %x, %s\n", - portstatus, portchange, - portspeed(portstatus)); + debug("portstatus %x, change %x, %s\n", portstatus, portchange, + portspeed(portstatus));
- USB_HUB_PRINTF("STAT_C_CONNECTION = %d STAT_CONNECTION = %d" \ - " USB_PORT_STAT_ENABLE %d\n", - (portchange & USB_PORT_STAT_C_CONNECTION) ? 1 : 0, - (portstatus & USB_PORT_STAT_CONNECTION) ? 1 : 0, - (portstatus & USB_PORT_STAT_ENABLE) ? 1 : 0); + debug("STAT_C_CONNECTION = %d STAT_CONNECTION = %d" \ + " USB_PORT_STAT_ENABLE %d\n", + (portchange & USB_PORT_STAT_C_CONNECTION) ? 1 : 0, + (portstatus & USB_PORT_STAT_CONNECTION) ? 1 : 0, + (portstatus & USB_PORT_STAT_ENABLE) ? 1 : 0);
if ((portchange & USB_PORT_STAT_C_CONNECTION) || !(portstatus & USB_PORT_STAT_CONNECTION)) @@ -192,9 +180,9 @@ int hub_port_reset(struct usb_device *dev, int port, }
if (tries == MAX_TRIES) { - USB_HUB_PRINTF("Cannot enable port %i after %i retries, " \ - "disabling port.\n", port + 1, MAX_TRIES); - USB_HUB_PRINTF("Maybe the USB cable is bad?\n"); + debug("Cannot enable port %i after %i retries, " \ + "disabling port.\n", port + 1, MAX_TRIES); + debug("Maybe the USB cable is bad?\n"); return -1; }
@@ -212,15 +200,15 @@ void usb_hub_port_connect_change(struct usb_device *dev, int port)
/* Check status */ if (usb_get_port_status(dev, port + 1, portsts) < 0) { - USB_HUB_PRINTF("get_port_status failed\n"); + debug("get_port_status failed\n"); return; }
portstatus = le16_to_cpu(portsts->wPortStatus); - USB_HUB_PRINTF("portstatus %x, change %x, %s\n", - portstatus, - le16_to_cpu(portsts->wPortChange), - portspeed(portstatus)); + debug("portstatus %x, change %x, %s\n", + portstatus, + le16_to_cpu(portsts->wPortChange), + portspeed(portstatus));
/* Clear the connection change status */ usb_clear_port_feature(dev, port + 1, USB_PORT_FEAT_C_CONNECTION); @@ -228,7 +216,7 @@ void usb_hub_port_connect_change(struct usb_device *dev, int port) /* Disconnect any existing devices under this port */ if (((!(portstatus & USB_PORT_STAT_CONNECTION)) && (!(portstatus & USB_PORT_STAT_ENABLE))) || (dev->children[port])) { - USB_HUB_PRINTF("usb_disconnect(&hub->children[port]);\n"); + debug("usb_disconnect(&hub->children[port]);\n"); /* Return now if nothing is connected */ if (!(portstatus & USB_PORT_STAT_CONNECTION)) return; @@ -261,7 +249,7 @@ void usb_hub_port_connect_change(struct usb_device *dev, int port) /* Woops, disable the port */ usb_free_device(); dev->children[port] = NULL; - USB_HUB_PRINTF("hub: disabling port %d\n", port + 1); + debug("hub: disabling port %d\n", port + 1); usb_clear_port_feature(dev, port + 1, USB_PORT_FEAT_ENABLE); } } @@ -275,9 +263,7 @@ static int usb_hub_configure(struct usb_device *dev) short hubCharacteristics; struct usb_hub_descriptor *descriptor; struct usb_hub_device *hub; -#ifdef USB_HUB_DEBUG - struct usb_hub_status *hubsts; -#endif + __maybe_unused struct usb_hub_status *hubsts;
/* "allocate" Hub device */ hub = usb_hub_allocate(); @@ -286,8 +272,8 @@ static int usb_hub_configure(struct usb_device *dev) hub->pusb_dev = dev; /* Get the the hub descriptor */ if (usb_get_hub_descriptor(dev, buffer, 4) < 0) { - USB_HUB_PRINTF("usb_hub_configure: failed to get hub " \ - "descriptor, giving up %lX\n", dev->status); + debug("usb_hub_configure: failed to get hub " \ + "descriptor, giving up %lX\n", dev->status); return -1; } descriptor = (struct usb_hub_descriptor *)buffer; @@ -295,15 +281,14 @@ static int usb_hub_configure(struct usb_device *dev) /* silence compiler warning if USB_BUFSIZ is > 256 [= sizeof(char)] */ i = descriptor->bLength; if (i > USB_BUFSIZ) { - USB_HUB_PRINTF("usb_hub_configure: failed to get hub " \ - "descriptor - too long: %d\n", - descriptor->bLength); + debug("usb_hub_configure: failed to get hub " \ + "descriptor - too long: %d\n", descriptor->bLength); return -1; }
if (usb_get_hub_descriptor(dev, buffer, descriptor->bLength) < 0) { - USB_HUB_PRINTF("usb_hub_configure: failed to get hub " \ - "descriptor 2nd giving up %lX\n", dev->status); + debug("usb_hub_configure: failed to get hub " \ + "descriptor 2nd giving up %lX\n", dev->status); return -1; } memcpy((unsigned char *)&hub->desc, buffer, descriptor->bLength); @@ -325,74 +310,75 @@ static int usb_hub_configure(struct usb_device *dev) hub->desc.PortPowerCtrlMask[i] = descriptor->PortPowerCtrlMask[i];
dev->maxchild = descriptor->bNbrPorts; - USB_HUB_PRINTF("%d ports detected\n", dev->maxchild); + debug("%d ports detected\n", dev->maxchild);
hubCharacteristics = get_unaligned(&hub->desc.wHubCharacteristics); switch (hubCharacteristics & HUB_CHAR_LPSM) { case 0x00: - USB_HUB_PRINTF("ganged power switching\n"); + debug("ganged power switching\n"); break; case 0x01: - USB_HUB_PRINTF("individual port power switching\n"); + debug("individual port power switching\n"); break; case 0x02: case 0x03: - USB_HUB_PRINTF("unknown reserved power switching mode\n"); + debug("unknown reserved power switching mode\n"); break; }
if (hubCharacteristics & HUB_CHAR_COMPOUND) - USB_HUB_PRINTF("part of a compound device\n"); + debug("part of a compound device\n"); else - USB_HUB_PRINTF("standalone hub\n"); + debug("standalone hub\n");
switch (hubCharacteristics & HUB_CHAR_OCPM) { case 0x00: - USB_HUB_PRINTF("global over-current protection\n"); + debug("global over-current protection\n"); break; case 0x08: - USB_HUB_PRINTF("individual port over-current protection\n"); + debug("individual port over-current protection\n"); break; case 0x10: case 0x18: - USB_HUB_PRINTF("no over-current protection\n"); + debug("no over-current protection\n"); break; }
- USB_HUB_PRINTF("power on to power good time: %dms\n", - descriptor->bPwrOn2PwrGood * 2); - USB_HUB_PRINTF("hub controller current requirement: %dmA\n", - descriptor->bHubContrCurrent); + debug("power on to power good time: %dms\n", + descriptor->bPwrOn2PwrGood * 2); + debug("hub controller current requirement: %dmA\n", + descriptor->bHubContrCurrent);
for (i = 0; i < dev->maxchild; i++) - USB_HUB_PRINTF("port %d is%s removable\n", i + 1, - hub->desc.DeviceRemovable[(i + 1) / 8] & \ - (1 << ((i + 1) % 8)) ? " not" : ""); + debug("port %d is%s removable\n", i + 1, + hub->desc.DeviceRemovable[(i + 1) / 8] & \ + (1 << ((i + 1) % 8)) ? " not" : "");
if (sizeof(struct usb_hub_status) > USB_BUFSIZ) { - USB_HUB_PRINTF("usb_hub_configure: failed to get Status - " \ - "too long: %d\n", descriptor->bLength); + debug("usb_hub_configure: failed to get Status - " \ + "too long: %d\n", descriptor->bLength); return -1; }
if (usb_get_hub_status(dev, buffer) < 0) { - USB_HUB_PRINTF("usb_hub_configure: failed to get Status %lX\n", - dev->status); + debug("usb_hub_configure: failed to get Status %lX\n", + dev->status); return -1; }
-#ifdef USB_HUB_DEBUG +#ifdef DEBUG hubsts = (struct usb_hub_status *)buffer; #endif - USB_HUB_PRINTF("get_hub_status returned status %X, change %X\n", - le16_to_cpu(hubsts->wHubStatus), - le16_to_cpu(hubsts->wHubChange)); - USB_HUB_PRINTF("local power source is %s\n", - (le16_to_cpu(hubsts->wHubStatus) & HUB_STATUS_LOCAL_POWER) ? \ - "lost (inactive)" : "good"); - USB_HUB_PRINTF("%sover-current condition exists\n", - (le16_to_cpu(hubsts->wHubStatus) & HUB_STATUS_OVERCURRENT) ? \ - "" : "no "); + + debug("get_hub_status returned status %X, change %X\n", + le16_to_cpu(hubsts->wHubStatus), + le16_to_cpu(hubsts->wHubChange)); + debug("local power source is %s\n", + (le16_to_cpu(hubsts->wHubStatus) & HUB_STATUS_LOCAL_POWER) ? \ + "lost (inactive)" : "good"); + debug("%sover-current condition exists\n", + (le16_to_cpu(hubsts->wHubStatus) & HUB_STATUS_OVERCURRENT) ? \ + "" : "no "); usb_hub_power_on(hub);
for (i = 0; i < dev->maxchild; i++) { @@ -412,7 +398,7 @@ static int usb_hub_configure(struct usb_device *dev) do { ret = usb_get_port_status(dev, i + 1, portsts); if (ret < 0) { - USB_HUB_PRINTF("get_port_status failed\n"); + debug("get_port_status failed\n"); break; }
@@ -429,16 +415,16 @@ static int usb_hub_configure(struct usb_device *dev) if (ret < 0) continue;
- USB_HUB_PRINTF("Port %d Status %X Change %X\n", - i + 1, portstatus, portchange); + debug("Port %d Status %X Change %X\n", + i + 1, portstatus, portchange);
if (portchange & USB_PORT_STAT_C_CONNECTION) { - USB_HUB_PRINTF("port %d connection change\n", i + 1); + debug("port %d connection change\n", i + 1); usb_hub_port_connect_change(dev, i); } if (portchange & USB_PORT_STAT_C_ENABLE) { - USB_HUB_PRINTF("port %d enable change, status %x\n", - i + 1, portstatus); + debug("port %d enable change, status %x\n", + i + 1, portstatus); usb_clear_port_feature(dev, i + 1, USB_PORT_FEAT_C_ENABLE);
@@ -448,27 +434,27 @@ static int usb_hub_configure(struct usb_device *dev) if (!(portstatus & USB_PORT_STAT_ENABLE) && (portstatus & USB_PORT_STAT_CONNECTION) && ((dev->children[i]))) { - USB_HUB_PRINTF("already running port %i " \ - "disabled by hub (EMI?), " \ - "re-enabling...\n", i + 1); - usb_hub_port_connect_change(dev, i); + debug("already running port %i " \ + "disabled by hub (EMI?), " \ + "re-enabling...\n", i + 1); + usb_hub_port_connect_change(dev, i); } } if (portstatus & USB_PORT_STAT_SUSPEND) { - USB_HUB_PRINTF("port %d suspend change\n", i + 1); + debug("port %d suspend change\n", i + 1); usb_clear_port_feature(dev, i + 1, USB_PORT_FEAT_SUSPEND); }
if (portchange & USB_PORT_STAT_C_OVERCURRENT) { - USB_HUB_PRINTF("port %d over-current change\n", i + 1); + debug("port %d over-current change\n", i + 1); usb_clear_port_feature(dev, i + 1, USB_PORT_FEAT_C_OVER_CURRENT); usb_hub_power_on(hub); }
if (portchange & USB_PORT_STAT_C_RESET) { - USB_HUB_PRINTF("port %d reset change\n", i + 1); + debug("port %d reset change\n", i + 1); usb_clear_port_feature(dev, i + 1, USB_PORT_FEAT_C_RESET); } @@ -503,7 +489,7 @@ int usb_hub_probe(struct usb_device *dev, int ifnum) if ((ep->bmAttributes & 3) != 3) return 0; /* We found a hub */ - USB_HUB_PRINTF("USB hub found\n"); + debug("USB hub found\n"); ret = usb_hub_configure(dev); return ret; } diff --git a/common/usb_kbd.c b/common/usb_kbd.c index 4efbcfe..b962849 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -31,12 +31,6 @@
#include <usb.h>
-#ifdef USB_KBD_DEBUG -#define USB_KBD_PRINTF(fmt, args...) printf(fmt, ##args) -#else -#define USB_KBD_PRINTF(fmt, args...) -#endif - /* * If overwrite_console returns 1, the stdin, stderr and stdout * are switched to the serial port, else the settings in the @@ -262,7 +256,7 @@ static int usb_kbd_translate(struct usb_kbd_pdata *data, unsigned char scancode,
/* Report keycode if any */ if (keycode) { - USB_KBD_PRINTF("%c", keycode); + debug("%c", keycode); usb_kbd_put_queue(data, keycode); }
@@ -324,8 +318,8 @@ static int usb_kbd_irq_worker(struct usb_device *dev) static int usb_kbd_irq(struct usb_device *dev) { if ((dev->irq_status != 0) || (dev->irq_act_len != 8)) { - USB_KBD_PRINTF("USB KBD: Error %lX, len %d\n", - dev->irq_status, dev->irq_act_len); + debug("USB KBD: Error %lX, len %d\n", + dev->irq_status, dev->irq_act_len); return 1; }
@@ -437,7 +431,7 @@ static int usb_kbd_probe(struct usb_device *dev, unsigned int ifnum) if ((ep->bmAttributes & 3) != 3) return 0;
- USB_KBD_PRINTF("USB KBD: found set protocol...\n"); + debug("USB KBD: found set protocol...\n");
data = malloc(sizeof(struct usb_kbd_pdata)); if (!data) { @@ -463,10 +457,10 @@ static int usb_kbd_probe(struct usb_device *dev, unsigned int ifnum) /* We found a USB Keyboard, install it. */ usb_set_protocol(dev, iface->desc.bInterfaceNumber, 0);
- USB_KBD_PRINTF("USB KBD: found set idle...\n"); + debug("USB KBD: found set idle...\n"); usb_set_idle(dev, iface->desc.bInterfaceNumber, REPEAT_RATE, 0);
- USB_KBD_PRINTF("USB KBD: enable interrupt pipe...\n"); + debug("USB KBD: enable interrupt pipe...\n"); usb_submit_int_msg(dev, pipe, data->new, maxp > 8 ? 8 : maxp, ep->bInterval);
@@ -497,16 +491,16 @@ int drv_usb_kbd_init(void) continue;
/* We found a keyboard, check if it is already registered. */ - USB_KBD_PRINTF("USB KBD: found set up device.\n"); + debug("USB KBD: found set up device.\n"); old_dev = stdio_get_by_name(DEVNAME); if (old_dev) { /* Already registered, just return ok. */ - USB_KBD_PRINTF("USB KBD: is already registered.\n"); + debug("USB KBD: is already registered.\n"); return 1; }
/* Register the keyboard */ - USB_KBD_PRINTF("USB KBD: register.\n"); + debug("USB KBD: register.\n"); memset(&usb_kbd_dev, 0, sizeof(struct stdio_dev)); strcpy(usb_kbd_dev.name, DEVNAME); usb_kbd_dev.flags = DEV_FLAGS_INPUT | DEV_FLAGS_SYSTEM; diff --git a/common/usb_storage.c b/common/usb_storage.c index fb322b4..983cbab 100644 --- a/common/usb_storage.c +++ b/common/usb_storage.c @@ -59,14 +59,6 @@ #undef BBB_COMDAT_TRACE #undef BBB_XPORT_TRACE
-#ifdef USB_STOR_DEBUG -#define USB_BLK_DEBUG 1 -#else -#define USB_BLK_DEBUG 0 -#endif - -#define USB_STOR_PRINTF(fmt, args...) debug_cond(USB_BLK_DEBUG, fmt, ##args) - #include <scsi.h> /* direction table -- this indicates the direction of the data * transfer for each command code -- a 1 indicates input @@ -228,8 +220,7 @@ static unsigned int usb_get_max_lun(struct us_data *us) 0, us->ifnum, result, sizeof(char), USB_CNTL_TIMEOUT * 5); - USB_STOR_PRINTF("Get Max LUN -> len = %i, result = %i\n", - len, (int) *result); + debug("Get Max LUN -> len = %i, result = %i\n", len, (int) *result); return (len > 0) ? *result : 0; }
@@ -262,7 +253,7 @@ int usb_stor_scan(int mode) usb_max_devs = 0; for (i = 0; i < USB_MAX_DEVICE; i++) { dev = usb_get_dev_index(i); /* get device */ - USB_STOR_PRINTF("i=%d\n", i); + debug("i=%d\n", i); if (dev == NULL) break; /* no more devices available */
@@ -309,7 +300,7 @@ static int usb_stor_irq(struct usb_device *dev) }
-#ifdef USB_STOR_DEBUG +#ifdef DEBUG
static void usb_show_srb(ccb *pccb) { @@ -361,45 +352,49 @@ static int us_one_transfer(struct us_data *us, int pipe, char *buf, int length) /* set up the transfer loop */ do { /* transfer the data */ - USB_STOR_PRINTF("Bulk xfer 0x%x(%d) try #%d\n", - (unsigned int)buf, this_xfer, 11 - maxtry); + debug("Bulk xfer 0x%x(%d) try #%d\n", + (unsigned int)buf, this_xfer, 11 - maxtry); result = usb_bulk_msg(us->pusb_dev, pipe, buf, this_xfer, &partial, USB_CNTL_TIMEOUT * 5); - USB_STOR_PRINTF("bulk_msg returned %d xferred %d/%d\n", - result, partial, this_xfer); + debug("bulk_msg returned %d xferred %d/%d\n", + result, partial, this_xfer); if (us->pusb_dev->status != 0) { /* if we stall, we need to clear it before * we go on */ -#ifdef USB_STOR_DEBUG +#ifdef DEBUG display_int_status(us->pusb_dev->status); #endif if (us->pusb_dev->status & USB_ST_STALLED) { - USB_STOR_PRINTF("stalled ->clearing endpoint halt for pipe 0x%x\n", pipe); + debug("stalled ->clearing endpoint" \ + "halt for pipe 0x%x\n", pipe); stat = us->pusb_dev->status; usb_clear_halt(us->pusb_dev, pipe); us->pusb_dev->status = stat; if (this_xfer == partial) { - USB_STOR_PRINTF("bulk transferred with error %lX, but data ok\n", us->pusb_dev->status); + debug("bulk transferred" \ + "with error %lX," \ + " but data ok\n", + us->pusb_dev->status); return 0; } else return result; } if (us->pusb_dev->status & USB_ST_NAK_REC) { - USB_STOR_PRINTF("Device NAKed bulk_msg\n"); + debug("Device NAKed bulk_msg\n"); return result; } - USB_STOR_PRINTF("bulk transferred with error"); + debug("bulk transferred with error"); if (this_xfer == partial) { - USB_STOR_PRINTF(" %ld, but data ok\n", - us->pusb_dev->status); + debug(" %ld, but data ok\n", + us->pusb_dev->status); return 0; } /* if our try counter reaches 0, bail out */ - USB_STOR_PRINTF(" %ld, data %d\n", - us->pusb_dev->status, partial); + debug(" %ld, data %d\n", + us->pusb_dev->status, partial); if (!maxtry--) return result; } @@ -433,35 +428,34 @@ static int usb_stor_BBB_reset(struct us_data *us) * * This comment stolen from FreeBSD's /sys/dev/usb/umass.c. */ - USB_STOR_PRINTF("BBB_reset\n"); + debug("BBB_reset\n"); result = usb_control_msg(us->pusb_dev, usb_sndctrlpipe(us->pusb_dev, 0), US_BBB_RESET, USB_TYPE_CLASS | USB_RECIP_INTERFACE, 0, us->ifnum, NULL, 0, USB_CNTL_TIMEOUT * 5);
if ((result < 0) && (us->pusb_dev->status & USB_ST_STALLED)) { - USB_STOR_PRINTF("RESET:stall\n"); + debug("RESET:stall\n"); return -1; }
/* long wait for reset */ mdelay(150); - USB_STOR_PRINTF("BBB_reset result %d: status %lX reset\n", result, - us->pusb_dev->status); + debug("BBB_reset result %d: status %lX reset\n", + result, us->pusb_dev->status); pipe = usb_rcvbulkpipe(us->pusb_dev, us->ep_in); result = usb_clear_halt(us->pusb_dev, pipe); /* long wait for reset */ mdelay(150); - USB_STOR_PRINTF("BBB_reset result %d: status %lX clearing IN endpoint\n", - result, us->pusb_dev->status); + debug("BBB_reset result %d: status %lX clearing IN endpoint\n", + result, us->pusb_dev->status); /* long wait for reset */ pipe = usb_sndbulkpipe(us->pusb_dev, us->ep_out); result = usb_clear_halt(us->pusb_dev, pipe); mdelay(150); - USB_STOR_PRINTF("BBB_reset result %d: status %lX" - " clearing OUT endpoint\n", result, - us->pusb_dev->status); - USB_STOR_PRINTF("BBB_reset done\n"); + debug("BBB_reset result %d: status %lX clearing OUT endpoint\n", + result, us->pusb_dev->status); + debug("BBB_reset done\n"); return 0; }
@@ -474,7 +468,7 @@ static int usb_stor_CB_reset(struct us_data *us) unsigned char cmd[12]; int result;
- USB_STOR_PRINTF("CB_reset\n"); + debug("CB_reset\n"); memset(cmd, 0xff, sizeof(cmd)); cmd[0] = SCSI_SEND_DIAG; cmd[1] = 4; @@ -486,13 +480,12 @@ static int usb_stor_CB_reset(struct us_data *us)
/* long wait for reset */ mdelay(1500); - USB_STOR_PRINTF("CB_reset result %d: status %lX" - " clearing endpoint halt\n", result, - us->pusb_dev->status); + debug("CB_reset result %d: status %lX clearing endpoint halt\n", + result, us->pusb_dev->status); usb_clear_halt(us->pusb_dev, usb_rcvbulkpipe(us->pusb_dev, us->ep_in)); usb_clear_halt(us->pusb_dev, usb_rcvbulkpipe(us->pusb_dev, us->ep_out));
- USB_STOR_PRINTF("CB_reset done\n"); + debug("CB_reset done\n"); return 0; }
@@ -522,7 +515,7 @@ static int usb_stor_BBB_comdat(ccb *srb, struct us_data *us) #endif /* sanity checks */ if (!(srb->cmdlen <= CBWCDBLENGTH)) { - USB_STOR_PRINTF("usb_stor_BBB_comdat:cmdlen too large\n"); + debug("usb_stor_BBB_comdat:cmdlen too large\n"); return -1; }
@@ -541,7 +534,7 @@ static int usb_stor_BBB_comdat(ccb *srb, struct us_data *us) result = usb_bulk_msg(us->pusb_dev, pipe, cbw, UMASS_BBB_CBW_SIZE, &actlen, USB_CNTL_TIMEOUT * 5); if (result < 0) - USB_STOR_PRINTF("usb_stor_BBB_comdat:usb_bulk_msg error\n"); + debug("usb_stor_BBB_comdat:usb_bulk_msg error\n"); return result; }
@@ -564,8 +557,8 @@ static int usb_stor_CB_comdat(ccb *srb, struct us_data *us) pipe = usb_sndbulkpipe(us->pusb_dev, us->ep_out);
while (retry--) { - USB_STOR_PRINTF("CBI gets a command: Try %d\n", 5 - retry); -#ifdef USB_STOR_DEBUG + debug("CBI gets a command: Try %d\n", 5 - retry); +#ifdef DEBUG usb_show_srb(srb); #endif /* let's send the command via the control pipe */ @@ -576,35 +569,35 @@ static int usb_stor_CB_comdat(ccb *srb, struct us_data *us) 0, us->ifnum, srb->cmd, srb->cmdlen, USB_CNTL_TIMEOUT * 5); - USB_STOR_PRINTF("CB_transport: control msg returned %d," - " status %lX\n", result, us->pusb_dev->status); + debug("CB_transport: control msg returned %d, status %lX\n", + result, us->pusb_dev->status); /* check the return code for the command */ if (result < 0) { if (us->pusb_dev->status & USB_ST_STALLED) { status = us->pusb_dev->status; - USB_STOR_PRINTF(" stall during command found," - " clear pipe\n"); + debug(" stall during command found," \ + " clear pipe\n"); usb_clear_halt(us->pusb_dev, usb_sndctrlpipe(us->pusb_dev, 0)); us->pusb_dev->status = status; } - USB_STOR_PRINTF(" error during command %02X" - " Stat = %lX\n", srb->cmd[0], - us->pusb_dev->status); + debug(" error during command %02X" \ + " Stat = %lX\n", srb->cmd[0], + us->pusb_dev->status); return result; } /* transfer the data payload for this command, if one exists*/
- USB_STOR_PRINTF("CB_transport: control msg returned %d," - " direction is %s to go 0x%lx\n", result, - dir_in ? "IN" : "OUT", srb->datalen); + debug("CB_transport: control msg returned %d," \ + " direction is %s to go 0x%lx\n", result, + dir_in ? "IN" : "OUT", srb->datalen); if (srb->datalen) { result = us_one_transfer(us, pipe, (char *)srb->pdata, srb->datalen); - USB_STOR_PRINTF("CBI attempted to transfer data," - " result is %d status %lX, len %d\n", - result, us->pusb_dev->status, - us->pusb_dev->act_len); + debug("CBI attempted to transfer data," \ + " result is %d status %lX, len %d\n", + result, us->pusb_dev->status, + us->pusb_dev->act_len); if (!(us->pusb_dev->status & USB_ST_NAK_REC)) break; } /* if (srb->datalen) */ @@ -635,10 +628,9 @@ static int usb_stor_CBI_get_status(ccb *srb, struct us_data *us) us->ip_wanted = 0; return USB_STOR_TRANSPORT_ERROR; } - USB_STOR_PRINTF - ("Got interrupt data 0x%x, transfered %d status 0x%lX\n", - us->ip_data, us->pusb_dev->irq_act_len, - us->pusb_dev->irq_status); + debug("Got interrupt data 0x%x, transfered %d status 0x%lX\n", + us->ip_data, us->pusb_dev->irq_act_len, + us->pusb_dev->irq_status); /* UFI gives us ASC and ASCQ, like a request sense */ if (us->subclass == US_SC_UFI) { if (srb->cmd[0] == SCSI_REQ_SENSE || @@ -691,11 +683,11 @@ static int usb_stor_BBB_transport(ccb *srb, struct us_data *us) dir_in = US_DIRECTION(srb->cmd[0]);
/* COMMAND phase */ - USB_STOR_PRINTF("COMMAND phase\n"); + debug("COMMAND phase\n"); result = usb_stor_BBB_comdat(srb, us); if (result < 0) { - USB_STOR_PRINTF("failed to send CBW status %ld\n", - us->pusb_dev->status); + debug("failed to send CBW status %ld\n", + us->pusb_dev->status); usb_stor_BBB_reset(us); return USB_STOR_TRANSPORT_FAILED; } @@ -708,7 +700,7 @@ static int usb_stor_BBB_transport(ccb *srb, struct us_data *us) /* no data, go immediately to the STATUS phase */ if (srb->datalen == 0) goto st; - USB_STOR_PRINTF("DATA phase\n"); + debug("DATA phase\n"); if (dir_in) pipe = pipein; else @@ -717,7 +709,7 @@ static int usb_stor_BBB_transport(ccb *srb, struct us_data *us) &data_actlen, USB_CNTL_TIMEOUT * 5); /* special handling of STALL in DATA phase */ if ((result < 0) && (us->pusb_dev->status & USB_ST_STALLED)) { - USB_STOR_PRINTF("DATA:stall\n"); + debug("DATA:stall\n"); /* clear the STALL on the endpoint */ result = usb_stor_BBB_clear_endpt_stall(us, dir_in ? us->ep_in : us->ep_out); @@ -726,8 +718,8 @@ static int usb_stor_BBB_transport(ccb *srb, struct us_data *us) goto st; } if (result < 0) { - USB_STOR_PRINTF("usb_bulk_msg error status %ld\n", - us->pusb_dev->status); + debug("usb_bulk_msg error status %ld\n", + us->pusb_dev->status); usb_stor_BBB_reset(us); return USB_STOR_TRANSPORT_FAILED; } @@ -740,14 +732,14 @@ static int usb_stor_BBB_transport(ccb *srb, struct us_data *us) st: retry = 0; again: - USB_STOR_PRINTF("STATUS phase\n"); + debug("STATUS phase\n"); result = usb_bulk_msg(us->pusb_dev, pipein, csw, UMASS_BBB_CSW_SIZE, &actlen, USB_CNTL_TIMEOUT*5);
/* special handling of STALL in STATUS phase */ if ((result < 0) && (retry < 1) && (us->pusb_dev->status & USB_ST_STALLED)) { - USB_STOR_PRINTF("STATUS:stall\n"); + debug("STATUS:stall\n"); /* clear the STALL on the endpoint */ result = usb_stor_BBB_clear_endpt_stall(us, us->ep_in); if (result >= 0 && (retry++ < 1)) @@ -755,8 +747,8 @@ again: goto again; } if (result < 0) { - USB_STOR_PRINTF("usb_bulk_msg error status %ld\n", - us->pusb_dev->status); + debug("usb_bulk_msg error status %ld\n", + us->pusb_dev->status); usb_stor_BBB_reset(us); return USB_STOR_TRANSPORT_FAILED; } @@ -771,27 +763,27 @@ again: if (pipe == 0 && srb->datalen != 0 && srb->datalen - data_actlen != 0) pipe = srb->datalen - data_actlen; if (CSWSIGNATURE != le32_to_cpu(csw->dCSWSignature)) { - USB_STOR_PRINTF("!CSWSIGNATURE\n"); + debug("!CSWSIGNATURE\n"); usb_stor_BBB_reset(us); return USB_STOR_TRANSPORT_FAILED; } else if ((CBWTag - 1) != le32_to_cpu(csw->dCSWTag)) { - USB_STOR_PRINTF("!Tag\n"); + debug("!Tag\n"); usb_stor_BBB_reset(us); return USB_STOR_TRANSPORT_FAILED; } else if (csw->bCSWStatus > CSWSTATUS_PHASE) { - USB_STOR_PRINTF(">PHASE\n"); + debug(">PHASE\n"); usb_stor_BBB_reset(us); return USB_STOR_TRANSPORT_FAILED; } else if (csw->bCSWStatus == CSWSTATUS_PHASE) { - USB_STOR_PRINTF("=PHASE\n"); + debug("=PHASE\n"); usb_stor_BBB_reset(us); return USB_STOR_TRANSPORT_FAILED; } else if (data_actlen > srb->datalen) { - USB_STOR_PRINTF("transferred %dB instead of %ldB\n", - data_actlen, srb->datalen); + debug("transferred %dB instead of %ldB\n", + data_actlen, srb->datalen); return USB_STOR_TRANSPORT_FAILED; } else if (csw->bCSWStatus == CSWSTATUS_FAILED) { - USB_STOR_PRINTF("FAILED\n"); + debug("FAILED\n"); return USB_STOR_TRANSPORT_FAILED; }
@@ -812,14 +804,14 @@ static int usb_stor_CB_transport(ccb *srb, struct us_data *us) /* issue the command */ do_retry: result = usb_stor_CB_comdat(srb, us); - USB_STOR_PRINTF("command / Data returned %d, status %lX\n", - result, us->pusb_dev->status); + debug("command / Data returned %d, status %lX\n", + result, us->pusb_dev->status); /* if this is an CBI Protocol, get IRQ */ if (us->protocol == US_PR_CBI) { status = usb_stor_CBI_get_status(srb, us); /* if the status is error, report it */ if (status == USB_STOR_TRANSPORT_ERROR) { - USB_STOR_PRINTF(" USB CBI Command Error\n"); + debug(" USB CBI Command Error\n"); return status; } srb->sense_buf[12] = (unsigned char)(us->ip_data >> 8); @@ -827,7 +819,7 @@ do_retry: if (!us->ip_data) { /* if the status is good, report it */ if (status == USB_STOR_TRANSPORT_GOOD) { - USB_STOR_PRINTF(" USB CBI Command Good\n"); + debug(" USB CBI Command Good\n"); return status; } } @@ -835,7 +827,7 @@ do_retry: /* do we have to issue an auto request? */ /* HERE we have to check the result */ if ((result < 0) && !(us->pusb_dev->status & USB_ST_STALLED)) { - USB_STOR_PRINTF("ERROR %lX\n", us->pusb_dev->status); + debug("ERROR %lX\n", us->pusb_dev->status); us->transport_reset(us); return USB_STOR_TRANSPORT_ERROR; } @@ -843,7 +835,7 @@ do_retry: ((srb->cmd[0] == SCSI_REQ_SENSE) || (srb->cmd[0] == SCSI_INQUIRY))) { /* do not issue an autorequest after request sense */ - USB_STOR_PRINTF("No auto request and good\n"); + debug("No auto request and good\n"); return USB_STOR_TRANSPORT_GOOD; } /* issue an request_sense */ @@ -856,19 +848,19 @@ do_retry: psrb->cmdlen = 12; /* issue the command */ result = usb_stor_CB_comdat(psrb, us); - USB_STOR_PRINTF("auto request returned %d\n", result); + debug("auto request returned %d\n", result); /* if this is an CBI Protocol, get IRQ */ if (us->protocol == US_PR_CBI) status = usb_stor_CBI_get_status(psrb, us);
if ((result < 0) && !(us->pusb_dev->status & USB_ST_STALLED)) { - USB_STOR_PRINTF(" AUTO REQUEST ERROR %ld\n", - us->pusb_dev->status); + debug(" AUTO REQUEST ERROR %ld\n", + us->pusb_dev->status); return USB_STOR_TRANSPORT_ERROR; } - USB_STOR_PRINTF("autorequest returned 0x%02X 0x%02X 0x%02X 0x%02X\n", - srb->sense_buf[0], srb->sense_buf[2], - srb->sense_buf[12], srb->sense_buf[13]); + debug("autorequest returned 0x%02X 0x%02X 0x%02X 0x%02X\n", + srb->sense_buf[0], srb->sense_buf[2], + srb->sense_buf[12], srb->sense_buf[13]); /* Check the auto request result */ if ((srb->sense_buf[2] == 0) && (srb->sense_buf[12] == 0) && @@ -923,7 +915,7 @@ static int usb_inquiry(ccb *srb, struct us_data *ss) srb->datalen = 36; srb->cmdlen = 12; i = ss->transport(srb, ss); - USB_STOR_PRINTF("inquiry returns %d\n", i); + debug("inquiry returns %d\n", i); if (i == 0) break; } while (--retry); @@ -948,9 +940,9 @@ static int usb_request_sense(ccb *srb, struct us_data *ss) srb->pdata = &srb->sense_buf[0]; srb->cmdlen = 12; ss->transport(srb, ss); - USB_STOR_PRINTF("Request Sense returned %02X %02X %02X\n", - srb->sense_buf[2], srb->sense_buf[12], - srb->sense_buf[13]); + debug("Request Sense returned %02X %02X %02X\n", + srb->sense_buf[2], srb->sense_buf[12], + srb->sense_buf[13]); srb->pdata = (uchar *)ptr; return 0; } @@ -1017,7 +1009,7 @@ static int usb_read_10(ccb *srb, struct us_data *ss, unsigned long start, srb->cmd[7] = ((unsigned char) (blocks >> 8)) & 0xff; srb->cmd[8] = (unsigned char) blocks & 0xff; srb->cmdlen = 12; - USB_STOR_PRINTF("read10: start %lx blocks %x\n", start, blocks); + debug("read10: start %lx blocks %x\n", start, blocks); return ss->transport(srb, ss); }
@@ -1034,7 +1026,7 @@ static int usb_write_10(ccb *srb, struct us_data *ss, unsigned long start, srb->cmd[7] = ((unsigned char) (blocks >> 8)) & 0xff; srb->cmd[8] = (unsigned char) blocks & 0xff; srb->cmdlen = 12; - USB_STOR_PRINTF("write10: start %lx blocks %x\n", start, blocks); + debug("write10: start %lx blocks %x\n", start, blocks); return ss->transport(srb, ss); }
@@ -1078,7 +1070,7 @@ unsigned long usb_stor_read(int device, unsigned long blknr,
device &= 0xff; /* Setup device */ - USB_STOR_PRINTF("\nusb_read: dev %d \n", device); + debug("\nusb_read: dev %d \n", device); dev = NULL; for (i = 0; i < USB_MAX_DEVICE; i++) { dev = usb_get_dev_index(i); @@ -1095,8 +1087,8 @@ unsigned long usb_stor_read(int device, unsigned long blknr, start = blknr; blks = blkcnt;
- USB_STOR_PRINTF("\nusb_read: dev %d startblk " LBAF ", blccnt " LBAF - " buffer %lx\n", device, start, blks, buf_addr); + debug("\nusb_read: dev %d startblk " LBAF ", blccnt " LBAF + " buffer %lx\n", device, start, blks, buf_addr);
do { /* XXX need some comment here */ @@ -1112,7 +1104,7 @@ retry_it: srb->datalen = usb_dev_desc[device].blksz * smallblks; srb->pdata = (unsigned char *)buf_addr; if (usb_read_10(srb, ss, start, smallblks)) { - USB_STOR_PRINTF("Read ERROR\n"); + debug("Read ERROR\n"); usb_request_sense(srb, ss); if (retry--) goto retry_it; @@ -1125,9 +1117,9 @@ retry_it: } while (blks != 0); ss->flags &= ~USB_READY;
- USB_STOR_PRINTF("usb_read: end startblk " LBAF - ", blccnt %x buffer %lx\n", - start, smallblks, buf_addr); + debug("usb_read: end startblk " LBAF + ", blccnt %x buffer %lx\n", + start, smallblks, buf_addr);
usb_disable_asynch(0); /* asynch transfer allowed */ if (blkcnt >= USB_MAX_XFER_BLK) @@ -1151,7 +1143,7 @@ unsigned long usb_stor_write(int device, unsigned long blknr,
device &= 0xff; /* Setup device */ - USB_STOR_PRINTF("\nusb_write: dev %d \n", device); + debug("\nusb_write: dev %d \n", device); dev = NULL; for (i = 0; i < USB_MAX_DEVICE; i++) { dev = usb_get_dev_index(i); @@ -1169,8 +1161,8 @@ unsigned long usb_stor_write(int device, unsigned long blknr, start = blknr; blks = blkcnt;
- USB_STOR_PRINTF("\nusb_write: dev %d startblk " LBAF ", blccnt " LBAF - " buffer %lx\n", device, start, blks, buf_addr); + debug("\nusb_write: dev %d startblk " LBAF ", blccnt " LBAF + " buffer %lx\n", device, start, blks, buf_addr);
do { /* If write fails retry for max retry count else @@ -1188,7 +1180,7 @@ retry_it: srb->datalen = usb_dev_desc[device].blksz * smallblks; srb->pdata = (unsigned char *)buf_addr; if (usb_write_10(srb, ss, start, smallblks)) { - USB_STOR_PRINTF("Write ERROR\n"); + debug("Write ERROR\n"); usb_request_sense(srb, ss); if (retry--) goto retry_it; @@ -1201,9 +1193,8 @@ retry_it: } while (blks != 0); ss->flags &= ~USB_READY;
- USB_STOR_PRINTF("usb_write: end startblk " LBAF - ", blccnt %x buffer %lx\n", - start, smallblks, buf_addr); + debug("usb_write: end startblk " LBAF ", blccnt %x buffer %lx\n", + start, smallblks, buf_addr);
usb_disable_asynch(0); /* asynch transfer allowed */ if (blkcnt >= USB_MAX_XFER_BLK) @@ -1228,12 +1219,12 @@ int usb_storage_probe(struct usb_device *dev, unsigned int ifnum,
#if 0 /* this is the place to patch some storage devices */ - USB_STOR_PRINTF("iVendor %X iProduct %X\n", dev->descriptor.idVendor, + debug("iVendor %X iProduct %X\n", dev->descriptor.idVendor, dev->descriptor.idProduct);
if ((dev->descriptor.idVendor) == 0x066b && (dev->descriptor.idProduct) == 0x0103) { - USB_STOR_PRINTF("patched for E-USB\n"); + debug("patched for E-USB\n"); protocol = US_PR_CB; subclass = US_SC_UFI; /* an assumption */ } @@ -1250,7 +1241,7 @@ int usb_storage_probe(struct usb_device *dev, unsigned int ifnum, memset(ss, 0, sizeof(struct us_data));
/* At this point, we know we've got a live one */ - USB_STOR_PRINTF("\n\nUSB Mass Storage device detected\n"); + debug("\n\nUSB Mass Storage device detected\n");
/* Initialize the us_data structure with some useful info */ ss->flags = flags; @@ -1270,21 +1261,21 @@ int usb_storage_probe(struct usb_device *dev, unsigned int ifnum, }
/* set the handler pointers based on the protocol */ - USB_STOR_PRINTF("Transport: "); + debug("Transport: "); switch (ss->protocol) { case US_PR_CB: - USB_STOR_PRINTF("Control/Bulk\n"); + debug("Control/Bulk\n"); ss->transport = usb_stor_CB_transport; ss->transport_reset = usb_stor_CB_reset; break;
case US_PR_CBI: - USB_STOR_PRINTF("Control/Bulk/Interrupt\n"); + debug("Control/Bulk/Interrupt\n"); ss->transport = usb_stor_CB_transport; ss->transport_reset = usb_stor_CB_reset; break; case US_PR_BULK: - USB_STOR_PRINTF("Bulk/Bulk/Bulk\n"); + debug("Bulk/Bulk/Bulk\n"); ss->transport = usb_stor_BBB_transport; ss->transport_reset = usb_stor_BBB_reset; break; @@ -1320,14 +1311,14 @@ int usb_storage_probe(struct usb_device *dev, unsigned int ifnum, ss->irqinterval = iface->ep_desc[i].bInterval; } } - USB_STOR_PRINTF("Endpoints In %d Out %d Int %d\n", - ss->ep_in, ss->ep_out, ss->ep_int); + debug("Endpoints In %d Out %d Int %d\n", + ss->ep_in, ss->ep_out, ss->ep_int);
/* Do some basic sanity checks, and bail if we find a problem */ if (usb_set_interface(dev, iface->desc.bInterfaceNumber, 0) || !ss->ep_in || !ss->ep_out || (ss->protocol == US_PR_CBI && ss->ep_int == 0)) { - USB_STOR_PRINTF("Problems with device\n"); + debug("Problems with device\n"); return 0; } /* set class specific stuff */ @@ -1366,7 +1357,7 @@ int usb_stor_get_info(struct usb_device *dev, struct us_data *ss,
dev_desc->target = dev->devnum; pccb->lun = dev_desc->lun; - USB_STOR_PRINTF(" address %d\n", dev_desc->target); + debug(" address %d\n", dev_desc->target);
if (usb_inquiry(pccb, ss)) return -1; @@ -1392,8 +1383,8 @@ int usb_stor_get_info(struct usb_device *dev, struct us_data *ss, usb_bin_fixup(dev->descriptor, (uchar *)dev_desc->vendor, (uchar *)dev_desc->product); #endif /* CONFIG_USB_BIN_FIXUP */ - USB_STOR_PRINTF("ISO Vers %X, Response Data %X\n", usb_stor_buf[2], - usb_stor_buf[3]); + debug("ISO Vers %X, Response Data %X\n", usb_stor_buf[2], + usb_stor_buf[3]); if (usb_test_unit_ready(pccb, ss)) { printf("Device NOT ready\n" " Request Sense returned %02X %02X %02X\n", @@ -1413,8 +1404,7 @@ int usb_stor_get_info(struct usb_device *dev, struct us_data *ss, cap[1] = 0x200; } ss->flags &= ~USB_READY; - USB_STOR_PRINTF("Read Capacity returns: 0x%lx, 0x%lx\n", cap[0], - cap[1]); + debug("Read Capacity returns: 0x%lx, 0x%lx\n", cap[0], cap[1]); #if 0 if (cap[0] > (0x200000 * 10)) /* greater than 10 GByte */ cap[0] >>= 16; @@ -1426,16 +1416,15 @@ int usb_stor_get_info(struct usb_device *dev, struct us_data *ss, cap[0] += 1; capacity = &cap[0]; blksz = &cap[1]; - USB_STOR_PRINTF("Capacity = 0x%lx, blocksz = 0x%lx\n", - *capacity, *blksz); + debug("Capacity = 0x%lx, blocksz = 0x%lx\n", *capacity, *blksz); dev_desc->lba = *capacity; dev_desc->blksz = *blksz; dev_desc->type = perq; - USB_STOR_PRINTF(" address %d\n", dev_desc->target); - USB_STOR_PRINTF("partype: %d\n", dev_desc->part_type); + debug(" address %d\n", dev_desc->target); + debug("partype: %d\n", dev_desc->part_type);
init_part(dev_desc);
- USB_STOR_PRINTF("partype: %d\n", dev_desc->part_type); + debug("partype: %d\n", dev_desc->part_type); return 1; }
Some cleanup in usb framework, nothing much on feature side.
Signed-off-by: Vikas C Sajjan vikas.sajjan@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com ---
Changes from v2: - none
common/usb.c | 21 +++++++++++++-------- common/usb_storage.c | 30 ++++++++++++++++-------------- include/usb_defs.h | 2 +- 3 files changed, 30 insertions(+), 23 deletions(-)
diff --git a/common/usb.c b/common/usb.c index 50fa466..8407974 100644 --- a/common/usb.c +++ b/common/usb.c @@ -348,6 +348,7 @@ static int usb_parse_config(struct usb_device *dev, struct usb_descriptor_header *head; int index, ifno, epno, curr_if_num; u16 ep_wMaxPacketSize; + struct usb_interface *if_desc = NULL;
ifno = -1; epno = -1; @@ -375,23 +376,27 @@ static int usb_parse_config(struct usb_device *dev, &buffer[index])->bInterfaceNumber != curr_if_num) { /* this is a new interface, copy new desc */ ifno = dev->config.no_of_if; + if_desc = &dev->config.if_desc[ifno]; dev->config.no_of_if++; - memcpy(&dev->config.if_desc[ifno], - &buffer[index], buffer[index]); - dev->config.if_desc[ifno].no_of_ep = 0; - dev->config.if_desc[ifno].num_altsetting = 1; + memcpy(if_desc, &buffer[index], buffer[index]); + if_desc->no_of_ep = 0; + if_desc->num_altsetting = 1; curr_if_num = - dev->config.if_desc[ifno].desc.bInterfaceNumber; + if_desc->desc.bInterfaceNumber; } else { /* found alternate setting for the interface */ - dev->config.if_desc[ifno].num_altsetting++; + if (ifno >= 0) { + if_desc = &dev->config.if_desc[ifno]; + if_desc->num_altsetting++; + } } break; case USB_DT_ENDPOINT: epno = dev->config.if_desc[ifno].no_of_ep; + if_desc = &dev->config.if_desc[ifno]; /* found an endpoint */ - dev->config.if_desc[ifno].no_of_ep++; - memcpy(&dev->config.if_desc[ifno].ep_desc[epno], + if_desc->no_of_ep++; + memcpy(&if_desc->ep_desc[epno], &buffer[index], buffer[index]); ep_wMaxPacketSize = get_unaligned(&dev->config.\ if_desc[ifno].\ diff --git a/common/usb_storage.c b/common/usb_storage.c index 983cbab..2651042 100644 --- a/common/usb_storage.c +++ b/common/usb_storage.c @@ -269,9 +269,9 @@ int usb_stor_scan(int mode) lun++) { usb_dev_desc[usb_max_devs].lun = lun; if (usb_stor_get_info(dev, &usb_stor[start], - &usb_dev_desc[usb_max_devs]) == 1) { - usb_max_devs++; - } + &usb_dev_desc[usb_max_devs]) == 1) { + usb_max_devs++; + } } } /* if storage device */ @@ -504,7 +504,7 @@ static int usb_stor_BBB_comdat(ccb *srb, struct us_data *us) dir_in = US_DIRECTION(srb->cmd[0]);
#ifdef BBB_COMDAT_TRACE - printf("dir %d lun %d cmdlen %d cmd %p datalen %d pdata %p\n", + printf("dir %d lun %d cmdlen %d cmd %p datalen %lu pdata %p\n", dir_in, srb->lun, srb->cmdlen, srb->cmd, srb->datalen, srb->pdata); if (srb->cmdlen) { @@ -1209,6 +1209,7 @@ int usb_storage_probe(struct usb_device *dev, unsigned int ifnum, { struct usb_interface *iface; int i; + struct usb_endpoint_descriptor *ep_desc; unsigned int flags = 0;
int protocol = 0; @@ -1291,24 +1292,25 @@ int usb_storage_probe(struct usb_device *dev, unsigned int ifnum, * We will ignore any others. */ for (i = 0; i < iface->desc.bNumEndpoints; i++) { + ep_desc = &iface->ep_desc[i]; /* is it an BULK endpoint? */ - if ((iface->ep_desc[i].bmAttributes & + if ((ep_desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) == USB_ENDPOINT_XFER_BULK) { - if (iface->ep_desc[i].bEndpointAddress & USB_DIR_IN) - ss->ep_in = iface->ep_desc[i].bEndpointAddress & - USB_ENDPOINT_NUMBER_MASK; + if (ep_desc->bEndpointAddress & USB_DIR_IN) + ss->ep_in = ep_desc->bEndpointAddress & + USB_ENDPOINT_NUMBER_MASK; else ss->ep_out = - iface->ep_desc[i].bEndpointAddress & + ep_desc->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK; }
/* is it an interrupt endpoint? */ - if ((iface->ep_desc[i].bmAttributes & - USB_ENDPOINT_XFERTYPE_MASK) == USB_ENDPOINT_XFER_INT) { - ss->ep_int = iface->ep_desc[i].bEndpointAddress & - USB_ENDPOINT_NUMBER_MASK; - ss->irqinterval = iface->ep_desc[i].bInterval; + if ((ep_desc->bmAttributes & + USB_ENDPOINT_XFERTYPE_MASK) == USB_ENDPOINT_XFER_INT) { + ss->ep_int = ep_desc->bEndpointAddress & + USB_ENDPOINT_NUMBER_MASK; + ss->irqinterval = ep_desc->bInterval; } } debug("Endpoints In %d Out %d Int %d\n", diff --git a/include/usb_defs.h b/include/usb_defs.h index 9502544..0c78d9d 100644 --- a/include/usb_defs.h +++ b/include/usb_defs.h @@ -234,7 +234,7 @@ #define HUB_CHAR_OCPM 0x0018
/* - *Hub Status & Hub Change bit masks + * Hub Status & Hub Change bit masks */ #define HUB_STATUS_LOCAL_POWER 0x0001 #define HUB_STATUS_OVERCURRENT 0x0002
XHCI ports are powered on after a H/W reset, however EHCI ports are not. So disabling and re-enabling power on all ports invariably.
Signed-off-by: Amar amarendra.xt@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com ---
Changes from v2: - Replaced USB_HUB_PRINTFs to debug()
common/usb_hub.c | 34 ++++++++++++++++++++++++++++++++++ 1 files changed, 34 insertions(+), 0 deletions(-)
diff --git a/common/usb_hub.c b/common/usb_hub.c index f2a0285..e4f4e3c 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -100,11 +100,45 @@ static void usb_hub_power_on(struct usb_hub_device *hub) int i; struct usb_device *dev; unsigned pgood_delay = hub->desc.bPwrOn2PwrGood * 2; + ALLOC_CACHE_ALIGN_BUFFER(struct usb_port_status, portsts, 1); + unsigned short portstatus; + int ret;
dev = hub->pusb_dev; /* Enable power to the ports */ debug("enabling power on all ports\n"); for (i = 0; i < dev->maxchild; i++) { + /* + * Power-cycle the ports here: aka, + * turning them off and turning on again. + */ + usb_clear_port_feature(dev, i + 1, USB_PORT_FEAT_POWER); + debug("port %d returns %lX\n", i + 1, dev->status); + + /* Wait at least 2*bPwrOn2PwrGood for PP to change */ + mdelay(pgood_delay); + + ret = usb_get_port_status(dev, i + 1, portsts); + if (ret < 0) { + debug("port %d: get_port_status failed\n", i + 1); + return; + } + + /* + * Check to confirm the state of Port Power: + * xHCI says "After modifying PP, s/w shall read + * PP and confirm that it has reached the desired state + * before modifying it again, undefined behavior may occur + * if this procedure is not followed". + * EHCI doesn't say anything like this, but no harm in keeping + * this. + */ + portstatus = le16_to_cpu(portsts->wPortStatus); + if (portstatus & (USB_PORT_STAT_POWER << 1)) { + debug("port %d: Port power change failed\n", i + 1); + return; + } + usb_set_port_feature(dev, i + 1, USB_PORT_FEAT_POWER); debug("port %d returns %lX\n", i + 1, dev->status); }
XHCI ports are powered on after a H/W reset, however EHCI ports are not. So disabling and re-enabling power on all ports invariably.
Signed-off-by: Amar amarendra.xt@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
Changes from v2:
- Replaced USB_HUB_PRINTFs to debug()
common/usb_hub.c | 34 ++++++++++++++++++++++++++++++++++ 1 files changed, 34 insertions(+), 0 deletions(-)
diff --git a/common/usb_hub.c b/common/usb_hub.c index f2a0285..e4f4e3c 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -100,11 +100,45 @@ static void usb_hub_power_on(struct usb_hub_device *hub) int i; struct usb_device *dev; unsigned pgood_delay = hub->desc.bPwrOn2PwrGood * 2;
ALLOC_CACHE_ALIGN_BUFFER(struct usb_port_status, portsts, 1);
unsigned short portstatus;
int ret;
dev = hub->pusb_dev; /* Enable power to the ports */ debug("enabling power on all ports\n"); for (i = 0; i < dev->maxchild; i++) {
/** Power-cycle the ports here: aka,* turning them off and turning on again.*/usb_clear_port_feature(dev, i + 1, USB_PORT_FEAT_POWER);debug("port %d returns %lX\n", i + 1, dev->status);/* Wait at least 2*bPwrOn2PwrGood for PP to change */mdelay(pgood_delay);
This adds 20ms per port to USB boot times... is it really necessary? Why do we need to powercycle XHCI ports? Couldn't we just check if the ports are powered and only power them on if they aren't?
If you insist, please at least parallelize this. Disable power on all ports, wait, then check and reenable it.
ret = usb_get_port_status(dev, i + 1, portsts);if (ret < 0) {debug("port %d: get_port_status failed\n", i + 1);
This is a severe error (because the ports won't come up again), so I think it should be a printf() instead of a debug().
return;}/** Check to confirm the state of Port Power:* xHCI says "After modifying PP, s/w shall read* PP and confirm that it has reached the desired state* before modifying it again, undefined behavior may occur* if this procedure is not followed".* EHCI doesn't say anything like this, but no harm in keeping* this.*/portstatus = le16_to_cpu(portsts->wPortStatus);if (portstatus & (USB_PORT_STAT_POWER << 1)) {debug("port %d: Port power change failed\n", i + 1);
Same as above.
return;}- usb_set_port_feature(dev, i + 1, USB_PORT_FEAT_POWER); debug("port %d returns %lX\n", i + 1, dev->status); }
HI Julius,
On Sat, Apr 20, 2013 at 12:30 AM, Julius Werner jwerner@chromium.org wrote:
XHCI ports are powered on after a H/W reset, however EHCI ports are not. So disabling and re-enabling power on all ports invariably.
Signed-off-by: Amar amarendra.xt@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
Changes from v2:
- Replaced USB_HUB_PRINTFs to debug()
common/usb_hub.c | 34 ++++++++++++++++++++++++++++++++++ 1 files changed, 34 insertions(+), 0 deletions(-)
diff --git a/common/usb_hub.c b/common/usb_hub.c index f2a0285..e4f4e3c 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -100,11 +100,45 @@ static void usb_hub_power_on(struct usb_hub_device *hub) int i; struct usb_device *dev; unsigned pgood_delay = hub->desc.bPwrOn2PwrGood * 2;
ALLOC_CACHE_ALIGN_BUFFER(struct usb_port_status, portsts, 1);unsigned short portstatus;int ret; dev = hub->pusb_dev; /* Enable power to the ports */ debug("enabling power on all ports\n"); for (i = 0; i < dev->maxchild; i++) {/** Power-cycle the ports here: aka,* turning them off and turning on again.*/usb_clear_port_feature(dev, i + 1, USB_PORT_FEAT_POWER);debug("port %d returns %lX\n", i + 1, dev->status);/* Wait at least 2*bPwrOn2PwrGood for PP to change */mdelay(pgood_delay);This adds 20ms per port to USB boot times... is it really necessary? Why do we need to powercycle XHCI ports? Couldn't we just check if the ports are powered and only power them on if they aren't?
This 20ms delay is truely being taken to be on safer side (twice of Power-on-to-power-good), its not observational. In my earlier patch-series, we were doing the same way as you are suggesting here (power-on ports only if they aren't), however Marek suggested to power-cycle the ports. This would ensure that we don't have any spurious Port status (telling us that port is powered up).
Actually i was seeing a strage behavior on USB 2.0 protocol ports available with XHCI. Since all ports with XHCI are powered-up after a Chip-reset, the instant we do a power-on on these ports (as with original code - simply setting the PORT_POWER feature), the Connect status change bit used to get cleared (however Current connect status bit was still set).
So we arrived at solution that either we don't power-up XHCI ports or do a power-cycle on them.
If you insist, please at least parallelize this. Disable power on all ports, wait, then check and reenable it.
Hmm, so right now we are doing this for one port at a time. I am sure parallelising things as you suggested would be best to do here, but can you please explain, would handling one port at a time lead to unwanted behavior from Host's side somehow ?
ret = usb_get_port_status(dev, i + 1, portsts);if (ret < 0) {debug("port %d: get_port_status failed\n", i + 1);This is a severe error (because the ports won't come up again), so I think it should be a printf() instead of a debug().
Sure, will change this to pritnf()
return;}/** Check to confirm the state of Port Power:* xHCI says "After modifying PP, s/w shall read* PP and confirm that it has reached the desired state* before modifying it again, undefined behavior may occur* if this procedure is not followed".* EHCI doesn't say anything like this, but no harm in keeping* this.*/portstatus = le16_to_cpu(portsts->wPortStatus);if (portstatus & (USB_PORT_STAT_POWER << 1)) {debug("port %d: Port power change failed\n", i + 1);Same as above.
Sure, will change this.
return;}usb_set_port_feature(dev, i + 1, USB_PORT_FEAT_POWER); debug("port %d returns %lX\n", i + 1, dev->status); }
This 20ms delay is truely being taken to be on safer side (twice of Power-on-to-power-good), its not observational. In my earlier patch-series, we were doing the same way as you are suggesting here (power-on ports only if they aren't), however Marek suggested to power-cycle the ports. This would ensure that we don't have any spurious Port status (telling us that port is powered up).
Fair enough... I guess 20ms overall is not a big deal in the whole picture. I sometimes tend to overoptimize things...
Actually i was seeing a strage behavior on USB 2.0 protocol ports available with XHCI. Since all ports with XHCI are powered-up after a Chip-reset, the instant we do a power-on on these ports (as with original code - simply setting the PORT_POWER feature), the Connect status change bit used to get cleared (however Current connect status bit was still set).
This is a bug in your XHCI code I hadn't spotted before: in xhci_submit_root(SET_FEATURE) you read the PORTSC register, add a bit, and write it again... without calling xhci_port_state_to_neutral() inbetween. Thus you clear any pending change events when you set PORT_POWER. (Seems the EHCI code has a similar bug in CLEAR_FEATURE, now that I'm looking at it... someone should put a 'reg &= ~EHCI_PS_CLEAR;' in there.)
Hmm, so right now we are doing this for one port at a time. I am sure parallelising things as you suggested would be best to do here, but can you please explain, would handling one port at a time lead to unwanted behavior from Host's side somehow ?
It doesn't affect correctness, just the amount of time "wasted". Doing it one port at a time means you wait 100ms on a 5 port root hub, while you could get by with 20ms overall by parallelizing it.
Hi,
On Tue, Apr 23, 2013 at 3:32 AM, Julius Werner jwerner@chromium.org wrote:
This 20ms delay is truely being taken to be on safer side (twice of Power-on-to-power-good), its not observational. In my earlier patch-series, we were doing the same way as you are suggesting here (power-on ports only if they aren't), however Marek suggested to power-cycle the ports. This would ensure that we don't have any spurious Port status (telling us that port is powered up).
Fair enough... I guess 20ms overall is not a big deal in the whole picture. I sometimes tend to overoptimize things...
Actually i was seeing a strage behavior on USB 2.0 protocol ports available with XHCI. Since all ports with XHCI are powered-up after a Chip-reset, the instant we do a power-on on these ports (as with original code - simply setting the PORT_POWER feature), the Connect status change bit used to get cleared (however Current connect status bit was still set).
This is a bug in your XHCI code I hadn't spotted before: in xhci_submit_root(SET_FEATURE) you read the PORTSC register, add a bit, and write it again... without calling xhci_port_state_to_neutral() inbetween. Thus you clear any pending change events when you set PORT_POWER.
Right, we need to do that.
(Seems the EHCI code has a similar bug in CLEAR_FEATURE, now that I'm looking at it... someone should put a 'reg &= ~EHCI_PS_CLEAR;' in there.)
True, EHCI has it for SET_FEATURE but not for CLEAR_FEATURE.
Hmm, so right now we are doing this for one port at a time. I am sure parallelising things as you suggested would be best to do here, but can you please explain, would handling one port at a time lead to unwanted behavior from Host's side somehow ?
It doesn't affect correctness, just the amount of time "wasted". Doing it one port at a time means you wait 100ms on a 5 port root hub, while you could get by with 20ms overall by parallelizing it.
True, will amend this as required.
Fetch the device class into usb device's dwcriptors, so that the host controller's driver can use this info to differentiate between HUB and DEVICE.
Signed-off-by: Amar amarendra.xt@samsung.com ---
Changes from v2: - none
common/usb.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/common/usb.c b/common/usb.c index 8407974..3a96a34 100644 --- a/common/usb.c +++ b/common/usb.c @@ -879,6 +879,11 @@ int usb_new_device(struct usb_device *dev) }
dev->descriptor.bMaxPacketSize0 = desc->bMaxPacketSize0; + /* + * Fetch the device class, driver can use this info + * to differentiate between HUB and DEVICE. + */ + dev->descriptor.bDeviceClass = desc->bDeviceClass;
/* find the port number we're at */ if (parent) {
Fetch the device class into usb device's dwcriptors, so that the host controller's driver can use this info to differentiate between HUB and DEVICE.
Signed-off-by: Amar amarendra.xt@samsung.com
Changes from v2:
- none
common/usb.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/common/usb.c b/common/usb.c index 8407974..3a96a34 100644 --- a/common/usb.c +++ b/common/usb.c @@ -879,6 +879,11 @@ int usb_new_device(struct usb_device *dev) }
dev->descriptor.bMaxPacketSize0 = desc->bMaxPacketSize0;
- /*
* Fetch the device class, driver can use this info* to differentiate between HUB and DEVICE.*/- dev->descriptor.bDeviceClass = desc->bDeviceClass;
There is nothing wrong with this, but I also don't think you need it. In those places where you check for the hub class in your XHCI stack, you actually want to check whether the device is the root hub... so you should do that differently to avoid breaking external hubs (e.g. by comparing the device address). I think we should drop this since there is no real use for it.
/* find the port number we're at */ if (parent) {
Patch b6d7852c increases timeout for enumeration, taking worst case to be 10 sec. get_timer() api returns timestamp in milliseconds, which is what we are checking in the do-while() loop in usb_hub_configure() (get_timer(start) < CONFIG_SYS_HZ * 10). This should give us a required check for 10 seconds, and thereby we don't need to add additional mdelay of 100 microseconds in each cycle.
Signed-off-by: Vivek Gautam gautam.vivek@samsung.com Reviewed-by: Vipin Kumar vipin.kumar@st.com ---
Changes from v2: - none
common/usb_hub.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/common/usb_hub.c b/common/usb_hub.c index e4f4e3c..ab41943 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -443,7 +443,6 @@ static int usb_hub_configure(struct usb_device *dev) (portstatus & USB_PORT_STAT_CONNECTION)) break;
- mdelay(100); } while (get_timer(start) < CONFIG_SYS_HZ * 10);
if (ret < 0)
This adds usb framework support for super-speed usb, which will further facilitate to add stack support for xHCI.
Signed-off-by: Vikas C Sajjan vikas.sajjan@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com ---
Changes from v2: - Replacing if-else with switch-case in portspeed() in cmd_usb.c
common/cmd_usb.c | 24 ++++++++++++++++++------ common/usb.c | 5 +++++ common/usb_hub.c | 8 ++++++-- include/usb.h | 6 ++++++ include/usb_defs.h | 24 +++++++++++++++++++++++- 5 files changed, 58 insertions(+), 9 deletions(-)
diff --git a/common/cmd_usb.c b/common/cmd_usb.c index dacdc2d..594385a 100644 --- a/common/cmd_usb.c +++ b/common/cmd_usb.c @@ -271,12 +271,24 @@ static void usb_display_config(struct usb_device *dev)
static inline char *portspeed(int speed) { - if (speed == USB_SPEED_HIGH) - return "480 Mb/s"; - else if (speed == USB_SPEED_LOW) - return "1.5 Mb/s"; - else - return "12 Mb/s"; + char *speed_str; + + switch (speed) { + case USB_SPEED_SUPER: + speed_str = "5 Gb/s"; + break; + case USB_SPEED_HIGH: + speed_str = "480 Mb/s"; + break; + case USB_SPEED_LOW: + speed_str = "1.5 Mb/s"; + break; + default: + speed_str = "12 Mb/s"; + break; + } + + return speed_str; }
/* shows the device tree recursively */ diff --git a/common/usb.c b/common/usb.c index 3a96a34..55fff5b 100644 --- a/common/usb.c +++ b/common/usb.c @@ -409,6 +409,11 @@ static int usb_parse_config(struct usb_device *dev, wMaxPacketSize); debug("if %d, ep %d\n", ifno, epno); break; + case USB_DT_SS_ENDPOINT_COMP: + if_desc = &dev->config.if_desc[ifno]; + memcpy(&if_desc->ss_ep_comp_desc[epno], + &buffer[index], buffer[index]); + break; default: if (head->bLength == 0) return 1; diff --git a/common/usb_hub.c b/common/usb_hub.c index ab41943..1e225e6 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -165,7 +165,9 @@ static struct usb_hub_device *usb_hub_allocate(void)
static inline char *portspeed(int portstatus) { - if (portstatus & (1 << USB_PORT_FEAT_HIGHSPEED)) + if (portstatus & (1 << USB_PORT_FEAT_SUPERSPEED)) + return "5 Gb/s"; + else if (portstatus & (1 << USB_PORT_FEAT_HIGHSPEED)) return "480 Mb/s"; else if (portstatus & (1 << USB_PORT_FEAT_LOWSPEED)) return "1.5 Mb/s"; @@ -268,7 +270,9 @@ void usb_hub_port_connect_change(struct usb_device *dev, int port) /* Allocate a new device struct for it */ usb = usb_alloc_new_device(dev->controller);
- if (portstatus & USB_PORT_STAT_HIGH_SPEED) + if (portstatus & USB_PORT_STAT_SUPER_SPEED) + usb->speed = USB_SPEED_SUPER; + else if (portstatus & USB_PORT_STAT_HIGH_SPEED) usb->speed = USB_SPEED_HIGH; else if (portstatus & USB_PORT_STAT_LOW_SPEED) usb->speed = USB_SPEED_LOW; diff --git a/include/usb.h b/include/usb.h index d79c865..d7b082d 100644 --- a/include/usb.h +++ b/include/usb.h @@ -76,6 +76,12 @@ struct usb_interface { unsigned char act_altsetting;
struct usb_endpoint_descriptor ep_desc[USB_MAXENDPOINTS]; + /* + * Super Speed Device will have Super Speed Endpoint + * Companion Descriptor (section 9.6.7 of usb 3.0 spec) + * Revision 1.0 June 6th 2011 + */ + struct usb_ss_ep_comp_descriptor ss_ep_comp_desc[USB_MAXENDPOINTS]; } __attribute__ ((packed));
/* Configuration information.. */ diff --git a/include/usb_defs.h b/include/usb_defs.h index 0c78d9d..e2aaef3 100644 --- a/include/usb_defs.h +++ b/include/usb_defs.h @@ -203,6 +203,8 @@ #define USB_PORT_FEAT_POWER 8 #define USB_PORT_FEAT_LOWSPEED 9 #define USB_PORT_FEAT_HIGHSPEED 10 +#define USB_PORT_FEAT_FULLSPEED 11 +#define USB_PORT_FEAT_SUPERSPEED 12 #define USB_PORT_FEAT_C_CONNECTION 16 #define USB_PORT_FEAT_C_ENABLE 17 #define USB_PORT_FEAT_C_SUSPEND 18 @@ -218,8 +220,20 @@ #define USB_PORT_STAT_POWER 0x0100 #define USB_PORT_STAT_LOW_SPEED 0x0200 #define USB_PORT_STAT_HIGH_SPEED 0x0400 /* support for EHCI */ +#define USB_PORT_STAT_FULL_SPEED 0x0800 +#define USB_PORT_STAT_SUPER_SPEED 0x1000 /* support for XHCI */ #define USB_PORT_STAT_SPEED \ - (USB_PORT_STAT_LOW_SPEED | USB_PORT_STAT_HIGH_SPEED) + (USB_PORT_STAT_LOW_SPEED | USB_PORT_STAT_HIGH_SPEED | \ + USB_PORT_STAT_FULL_SPEED | USB_PORT_STAT_SUPER_SPEED) + +/* + * Additions to wPortStatus bit field from USB 3.0 + * See USB 3.0 spec Table 10-10 + */ +#define USB_PORT_STAT_LINK_STATE 0x01e0 +#define USB_SS_PORT_STAT_POWER 0x0200 +#define USB_SS_PORT_STAT_SPEED 0x1c00 +#define USB_PORT_STAT_SPEED_5GBPS 0x0000
/* wPortChange bits */ #define USB_PORT_STAT_C_CONNECTION 0x0001 @@ -228,6 +242,14 @@ #define USB_PORT_STAT_C_OVERCURRENT 0x0008 #define USB_PORT_STAT_C_RESET 0x0010
+/* + * Addition to wPortChange bit fields form USB 3.0 + * See USB 3.0 spec Table 10-11 + */ +#define USB_PORT_STAT_C_BH_RESET 0x0020 +#define USB_PORT_STAT_C_LINK_STATE 0x0040 +#define USB_PORT_STAT_C_CONFIG_ERROR 0x0080 + /* wHubCharacteristics (masks) */ #define HUB_CHAR_LPSM 0x0003 #define HUB_CHAR_COMPOUND 0x0004
Migrating my comments here for public discussion.
This adds usb framework support for super-speed usb, which will further facilitate to add stack support for xHCI.
Signed-off-by: Vikas C Sajjan vikas.sajjan@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
Changes from v2:
- Replacing if-else with switch-case in portspeed() in cmd_usb.c
common/cmd_usb.c | 24 ++++++++++++++++++------ common/usb.c | 5 +++++ common/usb_hub.c | 8 ++++++-- include/usb.h | 6 ++++++ include/usb_defs.h | 24 +++++++++++++++++++++++- 5 files changed, 58 insertions(+), 9 deletions(-)
diff --git a/common/cmd_usb.c b/common/cmd_usb.c index dacdc2d..594385a 100644 --- a/common/cmd_usb.c +++ b/common/cmd_usb.c @@ -271,12 +271,24 @@ static void usb_display_config(struct usb_device *dev)
static inline char *portspeed(int speed) {
- if (speed == USB_SPEED_HIGH)
return "480 Mb/s";- else if (speed == USB_SPEED_LOW)
return "1.5 Mb/s";- else
return "12 Mb/s";
- char *speed_str;
- switch (speed) {
- case USB_SPEED_SUPER:
speed_str = "5 Gb/s";break;- case USB_SPEED_HIGH:
speed_str = "480 Mb/s";break;- case USB_SPEED_LOW:
speed_str = "1.5 Mb/s";break;- default:
speed_str = "12 Mb/s";break;- }
- return speed_str;
}
/* shows the device tree recursively */ diff --git a/common/usb.c b/common/usb.c index 3a96a34..55fff5b 100644 --- a/common/usb.c +++ b/common/usb.c @@ -409,6 +409,11 @@ static int usb_parse_config(struct usb_device *dev, wMaxPacketSize); debug("if %d, ep %d\n", ifno, epno); break;
case USB_DT_SS_ENDPOINT_COMP:if_desc = &dev->config.if_desc[ifno];memcpy(&if_desc->ss_ep_comp_desc[epno],&buffer[index], buffer[index]);break;diff --git a/common/usb_hub.c b/common/usb_hub.c index ab41943..1e225e6 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -165,7 +165,9 @@ static struct usb_hub_device *usb_hub_allocate(void)
static inline char *portspeed(int portstatus) {
- if (portstatus & (1 << USB_PORT_FEAT_HIGHSPEED))
- if (portstatus & (1 << USB_PORT_FEAT_SUPERSPEED))
It doesn't make sense to use the USB_PORT_FEAT_ constants to read a port status here. These should be USB_PORT_STAT_ instead (you could use a switch statement if you mask and shift them correctly).
return "5 Gb/s";- else if (portstatus & (1 << USB_PORT_FEAT_HIGHSPEED)) return "480 Mb/s"; else if (portstatus & (1 << USB_PORT_FEAT_LOWSPEED)) return "1.5 Mb/s";
@@ -268,7 +270,9 @@ void usb_hub_port_connect_change(struct usb_device *dev, int port) /* Allocate a new device struct for it */ usb = usb_alloc_new_device(dev->controller);
- if (portstatus & USB_PORT_STAT_HIGH_SPEED)
- if (portstatus & USB_PORT_STAT_SUPER_SPEED)
usb->speed = USB_SPEED_SUPER;- else if (portstatus & USB_PORT_STAT_HIGH_SPEED) usb->speed = USB_SPEED_HIGH; else if (portstatus & USB_PORT_STAT_LOW_SPEED) usb->speed = USB_SPEED_LOW;
Should change to switch statement after implementing my suggestion to usb_defs.h
diff --git a/include/usb.h b/include/usb.h index d79c865..d7b082d 100644 --- a/include/usb.h +++ b/include/usb.h @@ -76,6 +76,12 @@ struct usb_interface { unsigned char act_altsetting;
struct usb_endpoint_descriptor ep_desc[USB_MAXENDPOINTS];
- /*
* Super Speed Device will have Super Speed Endpoint* Companion Descriptor (section 9.6.7 of usb 3.0 spec)* Revision 1.0 June 6th 2011*/- struct usb_ss_ep_comp_descriptor ss_ep_comp_desc[USB_MAXENDPOINTS];
} __attribute__ ((packed));
/* Configuration information.. */ diff --git a/include/usb_defs.h b/include/usb_defs.h index 0c78d9d..e2aaef3 100644 --- a/include/usb_defs.h +++ b/include/usb_defs.h @@ -203,6 +203,8 @@ #define USB_PORT_FEAT_POWER 8 #define USB_PORT_FEAT_LOWSPEED 9 #define USB_PORT_FEAT_HIGHSPEED 10 +#define USB_PORT_FEAT_FULLSPEED 11 +#define USB_PORT_FEAT_SUPERSPEED 12
Speed is never set as a feature anyway. We should just get rid of all of these and always use USB_PORT_STAT_ for them.
#define USB_PORT_FEAT_C_CONNECTION 16 #define USB_PORT_FEAT_C_ENABLE 17 #define USB_PORT_FEAT_C_SUSPEND 18 @@ -218,8 +220,20 @@ #define USB_PORT_STAT_POWER 0x0100 #define USB_PORT_STAT_LOW_SPEED 0x0200 #define USB_PORT_STAT_HIGH_SPEED 0x0400 /* support for EHCI */ +#define USB_PORT_STAT_FULL_SPEED 0x0800 +#define USB_PORT_STAT_SUPER_SPEED 0x1000 /* support for XHCI */
These two new values are *not* correct. 0x800 is actually PORT_TEST, and 0x1000 is PORT_INDICATOR. Full speed has always been reported by having both the LOW_SPEED and the HIGH_SPEED bits zero, and you should define it that way (for use in switch statements).
If you want to fake SuperSpeed into the USB 2.0 hub descriptor (see my comment on xhci_port_speed in the unreleased CL), you could represent it with 0xc00, which should be a reserved value for all existing 1.0 and 2.0 hubs.
#define USB_PORT_STAT_SPEED \
- (USB_PORT_STAT_LOW_SPEED | USB_PORT_STAT_HIGH_SPEED)
- (USB_PORT_STAT_LOW_SPEED | USB_PORT_STAT_HIGH_SPEED | \
- USB_PORT_STAT_FULL_SPEED | USB_PORT_STAT_SUPER_SPEED)
Maybe this should be renamed to USB_PORT_STAT_SPEED_MASK to make its purpose clearer.
+/*
- Additions to wPortStatus bit field from USB 3.0
I would reword "Additions" to "Changes" and explicitly state that these fields concern the new USB 3.0 hub descriptor format, which is different and exclusively applies to SuperSpeed hubs.
- See USB 3.0 spec Table 10-10
In my spec it's Table 10-11... are you sure you have the most recent version?
- */
+#define USB_PORT_STAT_LINK_STATE 0x01e0
For consistency, I would prefix all values in here with USB_SS_PORT_STAT_ (to make extra clear that they belong to something different than the ones above).
+#define USB_SS_PORT_STAT_POWER 0x0200 +#define USB_SS_PORT_STAT_SPEED 0x1c00 +#define USB_PORT_STAT_SPEED_5GBPS 0x0000
/* wPortChange bits */ #define USB_PORT_STAT_C_CONNECTION 0x0001 @@ -228,6 +242,14 @@ #define USB_PORT_STAT_C_OVERCURRENT 0x0008 #define USB_PORT_STAT_C_RESET 0x0010
+/*
- Addition to wPortChange bit fields form USB 3.0
- See USB 3.0 spec Table 10-11
- */
+#define USB_PORT_STAT_C_BH_RESET 0x0020
Same here. New incompatible port status format for SuperSpeed hubs, please make the comment clearer and prefix as USB_SS_PORT_STAT_C_
+#define USB_PORT_STAT_C_LINK_STATE 0x0040 +#define USB_PORT_STAT_C_CONFIG_ERROR 0x0080
/* wHubCharacteristics (masks) */ #define HUB_CHAR_LPSM 0x0003 #define HUB_CHAR_COMPOUND 0x0004
These patches haven't gone in yet, right? I think Vivek wants to discuss/update them himself, he just asked me to move my reviews to this thread.
Dear Julius Werner,
These patches haven't gone in yet, right? I think Vivek wants to discuss/update them himself, he just asked me to move my reviews to this thread.
They're not in, but they're already pushed in my tree. It'd be also easier to review diff instead of full patches again.
Best regards, Marek Vasut
Hi Marek,
On Sat, Apr 20, 2013 at 5:27 PM, Marek Vasut marex@denx.de wrote:
Dear Julius Werner,
These patches haven't gone in yet, right? I think Vivek wants to discuss/update them himself, he just asked me to move my reviews to this thread.
They're not in, but they're already pushed in my tree. It'd be also easier to review diff instead of full patches again.
I shall send a diff patchset for these changes, but do we have a possibility of squashing the changes to earler commits at some point of time later, so that clean changes get to u-boot ? Or we shall go as you suggest.
Dear Vivek Gautam,
Hi Marek,
On Sat, Apr 20, 2013 at 5:27 PM, Marek Vasut marex@denx.de wrote:
Dear Julius Werner,
These patches haven't gone in yet, right? I think Vivek wants to discuss/update them himself, he just asked me to move my reviews to this thread.
They're not in, but they're already pushed in my tree. It'd be also easier to review diff instead of full patches again.
I shall send a diff patchset for these changes, but do we have a possibility of squashing the changes to earler commits at some point of time later, so that clean changes get to u-boot ? Or we shall go as you suggest.
Just send a subsequent patch ;-)
Best regards, Marek Vasut
Hi Marek,
On Tue, Apr 23, 2013 at 7:54 AM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Hi Marek,
On Sat, Apr 20, 2013 at 5:27 PM, Marek Vasut marex@denx.de wrote:
Dear Julius Werner,
These patches haven't gone in yet, right? I think Vivek wants to discuss/update them himself, he just asked me to move my reviews to this thread.
They're not in, but they're already pushed in my tree. It'd be also easier to review diff instead of full patches again.
I shall send a diff patchset for these changes, but do we have a possibility of squashing the changes to earler commits at some point of time later, so that clean changes get to u-boot ? Or we shall go as you suggest.
Just send a subsequent patch ;-)
Sure, will send subsequent patches on u-boot-usb/next
Best regards, Marek Vasut
Hi Julius,
On Fri, Apr 19, 2013 at 11:52 PM, Julius Werner jwerner@chromium.org wrote:
Migrating my comments here for public discussion.
Thanks for your valuable comments here.
This adds usb framework support for super-speed usb, which will further facilitate to add stack support for xHCI.
Signed-off-by: Vikas C Sajjan vikas.sajjan@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
Changes from v2:
- Replacing if-else with switch-case in portspeed() in cmd_usb.c
common/cmd_usb.c | 24 ++++++++++++++++++------ common/usb.c | 5 +++++ common/usb_hub.c | 8 ++++++-- include/usb.h | 6 ++++++ include/usb_defs.h | 24 +++++++++++++++++++++++- 5 files changed, 58 insertions(+), 9 deletions(-)
diff --git a/common/cmd_usb.c b/common/cmd_usb.c index dacdc2d..594385a 100644 --- a/common/cmd_usb.c +++ b/common/cmd_usb.c @@ -271,12 +271,24 @@ static void usb_display_config(struct usb_device *dev)
static inline char *portspeed(int speed) {
if (speed == USB_SPEED_HIGH)return "480 Mb/s";else if (speed == USB_SPEED_LOW)return "1.5 Mb/s";elsereturn "12 Mb/s";
char *speed_str;switch (speed) {case USB_SPEED_SUPER:speed_str = "5 Gb/s";break;case USB_SPEED_HIGH:speed_str = "480 Mb/s";break;case USB_SPEED_LOW:speed_str = "1.5 Mb/s";break;default:speed_str = "12 Mb/s";break;}return speed_str;}
/* shows the device tree recursively */ diff --git a/common/usb.c b/common/usb.c index 3a96a34..55fff5b 100644 --- a/common/usb.c +++ b/common/usb.c @@ -409,6 +409,11 @@ static int usb_parse_config(struct usb_device *dev, wMaxPacketSize); debug("if %d, ep %d\n", ifno, epno); break;
case USB_DT_SS_ENDPOINT_COMP:if_desc = &dev->config.if_desc[ifno];memcpy(&if_desc->ss_ep_comp_desc[epno],&buffer[index], buffer[index]);break; default: if (head->bLength == 0) return 1;diff --git a/common/usb_hub.c b/common/usb_hub.c index ab41943..1e225e6 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -165,7 +165,9 @@ static struct usb_hub_device *usb_hub_allocate(void)
static inline char *portspeed(int portstatus) {
if (portstatus & (1 << USB_PORT_FEAT_HIGHSPEED))
if (portstatus & (1 << USB_PORT_FEAT_SUPERSPEED))It doesn't make sense to use the USB_PORT_FEAT_ constants to read a port status here. These should be USB_PORT_STAT_ instead (you could use a switch statement if you mask and shift them correctly).
True, we should be 'ANDing' here with respectve PORT_STAT_ constants, will modify this as required.
return "5 Gb/s";else if (portstatus & (1 << USB_PORT_FEAT_HIGHSPEED)) return "480 Mb/s"; else if (portstatus & (1 << USB_PORT_FEAT_LOWSPEED)) return "1.5 Mb/s";@@ -268,7 +270,9 @@ void usb_hub_port_connect_change(struct usb_device *dev, int port) /* Allocate a new device struct for it */ usb = usb_alloc_new_device(dev->controller);
if (portstatus & USB_PORT_STAT_HIGH_SPEED)
if (portstatus & USB_PORT_STAT_SUPER_SPEED)usb->speed = USB_SPEED_SUPER;else if (portstatus & USB_PORT_STAT_HIGH_SPEED) usb->speed = USB_SPEED_HIGH; else if (portstatus & USB_PORT_STAT_LOW_SPEED) usb->speed = USB_SPEED_LOW;Should change to switch statement after implementing my suggestion to usb_defs.h
Sure,
diff --git a/include/usb.h b/include/usb.h index d79c865..d7b082d 100644 --- a/include/usb.h +++ b/include/usb.h @@ -76,6 +76,12 @@ struct usb_interface { unsigned char act_altsetting;
struct usb_endpoint_descriptor ep_desc[USB_MAXENDPOINTS];
/** Super Speed Device will have Super Speed Endpoint* Companion Descriptor (section 9.6.7 of usb 3.0 spec)* Revision 1.0 June 6th 2011*/struct usb_ss_ep_comp_descriptor ss_ep_comp_desc[USB_MAXENDPOINTS];} __attribute__ ((packed));
/* Configuration information.. */ diff --git a/include/usb_defs.h b/include/usb_defs.h index 0c78d9d..e2aaef3 100644 --- a/include/usb_defs.h +++ b/include/usb_defs.h @@ -203,6 +203,8 @@ #define USB_PORT_FEAT_POWER 8 #define USB_PORT_FEAT_LOWSPEED 9 #define USB_PORT_FEAT_HIGHSPEED 10 +#define USB_PORT_FEAT_FULLSPEED 11 +#define USB_PORT_FEAT_SUPERSPEED 12
Speed is never set as a feature anyway. We should just get rid of all of these and always use USB_PORT_STAT_ for them.
These are simply 'Hub class feature numbers' given in USB 2.0 and 3.0 specs. Why don't we just keep them like that (I shall be removing the USB_PORT_FEAT_FULLSPEED and USB_PORT_FEAT_SUPERSPEED definitely from here). In case we want to check the port status field, we would check against PORT_STAT_ constants.
#define USB_PORT_FEAT_C_CONNECTION 16 #define USB_PORT_FEAT_C_ENABLE 17 #define USB_PORT_FEAT_C_SUSPEND 18 @@ -218,8 +220,20 @@ #define USB_PORT_STAT_POWER 0x0100 #define USB_PORT_STAT_LOW_SPEED 0x0200 #define USB_PORT_STAT_HIGH_SPEED 0x0400 /* support for EHCI */ +#define USB_PORT_STAT_FULL_SPEED 0x0800 +#define USB_PORT_STAT_SUPER_SPEED 0x1000 /* support for XHCI */
These two new values are *not* correct. 0x800 is actually PORT_TEST, and 0x1000 is PORT_INDICATOR. Full speed has always been reported by having both the LOW_SPEED and the HIGH_SPEED bits zero, and you should define it that way (for use in switch statements).
Sure, will remove these two constants.
If you want to fake SuperSpeed into the USB 2.0 hub descriptor (see my comment on xhci_port_speed in the unreleased CL), you could represent it with 0xc00, which should be a reserved value for all existing 1.0 and 2.0 hubs.
#define USB_PORT_STAT_SPEED \
(USB_PORT_STAT_LOW_SPEED | USB_PORT_STAT_HIGH_SPEED)
(USB_PORT_STAT_LOW_SPEED | USB_PORT_STAT_HIGH_SPEED | \USB_PORT_STAT_FULL_SPEED | USB_PORT_STAT_SUPER_SPEED)Maybe this should be renamed to USB_PORT_STAT_SPEED_MASK to make its purpose clearer.
Alright, shall rename it to USB_PORT_STAT_SPEED_MASK.
+/*
- Additions to wPortStatus bit field from USB 3.0
I would reword "Additions" to "Changes" and explicitly state that these fields concern the new USB 3.0 hub descriptor format, which is different and exclusively applies to SuperSpeed hubs.
This bit fields are actually additions to USB 3.0 framework. These fields are not defined in USB 2.0 specs, or else they are reserved there. If you insist i can modify this as suggested.
- See USB 3.0 spec Table 10-10
In my spec it's Table 10-11... are you sure you have the most recent version?
Yea, my mistake, typo !!
- */
+#define USB_PORT_STAT_LINK_STATE 0x01e0
For consistency, I would prefix all values in here with USB_SS_PORT_STAT_ (to make extra clear that they belong to something different than the ones above).
True, shall change this to make things consistent.
+#define USB_SS_PORT_STAT_POWER 0x0200 +#define USB_SS_PORT_STAT_SPEED 0x1c00 +#define USB_PORT_STAT_SPEED_5GBPS 0x0000
/* wPortChange bits */ #define USB_PORT_STAT_C_CONNECTION 0x0001 @@ -228,6 +242,14 @@ #define USB_PORT_STAT_C_OVERCURRENT 0x0008 #define USB_PORT_STAT_C_RESET 0x0010
+/*
- Addition to wPortChange bit fields form USB 3.0
- See USB 3.0 spec Table 10-11
- */
+#define USB_PORT_STAT_C_BH_RESET 0x0020
Same here. New incompatible port status format for SuperSpeed hubs, please make the comment clearer and prefix as USB_SS_PORT_STAT_C_
Sure, will add the relavant prefix, and write proper comment.
+#define USB_PORT_STAT_C_LINK_STATE 0x0040 +#define USB_PORT_STAT_C_CONFIG_ERROR 0x0080
/* wHubCharacteristics (masks) */ #define HUB_CHAR_LPSM 0x0003 #define HUB_CHAR_COMPOUND 0x0004
I would reword "Additions" to "Changes" and explicitly state that these fields concern the new USB 3.0 hub descriptor format, which is different and exclusively applies to SuperSpeed hubs.
This bit fields are actually additions to USB 3.0 framework. These fields are not defined in USB 2.0 specs, or else they are reserved there. If you insist i can modify this as suggested.
Yes, okay, I think we mean the same thing here. I just wanted to point out that the USB 3.0 port status is actually a completely different thing from the USB 2.0 port status (which is still valid for non-SuperSpeed hubs in USB 3.0 environments, even though the spec hasn't printed it out again). They did not just *add* a few bits to the old status bit field, they actually *changed* existing bits (not just reserved ones)... e.g. bit 9 means LOW_SPEED for USB 2.0 hubs but PORT_POWER for SuperSpeed hubs. I found this very confusing when I first read it, so I thought it might be worth making it extra obvious through comments.
Hi Julius,
On Tue, Apr 23, 2013 at 3:44 AM, Julius Werner jwerner@chromium.org wrote:
I would reword "Additions" to "Changes" and explicitly state that these fields concern the new USB 3.0 hub descriptor format, which is different and exclusively applies to SuperSpeed hubs.
This bit fields are actually additions to USB 3.0 framework. These fields are not defined in USB 2.0 specs, or else they are reserved there. If you insist i can modify this as suggested.
Yes, okay, I think we mean the same thing here. I just wanted to point out that the USB 3.0 port status is actually a completely different thing from the USB 2.0 port status (which is still valid for non-SuperSpeed hubs in USB 3.0 environments, even though the spec hasn't printed it out again). They did not just *add* a few bits to the old status bit field, they actually *changed* existing bits (not just reserved ones)... e.g. bit 9 means LOW_SPEED for USB 2.0 hubs but PORT_POWER for SuperSpeed hubs. I found this very confusing when I first read it, so I thought it might be worth making it extra obvious through comments.
Fair enough, i shall change this accordingly, Thanks !!
As per XHCI specifications USB 3.0 protocol ports attempt to advance to 'Enabled' state; however USB 2.0 protocol ports require software reset to advance them to 'Enabled' state. Thereby, inferring that software need to reset USB 2.0 protocol ports invariably (as per EHCI spec or xHCI spec).
Signed-off-by: Vivek Gautam gautam.vivek@samsung.com ---
This patch added in V3(current-version) of this patch-series.
common/usb_hub.c | 23 +++++++++++++++++++---- 1 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/common/usb_hub.c b/common/usb_hub.c index 1e225e6..eedbcf2 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -233,6 +233,7 @@ void usb_hub_port_connect_change(struct usb_device *dev, int port) struct usb_device *usb; ALLOC_CACHE_ALIGN_BUFFER(struct usb_port_status, portsts, 1); unsigned short portstatus; + uint32_t do_port_reset = 1;
/* Check status */ if (usb_get_port_status(dev, port + 1, portsts) < 0) { @@ -246,6 +247,9 @@ void usb_hub_port_connect_change(struct usb_device *dev, int port) le16_to_cpu(portsts->wPortChange), portspeed(portstatus));
+ if (portstatus & (1 << USB_PORT_FEAT_SUPERSPEED)) + do_port_reset = 0; + /* Clear the connection change status */ usb_clear_port_feature(dev, port + 1, USB_PORT_FEAT_C_CONNECTION);
@@ -259,10 +263,21 @@ void usb_hub_port_connect_change(struct usb_device *dev, int port) } mdelay(200);
- /* Reset the port */ - if (hub_port_reset(dev, port, &portstatus) < 0) { - printf("cannot reset port %i!?\n", port + 1); - return; + /* + * Reset the port: + * As per xHCI protocol, USB 3.0 devices do not require + * a port reset, however USB 2.0 device do require the same + * to let ports proceed to 'enabled' state + * + * XXX: Will this break EHCI ?? + * probably not, above condition for 'do_port_reset' checks for + * speed, and for EHCI it can't reach Super speed anyways. + */ + if (do_port_reset) { + if (hub_port_reset(dev, port, &portstatus) < 0) { + printf("cannot reset port %i!?\n", port + 1); + return; + } }
mdelay(200);
Sorry, forgot this one yesterday. I would consider to just drop/revert this patch entirely. It's not wrong, but it adds complexity where it is not needed. You don't have to reset SuperSpeed devices, but it shouldn't hurt either and from what I can tell Linux does it as well.
As per XHCI specifications USB 3.0 protocol ports attempt to advance to 'Enabled' state; however USB 2.0 protocol ports require software reset to advance them to 'Enabled' state. Thereby, inferring that software need to reset USB 2.0 protocol ports invariably (as per EHCI spec or xHCI spec).
Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
This patch added in V3(current-version) of this patch-series.
common/usb_hub.c | 23 +++++++++++++++++++---- 1 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/common/usb_hub.c b/common/usb_hub.c index 1e225e6..eedbcf2 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -233,6 +233,7 @@ void usb_hub_port_connect_change(struct usb_device *dev, int port) struct usb_device *usb; ALLOC_CACHE_ALIGN_BUFFER(struct usb_port_status, portsts, 1); unsigned short portstatus;
uint32_t do_port_reset = 1;
/* Check status */ if (usb_get_port_status(dev, port + 1, portsts) < 0) {
@@ -246,6 +247,9 @@ void usb_hub_port_connect_change(struct usb_device *dev, int port) le16_to_cpu(portsts->wPortChange), portspeed(portstatus));
- if (portstatus & (1 << USB_PORT_FEAT_SUPERSPEED))
do_port_reset = 0;- /* Clear the connection change status */ usb_clear_port_feature(dev, port + 1, USB_PORT_FEAT_C_CONNECTION);
@@ -259,10 +263,21 @@ void usb_hub_port_connect_change(struct usb_device *dev, int port) } mdelay(200);
- /* Reset the port */
- if (hub_port_reset(dev, port, &portstatus) < 0) {
printf("cannot reset port %i!?\n", port + 1);return;
/*
* Reset the port:* As per xHCI protocol, USB 3.0 devices do not require* a port reset, however USB 2.0 device do require the same* to let ports proceed to 'enabled' state** XXX: Will this break EHCI ??* probably not, above condition for 'do_port_reset' checks for* speed, and for EHCI it can't reach Super speed anyways.*/if (do_port_reset) {
if (hub_port_reset(dev, port, &portstatus) < 0) {printf("cannot reset port %i!?\n", port + 1);return;}}
mdelay(200);
Dear Julius Werner,
Sorry, forgot this one yesterday. I would consider to just drop/revert this patch entirely. It's not wrong, but it adds complexity where it is not needed. You don't have to reset SuperSpeed devices, but it shouldn't hurt either and from what I can tell Linux does it as well.
Ok, I can drop this one.
As per XHCI specifications USB 3.0 protocol ports attempt to advance to 'Enabled' state; however USB 2.0 protocol ports require software reset to advance them to 'Enabled' state. Thereby, inferring that software need to reset USB 2.0 protocol ports invariably (as per EHCI spec or xHCI spec).
Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
This patch added in V3(current-version) of this patch-series.
common/usb_hub.c | 23 +++++++++++++++++++---- 1 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/common/usb_hub.c b/common/usb_hub.c index 1e225e6..eedbcf2 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -233,6 +233,7 @@ void usb_hub_port_connect_change(struct usb_device *dev, int port)
struct usb_device *usb; ALLOC_CACHE_ALIGN_BUFFER(struct usb_port_status, portsts, 1); unsigned short portstatus;
uint32_t do_port_reset = 1;
/* Check status */ if (usb_get_port_status(dev, port + 1, portsts) < 0) {
@@ -246,6 +247,9 @@ void usb_hub_port_connect_change(struct usb_device *dev, int port)
le16_to_cpu(portsts->wPortChange), portspeed(portstatus));
if (portstatus & (1 << USB_PORT_FEAT_SUPERSPEED))
do_port_reset = 0;/* Clear the connection change status */ usb_clear_port_feature(dev, port + 1, USB_PORT_FEAT_C_CONNECTION);
@@ -259,10 +263,21 @@ void usb_hub_port_connect_change(struct usb_device *dev, int port)
} mdelay(200);
- /* Reset the port */
- if (hub_port_reset(dev, port, &portstatus) < 0) {
printf("cannot reset port %i!?\n", port + 1);return;
/*
* Reset the port:* As per xHCI protocol, USB 3.0 devices do not require* a port reset, however USB 2.0 device do require the same* to let ports proceed to 'enabled' state** XXX: Will this break EHCI ??* probably not, above condition for 'do_port_reset' checks for* speed, and for EHCI it can't reach Super speed anyways.*/if (do_port_reset) {
if (hub_port_reset(dev, port, &portstatus) < 0) {printf("cannot reset port %i!?\n", port + 1);return;}}
mdelay(200);
Best regards, Marek Vasut
On Wed, Apr 24, 2013 at 5:51 AM, Marek Vasut marex@denx.de wrote:
Dear Julius Werner,
Sorry, forgot this one yesterday. I would consider to just drop/revert this patch entirely. It's not wrong, but it adds complexity where it is not needed. You don't have to reset SuperSpeed devices, but it shouldn't hurt either and from what I can tell Linux does it as well.
Ok, I can drop this one.
Alright, we will drop this. I think a possible bug related to calling xhci_port_state_to_neutral() in xhci_submit_root() (one pointed by Julius in [PATCH v3 3/8] usb: hub: Power-cycle on root-hub ports) was causing USB 3.0 protocol ports fail while resetting. Now things are fine.
As per XHCI specifications USB 3.0 protocol ports attempt to advance to 'Enabled' state; however USB 2.0 protocol ports require software reset to advance them to 'Enabled' state. Thereby, inferring that software need to reset USB 2.0 protocol ports invariably (as per EHCI spec or xHCI spec).
Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
This patch added in V3(current-version) of this patch-series.
common/usb_hub.c | 23 +++++++++++++++++++---- 1 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/common/usb_hub.c b/common/usb_hub.c index 1e225e6..eedbcf2 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -233,6 +233,7 @@ void usb_hub_port_connect_change(struct usb_device *dev, int port)
struct usb_device *usb; ALLOC_CACHE_ALIGN_BUFFER(struct usb_port_status, portsts, 1); unsigned short portstatus;
uint32_t do_port_reset = 1;
/* Check status */ if (usb_get_port_status(dev, port + 1, portsts) < 0) {
@@ -246,6 +247,9 @@ void usb_hub_port_connect_change(struct usb_device *dev, int port)
le16_to_cpu(portsts->wPortChange), portspeed(portstatus));
if (portstatus & (1 << USB_PORT_FEAT_SUPERSPEED))
do_port_reset = 0;/* Clear the connection change status */ usb_clear_port_feature(dev, port + 1, USB_PORT_FEAT_C_CONNECTION);
@@ -259,10 +263,21 @@ void usb_hub_port_connect_change(struct usb_device *dev, int port)
} mdelay(200);
- /* Reset the port */
- if (hub_port_reset(dev, port, &portstatus) < 0) {
printf("cannot reset port %i!?\n", port + 1);return;
/*
- Reset the port:
- As per xHCI protocol, USB 3.0 devices do not require
- a port reset, however USB 2.0 device do require the same
- to let ports proceed to 'enabled' state
- XXX: Will this break EHCI ??
- probably not, above condition for 'do_port_reset' checks for
- speed, and for EHCI it can't reach Super speed anyways.
*/
if (do_port_reset) {
if (hub_port_reset(dev, port, &portstatus) < 0) {printf("cannot reset port %i!?\n", port + 1);return;}}
mdelay(200);
Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi Marek,
On Wed, Apr 24, 2013 at 11:38 AM, Vivek Gautam gautamvivek1987@gmail.com wrote:
On Wed, Apr 24, 2013 at 5:51 AM, Marek Vasut marex@denx.de wrote:
Dear Julius Werner,
Sorry, forgot this one yesterday. I would consider to just drop/revert this patch entirely. It's not wrong, but it adds complexity where it is not needed. You don't have to reset SuperSpeed devices, but it shouldn't hurt either and from what I can tell Linux does it as well.
Ok, I can drop this one.
We need to drop this from u-boot-usb/next branch, right ? I am sure this is added to you to-do list, just wanted to make sure since i could see it in 'next' right now. Thanks !!
Alright, we will drop this. I think a possible bug related to calling xhci_port_state_to_neutral() in xhci_submit_root() (one pointed by Julius in [PATCH v3 3/8] usb: hub: Power-cycle on root-hub ports) was causing USB 3.0 protocol ports fail while resetting. Now things are fine.
As per XHCI specifications USB 3.0 protocol ports attempt to advance to 'Enabled' state; however USB 2.0 protocol ports require software reset to advance them to 'Enabled' state. Thereby, inferring that software need to reset USB 2.0 protocol ports invariably (as per EHCI spec or xHCI spec).
Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
This patch added in V3(current-version) of this patch-series.
common/usb_hub.c | 23 +++++++++++++++++++---- 1 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/common/usb_hub.c b/common/usb_hub.c index 1e225e6..eedbcf2 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -233,6 +233,7 @@ void usb_hub_port_connect_change(struct usb_device *dev, int port)
struct usb_device *usb; ALLOC_CACHE_ALIGN_BUFFER(struct usb_port_status, portsts, 1); unsigned short portstatus;
uint32_t do_port_reset = 1;
/* Check status */ if (usb_get_port_status(dev, port + 1, portsts) < 0) {
@@ -246,6 +247,9 @@ void usb_hub_port_connect_change(struct usb_device *dev, int port)
le16_to_cpu(portsts->wPortChange), portspeed(portstatus));
if (portstatus & (1 << USB_PORT_FEAT_SUPERSPEED))
do_port_reset = 0;/* Clear the connection change status */ usb_clear_port_feature(dev, port + 1, USB_PORT_FEAT_C_CONNECTION);
@@ -259,10 +263,21 @@ void usb_hub_port_connect_change(struct usb_device *dev, int port)
} mdelay(200);
- /* Reset the port */
- if (hub_port_reset(dev, port, &portstatus) < 0) {
printf("cannot reset port %i!?\n", port + 1);return;
/*
- Reset the port:
- As per xHCI protocol, USB 3.0 devices do not require
- a port reset, however USB 2.0 device do require the same
- to let ports proceed to 'enabled' state
- XXX: Will this break EHCI ??
- probably not, above condition for 'do_port_reset' checks for
- speed, and for EHCI it can't reach Super speed anyways.
*/
if (do_port_reset) {
if (hub_port_reset(dev, port, &portstatus) < 0) {printf("cannot reset port %i!?\n", port + 1);return;}}
mdelay(200);
Dear Vivek Gautam,
Hi Marek,
On Wed, Apr 24, 2013 at 11:38 AM, Vivek Gautam
gautamvivek1987@gmail.com wrote:
On Wed, Apr 24, 2013 at 5:51 AM, Marek Vasut marex@denx.de wrote:
Dear Julius Werner,
Sorry, forgot this one yesterday. I would consider to just drop/revert this patch entirely. It's not wrong, but it adds complexity where it is not needed. You don't have to reset SuperSpeed devices, but it shouldn't hurt either and from what I can tell Linux does it as well.
Ok, I can drop this one.
We need to drop this from u-boot-usb/next branch, right ? I am sure this is added to you to-do list, just wanted to make sure since i could see it in 'next' right now. Thanks !!
Dropped and pushed to -next .
Best regards, Marek Vasut
We can use a common global macro for calculating minimum of 3 numbers. Put the same in 'common header' and let 'ehci' use it.
Signed-off-by: Vivek Gautam gautam.vivek@samsung.com ---
This patch added in V3(current-version) of this patch-series.
drivers/usb/host/ehci-hcd.c | 10 ---------- include/common.h | 2 ++ 2 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index c816878..bcecae3 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -587,16 +587,6 @@ fail: return -1; }
-static inline int min3(int a, int b, int c) -{ - - if (b < a) - a = b; - if (c < a) - a = c; - return a; -} - int ehci_submit_root(struct usb_device *dev, unsigned long pipe, void *buffer, int length, struct devrequest *req) diff --git a/include/common.h b/include/common.h index d41aeb4..37269c7 100644 --- a/include/common.h +++ b/include/common.h @@ -211,6 +211,8 @@ typedef void (interrupt_handler_t)(void *); #define MIN(x, y) min(x, y) #define MAX(x, y) max(x, y)
+#define min3(a, b, c) min(min(a, b), c) + /* * Return the absolute value of a number. *
Dear Vivek Gautam,
We can use a common global macro for calculating minimum of 3 numbers. Put the same in 'common header' and let 'ehci' use it.
Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
This patch added in V3(current-version) of this patch-series.
drivers/usb/host/ehci-hcd.c | 10 ---------- include/common.h | 2 ++ 2 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index c816878..bcecae3 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -587,16 +587,6 @@ fail: return -1; }
-static inline int min3(int a, int b, int c) -{
- if (b < a)
a = b;- if (c < a)
a = c;- return a;
-}
int ehci_submit_root(struct usb_device *dev, unsigned long pipe, void *buffer, int length, struct devrequest *req) diff --git a/include/common.h b/include/common.h index d41aeb4..37269c7 100644 --- a/include/common.h +++ b/include/common.h @@ -211,6 +211,8 @@ typedef void (interrupt_handler_t)(void *); #define MIN(x, y) min(x, y) #define MAX(x, y) max(x, y)
+#define min3(a, b, c) min(min(a, b), c)
You might want to keep it as an inline-function to allow GCC do the type- checking?
/*
- Return the absolute value of a number.
Best regards, Marek Vasut
We can use a common global method for calculating minimum of 3 numbers. Put the same in 'common header' and let 'ehci' use it.
Signed-off-by: Vivek Gautam gautam.vivek@samsung.com --- drivers/usb/host/ehci-hcd.c | 10 ---------- include/common.h | 5 +++++ 2 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index 19d4352..e0f3e4b 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -603,16 +603,6 @@ fail: return -1; }
-static inline int min3(int a, int b, int c) -{ - - if (b < a) - a = b; - if (c < a) - a = c; - return a; -} - int ehci_submit_root(struct usb_device *dev, unsigned long pipe, void *buffer, int length, struct devrequest *req) diff --git a/include/common.h b/include/common.h index 0cfa6a8..611edca 100644 --- a/include/common.h +++ b/include/common.h @@ -211,6 +211,11 @@ typedef void (interrupt_handler_t)(void *); #define MIN(x, y) min(x, y) #define MAX(x, y) max(x, y)
+static inline int min3(int a, int b, int c) +{ + return min(min(a, b), c); +} + /* * Return the absolute value of a number. *
Dear Vivek Gautam,
We can use a common global method for calculating minimum of 3 numbers. Put the same in 'common header' and let 'ehci' use it.
Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
Applied, thanks
drivers/usb/host/ehci-hcd.c | 10 ---------- include/common.h | 5 +++++ 2 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index 19d4352..e0f3e4b 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -603,16 +603,6 @@ fail: return -1; }
-static inline int min3(int a, int b, int c) -{
- if (b < a)
a = b;- if (c < a)
a = c;- return a;
-}
int ehci_submit_root(struct usb_device *dev, unsigned long pipe, void *buffer, int length, struct devrequest *req) diff --git a/include/common.h b/include/common.h index 0cfa6a8..611edca 100644 --- a/include/common.h +++ b/include/common.h @@ -211,6 +211,11 @@ typedef void (interrupt_handler_t)(void *); #define MIN(x, y) min(x, y) #define MAX(x, y) max(x, y)
+static inline int min3(int a, int b, int c) +{
- return min(min(a, b), c);
+}
/*
- Return the absolute value of a number.
Best regards, Marek Vasut
On Fri, Apr 19, 2013 at 01:29:12PM +0200, Marek Vasut wrote:
Dear Vivek Gautam,
We can use a common global method for calculating minimum of 3 numbers. Put the same in 'common header' and let 'ehci' use it.
Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
Applied, thanks
NAK, sorry. Lets re-sync with the kernel's min/max/min3/max3 defines here instead.
On Mon, Apr 22, 2013 at 7:15 PM, Tom Rini trini@ti.com wrote:
On Fri, Apr 19, 2013 at 01:29:12PM +0200, Marek Vasut wrote:
Dear Vivek Gautam,
We can use a common global method for calculating minimum of 3 numbers. Put the same in 'common header' and let 'ehci' use it.
Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
Applied, thanks
NAK, sorry. Lets re-sync with the kernel's min/max/min3/max3 defines here instead.
Alright, i shall sync the definitions with Linux kernel and send the patch for same.
Hi Marek,
On Wed, Apr 24, 2013 at 11:49 AM, Vivek Gautam gautamvivek1987@gmail.com wrote:
On Mon, Apr 22, 2013 at 7:15 PM, Tom Rini trini@ti.com wrote:
On Fri, Apr 19, 2013 at 01:29:12PM +0200, Marek Vasut wrote:
Dear Vivek Gautam,
We can use a common global method for calculating minimum of 3 numbers. Put the same in 'common header' and let 'ehci' use it.
Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
Applied, thanks
NAK, sorry. Lets re-sync with the kernel's min/max/min3/max3 defines here instead.
Alright, i shall sync the definitions with Linux kernel and send the patch for same.
Will you be dropping this patch from u-boot-usb/next or shall i send a subsequent patch on top of this ?
Dear Vivek Gautam,
Hi Marek,
On Wed, Apr 24, 2013 at 11:49 AM, Vivek Gautam
gautamvivek1987@gmail.com wrote:
On Mon, Apr 22, 2013 at 7:15 PM, Tom Rini trini@ti.com wrote:
On Fri, Apr 19, 2013 at 01:29:12PM +0200, Marek Vasut wrote:
Dear Vivek Gautam,
We can use a common global method for calculating minimum of 3 numbers. Put the same in 'common header' and let 'ehci' use it.
Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
Applied, thanks
NAK, sorry. Lets re-sync with the kernel's min/max/min3/max3 defines here instead.
Alright, i shall sync the definitions with Linux kernel and send the patch for same.
Will you be dropping this patch from u-boot-usb/next or shall i send a subsequent patch on top of this ?
Dropped and pushed
Best regards, Marek Vasut
Dear Vivek Gautam,
Based on 'u-boot-usb' master branch.
This patch-series includes majorly some clean-up, few fixes and then some basic super-speed usb infrastructure addition, to help put support for XHCI in near future.
Changes from v2:
- Added a patch "usb: common: Weed out USB_**_PRINTFs from usb framework" to replace USB_***_PRINTF with debug() so as to get rid of unnecessary DEBUG macros in usb common framework and thereby rebasing other patches on top of this so that no USB_PRINTF appears further.
- Added a patch to reset only USB 2.0 ports, since USB 3.0 protocol ports don't require a reset to happen to move to 'enabled' state.
- Added a patch to move definition of 'min3()' out of ehci-hcd and putting the same as macro definition in common header.
- Using a 'switch-case' in portspeed() in cmd_usb.c
Changes from v1:
- Fixing the issue with 'ifno' as well as added 'if_desc'.
- Instead of turning-on only powered-off hub ports, power-cycle all the hub ports (aka: turning off and then turning on again power to all the ports.
- Fixing commenting style problem in 'usb: Update device class in usb device's descriptor'
- Fixing typo in commit message for 'usb: hub: Fix enumration timeout'
- Removing separate definition for 'struct usb_ep_desc'; thereby adding 'struct usb_ss_ep_comp_descriptor' to 'struct usb_interface' only. As a result modifying the patch accordingly also dropping the patch 'usb: eth: Fix for updated usb interface descriptor structure'
- Dropping the patch 'usb: hub: Increase device enumeration timeout for
broken drives' for now (will come back with a solution at later point of time).
Applied all but the last one, let's now base all subsequent patches on u-boot- usb/next please.
Best regards, Marek Vasut
Hi,
On Sun, Apr 14, 2013 at 10:44 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Based on 'u-boot-usb' master branch.
This patch-series includes majorly some clean-up, few fixes and then some basic super-speed usb infrastructure addition, to help put support for XHCI in near future.
Changes from v2:
- Added a patch "usb: common: Weed out USB_**_PRINTFs from usb framework" to replace USB_***_PRINTF with debug() so as to get rid of unnecessary DEBUG macros in usb common framework and thereby rebasing other patches on top of this so that no USB_PRINTF appears further.
- Added a patch to reset only USB 2.0 ports, since USB 3.0 protocol ports don't require a reset to happen to move to 'enabled' state.
- Added a patch to move definition of 'min3()' out of ehci-hcd and putting the same as macro definition in common header.
- Using a 'switch-case' in portspeed() in cmd_usb.c
Changes from v1:
- Fixing the issue with 'ifno' as well as added 'if_desc'.
- Instead of turning-on only powered-off hub ports, power-cycle all the hub ports (aka: turning off and then turning on again power to all the ports.
- Fixing commenting style problem in 'usb: Update device class in usb device's descriptor'
- Fixing typo in commit message for 'usb: hub: Fix enumration timeout'
- Removing separate definition for 'struct usb_ep_desc'; thereby adding 'struct usb_ss_ep_comp_descriptor' to 'struct usb_interface' only. As a result modifying the patch accordingly also dropping the patch 'usb: eth: Fix for updated usb interface descriptor structure'
- Dropping the patch 'usb: hub: Increase device enumeration timeout for
broken drives' for now (will come back with a solution at later point of time).
Applied all but the last one, let's now base all subsequent patches on u-boot- usb/next please.
And i can't see these patches on u-boot-usb/next. Possibly you haven't pushed out yet ??
Dear Vivek Gautam,
Hi,
On Sun, Apr 14, 2013 at 10:44 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Based on 'u-boot-usb' master branch.
This patch-series includes majorly some clean-up, few fixes and then some basic super-speed usb infrastructure addition, to help put support for XHCI in near future.
Changes from v2:
- Added a patch "usb: common: Weed out USB_**_PRINTFs from usb
framework"
to replace USB_***_PRINTF with debug() so as to get rid of unnecessary DEBUG macros in usb common framework and thereby rebasing other patches on top of this so that no USB_PRINTF appears further.
- Added a patch to reset only USB 2.0 ports, since USB 3.0 protocol
ports
don't require a reset to happen to move to 'enabled' state.
- Added a patch to move definition of 'min3()' out of ehci-hcd and
putting
the same as macro definition in common header.
- Using a 'switch-case' in portspeed() in cmd_usb.c
Changes from v1:
Fixing the issue with 'ifno' as well as added 'if_desc'.
Instead of turning-on only powered-off hub ports, power-cycle all
the hub ports (aka: turning off and then turning on again power to all the ports.
Fixing commenting style problem in
'usb: Update device class in usb device's descriptor'
Fixing typo in commit message for
'usb: hub: Fix enumration timeout'
Removing separate definition for 'struct usb_ep_desc'; thereby adding
'struct usb_ss_ep_comp_descriptor' to 'struct usb_interface' only. As a result modifying the patch accordingly also dropping the patch 'usb: eth: Fix for updated usb interface descriptor structure'
Dropping the patch 'usb: hub: Increase device enumeration timeout for
broken drives' for now (will come back with a solution at later point of time).
Applied all but the last one, let's now base all subsequent patches on u-boot- usb/next please.
And i can't see these patches on u-boot-usb/next. Possibly you haven't pushed out yet ??
You're right, sorry! Fixed now
Best regards, Marek Vasut
On Thu, Apr 18, 2013 at 6:08 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Hi,
On Sun, Apr 14, 2013 at 10:44 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Based on 'u-boot-usb' master branch.
This patch-series includes majorly some clean-up, few fixes and then some basic super-speed usb infrastructure addition, to help put support for XHCI in near future.
Changes from v2:
- Added a patch "usb: common: Weed out USB_**_PRINTFs from usb
framework"
to replace USB_***_PRINTF with debug() so as to get rid of unnecessary DEBUG macros in usb common framework and thereby rebasing other patches on top of this so that no USB_PRINTF appears further.
- Added a patch to reset only USB 2.0 ports, since USB 3.0 protocol
ports
don't require a reset to happen to move to 'enabled' state.
- Added a patch to move definition of 'min3()' out of ehci-hcd and
putting
the same as macro definition in common header.
- Using a 'switch-case' in portspeed() in cmd_usb.c
Changes from v1:
Fixing the issue with 'ifno' as well as added 'if_desc'.
Instead of turning-on only powered-off hub ports, power-cycle all
the hub ports (aka: turning off and then turning on again power to all the ports.
Fixing commenting style problem in
'usb: Update device class in usb device's descriptor'
Fixing typo in commit message for
'usb: hub: Fix enumration timeout'
Removing separate definition for 'struct usb_ep_desc'; thereby adding
'struct usb_ss_ep_comp_descriptor' to 'struct usb_interface' only. As a result modifying the patch accordingly also dropping the patch 'usb: eth: Fix for updated usb interface descriptor structure'
Dropping the patch 'usb: hub: Increase device enumeration timeout for
broken drives' for now (will come back with a solution at later point of time).
Applied all but the last one, let's now base all subsequent patches on u-boot- usb/next please.
And i can't see these patches on u-boot-usb/next. Possibly you haven't pushed out yet ??
You're right, sorry! Fixed now
Thanks you so much !!
Dear Vivek Gautam,
Based on 'u-boot-usb' master branch.
This patch-series includes majorly some clean-up, few fixes and then some basic super-speed usb infrastructure addition, to help put support for XHCI in near future.
btw can you test your patches with MAKEALL -a arm? I get this:
--------------------- SUMMARY ---------------------------- Boards compiled: 306 Boards with errors: 65 ( qong mx35pdk gplugd at91sam9m10g45ek_nandflash pogo_e02 dns325 iconnect lschlv2 lsxhl d2net_v2 inetspace_v2 net2big_v2 netspace_lite_v2 netspace_max_v2 netspace_v2 wireless_space dreamplug guruplug mv88f6281gtw_ge openrd_base openrd_client openrd_ultimate rd6281a sheevaplug ib62x0 dockstar tk71 zmx25 mx23_olinuxino apx4devkit mx23evk m28evk mx28evk sc_sps_1 edminiv2 mx51_efikamx mx51_efikasb mx51evk mx53loco mx6qsabreauto mx6qsabrelite nitrogen6dl nitrogen6dl2g nitrogen6q nitrogen6q2g nitrogen6s nitrogen6s1g cm_t35 mt_ventoux omap3_beagle mcx twister omap4_panda snow smdk5250 harmony seaboard ventana whistler colibri_t20_iris plutux medcom-wide tec paz00 trimslice ) ----------------------------------------------------------
Best regards, Marek Vasut
Hi Marek,
On Sun, Apr 14, 2013 at 11:43 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Based on 'u-boot-usb' master branch.
This patch-series includes majorly some clean-up, few fixes and then some basic super-speed usb infrastructure addition, to help put support for XHCI in near future.
btw can you test your patches with MAKEALL -a arm? I get this:
--------------------- SUMMARY ---------------------------- Boards compiled: 306 Boards with errors: 65 ( qong mx35pdk gplugd at91sam9m10g45ek_nandflash pogo_e02 dns325 iconnect lschlv2 lsxhl d2net_v2 inetspace_v2 net2big_v2 netspace_lite_v2 netspace_max_v2 netspace_v2 wireless_space dreamplug guruplug mv88f6281gtw_ge openrd_base openrd_client openrd_ultimate rd6281a sheevaplug ib62x0 dockstar tk71 zmx25 mx23_olinuxino apx4devkit mx23evk m28evk mx28evk sc_sps_1 edminiv2 mx51_efikamx mx51_efikasb mx51evk mx53loco mx6qsabreauto mx6qsabrelite nitrogen6dl nitrogen6dl2g nitrogen6q nitrogen6q2g nitrogen6s nitrogen6s1g cm_t35 mt_ventoux omap3_beagle mcx twister omap4_panda snow smdk5250 harmony seaboard ventana whistler colibri_t20_iris plutux medcom-wide tec paz00 trimslice )
Tried with MAKEALL got following result
--------------------- SUMMARY ---------------------------- Boards compiled: 306 Boards with errors: 1 ( omap3_evm ) Boards with warnings but no errors: 17 ( VCMA9 smdk2410 kzm9g balloon3 h2200 lubbock palmld palmtc polaris pxa255_idp trizepsiv vpac270_nor_128 vpac270_nor_256 vpac270_ond_256 xaeniax zipitz2 colibri_pxa270 ) ----------------------------------------------------------
There's something to do with Cross Compiler version ?? btw what environment are you compiling the source with.
On Thu, Apr 18, 2013 at 11:54 AM, Vivek Gautam gautamvivek1987@gmail.com wrote:
Hi Marek,
On Sun, Apr 14, 2013 at 11:43 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Based on 'u-boot-usb' master branch.
This patch-series includes majorly some clean-up, few fixes and then some basic super-speed usb infrastructure addition, to help put support for XHCI in near future.
btw can you test your patches with MAKEALL -a arm? I get this:
--------------------- SUMMARY ---------------------------- Boards compiled: 306 Boards with errors: 65 ( qong mx35pdk gplugd at91sam9m10g45ek_nandflash pogo_e02 dns325 iconnect lschlv2 lsxhl d2net_v2 inetspace_v2 net2big_v2 netspace_lite_v2 netspace_max_v2 netspace_v2 wireless_space dreamplug guruplug mv88f6281gtw_ge openrd_base openrd_client openrd_ultimate rd6281a sheevaplug ib62x0 dockstar tk71 zmx25 mx23_olinuxino apx4devkit mx23evk m28evk mx28evk sc_sps_1 edminiv2 mx51_efikamx mx51_efikasb mx51evk mx53loco mx6qsabreauto mx6qsabrelite nitrogen6dl nitrogen6dl2g nitrogen6q nitrogen6q2g nitrogen6s nitrogen6s1g cm_t35 mt_ventoux omap3_beagle mcx twister omap4_panda snow smdk5250 harmony seaboard ventana whistler colibri_t20_iris plutux medcom-wide tec paz00 trimslice )
Tried with MAKEALL got following result
--------------------- SUMMARY ---------------------------- Boards compiled: 306 Boards with errors: 1 ( omap3_evm ) Boards with warnings but no errors: 17 ( VCMA9 smdk2410 kzm9g balloon3 h2200 lubbock palmld palmtc polaris pxa255_idp trizepsiv vpac270_nor_128 vpac270_nor_256 vpac270_ond_256 xaeniax zipitz2 colibri_pxa270 )
**Without my patches i get following result
--------------------- SUMMARY ---------------------------- Boards compiled: 306 Boards with warnings but no errors: 17 ( VCMA9 smdk2410 kzm9g balloon3 h2200 lubbock palmld palmtc polaris pxa255_idp trizepsiv vpac270_nor_128 vpac270_nor_256 vpac270_ond_256 xaeniax zipitz2 colibri_pxa270 ) ----------------------------------------------------------
There's something to do with Cross Compiler version ?? btw what environment are you compiling the source with.
HI Marek,
On Thu, Apr 18, 2013 at 4:29 PM, Vivek Gautam gautamvivek1987@gmail.com wrote:
On Thu, Apr 18, 2013 at 11:54 AM, Vivek Gautam gautamvivek1987@gmail.com wrote:
Hi Marek,
On Sun, Apr 14, 2013 at 11:43 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Based on 'u-boot-usb' master branch.
This patch-series includes majorly some clean-up, few fixes and then some basic super-speed usb infrastructure addition, to help put support for XHCI in near future.
btw can you test your patches with MAKEALL -a arm? I get this:
--------------------- SUMMARY ---------------------------- Boards compiled: 306 Boards with errors: 65 ( qong mx35pdk gplugd at91sam9m10g45ek_nandflash pogo_e02 dns325 iconnect lschlv2 lsxhl d2net_v2 inetspace_v2 net2big_v2 netspace_lite_v2 netspace_max_v2 netspace_v2 wireless_space dreamplug guruplug mv88f6281gtw_ge openrd_base openrd_client openrd_ultimate rd6281a sheevaplug ib62x0 dockstar tk71 zmx25 mx23_olinuxino apx4devkit mx23evk m28evk mx28evk sc_sps_1 edminiv2 mx51_efikamx mx51_efikasb mx51evk mx53loco mx6qsabreauto mx6qsabrelite nitrogen6dl nitrogen6dl2g nitrogen6q nitrogen6q2g nitrogen6s nitrogen6s1g cm_t35 mt_ventoux omap3_beagle mcx twister omap4_panda snow smdk5250 harmony seaboard ventana whistler colibri_t20_iris plutux medcom-wide tec paz00 trimslice )
Tried with MAKEALL got following result
--------------------- SUMMARY ---------------------------- Boards compiled: 306 Boards with errors: 1 ( omap3_evm ) Boards with warnings but no errors: 17 ( VCMA9 smdk2410 kzm9g balloon3 h2200 lubbock palmld palmtc polaris pxa255_idp trizepsiv vpac270_nor_128 vpac270_nor_256 vpac270_ond_256 xaeniax zipitz2 colibri_pxa270 )
I actually checked now just for omap3_evm configuration by trying out: make distclean make omap3_evm_config make
But strangely i didn't get any build errros for omap3_evm board on explicitly compiling for it. Any clue ?
**Without my patches i get following result
--------------------- SUMMARY ---------------------------- Boards compiled: 306 Boards with warnings but no errors: 17 ( VCMA9 smdk2410 kzm9g balloon3 h2200 lubbock palmld palmtc polaris pxa255_idp trizepsiv vpac270_nor_128 vpac270_nor_256 vpac270_ond_256 xaeniax zipitz2 colibri_pxa270 )
There's something to do with Cross Compiler version ?? btw what environment are you compiling the source with.
I am using "arm-2011.09" cross toolchain.
Dear Vivek Gautam,
HI Marek,
On Thu, Apr 18, 2013 at 4:29 PM, Vivek Gautam gautamvivek1987@gmail.com
wrote:
On Thu, Apr 18, 2013 at 11:54 AM, Vivek Gautam
gautamvivek1987@gmail.com wrote:
Hi Marek,
On Sun, Apr 14, 2013 at 11:43 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Based on 'u-boot-usb' master branch.
This patch-series includes majorly some clean-up, few fixes and then some basic super-speed usb infrastructure addition, to help put support for XHCI in near future.
btw can you test your patches with MAKEALL -a arm? I get this:
--------------------- SUMMARY ---------------------------- Boards compiled: 306 Boards with errors: 65 ( qong mx35pdk gplugd at91sam9m10g45ek_nandflash pogo_e02 dns325 iconnect lschlv2 lsxhl d2net_v2 inetspace_v2 net2big_v2 netspace_lite_v2 netspace_max_v2 netspace_v2 wireless_space dreamplug guruplug mv88f6281gtw_ge openrd_base openrd_client openrd_ultimate rd6281a sheevaplug ib62x0 dockstar tk71 zmx25 mx23_olinuxino apx4devkit mx23evk m28evk mx28evk sc_sps_1 edminiv2 mx51_efikamx mx51_efikasb mx51evk mx53loco mx6qsabreauto mx6qsabrelite nitrogen6dl nitrogen6dl2g nitrogen6q nitrogen6q2g nitrogen6s nitrogen6s1g cm_t35 mt_ventoux omap3_beagle mcx twister omap4_panda snow smdk5250 harmony seaboard ventana whistler colibri_t20_iris plutux medcom-wide tec paz00 trimslice )
Tried with MAKEALL got following result
--------------------- SUMMARY ---------------------------- Boards compiled: 306 Boards with errors: 1 ( omap3_evm ) Boards with warnings but no errors: 17 ( VCMA9 smdk2410 kzm9g balloon3 h2200 lubbock palmld palmtc polaris pxa255_idp trizepsiv vpac270_nor_128 vpac270_nor_256 vpac270_ond_256 xaeniax zipitz2 colibri_pxa270 )
I actually checked now just for omap3_evm configuration by trying out: make distclean make omap3_evm_config make
But strangely i didn't get any build errros for omap3_evm board on explicitly compiling for it. Any clue ?
**Without my patches i get following result
--------------------- SUMMARY ---------------------------- Boards compiled: 306 Boards with warnings but no errors: 17 ( VCMA9 smdk2410 kzm9g balloon3 h2200 lubbock palmld palmtc polaris pxa255_idp trizepsiv vpac270_nor_128 vpac270_nor_256 vpac270_ond_256 xaeniax zipitz2 colibri_pxa270 )
There's something to do with Cross Compiler version ?? btw what environment are you compiling the source with.
I am using "arm-2011.09" cross toolchain.
I use ELDK 5.3 and Debian 4.7.2-5 to do by builds. But now that I'm looking at it, it's this patch that caused it, sorry.
commit 28b31a5937b89528c40df24dd6c9122578880605 Author: Julius Werner jwerner@chromium.org Date: Thu Feb 28 18:08:40 2013 +0000
usb: Add new command to set USB 2.0 port test modes
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index 8464850..19d4352 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -630,7 +630,7 @@ ehci_submit_root(struct usb_device *dev, unsigned long pipe, void *buffer, printf("The request port(%d) is not configured\n", port - 1); return -1; } - status_reg = (uint32_t *)&hcor->or_portsc[port - 1]; + status_reg = (uint32_t *)&ctrl->hcor->or_portsc[port - 1]; srclen = 0;
debug("req=%u (%#x), type=%u (%#x), value=%u, index=%u\n",
This change fixes is, right Julius ?
Best regards, Marek Vasut
Hi Marek,
I'm sorry, that must have slipped by when I ported the change from my local fork. Your patch is correct, you just need to add the "ctrl->" there.
On Thu, Apr 18, 2013 at 5:43 AM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
HI Marek,
On Thu, Apr 18, 2013 at 4:29 PM, Vivek Gautam gautamvivek1987@gmail.com
wrote:
On Thu, Apr 18, 2013 at 11:54 AM, Vivek Gautam
gautamvivek1987@gmail.com wrote:
Hi Marek,
On Sun, Apr 14, 2013 at 11:43 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Based on 'u-boot-usb' master branch.
This patch-series includes majorly some clean-up, few fixes and then some basic super-speed usb infrastructure addition, to help put support for XHCI in near future.
btw can you test your patches with MAKEALL -a arm? I get this:
--------------------- SUMMARY ---------------------------- Boards compiled: 306 Boards with errors: 65 ( qong mx35pdk gplugd at91sam9m10g45ek_nandflash pogo_e02 dns325 iconnect lschlv2 lsxhl d2net_v2 inetspace_v2 net2big_v2 netspace_lite_v2 netspace_max_v2 netspace_v2 wireless_space dreamplug guruplug mv88f6281gtw_ge openrd_base openrd_client openrd_ultimate rd6281a sheevaplug ib62x0 dockstar tk71 zmx25 mx23_olinuxino apx4devkit mx23evk m28evk mx28evk sc_sps_1 edminiv2 mx51_efikamx mx51_efikasb mx51evk mx53loco mx6qsabreauto mx6qsabrelite nitrogen6dl nitrogen6dl2g nitrogen6q nitrogen6q2g nitrogen6s nitrogen6s1g cm_t35 mt_ventoux omap3_beagle mcx twister omap4_panda snow smdk5250 harmony seaboard ventana whistler colibri_t20_iris plutux medcom-wide tec paz00 trimslice )
Tried with MAKEALL got following result
--------------------- SUMMARY ---------------------------- Boards compiled: 306 Boards with errors: 1 ( omap3_evm ) Boards with warnings but no errors: 17 ( VCMA9 smdk2410 kzm9g balloon3 h2200 lubbock palmld palmtc polaris pxa255_idp trizepsiv vpac270_nor_128 vpac270_nor_256 vpac270_ond_256 xaeniax zipitz2 colibri_pxa270 )
I actually checked now just for omap3_evm configuration by trying out: make distclean make omap3_evm_config make
But strangely i didn't get any build errros for omap3_evm board on explicitly compiling for it. Any clue ?
**Without my patches i get following result
--------------------- SUMMARY ---------------------------- Boards compiled: 306 Boards with warnings but no errors: 17 ( VCMA9 smdk2410 kzm9g balloon3 h2200 lubbock palmld palmtc polaris pxa255_idp trizepsiv vpac270_nor_128 vpac270_nor_256 vpac270_ond_256 xaeniax zipitz2 colibri_pxa270 )
There's something to do with Cross Compiler version ?? btw what environment are you compiling the source with.
I am using "arm-2011.09" cross toolchain.
I use ELDK 5.3 and Debian 4.7.2-5 to do by builds. But now that I'm looking at it, it's this patch that caused it, sorry.
commit 28b31a5937b89528c40df24dd6c9122578880605 Author: Julius Werner jwerner@chromium.org Date: Thu Feb 28 18:08:40 2013 +0000
usb: Add new command to set USB 2.0 port test modesdiff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index 8464850..19d4352 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -630,7 +630,7 @@ ehci_submit_root(struct usb_device *dev, unsigned long pipe, void *buffer, printf("The request port(%d) is not configured\n", port - 1); return -1; }
status_reg = (uint32_t *)&hcor->or_portsc[port - 1];
status_reg = (uint32_t *)&ctrl->hcor->or_portsc[port - 1]; srclen = 0; debug("req=%u (%#x), type=%u (%#x), value=%u, index=%u\n",This change fixes is, right Julius ?
Best regards, Marek Vasut
Hi Julius,
Hi Marek,
I'm sorry, that must have slipped by when I ported the change from my local fork. Your patch is correct, you just need to add the "ctrl->" there.
Well, next time please make sure to compile-test your patches at least before you send them.
The stuff is now pushed, Vivek.
Best regards, Marek Vasut
On Fri, Apr 19, 2013 at 12:45 AM, Marek Vasut marex@denx.de wrote:
Hi Julius,
Hi Marek,
I'm sorry, that must have slipped by when I ported the change from my local fork. Your patch is correct, you just need to add the "ctrl->" there.
Well, next time please make sure to compile-test your patches at least before you send them.
The stuff is now pushed, Vivek.
Thanks Marek.
participants (5)
-
Julius Werner -
Marek Vasut -
Tom Rini -
Vivek Gautam -
Vivek Gautam