[U-Boot] [PATCH v2 0/4] musb device model support series

[U-Boot] [PATCH v2 0/4] musb device model support series

[Hans de Goede]

Hi Simon, Marek,

Here is a series with just the patches which needed work after my v1
posting of this series.

I've a rebased version of the entire series (some patches needed adjustment
after the recent Kconfig / defconfig changes) in my personal tree.

Simon, AFAIK the plan is still for this to go upstream through your tree,
let me know when you're ready to take this series, and on which branch you
want it based, and I'll create a branch for you to pull.

Regards,

Hans

[U-Boot] [PATCH v2 1/4] dm: Export device_remove_children / device_unbind_children

[Hans de Goede]

These functions are useful to remove all children from an usb bus before
rescanning the bus. Give them a better name and export them.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Not only export but also rename the functions to device_remove_children /
 device_unbind_children
---
 drivers/core/device-remove.c | 22 ++++------------------
 include/dm/device-internal.h | 26 ++++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c
index 6a16b4f..6b87f86 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_unbind_children(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_remove_children(struct udevice *dev)
 {
 	struct udevice *pos, *n;
 	int ret;
@@ -84,7 +70,7 @@ int device_unbind(struct udevice *dev)
 			return ret;
 	}
 
-	ret = device_chld_unbind(dev);
+	ret = device_unbind_children(dev);
 	if (ret)
 		return ret;
 
@@ -159,7 +145,7 @@ int device_remove(struct udevice *dev)
 	if (ret)
 		return ret;
 
-	ret = device_chld_remove(dev);
+	ret = device_remove_children(dev);
 	if (ret)
 		goto err;
 
diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h
index 687462b..402304f 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_remove_children() - 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_remove_children(struct udevice *dev);
+#else
+static inline int device_remove_children(struct udevice *dev) { return 0; }
+#endif
+
+/**
+ * device_unbind_children() - 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_unbind_children(struct udevice *dev);
+#else
+static inline int device_unbind_children(struct udevice *dev) { return 0; }
+#endif
+
 #ifdef CONFIG_DM_DEVICE_REMOVE
 void device_free(struct udevice *dev);
 #else
-- 
2.4.3

[U-Boot] [PATCH v2 2/4] dm: usb: Document that mixing DM_DEVICE_REMOVE and DM_USB is a bad idea

[Hans de Goede]

Document that mixing DM_DEVICE_REMOVE and DM_USB is a bad idea, and also why
this is a bad idea.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/core/Kconfig | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
index 2861b43..e40372d 100644
--- a/drivers/core/Kconfig
+++ b/drivers/core/Kconfig
@@ -38,6 +38,10 @@ config DM_DEVICE_REMOVE
 	  device. This is not normally required in SPL, so by default this
 	  option is disabled for SPL.
 
+	  Note that this may have undesirable results in the USB subsystem as
+	  it causes unplugged devices to linger around in the dm-tree, and it
+	  causes USB host controllers to not be stopped when booting the OS.
+
 config DM_STDIO
 	bool "Support stdio registration"
 	depends on DM
-- 
2.4.3

[U-Boot] [PATCH v2 3/4] dm: usb: Use device_unbind_children to clean up usb devs on stop

[Hans de Goede]

On an usb stop instead of leaving orphan usb devices behind simply remove
them.

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.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-We only need to call device_unbind_children, the children are removed
 already by the device_remove call on the host
-Do not add #ifdef-s around usb_stop()
---
 drivers/usb/host/usb-uclass.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index bce6cec..2df6740 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -146,6 +146,9 @@ int usb_stop(void)
 		ret = device_remove(bus);
 		if (ret && !err)
 			err = ret;
+		ret = device_unbind_children(bus);
+		if (ret && !err)
+			err = ret;
 	}
 
 #ifdef CONFIG_SANDBOX
-- 
2.4.3

[U-Boot] [PATCH v2 4/4] dm: usb: Rename usb_find_child to usb_find_emul_child

[Hans de Goede]

Now that we unbind usb devices from usb_stop() usb_find_child() is
only necessary to deal with emulated usb devices.

Rename it to make this clear and add a #ifdef to make it a nop in
other cases.

Note the #ifdef turns usb_find_emul_child() into a nop, rather then not
building it and adding another #ifdef to the caller, this is done this way
because adding a #ifdef to the caller is somewhat hairy.

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

diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index 2df6740..fd11cc6 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -485,15 +485,14 @@ error:
 }
 
 /**
- * usb_find_child() - Find an existing device which matches our needs
- *
- *
+ * usb_find_emul_child() - Find an existing device for emulated devices
  */
-static int usb_find_child(struct udevice *parent,
-			  struct usb_device_descriptor *desc,
-			  struct usb_interface_descriptor *iface,
-			  struct udevice **devp)
+static int usb_find_emul_child(struct udevice *parent,
+			       struct usb_device_descriptor *desc,
+			       struct usb_interface_descriptor *iface,
+			       struct udevice **devp)
 {
+#ifdef CONFIG_SANDBOX
 	struct udevice *dev;
 
 	*devp = NULL;
@@ -512,7 +511,7 @@ static int usb_find_child(struct udevice *parent,
 			return 0;
 		}
 	}
-
+#endif
 	return -ENOENT;
 }
 
@@ -572,8 +571,8 @@ int usb_scan_device(struct udevice *parent, int port,
 	debug("read_descriptor for '%s': ret=%d\n", parent->name, ret);
 	if (ret)
 		return ret;
-	ret = usb_find_child(parent, &udev->descriptor, iface, &dev);
-	debug("** usb_find_child returns %d\n", ret);
+	ret = usb_find_emul_child(parent, &udev->descriptor, iface, &dev);
+	debug("** usb_find_emul_child returns %d\n", ret);
 	if (ret) {
 		if (ret != -ENOENT)
 			return ret;
-- 
2.4.3

[U-Boot] [PATCH v2 0/4] musb device model support series

[Simon Glass]

Hi Hans,

On 1 July 2015 at 12:52, Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi Simon, Marek,
>
> Here is a series with just the patches which needed work after my v1
> posting of this series.
>
> I've a rebased version of the entire series (some patches needed adjustment
> after the recent Kconfig / defconfig changes) in my personal tree.
>
> Simon, AFAIK the plan is still for this to go upstream through your tree,
> let me know when you're ready to take this series, and on which branch you
> want it based, and I'll create a branch for you to pull.

Do you mean I should apply the entire series, or just the first 15 patches?

I've set up a u-boot-dm/next. I'm planning to do that again to speed
up merging when the window opens.

But I'd prefer to take patches from patchwork - can I just do that?

Regards,
Simon

[U-Boot] [PATCH v2 1/4] dm: Export device_remove_children / device_unbind_children

[Simon Glass]

On 1 July 2015 at 12:52, Hans de Goede <hdegoede@redhat.com> wrote:
> These functions are useful to remove all children from an usb bus before
> rescanning the bus. Give them a better name and export them.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Not only export but also rename the functions to device_remove_children /
>  device_unbind_children
> ---
>  drivers/core/device-remove.c | 22 ++++------------------
>  include/dm/device-internal.h | 26 ++++++++++++++++++++++++++
>  2 files changed, 30 insertions(+), 18 deletions(-)

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

[U-Boot] [PATCH v2 2/4] dm: usb: Document that mixing DM_DEVICE_REMOVE and DM_USB is a bad idea

[Simon Glass]

On 1 July 2015 at 12:52, Hans de Goede <hdegoede@redhat.com> wrote:
> Document that mixing DM_DEVICE_REMOVE and DM_USB is a bad idea, and also why
> this is a bad idea.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/core/Kconfig | 4 ++++
>  1 file changed, 4 insertions(+)

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

[U-Boot] [PATCH v2 3/4] dm: usb: Use device_unbind_children to clean up usb devs on stop

[Simon Glass]

On 1 July 2015 at 12:53, Hans de Goede <hdegoede@redhat.com> wrote:
> On an usb stop instead of leaving orphan usb devices behind simply remove
> them.
>
> 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.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -We only need to call device_unbind_children, the children are removed
>  already by the device_remove call on the host
> -Do not add #ifdef-s around usb_stop()
> ---
>  drivers/usb/host/usb-uclass.c | 3 +++
>  1 file changed, 3 insertions(+)

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

[U-Boot] [PATCH v2 4/4] dm: usb: Rename usb_find_child to usb_find_emul_child

[Simon Glass]

On 1 July 2015 at 12:53, Hans de Goede <hdegoede@redhat.com> wrote:
> Now that we unbind usb devices from usb_stop() usb_find_child() is
> only necessary to deal with emulated usb devices.
>
> Rename it to make this clear and add a #ifdef to make it a nop in
> other cases.
>
> Note the #ifdef turns usb_find_emul_child() into a nop, rather then not
> building it and adding another #ifdef to the caller, this is done this way
> because adding a #ifdef to the caller is somewhat hairy.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/usb/host/usb-uclass.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)

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

[U-Boot] [PATCH v2 0/4] musb device model support series

[Hans de Goede]

Hi,

On 01-07-15 21:47, Simon Glass wrote:
> Hi Hans,
>
> On 1 July 2015 at 12:52, Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi Simon, Marek,
>>
>> Here is a series with just the patches which needed work after my v1
>> posting of this series.
>>
>> I've a rebased version of the entire series (some patches needed adjustment
>> after the recent Kconfig / defconfig changes) in my personal tree.
>>
>> Simon, AFAIK the plan is still for this to go upstream through your tree,
>> let me know when you're ready to take this series, and on which branch you
>> want it based, and I'll create a branch for you to pull.
>
> Do you mean I should apply the entire series, or just the first 15 patches?

My initial intention was for you to take the entire series, but given
the recent Kconfig changes and conflicts in the sunxi bits I think it
is best if you just take the dm/usb and musb device-model conversion
patches. Then I'll merge the sunxi bits after u-boot-dm/next has
been merged.

> I've set up a u-boot-dm/next. I'm planning to do that again to speed
> up merging when the window opens.
>
> But I'd prefer to take patches from patchwork - can I just do that?

Yes that should work. Note that my v2 series introduces a few new patches
as a result of your review of v1, so the entire series I would like you
to merge is (in order they should be merged):

usb: Drop device-model specific copy of usb_legacy_port_reset
usb: usb_setup_device: Drop unneeded portnr function argument
usb: Pass device instead of portnr to usb_legacy_port_reset
usb: Add an usb_device parameter to usb_reset_root_port
dm: Export device_remove_children / device_unbind_children (v2)
dm: usb: Fix "usb tree" output
dm: usb: Document that mixing DM_DEVICE_REMOVE and DM_USB is a bad idea (v2)
dm: usb: Use device_unbind_children to clean up usb devs on stop (v2)
dm: usb: Rename usb_find_child to usb_find_emul_child (v2)
dm: usb: Allow usb host drivers to implement usb_reset_root_port
dm: usb: Do not assume that first child is always a hub
musb: Allow musb_platform_enable to return an error code
musb: Update usb-compat to work with struct usb_device without a parent ptr
musb: Rename and wrap public functions
musb: Add musb_host_data struct to hold global data
musb: Add device-model support to the musb-host u-boot glue

I think you should be able to pick these from patchwork without
issues. Let me know if you hit any problems, then I will post a v3
of the entire series re-based on top of u-boot-dm/next.

Regards,

Hans

[U-Boot] [PATCH v2 1/4] dm: Export device_remove_children / device_unbind_children

[Simon Glass]

On 3 July 2015 at 17:06, Simon Glass <sjg@chromium.org> wrote:
> On 1 July 2015 at 12:52, Hans de Goede <hdegoede@redhat.com> wrote:
>> These functions are useful to remove all children from an usb bus before
>> rescanning the bus. Give them a better name and export them.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Changes in v2:
>> -Not only export but also rename the functions to device_remove_children /
>>  device_unbind_children
>> ---
>>  drivers/core/device-remove.c | 22 ++++------------------
>>  include/dm/device-internal.h | 26 ++++++++++++++++++++++++++
>>  2 files changed, 30 insertions(+), 18 deletions(-)
>
> Acked-by: Simon Glass <sjg@chromium.org>

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

[U-Boot] [PATCH v2 2/4] dm: usb: Document that mixing DM_DEVICE_REMOVE and DM_USB is a bad idea

[Simon Glass]

On 3 July 2015 at 17:06, Simon Glass <sjg@chromium.org> wrote:
> On 1 July 2015 at 12:52, Hans de Goede <hdegoede@redhat.com> wrote:
>> Document that mixing DM_DEVICE_REMOVE and DM_USB is a bad idea, and also why
>> this is a bad idea.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/core/Kconfig | 4 ++++
>>  1 file changed, 4 insertions(+)
>
> Acked-by: Simon Glass <sjg@chromium.org>

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

[U-Boot] [PATCH v2 3/4] dm: usb: Use device_unbind_children to clean up usb devs on stop

[Simon Glass]

On 3 July 2015 at 17:06, Simon Glass <sjg@chromium.org> wrote:
> On 1 July 2015 at 12:53, Hans de Goede <hdegoede@redhat.com> wrote:
>> On an usb stop instead of leaving orphan usb devices behind simply remove
>> them.
>>
>> 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.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Changes in v2:
>> -We only need to call device_unbind_children, the children are removed
>>  already by the device_remove call on the host
>> -Do not add #ifdef-s around usb_stop()
>> ---
>>  drivers/usb/host/usb-uclass.c | 3 +++
>>  1 file changed, 3 insertions(+)
>
> Acked-by: Simon Glass <sjg@chromium.org>

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

[U-Boot] [PATCH v2 4/4] dm: usb: Rename usb_find_child to usb_find_emul_child

[Simon Glass]

On 3 July 2015 at 17:06, Simon Glass <sjg@chromium.org> wrote:
> On 1 July 2015 at 12:53, Hans de Goede <hdegoede@redhat.com> wrote:
>> Now that we unbind usb devices from usb_stop() usb_find_child() is
>> only necessary to deal with emulated usb devices.
>>
>> Rename it to make this clear and add a #ifdef to make it a nop in
>> other cases.
>>
>> Note the #ifdef turns usb_find_emul_child() into a nop, rather then not
>> building it and adding another #ifdef to the caller, this is done this way
>> because adding a #ifdef to the caller is somewhat hairy.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/usb/host/usb-uclass.c | 19 +++++++++----------
>>  1 file changed, 9 insertions(+), 10 deletions(-)
>
> Acked-by: Simon Glass <sjg@chromium.org>

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