[PATCHv2 00/15] Get MUSB PM runtime working again

[PATCHv2 00/15] Get MUSB PM runtime working again

[Tony Lindgren]

Hi all,

Here's the whole series reposted with a bunch of additional changes.
It seems to now properly work with with multiple phy cable status
events, and should work for Ivaylo on n900 too.

Please re-review and re-test.

Regards,

Tony

Tony Lindgren (15):
  usb: musb: Fix idling after host mode by increasing autosuspend delay
  usb: musb: Remove unnecessary shutdown function
  usb: musb: Update to use PM runtime autosuspend
  usb: musb: Split PM runtime between wrapper IP and musb core
  usb: musb: Remove conditional PM runtime calls for musb_gadget
  usb: musb: Use delayed for musb_gadget_pullup
  usb: musb: Handle cable status better for 2430 glue layer
  usb: musb: Improve PM runtime and phy handling for 2430 glue layer
  usb: musb: Remove try_idle for 2430 glue layer
  usb: musb: Don't set d+ high before enable for 2430 glue layer
  usb: musb: Return error value from musb_mailbox
  usb: musb: Remove extra PM runtime calls from 2430 glue layer
  usb: musb: Remove pm_runtime_set_irq_safe
  usb: musb: Use normal module_init for 2430 glue
  usb: phy: Check initial state for twl6030

 drivers/phy/phy-twl4030-usb.c     |  14 +-
 drivers/usb/musb/musb_core.c      |  82 +++++-------
 drivers/usb/musb/musb_core.h      |   3 +-
 drivers/usb/musb/musb_gadget.c    |  34 +++--
 drivers/usb/musb/omap2430.c       | 260 ++++++++++++++------------------------
 drivers/usb/phy/phy-twl6030-usb.c |  29 ++++-
 include/linux/usb/musb.h          |   5 +-
 7 files changed, 189 insertions(+), 238 deletions(-)

-- 
2.8.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 01/15] usb: musb: Fix idling after host mode by increasing autosuspend delay

[Tony Lindgren]

Looks like at least 2430 glue won't idle reliably with the 200 ms
autosuspend delay. This causes deeper idle states being blocked for
the whole SoC when disconnecting OTG A cable.

Increasing the delay to 500 ms seems to idle both MUSB and the PHY
reliably. This is probably because of time needed by the hardware
based negotiation between MUSB and the PHY.

Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
 drivers/usb/musb/musb_core.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 39fd958..460176c 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -2028,9 +2028,15 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
 	musb_readl = musb_default_readl;
 	musb_writel = musb_default_writel;
 
-	/* We need musb_read/write functions initialized for PM */
+	/*
+	 * We need musb_read/write functions initialized for PM.
+	 * Note that at least 2430 glue needs autosuspend delay
+	 * somewhere above 300 ms for the hardware to idle properly
+	 * after disconnecting the cable in host mode. Let's use
+	 * 500 ms for some margin.
+	 */
 	pm_runtime_use_autosuspend(musb->controller);
-	pm_runtime_set_autosuspend_delay(musb->controller, 200);
+	pm_runtime_set_autosuspend_delay(musb->controller, 500);
 	pm_runtime_enable(musb->controller);
 
 	/* The musb_platform_init() call:
-- 
2.8.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 02/15] usb: musb: Remove unnecessary shutdown function

[Tony Lindgren]

We have remove() already calling shutdown(), so let's drop it
and move the code to remove(). No code changes, we'll drop the
the FIXME in the following patch with more clean-up.

Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
 drivers/usb/musb/musb_core.c | 39 ++++++++++++++-------------------------
 1 file changed, 14 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 460176c..c370ed5 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1090,29 +1090,6 @@ void musb_stop(struct musb *musb)
 	musb_platform_try_idle(musb, 0);
 }
 
-static void musb_shutdown(struct platform_device *pdev)
-{
-	struct musb	*musb = dev_to_musb(&pdev->dev);
-	unsigned long	flags;
-
-	pm_runtime_get_sync(musb->controller);
-
-	musb_host_cleanup(musb);
-	musb_gadget_cleanup(musb);
-
-	spin_lock_irqsave(&musb->lock, flags);
-	musb_platform_disable(musb);
-	musb_generic_disable(musb);
-	spin_unlock_irqrestore(&musb->lock, flags);
-
-	musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
-	musb_platform_exit(musb);
-
-	pm_runtime_put(musb->controller);
-	/* FIXME power down */
-}
-
-
 /*-------------------------------------------------------------------------*/
 
 /*
@@ -2318,6 +2295,7 @@ static int musb_remove(struct platform_device *pdev)
 {
 	struct device	*dev = &pdev->dev;
 	struct musb	*musb = dev_to_musb(dev);
+	unsigned long	flags;
 
 	/* this gets called on rmmod.
 	 *  - Host mode: host may still be active
@@ -2325,7 +2303,19 @@ static int musb_remove(struct platform_device *pdev)
 	 *  - OTG mode: both roles are deactivated (or never-activated)
 	 */
 	musb_exit_debugfs(musb);
-	musb_shutdown(pdev);
+
+	pm_runtime_get_sync(musb->controller);
+	musb_host_cleanup(musb);
+	musb_gadget_cleanup(musb);
+	spin_lock_irqsave(&musb->lock, flags);
+	musb_platform_disable(musb);
+	musb_generic_disable(musb);
+	spin_unlock_irqrestore(&musb->lock, flags);
+	musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
+	musb_platform_exit(musb);
+	pm_runtime_put(musb->controller);
+	/* FIXME power down */
+
 	musb_phy_callback = NULL;
 
 	if (musb->dma_controller)
@@ -2618,7 +2608,6 @@ static struct platform_driver musb_driver = {
 	},
 	.probe		= musb_probe,
 	.remove		= musb_remove,
-	.shutdown	= musb_shutdown,
 };
 
 module_platform_driver(musb_driver);
-- 
2.8.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 03/15] usb: musb: Update to use PM runtime autosuspend

[Tony Lindgren]

Let's make the PM runtime use the standard autosuspend calls.
Commit 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe
error and driver unbind") means we must pair use_autosuspend with
dont_use_autosuspend and then use put_sync to properly idle the
device.

Note that we'll be removing the PM runtime calls from the glue
layer to the MUSB core in the next patch. And we can also remove
the pointless FIXME comment now.

Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
 drivers/usb/musb/musb_core.c   | 9 ++++++---
 drivers/usb/musb/musb_gadget.c | 3 ++-
 drivers/usb/musb/omap2430.c    | 5 +++--
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index c370ed5..89c270a 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -2220,7 +2220,8 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
 	if (status)
 		goto fail5;
 
-	pm_runtime_put(musb->controller);
+	pm_runtime_mark_last_busy(musb->controller);
+	pm_runtime_put_autosuspend(musb->controller);
 
 	/*
 	 * For why this is currently needed, see commit 3e43a0725637
@@ -2248,6 +2249,7 @@ fail2_5:
 	usb_phy_shutdown(musb->xceiv);
 
 err_usb_phy_init:
+	pm_runtime_dont_use_autosuspend(musb->controller);
 	pm_runtime_put_sync(musb->controller);
 
 fail2:
@@ -2313,8 +2315,6 @@ static int musb_remove(struct platform_device *pdev)
 	spin_unlock_irqrestore(&musb->lock, flags);
 	musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
 	musb_platform_exit(musb);
-	pm_runtime_put(musb->controller);
-	/* FIXME power down */
 
 	musb_phy_callback = NULL;
 
@@ -2326,6 +2326,9 @@ static int musb_remove(struct platform_device *pdev)
 	cancel_work_sync(&musb->irq_work);
 	cancel_delayed_work_sync(&musb->finish_resume_work);
 	cancel_delayed_work_sync(&musb->deassert_reset_work);
+	pm_runtime_dont_use_autosuspend(musb->controller);
+	pm_runtime_put_sync(musb->controller);
+	pm_runtime_disable(musb->controller);
 	musb_free(musb);
 	device_init_wakeup(dev, 0);
 	return 0;
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index 152865b..fff5a8a 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -1963,7 +1963,8 @@ static int musb_gadget_stop(struct usb_gadget *g)
 	 * that currently misbehaves.
 	 */
 
-	pm_runtime_put(musb->controller);
+	pm_runtime_mark_last_busy(musb->controller);
+	pm_runtime_put_autosuspend(musb->controller);
 
 	return 0;
 }
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index c84e0322..07363d2 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -435,8 +435,9 @@ static int omap2430_musb_init(struct musb *musb)
 	phy_init(musb->phy);
 	phy_power_on(musb->phy);
 
-	pm_runtime_put_noidle(musb->controller);
-	pm_runtime_put_noidle(glue->dev);
+	pm_runtime_mark_last_busy(musb->controller);
+	pm_runtime_put_autosuspend(musb->controller);
+	pm_runtime_put(glue->dev);
 	return 0;
 
 err1:
-- 
2.8.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 04/15] usb: musb: Split PM runtime between wrapper IP and musb core

[Tony Lindgren]

Let's not tinker with the PM runtime of musb core from the omap2430
wrapper. This allows us to initialize PM runtime for musb core later
on instead of doing it in stages. And omap2430 wrapper has no need
to for accessing musb core at this point.

Note that this does not remove all the PM runtime calls from the
glue layer, those will get removed in a later patch.

Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
 drivers/usb/musb/musb_core.c | 39 +++++++++++++++++----------------------
 drivers/usb/musb/omap2430.c  | 10 ----------
 2 files changed, 17 insertions(+), 32 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 89c270a..23888d5 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -2005,17 +2005,6 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
 	musb_readl = musb_default_readl;
 	musb_writel = musb_default_writel;
 
-	/*
-	 * We need musb_read/write functions initialized for PM.
-	 * Note that at least 2430 glue needs autosuspend delay
-	 * somewhere above 300 ms for the hardware to idle properly
-	 * after disconnecting the cable in host mode. Let's use
-	 * 500 ms for some margin.
-	 */
-	pm_runtime_use_autosuspend(musb->controller);
-	pm_runtime_set_autosuspend_delay(musb->controller, 500);
-	pm_runtime_enable(musb->controller);
-
 	/* The musb_platform_init() call:
 	 *   - adjusts musb->mregs
 	 *   - sets the musb->isr
@@ -2117,6 +2106,16 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
 	if (musb->ops->phy_callback)
 		musb_phy_callback = musb->ops->phy_callback;
 
+	/*
+	 * We need musb_read/write functions initialized for PM.
+	 * Note that at least 2430 glue needs autosuspend delay
+	 * somewhere above 300 ms for the hardware to idle properly
+	 * after disconnecting the cable in host mode. Let's use
+	 * 500 ms for some margin.
+	 */
+	pm_runtime_use_autosuspend(musb->controller);
+	pm_runtime_set_autosuspend_delay(musb->controller, 500);
+	pm_runtime_enable(musb->controller);
 	pm_runtime_get_sync(musb->controller);
 
 	status = usb_phy_init(musb->xceiv);
@@ -2251,6 +2250,7 @@ fail2_5:
 err_usb_phy_init:
 	pm_runtime_dont_use_autosuspend(musb->controller);
 	pm_runtime_put_sync(musb->controller);
+	pm_runtime_disable(musb->controller);
 
 fail2:
 	if (musb->irq_wake)
@@ -2258,7 +2258,6 @@ fail2:
 	musb_platform_exit(musb);
 
 fail1:
-	pm_runtime_disable(musb->controller);
 	dev_err(musb->controller,
 		"musb_init_controller failed with status %d\n", status);
 
@@ -2306,6 +2305,9 @@ static int musb_remove(struct platform_device *pdev)
 	 */
 	musb_exit_debugfs(musb);
 
+	cancel_work_sync(&musb->irq_work);
+	cancel_delayed_work_sync(&musb->finish_resume_work);
+	cancel_delayed_work_sync(&musb->deassert_reset_work);
 	pm_runtime_get_sync(musb->controller);
 	musb_host_cleanup(musb);
 	musb_gadget_cleanup(musb);
@@ -2314,21 +2316,14 @@ static int musb_remove(struct platform_device *pdev)
 	musb_generic_disable(musb);
 	spin_unlock_irqrestore(&musb->lock, flags);
 	musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
+	pm_runtime_dont_use_autosuspend(musb->controller);
+	pm_runtime_put_sync(musb->controller);
+	pm_runtime_disable(musb->controller);
 	musb_platform_exit(musb);
-
 	musb_phy_callback = NULL;
-
 	if (musb->dma_controller)
 		musb_dma_controller_destroy(musb->dma_controller);
-
 	usb_phy_shutdown(musb->xceiv);
-
-	cancel_work_sync(&musb->irq_work);
-	cancel_delayed_work_sync(&musb->finish_resume_work);
-	cancel_delayed_work_sync(&musb->deassert_reset_work);
-	pm_runtime_dont_use_autosuspend(musb->controller);
-	pm_runtime_put_sync(musb->controller);
-	pm_runtime_disable(musb->controller);
 	musb_free(musb);
 	device_init_wakeup(dev, 0);
 	return 0;
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 07363d2..3ce94bf 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -400,13 +400,6 @@ static int omap2430_musb_init(struct musb *musb)
 	if (status < 0)
 		goto err1;
 
-	status = pm_runtime_get_sync(dev);
-	if (status < 0) {
-		dev_err(dev, "pm_runtime_get_sync FAILED %d\n", status);
-		pm_runtime_put_sync(glue->dev);
-		goto err1;
-	}
-
 	l = musb_readl(musb->mregs, OTG_INTERFSEL);
 
 	if (data->interface_type == MUSB_INTERFACE_UTMI) {
@@ -434,9 +427,6 @@ static int omap2430_musb_init(struct musb *musb)
 
 	phy_init(musb->phy);
 	phy_power_on(musb->phy);

[PATCH 05/15] usb: musb: Remove conditional PM runtime calls for musb_gadget

[Tony Lindgren]

The conditional use of PM runtime does not work properly
for musb gadget. On cable disconnect we may not get any
USB_EVENT_NONE leaving the PM runtime call unpaired.

Let's fix the issue by making sure the PM runtime calls are
paired within the functions. The glue layer will take care
of the rest.

Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
 drivers/usb/musb/musb_gadget.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index fff5a8a..7ecc893 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -1914,8 +1914,8 @@ static int musb_gadget_start(struct usb_gadget *g,
 	if (musb->xceiv->last_event == USB_EVENT_ID)
 		musb_platform_set_vbus(musb, 1);
 
-	if (musb->xceiv->last_event == USB_EVENT_NONE)
-		pm_runtime_put(musb->controller);
+	pm_runtime_mark_last_busy(musb->controller);
+	pm_runtime_put_autosuspend(musb->controller);
 
 	return 0;
 
@@ -1934,8 +1934,7 @@ static int musb_gadget_stop(struct usb_gadget *g)
 	struct musb	*musb = gadget_to_musb(g);
 	unsigned long	flags;
 
-	if (musb->xceiv->last_event == USB_EVENT_NONE)
-		pm_runtime_get_sync(musb->controller);
+	pm_runtime_get_sync(musb->controller);
 
 	/*
 	 * REVISIT always use otg_set_peripheral() here too;
-- 
2.8.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 06/15] usb: musb: Use delayed for musb_gadget_pullup

[Tony Lindgren]

We have MUSB setting pm_runtime_irq_safe with the following
commits:

30a70b026b4c ("usb: musb: fix obex in g_nokia.ko causing kernel panic")
3e43a0725637 ("usb: musb: core: add pm_runtime_irq_safe()")

Let's fix things to use delayed work so we can remove the
pm_runtime_irq_safe.

Note that we may want to set this up in a generic way in the
gadget framework eventually.

Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
 drivers/usb/musb/musb_core.h   |  1 +
 drivers/usb/musb/musb_gadget.c | 24 ++++++++++++++++++------
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index b6afe9e..2947384 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -312,6 +312,7 @@ struct musb {
 	struct work_struct	irq_work;
 	struct delayed_work	deassert_reset_work;
 	struct delayed_work	finish_resume_work;
+	struct delayed_work	gadget_work;
 	u16			hwvers;
 
 	u16			intrrxe;
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index 7ecc893..af2a3a7 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -1656,6 +1656,20 @@ static int musb_gadget_vbus_draw(struct usb_gadget *gadget, unsigned mA)
 	return usb_phy_set_power(musb->xceiv, mA);
 }
 
+static void musb_gadget_work(struct work_struct *work)
+{
+	struct musb *musb;
+	unsigned long flags;
+
+	musb = container_of(work, struct musb, gadget_work.work);
+	pm_runtime_get_sync(musb->controller);
+	spin_lock_irqsave(&musb->lock, flags);
+	musb_pullup(musb, musb->softconnect);
+	spin_unlock_irqrestore(&musb->lock, flags);
+	pm_runtime_mark_last_busy(musb->controller);
+	pm_runtime_put_autosuspend(musb->controller);
+}
+
 static int musb_gadget_pullup(struct usb_gadget *gadget, int is_on)
 {
 	struct musb	*musb = gadget_to_musb(gadget);
@@ -1663,20 +1677,16 @@ static int musb_gadget_pullup(struct usb_gadget *gadget, int is_on)
 
 	is_on = !!is_on;
 
-	pm_runtime_get_sync(musb->controller);
-
 	/* NOTE: this assumes we are sensing vbus; we'd rather
 	 * not pullup unless the B-session is active.
 	 */
 	spin_lock_irqsave(&musb->lock, flags);
 	if (is_on != musb->softconnect) {
 		musb->softconnect = is_on;
-		musb_pullup(musb, is_on);
+		schedule_delayed_work(&musb->gadget_work, 0);
 	}
 	spin_unlock_irqrestore(&musb->lock, flags);
 
-	pm_runtime_put(musb->controller);
-
 	return 0;
 }
 
@@ -1845,7 +1855,7 @@ int musb_gadget_setup(struct musb *musb)
 #elif IS_ENABLED(CONFIG_USB_MUSB_GADGET)
 	musb->g.is_otg = 0;
 #endif

[PATCH 07/15] usb: musb: Handle cable status better for 2430 glue layer

[Tony Lindgren]

We may have drivers loaded but no configured gadgets and MUSB may be in
host mode. If gadgets are configured during host mode, PM runtime will
get confused.

Disable PM runtime from gadget state, and do it based on the cable
and last state.

Note that we may get multiple cable events, so we need to keep track
of the power state.

Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
 drivers/usb/musb/omap2430.c | 68 +++++++++++++++++++++++++++++++++++++++------
 1 file changed, 60 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 3ce94bf..02d40bc 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -49,6 +49,9 @@ struct omap2430_glue {
 	enum musb_vbus_id_status status;
 	struct work_struct	omap_musb_mailbox_work;
 	struct device		*control_otghs;
+	bool			cable_connected;
+	bool			enabled;
+	bool			powered;
 };
 #define glue_to_musb(g)		platform_get_drvdata(g->musb)
 
@@ -234,6 +237,45 @@ static inline void omap2430_low_level_init(struct musb *musb)
 	musb_writel(musb->mregs, OTG_FORCESTDBY, l);
 }
 
+/*
+ * We can get multiple cable events so we need to keep track
+ * of the power state. Only keep power enabled if USB cable is
+ * connected and a gadget is started.
+ */
+static void omap2430_set_power(struct musb *musb, bool enabled, bool cable)
+{
+	struct device *dev = musb->controller;
+	struct omap2430_glue *glue = dev_get_drvdata(dev->parent);
+	bool power_up;
+	int res;
+
+	if (glue->enabled != enabled)
+		glue->enabled = enabled;
+
+	if (glue->cable_connected != cable)
+		glue->cable_connected = cable;
+
+	power_up = glue->enabled && glue->cable_connected;
+	if (power_up == glue->powered) {
+		dev_warn(musb->controller, "power state already %i\n",
+			 power_up);
+		return;
+	}
+
+	glue->powered = power_up;
+
+	if (power_up) {
+		res = pm_runtime_get_sync(musb->controller);
+		if (res < 0) {
+			dev_err(musb->controller, "could not enable: %i", res);
+			glue->powered = false;
+		}
+	} else {
+		pm_runtime_mark_last_busy(musb->controller);
+		pm_runtime_put_autosuspend(musb->controller);
+	}
+}
+
 static void omap2430_musb_mailbox(enum musb_vbus_id_status status)
 {
 	struct omap2430_glue	*glue = _glue;
@@ -259,6 +301,13 @@ static void omap_musb_set_mailbox(struct omap2430_glue *glue)
 	struct musb_hdrc_platform_data *pdata = dev_get_platdata(dev);
 	struct omap_musb_board_data *data = pdata->board_data;
 	struct usb_otg *otg = musb->xceiv->otg;
+	bool cable_connected;
+
+	cable_connected = ((glue->status == MUSB_ID_GROUND) ||
+			   (glue->status == MUSB_VBUS_VALID));
+
+	if (cable_connected)
+		omap2430_set_power(musb, glue->enabled, cable_connected);
 
 	switch (glue->status) {
 	case MUSB_ID_GROUND:
@@ -268,7 +317,6 @@ static void omap_musb_set_mailbox(struct omap2430_glue *glue)
 		musb->xceiv->otg->state = OTG_STATE_A_IDLE;
 		musb->xceiv->last_event = USB_EVENT_ID;
 		if (musb->gadget_driver) {
-			pm_runtime_get_sync(dev);
 			omap_control_usb_set_mode(glue->control_otghs,
 				USB_MODE_HOST);
 			omap2430_musb_set_vbus(musb, 1);
@@ -281,8 +329,6 @@ static void omap_musb_set_mailbox(struct omap2430_glue *glue)
 		otg->default_a = false;
 		musb->xceiv->otg->state = OTG_STATE_B_IDLE;
 		musb->xceiv->last_event = USB_EVENT_VBUS;
-		if (musb->gadget_driver)
-			pm_runtime_get_sync(dev);
 		omap_control_usb_set_mode(glue->control_otghs, USB_MODE_DEVICE);
 		break;
 
@@ -291,11 +337,8 @@ static void omap_musb_set_mailbox(struct omap2430_glue *glue)
 		dev_dbg(dev, "VBUS Disconnect\n");
 
 		musb->xceiv->last_event = USB_EVENT_NONE;
-		if (musb->gadget_driver) {
+		if (musb->gadget_driver)
 			omap2430_musb_set_vbus(musb, 0);
-			pm_runtime_mark_last_busy(dev);
-			pm_runtime_put_autosuspend(dev);
-		}
 
 		if (data->interface_type == MUSB_INTERFACE_UTMI)
 			otg_set_vbus(musb->xceiv->otg, 0);
@@ -307,6 +350,9 @@ static void omap_musb_set_mailbox(struct omap2430_glue *glue)
 		dev_dbg(dev, "ID float\n");
 	}
 
+	if (!glue->cable_connected)
+		omap2430_set_power(musb, glue->enabled, cable_connected);
+
 	atomic_notifier_call_chain(&musb->xceiv->notifier,
 			musb->xceiv->last_event, NULL);
 }
@@ -443,6 +489,8 @@ static void omap2430_musb_enable(struct musb *musb)
 	struct musb_hdrc_platform_data *pdata = dev_get_platdata(dev);
 	struct omap_musb_board_data *data = pdata->board_data;
 
+	omap2430_set_power(musb, true, glue->cable_connected);
+
 	switch (glue->status) {
 
 	case MUSB_ID_GROUND:
@@ -481,6 +529,8 @@ static void omap2430_musb_disable(struct musb *musb)
 	if (glue->status != MUSB_UNKNOWN)
 		omap_control_usb_set_mode(glue->control_otghs,
 			USB_MODE_DISCONNECT);
+
+	omap2430_set_power(musb, false, glue->cable_connected);
 }
 
 static int omap2430_musb_exit(struct musb *musb)
@@ -653,11 +703,13 @@ err0:
 
 static int omap2430_remove(struct platform_device *pdev)
 {
-	struct omap2430_glue		*glue = platform_get_drvdata(pdev);
+	struct omap2430_glue *glue = platform_get_drvdata(pdev);
+	struct musb *musb = glue_to_musb(glue);
 
 	pm_runtime_get_sync(glue->dev);
 	cancel_work_sync(&glue->omap_musb_mailbox_work);
 	platform_device_unregister(glue->musb);
+	omap2430_set_power(musb, false, false);
 	pm_runtime_put_sync(glue->dev);
 	pm_runtime_disable(glue->dev);
 
-- 
2.8.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 07/15] usb: musb: Handle cable status better for 2430 glue layer

[Tony Lindgren]

* Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> [160511 17:56]:
> We may have drivers loaded but no configured gadgets and MUSB may be in
> host mode. If gadgets are configured during host mode, PM runtime will
> get confused.
...

> @@ -307,6 +350,9 @@ static void omap_musb_set_mailbox(struct omap2430_glue *glue)
>  		dev_dbg(dev, "ID float\n");
>  	}
>  
> +	if (!glue->cable_connected)
> +		omap2430_set_power(musb, glue->enabled, cable_connected);
> +

Oops sorry I messed up while cleaning up. This should be just cable_connected
here instead of glue->cable_connected. Otherwise we won't idle on cable
unplug as we're always using the cached value instead of the current
value.

Updated patch below.

Regards,

Tony

8< -----------------
From: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
Date: Mon, 9 May 2016 07:50:57 -0700
Subject: [PATCH] usb: musb: Handle cable status better for 2430 glue layer

We may have drivers loaded but no configured gadgets and MUSB may be in
host mode. If gadgets are configured during host mode, PM runtime will
get confused.

Disable PM runtime from gadget state, and do it based on the cable
and last state.

Note that we may get multiple cable events, so we need to keep track
of the power state.

Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>

--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -49,6 +49,9 @@ struct omap2430_glue {
 	enum musb_vbus_id_status status;
 	struct work_struct	omap_musb_mailbox_work;
 	struct device		*control_otghs;
+	bool			cable_connected;
+	bool			enabled;
+	bool			powered;
 };
 #define glue_to_musb(g)		platform_get_drvdata(g->musb)
 
@@ -234,6 +237,45 @@ static inline void omap2430_low_level_init(struct musb *musb)
 	musb_writel(musb->mregs, OTG_FORCESTDBY, l);
 }
 
+/*
+ * We can get multiple cable events so we need to keep track
+ * of the power state. Only keep power enabled if USB cable is
+ * connected and a gadget is started.
+ */
+static void omap2430_set_power(struct musb *musb, bool enabled, bool cable)
+{
+	struct device *dev = musb->controller;
+	struct omap2430_glue *glue = dev_get_drvdata(dev->parent);
+	bool power_up;
+	int res;
+
+	if (glue->enabled != enabled)
+		glue->enabled = enabled;
+
+	if (glue->cable_connected != cable)
+		glue->cable_connected = cable;
+
+	power_up = glue->enabled && glue->cable_connected;
+	if (power_up == glue->powered) {
+		dev_warn(musb->controller, "power state already %i\n",
+			 power_up);
+		return;
+	}
+
+	glue->powered = power_up;
+
+	if (power_up) {
+		res = pm_runtime_get_sync(musb->controller);
+		if (res < 0) {
+			dev_err(musb->controller, "could not enable: %i", res);
+			glue->powered = false;
+		}
+	} else {
+		pm_runtime_mark_last_busy(musb->controller);
+		pm_runtime_put_autosuspend(musb->controller);
+	}
+}
+
 static void omap2430_musb_mailbox(enum musb_vbus_id_status status)
 {
 	struct omap2430_glue	*glue = _glue;
@@ -259,6 +301,13 @@ static void omap_musb_set_mailbox(struct omap2430_glue *glue)
 	struct musb_hdrc_platform_data *pdata = dev_get_platdata(dev);
 	struct omap_musb_board_data *data = pdata->board_data;
 	struct usb_otg *otg = musb->xceiv->otg;
+	bool cable_connected;
+
+	cable_connected = ((glue->status == MUSB_ID_GROUND) ||
+			   (glue->status == MUSB_VBUS_VALID));
+
+	if (cable_connected)
+		omap2430_set_power(musb, glue->enabled, cable_connected);
 
 	switch (glue->status) {
 	case MUSB_ID_GROUND:
@@ -268,7 +317,6 @@ static void omap_musb_set_mailbox(struct omap2430_glue *glue)
 		musb->xceiv->otg->state = OTG_STATE_A_IDLE;
 		musb->xceiv->last_event = USB_EVENT_ID;
 		if (musb->gadget_driver) {
-			pm_runtime_get_sync(dev);
 			omap_control_usb_set_mode(glue->control_otghs,
 				USB_MODE_HOST);
 			omap2430_musb_set_vbus(musb, 1);
@@ -281,8 +329,6 @@ static void omap_musb_set_mailbox(struct omap2430_glue *glue)
 		otg->default_a = false;
 		musb->xceiv->otg->state = OTG_STATE_B_IDLE;
 		musb->xceiv->last_event = USB_EVENT_VBUS;
-		if (musb->gadget_driver)
-			pm_runtime_get_sync(dev);
 		omap_control_usb_set_mode(glue->control_otghs, USB_MODE_DEVICE);
 		break;
 
@@ -291,11 +337,8 @@ static void omap_musb_set_mailbox(struct omap2430_glue *glue)
 		dev_dbg(dev, "VBUS Disconnect\n");
 
 		musb->xceiv->last_event = USB_EVENT_NONE;
-		if (musb->gadget_driver) {
+		if (musb->gadget_driver)
 			omap2430_musb_set_vbus(musb, 0);
-			pm_runtime_mark_last_busy(dev);
-			pm_runtime_put_autosuspend(dev);
-		}
 
 		if (data->interface_type == MUSB_INTERFACE_UTMI)
 			otg_set_vbus(musb->xceiv->otg, 0);
@@ -307,6 +350,9 @@ static void omap_musb_set_mailbox(struct omap2430_glue *glue)
 		dev_dbg(dev, "ID float\n");
 	}
 
+	if (!cable_connected)
+		omap2430_set_power(musb, glue->enabled, cable_connected);
+
 	atomic_notifier_call_chain(&musb->xceiv->notifier,
 			musb->xceiv->last_event, NULL);
 }
@@ -443,6 +489,8 @@ static void omap2430_musb_enable(struct musb *musb)
 	struct musb_hdrc_platform_data *pdata = dev_get_platdata(dev);
 	struct omap_musb_board_data *data = pdata->board_data;
 
+	omap2430_set_power(musb, true, glue->cable_connected);
+
 	switch (glue->status) {
 
 	case MUSB_ID_GROUND:
@@ -481,6 +529,8 @@ static void omap2430_musb_disable(struct musb *musb)
 	if (glue->status != MUSB_UNKNOWN)
 		omap_control_usb_set_mode(glue->control_otghs,
 			USB_MODE_DISCONNECT);
+
+	omap2430_set_power(musb, false, glue->cable_connected);
 }
 
 static int omap2430_musb_exit(struct musb *musb)
@@ -653,11 +703,13 @@ err0:
 
 static int omap2430_remove(struct platform_device *pdev)
 {
-	struct omap2430_glue		*glue = platform_get_drvdata(pdev);
+	struct omap2430_glue *glue = platform_get_drvdata(pdev);
+	struct musb *musb = glue_to_musb(glue);
 
 	pm_runtime_get_sync(glue->dev);
 	cancel_work_sync(&glue->omap_musb_mailbox_work);
 	platform_device_unregister(glue->musb);
+	omap2430_set_power(musb, false, false);
 	pm_runtime_put_sync(glue->dev);
 	pm_runtime_disable(glue->dev);
 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 07/15] usb: musb: Handle cable status better for 2430 glue layer

[Bin Liu]

Hi,

On Wed, May 11, 2016 at 06:33:04PM -0700, Tony Lindgren wrote:
> * Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> [160511 17:56]:
> > We may have drivers loaded but no configured gadgets and MUSB may be in
> > host mode. If gadgets are configured during host mode, PM runtime will
> > get confused.
> ...
> 
> > @@ -307,6 +350,9 @@ static void omap_musb_set_mailbox(struct omap2430_glue *glue)
> >  		dev_dbg(dev, "ID float\n");
> >  	}
> >  
> > +	if (!glue->cable_connected)
> > +		omap2430_set_power(musb, glue->enabled, cable_connected);
> > +
> 
> Oops sorry I messed up while cleaning up. This should be just cable_connected
> here instead of glue->cable_connected. Otherwise we won't idle on cable
> unplug as we're always using the cached value instead of the current
> value.
> 
> Updated patch below.
> 
> Regards,
> 
> Tony
> 
> 8< -----------------
> From: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> Date: Mon, 9 May 2016 07:50:57 -0700
> Subject: [PATCH] usb: musb: Handle cable status better for 2430 glue layer
> 
> We may have drivers loaded but no configured gadgets and MUSB may be in
> host mode. If gadgets are configured during host mode, PM runtime will
> get confused.
> 
> Disable PM runtime from gadget state, and do it based on the cable
> and last state.
> 
> Note that we may get multiple cable events, so we need to keep track
> of the power state.
> 
> Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> 
> --- a/drivers/usb/musb/omap2430.c
> +++ b/drivers/usb/musb/omap2430.c
> @@ -49,6 +49,9 @@ struct omap2430_glue {
>  	enum musb_vbus_id_status status;
>  	struct work_struct	omap_musb_mailbox_work;
>  	struct device		*control_otghs;
> +	bool			cable_connected;
> +	bool			enabled;
> +	bool			powered;

This variable is only used within omap2430_set_power(), so it can be
local within that function to save a few bytes.

Regards,
-Bin.

>  };
>  #define glue_to_musb(g)		platform_get_drvdata(g->musb)
>  
> @@ -234,6 +237,45 @@ static inline void omap2430_low_level_init(struct musb *musb)
>  	musb_writel(musb->mregs, OTG_FORCESTDBY, l);
>  }
>  
> +/*
> + * We can get multiple cable events so we need to keep track
> + * of the power state. Only keep power enabled if USB cable is
> + * connected and a gadget is started.
> + */
> +static void omap2430_set_power(struct musb *musb, bool enabled, bool cable)
> +{
> +	struct device *dev = musb->controller;
> +	struct omap2430_glue *glue = dev_get_drvdata(dev->parent);
> +	bool power_up;
> +	int res;
> +
> +	if (glue->enabled != enabled)
> +		glue->enabled = enabled;
> +
> +	if (glue->cable_connected != cable)
> +		glue->cable_connected = cable;
> +
> +	power_up = glue->enabled && glue->cable_connected;
> +	if (power_up == glue->powered) {
> +		dev_warn(musb->controller, "power state already %i\n",
> +			 power_up);
> +		return;
> +	}
> +
> +	glue->powered = power_up;
> +
> +	if (power_up) {
> +		res = pm_runtime_get_sync(musb->controller);
> +		if (res < 0) {
> +			dev_err(musb->controller, "could not enable: %i", res);
> +			glue->powered = false;
> +		}
> +	} else {
> +		pm_runtime_mark_last_busy(musb->controller);
> +		pm_runtime_put_autosuspend(musb->controller);
> +	}
> +}
> +
>  static void omap2430_musb_mailbox(enum musb_vbus_id_status status)
>  {
>  	struct omap2430_glue	*glue = _glue;
> @@ -259,6 +301,13 @@ static void omap_musb_set_mailbox(struct omap2430_glue *glue)
>  	struct musb_hdrc_platform_data *pdata = dev_get_platdata(dev);
>  	struct omap_musb_board_data *data = pdata->board_data;
>  	struct usb_otg *otg = musb->xceiv->otg;
> +	bool cable_connected;
> +
> +	cable_connected = ((glue->status == MUSB_ID_GROUND) ||
> +			   (glue->status == MUSB_VBUS_VALID));
> +
> +	if (cable_connected)
> +		omap2430_set_power(musb, glue->enabled, cable_connected);
>  
>  	switch (glue->status) {
>  	case MUSB_ID_GROUND:
> @@ -268,7 +317,6 @@ static void omap_musb_set_mailbox(struct omap2430_glue *glue)
>  		musb->xceiv->otg->state = OTG_STATE_A_IDLE;
>  		musb->xceiv->last_event = USB_EVENT_ID;
>  		if (musb->gadget_driver) {
> -			pm_runtime_get_sync(dev);
>  			omap_control_usb_set_mode(glue->control_otghs,
>  				USB_MODE_HOST);
>  			omap2430_musb_set_vbus(musb, 1);
> @@ -281,8 +329,6 @@ static void omap_musb_set_mailbox(struct omap2430_glue *glue)
>  		otg->default_a = false;
>  		musb->xceiv->otg->state = OTG_STATE_B_IDLE;
>  		musb->xceiv->last_event = USB_EVENT_VBUS;
> -		if (musb->gadget_driver)
> -			pm_runtime_get_sync(dev);
>  		omap_control_usb_set_mode(glue->control_otghs, USB_MODE_DEVICE);
>  		break;
>  
> @@ -291,11 +337,8 @@ static void omap_musb_set_mailbox(struct omap2430_glue *glue)
>  		dev_dbg(dev, "VBUS Disconnect\n");
>  
>  		musb->xceiv->last_event = USB_EVENT_NONE;
> -		if (musb->gadget_driver) {
> +		if (musb->gadget_driver)
>  			omap2430_musb_set_vbus(musb, 0);
> -			pm_runtime_mark_last_busy(dev);
> -			pm_runtime_put_autosuspend(dev);
> -		}
>  
>  		if (data->interface_type == MUSB_INTERFACE_UTMI)
>  			otg_set_vbus(musb->xceiv->otg, 0);
> @@ -307,6 +350,9 @@ static void omap_musb_set_mailbox(struct omap2430_glue *glue)
>  		dev_dbg(dev, "ID float\n");
>  	}
>  
> +	if (!cable_connected)
> +		omap2430_set_power(musb, glue->enabled, cable_connected);
> +
>  	atomic_notifier_call_chain(&musb->xceiv->notifier,
>  			musb->xceiv->last_event, NULL);
>  }
> @@ -443,6 +489,8 @@ static void omap2430_musb_enable(struct musb *musb)
>  	struct musb_hdrc_platform_data *pdata = dev_get_platdata(dev);
>  	struct omap_musb_board_data *data = pdata->board_data;
>  
> +	omap2430_set_power(musb, true, glue->cable_connected);
> +
>  	switch (glue->status) {
>  
>  	case MUSB_ID_GROUND:
> @@ -481,6 +529,8 @@ static void omap2430_musb_disable(struct musb *musb)
>  	if (glue->status != MUSB_UNKNOWN)
>  		omap_control_usb_set_mode(glue->control_otghs,
>  			USB_MODE_DISCONNECT);
> +
> +	omap2430_set_power(musb, false, glue->cable_connected);
>  }
>  
>  static int omap2430_musb_exit(struct musb *musb)
> @@ -653,11 +703,13 @@ err0:
>  
>  static int omap2430_remove(struct platform_device *pdev)
>  {
> -	struct omap2430_glue		*glue = platform_get_drvdata(pdev);
> +	struct omap2430_glue *glue = platform_get_drvdata(pdev);
> +	struct musb *musb = glue_to_musb(glue);
>  
>  	pm_runtime_get_sync(glue->dev);
>  	cancel_work_sync(&glue->omap_musb_mailbox_work);
>  	platform_device_unregister(glue->musb);
> +	omap2430_set_power(musb, false, false);
>  	pm_runtime_put_sync(glue->dev);
>  	pm_runtime_disable(glue->dev);
>  
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 07/15] usb: musb: Handle cable status better for 2430 glue layer

[Tony Lindgren]

* Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160513 12:58]:
> On Wed, May 11, 2016 at 06:33:04PM -0700, Tony Lindgren wrote:
> > --- a/drivers/usb/musb/omap2430.c
> > +++ b/drivers/usb/musb/omap2430.c
> > @@ -49,6 +49,9 @@ struct omap2430_glue {
> >  	enum musb_vbus_id_status status;
> >  	struct work_struct	omap_musb_mailbox_work;
> >  	struct device		*control_otghs;
> > +	bool			cable_connected;
> > +	bool			enabled;
> > +	bool			powered;
> 
> This variable is only used within omap2430_set_power(), so it can be
> local within that function to save a few bytes.

Well it needs to be static as we keep things powered only if
glue is enabled and cable is connected.

I'd rather not add any more static stuff that's not specific to the
glue instance.. Got any better ideas?

BTW, we still have also at least _glue remaining in the 2430 glue.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 07/15] usb: musb: Handle cable status better for 2430 glue layer

[Bin Liu]

Hi,

On Fri, May 13, 2016 at 01:09:02PM -0700, Tony Lindgren wrote:
> * Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160513 12:58]:
> > On Wed, May 11, 2016 at 06:33:04PM -0700, Tony Lindgren wrote:
> > > --- a/drivers/usb/musb/omap2430.c
> > > +++ b/drivers/usb/musb/omap2430.c
> > > @@ -49,6 +49,9 @@ struct omap2430_glue {
> > >  	enum musb_vbus_id_status status;
> > >  	struct work_struct	omap_musb_mailbox_work;
> > >  	struct device		*control_otghs;
> > > +	bool			cable_connected;
> > > +	bool			enabled;
> > > +	bool			powered;
> > 
> > This variable is only used within omap2430_set_power(), so it can be
> > local within that function to save a few bytes.
> 
> Well it needs to be static as we keep things powered only if
> glue is enabled and cable is connected.

I think it does not have to static in omap2430_glue, how about in the
very beginning of omap2430_set_power(),

	bool powered = glue->enabled && glue->cable_connected;

before changing glue->enabled and glue->cable_connected, then this
local powered variable is equivalent to glue->powered.

> 
> I'd rather not add any more static stuff that's not specific to the
> glue instance.. Got any better ideas?

I didn't think hard enough for this, but I think your idea is acceptable
;).

Regards,
-Bin.

> 
> BTW, we still have also at least _glue remaining in the 2430 glue.
> 
> Regards,
> 
> Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 07/15] usb: musb: Handle cable status better for 2430 glue layer

[Bin Liu]

On Fri, May 13, 2016 at 03:21:55PM -0500, Bin Liu wrote:
> Hi,
> 
> On Fri, May 13, 2016 at 01:09:02PM -0700, Tony Lindgren wrote:
> > * Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160513 12:58]:
> > > On Wed, May 11, 2016 at 06:33:04PM -0700, Tony Lindgren wrote:
> > > > --- a/drivers/usb/musb/omap2430.c
> > > > +++ b/drivers/usb/musb/omap2430.c
> > > > @@ -49,6 +49,9 @@ struct omap2430_glue {
> > > >  	enum musb_vbus_id_status status;
> > > >  	struct work_struct	omap_musb_mailbox_work;
> > > >  	struct device		*control_otghs;
> > > > +	bool			cable_connected;
> > > > +	bool			enabled;
> > > > +	bool			powered;
> > > 
> > > This variable is only used within omap2430_set_power(), so it can be
> > > local within that function to save a few bytes.
> > 
> > Well it needs to be static as we keep things powered only if
> > glue is enabled and cable is connected.
> 
> I think it does not have to static in omap2430_glue, how about in the

I meant in omap2430_set_power().

> very beginning of omap2430_set_power(),
> 
> 	bool powered = glue->enabled && glue->cable_connected;
> 
> before changing glue->enabled and glue->cable_connected, then this
> local powered variable is equivalent to glue->powered.
> 
> > 
> > I'd rather not add any more static stuff that's not specific to the
> > glue instance.. Got any better ideas?
> 
> I didn't think hard enough for this, but I think your idea is acceptable
> ;).
> 
> Regards,
> -Bin.
> 
> > 
> > BTW, we still have also at least _glue remaining in the 2430 glue.
> > 
> > Regards,
> > 
> > Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 07/15] usb: musb: Handle cable status better for 2430 glue layer

[Tony Lindgren]

* Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160513 13:26]:
> On Fri, May 13, 2016 at 03:21:55PM -0500, Bin Liu wrote:
> > On Fri, May 13, 2016 at 01:09:02PM -0700, Tony Lindgren wrote:
> > > * Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160513 12:58]:
> > > > On Wed, May 11, 2016 at 06:33:04PM -0700, Tony Lindgren wrote:
> > > > > --- a/drivers/usb/musb/omap2430.c
> > > > > +++ b/drivers/usb/musb/omap2430.c
> > > > > @@ -49,6 +49,9 @@ struct omap2430_glue {
> > > > >  	enum musb_vbus_id_status status;
> > > > >  	struct work_struct	omap_musb_mailbox_work;
> > > > >  	struct device		*control_otghs;
> > > > > +	bool			cable_connected;
> > > > > +	bool			enabled;
> > > > > +	bool			powered;
> > > > 
> > > > This variable is only used within omap2430_set_power(), so it can be
> > > > local within that function to save a few bytes.
> > > 
> > > Well it needs to be static as we keep things powered only if
> > > glue is enabled and cable is connected.
> > 
> > I think it does not have to static in omap2430_glue, how about in the
> 
> I meant in omap2430_set_power().
> 
> > very beginning of omap2430_set_power(),
> > 
> > 	bool powered = glue->enabled && glue->cable_connected;
> > 
> > before changing glue->enabled and glue->cable_connected, then this
> > local powered variable is equivalent to glue->powered.

Hmm but pm_runtime_get_sync() can fail too. I was seeing -EACCES
there while chasing various PM runtime issues. That can happen for
example if the parent and child PM runtime use counts get out of sync
like happened in the -EPROBE_DEFER case. Now we at least know if
something goes wrong with the "could not enable" error.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 07/15] usb: musb: Handle cable status better for 2430 glue layer

[Bin Liu]

Hi,

On Fri, May 13, 2016 at 01:33:30PM -0700, Tony Lindgren wrote:
> * Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160513 13:26]:
> > On Fri, May 13, 2016 at 03:21:55PM -0500, Bin Liu wrote:
> > > On Fri, May 13, 2016 at 01:09:02PM -0700, Tony Lindgren wrote:
> > > > * Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160513 12:58]:
> > > > > On Wed, May 11, 2016 at 06:33:04PM -0700, Tony Lindgren wrote:
> > > > > > --- a/drivers/usb/musb/omap2430.c
> > > > > > +++ b/drivers/usb/musb/omap2430.c
> > > > > > @@ -49,6 +49,9 @@ struct omap2430_glue {
> > > > > >  	enum musb_vbus_id_status status;
> > > > > >  	struct work_struct	omap_musb_mailbox_work;
> > > > > >  	struct device		*control_otghs;
> > > > > > +	bool			cable_connected;
> > > > > > +	bool			enabled;
> > > > > > +	bool			powered;
> > > > > 
> > > > > This variable is only used within omap2430_set_power(), so it can be
> > > > > local within that function to save a few bytes.
> > > > 
> > > > Well it needs to be static as we keep things powered only if
> > > > glue is enabled and cable is connected.
> > > 
> > > I think it does not have to static in omap2430_glue, how about in the
> > 
> > I meant in omap2430_set_power().
> > 
> > > very beginning of omap2430_set_power(),
> > > 
> > > 	bool powered = glue->enabled && glue->cable_connected;
> > > 
> > > before changing glue->enabled and glue->cable_connected, then this
> > > local powered variable is equivalent to glue->powered.
> 
> Hmm but pm_runtime_get_sync() can fail too. I was seeing -EACCES
> there while chasing various PM runtime issues. That can happen for
> example if the parent and child PM runtime use counts get out of sync
> like happened in the -EPROBE_DEFER case. Now we at least know if
> something goes wrong with the "could not enable" error.

Well, cannot think of why this variable could cause this problem. But
anyway, please ignore my comments, it is only about 4 bytes.

Regards,
-Bin.

> 
> Regards,
> 
> Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 07/15] usb: musb: Handle cable status better for 2430 glue layer

[Tony Lindgren]

* Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160513 13:43]:
> Hi,
> 
> On Fri, May 13, 2016 at 01:33:30PM -0700, Tony Lindgren wrote:
> > * Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160513 13:26]:
> > > On Fri, May 13, 2016 at 03:21:55PM -0500, Bin Liu wrote:
> > > > On Fri, May 13, 2016 at 01:09:02PM -0700, Tony Lindgren wrote:
> > > > > * Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160513 12:58]:
> > > > > > On Wed, May 11, 2016 at 06:33:04PM -0700, Tony Lindgren wrote:
> > > > > > > --- a/drivers/usb/musb/omap2430.c
> > > > > > > +++ b/drivers/usb/musb/omap2430.c
> > > > > > > @@ -49,6 +49,9 @@ struct omap2430_glue {
> > > > > > >  	enum musb_vbus_id_status status;
> > > > > > >  	struct work_struct	omap_musb_mailbox_work;
> > > > > > >  	struct device		*control_otghs;
> > > > > > > +	bool			cable_connected;
> > > > > > > +	bool			enabled;
> > > > > > > +	bool			powered;
> > > > > > 
> > > > > > This variable is only used within omap2430_set_power(), so it can be
> > > > > > local within that function to save a few bytes.
> > > > > 
> > > > > Well it needs to be static as we keep things powered only if
> > > > > glue is enabled and cable is connected.
> > > > 
> > > > I think it does not have to static in omap2430_glue, how about in the
> > > 
> > > I meant in omap2430_set_power().
> > > 
> > > > very beginning of omap2430_set_power(),
> > > > 
> > > > 	bool powered = glue->enabled && glue->cable_connected;
> > > > 
> > > > before changing glue->enabled and glue->cable_connected, then this
> > > > local powered variable is equivalent to glue->powered.
> > 
> > Hmm but pm_runtime_get_sync() can fail too. I was seeing -EACCES
> > there while chasing various PM runtime issues. That can happen for
> > example if the parent and child PM runtime use counts get out of sync
> > like happened in the -EPROBE_DEFER case. Now we at least know if
> > something goes wrong with the "could not enable" error.
> 
> Well, cannot think of why this variable could cause this problem. But
> anyway, please ignore my comments, it is only about 4 bytes.

It's because we currently need to keep track of powered in addition
to cable_connected and enabled. Otherwise we don't get get warnings
related to PM runtime issues.

And if we use static bool powered in omap2430_set_power(), it won't
be specific to the glue layer driver instance :)

Tony



--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 08/15] usb: musb: Improve PM runtime and phy handling for 2430 glue layer

[Tony Lindgren]

This simplifies things and allows idling both MUSB and PHY
when nothing is configured. Let's just return early from PM
runtime if musb is not yet initialized.

Let's also warn if PHY is not configured.

Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
 drivers/usb/musb/omap2430.c | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 02d40bc..eb0f332 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -435,6 +435,7 @@ static int omap2430_musb_init(struct musb *musb)
 		return PTR_ERR(musb->phy);
 	}
 	musb->isr = omap2430_musb_interrupt;
+	phy_init(musb->phy);
 
 	/*
 	 * Enable runtime PM for musb parent (this driver). We can't
@@ -471,8 +472,6 @@ static int omap2430_musb_init(struct musb *musb)
 	if (glue->status != MUSB_UNKNOWN)
 		omap_musb_set_mailbox(glue);
 
-	phy_init(musb->phy);
-	phy_power_on(musb->phy);
 	pm_runtime_put(glue->dev);
 	return 0;
 
@@ -489,6 +488,9 @@ static void omap2430_musb_enable(struct musb *musb)
 	struct musb_hdrc_platform_data *pdata = dev_get_platdata(dev);
 	struct omap_musb_board_data *data = pdata->board_data;
 
+	if (!WARN_ON(!musb->phy))
+		phy_power_on(musb->phy);
+
 	omap2430_set_power(musb, true, glue->cable_connected);
 
 	switch (glue->status) {
@@ -526,6 +528,9 @@ static void omap2430_musb_disable(struct musb *musb)
 	struct device *dev = musb->controller;
 	struct omap2430_glue *glue = dev_get_drvdata(dev->parent);
 
+	if (!WARN_ON(!musb->phy))
+		phy_power_off(musb->phy);
+
 	if (glue->status != MUSB_UNKNOWN)
 		omap_control_usb_set_mode(glue->control_otghs,
 			USB_MODE_DISCONNECT);
@@ -535,11 +540,14 @@ static void omap2430_musb_disable(struct musb *musb)
 
 static int omap2430_musb_exit(struct musb *musb)
 {
-	del_timer_sync(&musb_idle_timer);
+	struct device *dev = musb->controller;
+	struct omap2430_glue *glue = dev_get_drvdata(dev->parent);
 
+	del_timer_sync(&musb_idle_timer);
 	omap2430_low_level_exit(musb);
-	phy_power_off(musb->phy);
 	phy_exit(musb->phy);
+	musb->phy = NULL;
+	cancel_work_sync(&glue->omap_musb_mailbox_work);
 
 	return 0;
 }
@@ -707,7 +715,6 @@ static int omap2430_remove(struct platform_device *pdev)
 	struct musb *musb = glue_to_musb(glue);
 
 	pm_runtime_get_sync(glue->dev);
-	cancel_work_sync(&glue->omap_musb_mailbox_work);
 	platform_device_unregister(glue->musb);
 	omap2430_set_power(musb, false, false);
 	pm_runtime_put_sync(glue->dev);
@@ -723,12 +730,13 @@ static int omap2430_runtime_suspend(struct device *dev)
 	struct omap2430_glue		*glue = dev_get_drvdata(dev);
 	struct musb			*musb = glue_to_musb(glue);
 
-	if (musb) {
-		musb->context.otg_interfsel = musb_readl(musb->mregs,
-				OTG_INTERFSEL);
+	if (!musb)
+		return 0;
 
-		omap2430_low_level_exit(musb);
-	}
+	musb->context.otg_interfsel = musb_readl(musb->mregs,
+						 OTG_INTERFSEL);
+
+	omap2430_low_level_exit(musb);
 
 	return 0;
 }
@@ -739,7 +747,7 @@ static int omap2430_runtime_resume(struct device *dev)
 	struct musb			*musb = glue_to_musb(glue);
 
 	if (!musb)
-		return -EPROBE_DEFER;
+		return 0;
 
 	omap2430_low_level_init(musb);
 	musb_writel(musb->mregs, OTG_INTERFSEL,
-- 
2.8.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [08/15] usb: musb: Improve PM runtime and phy handling for 2430 glue layer

[Laurent Pinchart]

Hi Tony,

On Wednesday 11 May 2016 17:53:09 Tony Lindgren wrote:
> This simplifies things and allows idling both MUSB and PHY
> when nothing is configured. Let's just return early from PM
> runtime if musb is not yet initialized.
> 
> Let's also warn if PHY is not configured.

git bisect pointed out to this patch today when I tried to find out what broke 
nfsroot over USB ethernet gadget on my Panda board :-/ Could you help me 
debugging and fixing that ?

> Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/usb/musb/omap2430.c | 30 +++++++++++++++++++-----------
>  1 file changed, 19 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
> index 02d40bc..eb0f332 100644
> --- a/drivers/usb/musb/omap2430.c
> +++ b/drivers/usb/musb/omap2430.c
> @@ -435,6 +435,7 @@ static int omap2430_musb_init(struct musb *musb)
>  		return PTR_ERR(musb->phy);
>  	}
>  	musb->isr = omap2430_musb_interrupt;
> +	phy_init(musb->phy);
> 
>  	/*
>  	 * Enable runtime PM for musb parent (this driver). We can't
> @@ -471,8 +472,6 @@ static int omap2430_musb_init(struct musb *musb)
>  	if (glue->status != MUSB_UNKNOWN)
>  		omap_musb_set_mailbox(glue);
> 
> -	phy_init(musb->phy);
> -	phy_power_on(musb->phy);
>  	pm_runtime_put(glue->dev);
>  	return 0;
> 
> @@ -489,6 +488,9 @@ static void omap2430_musb_enable(struct musb *musb)
>  	struct musb_hdrc_platform_data *pdata = dev_get_platdata(dev);
>  	struct omap_musb_board_data *data = pdata->board_data;
> 
> +	if (!WARN_ON(!musb->phy))
> +		phy_power_on(musb->phy);
> +
>  	omap2430_set_power(musb, true, glue->cable_connected);
> 
>  	switch (glue->status) {
> @@ -526,6 +528,9 @@ static void omap2430_musb_disable(struct musb *musb)
>  	struct device *dev = musb->controller;
>  	struct omap2430_glue *glue = dev_get_drvdata(dev->parent);
> 
> +	if (!WARN_ON(!musb->phy))
> +		phy_power_off(musb->phy);
> +
>  	if (glue->status != MUSB_UNKNOWN)
>  		omap_control_usb_set_mode(glue->control_otghs,
>  			USB_MODE_DISCONNECT);
> @@ -535,11 +540,14 @@ static void omap2430_musb_disable(struct musb *musb)
> 
>  static int omap2430_musb_exit(struct musb *musb)
>  {
> -	del_timer_sync(&musb_idle_timer);
> +	struct device *dev = musb->controller;
> +	struct omap2430_glue *glue = dev_get_drvdata(dev->parent);
> 
> +	del_timer_sync(&musb_idle_timer);
>  	omap2430_low_level_exit(musb);
> -	phy_power_off(musb->phy);
>  	phy_exit(musb->phy);
> +	musb->phy = NULL;
> +	cancel_work_sync(&glue->omap_musb_mailbox_work);
> 
>  	return 0;
>  }
> @@ -707,7 +715,6 @@ static int omap2430_remove(struct platform_device *pdev)
> struct musb *musb = glue_to_musb(glue);
> 
>  	pm_runtime_get_sync(glue->dev);
> -	cancel_work_sync(&glue->omap_musb_mailbox_work);
>  	platform_device_unregister(glue->musb);
>  	omap2430_set_power(musb, false, false);
>  	pm_runtime_put_sync(glue->dev);
> @@ -723,12 +730,13 @@ static int omap2430_runtime_suspend(struct device
> *dev) struct omap2430_glue		*glue = dev_get_drvdata(dev);
>  	struct musb			*musb = glue_to_musb(glue);
> 
> -	if (musb) {
> -		musb->context.otg_interfsel = musb_readl(musb->mregs,
> -				OTG_INTERFSEL);
> +	if (!musb)
> +		return 0;
> 
> -		omap2430_low_level_exit(musb);
> -	}
> +	musb->context.otg_interfsel = musb_readl(musb->mregs,
> +						 OTG_INTERFSEL);
> +
> +	omap2430_low_level_exit(musb);
> 
>  	return 0;
>  }
> @@ -739,7 +747,7 @@ static int omap2430_runtime_resume(struct device *dev)
>  	struct musb			*musb = glue_to_musb(glue);
> 
>  	if (!musb)
> -		return -EPROBE_DEFER;
> +		return 0;
> 
>  	omap2430_low_level_init(musb);
>  	musb_writel(musb->mregs, OTG_INTERFSEL,

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [08/15] usb: musb: Improve PM runtime and phy handling for 2430 glue layer

[Tony Lindgren]

* Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> [160909 10:08]:
> Hi Tony,
> 
> On Wednesday 11 May 2016 17:53:09 Tony Lindgren wrote:
> > This simplifies things and allows idling both MUSB and PHY
> > when nothing is configured. Let's just return early from PM
> > runtime if musb is not yet initialized.
> > 
> > Let's also warn if PHY is not configured.
> 
> git bisect pointed out to this patch today when I tried to find out what broke 
> nfsroot over USB ethernet gadget on my Panda board :-/ Could you help me 
> debugging and fixing that ?

OK, do you have commit 2c5575401e34 ("usb: musb: Fix locking errors for
host only mode")? That's now in v4.8-rc5. Or which kernel are we talking
about here?

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [08/15] usb: musb: Improve PM runtime and phy handling for 2430 glue layer

[Laurent Pinchart]

Hi Tony,

On Friday 09 Sep 2016 10:24:47 Tony Lindgren wrote:
> * Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> [160909 10:08]:
> > On Wednesday 11 May 2016 17:53:09 Tony Lindgren wrote:
> > > This simplifies things and allows idling both MUSB and PHY
> > > when nothing is configured. Let's just return early from PM
> > > runtime if musb is not yet initialized.
> > > 
> > > Let's also warn if PHY is not configured.
> > 
> > git bisect pointed out to this patch today when I tried to find out what
> > broke nfsroot over USB ethernet gadget on my Panda board :-/ Could you
> > help me debugging and fixing that ?
> 
> OK, do you have commit 2c5575401e34 ("usb: musb: Fix locking errors for
> host only mode")? That's now in v4.8-rc5. Or which kernel are we talking
> about here?

I've originally ran into the issue on v4.8-rc2. I've then bisected the issue 
to this commit, and confirmed that both v4.7 and v4.8-rc5 are broken.

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [08/15] usb: musb: Improve PM runtime and phy handling for 2430 glue layer

[Laurent Pinchart]

Hi Tony,

On Friday 09 Sep 2016 21:33:35 Laurent Pinchart wrote:
> On Friday 09 Sep 2016 10:24:47 Tony Lindgren wrote:
> > * Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> [160909 10:08]:
> > > On Wednesday 11 May 2016 17:53:09 Tony Lindgren wrote:
> > > > This simplifies things and allows idling both MUSB and PHY
> > > > when nothing is configured. Let's just return early from PM
> > > > runtime if musb is not yet initialized.
> > > > 
> > > > Let's also warn if PHY is not configured.
> > > 
> > > git bisect pointed out to this patch today when I tried to find out what
> > > broke nfsroot over USB ethernet gadget on my Panda board :-/ Could you
> > > help me debugging and fixing that ?
> > 
> > OK, do you have commit 2c5575401e34 ("usb: musb: Fix locking errors for
> > host only mode")? That's now in v4.8-rc5. Or which kernel are we talking
> > about here?
> 
> I've originally ran into the issue on v4.8-rc2. I've then bisected the issue
> to this commit, and confirmed that both v4.7 and v4.8-rc5 are broken.

"[PATCH v2] musb: omap2430: do not assume balanced enable()/disable()login 
register mail settings" (https://patchwork.kernel.org/patch/9261611/) fixes 
the issue.

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [08/15] usb: musb: Improve PM runtime and phy handling for 2430 glue layer

[Tony Lindgren]

* Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> [160909 12:24]:
> Hi Tony,
> 
> On Friday 09 Sep 2016 21:33:35 Laurent Pinchart wrote:
> > On Friday 09 Sep 2016 10:24:47 Tony Lindgren wrote:
> > > * Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> [160909 10:08]:
> > > > On Wednesday 11 May 2016 17:53:09 Tony Lindgren wrote:
> > > > > This simplifies things and allows idling both MUSB and PHY
> > > > > when nothing is configured. Let's just return early from PM
> > > > > runtime if musb is not yet initialized.
> > > > > 
> > > > > Let's also warn if PHY is not configured.
> > > > 
> > > > git bisect pointed out to this patch today when I tried to find out what
> > > > broke nfsroot over USB ethernet gadget on my Panda board :-/ Could you
> > > > help me debugging and fixing that ?
> > > 
> > > OK, do you have commit 2c5575401e34 ("usb: musb: Fix locking errors for
> > > host only mode")? That's now in v4.8-rc5. Or which kernel are we talking
> > > about here?
> > 
> > I've originally ran into the issue on v4.8-rc2. I've then bisected the issue
> > to this commit, and confirmed that both v4.7 and v4.8-rc5 are broken.
> 
> "[PATCH v2] musb: omap2430: do not assume balanced enable()/disable()login 
> register mail settings" (https://patchwork.kernel.org/patch/9261611/) fixes 
> the issue.

Oh it's a different issue then. Sounds like it's probably related to
drivers/phy vs drivers/usb/phy. Will take a look on my panda.

Tony



Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [08/15] usb: musb: Improve PM runtime and phy handling for 2430 glue layer

[Tony Lindgren]

* Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> [160909 11:33]:
> Hi Tony,
> 
> On Friday 09 Sep 2016 10:24:47 Tony Lindgren wrote:
> > * Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> [160909 10:08]:
> > > On Wednesday 11 May 2016 17:53:09 Tony Lindgren wrote:
> > > > This simplifies things and allows idling both MUSB and PHY
> > > > when nothing is configured. Let's just return early from PM
> > > > runtime if musb is not yet initialized.
> > > > 
> > > > Let's also warn if PHY is not configured.
> > > 
> > > git bisect pointed out to this patch today when I tried to find out what
> > > broke nfsroot over USB ethernet gadget on my Panda board :-/ Could you
> > > help me debugging and fixing that ?
> > 
> > OK, do you have commit 2c5575401e34 ("usb: musb: Fix locking errors for
> > host only mode")? That's now in v4.8-rc5. Or which kernel are we talking
> > about here?
> 
> I've originally ran into the issue on v4.8-rc2. I've then bisected the issue 
> to this commit, and confirmed that both v4.7 and v4.8-rc5 are broken.

OK yeah sounds like this is some legacy phy related breakage. Will
take a look.

Thanks,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 09/15] usb: musb: Remove try_idle for 2430 glue layer

[Tony Lindgren]

This is no longer needed with PM runtime at least for 2430 glue.
We can now rely only on PM runtime and cable detection.

The other glue layers can probably remove try_idle too, but that
needs to be tested for each platform before doing it.

Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
 drivers/usb/musb/omap2430.c | 91 ---------------------------------------------
 1 file changed, 91 deletions(-)

diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index eb0f332..79e4cd7 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -57,92 +57,6 @@ struct omap2430_glue {
 
 static struct omap2430_glue	*_glue;
 
-static struct timer_list musb_idle_timer;
-
-static void musb_do_idle(unsigned long _musb)
-{
-	struct musb	*musb = (void *)_musb;
-	unsigned long	flags;
-	u8	power;
-	u8	devctl;
-
-	spin_lock_irqsave(&musb->lock, flags);
-
-	switch (musb->xceiv->otg->state) {
-	case OTG_STATE_A_WAIT_BCON:
-
-		devctl = musb_readb(musb->mregs, MUSB_DEVCTL);
-		if (devctl & MUSB_DEVCTL_BDEVICE) {
-			musb->xceiv->otg->state = OTG_STATE_B_IDLE;
-			MUSB_DEV_MODE(musb);
-		} else {
-			musb->xceiv->otg->state = OTG_STATE_A_IDLE;
-			MUSB_HST_MODE(musb);
-		}
-		break;
-	case OTG_STATE_A_SUSPEND:
-		/* finish RESUME signaling? */
-		if (musb->port1_status & MUSB_PORT_STAT_RESUME) {
-			power = musb_readb(musb->mregs, MUSB_POWER);
-			power &= ~MUSB_POWER_RESUME;
-			dev_dbg(musb->controller, "root port resume stopped, power %02x\n", power);
-			musb_writeb(musb->mregs, MUSB_POWER, power);
-			musb->is_active = 1;
-			musb->port1_status &= ~(USB_PORT_STAT_SUSPEND
-						| MUSB_PORT_STAT_RESUME);
-			musb->port1_status |= USB_PORT_STAT_C_SUSPEND << 16;
-			usb_hcd_poll_rh_status(musb->hcd);
-			/* NOTE: it might really be A_WAIT_BCON ... */
-			musb->xceiv->otg->state = OTG_STATE_A_HOST;
-		}
-		break;
-	case OTG_STATE_A_HOST:
-		devctl = musb_readb(musb->mregs, MUSB_DEVCTL);
-		if (devctl &  MUSB_DEVCTL_BDEVICE)
-			musb->xceiv->otg->state = OTG_STATE_B_IDLE;
-		else
-			musb->xceiv->otg->state = OTG_STATE_A_WAIT_BCON;
-	default:
-		break;
-	}
-	spin_unlock_irqrestore(&musb->lock, flags);
-}
-
-
-static void omap2430_musb_try_idle(struct musb *musb, unsigned long timeout)
-{
-	unsigned long		default_timeout = jiffies + msecs_to_jiffies(3);
-	static unsigned long	last_timer;
-
-	if (timeout == 0)
-		timeout = default_timeout;
-
-	/* Never idle if active, or when VBUS timeout is not set as host */
-	if (musb->is_active || ((musb->a_wait_bcon == 0)
-			&& (musb->xceiv->otg->state == OTG_STATE_A_WAIT_BCON))) {
-		dev_dbg(musb->controller, "%s active, deleting timer\n",
-			usb_otg_state_string(musb->xceiv->otg->state));
-		del_timer(&musb_idle_timer);
-		last_timer = jiffies;
-		return;
-	}
-
-	if (time_after(last_timer, timeout)) {
-		if (!timer_pending(&musb_idle_timer))
-			last_timer = timeout;
-		else {
-			dev_dbg(musb->controller, "Longer idle timer already pending, ignoring\n");
-			return;
-		}
-	}
-	last_timer = timeout;
-
-	dev_dbg(musb->controller, "%s inactive, for idle timer for %lu ms\n",
-		usb_otg_state_string(musb->xceiv->otg->state),
-		(unsigned long)jiffies_to_msecs(timeout - jiffies));
-	mod_timer(&musb_idle_timer, timeout);
-}
-
 static void omap2430_musb_set_vbus(struct musb *musb, int is_on)
 {
 	struct usb_otg	*otg = musb->xceiv->otg;
@@ -467,8 +381,6 @@ static int omap2430_musb_init(struct musb *musb)
 			musb_readl(musb->mregs, OTG_INTERFSEL),
 			musb_readl(musb->mregs, OTG_SIMENABLE));
 
-	setup_timer(&musb_idle_timer, musb_do_idle, (unsigned long) musb);
-
 	if (glue->status != MUSB_UNKNOWN)
 		omap_musb_set_mailbox(glue);
 
@@ -543,7 +455,6 @@ static int omap2430_musb_exit(struct musb *musb)
 	struct device *dev = musb->controller;
 	struct omap2430_glue *glue = dev_get_drvdata(dev->parent);
 
-	del_timer_sync(&musb_idle_timer);
 	omap2430_low_level_exit(musb);
 	phy_exit(musb->phy);
 	musb->phy = NULL;
@@ -562,8 +473,6 @@ static const struct musb_platform_ops omap2430_ops = {
 	.exit		= omap2430_musb_exit,
 
 	.set_mode	= omap2430_musb_set_mode,
-	.try_idle	= omap2430_musb_try_idle,

[PATCH 10/15] usb: musb: Don't set d+ high before enable for 2430 glue layer

[Tony Lindgren]

At least 2430 glue layer pulls d+ high on start up even if there are
no gadgets configured. This is bad at least for anything using a separate
battery charger chip as it can confuse the charger detection.

Let's fix the issue by getting rid of omap2430_musb_set_mode() and only
write session bit in omap2430_musb_enable().

Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
 drivers/usb/musb/omap2430.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 79e4cd7..958ae6a 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -122,16 +122,6 @@ static void omap2430_musb_set_vbus(struct musb *musb, int is_on)
 		musb_readb(musb->mregs, MUSB_DEVCTL));
 }
 
-static int omap2430_musb_set_mode(struct musb *musb, u8 musb_mode)
-{
-	u8	devctl = musb_readb(musb->mregs, MUSB_DEVCTL);
-
-	devctl |= MUSB_DEVCTL_SESSION;
-	musb_writeb(musb->mregs, MUSB_DEVCTL, devctl);
-
-	return 0;
-}

Re: [PATCH 10/15] usb: musb: Don't set d+ high before enable for 2430 glue layer

[Bin Liu]

Hi,

On Wed, May 11, 2016 at 05:53:11PM -0700, Tony Lindgren wrote:
> At least 2430 glue layer pulls d+ high on start up even if there are
> no gadgets configured. This is bad at least for anything using a separate
> battery charger chip as it can confuse the charger detection.
> 
> Let's fix the issue by getting rid of omap2430_musb_set_mode() and only

By doing so, you lost the feature of switching mode from sysfs, I am not
sure if there is anyone using it though, still, it is a regression.

> write session bit in omap2430_musb_enable().

I don't think session bit needs to be set in _enable(), musb_start() in
core takes care of this bit.

> 
> Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/usb/musb/omap2430.c | 14 +++-----------
>  1 file changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
> index 79e4cd7..958ae6a 100644
> --- a/drivers/usb/musb/omap2430.c
> +++ b/drivers/usb/musb/omap2430.c
> @@ -122,16 +122,6 @@ static void omap2430_musb_set_vbus(struct musb *musb, int is_on)
>  		musb_readb(musb->mregs, MUSB_DEVCTL));
>  }
>  
> -static int omap2430_musb_set_mode(struct musb *musb, u8 musb_mode)
> -{
> -	u8	devctl = musb_readb(musb->mregs, MUSB_DEVCTL);
> -
> -	devctl |= MUSB_DEVCTL_SESSION;
> -	musb_writeb(musb->mregs, MUSB_DEVCTL, devctl);

In stead of removing it, session bit should only be set when musb_mode
== MUSB_HOST, will this fix the D+ pullup problem?

> -
> -	return 0;
> -}
> -
>  static inline void omap2430_low_level_exit(struct musb *musb)
>  {
>  	u32 l;
> @@ -428,6 +418,9 @@ static void omap2430_musb_enable(struct musb *musb)
>  
>  	case MUSB_VBUS_VALID:
>  		omap_control_usb_set_mode(glue->control_otghs, USB_MODE_DEVICE);
> +		devctl = musb_readb(musb->mregs, MUSB_DEVCTL);
> +		devctl |= MUSB_DEVCTL_SESSION;
> +		musb_writeb(musb->mregs, MUSB_DEVCTL, devctl);

session bit is only set for host mode, VBUS_VALID is not a signal for
host mode. So you don't have to set the bit here.

>  		break;
>  
>  	default:
> @@ -472,7 +465,6 @@ static const struct musb_platform_ops omap2430_ops = {
>  	.init		= omap2430_musb_init,
>  	.exit		= omap2430_musb_exit,
>  
> -	.set_mode	= omap2430_musb_set_mode,
>  	.set_vbus	= omap2430_musb_set_vbus,
>  
>  	.enable		= omap2430_musb_enable,
> -- 
> 2.8.1
> 

Regards,
-Bin.

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/15] usb: musb: Don't set d+ high before enable for 2430 glue layer

[Tony Lindgren]

* Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160513 14:05]:
> Hi,
> 
> On Wed, May 11, 2016 at 05:53:11PM -0700, Tony Lindgren wrote:
> > At least 2430 glue layer pulls d+ high on start up even if there are
> > no gadgets configured. This is bad at least for anything using a separate
> > battery charger chip as it can confuse the charger detection.
> > 
> > Let's fix the issue by getting rid of omap2430_musb_set_mode() and only
> 
> By doing so, you lost the feature of switching mode from sysfs, I am not
> sure if there is anyone using it though, still, it is a regression.

Oh right, that's a good point.

How about we change musb_core to call the optional set_mode() if implemented,
and then set the session bit in host mode only? That way we can get rid of
the musb core tinkering in the glue layer drivers eventually?

> > write session bit in omap2430_musb_enable().
> 
> I don't think session bit needs to be set in _enable(), musb_start() in
> core takes care of this bit.

OK sounds like that should just work then.

> > -static int omap2430_musb_set_mode(struct musb *musb, u8 musb_mode)
> > -{
> > -	u8	devctl = musb_readb(musb->mregs, MUSB_DEVCTL);
> > -
> > -	devctl |= MUSB_DEVCTL_SESSION;
> > -	musb_writeb(musb->mregs, MUSB_DEVCTL, devctl);
> 
> In stead of removing it, session bit should only be set when musb_mode
> == MUSB_HOST, will this fix the D+ pullup problem?

Good point, I forgot about it being specific to host mode. I'll check.

> >  static inline void omap2430_low_level_exit(struct musb *musb)
> >  {
> >  	u32 l;
> > @@ -428,6 +418,9 @@ static void omap2430_musb_enable(struct musb *musb)
> >  
> >  	case MUSB_VBUS_VALID:
> >  		omap_control_usb_set_mode(glue->control_otghs, USB_MODE_DEVICE);
> > +		devctl = musb_readb(musb->mregs, MUSB_DEVCTL);
> > +		devctl |= MUSB_DEVCTL_SESSION;
> > +		musb_writeb(musb->mregs, MUSB_DEVCTL, devctl);
> 
> session bit is only set for host mode, VBUS_VALID is not a signal for
> host mode. So you don't have to set the bit here.

OK thanks.

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/15] usb: musb: Don't set d+ high before enable for 2430 glue layer

[Bin Liu]

Hi,

On Fri, May 13, 2016 at 02:17:39PM -0700, Tony Lindgren wrote:
> * Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160513 14:05]:
> > Hi,
> > 
> > On Wed, May 11, 2016 at 05:53:11PM -0700, Tony Lindgren wrote:
> > > At least 2430 glue layer pulls d+ high on start up even if there are
> > > no gadgets configured. This is bad at least for anything using a separate
> > > battery charger chip as it can confuse the charger detection.
> > > 
> > > Let's fix the issue by getting rid of omap2430_musb_set_mode() and only
> > 
> > By doing so, you lost the feature of switching mode from sysfs, I am not
> > sure if there is anyone using it though, still, it is a regression.
> 
> Oh right, that's a good point.
> 
> How about we change musb_core to call the optional set_mode() if implemented,

The core already does so. Please check musb_core.h.

Regards,
-Bin.

> and then set the session bit in host mode only? That way we can get rid of
> the musb core tinkering in the glue layer drivers eventually?
> 
> > > write session bit in omap2430_musb_enable().
> > 
> > I don't think session bit needs to be set in _enable(), musb_start() in
> > core takes care of this bit.
> 
> OK sounds like that should just work then.
> 
> > > -static int omap2430_musb_set_mode(struct musb *musb, u8 musb_mode)
> > > -{
> > > -	u8	devctl = musb_readb(musb->mregs, MUSB_DEVCTL);
> > > -
> > > -	devctl |= MUSB_DEVCTL_SESSION;
> > > -	musb_writeb(musb->mregs, MUSB_DEVCTL, devctl);
> > 
> > In stead of removing it, session bit should only be set when musb_mode
> > == MUSB_HOST, will this fix the D+ pullup problem?
> 
> Good point, I forgot about it being specific to host mode. I'll check.
> 
> > >  static inline void omap2430_low_level_exit(struct musb *musb)
> > >  {
> > >  	u32 l;
> > > @@ -428,6 +418,9 @@ static void omap2430_musb_enable(struct musb *musb)
> > >  
> > >  	case MUSB_VBUS_VALID:
> > >  		omap_control_usb_set_mode(glue->control_otghs, USB_MODE_DEVICE);
> > > +		devctl = musb_readb(musb->mregs, MUSB_DEVCTL);
> > > +		devctl |= MUSB_DEVCTL_SESSION;
> > > +		musb_writeb(musb->mregs, MUSB_DEVCTL, devctl);
> > 
> > session bit is only set for host mode, VBUS_VALID is not a signal for
> > host mode. So you don't have to set the bit here.
> 
> OK thanks.
> 
> Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/15] usb: musb: Don't set d+ high before enable for 2430 glue layer

[Tony Lindgren]

* Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160513 14:24]:
> Hi,
> 
> On Fri, May 13, 2016 at 02:17:39PM -0700, Tony Lindgren wrote:
> > * Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160513 14:05]:
> > > Hi,
> > > 
> > > On Wed, May 11, 2016 at 05:53:11PM -0700, Tony Lindgren wrote:
> > > > At least 2430 glue layer pulls d+ high on start up even if there are
> > > > no gadgets configured. This is bad at least for anything using a separate
> > > > battery charger chip as it can confuse the charger detection.
> > > > 
> > > > Let's fix the issue by getting rid of omap2430_musb_set_mode() and only
> > > 
> > > By doing so, you lost the feature of switching mode from sysfs, I am not
> > > sure if there is anyone using it though, still, it is a regression.
> > 
> > Oh right, that's a good point.
> > 
> > How about we change musb_core to call the optional set_mode() if implemented,
> 
> The core already does so. Please check musb_core.h.

Oh do you have some pending patches for this already not yet
in Linux next?

> > and then set the session bit in host mode only? That way we can get rid of
> > the musb core tinkering in the glue layer drivers eventually?

So currently we have this in musb_core.h:

static inline int musb_platform_set_mode(struct musb *musb, u8 mode)
{
	if (!musb->ops->set_mode)
		return 0;

	return musb->ops->set_mode(musb, mode);
}

What I meant is we could add generic support for the session bit:

static inline int musb_platform_set_mode(struct musb *musb, u8 mode)
{
	if (!musb->ops->set_mode)
		return musb_default_set_mode(musb, mode);

	return musb->ops->set_mode(musb, mode);
}

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/15] usb: musb: Don't set d+ high before enable for 2430 glue layer

[Bin Liu]

Hi,

On Fri, May 13, 2016 at 02:39:01PM -0700, Tony Lindgren wrote:
> * Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160513 14:24]:
> > Hi,
> > 
> > On Fri, May 13, 2016 at 02:17:39PM -0700, Tony Lindgren wrote:
> > > * Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160513 14:05]:
> > > > Hi,
> > > > 
> > > > On Wed, May 11, 2016 at 05:53:11PM -0700, Tony Lindgren wrote:
> > > > > At least 2430 glue layer pulls d+ high on start up even if there are
> > > > > no gadgets configured. This is bad at least for anything using a separate
> > > > > battery charger chip as it can confuse the charger detection.
> > > > > 
> > > > > Let's fix the issue by getting rid of omap2430_musb_set_mode() and only
> > > > 
> > > > By doing so, you lost the feature of switching mode from sysfs, I am not
> > > > sure if there is anyone using it though, still, it is a regression.
> > > 
> > > Oh right, that's a good point.
> > > 
> > > How about we change musb_core to call the optional set_mode() if implemented,
> > 
> > The core already does so. Please check musb_core.h.
> 
> Oh do you have some pending patches for this already not yet
> in Linux next?

No.

> 
> > > and then set the session bit in host mode only? That way we can get rid of
> > > the musb core tinkering in the glue layer drivers eventually?
> 
> So currently we have this in musb_core.h:
> 
> static inline int musb_platform_set_mode(struct musb *musb, u8 mode)
> {
> 	if (!musb->ops->set_mode)
> 		return 0;
> 
> 	return musb->ops->set_mode(musb, mode);
> }
> 
> What I meant is we could add generic support for the session bit:
> 
> static inline int musb_platform_set_mode(struct musb *musb, u8 mode)
> {
> 	if (!musb->ops->set_mode)
> 		return musb_default_set_mode(musb, mode);
> 
> 	return musb->ops->set_mode(musb, mode);
> }

But what would be in musb_default_set_mode()? Currently only am35x,
da8xx, dsps, and omap2430 glues implement _set_mode(), but they don't
have any in common. Only omap2430 sets session bit in _set_mode(), no
one else does so.

Regards,
-Bin.

> 
> Regards,
> 
> Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/15] usb: musb: Don't set d+ high before enable for 2430 glue layer

[Tony Lindgren]

* Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160513 15:04]:
> 
> But what would be in musb_default_set_mode()? Currently only am35x,
> da8xx, dsps, and omap2430 glues implement _set_mode(), but they don't
> have any in common. Only omap2430 sets session bit in _set_mode(), no
> one else does so.

Well how about the following if no glue specific configuration of the
ID pin is possible?

Tony

8< -----------------------
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -416,6 +416,26 @@ void musb_write_fifo(struct musb_hw_ep *hw_ep, u16 len, const u8 *src)
 	return hw_ep->musb->io.write_fifo(hw_ep, len, src);
 }
 
+static int musb_try_set_mode(struct musb *musb, u8 mode)
+{
+	if (musb->ops->set_mode)
+		return musb->ops->set_mode(musb, mode);
+
+	if (mode == MUSB_HOST) {
+		u8 devctl;
+
+		devctl = musb_readb(musb->mregs, MUSB_DEVCTL);
+		devctl |= MUSB_DEVCTL_SESSION;
+		musb_writeb(musb->mregs, MUSB_DEVCTL, devctl);
+	} else {
+		dev_warn(musb->controller,
+			 "platform specific set_mode not implemnted: %i\n",
+			 mode);
+	}
+
+	return 0;
+}
+
 /*-------------------------------------------------------------------------*/
 
 /* for high speed test mode; see USB 2.0 spec 7.1.20 */
@@ -1723,11 +1743,11 @@ musb_mode_store(struct device *dev, struct device_attribute *attr,
 
 	spin_lock_irqsave(&musb->lock, flags);
 	if (sysfs_streq(buf, "host"))
-		status = musb_platform_set_mode(musb, MUSB_HOST);
+		status = musb_try_set_mode(musb, MUSB_HOST);
 	else if (sysfs_streq(buf, "peripheral"))
-		status = musb_platform_set_mode(musb, MUSB_PERIPHERAL);
+		status = musb_try_set_mode(musb, MUSB_PERIPHERAL);
 	else if (sysfs_streq(buf, "otg"))
-		status = musb_platform_set_mode(musb, MUSB_OTG);
+		status = musb_try_set_mode(musb, MUSB_OTG);
 	else
 		status = -EINVAL;
 	spin_unlock_irqrestore(&musb->lock, flags);
@@ -2185,13 +2205,13 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
 		status = musb_host_setup(musb, plat->power);
 		if (status < 0)
 			goto fail3;
-		status = musb_platform_set_mode(musb, MUSB_HOST);
+		status = musb_try_set_mode(musb, MUSB_HOST);
 		break;
 	case MUSB_PORT_MODE_GADGET:
 		status = musb_gadget_setup(musb);
 		if (status < 0)
 			goto fail3;
-		status = musb_platform_set_mode(musb, MUSB_PERIPHERAL);
+		status = musb_try_set_mode(musb, MUSB_PERIPHERAL);
 		break;
 	case MUSB_PORT_MODE_DUAL_ROLE:
 		status = musb_host_setup(musb, plat->power);
@@ -2202,7 +2222,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
 			musb_host_cleanup(musb);
 			goto fail3;
 		}
-		status = musb_platform_set_mode(musb, MUSB_OTG);
+		status = musb_try_set_mode(musb, MUSB_OTG);
 		break;
 	default:
 		dev_err(dev, "unsupported port mode %d\n", musb->port_mode);
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -556,14 +556,6 @@ static inline void musb_platform_disable(struct musb *musb)
 		musb->ops->disable(musb);
 }
 
-static inline int musb_platform_set_mode(struct musb *musb, u8 mode)
-{
-	if (!musb->ops->set_mode)
-		return 0;
-
-	return musb->ops->set_mode(musb, mode);
-}

Re: [PATCH 10/15] usb: musb: Don't set d+ high before enable for 2430 glue layer

[Bin Liu]

Hi,

On Fri, May 13, 2016 at 03:25:17PM -0700, Tony Lindgren wrote:
> * Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160513 15:04]:
> > 
> > But what would be in musb_default_set_mode()? Currently only am35x,
> > da8xx, dsps, and omap2430 glues implement _set_mode(), but they don't
> > have any in common. Only omap2430 sets session bit in _set_mode(), no
> > one else does so.
> 
> Well how about the following if no glue specific configuration of the
> ID pin is possible?

ID pin configuration is required, either in sw or hw, for the controller
otg state transition.

In my understanding, if ID pin is grounded, setting session bit make the
controller transition to host mode; or if ID pin is float, setting
session bit triggers srp request.

I think I don't understand what you are trying to solve by adding
default set_mode() in core.

Regards,
-Bin.

> 
> Tony
> 
> 8< -----------------------
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -416,6 +416,26 @@ void musb_write_fifo(struct musb_hw_ep *hw_ep, u16 len, const u8 *src)
>  	return hw_ep->musb->io.write_fifo(hw_ep, len, src);
>  }
>  
> +static int musb_try_set_mode(struct musb *musb, u8 mode)
> +{
> +	if (musb->ops->set_mode)
> +		return musb->ops->set_mode(musb, mode);
> +
> +	if (mode == MUSB_HOST) {
> +		u8 devctl;
> +
> +		devctl = musb_readb(musb->mregs, MUSB_DEVCTL);
> +		devctl |= MUSB_DEVCTL_SESSION;
> +		musb_writeb(musb->mregs, MUSB_DEVCTL, devctl);
> +	} else {
> +		dev_warn(musb->controller,
> +			 "platform specific set_mode not implemnted: %i\n",
> +			 mode);
> +	}
> +
> +	return 0;
> +}
> +
>  /*-------------------------------------------------------------------------*/
>  
>  /* for high speed test mode; see USB 2.0 spec 7.1.20 */
> @@ -1723,11 +1743,11 @@ musb_mode_store(struct device *dev, struct device_attribute *attr,
>  
>  	spin_lock_irqsave(&musb->lock, flags);
>  	if (sysfs_streq(buf, "host"))
> -		status = musb_platform_set_mode(musb, MUSB_HOST);
> +		status = musb_try_set_mode(musb, MUSB_HOST);
>  	else if (sysfs_streq(buf, "peripheral"))
> -		status = musb_platform_set_mode(musb, MUSB_PERIPHERAL);
> +		status = musb_try_set_mode(musb, MUSB_PERIPHERAL);
>  	else if (sysfs_streq(buf, "otg"))
> -		status = musb_platform_set_mode(musb, MUSB_OTG);
> +		status = musb_try_set_mode(musb, MUSB_OTG);
>  	else
>  		status = -EINVAL;
>  	spin_unlock_irqrestore(&musb->lock, flags);
> @@ -2185,13 +2205,13 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
>  		status = musb_host_setup(musb, plat->power);
>  		if (status < 0)
>  			goto fail3;
> -		status = musb_platform_set_mode(musb, MUSB_HOST);
> +		status = musb_try_set_mode(musb, MUSB_HOST);
>  		break;
>  	case MUSB_PORT_MODE_GADGET:
>  		status = musb_gadget_setup(musb);
>  		if (status < 0)
>  			goto fail3;
> -		status = musb_platform_set_mode(musb, MUSB_PERIPHERAL);
> +		status = musb_try_set_mode(musb, MUSB_PERIPHERAL);
>  		break;
>  	case MUSB_PORT_MODE_DUAL_ROLE:
>  		status = musb_host_setup(musb, plat->power);
> @@ -2202,7 +2222,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
>  			musb_host_cleanup(musb);
>  			goto fail3;
>  		}
> -		status = musb_platform_set_mode(musb, MUSB_OTG);
> +		status = musb_try_set_mode(musb, MUSB_OTG);
>  		break;
>  	default:
>  		dev_err(dev, "unsupported port mode %d\n", musb->port_mode);
> --- a/drivers/usb/musb/musb_core.h
> +++ b/drivers/usb/musb/musb_core.h
> @@ -556,14 +556,6 @@ static inline void musb_platform_disable(struct musb *musb)
>  		musb->ops->disable(musb);
>  }
>  
> -static inline int musb_platform_set_mode(struct musb *musb, u8 mode)
> -{
> -	if (!musb->ops->set_mode)
> -		return 0;
> -
> -	return musb->ops->set_mode(musb, mode);
> -}
> -
>  static inline void musb_platform_try_idle(struct musb *musb,
>  		unsigned long timeout)
>  {
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/15] usb: musb: Don't set d+ high before enable for 2430 glue layer

[Tony Lindgren]

* Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160514 21:44]:
> Hi,
> 
> On Fri, May 13, 2016 at 03:25:17PM -0700, Tony Lindgren wrote:
> > * Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160513 15:04]:
> > > 
> > > But what would be in musb_default_set_mode()? Currently only am35x,
> > > da8xx, dsps, and omap2430 glues implement _set_mode(), but they don't
> > > have any in common. Only omap2430 sets session bit in _set_mode(), no
> > > one else does so.
> > 
> > Well how about the following if no glue specific configuration of the
> > ID pin is possible?
> 
> ID pin configuration is required, either in sw or hw, for the controller
> otg state transition.
> 
> In my understanding, if ID pin is grounded, setting session bit make the
> controller transition to host mode; or if ID pin is float, setting
> session bit triggers srp request.

OK so let's rip out this code then. It does not seem to be needed for
enumerating devices at all, updated patch below.

> I think I don't understand what you are trying to solve by adding
> default set_mode() in core.

Naturally no need for this if we don't need this code at all :)

Regards,

Tony

8< -----------------
From: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
Date: Fri, 13 May 2016 07:59:35 -0700
Subject: [PATCH] usb: musb: Don't set d+ high before enable for 2430 glue
 layer

At least 2430 glue layer pulls d+ high on start up even if there are
no gadgets configured. This is bad at least for anything using a separate
battery charger chip as it can confuse the charger detection.

Let's fix the issue by removing the bogus glue layer code tinkering
with the SESSION bit. As pointed out Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> and
Sergei Shtylyov <sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>, the SESSION
bit just starts host mode if ID pin is grounded, and starts the
srp is ID pin is floating. So without the ID pin changing, it's
unable to force musb mode to anything. And just for starting a
host mode, things work fine without this code.

Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>

--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -122,16 +122,6 @@ static void omap2430_musb_set_vbus(struct musb *musb, int is_on)
 		musb_readb(musb->mregs, MUSB_DEVCTL));
 }
 
-static int omap2430_musb_set_mode(struct musb *musb, u8 musb_mode)
-{
-	u8	devctl = musb_readb(musb->mregs, MUSB_DEVCTL);
-
-	devctl |= MUSB_DEVCTL_SESSION;
-	musb_writeb(musb->mregs, MUSB_DEVCTL, devctl);
-
-	return 0;
-}

Re: [PATCH 10/15] usb: musb: Don't set d+ high before enable for 2430 glue layer

[Tony Lindgren]

* Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> [160513 14:19]:
> * Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160513 14:05]:
> > In stead of removing it, session bit should only be set when musb_mode
> > == MUSB_HOST, will this fix the D+ pullup problem?
> 
> Good point, I forgot about it being specific to host mode. I'll check.

Yeah good call, the patch below fixes the issue. Then we can remove the
set_mode later if the generic approach looks OK to you.

Regards,

Tony

8< ----------------------
From: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
Date: Fri, 13 May 2016 07:59:35 -0700
Subject: [PATCH] usb: musb: Don't set d+ high before enable for 2430 glue
 layer

At least 2430 glue layer pulls d+ high on start up even if there are
no gadgets configured. This is bad at least for anything using a separate
battery charger chip as it can confuse the charger detection.

Let's fix the issue by only setting the SESSION bit in host mode
as suggested by Bin Liu <b-liu-l0cyMroinI0@public.gmane.org>.

Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>

--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -124,8 +124,12 @@ static void omap2430_musb_set_vbus(struct musb *musb, int is_on)
 
 static int omap2430_musb_set_mode(struct musb *musb, u8 musb_mode)
 {
-	u8	devctl = musb_readb(musb->mregs, MUSB_DEVCTL);
+	u8 devctl;
 
+	if (musb_mode != MUSB_HOST)
+		return 0;
+
+	devctl = musb_readb(musb->mregs, MUSB_DEVCTL);
 	devctl |= MUSB_DEVCTL_SESSION;
 	musb_writeb(musb->mregs, MUSB_DEVCTL, devctl);
 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/15] usb: musb: Don't set d+ high before enable for 2430 glue layer

[Bin Liu]

Hi,

On Fri, May 13, 2016 at 03:35:40PM -0700, Tony Lindgren wrote:
> * Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> [160513 14:19]:
> > * Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160513 14:05]:
> > > In stead of removing it, session bit should only be set when musb_mode
> > > == MUSB_HOST, will this fix the D+ pullup problem?
> > 
> > Good point, I forgot about it being specific to host mode. I'll check.
> 
> Yeah good call, the patch below fixes the issue. Then we can remove the

Good to know it fixes the issue.

> set_mode later if the generic approach looks OK to you.

I don't think we need to add the generic approach which sets session bit
in core set_mode(), at least for now, because only omap2430 glue needs
to set the session bit in its set_mode(), no one else does so.

Regards,
-Bin.

> 
> Regards,
> 
> Tony
> 
> 8< ----------------------
> From: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> Date: Fri, 13 May 2016 07:59:35 -0700
> Subject: [PATCH] usb: musb: Don't set d+ high before enable for 2430 glue
>  layer
> 
> At least 2430 glue layer pulls d+ high on start up even if there are
> no gadgets configured. This is bad at least for anything using a separate
> battery charger chip as it can confuse the charger detection.
> 
> Let's fix the issue by only setting the SESSION bit in host mode
> as suggested by Bin Liu <b-liu-l0cyMroinI0@public.gmane.org>.
> 
> Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> 
> --- a/drivers/usb/musb/omap2430.c
> +++ b/drivers/usb/musb/omap2430.c
> @@ -124,8 +124,12 @@ static void omap2430_musb_set_vbus(struct musb *musb, int is_on)
>  
>  static int omap2430_musb_set_mode(struct musb *musb, u8 musb_mode)
>  {
> -	u8	devctl = musb_readb(musb->mregs, MUSB_DEVCTL);
> +	u8 devctl;
>  
> +	if (musb_mode != MUSB_HOST)
> +		return 0;
> +
> +	devctl = musb_readb(musb->mregs, MUSB_DEVCTL);
>  	devctl |= MUSB_DEVCTL_SESSION;
>  	musb_writeb(musb->mregs, MUSB_DEVCTL, devctl);
>  
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/15] usb: musb: Don't set d+ high before enable for 2430 glue layer

[Sergei Shtylyov]

On 5/14/2016 12:03 AM, Bin Liu wrote:

>> At least 2430 glue layer pulls d+ high on start up even if there are
>> no gadgets configured. This is bad at least for anything using a separate
>> battery charger chip as it can confuse the charger detection.
>>
>> Let's fix the issue by getting rid of omap2430_musb_set_mode() and only
>
> By doing so, you lost the feature of switching mode from sysfs, I am not
> sure if there is anyone using it though, still, it is a regression.

    BTW, set_mode() implemented in the OMAP glue always seemed bogus to me.
Instead of forcing host/gadget/OTG modes, it sets the Session bit...

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/15] usb: musb: Don't set d+ high before enable for 2430 glue layer

[Bin Liu]

Hi,

On Sat, May 14, 2016 at 04:30:32PM +0300, Sergei Shtylyov wrote:
> On 5/14/2016 12:03 AM, Bin Liu wrote:
> 
> >>At least 2430 glue layer pulls d+ high on start up even if there are
> >>no gadgets configured. This is bad at least for anything using a separate
> >>battery charger chip as it can confuse the charger detection.
> >>
> >>Let's fix the issue by getting rid of omap2430_musb_set_mode() and only
> >
> >By doing so, you lost the feature of switching mode from sysfs, I am not
> >sure if there is anyone using it though, still, it is a regression.
> 
>    BTW, set_mode() implemented in the OMAP glue always seemed bogus to me.
> Instead of forcing host/gadget/OTG modes, it sets the Session bit...

Setting the session bit here seems to be okay. Other glues rely
otg_timer() instead.

Regards,
-Bin.

> 
> MBR, Sergei
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/15] usb: musb: Don't set d+ high before enable for 2430 glue layer

[Tony Lindgren]

* Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160516 07:17]:
> Hi,
> 
> On Sat, May 14, 2016 at 04:30:32PM +0300, Sergei Shtylyov wrote:
> > On 5/14/2016 12:03 AM, Bin Liu wrote:
> > 
> > >>At least 2430 glue layer pulls d+ high on start up even if there are
> > >>no gadgets configured. This is bad at least for anything using a separate
> > >>battery charger chip as it can confuse the charger detection.
> > >>
> > >>Let's fix the issue by getting rid of omap2430_musb_set_mode() and only
> > >
> > >By doing so, you lost the feature of switching mode from sysfs, I am not
> > >sure if there is anyone using it though, still, it is a regression.
> > 
> >    BTW, set_mode() implemented in the OMAP glue always seemed bogus to me.
> > Instead of forcing host/gadget/OTG modes, it sets the Session bit...
> 
> Setting the session bit here seems to be okay. Other glues rely
> otg_timer() instead.

Looks like host mode starts just fine without writing to session bit
for 2430 glue. Let's just remove it, I'll send an updated patch shortly.

For other musb core register tinkering, let's attempt to remove
those from the glue layers eventually. I think Felipe wanted to do this
for a long time as this allows us to switch to a shared interrupt model
for the glue layer and musb core.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/15] usb: musb: Don't set d+ high before enable for 2430 glue layer

[Bin Liu]

Hi,

On Mon, May 16, 2016 at 07:57:57AM -0700, Tony Lindgren wrote:
> * Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160516 07:17]:
> > Hi,
> > 
> > On Sat, May 14, 2016 at 04:30:32PM +0300, Sergei Shtylyov wrote:
> > > On 5/14/2016 12:03 AM, Bin Liu wrote:
> > > 
> > > >>At least 2430 glue layer pulls d+ high on start up even if there are
> > > >>no gadgets configured. This is bad at least for anything using a separate
> > > >>battery charger chip as it can confuse the charger detection.
> > > >>
> > > >>Let's fix the issue by getting rid of omap2430_musb_set_mode() and only
> > > >
> > > >By doing so, you lost the feature of switching mode from sysfs, I am not
> > > >sure if there is anyone using it though, still, it is a regression.
> > > 
> > >    BTW, set_mode() implemented in the OMAP glue always seemed bogus to me.
> > > Instead of forcing host/gadget/OTG modes, it sets the Session bit...
> > 
> > Setting the session bit here seems to be okay. Other glues rely
> > otg_timer() instead.
> 
> Looks like host mode starts just fine without writing to session bit
> for 2430 glue. Let's just remove it, I'll send an updated patch shortly.

So setting host mode in sysfs also works without writting to session
bit? If so, yeah, we can remove it.

Regards,
-Bin.

> 
> For other musb core register tinkering, let's attempt to remove
> those from the glue layers eventually. I think Felipe wanted to do this
> for a long time as this allows us to switch to a shared interrupt model
> for the glue layer and musb core.
> 
> Regards,
> 
> Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/15] usb: musb: Don't set d+ high before enable for 2430 glue layer

[Tony Lindgren]

* Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160516 08:24]:
> Hi,
> 
> On Mon, May 16, 2016 at 07:57:57AM -0700, Tony Lindgren wrote:
> > * Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160516 07:17]:
> > > Hi,
> > > 
> > > On Sat, May 14, 2016 at 04:30:32PM +0300, Sergei Shtylyov wrote:
> > > > On 5/14/2016 12:03 AM, Bin Liu wrote:
> > > > 
> > > > >>At least 2430 glue layer pulls d+ high on start up even if there are
> > > > >>no gadgets configured. This is bad at least for anything using a separate
> > > > >>battery charger chip as it can confuse the charger detection.
> > > > >>
> > > > >>Let's fix the issue by getting rid of omap2430_musb_set_mode() and only
> > > > >
> > > > >By doing so, you lost the feature of switching mode from sysfs, I am not
> > > > >sure if there is anyone using it though, still, it is a regression.
> > > > 
> > > >    BTW, set_mode() implemented in the OMAP glue always seemed bogus to me.
> > > > Instead of forcing host/gadget/OTG modes, it sets the Session bit...
> > > 
> > > Setting the session bit here seems to be okay. Other glues rely
> > > otg_timer() instead.
> > 
> > Looks like host mode starts just fine without writing to session bit
> > for 2430 glue. Let's just remove it, I'll send an updated patch shortly.
> 
> So setting host mode in sysfs also works without writting to session
> bit? If so, yeah, we can remove it.

Testing with USB-B cable with ID floating, host mode is not starting
for me with or without omap2430_musb_set_mode. Things work just fine
using USB-A cable with ID grounded. So yeah, this code seems bogus as
Sergei pointed out.

Regards,

Tony


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 11/15] usb: musb: Return error value from musb_mailbox

[Tony Lindgren]

At least on n900 we have phy-twl4030-usb only generating cable
interrupts, and then have a separate USB PHY.

In order for musb to know the real cable status, we need to
clear any cached state until musb is ready. Otherwise the cable
status interrupts will get just ignored if the status does
not change from the initial state.

To do this, let's add a return value to musb_mailbox(), and
reset cached linkstat to MUSB_UNKNOWN on error. Sorry to cause
a bit of churn here, I should have added that already last time
patching musb_mailbox().

Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
 drivers/phy/phy-twl4030-usb.c     | 14 ++++++++++----
 drivers/usb/musb/musb_core.c      |  7 ++++---
 drivers/usb/musb/musb_core.h      |  2 +-
 drivers/usb/musb/omap2430.c       |  8 +++++---
 drivers/usb/phy/phy-twl6030-usb.c | 12 +++++++++---
 include/linux/usb/musb.h          |  5 +++--
 6 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
index 6b6af6c..d9b10a3 100644
--- a/drivers/phy/phy-twl4030-usb.c
+++ b/drivers/phy/phy-twl4030-usb.c
@@ -463,7 +463,8 @@ static int twl4030_phy_power_on(struct phy *phy)
 	twl4030_usb_set_mode(twl, twl->usb_mode);
 	if (twl->usb_mode == T2_USB_MODE_ULPI)
 		twl4030_i2c_access(twl, 0);
-	schedule_delayed_work(&twl->id_workaround_work, 0);
+	twl->linkstat = MUSB_UNKNOWN;
+	schedule_delayed_work(&twl->id_workaround_work, HZ);
 
 	return 0;
 }
@@ -537,6 +538,7 @@ static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
 	struct twl4030_usb *twl = _twl;
 	enum musb_vbus_id_status status;
 	bool status_changed = false;
+	int err;
 
 	status = twl4030_usb_linkstat(twl);
 
@@ -567,7 +569,9 @@ static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
 			pm_runtime_mark_last_busy(twl->dev);
 			pm_runtime_put_autosuspend(twl->dev);
 		}
-		musb_mailbox(status);
+		err = musb_mailbox(status);
+		if (err)
+			twl->linkstat = MUSB_UNKNOWN;
 	}
 
 	/* don't schedule during sleep - irq works right then */
@@ -595,7 +599,8 @@ static int twl4030_phy_init(struct phy *phy)
 	struct twl4030_usb *twl = phy_get_drvdata(phy);
 
 	pm_runtime_get_sync(twl->dev);
-	schedule_delayed_work(&twl->id_workaround_work, 0);
+	twl->linkstat = MUSB_UNKNOWN;
+	schedule_delayed_work(&twl->id_workaround_work, HZ);
 	pm_runtime_mark_last_busy(twl->dev);
 	pm_runtime_put_autosuspend(twl->dev);
 
@@ -763,7 +768,8 @@ static int twl4030_usb_remove(struct platform_device *pdev)
 	if (cable_present(twl->linkstat))
 		pm_runtime_put_noidle(twl->dev);
 	pm_runtime_mark_last_busy(twl->dev);
-	pm_runtime_put_sync_suspend(twl->dev);
+	pm_runtime_dont_use_autosuspend(&pdev->dev);
+	pm_runtime_put_sync(twl->dev);
 	pm_runtime_disable(twl->dev);
 
 	/* autogate 60MHz ULPI clock,
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 23888d5..6469eff 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1679,7 +1679,7 @@ EXPORT_SYMBOL_GPL(musb_dma_completion);
 #define use_dma			0
 #endif
 
-static void (*musb_phy_callback)(enum musb_vbus_id_status status);
+static int (*musb_phy_callback)(enum musb_vbus_id_status status);
 
 /*
  * musb_mailbox - optional phy notifier function
@@ -1688,11 +1688,12 @@ static void (*musb_phy_callback)(enum musb_vbus_id_status status);
  * Optionally gets called from the USB PHY. Note that the USB PHY must be
  * disabled at the point the phy_callback is registered or unregistered.
  */
-void musb_mailbox(enum musb_vbus_id_status status)
+int musb_mailbox(enum musb_vbus_id_status status)
 {
 	if (musb_phy_callback)
-		musb_phy_callback(status);
+		return musb_phy_callback(status);
 
+	return -ENODEV;
 };
 EXPORT_SYMBOL_GPL(musb_mailbox);
 
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index 2947384..b55a776 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -215,7 +215,7 @@ struct musb_platform_ops {
 				dma_addr_t *dma_addr, u32 *len);
 	void	(*pre_root_reset_end)(struct musb *musb);
 	void	(*post_root_reset_end)(struct musb *musb);
-	void	(*phy_callback)(enum musb_vbus_id_status status);
+	int	(*phy_callback)(enum musb_vbus_id_status status);
 };
 
 /*
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 958ae6a..0b8b5c6 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -180,22 +180,24 @@ static void omap2430_set_power(struct musb *musb, bool enabled, bool cable)
 	}
 }
 
-static void omap2430_musb_mailbox(enum musb_vbus_id_status status)
+static int omap2430_musb_mailbox(enum musb_vbus_id_status status)
 {
 	struct omap2430_glue	*glue = _glue;
 
 	if (!glue) {
 		pr_err("%s: musb core is not yet initialized\n", __func__);
-		return;
+		return -EPROBE_DEFER;
 	}
 	glue->status = status;
 
 	if (!glue_to_musb(glue)) {
 		pr_err("%s: musb core is not yet ready\n", __func__);
-		return;
+		return -EPROBE_DEFER;
 	}
 
 	schedule_work(&glue->omap_musb_mailbox_work);
+
+	return 0;
 }
 
 static void omap_musb_set_mailbox(struct omap2430_glue *glue)
diff --git a/drivers/usb/phy/phy-twl6030-usb.c b/drivers/usb/phy/phy-twl6030-usb.c
index 24e2b3c..c66a447 100644
--- a/drivers/usb/phy/phy-twl6030-usb.c
+++ b/drivers/usb/phy/phy-twl6030-usb.c
@@ -227,12 +227,16 @@ static irqreturn_t twl6030_usb_irq(int irq, void *_twl)
 			twl->asleep = 1;
 			status = MUSB_VBUS_VALID;
 			twl->linkstat = status;
-			musb_mailbox(status);
+			ret = musb_mailbox(status);
+			if (ret)
+				twl->linkstat = MUSB_UNKNOWN;
 		} else {
 			if (twl->linkstat != MUSB_UNKNOWN) {
 				status = MUSB_VBUS_OFF;
 				twl->linkstat = status;
-				musb_mailbox(status);
+				ret = musb_mailbox(status);
+				if (ret)
+					twl->linkstat = MUSB_UNKNOWN;
 				if (twl->asleep) {
 					regulator_disable(twl->usb3v3);
 					twl->asleep = 0;
@@ -264,7 +268,9 @@ static irqreturn_t twl6030_usbotg_irq(int irq, void *_twl)
 		twl6030_writeb(twl, TWL_MODULE_USB, 0x10, USB_ID_INT_EN_HI_SET);
 		status = MUSB_ID_GROUND;
 		twl->linkstat = status;
-		musb_mailbox(status);
+		ret = musb_mailbox(status);
+		if (ret)
+			twl->linkstat = MUSB_UNKNOWN;
 	} else  {
 		twl6030_writeb(twl, TWL_MODULE_USB, 0x10, USB_ID_INT_EN_HI_CLR);
 		twl6030_writeb(twl, TWL_MODULE_USB, 0x1, USB_ID_INT_EN_HI_SET);
diff --git a/include/linux/usb/musb.h b/include/linux/usb/musb.h
index 0b3da40..d315c89 100644
--- a/include/linux/usb/musb.h
+++ b/include/linux/usb/musb.h
@@ -142,10 +142,11 @@ enum musb_vbus_id_status {
 };
 
 #if IS_ENABLED(CONFIG_USB_MUSB_HDRC)
-void musb_mailbox(enum musb_vbus_id_status status);
+int musb_mailbox(enum musb_vbus_id_status status);
 #else
-static inline void musb_mailbox(enum musb_vbus_id_status status)
+static inline int musb_mailbox(enum musb_vbus_id_status status)
 {
+	return 0;
 }
 #endif
 
-- 
2.8.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 12/15] usb: musb: Remove extra PM runtime calls from 2430 glue layer

[Tony Lindgren]

With PM runtime behaving, these are all now unnecessary.
Doing pm_runtime_get(musb->controller) will keep the parent
glue layer also active.

Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
 drivers/usb/musb/omap2430.c | 28 ++++------------------------
 1 file changed, 4 insertions(+), 24 deletions(-)

diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 0b8b5c6..97bd365 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -268,13 +268,8 @@ static void omap_musb_mailbox_work(struct work_struct *mailbox_work)
 {
 	struct omap2430_glue *glue = container_of(mailbox_work,
 				struct omap2430_glue, omap_musb_mailbox_work);
-	struct musb *musb = glue_to_musb(glue);
-	struct device *dev = musb->controller;
 
-	pm_runtime_get_sync(dev);
 	omap_musb_set_mailbox(glue);
-	pm_runtime_mark_last_busy(dev);
-	pm_runtime_put_autosuspend(dev);
 }
 
 static irqreturn_t omap2430_musb_interrupt(int irq, void *__hci)
@@ -343,16 +338,6 @@ static int omap2430_musb_init(struct musb *musb)
 	musb->isr = omap2430_musb_interrupt;
 	phy_init(musb->phy);
 
-	/*
-	 * Enable runtime PM for musb parent (this driver). We can't
-	 * do it earlier as struct musb is not yet allocated and we
-	 * need to touch the musb registers for runtime PM.
-	 */
-	pm_runtime_enable(glue->dev);
-	status = pm_runtime_get_sync(glue->dev);
-	if (status < 0)
-		goto err1;
-
 	l = musb_readl(musb->mregs, OTG_INTERFSEL);
 
 	if (data->interface_type == MUSB_INTERFACE_UTMI) {
@@ -376,11 +361,7 @@ static int omap2430_musb_init(struct musb *musb)
 	if (glue->status != MUSB_UNKNOWN)
 		omap_musb_set_mailbox(glue);
 
-	pm_runtime_put(glue->dev);
 	return 0;
-
-err1:
-	return status;
 }
 
 static void omap2430_musb_enable(struct musb *musb)
@@ -591,11 +572,9 @@ static int omap2430_probe(struct platform_device *pdev)
 		goto err2;
 	}
 
-	/*
-	 * Note that we cannot enable PM runtime yet for this
-	 * driver as we need struct musb initialized first.
-	 * See omap2430_musb_init above.
-	 */
+	pm_runtime_enable(glue->dev);
+	pm_runtime_use_autosuspend(glue->dev);
+	pm_runtime_set_autosuspend_delay(glue->dev, 500);
 
 	ret = platform_device_add(musb);
 	if (ret) {
@@ -621,6 +600,7 @@ static int omap2430_remove(struct platform_device *pdev)
 	platform_device_unregister(glue->musb);
 	omap2430_set_power(musb, false, false);
 	pm_runtime_put_sync(glue->dev);
+	pm_runtime_dont_use_autosuspend(glue->dev);
 	pm_runtime_disable(glue->dev);
 
 	return 0;
-- 
2.8.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 13/15] usb: musb: Remove pm_runtime_set_irq_safe

[Tony Lindgren]

With the pull up being handled with delayed work, we can
now finally remove pm_runtime_set_irq_safe that blocks the
MUSB glue from idling.

Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
 drivers/usb/musb/musb_core.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 6469eff..0286a39 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -2223,12 +2223,6 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
 	pm_runtime_mark_last_busy(musb->controller);
 	pm_runtime_put_autosuspend(musb->controller);
 
-	/*
-	 * For why this is currently needed, see commit 3e43a0725637
-	 * ("usb: musb: core: add pm_runtime_irq_safe()")
-	 */
-	pm_runtime_irq_safe(musb->controller);

[PATCH 14/15] usb: musb: Use normal module_init for 2430 glue

[Tony Lindgren]

There's no longer any need for custom initcall level for
musb.

Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
 drivers/usb/musb/omap2430.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 97bd365..ab0f0f0 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -672,18 +672,8 @@ static struct platform_driver omap2430_driver = {
 	},
 };
 
+module_platform_driver(omap2430_driver);
+
 MODULE_DESCRIPTION("OMAP2PLUS MUSB Glue Layer");
 MODULE_AUTHOR("Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>");
 MODULE_LICENSE("GPL v2");
-
-static int __init omap2430_init(void)
-{
-	return platform_driver_register(&omap2430_driver);
-}
-subsys_initcall(omap2430_init);

[PATCH 15/15] usb: phy: Check initial state for twl6030

[Tony Lindgren]

We need to check the state for the PHY with delayed_work
as otherwise MUSB will get confused and idles immediately.

Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
 drivers/usb/phy/phy-twl6030-usb.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/phy/phy-twl6030-usb.c b/drivers/usb/phy/phy-twl6030-usb.c
index c66a447..a72e8d6 100644
--- a/drivers/usb/phy/phy-twl6030-usb.c
+++ b/drivers/usb/phy/phy-twl6030-usb.c
@@ -97,6 +97,9 @@ struct twl6030_usb {
 
 	struct regulator		*usb3v3;
 
+	/* used to check initial cable status after probe */
+	struct delayed_work	get_status_work;
+
 	/* used to set vbus, in atomic path */
 	struct work_struct	set_vbus_work;
 
@@ -280,6 +283,15 @@ static irqreturn_t twl6030_usbotg_irq(int irq, void *_twl)
 	return IRQ_HANDLED;
 }
 
+static void twl6030_status_work(struct work_struct *work)
+{
+	struct twl6030_usb *twl = container_of(work, struct twl6030_usb,
+					       get_status_work.work);
+
+	twl6030_usb_irq(twl->irq2, twl);
+	twl6030_usbotg_irq(twl->irq1, twl);
+}
+
 static int twl6030_enable_irq(struct twl6030_usb *twl)
 {
 	twl6030_writeb(twl, TWL_MODULE_USB, 0x1, USB_ID_INT_EN_HI_SET);
@@ -290,8 +302,6 @@ static int twl6030_enable_irq(struct twl6030_usb *twl)
 				REG_INT_MSK_LINE_C);
 	twl6030_interrupt_unmask(TWL6030_CHARGER_CTRL_INT_MASK,
 				REG_INT_MSK_STS_C);
-	twl6030_usb_irq(twl->irq2, twl);
-	twl6030_usbotg_irq(twl->irq1, twl);
 
 	return 0;
 }
@@ -377,6 +387,7 @@ static int twl6030_usb_probe(struct platform_device *pdev)
 		dev_warn(&pdev->dev, "could not create sysfs file\n");
 
 	INIT_WORK(&twl->set_vbus_work, otg_set_vbus_work);
+	INIT_DELAYED_WORK(&twl->get_status_work, twl6030_status_work);
 
 	status = request_threaded_irq(twl->irq1, NULL, twl6030_usbotg_irq,
 			IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING | IRQF_ONESHOT,
@@ -401,6 +412,7 @@ static int twl6030_usb_probe(struct platform_device *pdev)
 
 	twl->asleep = 0;
 	twl6030_enable_irq(twl);
+	schedule_delayed_work(&twl->get_status_work, HZ);
 	dev_info(&pdev->dev, "Initialized TWL6030 USB module\n");
 
 	return 0;
@@ -410,6 +422,7 @@ static int twl6030_usb_remove(struct platform_device *pdev)
 {
 	struct twl6030_usb *twl = platform_get_drvdata(pdev);
 
+	cancel_delayed_work(&twl->get_status_work);
 	twl6030_interrupt_mask(TWL6030_USBOTG_INT_MASK,
 		REG_INT_MSK_LINE_C);
 	twl6030_interrupt_mask(TWL6030_USBOTG_INT_MASK,
-- 
2.8.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 00/15] Get MUSB PM runtime working again

[Bin Liu]

Hi Tony,

On Wed, May 11, 2016 at 05:53:01PM -0700, Tony Lindgren wrote:
> Hi all,
> 
> Here's the whole series reposted with a bunch of additional changes.
> It seems to now properly work with with multiple phy cable status
> events, and should work for Ivaylo on n900 too.
> 
> Please re-review and re-test.

While trying to test this patch set, I just found that commit 56f487c (PM
/ Runtime: Update last_busy in rpm_resume) breaks dsps glue (TI am335x
device). Do you have an am335x board with has otg port on it to take a
look at this problem? The only board I know of which has an otg port is
TI am335x GP evm.

Regards,
-Bin.

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 00/15] Get MUSB PM runtime working again

[Tony Lindgren]

* Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160517 14:17]:
> Hi Tony,
> 
> On Wed, May 11, 2016 at 05:53:01PM -0700, Tony Lindgren wrote:
> > Hi all,
> > 
> > Here's the whole series reposted with a bunch of additional changes.
> > It seems to now properly work with with multiple phy cable status
> > events, and should work for Ivaylo on n900 too.
> > 
> > Please re-review and re-test.
> 
> While trying to test this patch set, I just found that commit 56f487c (PM
> / Runtime: Update last_busy in rpm_resume) breaks dsps glue (TI am335x
> device). Do you have an am335x board with has otg port on it to take a
> look at this problem? The only board I know of which has an otg port is
> TI am335x GP evm.

Hmm sure I can take a lok. Chances are that just doing a change for
s/pm_runtime_put/pm_runtime_put_sync/ will fix it. I'm wondering why
it breaks though as dsps glue does not use autosuspend? It could also
be a case where the parent and child PM runtime use counts get out
of sync?

BTW, I did give my series a quick try here on j5eco-evm and that behaved
for peripheral and host enumeration. It's pretty much the same setup as
on am335x I believe.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 00/15] Get MUSB PM runtime working again

[Bin Liu]

On Tue, May 17, 2016 at 02:54:04PM -0700, Tony Lindgren wrote:
> * Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160517 14:17]:
> > Hi Tony,
> > 
> > On Wed, May 11, 2016 at 05:53:01PM -0700, Tony Lindgren wrote:
> > > Hi all,
> > > 
> > > Here's the whole series reposted with a bunch of additional changes.
> > > It seems to now properly work with with multiple phy cable status
> > > events, and should work for Ivaylo on n900 too.
> > > 
> > > Please re-review and re-test.
> > 
> > While trying to test this patch set, I just found that commit 56f487c (PM
> > / Runtime: Update last_busy in rpm_resume) breaks dsps glue (TI am335x
> > device). Do you have an am335x board with has otg port on it to take a
> > look at this problem? The only board I know of which has an otg port is
> > TI am335x GP evm.
> 
> Hmm sure I can take a lok. Chances are that just doing a change for

Ok, thanks.

> s/pm_runtime_put/pm_runtime_put_sync/ will fix it. I'm wondering why
> it breaks though as dsps glue does not use autosuspend? It could also

I am not familiar with pm-runtime framework, but I thought dsps glue
does use autosuspend.

> be a case where the parent and child PM runtime use counts get out
> of sync?
> 
> BTW, I did give my series a quick try here on j5eco-evm and that behaved
> for peripheral and host enumeration. It's pretty much the same setup as
> on am335x I believe.

Does j5eco-evm have a otg port? The case which is broken on mine is

- boot the board;
- modprobe g-zero;
- plug in a usb deivce with A-cable to the otg port;

It generates drvvbus interrupt (0x100) and connect interrupt (0x10), but
then session bit is gone and vbus has no power, so enum failed.

I tried to enable some dynamic-debug log, but was unable to see any
glue. I will try to enable musb reg access log tomorrow to see if I can
find anything.

Regards,
-Bin.

> 
> Regards,
> 
> Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 00/15] Get MUSB PM runtime working again

[Bin Liu]

Hi,

On Tue, May 17, 2016 at 05:05:02PM -0500, Bin Liu wrote:
> On Tue, May 17, 2016 at 02:54:04PM -0700, Tony Lindgren wrote:
> > * Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160517 14:17]:
> > > Hi Tony,
> > > 
> > > On Wed, May 11, 2016 at 05:53:01PM -0700, Tony Lindgren wrote:
> > > > Hi all,
> > > > 
> > > > Here's the whole series reposted with a bunch of additional changes.
> > > > It seems to now properly work with with multiple phy cable status
> > > > events, and should work for Ivaylo on n900 too.
> > > > 
> > > > Please re-review and re-test.
> > > 
> > > While trying to test this patch set, I just found that commit 56f487c (PM
> > > / Runtime: Update last_busy in rpm_resume) breaks dsps glue (TI am335x
> > > device). Do you have an am335x board with has otg port on it to take a
> > > look at this problem? The only board I know of which has an otg port is
> > > TI am335x GP evm.
> > 
> > Hmm sure I can take a lok. Chances are that just doing a change for
> 
> Ok, thanks.
> 
> > s/pm_runtime_put/pm_runtime_put_sync/ will fix it. I'm wondering why
> > it breaks though as dsps glue does not use autosuspend? It could also
> 
> I am not familiar with pm-runtime framework, but I thought dsps glue
> does use autosuspend.

I think I found the problem. Before having the offending patch,
musb_gadget_pullup() got called by musb_gadget_start(), then pm_suspend
immediately (~2us) happens, and musb_save_context() backs up DEVCTL register
which has session bit set.

But with the offending patch, pm_suspend happends much later (~200ms)
after musb_gadget_pullup() is called, so by this time when
musb_save_context() is called, session bit is already cleared by the
controller, so the backup of DEVCTL register does not have session bit
set. Later when pm_resume, musb_restore_context() cleared the seesion
bit.

Regards,
-Bin.

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 00/15] Get MUSB PM runtime working again

[Tony Lindgren]

* Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160518 10:37]:
> I think I found the problem. Before having the offending patch,
> musb_gadget_pullup() got called by musb_gadget_start(), then pm_suspend
> immediately (~2us) happens, and musb_save_context() backs up DEVCTL register
> which has session bit set.
> 
> But with the offending patch, pm_suspend happends much later (~200ms)
> after musb_gadget_pullup() is called, so by this time when
> musb_save_context() is called, session bit is already cleared by the
> controller, so the backup of DEVCTL register does not have session bit
> set. Later when pm_resume, musb_restore_context() cleared the seesion
> bit.

OK sounds like that's expected as that's the current autosuspend
timeout. See for example e6244deed843 ("i2c: omap: Fix PM regression
with deferred probe for pm_runtime_reinit") for references why
pm_runtime_put_sync_suspend() needs to be used if autosuspend has
been set.

So you doing pm_runtime_put_sync_suspend() probably fixes your
issue. (Sorry I think I mentioned just pm_runtime_put_sync()
earlier..)

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 00/15] Get MUSB PM runtime working again

[Alan Stern]

On Wed, 18 May 2016, Tony Lindgren wrote:

> * Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160518 10:37]:
> > I think I found the problem. Before having the offending patch,
> > musb_gadget_pullup() got called by musb_gadget_start(), then pm_suspend
> > immediately (~2us) happens, and musb_save_context() backs up DEVCTL register
> > which has session bit set.
> > 
> > But with the offending patch, pm_suspend happends much later (~200ms)
> > after musb_gadget_pullup() is called, so by this time when
> > musb_save_context() is called, session bit is already cleared by the
> > controller, so the backup of DEVCTL register does not have session bit
> > set. Later when pm_resume, musb_restore_context() cleared the seesion
> > bit.
> 
> OK sounds like that's expected as that's the current autosuspend
> timeout. See for example e6244deed843 ("i2c: omap: Fix PM regression
> with deferred probe for pm_runtime_reinit") for references why
> pm_runtime_put_sync_suspend() needs to be used if autosuspend has
> been set.
> 
> So you doing pm_runtime_put_sync_suspend() probably fixes your
> issue. (Sorry I think I mentioned just pm_runtime_put_sync()
> earlier..)

Actually it may be the other way around.  
pm_runtime_put_sync_suspend() will do an immediate suspend regardless
of the autosuspend timer, whereas pm_runtime_put_sync() will not
suspend the device until the autosuspend timer expires.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 00/15] Get MUSB PM runtime working again

[Tony Lindgren]

* Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org> [160518 11:47]:
> On Wed, 18 May 2016, Tony Lindgren wrote:
> 
> > * Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160518 10:37]:
> > > I think I found the problem. Before having the offending patch,
> > > musb_gadget_pullup() got called by musb_gadget_start(), then pm_suspend
> > > immediately (~2us) happens, and musb_save_context() backs up DEVCTL register
> > > which has session bit set.
> > > 
> > > But with the offending patch, pm_suspend happends much later (~200ms)
> > > after musb_gadget_pullup() is called, so by this time when
> > > musb_save_context() is called, session bit is already cleared by the
> > > controller, so the backup of DEVCTL register does not have session bit
> > > set. Later when pm_resume, musb_restore_context() cleared the seesion
> > > bit.
> > 
> > OK sounds like that's expected as that's the current autosuspend
> > timeout. See for example e6244deed843 ("i2c: omap: Fix PM regression
> > with deferred probe for pm_runtime_reinit") for references why
> > pm_runtime_put_sync_suspend() needs to be used if autosuspend has
> > been set.
> > 
> > So you doing pm_runtime_put_sync_suspend() probably fixes your
> > issue. (Sorry I think I mentioned just pm_runtime_put_sync()
> > earlier..)
> 
> Actually it may be the other way around.  
> pm_runtime_put_sync_suspend() will do an immediate suspend regardless
> of the autosuspend timer, whereas pm_runtime_put_sync() will not
> suspend the device until the autosuspend timer expires.

In this case it seems that pm_runtime_put_sync_suspend() is needed
to save the DEVCTL session bit before it automagically clears..
Would probably be best to handle the session bit separately and
not rely on save/restore for setting it.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 00/15] Get MUSB PM runtime working again

[Bin Liu]

On Wed, May 18, 2016 at 12:12:40PM -0700, Tony Lindgren wrote:
> * Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org> [160518 11:47]:
> > On Wed, 18 May 2016, Tony Lindgren wrote:
> > 
> > > * Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160518 10:37]:
> > > > I think I found the problem. Before having the offending patch,
> > > > musb_gadget_pullup() got called by musb_gadget_start(), then pm_suspend
> > > > immediately (~2us) happens, and musb_save_context() backs up DEVCTL register
> > > > which has session bit set.
> > > > 
> > > > But with the offending patch, pm_suspend happends much later (~200ms)
> > > > after musb_gadget_pullup() is called, so by this time when
> > > > musb_save_context() is called, session bit is already cleared by the
> > > > controller, so the backup of DEVCTL register does not have session bit
> > > > set. Later when pm_resume, musb_restore_context() cleared the seesion
> > > > bit.
> > > 
> > > OK sounds like that's expected as that's the current autosuspend
> > > timeout. See for example e6244deed843 ("i2c: omap: Fix PM regression
> > > with deferred probe for pm_runtime_reinit") for references why
> > > pm_runtime_put_sync_suspend() needs to be used if autosuspend has
> > > been set.
> > > 
> > > So you doing pm_runtime_put_sync_suspend() probably fixes your
> > > issue. (Sorry I think I mentioned just pm_runtime_put_sync()
> > > earlier..)
> > 
> > Actually it may be the other way around.  
> > pm_runtime_put_sync_suspend() will do an immediate suspend regardless
> > of the autosuspend timer, whereas pm_runtime_put_sync() will not
> > suspend the device until the autosuspend timer expires.
> 
> In this case it seems that pm_runtime_put_sync_suspend() is needed
> to save the DEVCTL session bit before it automagically clears..
> Would probably be best to handle the session bit separately and
> not rely on save/restore for setting it.

Yeah, that is what I am thinking for the fix - conditionally restore
session bit. I am on something else now, will back on this issue later.

Regards,
-Bin.

> 
> Regards,
> 
> Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 00/15] Get MUSB PM runtime working again

[Tony Lindgren]

* Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160517 15:06]:
> On Tue, May 17, 2016 at 02:54:04PM -0700, Tony Lindgren wrote:
> 
> > s/pm_runtime_put/pm_runtime_put_sync/ will fix it. I'm wondering why
> > it breaks though as dsps glue does not use autosuspend? It could also
> 
> I am not familiar with pm-runtime framework, but I thought dsps glue
> does use autosuspend.
> 
> > be a case where the parent and child PM runtime use counts get out
> > of sync?
> > 
> > BTW, I did give my series a quick try here on j5eco-evm and that behaved
> > for peripheral and host enumeration. It's pretty much the same setup as
> > on am335x I believe.
> 
> Does j5eco-evm have a otg port?

Yes it has a mini-AB port and usb0_id is wired.

> The case which is broken on mine is

> - boot the board;
> - modprobe g-zero;
> - plug in a usb deivce with A-cable to the otg port;
> 
> It generates drvvbus interrupt (0x100) and connect interrupt (0x10), but
> then session bit is gone and vbus has no power, so enum failed.

Is this with or without the $subject series?

> I tried to enable some dynamic-debug log, but was unable to see any
> glue. I will try to enable musb reg access log tomorrow to see if I can
> find anything.

OK, I'm looking at implementing proper PM runtime for dsps.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 00/15] Get MUSB PM runtime working again

[Bin Liu]

On Wed, May 18, 2016 at 11:07:37AM -0700, Tony Lindgren wrote:
> * Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160517 15:06]:
> > On Tue, May 17, 2016 at 02:54:04PM -0700, Tony Lindgren wrote:
> > 
> > > s/pm_runtime_put/pm_runtime_put_sync/ will fix it. I'm wondering why
> > > it breaks though as dsps glue does not use autosuspend? It could also
> > 
> > I am not familiar with pm-runtime framework, but I thought dsps glue
> > does use autosuspend.
> > 
> > > be a case where the parent and child PM runtime use counts get out
> > > of sync?
> > > 
> > > BTW, I did give my series a quick try here on j5eco-evm and that behaved
> > > for peripheral and host enumeration. It's pretty much the same setup as
> > > on am335x I believe.
> > 
> > Does j5eco-evm have a otg port?
> 
> Yes it has a mini-AB port and usb0_id is wired.
> 
> > The case which is broken on mine is
> 
> > - boot the board;
> > - modprobe g-zero;
> > - plug in a usb deivce with A-cable to the otg port;
> > 
> > It generates drvvbus interrupt (0x100) and connect interrupt (0x10), but
> > then session bit is gone and vbus has no power, so enum failed.
> 
> Is this with or without the $subject series?

Without.

> 
> > I tried to enable some dynamic-debug log, but was unable to see any
> > glue. I will try to enable musb reg access log tomorrow to see if I can
> > find anything.
> 
> OK, I'm looking at implementing proper PM runtime for dsps.
> 
> Regards,
> 
> Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 00/15] Get MUSB PM runtime working again

[Tony Lindgren]

Hi,

* Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160518 11:12]:
> On Wed, May 18, 2016 at 11:07:37AM -0700, Tony Lindgren wrote:
> > 
> > Is this with or without the $subject series?
> 
> Without.

OK sounds like you have a fix coming for that issue.

> > > I tried to enable some dynamic-debug log, but was unable to see any
> > > glue. I will try to enable musb reg access log tomorrow to see if I can
> > > find anything.
> > 
> > OK, I'm looking at implementing proper PM runtime for dsps.

And here's something to play with for PM runtime on top of the
$subject series.

Currently it only works in PIO mode and relies on keeping the
polling going.. See notes below about the GPIO mode.

BTW, looks like we can then just also remove try_idle also for
dsps glue layer.

Regards,

Tony

8< ----------------
From: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
Date: Wed, 18 May 2016 08:22:31 -0700
Subject: [PATCH] usb: musb: Add PM runtime support for dsps glue for PIO mode

This gets basic PM runtime working for dsps glue layer uwing
PIO mode.

For now, we must keep polling enabled as typically at least
one of the controllers has ID pin tied down. Later on we can
add support for remuxing USB data lines to GPIO mode to leave
out the polling where possible.

Note that this shuts down the device currently only with
CONFIG_MUSB_PIO_ONLY=y as cppi41 is a child of musb_am335x.c
and needs PM runtime fixed up first.

Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>

--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -144,6 +144,7 @@ struct dsps_glue {
 	const struct dsps_musb_wrapper *wrp; /* wrapper register offsets */
 	struct timer_list timer;	/* otg_workaround timer */
 	unsigned long last_timer;    /* last timer data for each instance */
+	bool powered;
 	bool sw_babble_enabled;
 
 	struct dsps_context context;
@@ -250,6 +251,35 @@ static void dsps_musb_disable(struct musb *musb)
 	dsps_writeb(musb->mregs, MUSB_DEVCTL, 0);
 }
 
+static void dsps_musb_set_power(struct musb *musb, bool enabled)
+{
+	struct dsps_glue *glue = dev_get_drvdata(musb->controller->parent);
+	int err;
+
+	if (enabled == glue->powered) {
+		dev_warn(musb->controller, "Power already %i\n",
+			 glue->powered);
+		return;
+	}
+
+	if (glue->powered) {
+		dev_dbg(musb->controller, "Disabling power\n");
+		pm_runtime_mark_last_busy(musb->controller);
+		pm_runtime_put_autosuspend(musb->controller);
+		glue->powered = false;
+	} else {
+		dev_dbg(musb->controller, "Enabling power\n");
+		err = pm_runtime_get_sync(musb->controller);
+		if (err < 0) {
+			dev_err(musb->controller,
+				"Could not pm_runtime_get: %i\n",
+				err);
+			return;
+		}
+		glue->powered = true;
+	}
+}
+
 static void otg_timer(unsigned long _musb)
 {
 	struct musb *musb = (void *)_musb;
@@ -260,6 +290,11 @@ static void otg_timer(unsigned long _musb)
 	u8 devctl;
 	unsigned long flags;
 	int skip_session = 0;
+	int err;
+
+	err = pm_runtime_get_sync(dev);
+	if (err < 0)
+		dev_err(dev, "Poll could not pm_runtime_get: %i\n", err);
 
 	/*
 	 * We poll because DSPS IP's won't expose several OTG-critical
@@ -287,6 +322,7 @@ static void otg_timer(unsigned long _musb)
 		}
 		if (!(devctl & MUSB_DEVCTL_SESSION) && !skip_session)
 			dsps_writeb(mregs, MUSB_DEVCTL, MUSB_DEVCTL_SESSION);
+		dsps_musb_set_power(musb, false);
 		mod_timer(&glue->timer, jiffies +
 				msecs_to_jiffies(wrp->poll_timeout));
 		break;
@@ -294,11 +330,18 @@ static void otg_timer(unsigned long _musb)
 		musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
 		dsps_writel(musb->ctrl_base, wrp->coreintr_set,
 			    MUSB_INTR_VBUSERROR << wrp->usb_shift);
+		dsps_musb_set_power(musb, false);
+		break;
+	case OTG_STATE_UNDEFINED:
 		break;
 	default:
+		dsps_musb_set_power(musb, true);
 		break;
 	}
 	spin_unlock_irqrestore(&musb->lock, flags);
+
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
 }
 
 static irqreturn_t dsps_interrupt(int irq, void *hci)
@@ -362,7 +405,6 @@ static irqreturn_t dsps_interrupt(int irq, void *hci)
 			MUSB_HST_MODE(musb);
 			musb->xceiv->otg->default_a = 1;
 			musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
-			del_timer(&glue->timer);
 		} else {
 			musb->is_active = 0;
 			MUSB_DEV_MODE(musb);
@@ -382,9 +424,10 @@ static irqreturn_t dsps_interrupt(int irq, void *hci)
 	if (musb->int_tx || musb->int_rx || musb->int_usb)
 		ret |= musb_interrupt(musb);
 
-	/* Poll for ID change in OTG port mode */
-	if (musb->xceiv->otg->state == OTG_STATE_B_IDLE &&
-			musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE)
+	/* Poll for ID change and device connect */
+	if (musb->xceiv->otg->state == OTG_STATE_A_IDLE ||
+	    musb->xceiv->otg->state == OTG_STATE_A_WAIT_BCON ||
+	    musb->xceiv->otg->state == OTG_STATE_B_IDLE)
 		mod_timer(&glue->timer, jiffies +
 				msecs_to_jiffies(wrp->poll_timeout));
 out:
@@ -498,6 +541,7 @@ static int dsps_musb_exit(struct musb *musb)
 	phy_power_off(musb->phy);
 	phy_exit(musb->phy);
 	debugfs_remove_recursive(glue->dbgfs_root);
+	dsps_musb_set_power(musb, false);
 
 	return 0;
 }
@@ -808,6 +852,8 @@ static int dsps_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, glue);
 	pm_runtime_enable(&pdev->dev);
+	pm_runtime_use_autosuspend(&pdev->dev);
+	pm_runtime_set_autosuspend_delay(&pdev->dev, 500);
 
 	ret = pm_runtime_get_sync(&pdev->dev);
 	if (ret < 0) {
@@ -819,11 +865,15 @@ static int dsps_probe(struct platform_device *pdev)
 	if (ret)
 		goto err3;
 
+	pm_runtime_mark_last_busy(&pdev->dev);
+	pm_runtime_put_autosuspend(&pdev->dev);
+
 	return 0;
 
 err3:
-	pm_runtime_put(&pdev->dev);
+	pm_runtime_put_sync(&pdev->dev);
 err2:
+	pm_runtime_dont_use_autosuspend(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 	return ret;
 }
@@ -835,7 +885,8 @@ static int dsps_remove(struct platform_device *pdev)
 	platform_device_unregister(glue->musb);
 
 	/* disable usbss clocks */
-	pm_runtime_put(&pdev->dev);
+	pm_runtime_dont_use_autosuspend(&pdev->dev);
+	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
 	return 0;
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 00/15] Get MUSB PM runtime working again

[Tony Lindgren]

* Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> [160519 08:28]:
> +static void dsps_musb_set_power(struct musb *musb, bool enabled)
> +{
> +	struct dsps_glue *glue = dev_get_drvdata(musb->controller->parent);
> +	int err;
> +
> +	if (enabled == glue->powered) {
> +		dev_warn(musb->controller, "Power already %i\n",
> +			 glue->powered);
> +		return;
> +	}
> +
> +	if (glue->powered) {

Heh this should be enabled not glue->powered here :) And enumeration
fails in peripheral mode if cable is connected on start up, that
needs more handling for the interrupt. Updated patch below.

Then this set_power should probably become something more generic
like:

void musb_set_cable_connected(struct musb *musb, bool connected);

That way the glue layers can call it after making sense of the
cable state.

Tony

8< ---------------
From: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
Date: Thu, 19 May 2016 09:53:26 -0700
Subject: [PATCH] usb: musb: Add PM runtime support for dsps glue for PIO mode

This gets basic PM runtime working for dsps glue layer uwing
PIO mode.

For now, we must keep polling enabled as typically at least
one of the controllers has ID pin tied down. Later on we can
add support for remuxing USB data lines to GPIO mode to leave
out the polling where possible.

Note that this shuts down the device currently only with
CONFIG_MUSB_PIO_ONLY=y as cppi41 is a child of musb_am335x.c
and needs PM runtime fixed up first.

Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>

--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -103,6 +103,7 @@ struct dsps_musb_wrapper {
 	u32		usb_mask;
 	u32		usb_bitmap;
 	unsigned	drvvbus:5;
+	unsigned	connected:5;
 
 	unsigned	txep_shift:5;
 	u32		txep_mask;
@@ -144,6 +145,7 @@ struct dsps_glue {
 	const struct dsps_musb_wrapper *wrp; /* wrapper register offsets */
 	struct timer_list timer;	/* otg_workaround timer */
 	unsigned long last_timer;    /* last timer data for each instance */
+	bool powered;
 	bool sw_babble_enabled;
 
 	struct dsps_context context;
@@ -250,6 +252,35 @@ static void dsps_musb_disable(struct musb *musb)
 	dsps_writeb(musb->mregs, MUSB_DEVCTL, 0);
 }
 
+static void dsps_musb_set_power(struct musb *musb, bool enable)
+{
+	struct dsps_glue *glue = dev_get_drvdata(musb->controller->parent);
+	int err;
+
+	if (enable == glue->powered) {
+		dev_warn(musb->controller, "Power already %i\n",
+			 glue->powered);
+		return;
+	}
+
+	if (enable) {
+		dev_dbg(musb->controller, "Enabling power\n");
+		err = pm_runtime_get_sync(musb->controller);
+		if (err < 0) {
+			dev_err(musb->controller,
+				"Could not pm_runtime_get: %i\n",
+				err);
+			return;
+		}
+		glue->powered = true;
+	} else {
+		dev_dbg(musb->controller, "Disabling power\n");
+		pm_runtime_mark_last_busy(musb->controller);
+		pm_runtime_put_autosuspend(musb->controller);
+		glue->powered = false;
+	}
+}
+
 static void otg_timer(unsigned long _musb)
 {
 	struct musb *musb = (void *)_musb;
@@ -260,6 +291,11 @@ static void otg_timer(unsigned long _musb)
 	u8 devctl;
 	unsigned long flags;
 	int skip_session = 0;
+	int err;
+
+	err = pm_runtime_get_sync(dev);
+	if (err < 0)
+		dev_err(dev, "Poll could not pm_runtime_get: %i\n", err);
 
 	/*
 	 * We poll because DSPS IP's won't expose several OTG-critical
@@ -287,6 +323,7 @@ static void otg_timer(unsigned long _musb)
 		}
 		if (!(devctl & MUSB_DEVCTL_SESSION) && !skip_session)
 			dsps_writeb(mregs, MUSB_DEVCTL, MUSB_DEVCTL_SESSION);
+		dsps_musb_set_power(musb, false);
 		mod_timer(&glue->timer, jiffies +
 				msecs_to_jiffies(wrp->poll_timeout));
 		break;
@@ -294,11 +331,18 @@ static void otg_timer(unsigned long _musb)
 		musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
 		dsps_writel(musb->ctrl_base, wrp->coreintr_set,
 			    MUSB_INTR_VBUSERROR << wrp->usb_shift);
+		dsps_musb_set_power(musb, false);
+		break;
+	case OTG_STATE_UNDEFINED:
 		break;
 	default:
+		dsps_musb_set_power(musb, true);
 		break;
 	}
 	spin_unlock_irqrestore(&musb->lock, flags);
+
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
 }
 
 static irqreturn_t dsps_interrupt(int irq, void *hci)
@@ -362,7 +406,6 @@ static irqreturn_t dsps_interrupt(int irq, void *hci)
 			MUSB_HST_MODE(musb);
 			musb->xceiv->otg->default_a = 1;
 			musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
-			del_timer(&glue->timer);
 		} else {
 			musb->is_active = 0;
 			MUSB_DEV_MODE(musb);
@@ -379,12 +422,18 @@ static irqreturn_t dsps_interrupt(int irq, void *hci)
 		ret = IRQ_HANDLED;
 	}
 
+	if (usbintr & ((1 << wrp->connected) << wrp->usb_shift)) {
+		mod_timer(&glue->timer, jiffies +
+			  msecs_to_jiffies(wrp->poll_timeout));
+	}
+
 	if (musb->int_tx || musb->int_rx || musb->int_usb)
 		ret |= musb_interrupt(musb);
 
-	/* Poll for ID change in OTG port mode */
-	if (musb->xceiv->otg->state == OTG_STATE_B_IDLE &&
-			musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE)
+	/* Poll for ID change and device connect */
+	if (musb->xceiv->otg->state == OTG_STATE_A_IDLE ||
+	    musb->xceiv->otg->state == OTG_STATE_A_WAIT_BCON ||
+	    musb->xceiv->otg->state == OTG_STATE_B_IDLE)
 		mod_timer(&glue->timer, jiffies +
 				msecs_to_jiffies(wrp->poll_timeout));
 out:
@@ -498,6 +547,7 @@ static int dsps_musb_exit(struct musb *musb)
 	phy_power_off(musb->phy);
 	phy_exit(musb->phy);
 	debugfs_remove_recursive(glue->dbgfs_root);
+	dsps_musb_set_power(musb, false);
 
 	return 0;
 }
@@ -808,6 +858,8 @@ static int dsps_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, glue);
 	pm_runtime_enable(&pdev->dev);
+	pm_runtime_use_autosuspend(&pdev->dev);
+	pm_runtime_set_autosuspend_delay(&pdev->dev, 500);
 
 	ret = pm_runtime_get_sync(&pdev->dev);
 	if (ret < 0) {
@@ -819,11 +871,15 @@ static int dsps_probe(struct platform_device *pdev)
 	if (ret)
 		goto err3;
 
+	pm_runtime_mark_last_busy(&pdev->dev);
+	pm_runtime_put_autosuspend(&pdev->dev);
+
 	return 0;
 
 err3:
-	pm_runtime_put(&pdev->dev);
+	pm_runtime_put_sync(&pdev->dev);
 err2:
+	pm_runtime_dont_use_autosuspend(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 	return ret;
 }
@@ -835,7 +891,8 @@ static int dsps_remove(struct platform_device *pdev)
 	platform_device_unregister(glue->musb);
 
 	/* disable usbss clocks */
-	pm_runtime_put(&pdev->dev);
+	pm_runtime_dont_use_autosuspend(&pdev->dev);
+	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
 	return 0;
@@ -863,6 +920,7 @@ static const struct dsps_musb_wrapper am33xx_driver_data = {
 	.usb_mask		= 0x1ff,
 	.usb_bitmap		= (0x1ff << 0),
 	.drvvbus		= 8,
+	.connected		= 4,
 	.txep_shift		= 0,
 	.txep_mask		= 0xffff,
 	.txep_bitmap		= (0xffff << 0),
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html