All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v3 0/8] usb: ss: Some fixes and cleanup for USB super-speed support
@ 2013-04-12 11:04 Vivek Gautam
  2013-04-12 11:04 ` [U-Boot] [PATCH v3 1/8] usb: common: Weed out USB_**_PRINTFs from usb framework Vivek Gautam
                   ` (9 more replies)
  0 siblings, 10 replies; 48+ messages in thread
From: Vivek Gautam @ 2013-04-12 11:04 UTC (permalink / raw)
  To: u-boot

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(-)

-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v3 1/8] usb: common: Weed out USB_**_PRINTFs from usb framework
  2013-04-12 11:04 [U-Boot] [PATCH v3 0/8] usb: ss: Some fixes and cleanup for USB super-speed support Vivek Gautam
@ 2013-04-12 11:04 ` Vivek Gautam
  2013-04-12 11:04 ` [U-Boot] [PATCH v3 2/8] USB: Some cleanup prior to USB 3.0 interface addition Vivek Gautam
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 48+ messages in thread
From: Vivek Gautam @ 2013-04-12 11:04 UTC (permalink / raw)
  To: u-boot

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;
 }
-- 
1.7.6.5

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v3 2/8] USB: Some cleanup prior to USB 3.0 interface addition
  2013-04-12 11:04 [U-Boot] [PATCH v3 0/8] usb: ss: Some fixes and cleanup for USB super-speed support Vivek Gautam
  2013-04-12 11:04 ` [U-Boot] [PATCH v3 1/8] usb: common: Weed out USB_**_PRINTFs from usb framework Vivek Gautam
@ 2013-04-12 11:04 ` Vivek Gautam
  2013-04-12 11:04 ` [U-Boot] [PATCH v3 3/8] usb: hub: Power-cycle on root-hub ports Vivek Gautam
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 48+ messages in thread
From: Vivek Gautam @ 2013-04-12 11:04 UTC (permalink / raw)
  To: u-boot

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
-- 
1.7.6.5

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v3 3/8] usb: hub: Power-cycle on root-hub ports
  2013-04-12 11:04 [U-Boot] [PATCH v3 0/8] usb: ss: Some fixes and cleanup for USB super-speed support Vivek Gautam
  2013-04-12 11:04 ` [U-Boot] [PATCH v3 1/8] usb: common: Weed out USB_**_PRINTFs from usb framework Vivek Gautam
  2013-04-12 11:04 ` [U-Boot] [PATCH v3 2/8] USB: Some cleanup prior to USB 3.0 interface addition Vivek Gautam
@ 2013-04-12 11:04 ` Vivek Gautam
  2013-04-19 19:00   ` [U-Boot] [U-Boot, v3, " Julius Werner
  2013-04-12 11:04 ` [U-Boot] [PATCH v3 4/8] usb: Update device class in usb device's descriptor Vivek Gautam
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 48+ messages in thread
From: Vivek Gautam @ 2013-04-12 11:04 UTC (permalink / raw)
  To: u-boot

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);
 	}
-- 
1.7.6.5

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v3 4/8] usb: Update device class in usb device's descriptor
  2013-04-12 11:04 [U-Boot] [PATCH v3 0/8] usb: ss: Some fixes and cleanup for USB super-speed support Vivek Gautam
                   ` (2 preceding siblings ...)
  2013-04-12 11:04 ` [U-Boot] [PATCH v3 3/8] usb: hub: Power-cycle on root-hub ports Vivek Gautam
@ 2013-04-12 11:04 ` Vivek Gautam
  2013-04-19 18:39   ` [U-Boot] [U-Boot, v3, " Julius Werner
  2013-04-12 11:04 ` [U-Boot] [PATCH v3 5/8] usb: hub: Fix enumration timeout Vivek Gautam
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 48+ messages in thread
From: Vivek Gautam @ 2013-04-12 11:04 UTC (permalink / raw)
  To: u-boot

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) {
-- 
1.7.6.5

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v3 5/8] usb: hub: Fix enumration timeout
  2013-04-12 11:04 [U-Boot] [PATCH v3 0/8] usb: ss: Some fixes and cleanup for USB super-speed support Vivek Gautam
                   ` (3 preceding siblings ...)
  2013-04-12 11:04 ` [U-Boot] [PATCH v3 4/8] usb: Update device class in usb device's descriptor Vivek Gautam
@ 2013-04-12 11:04 ` Vivek Gautam
  2013-04-12 11:04 ` [U-Boot] [PATCH v3 6/8] USB: SS: Add support for Super Speed USB interface Vivek Gautam
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 48+ messages in thread
From: Vivek Gautam @ 2013-04-12 11:04 UTC (permalink / raw)
  To: u-boot

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)
-- 
1.7.6.5

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v3 6/8] USB: SS: Add support for Super Speed USB interface
  2013-04-12 11:04 [U-Boot] [PATCH v3 0/8] usb: ss: Some fixes and cleanup for USB super-speed support Vivek Gautam
                   ` (4 preceding siblings ...)
  2013-04-12 11:04 ` [U-Boot] [PATCH v3 5/8] usb: hub: Fix enumration timeout Vivek Gautam
@ 2013-04-12 11:04 ` Vivek Gautam
  2013-04-19 18:22   ` [U-Boot] [U-Boot, v3, " Julius Werner
  2013-04-12 11:04 ` [U-Boot] [PATCH v3 7/8] usb: hub: Reset only usb 2.0 ports Vivek Gautam
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 48+ messages in thread
From: Vivek Gautam @ 2013-04-12 11:04 UTC (permalink / raw)
  To: u-boot

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
-- 
1.7.6.5

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v3 7/8] usb: hub: Reset only usb 2.0 ports
  2013-04-12 11:04 [U-Boot] [PATCH v3 0/8] usb: ss: Some fixes and cleanup for USB super-speed support Vivek Gautam
                   ` (5 preceding siblings ...)
  2013-04-12 11:04 ` [U-Boot] [PATCH v3 6/8] USB: SS: Add support for Super Speed USB interface Vivek Gautam
@ 2013-04-12 11:04 ` Vivek Gautam
  2013-04-23 17:23   ` [U-Boot] [U-Boot,v3,7/8] " Julius Werner
  2013-04-12 11:04 ` [U-Boot] [PATCH v3 8/8] usb: common: Use a global macro for 'min3' Vivek Gautam
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 48+ messages in thread
From: Vivek Gautam @ 2013-04-12 11:04 UTC (permalink / raw)
  To: u-boot

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);
-- 
1.7.6.5

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v3 8/8] usb: common: Use a global macro for 'min3'
  2013-04-12 11:04 [U-Boot] [PATCH v3 0/8] usb: ss: Some fixes and cleanup for USB super-speed support Vivek Gautam
                   ` (6 preceding siblings ...)
  2013-04-12 11:04 ` [U-Boot] [PATCH v3 7/8] usb: hub: Reset only usb 2.0 ports Vivek Gautam
@ 2013-04-12 11:04 ` Vivek Gautam
  2013-04-14 17:11   ` Marek Vasut
  2013-04-14 17:14 ` [U-Boot] [PATCH v3 0/8] usb: ss: Some fixes and cleanup for USB super-speed support Marek Vasut
  2013-04-14 18:13 ` Marek Vasut
  9 siblings, 1 reply; 48+ messages in thread
From: Vivek Gautam @ 2013-04-12 11:04 UTC (permalink / raw)
  To: u-boot

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.
  *
-- 
1.7.6.5

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v3 8/8] usb: common: Use a global macro for 'min3'
  2013-04-12 11:04 ` [U-Boot] [PATCH v3 8/8] usb: common: Use a global macro for 'min3' Vivek Gautam
@ 2013-04-14 17:11   ` Marek Vasut
  2013-04-19  9:38     ` [U-Boot] [PATCH] usb: common: Use a global definition " Vivek Gautam
  0 siblings, 1 reply; 48+ messages in thread
From: Marek Vasut @ 2013-04-14 17:11 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v3 0/8] usb: ss: Some fixes and cleanup for USB super-speed support
  2013-04-12 11:04 [U-Boot] [PATCH v3 0/8] usb: ss: Some fixes and cleanup for USB super-speed support Vivek Gautam
                   ` (7 preceding siblings ...)
  2013-04-12 11:04 ` [U-Boot] [PATCH v3 8/8] usb: common: Use a global macro for 'min3' Vivek Gautam
@ 2013-04-14 17:14 ` Marek Vasut
  2013-04-18  6:25   ` Vivek Gautam
  2013-04-14 18:13 ` Marek Vasut
  9 siblings, 1 reply; 48+ messages in thread
From: Marek Vasut @ 2013-04-14 17:14 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v3 0/8] usb: ss: Some fixes and cleanup for USB super-speed support
  2013-04-12 11:04 [U-Boot] [PATCH v3 0/8] usb: ss: Some fixes and cleanup for USB super-speed support Vivek Gautam
                   ` (8 preceding siblings ...)
  2013-04-14 17:14 ` [U-Boot] [PATCH v3 0/8] usb: ss: Some fixes and cleanup for USB super-speed support Marek Vasut
@ 2013-04-14 18:13 ` Marek Vasut
  2013-04-18  6:24   ` Vivek Gautam
  9 siblings, 1 reply; 48+ messages in thread
From: Marek Vasut @ 2013-04-14 18:13 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v3 0/8] usb: ss: Some fixes and cleanup for USB super-speed support
  2013-04-14 18:13 ` Marek Vasut
@ 2013-04-18  6:24   ` Vivek Gautam
  2013-04-18 10:59     ` Vivek Gautam
  0 siblings, 1 reply; 48+ messages in thread
From: Vivek Gautam @ 2013-04-18  6:24 UTC (permalink / raw)
  To: u-boot

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.


-- 
Thanks & Regards
Vivek

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v3 0/8] usb: ss: Some fixes and cleanup for USB super-speed support
  2013-04-14 17:14 ` [U-Boot] [PATCH v3 0/8] usb: ss: Some fixes and cleanup for USB super-speed support Marek Vasut
@ 2013-04-18  6:25   ` Vivek Gautam
  2013-04-18 12:38     ` Marek Vasut
  0 siblings, 1 reply; 48+ messages in thread
From: Vivek Gautam @ 2013-04-18  6:25 UTC (permalink / raw)
  To: u-boot

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 ??



-- 
Thanks & Regards
Vivek

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v3 0/8] usb: ss: Some fixes and cleanup for USB super-speed support
  2013-04-18  6:24   ` Vivek Gautam
@ 2013-04-18 10:59     ` Vivek Gautam
  2013-04-18 11:08       ` Vivek Gautam
  0 siblings, 1 reply; 48+ messages in thread
From: Vivek Gautam @ 2013-04-18 10:59 UTC (permalink / raw)
  To: u-boot

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.


-- 
Thanks & Regards
Vivek

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v3 0/8] usb: ss: Some fixes and cleanup for USB super-speed support
  2013-04-18 10:59     ` Vivek Gautam
@ 2013-04-18 11:08       ` Vivek Gautam
  2013-04-18 12:43         ` Marek Vasut
  0 siblings, 1 reply; 48+ messages in thread
From: Vivek Gautam @ 2013-04-18 11:08 UTC (permalink / raw)
  To: u-boot

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.



-- 
Thanks & Regards
Vivek

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v3 0/8] usb: ss: Some fixes and cleanup for USB super-speed support
  2013-04-18  6:25   ` Vivek Gautam
@ 2013-04-18 12:38     ` Marek Vasut
  2013-04-18 13:08       ` Vivek Gautam
  0 siblings, 1 reply; 48+ messages in thread
From: Marek Vasut @ 2013-04-18 12:38 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v3 0/8] usb: ss: Some fixes and cleanup for USB super-speed support
  2013-04-18 11:08       ` Vivek Gautam
@ 2013-04-18 12:43         ` Marek Vasut
  2013-04-18 17:11           ` Julius Werner
  0 siblings, 1 reply; 48+ messages in thread
From: Marek Vasut @ 2013-04-18 12:43 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v3 0/8] usb: ss: Some fixes and cleanup for USB super-speed support
  2013-04-18 12:38     ` Marek Vasut
@ 2013-04-18 13:08       ` Vivek Gautam
  0 siblings, 0 replies; 48+ messages in thread
From: Vivek Gautam @ 2013-04-18 13:08 UTC (permalink / raw)
  To: u-boot

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 !!

-- 
Thanks & Regards
Vivek

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v3 0/8] usb: ss: Some fixes and cleanup for USB super-speed support
  2013-04-18 12:43         ` Marek Vasut
@ 2013-04-18 17:11           ` Julius Werner
  2013-04-18 19:15             ` Marek Vasut
  0 siblings, 1 reply; 48+ messages in thread
From: Julius Werner @ 2013-04-18 17:11 UTC (permalink / raw)
  To: u-boot

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 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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v3 0/8] usb: ss: Some fixes and cleanup for USB super-speed support
  2013-04-18 17:11           ` Julius Werner
@ 2013-04-18 19:15             ` Marek Vasut
  2013-04-19  4:51               ` Vivek Gautam
  0 siblings, 1 reply; 48+ messages in thread
From: Marek Vasut @ 2013-04-18 19:15 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v3 0/8] usb: ss: Some fixes and cleanup for USB super-speed support
  2013-04-18 19:15             ` Marek Vasut
@ 2013-04-19  4:51               ` Vivek Gautam
  0 siblings, 0 replies; 48+ messages in thread
From: Vivek Gautam @ 2013-04-19  4:51 UTC (permalink / raw)
  To: u-boot

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.



-- 
Best Regards
Vivek

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH] usb: common: Use a global definition for 'min3'
  2013-04-14 17:11   ` Marek Vasut
@ 2013-04-19  9:38     ` Vivek Gautam
  2013-04-19 11:29       ` Marek Vasut
  0 siblings, 1 reply; 48+ messages in thread
From: Vivek Gautam @ 2013-04-19  9:38 UTC (permalink / raw)
  To: u-boot

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.
  *
-- 
1.7.6.5

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH] usb: common: Use a global definition for 'min3'
  2013-04-19  9:38     ` [U-Boot] [PATCH] usb: common: Use a global definition " Vivek Gautam
@ 2013-04-19 11:29       ` Marek Vasut
  2013-04-22 13:45         ` Tom Rini
  0 siblings, 1 reply; 48+ messages in thread
From: Marek Vasut @ 2013-04-19 11:29 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot, v3, 6/8] USB: SS: Add support for Super Speed USB interface
  2013-04-12 11:04 ` [U-Boot] [PATCH v3 6/8] USB: SS: Add support for Super Speed USB interface Vivek Gautam
@ 2013-04-19 18:22   ` Julius Werner
  2013-04-19 18:38     ` Marek Vasut
  2013-04-22  6:43     ` Vivek Gautam
  0 siblings, 2 replies; 48+ messages in thread
From: Julius Werner @ 2013-04-19 18:22 UTC (permalink / raw)
  To: u-boot

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;
>  		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).

> +		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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot, v3, 6/8] USB: SS: Add support for Super Speed USB interface
  2013-04-19 18:22   ` [U-Boot] [U-Boot, v3, " Julius Werner
@ 2013-04-19 18:38     ` Marek Vasut
  2013-04-19 20:32       ` Julius Werner
  2013-04-22  6:43     ` Vivek Gautam
  1 sibling, 1 reply; 48+ messages in thread
From: Marek Vasut @ 2013-04-19 18:38 UTC (permalink / raw)
  To: u-boot

Hi Julius,

> Migrating my comments here for public discussion.

Maybe you can make a patch(set) against u-boot-usb/next ?

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot, v3, 4/8] usb: Update device class in usb device's descriptor
  2013-04-12 11:04 ` [U-Boot] [PATCH v3 4/8] usb: Update device class in usb device's descriptor Vivek Gautam
@ 2013-04-19 18:39   ` Julius Werner
  0 siblings, 0 replies; 48+ messages in thread
From: Julius Werner @ 2013-04-19 18:39 UTC (permalink / raw)
  To: u-boot

> 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) {

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot, v3, 3/8] usb: hub: Power-cycle on root-hub ports
  2013-04-12 11:04 ` [U-Boot] [PATCH v3 3/8] usb: hub: Power-cycle on root-hub ports Vivek Gautam
@ 2013-04-19 19:00   ` Julius Werner
  2013-04-22  8:21     ` Vivek Gautam
  0 siblings, 1 reply; 48+ messages in thread
From: Julius Werner @ 2013-04-19 19:00 UTC (permalink / raw)
  To: u-boot

> 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);
>  	}

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot, v3, 6/8] USB: SS: Add support for Super Speed USB interface
  2013-04-19 18:38     ` Marek Vasut
@ 2013-04-19 20:32       ` Julius Werner
  2013-04-20 11:57         ` Marek Vasut
  0 siblings, 1 reply; 48+ messages in thread
From: Julius Werner @ 2013-04-19 20:32 UTC (permalink / raw)
  To: u-boot

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.

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot, v3, 6/8] USB: SS: Add support for Super Speed USB interface
  2013-04-19 20:32       ` Julius Werner
@ 2013-04-20 11:57         ` Marek Vasut
  2013-04-22  6:46           ` Vivek Gautam
  0 siblings, 1 reply; 48+ messages in thread
From: Marek Vasut @ 2013-04-20 11:57 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot, v3, 6/8] USB: SS: Add support for Super Speed USB interface
  2013-04-19 18:22   ` [U-Boot] [U-Boot, v3, " Julius Werner
  2013-04-19 18:38     ` Marek Vasut
@ 2013-04-22  6:43     ` Vivek Gautam
  2013-04-22 22:14       ` Julius Werner
  1 sibling, 1 reply; 48+ messages in thread
From: Vivek Gautam @ 2013-04-22  6:43 UTC (permalink / raw)
  To: u-boot

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";
>> -     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))
>
> 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



-- 
Best Regards
Vivek

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot, v3, 6/8] USB: SS: Add support for Super Speed USB interface
  2013-04-20 11:57         ` Marek Vasut
@ 2013-04-22  6:46           ` Vivek Gautam
  2013-04-23  2:24             ` Marek Vasut
  0 siblings, 1 reply; 48+ messages in thread
From: Vivek Gautam @ 2013-04-22  6:46 UTC (permalink / raw)
  To: u-boot

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.



-- 
Best Regards
Vivek

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot, v3, 3/8] usb: hub: Power-cycle on root-hub ports
  2013-04-19 19:00   ` [U-Boot] [U-Boot, v3, " Julius Werner
@ 2013-04-22  8:21     ` Vivek Gautam
  2013-04-22 22:02       ` Julius Werner
  0 siblings, 1 reply; 48+ messages in thread
From: Vivek Gautam @ 2013-04-22  8:21 UTC (permalink / raw)
  To: u-boot

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);
>>       }



-- 
Best Regards
Vivek

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH] usb: common: Use a global definition for 'min3'
  2013-04-19 11:29       ` Marek Vasut
@ 2013-04-22 13:45         ` Tom Rini
  2013-04-24  6:19           ` Vivek Gautam
  0 siblings, 1 reply; 48+ messages in thread
From: Tom Rini @ 2013-04-22 13:45 UTC (permalink / raw)
  To: u-boot

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.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130422/8c39b4fe/attachment.pgp>

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot, v3, 3/8] usb: hub: Power-cycle on root-hub ports
  2013-04-22  8:21     ` Vivek Gautam
@ 2013-04-22 22:02       ` Julius Werner
  2013-04-23  6:45         ` Vivek Gautam
  0 siblings, 1 reply; 48+ messages in thread
From: Julius Werner @ 2013-04-22 22:02 UTC (permalink / raw)
  To: u-boot

> 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.

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot, v3, 6/8] USB: SS: Add support for Super Speed USB interface
  2013-04-22  6:43     ` Vivek Gautam
@ 2013-04-22 22:14       ` Julius Werner
  2013-04-23  4:39         ` Vivek Gautam
  0 siblings, 1 reply; 48+ messages in thread
From: Julius Werner @ 2013-04-22 22:14 UTC (permalink / raw)
  To: u-boot

>> 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.

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot, v3, 6/8] USB: SS: Add support for Super Speed USB interface
  2013-04-22  6:46           ` Vivek Gautam
@ 2013-04-23  2:24             ` Marek Vasut
  2013-04-23  6:46               ` Vivek Gautam
  0 siblings, 1 reply; 48+ messages in thread
From: Marek Vasut @ 2013-04-23  2:24 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot, v3, 6/8] USB: SS: Add support for Super Speed USB interface
  2013-04-22 22:14       ` Julius Werner
@ 2013-04-23  4:39         ` Vivek Gautam
  0 siblings, 0 replies; 48+ messages in thread
From: Vivek Gautam @ 2013-04-23  4:39 UTC (permalink / raw)
  To: u-boot

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 !!



-- 
Best Regards
Vivek

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot, v3, 3/8] usb: hub: Power-cycle on root-hub ports
  2013-04-22 22:02       ` Julius Werner
@ 2013-04-23  6:45         ` Vivek Gautam
  0 siblings, 0 replies; 48+ messages in thread
From: Vivek Gautam @ 2013-04-23  6:45 UTC (permalink / raw)
  To: u-boot

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.



-- 
Best Regards
Vivek

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot, v3, 6/8] USB: SS: Add support for Super Speed USB interface
  2013-04-23  2:24             ` Marek Vasut
@ 2013-04-23  6:46               ` Vivek Gautam
  0 siblings, 0 replies; 48+ messages in thread
From: Vivek Gautam @ 2013-04-23  6:46 UTC (permalink / raw)
  To: u-boot

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



-- 
Best Regards
Vivek

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot,v3,7/8] usb: hub: Reset only usb 2.0 ports
  2013-04-12 11:04 ` [U-Boot] [PATCH v3 7/8] usb: hub: Reset only usb 2.0 ports Vivek Gautam
@ 2013-04-23 17:23   ` Julius Werner
  2013-04-24  0:21     ` Marek Vasut
  0 siblings, 1 reply; 48+ messages in thread
From: Julius Werner @ 2013-04-23 17:23 UTC (permalink / raw)
  To: u-boot

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);

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot,v3,7/8] usb: hub: Reset only usb 2.0 ports
  2013-04-23 17:23   ` [U-Boot] [U-Boot,v3,7/8] " Julius Werner
@ 2013-04-24  0:21     ` Marek Vasut
  2013-04-24  6:08       ` Vivek Gautam
  0 siblings, 1 reply; 48+ messages in thread
From: Marek Vasut @ 2013-04-24  0:21 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot,v3,7/8] usb: hub: Reset only usb 2.0 ports
  2013-04-24  0:21     ` Marek Vasut
@ 2013-04-24  6:08       ` Vivek Gautam
  2013-04-30 12:24         ` Vivek Gautam
  0 siblings, 1 reply; 48+ messages in thread
From: Vivek Gautam @ 2013-04-24  6:08 UTC (permalink / raw)
  To: u-boot

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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot



-- 
Best Regards
Vivek

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH] usb: common: Use a global definition for 'min3'
  2013-04-22 13:45         ` Tom Rini
@ 2013-04-24  6:19           ` Vivek Gautam
  2013-04-24  6:21             ` Vivek Gautam
  0 siblings, 1 reply; 48+ messages in thread
From: Vivek Gautam @ 2013-04-24  6:19 UTC (permalink / raw)
  To: u-boot

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.




-- 
Best Regards
Vivek

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH] usb: common: Use a global definition for 'min3'
  2013-04-24  6:19           ` Vivek Gautam
@ 2013-04-24  6:21             ` Vivek Gautam
  2013-04-24 12:03               ` Marek Vasut
  0 siblings, 1 reply; 48+ messages in thread
From: Vivek Gautam @ 2013-04-24  6:21 UTC (permalink / raw)
  To: u-boot

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 ?


-- 
Best Regards
Vivek

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH] usb: common: Use a global definition for 'min3'
  2013-04-24  6:21             ` Vivek Gautam
@ 2013-04-24 12:03               ` Marek Vasut
  0 siblings, 0 replies; 48+ messages in thread
From: Marek Vasut @ 2013-04-24 12:03 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot,v3,7/8] usb: hub: Reset only usb 2.0 ports
  2013-04-24  6:08       ` Vivek Gautam
@ 2013-04-30 12:24         ` Vivek Gautam
  2013-04-30 17:11           ` Marek Vasut
  0 siblings, 1 reply; 48+ messages in thread
From: Vivek Gautam @ 2013-04-30 12:24 UTC (permalink / raw)
  To: 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);


-- 
Best Regards
Vivek

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot,v3,7/8] usb: hub: Reset only usb 2.0 ports
  2013-04-30 12:24         ` Vivek Gautam
@ 2013-04-30 17:11           ` Marek Vasut
  0 siblings, 0 replies; 48+ messages in thread
From: Marek Vasut @ 2013-04-30 17:11 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 48+ messages in thread

end of thread, other threads:[~2013-04-30 17:11 UTC | newest]

Thread overview: 48+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-04-12 11:04 [U-Boot] [PATCH v3 0/8] usb: ss: Some fixes and cleanup for USB super-speed support Vivek Gautam
2013-04-12 11:04 ` [U-Boot] [PATCH v3 1/8] usb: common: Weed out USB_**_PRINTFs from usb framework Vivek Gautam
2013-04-12 11:04 ` [U-Boot] [PATCH v3 2/8] USB: Some cleanup prior to USB 3.0 interface addition Vivek Gautam
2013-04-12 11:04 ` [U-Boot] [PATCH v3 3/8] usb: hub: Power-cycle on root-hub ports Vivek Gautam
2013-04-19 19:00   ` [U-Boot] [U-Boot, v3, " Julius Werner
2013-04-22  8:21     ` Vivek Gautam
2013-04-22 22:02       ` Julius Werner
2013-04-23  6:45         ` Vivek Gautam
2013-04-12 11:04 ` [U-Boot] [PATCH v3 4/8] usb: Update device class in usb device's descriptor Vivek Gautam
2013-04-19 18:39   ` [U-Boot] [U-Boot, v3, " Julius Werner
2013-04-12 11:04 ` [U-Boot] [PATCH v3 5/8] usb: hub: Fix enumration timeout Vivek Gautam
2013-04-12 11:04 ` [U-Boot] [PATCH v3 6/8] USB: SS: Add support for Super Speed USB interface Vivek Gautam
2013-04-19 18:22   ` [U-Boot] [U-Boot, v3, " Julius Werner
2013-04-19 18:38     ` Marek Vasut
2013-04-19 20:32       ` Julius Werner
2013-04-20 11:57         ` Marek Vasut
2013-04-22  6:46           ` Vivek Gautam
2013-04-23  2:24             ` Marek Vasut
2013-04-23  6:46               ` Vivek Gautam
2013-04-22  6:43     ` Vivek Gautam
2013-04-22 22:14       ` Julius Werner
2013-04-23  4:39         ` Vivek Gautam
2013-04-12 11:04 ` [U-Boot] [PATCH v3 7/8] usb: hub: Reset only usb 2.0 ports Vivek Gautam
2013-04-23 17:23   ` [U-Boot] [U-Boot,v3,7/8] " Julius Werner
2013-04-24  0:21     ` Marek Vasut
2013-04-24  6:08       ` Vivek Gautam
2013-04-30 12:24         ` Vivek Gautam
2013-04-30 17:11           ` Marek Vasut
2013-04-12 11:04 ` [U-Boot] [PATCH v3 8/8] usb: common: Use a global macro for 'min3' Vivek Gautam
2013-04-14 17:11   ` Marek Vasut
2013-04-19  9:38     ` [U-Boot] [PATCH] usb: common: Use a global definition " Vivek Gautam
2013-04-19 11:29       ` Marek Vasut
2013-04-22 13:45         ` Tom Rini
2013-04-24  6:19           ` Vivek Gautam
2013-04-24  6:21             ` Vivek Gautam
2013-04-24 12:03               ` Marek Vasut
2013-04-14 17:14 ` [U-Boot] [PATCH v3 0/8] usb: ss: Some fixes and cleanup for USB super-speed support Marek Vasut
2013-04-18  6:25   ` Vivek Gautam
2013-04-18 12:38     ` Marek Vasut
2013-04-18 13:08       ` Vivek Gautam
2013-04-14 18:13 ` Marek Vasut
2013-04-18  6:24   ` Vivek Gautam
2013-04-18 10:59     ` Vivek Gautam
2013-04-18 11:08       ` Vivek Gautam
2013-04-18 12:43         ` Marek Vasut
2013-04-18 17:11           ` Julius Werner
2013-04-18 19:15             ` Marek Vasut
2013-04-19  4:51               ` Vivek Gautam

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.