[U-Boot] [PATCH v2 0/9] usb: driver-model fixes and dm support sunxi-ehci.c

[U-Boot] [PATCH v2 0/9] usb: driver-model fixes and dm support sunxi-ehci.c

[Hans de Goede]

Hi Simon and Marek,

Here is v2 of my driver-model fixes and dm support for sunxi-ehci.c set.

Changes since v1:
-Improved the commit mesg for "dm: usb: Copy over usb_device values from
 usb_scan_device() to final usb_device" adding that doing select config
 twice does not work for (some) usb-1 devices plugged into a usb-2 hub.
-Added a new patch changing usb_get_bus to make it easier to use for
 callers.
-Replace "dm: usb: Use controller_dev in dm ehci code" with
 "dm: usb: Use usb_get_bus in dm ehci code"
-Replace "dm: usb: Store usb_device parent pointer in usb_device" with
 "dm: usb: Fix finding of first upstream usb-2 hub in the ehci dm code"
-"sunxi: ehci: Convert to the driver-model"
 -Added comments describing the various fields in struct ehci_sunxi_priv
 -Dropped the empty ehci_usb_ofdata_to_platdata() function

As discussed before the plan is for patches 1-8 to go upstream through the
dm tree, and then I'll take patch 9 upstream through the sunxi tree as that
depends on some pending sunxi changes.

Regards,

Hans

[U-Boot] [PATCH v2 1/9] dm: usb: Copy over usb_device values from usb_scan_device() to final usb_device

[Hans de Goede]

Currently we copy over a number of usb_device values stored in the on stack
struct usb_device probed in usb_scan_device() to the final driver-model managed
struct usb_device in usb_child_pre_probe() through usb_device_platdata, and
then call usb_select_config() to fill in the rest.

There are 3 problems with this approach:

1) It does not fill in enough fields before calling usb_select_config(),
specifically it does not fill in ep0's maxpacketsize causing a div by zero
exception in the ehci driver.

2) It unnecessarily redoes a number of usb requests making usb probing slower

3) Calling usb_select_config() a second time fails on some usb-1 devices
plugged into usb-2 hubs, causing u-boot to not recognize these devices.

This commit fixes these issues by removing the usb_select_config() call from
usb_child_pre_probe(), and instead of copying over things field by field
through usb_device_platdata, store a pointer to the in stack usb_device
(which is still valid when usb_child_pre_probe() gets called) and copy
over the entire struct.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/usb/host/usb-uclass.c | 27 ++++++---------------------
 include/usb.h                 |  5 +----
 2 files changed, 7 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index 714bc0e..95e371d 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -535,12 +535,7 @@ int usb_scan_device(struct udevice *parent, int port,
 	}
 	plat = dev_get_parent_platdata(dev);
 	debug("%s: Probing '%s', plat=%p\n", __func__, dev->name, plat);
-	plat->devnum = udev->devnum;
-	plat->speed = udev->speed;
-	plat->slot_id = udev->slot_id;
-	plat->portnr = port;
-	debug("** device '%s': stashing slot_id=%d\n", dev->name,
-	      plat->slot_id);
+	plat->udev = udev;
 	priv->next_addr++;
 	ret = device_probe(dev);
 	if (ret) {
@@ -599,25 +594,15 @@ int usb_get_bus(struct udevice *dev, struct udevice **busp)
 
 int usb_child_pre_probe(struct udevice *dev)
 {
-	struct udevice *bus;
 	struct usb_device *udev = dev_get_parentdata(dev);
 	struct usb_dev_platdata *plat = dev_get_parent_platdata(dev);
-	int ret;
 
-	ret = usb_get_bus(dev, &bus);
-	if (ret)
-		return ret;
-	udev->controller_dev = bus;
+	/*
+	 * Copy over all the values set in the on stack struct usb_device in
+	 * usb_scan_device() to our final struct usb_device for this dev.
+	 */
+	*udev = *(plat->udev);
 	udev->dev = dev;
-	udev->devnum = plat->devnum;
-	udev->slot_id = plat->slot_id;
-	udev->portnr = plat->portnr;
-	udev->speed = plat->speed;
-	debug("** device '%s': getting slot_id=%d\n", dev->name, plat->slot_id);
-
-	ret = usb_select_config(udev);
-	if (ret)
-		return ret;
 
 	return 0;
 }
diff --git a/include/usb.h b/include/usb.h
index 1984e8f..1b667ff 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -581,10 +581,7 @@ struct usb_platdata {
  */
 struct usb_dev_platdata {
 	struct usb_device_id id;
-	enum usb_device_speed speed;
-	int devnum;
-	int slot_id;
-	int portnr;	/* Hub port number, 1..n */
+	struct usb_device *udev;
 #ifdef CONFIG_SANDBOX
 	struct usb_string *strings;
 	/* NULL-terminated list of descriptor pointers */
-- 
2.3.6

[U-Boot] [PATCH v2 2/9] dm: usb: Make usb_get_bus easier to use for callers

[Hans de Goede]

Make usb_get_bus easier to use for callers, by directly returning the bus
rather then returning it via a pass-by-ref argument.

This also removes the error checking from the single current caller, as
we alreayd have an assert() for bus not being NULL in usb_get_bus().

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/usb/host/usb-uclass.c | 11 +++--------
 include/usb.h                 |  7 +++----
 2 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index 95e371d..9f690dc 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -477,9 +477,7 @@ int usb_scan_device(struct udevice *parent, int port,
 
 	*devp = NULL;
 	memset(udev, '\0', sizeof(*udev));
-	ret = usb_get_bus(parent, &udev->controller_dev);
-	if (ret)
-		return ret;
+	udev->controller_dev = usb_get_bus(parent);
 	priv = dev_get_uclass_priv(udev->controller_dev);
 
 	/*
@@ -574,22 +572,19 @@ int usb_child_post_bind(struct udevice *dev)
 	return 0;
 }
 
-int usb_get_bus(struct udevice *dev, struct udevice **busp)
+struct udevice *usb_get_bus(struct udevice *dev)
 {
 	struct udevice *bus;
 
-	*busp = NULL;
 	for (bus = dev; bus && device_get_uclass_id(bus) != UCLASS_USB; )
 		bus = bus->parent;
 	if (!bus) {
 		/* By design this cannot happen */
 		assert(bus);
 		debug("USB HUB '%s' does not have a controller\n", dev->name);
-		return -EXDEV;
 	}
-	*busp = bus;
 
-	return 0;
+	return bus;
 }
 
 int usb_child_pre_probe(struct udevice *dev)
diff --git a/include/usb.h b/include/usb.h
index 1b667ff..d8bde1d 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -739,11 +739,10 @@ int usb_scan_device(struct udevice *parent, int port,
  * will be a device with uclass UCLASS_USB.
  *
  * @dev:	Device to check
- * @busp:	Returns bus, or NULL if not found
- * @return 0 if OK, -EXDEV is somehow this bus does not have a controller (this
- *	indicates a critical error in the USB stack
+ * @return The bus, or NULL if not found (this indicates a critical error in
+ *	the USB stack
  */
-int usb_get_bus(struct udevice *dev, struct udevice **busp);
+struct udevice *usb_get_bus(struct udevice *dev);
 
 /**
  * usb_select_config() - Set up a device ready for use
-- 
2.3.6

[U-Boot] [PATCH v2 3/9] dm: usb: Use usb_get_bus in dm ehci code

[Hans de Goede]

Use usb_get_bus in dm ehci code rather then re-implementing it.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/usb/host/ehci-hcd.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index bd9861d..85adbf4 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -125,14 +125,7 @@ static struct descriptor {
 static struct ehci_ctrl *ehci_get_ctrl(struct usb_device *udev)
 {
 #ifdef CONFIG_DM_USB
-	struct udevice *dev;
-
-	/* Find the USB controller */
-	for (dev = udev->dev;
-	     device_get_uclass_id(dev) != UCLASS_USB;
-	     dev = dev->parent)
-		;
-	return dev_get_priv(dev);
+	return dev_get_priv(usb_get_bus(udev->dev));
 #else
 	return udev->controller;
 #endif
-- 
2.3.6

[U-Boot] [PATCH v2 4/9] dm: usb: Fix finding of first upstream usb-2 hub in the ehci dm code

[Hans de Goede]

The ehci driver model code for finding the first upstream usb-2 hub before
this commit has a number of issues:

1) "if (!ttdev->speed != USB_SPEED_HIGH)" does not work because the '!'
   takes presedence over the '!=' this should simply be
   "if (ttdev->speed == USB_SPEED_HIGH)"
2) It makes ttdev point to the first upstream usb-2 hub, but ttdev should
   point to the last usb-1 device before the first usb-2 hub (when going
   upstream from the device), as ttdev is used to find the port of the
   first usb-2 hub to which the the last usb-1 device is connected.
3) parent_devnum however should be set to the devnum of the first usb-2
   hub, so we need to keep pointers around to both usb_device structs.

To complicate things further during enumeration usb_device.dev will point
to the parent udevice, where as during normal use it will point to
the actual udevice, we must handle both cases correctly.

This commit fixes all this making usb-1 devices attached to usb-2 hubs,
including usb-1 devices attached to usb-1 hubs attached to usb-2 hubs, work.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/usb/host/ehci-hcd.c | 34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 85adbf4..9471bcb 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -303,23 +303,33 @@ static void ehci_update_endpt2_dev_n_port(struct usb_device *udev,
 	 * 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_parentdata(parent);
 
-	for (ttdev = udev; ; ) {
-		struct udevice *dev = ttdev->dev;
+	while (uparent->speed != USB_SPEED_HIGH) {
+		struct udevice *dev = parent;
 
-		if (dev->parent &&
-		    device_get_uclass_id(dev->parent) == UCLASS_USB_HUB)
-			parent = dev->parent;
-		else
-			parent = NULL;
-		if (!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_parentdata(parent);
-		if (!ttdev->speed != USB_SPEED_HIGH)
-			break;
+		}
+
+		ttdev = dev_get_parentdata(dev);
+		parent = dev->parent;
+		uparent = dev_get_parentdata(parent);
 	}
-	parent_devnum = ttdev->devnum;
+	parent_devnum = uparent->devnum;
 #else
 	ttdev = udev;
 	while (ttdev->parent && ttdev->parent->speed != USB_SPEED_HIGH)
-- 
2.3.6

[U-Boot] [PATCH v2 5/9] dm: usb: Set desc_before_addr from ehci dm code

[Hans de Goede]

Without this usb-1 device descriptors do not get read properly.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Acked-by: Simon Glass <sjg@chromium.org>
---
 drivers/usb/host/ehci-hcd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 9471bcb..46d01d4 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1579,12 +1579,15 @@ int ehci_register(struct udevice *dev, struct ehci_hccr *hccr,
 		  struct ehci_hcor *hcor, const struct ehci_ops *ops,
 		  uint tweaks, enum usb_init_type init)
 {
+	struct usb_bus_priv *priv = dev_get_uclass_priv(dev);
 	struct ehci_ctrl *ctrl = dev_get_priv(dev);
 	int ret;
 
 	debug("%s: dev='%s', ctrl=%p, hccr=%p, hcor=%p, init=%d\n", __func__,
 	      dev->name, ctrl, hccr, hcor, init);
 
+	priv->desc_before_addr = true;
+
 	ehci_setup_ops(ctrl, ops);
 	ctrl->hccr = hccr;
 	ctrl->hcor = hcor;
-- 
2.3.6

[U-Boot] [PATCH v2 6/9] dm: usb: Add support for interrupt queues to the dm usb code

[Hans de Goede]

Interrupt endpoints typically are polled for a long time by the usb
controller before they return anything, so calls to submit_int_msg() can
take a long time to complete this.

To avoid this the u-boot code has the an interrupt queue mechanism / API,
add support for this to the driver-model usb code.

See the added doc comments for more details.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Acked-by: Simon Glass <sjg@chromium.org>
---
 drivers/usb/host/usb-uclass.c | 36 ++++++++++++++++++++++++++++++++
 include/usb.h                 | 48 ++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 83 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index 9f690dc..a3c3080 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -65,6 +65,42 @@ int submit_bulk_msg(struct usb_device *udev, unsigned long pipe, void *buffer,
 	return ops->bulk(bus, udev, pipe, buffer, length);
 }
 
+struct int_queue *create_int_queue(struct usb_device *udev,
+		unsigned long pipe, int queuesize, int elementsize,
+		void *buffer, int interval)
+{
+	struct udevice *bus = udev->controller_dev;
+	struct dm_usb_ops *ops = usb_get_ops(bus);
+
+	if (!ops->create_int_queue)
+		return NULL;
+
+	return ops->create_int_queue(bus, udev, pipe, queuesize, elementsize,
+				     buffer, interval);
+}
+
+void *poll_int_queue(struct usb_device *udev, struct int_queue *queue)
+{
+	struct udevice *bus = udev->controller_dev;
+	struct dm_usb_ops *ops = usb_get_ops(bus);
+
+	if (!ops->poll_int_queue)
+		return NULL;
+
+	return ops->poll_int_queue(bus, udev, queue);
+}
+
+int destroy_int_queue(struct usb_device *udev, struct int_queue *queue)
+{
+	struct udevice *bus = udev->controller_dev;
+	struct dm_usb_ops *ops = usb_get_ops(bus);
+
+	if (!ops->destroy_int_queue)
+		return -ENOSYS;
+
+	return ops->destroy_int_queue(bus, udev, queue);
+}
+
 int usb_alloc_device(struct usb_device *udev)
 {
 	struct udevice *bus = udev->controller_dev;
diff --git a/include/usb.h b/include/usb.h
index d8bde1d..92c5bbd 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -198,7 +198,7 @@ int submit_control_msg(struct usb_device *dev, unsigned long pipe, void *buffer,
 int submit_int_msg(struct usb_device *dev, unsigned long pipe, void *buffer,
 			int transfer_len, int interval);
 
-#if defined CONFIG_USB_EHCI || defined CONFIG_MUSB_HOST
+#if defined CONFIG_USB_EHCI || defined CONFIG_MUSB_HOST || defined(CONFIG_DM_USB)
 struct int_queue *create_int_queue(struct usb_device *dev, unsigned long pipe,
 	int queuesize, int elementsize, void *buffer, int interval);
 int destroy_int_queue(struct usb_device *dev, struct int_queue *queue);
@@ -654,6 +654,52 @@ struct dm_usb_ops {
 	int (*interrupt)(struct udevice *bus, struct usb_device *udev,
 			 unsigned long pipe, void *buffer, int length,
 			 int interval);
+
+	/**
+	 * create_int_queue() - Create and queue interrupt packets
+	 *
+	 * Create and queue @queuesize number of interrupt usb packets of
+	 * @elementsize bytes each. @buffer must be atleast @queuesize *
+	 * @elementsize bytes.
+	 *
+	 * Note some controllers only support a queuesize of 1.
+	 *
+	 * @interval: Interrupt interval
+	 *
+	 * @return A pointer to the created interrupt queue or NULL on error
+	 */
+	struct int_queue *(*create_int_queue)(struct udevice *bus,
+				struct usb_device *udev, unsigned long pipe,
+				int queuesize, int elementsize, void *buffer,
+				int interval);
+
+	/**
+	 * poll_int_queue() - Poll an interrupt queue for completed packets
+	 *
+	 * Poll an interrupt queue for completed packets. The return value
+	 * points to the part of the buffer passed to create_int_queue()
+	 * corresponding to the completed packet.
+	 *
+	 * @queue: queue to poll
+	 *
+	 * @return Pointer to the data of the first completed packet, or
+	 *         NULL if no packets are ready
+	 */
+	void *(*poll_int_queue)(struct udevice *bus, struct usb_device *udev,
+				struct int_queue *queue);
+
+	/**
+	 * destroy_int_queue() - Destroy an interrupt queue
+	 *
+	 * Destroy an interrupt queue created by create_int_queue().
+	 *
+	 * @queue: queue to poll
+	 *
+	 * @return 0 if OK, -ve on error
+	 */
+	int (*destroy_int_queue)(struct udevice *bus, struct usb_device *udev,
+				 struct int_queue *queue);
+
 	/**
 	 * alloc_device() - Allocate a new device context (XHCI)
 	 *
-- 
2.3.6

[U-Boot] [PATCH v2 7/9] dm: usb: Prefix ehci interrupt-queue functions with _ehci_

[Hans de Goede]

This is a preparation patch for adding interrupt-queue support to the
ehci dm code.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Acked-by: Simon Glass <sjg@chromium.org>
---
 drivers/usb/host/ehci-hcd.c | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 46d01d4..363ce38 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1252,9 +1252,9 @@ disable_periodic(struct ehci_ctrl *ctrl)
 	return 0;
 }
 
-struct int_queue *
-create_int_queue(struct usb_device *dev, unsigned long pipe, int queuesize,
-		 int elementsize, void *buffer, int interval)
+static struct int_queue *_ehci_create_int_queue(struct usb_device *dev,
+			unsigned long pipe, int queuesize, int elementsize,
+			void *buffer, int interval)
 {
 	struct ehci_ctrl *ctrl = ehci_get_ctrl(dev);
 	struct int_queue *result = NULL;
@@ -1410,7 +1410,8 @@ fail1:
 	return NULL;
 }
 
-void *poll_int_queue(struct usb_device *dev, struct int_queue *queue)
+static void *_ehci_poll_int_queue(struct usb_device *dev,
+				  struct int_queue *queue)
 {
 	struct QH *cur = queue->current;
 	struct qTD *cur_td;
@@ -1445,8 +1446,8 @@ void *poll_int_queue(struct usb_device *dev, struct int_queue *queue)
 }
 
 /* Do not free buffers associated with QHs, they're owned by someone else */
-int
-destroy_int_queue(struct usb_device *dev, struct int_queue *queue)
+static int _ehci_destroy_int_queue(struct usb_device *dev,
+				   struct int_queue *queue)
 {
 	struct ehci_ctrl *ctrl = ehci_get_ctrl(dev);
 	int result = -1;
@@ -1547,6 +1548,24 @@ int submit_int_msg(struct usb_device *dev, unsigned long pipe,
 {
 	return _ehci_submit_int_msg(dev, pipe, buffer, length, interval);
 }
+
+struct int_queue *create_int_queue(struct usb_device *dev,
+		unsigned long pipe, int queuesize, int elementsize,
+		void *buffer, int interval)
+{
+	return _ehci_create_int_queue(dev, pipe, queuesize, elementsize,
+				      buffer, interval);
+}
+
+void *poll_int_queue(struct usb_device *dev, struct int_queue *queue)
+{
+	return _ehci_poll_int_queue(dev, queue);
+}
+
+int destroy_int_queue(struct usb_device *dev, struct int_queue *queue)
+{
+	return _ehci_destroy_int_queue(dev, queue);
+}
 #endif
 
 #ifdef CONFIG_DM_USB
-- 
2.3.6

[U-Boot] [PATCH v2 8/9] dm: usb: Add support for interrupt queues to the dm ehci code

[Hans de Goede]

Add support for interrupt queues to the dm ehci code.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Acked-by: Simon Glass <sjg@chromium.org>
---
 drivers/usb/host/ehci-hcd.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 363ce38..0e84265 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1594,6 +1594,29 @@ static int ehci_submit_int_msg(struct udevice *dev, struct usb_device *udev,
 	return _ehci_submit_int_msg(udev, pipe, buffer, length, interval);
 }
 
+static struct int_queue *ehci_create_int_queue(struct udevice *dev,
+		struct usb_device *udev, unsigned long pipe, int queuesize,
+		int elementsize, void *buffer, int interval)
+{
+	debug("%s: dev='%s', udev=%p\n", __func__, dev->name, udev);
+	return _ehci_create_int_queue(udev, pipe, queuesize, elementsize,
+				      buffer, interval);
+}
+
+static void *ehci_poll_int_queue(struct udevice *dev, struct usb_device *udev,
+				 struct int_queue *queue)
+{
+	debug("%s: dev='%s', udev=%p\n", __func__, dev->name, udev);
+	return _ehci_poll_int_queue(udev, queue);
+}
+
+static int ehci_destroy_int_queue(struct udevice *dev, struct usb_device *udev,
+				  struct int_queue *queue)
+{
+	debug("%s: dev='%s', udev=%p\n", __func__, dev->name, udev);
+	return _ehci_destroy_int_queue(udev, queue);
+}
+
 int ehci_register(struct udevice *dev, struct ehci_hccr *hccr,
 		  struct ehci_hcor *hcor, const struct ehci_ops *ops,
 		  uint tweaks, enum usb_init_type init)
@@ -1642,6 +1665,9 @@ struct dm_usb_ops ehci_usb_ops = {
 	.control = ehci_submit_control_msg,
 	.bulk = ehci_submit_bulk_msg,
 	.interrupt = ehci_submit_int_msg,
+	.create_int_queue = ehci_create_int_queue,
+	.poll_int_queue = ehci_poll_int_queue,
+	.destroy_int_queue = ehci_destroy_int_queue,
 };
 
 #endif
-- 
2.3.6

[U-Boot] [PATCH v2 9/9] sunxi: ehci: Convert to the driver-model

[Hans de Goede]

Convert sunxi-boards which use the sunxi-ehci code to the driver-model.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Acked-by: Simon Glass <sjg@chromium.org>
---
 board/sunxi/Kconfig           |  3 ++
 drivers/usb/host/ehci-sunxi.c | 89 +++++++++++++++++++++++++++++--------------
 2 files changed, 63 insertions(+), 29 deletions(-)

diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
index 18e5561..6dcbff3 100644
--- a/board/sunxi/Kconfig
+++ b/board/sunxi/Kconfig
@@ -559,4 +559,7 @@ config DM_ETH
 config DM_SERIAL
 	default y
 
+config DM_USB
+	default y if !USB_MUSB_SUNXI
+
 endif
diff --git a/drivers/usb/host/ehci-sunxi.c b/drivers/usb/host/ehci-sunxi.c
index 0edb643..91443ea 100644
--- a/drivers/usb/host/ehci-sunxi.c
+++ b/drivers/usb/host/ehci-sunxi.c
@@ -14,53 +14,84 @@
 #include <asm/arch/clock.h>
 #include <asm/arch/usb_phy.h>
 #include <asm/io.h>
+#include <dm.h>
 #include "ehci.h"
 
-int ehci_hcd_init(int index, enum usb_init_type init, struct ehci_hccr **hccr,
-		struct ehci_hcor **hcor)
+struct ehci_sunxi_priv {
+	struct ehci_ctrl ehci;
+	int ahb_gate_mask; /* Mask of ahb_gate0 clk gate bits for this hcd */
+	int phy_index;     /* Index of the usb-phy attached to this hcd */
+};
+
+static int ehci_usb_probe(struct udevice *dev)
 {
 	struct sunxi_ccm_reg *ccm = (struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
-	int ahb_gate_offset;
+	struct usb_platdata *plat = dev_get_platdata(dev);
+	struct ehci_sunxi_priv *priv = dev_get_priv(dev);
+	struct ehci_hccr *hccr = (struct ehci_hccr *)dev_get_addr(dev);
+	struct ehci_hcor *hcor;
+
+	if (hccr == (void *)SUNXI_USB1_BASE) {
+		priv->ahb_gate_mask = 1 << AHB_GATE_OFFSET_USB_EHCI0;
+		priv->phy_index = 1;
+	} else {
+		priv->ahb_gate_mask = 1 << AHB_GATE_OFFSET_USB_EHCI1;
+		priv->phy_index = 2;
+	}
 
-	ahb_gate_offset = index ? AHB_GATE_OFFSET_USB_EHCI1 :
-				  AHB_GATE_OFFSET_USB_EHCI0;
-	setbits_le32(&ccm->ahb_gate0, 1 << ahb_gate_offset);
+	setbits_le32(&ccm->ahb_gate0, priv->ahb_gate_mask);
 #ifdef CONFIG_SUNXI_GEN_SUN6I
-	setbits_le32(&ccm->ahb_reset0_cfg, 1 << ahb_gate_offset);
+	setbits_le32(&ccm->ahb_reset0_cfg, priv->ahb_gate_mask);
 #endif
 
-	sunxi_usb_phy_init(index + 1);
-	sunxi_usb_phy_power_on(index + 1);
-
-	if (index == 0)
-		*hccr = (void *)SUNXI_USB1_BASE;
-	else
-		*hccr = (void *)SUNXI_USB2_BASE;
-
-	*hcor = (struct ehci_hcor *)((uint32_t) *hccr
-				+ HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase)));
+	sunxi_usb_phy_init(priv->phy_index);
+	sunxi_usb_phy_power_on(priv->phy_index);
 
-	debug("sunxi-ehci: init hccr %x and hcor %x hc_length %d\n",
-	      (uint32_t)*hccr, (uint32_t)*hcor,
-	      (uint32_t)HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase)));
+	hcor = (struct ehci_hcor *)((uint32_t)hccr +
+				    HC_LENGTH(ehci_readl(&hccr->cr_capbase)));
 
-	return 0;
+	return ehci_register(dev, hccr, hcor, NULL, 0, plat->init_type);
 }
 
-int ehci_hcd_stop(int index)
+static int ehci_usb_remove(struct udevice *dev)
 {
 	struct sunxi_ccm_reg *ccm = (struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
-	int ahb_gate_offset;
+	struct ehci_sunxi_priv *priv = dev_get_priv(dev);
+	int ret;
+
+	ret = ehci_deregister(dev);
+	if (ret)
+		return ret;
 
-	sunxi_usb_phy_power_off(index + 1);
-	sunxi_usb_phy_exit(index + 1);
+	sunxi_usb_phy_power_off(priv->phy_index);
+	sunxi_usb_phy_exit(priv->phy_index);
 
-	ahb_gate_offset = index ? AHB_GATE_OFFSET_USB_EHCI1 :
-				  AHB_GATE_OFFSET_USB_EHCI0;
 #ifdef CONFIG_SUNXI_GEN_SUN6I
-	clrbits_le32(&ccm->ahb_reset0_cfg, 1 << ahb_gate_offset);
+	clrbits_le32(&ccm->ahb_reset0_cfg, priv->ahb_gate_mask);
 #endif
-	clrbits_le32(&ccm->ahb_gate0, 1 << ahb_gate_offset);
+	clrbits_le32(&ccm->ahb_gate0, priv->ahb_gate_mask);
 
 	return 0;
 }
+
+static const struct udevice_id ehci_usb_ids[] = {
+	{ .compatible = "allwinner,sun4i-a10-ehci", },
+	{ .compatible = "allwinner,sun5i-a13-ehci", },
+	{ .compatible = "allwinner,sun6i-a31-ehci", },
+	{ .compatible = "allwinner,sun7i-a20-ehci", },
+	{ .compatible = "allwinner,sun8i-a23-ehci", },
+	{ .compatible = "allwinner,sun9i-a80-ehci", },
+	{ }
+};
+
+U_BOOT_DRIVER(usb_ehci) = {
+	.name	= "ehci_sunxi",
+	.id	= UCLASS_USB,
+	.of_match = ehci_usb_ids,
+	.probe = ehci_usb_probe,
+	.remove = ehci_usb_remove,
+	.ops	= &ehci_usb_ops,
+	.platdata_auto_alloc_size = sizeof(struct usb_platdata),
+	.priv_auto_alloc_size = sizeof(struct ehci_sunxi_priv),
+	.flags	= DM_FLAG_ALLOC_PRIV_DMA,
+};
-- 
2.3.6

[U-Boot] [PATCH v2 1/9] dm: usb: Copy over usb_device values from usb_scan_device() to final usb_device

[Simon Glass]

Hi Hans,

On 1 May 2015 at 04:04, Hans de Goede <hdegoede@redhat.com> wrote:
>
> Currently we copy over a number of usb_device values stored in the on stack
> struct usb_device probed in usb_scan_device() to the final driver-model managed
> struct usb_device in usb_child_pre_probe() through usb_device_platdata, and
> then call usb_select_config() to fill in the rest.
>
> There are 3 problems with this approach:
>
> 1) It does not fill in enough fields before calling usb_select_config(),
> specifically it does not fill in ep0's maxpacketsize causing a div by zero
> exception in the ehci driver.
>
> 2) It unnecessarily redoes a number of usb requests making usb probing slower
>
> 3) Calling usb_select_config() a second time fails on some usb-1 devices
> plugged into usb-2 hubs, causing u-boot to not recognize these devices.
>
> This commit fixes these issues by removing the usb_select_config() call from
> usb_child_pre_probe(), and instead of copying over things field by field
> through usb_device_platdata, store a pointer to the in stack usb_device
> (which is still valid when usb_child_pre_probe() gets called) and copy
> over the entire struct.

OK well you've heard my reservations. Given that we need to fix this
and I don't have the inclination to do the full refactor now (to allow
USB drivers to send requests without a device) I think we should go
along with what you propose.

Let's make sure no one builds on it, by adding comments about the
intended direction (separating out the request from the device that
issues it).

>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/usb/host/usb-uclass.c | 27 ++++++---------------------
>  include/usb.h                 |  5 +----
>  2 files changed, 7 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
> index 714bc0e..95e371d 100644
> --- a/drivers/usb/host/usb-uclass.c
> +++ b/drivers/usb/host/usb-uclass.c
> @@ -535,12 +535,7 @@ int usb_scan_device(struct udevice *parent, int port,
>         }
>         plat = dev_get_parent_platdata(dev);
>         debug("%s: Probing '%s', plat=%p\n", __func__, dev->name, plat);
> -       plat->devnum = udev->devnum;
> -       plat->speed = udev->speed;
> -       plat->slot_id = udev->slot_id;
> -       plat->portnr = port;
> -       debug("** device '%s': stashing slot_id=%d\n", dev->name,
> -             plat->slot_id);
> +       plat->udev = udev;

This is a local variable. How can it be safe to use this pointer and
then return from this function?

For now, how about putting the whole usb_device in the plat structure.
I think it is a lesser evil. You can add a large TODO to fix it up.

>         priv->next_addr++;
>         ret = device_probe(dev);
>         if (ret) {
> @@ -599,25 +594,15 @@ int usb_get_bus(struct udevice *dev, struct udevice **busp)
>
>  int usb_child_pre_probe(struct udevice *dev)
>  {
> -       struct udevice *bus;
>         struct usb_device *udev = dev_get_parentdata(dev);
>         struct usb_dev_platdata *plat = dev_get_parent_platdata(dev);
> -       int ret;
>
> -       ret = usb_get_bus(dev, &bus);
> -       if (ret)
> -               return ret;
> -       udev->controller_dev = bus;
> +       /*
> +        * Copy over all the values set in the on stack struct usb_device in
> +        * usb_scan_device() to our final struct usb_device for this dev.
> +        */
> +       *udev = *(plat->udev);
>         udev->dev = dev;
> -       udev->devnum = plat->devnum;
> -       udev->slot_id = plat->slot_id;
> -       udev->portnr = plat->portnr;
> -       udev->speed = plat->speed;
> -       debug("** device '%s': getting slot_id=%d\n", dev->name, plat->slot_id);
> -
> -       ret = usb_select_config(udev);
> -       if (ret)
> -               return ret;
>
>         return 0;
>  }
> diff --git a/include/usb.h b/include/usb.h
> index 1984e8f..1b667ff 100644
> --- a/include/usb.h
> +++ b/include/usb.h
> @@ -581,10 +581,7 @@ struct usb_platdata {
>   */
>  struct usb_dev_platdata {
>         struct usb_device_id id;
> -       enum usb_device_speed speed;
> -       int devnum;
> -       int slot_id;
> -       int portnr;     /* Hub port number, 1..n */
> +       struct usb_device *udev;

Structure comment needs updating. I think it could do with some big
hairy warnings also.

>  #ifdef CONFIG_SANDBOX
>         struct usb_string *strings;
>         /* NULL-terminated list of descriptor pointers */
> --
> 2.3.6
>

Regards,
Simon

[U-Boot] [PATCH v2 3/9] dm: usb: Use usb_get_bus in dm ehci code

[Simon Glass]

Hi Simon,

On 1 May 2015 at 04:04, Hans de Goede <hdegoede@redhat.com> wrote:
> Use usb_get_bus in dm ehci code rather then re-implementing it.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/usb/host/ehci-hcd.c | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
>

> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> index bd9861d..85adbf4 100644
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -125,14 +125,7 @@ static struct descriptor {
>  static struct ehci_ctrl *ehci_get_ctrl(struct usb_device *udev)
>  {
>  #ifdef CONFIG_DM_USB
> -       struct udevice *dev;
> -
> -       /* Find the USB controller */
> -       for (dev = udev->dev;
> -            device_get_uclass_id(dev) != UCLASS_USB;
> -            dev = dev->parent)
> -               ;
> -       return dev_get_priv(dev);
> +       return dev_get_priv(usb_get_bus(udev->dev));

To be safe shouldn't we check for NULL here?

>  #else
>         return udev->controller;
>  #endif
> --
> 2.3.6
>

Regards,
Simon

[U-Boot] [PATCH v2 2/9] dm: usb: Make usb_get_bus easier to use for callers

[Simon Glass]

On 1 May 2015 at 04:04, Hans de Goede <hdegoede@redhat.com> wrote:
> Make usb_get_bus easier to use for callers, by directly returning the bus
> rather then returning it via a pass-by-ref argument.
>
> This also removes the error checking from the single current caller, as
> we alreayd have an assert() for bus not being NULL in usb_get_bus().
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/usb/host/usb-uclass.c | 11 +++--------
>  include/usb.h                 |  7 +++----
>  2 files changed, 6 insertions(+), 12 deletions(-)
>

Acked-by: Simon Glass <sjg@chromium.org>

> diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
> index 95e371d..9f690dc 100644
> --- a/drivers/usb/host/usb-uclass.c
> +++ b/drivers/usb/host/usb-uclass.c
> @@ -477,9 +477,7 @@ int usb_scan_device(struct udevice *parent, int port,
>
>         *devp = NULL;
>         memset(udev, '\0', sizeof(*udev));
> -       ret = usb_get_bus(parent, &udev->controller_dev);
> -       if (ret)
> -               return ret;
> +       udev->controller_dev = usb_get_bus(parent);
>         priv = dev_get_uclass_priv(udev->controller_dev);
>
>         /*
> @@ -574,22 +572,19 @@ int usb_child_post_bind(struct udevice *dev)
>         return 0;
>  }
>
> -int usb_get_bus(struct udevice *dev, struct udevice **busp)
> +struct udevice *usb_get_bus(struct udevice *dev)
>  {
>         struct udevice *bus;
>
> -       *busp = NULL;
>         for (bus = dev; bus && device_get_uclass_id(bus) != UCLASS_USB; )
>                 bus = bus->parent;
>         if (!bus) {
>                 /* By design this cannot happen */
>                 assert(bus);
>                 debug("USB HUB '%s' does not have a controller\n", dev->name);
> -               return -EXDEV;
>         }
> -       *busp = bus;
>
> -       return 0;
> +       return bus;
>  }
>
>  int usb_child_pre_probe(struct udevice *dev)
> diff --git a/include/usb.h b/include/usb.h
> index 1b667ff..d8bde1d 100644
> --- a/include/usb.h
> +++ b/include/usb.h
> @@ -739,11 +739,10 @@ int usb_scan_device(struct udevice *parent, int port,
>   * will be a device with uclass UCLASS_USB.
>   *
>   * @dev:       Device to check
> - * @busp:      Returns bus, or NULL if not found
> - * @return 0 if OK, -EXDEV is somehow this bus does not have a controller (this
> - *     indicates a critical error in the USB stack
> + * @return The bus, or NULL if not found (this indicates a critical error in
> + *     the USB stack

) here

>   */
> -int usb_get_bus(struct udevice *dev, struct udevice **busp);
> +struct udevice *usb_get_bus(struct udevice *dev);
>
>  /**
>   * usb_select_config() - Set up a device ready for use
> --
> 2.3.6
>

[U-Boot] [PATCH v2 3/9] dm: usb: Use usb_get_bus in dm ehci code

[Simon Glass]

Hi Hans,

On 1 May 2015 at 04:04, Hans de Goede <hdegoede@redhat.com> wrote:
> Use usb_get_bus in dm ehci code rather then re-implementing it.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/usb/host/ehci-hcd.c | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
>

Acked-by: Simon Glass <sjg@chromium.org>

> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> index bd9861d..85adbf4 100644
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -125,14 +125,7 @@ static struct descriptor {
>  static struct ehci_ctrl *ehci_get_ctrl(struct usb_device *udev)
>  {
>  #ifdef CONFIG_DM_USB
> -       struct udevice *dev;
> -
> -       /* Find the USB controller */
> -       for (dev = udev->dev;
> -            device_get_uclass_id(dev) != UCLASS_USB;
> -            dev = dev->parent)
> -               ;
> -       return dev_get_priv(dev);
> +       return dev_get_priv(usb_get_bus(udev->dev));

To be safe shouldn't we check for NULL here?

>  #else
>         return udev->controller;
>  #endif
> --
> 2.3.6
>

Regards,
Simon

[U-Boot] [PATCH v2 4/9] dm: usb: Fix finding of first upstream usb-2 hub in the ehci dm code

[Simon Glass]

On 1 May 2015 at 04:04, Hans de Goede <hdegoede@redhat.com> wrote:
> The ehci driver model code for finding the first upstream usb-2 hub before
> this commit has a number of issues:
>
> 1) "if (!ttdev->speed != USB_SPEED_HIGH)" does not work because the '!'
>    takes presedence over the '!=' this should simply be
>    "if (ttdev->speed == USB_SPEED_HIGH)"
> 2) It makes ttdev point to the first upstream usb-2 hub, but ttdev should
>    point to the last usb-1 device before the first usb-2 hub (when going
>    upstream from the device), as ttdev is used to find the port of the
>    first usb-2 hub to which the the last usb-1 device is connected.
> 3) parent_devnum however should be set to the devnum of the first usb-2
>    hub, so we need to keep pointers around to both usb_device structs.
>
> To complicate things further during enumeration usb_device.dev will point
> to the parent udevice, where as during normal use it will point to
> the actual udevice, we must handle both cases correctly.
>
> This commit fixes all this making usb-1 devices attached to usb-2 hubs,
> including usb-1 devices attached to usb-1 hubs attached to usb-2 hubs, work.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

I'm pleased that you understand this logic.

Acked-by: Simon Glass <sjg@chromium.org>

> ---
>  drivers/usb/host/ehci-hcd.c | 34 ++++++++++++++++++++++------------
>  1 file changed, 22 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> index 85adbf4..9471bcb 100644
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -303,23 +303,33 @@ static void ehci_update_endpt2_dev_n_port(struct usb_device *udev,
>          * 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_parentdata(parent);
>
> -       for (ttdev = udev; ; ) {
> -               struct udevice *dev = ttdev->dev;
> +       while (uparent->speed != USB_SPEED_HIGH) {
> +               struct udevice *dev = parent;
>
> -               if (dev->parent &&
> -                   device_get_uclass_id(dev->parent) == UCLASS_USB_HUB)
> -                       parent = dev->parent;
> -               else
> -                       parent = NULL;
> -               if (!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_parentdata(parent);
> -               if (!ttdev->speed != USB_SPEED_HIGH)
> -                       break;
> +               }
> +
> +               ttdev = dev_get_parentdata(dev);
> +               parent = dev->parent;
> +               uparent = dev_get_parentdata(parent);
>         }
> -       parent_devnum = ttdev->devnum;
> +       parent_devnum = uparent->devnum;
>  #else
>         ttdev = udev;
>         while (ttdev->parent && ttdev->parent->speed != USB_SPEED_HIGH)
> --
> 2.3.6
>

[U-Boot] [PATCH v2 3/9] dm: usb: Use usb_get_bus in dm ehci code

[Hans de Goede]

Hi,

On 05/03/2015 06:59 PM, Simon Glass wrote:
> Hi Hans,
>
> On 1 May 2015 at 04:04, Hans de Goede <hdegoede@redhat.com> wrote:
>> Use usb_get_bus in dm ehci code rather then re-implementing it.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/usb/host/ehci-hcd.c | 9 +--------
>>   1 file changed, 1 insertion(+), 8 deletions(-)
>>
>
> Acked-by: Simon Glass <sjg@chromium.org>
>
>> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
>> index bd9861d..85adbf4 100644
>> --- a/drivers/usb/host/ehci-hcd.c
>> +++ b/drivers/usb/host/ehci-hcd.c
>> @@ -125,14 +125,7 @@ static struct descriptor {
>>   static struct ehci_ctrl *ehci_get_ctrl(struct usb_device *udev)
>>   {
>>   #ifdef CONFIG_DM_USB
>> -       struct udevice *dev;
>> -
>> -       /* Find the USB controller */
>> -       for (dev = udev->dev;
>> -            device_get_uclass_id(dev) != UCLASS_USB;
>> -            dev = dev->parent)
>> -               ;
>> -       return dev_get_priv(dev);
>> +       return dev_get_priv(usb_get_bus(udev->dev));
>
> To be safe shouldn't we check for NULL here?

That should never happen, and there already is an assert for
that in usb_get_bus, or you mean dev_get_priv returning NULL.

Regards,

Hans



>
>>   #else
>>          return udev->controller;
>>   #endif
>> --
>> 2.3.6
>>
>
> Regards,
> Simon
>

[U-Boot] [PATCH v2 3/9] dm: usb: Use usb_get_bus in dm ehci code

[Simon Glass]

Hi Hans,

On 3 May 2015 at 11:15, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
>
> On 05/03/2015 06:59 PM, Simon Glass wrote:
>>
>> Hi Hans,
>>
>> On 1 May 2015 at 04:04, Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>> Use usb_get_bus in dm ehci code rather then re-implementing it.
>>>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>>   drivers/usb/host/ehci-hcd.c | 9 +--------
>>>   1 file changed, 1 insertion(+), 8 deletions(-)
>>>
>>
>> Acked-by: Simon Glass <sjg@chromium.org>
>>
>>> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
>>> index bd9861d..85adbf4 100644
>>> --- a/drivers/usb/host/ehci-hcd.c
>>> +++ b/drivers/usb/host/ehci-hcd.c
>>> @@ -125,14 +125,7 @@ static struct descriptor {
>>>   static struct ehci_ctrl *ehci_get_ctrl(struct usb_device *udev)
>>>   {
>>>   #ifdef CONFIG_DM_USB
>>> -       struct udevice *dev;
>>> -
>>> -       /* Find the USB controller */
>>> -       for (dev = udev->dev;
>>> -            device_get_uclass_id(dev) != UCLASS_USB;
>>> -            dev = dev->parent)
>>> -               ;
>>> -       return dev_get_priv(dev);
>>> +       return dev_get_priv(usb_get_bus(udev->dev));
>>
>>
>> To be safe shouldn't we check for NULL here?
>
>
> That should never happen, and there already is an assert for
> that in usb_get_bus, or you mean dev_get_priv returning NULL.

No I mean usb_get_bus(). It could be hard to debug if it does happen,
but hopefully the first thing they do is turn on DEBUG in that file
and see the assert().

I think it is OK.

Regards,
Simon