[U-Boot] [PATCH 00/22] Convert musb host mode code to the device-model

[U-Boot] [PATCH 00/22] Convert musb host mode code to the device-model

[Hans de Goede]

Hi Marek and Simon,

This series started out with the idea that it would be a nice small project
for the weekend, but it turned out to be a bit more work...

The main purpose of this series is to convert the musb host mode code to the
device-model this has also resulted in various usb fixes / cleanups /
reworking to make this possible, both in the generic usb code as well as in
the device model usb code.

Given that this touches both, I think it is probably best to merge the
first 15 patches through Simon's tree like we did last time, then once those
are place I can merge the sunxi bits. Note this is intended for v2015.10
(ofcourse).

Note that this series is useful for a bunch more boards then just the
single one the last patch updates to use musb + ehci + ohci, but that is
the one I've been testing with other defconfig-s will be updated with
followup patches.

Please review.

Regards,

Hans

[U-Boot] [PATCH 01/22] usb: Always declare usb function prototypes

[Hans de Goede]

There is no harm in declaring the function prototypes even if nothing
implements them, and when CONFIG_DM_USB=y the various usb functions are
available regardless of any controller drivers being enabled.

This fixes compile warnings due to missing prototypes on ARCHs where
the ARCH Kconfig always enables CONFIG_DM_USB and various usb drivers.

One could argue that in the case of no controllers CONFIG_DM_USB should not
be set, but this problem is typically seen during bringup of boards which
do actually have usb controllers.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Simply always define the function prototypes instead of adding yet another
 condition to the already unwieldly #if def ... || def ... condition
---
 include/usb.h | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/include/usb.h b/include/usb.h
index c709ce2..dca512d 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -171,17 +171,6 @@ enum usb_init_type {
  * this is how the lowlevel part communicate with the outer world
  */
 
-#if defined(CONFIG_USB_UHCI) || defined(CONFIG_USB_OHCI) || \
-	defined(CONFIG_USB_EHCI) || defined(CONFIG_USB_OHCI_NEW) || \
-	defined(CONFIG_USB_SL811HS) || defined(CONFIG_USB_ISP116X_HCD) || \
-	defined(CONFIG_USB_R8A66597_HCD) || defined(CONFIG_USB_DAVINCI) || \
-	defined(CONFIG_USB_OMAP3) || defined(CONFIG_USB_DA8XX) || \
-	defined(CONFIG_USB_BLACKFIN) || defined(CONFIG_USB_AM35X) || \
-	defined(CONFIG_USB_MUSB_DSPS) || defined(CONFIG_USB_MUSB_AM35X) || \
-	defined(CONFIG_USB_MUSB_OMAP2PLUS) || defined(CONFIG_USB_MUSB_SUNXI) || \
-	defined(CONFIG_USB_XHCI) || defined(CONFIG_USB_DWC2) || \
-	defined(CONFIG_USB_EMUL)
-
 int usb_lowlevel_init(int index, enum usb_init_type init, void **controller);
 int usb_lowlevel_stop(int index);
 
@@ -216,12 +205,8 @@ void *poll_int_queue(struct usb_device *dev, struct int_queue *queue);
  * in boards init functions e.g. udc_disconnect() used for
  * forced device disconnection from host.
  */
-#elif defined(CONFIG_USB_GADGET_PXA2XX)
-
 extern void udc_disconnect(void);
 
-#endif
-
 /*
  * board-specific hardware initialization, called by
  * usb drivers and u-boot commands
-- 
2.4.3

[U-Boot] [PATCH 02/22] usb: Drop device-model specific copy of usb_legacy_port_reset

[Hans de Goede]

The device-model usb_legacy_port_reset function calls the device-model
usb_port_reset function which is a 1 on 1 copy of the non dm
usb_legacy_port_reset and this is the only use of usb_port_reset in all
of u-boot.

Drop both, and alway use the usb_legacy_port_reset() version in
common/usb.c .

Also while at it make it static as it is only used in common/usb.c .

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 common/usb.c                  |  4 +---
 drivers/usb/host/usb-uclass.c | 29 -----------------------------
 include/usb.h                 |  8 --------
 3 files changed, 1 insertion(+), 40 deletions(-)

diff --git a/common/usb.c b/common/usb.c
index 7ff8ac5..4ddf98f 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -911,8 +911,7 @@ __weak int usb_alloc_device(struct usb_device *udev)
 }
 #endif /* !CONFIG_DM_USB */
 
-#ifndef CONFIG_DM_USB
-int usb_legacy_port_reset(struct usb_device *hub, int portnr)
+static int usb_legacy_port_reset(struct usb_device *hub, int portnr)
 {
 	if (hub) {
 		unsigned short portstatus;
@@ -930,7 +929,6 @@ int usb_legacy_port_reset(struct usb_device *hub, int portnr)
 
 	return 0;
 }
-#endif
 
 static int get_descriptor_len(struct usb_device *dev, int len, int expect_len)
 {
diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index 6e86f4a..10d4712 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -310,35 +310,6 @@ int usb_post_bind(struct udevice *dev)
 	return dm_scan_fdt_node(dev, gd->fdt_blob, dev->of_offset, false);
 }
 
-int usb_port_reset(struct usb_device *parent, int portnr)
-{
-	unsigned short portstatus;
-	int ret;
-
-	debug("%s: start\n", __func__);
-
-	if (parent) {
-		/* reset the port for the second time */
-		assert(portnr > 0);
-		debug("%s: reset %d\n", __func__, portnr - 1);
-		ret = legacy_hub_port_reset(parent, portnr - 1, &portstatus);
-		if (ret < 0) {
-			printf("\n     Couldn't reset port %i\n", portnr);
-			return ret;
-		}
-	} else {
-		debug("%s: reset root\n", __func__);
-		usb_reset_root_port();
-	}
-
-	return 0;
-}
-
-int usb_legacy_port_reset(struct usb_device *parent, int portnr)
-{
-	return usb_port_reset(parent, portnr);
-}
-
 int usb_setup_ehci_gadget(struct ehci_ctrl **ctlrp)
 {
 	struct usb_platdata *plat;
diff --git a/include/usb.h b/include/usb.h
index dca512d..6b5d536 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -730,14 +730,6 @@ int usb_reset_root_port(void);
 struct usb_device *usb_get_dev_index(struct udevice *bus, int index);
 
 /**
- * usb_legacy_port_reset() - Legacy function to reset a hub port
- *
- * @hub:	Hub device
- * @portnr:	Port number (1=first)
- */
-int usb_legacy_port_reset(struct usb_device *hub, int portnr);
-
-/**
  * usb_setup_device() - set up a device ready for use
  *
  * @dev:	USB device pointer. This need not be a real device - it is
-- 
2.4.3

[U-Boot] [PATCH 03/22] usb: usb_setup_device: Drop unneeded portnr function argument

[Hans de Goede]

Drop the unneeded portnr function argument, the portnr is part of the
usb_device struct which is passed via the dev argument.

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

diff --git a/common/usb.c b/common/usb.c
index 4ddf98f..d237478 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -1030,7 +1030,7 @@ static int usb_setup_descriptor(struct usb_device *dev, bool do_read)
 }
 
 static int usb_prepare_device(struct usb_device *dev, int addr, bool do_read,
-			      struct usb_device *parent, int portnr)
+			      struct usb_device *parent)
 {
 	int err;
 
@@ -1048,7 +1048,7 @@ static int usb_prepare_device(struct usb_device *dev, int addr, bool do_read,
 	err = usb_setup_descriptor(dev, do_read);
 	if (err)
 		return err;
-	err = usb_legacy_port_reset(parent, portnr);
+	err = usb_legacy_port_reset(parent, dev->portnr);
 	if (err)
 		return err;
 
@@ -1126,7 +1126,7 @@ int usb_select_config(struct usb_device *dev)
 }
 
 int usb_setup_device(struct usb_device *dev, bool do_read,
-		     struct usb_device *parent, int portnr)
+		     struct usb_device *parent)
 {
 	int addr;
 	int ret;
@@ -1135,7 +1135,7 @@ int usb_setup_device(struct usb_device *dev, bool do_read,
 	addr = dev->devnum;
 	dev->devnum = 0;
 
-	ret = usb_prepare_device(dev, addr, do_read, parent, portnr);
+	ret = usb_prepare_device(dev, addr, do_read, parent);
 	if (ret)
 		return ret;
 	ret = usb_select_config(dev);
@@ -1165,7 +1165,7 @@ int usb_new_device(struct usb_device *dev)
 #ifdef CONFIG_USB_XHCI
 	do_read = false;
 #endif
-	err = usb_setup_device(dev, do_read, dev->parent, dev->portnr);
+	err = usb_setup_device(dev, do_read, dev->parent);
 	if (err)
 		return err;
 
diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index 10d4712..18680c9 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -565,7 +565,7 @@ int usb_scan_device(struct udevice *parent, int port,
 	debug("Calling usb_setup_device(), portnr=%d\n", udev->portnr);
 	parent_udev = device_get_uclass_id(parent) == UCLASS_USB_HUB ?
 		dev_get_parentdata(parent) : NULL;
-	ret = usb_setup_device(udev, priv->desc_before_addr, parent_udev, port);
+	ret = usb_setup_device(udev, priv->desc_before_addr, parent_udev);
 	debug("read_descriptor for '%s': ret=%d\n", parent->name, ret);
 	if (ret)
 		return ret;
diff --git a/include/usb.h b/include/usb.h
index 6b5d536..8a71e28 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -734,14 +734,14 @@ struct usb_device *usb_get_dev_index(struct udevice *bus, int index);
  *
  * @dev:	USB device pointer. This need not be a real device - it is
  *		common for it to just be a local variable with its ->dev
- *		member (i.e. @dev->dev) set to the parent device
+ *		member (i.e. @dev->dev) set to the parent device and
+ *		dev->portnr set to the port number on the hub (1=first)
  * @do_read:	true to read the device descriptor before an address is set
  *		(should be false for XHCI buses, true otherwise)
  * @parent:	Parent device (either UCLASS_USB or UCLASS_USB_HUB)
- * @portnr:	Port number on hub (1=first) or 0 for none
  * @return 0 if OK, -ve on error */
 int usb_setup_device(struct usb_device *dev, bool do_read,
-		     struct usb_device *parent, int portnr);
+		     struct usb_device *parent);
 
 /**
  * usb_hub_scan() - Scan a hub and find its devices
-- 
2.4.3

[U-Boot] [PATCH 04/22] usb: Pass device instead of portnr to usb_legacy_port_reset

[Hans de Goede]

Pass the usb_device instead of the portnr to usb_legacy_port_reset and
rename it to usb_hub_port_reset as there is nothing legacy about it.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 common/usb.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/common/usb.c b/common/usb.c
index d237478..d204ba2 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -911,16 +911,16 @@ __weak int usb_alloc_device(struct usb_device *udev)
 }
 #endif /* !CONFIG_DM_USB */
 
-static int usb_legacy_port_reset(struct usb_device *hub, int portnr)
+static int usb_hub_port_reset(struct usb_device *dev, struct usb_device *hub)
 {
 	if (hub) {
 		unsigned short portstatus;
 		int err;
 
 		/* reset the port for the second time */
-		err = legacy_hub_port_reset(hub, portnr - 1, &portstatus);
+		err = legacy_hub_port_reset(hub, dev->portnr - 1, &portstatus);
 		if (err < 0) {
-			printf("\n     Couldn't reset port %i\n", portnr);
+			printf("\n     Couldn't reset port %i\n", dev->portnr);
 			return err;
 		}
 	} else {
@@ -1048,7 +1048,7 @@ static int usb_prepare_device(struct usb_device *dev, int addr, bool do_read,
 	err = usb_setup_descriptor(dev, do_read);
 	if (err)
 		return err;
-	err = usb_legacy_port_reset(parent, dev->portnr);
+	err = usb_hub_port_reset(dev, parent);
 	if (err)
 		return err;
 
-- 
2.4.3

[U-Boot] [PATCH 05/22] usb: Add an usb_device parameter to usb_reset_root_port

[Hans de Goede]

Add an usb_device parameter to usb_reset_root_port so that it knows which
root-port it is resetting. This is necessary for proper device-model support
for usb_reset_root_port.

Also remove a duplicate declaration of usb_reset_root_port() from usb.h .

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 common/usb.c                      | 2 +-
 drivers/usb/host/usb-uclass.c     | 2 +-
 drivers/usb/musb-new/musb_uboot.c | 4 ++--
 include/usb.h                     | 8 ++------
 4 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/common/usb.c b/common/usb.c
index d204ba2..fbaf8ec 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -924,7 +924,7 @@ static int usb_hub_port_reset(struct usb_device *dev, struct usb_device *hub)
 			return err;
 		}
 	} else {
-		usb_reset_root_port();
+		usb_reset_root_port(dev);
 	}
 
 	return 0;
diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index 18680c9..bce6cec 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -265,7 +265,7 @@ int usb_init(void)
 	return usb_started ? 0 : -1;
 }
 
-int usb_reset_root_port(void)
+int usb_reset_root_port(struct usb_device *udev)
 {
 	return -ENOSYS;
 }
diff --git a/drivers/usb/musb-new/musb_uboot.c b/drivers/usb/musb-new/musb_uboot.c
index d1ee5f8..1bf676c 100644
--- a/drivers/usb/musb-new/musb_uboot.c
+++ b/drivers/usb/musb-new/musb_uboot.c
@@ -180,7 +180,7 @@ void *poll_int_queue(struct usb_device *dev, struct int_queue *queue)
 	return NULL; /* URB still pending */
 }
 
-int usb_reset_root_port(void)
+int usb_reset_root_port(struct usb_device *dev)
 {
 	void *mbase = host->mregs;
 	u8 power;
@@ -232,7 +232,7 @@ int usb_lowlevel_init(int index, enum usb_init_type init, void **controller)
 	if (get_timer(0) >= timeout)
 		return -ENODEV;
 
-	usb_reset_root_port();
+	usb_reset_root_port(NULL);
 	host->is_active = 1;
 	hcd.hcd_priv = host;
 
diff --git a/include/usb.h b/include/usb.h
index 8a71e28..2bb6a06 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -175,9 +175,9 @@ int usb_lowlevel_init(int index, enum usb_init_type init, void **controller);
 int usb_lowlevel_stop(int index);
 
 #if defined(CONFIG_MUSB_HOST) || defined(CONFIG_DM_USB)
-int usb_reset_root_port(void);
+int usb_reset_root_port(struct usb_device *dev);
 #else
-#define usb_reset_root_port()
+#define usb_reset_root_port(dev)
 #endif
 
 int submit_bulk_msg(struct usb_device *dev, unsigned long pipe,
@@ -710,10 +710,6 @@ struct dm_usb_ops {
 #define usb_get_ops(dev)	((struct dm_usb_ops *)(dev)->driver->ops)
 #define usb_get_emul_ops(dev)	((struct dm_usb_ops *)(dev)->driver->ops)
 
-#ifdef CONFIG_MUSB_HOST
-int usb_reset_root_port(void);
-#endif
-
 /**
  * usb_get_dev_index() - look up a device index number
  *
-- 
2.4.3

[U-Boot] [PATCH 06/22] dm: Export device_chld_remove / device_chld_unbind

[Hans de Goede]

These functions are useful to remove all children from an usb bus before
rescanning the bus.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/core/device-remove.c | 18 ++----------------
 include/dm/device-internal.h | 26 ++++++++++++++++++++++++++
 2 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c
index 6a16b4f..06de7e3 100644
--- a/drivers/core/device-remove.c
+++ b/drivers/core/device-remove.c
@@ -18,16 +18,7 @@
 #include <dm/uclass-internal.h>
 #include <dm/util.h>
 
-/**
- * device_chld_unbind() - Unbind all device's children from the device
- *
- * On error, the function continues to unbind all children, and reports the
- * first error.
- *
- * @dev:	The device that is to be stripped of its children
- * @return 0 on success, -ve on error
- */
-static int device_chld_unbind(struct udevice *dev)
+int device_chld_unbind(struct udevice *dev)
 {
 	struct udevice *pos, *n;
 	int ret, saved_ret = 0;
@@ -43,12 +34,7 @@ static int device_chld_unbind(struct udevice *dev)
 	return saved_ret;
 }
 
-/**
- * device_chld_remove() - Stop all device's children
- * @dev:	The device whose children are to be removed
- * @return 0 on success, -ve on error
- */
-static int device_chld_remove(struct udevice *dev)
+int device_chld_remove(struct udevice *dev)
 {
 	struct udevice *pos, *n;
 	int ret;
diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h
index 687462b..6c8fe23 100644
--- a/include/dm/device-internal.h
+++ b/include/dm/device-internal.h
@@ -107,6 +107,32 @@ int device_unbind(struct udevice *dev);
 static inline int device_unbind(struct udevice *dev) { return 0; }
 #endif
 
+/**
+ * device_chld_remove() - Stop all device's children
+ * @dev:	The device whose children are to be removed
+ * @return 0 on success, -ve on error
+ */
+#ifdef CONFIG_DM_DEVICE_REMOVE
+int device_chld_remove(struct udevice *dev);
+#else
+static inline int device_chld_remove(struct udevice *dev) { return 0; }
+#endif
+
+/**
+ * device_chld_unbind() - Unbind all device's children from the device
+ *
+ * On error, the function continues to unbind all children, and reports the
+ * first error.
+ *
+ * @dev:	The device that is to be stripped of its children
+ * @return 0 on success, -ve on error
+ */
+#ifdef CONFIG_DM_DEVICE_REMOVE
+int device_chld_unbind(struct udevice *dev);
+#else
+static inline int device_chld_unbind(struct udevice *dev) { return 0; }
+#endif
+
 #ifdef CONFIG_DM_DEVICE_REMOVE
 void device_free(struct udevice *dev);
 #else
-- 
2.4.3

[U-Boot] [PATCH 07/22] dm: usb: Fix "usb tree" output

[Hans de Goede]

last_child was abused by the old usb code to first store 1 if the
usb_device was not the root of the usb tree, and then later on re-used
to store whether or not the usb_device is actually the last child.

The dm-usb code was always setting it to actually reflect the last-child
status which is wrong for the last child leading to output like this:

USB device tree:
  1  Hub (12 Mb/s, 100mA)
  |  ALCOR USB Hub 2.0
  |
  | 2  Mass Storage (12 Mb/s, 100mA)
  |    USB Flash Disk 4C0E960F
  |
  +-3  Human Interface (1.5 Mb/s, 100mA)
       SINO WEALTH USB Composite Device

Instead of this:

USB device tree:
  1  Hub (12 Mb/s, 100mA)
  |  ALCOR USB Hub 2.0
  |
  +-2  Mass Storage (12 Mb/s, 100mA)
  |    USB Flash Disk 4C0E960F
  |
  +-3  Human Interface (1.5 Mb/s, 100mA)
       SINO WEALTH USB Composite Device

This commit fixes this by first checking that the device is not root,
and then setting last_child. This commit also updates the old code to not
abuse the last_child variable to store the root check result.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 common/cmd_usb.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/common/cmd_usb.c b/common/cmd_usb.c
index eab55cd..ca06826 100644
--- a/common/cmd_usb.c
+++ b/common/cmd_usb.c
@@ -355,12 +355,12 @@ static void usb_show_tree_graph(struct usb_device *dev, char *pre)
 #endif
 	/* check if we are the last one */
 #ifdef CONFIG_DM_USB
-	last_child = device_is_last_sibling(dev->dev);
+	/* Not the root of the usb tree? */
+	if (device_get_uclass_id(dev->dev->parent) != UCLASS_USB) {
+		last_child = device_is_last_sibling(dev->dev);
 #else
-	last_child = (dev->parent != NULL);
-#endif
-	if (last_child) {
-#ifndef CONFIG_DM_USB
+	if (dev->parent != NULL) { /* not root? */
+		last_child = 1;
 		for (i = 0; i < dev->parent->maxchild; i++) {
 			/* search for children */
 			if (dev->parent->children[i] == dev) {
-- 
2.4.3

[U-Boot] [PATCH 08/22] dm: usb: Use device_chld_remove and _unbind to clean up usb devs on stop

[Hans de Goede]

On an usb stop instead of leaving orphan usb devices behind simply remove
them. This requires CONFIG_DM_DEVICE_REMOVE to be set, so only build
usb_stop() when that is set.

The result of this commit is best seen in the output of "dm tree" after
plugging out an usb hub with 2 devices plugges in and plugging in a keyb.
instead, before this commit the output would be:

 usb         [ + ]    `-- sunxi-musb
 usb_hub     [   ]        |-- usb_hub
 usb_mass_st [   ]        |   |-- usb_mass_storage
 usb_dev_gen [   ]        |   `-- generic_bus_0_dev_3
 usb_dev_gen [ + ]        `-- generic_bus_0_dev_1

Notice the non active usb_hub child and its 2 non active children. The
first child being non-active as in this example also causes usb_get_dev_index
to return NULL when probing the first child, which results in the usb kbd
code not binding to the keyboard.

With this commit in place the output after swapping and "usb reset" is:

 usb         [ + ]    `-- sunxi-musb
 usb_dev_gen [ + ]        `-- generic_bus_0_dev_1

As expected, and usb_get_dev_index works properly and the keyboard works.

After this commit usb_find_child() is only necessary for emulated usb
devices, so make its body #ifdef CONFIG_USB_EMUL.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/usb/host/usb-uclass.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index bce6cec..8f26e35 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -128,6 +128,7 @@ int usb_alloc_device(struct usb_device *udev)
 	return ops->alloc_device(bus, udev);
 }
 
+#ifdef CONFIG_DM_DEVICE_REMOVE
 int usb_stop(void)
 {
 	struct udevice *bus;
@@ -143,6 +144,12 @@ int usb_stop(void)
 	uc_priv = uc->priv;
 
 	uclass_foreach_dev(bus, uc) {
+		ret = device_chld_remove(bus);
+		if (ret && !err)
+			err = ret;
+		ret = device_chld_unbind(bus);
+		if (ret && !err)
+			err = ret;
 		ret = device_remove(bus);
 		if (ret && !err)
 			err = ret;
@@ -166,6 +173,7 @@ int usb_stop(void)
 
 	return err;
 }
+#endif
 
 static void usb_scan_bus(struct udevice *bus, bool recurse)
 {
@@ -491,6 +499,7 @@ static int usb_find_child(struct udevice *parent,
 			  struct usb_interface_descriptor *iface,
 			  struct udevice **devp)
 {
+#ifdef CONFIG_USB_EMUL /* Emulated devices are explictily bound */
 	struct udevice *dev;
 
 	*devp = NULL;
@@ -509,6 +518,7 @@ static int usb_find_child(struct udevice *parent,
 			return 0;
 		}
 	}
+#endif
 
 	return -ENOENT;
 }
-- 
2.4.3

[U-Boot] [PATCH 09/22] dm: usb: Allow usb host drivers to implement usb_reset_root_port

[Hans de Goede]

Allow usb uclass host drivers to implement usb_reset_root_port, this is
used by single port usb hosts which do not emulate a hub, such as otg
controllers.

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

diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index 8f26e35..afa8655 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -128,6 +128,17 @@ int usb_alloc_device(struct usb_device *udev)
 	return ops->alloc_device(bus, udev);
 }
 
+int usb_reset_root_port(struct usb_device *udev)
+{
+	struct udevice *bus = udev->controller_dev;
+	struct dm_usb_ops *ops = usb_get_ops(bus);
+
+	if (!ops->reset_root_port)
+		return -ENOSYS;
+
+	return ops->reset_root_port(bus, udev);
+}
+
 #ifdef CONFIG_DM_DEVICE_REMOVE
 int usb_stop(void)
 {
@@ -273,11 +284,6 @@ int usb_init(void)
 	return usb_started ? 0 : -1;
 }
 
-int usb_reset_root_port(struct usb_device *udev)
-{
-	return -ENOSYS;
-}
-
 static struct usb_device *find_child_devnum(struct udevice *parent, int devnum)
 {
 	struct usb_device *udev;
diff --git a/include/usb.h b/include/usb.h
index 2bb6a06..25f8543 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -705,6 +705,11 @@ struct dm_usb_ops {
 	 * is read). This should be NULL for EHCI, which does not need this.
 	 */
 	int (*alloc_device)(struct udevice *bus, struct usb_device *udev);
+
+	/**
+	 * reset_root_port() - Reset usb root port
+	 */
+	int (*reset_root_port)(struct udevice *bus, struct usb_device *udev);
 };
 
 #define usb_get_ops(dev)	((struct dm_usb_ops *)(dev)->driver->ops)
-- 
2.4.3

[U-Boot] [PATCH 10/22] dm: usb: Do not assume that first child is always a hub

[Hans de Goede]

On some single port (otg) controllers there is no emulated root hub, so
the first child (if any) may be one of: UCLASS_MASS_STORAGE,
UCLASS_USB_DEV_GENERIC or UCLASS_USB_HUB.

All three of these (and in the future others) are suitable for our
purposes, remove the check for the device being a hub, and add a check to
deal with the fact that there may be no child-dev.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 common/cmd_usb.c              |  9 ++++-----
 drivers/usb/host/usb-uclass.c | 10 +++++-----
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/common/cmd_usb.c b/common/cmd_usb.c
index ca06826..2ed4cb4 100644
--- a/common/cmd_usb.c
+++ b/common/cmd_usb.c
@@ -630,12 +630,11 @@ static int do_usb(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		     bus;
 		     uclass_next_device(&bus)) {
 			struct usb_device *udev;
-			struct udevice *hub;
+			struct udevice *dev;
 
-			device_find_first_child(bus, &hub);
-			if (device_get_uclass_id(hub) == UCLASS_USB_HUB &&
-			    device_active(hub)) {
-				udev = dev_get_parentdata(hub);
+			device_find_first_child(bus, &dev);
+			if (dev && device_active(dev)) {
+				udev = dev_get_parentdata(dev);
 				usb_show_tree(udev);
 			}
 		}
diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index afa8655..5fcbde9 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -308,14 +308,14 @@ static struct usb_device *find_child_devnum(struct udevice *parent, int devnum)
 
 struct usb_device *usb_get_dev_index(struct udevice *bus, int index)
 {
-	struct udevice *hub;
+	struct udevice *dev;
 	int devnum = index + 1; /* Addresses are allocated from 1 on USB */
 
-	device_find_first_child(bus, &hub);
-	if (device_get_uclass_id(hub) == UCLASS_USB_HUB)
-		return find_child_devnum(hub, devnum);
+	device_find_first_child(bus, &dev);
+	if (!dev)
+		return NULL;
 
-	return NULL;
+	return find_child_devnum(dev, devnum);
 }
 
 int usb_post_bind(struct udevice *dev)
-- 
2.4.3

[U-Boot] [PATCH 11/22] musb: Allow musb_platform_enable to return an error code

[Hans de Goede]

Allow musb_platform_enable to return an error code and propagate it up to
usb_lowlevel_init().

This allows moving the checks for an external vbus being present to be
moved from platform_init to platform_enable, so that the user can unplug a
charger, plug in a host adapter with a usb-device, do a "usb reset" and
have things working.

This also allows adding a check for the id-pin to platform_enable, so that
it can short circuit the 1s delay in usb_lowlevel_init() when no host cable
is plugged in and thus waiting for a device to show up is useless.

Note that all the changes to code shared with the kernel are wrapped in
the kernel.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/usb/musb-new/am35x.c      |  7 +++++++
 drivers/usb/musb-new/musb_core.c  | 20 ++++++++++++++++++++
 drivers/usb/musb-new/musb_core.h  | 18 ++++++++++++++++++
 drivers/usb/musb-new/musb_dsps.c  |  6 ++++++
 drivers/usb/musb-new/musb_uboot.c |  6 +++++-
 drivers/usb/musb-new/omap2430.c   |  5 +++++
 drivers/usb/musb-new/sunxi.c      |  5 +++--
 7 files changed, 64 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/musb-new/am35x.c b/drivers/usb/musb-new/am35x.c
index 857d7eb..d158454 100644
--- a/drivers/usb/musb-new/am35x.c
+++ b/drivers/usb/musb-new/am35x.c
@@ -100,7 +100,11 @@ struct am35x_glue {
 /*
  * am35x_musb_enable - enable interrupts
  */
+#ifndef __UBOOT__
 static void am35x_musb_enable(struct musb *musb)
+#else
+static int am35x_musb_enable(struct musb *musb)
+#endif
 {
 	void __iomem *reg_base = musb->ctrl_base;
 	u32 epmask;
@@ -116,6 +120,9 @@ static void am35x_musb_enable(struct musb *musb)
 	if (is_otg_enabled(musb))
 		musb_writel(reg_base, CORE_INTR_SRC_SET_REG,
 			    AM35X_INTR_DRVVBUS << AM35X_INTR_USB_SHIFT);
+#ifdef __UBOOT__
+	return 0;
+#endif
 }
 
 /*
diff --git a/drivers/usb/musb-new/musb_core.c b/drivers/usb/musb-new/musb_core.c
index 242cc30..f530af4 100644
--- a/drivers/usb/musb-new/musb_core.c
+++ b/drivers/usb/musb-new/musb_core.c
@@ -926,10 +926,17 @@ b_host:
 /*
 * Program the HDRC to start (enable interrupts, dma, etc.).
 */
+#ifndef __UBOOT__
 void musb_start(struct musb *musb)
+#else
+int musb_start(struct musb *musb)
+#endif
 {
 	void __iomem	*regs = musb->mregs;
 	u8		devctl = musb_readb(regs, MUSB_DEVCTL);
+#ifdef __UBOOT__
+	int ret;
+#endif
 
 	dev_dbg(musb->controller, "<== devctl %02x\n", devctl);
 
@@ -972,8 +979,21 @@ void musb_start(struct musb *musb)
 		if ((devctl & MUSB_DEVCTL_VBUS) == MUSB_DEVCTL_VBUS)
 			musb->is_active = 1;
 	}
+
+#ifndef __UBOOT__
 	musb_platform_enable(musb);
+#else
+	ret = musb_platform_enable(musb);
+	if (ret) {
+		musb->is_active = 0;
+		return ret;
+	}
+#endif
 	musb_writeb(regs, MUSB_DEVCTL, devctl);
+
+#ifdef __UBOOT__
+	return 0;
+#endif
 }
 
 
diff --git a/drivers/usb/musb-new/musb_core.h b/drivers/usb/musb-new/musb_core.h
index 2695742..8727f64 100644
--- a/drivers/usb/musb-new/musb_core.h
+++ b/drivers/usb/musb-new/musb_core.h
@@ -231,7 +231,11 @@ struct musb_platform_ops {
 	int	(*init)(struct musb *musb);
 	int	(*exit)(struct musb *musb);
 
+#ifndef __UBOOT__
 	void	(*enable)(struct musb *musb);
+#else
+	int	(*enable)(struct musb *musb);
+#endif
 	void	(*disable)(struct musb *musb);
 
 	int	(*set_mode)(struct musb *musb, u8 mode);
@@ -546,7 +550,11 @@ static inline void musb_configure_ep0(struct musb *musb)
 
 extern const char musb_driver_name[];
 
+#ifndef __UBOOT__
 extern void musb_start(struct musb *musb);
+#else
+extern int musb_start(struct musb *musb);
+#endif
 extern void musb_stop(struct musb *musb);
 
 extern void musb_write_fifo(struct musb_hw_ep *ep, u16 len, const u8 *src);
@@ -564,11 +572,21 @@ static inline void musb_platform_set_vbus(struct musb *musb, int is_on)
 		musb->ops->set_vbus(musb, is_on);
 }
 
+#ifndef __UBOOT__
 static inline void musb_platform_enable(struct musb *musb)
 {
 	if (musb->ops->enable)
 		musb->ops->enable(musb);
 }
+#else
+static inline int musb_platform_enable(struct musb *musb)
+{
+	if (!musb->ops->enable)
+		return 0;
+
+	return musb->ops->enable(musb);
+}
+#endif
 
 static inline void musb_platform_disable(struct musb *musb)
 {
diff --git a/drivers/usb/musb-new/musb_dsps.c b/drivers/usb/musb-new/musb_dsps.c
index 17ed224..8959397 100644
--- a/drivers/usb/musb-new/musb_dsps.c
+++ b/drivers/usb/musb-new/musb_dsps.c
@@ -156,7 +156,11 @@ struct dsps_glue {
 /**
  * dsps_musb_enable - enable interrupts
  */
+#ifndef __UBOOT__
 static void dsps_musb_enable(struct musb *musb)
+#else
+static int dsps_musb_enable(struct musb *musb)
+#endif
 {
 #ifndef __UBOOT__
 	struct device *dev = musb->controller;
@@ -181,6 +185,8 @@ static void dsps_musb_enable(struct musb *musb)
 	if (is_otg_enabled(musb))
 		dsps_writel(reg_base, wrp->coreintr_set,
 			    (1 << wrp->drvvbus) << wrp->usb_shift);
+#else
+	return 0;
 #endif
 }
 
diff --git a/drivers/usb/musb-new/musb_uboot.c b/drivers/usb/musb-new/musb_uboot.c
index 1bf676c..70e87c9 100644
--- a/drivers/usb/musb-new/musb_uboot.c
+++ b/drivers/usb/musb-new/musb_uboot.c
@@ -217,13 +217,17 @@ int usb_lowlevel_init(int index, enum usb_init_type init, void **controller)
 	void *mbase;
 	/* USB spec says it may take up to 1 second for a device to connect */
 	unsigned long timeout = get_timer(0) + 1000;
+	int ret;
 
 	if (!host) {
 		printf("MUSB host is not registered\n");
 		return -ENODEV;
 	}
 
-	musb_start(host);
+	ret = musb_start(host);
+	if (ret)
+		return ret;
+
 	mbase = host->mregs;
 	do {
 		if (musb_readb(mbase, MUSB_DEVCTL) & MUSB_DEVCTL_HM)
diff --git a/drivers/usb/musb-new/omap2430.c b/drivers/usb/musb-new/omap2430.c
index 31a280e..77273a4 100644
--- a/drivers/usb/musb-new/omap2430.c
+++ b/drivers/usb/musb-new/omap2430.c
@@ -400,7 +400,11 @@ err1:
 	return status;
 }
 
+#ifndef __UBOOT__
 static void omap2430_musb_enable(struct musb *musb)
+#else
+static int omap2430_musb_enable(struct musb *musb)
+#endif
 {
 #ifndef __UBOOT__
 	u8		devctl;
@@ -445,6 +449,7 @@ static void omap2430_musb_enable(struct musb *musb)
 				__PRETTY_FUNCTION__);
 	}
 #endif
+	return 0;
 #endif
 }
 
diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c
index 052e065..c123d61 100644
--- a/drivers/usb/musb-new/sunxi.c
+++ b/drivers/usb/musb-new/sunxi.c
@@ -199,12 +199,12 @@ static irqreturn_t sunxi_musb_interrupt(int irq, void *__hci)
 /* musb_core does not call enable / disable in a balanced manner <sigh> */
 static bool enabled = false;
 
-static void sunxi_musb_enable(struct musb *musb)
+static int sunxi_musb_enable(struct musb *musb)
 {
 	pr_debug("%s():\n", __func__);
 
 	if (enabled)
-		return;
+		return 0;
 
 	/* select PIO mode */
 	musb_writeb(musb->mregs, USBC_REG_o_VEND0, 0);
@@ -215,6 +215,7 @@ static void sunxi_musb_enable(struct musb *musb)
 	USBC_ForceVbusValidToHigh(musb->mregs);
 
 	enabled = true;
+	return 0;
 }
 
 static void sunxi_musb_disable(struct musb *musb)
-- 
2.4.3

[U-Boot] [PATCH 12/22] musb: Update usb-compat to work with struct usb_device without a parent ptr

[Hans de Goede]

When building with CONFIG_DM_USB=y struct usb_device does not have a parent
pointer. This commit adds support to the musb code to deal with this.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/usb/musb-new/musb_host.c  |  4 +++
 drivers/usb/musb-new/musb_uboot.c |  2 +-
 drivers/usb/musb-new/usb-compat.h | 70 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/musb-new/musb_host.c b/drivers/usb/musb-new/musb_host.c
index 437309c..40b9c66 100644
--- a/drivers/usb/musb-new/musb_host.c
+++ b/drivers/usb/musb-new/musb_host.c
@@ -2067,7 +2067,11 @@ int musb_urb_enqueue(
 
 	/* precompute addressing for external hub/tt ports */
 	if (musb->is_multipoint) {
+#ifndef __UBOOT__
 		struct usb_device	*parent = urb->dev->parent;
+#else
+		struct usb_device	*parent = usb_dev_get_parent(urb->dev);
+#endif
 
 #ifndef __UBOOT__
 		if (parent != hcd->self.root_hub) {
diff --git a/drivers/usb/musb-new/musb_uboot.c b/drivers/usb/musb-new/musb_uboot.c
index 70e87c9..a96e8d2 100644
--- a/drivers/usb/musb-new/musb_uboot.c
+++ b/drivers/usb/musb-new/musb_uboot.c
@@ -97,7 +97,7 @@ int submit_control_msg(struct usb_device *dev, unsigned long pipe,
 		      buffer, len, setup, 0);
 
 	/* Fix speed for non hub-attached devices */
-	if (!dev->parent)
+	if (!usb_dev_get_parent(dev))
 		dev->speed = host_speed;
 
 	return submit_urb(&hcd, &urb);
diff --git a/drivers/usb/musb-new/usb-compat.h b/drivers/usb/musb-new/usb-compat.h
index 50bad37..53fe4ff 100644
--- a/drivers/usb/musb-new/usb-compat.h
+++ b/drivers/usb/musb-new/usb-compat.h
@@ -1,6 +1,7 @@
 #ifndef __USB_COMPAT_H__
 #define __USB_COMPAT_H__
 
+#include <dm.h>
 #include "usb.h"
 
 struct usb_hcd {
@@ -66,6 +67,68 @@ static inline int usb_hcd_unmap_urb_for_dma(struct usb_hcd *hcd,
 	return 0;
 }
 
+#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_parentdata(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_parentdata(dev);
+		parent = dev->parent;
+		uparent = dev_get_parentdata(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;
+
+	/*
+	 * 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.
+	 *
+	 * If dev is an usb-bus, then we are called from usb_scan_device() for
+	 * an usb-device plugged directly into the root port, return NULL.
+	 */
+	if (device_get_uclass_id(udev->dev) == UCLASS_USB)
+		return NULL;
+
+	/*
+	 * If these 2 are not the same we are being called from
+	 * usb_scan_device() and udev itself is the parent.
+	 */
+	if (dev_get_parentdata(udev->dev) != udev)
+		return udev;
+
+	/* We are being called normally, use the parent pointer */
+	if (device_get_uclass_id(parent) == UCLASS_USB_HUB)
+		return dev_get_parentdata(parent);
+
+	return NULL;
+}
+#else
 static inline u16 find_tt(struct usb_device *dev)
 {
 	u8 chid;
@@ -86,4 +149,11 @@ static inline u16 find_tt(struct usb_device *dev)
 
 	return (hub << 8) | chid;
 }
+
+static inline struct usb_device *usb_dev_get_parent(struct usb_device *dev)
+{
+	return dev->parent;
+}
+#endif
+
 #endif /* __USB_COMPAT_H__ */
-- 
2.4.3

[U-Boot] [PATCH 13/22] musb: Rename and wrap public functions

[Hans de Goede]

Rename and wrap the usb host API public functions, this is a preparation
patch for adding device-model support.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/usb/musb-new/musb_uboot.c | 70 +++++++++++++++++++++++++++++++++------
 1 file changed, 59 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/musb-new/musb_uboot.c b/drivers/usb/musb-new/musb_uboot.c
index a96e8d2..10c7cc4 100644
--- a/drivers/usb/musb-new/musb_uboot.c
+++ b/drivers/usb/musb-new/musb_uboot.c
@@ -90,7 +90,7 @@ static int submit_urb(struct usb_hcd *hcd, struct urb *urb)
 	return urb->status;
 }
 
-int submit_control_msg(struct usb_device *dev, unsigned long pipe,
+static int _musb_submit_control_msg(struct usb_device *dev, unsigned long pipe,
 			void *buffer, int len, struct devrequest *setup)
 {
 	construct_urb(&urb, &hep, dev, USB_ENDPOINT_XFER_CONTROL, pipe,
@@ -103,8 +103,7 @@ int submit_control_msg(struct usb_device *dev, unsigned long pipe,
 	return submit_urb(&hcd, &urb);
 }
 
-
-int submit_bulk_msg(struct usb_device *dev, unsigned long pipe,
+static int _musb_submit_bulk_msg(struct usb_device *dev, unsigned long pipe,
 					void *buffer, int len)
 {
 	construct_urb(&urb, &hep, dev, USB_ENDPOINT_XFER_BULK, pipe,
@@ -112,7 +111,7 @@ int submit_bulk_msg(struct usb_device *dev, unsigned long pipe,
 	return submit_urb(&hcd, &urb);
 }
 
-int submit_int_msg(struct usb_device *dev, unsigned long pipe,
+static int _musb_submit_int_msg(struct usb_device *dev, unsigned long pipe,
 				void *buffer, int len, int interval)
 {
 	construct_urb(&urb, &hep, dev, USB_ENDPOINT_XFER_INT, pipe,
@@ -120,8 +119,9 @@ int submit_int_msg(struct usb_device *dev, unsigned long pipe,
 	return submit_urb(&hcd, &urb);
 }
 
-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 *_musb_create_int_queue(struct usb_device *dev,
+			unsigned long pipe, int queuesize, int elementsize,
+			void *buffer, int interval)
 {
 	struct int_queue *queue;
 	int ret, index = usb_pipein(pipe) * 16 + usb_pipeendpoint(pipe);
@@ -154,7 +154,8 @@ struct int_queue *create_int_queue(struct usb_device *dev, unsigned long pipe,
 	return queue;
 }
 
-int destroy_int_queue(struct usb_device *dev, struct int_queue *queue)
+static int _musb_destroy_int_queue(struct usb_device *dev,
+				   struct int_queue *queue)
 {
 	int index = usb_pipein(queue->urb.pipe) * 16 + 
 		    usb_pipeendpoint(queue->urb.pipe);
@@ -167,7 +168,8 @@ int destroy_int_queue(struct usb_device *dev, struct int_queue *queue)
 	return 0;
 }
 
-void *poll_int_queue(struct usb_device *dev, struct int_queue *queue)
+static void *_musb_poll_int_queue(struct usb_device *dev,
+				  struct int_queue *queue)
 {
 	if (queue->urb.status != -EINPROGRESS)
 		return NULL; /* URB has already completed in a prev. poll */
@@ -180,7 +182,7 @@ void *poll_int_queue(struct usb_device *dev, struct int_queue *queue)
 	return NULL; /* URB still pending */
 }
 
-int usb_reset_root_port(struct usb_device *dev)
+static int _musb_reset_root_port(struct usb_device *dev)
 {
 	void *mbase = host->mregs;
 	u8 power;
@@ -212,7 +214,7 @@ int usb_reset_root_port(struct usb_device *dev)
 	return 0;
 }
 
-int usb_lowlevel_init(int index, enum usb_init_type init, void **controller)
+int musb_lowlevel_init(void)
 {
 	void *mbase;
 	/* USB spec says it may take up to 1 second for a device to connect */
@@ -236,7 +238,7 @@ int usb_lowlevel_init(int index, enum usb_init_type init, void **controller)
 	if (get_timer(0) >= timeout)
 		return -ENODEV;
 
-	usb_reset_root_port(NULL);
+	_musb_reset_root_port(NULL);
 	host->is_active = 1;
 	hcd.hcd_priv = host;
 
@@ -253,6 +255,52 @@ int usb_lowlevel_stop(int index)
 	musb_stop(host);
 	return 0;
 }
+
+int submit_bulk_msg(struct usb_device *dev, unsigned long pipe,
+			    void *buffer, int length)
+{
+	return _musb_submit_bulk_msg(dev, pipe, buffer, length);
+}
+
+int submit_control_msg(struct usb_device *dev, unsigned long pipe,
+		       void *buffer, int length, struct devrequest *setup)
+{
+	return _musb_submit_control_msg(dev, pipe, buffer, length, setup);
+}
+
+int submit_int_msg(struct usb_device *dev, unsigned long pipe,
+		   void *buffer, int length, int interval)
+{
+	return _musb_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 _musb_create_int_queue(dev, pipe, queuesize, elementsize,
+				      buffer, interval);
+}
+
+void *poll_int_queue(struct usb_device *dev, struct int_queue *queue)
+{
+	return _musb_poll_int_queue(dev, queue);
+}
+
+int destroy_int_queue(struct usb_device *dev, struct int_queue *queue)
+{
+	return _musb_destroy_int_queue(dev, queue);
+}
+
+int usb_reset_root_port(struct usb_device *dev)
+{
+	return _musb_reset_root_port(dev);
+}
+
+int usb_lowlevel_init(int index, enum usb_init_type init, void **controller)
+{
+	return musb_lowlevel_init();
+}
 #endif /* CONFIG_MUSB_HOST */
 
 #ifdef CONFIG_MUSB_GADGET
-- 
2.4.3

[U-Boot] [PATCH 14/22] musb: Add musb_host_data struct to hold global data

[Hans de Goede]

Add a musb_host_data struct to hold all the global data host related musb
data. This is a preparation patch for adding device-model support.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/usb/musb-new/musb_uboot.c | 107 +++++++++++++++++++-------------------
 drivers/usb/musb-new/musb_uboot.h |  24 +++++++++
 2 files changed, 77 insertions(+), 54 deletions(-)
 create mode 100644 drivers/usb/musb-new/musb_uboot.h

diff --git a/drivers/usb/musb-new/musb_uboot.c b/drivers/usb/musb-new/musb_uboot.c
index 10c7cc4..9875100 100644
--- a/drivers/usb/musb-new/musb_uboot.c
+++ b/drivers/usb/musb-new/musb_uboot.c
@@ -13,6 +13,7 @@
 #include "musb_core.h"
 #include "musb_host.h"
 #include "musb_gadget.h"
+#include "musb_uboot.h"
 
 #ifdef CONFIG_MUSB_HOST
 struct int_queue {
@@ -20,9 +21,7 @@ struct int_queue {
 	struct urb urb;
 };
 
-static struct musb *host;
-static struct usb_hcd hcd;
-static enum usb_device_speed host_speed;
+struct musb_host_data musb_host;
 
 static void musb_host_complete_urb(struct urb *urb)
 {
@@ -30,9 +29,6 @@ static void musb_host_complete_urb(struct urb *urb)
 	urb->dev->act_len = urb->actual_length;
 }
 
-static struct usb_host_endpoint hep;
-static struct urb urb;
-
 static void construct_urb(struct urb *urb, struct usb_host_endpoint *hep,
 			  struct usb_device *dev, int endpoint_type,
 			  unsigned long pipe, void *buffer, int len,
@@ -90,38 +86,40 @@ static int submit_urb(struct usb_hcd *hcd, struct urb *urb)
 	return urb->status;
 }
 
-static int _musb_submit_control_msg(struct usb_device *dev, unsigned long pipe,
-			void *buffer, int len, struct devrequest *setup)
+static int _musb_submit_control_msg(struct musb_host_data *host,
+	struct usb_device *dev, unsigned long pipe,
+	void *buffer, int len, struct devrequest *setup)
 {
-	construct_urb(&urb, &hep, dev, USB_ENDPOINT_XFER_CONTROL, pipe,
-		      buffer, len, setup, 0);
+	construct_urb(&host->urb, &host->hep, dev, USB_ENDPOINT_XFER_CONTROL,
+		      pipe, buffer, len, setup, 0);
 
 	/* Fix speed for non hub-attached devices */
 	if (!usb_dev_get_parent(dev))
-		dev->speed = host_speed;
+		dev->speed = host->host_speed;
 
-	return submit_urb(&hcd, &urb);
+	return submit_urb(&host->hcd, &host->urb);
 }
 
-static int _musb_submit_bulk_msg(struct usb_device *dev, unsigned long pipe,
-					void *buffer, int len)
+static int _musb_submit_bulk_msg(struct musb_host_data *host,
+	struct usb_device *dev, unsigned long pipe, void *buffer, int len)
 {
-	construct_urb(&urb, &hep, dev, USB_ENDPOINT_XFER_BULK, pipe,
-		      buffer, len, NULL, 0);
-	return submit_urb(&hcd, &urb);
+	construct_urb(&host->urb, &host->hep, dev, USB_ENDPOINT_XFER_BULK,
+		      pipe, buffer, len, NULL, 0);
+	return submit_urb(&host->hcd, &host->urb);
 }
 
-static int _musb_submit_int_msg(struct usb_device *dev, unsigned long pipe,
-				void *buffer, int len, int interval)
+static int _musb_submit_int_msg(struct musb_host_data *host,
+	struct usb_device *dev, unsigned long pipe,
+	void *buffer, int len, int interval)
 {
-	construct_urb(&urb, &hep, dev, USB_ENDPOINT_XFER_INT, pipe,
+	construct_urb(&host->urb, &host->hep, dev, USB_ENDPOINT_XFER_INT, pipe,
 		      buffer, len, NULL, interval);
-	return submit_urb(&hcd, &urb);
+	return submit_urb(&host->hcd, &host->urb);
 }
 
-static struct int_queue *_musb_create_int_queue(struct usb_device *dev,
-			unsigned long pipe, int queuesize, int elementsize,
-			void *buffer, int interval)
+static struct int_queue *_musb_create_int_queue(struct musb_host_data *host,
+	struct usb_device *dev, unsigned long pipe, int queuesize,
+	int elementsize, void *buffer, int interval)
 {
 	struct int_queue *queue;
 	int ret, index = usb_pipein(pipe) * 16 + usb_pipeendpoint(pipe);
@@ -143,7 +141,7 @@ static struct int_queue *_musb_create_int_queue(struct usb_device *dev,
 	construct_urb(&queue->urb, &queue->hep, dev, USB_ENDPOINT_XFER_INT,
 		      pipe, buffer, elementsize, NULL, interval);
 
-	ret = musb_urb_enqueue(&hcd, &queue->urb, 0);
+	ret = musb_urb_enqueue(&host->hcd, &queue->urb, 0);
 	if (ret < 0) {
 		printf("Failed to enqueue URB to controller\n");
 		free(queue);
@@ -154,27 +152,27 @@ static struct int_queue *_musb_create_int_queue(struct usb_device *dev,
 	return queue;
 }
 
-static int _musb_destroy_int_queue(struct usb_device *dev,
-				   struct int_queue *queue)
+static int _musb_destroy_int_queue(struct musb_host_data *host,
+	struct usb_device *dev, struct int_queue *queue)
 {
 	int index = usb_pipein(queue->urb.pipe) * 16 + 
 		    usb_pipeendpoint(queue->urb.pipe);
 
 	if (queue->urb.status == -EINPROGRESS)
-		musb_urb_dequeue(&hcd, &queue->urb, -ETIME);
+		musb_urb_dequeue(&host->hcd, &queue->urb, -ETIME);
 
 	dev->int_pending &= ~(1 << index);
 	free(queue);
 	return 0;
 }
 
-static void *_musb_poll_int_queue(struct usb_device *dev,
-				  struct int_queue *queue)
+static void *_musb_poll_int_queue(struct musb_host_data *host,
+	struct usb_device *dev, struct int_queue *queue)
 {
 	if (queue->urb.status != -EINPROGRESS)
 		return NULL; /* URB has already completed in a prev. poll */
 
-	host->isr(0, host);
+	host->host->isr(0, host->host);
 
 	if (queue->urb.status != -EINPROGRESS)
 		return queue->urb.transfer_buffer; /* Done */
@@ -182,9 +180,10 @@ static void *_musb_poll_int_queue(struct usb_device *dev,
 	return NULL; /* URB still pending */
 }
 
-static int _musb_reset_root_port(struct usb_device *dev)
+static int _musb_reset_root_port(struct musb_host_data *host,
+	struct usb_device *dev)
 {
-	void *mbase = host->mregs;
+	void *mbase = host->host->mregs;
 	u8 power;
 
 	power = musb_readb(mbase, MUSB_POWER);
@@ -204,33 +203,33 @@ static int _musb_reset_root_port(struct usb_device *dev)
 #ifdef CONFIG_ARCH_SUNXI
 	sunxi_usb_phy_enable_squelch_detect(0, 1);
 #endif
-	host->isr(0, host);
-	host_speed = (musb_readb(mbase, MUSB_POWER) & MUSB_POWER_HSMODE) ?
+	host->host->isr(0, host->host);
+	host->host_speed = (musb_readb(mbase, MUSB_POWER) & MUSB_POWER_HSMODE) ?
 			USB_SPEED_HIGH :
 			(musb_readb(mbase, MUSB_DEVCTL) & MUSB_DEVCTL_FSDEV) ?
 			USB_SPEED_FULL : USB_SPEED_LOW;
-	mdelay((host_speed == USB_SPEED_LOW) ? 200 : 50);
+	mdelay((host->host_speed == USB_SPEED_LOW) ? 200 : 50);
 
 	return 0;
 }
 
-int musb_lowlevel_init(void)
+int musb_lowlevel_init(struct musb_host_data *host)
 {
 	void *mbase;
 	/* USB spec says it may take up to 1 second for a device to connect */
 	unsigned long timeout = get_timer(0) + 1000;
 	int ret;
 
-	if (!host) {
+	if (!host->host) {
 		printf("MUSB host is not registered\n");
 		return -ENODEV;
 	}
 
-	ret = musb_start(host);
+	ret = musb_start(host->host);
 	if (ret)
 		return ret;
 
-	mbase = host->mregs;
+	mbase = host->host->mregs;
 	do {
 		if (musb_readb(mbase, MUSB_DEVCTL) & MUSB_DEVCTL_HM)
 			break;
@@ -238,68 +237,68 @@ int musb_lowlevel_init(void)
 	if (get_timer(0) >= timeout)
 		return -ENODEV;
 
-	_musb_reset_root_port(NULL);
-	host->is_active = 1;
-	hcd.hcd_priv = host;
+	_musb_reset_root_port(host, NULL);
+	host->host->is_active = 1;
+	host->hcd.hcd_priv = host->host;
 
 	return 0;
 }
 
 int usb_lowlevel_stop(int index)
 {
-	if (!host) {
+	if (!musb_host.host) {
 		printf("MUSB host is not registered\n");
 		return -ENODEV;
 	}
 
-	musb_stop(host);
+	musb_stop(musb_host.host);
 	return 0;
 }
 
 int submit_bulk_msg(struct usb_device *dev, unsigned long pipe,
 			    void *buffer, int length)
 {
-	return _musb_submit_bulk_msg(dev, pipe, buffer, length);
+	return _musb_submit_bulk_msg(&musb_host, dev, pipe, buffer, length);
 }
 
 int submit_control_msg(struct usb_device *dev, unsigned long pipe,
 		       void *buffer, int length, struct devrequest *setup)
 {
-	return _musb_submit_control_msg(dev, pipe, buffer, length, setup);
+	return _musb_submit_control_msg(&musb_host, dev, pipe, buffer, length, setup);
 }
 
 int submit_int_msg(struct usb_device *dev, unsigned long pipe,
 		   void *buffer, int length, int interval)
 {
-	return _musb_submit_int_msg(dev, pipe, buffer, length, interval);
+	return _musb_submit_int_msg(&musb_host, 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 _musb_create_int_queue(dev, pipe, queuesize, elementsize,
+	return _musb_create_int_queue(&musb_host, dev, pipe, queuesize, elementsize,
 				      buffer, interval);
 }
 
 void *poll_int_queue(struct usb_device *dev, struct int_queue *queue)
 {
-	return _musb_poll_int_queue(dev, queue);
+	return _musb_poll_int_queue(&musb_host, dev, queue);
 }
 
 int destroy_int_queue(struct usb_device *dev, struct int_queue *queue)
 {
-	return _musb_destroy_int_queue(dev, queue);
+	return _musb_destroy_int_queue(&musb_host, dev, queue);
 }
 
 int usb_reset_root_port(struct usb_device *dev)
 {
-	return _musb_reset_root_port(dev);
+	return _musb_reset_root_port(&musb_host, dev);
 }
 
 int usb_lowlevel_init(int index, enum usb_init_type init, void **controller)
 {
-	return musb_lowlevel_init();
+	return musb_lowlevel_init(&musb_host);
 }
 #endif /* CONFIG_MUSB_HOST */
 
@@ -363,7 +362,7 @@ int musb_register(struct musb_hdrc_platform_data *plat, void *bdata,
 	switch (plat->mode) {
 #ifdef CONFIG_MUSB_HOST
 	case MUSB_HOST:
-		musbp = &host;
+		musbp = &musb_host.host;
 		break;
 #endif
 #ifdef CONFIG_MUSB_GADGET
diff --git a/drivers/usb/musb-new/musb_uboot.h b/drivers/usb/musb-new/musb_uboot.h
new file mode 100644
index 0000000..69b7977
--- /dev/null
+++ b/drivers/usb/musb-new/musb_uboot.h
@@ -0,0 +1,24 @@
+/*
+ * MUSB OTG driver u-boot specific functions
+ *
+ * Copyright ? 2015 Hans de Goede <hdegoede@redhat.com>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+#ifndef __MUSB_UBOOT_H__
+#define __MUSB_UBOOT_H__
+
+#include <usb.h>
+#include "linux-compat.h"
+#include "usb-compat.h"
+#include "musb_core.h"
+
+struct musb_host_data {
+	struct musb *host;
+	struct usb_hcd hcd;
+	enum usb_device_speed host_speed;
+	struct usb_host_endpoint hep;
+	struct urb urb;
+};
+
+#endif
-- 
2.4.3

[U-Boot] [PATCH 15/22] musb: Add device-model support to the musb-host u-boot glue

[Hans de Goede]

Add device-model support to the musb-host u-boot glue, note this only
adds device-model support to the musb-core glue code, it does not add
support for device-model to any of the SoC specific musb glue code.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/usb/musb-new/musb_uboot.c | 70 ++++++++++++++++++++++++++++++++++++++-
 drivers/usb/musb-new/musb_uboot.h |  4 +++
 2 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/musb-new/musb_uboot.c b/drivers/usb/musb-new/musb_uboot.c
index 9875100..9b56e90 100644
--- a/drivers/usb/musb-new/musb_uboot.c
+++ b/drivers/usb/musb-new/musb_uboot.c
@@ -21,7 +21,9 @@ struct int_queue {
 	struct urb urb;
 };
 
+#ifndef CONFIG_DM_USB
 struct musb_host_data musb_host;
+#endif
 
 static void musb_host_complete_urb(struct urb *urb)
 {
@@ -244,6 +246,7 @@ int musb_lowlevel_init(struct musb_host_data *host)
 	return 0;
 }
 
+#ifndef CONFIG_DM_USB
 int usb_lowlevel_stop(int index)
 {
 	if (!musb_host.host) {
@@ -300,6 +303,71 @@ int usb_lowlevel_init(int index, enum usb_init_type init, void **controller)
 {
 	return musb_lowlevel_init(&musb_host);
 }
+#endif /* !CONFIG_DM_USB */
+
+#ifdef CONFIG_DM_USB
+static int musb_submit_control_msg(struct udevice *dev, struct usb_device *udev,
+				   unsigned long pipe, void *buffer, int length,
+				   struct devrequest *setup)
+{
+	struct musb_host_data *host = dev_get_priv(dev);
+	return _musb_submit_control_msg(host, udev, pipe, buffer, length, setup);
+}
+
+static int musb_submit_bulk_msg(struct udevice *dev, struct usb_device *udev,
+				unsigned long pipe, void *buffer, int length)
+{
+	struct musb_host_data *host = dev_get_priv(dev);
+	return _musb_submit_bulk_msg(host, udev, pipe, buffer, length);
+}
+
+static int musb_submit_int_msg(struct udevice *dev, struct usb_device *udev,
+			       unsigned long pipe, void *buffer, int length,
+			       int interval)
+{
+	struct musb_host_data *host = dev_get_priv(dev);
+	return _musb_submit_int_msg(host, udev, pipe, buffer, length, interval);
+}
+
+static struct int_queue *musb_create_int_queue(struct udevice *dev,
+		struct usb_device *udev, unsigned long pipe, int queuesize,
+		int elementsize, void *buffer, int interval)
+{
+	struct musb_host_data *host = dev_get_priv(dev);
+	return _musb_create_int_queue(host, udev, pipe, queuesize, elementsize,
+				      buffer, interval);
+}
+
+static void *musb_poll_int_queue(struct udevice *dev, struct usb_device *udev,
+				 struct int_queue *queue)
+{
+	struct musb_host_data *host = dev_get_priv(dev);
+	return _musb_poll_int_queue(host, udev, queue);
+}
+
+static int musb_destroy_int_queue(struct udevice *dev, struct usb_device *udev,
+				  struct int_queue *queue)
+{
+	struct musb_host_data *host = dev_get_priv(dev);
+	return _musb_destroy_int_queue(host, udev, queue);
+}
+
+static int musb_reset_root_port(struct udevice *dev, struct usb_device *udev)
+{
+	struct musb_host_data *host = dev_get_priv(dev);
+	return _musb_reset_root_port(host, udev);
+}
+
+struct dm_usb_ops musb_usb_ops = {
+	.control = musb_submit_control_msg,
+	.bulk = musb_submit_bulk_msg,
+	.interrupt = musb_submit_int_msg,
+	.create_int_queue = musb_create_int_queue,
+	.poll_int_queue = musb_poll_int_queue,
+	.destroy_int_queue = musb_destroy_int_queue,
+	.reset_root_port = musb_reset_root_port,
+};
+#endif /* CONFIG_DM_USB */
 #endif /* CONFIG_MUSB_HOST */
 
 #ifdef CONFIG_MUSB_GADGET
@@ -360,7 +428,7 @@ int musb_register(struct musb_hdrc_platform_data *plat, void *bdata,
 	struct musb **musbp;
 
 	switch (plat->mode) {
-#ifdef CONFIG_MUSB_HOST
+#if defined(CONFIG_MUSB_HOST) && !defined(CONFIG_DM_USB)
 	case MUSB_HOST:
 		musbp = &musb_host.host;
 		break;
diff --git a/drivers/usb/musb-new/musb_uboot.h b/drivers/usb/musb-new/musb_uboot.h
index 69b7977..6312cd2 100644
--- a/drivers/usb/musb-new/musb_uboot.h
+++ b/drivers/usb/musb-new/musb_uboot.h
@@ -21,4 +21,8 @@ struct musb_host_data {
 	struct urb urb;
 };
 
+extern struct dm_usb_ops musb_usb_ops;
+
+int musb_lowlevel_init(struct musb_host_data *host);
+
 #endif
-- 
2.4.3

[U-Boot] [PATCH 16/22] sunxi: usb-phy: Add support for reading otg id pin value

[Hans de Goede]

Add support for reading the id pin value of the otg connector to the usb
phy code.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 arch/arm/cpu/armv7/sunxi/usb_phy.c        | 34 +++++++++++++++++++++++++++++++
 arch/arm/include/asm/arch-sunxi/usb_phy.h |  1 +
 board/sunxi/Kconfig                       |  7 +++++++
 3 files changed, 42 insertions(+)

diff --git a/arch/arm/cpu/armv7/sunxi/usb_phy.c b/arch/arm/cpu/armv7/sunxi/usb_phy.c
index b07d67f..5e82ddc 100644
--- a/arch/arm/cpu/armv7/sunxi/usb_phy.c
+++ b/arch/arm/cpu/armv7/sunxi/usb_phy.c
@@ -44,6 +44,7 @@ static struct sunxi_usb_phy {
 	int usb_rst_mask;
 	int gpio_vbus;
 	int gpio_vbus_det;
+	int gpio_id_det;
 	int id;
 	int init_count;
 	int power_on_count;
@@ -82,6 +83,14 @@ static int get_vbus_detect_gpio(int index)
 	return -EINVAL;
 }
 
+static int get_id_detect_gpio(int index)
+{
+	switch (index) {
+	case 0: return sunxi_name_to_gpio(CONFIG_USB0_ID_DET);
+	}
+	return -EINVAL;
+}
+
 static void usb_phy_write(struct sunxi_usb_phy *phy, int addr,
 			  int data, int len)
 {
@@ -247,6 +256,16 @@ int sunxi_usb_phy_vbus_detect(int index)
 	return err;
 }
 
+int sunxi_usb_phy_id_detect(int index)
+{
+	struct sunxi_usb_phy *phy = &sunxi_usb_phy[index];
+
+	if (phy->gpio_id_det < 0)
+		return phy->gpio_id_det;
+
+	return gpio_get_value(phy->gpio_id_det);
+}
+
 int sunxi_usb_phy_probe(void)
 {
 	struct sunxi_ccm_reg *ccm = (struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
@@ -275,6 +294,18 @@ int sunxi_usb_phy_probe(void)
 			if (ret)
 				return ret;
 		}
+
+		phy->gpio_id_det = get_id_detect_gpio(i);
+		if (phy->gpio_id_det >= 0) {
+			ret = gpio_request(phy->gpio_id_det, "usb_id_det");
+			if (ret)
+				return ret;
+			ret = gpio_direction_input(phy->gpio_id_det);
+			if (ret)
+				return ret;
+			sunxi_gpio_set_pull(phy->gpio_id_det,
+					    SUNXI_GPIO_PULL_UP);
+		}
 	}
 
 	setbits_le32(&ccm->usb_clk_cfg, CCM_USB_CTRL_PHYGATE);
@@ -298,6 +329,9 @@ int sunxi_usb_phy_remove(void)
 
 		if (phy->gpio_vbus_det >= 0)
 			gpio_free(phy->gpio_vbus_det);
+
+		if (phy->gpio_id_det >= 0)
+			gpio_free(phy->gpio_id_det);
 	}
 
 	return 0;
diff --git a/arch/arm/include/asm/arch-sunxi/usb_phy.h b/arch/arm/include/asm/arch-sunxi/usb_phy.h
index b7b831e..5a9cacb 100644
--- a/arch/arm/include/asm/arch-sunxi/usb_phy.h
+++ b/arch/arm/include/asm/arch-sunxi/usb_phy.h
@@ -17,4 +17,5 @@ void sunxi_usb_phy_exit(int index);
 void sunxi_usb_phy_power_on(int index);
 void sunxi_usb_phy_power_off(int index);
 int sunxi_usb_phy_vbus_detect(int index);
+int sunxi_usb_phy_id_detect(int index);
 void sunxi_usb_phy_enable_squelch_detect(int index, int enable);
diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
index b2eca51..4311c3e 100644
--- a/board/sunxi/Kconfig
+++ b/board/sunxi/Kconfig
@@ -306,6 +306,13 @@ config USB0_VBUS_DET
 	Set the Vbus detect pin for usb0 (otg). This takes a string in the
 	format understood by sunxi_name_to_gpio, e.g. PH1 for pin 1 of port H.
 
+config USB0_ID_DET
+	string "ID detect pin for usb0 (otg)"
+	default ""
+	---help---
+	Set the ID detect pin for usb0 (otg). This takes a string in the
+	format understood by sunxi_name_to_gpio, e.g. PH1 for pin 1 of port H.
+
 config USB1_VBUS_PIN
 	string "Vbus enable pin for usb1 (ehci0)"
 	default "PH6" if MACH_SUN4I || MACH_SUN7I
-- 
2.4.3

[U-Boot] [PATCH 17/22] sunxi: musb: Move vbus check to sunxi_musb_enable

[Hans de Goede]

This way it can be re-checked on "usb reset".

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/usb/musb-new/sunxi.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c
index c123d61..ee018c7 100644
--- a/drivers/usb/musb-new/sunxi.c
+++ b/drivers/usb/musb-new/sunxi.c
@@ -201,6 +201,8 @@ static bool enabled = false;
 
 static int sunxi_musb_enable(struct musb *musb)
 {
+	int ret;
+
 	pr_debug("%s():\n", __func__);
 
 	if (enabled)
@@ -209,8 +211,14 @@ static int sunxi_musb_enable(struct musb *musb)
 	/* select PIO mode */
 	musb_writeb(musb->mregs, USBC_REG_o_VEND0, 0);
 
-	if (is_host_enabled(musb))
+	if (is_host_enabled(musb)) {
+		ret = sunxi_usb_phy_vbus_detect(0);
+		if (ret) {
+			printf("A charger is plugged into the OTG: ");
+			return -ENODEV;
+		}
 		sunxi_usb_phy_power_on(0); /* port power on */
+	}
 
 	USBC_ForceVbusValidToHigh(musb->mregs);
 
@@ -237,18 +245,9 @@ static void sunxi_musb_disable(struct musb *musb)
 static int sunxi_musb_init(struct musb *musb)
 {
 	struct sunxi_ccm_reg *ccm = (struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
-	int err;
 
 	pr_debug("%s():\n", __func__);
 
-	if (is_host_enabled(musb)) {
-		err = sunxi_usb_phy_vbus_detect(0);
-		if (err) {
-			eprintf("Error: A charger is plugged into the OTG\n");
-			return -EIO;
-		}
-	}
-
 	musb->isr = sunxi_musb_interrupt;
 
 	setbits_le32(&ccm->ahb_gate0, 1 << AHB_GATE_OFFSET_USB0);
-- 
2.4.3

[U-Boot] [PATCH 18/22] sunxi: musb: Add id pin support

[Hans de Goede]

When in host mode check if there is a host cable inserted into the otg
port by checking the id pin. If there is no host cable return an error to
make usb_lowlevel_init() exit early, rather then waiting for 1 second
for a device which will never show up.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 configs/Ippo_q8h_v1_2_a33_1024x600_defconfig | 1 +
 configs/Ippo_q8h_v1_2_defconfig              | 1 +
 configs/Ippo_q8h_v5_defconfig                | 1 +
 drivers/usb/musb-new/sunxi.c                 | 5 +++++
 4 files changed, 8 insertions(+)

diff --git a/configs/Ippo_q8h_v1_2_a33_1024x600_defconfig b/configs/Ippo_q8h_v1_2_a33_1024x600_defconfig
index 5b1080f..51d75c9 100644
--- a/configs/Ippo_q8h_v1_2_a33_1024x600_defconfig
+++ b/configs/Ippo_q8h_v1_2_a33_1024x600_defconfig
@@ -5,6 +5,7 @@ CONFIG_DRAM_CLK=480
 CONFIG_DRAM_ZQ=15291
 CONFIG_USB0_VBUS_PIN="AXP0-VBUS-ENABLE"
 CONFIG_USB0_VBUS_DET="AXP0-VBUS-DETECT"
+CONFIG_USB0_ID_DET="PH8"
 CONFIG_AXP_GPIO=y
 CONFIG_VIDEO_LCD_MODE="x:1024,y:600,depth:18,pclk_khz:51000,le:159,ri:160,up:22,lo:12,hs:1,vs:1,sync:3,vmode:0"
 CONFIG_VIDEO_LCD_DCLK_PHASE=0
diff --git a/configs/Ippo_q8h_v1_2_defconfig b/configs/Ippo_q8h_v1_2_defconfig
index 8d03300..20fc311 100644
--- a/configs/Ippo_q8h_v1_2_defconfig
+++ b/configs/Ippo_q8h_v1_2_defconfig
@@ -5,6 +5,7 @@ CONFIG_DRAM_CLK=432
 CONFIG_DRAM_ZQ=63306
 CONFIG_USB0_VBUS_PIN="AXP0-VBUS-ENABLE"
 CONFIG_USB0_VBUS_DET="AXP0-VBUS-DETECT"
+CONFIG_USB0_ID_DET="PH8"
 CONFIG_AXP_GPIO=y
 CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:87,ri:167,up:31,lo:13,hs:1,vs:1,sync:3,vmode:0"
 CONFIG_VIDEO_LCD_DCLK_PHASE=0
diff --git a/configs/Ippo_q8h_v5_defconfig b/configs/Ippo_q8h_v5_defconfig
index 1a07064..3264a20 100644
--- a/configs/Ippo_q8h_v5_defconfig
+++ b/configs/Ippo_q8h_v5_defconfig
@@ -5,6 +5,7 @@ CONFIG_DRAM_CLK=480
 CONFIG_DRAM_ZQ=63351
 CONFIG_USB0_VBUS_PIN="AXP0-VBUS-ENABLE"
 CONFIG_USB0_VBUS_DET="AXP0-VBUS-DETECT"
+CONFIG_USB0_ID_DET="PH8"
 CONFIG_AXP_GPIO=y
 CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:87,ri:168,up:31,lo:13,hs:1,vs:1,sync:3,vmode:0"
 CONFIG_VIDEO_LCD_DCLK_PHASE=0
diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c
index ee018c7..cafb480 100644
--- a/drivers/usb/musb-new/sunxi.c
+++ b/drivers/usb/musb-new/sunxi.c
@@ -217,6 +217,11 @@ static int sunxi_musb_enable(struct musb *musb)
 			printf("A charger is plugged into the OTG: ");
 			return -ENODEV;
 		}
+		ret = sunxi_usb_phy_id_detect(0);
+		if (ret == 1) {
+			printf("No host cable detected: ");
+			return -ENODEV;
+		}
 		sunxi_usb_phy_power_on(0); /* port power on */
 	}
 
-- 
2.4.3

[U-Boot] [PATCH 19/22] sunxi: musb: Move musb config and platdata to the sunxi-musb glue

[Hans de Goede]

Move the musb config and platdata to the sunxi-musb glue, which is where
it really belongs. This is preparation patch for adding device-model
support for the sunxi-musb-host code.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 arch/arm/include/asm/arch-sunxi/usb_phy.h |  7 +++++++
 board/sunxi/board.c                       | 28 ++-----------------------
 drivers/usb/musb-new/sunxi.c              | 35 ++++++++++++++++++++++---------
 3 files changed, 34 insertions(+), 36 deletions(-)

diff --git a/arch/arm/include/asm/arch-sunxi/usb_phy.h b/arch/arm/include/asm/arch-sunxi/usb_phy.h
index 5a9cacb..17d31b8 100644
--- a/arch/arm/include/asm/arch-sunxi/usb_phy.h
+++ b/arch/arm/include/asm/arch-sunxi/usb_phy.h
@@ -19,3 +19,10 @@ void sunxi_usb_phy_power_off(int index);
 int sunxi_usb_phy_vbus_detect(int index);
 int sunxi_usb_phy_id_detect(int index);
 void sunxi_usb_phy_enable_squelch_detect(int index, int enable);
+
+/* Not really phy related, but we have to declare this somewhere ... */
+#if defined(CONFIG_MUSB_HOST) || defined(CONFIG_MUSB_GADGET)
+void sunxi_musb_board_init(void);
+#else
+#define sunxi_musb_board_init()
+#endif
diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index 2638bc1..e14725f 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -34,7 +34,6 @@
 #include <asm/arch/usb_phy.h>
 #include <asm/gpio.h>
 #include <asm/io.h>
-#include <linux/usb/musb.h>
 #include <net.h>
 
 #if defined CONFIG_VIDEO_LCD_PANEL_I2C && !(defined CONFIG_SPL_BUILD)
@@ -467,28 +466,6 @@ void sunxi_board_init(void)
 }
 #endif
 
-#if defined(CONFIG_MUSB_HOST) || defined(CONFIG_MUSB_GADGET)
-extern const struct musb_platform_ops sunxi_musb_ops;
-
-static struct musb_hdrc_config musb_config = {
-	.multipoint     = 1,
-	.dyn_fifo       = 1,
-	.num_eps        = 6,
-	.ram_bits       = 11,
-};
-
-static struct musb_hdrc_platform_data musb_plat = {
-#if defined(CONFIG_MUSB_HOST)
-	.mode           = MUSB_HOST,
-#else
-	.mode		= MUSB_PERIPHERAL,
-#endif
-	.config         = &musb_config,
-	.power          = 250,
-	.platform_ops	= &sunxi_musb_ops,
-};
-#endif
-
 #ifdef CONFIG_USB_GADGET
 int g_dnl_board_usb_cable_connected(void)
 {
@@ -551,9 +528,8 @@ int misc_init_r(void)
 	if (ret)
 		return ret;
 #endif
-#if defined(CONFIG_MUSB_HOST) || defined(CONFIG_MUSB_GADGET)
-	musb_register(&musb_plat, NULL, (void *)SUNXI_USB0_BASE);
-#endif
+	sunxi_musb_board_init();
+
 	return 0;
 }
 #endif
diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c
index cafb480..cbd2954 100644
--- a/drivers/usb/musb-new/sunxi.c
+++ b/drivers/usb/musb-new/sunxi.c
@@ -26,17 +26,9 @@
 #include <asm/arch/gpio.h>
 #include <asm/arch/usb_phy.h>
 #include <asm-generic/gpio.h>
+#include <linux/usb/musb.h>
 #include "linux-compat.h"
 #include "musb_core.h"
-#ifdef CONFIG_AXP152_POWER
-#include <axp152.h>
-#endif
-#ifdef CONFIG_AXP209_POWER
-#include <axp209.h>
-#endif
-#ifdef CONFIG_AXP221_POWER
-#include <axp221.h>
-#endif
 
 /******************************************************************************
  ******************************************************************************
@@ -277,8 +269,31 @@ static int sunxi_musb_init(struct musb *musb)
 	return 0;
 }
 
-const struct musb_platform_ops sunxi_musb_ops = {
+static const struct musb_platform_ops sunxi_musb_ops = {
 	.init		= sunxi_musb_init,
 	.enable		= sunxi_musb_enable,
 	.disable	= sunxi_musb_disable,
 };
+
+static struct musb_hdrc_config musb_config = {
+	.multipoint     = 1,
+	.dyn_fifo       = 1,
+	.num_eps        = 6,
+	.ram_bits       = 11,
+};
+
+static struct musb_hdrc_platform_data musb_plat = {
+#if defined(CONFIG_MUSB_HOST)
+	.mode           = MUSB_HOST,
+#else
+	.mode		= MUSB_PERIPHERAL,
+#endif
+	.config         = &musb_config,
+	.power          = 250,
+	.platform_ops	= &sunxi_musb_ops,
+};
+
+void sunxi_musb_board_init(void)
+{
+	musb_register(&musb_plat, NULL, (void *)SUNXI_USB0_BASE);
+}
-- 
2.4.3

[U-Boot] [PATCH 20/22] sunxi: musb: Use device-model for musb host mode

[Hans de Goede]

Modify the sunxi musb glue to use the device-model for musb host mode.

This allows using musb in host mode together with other host drivers
such as ehci / ohci, which is esp. useful on boards which use the
musb controller in host-only mode, these boards have e.g. an usb-a
receptacle or an usb to sata converter attached to the musb controller.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 board/sunxi/Kconfig          |  2 +-
 drivers/usb/musb-new/sunxi.c | 57 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
index 4311c3e..6a19f85 100644
--- a/board/sunxi/Kconfig
+++ b/board/sunxi/Kconfig
@@ -586,7 +586,7 @@ config DM_SERIAL
 	default y
 
 config DM_USB
-	default y if !USB_MUSB_SUNXI
+	default y
 
 config CMD_SETEXPR
 	default y
diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c
index cbd2954..d1cb8e0 100644
--- a/drivers/usb/musb-new/sunxi.c
+++ b/drivers/usb/musb-new/sunxi.c
@@ -26,9 +26,12 @@
 #include <asm/arch/gpio.h>
 #include <asm/arch/usb_phy.h>
 #include <asm-generic/gpio.h>
+#include <dm/lists.h>
+#include <dm/root.h>
 #include <linux/usb/musb.h>
 #include "linux-compat.h"
 #include "musb_core.h"
+#include "musb_uboot.h"
 
 /******************************************************************************
  ******************************************************************************
@@ -293,7 +296,61 @@ static struct musb_hdrc_platform_data musb_plat = {
 	.platform_ops	= &sunxi_musb_ops,
 };
 
+#ifdef CONFIG_MUSB_HOST
+int musb_usb_probe(struct udevice *dev)
+{
+	struct musb_host_data *host = dev_get_priv(dev);
+	struct usb_bus_priv *priv = dev_get_uclass_priv(dev);
+
+	priv->desc_before_addr = true;
+
+	if (!host->host) {
+		host->host = musb_init_controller(&musb_plat, NULL,
+						  (void *)SUNXI_USB0_BASE);
+		if (!host->host) {
+			printf("Failed to init the controller\n");
+			return -EIO;
+		}
+	}
+
+	printf("MUSB OTG in host-mode\n");
+
+	return musb_lowlevel_init(host);
+}
+
+int musb_usb_remove(struct udevice *dev)
+{
+	struct musb_host_data *host = dev_get_priv(dev);
+
+	musb_stop(host->host);
+
+	return 0;
+}
+
+U_BOOT_DRIVER(usb_musb) = {
+	.name	= "sunxi-musb",
+	.id	= UCLASS_USB,
+	.probe = musb_usb_probe,
+	.remove = musb_usb_remove,
+	.ops	= &musb_usb_ops,
+	.platdata_auto_alloc_size = sizeof(struct usb_platdata),
+	.priv_auto_alloc_size = sizeof(struct musb_host_data),
+};
+#endif
+
 void sunxi_musb_board_init(void)
 {
+#ifdef CONFIG_MUSB_HOST
+	struct udevice *dev;
+
+	/*
+	 * Bind the driver directly for now as musb linux kernel support is
+	 * still pending upstream so our dts files do not have the necessay
+	 * nodes yet. TODO: Remove this as soon as the dts nodes are in place
+	 * and bind by compatible instead.
+	 */
+	device_bind_driver(dm_root(), "sunxi-musb", "sunxi-musb", &dev);
+#else
 	musb_register(&musb_plat, NULL, (void *)SUNXI_USB0_BASE);
+#endif
 }
-- 
2.4.3

[U-Boot] [PATCH 21/22] sunxi: Kconfig: Enable CONFIG_USB and friends by default on sunxi

[Hans de Goede]

Start using the new Kconfig options which are available for most of the
USB settings now.

This also allows us to use "CONFIG_USB_EHCI_HCD=y" in defconfig files
for new boards instead of CONFIG_SYS_EXTRA_OPTIONS="USB_EHCI".

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 board/sunxi/Kconfig            | 9 +++++++++
 include/configs/sunxi-common.h | 5 -----
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
index 6a19f85..8d55cd6 100644
--- a/board/sunxi/Kconfig
+++ b/board/sunxi/Kconfig
@@ -594,4 +594,13 @@ config CMD_SETEXPR
 config CMD_NET
 	default y
 
+config CMD_USB
+	default y
+
+config USB
+	default y
+
+config USB_STORAGE
+	default y
+
 endif
diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
index 063abd5..e2371a1 100644
--- a/include/configs/sunxi-common.h
+++ b/include/configs/sunxi-common.h
@@ -338,11 +338,6 @@ extern int soft_i2c_gpio_scl;
 #define CONFIG_MUSB_PIO_ONLY
 #endif
 
-#if defined CONFIG_USB_EHCI || defined CONFIG_USB_MUSB_SUNXI
-#define CONFIG_CMD_USB
-#define CONFIG_USB_STORAGE
-#endif
-
 #ifdef CONFIG_USB_KEYBOARD
 #define CONFIG_CONSOLE_MUX
 #define CONFIG_PREBOOT
-- 
2.4.3

[U-Boot] [PATCH 22/22] sunxi: ga10h: Enable both otg and regular usb host controllers

[Hans de Goede]

This allows using devices plugged into both ports of the tablet.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 arch/arm/dts/sun8i-a23-a33.dtsi       | 18 ++++++++++++++++++
 arch/arm/dts/sun8i-a33-ga10h-v1.1.dts |  8 ++++++++
 configs/ga10h_v1_1_defconfig          |  2 ++
 3 files changed, 28 insertions(+)

diff --git a/arch/arm/dts/sun8i-a23-a33.dtsi b/arch/arm/dts/sun8i-a23-a33.dtsi
index faea94e..eeb6655 100644
--- a/arch/arm/dts/sun8i-a23-a33.dtsi
+++ b/arch/arm/dts/sun8i-a23-a33.dtsi
@@ -332,6 +332,24 @@
 			#size-cells = <0>;
 		};
 
+		ehci0: usb at 01c1a000 {
+			compatible = "allwinner,sun8i-a23-ehci", "generic-ehci";
+			reg = <0x01c1a000 0x100>;
+			interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ahb1_gates 26>;
+			resets = <&ahb1_rst 26>;
+			status = "disabled";
+		};
+
+		ohci0: usb at 01c1a400 {
+			compatible = "allwinner,sun8i-a23-ohci", "generic-ohci";
+			reg = <0x01c1a400 0x100>;
+			interrupts = <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ahb1_gates 29>, <&usb_clk 16>;
+			resets = <&ahb1_rst 29>;
+			status = "disabled";
+		};
+
 		pio: pinctrl at 01c20800 {
 			/* compatible gets set in SoC specific dtsi file */
 			reg = <0x01c20800 0x400>;
diff --git a/arch/arm/dts/sun8i-a33-ga10h-v1.1.dts b/arch/arm/dts/sun8i-a33-ga10h-v1.1.dts
index 8667033..da16343 100644
--- a/arch/arm/dts/sun8i-a33-ga10h-v1.1.dts
+++ b/arch/arm/dts/sun8i-a33-ga10h-v1.1.dts
@@ -61,6 +61,10 @@
 	};
 };
 
+&ehci0 {
+	status = "okay";
+};
+
 &i2c0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&i2c0_pins_a>;
@@ -109,6 +113,10 @@
 	status = "okay";
 };
 
+&ohci0 {
+	status = "okay";
+};
+
 &pio {
 	mmc0_cd_pin_q8h: mmc0_cd_pin at 0 {
 		allwinner,pins = "PB4";
diff --git a/configs/ga10h_v1_1_defconfig b/configs/ga10h_v1_1_defconfig
index 67b40c2..8d3063b 100644
--- a/configs/ga10h_v1_1_defconfig
+++ b/configs/ga10h_v1_1_defconfig
@@ -6,6 +6,7 @@ CONFIG_DRAM_ZQ=15291
 CONFIG_DRAM_ODT_EN=y
 CONFIG_USB0_VBUS_PIN="AXP0-VBUS-ENABLE"
 CONFIG_USB0_VBUS_DET="AXP0-VBUS-DETECT"
+CONFIG_USB0_ID_DET="PH8"
 CONFIG_AXP_GPIO=y
 CONFIG_VIDEO_LCD_MODE="x:1024,y:600,depth:18,pclk_khz:52000,le:138,ri:162,up:22,lo:10,hs:20,vs:3,sync:3,vmode:0"
 CONFIG_VIDEO_LCD_DCLK_PHASE=0
@@ -19,3 +20,4 @@ CONFIG_SPL=y
 CONFIG_SYS_EXTRA_OPTIONS="CONS_INDEX=5"
 CONFIG_AXP221_DLDO1_VOLT=3300
 CONFIG_AXP221_ALDO1_VOLT=3000
+CONFIG_USB_EHCI_HCD=y
-- 
2.4.3

[U-Boot] [PATCH 16/22] sunxi: usb-phy: Add support for reading otg id pin value

[Ian Campbell]

On Wed, 2015-06-17 at 21:33 +0200, Hans de Goede wrote:
> Add support for reading the id pin value of the otg connector to the usb
> phy code.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Ian Campbell <ijc@hellion.org.uk>

[U-Boot] [PATCH 17/22] sunxi: musb: Move vbus check to sunxi_musb_enable

[Ian Campbell]

On Wed, 2015-06-17 at 21:34 +0200, Hans de Goede wrote:
> This way it can be re-checked on "usb reset".
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Ian Campbell <ijc@hellion.org.uk>

[U-Boot] [PATCH 18/22] sunxi: musb: Add id pin support

[Ian Campbell]

On Wed, 2015-06-17 at 21:34 +0200, Hans de Goede wrote:
> When in host mode check if there is a host cable inserted into the otg
> port by checking the id pin. If there is no host cable return an error to
> make usb_lowlevel_init() exit early, rather then waiting for 1 second
> for a device which will never show up.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

I guess the defconfig fiddling is here because this is the culmination
of these three patches which makes it useful to set this value? In which
case:

Acked-by: Ian Campbell <ijc@hellion.org.uk>

Although I'd have been inclined to do it where the Kconfig option was
added (ack to that too if you want to move).

Ian.

[U-Boot] [PATCH 19/22] sunxi: musb: Move musb config and platdata to the sunxi-musb glue

[Ian Campbell]

On Wed, 2015-06-17 at 21:34 +0200, Hans de Goede wrote:
> Move the musb config and platdata to the sunxi-musb glue, which is where
> it really belongs. This is preparation patch for adding device-model
> support for the sunxi-musb-host code.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  arch/arm/include/asm/arch-sunxi/usb_phy.h |  7 +++++++
>  board/sunxi/board.c                       | 28 ++-----------------------
>  drivers/usb/musb-new/sunxi.c              | 35 ++++++++++++++++++++++---------
>  3 files changed, 34 insertions(+), 36 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-sunxi/usb_phy.h b/arch/arm/include/asm/arch-sunxi/usb_phy.h
> index 5a9cacb..17d31b8 100644
> --- a/arch/arm/include/asm/arch-sunxi/usb_phy.h
> +++ b/arch/arm/include/asm/arch-sunxi/usb_phy.h
> @@ -19,3 +19,10 @@ void sunxi_usb_phy_power_off(int index);
>  int sunxi_usb_phy_vbus_detect(int index);
>  int sunxi_usb_phy_id_detect(int index);
>  void sunxi_usb_phy_enable_squelch_detect(int index, int enable);
> +
> +/* Not really phy related, but we have to declare this somewhere ... */

I guess arch/arm/include/asm/arch-sunxi/usbc.h isn't any better?

With it here or there:

Acked-by: Ian Campbell <ijc@hellion.org.uk>

> +#if defined(CONFIG_MUSB_HOST) || defined(CONFIG_MUSB_GADGET)
> +void sunxi_musb_board_init(void);
> +#else
> +#define sunxi_musb_board_init()
> +#endif

Ian.

[U-Boot] [PATCH 20/22] sunxi: musb: Use device-model for musb host mode

[Ian Campbell]

On Wed, 2015-06-17 at 21:34 +0200, Hans de Goede wrote:
> [...]

> +	 * Bind the driver directly for now as musb linux kernel support is
> +	 * still pending upstream so our dts files do not have the necessay

"necessary". Otherwise:

Acked-by: Ian Campbell <ijc@hellion.org.uk>

So far as this being a sunxi change goes, although I think this probably
needs other acks from the CC line too.

Ian.

> +	 * nodes yet. TODO: Remove this as soon as the dts nodes are in place
> +	 * and bind by compatible instead.
> +	 */
> +	device_bind_driver(dm_root(), "sunxi-musb", "sunxi-musb", &dev);
> +#else
>  	musb_register(&musb_plat, NULL, (void *)SUNXI_USB0_BASE);
> +#endif
>  }

[U-Boot] [PATCH 21/22] sunxi: Kconfig: Enable CONFIG_USB and friends by default on sunxi

[Ian Campbell]

On Wed, 2015-06-17 at 21:34 +0200, Hans de Goede wrote:
> Start using the new Kconfig options which are available for most of the
> USB settings now.
> 
> This also allows us to use "CONFIG_USB_EHCI_HCD=y" in defconfig files
> for new boards instead of CONFIG_SYS_EXTRA_OPTIONS="USB_EHCI".

Should there not be a raft of defconfig changes at the same time? Or is
that nice but not necessary at this stage?

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  board/sunxi/Kconfig            | 9 +++++++++
>  include/configs/sunxi-common.h | 5 -----
>  2 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
> index 6a19f85..8d55cd6 100644
> --- a/board/sunxi/Kconfig
> +++ b/board/sunxi/Kconfig
> @@ -594,4 +594,13 @@ config CMD_SETEXPR
>  config CMD_NET
>  	default y
>  
> +config CMD_USB
> +	default y
> +
> +config USB
> +	default y
> +
> +config USB_STORAGE
> +	default y
> +
>  endif
> diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
> index 063abd5..e2371a1 100644
> --- a/include/configs/sunxi-common.h
> +++ b/include/configs/sunxi-common.h
> @@ -338,11 +338,6 @@ extern int soft_i2c_gpio_scl;
>  #define CONFIG_MUSB_PIO_ONLY
>  #endif
>  
> -#if defined CONFIG_USB_EHCI || defined CONFIG_USB_MUSB_SUNXI
> -#define CONFIG_CMD_USB
> -#define CONFIG_USB_STORAGE
> -#endif
> -
>  #ifdef CONFIG_USB_KEYBOARD
>  #define CONFIG_CONSOLE_MUX
>  #define CONFIG_PREBOOT

[U-Boot] [PATCH 22/22] sunxi: ga10h: Enable both otg and regular usb host controllers

[Ian Campbell]

On Wed, 2015-06-17 at 21:34 +0200, Hans de Goede wrote:
> This allows using devices plugged into both ports of the tablet.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Ian Campbell <ijc@hellion.org.uk>

[U-Boot] [PATCH 18/22] sunxi: musb: Add id pin support

[Hans de Goede]

Hi,

On 19-06-15 09:40, Ian Campbell wrote:
> On Wed, 2015-06-17 at 21:34 +0200, Hans de Goede wrote:
>> When in host mode check if there is a host cable inserted into the otg
>> port by checking the id pin. If there is no host cable return an error to
>> make usb_lowlevel_init() exit early, rather then waiting for 1 second
>> for a device which will never show up.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>
> I guess the defconfig fiddling is here because this is the culmination
> of these three patches which makes it useful to set this value?

Right.

> In which case:
>
> Acked-by: Ian Campbell <ijc@hellion.org.uk>

Thanks,

Hans

>
> Although I'd have been inclined to do it where the Kconfig option was
> added (ack to that too if you want to move).
>
> Ian.
>
>

[U-Boot] [PATCH 19/22] sunxi: musb: Move musb config and platdata to the sunxi-musb glue

[Hans de Goede]

Hi,

On 19-06-15 09:43, Ian Campbell wrote:
> On Wed, 2015-06-17 at 21:34 +0200, Hans de Goede wrote:
>> Move the musb config and platdata to the sunxi-musb glue, which is where
>> it really belongs. This is preparation patch for adding device-model
>> support for the sunxi-musb-host code.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   arch/arm/include/asm/arch-sunxi/usb_phy.h |  7 +++++++
>>   board/sunxi/board.c                       | 28 ++-----------------------
>>   drivers/usb/musb-new/sunxi.c              | 35 ++++++++++++++++++++++---------
>>   3 files changed, 34 insertions(+), 36 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/arch-sunxi/usb_phy.h b/arch/arm/include/asm/arch-sunxi/usb_phy.h
>> index 5a9cacb..17d31b8 100644
>> --- a/arch/arm/include/asm/arch-sunxi/usb_phy.h
>> +++ b/arch/arm/include/asm/arch-sunxi/usb_phy.h
>> @@ -19,3 +19,10 @@ void sunxi_usb_phy_power_off(int index);
>>   int sunxi_usb_phy_vbus_detect(int index);
>>   int sunxi_usb_phy_id_detect(int index);
>>   void sunxi_usb_phy_enable_squelch_detect(int index, int enable);
>> +
>> +/* Not really phy related, but we have to declare this somewhere ... */
>
> I guess arch/arm/include/asm/arch-sunxi/usbc.h isn't any better?

Well we do not have that, and I did not feel it was worth adding yet
another .h file for just this one function.

>
> With it here or there:
>
> Acked-by: Ian Campbell <ijc@hellion.org.uk>

Regards,

Hans


>
>> +#if defined(CONFIG_MUSB_HOST) || defined(CONFIG_MUSB_GADGET)
>> +void sunxi_musb_board_init(void);
>> +#else
>> +#define sunxi_musb_board_init()
>> +#endif
>
> Ian.
>

[U-Boot] [PATCH 21/22] sunxi: Kconfig: Enable CONFIG_USB and friends by default on sunxi

[Hans de Goede]

Hi,

On 19-06-15 09:46, Ian Campbell wrote:
> On Wed, 2015-06-17 at 21:34 +0200, Hans de Goede wrote:
>> Start using the new Kconfig options which are available for most of the
>> USB settings now.
>>
>> This also allows us to use "CONFIG_USB_EHCI_HCD=y" in defconfig files
>> for new boards instead of CONFIG_SYS_EXTRA_OPTIONS="USB_EHCI".
>
> Should there not be a raft of defconfig changes at the same time? Or is
> that nice but not necessary at this stage?

There could be a of defconfig changes at the same time, but it is not
necessary, so I decided to wait with cleaning this up till later.

I would like to first see all the other stuff from CONFIG_SYS_EXTRA_OPTIONS
by Kconfig-ified too, and then we can cleanup all the CONFIG_SYS_EXTRA_OPTIONS
usage in one go.

Regards,

Hans


>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   board/sunxi/Kconfig            | 9 +++++++++
>>   include/configs/sunxi-common.h | 5 -----
>>   2 files changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
>> index 6a19f85..8d55cd6 100644
>> --- a/board/sunxi/Kconfig
>> +++ b/board/sunxi/Kconfig
>> @@ -594,4 +594,13 @@ config CMD_SETEXPR
>>   config CMD_NET
>>   	default y
>>
>> +config CMD_USB
>> +	default y
>> +
>> +config USB
>> +	default y
>> +
>> +config USB_STORAGE
>> +	default y
>> +
>>   endif
>> diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
>> index 063abd5..e2371a1 100644
>> --- a/include/configs/sunxi-common.h
>> +++ b/include/configs/sunxi-common.h
>> @@ -338,11 +338,6 @@ extern int soft_i2c_gpio_scl;
>>   #define CONFIG_MUSB_PIO_ONLY
>>   #endif
>>
>> -#if defined CONFIG_USB_EHCI || defined CONFIG_USB_MUSB_SUNXI
>> -#define CONFIG_CMD_USB
>> -#define CONFIG_USB_STORAGE
>> -#endif
>> -
>>   #ifdef CONFIG_USB_KEYBOARD
>>   #define CONFIG_CONSOLE_MUX
>>   #define CONFIG_PREBOOT
>
>

[U-Boot] [PATCH 00/22] Convert musb host mode code to the device-model

[Simon Glass]

Hi Hans,

On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi Marek and Simon,
>
> This series started out with the idea that it would be a nice small project
> for the weekend, but it turned out to be a bit more work...
>
> The main purpose of this series is to convert the musb host mode code to the
> device-model this has also resulted in various usb fixes / cleanups /
> reworking to make this possible, both in the generic usb code as well as in
> the device model usb code.
>
> Given that this touches both, I think it is probably best to merge the
> first 15 patches through Simon's tree like we did last time, then once those
> are place I can merge the sunxi bits. Note this is intended for v2015.10
> (ofcourse).
>
> Note that this series is useful for a bunch more boards then just the
> single one the last patch updates to use musb + ehci + ohci, but that is
> the one I've been testing with other defconfig-s will be updated with
> followup patches.
>
> Please review.

Thanks for putting this together. Actually I'm not sure what musb is.
Can you explain what it is used for - it seems to be referred to as a
new version of something else. Why do we have musb and non-musb? Is
there a README somewhere I could read?

Regards,
Simon

[U-Boot] [PATCH 00/22] Convert musb host mode code to the device-model

[Hans de Goede]

Hi,

On 19-06-15 15:10, Simon Glass wrote:
> Hi Hans,
>
> On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi Marek and Simon,
>>
>> This series started out with the idea that it would be a nice small project
>> for the weekend, but it turned out to be a bit more work...
>>
>> The main purpose of this series is to convert the musb host mode code to the
>> device-model this has also resulted in various usb fixes / cleanups /
>> reworking to make this possible, both in the generic usb code as well as in
>> the device model usb code.
>>
>> Given that this touches both, I think it is probably best to merge the
>> first 15 patches through Simon's tree like we did last time, then once those
>> are place I can merge the sunxi bits. Note this is intended for v2015.10
>> (ofcourse).
>>
>> Note that this series is useful for a bunch more boards then just the
>> single one the last patch updates to use musb + ehci + ohci, but that is
>> the one I've been testing with other defconfig-s will be updated with
>> followup patches.
>>
>> Please review.
>
> Thanks for putting this together. Actually I'm not sure what musb is.
> Can you explain what it is used for - it seems to be referred to as a
> new version of something else. Why do we have musb and non-musb? Is
> there a README somewhere I could read?

musb stands for mentor-graphics usb it is an otg usb2 ip-block used in
various SoCs as otg controller. As an otg controller it can be used in
both host or peripheral mode. This series converts the code for using
it in host-mode to the device-model.

Regards,

Hans

[U-Boot] [PATCH 00/22] Convert musb host mode code to the device-model

[Hans de Goede]

Hi,

On 19-06-15 15:10, Simon Glass wrote:
> Hi Hans,
>
> On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi Marek and Simon,
>>
>> This series started out with the idea that it would be a nice small project
>> for the weekend, but it turned out to be a bit more work...
>>
>> The main purpose of this series is to convert the musb host mode code to the
>> device-model this has also resulted in various usb fixes / cleanups /
>> reworking to make this possible, both in the generic usb code as well as in
>> the device model usb code.
>>
>> Given that this touches both, I think it is probably best to merge the
>> first 15 patches through Simon's tree like we did last time, then once those
>> are place I can merge the sunxi bits. Note this is intended for v2015.10
>> (ofcourse).
>>
>> Note that this series is useful for a bunch more boards then just the
>> single one the last patch updates to use musb + ehci + ohci, but that is
>> the one I've been testing with other defconfig-s will be updated with
>> followup patches.
>>
>> Please review.
>
> Thanks for putting this together. Actually I'm not sure what musb is.
> Can you explain what it is used for - it seems to be referred to as a
> new version of something else. Why do we have musb and non-musb? Is
> there a README somewhere I could read?

p.s.

As for there being both a drivers/usb/musb and a driver/usb/musb-new
directoyry, I believe that they both are drivers for the same hardware,
but not all boards have been ported to musb-new yet. musb-new is
derived from recent kernel code for the musb controller.

Regards,

Hans



>
> Regards,
> Simon
>

[U-Boot] [PATCH 19/22] sunxi: musb: Move musb config and platdata to the sunxi-musb glue

[Ian Campbell]

On Fri, 2015-06-19 at 11:35 +0200, Hans de Goede wrote:
> Hi,
> 
> On 19-06-15 09:43, Ian Campbell wrote:
> > On Wed, 2015-06-17 at 21:34 +0200, Hans de Goede wrote:
> >> Move the musb config and platdata to the sunxi-musb glue, which is where
> >> it really belongs. This is preparation patch for adding device-model
> >> support for the sunxi-musb-host code.
> >>
> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >> ---
> >>   arch/arm/include/asm/arch-sunxi/usb_phy.h |  7 +++++++
> >>   board/sunxi/board.c                       | 28 ++-----------------------
> >>   drivers/usb/musb-new/sunxi.c              | 35 ++++++++++++++++++++++---------
> >>   3 files changed, 34 insertions(+), 36 deletions(-)
> >>
> >> diff --git a/arch/arm/include/asm/arch-sunxi/usb_phy.h b/arch/arm/include/asm/arch-sunxi/usb_phy.h
> >> index 5a9cacb..17d31b8 100644
> >> --- a/arch/arm/include/asm/arch-sunxi/usb_phy.h
> >> +++ b/arch/arm/include/asm/arch-sunxi/usb_phy.h
> >> @@ -19,3 +19,10 @@ void sunxi_usb_phy_power_off(int index);
> >>   int sunxi_usb_phy_vbus_detect(int index);
> >>   int sunxi_usb_phy_id_detect(int index);
> >>   void sunxi_usb_phy_enable_squelch_detect(int index, int enable);
> >> +
> >> +/* Not really phy related, but we have to declare this somewhere ... */
> >
> > I guess arch/arm/include/asm/arch-sunxi/usbc.h isn't any better?
> 
> Well we do not have that, and I did not feel it was worth adding yet
> another .h file for just this one function.

Ah, my tree had:

u-boot.git$ wc -l arch/arm/include/asm/arch-sunxi/usbc.h
24 arch/arm/include/asm/arch-sunxi/usbc.h

But it was stale and the file has now gone in sunxi#next.

Ian.

[U-Boot] [PATCH 21/22] sunxi: Kconfig: Enable CONFIG_USB and friends by default on sunxi

[Ian Campbell]

On Fri, 2015-06-19 at 11:37 +0200, Hans de Goede wrote:
> Hi,
> 
> On 19-06-15 09:46, Ian Campbell wrote:
> > On Wed, 2015-06-17 at 21:34 +0200, Hans de Goede wrote:
> >> Start using the new Kconfig options which are available for most of the
> >> USB settings now.
> >>
> >> This also allows us to use "CONFIG_USB_EHCI_HCD=y" in defconfig files
> >> for new boards instead of CONFIG_SYS_EXTRA_OPTIONS="USB_EHCI".
> >
> > Should there not be a raft of defconfig changes at the same time? Or is
> > that nice but not necessary at this stage?
> 
> There could be a of defconfig changes at the same time, but it is not
> necessary, so I decided to wait with cleaning this up till later.

OK:
Acked-by: Ian Campbell <ijc@hellion.org.uk>

> 
> I would like to first see all the other stuff from CONFIG_SYS_EXTRA_OPTIONS
> by Kconfig-ified too, and then we can cleanup all the CONFIG_SYS_EXTRA_OPTIONS
> usage in one go.

[U-Boot] [PATCH 01/22] usb: Always declare usb function prototypes

[Simon Glass]

On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
> There is no harm in declaring the function prototypes even if nothing
> implements them, and when CONFIG_DM_USB=y the various usb functions are
> available regardless of any controller drivers being enabled.
>
> This fixes compile warnings due to missing prototypes on ARCHs where
> the ARCH Kconfig always enables CONFIG_DM_USB and various usb drivers.
>
> One could argue that in the case of no controllers CONFIG_DM_USB should not
> be set, but this problem is typically seen during bringup of boards which
> do actually have usb controllers.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Simply always define the function prototypes instead of adding yet another
>  condition to the already unwieldly #if def ... || def ... condition
> ---
>  include/usb.h | 15 ---------------
>  1 file changed, 15 deletions(-)

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

[U-Boot] [PATCH 02/22] usb: Drop device-model specific copy of usb_legacy_port_reset

[Simon Glass]

On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
> The device-model usb_legacy_port_reset function calls the device-model
> usb_port_reset function which is a 1 on 1 copy of the non dm
> usb_legacy_port_reset and this is the only use of usb_port_reset in all
> of u-boot.
>
> Drop both, and alway use the usb_legacy_port_reset() version in
> common/usb.c .
>
> Also while at it make it static as it is only used in common/usb.c .
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  common/usb.c                  |  4 +---
>  drivers/usb/host/usb-uclass.c | 29 -----------------------------
>  include/usb.h                 |  8 --------
>  3 files changed, 1 insertion(+), 40 deletions(-)

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

[U-Boot] [PATCH 03/22] usb: usb_setup_device: Drop unneeded portnr function argument

[Simon Glass]

Hi Hans.

On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
> Drop the unneeded portnr function argument, the portnr is part of the
> usb_device struct which is passed via the dev argument.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  common/usb.c                  | 10 +++++-----
>  drivers/usb/host/usb-uclass.c |  2 +-
>  include/usb.h                 |  6 +++---
>  3 files changed, 9 insertions(+), 9 deletions(-)

This was needed in the case where a fake usb_device was passed in. Has
your previous refactoring changed that?

Regards,
Simon

[U-Boot] [PATCH 04/22] usb: Pass device instead of portnr to usb_legacy_port_reset

[Simon Glass]

Hi Hans,

On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
> Pass the usb_device instead of the portnr to usb_legacy_port_reset and
> rename it to usb_hub_port_reset as there is nothing legacy about it.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  common/usb.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Legacy as in not driver model.

Regards,
Simon

[U-Boot] [PATCH 05/22] usb: Add an usb_device parameter to usb_reset_root_port

[Simon Glass]

On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
> Add an usb_device parameter to usb_reset_root_port so that it knows which
> root-port it is resetting. This is necessary for proper device-model support
> for usb_reset_root_port.
>
> Also remove a duplicate declaration of usb_reset_root_port() from usb.h .
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  common/usb.c                      | 2 +-
>  drivers/usb/host/usb-uclass.c     | 2 +-
>  drivers/usb/musb-new/musb_uboot.c | 4 ++--
>  include/usb.h                     | 8 ++------
>  4 files changed, 6 insertions(+), 10 deletions(-)

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

[U-Boot] [PATCH 06/22] dm: Export device_chld_remove / device_chld_unbind

[Simon Glass]

Hi Hans,

On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
> These functions are useful to remove all children from an usb bus before
> rescanning the bus.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/core/device-remove.c | 18 ++----------------
>  include/dm/device-internal.h | 26 ++++++++++++++++++++++++++
>  2 files changed, 28 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c
> index 6a16b4f..06de7e3 100644
> --- a/drivers/core/device-remove.c
> +++ b/drivers/core/device-remove.c
> @@ -18,16 +18,7 @@
>  #include <dm/uclass-internal.h>
>  #include <dm/util.h>
>
> -/**
> - * device_chld_unbind() - Unbind all device's children from the device
> - *
> - * On error, the function continues to unbind all children, and reports the
> - * first error.
> - *
> - * @dev:       The device that is to be stripped of its children
> - * @return 0 on success, -ve on error
> - */
> -static int device_chld_unbind(struct udevice *dev)
> +int device_chld_unbind(struct udevice *dev)

If we are exporting this can we give it a better name? Maybe
device_unbind_children()?

>  {
>         struct udevice *pos, *n;
>         int ret, saved_ret = 0;
> @@ -43,12 +34,7 @@ static int device_chld_unbind(struct udevice *dev)
>         return saved_ret;
>  }
>
> -/**
> - * device_chld_remove() - Stop all device's children
> - * @dev:       The device whose children are to be removed
> - * @return 0 on success, -ve on error
> - */
> -static int device_chld_remove(struct udevice *dev)
> +int device_chld_remove(struct udevice *dev)
>  {
>         struct udevice *pos, *n;
>         int ret;
> diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h
> index 687462b..6c8fe23 100644
> --- a/include/dm/device-internal.h
> +++ b/include/dm/device-internal.h
> @@ -107,6 +107,32 @@ int device_unbind(struct udevice *dev);
>  static inline int device_unbind(struct udevice *dev) { return 0; }
>  #endif
>
> +/**
> + * device_chld_remove() - Stop all device's children
> + * @dev:       The device whose children are to be removed
> + * @return 0 on success, -ve on error
> + */
> +#ifdef CONFIG_DM_DEVICE_REMOVE
> +int device_chld_remove(struct udevice *dev);
> +#else
> +static inline int device_chld_remove(struct udevice *dev) { return 0; }
> +#endif
> +
> +/**
> + * device_chld_unbind() - Unbind all device's children from the device
> + *
> + * On error, the function continues to unbind all children, and reports the
> + * first error.
> + *
> + * @dev:       The device that is to be stripped of its children
> + * @return 0 on success, -ve on error
> + */
> +#ifdef CONFIG_DM_DEVICE_REMOVE
> +int device_chld_unbind(struct udevice *dev);
> +#else
> +static inline int device_chld_unbind(struct udevice *dev) { return 0; }
> +#endif
> +
>  #ifdef CONFIG_DM_DEVICE_REMOVE
>  void device_free(struct udevice *dev);
>  #else
> --
> 2.4.3
>

Regards,
Simon

[U-Boot] [PATCH 07/22] dm: usb: Fix "usb tree" output

[Simon Glass]

On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
> last_child was abused by the old usb code to first store 1 if the
> usb_device was not the root of the usb tree, and then later on re-used
> to store whether or not the usb_device is actually the last child.
>
> The dm-usb code was always setting it to actually reflect the last-child
> status which is wrong for the last child leading to output like this:
>
> USB device tree:
>   1  Hub (12 Mb/s, 100mA)
>   |  ALCOR USB Hub 2.0
>   |
>   | 2  Mass Storage (12 Mb/s, 100mA)
>   |    USB Flash Disk 4C0E960F
>   |
>   +-3  Human Interface (1.5 Mb/s, 100mA)
>        SINO WEALTH USB Composite Device
>
> Instead of this:
>
> USB device tree:
>   1  Hub (12 Mb/s, 100mA)
>   |  ALCOR USB Hub 2.0
>   |
>   +-2  Mass Storage (12 Mb/s, 100mA)
>   |    USB Flash Disk 4C0E960F
>   |
>   +-3  Human Interface (1.5 Mb/s, 100mA)
>        SINO WEALTH USB Composite Device
>
> This commit fixes this by first checking that the device is not root,
> and then setting last_child. This commit also updates the old code to not
> abuse the last_child variable to store the root check result.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  common/cmd_usb.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)

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

[U-Boot] [PATCH 08/22] dm: usb: Use device_chld_remove and _unbind to clean up usb devs on stop

[Simon Glass]

Hi Hans,

On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
> On an usb stop instead of leaving orphan usb devices behind simply remove

On a usb_stop()

or

On a 'usb stop' command

?

> them. This requires CONFIG_DM_DEVICE_REMOVE to be set, so only build
> usb_stop() when that is set.

This seems a little unfortunate. I can see the reasoning, but do you
think this is necessary? I suspect people chasing code size may remove
that option and still want to use USB properly.

>
> The result of this commit is best seen in the output of "dm tree" after
> plugging out an usb hub with 2 devices plugges in and plugging in a keyb.
> instead, before this commit the output would be:
>
>  usb         [ + ]    `-- sunxi-musb
>  usb_hub     [   ]        |-- usb_hub
>  usb_mass_st [   ]        |   |-- usb_mass_storage
>  usb_dev_gen [   ]        |   `-- generic_bus_0_dev_3
>  usb_dev_gen [ + ]        `-- generic_bus_0_dev_1
>
> Notice the non active usb_hub child and its 2 non active children. The
> first child being non-active as in this example also causes usb_get_dev_index
> to return NULL when probing the first child, which results in the usb kbd
> code not binding to the keyboard.

Although I suspect that could be fixed.

>
> With this commit in place the output after swapping and "usb reset" is:
>
>  usb         [ + ]    `-- sunxi-musb
>  usb_dev_gen [ + ]        `-- generic_bus_0_dev_1
>
> As expected, and usb_get_dev_index works properly and the keyboard works.
>
> After this commit usb_find_child() is only necessary for emulated usb
> devices, so make its body #ifdef CONFIG_USB_EMUL.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/usb/host/usb-uclass.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
> index bce6cec..8f26e35 100644
> --- a/drivers/usb/host/usb-uclass.c
> +++ b/drivers/usb/host/usb-uclass.c
> @@ -128,6 +128,7 @@ int usb_alloc_device(struct usb_device *udev)
>         return ops->alloc_device(bus, udev);
>  }
>
> +#ifdef CONFIG_DM_DEVICE_REMOVE
>  int usb_stop(void)
>  {
>         struct udevice *bus;
> @@ -143,6 +144,12 @@ int usb_stop(void)
>         uc_priv = uc->priv;
>
>         uclass_foreach_dev(bus, uc) {
> +               ret = device_chld_remove(bus);
> +               if (ret && !err)
> +                       err = ret;
> +               ret = device_chld_unbind(bus);
> +               if (ret && !err)
> +                       err = ret;
>                 ret = device_remove(bus);
>                 if (ret && !err)
>                         err = ret;
> @@ -166,6 +173,7 @@ int usb_stop(void)
>
>         return err;
>  }
> +#endif
>
>  static void usb_scan_bus(struct udevice *bus, bool recurse)
>  {
> @@ -491,6 +499,7 @@ static int usb_find_child(struct udevice *parent,
>                           struct usb_interface_descriptor *iface,
>                           struct udevice **devp)
>  {
> +#ifdef CONFIG_USB_EMUL /* Emulated devices are explictily bound */

explicitly

Can you add a comment about this? It seems that we should rename this
function to usb_find_emul_child() and have it present only when
CONFIG_USB_EMUL is around?

Also, why bother with the #ifdef if this function is never called
outside sandbox?

>         struct udevice *dev;
>
>         *devp = NULL;
> @@ -509,6 +518,7 @@ static int usb_find_child(struct udevice *parent,
>                         return 0;
>                 }
>         }
> +#endif
>
>         return -ENOENT;
>  }
> --
> 2.4.3
>

Regards,
Simon

[U-Boot] [PATCH 09/22] dm: usb: Allow usb host drivers to implement usb_reset_root_port

[Simon Glass]

On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
> Allow usb uclass host drivers to implement usb_reset_root_port, this is
> used by single port usb hosts which do not emulate a hub, such as otg
> controllers.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/usb/host/usb-uclass.c | 16 +++++++++++-----
>  include/usb.h                 |  5 +++++
>  2 files changed, 16 insertions(+), 5 deletions(-)

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

[U-Boot] [PATCH 10/22] dm: usb: Do not assume that first child is always a hub

[Simon Glass]

On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
> On some single port (otg) controllers there is no emulated root hub, so
> the first child (if any) may be one of: UCLASS_MASS_STORAGE,
> UCLASS_USB_DEV_GENERIC or UCLASS_USB_HUB.
>
> All three of these (and in the future others) are suitable for our
> purposes, remove the check for the device being a hub, and add a check to
> deal with the fact that there may be no child-dev.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  common/cmd_usb.c              |  9 ++++-----
>  drivers/usb/host/usb-uclass.c | 10 +++++-----
>  2 files changed, 9 insertions(+), 10 deletions(-)

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

[U-Boot] [PATCH 11/22] musb: Allow musb_platform_enable to return an error code

[Simon Glass]

On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
> Allow musb_platform_enable to return an error code and propagate it up to
> usb_lowlevel_init().
>
> This allows moving the checks for an external vbus being present to be
> moved from platform_init to platform_enable, so that the user can unplug a
> charger, plug in a host adapter with a usb-device, do a "usb reset" and
> have things working.
>
> This also allows adding a check for the id-pin to platform_enable, so that
> it can short circuit the 1s delay in usb_lowlevel_init() when no host cable
> is plugged in and thus waiting for a device to show up is useless.
>
> Note that all the changes to code shared with the kernel are wrapped in
> the kernel.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/usb/musb-new/am35x.c      |  7 +++++++
>  drivers/usb/musb-new/musb_core.c  | 20 ++++++++++++++++++++
>  drivers/usb/musb-new/musb_core.h  | 18 ++++++++++++++++++
>  drivers/usb/musb-new/musb_dsps.c  |  6 ++++++
>  drivers/usb/musb-new/musb_uboot.c |  6 +++++-
>  drivers/usb/musb-new/omap2430.c   |  5 +++++
>  drivers/usb/musb-new/sunxi.c      |  5 +++--
>  7 files changed, 64 insertions(+), 3 deletions(-)

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

[U-Boot] [PATCH 12/22] musb: Update usb-compat to work with struct usb_device without a parent ptr

[Simon Glass]

Hi Hans,

On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
> When building with CONFIG_DM_USB=y struct usb_device does not have a parent
> pointer. This commit adds support to the musb code to deal with this.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/usb/musb-new/musb_host.c  |  4 +++
>  drivers/usb/musb-new/musb_uboot.c |  2 +-
>  drivers/usb/musb-new/usb-compat.h | 70 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 75 insertions(+), 1 deletion(-)
>

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

See note below.

> diff --git a/drivers/usb/musb-new/musb_host.c b/drivers/usb/musb-new/musb_host.c
> index 437309c..40b9c66 100644
> --- a/drivers/usb/musb-new/musb_host.c
> +++ b/drivers/usb/musb-new/musb_host.c
> @@ -2067,7 +2067,11 @@ int musb_urb_enqueue(
>
>         /* precompute addressing for external hub/tt ports */
>         if (musb->is_multipoint) {
> +#ifndef __UBOOT__
>                 struct usb_device       *parent = urb->dev->parent;
> +#else
> +               struct usb_device       *parent = usb_dev_get_parent(urb->dev);
> +#endif
>
>  #ifndef __UBOOT__
>                 if (parent != hcd->self.root_hub) {
> diff --git a/drivers/usb/musb-new/musb_uboot.c b/drivers/usb/musb-new/musb_uboot.c
> index 70e87c9..a96e8d2 100644
> --- a/drivers/usb/musb-new/musb_uboot.c
> +++ b/drivers/usb/musb-new/musb_uboot.c
> @@ -97,7 +97,7 @@ int submit_control_msg(struct usb_device *dev, unsigned long pipe,
>                       buffer, len, setup, 0);
>
>         /* Fix speed for non hub-attached devices */
> -       if (!dev->parent)
> +       if (!usb_dev_get_parent(dev))
>                 dev->speed = host_speed;
>
>         return submit_urb(&hcd, &urb);
> diff --git a/drivers/usb/musb-new/usb-compat.h b/drivers/usb/musb-new/usb-compat.h
> index 50bad37..53fe4ff 100644
> --- a/drivers/usb/musb-new/usb-compat.h
> +++ b/drivers/usb/musb-new/usb-compat.h
> @@ -1,6 +1,7 @@
>  #ifndef __USB_COMPAT_H__
>  #define __USB_COMPAT_H__
>
> +#include <dm.h>
>  #include "usb.h"
>
>  struct usb_hcd {
> @@ -66,6 +67,68 @@ static inline int usb_hcd_unmap_urb_for_dma(struct usb_hcd *hcd,
>         return 0;
>  }
>
> +#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_parentdata(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_parentdata(dev);
> +               parent = dev->parent;
> +               uparent = dev_get_parentdata(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;
> +
> +       /*
> +        * 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.

Another option here is to somehow allow devices to be added before we
know what they are. In this case we could bind a 'generic' USB device
(UCLASS_USB_DEV_GENERIC). Then when we work out what it is, we could
unbind it (without throwing away the udevice and usb_device) and have
it bind again as the correct device. Something like
device_morph_child().

Just a thought. I'm not sure which is worse yet.

> +        *
> +        * If dev is an usb-bus, then we are called from usb_scan_device() for
> +        * an usb-device plugged directly into the root port, return NULL.
> +        */
> +       if (device_get_uclass_id(udev->dev) == UCLASS_USB)
> +               return NULL;
> +
> +       /*
> +        * If these 2 are not the same we are being called from
> +        * usb_scan_device() and udev itself is the parent.
> +        */
> +       if (dev_get_parentdata(udev->dev) != udev)
> +               return udev;
> +
> +       /* We are being called normally, use the parent pointer */
> +       if (device_get_uclass_id(parent) == UCLASS_USB_HUB)
> +               return dev_get_parentdata(parent);
> +
> +       return NULL;
> +}
> +#else
>  static inline u16 find_tt(struct usb_device *dev)
>  {
>         u8 chid;
> @@ -86,4 +149,11 @@ static inline u16 find_tt(struct usb_device *dev)
>
>         return (hub << 8) | chid;
>  }
> +
> +static inline struct usb_device *usb_dev_get_parent(struct usb_device *dev)
> +{
> +       return dev->parent;
> +}
> +#endif
> +
>  #endif /* __USB_COMPAT_H__ */
> --
> 2.4.3
>

Regards,
Simon

[U-Boot] [PATCH 13/22] musb: Rename and wrap public functions

[Simon Glass]

On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
> Rename and wrap the usb host API public functions, this is a preparation
> patch for adding device-model support.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/usb/musb-new/musb_uboot.c | 70 +++++++++++++++++++++++++++++++++------
>  1 file changed, 59 insertions(+), 11 deletions(-)

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

[U-Boot] [PATCH 14/22] musb: Add musb_host_data struct to hold global data

[Simon Glass]

Hi Hans,

On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
> Add a musb_host_data struct to hold all the global data host related musb
> data. This is a preparation patch for adding device-model support.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/usb/musb-new/musb_uboot.c | 107 +++++++++++++++++++-------------------
>  drivers/usb/musb-new/musb_uboot.h |  24 +++++++++
>  2 files changed, 77 insertions(+), 54 deletions(-)
>  create mode 100644 drivers/usb/musb-new/musb_uboot.h

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

Regards,
Simon

[U-Boot] [PATCH 15/22] musb: Add device-model support to the musb-host u-boot glue

[Simon Glass]

Hi Hans,

On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
> Add device-model support to the musb-host u-boot glue, note this only
> adds device-model support to the musb-core glue code, it does not add
> support for device-model to any of the SoC specific musb glue code.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/usb/musb-new/musb_uboot.c | 70 ++++++++++++++++++++++++++++++++++++++-
>  drivers/usb/musb-new/musb_uboot.h |  4 +++
>  2 files changed, 73 insertions(+), 1 deletion(-)
>

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

Nit below.

> diff --git a/drivers/usb/musb-new/musb_uboot.c b/drivers/usb/musb-new/musb_uboot.c
> index 9875100..9b56e90 100644
> --- a/drivers/usb/musb-new/musb_uboot.c
> +++ b/drivers/usb/musb-new/musb_uboot.c
> @@ -21,7 +21,9 @@ struct int_queue {
>         struct urb urb;
>  };
>
> +#ifndef CONFIG_DM_USB
>  struct musb_host_data musb_host;
> +#endif
>
>  static void musb_host_complete_urb(struct urb *urb)
>  {
> @@ -244,6 +246,7 @@ int musb_lowlevel_init(struct musb_host_data *host)
>         return 0;
>  }
>
> +#ifndef CONFIG_DM_USB
>  int usb_lowlevel_stop(int index)
>  {
>         if (!musb_host.host) {
> @@ -300,6 +303,71 @@ int usb_lowlevel_init(int index, enum usb_init_type init, void **controller)
>  {
>         return musb_lowlevel_init(&musb_host);
>  }
> +#endif /* !CONFIG_DM_USB */
> +
> +#ifdef CONFIG_DM_USB
> +static int musb_submit_control_msg(struct udevice *dev, struct usb_device *udev,
> +                                  unsigned long pipe, void *buffer, int length,
> +                                  struct devrequest *setup)
> +{
> +       struct musb_host_data *host = dev_get_priv(dev);
> +       return _musb_submit_control_msg(host, udev, pipe, buffer, length, setup);
> +}
> +
> +static int musb_submit_bulk_msg(struct udevice *dev, struct usb_device *udev,
> +                               unsigned long pipe, void *buffer, int length)
> +{
> +       struct musb_host_data *host = dev_get_priv(dev);
> +       return _musb_submit_bulk_msg(host, udev, pipe, buffer, length);
> +}
> +
> +static int musb_submit_int_msg(struct udevice *dev, struct usb_device *udev,
> +                              unsigned long pipe, void *buffer, int length,
> +                              int interval)
> +{
> +       struct musb_host_data *host = dev_get_priv(dev);
> +       return _musb_submit_int_msg(host, udev, pipe, buffer, length, interval);
> +}
> +
> +static struct int_queue *musb_create_int_queue(struct udevice *dev,
> +               struct usb_device *udev, unsigned long pipe, int queuesize,
> +               int elementsize, void *buffer, int interval)
> +{
> +       struct musb_host_data *host = dev_get_priv(dev);

Can we have newlines after declarations?

> +       return _musb_create_int_queue(host, udev, pipe, queuesize, elementsize,
> +                                     buffer, interval);
> +}
> +
> +static void *musb_poll_int_queue(struct udevice *dev, struct usb_device *udev,
> +                                struct int_queue *queue)
> +{
> +       struct musb_host_data *host = dev_get_priv(dev);
> +       return _musb_poll_int_queue(host, udev, queue);
> +}
> +
> +static int musb_destroy_int_queue(struct udevice *dev, struct usb_device *udev,
> +                                 struct int_queue *queue)
> +{
> +       struct musb_host_data *host = dev_get_priv(dev);
> +       return _musb_destroy_int_queue(host, udev, queue);
> +}
> +
> +static int musb_reset_root_port(struct udevice *dev, struct usb_device *udev)
> +{
> +       struct musb_host_data *host = dev_get_priv(dev);
> +       return _musb_reset_root_port(host, udev);
> +}
> +
> +struct dm_usb_ops musb_usb_ops = {
> +       .control = musb_submit_control_msg,
> +       .bulk = musb_submit_bulk_msg,
> +       .interrupt = musb_submit_int_msg,
> +       .create_int_queue = musb_create_int_queue,
> +       .poll_int_queue = musb_poll_int_queue,
> +       .destroy_int_queue = musb_destroy_int_queue,
> +       .reset_root_port = musb_reset_root_port,
> +};
> +#endif /* CONFIG_DM_USB */
>  #endif /* CONFIG_MUSB_HOST */
>
>  #ifdef CONFIG_MUSB_GADGET
> @@ -360,7 +428,7 @@ int musb_register(struct musb_hdrc_platform_data *plat, void *bdata,
>         struct musb **musbp;
>
>         switch (plat->mode) {
> -#ifdef CONFIG_MUSB_HOST
> +#if defined(CONFIG_MUSB_HOST) && !defined(CONFIG_DM_USB)
>         case MUSB_HOST:
>                 musbp = &musb_host.host;
>                 break;
> diff --git a/drivers/usb/musb-new/musb_uboot.h b/drivers/usb/musb-new/musb_uboot.h
> index 69b7977..6312cd2 100644
> --- a/drivers/usb/musb-new/musb_uboot.h
> +++ b/drivers/usb/musb-new/musb_uboot.h
> @@ -21,4 +21,8 @@ struct musb_host_data {
>         struct urb urb;
>  };
>
> +extern struct dm_usb_ops musb_usb_ops;
> +
> +int musb_lowlevel_init(struct musb_host_data *host);
> +
>  #endif
> --
> 2.4.3
>

Regards,
Simon

[U-Boot] [PATCH 03/22] usb: usb_setup_device: Drop unneeded portnr function argument

[Hans de Goede]

Hi,

On 29-06-15 05:44, Simon Glass wrote:
> Hi Hans.
>
> On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
>> Drop the unneeded portnr function argument, the portnr is part of the
>> usb_device struct which is passed via the dev argument.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   common/usb.c                  | 10 +++++-----
>>   drivers/usb/host/usb-uclass.c |  2 +-
>>   include/usb.h                 |  6 +++---
>>   3 files changed, 9 insertions(+), 9 deletions(-)
>
> This was needed in the case where a fake usb_device was passed in. Has
> your previous refactoring changed that?

The portnr is still passed but it is padded via the usb_device struct's
portnr member. When doing a CONFIG_DM_USB=y build the only call site of
usb_setup_device() is usb_scan_device() from drivers/usb/host/usb-uclass.c
which does:

         udev->portnr = port;
         debug("Calling usb_setup_device(), portnr=%d\n", udev->portnr);
         parent_udev = device_get_uclass_id(parent) == UCLASS_USB_HUB ?
                 dev_get_parentdata(parent) : NULL;
         ret = usb_setup_device(udev, priv->desc_before_addr, parent_udev);

So portnr is always set in the usb_device strict, and that is what gets
used after this patch.

Regards,

Hans

[U-Boot] [PATCH 04/22] usb: Pass device instead of portnr to usb_legacy_port_reset

[Hans de Goede]

Hi,

On 29-06-15 05:44, Simon Glass wrote:
> Hi Hans,
>
> On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
>> Pass the usb_device instead of the portnr to usb_legacy_port_reset and
>> rename it to usb_hub_port_reset as there is nothing legacy about it.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   common/usb.c | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> Legacy as in not driver model.

Except that it gets used in both device-model and non device model
builds of the usb-stack.

Regards,

Hans

p.s.

Thanks for reviewing this largish series!

[U-Boot] [PATCH 06/22] dm: Export device_chld_remove / device_chld_unbind

[Hans de Goede]

Hi,

On 29-06-15 05:45, Simon Glass wrote:
> Hi Hans,
>
> On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
>> These functions are useful to remove all children from an usb bus before
>> rescanning the bus.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/core/device-remove.c | 18 ++----------------
>>   include/dm/device-internal.h | 26 ++++++++++++++++++++++++++
>>   2 files changed, 28 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c
>> index 6a16b4f..06de7e3 100644
>> --- a/drivers/core/device-remove.c
>> +++ b/drivers/core/device-remove.c
>> @@ -18,16 +18,7 @@
>>   #include <dm/uclass-internal.h>
>>   #include <dm/util.h>
>>
>> -/**
>> - * device_chld_unbind() - Unbind all device's children from the device
>> - *
>> - * On error, the function continues to unbind all children, and reports the
>> - * first error.
>> - *
>> - * @dev:       The device that is to be stripped of its children
>> - * @return 0 on success, -ve on error
>> - */
>> -static int device_chld_unbind(struct udevice *dev)
>> +int device_chld_unbind(struct udevice *dev)
>
> If we are exporting this can we give it a better name? Maybe
> device_unbind_children()?

Will do for v2 of this set.

Regards,

Hans

>
>>   {
>>          struct udevice *pos, *n;
>>          int ret, saved_ret = 0;
>> @@ -43,12 +34,7 @@ static int device_chld_unbind(struct udevice *dev)
>>          return saved_ret;
>>   }
>>
>> -/**
>> - * device_chld_remove() - Stop all device's children
>> - * @dev:       The device whose children are to be removed
>> - * @return 0 on success, -ve on error
>> - */
>> -static int device_chld_remove(struct udevice *dev)
>> +int device_chld_remove(struct udevice *dev)
>>   {
>>          struct udevice *pos, *n;
>>          int ret;
>> diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h
>> index 687462b..6c8fe23 100644
>> --- a/include/dm/device-internal.h
>> +++ b/include/dm/device-internal.h
>> @@ -107,6 +107,32 @@ int device_unbind(struct udevice *dev);
>>   static inline int device_unbind(struct udevice *dev) { return 0; }
>>   #endif
>>
>> +/**
>> + * device_chld_remove() - Stop all device's children
>> + * @dev:       The device whose children are to be removed
>> + * @return 0 on success, -ve on error
>> + */
>> +#ifdef CONFIG_DM_DEVICE_REMOVE
>> +int device_chld_remove(struct udevice *dev);
>> +#else
>> +static inline int device_chld_remove(struct udevice *dev) { return 0; }
>> +#endif
>> +
>> +/**
>> + * device_chld_unbind() - Unbind all device's children from the device
>> + *
>> + * On error, the function continues to unbind all children, and reports the
>> + * first error.
>> + *
>> + * @dev:       The device that is to be stripped of its children
>> + * @return 0 on success, -ve on error
>> + */
>> +#ifdef CONFIG_DM_DEVICE_REMOVE
>> +int device_chld_unbind(struct udevice *dev);
>> +#else
>> +static inline int device_chld_unbind(struct udevice *dev) { return 0; }
>> +#endif
>> +
>>   #ifdef CONFIG_DM_DEVICE_REMOVE
>>   void device_free(struct udevice *dev);
>>   #else
>> --
>> 2.4.3
>>
>
> Regards,
> Simon
>

[U-Boot] [PATCH 08/22] dm: usb: Use device_chld_remove and _unbind to clean up usb devs on stop

[Hans de Goede]

Hi,

On 29-06-15 05:45, Simon Glass wrote:
> Hi Hans,
>
> On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
>> On an usb stop instead of leaving orphan usb devices behind simply remove
>
> On a usb_stop()
>
> or
>
> On a 'usb stop' command  ?

My intention was for both, since I was under the assumption that "usb stop"
on the cmdline, was the only caller of usb_stop(), but a quick grep to the
sources show that I'm wrong ...

>> them. This requires CONFIG_DM_DEVICE_REMOVE to be set, so only build
>> usb_stop() when that is set.
>
> This seems a little unfortunate. I can see the reasoning, but do you
> think this is necessary? I suspect people chasing code size may remove
> that option and still want to use USB properly.

This was mostly a result of my thinking that usb_stop() is only used
on "usb stop" at the cmdline, which I know realize is wrong.

However my quick grep has learned that we do really need CONFIG_DM_DEVICE_REMOVE
to properly implement usb_stop():

 From common/bootm.c :

#if defined(CONFIG_CMD_USB)
         /*
          * turn off USB to prevent the host controller from writing to the
          * SDRAM while Linux is booting. This could happen (at least for OHCI
          * controller), because the HCCA (Host Controller Communication Area)
          * lies within the SDRAM and the host controller writes continously to
          * this area (as busmaster!). The HccaFrameNumber is for example
          * updated every 1 ms within the HCCA structure in SDRAM! For more
          * details see the OpenHCI specification.
          */
         usb_stop();
#endif

And without CONFIG_DM_DEVICE_REMOVE we end up never calling the hcd's remove
callback and thus do not properly stop the usb controller.

So this problem of usb_stop() needing CONFIG_DM_DEVICE_REMOVE already exists
before this patch. If you want I can split out the adding of the #ifdef
in a separate commit, spelling out why usb_stop() MUST have
CONFIG_DM_DEVICE_REMOVE in the commit message. Or maybe just move this all to
Kconfig and make DM_USB conflict with CONFIG_DM_DEVICE_REMOVE?



>
>>
>> The result of this commit is best seen in the output of "dm tree" after
>> plugging out an usb hub with 2 devices plugges in and plugging in a keyb.
>> instead, before this commit the output would be:
>>
>>   usb         [ + ]    `-- sunxi-musb
>>   usb_hub     [   ]        |-- usb_hub
>>   usb_mass_st [   ]        |   |-- usb_mass_storage
>>   usb_dev_gen [   ]        |   `-- generic_bus_0_dev_3
>>   usb_dev_gen [ + ]        `-- generic_bus_0_dev_1
>>
>> Notice the non active usb_hub child and its 2 non active children. The
>> first child being non-active as in this example also causes usb_get_dev_index
>> to return NULL when probing the first child, which results in the usb kbd
>> code not binding to the keyboard.
>
> Although I suspect that could be fixed.

Right, but just removing the children is a much cleaner solution, and also
makes the output of "dm tree" properly reflect reality.

>
>>
>> With this commit in place the output after swapping and "usb reset" is:
>>
>>   usb         [ + ]    `-- sunxi-musb
>>   usb_dev_gen [ + ]        `-- generic_bus_0_dev_1
>>
>> As expected, and usb_get_dev_index works properly and the keyboard works.
>>
>> After this commit usb_find_child() is only necessary for emulated usb
>> devices, so make its body #ifdef CONFIG_USB_EMUL.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/usb/host/usb-uclass.c | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
>> index bce6cec..8f26e35 100644
>> --- a/drivers/usb/host/usb-uclass.c
>> +++ b/drivers/usb/host/usb-uclass.c
>> @@ -128,6 +128,7 @@ int usb_alloc_device(struct usb_device *udev)
>>          return ops->alloc_device(bus, udev);
>>   }
>>
>> +#ifdef CONFIG_DM_DEVICE_REMOVE
>>   int usb_stop(void)
>>   {
>>          struct udevice *bus;
>> @@ -143,6 +144,12 @@ int usb_stop(void)
>>          uc_priv = uc->priv;
>>
>>          uclass_foreach_dev(bus, uc) {
>> +               ret = device_chld_remove(bus);
>> +               if (ret && !err)
>> +                       err = ret;
>> +               ret = device_chld_unbind(bus);
>> +               if (ret && !err)
>> +                       err = ret;
>>                  ret = device_remove(bus);
>>                  if (ret && !err)
>>                          err = ret;
>> @@ -166,6 +173,7 @@ int usb_stop(void)
>>
>>          return err;
>>   }
>> +#endif
>>
>>   static void usb_scan_bus(struct udevice *bus, bool recurse)
>>   {
>> @@ -491,6 +499,7 @@ static int usb_find_child(struct udevice *parent,
>>                            struct usb_interface_descriptor *iface,
>>                            struct udevice **devp)
>>   {
>> +#ifdef CONFIG_USB_EMUL /* Emulated devices are explictily bound */
>
> explicitly

Ack.

> Can you add a comment about this? It seems that we should rename this
> function to usb_find_emul_child() and have it present only when
> CONFIG_USB_EMUL is around?

Renaming it to usb_find_emul_child() and only defining the function when
CONFIG_USB_EMUL works for me I will do that for v2.

> Also, why bother with the #ifdef if this function is never called
> outside sandbox?

Because its call site does not have a #ifdef, I'll add an #ifdef at its
call site to make it more clear that this is only used for emulated
devices.

>
>>          struct udevice *dev;
>>
>>          *devp = NULL;
>> @@ -509,6 +518,7 @@ static int usb_find_child(struct udevice *parent,
>>                          return 0;
>>                  }
>>          }
>> +#endif
>>
>>          return -ENOENT;
>>   }
>> --
>> 2.4.3
>>
>
> Regards,
> Simon
>

Regards,

Hans

[U-Boot] [PATCH 03/22] usb: usb_setup_device: Drop unneeded portnr function argument

[Simon Glass]

Hi Hans,

On 30 June 2015 at 06:29, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
>
> On 29-06-15 05:44, Simon Glass wrote:
>>
>> Hi Hans.
>>
>> On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>> Drop the unneeded portnr function argument, the portnr is part of the
>>> usb_device struct which is passed via the dev argument.
>>>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>>   common/usb.c                  | 10 +++++-----
>>>   drivers/usb/host/usb-uclass.c |  2 +-
>>>   include/usb.h                 |  6 +++---
>>>   3 files changed, 9 insertions(+), 9 deletions(-)
>>
>>
>> This was needed in the case where a fake usb_device was passed in. Has
>> your previous refactoring changed that?
>
>
> The portnr is still passed but it is padded via the usb_device struct's
> portnr member. When doing a CONFIG_DM_USB=y build the only call site of
> usb_setup_device() is usb_scan_device() from drivers/usb/host/usb-uclass.c
> which does:
>
>         udev->portnr = port;
>         debug("Calling usb_setup_device(), portnr=%d\n", udev->portnr);
>         parent_udev = device_get_uclass_id(parent) == UCLASS_USB_HUB ?
>                 dev_get_parentdata(parent) : NULL;
>         ret = usb_setup_device(udev, priv->desc_before_addr, parent_udev);
>
> So portnr is always set in the usb_device strict, and that is what gets
> used after this patch.

OK thanks for explaining that.

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

Regards,
Simon

[U-Boot] [PATCH 04/22] usb: Pass device instead of portnr to usb_legacy_port_reset

[Simon Glass]

Hi Hans,

On 30 June 2015 at 06:31, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
>
> On 29-06-15 05:44, Simon Glass wrote:
>>
>> Hi Hans,
>>
>> On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>> Pass the usb_device instead of the portnr to usb_legacy_port_reset and
>>> rename it to usb_hub_port_reset as there is nothing legacy about it.
>>>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>>   common/usb.c | 8 ++++----
>>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>>
>> Legacy as in not driver model.
>
>
> Except that it gets used in both device-model and non device model
> builds of the usb-stack.

Yes, and the non-driver-model stuff can presumably be considered
'legacy' at this point?

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

>
> Regards,
>
> Hans
>
> p.s.
>
> Thanks for reviewing this largish series!

You're welcome. It's a lot easier than writing it :-)

Regards,
Simon

[U-Boot] [PATCH 08/22] dm: usb: Use device_chld_remove and _unbind to clean up usb devs on stop

[Simon Glass]

Hi Hans,

On 30 June 2015 at 06:54, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 29-06-15 05:45, Simon Glass wrote:
>>
>> Hi Hans,
>>
>> On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>> On an usb stop instead of leaving orphan usb devices behind simply remove
>>
>>
>> On a usb_stop()
>>
>> or
>>
>> On a 'usb stop' command  ?
>
>
> My intention was for both, since I was under the assumption that "usb stop"
> on the cmdline, was the only caller of usb_stop(), but a quick grep to the
> sources show that I'm wrong ...
>
>>> them. This requires CONFIG_DM_DEVICE_REMOVE to be set, so only build
>>> usb_stop() when that is set.
>>
>>
>> This seems a little unfortunate. I can see the reasoning, but do you
>> think this is necessary? I suspect people chasing code size may remove
>> that option and still want to use USB properly.
>
>
> This was mostly a result of my thinking that usb_stop() is only used
> on "usb stop" at the cmdline, which I know realize is wrong.
>
> However my quick grep has learned that we do really need
> CONFIG_DM_DEVICE_REMOVE
> to properly implement usb_stop():
>
> From common/bootm.c :
>
> #if defined(CONFIG_CMD_USB)
>         /*
>          * turn off USB to prevent the host controller from writing to the
>          * SDRAM while Linux is booting. This could happen (at least for
> OHCI
>          * controller), because the HCCA (Host Controller Communication
> Area)
>          * lies within the SDRAM and the host controller writes continously
> to
>          * this area (as busmaster!). The HccaFrameNumber is for example
>          * updated every 1 ms within the HCCA structure in SDRAM! For more
>          * details see the OpenHCI specification.
>          */
>         usb_stop();
> #endif
>
> And without CONFIG_DM_DEVICE_REMOVE we end up never calling the hcd's remove
> callback and thus do not properly stop the usb controller.
>
> So this problem of usb_stop() needing CONFIG_DM_DEVICE_REMOVE already exists
> before this patch. If you want I can split out the adding of the #ifdef
> in a separate commit, spelling out why usb_stop() MUST have
> CONFIG_DM_DEVICE_REMOVE in the commit message. Or maybe just move this all
> to
> Kconfig and make DM_USB conflict with CONFIG_DM_DEVICE_REMOVE?
>

I don't think that is necessary, it feels a bit too inflexible. But
perhaps you could add a comment to the Kconfig help for
CONFIG_DM_DEVICE_REMOVE?

It is remove() that is needed, not unbind(). Actually I think it is
quite unfortunate to make usb_stop() call unbind. It is a waste of
time to do this just before booting the kernel - the current design
leaves all devices bound (but I hope we can remove() them at some
point).

Instead, I wonder if we can remove the children when we probe the bus?
Also, what happens to children that are in the device tree - i.e.
static USB devices like WiFi? The device tree might have parameters
for them. Still, that might not matter as I'm not sure that case is
handled correctly today.

>
>
>>
>>>
>>> The result of this commit is best seen in the output of "dm tree" after
>>> plugging out an usb hub with 2 devices plugges in and plugging in a keyb.
>>> instead, before this commit the output would be:
>>>
>>>   usb         [ + ]    `-- sunxi-musb
>>>   usb_hub     [   ]        |-- usb_hub
>>>   usb_mass_st [   ]        |   |-- usb_mass_storage
>>>   usb_dev_gen [   ]        |   `-- generic_bus_0_dev_3
>>>   usb_dev_gen [ + ]        `-- generic_bus_0_dev_1
>>>
>>> Notice the non active usb_hub child and its 2 non active children. The
>>> first child being non-active as in this example also causes
>>> usb_get_dev_index
>>> to return NULL when probing the first child, which results in the usb kbd
>>> code not binding to the keyboard.
>>
>>
>> Although I suspect that could be fixed.
>
>
> Right, but just removing the children is a much cleaner solution, and also
> makes the output of "dm tree" properly reflect reality.

True, although you also have 'usb tree' for that. Another option would
be to mark devices that were found and remove the others after the
scan.

>
>
>>
>>>
>>> With this commit in place the output after swapping and "usb reset" is:
>>>
>>>   usb         [ + ]    `-- sunxi-musb
>>>   usb_dev_gen [ + ]        `-- generic_bus_0_dev_1
>>>
>>> As expected, and usb_get_dev_index works properly and the keyboard works.
>>>
>>> After this commit usb_find_child() is only necessary for emulated usb
>>> devices, so make its body #ifdef CONFIG_USB_EMUL.
>>>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>>   drivers/usb/host/usb-uclass.c | 10 ++++++++++
>>>   1 file changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/usb/host/usb-uclass.c
>>> b/drivers/usb/host/usb-uclass.c
>>> index bce6cec..8f26e35 100644
>>> --- a/drivers/usb/host/usb-uclass.c
>>> +++ b/drivers/usb/host/usb-uclass.c
>>> @@ -128,6 +128,7 @@ int usb_alloc_device(struct usb_device *udev)
>>>          return ops->alloc_device(bus, udev);
>>>   }
>>>
>>> +#ifdef CONFIG_DM_DEVICE_REMOVE
>>>   int usb_stop(void)
>>>   {
>>>          struct udevice *bus;
>>> @@ -143,6 +144,12 @@ int usb_stop(void)
>>>          uc_priv = uc->priv;
>>>
>>>          uclass_foreach_dev(bus, uc) {
>>> +               ret = device_chld_remove(bus);
>>> +               if (ret && !err)
>>> +                       err = ret;
>>> +               ret = device_chld_unbind(bus);
>>> +               if (ret && !err)
>>> +                       err = ret;
>>>                  ret = device_remove(bus);
>>>                  if (ret && !err)
>>>                          err = ret;
>>> @@ -166,6 +173,7 @@ int usb_stop(void)
>>>
>>>          return err;
>>>   }
>>> +#endif
>>>
>>>   static void usb_scan_bus(struct udevice *bus, bool recurse)
>>>   {
>>> @@ -491,6 +499,7 @@ static int usb_find_child(struct udevice *parent,
>>>                            struct usb_interface_descriptor *iface,
>>>                            struct udevice **devp)
>>>   {
>>> +#ifdef CONFIG_USB_EMUL /* Emulated devices are explictily bound */
>>
>>
>> explicitly
>
>
> Ack.
>
>> Can you add a comment about this? It seems that we should rename this
>> function to usb_find_emul_child() and have it present only when
>> CONFIG_USB_EMUL is around?
>
>
> Renaming it to usb_find_emul_child() and only defining the function when
> CONFIG_USB_EMUL works for me I will do that for v2.
>
>> Also, why bother with the #ifdef if this function is never called
>> outside sandbox?
>
>
> Because its call site does not have a #ifdef, I'll add an #ifdef at its
> call site to make it more clear that this is only used for emulated
> devices.
>
>>
>>>          struct udevice *dev;
>>>
>>>          *devp = NULL;
>>> @@ -509,6 +518,7 @@ static int usb_find_child(struct udevice *parent,
>>>                          return 0;
>>>                  }
>>>          }
>>> +#endif
>>>
>>>          return -ENOENT;
>>>   }
>>> --
>>> 2.4.3
>>>

Regards,
Simon

[U-Boot] [PATCH 08/22] dm: usb: Use device_chld_remove and _unbind to clean up usb devs on stop

[Hans de Goede]

Hi,

On 06/30/2015 04:58 PM, Simon Glass wrote:
> Hi Hans,
>
> On 30 June 2015 at 06:54, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>> On 29-06-15 05:45, Simon Glass wrote:
>>>
>>> Hi Hans,
>>>
>>> On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>
>>>> On an usb stop instead of leaving orphan usb devices behind simply remove
>>>
>>>
>>> On a usb_stop()
>>>
>>> or
>>>
>>> On a 'usb stop' command  ?
>>
>>
>> My intention was for both, since I was under the assumption that "usb stop"
>> on the cmdline, was the only caller of usb_stop(), but a quick grep to the
>> sources show that I'm wrong ...
>>
>>>> them. This requires CONFIG_DM_DEVICE_REMOVE to be set, so only build
>>>> usb_stop() when that is set.
>>>
>>>
>>> This seems a little unfortunate. I can see the reasoning, but do you
>>> think this is necessary? I suspect people chasing code size may remove
>>> that option and still want to use USB properly.
>>
>>
>> This was mostly a result of my thinking that usb_stop() is only used
>> on "usb stop" at the cmdline, which I know realize is wrong.
>>
>> However my quick grep has learned that we do really need
>> CONFIG_DM_DEVICE_REMOVE
>> to properly implement usb_stop():
>>
>>  From common/bootm.c :
>>
>> #if defined(CONFIG_CMD_USB)
>>          /*
>>           * turn off USB to prevent the host controller from writing to the
>>           * SDRAM while Linux is booting. This could happen (at least for
>> OHCI
>>           * controller), because the HCCA (Host Controller Communication
>> Area)
>>           * lies within the SDRAM and the host controller writes continously
>> to
>>           * this area (as busmaster!). The HccaFrameNumber is for example
>>           * updated every 1 ms within the HCCA structure in SDRAM! For more
>>           * details see the OpenHCI specification.
>>           */
>>          usb_stop();
>> #endif
>>
>> And without CONFIG_DM_DEVICE_REMOVE we end up never calling the hcd's remove
>> callback and thus do not properly stop the usb controller.
>>
>> So this problem of usb_stop() needing CONFIG_DM_DEVICE_REMOVE already exists
>> before this patch. If you want I can split out the adding of the #ifdef
>> in a separate commit, spelling out why usb_stop() MUST have
>> CONFIG_DM_DEVICE_REMOVE in the commit message. Or maybe just move this all
>> to
>> Kconfig and make DM_USB conflict with CONFIG_DM_DEVICE_REMOVE?
>>
>
> I don't think that is necessary, it feels a bit too inflexible. But
> perhaps you could add a comment to the Kconfig help for
> CONFIG_DM_DEVICE_REMOVE?

Ok will do.

> It is remove() that is needed, not unbind(). Actually I think it is
> quite unfortunate to make usb_stop() call unbind. It is a waste of
> time to do this just before booting the kernel - the current design
> leaves all devices bound (but I hope we can remove() them at some
> point).
>
> Instead, I wonder if we can remove the children when we probe the bus?

That should work, but I do not really see any advantage in that,
removing the children is not that expensive and it feels like a
kludge.

> Also, what happens to children that are in the device tree - i.e.
> static USB devices like WiFi? The device tree might have parameters
> for them. Still, that might not matter as I'm not sure that case is
> handled correctly today.

AFAIK there is no such thing as usb devices in devicetree, which
makes sense as usb is a fully discoverable bus.



>
>>
>>
>>>
>>>>
>>>> The result of this commit is best seen in the output of "dm tree" after
>>>> plugging out an usb hub with 2 devices plugges in and plugging in a keyb.
>>>> instead, before this commit the output would be:
>>>>
>>>>    usb         [ + ]    `-- sunxi-musb
>>>>    usb_hub     [   ]        |-- usb_hub
>>>>    usb_mass_st [   ]        |   |-- usb_mass_storage
>>>>    usb_dev_gen [   ]        |   `-- generic_bus_0_dev_3
>>>>    usb_dev_gen [ + ]        `-- generic_bus_0_dev_1
>>>>
>>>> Notice the non active usb_hub child and its 2 non active children. The
>>>> first child being non-active as in this example also causes
>>>> usb_get_dev_index
>>>> to return NULL when probing the first child, which results in the usb kbd
>>>> code not binding to the keyboard.
>>>
>>>
>>> Although I suspect that could be fixed.
>>
>>
>> Right, but just removing the children is a much cleaner solution, and also
>> makes the output of "dm tree" properly reflect reality.
>
> True, although you also have 'usb tree' for that. Another option would
> be to mark devices that were found and remove the others after the
> scan.

That seems like needless complexity. I believe that simply removing + unbinding
the children on usb_stop is the right thing to do, and it also is the KISS
solution.

Regards,

Hans

[U-Boot] [PATCH 08/22] dm: usb: Use device_chld_remove and _unbind to clean up usb devs on stop

[Simon Glass]

Hi Hans,

On 30 June 2015 at 09:54, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
>
> On 06/30/2015 04:58 PM, Simon Glass wrote:
>>
>> Hi Hans,
>>
>> On 30 June 2015 at 06:54, Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>> Hi,
>>>
>>> On 29-06-15 05:45, Simon Glass wrote:
>>>>
>>>>
>>>> Hi Hans,
>>>>
>>>> On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>
>>>>>
>>>>> On an usb stop instead of leaving orphan usb devices behind simply
>>>>> remove
>>>>
>>>>
>>>>
>>>> On a usb_stop()
>>>>
>>>> or
>>>>
>>>> On a 'usb stop' command  ?
>>>
>>>
>>>
>>> My intention was for both, since I was under the assumption that "usb
>>> stop"
>>> on the cmdline, was the only caller of usb_stop(), but a quick grep to
>>> the
>>> sources show that I'm wrong ...
>>>
>>>>> them. This requires CONFIG_DM_DEVICE_REMOVE to be set, so only build
>>>>> usb_stop() when that is set.
>>>>
>>>>
>>>>
>>>> This seems a little unfortunate. I can see the reasoning, but do you
>>>> think this is necessary? I suspect people chasing code size may remove
>>>> that option and still want to use USB properly.
>>>
>>>
>>>
>>> This was mostly a result of my thinking that usb_stop() is only used
>>> on "usb stop" at the cmdline, which I know realize is wrong.
>>>
>>> However my quick grep has learned that we do really need
>>> CONFIG_DM_DEVICE_REMOVE
>>> to properly implement usb_stop():
>>>
>>>  From common/bootm.c :
>>>
>>> #if defined(CONFIG_CMD_USB)
>>>          /*
>>>           * turn off USB to prevent the host controller from writing to
>>> the
>>>           * SDRAM while Linux is booting. This could happen (at least for
>>> OHCI
>>>           * controller), because the HCCA (Host Controller Communication
>>> Area)
>>>           * lies within the SDRAM and the host controller writes
>>> continously
>>> to
>>>           * this area (as busmaster!). The HccaFrameNumber is for example
>>>           * updated every 1 ms within the HCCA structure in SDRAM! For
>>> more
>>>           * details see the OpenHCI specification.
>>>           */
>>>          usb_stop();
>>> #endif
>>>
>>> And without CONFIG_DM_DEVICE_REMOVE we end up never calling the hcd's
>>> remove
>>> callback and thus do not properly stop the usb controller.
>>>
>>> So this problem of usb_stop() needing CONFIG_DM_DEVICE_REMOVE already
>>> exists
>>> before this patch. If you want I can split out the adding of the #ifdef
>>> in a separate commit, spelling out why usb_stop() MUST have
>>> CONFIG_DM_DEVICE_REMOVE in the commit message. Or maybe just move this
>>> all
>>> to
>>> Kconfig and make DM_USB conflict with CONFIG_DM_DEVICE_REMOVE?
>>>
>>
>> I don't think that is necessary, it feels a bit too inflexible. But
>> perhaps you could add a comment to the Kconfig help for
>> CONFIG_DM_DEVICE_REMOVE?
>
>
> Ok will do.
>
>> It is remove() that is needed, not unbind(). Actually I think it is
>> quite unfortunate to make usb_stop() call unbind. It is a waste of
>> time to do this just before booting the kernel - the current design
>> leaves all devices bound (but I hope we can remove() them at some
>> point).
>>
>> Instead, I wonder if we can remove the children when we probe the bus?
>
>
> That should work, but I do not really see any advantage in that,
> removing the children is not that expensive and it feels like a
> kludge.

That's how it currently works, from what I can see in the code. But
since there is a 'usb_started' boolean this is irrelevant.

>
>> Also, what happens to children that are in the device tree - i.e.
>> static USB devices like WiFi? The device tree might have parameters
>> for them. Still, that might not matter as I'm not sure that case is
>> handled correctly today.
>
>
> AFAIK there is no such thing as usb devices in devicetree, which
> makes sense as usb is a fully discoverable bus.

Sort-of. But as with PCI it is useful to be able to add settings for
the devices in some cases. You can match them using vendor/device or
interface IDs. Then the driver can access its settings.

That's why I'm suggesting we unbind the devices that are no-longer present.

>
>
>
>>
>>>
>>>
>>>>
>>>>>
>>>>> The result of this commit is best seen in the output of "dm tree" after
>>>>> plugging out an usb hub with 2 devices plugges in and plugging in a
>>>>> keyb.
>>>>> instead, before this commit the output would be:
>>>>>
>>>>>    usb         [ + ]    `-- sunxi-musb
>>>>>    usb_hub     [   ]        |-- usb_hub
>>>>>    usb_mass_st [   ]        |   |-- usb_mass_storage
>>>>>    usb_dev_gen [   ]        |   `-- generic_bus_0_dev_3
>>>>>    usb_dev_gen [ + ]        `-- generic_bus_0_dev_1
>>>>>
>>>>> Notice the non active usb_hub child and its 2 non active children. The
>>>>> first child being non-active as in this example also causes
>>>>> usb_get_dev_index
>>>>> to return NULL when probing the first child, which results in the usb
>>>>> kbd
>>>>> code not binding to the keyboard.
>>>>
>>>>
>>>>
>>>> Although I suspect that could be fixed.
>>>
>>>
>>>
>>> Right, but just removing the children is a much cleaner solution, and
>>> also
>>> makes the output of "dm tree" properly reflect reality.
>>
>>
>> True, although you also have 'usb tree' for that. Another option would
>> be to mark devices that were found and remove the others after the
>> scan.
>
>
> That seems like needless complexity. I believe that simply removing +
> unbinding
> the children on usb_stop is the right thing to do, and it also is the KISS
> solution.

I'm good with the remove(), but less sure about the unbind(). The
state of 'dm tree' does not bother me, and I worry that we then limit
our ability to use usb_find_child() to locate a device's parameters
(i.e. support for more complex devices which need settings might be
harder).

For now, can we just leave this alone? I really don't want to re-visit
this later.

Regards,
Simon

[U-Boot] [PATCH 08/22] dm: usb: Use device_chld_remove and _unbind to clean up usb devs on stop

[Hans de Goede]

Hi,

On 06/30/2015 06:07 PM, Simon Glass wrote:

<snip>

>>> Instead, I wonder if we can remove the children when we probe the bus?
>>
>>
>> That should work, but I do not really see any advantage in that,
>> removing the children is not that expensive and it feels like a
>> kludge.
>
> That's how it currently works, from what I can see in the code. But
> since there is a 'usb_started' boolean this is irrelevant.
>
>>
>>> Also, what happens to children that are in the device tree - i.e.
>>> static USB devices like WiFi? The device tree might have parameters
>>> for them. Still, that might not matter as I'm not sure that case is
>>> handled correctly today.
>>
>>
>> AFAIK there is no such thing as usb devices in devicetree, which
>> makes sense as usb is a fully discoverable bus.
>
> Sort-of. But as with PCI it is useful to be able to add settings for
> the devices in some cases. You can match them using vendor/device or
> interface IDs. Then the driver can access its settings.

AFAIK there is not a single example of having settings in devicetree
for an usb device, since usb-devices are always 100% self describing
since usb is a bus designed for hot(un)plug from the outset.

> That's why I'm suggesting we unbind the devices that are no-longer present.

You're asking to make the code more complicated here using a what if
reasoning with a "what if" which is likely to never happen.

>
>>
>>
>>
>>>
>>>>
>>>>
>>>>>
>>>>>>
>>>>>> The result of this commit is best seen in the output of "dm tree" after
>>>>>> plugging out an usb hub with 2 devices plugges in and plugging in a
>>>>>> keyb.
>>>>>> instead, before this commit the output would be:
>>>>>>
>>>>>>     usb         [ + ]    `-- sunxi-musb
>>>>>>     usb_hub     [   ]        |-- usb_hub
>>>>>>     usb_mass_st [   ]        |   |-- usb_mass_storage
>>>>>>     usb_dev_gen [   ]        |   `-- generic_bus_0_dev_3
>>>>>>     usb_dev_gen [ + ]        `-- generic_bus_0_dev_1
>>>>>>
>>>>>> Notice the non active usb_hub child and its 2 non active children. The
>>>>>> first child being non-active as in this example also causes
>>>>>> usb_get_dev_index
>>>>>> to return NULL when probing the first child, which results in the usb
>>>>>> kbd
>>>>>> code not binding to the keyboard.
>>>>>
>>>>>
>>>>>
>>>>> Although I suspect that could be fixed.
>>>>
>>>>
>>>>
>>>> Right, but just removing the children is a much cleaner solution, and
>>>> also
>>>> makes the output of "dm tree" properly reflect reality.
>>>
>>>
>>> True, although you also have 'usb tree' for that. Another option would
>>> be to mark devices that were found and remove the others after the
>>> scan.
>>
>>
>> That seems like needless complexity. I believe that simply removing +
>> unbinding
>> the children on usb_stop is the right thing to do, and it also is the KISS
>> solution.
>
> I'm good with the remove(), but less sure about the unbind().

The unbind is necessary for usb_get_dev_index() to work properly,
which is necessary for proper output of "usb tree" and for driver
binding to work properly, without the unbind usb-keyboards will e.g.
not work in certain circumstances.

> The
> state of 'dm tree' does not bother me,

The state if dm tree is what usb_get_dev_index() works from, so if
it is not in a good state, then usb_get_dev_index() will not work.

> and I worry that we then limit
> our ability to use usb_find_child() to locate a device's parameters
> (i.e. support for more complex devices which need settings might be
> harder).

Again this is a what if reasoning for a hypothetical future problem
which will likely never happen, where as the broken state of the
dm tree after a "usb reset" is causing real problems.

> For now, can we just leave this alone? I really don't want to re-visit
> this later.

Nope we cannot leave this alone, without unbinding usb devices which
no longer exist, the dm tree will be broken and with it
usb_get_dev_index() and through usb_get_dev_index() the keyboard
driver.

Regards,

Hans

[U-Boot] [PATCH 08/22] dm: usb: Use device_chld_remove and _unbind to clean up usb devs on stop

[Simon Glass]

HI Hans,

On 30 June 2015 at 14:20, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 06/30/2015 06:07 PM, Simon Glass wrote:
>
> <snip>
>
>>>> Instead, I wonder if we can remove the children when we probe the bus?
>>>
>>>
>>>
>>> That should work, but I do not really see any advantage in that,
>>> removing the children is not that expensive and it feels like a
>>> kludge.
>>
>>
>> That's how it currently works, from what I can see in the code. But
>> since there is a 'usb_started' boolean this is irrelevant.
>>
>>>
>>>> Also, what happens to children that are in the device tree - i.e.
>>>> static USB devices like WiFi? The device tree might have parameters
>>>> for them. Still, that might not matter as I'm not sure that case is
>>>> handled correctly today.
>>>
>>>
>>>
>>> AFAIK there is no such thing as usb devices in devicetree, which
>>> makes sense as usb is a fully discoverable bus.
>>
>>
>> Sort-of. But as with PCI it is useful to be able to add settings for
>> the devices in some cases. You can match them using vendor/device or
>> interface IDs. Then the driver can access its settings.
>
>
> AFAIK there is not a single example of having settings in devicetree
> for an usb device, since usb-devices are always 100% self describing
> since usb is a bus designed for hot(un)plug from the outset.



>
>> That's why I'm suggesting we unbind the devices that are no-longer
>> present.
>
>
> You're asking to make the code more complicated here using a what if
> reasoning with a "what if" which is likely to never happen.
>
>
>>
>>>
>>>
>>>
>>>>
>>>>>
>>>>>
>>>>>>
>>>>>>>
>>>>>>> The result of this commit is best seen in the output of "dm tree"
>>>>>>> after
>>>>>>> plugging out an usb hub with 2 devices plugges in and plugging in a
>>>>>>> keyb.
>>>>>>> instead, before this commit the output would be:
>>>>>>>
>>>>>>>     usb         [ + ]    `-- sunxi-musb
>>>>>>>     usb_hub     [   ]        |-- usb_hub
>>>>>>>     usb_mass_st [   ]        |   |-- usb_mass_storage
>>>>>>>     usb_dev_gen [   ]        |   `-- generic_bus_0_dev_3
>>>>>>>     usb_dev_gen [ + ]        `-- generic_bus_0_dev_1
>>>>>>>
>>>>>>> Notice the non active usb_hub child and its 2 non active children.
>>>>>>> The
>>>>>>> first child being non-active as in this example also causes
>>>>>>> usb_get_dev_index
>>>>>>> to return NULL when probing the first child, which results in the usb
>>>>>>> kbd
>>>>>>> code not binding to the keyboard.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Although I suspect that could be fixed.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Right, but just removing the children is a much cleaner solution, and
>>>>> also
>>>>> makes the output of "dm tree" properly reflect reality.
>>>>
>>>>
>>>>
>>>> True, although you also have 'usb tree' for that. Another option would
>>>> be to mark devices that were found and remove the others after the
>>>> scan.
>>>
>>>
>>>
>>> That seems like needless complexity. I believe that simply removing +
>>> unbinding
>>> the children on usb_stop is the right thing to do, and it also is the
>>> KISS
>>> solution.
>>
>>
>> I'm good with the remove(), but less sure about the unbind().
>
>
> The unbind is necessary for usb_get_dev_index() to work properly,
> which is necessary for proper output of "usb tree" and for driver
> binding to work properly, without the unbind usb-keyboards will e.g.
> not work in certain circumstances.
>
>> The
>> state of 'dm tree' does not bother me,
>
>
> The state if dm tree is what usb_get_dev_index() works from, so if
> it is not in a good state, then usb_get_dev_index() will not work.
>
>> and I worry that we then limit
>> our ability to use usb_find_child() to locate a device's parameters
>> (i.e. support for more complex devices which need settings might be
>> harder).
>
>
> Again this is a what if reasoning for a hypothetical future problem
> which will likely never happen, where as the broken state of the
> dm tree after a "usb reset" is causing real problems.
>
>> For now, can we just leave this alone? I really don't want to re-visit
>> this later.
>
>
> Nope we cannot leave this alone, without unbinding usb devices which
> no longer exist, the dm tree will be broken and with it
> usb_get_dev_index() and through usb_get_dev_index() the keyboard
> driver.

We really should not be calling usb_get_dev_index() from the keyboard
driver - that function is an interim port-helper. If it were ported
fully to driver model it could use USB_DEVICE() like mass storage. We
could fairly easily change the devnum to 0 for all devices instead of
deleting them, and I suspect that would solve the problem.

But we would have to port usb ethernet which uses the same technique.

Anyway I think you've persuaded me that I might be worrying too much
about what is to come. USB is not really the same as PCI in the sense
that settings don't seem to be added to the device like PCI. Let's go
with your approach. We still have the USB emulation stuff working, and
that's the only real use case today.

Regards,
Simon

[U-Boot] [PATCH 12/22] musb: Update usb-compat to work with struct usb_device without a parent ptr

[Hans de Goede]

Hi,

On 29-06-15 05:45, Simon Glass wrote:
> Hi Hans,
>
> On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
>> When building with CONFIG_DM_USB=y struct usb_device does not have a parent
>> pointer. This commit adds support to the musb code to deal with this.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/usb/musb-new/musb_host.c  |  4 +++
>>   drivers/usb/musb-new/musb_uboot.c |  2 +-
>>   drivers/usb/musb-new/usb-compat.h | 70 +++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 75 insertions(+), 1 deletion(-)
>>
>
> Acked-by: Simon Glass <sjg@chromium.org>
>
> See note below.
>
>> diff --git a/drivers/usb/musb-new/musb_host.c b/drivers/usb/musb-new/musb_host.c
>> index 437309c..40b9c66 100644
>> --- a/drivers/usb/musb-new/musb_host.c
>> +++ b/drivers/usb/musb-new/musb_host.c
>> @@ -2067,7 +2067,11 @@ int musb_urb_enqueue(
>>
>>          /* precompute addressing for external hub/tt ports */
>>          if (musb->is_multipoint) {
>> +#ifndef __UBOOT__
>>                  struct usb_device       *parent = urb->dev->parent;
>> +#else
>> +               struct usb_device       *parent = usb_dev_get_parent(urb->dev);
>> +#endif
>>
>>   #ifndef __UBOOT__
>>                  if (parent != hcd->self.root_hub) {
>> diff --git a/drivers/usb/musb-new/musb_uboot.c b/drivers/usb/musb-new/musb_uboot.c
>> index 70e87c9..a96e8d2 100644
>> --- a/drivers/usb/musb-new/musb_uboot.c
>> +++ b/drivers/usb/musb-new/musb_uboot.c
>> @@ -97,7 +97,7 @@ int submit_control_msg(struct usb_device *dev, unsigned long pipe,
>>                        buffer, len, setup, 0);
>>
>>          /* Fix speed for non hub-attached devices */
>> -       if (!dev->parent)
>> +       if (!usb_dev_get_parent(dev))
>>                  dev->speed = host_speed;
>>
>>          return submit_urb(&hcd, &urb);
>> diff --git a/drivers/usb/musb-new/usb-compat.h b/drivers/usb/musb-new/usb-compat.h
>> index 50bad37..53fe4ff 100644
>> --- a/drivers/usb/musb-new/usb-compat.h
>> +++ b/drivers/usb/musb-new/usb-compat.h
>> @@ -1,6 +1,7 @@
>>   #ifndef __USB_COMPAT_H__
>>   #define __USB_COMPAT_H__
>>
>> +#include <dm.h>
>>   #include "usb.h"
>>
>>   struct usb_hcd {
>> @@ -66,6 +67,68 @@ static inline int usb_hcd_unmap_urb_for_dma(struct usb_hcd *hcd,
>>          return 0;
>>   }
>>
>> +#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_parentdata(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_parentdata(dev);
>> +               parent = dev->parent;
>> +               uparent = dev_get_parentdata(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;
>> +
>> +       /*
>> +        * 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.
>
> Another option here is to somehow allow devices to be added before we
> know what they are. In this case we could bind a 'generic' USB device
> (UCLASS_USB_DEV_GENERIC). Then when we work out what it is, we could
> unbind it (without throwing away the udevice and usb_device) and have
> it bind again as the correct device. Something like
> device_morph_child().

Right, I think that may end up being cleaner in the long term.

Regards,

Hans

[U-Boot] [PATCH 01/22] usb: Always declare usb function prototypes

[Simon Glass]

On 28 June 2015 at 21:44, Simon Glass <sjg@chromium.org> wrote:
> On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
>> There is no harm in declaring the function prototypes even if nothing
>> implements them, and when CONFIG_DM_USB=y the various usb functions are
>> available regardless of any controller drivers being enabled.
>>
>> This fixes compile warnings due to missing prototypes on ARCHs where
>> the ARCH Kconfig always enables CONFIG_DM_USB and various usb drivers.
>>
>> One could argue that in the case of no controllers CONFIG_DM_USB should not
>> be set, but this problem is typically seen during bringup of boards which
>> do actually have usb controllers.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Changes in v2:
>> -Simply always define the function prototypes instead of adding yet another
>>  condition to the already unwieldly #if def ... || def ... condition
>> ---
>>  include/usb.h | 15 ---------------
>>  1 file changed, 15 deletions(-)
>
> Acked-by: Simon Glass <sjg@chromium.org>

Note: this was already applied to mainline.

[U-Boot] [PATCH 02/22] usb: Drop device-model specific copy of usb_legacy_port_reset

[Simon Glass]

On 28 June 2015 at 21:44, Simon Glass <sjg@chromium.org> wrote:
> On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
>> The device-model usb_legacy_port_reset function cApplied to u-boot-dm, thanks!
alls the device-model
>> usb_port_reset function which is a 1 on 1 copy of the non dm
>> usb_legacy_port_reset and this is the only use of usb_port_reset in all
>> of u-boot.
>>
>> Drop both, and alway use the usb_legacy_port_reset() version in
>> common/usb.c .
>>
>> Also while at it make it static as it is only used in common/usb.c .
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  common/usb.c                  |  4 +---
>>  drivers/usb/host/usb-uclass.c | 29 -----------------------------
>>  include/usb.h                 |  8 --------
>>  3 files changed, 1 insertion(+), 40 deletions(-)
>
> Acked-by: Simon Glass <sjg@chromium.org>

Applied to u-boot-dm/next, thanks!

[U-Boot] [PATCH 03/22] usb: usb_setup_device: Drop unneeded portnr function argument

[Simon Glass]

On 30 June 2015 at 08:51, Simon Glass <sjg@chromium.org> wrote:
> Hi Hans,
>
> On 30 June 2015 at 06:29, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>>
>> On 29-06-15 05:44, Simon Glass wrote:
>>>
>>> Hi Hans.
>>>
>>> On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>
>>>> Drop the unneeded portnr function argument, the portnr is part of the
>>>> usb_device struct which is passed via the dev argument.
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>> ---
>>>>   common/usb.c                  | 10 +++++-----
>>>>   drivers/usb/host/usb-uclass.c |  2 +-
>>>>   include/usb.h                 |  6 +++---
>>>>   3 files changed, 9 insertions(+), 9 deletions(-)
>>>
>>>
>>> This was needed in the case where a fake usb_device was passed in. Has
>>> your previous refactoring changed that?
>>
>>
>> The portnr is still passed but it is padded via the usb_device struct's
>> portnr member. When doing a CONFIG_DM_USB=y build the only call site of
>> usb_setup_device() is usb_scan_device() from drivers/usb/host/usb-uclass.c
>> which does:
>>
>>         udev->portnr = port;
>>         debug("Calling usb_setup_device(), portnr=%d\n", udev->portnr);
>>         parent_udev = device_get_uclass_id(parent) == UCLASS_USB_HUB ?
>>                 dev_get_parentdata(parent) : NULL;
>>         ret = usb_setup_device(udev, priv->desc_before_addr, parent_udev);
>>
>> So portnr is always set in the usb_device strict, and that is what gets
>> used after this patch.
>
> OK thanks for explaining that.
>
> Acked-by: Simon Glass <sjg@chromium.org>

Applied to u-boot-dm/next, thanks!

[U-Boot] [PATCH 04/22] usb: Pass device instead of portnr to usb_legacy_port_reset

[Simon Glass]

On 30 June 2015 at 08:58, Simon Glass <sjg@chromium.org> wrote:
> Hi Hans,
>
> On 30 June 2015 at 06:31, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>>
>> On 29-06-15 05:44, Simon Glass wrote:
>>>
>>> Hi Hans,
>>>
>>> On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>
>>>> Pass the usb_device instead of the portnr to usb_legacy_port_reset and
>>>> rename it to usb_hub_port_reset as there is nothing legacy about it.
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>> ---
>>>>   common/usb.c | 8 ++++----
>>>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>>
>>> Legacy as in not driver model.
>>
>>
>> Except that it gets used in both device-model and non device model
>> builds of the usb-stack.
>
> Yes, and the non-driver-model stuff can presumably be considered
> 'legacy' at this point?
>
> Acked-by: Simon Glass <sjg@chromium.org>
>
>>
>> Regards,
>>
>> Hans
>>
>> p.s.
>>
>> Thanks for reviewing this largish series!
>
> You're welcome. It's a lot easier than writing it :-)
>
> Regards,
> Simon

Applied to u-boot-dm/next, thanks!

[U-Boot] [PATCH 05/22] usb: Add an usb_device parameter to usb_reset_root_port

[Simon Glass]

On 28 June 2015 at 21:45, Simon Glass <sjg@chromium.org> wrote:
> On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
>> Add an usb_device parameter to usb_reset_root_port so that it knows which
>> root-port it is resetting. This is necessary for proper device-model support
>> for usb_reset_root_port.
>>
>> Also remove a duplicate declaration of usb_reset_root_port() from usb.h .
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  common/usb.c                      | 2 +-
>>  drivers/usb/host/usb-uclass.c     | 2 +-
>>  drivers/usb/musb-new/musb_uboot.c | 4 ++--
>>  include/usb.h                     | 8 ++------
>>  4 files changed, 6 insertions(+), 10 deletions(-)
>
> Acked-by: Simon Glass <sjg@chromium.org>

Applied to u-boot-dm/next, thanks!

[U-Boot] [PATCH 07/22] dm: usb: Fix "usb tree" output

[Simon Glass]

On 28 June 2015 at 21:45, Simon Glass <sjg@chromium.org> wrote:
> On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
>> last_child was abused by the old usb code to first store 1 if the
>> usb_device was not the root of the usb tree, and then later on re-used
>> to store whether or not the usb_device is actually the last child.
>>
>> The dm-usb code was always setting it to actually reflect the last-child
>> status which is wrong for the last child leading to output like this:
>>
>> USB device tree:
>>   1  Hub (12 Mb/s, 100mA)
>>   |  ALCOR USB Hub 2.0
>>   |
>>   | 2  Mass Storage (12 Mb/s, 100mA)
>>   |    USB Flash Disk 4C0E960F
>>   |
>>   +-3  Human Interface (1.5 Mb/s, 100mA)
>>        SINO WEALTH USB Composite Device
>>
>> Instead of this:
>>
>> USB device tree:
>>   1  Hub (12 Mb/s, 100mA)
>>   |  ALCOR USB Hub 2.0
>>   |
>>   +-2  Mass Storage (12 Mb/s, 100mA)
>>   |    USB Flash Disk 4C0E960F
>>   |
>>   +-3  Human Interface (1.5 Mb/s, 100mA)
>>        SINO WEALTH USB Composite Device
>>
>> This commit fixes this by first checking that the device is not root,
>> and then setting last_child. This commit also updates the old code to not
>> abuse the last_child variable to store the root check result.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  common/cmd_usb.c | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> Acked-by: Simon Glass <sjg@chromium.org>

Applied to u-boot-dm/next, thanks!

[U-Boot] [PATCH 09/22] dm: usb: Allow usb host drivers to implement usb_reset_root_port

[Simon Glass]

On 28 June 2015 at 21:45, Simon Glass <sjg@chromium.org> wrote:
> On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
>> Allow usb uclass host drivers to implement usb_reset_root_port, this is
>> used by single port usb hosts which do not emulate a hub, such as otg
>> controllers.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/usb/host/usb-uclass.c | 16 +++++++++++-----
>>  include/usb.h                 |  5 +++++
>>  2 files changed, 16 insertions(+), 5 deletions(-)
>
> Acked-by: Simon Glass <sjg@chromium.org>

Applied to u-boot-dm/next, thanks!

[U-Boot] [PATCH 10/22] dm: usb: Do not assume that first child is always a hub

[Simon Glass]

On 28 June 2015 at 21:45, Simon Glass <sjg@chromium.org> wrote:
> On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
>> On some single port (otg) controllers there is no emulated root hub, so
>> the first child (if any) may be one of: UCLASS_MASS_STORAGE,
>> UCLASS_USB_DEV_GENERIC or UCLASS_USB_HUB.
>>
>> All three of these (and in the future others) are suitable for our
>> purposes, remove the check for the device being a hub, and add a check to
>> deal with the fact that there may be no child-dev.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  common/cmd_usb.c              |  9 ++++-----
>>  drivers/usb/host/usb-uclass.c | 10 +++++-----
>>  2 files changed, 9 insertions(+), 10 deletions(-)
>
> Acked-by: Simon Glass <sjg@chromium.org>

Applied to u-boot-dm/next, thanks!

[U-Boot] [PATCH 11/22] musb: Allow musb_platform_enable to return an error code

[Simon Glass]

On 28 June 2015 at 21:45, Simon Glass <sjg@chromium.org> wrote:
> On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
>> Allow musb_platform_enable to return an error code and propagate it up to
>> usb_lowlevel_init().
>>
>> This allows moving the checks for an external vbus being present to be
>> moved from platform_init to platform_enable, so that the user can unplug a
>> charger, plug in a host adapter with a usb-device, do a "usb reset" and
>> have things working.
>>
>> This also allows adding a check for the id-pin to platform_enable, so that
>> it can short circuit the 1s delay in usb_lowlevel_init() when no host cable
>> is plugged in and thus waiting for a device to show up is useless.
>>
>> Note that all the changes to code shared with the kernel are wrapped in
>> the kernel.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/usb/musb-new/am35x.c      |  7 +++++++
>>  drivers/usb/musb-new/musb_core.c  | 20 ++++++++++++++++++++
>>  drivers/usb/musb-new/musb_core.h  | 18 ++++++++++++++++++
>>  drivers/usb/musb-new/musb_dsps.c  |  6 ++++++
>>  drivers/usb/musb-new/musb_uboot.c |  6 +++++-
>>  drivers/usb/musb-new/omap2430.c   |  5 +++++
>>  drivers/usb/musb-new/sunxi.c      |  5 +++--
>>  7 files changed, 64 insertions(+), 3 deletions(-)
>
> Acked-by: Simon Glass <sjg@chromium.org>

Applied to u-boot-dm/next, thanks!

[U-Boot] [PATCH 12/22] musb: Update usb-compat to work with struct usb_device without a parent ptr

[Simon Glass]

On 1 July 2015 at 08:57, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
>
> On 29-06-15 05:45, Simon Glass wrote:
>>
>> Hi Hans,
>>
>> On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>> When building with CONFIG_DM_USB=y struct usb_device does not have a
>>> parent
>>> pointer. This commit adds support to the musb code to deal with this.
>>>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>>   drivers/usb/musb-new/musb_host.c  |  4 +++
>>>   drivers/usb/musb-new/musb_uboot.c |  2 +-
>>>   drivers/usb/musb-new/usb-compat.h | 70
>>> +++++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 75 insertions(+), 1 deletion(-)
>>>
>>
>> Acked-by: Simon Glass <sjg@chromium.org>
>>
>> See note below.
>>
>>> diff --git a/drivers/usb/musb-new/musb_host.c
>>> b/drivers/usb/musb-new/musb_host.c
>>> index 437309c..40b9c66 100644
>>> --- a/drivers/usb/musb-new/musb_host.c
>>> +++ b/drivers/usb/musb-new/musb_host.c
>>> @@ -2067,7 +2067,11 @@ int musb_urb_enqueue(
>>>
>>>          /* precompute addressing for external hub/tt ports */
>>>          if (musb->is_multipoint) {
>>> +#ifndef __UBOOT__
>>>                  struct usb_device       *parent = urb->dev->parent;
>>> +#else
>>> +               struct usb_device       *parent =
>>> usb_dev_get_parent(urb->dev);
>>> +#endif
>>>
>>>   #ifndef __UBOOT__
>>>                  if (parent != hcd->self.root_hub) {
>>> diff --git a/drivers/usb/musb-new/musb_uboot.c
>>> b/drivers/usb/musb-new/musb_uboot.c
>>> index 70e87c9..a96e8d2 100644
>>> --- a/drivers/usb/musb-new/musb_uboot.c
>>> +++ b/drivers/usb/musb-new/musb_uboot.c
>>> @@ -97,7 +97,7 @@ int submit_control_msg(struct usb_device *dev, unsigned
>>> long pipe,
>>>                        buffer, len, setup, 0);
>>>
>>>          /* Fix speed for non hub-attached devices */
>>> -       if (!dev->parent)
>>> +       if (!usb_dev_get_parent(dev))
>>>                  dev->speed = host_speed;
>>>
>>>          return submit_urb(&hcd, &urb);
>>> diff --git a/drivers/usb/musb-new/usb-compat.h
>>> b/drivers/usb/musb-new/usb-compat.h
>>> index 50bad37..53fe4ff 100644
>>> --- a/drivers/usb/musb-new/usb-compat.h
>>> +++ b/drivers/usb/musb-new/usb-compat.h
>>> @@ -1,6 +1,7 @@
>>>   #ifndef __USB_COMPAT_H__
>>>   #define __USB_COMPAT_H__
>>>
>>> +#include <dm.h>
>>>   #include "usb.h"
>>>
>>>   struct usb_hcd {
>>> @@ -66,6 +67,68 @@ static inline int usb_hcd_unmap_urb_for_dma(struct
>>> usb_hcd *hcd,
>>>          return 0;
>>>   }
>>>
>>> +#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_parentdata(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_parentdata(dev);
>>> +               parent = dev->parent;
>>> +               uparent = dev_get_parentdata(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;
>>> +
>>> +       /*
>>> +        * 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.
>>
>>
>> Another option here is to somehow allow devices to be added before we
>> know what they are. In this case we could bind a 'generic' USB device
>> (UCLASS_USB_DEV_GENERIC). Then when we work out what it is, we could
>> unbind it (without throwing away the udevice and usb_device) and have
>> it bind again as the correct device. Something like
>> device_morph_child().
>
>
> Right, I think that may end up being cleaner in the long term.
>
> Regards,
>
> Hans

Applied to u-boot-dm/next, thanks!

[U-Boot] [PATCH 13/22] musb: Rename and wrap public functions

[Simon Glass]

On 28 June 2015 at 21:45, Simon Glass <sjg@chromium.org> wrote:
> On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
>> Rename and wrap the usb host API public functions, this is a preparation
>> patch for adding device-model support.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/usb/musb-new/musb_uboot.c | 70 +++++++++++++++++++++++++++++++++------
>>  1 file changed, 59 insertions(+), 11 deletions(-)
>
> Acked-by: Simon Glass <sjg@chromium.org>

Applied to u-boot-dm/next, thanks!

[U-Boot] [PATCH 14/22] musb: Add musb_host_data struct to hold global data

[Simon Glass]

On 28 June 2015 at 21:45, Simon Glass <sjg@chromium.org> wrote:
> Hi Hans,
>
> On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
>> Add a musb_host_data struct to hold all the global data host related musb
>> data. This is a preparation patch for adding device-model support.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/usb/musb-new/musb_uboot.c | 107 +++++++++++++++++++-------------------
>>  drivers/usb/musb-new/musb_uboot.h |  24 +++++++++
>>  2 files changed, 77 insertions(+), 54 deletions(-)
>>  create mode 100644 drivers/usb/musb-new/musb_uboot.h
>
> Acked-by: Simon Glass <sjg@chromium.org>
>
> Regards,
> Simon

Applied to u-boot-dm/next, thanks!

[U-Boot] [PATCH 15/22] musb: Add device-model support to the musb-host u-boot glue

[Simon Glass]

On 28 June 2015 at 21:45, Simon Glass <sjg@chromium.org> wrote:
> Hi Hans,
>
> On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
>> Add device-model support to the musb-host u-boot glue, note this only
>> adds device-model support to the musb-core glue code, it does not add
>> support for device-model to any of the SoC specific musb glue code.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/usb/musb-new/musb_uboot.c | 70 ++++++++++++++++++++++++++++++++++++++-
>>  drivers/usb/musb-new/musb_uboot.h |  4 +++
>>  2 files changed, 73 insertions(+), 1 deletion(-)
>>
>
> Acked-by: Simon Glass <sjg@chromium.org>
>
> Nit below.
>
>> diff --git a/drivers/usb/musb-new/musb_uboot.c b/drivers/usb/musb-new/musb_uboot.c
>> index 9875100..9b56e90 100644
>> --- a/drivers/usb/musb-new/musb_uboot.c
>> +++ b/drivers/usb/musb-new/musb_uboot.c
>> @@ -21,7 +21,9 @@ struct int_queue {
>>         struct urb urb;
>>  };
>>
>> +#ifndef CONFIG_DM_USB
>>  struct musb_host_data musb_host;
>> +#endif
>>
>>  static void musb_host_complete_urb(struct urb *urb)
>>  {
>> @@ -244,6 +246,7 @@ int musb_lowlevel_init(struct musb_host_data *host)
>>         return 0;
>>  }
>>
>> +#ifndef CONFIG_DM_USB
>>  int usb_lowlevel_stop(int index)
>>  {
>>         if (!musb_host.host) {
>> @@ -300,6 +303,71 @@ int usb_lowlevel_init(int index, enum usb_init_type init, void **controller)
>>  {
>>         return musb_lowlevel_init(&musb_host);
>>  }
>> +#endif /* !CONFIG_DM_USB */
>> +
>> +#ifdef CONFIG_DM_USB
>> +static int musb_submit_control_msg(struct udevice *dev, struct usb_device *udev,
>> +                                  unsigned long pipe, void *buffer, int length,
>> +                                  struct devrequest *setup)
>> +{
>> +       struct musb_host_data *host = dev_get_priv(dev);
>> +       return _musb_submit_control_msg(host, udev, pipe, buffer, length, setup);
>> +}
>> +
>> +static int musb_submit_bulk_msg(struct udevice *dev, struct usb_device *udev,
>> +                               unsigned long pipe, void *buffer, int length)
>> +{
>> +       struct musb_host_data *host = dev_get_priv(dev);
>> +       return _musb_submit_bulk_msg(host, udev, pipe, buffer, length);
>> +}
>> +
>> +static int musb_submit_int_msg(struct udevice *dev, struct usb_device *udev,
>> +                              unsigned long pipe, void *buffer, int length,
>> +                              int interval)
>> +{
>> +       struct musb_host_data *host = dev_get_priv(dev);
>> +       return _musb_submit_int_msg(host, udev, pipe, buffer, length, interval);
>> +}
>> +
>> +static struct int_queue *musb_create_int_queue(struct udevice *dev,
>> +               struct usb_device *udev, unsigned long pipe, int queuesize,
>> +               int elementsize, void *buffer, int interval)
>> +{
>> +       struct musb_host_data *host = dev_get_priv(dev);
>
> Can we have newlines after declarations?

Added these, and applied to u-boot-dm/next, thanks!

>
>> +       return _musb_create_int_queue(host, udev, pipe, queuesize, elementsize,
>> +                                     buffer, interval);
>> +}
>> +
>> +static void *musb_poll_int_queue(struct udevice *dev, struct usb_device *udev,
>> +                                struct int_queue *queue)
>> +{
>> +       struct musb_host_data *host = dev_get_priv(dev);
>> +       return _musb_poll_int_queue(host, udev, queue);
>> +}
>> +
>> +static int musb_destroy_int_queue(struct udevice *dev, struct usb_device *udev,
>> +                                 struct int_queue *queue)
>> +{
>> +       struct musb_host_data *host = dev_get_priv(dev);
>> +       return _musb_destroy_int_queue(host, udev, queue);
>> +}
>> +
>> +static int musb_reset_root_port(struct udevice *dev, struct usb_device *udev)
>> +{
>> +       struct musb_host_data *host = dev_get_priv(dev);
>> +       return _musb_reset_root_port(host, udev);
>> +}
>> +
>> +struct dm_usb_ops musb_usb_ops = {
>> +       .control = musb_submit_control_msg,
>> +       .bulk = musb_submit_bulk_msg,
>> +       .interrupt = musb_submit_int_msg,
>> +       .create_int_queue = musb_create_int_queue,
>> +       .poll_int_queue = musb_poll_int_queue,
>> +       .destroy_int_queue = musb_destroy_int_queue,
>> +       .reset_root_port = musb_reset_root_port,
>> +};
>> +#endif /* CONFIG_DM_USB */
>>  #endif /* CONFIG_MUSB_HOST */
>>
>>  #ifdef CONFIG_MUSB_GADGET
>> @@ -360,7 +428,7 @@ int musb_register(struct musb_hdrc_platform_data *plat, void *bdata,
>>         struct musb **musbp;
>>
>>         switch (plat->mode) {
>> -#ifdef CONFIG_MUSB_HOST
>> +#if defined(CONFIG_MUSB_HOST) && !defined(CONFIG_DM_USB)
>>         case MUSB_HOST:
>>                 musbp = &musb_host.host;
>>                 break;
>> diff --git a/drivers/usb/musb-new/musb_uboot.h b/drivers/usb/musb-new/musb_uboot.h
>> index 69b7977..6312cd2 100644
>> --- a/drivers/usb/musb-new/musb_uboot.h
>> +++ b/drivers/usb/musb-new/musb_uboot.h
>> @@ -21,4 +21,8 @@ struct musb_host_data {
>>         struct urb urb;
>>  };
>>
>> +extern struct dm_usb_ops musb_usb_ops;
>> +
>> +int musb_lowlevel_init(struct musb_host_data *host);
>> +
>>  #endif
>> --
>> 2.4.3
>>
>
> Regards,
> Simon