[U-Boot] [PATCH v4 1/2] usb: Move determination of TT hub address/port into separate function

[U-Boot] [PATCH v4 1/2] usb: Move determination of TT hub address/port into separate function

[Stefan Brüns]

Start split and complete split tokens need the hub address and the
downstream port of the first HS hub (device view).

The core of the function was duplicated in both host/ehci_hcd and
musb-new/usb-compat.h.

Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
Reviewed-by: Marek Vasut <marex@denx.de>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Tested-by: Hans de Goede <hdegoede@redhat.com>
---
v2: - renamed function to usb_find_usb2_hub_address_port()
    - put musb port numbering change into separate patch
v3: - do one assignment per line
v4: - fix error in musb code (udev => urb->dev)
    - added Reviewed-by: et al

 common/usb.c                      | 56 +++++++++++++++++++++++++++++++++++++++
 drivers/usb/host/ehci-hcd.c       | 50 ++++------------------------------
 drivers/usb/musb-new/musb_host.c  | 10 ++++---
 drivers/usb/musb-new/usb-compat.h | 53 ------------------------------------
 include/usb.h                     | 12 +++++++++
 5 files changed, 80 insertions(+), 101 deletions(-)

diff --git a/common/usb.c b/common/usb.c
index b177391..9f67cc1 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -1214,4 +1214,60 @@ bool usb_device_has_child_on_port(struct usb_device *parent, int port)
 #endif
 }
 
+#ifdef CONFIG_DM_USB
+void usb_find_usb2_hub_address_port(struct usb_device *udev,
+			       uint8_t *hub_address, uint8_t *hub_port)
+{
+	struct udevice *parent;
+	struct usb_device *uparent, *ttdev;
+
+	/*
+	 * When called from usb-uclass.c: usb_scan_device() udev->dev points
+	 * to the parent udevice, not the actual udevice belonging to the
+	 * udev as the device is not instantiated yet. So when searching
+	 * for the first usb-2 parent start with udev->dev not
+	 * udev->dev->parent .
+	 */
+	ttdev = udev;
+	parent = udev->dev;
+	uparent = dev_get_parent_priv(parent);
+
+	while (uparent->speed != USB_SPEED_HIGH) {
+		struct udevice *dev = parent;
+
+		if (device_get_uclass_id(dev->parent) != UCLASS_USB_HUB) {
+			printf("Error: Cannot find high speed parent of usb-1 device\n");
+			*hub_address = 0;
+			*hub_port = 0;
+			return;
+		}
+
+		ttdev = dev_get_parent_priv(dev);
+		parent = dev->parent;
+		uparent = dev_get_parent_priv(parent);
+	}
+	*hub_address = uparent->devnum;
+	*hub_port = ttdev->portnr;
+}
+#else
+void usb_find_usb2_hub_address_port(struct usb_device *udev,
+			       uint8_t *hub_address, uint8_t *hub_port)
+{
+	/* Find out the nearest parent which is high speed */
+	while (udev->parent->parent != NULL)
+		if (udev->parent->speed != USB_SPEED_HIGH) {
+			udev = udev->parent;
+		} else {
+			*hub_address = udev->parent->devnum;
+			*hub_port = udev->portnr;
+			return;
+		}
+
+	printf("Error: Cannot find high speed parent of usb-1 device\n");
+	*hub_address = 0;
+	*hub_port = 0;
+}
+#endif
+
+
 /* EOF */
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index c85dbce..af025de 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -279,56 +279,16 @@ static inline u8 ehci_encode_speed(enum usb_device_speed speed)
 static void ehci_update_endpt2_dev_n_port(struct usb_device *udev,
 					  struct QH *qh)
 {
-	struct usb_device *ttdev;
-	int parent_devnum;
+	uint8_t portnr = 0;
+	uint8_t hubaddr = 0;
 
 	if (udev->speed != USB_SPEED_LOW && udev->speed != USB_SPEED_FULL)
 		return;
 
-	/*
-	 * For full / low speed devices we need to get the devnum and portnr of
-	 * the tt, so of the first upstream usb-2 hub, there may be usb-1 hubs
-	 * in the tree before that one!
-	 */
-#ifdef CONFIG_DM_USB
-	/*
-	 * When called from usb-uclass.c: usb_scan_device() udev->dev points
-	 * to the parent udevice, not the actual udevice belonging to the
-	 * udev as the device is not instantiated yet. So when searching
-	 * for the first usb-2 parent start with udev->dev not
-	 * udev->dev->parent .
-	 */
-	struct udevice *parent;
-	struct usb_device *uparent;
-
-	ttdev = udev;
-	parent = udev->dev;
-	uparent = dev_get_parent_priv(parent);
-
-	while (uparent->speed != USB_SPEED_HIGH) {
-		struct udevice *dev = parent;
-
-		if (device_get_uclass_id(dev->parent) != UCLASS_USB_HUB) {
-			printf("ehci: Error cannot find high-speed parent of usb-1 device\n");
-			return;
-		}
-
-		ttdev = dev_get_parent_priv(dev);
-		parent = dev->parent;
-		uparent = dev_get_parent_priv(parent);
-	}
-	parent_devnum = uparent->devnum;
-#else
-	ttdev = udev;
-	while (ttdev->parent && ttdev->parent->speed != USB_SPEED_HIGH)
-		ttdev = ttdev->parent;
-	if (!ttdev->parent)
-		return;
-	parent_devnum = ttdev->parent->devnum;
-#endif
+	usb_find_usb2_hub_address_port(udev, &hubaddr, &portnr)
 
-	qh->qh_endpt2 |= cpu_to_hc32(QH_ENDPT2_PORTNUM(ttdev->portnr) |
-				     QH_ENDPT2_HUBADDR(parent_devnum));
+	qh->qh_endpt2 |= cpu_to_hc32(QH_ENDPT2_PORTNUM(portnr) |
+				     QH_ENDPT2_HUBADDR(hubaddr));
 }
 
 static int
diff --git a/drivers/usb/musb-new/musb_host.c b/drivers/usb/musb-new/musb_host.c
index 40b9c66..70f8a99 100644
--- a/drivers/usb/musb-new/musb_host.c
+++ b/drivers/usb/musb-new/musb_host.c
@@ -2092,9 +2092,13 @@ int musb_urb_enqueue(
 			}
 #else
 			if (tt_needed(musb, urb->dev)) {
-				u16 hub_port = find_tt(urb->dev);
-				qh->h_addr_reg = (u8) (hub_port >> 8);
-				qh->h_port_reg = (u8) (hub_port & 0xff);
+				uint8_t portnr = 0;
+				uint8_t hubaddr = 0;
+				usb_find_usb2_hub_address_port(urb->dev,
+							       &hubaddr,
+							       &portnr);
+				qh->h_addr_reg = hubaddr;
+				qh->h_port_reg = portnr - 1;
 			}
 #endif
 		}
diff --git a/drivers/usb/musb-new/usb-compat.h b/drivers/usb/musb-new/usb-compat.h
index 1c41e2a..760bd78 100644
--- a/drivers/usb/musb-new/usb-compat.h
+++ b/drivers/usb/musb-new/usb-compat.h
@@ -68,38 +68,6 @@ static inline int usb_hcd_unmap_urb_for_dma(struct usb_hcd *hcd,
 }
 
 #ifdef CONFIG_DM_USB
-static inline u16 find_tt(struct usb_device *udev)
-{
-	struct udevice *parent;
-	struct usb_device *uparent, *ttdev;
-
-	/*
-	 * When called from usb-uclass.c: usb_scan_device() udev->dev points
-	 * to the parent udevice, not the actual udevice belonging to the
-	 * udev as the device is not instantiated yet. So when searching
-	 * for the first usb-2 parent start with udev->dev not
-	 * udev->dev->parent .
-	 */
-	ttdev = udev;
-	parent = udev->dev;
-	uparent = dev_get_parent_priv(parent);
-
-	while (uparent->speed != USB_SPEED_HIGH) {
-		struct udevice *dev = parent;
-
-		if (device_get_uclass_id(dev->parent) != UCLASS_USB_HUB) {
-			printf("musb: Error cannot find high speed parent of usb-1 device\n");
-			return 0;
-		}
-
-		ttdev = dev_get_parent_priv(dev);
-		parent = dev->parent;
-		uparent = dev_get_parent_priv(parent);
-	}
-
-	return (uparent->devnum << 8) | (ttdev->portnr - 1);
-}
-
 static inline struct usb_device *usb_dev_get_parent(struct usb_device *udev)
 {
 	struct udevice *parent = udev->dev->parent;
@@ -129,27 +97,6 @@ static inline struct usb_device *usb_dev_get_parent(struct usb_device *udev)
 	return NULL;
 }
 #else
-static inline u16 find_tt(struct usb_device *dev)
-{
-	u8 chid;
-	u8 hub;
-
-	/* Find out the nearest parent which is high speed */
-	while (dev->parent->parent != NULL)
-		if (dev->parent->speed != USB_SPEED_HIGH)
-			dev = dev->parent;
-		else
-			break;
-
-	/* determine the port address at that hub */
-	hub = dev->parent->devnum;
-	for (chid = 0; chid < USB_MAXCHILDREN; chid++)
-		if (dev->parent->children[chid] == dev)
-			break;
-
-	return (hub << 8) | chid;
-}
-
 static inline struct usb_device *usb_dev_get_parent(struct usb_device *dev)
 {
 	return dev->parent;
diff --git a/include/usb.h b/include/usb.h
index 198ecbc..2539364 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -875,6 +875,18 @@ int legacy_hub_port_reset(struct usb_device *dev, int port,
 
 int hub_port_reset(struct udevice *dev, int port, unsigned short *portstat);
 
+/*
+ * usb_find_usb2_hub_address_port() - Get hub address and port for TT setting
+ *
+ * Searches for the first HS hub above the given device. If a
+ * HS hub is found, the hub address and the port the device is
+ * connected to is return, as required for SPLIT transactions
+ *
+ * @param: udev full speed or low speed device
+ */
+void usb_find_usb2_hub_address_port(struct usb_device *udev,
+				    uint8_t *hub_address, uint8_t *hub_port);
+
 /**
  * usb_alloc_new_device() - Allocate a new device
  *
-- 
2.1.4

[U-Boot] [PATCH v4 2/2] usb: musb: Fix hub port setting for SPLIT transactions

[Stefan Brüns]

The ifdef'ed Linux kernel code uses the 1 based port number, whereas U-Boot
puts a 0 based port number into the register. The reason the 0 based port
number apparently works can probably be taken from the USB 2.0 spec:

8.4.2.2 Start-Split Transaction Token
... The host must correctly set the port field for single and multiple TT
hub implementations. A single TT hub implementation *may ignore* the port
field.

Actually, as far as I understand, a multi TT hub defaults to single TT
(bAlternateSetting: 0) until switched via SetInterface, so even "port 42"
would work.

The change was verified by hardcoding the port number to a wrong value,
SPLIT transactions kept working (although using a DWC2 instead of MUSB).
Tested hubs are the RPi onboard SMC9514 and an external "05e3:0608
Genesys Logic, Inc. USB-2.0 4-Port HUB". The former is a multi TT hub,
the latter single TT only.

Addendum: Tested on sunxi/MUSB by Hans de Goede

Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Tested-by: Hans de Goede <hdegoede@redhat.com>
---
v2: - renamed function to usb_find_usb2_hub_address_port()
    - put musb port numbering change into separate patch
v3: - do one assignment per line
v4: - fix error in musb code (udev => urb->dev)
    - added Reviewed-by: et al

 drivers/usb/musb-new/musb_host.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/musb-new/musb_host.c b/drivers/usb/musb-new/musb_host.c
index 70f8a99..2515447 100644
--- a/drivers/usb/musb-new/musb_host.c
+++ b/drivers/usb/musb-new/musb_host.c
@@ -2098,7 +2098,7 @@ int musb_urb_enqueue(
 							       &hubaddr,
 							       &portnr);
 				qh->h_addr_reg = hubaddr;
-				qh->h_port_reg = portnr - 1;
+				qh->h_port_reg = portnr;
 			}
 #endif
 		}
-- 
2.1.4

[U-Boot] [PATCH v4 2/2] usb: musb: Fix hub port setting for SPLIT transactions

[Marek Vasut]

On Tuesday, December 22, 2015 at 01:21:04 AM, Stefan Br?ns wrote:
> The ifdef'ed Linux kernel code uses the 1 based port number, whereas U-Boot
> puts a 0 based port number into the register. The reason the 0 based port
> number apparently works can probably be taken from the USB 2.0 spec:
> 
> 8.4.2.2 Start-Split Transaction Token
> ... The host must correctly set the port field for single and multiple TT
> hub implementations. A single TT hub implementation *may ignore* the port
> field.
> 
> Actually, as far as I understand, a multi TT hub defaults to single TT
> (bAlternateSetting: 0) until switched via SetInterface, so even "port 42"
> would work.
> 
> The change was verified by hardcoding the port number to a wrong value,
> SPLIT transactions kept working (although using a DWC2 instead of MUSB).
> Tested hubs are the RPi onboard SMC9514 and an external "05e3:0608
> Genesys Logic, Inc. USB-2.0 4-Port HUB". The former is a multi TT hub,
> the latter single TT only.
> 
> Addendum: Tested on sunxi/MUSB by Hans de Goede
> 
> Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> Tested-by: Hans de Goede <hdegoede@redhat.com>

Applied both, thanks!

Best regards,
Marek Vasut

[U-Boot] [PATCH v4 1/2] usb: Move determination of TT hub address/port into separate function

[Marek Vasut]

On Tuesday, December 22, 2015 at 01:21:03 AM, Stefan Br?ns wrote:
> Start split and complete split tokens need the hub address and the
> downstream port of the first HS hub (device view).
> 
> The core of the function was duplicated in both host/ehci_hcd and
> musb-new/usb-compat.h.
> 
> Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
> Reviewed-by: Marek Vasut <marex@denx.de>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> Tested-by: Hans de Goede <hdegoede@redhat.com>
> ---
> v2: - renamed function to usb_find_usb2_hub_address_port()
>     - put musb port numbering change into separate patch
> v3: - do one assignment per line
> v4: - fix error in musb code (udev => urb->dev)
>     - added Reviewed-by: et al
> 
>  common/usb.c                      | 56
> +++++++++++++++++++++++++++++++++++++++ drivers/usb/host/ehci-hcd.c      
> | 50 ++++------------------------------ drivers/usb/musb-new/musb_host.c 
> | 10 ++++---
>  drivers/usb/musb-new/usb-compat.h | 53
> ------------------------------------ include/usb.h                     |
> 12 +++++++++
>  5 files changed, 80 insertions(+), 101 deletions(-)
> 
> diff --git a/common/usb.c b/common/usb.c
> index b177391..9f67cc1 100644
> --- a/common/usb.c
> +++ b/common/usb.c
> @@ -1214,4 +1214,60 @@ bool usb_device_has_child_on_port(struct usb_device
> *parent, int port) #endif
>  }
> 
> +#ifdef CONFIG_DM_USB
> +void usb_find_usb2_hub_address_port(struct usb_device *udev,
> +			       uint8_t *hub_address, uint8_t *hub_port)
> +{
> +	struct udevice *parent;
> +	struct usb_device *uparent, *ttdev;
> +
> +	/*
> +	 * When called from usb-uclass.c: usb_scan_device() udev->dev points
> +	 * to the parent udevice, not the actual udevice belonging to the
> +	 * udev as the device is not instantiated yet. So when searching
> +	 * for the first usb-2 parent start with udev->dev not
> +	 * udev->dev->parent .
> +	 */
> +	ttdev = udev;
> +	parent = udev->dev;
> +	uparent = dev_get_parent_priv(parent);
> +
> +	while (uparent->speed != USB_SPEED_HIGH) {
> +		struct udevice *dev = parent;
> +
> +		if (device_get_uclass_id(dev->parent) != UCLASS_USB_HUB) {
> +			printf("Error: Cannot find high speed parent of usb-1 
device\n");
> +			*hub_address = 0;
> +			*hub_port = 0;
> +			return;
> +		}
> +
> +		ttdev = dev_get_parent_priv(dev);
> +		parent = dev->parent;
> +		uparent = dev_get_parent_priv(parent);
> +	}
> +	*hub_address = uparent->devnum;
> +	*hub_port = ttdev->portnr;
> +}
> +#else
> +void usb_find_usb2_hub_address_port(struct usb_device *udev,
> +			       uint8_t *hub_address, uint8_t *hub_port)
> +{
> +	/* Find out the nearest parent which is high speed */
> +	while (udev->parent->parent != NULL)
> +		if (udev->parent->speed != USB_SPEED_HIGH) {
> +			udev = udev->parent;
> +		} else {
> +			*hub_address = udev->parent->devnum;
> +			*hub_port = udev->portnr;
> +			return;
> +		}
> +
> +	printf("Error: Cannot find high speed parent of usb-1 device\n");
> +	*hub_address = 0;
> +	*hub_port = 0;
> +}
> +#endif
> +
> +
>  /* EOF */
> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> index c85dbce..af025de 100644
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -279,56 +279,16 @@ static inline u8 ehci_encode_speed(enum
> usb_device_speed speed) static void ehci_update_endpt2_dev_n_port(struct
> usb_device *udev, struct QH *qh)
>  {
> -	struct usb_device *ttdev;
> -	int parent_devnum;
> +	uint8_t portnr = 0;
> +	uint8_t hubaddr = 0;
> 
>  	if (udev->speed != USB_SPEED_LOW && udev->speed != USB_SPEED_FULL)
>  		return;
> 
> -	/*
> -	 * For full / low speed devices we need to get the devnum and portnr of
> -	 * the tt, so of the first upstream usb-2 hub, there may be usb-1 hubs
> -	 * in the tree before that one!
> -	 */
> -#ifdef CONFIG_DM_USB
> -	/*
> -	 * When called from usb-uclass.c: usb_scan_device() udev->dev points
> -	 * to the parent udevice, not the actual udevice belonging to the
> -	 * udev as the device is not instantiated yet. So when searching
> -	 * for the first usb-2 parent start with udev->dev not
> -	 * udev->dev->parent .
> -	 */
> -	struct udevice *parent;
> -	struct usb_device *uparent;
> -
> -	ttdev = udev;
> -	parent = udev->dev;
> -	uparent = dev_get_parent_priv(parent);
> -
> -	while (uparent->speed != USB_SPEED_HIGH) {
> -		struct udevice *dev = parent;
> -
> -		if (device_get_uclass_id(dev->parent) != UCLASS_USB_HUB) {
> -			printf("ehci: Error cannot find high-speed parent of 
usb-1 device\n");
> -			return;
> -		}
> -
> -		ttdev = dev_get_parent_priv(dev);
> -		parent = dev->parent;
> -		uparent = dev_get_parent_priv(parent);
> -	}
> -	parent_devnum = uparent->devnum;
> -#else
> -	ttdev = udev;
> -	while (ttdev->parent && ttdev->parent->speed != USB_SPEED_HIGH)
> -		ttdev = ttdev->parent;
> -	if (!ttdev->parent)
> -		return;
> -	parent_devnum = ttdev->parent->devnum;
> -#endif
> +	usb_find_usb2_hub_address_port(udev, &hubaddr, &portnr)

Please compile-test your patches, there's a missing ; .