musb-new: Improve shutdown code

Some OTG controllers which operates on Peripheral mode are registered as UCLASS_USB_DEV_GENERIC.

So add support to shutdown them as well. shutdown happened during 'usb reset' and U-Boot handoff code for Linux boot.

controller restarting in 'usb reset' like probing again is still missing for this type of UCLASS.

Cc Simon Glass <[email protected]>

Signed-off-by Jagan Teki <[email protected]>

---
Note:
I tried of traversing for multiple UCLASS in removal code in usb_stop, but couldn't come with proper solutions. I don't think any other area of code need a requirement like this, or may be I'm missing. any help would appreciate.

Changes for v2:
- none

 drivers/usb/host/usb-uclass.c | 43 +++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

drivers/usb/host/usb-uclass.c +0 -0

index 611ea97a72..99cf3d2b49 100644

drivers/usb/host/usb-uclass.c +38 -0

+{
+       struct udevice *bus;
+       struct udevice *rh;
+       struct uclass *uc;
+       struct usb_uclass_priv *uc_priv;
+       int err = 0, ret;
+
+       /* De-activate any devices that have been activated */
+       ret = uclass_get(UCLASS_USB_DEV_GENERIC, &uc);
+       if (ret)
+               return ret;
+
+       uc_priv = uc->priv;
+
+       uclass_foreach_dev(bus, uc) {
+               ret = device_remove(bus, DM_REMOVE_NORMAL);
+               if (ret && !err)
+                       err = ret;
+
+               /* Locate root hub device */
+               device_find_first_child(bus, &rh);
+               if (rh) {
+                       /*
+                        * All USB devices are children of root hub.
+                        * Unbinding root hub will unbind all of its children.
+                        */
+                       ret = device_unbind(rh);
+                       if (ret && !err)
+                               err = ret;
+               }
+       }
+
+       uc_priv->companion_device_count = 0;
+       usb_started = 0;
+
+       return err;
+}
+
 int usb_stop(void)
 {
        struct udevice *bus;

drivers/usb/host/usb-uclass.c +0 -1

        /* De-activate any devices that have been activated */
        ret = uclass_get(UCLASS_USB, &uc);
        if (ret)
--

2.17.1

Hi Jagan,

Quoted 30 lines · Jagan Teki

Some OTG controllers which operates on Peripheral mode are registered as UCLASS_USB_DEV_GENERIC.

So add support to shutdown them as well. shutdown happened during 'usb reset' and U-Boot handoff code for Linux boot.

controller restarting in 'usb reset' like probing again is still missing for this type of UCLASS.

Cc Simon Glass <[email protected]>

Signed-off-by Jagan Teki <[email protected]>

---
Note:
I tried of traversing for multiple UCLASS in removal code in usb_stop, but couldn't come with proper solutions. I don't think any other area of code need a requirement like this, or may be I'm missing. any help would appreciate.

Changes for v2:
- none

 drivers/usb/host/usb-uclass.c | 43 +++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

drivers/usb/host/usb-uclass.c +0 -0

index 611ea97a72..99cf3d2b49 100644

drivers/usb/host/usb-uclass.c int usb_get_max_xfer_size(struct usb_device *udev, size_t *size) · line 161

Quoted 4 lines

drivers/usb/host/usb-uclass.c +1 -0

@@ -158,6 +158,45 @@ int usb_get_max_xfer_size(struct usb_device *udev, size_t *size)
        return ops->get_max_xfer_size(bus, size);
 }
 
+int __usb_stop(void)

Why the __ ? I think it should be something like usb_remove_and_unbind_all()

Quoted 45 lines

drivers/usb/host/usb-uclass.c +38 -0

+{
+       struct udevice *bus;
+       struct udevice *rh;
+       struct uclass *uc;
+       struct usb_uclass_priv *uc_priv;
+       int err = 0, ret;
+
+       /* De-activate any devices that have been activated */
+       ret = uclass_get(UCLASS_USB_DEV_GENERIC, &uc);
+       if (ret)
+               return ret;
+
+       uc_priv = uc->priv;
+
+       uclass_foreach_dev(bus, uc) {
+               ret = device_remove(bus, DM_REMOVE_NORMAL);
+               if (ret && !err)
+                       err = ret;
+
+               /* Locate root hub device */
+               device_find_first_child(bus, &rh);
+               if (rh) {
+                       /*
+                        * All USB devices are children of root hub.
+                        * Unbinding root hub will unbind all of its children.
+                        */
+                       ret = device_unbind(rh);
+                       if (ret && !err)
+                               err = ret;
+               }
+       }
+
+       uc_priv->companion_device_count = 0;
+       usb_started = 0;
+
+       return err;
+}
+
 int usb_stop(void)
 {
        struct udevice *bus;

@@ -166,6 +205,10 @@ int usb_stop(void)
        struct usb_uclass_priv *uc_priv;
        int err = 0, ret;
 

drivers/usb/host/usb-uclass.c int usb_stop(void) · line 211

Quoted 4 lines

drivers/usb/host/usb-uclass.c +4 -0

+       ret = __usb_stop();
+       if (ret)
+               return ret;
+

This looks like the same code that appears below here, or very similar. Why is this needed?

Quoted 85 lines · Simon Glass

Hi Jagan,

Quoted 30 lines · Jagan Teki

Some OTG controllers which operates on Peripheral mode are registered as UCLASS_USB_DEV_GENERIC.

So add support to shutdown them as well. shutdown happened during 'usb reset' and U-Boot handoff code for Linux boot.

controller restarting in 'usb reset' like probing again is still missing for this type of UCLASS.

Cc Simon Glass <[email protected]>

Signed-off-by Jagan Teki <[email protected]>

---
Note:
I tried of traversing for multiple UCLASS in removal code in usb_stop, but couldn't come with proper solutions. I don't think any other area of code need a requirement like this, or may be I'm missing. any help would appreciate.

Changes for v2:
- none

 drivers/usb/host/usb-uclass.c | 43 +++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

drivers/usb/host/usb-uclass.c +0 -0

index 611ea97a72..99cf3d2b49 100644

drivers/usb/host/usb-uclass.c int usb_get_max_xfer_size(struct usb_device *udev, size_t *size) · line 161

Quoted 4 lines

drivers/usb/host/usb-uclass.c +1 -0

@@ -158,6 +158,45 @@ int usb_get_max_xfer_size(struct usb_device *udev, size_t *size)
        return ops->get_max_xfer_size(bus, size);
 }
 
+int __usb_stop(void)

Why the __ ? I think it should be something like usb_remove_and_unbind_all()

Quoted 45 lines

drivers/usb/host/usb-uclass.c +38 -0

+{
+       struct udevice *bus;
+       struct udevice *rh;
+       struct uclass *uc;
+       struct usb_uclass_priv *uc_priv;
+       int err = 0, ret;
+
+       /* De-activate any devices that have been activated */
+       ret = uclass_get(UCLASS_USB_DEV_GENERIC, &uc);
+       if (ret)
+               return ret;
+
+       uc_priv = uc->priv;
+
+       uclass_foreach_dev(bus, uc) {
+               ret = device_remove(bus, DM_REMOVE_NORMAL);
+               if (ret && !err)
+                       err = ret;
+
+               /* Locate root hub device */
+               device_find_first_child(bus, &rh);
+               if (rh) {
+                       /*
+                        * All USB devices are children of root hub.
+                        * Unbinding root hub will unbind all of its children.
+                        */
+                       ret = device_unbind(rh);
+                       if (ret && !err)
+                               err = ret;
+               }
+       }
+
+       uc_priv->companion_device_count = 0;
+       usb_started = 0;
+
+       return err;
+}
+
 int usb_stop(void)
 {
        struct udevice *bus;

@@ -166,6 +205,10 @@ int usb_stop(void)
        struct usb_uclass_priv *uc_priv;
        int err = 0, ret;
 

Quoted 7 lines

drivers/usb/host/usb-uclass.c int usb_stop(void) · line 211

Quoted 4 lines

drivers/usb/host/usb-uclass.c +4 -0

+       ret = __usb_stop();
+       if (ret)
+               return ret;
+

This looks like the same code that appears below here, or very similar. Why is this needed?

Here the usage-case it to remove/unbind UCLASS_USB and UCLASS_USB_DEV_GENERIC and same code will use to do that, any suggestions?

Hi Jagan,

Quoted 94 lines · Jagan Teki

Quoted 92 lines · Simon Glass

Hi Jagan,

Quoted 30 lines · Jagan Teki

Some OTG controllers which operates on Peripheral mode are registered as UCLASS_USB_DEV_GENERIC.

So add support to shutdown them as well. shutdown happened during 'usb reset' and U-Boot handoff code for Linux boot.

controller restarting in 'usb reset' like probing again is still missing for this type of UCLASS.

Cc Simon Glass <[email protected]>

Signed-off-by Jagan Teki <[email protected]>

---
Note:
I tried of traversing for multiple UCLASS in removal code in usb_stop, but couldn't come with proper solutions. I don't think any other area of code need a requirement like this, or may be I'm missing. any help would appreciate.

Changes for v2:
- none

 drivers/usb/host/usb-uclass.c | 43 +++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

drivers/usb/host/usb-uclass.c +0 -0

index 611ea97a72..99cf3d2b49 100644

drivers/usb/host/usb-uclass.c int usb_get_max_xfer_size(struct usb_device *udev, size_t *size) · line 161

Quoted 4 lines

drivers/usb/host/usb-uclass.c +1 -0

@@ -158,6 +158,45 @@ int usb_get_max_xfer_size(struct usb_device *udev, size_t *size)
        return ops->get_max_xfer_size(bus, size);
 }
 
+int __usb_stop(void)

Why the __ ? I think it should be something like usb_remove_and_unbind_all()

Quoted 42 lines

drivers/usb/host/usb-uclass.c +38 -0

+{
+       struct udevice *bus;
+       struct udevice *rh;
+       struct uclass *uc;
+       struct usb_uclass_priv *uc_priv;
+       int err = 0, ret;
+
+       /* De-activate any devices that have been activated */
+       ret = uclass_get(UCLASS_USB_DEV_GENERIC, &uc);
+       if (ret)
+               return ret;
+
+       uc_priv = uc->priv;
+
+       uclass_foreach_dev(bus, uc) {
+               ret = device_remove(bus, DM_REMOVE_NORMAL);
+               if (ret && !err)
+                       err = ret;
+
+               /* Locate root hub device */
+               device_find_first_child(bus, &rh);
+               if (rh) {
+                       /*
+                        * All USB devices are children of root hub.
+                        * Unbinding root hub will unbind all of its children.
+                        */
+                       ret = device_unbind(rh);
+                       if (ret && !err)
+                               err = ret;
+               }
+       }
+
+       uc_priv->companion_device_count = 0;
+       usb_started = 0;
+
+       return err;
+}
+
 int usb_stop(void)
 {
        struct udevice *bus;

drivers/usb/host/usb-uclass.c int usb_stop(void) · line 211

Quoted 7 lines

drivers/usb/host/usb-uclass.c +4 -0

@@ -166,6 +205,10 @@ int usb_stop(void)
        struct usb_uclass_priv *uc_priv;
        int err = 0, ret;
 
+       ret = __usb_stop();
+       if (ret)
+               return ret;
+

This looks like the same code that appears below here, or very similar. Why is this needed?

Quoted 3 lines

Here the usage-case it to remove/unbind UCLASS_USB and UCLASS_USB_DEV_GENERIC and same code will use to do that, any suggestions?

I'm just wondering why you appear to be duplicating the exact code that is already there? Maybe I am missing something, iwc please can you explain it?

Hi Simon,

Quoted 101 lines · Simon Glass

Hi Jagan,

Quoted 97 lines · Jagan Teki

Quoted 85 lines · Simon Glass

Hi Jagan,

Quoted 30 lines · Jagan Teki

Some OTG controllers which operates on Peripheral mode are registered as UCLASS_USB_DEV_GENERIC.

So add support to shutdown them as well. shutdown happened during 'usb reset' and U-Boot handoff code for Linux boot.

controller restarting in 'usb reset' like probing again is still missing for this type of UCLASS.

Cc Simon Glass <[email protected]>

Signed-off-by Jagan Teki <[email protected]>

---
Note:
I tried of traversing for multiple UCLASS in removal code in usb_stop, but couldn't come with proper solutions. I don't think any other area of code need a requirement like this, or may be I'm missing. any help would appreciate.

Changes for v2:
- none

 drivers/usb/host/usb-uclass.c | 43 +++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

drivers/usb/host/usb-uclass.c +0 -0

index 611ea97a72..99cf3d2b49 100644

drivers/usb/host/usb-uclass.c int usb_get_max_xfer_size(struct usb_device *udev, size_t *size) · line 161

Quoted 4 lines

drivers/usb/host/usb-uclass.c +1 -0

@@ -158,6 +158,45 @@ int usb_get_max_xfer_size(struct usb_device *udev, size_t *size)
        return ops->get_max_xfer_size(bus, size);
 }
 
+int __usb_stop(void)

Why the __ ? I think it should be something like usb_remove_and_unbind_all()

Quoted 45 lines

drivers/usb/host/usb-uclass.c +38 -0

+{
+       struct udevice *bus;
+       struct udevice *rh;
+       struct uclass *uc;
+       struct usb_uclass_priv *uc_priv;
+       int err = 0, ret;
+
+       /* De-activate any devices that have been activated */
+       ret = uclass_get(UCLASS_USB_DEV_GENERIC, &uc);
+       if (ret)
+               return ret;
+
+       uc_priv = uc->priv;
+
+       uclass_foreach_dev(bus, uc) {
+               ret = device_remove(bus, DM_REMOVE_NORMAL);
+               if (ret && !err)
+                       err = ret;
+
+               /* Locate root hub device */
+               device_find_first_child(bus, &rh);
+               if (rh) {
+                       /*
+                        * All USB devices are children of root hub.
+                        * Unbinding root hub will unbind all of its children.
+                        */
+                       ret = device_unbind(rh);
+                       if (ret && !err)
+                               err = ret;
+               }
+       }
+
+       uc_priv->companion_device_count = 0;
+       usb_started = 0;
+
+       return err;
+}
+
 int usb_stop(void)
 {
        struct udevice *bus;

@@ -166,6 +205,10 @@ int usb_stop(void)
        struct usb_uclass_priv *uc_priv;
        int err = 0, ret;
 

Quoted 7 lines

drivers/usb/host/usb-uclass.c int usb_stop(void) · line 211

Quoted 4 lines

drivers/usb/host/usb-uclass.c +4 -0

+       ret = __usb_stop();
+       if (ret)
+               return ret;
+

This looks like the same code that appears below here, or very similar. Why is this needed?

Here the usage-case it to remove/unbind UCLASS_USB and UCLASS_USB_DEV_GENERIC and same code will use to do that, any suggestions?

Quoted 3 lines

I'm just wondering why you appear to be duplicating the exact code that is already there? Maybe I am missing something, iwc please can you explain it?

Yes, the code is duplicate. Here, I'm looking for suggestion to unbind/remove multiple UCLASS's like UCLASS_USB and ULASS_USB_DEV_GENERIC in this use-case. do you have any? or am I missing something?

This is v2 for previous series[1], by excluding sunxi phy changes.

One patch mark it as 'RFC' about "including UCLASS_USB_DEV_GENERIC into shutdown caller" so expecting some inputs on the same

[1] https://patchwork.ozlabs.org/cover/941588/

Jagan Teki (6):
usb: musb-new: Fix improper musb host pointer
usb: musb-new: sunxi: Allocate struct phy in private
dm: usb: Add UCLASS_USB_DEV_GENERIC shutdown
musb-new: sunxi: Access ahb_reset0_cfg via ccm offset
usb: musb-new: sunxi: Add proper musb exit support
usb: musb-new: Call musb_platform_exit from musb_stop

 board/compulab/cm_t3517/cm_t3517.c |   4 +-
 drivers/usb/host/usb-uclass.c      |  43 ++++++++++++
 drivers/usb/musb-new/musb_core.c   |   1 +
 drivers/usb/musb-new/musb_uboot.c  |  12 ++--
 drivers/usb/musb-new/pic32.c       |   6 +-
 drivers/usb/musb-new/sunxi.c       | 107 ++++++++++++++++++-----------
 include/linux/usb/musb.h           |   4 +-
 7 files changed, 123 insertions(+), 54 deletions(-)

This is v2 for previous series[1], by excluding sunxi phy changes.

One patch mark it as 'RFC' about "including UCLASS_USB_DEV_GENERIC into shutdown caller" so expecting some inputs on the same

[1] https://patchwork.ozlabs.org/cover/941588/

Jagan Teki (6):
usb: musb-new: Fix improper musb host pointer
usb: musb-new: sunxi: Allocate struct phy in private
dm: usb: Add UCLASS_USB_DEV_GENERIC shutdown
musb-new: sunxi: Access ahb_reset0_cfg via ccm offset
usb: musb-new: sunxi: Add proper musb exit support
usb: musb-new: Call musb_platform_exit from musb_stop

 board/compulab/cm_t3517/cm_t3517.c |   4 +-
 drivers/usb/host/usb-uclass.c      |  43 ++++++++++++
 drivers/usb/musb-new/musb_core.c   |   1 +
 drivers/usb/musb-new/musb_uboot.c  |  12 ++--
 drivers/usb/musb-new/pic32.c       |   6 +-
 drivers/usb/musb-new/sunxi.c       | 107 ++++++++++++++++++-----------
 include/linux/usb/musb.h           |   4 +-
 7 files changed, 123 insertions(+), 54 deletions(-)

--
2.17.1

Quoted 24 lines · Jagan Teki

This is v2 for previous series[1], by excluding sunxi phy changes.

One patch mark it as 'RFC' about "including UCLASS_USB_DEV_GENERIC into shutdown caller" so expecting some inputs on the same

[1] https://patchwork.ozlabs.org/cover/941588/

Jagan Teki (6):
usb: musb-new: Fix improper musb host pointer
usb: musb-new: sunxi: Allocate struct phy in private
dm: usb: Add UCLASS_USB_DEV_GENERIC shutdown
musb-new: sunxi: Access ahb_reset0_cfg via ccm offset
usb: musb-new: sunxi: Add proper musb exit support
usb: musb-new: Call musb_platform_exit from musb_stop

 board/compulab/cm_t3517/cm_t3517.c |   4 +-
 drivers/usb/host/usb-uclass.c      |  43 ++++++++++++
 drivers/usb/musb-new/musb_core.c   |   1 +
 drivers/usb/musb-new/musb_uboot.c  |  12 ++--
 drivers/usb/musb-new/pic32.c       |   6 +-
 drivers/usb/musb-new/sunxi.c       | 107 ++++++++++++++++++-----------
 include/linux/usb/musb.h           |   4 +-
 7 files changed, 123 insertions(+), 54 deletions(-)

Quoted 3 lines

--
2.17.1

Tested-by Chen-Yu Tsai <[email protected]>

on A33-OlinuXino. This fixes the abort when starting the boot sequence.

This is v2 for previous series[1], by excluding sunxi phy changes.

One patch mark it as 'RFC' about "including UCLASS_USB_DEV_GENERIC into shutdown caller" so expecting some inputs on the same

[1] https://patchwork.ozlabs.org/cover/941588/

Jagan Teki (6):
usb: musb-new: Fix improper musb host pointer
usb: musb-new: sunxi: Allocate struct phy in private
dm: usb: Add UCLASS_USB_DEV_GENERIC shutdown
musb-new: sunxi: Access ahb_reset0_cfg via ccm offset
usb: musb-new: sunxi: Add proper musb exit support
usb: musb-new: Call musb_platform_exit from musb_stop

Quoted 7 lines · Jagan Teki

This is v2 for previous series[1], by excluding sunxi phy changes.

One patch mark it as 'RFC' about "including UCLASS_USB_DEV_GENERIC into shutdown caller" so expecting some inputs on the same

[1] https://patchwork.ozlabs.org/cover/941588/

Quoted 7 lines

Jagan Teki (6):
usb: musb-new: Fix improper musb host pointer
usb: musb-new: sunxi: Allocate struct phy in private
dm: usb: Add UCLASS_USB_DEV_GENERIC shutdown
musb-new: sunxi: Access ahb_reset0_cfg via ccm offset
usb: musb-new: sunxi: Add proper musb exit support
usb: musb-new: Call musb_platform_exit from musb_stop

I'm planning to apply by excluding RFC patch, let me know if anyone has any inputs.

Quoted 17 lines · Jagan Teki

Quoted 14 lines · Jagan Teki

This is v2 for previous series[1], by excluding sunxi phy changes.

One patch mark it as 'RFC' about "including UCLASS_USB_DEV_GENERIC into shutdown caller" so expecting some inputs on the same

[1] https://patchwork.ozlabs.org/cover/941588/

Jagan Teki (6):
usb: musb-new: Fix improper musb host pointer
usb: musb-new: sunxi: Allocate struct phy in private
dm: usb: Add UCLASS_USB_DEV_GENERIC shutdown
musb-new: sunxi: Access ahb_reset0_cfg via ccm offset
usb: musb-new: sunxi: Add proper musb exit support
usb: musb-new: Call musb_platform_exit from musb_stop

Quoted 2 lines

I'm planning to apply by excluding RFC patch, let me know if anyone has any inputs.

Boots cleanly without the RFC patch on the Bananapi M2 Magic, with musb-new built in both host and gadget mode.

Curiously, both modes can be enabled at the same time in Kconfig, but host mode takes precedence in the actual operation.