[U-Boot] [PATCH 00/14] usb: xhci: Add interrupt transfer support and full speed device support

[U-Boot] [PATCH 00/14] usb: xhci: Add interrupt transfer support and full speed device support

[Bin Meng]

This adds the missing interrupt transfer support to xHCI driver, so
that devices like USB keyboard that uses interrupt transfer when
CONFIG_SYS_USB_EVENT_POLL is defined can work.

This also adds full speed device support. Unlike low/high/super speed
devices, full speed device may report its endpoint 0 max packet size
as 8/16/32/64. xHCI driver guesses 64 for the first attempt to get the
initial 8 bytes device descriptor, and later adjusts the context for
endpoint 0.

This series also made several updates to xHCI driver to conform with
the spec.

This series is available at u-boot-x86/xhci-working for testing.


Bin Meng (14):
  dm: usb: Fix broken usb_stop()
  dm: usb: Remove no longer needed blk_unbind_all()
  usb: xhci: Don't assume LS/FS devices are always behind a HS hub
  usb: Handle audio extension endpoint descriptor in usb_parse_config()
  usb: xhci: Add interrupt transfer support
  usb: Only get 64 bytes device descriptor for full speed devices
  usb: Read device descriptor after device is addressed for xHCI
  usb: xhci: Fix max packet size for full speed device endpoint 0
  usb: hub: Clear port reset before usb_hub_port_connect_change()
  usb: hub: Clear BH reset status change for a 3.0 hub
  usb: xhci: Honor endpoint's interval
  usb: xhci: Program max burst size for endpoint
  usb: xhci: Set 'Error Count' to 0 for isoch endpoints
  usb: xhci: Set 'Average TRB Length' to 8 for control endpoints

 common/usb.c                  |  45 +++++---
 common/usb_hub.c              |  16 ++-
 drivers/usb/host/usb-uclass.c |  19 +++-
 drivers/usb/host/xhci-mem.c   |  24 +++-
 drivers/usb/host/xhci.c       | 249 ++++++++++++++++++++++++++++++++++++++++--
 drivers/usb/host/xhci.h       |   5 +-
 include/linux/usb/ch9.h       |  20 ++++
 7 files changed, 336 insertions(+), 42 deletions(-)

-- 
2.9.2

[U-Boot] [PATCH 01/14] dm: usb: Fix broken usb_stop()

[Bin Meng]

At present we only do device_remove() during usb stop. The DM API
device_remove() only marks the device state as inactivated, but
still keeps its USB topology (eg: parent, children, etc) in the DM
device structure. There is no issue if we only start USB subsystem
once and never stop it. But a big issue occurs when we do 'usb stop'
and 'usb start' multiple times.

Strange things may be observed with current implementation, like:
- the enumeration may report only 1 mass storage device is detected,
  but the total number of USB devices is correct.
- USB keyboard does not work anymore after a bunch of 'usb reset'
  even if 'usb tree' shows it is correctly identified.
- read/write flash drive via 'fatload usb' may complain "Bad device"

In fact, every time when USB host controller starts the enumeration
process, it takes random time for each USB port to show up online,
hence each USB device may appear in a different order from previous
enumeration, and gets assigned to a totally different USB address.
As a result, we end up using a stale USB topology in the DM device
structure which still reflects the previous enumeration result, and
it may create an exact same DM device name like generic_bus_0_dev_7
that is already in the DM device structure. And since the DM device
structure is there, there is no device_bind() call to bind driver to
the device during current enumeration process, eventually creating
an inconsistent software representation of the hardware topology, a
non-working USB subsystem.

The fix is to clear the unused USB topology in the usb_stop(), by
calling device_unbind() on each controller's root hub device, and
the unbinding will unbind all of its children automatically.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 drivers/usb/host/usb-uclass.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index bc44fc3..e90614f 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -164,6 +164,7 @@ int usb_get_max_xfer_size(struct usb_device *udev, size_t *size)
 int usb_stop(void)
 {
 	struct udevice *bus;
+	struct udevice *rh;
 	struct uclass *uc;
 	struct usb_uclass_priv *uc_priv;
 	int err = 0, ret;
@@ -179,6 +180,18 @@ int usb_stop(void)
 		ret = device_remove(bus, DM_REMOVE_NORMAL);
 		if (ret && !err)
 			err = ret;
+
+		/* Locate root hub device */
+		device_find_first_child(bus, &rh);
+		if (rh) {
+			/*
+			 * All USB devices are children of root hub.
+			 * Unbinding root hub will unbind all of its children.
+			 */
+			ret = device_unbind(rh);
+			if (ret && !err)
+				err = ret;
+		}
 	}
 #ifdef CONFIG_BLK
 	ret = blk_unbind_all(IF_TYPE_USB);
-- 
2.9.2

[U-Boot] [PATCH 01/14] dm: usb: Fix broken usb_stop()

[Stefan Roese]

On 18.09.2017 15:40, Bin Meng wrote:
> At present we only do device_remove() during usb stop. The DM API
> device_remove() only marks the device state as inactivated, but
> still keeps its USB topology (eg: parent, children, etc) in the DM
> device structure. There is no issue if we only start USB subsystem
> once and never stop it. But a big issue occurs when we do 'usb stop'
> and 'usb start' multiple times.
> 
> Strange things may be observed with current implementation, like:
> - the enumeration may report only 1 mass storage device is detected,
>    but the total number of USB devices is correct.
> - USB keyboard does not work anymore after a bunch of 'usb reset'
>    even if 'usb tree' shows it is correctly identified.
> - read/write flash drive via 'fatload usb' may complain "Bad device"
> 
> In fact, every time when USB host controller starts the enumeration
> process, it takes random time for each USB port to show up online,
> hence each USB device may appear in a different order from previous
> enumeration, and gets assigned to a totally different USB address.
> As a result, we end up using a stale USB topology in the DM device
> structure which still reflects the previous enumeration result, and
> it may create an exact same DM device name like generic_bus_0_dev_7
> that is already in the DM device structure. And since the DM device
> structure is there, there is no device_bind() call to bind driver to
> the device during current enumeration process, eventually creating
> an inconsistent software representation of the hardware topology, a
> non-working USB subsystem.
> 
> The fix is to clear the unused USB topology in the usb_stop(), by
> calling device_unbind() on each controller's root hub device, and
> the unbinding will unbind all of its children automatically.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

Reviewed-by: Stefan Roese <sr@denx.de>
Tested-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[U-Boot] [PATCH 02/14] dm: usb: Remove no longer needed blk_unbind_all()

[Bin Meng]

With the root hub unbinding in usb_stop(), there is no need to do
a blk uclass specific unbind operation.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 drivers/usb/host/usb-uclass.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index e90614f..32c8657 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -193,11 +193,7 @@ int usb_stop(void)
 				err = ret;
 		}
 	}
-#ifdef CONFIG_BLK
-	ret = blk_unbind_all(IF_TYPE_USB);
-	if (ret && !err)
-		err = ret;
-#endif
+
 #ifdef CONFIG_SANDBOX
 	struct udevice *dev;
 
-- 
2.9.2

[U-Boot] [PATCH 02/14] dm: usb: Remove no longer needed blk_unbind_all()

[Stefan Roese]

On 18.09.2017 15:40, Bin Meng wrote:
> With the root hub unbinding in usb_stop(), there is no need to do
> a blk uclass specific unbind operation.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
> 
>   drivers/usb/host/usb-uclass.c | 6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
> index e90614f..32c8657 100644
> --- a/drivers/usb/host/usb-uclass.c
> +++ b/drivers/usb/host/usb-uclass.c
> @@ -193,11 +193,7 @@ int usb_stop(void)
>   				err = ret;
>   		}
>   	}
> -#ifdef CONFIG_BLK
> -	ret = blk_unbind_all(IF_TYPE_USB);
> -	if (ret && !err)
> -		err = ret;
> -#endif
> +
>   #ifdef CONFIG_SANDBOX
>   	struct udevice *dev;
>   
> 

Reviewed-by: Stefan Roese <sr@denx.de>
Tested-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[U-Boot] [PATCH 03/14] usb: xhci: Don't assume LS/FS devices are always behind a HS hub

[Bin Meng]

At present xHCI driver assumes LS/FS devices are attached directly
to a HS hub. If they are connected to a LS/FS hub, the driver will
fail to perform the USB enumeration process on such devices.

This is fixed by looking from the device itself all the way up to
the HS hub where the TT that serves the device is located.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 drivers/usb/host/xhci-mem.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index d5eab3a..84982a9 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -786,12 +786,22 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl,
 #ifdef CONFIG_DM_USB
 	/* Set up TT fields to support FS/LS devices */
 	if (speed == USB_SPEED_LOW || speed == USB_SPEED_FULL) {
-		dev = dev_get_parent_priv(udev->dev);
-		if (dev->speed == USB_SPEED_HIGH) {
-			hub = dev_get_uclass_priv(udev->dev);
+		struct udevice *parent = udev->dev;
+
+		dev = udev;
+		do {
+			port_num = dev->portnr;
+			dev = dev_get_parent_priv(parent);
+			if (usb_hub_is_root_hub(dev->dev))
+				break;
+			parent = dev->dev->parent;
+		} while (dev->speed != USB_SPEED_HIGH);
+
+		if (!usb_hub_is_root_hub(dev->dev)) {
+			hub = dev_get_uclass_priv(dev->dev);
 			if (hub->tt.multi)
 				slot_ctx->dev_info |= cpu_to_le32(DEV_MTT);
-			slot_ctx->tt_info |= cpu_to_le32(TT_PORT(udev->portnr));
+			slot_ctx->tt_info |= cpu_to_le32(TT_PORT(port_num));
 			slot_ctx->tt_info |= cpu_to_le32(TT_SLOT(dev->slot_id));
 		}
 	}
-- 
2.9.2

[U-Boot] [PATCH 03/14] usb: xhci: Don't assume LS/FS devices are always behind a HS hub

[Stefan Roese]

On 18.09.2017 15:40, Bin Meng wrote:
> At present xHCI driver assumes LS/FS devices are attached directly
> to a HS hub. If they are connected to a LS/FS hub, the driver will
> fail to perform the USB enumeration process on such devices.
> 
> This is fixed by looking from the device itself all the way up to
> the HS hub where the TT that serves the device is located.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
> 
>   drivers/usb/host/xhci-mem.c | 18 ++++++++++++++----
>   1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index d5eab3a..84982a9 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -786,12 +786,22 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl,
>   #ifdef CONFIG_DM_USB
>   	/* Set up TT fields to support FS/LS devices */
>   	if (speed == USB_SPEED_LOW || speed == USB_SPEED_FULL) {
> -		dev = dev_get_parent_priv(udev->dev);
> -		if (dev->speed == USB_SPEED_HIGH) {
> -			hub = dev_get_uclass_priv(udev->dev);
> +		struct udevice *parent = udev->dev;
> +
> +		dev = udev;
> +		do {
> +			port_num = dev->portnr;
> +			dev = dev_get_parent_priv(parent);
> +			if (usb_hub_is_root_hub(dev->dev))
> +				break;
> +			parent = dev->dev->parent;
> +		} while (dev->speed != USB_SPEED_HIGH);
> +
> +		if (!usb_hub_is_root_hub(dev->dev)) {
> +			hub = dev_get_uclass_priv(dev->dev);
>   			if (hub->tt.multi)
>   				slot_ctx->dev_info |= cpu_to_le32(DEV_MTT);
> -			slot_ctx->tt_info |= cpu_to_le32(TT_PORT(udev->portnr));
> +			slot_ctx->tt_info |= cpu_to_le32(TT_PORT(port_num));
>   			slot_ctx->tt_info |= cpu_to_le32(TT_SLOT(dev->slot_id));
>   		}
>   	}
> 

Reviewed-by: Stefan Roese <sr@denx.de>
Tested-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[U-Boot] [PATCH 04/14] usb: Handle audio extension endpoint descriptor in usb_parse_config()

[Bin Meng]

Normal endpoint descriptor size is 7, but for audio extension it is
9. Handle that correctly when parsing endpoint descriptor.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 common/usb.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/common/usb.c b/common/usb.c
index 0904259..6cb92ef 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -437,12 +437,13 @@ static int usb_parse_config(struct usb_device *dev,
 			}
 			break;
 		case USB_DT_ENDPOINT:
-			if (head->bLength != USB_DT_ENDPOINT_SIZE) {
+			if (head->bLength != USB_DT_ENDPOINT_SIZE &&
+			    head->bLength != USB_DT_ENDPOINT_AUDIO_SIZE) {
 				printf("ERROR: Invalid USB EP length (%d)\n",
 					head->bLength);
 				break;
 			}
-			if (index + USB_DT_ENDPOINT_SIZE >
+			if (index + head->bLength >
 			    dev->config.desc.wTotalLength) {
 				puts("USB EP descriptor overflowed buffer!\n");
 				break;
-- 
2.9.2

[U-Boot] [PATCH 04/14] usb: Handle audio extension endpoint descriptor in usb_parse_config()

[Stefan Roese]

On 18.09.2017 15:40, Bin Meng wrote:
> Normal endpoint descriptor size is 7, but for audio extension it is
> 9. Handle that correctly when parsing endpoint descriptor.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
> 
>   common/usb.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/common/usb.c b/common/usb.c
> index 0904259..6cb92ef 100644
> --- a/common/usb.c
> +++ b/common/usb.c
> @@ -437,12 +437,13 @@ static int usb_parse_config(struct usb_device *dev,
>   			}
>   			break;
>   		case USB_DT_ENDPOINT:
> -			if (head->bLength != USB_DT_ENDPOINT_SIZE) {
> +			if (head->bLength != USB_DT_ENDPOINT_SIZE &&
> +			    head->bLength != USB_DT_ENDPOINT_AUDIO_SIZE) {
>   				printf("ERROR: Invalid USB EP length (%d)\n",
>   					head->bLength);
>   				break;
>   			}
> -			if (index + USB_DT_ENDPOINT_SIZE >
> +			if (index + head->bLength >
>   			    dev->config.desc.wTotalLength) {
>   				puts("USB EP descriptor overflowed buffer!\n");
>   				break;
> 

Reviewed-by: Stefan Roese <sr@denx.de>
Tested-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[U-Boot] [PATCH 05/14] usb: xhci: Add interrupt transfer support

[Bin Meng]

xHCI uses normal TRBs for both bulk and interrupt. This adds the
missing interrupt transfer support to xHCI so that devices like
USB keyboard that uses interrupt transfer can work.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 drivers/usb/host/xhci.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 04eb1eb..4b3d58d 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -890,11 +890,18 @@ unknown:
 static int _xhci_submit_int_msg(struct usb_device *udev, unsigned long pipe,
 				void *buffer, int length, int interval)
 {
+	if (usb_pipetype(pipe) != PIPE_INTERRUPT) {
+		printf("non-interrupt pipe (type=%lu)", usb_pipetype(pipe));
+		return -EINVAL;
+	}
+
 	/*
-	 * TODO: Not addressing any interrupt type transfer requests
-	 * Add support for it later.
+	 * xHCI uses normal TRBs for both bulk and interrupt. When the
+	 * interrupt endpoint is to be serviced, the xHC will consume
+	 * (at most) one TD. A TD (comprised of sg list entries) can
+	 * take several service intervals to transmit.
 	 */
-	return -EINVAL;
+	return xhci_bulk_tx(udev, pipe, length, buffer);
 }
 
 /**
-- 
2.9.2

[U-Boot] [PATCH 05/14] usb: xhci: Add interrupt transfer support

[Stefan Roese]

On 18.09.2017 15:40, Bin Meng wrote:
> xHCI uses normal TRBs for both bulk and interrupt. This adds the
> missing interrupt transfer support to xHCI so that devices like
> USB keyboard that uses interrupt transfer can work.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
> 
>   drivers/usb/host/xhci.c | 13 ++++++++++---
>   1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 04eb1eb..4b3d58d 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -890,11 +890,18 @@ unknown:
>   static int _xhci_submit_int_msg(struct usb_device *udev, unsigned long pipe,
>   				void *buffer, int length, int interval)
>   {
> +	if (usb_pipetype(pipe) != PIPE_INTERRUPT) {
> +		printf("non-interrupt pipe (type=%lu)", usb_pipetype(pipe));
> +		return -EINVAL;
> +	}
> +
>   	/*
> -	 * TODO: Not addressing any interrupt type transfer requests
> -	 * Add support for it later.
> +	 * xHCI uses normal TRBs for both bulk and interrupt. When the
> +	 * interrupt endpoint is to be serviced, the xHC will consume
> +	 * (at most) one TD. A TD (comprised of sg list entries) can
> +	 * take several service intervals to transmit.
>   	 */
> -	return -EINVAL;
> +	return xhci_bulk_tx(udev, pipe, length, buffer);
>   }
>   
>   /**
> 

Reviewed-by: Stefan Roese <sr@denx.de>
Tested-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[U-Boot] [PATCH 06/14] usb: Only get 64 bytes device descriptor for full speed devices

[Bin Meng]

Full speed device endpoint 0 can have 8/16/32/64 bMaxPacketSize0.
Other speed devices report fixed value per USB spec. So it only
makes sense if we send a get device descriptor with 64 bytes to
full speed devices.

While we are here, update the comment block to be within 80 cols.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 common/usb.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/common/usb.c b/common/usb.c
index 6cb92ef..88cee81 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -970,23 +970,24 @@ static int usb_setup_descriptor(struct usb_device *dev, bool do_read)
 	dev->epmaxpacketin[0] = dev->descriptor.bMaxPacketSize0;
 	dev->epmaxpacketout[0] = dev->descriptor.bMaxPacketSize0;
 
-	if (do_read) {
+	if (do_read && dev->speed == USB_SPEED_FULL) {
 		int err;
 
 		/*
-		 * Validate we've received only at least 8 bytes, not that we've
-		 * received the entire descriptor. The reasoning is:
-		 * - The code only uses fields in the first 8 bytes, so that's all we
-		 *   need to have fetched at this stage.
-		 * - The smallest maxpacket size is 8 bytes. Before we know the actual
-		 *   maxpacket the device uses, the USB controller may only accept a
-		 *   single packet. Consequently we are only guaranteed to receive 1
-		 *   packet (at least 8 bytes) even in a non-error case.
+		 * Validate we've received only at least 8 bytes, not that
+		 * we've received the entire descriptor. The reasoning is:
+		 * - The code only uses fields in the first 8 bytes, so
+		 *   that's all we need to have fetched at this stage.
+		 * - The smallest maxpacket size is 8 bytes. Before we know
+		 *   the actual maxpacket the device uses, the USB controller
+		 *   may only accept a single packet. Consequently we are only
+		 *   guaranteed to receive 1 packet (at least 8 bytes) even in
+		 *   a non-error case.
 		 *
-		 * At least the DWC2 controller needs to be programmed with the number
-		 * of packets in addition to the number of bytes. A request for 64
-		 * bytes of data with the maxpacket guessed as 64 (above) yields a
-		 * request for 1 packet.
+		 * At least the DWC2 controller needs to be programmed with
+		 * the number of packets in addition to the number of bytes.
+		 * A request for 64 bytes of data with the maxpacket guessed
+		 * as 64 (above) yields a request for 1 packet.
 		 */
 		err = get_descriptor_len(dev, 64, 8);
 		if (err)
@@ -1009,7 +1010,7 @@ static int usb_setup_descriptor(struct usb_device *dev, bool do_read)
 		dev->maxpacketsize = PACKET_SIZE_64;
 		break;
 	default:
-		printf("usb_new_device: invalid max packet size\n");
+		printf("%s: invalid max packet size\n", __func__);
 		return -EIO;
 	}
 
-- 
2.9.2

[U-Boot] [PATCH 06/14] usb: Only get 64 bytes device descriptor for full speed devices

[Stefan Roese]

On 18.09.2017 15:40, Bin Meng wrote:
> Full speed device endpoint 0 can have 8/16/32/64 bMaxPacketSize0.
> Other speed devices report fixed value per USB spec. So it only
> makes sense if we send a get device descriptor with 64 bytes to
> full speed devices.
> 
> While we are here, update the comment block to be within 80 cols.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
> 
>   common/usb.c | 29 +++++++++++++++--------------
>   1 file changed, 15 insertions(+), 14 deletions(-)
> 
> diff --git a/common/usb.c b/common/usb.c
> index 6cb92ef..88cee81 100644
> --- a/common/usb.c
> +++ b/common/usb.c
> @@ -970,23 +970,24 @@ static int usb_setup_descriptor(struct usb_device *dev, bool do_read)
>   	dev->epmaxpacketin[0] = dev->descriptor.bMaxPacketSize0;
>   	dev->epmaxpacketout[0] = dev->descriptor.bMaxPacketSize0;
>   
> -	if (do_read) {
> +	if (do_read && dev->speed == USB_SPEED_FULL) {
>   		int err;
>   
>   		/*
> -		 * Validate we've received only at least 8 bytes, not that we've
> -		 * received the entire descriptor. The reasoning is:
> -		 * - The code only uses fields in the first 8 bytes, so that's all we
> -		 *   need to have fetched at this stage.
> -		 * - The smallest maxpacket size is 8 bytes. Before we know the actual
> -		 *   maxpacket the device uses, the USB controller may only accept a
> -		 *   single packet. Consequently we are only guaranteed to receive 1
> -		 *   packet (at least 8 bytes) even in a non-error case.
> +		 * Validate we've received only at least 8 bytes, not that
> +		 * we've received the entire descriptor. The reasoning is:
> +		 * - The code only uses fields in the first 8 bytes, so
> +		 *   that's all we need to have fetched at this stage.
> +		 * - The smallest maxpacket size is 8 bytes. Before we know
> +		 *   the actual maxpacket the device uses, the USB controller
> +		 *   may only accept a single packet. Consequently we are only
> +		 *   guaranteed to receive 1 packet (at least 8 bytes) even in
> +		 *   a non-error case.
>   		 *
> -		 * At least the DWC2 controller needs to be programmed with the number
> -		 * of packets in addition to the number of bytes. A request for 64
> -		 * bytes of data with the maxpacket guessed as 64 (above) yields a
> -		 * request for 1 packet.
> +		 * At least the DWC2 controller needs to be programmed with
> +		 * the number of packets in addition to the number of bytes.
> +		 * A request for 64 bytes of data with the maxpacket guessed
> +		 * as 64 (above) yields a request for 1 packet.
>   		 */
>   		err = get_descriptor_len(dev, 64, 8);
>   		if (err)
> @@ -1009,7 +1010,7 @@ static int usb_setup_descriptor(struct usb_device *dev, bool do_read)
>   		dev->maxpacketsize = PACKET_SIZE_64;
>   		break;
>   	default:
> -		printf("usb_new_device: invalid max packet size\n");
> +		printf("%s: invalid max packet size\n", __func__);
>   		return -EIO;
>   	}
>   
> 

Reviewed-by: Stefan Roese <sr@denx.de>
Tested-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[U-Boot] [PATCH 07/14] usb: Read device descriptor after device is addressed for xHCI

[Bin Meng]

For xHCI it is not possible to read a device descriptor before it
has been assigned an address. That's why usb_setup_descriptor()
was called with 'do_read' being false. But we really need try to
read the device descriptor before starting any real communication
with the default control endpoint.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 common/usb.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/common/usb.c b/common/usb.c
index 88cee81..8d27bc7 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -1052,6 +1052,17 @@ static int usb_prepare_device(struct usb_device *dev, int addr, bool do_read,
 
 	mdelay(10);	/* Let the SET_ADDRESS settle */
 
+	/*
+	 * If we haven't read device descriptor before, read it here
+	 * after device is assigned an address. This is only applicable
+	 * to xHCI so far.
+	 */
+	if (!do_read) {
+		err = usb_setup_descriptor(dev, true);
+		if (err)
+			return err;
+	}
+
 	return 0;
 }
 
-- 
2.9.2

[U-Boot] [PATCH 07/14] usb: Read device descriptor after device is addressed for xHCI

[Stefan Roese]

On 18.09.2017 15:40, Bin Meng wrote:
> For xHCI it is not possible to read a device descriptor before it
> has been assigned an address. That's why usb_setup_descriptor()
> was called with 'do_read' being false. But we really need try to
> read the device descriptor before starting any real communication
> with the default control endpoint.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
> 
>   common/usb.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/common/usb.c b/common/usb.c
> index 88cee81..8d27bc7 100644
> --- a/common/usb.c
> +++ b/common/usb.c
> @@ -1052,6 +1052,17 @@ static int usb_prepare_device(struct usb_device *dev, int addr, bool do_read,
>   
>   	mdelay(10);	/* Let the SET_ADDRESS settle */
>   
> +	/*
> +	 * If we haven't read device descriptor before, read it here
> +	 * after device is assigned an address. This is only applicable
> +	 * to xHCI so far.
> +	 */
> +	if (!do_read) {
> +		err = usb_setup_descriptor(dev, true);
> +		if (err)
> +			return err;
> +	}
> +
>   	return 0;
>   }
>   
> 

Reviewed-by: Stefan Roese <sr@denx.de>
Tested-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[U-Boot] [PATCH 08/14] usb: xhci: Fix max packet size for full speed device endpoint 0

[Bin Meng]

In xhci_check_maxpacket(), the control endpoint 0 max packet size
is wrongly taken from the interface's endpoint descriptor. However
the default endpoint 0 does not come with an endpoint descriptor
hence is not included in the interface structure. Change to use
epmaxpacketin[0] instead.

The other bug in this routine is that when setting max packet size
to the xHC endpoint 0 context, it does not clear its previous value
at all before programming a new one.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 drivers/usb/host/xhci.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 4b3d58d..ec82fa6 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -546,16 +546,13 @@ int xhci_check_maxpacket(struct usb_device *udev)
 	int max_packet_size;
 	int hw_max_packet_size;
 	int ret = 0;
-	struct usb_interface *ifdesc;
-
-	ifdesc = &udev->config.if_desc[0];
 
 	out_ctx = ctrl->devs[slot_id]->out_ctx;
 	xhci_inval_cache((uintptr_t)out_ctx->bytes, out_ctx->size);
 
 	ep_ctx = xhci_get_ep_ctx(ctrl, out_ctx, ep_index);
 	hw_max_packet_size = MAX_PACKET_DECODED(le32_to_cpu(ep_ctx->ep_info2));
-	max_packet_size = usb_endpoint_maxp(&ifdesc->ep_desc[0]);
+	max_packet_size = udev->epmaxpacketin[0];
 	if (hw_max_packet_size != max_packet_size) {
 		debug("Max Packet Size for ep 0 changed.\n");
 		debug("Max packet size in usb_device = %d\n", max_packet_size);
@@ -567,7 +564,8 @@ int xhci_check_maxpacket(struct usb_device *udev)
 				ctrl->devs[slot_id]->out_ctx, ep_index);
 		in_ctx = ctrl->devs[slot_id]->in_ctx;
 		ep_ctx = xhci_get_ep_ctx(ctrl, in_ctx, ep_index);
-		ep_ctx->ep_info2 &= cpu_to_le32(~MAX_PACKET_MASK);
+		ep_ctx->ep_info2 &= cpu_to_le32(~((0xffff & MAX_PACKET_MASK)
+						<< MAX_PACKET_SHIFT));
 		ep_ctx->ep_info2 |= cpu_to_le32(MAX_PACKET(max_packet_size));
 
 		/*
-- 
2.9.2

[U-Boot] [PATCH 08/14] usb: xhci: Fix max packet size for full speed device endpoint 0

[Stefan Roese]

On 18.09.2017 15:40, Bin Meng wrote:
> In xhci_check_maxpacket(), the control endpoint 0 max packet size
> is wrongly taken from the interface's endpoint descriptor. However
> the default endpoint 0 does not come with an endpoint descriptor
> hence is not included in the interface structure. Change to use
> epmaxpacketin[0] instead.
> 
> The other bug in this routine is that when setting max packet size
> to the xHC endpoint 0 context, it does not clear its previous value
> at all before programming a new one.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
> 
>   drivers/usb/host/xhci.c | 8 +++-----
>   1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 4b3d58d..ec82fa6 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -546,16 +546,13 @@ int xhci_check_maxpacket(struct usb_device *udev)
>   	int max_packet_size;
>   	int hw_max_packet_size;
>   	int ret = 0;
> -	struct usb_interface *ifdesc;
> -
> -	ifdesc = &udev->config.if_desc[0];
>   
>   	out_ctx = ctrl->devs[slot_id]->out_ctx;
>   	xhci_inval_cache((uintptr_t)out_ctx->bytes, out_ctx->size);
>   
>   	ep_ctx = xhci_get_ep_ctx(ctrl, out_ctx, ep_index);
>   	hw_max_packet_size = MAX_PACKET_DECODED(le32_to_cpu(ep_ctx->ep_info2));
> -	max_packet_size = usb_endpoint_maxp(&ifdesc->ep_desc[0]);
> +	max_packet_size = udev->epmaxpacketin[0];
>   	if (hw_max_packet_size != max_packet_size) {
>   		debug("Max Packet Size for ep 0 changed.\n");
>   		debug("Max packet size in usb_device = %d\n", max_packet_size);
> @@ -567,7 +564,8 @@ int xhci_check_maxpacket(struct usb_device *udev)
>   				ctrl->devs[slot_id]->out_ctx, ep_index);
>   		in_ctx = ctrl->devs[slot_id]->in_ctx;
>   		ep_ctx = xhci_get_ep_ctx(ctrl, in_ctx, ep_index);
> -		ep_ctx->ep_info2 &= cpu_to_le32(~MAX_PACKET_MASK);
> +		ep_ctx->ep_info2 &= cpu_to_le32(~((0xffff & MAX_PACKET_MASK)
> +						<< MAX_PACKET_SHIFT));
>   		ep_ctx->ep_info2 |= cpu_to_le32(MAX_PACKET(max_packet_size));
>   
>   		/*
> 

Reviewed-by: Stefan Roese <sr@denx.de>
Tested-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[U-Boot] [PATCH 09/14] usb: hub: Clear port reset before usb_hub_port_connect_change()

[Bin Meng]

During usb_hub_port_connect_change(), a port reset set feature
request is issued to the port, and later a port reset clear feature
is done to the same port before the function returns. However at
the end of usb_scan_port(), we attempt to clear port reset again
on a cached port status change variable, which should not be done.

Adjust the call to clear port reset to right before the call to
usb_hub_port_connect_change().

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 common/usb_hub.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/common/usb_hub.c b/common/usb_hub.c
index 86a3477..a9d21bc 100644
--- a/common/usb_hub.c
+++ b/common/usb_hub.c
@@ -489,6 +489,11 @@ static int usb_scan_port(struct usb_device_scan *usb_scan)
 		return 0;
 	}
 
+	if (portchange & USB_PORT_STAT_C_RESET) {
+		debug("port %d reset change\n", i + 1);
+		usb_clear_port_feature(dev, i + 1, USB_PORT_FEAT_C_RESET);
+	}
+
 	/* A new USB device is ready at this point */
 	debug("devnum=%d port=%d: USB dev found\n", dev->devnum, i + 1);
 
@@ -543,11 +548,6 @@ static int usb_scan_port(struct usb_device_scan *usb_scan)
 		       hub->overcurrent_count[i]);
 	}
 
-	if (portchange & USB_PORT_STAT_C_RESET) {
-		debug("port %d reset change\n", i + 1);
-		usb_clear_port_feature(dev, i + 1, USB_PORT_FEAT_C_RESET);
-	}
-
 	/*
 	 * We're done with this device, so let's remove this device from
 	 * scanning list
-- 
2.9.2

[U-Boot] [PATCH 09/14] usb: hub: Clear port reset before usb_hub_port_connect_change()

[Stefan Roese]

On 18.09.2017 15:40, Bin Meng wrote:
> During usb_hub_port_connect_change(), a port reset set feature
> request is issued to the port, and later a port reset clear feature
> is done to the same port before the function returns. However at
> the end of usb_scan_port(), we attempt to clear port reset again
> on a cached port status change variable, which should not be done.
> 
> Adjust the call to clear port reset to right before the call to
> usb_hub_port_connect_change().
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
> 
>   common/usb_hub.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/common/usb_hub.c b/common/usb_hub.c
> index 86a3477..a9d21bc 100644
> --- a/common/usb_hub.c
> +++ b/common/usb_hub.c
> @@ -489,6 +489,11 @@ static int usb_scan_port(struct usb_device_scan *usb_scan)
>   		return 0;
>   	}
>   
> +	if (portchange & USB_PORT_STAT_C_RESET) {
> +		debug("port %d reset change\n", i + 1);
> +		usb_clear_port_feature(dev, i + 1, USB_PORT_FEAT_C_RESET);
> +	}
> +
>   	/* A new USB device is ready at this point */
>   	debug("devnum=%d port=%d: USB dev found\n", dev->devnum, i + 1);
>   
> @@ -543,11 +548,6 @@ static int usb_scan_port(struct usb_device_scan *usb_scan)
>   		       hub->overcurrent_count[i]);
>   	}
>   
> -	if (portchange & USB_PORT_STAT_C_RESET) {
> -		debug("port %d reset change\n", i + 1);
> -		usb_clear_port_feature(dev, i + 1, USB_PORT_FEAT_C_RESET);
> -	}
> -
>   	/*
>   	 * We're done with this device, so let's remove this device from
>   	 * scanning list
> 

Reviewed-by: Stefan Roese <sr@denx.de>
Tested-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[U-Boot] [PATCH 10/14] usb: hub: Clear BH reset status change for a 3.0 hub

[Bin Meng]

USB 3.0 hubs report bit[5] in the port status change response as BH
reset. The hub shall set the C_BH_PORT_RESET field for this port.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 common/usb_hub.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/common/usb_hub.c b/common/usb_hub.c
index a9d21bc..325d16d 100644
--- a/common/usb_hub.c
+++ b/common/usb_hub.c
@@ -494,6 +494,12 @@ static int usb_scan_port(struct usb_device_scan *usb_scan)
 		usb_clear_port_feature(dev, i + 1, USB_PORT_FEAT_C_RESET);
 	}
 
+	if ((portchange & USB_SS_PORT_STAT_C_BH_RESET) &&
+	    usb_hub_is_superspeed(dev)) {
+		debug("port %d BH reset change\n", i + 1);
+		usb_clear_port_feature(dev, i + 1, USB_SS_PORT_FEAT_C_BH_RESET);
+	}
+
 	/* A new USB device is ready at this point */
 	debug("devnum=%d port=%d: USB dev found\n", dev->devnum, i + 1);
 
-- 
2.9.2

[U-Boot] [PATCH 10/14] usb: hub: Clear BH reset status change for a 3.0 hub

[Stefan Roese]

On 18.09.2017 15:40, Bin Meng wrote:
> USB 3.0 hubs report bit[5] in the port status change response as BH
> reset. The hub shall set the C_BH_PORT_RESET field for this port.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
> 
>   common/usb_hub.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/common/usb_hub.c b/common/usb_hub.c
> index a9d21bc..325d16d 100644
> --- a/common/usb_hub.c
> +++ b/common/usb_hub.c
> @@ -494,6 +494,12 @@ static int usb_scan_port(struct usb_device_scan *usb_scan)
>   		usb_clear_port_feature(dev, i + 1, USB_PORT_FEAT_C_RESET);
>   	}
>   
> +	if ((portchange & USB_SS_PORT_STAT_C_BH_RESET) &&
> +	    usb_hub_is_superspeed(dev)) {
> +		debug("port %d BH reset change\n", i + 1);
> +		usb_clear_port_feature(dev, i + 1, USB_SS_PORT_FEAT_C_BH_RESET);
> +	}
> +
>   	/* A new USB device is ready at this point */
>   	debug("devnum=%d port=%d: USB dev found\n", dev->devnum, i + 1);
>   
> 

Reviewed-by: Stefan Roese <sr@denx.de>
Tested-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[U-Boot] [PATCH 11/14] usb: xhci: Honor endpoint's interval

[Bin Meng]

USB endpoint reports the period between consecutive requests to send
or receive data as bInverval in its endpoint descriptor. So far this
is ignored by xHCI driver and the 'Interval' field in xHC's endpoint
context is always programmed to zero which means 1ms for low speed
or full speed , or 125us for high speed or super speed. We should
honor the interval by getting it from endpoint descriptor.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 drivers/usb/host/xhci.c | 195 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/host/xhci.h |   5 +-
 include/linux/usb/ch9.h |  20 +++++
 3 files changed, 218 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index ec82fa6..8aed428 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -257,6 +257,172 @@ static unsigned int xhci_get_ep_index(struct usb_endpoint_descriptor *desc)
 	return index;
 }
 
+/*
+ * Convert bInterval expressed in microframes (in 1-255 range) to exponent of
+ * microframes, rounded down to nearest power of 2.
+ */
+static unsigned int xhci_microframes_to_exponent(unsigned int desc_interval,
+						 unsigned int min_exponent,
+						 unsigned int max_exponent)
+{
+	unsigned int interval;
+
+	interval = fls(desc_interval) - 1;
+	interval = clamp_val(interval, min_exponent, max_exponent);
+	if ((1 << interval) != desc_interval)
+		debug("rounding interval to %d microframes, "\
+		      "ep desc says %d microframes\n",
+		      1 << interval, desc_interval);
+
+	return interval;
+}
+
+static unsigned int xhci_parse_microframe_interval(struct usb_device *udev,
+	struct usb_endpoint_descriptor *endpt_desc)
+{
+	if (endpt_desc->bInterval == 0)
+		return 0;
+
+	return xhci_microframes_to_exponent(endpt_desc->bInterval, 0, 15);
+}
+
+static unsigned int xhci_parse_frame_interval(struct usb_device *udev,
+	struct usb_endpoint_descriptor *endpt_desc)
+{
+	return xhci_microframes_to_exponent(endpt_desc->bInterval * 8, 3, 10);
+}
+
+/*
+ * Convert interval expressed as 2^(bInterval - 1) == interval into
+ * straight exponent value 2^n == interval.
+ */
+static unsigned int xhci_parse_exponent_interval(struct usb_device *udev,
+	struct usb_endpoint_descriptor *endpt_desc)
+{
+	unsigned int interval;
+
+	interval = clamp_val(endpt_desc->bInterval, 1, 16) - 1;
+	if (interval != endpt_desc->bInterval - 1)
+		debug("ep %#x - rounding interval to %d %sframes\n",
+		      endpt_desc->bEndpointAddress, 1 << interval,
+		      udev->speed == USB_SPEED_FULL ? "" : "micro");
+
+	if (udev->speed == USB_SPEED_FULL) {
+		/*
+		 * Full speed isoc endpoints specify interval in frames,
+		 * not microframes. We are using microframes everywhere,
+		 * so adjust accordingly.
+		 */
+		interval += 3;	/* 1 frame = 2^3 uframes */
+	}
+
+	return interval;
+}
+
+/*
+ * Return the polling or NAK interval.
+ *
+ * The polling interval is expressed in "microframes". If xHCI's Interval field
+ * is set to N, it will service the endpoint every 2^(Interval)*125us.
+ *
+ * The NAK interval is one NAK per 1 to 255 microframes, or no NAKs if interval
+ * is set to 0.
+ */
+static unsigned int xhci_get_endpoint_interval(struct usb_device *udev,
+	struct usb_endpoint_descriptor *endpt_desc)
+{
+	unsigned int interval = 0;
+
+	switch (udev->speed) {
+	case USB_SPEED_HIGH:
+		/* Max NAK rate */
+		if (usb_endpoint_xfer_control(endpt_desc) ||
+		    usb_endpoint_xfer_bulk(endpt_desc)) {
+			interval = xhci_parse_microframe_interval(udev,
+								  endpt_desc);
+			break;
+		}
+		/* Fall through - SS and HS isoc/int have same decoding */
+
+	case USB_SPEED_SUPER:
+		if (usb_endpoint_xfer_int(endpt_desc) ||
+		    usb_endpoint_xfer_isoc(endpt_desc)) {
+			interval = xhci_parse_exponent_interval(udev,
+								endpt_desc);
+		}
+		break;
+
+	case USB_SPEED_FULL:
+		if (usb_endpoint_xfer_isoc(endpt_desc)) {
+			interval = xhci_parse_exponent_interval(udev,
+								endpt_desc);
+			break;
+		}
+		/*
+		 * Fall through for interrupt endpoint interval decoding
+		 * since it uses the same rules as low speed interrupt
+		 * endpoints.
+		 */
+
+	case USB_SPEED_LOW:
+		if (usb_endpoint_xfer_int(endpt_desc) ||
+		    usb_endpoint_xfer_isoc(endpt_desc)) {
+			interval = xhci_parse_frame_interval(udev, endpt_desc);
+		}
+		break;
+
+	default:
+		BUG();
+	}
+
+	return interval;
+}
+
+/*
+ * The "Mult" field in the endpoint context is only set for SuperSpeed isoc eps.
+ * High speed endpoint descriptors can define "the number of additional
+ * transaction opportunities per microframe", but that goes in the Max Burst
+ * endpoint context field.
+ */
+static u32 xhci_get_endpoint_mult(struct usb_device *udev,
+	struct usb_endpoint_descriptor *endpt_desc,
+	struct usb_ss_ep_comp_descriptor *ss_ep_comp_desc)
+{
+	if (udev->speed < USB_SPEED_SUPER ||
+	    !usb_endpoint_xfer_isoc(endpt_desc))
+		return 0;
+
+	return ss_ep_comp_desc->bmAttributes;
+}
+
+/*
+ * Return the maximum endpoint service interval time (ESIT) payload.
+ * Basically, this is the maxpacket size, multiplied by the burst size
+ * and mult size.
+ */
+static u32 xhci_get_max_esit_payload(struct usb_device *udev,
+	struct usb_endpoint_descriptor *endpt_desc,
+	struct usb_ss_ep_comp_descriptor *ss_ep_comp_desc)
+{
+	int max_burst;
+	int max_packet;
+
+	/* Only applies for interrupt or isochronous endpoints */
+	if (usb_endpoint_xfer_control(endpt_desc) ||
+	    usb_endpoint_xfer_bulk(endpt_desc))
+		return 0;
+
+	/* SuperSpeed Isoc ep with less than 48k per esit */
+	if (udev->speed >= USB_SPEED_SUPER)
+		return le16_to_cpu(ss_ep_comp_desc->wBytesPerInterval);
+
+	max_packet = usb_endpoint_maxp(endpt_desc);
+	max_burst = usb_endpoint_maxp_mult(endpt_desc);
+
+	/* A 0 in max burst means 1 transfer per ESIT */
+	return max_packet * max_burst;
+}
+
 /**
  * Issue a configure endpoint command or evaluate context command
  * and wait for it to finish.
@@ -324,6 +490,10 @@ static int xhci_set_configuration(struct usb_device *udev)
 	int slot_id = udev->slot_id;
 	struct xhci_virt_device *virt_dev = ctrl->devs[slot_id];
 	struct usb_interface *ifdesc;
+	u32 max_esit_payload;
+	unsigned int interval;
+	unsigned int mult;
+	unsigned int avg_trb_len;
 
 	out_ctx = virt_dev->out_ctx;
 	in_ctx = virt_dev->in_ctx;
@@ -357,10 +527,26 @@ static int xhci_set_configuration(struct usb_device *udev)
 	/* filling up ep contexts */
 	for (cur_ep = 0; cur_ep < num_of_ep; cur_ep++) {
 		struct usb_endpoint_descriptor *endpt_desc = NULL;
+		struct usb_ss_ep_comp_descriptor *ss_ep_comp_desc = NULL;
 
 		endpt_desc = &ifdesc->ep_desc[cur_ep];
+		ss_ep_comp_desc = &ifdesc->ss_ep_comp_desc[cur_ep];
 		trb_64 = 0;
 
+		/*
+		 * Get values to fill the endpoint context, mostly from ep
+		 * descriptor. The average TRB buffer lengt for bulk endpoints
+		 * is unclear as we have no clue on scatter gather list entry
+		 * size. For Isoc and Int, set it to max available.
+		 * See xHCI 1.1 spec 4.14.1.1 for details.
+		 */
+		max_esit_payload = xhci_get_max_esit_payload(udev, endpt_desc,
+							     ss_ep_comp_desc);
+		interval = xhci_get_endpoint_interval(udev, endpt_desc);
+		mult = xhci_get_endpoint_mult(udev, endpt_desc,
+					      ss_ep_comp_desc);
+		avg_trb_len = max_esit_payload;
+
 		ep_index = xhci_get_ep_index(endpt_desc);
 		ep_ctx[ep_index] = xhci_get_ep_ctx(ctrl, in_ctx, ep_index);
 
@@ -372,6 +558,11 @@ static int xhci_set_configuration(struct usb_device *udev)
 		/*NOTE: ep_desc[0] actually represents EP1 and so on */
 		dir = (((endpt_desc->bEndpointAddress) & (0x80)) >> 7);
 		ep_type = (((endpt_desc->bmAttributes) & (0x3)) | (dir << 2));
+
+		ep_ctx[ep_index]->ep_info =
+			cpu_to_le32(EP_MAX_ESIT_PAYLOAD_HI(max_esit_payload) |
+			EP_INTERVAL(interval) | EP_MULT(mult));
+
 		ep_ctx[ep_index]->ep_info2 =
 			cpu_to_le32(ep_type << EP_TYPE_SHIFT);
 		ep_ctx[ep_index]->ep_info2 |=
@@ -386,6 +577,10 @@ static int xhci_set_configuration(struct usb_device *udev)
 				virt_dev->eps[ep_index].ring->enqueue;
 		ep_ctx[ep_index]->deq = cpu_to_le64(trb_64 |
 				virt_dev->eps[ep_index].ring->cycle_state);
+
+		ep_ctx[ep_index]->tx_info =
+			cpu_to_le32(EP_MAX_ESIT_PAYLOAD_LO(max_esit_payload) |
+			EP_AVG_TRB_LENGTH(avg_trb_len));
 	}
 
 	return xhci_configure_endpoints(udev, false);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index a497d9d..6deefbf 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -663,8 +663,9 @@ struct xhci_ep_ctx {
 #define GET_MAX_PACKET(p)	((p) & 0x7ff)
 
 /* tx_info bitmasks */
-#define AVG_TRB_LENGTH_FOR_EP(p)	((p) & 0xffff)
-#define MAX_ESIT_PAYLOAD_FOR_EP(p)	(((p) & 0xffff) << 16)
+#define EP_AVG_TRB_LENGTH(p)		((p) & 0xffff)
+#define EP_MAX_ESIT_PAYLOAD_LO(p)	(((p) & 0xffff) << 16)
+#define EP_MAX_ESIT_PAYLOAD_HI(p)	((((p) >> 16) & 0xff) << 24)
 #define CTX_TO_MAX_ESIT_PAYLOAD(p)	(((p) >> 16) & 0xffff)
 
 /* deq bitmasks */
diff --git a/include/linux/usb/ch9.h b/include/linux/usb/ch9.h
index 0ad4782..264c971 100644
--- a/include/linux/usb/ch9.h
+++ b/include/linux/usb/ch9.h
@@ -418,6 +418,12 @@ struct __packed usb_class_report_descriptor {
 #define USB_ENDPOINT_XFER_INT		3
 #define USB_ENDPOINT_MAX_ADJUSTABLE	0x80
 
+#define USB_ENDPOINT_MAXP_MASK		0x07ff
+#define USB_EP_MAXP_MULT_SHIFT		11
+#define USB_EP_MAXP_MULT_MASK		(3 << USB_EP_MAXP_MULT_SHIFT)
+#define USB_EP_MAXP_MULT(m)		\
+	(((m) & USB_EP_MAXP_MULT_MASK) >> USB_EP_MAXP_MULT_SHIFT)
+
 /* The USB 3.0 spec redefines bits 5:4 of bmAttributes as interrupt ep type. */
 #define USB_ENDPOINT_INTRTYPE		0x30
 #define USB_ENDPOINT_INTR_PERIODIC	(0 << 4)
@@ -625,6 +631,20 @@ static inline int usb_endpoint_maxp(const struct usb_endpoint_descriptor *epd)
 	return __le16_to_cpu(get_unaligned(&epd->wMaxPacketSize));
 }
 
+/**
+ * usb_endpoint_maxp_mult - get endpoint's transactional opportunities
+ * @epd: endpoint to be checked
+ *
+ * Return @epd's wMaxPacketSize[12:11] + 1
+ */
+static inline int
+usb_endpoint_maxp_mult(const struct usb_endpoint_descriptor *epd)
+{
+	int maxp = __le16_to_cpu(epd->wMaxPacketSize);
+
+	return USB_EP_MAXP_MULT(maxp) + 1;
+}
+
 static inline int usb_endpoint_interrupt_type(
 		const struct usb_endpoint_descriptor *epd)
 {
-- 
2.9.2

[U-Boot] [PATCH 11/14] usb: xhci: Honor endpoint's interval

[Stefan Roese]

On 18.09.2017 15:40, Bin Meng wrote:
> USB endpoint reports the period between consecutive requests to send
> or receive data as bInverval in its endpoint descriptor. So far this
> is ignored by xHCI driver and the 'Interval' field in xHC's endpoint
> context is always programmed to zero which means 1ms for low speed
> or full speed , or 125us for high speed or super speed. We should
> honor the interval by getting it from endpoint descriptor.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
> 
>   drivers/usb/host/xhci.c | 195 ++++++++++++++++++++++++++++++++++++++++++++++++
>   drivers/usb/host/xhci.h |   5 +-
>   include/linux/usb/ch9.h |  20 +++++
>   3 files changed, 218 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index ec82fa6..8aed428 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -257,6 +257,172 @@ static unsigned int xhci_get_ep_index(struct usb_endpoint_descriptor *desc)
>   	return index;
>   }
>   
> +/*
> + * Convert bInterval expressed in microframes (in 1-255 range) to exponent of
> + * microframes, rounded down to nearest power of 2.
> + */
> +static unsigned int xhci_microframes_to_exponent(unsigned int desc_interval,
> +						 unsigned int min_exponent,
> +						 unsigned int max_exponent)
> +{
> +	unsigned int interval;
> +
> +	interval = fls(desc_interval) - 1;
> +	interval = clamp_val(interval, min_exponent, max_exponent);
> +	if ((1 << interval) != desc_interval)
> +		debug("rounding interval to %d microframes, "\
> +		      "ep desc says %d microframes\n",
> +		      1 << interval, desc_interval);
> +
> +	return interval;
> +}
> +
> +static unsigned int xhci_parse_microframe_interval(struct usb_device *udev,
> +	struct usb_endpoint_descriptor *endpt_desc)
> +{
> +	if (endpt_desc->bInterval == 0)
> +		return 0;
> +
> +	return xhci_microframes_to_exponent(endpt_desc->bInterval, 0, 15);
> +}
> +
> +static unsigned int xhci_parse_frame_interval(struct usb_device *udev,
> +	struct usb_endpoint_descriptor *endpt_desc)
> +{
> +	return xhci_microframes_to_exponent(endpt_desc->bInterval * 8, 3, 10);
> +}
> +
> +/*
> + * Convert interval expressed as 2^(bInterval - 1) == interval into
> + * straight exponent value 2^n == interval.
> + */
> +static unsigned int xhci_parse_exponent_interval(struct usb_device *udev,
> +	struct usb_endpoint_descriptor *endpt_desc)
> +{
> +	unsigned int interval;
> +
> +	interval = clamp_val(endpt_desc->bInterval, 1, 16) - 1;
> +	if (interval != endpt_desc->bInterval - 1)
> +		debug("ep %#x - rounding interval to %d %sframes\n",
> +		      endpt_desc->bEndpointAddress, 1 << interval,
> +		      udev->speed == USB_SPEED_FULL ? "" : "micro");
> +
> +	if (udev->speed == USB_SPEED_FULL) {
> +		/*
> +		 * Full speed isoc endpoints specify interval in frames,
> +		 * not microframes. We are using microframes everywhere,
> +		 * so adjust accordingly.
> +		 */
> +		interval += 3;	/* 1 frame = 2^3 uframes */
> +	}
> +
> +	return interval;
> +}
> +
> +/*
> + * Return the polling or NAK interval.
> + *
> + * The polling interval is expressed in "microframes". If xHCI's Interval field
> + * is set to N, it will service the endpoint every 2^(Interval)*125us.
> + *
> + * The NAK interval is one NAK per 1 to 255 microframes, or no NAKs if interval
> + * is set to 0.
> + */
> +static unsigned int xhci_get_endpoint_interval(struct usb_device *udev,
> +	struct usb_endpoint_descriptor *endpt_desc)
> +{
> +	unsigned int interval = 0;
> +
> +	switch (udev->speed) {
> +	case USB_SPEED_HIGH:
> +		/* Max NAK rate */
> +		if (usb_endpoint_xfer_control(endpt_desc) ||
> +		    usb_endpoint_xfer_bulk(endpt_desc)) {
> +			interval = xhci_parse_microframe_interval(udev,
> +								  endpt_desc);
> +			break;
> +		}
> +		/* Fall through - SS and HS isoc/int have same decoding */
> +
> +	case USB_SPEED_SUPER:
> +		if (usb_endpoint_xfer_int(endpt_desc) ||
> +		    usb_endpoint_xfer_isoc(endpt_desc)) {
> +			interval = xhci_parse_exponent_interval(udev,
> +								endpt_desc);
> +		}
> +		break;
> +
> +	case USB_SPEED_FULL:
> +		if (usb_endpoint_xfer_isoc(endpt_desc)) {
> +			interval = xhci_parse_exponent_interval(udev,
> +								endpt_desc);
> +			break;
> +		}
> +		/*
> +		 * Fall through for interrupt endpoint interval decoding
> +		 * since it uses the same rules as low speed interrupt
> +		 * endpoints.
> +		 */
> +
> +	case USB_SPEED_LOW:
> +		if (usb_endpoint_xfer_int(endpt_desc) ||
> +		    usb_endpoint_xfer_isoc(endpt_desc)) {
> +			interval = xhci_parse_frame_interval(udev, endpt_desc);
> +		}
> +		break;
> +
> +	default:
> +		BUG();
> +	}
> +
> +	return interval;
> +}
> +
> +/*
> + * The "Mult" field in the endpoint context is only set for SuperSpeed isoc eps.
> + * High speed endpoint descriptors can define "the number of additional
> + * transaction opportunities per microframe", but that goes in the Max Burst
> + * endpoint context field.
> + */
> +static u32 xhci_get_endpoint_mult(struct usb_device *udev,
> +	struct usb_endpoint_descriptor *endpt_desc,
> +	struct usb_ss_ep_comp_descriptor *ss_ep_comp_desc)
> +{
> +	if (udev->speed < USB_SPEED_SUPER ||
> +	    !usb_endpoint_xfer_isoc(endpt_desc))
> +		return 0;
> +
> +	return ss_ep_comp_desc->bmAttributes;
> +}
> +
> +/*
> + * Return the maximum endpoint service interval time (ESIT) payload.
> + * Basically, this is the maxpacket size, multiplied by the burst size
> + * and mult size.
> + */
> +static u32 xhci_get_max_esit_payload(struct usb_device *udev,
> +	struct usb_endpoint_descriptor *endpt_desc,
> +	struct usb_ss_ep_comp_descriptor *ss_ep_comp_desc)
> +{
> +	int max_burst;
> +	int max_packet;
> +
> +	/* Only applies for interrupt or isochronous endpoints */
> +	if (usb_endpoint_xfer_control(endpt_desc) ||
> +	    usb_endpoint_xfer_bulk(endpt_desc))
> +		return 0;
> +
> +	/* SuperSpeed Isoc ep with less than 48k per esit */
> +	if (udev->speed >= USB_SPEED_SUPER)
> +		return le16_to_cpu(ss_ep_comp_desc->wBytesPerInterval);
> +
> +	max_packet = usb_endpoint_maxp(endpt_desc);
> +	max_burst = usb_endpoint_maxp_mult(endpt_desc);
> +
> +	/* A 0 in max burst means 1 transfer per ESIT */
> +	return max_packet * max_burst;
> +}
> +
>   /**
>    * Issue a configure endpoint command or evaluate context command
>    * and wait for it to finish.
> @@ -324,6 +490,10 @@ static int xhci_set_configuration(struct usb_device *udev)
>   	int slot_id = udev->slot_id;
>   	struct xhci_virt_device *virt_dev = ctrl->devs[slot_id];
>   	struct usb_interface *ifdesc;
> +	u32 max_esit_payload;
> +	unsigned int interval;
> +	unsigned int mult;
> +	unsigned int avg_trb_len;
>   
>   	out_ctx = virt_dev->out_ctx;
>   	in_ctx = virt_dev->in_ctx;
> @@ -357,10 +527,26 @@ static int xhci_set_configuration(struct usb_device *udev)
>   	/* filling up ep contexts */
>   	for (cur_ep = 0; cur_ep < num_of_ep; cur_ep++) {
>   		struct usb_endpoint_descriptor *endpt_desc = NULL;
> +		struct usb_ss_ep_comp_descriptor *ss_ep_comp_desc = NULL;
>   
>   		endpt_desc = &ifdesc->ep_desc[cur_ep];
> +		ss_ep_comp_desc = &ifdesc->ss_ep_comp_desc[cur_ep];
>   		trb_64 = 0;
>   
> +		/*
> +		 * Get values to fill the endpoint context, mostly from ep
> +		 * descriptor. The average TRB buffer lengt for bulk endpoints
> +		 * is unclear as we have no clue on scatter gather list entry
> +		 * size. For Isoc and Int, set it to max available.
> +		 * See xHCI 1.1 spec 4.14.1.1 for details.
> +		 */
> +		max_esit_payload = xhci_get_max_esit_payload(udev, endpt_desc,
> +							     ss_ep_comp_desc);
> +		interval = xhci_get_endpoint_interval(udev, endpt_desc);
> +		mult = xhci_get_endpoint_mult(udev, endpt_desc,
> +					      ss_ep_comp_desc);
> +		avg_trb_len = max_esit_payload;
> +
>   		ep_index = xhci_get_ep_index(endpt_desc);
>   		ep_ctx[ep_index] = xhci_get_ep_ctx(ctrl, in_ctx, ep_index);
>   
> @@ -372,6 +558,11 @@ static int xhci_set_configuration(struct usb_device *udev)
>   		/*NOTE: ep_desc[0] actually represents EP1 and so on */
>   		dir = (((endpt_desc->bEndpointAddress) & (0x80)) >> 7);
>   		ep_type = (((endpt_desc->bmAttributes) & (0x3)) | (dir << 2));
> +
> +		ep_ctx[ep_index]->ep_info =
> +			cpu_to_le32(EP_MAX_ESIT_PAYLOAD_HI(max_esit_payload) |
> +			EP_INTERVAL(interval) | EP_MULT(mult));
> +
>   		ep_ctx[ep_index]->ep_info2 =
>   			cpu_to_le32(ep_type << EP_TYPE_SHIFT);
>   		ep_ctx[ep_index]->ep_info2 |=
> @@ -386,6 +577,10 @@ static int xhci_set_configuration(struct usb_device *udev)
>   				virt_dev->eps[ep_index].ring->enqueue;
>   		ep_ctx[ep_index]->deq = cpu_to_le64(trb_64 |
>   				virt_dev->eps[ep_index].ring->cycle_state);
> +
> +		ep_ctx[ep_index]->tx_info =
> +			cpu_to_le32(EP_MAX_ESIT_PAYLOAD_LO(max_esit_payload) |
> +			EP_AVG_TRB_LENGTH(avg_trb_len));
>   	}
>   
>   	return xhci_configure_endpoints(udev, false);
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index a497d9d..6deefbf 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -663,8 +663,9 @@ struct xhci_ep_ctx {
>   #define GET_MAX_PACKET(p)	((p) & 0x7ff)
>   
>   /* tx_info bitmasks */
> -#define AVG_TRB_LENGTH_FOR_EP(p)	((p) & 0xffff)
> -#define MAX_ESIT_PAYLOAD_FOR_EP(p)	(((p) & 0xffff) << 16)
> +#define EP_AVG_TRB_LENGTH(p)		((p) & 0xffff)
> +#define EP_MAX_ESIT_PAYLOAD_LO(p)	(((p) & 0xffff) << 16)
> +#define EP_MAX_ESIT_PAYLOAD_HI(p)	((((p) >> 16) & 0xff) << 24)
>   #define CTX_TO_MAX_ESIT_PAYLOAD(p)	(((p) >> 16) & 0xffff)
>   
>   /* deq bitmasks */
> diff --git a/include/linux/usb/ch9.h b/include/linux/usb/ch9.h
> index 0ad4782..264c971 100644
> --- a/include/linux/usb/ch9.h
> +++ b/include/linux/usb/ch9.h
> @@ -418,6 +418,12 @@ struct __packed usb_class_report_descriptor {
>   #define USB_ENDPOINT_XFER_INT		3
>   #define USB_ENDPOINT_MAX_ADJUSTABLE	0x80
>   
> +#define USB_ENDPOINT_MAXP_MASK		0x07ff
> +#define USB_EP_MAXP_MULT_SHIFT		11
> +#define USB_EP_MAXP_MULT_MASK		(3 << USB_EP_MAXP_MULT_SHIFT)
> +#define USB_EP_MAXP_MULT(m)		\
> +	(((m) & USB_EP_MAXP_MULT_MASK) >> USB_EP_MAXP_MULT_SHIFT)
> +
>   /* The USB 3.0 spec redefines bits 5:4 of bmAttributes as interrupt ep type. */
>   #define USB_ENDPOINT_INTRTYPE		0x30
>   #define USB_ENDPOINT_INTR_PERIODIC	(0 << 4)
> @@ -625,6 +631,20 @@ static inline int usb_endpoint_maxp(const struct usb_endpoint_descriptor *epd)
>   	return __le16_to_cpu(get_unaligned(&epd->wMaxPacketSize));
>   }
>   
> +/**
> + * usb_endpoint_maxp_mult - get endpoint's transactional opportunities
> + * @epd: endpoint to be checked
> + *
> + * Return @epd's wMaxPacketSize[12:11] + 1
> + */
> +static inline int
> +usb_endpoint_maxp_mult(const struct usb_endpoint_descriptor *epd)
> +{
> +	int maxp = __le16_to_cpu(epd->wMaxPacketSize);
> +
> +	return USB_EP_MAXP_MULT(maxp) + 1;
> +}
> +
>   static inline int usb_endpoint_interrupt_type(
>   		const struct usb_endpoint_descriptor *epd)
>   {
> 

Reviewed-by: Stefan Roese <sr@denx.de>
Tested-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[U-Boot] [PATCH 12/14] usb: xhci: Program max burst size for endpoint

[Bin Meng]

The 'Max Burst Size' indicates to the xHC the maximum number of
consecutive USB transactions that should be executed per scheduling
opportunity. This is a “zero-based” value, where 0 to 15 represents
burst sizes of 1 to 16, but at present this is always set to zero.
Let's program the required value according to real needs.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 drivers/usb/host/xhci.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 8aed428..dfb188d 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -395,6 +395,22 @@ static u32 xhci_get_endpoint_mult(struct usb_device *udev,
 	return ss_ep_comp_desc->bmAttributes;
 }
 
+static u32 xhci_get_endpoint_max_burst(struct usb_device *udev,
+	struct usb_endpoint_descriptor *endpt_desc,
+	struct usb_ss_ep_comp_descriptor *ss_ep_comp_desc)
+{
+	/* Super speed and Plus have max burst in ep companion desc */
+	if (udev->speed >= USB_SPEED_SUPER)
+		return ss_ep_comp_desc->bMaxBurst;
+
+	if (udev->speed == USB_SPEED_HIGH &&
+	    (usb_endpoint_xfer_isoc(endpt_desc) ||
+	     usb_endpoint_xfer_int(endpt_desc)))
+		return usb_endpoint_maxp_mult(endpt_desc) - 1;
+
+	return 0;
+}
+
 /*
  * Return the maximum endpoint service interval time (ESIT) payload.
  * Basically, this is the maxpacket size, multiplied by the burst size
@@ -493,6 +509,7 @@ static int xhci_set_configuration(struct usb_device *udev)
 	u32 max_esit_payload;
 	unsigned int interval;
 	unsigned int mult;
+	unsigned int max_burst;
 	unsigned int avg_trb_len;
 
 	out_ctx = virt_dev->out_ctx;
@@ -545,6 +562,8 @@ static int xhci_set_configuration(struct usb_device *udev)
 		interval = xhci_get_endpoint_interval(udev, endpt_desc);
 		mult = xhci_get_endpoint_mult(udev, endpt_desc,
 					      ss_ep_comp_desc);
+		max_burst = xhci_get_endpoint_max_burst(udev, endpt_desc,
+							ss_ep_comp_desc);
 		avg_trb_len = max_esit_payload;
 
 		ep_index = xhci_get_ep_index(endpt_desc);
@@ -570,7 +589,7 @@ static int xhci_set_configuration(struct usb_device *udev)
 			(get_unaligned(&endpt_desc->wMaxPacketSize)));
 
 		ep_ctx[ep_index]->ep_info2 |=
-			cpu_to_le32(((0 & MAX_BURST_MASK) << MAX_BURST_SHIFT) |
+			cpu_to_le32(MAX_BURST(max_burst) |
 			((3 & ERROR_COUNT_MASK) << ERROR_COUNT_SHIFT));
 
 		trb_64 = (uintptr_t)
-- 
2.9.2

[U-Boot] [PATCH 12/14] usb: xhci: Program max burst size for endpoint

[Stefan Roese]

On 18.09.2017 15:40, Bin Meng wrote:
> The 'Max Burst Size' indicates to the xHC the maximum number of
> consecutive USB transactions that should be executed per scheduling
> opportunity. This is a “zero-based” value, where 0 to 15 represents
> burst sizes of 1 to 16, but at present this is always set to zero.
> Let's program the required value according to real needs.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
> 
>   drivers/usb/host/xhci.c | 21 ++++++++++++++++++++-
>   1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 8aed428..dfb188d 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -395,6 +395,22 @@ static u32 xhci_get_endpoint_mult(struct usb_device *udev,
>   	return ss_ep_comp_desc->bmAttributes;
>   }
>   
> +static u32 xhci_get_endpoint_max_burst(struct usb_device *udev,
> +	struct usb_endpoint_descriptor *endpt_desc,
> +	struct usb_ss_ep_comp_descriptor *ss_ep_comp_desc)
> +{
> +	/* Super speed and Plus have max burst in ep companion desc */
> +	if (udev->speed >= USB_SPEED_SUPER)
> +		return ss_ep_comp_desc->bMaxBurst;
> +
> +	if (udev->speed == USB_SPEED_HIGH &&
> +	    (usb_endpoint_xfer_isoc(endpt_desc) ||
> +	     usb_endpoint_xfer_int(endpt_desc)))
> +		return usb_endpoint_maxp_mult(endpt_desc) - 1;
> +
> +	return 0;
> +}
> +
>   /*
>    * Return the maximum endpoint service interval time (ESIT) payload.
>    * Basically, this is the maxpacket size, multiplied by the burst size
> @@ -493,6 +509,7 @@ static int xhci_set_configuration(struct usb_device *udev)
>   	u32 max_esit_payload;
>   	unsigned int interval;
>   	unsigned int mult;
> +	unsigned int max_burst;
>   	unsigned int avg_trb_len;
>   
>   	out_ctx = virt_dev->out_ctx;
> @@ -545,6 +562,8 @@ static int xhci_set_configuration(struct usb_device *udev)
>   		interval = xhci_get_endpoint_interval(udev, endpt_desc);
>   		mult = xhci_get_endpoint_mult(udev, endpt_desc,
>   					      ss_ep_comp_desc);
> +		max_burst = xhci_get_endpoint_max_burst(udev, endpt_desc,
> +							ss_ep_comp_desc);
>   		avg_trb_len = max_esit_payload;
>   
>   		ep_index = xhci_get_ep_index(endpt_desc);
> @@ -570,7 +589,7 @@ static int xhci_set_configuration(struct usb_device *udev)
>   			(get_unaligned(&endpt_desc->wMaxPacketSize)));
>   
>   		ep_ctx[ep_index]->ep_info2 |=
> -			cpu_to_le32(((0 & MAX_BURST_MASK) << MAX_BURST_SHIFT) |
> +			cpu_to_le32(MAX_BURST(max_burst) |
>   			((3 & ERROR_COUNT_MASK) << ERROR_COUNT_SHIFT));
>   
>   		trb_64 = (uintptr_t)
> 

Reviewed-by: Stefan Roese <sr@denx.de>
Tested-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[U-Boot] [PATCH 13/14] usb: xhci: Set 'Error Count' to 0 for isoch endpoints

[Bin Meng]

Per xHCI spec, 'Error Count' should be set to 0 for isoch endpoints.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 drivers/usb/host/xhci.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index dfb188d..93737b0 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -511,6 +511,7 @@ static int xhci_set_configuration(struct usb_device *udev)
 	unsigned int mult;
 	unsigned int max_burst;
 	unsigned int avg_trb_len;
+	unsigned int err_count = 0;
 
 	out_ctx = virt_dev->out_ctx;
 	in_ctx = virt_dev->in_ctx;
@@ -588,9 +589,12 @@ static int xhci_set_configuration(struct usb_device *udev)
 			cpu_to_le32(MAX_PACKET
 			(get_unaligned(&endpt_desc->wMaxPacketSize)));
 
+		/* Allow 3 retries for everything but isoc, set CErr = 3 */
+		if (!usb_endpoint_xfer_isoc(endpt_desc))
+			err_count = 3;
 		ep_ctx[ep_index]->ep_info2 |=
 			cpu_to_le32(MAX_BURST(max_burst) |
-			((3 & ERROR_COUNT_MASK) << ERROR_COUNT_SHIFT));
+			ERROR_COUNT(err_count));
 
 		trb_64 = (uintptr_t)
 				virt_dev->eps[ep_index].ring->enqueue;
-- 
2.9.2

[U-Boot] [PATCH 13/14] usb: xhci: Set 'Error Count' to 0 for isoch endpoints

[Stefan Roese]

On 18.09.2017 15:40, Bin Meng wrote:
> Per xHCI spec, 'Error Count' should be set to 0 for isoch endpoints.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
> 
>   drivers/usb/host/xhci.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index dfb188d..93737b0 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -511,6 +511,7 @@ static int xhci_set_configuration(struct usb_device *udev)
>   	unsigned int mult;
>   	unsigned int max_burst;
>   	unsigned int avg_trb_len;
> +	unsigned int err_count = 0;
>   
>   	out_ctx = virt_dev->out_ctx;
>   	in_ctx = virt_dev->in_ctx;
> @@ -588,9 +589,12 @@ static int xhci_set_configuration(struct usb_device *udev)
>   			cpu_to_le32(MAX_PACKET
>   			(get_unaligned(&endpt_desc->wMaxPacketSize)));
>   
> +		/* Allow 3 retries for everything but isoc, set CErr = 3 */
> +		if (!usb_endpoint_xfer_isoc(endpt_desc))
> +			err_count = 3;
>   		ep_ctx[ep_index]->ep_info2 |=
>   			cpu_to_le32(MAX_BURST(max_burst) |
> -			((3 & ERROR_COUNT_MASK) << ERROR_COUNT_SHIFT));
> +			ERROR_COUNT(err_count));
>   
>   		trb_64 = (uintptr_t)
>   				virt_dev->eps[ep_index].ring->enqueue;
> 

Reviewed-by: Stefan Roese <sr@denx.de>
Tested-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[U-Boot] [PATCH 14/14] usb: xhci: Set 'Average TRB Length' to 8 for control endpoints

[Bin Meng]

Update the codes to conform with xHCI spec chapter 6.2.3.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

---

 drivers/usb/host/xhci-mem.c | 6 ++++++
 drivers/usb/host/xhci.c     | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 84982a9..0582a9b 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -850,6 +850,12 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl,
 	trb_64 = (uintptr_t)virt_dev->eps[0].ring->first_seg->trbs;
 	ep0_ctx->deq = cpu_to_le64(trb_64 | virt_dev->eps[0].ring->cycle_state);
 
+	/*
+	 * xHCI spec 6.2.3:
+	 * software shall set 'Average TRB Length' to 8 for control endpoints.
+	 */
+	ep0_ctx->tx_info = cpu_to_le32(EP_AVG_TRB_LENGTH(8));
+
 	/* Steps 7 and 8 were done in xhci_alloc_virt_device() */
 
 	xhci_flush_cache((uintptr_t)ep0_ctx, sizeof(struct xhci_ep_ctx));
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 93737b0..4673738 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -601,6 +601,12 @@ static int xhci_set_configuration(struct usb_device *udev)
 		ep_ctx[ep_index]->deq = cpu_to_le64(trb_64 |
 				virt_dev->eps[ep_index].ring->cycle_state);
 
+		/*
+		 * xHCI spec 6.2.3:
+		 * 'Average TRB Length' should be 8 for control endpoints.
+		 */
+		if (usb_endpoint_xfer_control(endpt_desc))
+			avg_trb_len = 8;
 		ep_ctx[ep_index]->tx_info =
 			cpu_to_le32(EP_MAX_ESIT_PAYLOAD_LO(max_esit_payload) |
 			EP_AVG_TRB_LENGTH(avg_trb_len));
-- 
2.9.2

[U-Boot] [PATCH 14/14] usb: xhci: Set 'Average TRB Length' to 8 for control endpoints

[Stefan Roese]

On 18.09.2017 15:40, Bin Meng wrote:
> Update the codes to conform with xHCI spec chapter 6.2.3.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> 
> ---
> 
>   drivers/usb/host/xhci-mem.c | 6 ++++++
>   drivers/usb/host/xhci.c     | 6 ++++++
>   2 files changed, 12 insertions(+)
> 
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index 84982a9..0582a9b 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -850,6 +850,12 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl,
>   	trb_64 = (uintptr_t)virt_dev->eps[0].ring->first_seg->trbs;
>   	ep0_ctx->deq = cpu_to_le64(trb_64 | virt_dev->eps[0].ring->cycle_state);
>   
> +	/*
> +	 * xHCI spec 6.2.3:
> +	 * software shall set 'Average TRB Length' to 8 for control endpoints.
> +	 */
> +	ep0_ctx->tx_info = cpu_to_le32(EP_AVG_TRB_LENGTH(8));
> +
>   	/* Steps 7 and 8 were done in xhci_alloc_virt_device() */
>   
>   	xhci_flush_cache((uintptr_t)ep0_ctx, sizeof(struct xhci_ep_ctx));
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 93737b0..4673738 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -601,6 +601,12 @@ static int xhci_set_configuration(struct usb_device *udev)
>   		ep_ctx[ep_index]->deq = cpu_to_le64(trb_64 |
>   				virt_dev->eps[ep_index].ring->cycle_state);
>   
> +		/*
> +		 * xHCI spec 6.2.3:
> +		 * 'Average TRB Length' should be 8 for control endpoints.
> +		 */
> +		if (usb_endpoint_xfer_control(endpt_desc))
> +			avg_trb_len = 8;
>   		ep_ctx[ep_index]->tx_info =
>   			cpu_to_le32(EP_MAX_ESIT_PAYLOAD_LO(max_esit_payload) |
>   			EP_AVG_TRB_LENGTH(avg_trb_len));
> 

Reviewed-by: Stefan Roese <sr@denx.de>
Tested-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[U-Boot] [PATCH 00/14] usb: xhci: Add interrupt transfer support and full speed device support

[Marek Vasut]

On 09/18/2017 03:40 PM, Bin Meng wrote:
> 
> This adds the missing interrupt transfer support to xHCI driver, so
> that devices like USB keyboard that uses interrupt transfer when
> CONFIG_SYS_USB_EVENT_POLL is defined can work.
> 
> This also adds full speed device support. Unlike low/high/super speed
> devices, full speed device may report its endpoint 0 max packet size
> as 8/16/32/64. xHCI driver guesses 64 for the first attempt to get the
> initial 8 bytes device descriptor, and later adjusts the context for
> endpoint 0.
> 
> This series also made several updates to xHCI driver to conform with
> the spec.
> 
> This series is available at u-boot-x86/xhci-working for testing.
Looks good to me. Stefan, can you please give A-B/R-B on the series ?
I'd like to pick it for 2017.09 :)

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 00/14] usb: xhci: Add interrupt transfer support and full speed device support

[Stefan Roese]

On 18.09.2017 17:13, Marek Vasut wrote:
> On 09/18/2017 03:40 PM, Bin Meng wrote:
>>
>> This adds the missing interrupt transfer support to xHCI driver, so
>> that devices like USB keyboard that uses interrupt transfer when
>> CONFIG_SYS_USB_EVENT_POLL is defined can work.
>>
>> This also adds full speed device support. Unlike low/high/super speed
>> devices, full speed device may report its endpoint 0 max packet size
>> as 8/16/32/64. xHCI driver guesses 64 for the first attempt to get the
>> initial 8 bytes device descriptor, and later adjusts the context for
>> endpoint 0.
>>
>> This series also made several updates to xHCI driver to conform with
>> the spec.
>>
>> This series is available at u-boot-x86/xhci-working for testing.
> Looks good to me. Stefan, can you please give A-B/R-B on the series ?
> I'd like to pick it for 2017.09 :)

That would be 2017.11, right?

Sure, I'll review and test this series most likely tomorrow.

Thanks,
Stefan

[U-Boot] [PATCH 00/14] usb: xhci: Add interrupt transfer support and full speed device support

[Marek Vasut]

On 09/18/2017 05:26 PM, Stefan Roese wrote:
> On 18.09.2017 17:13, Marek Vasut wrote:
>> On 09/18/2017 03:40 PM, Bin Meng wrote:
>>>
>>> This adds the missing interrupt transfer support to xHCI driver, so
>>> that devices like USB keyboard that uses interrupt transfer when
>>> CONFIG_SYS_USB_EVENT_POLL is defined can work.
>>>
>>> This also adds full speed device support. Unlike low/high/super speed
>>> devices, full speed device may report its endpoint 0 max packet size
>>> as 8/16/32/64. xHCI driver guesses 64 for the first attempt to get the
>>> initial 8 bytes device descriptor, and later adjusts the context for
>>> endpoint 0.
>>>
>>> This series also made several updates to xHCI driver to conform with
>>> the spec.
>>>
>>> This series is available at u-boot-x86/xhci-working for testing.
>> Looks good to me. Stefan, can you please give A-B/R-B on the series ?
>> I'd like to pick it for 2017.09 :)
> 
> That would be 2017.11, right?

Yes :)

> Sure, I'll review and test this series most likely tomorrow.

Thanks

> Thanks,
> Stefan


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 00/14] usb: xhci: Add interrupt transfer support and full speed device support

[Stefan Roese]

Hi Bin,

On 18.09.2017 17:26, Stefan Roese wrote:
> On 18.09.2017 17:13, Marek Vasut wrote:
>> On 09/18/2017 03:40 PM, Bin Meng wrote:
>>>
>>> This adds the missing interrupt transfer support to xHCI driver, so
>>> that devices like USB keyboard that uses interrupt transfer when
>>> CONFIG_SYS_USB_EVENT_POLL is defined can work.
>>>
>>> This also adds full speed device support. Unlike low/high/super speed
>>> devices, full speed device may report its endpoint 0 max packet size
>>> as 8/16/32/64. xHCI driver guesses 64 for the first attempt to get the
>>> initial 8 bytes device descriptor, and later adjusts the context for
>>> endpoint 0.
>>>
>>> This series also made several updates to xHCI driver to conform with
>>> the spec.
>>>
>>> This series is available at u-boot-x86/xhci-working for testing.
>> Looks good to me. Stefan, can you please give A-B/R-B on the series ?
>> I'd like to pick it for 2017.09 :)
> 
> That would be 2017.11, right?
> 
> Sure, I'll review and test this series most likely tomorrow.

I did a quick test and most really looks very good. Thanks for all
this great work on this xHCI stuff.

BUT, issuing a few commands, I was able to hang my board. This
sequence is reproducible on my board:


U-Boot 2017.09-00246-g28758a2992 (Sep 18 2017 - 17:29:52 +0200)

CPU: x86_64, vendor Intel, device 30679h
DRAM:  4 GiB
MMC:   pci_mmc: 0, pci_mmc: 1, pci_mmc: 2
SF: Detected w25q64cv with page size 256 Bytes, erase size 4 KiB, total 8 MiB
Model: theadorable-x86-DFI-BT700
Net:   No ethernet found.
scanning bus for devices...
Target spinup took 0 ms.
SATA link 1 timeout.
AHCI 0001.0300 32 slots 2 ports 3 Gbps 0x3 impl SATA mode
flags: 64bit ncq stag pm led clo pio slum part sxs 
  Device 0: (0:0) Vendor: ATA Prod.: SanDisk Ultra II Rev: X411
            Type: Hard Disk
            Capacity: 457862.8 MB = 447.1 GB (937703088 x 512)
960138 bytes read in 72 ms (12.7 MiB/s)
Video: 800x600x16
Hit any key to stop autoboot:  0 
=> 
=> 
=> usb reset
resetting USB...
USB0:   Register 7000820 NbrPorts 7
Starting the controller
USB XHCI 1.00
scanning bus 0 for devices... cannot reset port 1!?
7 USB Device(s) found
       scanning usb for storage devices... 2 Storage Device(s) found
=> usb tree
USB device tree:
  1  Hub (5 Gb/s, 0mA)
  |  U-Boot XHCI Host Controller 
  |
  +-2  Hub (480 Mb/s, 100mA)
  | |
  | +-5  Hub (12 Mb/s, 100mA)
  |    
  +-3  Hub (480 Mb/s, 2mA)
  | |
  | +-6  Mass Storage (480 Mb/s, 98mA)
  | |    USBest Technology USB Mass Storage Device 09092207fbf0c4
  | |  
  | +-7  Mass Storage (480 Mb/s, 500mA)
  |      General  USB Flash Disk   000000000000034B
  |    
  +-4  Vendor specific (5 Gb/s, 64mA)
       Realtek USB 10/100/1000 LAN 000002000000
     
=> ls usb 1
            System Volume Information/
 16779264   PRIME-B350-PLUS-ASUS-0613.CAP
 16779264   PRIME-B350-PLUS-ASUS-0805.CAP

3 file(s), 1 dir(s)

=> usb tree
USB device tree:
  1  Hub (5 Gb/s, 0mA)
  |  U-Boot XHCI Host Controller 
  |
  +-2  Hub (480 Mb/s, 100mA)
  | |
  | +-5  Hub (12 Mb/s, 100mA)
  |    
  +-3  Hub (480 Mb/s, 2mA)
  | |
  | +-6  Mass Storage (480 Mb/s, 98mA)
  | |    USBest Technology USB Mass Storage Device 09092207fbf0c4
  | |  
  | +-7  Mass Storage (480 Mb/s, 500mA)
  |   |  General  USB Flash Disk   000000000000034B
  |   |
  |   |General Protection
EIP: 0010:[<7b56f724>] EFLAGS: 00010246
Original EIP :[<fff1e724>]
EAX: 7b33ac18 EBX: 00000000 ECX: ffffffbf EDX: 7b33ac18
ESI: 7b33ad30 EDI: 00000006 EBP: 7b37b188 ESP: 7b33ac08
 DS: 0018 ES: 0018 FS: 0020 GS: 0018 SS: 0018
CR0: 00000033 CR2: 00000000 CR3: 00000000 CR4: 00000600
DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
DR6: ffff0ff0 DR7: 00000400
Stack:
    0x7b33ac48 : 0x7b37b160
    0x7b33ac44 : 0x00000000
    0x7b33ac40 : 0x00000000
    0x7b33ac3c : 0x00000000
    0x7b33ac38 : 0x00000000
    0x7b33ac34 : 0x20202020
    0x7b33ac30 : 0x52474d54
    0x7b33ac2c : 0x7b37baa8
    0x7b33ac28 : 0x7b37baa8
    0x7b33ac24 : 0x7b33ad30
    0x7b33ac20 : 0x7b5aa84e
    0x7b33ac1c : 0x7b55f008
    0x7b33ac18 : 0xffffffbf
    0x7b33ac14 : 0xcc0a06e4
    0x7b33ac10 : 0xc0e88a40
    0x7b33ac0c : 0x568ad686
--->0x7b33ac08 : 0x7b56f776
    0x7b33ac04 : 0x00010246
    0x7b33ac00 : 0x00000010
    0x7b33abfc : 0x7b56f724
### ERROR ### Please RESET the board ###


Are you able to reproduce this on your board(s) as well? Any ideas
on this? Please let me know if I can do any further tests.

Thanks,
Stefan

[U-Boot] [PATCH 00/14] usb: xhci: Add interrupt transfer support and full speed device support

[Bin Meng]

Hi Stefan,

On Mon, Sep 18, 2017 at 11:38 PM, Stefan Roese <sr@denx.de> wrote:
> Hi Bin,
>
> On 18.09.2017 17:26, Stefan Roese wrote:
>> On 18.09.2017 17:13, Marek Vasut wrote:
>>> On 09/18/2017 03:40 PM, Bin Meng wrote:
>>>>
>>>> This adds the missing interrupt transfer support to xHCI driver, so
>>>> that devices like USB keyboard that uses interrupt transfer when
>>>> CONFIG_SYS_USB_EVENT_POLL is defined can work.
>>>>
>>>> This also adds full speed device support. Unlike low/high/super speed
>>>> devices, full speed device may report its endpoint 0 max packet size
>>>> as 8/16/32/64. xHCI driver guesses 64 for the first attempt to get the
>>>> initial 8 bytes device descriptor, and later adjusts the context for
>>>> endpoint 0.
>>>>
>>>> This series also made several updates to xHCI driver to conform with
>>>> the spec.
>>>>
>>>> This series is available at u-boot-x86/xhci-working for testing.
>>> Looks good to me. Stefan, can you please give A-B/R-B on the series ?
>>> I'd like to pick it for 2017.09 :)
>>
>> That would be 2017.11, right?
>>
>> Sure, I'll review and test this series most likely tomorrow.
>
> I did a quick test and most really looks very good. Thanks for all
> this great work on this xHCI stuff.
>
> BUT, issuing a few commands, I was able to hang my board. This
> sequence is reproducible on my board:
>
>
> U-Boot 2017.09-00246-g28758a2992 (Sep 18 2017 - 17:29:52 +0200)
>
> CPU: x86_64, vendor Intel, device 30679h
> DRAM:  4 GiB
> MMC:   pci_mmc: 0, pci_mmc: 1, pci_mmc: 2
> SF: Detected w25q64cv with page size 256 Bytes, erase size 4 KiB, total 8 MiB
> Model: theadorable-x86-DFI-BT700
> Net:   No ethernet found.
> scanning bus for devices...
> Target spinup took 0 ms.
> SATA link 1 timeout.
> AHCI 0001.0300 32 slots 2 ports 3 Gbps 0x3 impl SATA mode
> flags: 64bit ncq stag pm led clo pio slum part sxs
>   Device 0: (0:0) Vendor: ATA Prod.: SanDisk Ultra II Rev: X411
>             Type: Hard Disk
>             Capacity: 457862.8 MB = 447.1 GB (937703088 x 512)
> 960138 bytes read in 72 ms (12.7 MiB/s)
> Video: 800x600x16
> Hit any key to stop autoboot:  0
> =>
> =>
> => usb reset
> resetting USB...
> USB0:   Register 7000820 NbrPorts 7
> Starting the controller
> USB XHCI 1.00
> scanning bus 0 for devices... cannot reset port 1!?
> 7 USB Device(s) found
>        scanning usb for storage devices... 2 Storage Device(s) found
> => usb tree
> USB device tree:
>   1  Hub (5 Gb/s, 0mA)
>   |  U-Boot XHCI Host Controller
>   |
>   +-2  Hub (480 Mb/s, 100mA)
>   | |
>   | +-5  Hub (12 Mb/s, 100mA)
>   |
>   +-3  Hub (480 Mb/s, 2mA)
>   | |
>   | +-6  Mass Storage (480 Mb/s, 98mA)
>   | |    USBest Technology USB Mass Storage Device 09092207fbf0c4
>   | |
>   | +-7  Mass Storage (480 Mb/s, 500mA)
>   |      General  USB Flash Disk   000000000000034B
>   |
>   +-4  Vendor specific (5 Gb/s, 64mA)
>        Realtek USB 10/100/1000 LAN 000002000000
>
> => ls usb 1
>             System Volume Information/
>  16779264   PRIME-B350-PLUS-ASUS-0613.CAP
>  16779264   PRIME-B350-PLUS-ASUS-0805.CAP
>
> 3 file(s), 1 dir(s)
>
> => usb tree
> USB device tree:
>   1  Hub (5 Gb/s, 0mA)
>   |  U-Boot XHCI Host Controller
>   |
>   +-2  Hub (480 Mb/s, 100mA)
>   | |
>   | +-5  Hub (12 Mb/s, 100mA)
>   |
>   +-3  Hub (480 Mb/s, 2mA)
>   | |
>   | +-6  Mass Storage (480 Mb/s, 98mA)
>   | |    USBest Technology USB Mass Storage Device 09092207fbf0c4
>   | |
>   | +-7  Mass Storage (480 Mb/s, 500mA)
>   |   |  General  USB Flash Disk   000000000000034B
>   |   |
>   |   |General Protection
> EIP: 0010:[<7b56f724>] EFLAGS: 00010246
> Original EIP :[<fff1e724>]
> EAX: 7b33ac18 EBX: 00000000 ECX: ffffffbf EDX: 7b33ac18
> ESI: 7b33ad30 EDI: 00000006 EBP: 7b37b188 ESP: 7b33ac08
>  DS: 0018 ES: 0018 FS: 0020 GS: 0018 SS: 0018
> CR0: 00000033 CR2: 00000000 CR3: 00000000 CR4: 00000600
> DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
> DR6: ffff0ff0 DR7: 00000400
> Stack:
>     0x7b33ac48 : 0x7b37b160
>     0x7b33ac44 : 0x00000000
>     0x7b33ac40 : 0x00000000
>     0x7b33ac3c : 0x00000000
>     0x7b33ac38 : 0x00000000
>     0x7b33ac34 : 0x20202020
>     0x7b33ac30 : 0x52474d54
>     0x7b33ac2c : 0x7b37baa8
>     0x7b33ac28 : 0x7b37baa8
>     0x7b33ac24 : 0x7b33ad30
>     0x7b33ac20 : 0x7b5aa84e
>     0x7b33ac1c : 0x7b55f008
>     0x7b33ac18 : 0xffffffbf
>     0x7b33ac14 : 0xcc0a06e4
>     0x7b33ac10 : 0xc0e88a40
>     0x7b33ac0c : 0x568ad686
> --->0x7b33ac08 : 0x7b56f776
>     0x7b33ac04 : 0x00010246
>     0x7b33ac00 : 0x00000010
>     0x7b33abfc : 0x7b56f724
> ### ERROR ### Please RESET the board ###
>
>
> Are you able to reproduce this on your board(s) as well? Any ideas
> on this? Please let me know if I can do any further tests.

This is an known issue of 'usb info' and 'usb tree' command. See
previous patch on ML @ http://patchwork.ozlabs.org/patch/810732/.
However, that patch has serious issues and needs rework. I can rework
that patch and include it as part of v2 if you like.

Regards,
Bin

[U-Boot] [PATCH 00/14] usb: xhci: Add interrupt transfer support and full speed device support

[Stefan Roese]

Hi Bin,

On 19.09.2017 03:38, Bin Meng wrote:
> On Mon, Sep 18, 2017 at 11:38 PM, Stefan Roese <sr@denx.de> wrote:
>> Hi Bin,
>>
>> On 18.09.2017 17:26, Stefan Roese wrote:
>>> On 18.09.2017 17:13, Marek Vasut wrote:
>>>> On 09/18/2017 03:40 PM, Bin Meng wrote:
>>>>>
>>>>> This adds the missing interrupt transfer support to xHCI driver, so
>>>>> that devices like USB keyboard that uses interrupt transfer when
>>>>> CONFIG_SYS_USB_EVENT_POLL is defined can work.
>>>>>
>>>>> This also adds full speed device support. Unlike low/high/super speed
>>>>> devices, full speed device may report its endpoint 0 max packet size
>>>>> as 8/16/32/64. xHCI driver guesses 64 for the first attempt to get the
>>>>> initial 8 bytes device descriptor, and later adjusts the context for
>>>>> endpoint 0.
>>>>>
>>>>> This series also made several updates to xHCI driver to conform with
>>>>> the spec.
>>>>>
>>>>> This series is available at u-boot-x86/xhci-working for testing.
>>>> Looks good to me. Stefan, can you please give A-B/R-B on the series ?
>>>> I'd like to pick it for 2017.09 :)
>>>
>>> That would be 2017.11, right?
>>>
>>> Sure, I'll review and test this series most likely tomorrow.
>>
>> I did a quick test and most really looks very good. Thanks for all
>> this great work on this xHCI stuff.
>>
>> BUT, issuing a few commands, I was able to hang my board. This
>> sequence is reproducible on my board:
>>
>>
>> U-Boot 2017.09-00246-g28758a2992 (Sep 18 2017 - 17:29:52 +0200)
>>
>> CPU: x86_64, vendor Intel, device 30679h
>> DRAM:  4 GiB
>> MMC:   pci_mmc: 0, pci_mmc: 1, pci_mmc: 2
>> SF: Detected w25q64cv with page size 256 Bytes, erase size 4 KiB, total 8 MiB
>> Model: theadorable-x86-DFI-BT700
>> Net:   No ethernet found.
>> scanning bus for devices...
>> Target spinup took 0 ms.
>> SATA link 1 timeout.
>> AHCI 0001.0300 32 slots 2 ports 3 Gbps 0x3 impl SATA mode
>> flags: 64bit ncq stag pm led clo pio slum part sxs
>>    Device 0: (0:0) Vendor: ATA Prod.: SanDisk Ultra II Rev: X411
>>              Type: Hard Disk
>>              Capacity: 457862.8 MB = 447.1 GB (937703088 x 512)
>> 960138 bytes read in 72 ms (12.7 MiB/s)
>> Video: 800x600x16
>> Hit any key to stop autoboot:  0
>> =>
>> =>
>> => usb reset
>> resetting USB...
>> USB0:   Register 7000820 NbrPorts 7
>> Starting the controller
>> USB XHCI 1.00
>> scanning bus 0 for devices... cannot reset port 1!?
>> 7 USB Device(s) found
>>         scanning usb for storage devices... 2 Storage Device(s) found
>> => usb tree
>> USB device tree:
>>    1  Hub (5 Gb/s, 0mA)
>>    |  U-Boot XHCI Host Controller
>>    |
>>    +-2  Hub (480 Mb/s, 100mA)
>>    | |
>>    | +-5  Hub (12 Mb/s, 100mA)
>>    |
>>    +-3  Hub (480 Mb/s, 2mA)
>>    | |
>>    | +-6  Mass Storage (480 Mb/s, 98mA)
>>    | |    USBest Technology USB Mass Storage Device 09092207fbf0c4
>>    | |
>>    | +-7  Mass Storage (480 Mb/s, 500mA)
>>    |      General  USB Flash Disk   000000000000034B
>>    |
>>    +-4  Vendor specific (5 Gb/s, 64mA)
>>         Realtek USB 10/100/1000 LAN 000002000000
>>
>> => ls usb 1
>>              System Volume Information/
>>   16779264   PRIME-B350-PLUS-ASUS-0613.CAP
>>   16779264   PRIME-B350-PLUS-ASUS-0805.CAP
>>
>> 3 file(s), 1 dir(s)
>>
>> => usb tree
>> USB device tree:
>>    1  Hub (5 Gb/s, 0mA)
>>    |  U-Boot XHCI Host Controller
>>    |
>>    +-2  Hub (480 Mb/s, 100mA)
>>    | |
>>    | +-5  Hub (12 Mb/s, 100mA)
>>    |
>>    +-3  Hub (480 Mb/s, 2mA)
>>    | |
>>    | +-6  Mass Storage (480 Mb/s, 98mA)
>>    | |    USBest Technology USB Mass Storage Device 09092207fbf0c4
>>    | |
>>    | +-7  Mass Storage (480 Mb/s, 500mA)
>>    |   |  General  USB Flash Disk   000000000000034B
>>    |   |
>>    |   |General Protection
>> EIP: 0010:[<7b56f724>] EFLAGS: 00010246
>> Original EIP :[<fff1e724>]
>> EAX: 7b33ac18 EBX: 00000000 ECX: ffffffbf EDX: 7b33ac18
>> ESI: 7b33ad30 EDI: 00000006 EBP: 7b37b188 ESP: 7b33ac08
>>   DS: 0018 ES: 0018 FS: 0020 GS: 0018 SS: 0018
>> CR0: 00000033 CR2: 00000000 CR3: 00000000 CR4: 00000600
>> DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
>> DR6: ffff0ff0 DR7: 00000400
>> Stack:
>>      0x7b33ac48 : 0x7b37b160
>>      0x7b33ac44 : 0x00000000
>>      0x7b33ac40 : 0x00000000
>>      0x7b33ac3c : 0x00000000
>>      0x7b33ac38 : 0x00000000
>>      0x7b33ac34 : 0x20202020
>>      0x7b33ac30 : 0x52474d54
>>      0x7b33ac2c : 0x7b37baa8
>>      0x7b33ac28 : 0x7b37baa8
>>      0x7b33ac24 : 0x7b33ad30
>>      0x7b33ac20 : 0x7b5aa84e
>>      0x7b33ac1c : 0x7b55f008
>>      0x7b33ac18 : 0xffffffbf
>>      0x7b33ac14 : 0xcc0a06e4
>>      0x7b33ac10 : 0xc0e88a40
>>      0x7b33ac0c : 0x568ad686
>> --->0x7b33ac08 : 0x7b56f776
>>      0x7b33ac04 : 0x00010246
>>      0x7b33ac00 : 0x00000010
>>      0x7b33abfc : 0x7b56f724
>> ### ERROR ### Please RESET the board ###
>>
>>
>> Are you able to reproduce this on your board(s) as well? Any ideas
>> on this? Please let me know if I can do any further tests.
> 
> This is an known issue of 'usb info' and 'usb tree' command. See
> previous patch on ML @ http://patchwork.ozlabs.org/patch/810732/.
> However, that patch has serious issues and needs rework. I can rework
> that patch and include it as part of v2 if you like.

Ah, thanks. I had noticed this patch but was not aware of the
severity here.

I've tested with the v2 and it seems to work just fine now.

Thanks,
Stefan

[U-Boot] [PATCH 00/14] usb: xhci: Add interrupt transfer support and full speed device support

[Bin Meng]

Hi Stefan,

On Tue, Sep 19, 2017 at 12:54 PM, Stefan Roese <sr@denx.de> wrote:
> Hi Bin,
>
>
> On 19.09.2017 03:38, Bin Meng wrote:
>>
>> On Mon, Sep 18, 2017 at 11:38 PM, Stefan Roese <sr@denx.de> wrote:
>>>
>>> Hi Bin,
>>>
>>> On 18.09.2017 17:26, Stefan Roese wrote:
>>>>
>>>> On 18.09.2017 17:13, Marek Vasut wrote:
>>>>>
>>>>> On 09/18/2017 03:40 PM, Bin Meng wrote:
>>>>>>
>>>>>>
>>>>>> This adds the missing interrupt transfer support to xHCI driver, so
>>>>>> that devices like USB keyboard that uses interrupt transfer when
>>>>>> CONFIG_SYS_USB_EVENT_POLL is defined can work.
>>>>>>
>>>>>> This also adds full speed device support. Unlike low/high/super speed
>>>>>> devices, full speed device may report its endpoint 0 max packet size
>>>>>> as 8/16/32/64. xHCI driver guesses 64 for the first attempt to get the
>>>>>> initial 8 bytes device descriptor, and later adjusts the context for
>>>>>> endpoint 0.
>>>>>>
>>>>>> This series also made several updates to xHCI driver to conform with
>>>>>> the spec.
>>>>>>
>>>>>> This series is available at u-boot-x86/xhci-working for testing.
>>>>>
>>>>> Looks good to me. Stefan, can you please give A-B/R-B on the series ?
>>>>> I'd like to pick it for 2017.09 :)
>>>>
>>>>
>>>> That would be 2017.11, right?
>>>>
>>>> Sure, I'll review and test this series most likely tomorrow.
>>>
>>>
>>> I did a quick test and most really looks very good. Thanks for all
>>> this great work on this xHCI stuff.
>>>
>>> BUT, issuing a few commands, I was able to hang my board. This
>>> sequence is reproducible on my board:
>>>
>>>
>>> U-Boot 2017.09-00246-g28758a2992 (Sep 18 2017 - 17:29:52 +0200)
>>>
>>> CPU: x86_64, vendor Intel, device 30679h
>>> DRAM:  4 GiB
>>> MMC:   pci_mmc: 0, pci_mmc: 1, pci_mmc: 2
>>> SF: Detected w25q64cv with page size 256 Bytes, erase size 4 KiB, total 8
>>> MiB
>>> Model: theadorable-x86-DFI-BT700
>>> Net:   No ethernet found.
>>> scanning bus for devices...
>>> Target spinup took 0 ms.
>>> SATA link 1 timeout.
>>> AHCI 0001.0300 32 slots 2 ports 3 Gbps 0x3 impl SATA mode
>>> flags: 64bit ncq stag pm led clo pio slum part sxs
>>>    Device 0: (0:0) Vendor: ATA Prod.: SanDisk Ultra II Rev: X411
>>>              Type: Hard Disk
>>>              Capacity: 457862.8 MB = 447.1 GB (937703088 x 512)
>>> 960138 bytes read in 72 ms (12.7 MiB/s)
>>> Video: 800x600x16
>>> Hit any key to stop autoboot:  0
>>> =>
>>> =>
>>> => usb reset
>>> resetting USB...
>>> USB0:   Register 7000820 NbrPorts 7
>>> Starting the controller
>>> USB XHCI 1.00
>>> scanning bus 0 for devices... cannot reset port 1!?
>>> 7 USB Device(s) found
>>>         scanning usb for storage devices... 2 Storage Device(s) found
>>> => usb tree
>>> USB device tree:
>>>    1  Hub (5 Gb/s, 0mA)
>>>    |  U-Boot XHCI Host Controller
>>>    |
>>>    +-2  Hub (480 Mb/s, 100mA)
>>>    | |
>>>    | +-5  Hub (12 Mb/s, 100mA)
>>>    |
>>>    +-3  Hub (480 Mb/s, 2mA)
>>>    | |
>>>    | +-6  Mass Storage (480 Mb/s, 98mA)
>>>    | |    USBest Technology USB Mass Storage Device 09092207fbf0c4
>>>    | |
>>>    | +-7  Mass Storage (480 Mb/s, 500mA)
>>>    |      General  USB Flash Disk   000000000000034B
>>>    |
>>>    +-4  Vendor specific (5 Gb/s, 64mA)
>>>         Realtek USB 10/100/1000 LAN 000002000000
>>>
>>> => ls usb 1
>>>              System Volume Information/
>>>   16779264   PRIME-B350-PLUS-ASUS-0613.CAP
>>>   16779264   PRIME-B350-PLUS-ASUS-0805.CAP
>>>
>>> 3 file(s), 1 dir(s)
>>>
>>> => usb tree
>>> USB device tree:
>>>    1  Hub (5 Gb/s, 0mA)
>>>    |  U-Boot XHCI Host Controller
>>>    |
>>>    +-2  Hub (480 Mb/s, 100mA)
>>>    | |
>>>    | +-5  Hub (12 Mb/s, 100mA)
>>>    |
>>>    +-3  Hub (480 Mb/s, 2mA)
>>>    | |
>>>    | +-6  Mass Storage (480 Mb/s, 98mA)
>>>    | |    USBest Technology USB Mass Storage Device 09092207fbf0c4
>>>    | |
>>>    | +-7  Mass Storage (480 Mb/s, 500mA)
>>>    |   |  General  USB Flash Disk   000000000000034B
>>>    |   |
>>>    |   |General Protection
>>> EIP: 0010:[<7b56f724>] EFLAGS: 00010246
>>> Original EIP :[<fff1e724>]
>>> EAX: 7b33ac18 EBX: 00000000 ECX: ffffffbf EDX: 7b33ac18
>>> ESI: 7b33ad30 EDI: 00000006 EBP: 7b37b188 ESP: 7b33ac08
>>>   DS: 0018 ES: 0018 FS: 0020 GS: 0018 SS: 0018
>>> CR0: 00000033 CR2: 00000000 CR3: 00000000 CR4: 00000600
>>> DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
>>> DR6: ffff0ff0 DR7: 00000400
>>> Stack:
>>>      0x7b33ac48 : 0x7b37b160
>>>      0x7b33ac44 : 0x00000000
>>>      0x7b33ac40 : 0x00000000
>>>      0x7b33ac3c : 0x00000000
>>>      0x7b33ac38 : 0x00000000
>>>      0x7b33ac34 : 0x20202020
>>>      0x7b33ac30 : 0x52474d54
>>>      0x7b33ac2c : 0x7b37baa8
>>>      0x7b33ac28 : 0x7b37baa8
>>>      0x7b33ac24 : 0x7b33ad30
>>>      0x7b33ac20 : 0x7b5aa84e
>>>      0x7b33ac1c : 0x7b55f008
>>>      0x7b33ac18 : 0xffffffbf
>>>      0x7b33ac14 : 0xcc0a06e4
>>>      0x7b33ac10 : 0xc0e88a40
>>>      0x7b33ac0c : 0x568ad686
>>> --->0x7b33ac08 : 0x7b56f776
>>>      0x7b33ac04 : 0x00010246
>>>      0x7b33ac00 : 0x00000010
>>>      0x7b33abfc : 0x7b56f724
>>> ### ERROR ### Please RESET the board ###
>>>
>>>
>>> Are you able to reproduce this on your board(s) as well? Any ideas
>>> on this? Please let me know if I can do any further tests.
>>
>>
>> This is an known issue of 'usb info' and 'usb tree' command. See
>> previous patch on ML @ http://patchwork.ozlabs.org/patch/810732/.
>> However, that patch has serious issues and needs rework. I can rework
>> that patch and include it as part of v2 if you like.
>
>
> Ah, thanks. I had noticed this patch but was not aware of the
> severity here.
>
> I've tested with the v2 and it seems to work just fine now.

Yep, I posted some comments to that v2 patch.

For this xHCI series, I've moved the patch state back to "New".

Regards,
Bin