[U-Boot] [PATCH v2 0/4] musb device model support series
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
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
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
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!
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
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
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!
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
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
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!
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;
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
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!
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
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
participants (2)
-
Hans de Goede -
Simon Glass