[U-Boot] [PATCH v2 0/6] musb-new: Improve shutdown code

[U-Boot] [PATCH v2 0/6] musb-new: Improve shutdown code

[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(-)

-- 
2.17.1

[U-Boot] [PATCH v2 1/6] usb: musb-new: Fix improper musb host pointer

[Jagan Teki]

When MUSB is operating in peripheral mode, probe registering
musb core using musb_register which intern return int value
for validation. so there is no scope to preserve struct musb
pointer but the same can be used in .remove musb_stop.
So fix this by return musb_register with struct musb pointer.

Cc: Igor Grinberg <grinberg@compulab.co.il>
Cc: Purna Chandra Mandal <purna.mandal@microchip.com>
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v2:
- use proper error codes for musb_register

 board/compulab/cm_t3517/cm_t3517.c |  4 ++--
 drivers/usb/musb-new/musb_uboot.c  | 12 ++++++------
 drivers/usb/musb-new/pic32.c       |  6 ++++--
 drivers/usb/musb-new/sunxi.c       |  8 +++++---
 include/linux/usb/musb.h           |  4 ++--
 5 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/board/compulab/cm_t3517/cm_t3517.c b/board/compulab/cm_t3517/cm_t3517.c
index 09cb27def0..668bb7631a 100644
--- a/board/compulab/cm_t3517/cm_t3517.c
+++ b/board/compulab/cm_t3517/cm_t3517.c
@@ -74,8 +74,8 @@ static void cm_t3517_musb_init(void)
 			CONF2_REFFREQ_13MHZ | CONF2_SESENDEN |
 			CONF2_VBDTCTEN | CONF2_DATPOL);
 
-	if (musb_register(&cm_t3517_musb_pdata, &cm_t3517_musb_board_data,
-			  (void *)AM35XX_IPSS_USBOTGSS_BASE))
+	if (!musb_register(&cm_t3517_musb_pdata, &cm_t3517_musb_board_data,
+			   (void *)AM35XX_IPSS_USBOTGSS_BASE))
 		printf("Failed initializing AM35x MUSB!\n");
 }
 #else
diff --git a/drivers/usb/musb-new/musb_uboot.c b/drivers/usb/musb-new/musb_uboot.c
index 2b04fbd046..2bf918eab4 100644
--- a/drivers/usb/musb-new/musb_uboot.c
+++ b/drivers/usb/musb-new/musb_uboot.c
@@ -419,8 +419,8 @@ int usb_gadget_unregister_driver(struct usb_gadget_driver *driver)
 }
 #endif /* CONFIG_USB_MUSB_GADGET */
 
-int musb_register(struct musb_hdrc_platform_data *plat, void *bdata,
-			void *ctl_regs)
+struct musb *musb_register(struct musb_hdrc_platform_data *plat, void *bdata,
+			   void *ctl_regs)
 {
 	struct musb **musbp;
 
@@ -436,14 +436,14 @@ int musb_register(struct musb_hdrc_platform_data *plat, void *bdata,
 		break;
 #endif
 	default:
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 	}
 
 	*musbp = musb_init_controller(plat, (struct device *)bdata, ctl_regs);
-	if (!*musbp) {
+	if (IS_ERR(*musbp)) {
 		printf("Failed to init the controller\n");
-		return -EIO;
+		return ERR_CAST(*musbp);
 	}
 
-	return 0;
+	return *musbp;
 }
diff --git a/drivers/usb/musb-new/pic32.c b/drivers/usb/musb-new/pic32.c
index f04719d7af..3a19900e21 100644
--- a/drivers/usb/musb-new/pic32.c
+++ b/drivers/usb/musb-new/pic32.c
@@ -251,9 +251,11 @@ static int musb_usb_probe(struct udevice *dev)
 	ret = musb_lowlevel_init(mdata);
 #else
 	pic32_musb_plat.mode = MUSB_PERIPHERAL;
-	ret = musb_register(&pic32_musb_plat, &pdata->dev, mregs);
+	mdata->host = musb_register(&pic32_musb_plat, &pdata->dev, mregs);
+	if (!mdata->host)
+		return -EIO;
 #endif
-	if (ret == 0)
+	if ((ret == 0) && mdata->host)
 		printf("PIC32 MUSB OTG\n");
 
 	return ret;
diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c
index 08de9c69c7..b846afb094 100644
--- a/drivers/usb/musb-new/sunxi.c
+++ b/drivers/usb/musb-new/sunxi.c
@@ -444,9 +444,11 @@ static int musb_usb_probe(struct udevice *dev)
 		printf("Allwinner mUSB OTG (Host)\n");
 #else
 	pdata.mode = MUSB_PERIPHERAL;
-	ret = musb_register(&pdata, &glue->dev, base);
-	if (!ret)
-		printf("Allwinner mUSB OTG (Peripheral)\n");
+	host->host = musb_register(&pdata, &glue->dev, base);
+	if (!host->host)
+		return -EIO;
+
+	printf("Allwinner mUSB OTG (Peripheral)\n");
 #endif
 
 	return ret;
diff --git a/include/linux/usb/musb.h b/include/linux/usb/musb.h
index 9104414cf0..a31ce67a81 100644
--- a/include/linux/usb/musb.h
+++ b/include/linux/usb/musb.h
@@ -150,7 +150,7 @@ extern int tusb6010_platform_retime(unsigned is_refclk);
 /*
  * U-Boot specfic stuff
  */
-int musb_register(struct musb_hdrc_platform_data *plat, void *bdata,
-			void *ctl_regs);
+struct musb *musb_register(struct musb_hdrc_platform_data *plat, void *bdata,
+			   void *ctl_regs);
 
 #endif /* __LINUX_USB_MUSB_H */
-- 
2.17.1

[U-Boot] [PATCH v2 2/6] usb: musb-new: sunxi: Allocate struct phy in private

[Jagan Teki]

Allocate struct phy in private structure instead of allocating
locally and assign it to a pointer. This eventually fix miss
alignment phy which is used in another functions.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v2:
- none

 drivers/usb/musb-new/sunxi.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c
index b846afb094..aa2880eeb9 100644
--- a/drivers/usb/musb-new/sunxi.c
+++ b/drivers/usb/musb-new/sunxi.c
@@ -87,7 +87,7 @@ struct sunxi_glue {
 	struct sunxi_ccm_reg *ccm;
 	struct sunxi_musb_config *cfg;
 	struct device dev;
-	struct phy *phy;
+	struct phy phy;
 };
 #define to_sunxi_glue(d)	container_of(d, struct sunxi_glue, dev)
 
@@ -235,19 +235,19 @@ static int sunxi_musb_enable(struct musb *musb)
 	musb_writeb(musb->mregs, USBC_REG_o_VEND0, 0);
 
 	if (is_host_enabled(musb)) {
-		ret = sun4i_usb_phy_vbus_detect(glue->phy);
+		ret = sun4i_usb_phy_vbus_detect(&glue->phy);
 		if (ret == 1) {
 			printf("A charger is plugged into the OTG: ");
 			return -ENODEV;
 		}
 
-		ret = sun4i_usb_phy_id_detect(glue->phy);
+		ret = sun4i_usb_phy_id_detect(&glue->phy);
 		if (ret == 1) {
 			printf("No host cable detected: ");
 			return -ENODEV;
 		}
 
-		ret = generic_phy_power_on(glue->phy);
+		ret = generic_phy_power_on(&glue->phy);
 		if (ret) {
 			pr_err("failed to power on USB PHY\n");
 			return ret;
@@ -271,7 +271,7 @@ static void sunxi_musb_disable(struct musb *musb)
 		return;
 
 	if (is_host_enabled(musb)) {
-		ret = generic_phy_power_off(glue->phy);
+		ret = generic_phy_power_off(&glue->phy);
 		if (ret) {
 			pr_err("failed to power off USB PHY\n");
 			return;
@@ -291,7 +291,7 @@ static int sunxi_musb_init(struct musb *musb)
 
 	pr_debug("%s():\n", __func__);
 
-	ret = generic_phy_init(glue->phy);
+	ret = generic_phy_init(&glue->phy);
 	if (ret) {
 		pr_err("failed to init USB PHY\n");
 		return ret;
@@ -330,14 +330,14 @@ static void sunxi_musb_pre_root_reset_end(struct musb *musb)
 {
 	struct sunxi_glue *glue = to_sunxi_glue(musb->controller);
 
-	sun4i_usb_phy_set_squelch_detect(glue->phy, false);
+	sun4i_usb_phy_set_squelch_detect(&glue->phy, false);
 }
 
 static void sunxi_musb_post_root_reset_end(struct musb *musb)
 {
 	struct sunxi_glue *glue = to_sunxi_glue(musb->controller);
 
-	sun4i_usb_phy_set_squelch_detect(glue->phy, true);
+	sun4i_usb_phy_set_squelch_detect(&glue->phy, true);
 }
 
 static const struct musb_platform_ops sunxi_musb_ops = {
@@ -405,7 +405,6 @@ static int musb_usb_probe(struct udevice *dev)
 	struct usb_bus_priv *priv = dev_get_uclass_priv(dev);
 	struct musb_hdrc_platform_data pdata;
 	void *base = dev_read_addr_ptr(dev);
-	struct phy phy;
 	int ret;
 
 	if (!base)
@@ -419,13 +418,12 @@ static int musb_usb_probe(struct udevice *dev)
 	if (IS_ERR(glue->ccm))
 		return PTR_ERR(glue->ccm);
 
-	ret = generic_phy_get_by_name(dev, "usb", &phy);
+	ret = generic_phy_get_by_name(dev, "usb", &glue->phy);
 	if (ret) {
 		pr_err("failed to get usb PHY\n");
 		return ret;
 	}
 
-	glue->phy = &phy;
 	priv->desc_before_addr = true;
 
 	memset(&pdata, 0, sizeof(pdata));
@@ -460,8 +458,8 @@ static int musb_usb_remove(struct udevice *dev)
 	struct musb_host_data *host = &glue->mdata;
 	int ret;
 
-	if (generic_phy_valid(glue->phy)) {
-		ret = generic_phy_exit(glue->phy);
+	if (generic_phy_valid(&glue->phy)) {
+		ret = generic_phy_exit(&glue->phy);
 		if (ret) {
 			pr_err("failed to exit %s USB PHY\n", dev->name);
 			return ret;
-- 
2.17.1

[U-Boot] [RFC PATCH v2 3/6] dm: usb: Add UCLASS_USB_DEV_GENERIC shutdown

[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 <sjg@chromium.org>
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
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(+)

diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index 611ea97a72..99cf3d2b49 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -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)
+{
+	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;
 
+	ret = __usb_stop();
+	if (ret)
+		return ret;
+
 	/* De-activate any devices that have been activated */
 	ret = uclass_get(UCLASS_USB, &uc);
 	if (ret)
-- 
2.17.1

[U-Boot] [PATCH v2 4/6] musb-new: sunxi: Access ahb_reset0_cfg via ccm offset

[Jagan Teki]

reset0 is not available for sun4i, 5i and 7i so access
the reset0 offset from ccm via driver data for relevant
Allwinner SoC. this will eventually drop the existing
ifdef for SUN6I.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v2:
- new patch

 drivers/usb/musb-new/sunxi.c | 40 +++++++++++++++++++++++++-----------
 1 file changed, 28 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c
index aa2880eeb9..d909b9ea53 100644
--- a/drivers/usb/musb-new/sunxi.c
+++ b/drivers/usb/musb-new/sunxi.c
@@ -76,15 +76,20 @@
  * From usbc/usbc.c
  ******************************************************************************/
 
+#define OFF_SUN6I_AHB_RESET0	0x2c0
+
 struct sunxi_musb_config {
 	struct musb_hdrc_config *config;
+	bool has_reset;
 	u8 rst_bit;
 	u8 clkgate_bit;
+	u32 off_reset0;
 };
 
 struct sunxi_glue {
 	struct musb_host_data mdata;
 	struct sunxi_ccm_reg *ccm;
+	u32 *reg_reset0;
 	struct sunxi_musb_config *cfg;
 	struct device dev;
 	struct phy phy;
@@ -303,12 +308,12 @@ static int sunxi_musb_init(struct musb *musb)
 	if (glue->cfg->clkgate_bit)
 		setbits_le32(&glue->ccm->ahb_gate0,
 			     BIT(glue->cfg->clkgate_bit));
-#ifdef CONFIG_SUNXI_GEN_SUN6I
-	setbits_le32(&glue->ccm->ahb_reset0_cfg, BIT(AHB_GATE_OFFSET_USB0));
+
+	if (glue->cfg->has_reset)
+		setbits_le32(glue->reg_reset0, BIT(AHB_GATE_OFFSET_USB0));
+
 	if (glue->cfg->rst_bit)
-		setbits_le32(&glue->ccm->ahb_reset0_cfg,
-			     BIT(glue->cfg->rst_bit));
-#endif
+		setbits_le32(glue->reg_reset0, BIT(glue->cfg->rst_bit));
 
 	USBC_ConfigFIFO_Base();
 	USBC_EnableDpDmPullUp(musb->mregs);
@@ -418,6 +423,8 @@ static int musb_usb_probe(struct udevice *dev)
 	if (IS_ERR(glue->ccm))
 		return PTR_ERR(glue->ccm);
 
+	glue->reg_reset0 = (void *)glue->ccm + glue->cfg->off_reset0;
+
 	ret = generic_phy_get_by_name(dev, "usb", &glue->phy);
 	if (ret) {
 		pr_err("failed to get usb PHY\n");
@@ -468,12 +475,12 @@ static int musb_usb_remove(struct udevice *dev)
 
 	musb_stop(host->host);
 
-#ifdef CONFIG_SUNXI_GEN_SUN6I
-	clrbits_le32(&glue->ccm->ahb_reset0_cfg, BIT(AHB_GATE_OFFSET_USB0));
+	if (glue->cfg->has_reset)
+		clrbits_le32(glue->reg_reset0, BIT(AHB_GATE_OFFSET_USB0));
+
 	if (glue->cfg->rst_bit)
-		clrbits_le32(&glue->ccm->ahb_reset0_cfg,
-			     BIT(glue->cfg->rst_bit));
-#endif
+		clrbits_le32(glue->reg_reset0, BIT(glue->cfg->rst_bit));
+
 	clrbits_le32(&glue->ccm->ahb_gate0, BIT(AHB_GATE_OFFSET_USB0));
 	if (glue->cfg->clkgate_bit)
 		clrbits_le32(&glue->ccm->ahb_gate0,
@@ -487,21 +494,30 @@ static int musb_usb_remove(struct udevice *dev)
 
 static const struct sunxi_musb_config sun4i_a10_cfg = {
 	.config = &musb_config,
+	.has_reset = false,
+};
+
+static const struct sunxi_musb_config sun6i_a31_cfg = {
+	.config = &musb_config,
+	.has_reset = true,
+	.off_reset0 = OFF_SUN6I_AHB_RESET0,
 };
 
 static const struct sunxi_musb_config sun8i_h3_cfg = {
 	.config = &musb_config_h3,
+	.has_reset = true,
 	.rst_bit = 23,
 	.clkgate_bit = 23,
+	.off_reset0 = OFF_SUN6I_AHB_RESET0,
 };
 
 static const struct udevice_id sunxi_musb_ids[] = {
 	{ .compatible = "allwinner,sun4i-a10-musb",
 			.data = (ulong)&sun4i_a10_cfg },
 	{ .compatible = "allwinner,sun6i-a31-musb",
-			.data = (ulong)&sun4i_a10_cfg },
+			.data = (ulong)&sun6i_a31_cfg },
 	{ .compatible = "allwinner,sun8i-a33-musb",
-			.data = (ulong)&sun4i_a10_cfg },
+			.data = (ulong)&sun6i_a31_cfg },
 	{ .compatible = "allwinner,sun8i-h3-musb",
 			.data = (ulong)&sun8i_h3_cfg },
 	{ }
-- 
2.17.1

[U-Boot] [PATCH v2 5/6] usb: musb-new: sunxi: Add proper musb exit support

[Jagan Teki]

musb have platform ops to do proper graceful exit,
so add the exit call and move musb platform exit code
instead of keeping it in driver remove.
This make proper shutdown of musb where .remove will
call disable, exit serially via musb_stop.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v2:
- use driver data for sun6i changes

 drivers/usb/musb-new/sunxi.c | 49 ++++++++++++++++++++----------------
 1 file changed, 28 insertions(+), 21 deletions(-)

diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c
index d909b9ea53..6cf9826cda 100644
--- a/drivers/usb/musb-new/sunxi.c
+++ b/drivers/usb/musb-new/sunxi.c
@@ -331,6 +331,33 @@ static int sunxi_musb_init(struct musb *musb)
 	return 0;
 }
 
+static int sunxi_musb_exit(struct musb *musb)
+{
+	struct sunxi_glue *glue = to_sunxi_glue(musb->controller);
+	int ret = 0;
+
+	if (generic_phy_valid(&glue->phy)) {
+		ret = generic_phy_exit(&glue->phy);
+		if (ret) {
+			dev_err(dev, "failed to power off usb phy\n");
+			return ret;
+		}
+	}
+
+	if (glue->cfg->has_reset)
+		clrbits_le32(glue->reg_reset0, BIT(AHB_GATE_OFFSET_USB0));
+
+	if (glue->cfg->rst_bit)
+		clrbits_le32(glue->reg_reset0, BIT(glue->cfg->rst_bit));
+
+	clrbits_le32(&glue->ccm->ahb_gate0, BIT(AHB_GATE_OFFSET_USB0));
+	if (glue->cfg->clkgate_bit)
+		clrbits_le32(&glue->ccm->ahb_gate0,
+			     BIT(glue->cfg->clkgate_bit));
+
+	return 0;
+}
+
 static void sunxi_musb_pre_root_reset_end(struct musb *musb)
 {
 	struct sunxi_glue *glue = to_sunxi_glue(musb->controller);
@@ -347,6 +374,7 @@ static void sunxi_musb_post_root_reset_end(struct musb *musb)
 
 static const struct musb_platform_ops sunxi_musb_ops = {
 	.init		= sunxi_musb_init,
+	.exit		= sunxi_musb_exit,
 	.enable		= sunxi_musb_enable,
 	.disable	= sunxi_musb_disable,
 	.pre_root_reset_end = sunxi_musb_pre_root_reset_end,
@@ -463,29 +491,8 @@ static int musb_usb_remove(struct udevice *dev)
 {
 	struct sunxi_glue *glue = dev_get_priv(dev);
 	struct musb_host_data *host = &glue->mdata;
-	int ret;
-
-	if (generic_phy_valid(&glue->phy)) {
-		ret = generic_phy_exit(&glue->phy);
-		if (ret) {
-			pr_err("failed to exit %s USB PHY\n", dev->name);
-			return ret;
-		}
-	}
 
 	musb_stop(host->host);
-
-	if (glue->cfg->has_reset)
-		clrbits_le32(glue->reg_reset0, BIT(AHB_GATE_OFFSET_USB0));
-
-	if (glue->cfg->rst_bit)
-		clrbits_le32(glue->reg_reset0, BIT(glue->cfg->rst_bit));
-
-	clrbits_le32(&glue->ccm->ahb_gate0, BIT(AHB_GATE_OFFSET_USB0));
-	if (glue->cfg->clkgate_bit)
-		clrbits_le32(&glue->ccm->ahb_gate0,
-			     BIT(glue->cfg->clkgate_bit));
-
 	free(host->host);
 	host->host = NULL;
 
-- 
2.17.1

[U-Boot] [PATCH v2 6/6] usb: musb-new: Call musb_platform_exit from musb_stop

[Jagan Teki]

musb stop is musb core call during unregister or shutting down
gadget or host musb. For graceful exit add musb_platform_exit
on musb_stop so-that it can exit the musb platform driver as well.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v2:
- update proper commit message

 drivers/usb/musb-new/musb_core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/musb-new/musb_core.c b/drivers/usb/musb-new/musb_core.c
index 8fec6f38ad..afea9fbcef 100644
--- a/drivers/usb/musb-new/musb_core.c
+++ b/drivers/usb/musb-new/musb_core.c
@@ -1007,6 +1007,7 @@ void musb_stop(struct musb *musb)
 	 *  - ...
 	 */
 	musb_platform_try_idle(musb, 0);
+	musb_platform_exit(musb);
 }
 
 #ifndef __UBOOT__
-- 
2.17.1

[U-Boot] [PATCH v2 0/6] musb-new: Improve shutdown code

[Marek Vasut]

On 07/20/2018 09:13 AM, Jagan Teki wrote:
> 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(-)
> 

+CC Vasily, I want his AB/TB on this. Please keep him CCed on those USB
patches.

-- 
Best regards,
Marek Vasut

[U-Boot] [RFC PATCH v2 3/6] dm: usb: Add UCLASS_USB_DEV_GENERIC shutdown

[Simon Glass]

Hi Jagan,

On 20 July 2018 at 01:13, Jagan Teki <jagan@amarulasolutions.com> wrote:
> 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 <sjg@chromium.org>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
> 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(+)
>
> diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
> index 611ea97a72..99cf3d2b49 100644
> --- a/drivers/usb/host/usb-uclass.c
> +++ b/drivers/usb/host/usb-uclass.c
> @@ -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()

> +{
> +       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;
>
> +       ret = __usb_stop();
> +       if (ret)
> +               return ret;
> +

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

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

Regards,
Simon

[U-Boot] [RFC PATCH v2 3/6] dm: usb: Add UCLASS_USB_DEV_GENERIC shutdown

[Jagan Teki]

On Tue, Jul 24, 2018 at 5:18 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Jagan,
>
> On 20 July 2018 at 01:13, Jagan Teki <jagan@amarulasolutions.com> wrote:
>> 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 <sjg@chromium.org>
>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>> ---
>> 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(+)
>>
>> diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
>> index 611ea97a72..99cf3d2b49 100644
>> --- a/drivers/usb/host/usb-uclass.c
>> +++ b/drivers/usb/host/usb-uclass.c
>> @@ -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()
>
>> +{
>> +       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;
>>
>> +       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?

[U-Boot] [PATCH v2 0/6] musb-new: Improve shutdown code

[Chen-Yu Tsai]

On Fri, Jul 20, 2018 at 3:13 PM, Jagan Teki <jagan@amarulasolutions.com> wrote:
> 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
>

Tested-by: Chen-Yu Tsai <wens@csie.org>

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

[U-Boot] [RFC PATCH v2 3/6] dm: usb: Add UCLASS_USB_DEV_GENERIC shutdown

[Simon Glass]

Hi Jagan,

On 7 August 2018 at 01:03, Jagan Teki <jagan@amarulasolutions.com> wrote:
> On Tue, Jul 24, 2018 at 5:18 AM, Simon Glass <sjg@chromium.org> wrote:
>> Hi Jagan,
>>
>> On 20 July 2018 at 01:13, Jagan Teki <jagan@amarulasolutions.com> wrote:
>>> 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 <sjg@chromium.org>
>>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>>> ---
>>> 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(+)
>>>
>>> diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
>>> index 611ea97a72..99cf3d2b49 100644
>>> --- a/drivers/usb/host/usb-uclass.c
>>> +++ b/drivers/usb/host/usb-uclass.c
>>> @@ -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()
>>
>>> +{
>>> +       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;
>>>
>>> +       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?

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?

Thanks,
Simon

[U-Boot] [RFC PATCH v2 3/6] dm: usb: Add UCLASS_USB_DEV_GENERIC shutdown

[Jagan Teki]

Hi Simon,

On Fri, Aug 17, 2018 at 6:18 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi Jagan,
>
> On 7 August 2018 at 01:03, Jagan Teki <jagan@amarulasolutions.com> wrote:
>> On Tue, Jul 24, 2018 at 5:18 AM, Simon Glass <sjg@chromium.org> wrote:
>>> Hi Jagan,
>>>
>>> On 20 July 2018 at 01:13, Jagan Teki <jagan@amarulasolutions.com> wrote:
>>>> 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 <sjg@chromium.org>
>>>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>>>> ---
>>>> 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(+)
>>>>
>>>> diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
>>>> index 611ea97a72..99cf3d2b49 100644
>>>> --- a/drivers/usb/host/usb-uclass.c
>>>> +++ b/drivers/usb/host/usb-uclass.c
>>>> @@ -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()
>>>
>>>> +{
>>>> +       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;
>>>>
>>>> +       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?
>
> 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?

Jagan.

[U-Boot] [PATCH v2 0/6] musb-new: Improve shutdown code

[Jagan Teki]

On Mon, Aug 13, 2018 at 4:33 PM, Chen-Yu Tsai <wens@csie.org> wrote:
> On Fri, Jul 20, 2018 at 3:13 PM, Jagan Teki <jagan@amarulasolutions.com> wrote:
>> 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
>>
>
> Tested-by: Chen-Yu Tsai <wens@csie.org>
>
> on A33-OlinuXino. This fixes the abort when starting the boot sequence.

Does it fixes w/o 3/6? because it an RFC and still in discussion

[U-Boot] [PATCH v2 0/6] musb-new: Improve shutdown code

[Jagan Teki]

On Fri, Jul 20, 2018 at 12:43 PM, Jagan Teki <jagan@amarulasolutions.com> wrote:
> 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

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

Jagan.

[U-Boot] [RFC PATCH v2 3/6] dm: usb: Add UCLASS_USB_DEV_GENERIC shutdown

[Simon Glass]

Hi Jagan,

On 17 August 2018 at 07:37, Jagan Teki <jagan@amarulasolutions.com> wrote:
> Hi Simon,
>
> On Fri, Aug 17, 2018 at 6:18 PM, Simon Glass <sjg@chromium.org> wrote:
>> Hi Jagan,
>>
>> On 7 August 2018 at 01:03, Jagan Teki <jagan@amarulasolutions.com> wrote:
>>> On Tue, Jul 24, 2018 at 5:18 AM, Simon Glass <sjg@chromium.org> wrote:
>>>> Hi Jagan,
>>>>
>>>> On 20 July 2018 at 01:13, Jagan Teki <jagan@amarulasolutions.com> wrote:
>>>>> 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 <sjg@chromium.org>
>>>>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>>>>> ---
>>>>> 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(+)
>>>>>
>>>>> diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
>>>>> index 611ea97a72..99cf3d2b49 100644
>>>>> --- a/drivers/usb/host/usb-uclass.c
>>>>> +++ b/drivers/usb/host/usb-uclass.c
>>>>> @@ -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()
>>>>
>>>>> +{
>>>>> +       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;
>>>>>
>>>>> +       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?
>>
>> 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?

But if you unbind the bus, doesn't this automatically unbind its children?

At the very least, it seems like there is some common code here that
you should factor out.

Regards,
Simon

[U-Boot] [linux-sunxi] Re: [PATCH v2 0/6] musb-new: Improve shutdown code

[Chen-Yu Tsai]

On Tue, Aug 21, 2018 at 9:04 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> On Fri, Jul 20, 2018 at 12:43 PM, Jagan Teki <jagan@amarulasolutions.com> wrote:
> > 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
>
> 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.

ChenYu

[U-Boot] [linux-sunxi] Re: [PATCH v2 0/6] musb-new: Improve shutdown code

[Jagan Teki]

On Wed, Aug 22, 2018 at 8:11 AM, Chen-Yu Tsai <wens@csie.org> wrote:
> On Tue, Aug 21, 2018 at 9:04 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>>
>> On Fri, Jul 20, 2018 at 12:43 PM, Jagan Teki <jagan@amarulasolutions.com> wrote:
>> > 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
>>
>> 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.

Thanks.

Collected Tested-by and Applied to u-boot-sunxi/master

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

Look like Yes. I think we can grab dr_mode to set respective mode.