[PATCH 0/7] usb: otg: twl4030-usb fixes

[PATCH 0/7] usb: otg: twl4030-usb fixes

[Grazvydas Ignotas]

I have a pandora board which has similar musb setup to beagleboard
(OMAP3530 + TWL4030) and musb never worked well on it for me in mainline.
Well it usually works if you plug the cable once, but as soon as you start
replugging cables and mixing host adapter into the game it totally breaks
and reboot is then needed. Host mode is especially broken, any replugs
after musb has been in host mode result in dead port that needs reboot
to recover.

With this series I can switch host/peripheral cables any way I like and
even suspend works with cable plugged with musb in peripheral mode!
("ARM: OMAP3: hwmod data: disable MIDLEMODE control for musb" is needed
that was sent separately). This also fixes power drain when cable is
plugged an no gadget driver is loaded.

Grazvydas Ignotas (7):
  usb: otg: twl4030-usb: don't enable PHY during init
  usb: otg: twl4030-usb: ignore duplicate events
  usb: otg: twl4030-usb: don't switch the phy on/off needlessly
  usb: otg: twl4030-usb: poll for ID disconnect
  usb: otg: twl4030-usb: check if vbus is driven by twl itself
  usb: musb: omap2430: turn off vbus on cable disconnect
  usb: musb: gadget: use platform callback to enable vbus

 drivers/usb/musb/musb_gadget.c |    5 +-
 drivers/usb/musb/omap2430.c    |    1 +
 drivers/usb/otg/twl4030-usb.c  |  105 ++++++++++++++++++++++++++++++----------
 3 files changed, 82 insertions(+), 29 deletions(-)

-- 
1.7.9.5

--
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 1/7] usb: otg: twl4030-usb: don't enable PHY during init

[Grazvydas Ignotas]

There is no need to do it, otg.set_suspend(false) (which itself
comes from runtime_pm OMAP glue calls) will enable it later anyway.
This used to be the place where things were enabled if booted with
cable connected before runtime_pm conversion, but now can be dropped.

Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
---
 drivers/usb/otg/twl4030-usb.c |   23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/otg/twl4030-usb.c b/drivers/usb/otg/twl4030-usb.c
index a994715..1515c0b 100644
--- a/drivers/usb/otg/twl4030-usb.c
+++ b/drivers/usb/otg/twl4030-usb.c
@@ -522,19 +522,17 @@ static void twl4030_usb_phy_init(struct twl4030_usb *twl)
 {
 	enum omap_musb_vbus_id_status status;
 
-	status = twl4030_usb_linkstat(twl);
-	if (status > 0) {
-		if (status == OMAP_MUSB_VBUS_OFF ||
-				status == OMAP_MUSB_ID_FLOAT) {
-			__twl4030_phy_power(twl, 0);
-			twl->asleep = 1;
-		} else {
-			__twl4030_phy_resume(twl);
-			twl->asleep = 0;
-		}
+	/*
+	 * Start in sleep state, we'll get called through set_suspend()
+	 * callback when musb is runtime resumed and it's time to start.
+	 */
+	__twl4030_phy_power(twl, 0);
+	twl->asleep = 1;
 
+	status = twl4030_usb_linkstat(twl);
+	if (status > 0)
 		omap_musb_mailbox(twl->linkstat);
-	}
+
 	sysfs_notify(&twl->dev->kobj, NULL, "vbus");
 }
 
@@ -649,9 +647,6 @@ static int twl4030_usb_probe(struct platform_device *pdev)
 		return status;
 	}
 
-	/* Power down phy or make it work according to
-	 * current link state.
-	 */
 	twl4030_usb_phy_init(twl);
 
 	dev_info(&pdev->dev, "Initialized TWL4030 USB module\n");
-- 
1.7.9.5

[PATCH 2/7] usb: otg: twl4030-usb: ignore duplicate events

[Grazvydas Ignotas]

In some rare cases we may get multiple interrupts that will generate
duplicate omap_musb_mailbox() calls. This is a problem because each
VBUS/ID event generates runtime_pm call in OMAP glue code, causing
unbalanced gets or puts and breaking PM.

The same goes for initial state, glue already defaults to "no cable"
state, so only bother it if we have VBUS or ID.

Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
---
 drivers/usb/otg/twl4030-usb.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/otg/twl4030-usb.c b/drivers/usb/otg/twl4030-usb.c
index 1515c0b..0ea576a 100644
--- a/drivers/usb/otg/twl4030-usb.c
+++ b/drivers/usb/otg/twl4030-usb.c
@@ -491,9 +491,10 @@ static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
 {
 	struct twl4030_usb *twl = _twl;
 	enum omap_musb_vbus_id_status status;
+	enum omap_musb_vbus_id_status status_prev = twl->linkstat;
 
 	status = twl4030_usb_linkstat(twl);
-	if (status > 0) {
+	if (status > 0 && status != status_prev) {
 		/* FIXME add a set_power() method so that B-devices can
 		 * configure the charger appropriately.  It's not always
 		 * correct to consume VBUS power, and how much current to
@@ -530,7 +531,7 @@ static void twl4030_usb_phy_init(struct twl4030_usb *twl)
 	twl->asleep = 1;
 
 	status = twl4030_usb_linkstat(twl);
-	if (status > 0)
+	if (status == OMAP_MUSB_ID_GROUND || status == OMAP_MUSB_VBUS_VALID)
 		omap_musb_mailbox(twl->linkstat);
 
 	sysfs_notify(&twl->dev->kobj, NULL, "vbus");
-- 
1.7.9.5

[PATCH 3/7] usb: otg: twl4030-usb: don't switch the phy on/off needlessly

[Grazvydas Ignotas]

With runtime_pm in place there is no longer need to turn the phy
on/off in OTG layer on cable connect/disconnect, OMAP glue does
this through otg.set_suspend() callback after it's called through
omap_musb_mailbox() on VBUS/ID interrupt. Not doing this will save
power when cable is connected but no gadget driver is loaded.

This will also have side effect of automatic USB charging no longer
working without twl4030_charger driver, so be sure to enable it if
charging is needed.

Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
---
 drivers/usb/otg/twl4030-usb.c |    6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/usb/otg/twl4030-usb.c b/drivers/usb/otg/twl4030-usb.c
index 0ea576a..90a19ff 100644
--- a/drivers/usb/otg/twl4030-usb.c
+++ b/drivers/usb/otg/twl4030-usb.c
@@ -506,12 +506,6 @@ static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
 		 * USB_LINK_VBUS state.  musb_hdrc won't care until it
 		 * starts to handle softconnect right.
 		 */
-		if (status == OMAP_MUSB_VBUS_OFF ||
-				status == OMAP_MUSB_ID_FLOAT)
-			twl4030_phy_suspend(twl, 0);
-		else
-			twl4030_phy_resume(twl);
-
 		omap_musb_mailbox(twl->linkstat);
 	}
 	sysfs_notify(&twl->dev->kobj, NULL, "vbus");
-- 
1.7.9.5

[PATCH 4/7] usb: otg: twl4030-usb: poll for ID disconnect

[Grazvydas Ignotas]

On pandora, STS_USB interrupt doesn't arrive on USB host cable disconnect
for some reason while VBUS is driven by twl itself, but STS_HW_CONDITIONS
is updated correctly. It does work fine when PHY is powered down though.
To work around that we have to poll.

TI PSP kernels have similar workarounds, so (many?) more boards are likely
affected.

Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
---
 drivers/usb/otg/twl4030-usb.c |   37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/usb/otg/twl4030-usb.c b/drivers/usb/otg/twl4030-usb.c
index 90a19ff..2c1c27e 100644
--- a/drivers/usb/otg/twl4030-usb.c
+++ b/drivers/usb/otg/twl4030-usb.c
@@ -163,6 +163,8 @@ struct twl4030_usb {
 	bool			vbus_supplied;
 	u8			asleep;
 	bool			irq_enabled;
+
+	struct delayed_work	id_workaround_work;
 };
 
 /* internal define on top of container_of */
@@ -412,6 +414,16 @@ static void twl4030_phy_resume(struct twl4030_usb *twl)
 	__twl4030_phy_resume(twl);
 	twl->asleep = 0;
 	dev_dbg(twl->dev, "%s\n", __func__);
+
+	/*
+	 * XXX When VBUS gets driven after musb goes to A mode,
+	 * ID_PRES related interrupts no longer arrive, why?
+	 * Register itself is updated fine though, so we must poll.
+	 */
+	if (twl->linkstat == OMAP_MUSB_ID_GROUND) {
+		cancel_delayed_work(&twl->id_workaround_work);
+		schedule_delayed_work(&twl->id_workaround_work, HZ);
+	}
 }
 
 static int twl4030_usb_ldo_init(struct twl4030_usb *twl)
@@ -513,6 +525,28 @@ static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
 	return IRQ_HANDLED;
 }
 
+static void twl4030_id_workaround_work(struct work_struct *work)
+{
+	struct twl4030_usb *twl = container_of(work, struct twl4030_usb,
+		id_workaround_work.work);
+	enum omap_musb_vbus_id_status status_prev = twl->linkstat;
+	enum omap_musb_vbus_id_status status;
+
+	status = twl4030_usb_linkstat(twl);
+	if (status != status_prev) {
+		dev_dbg(twl->dev, "handle missing status change: %d->%d\n",
+			status_prev, status);
+		twl->linkstat = status_prev;
+		twl4030_usb_irq(0, twl);
+	}
+
+	/* don't schedule during sleep - irq works right then */
+	if (status == OMAP_MUSB_ID_GROUND && !twl->asleep) {
+		cancel_delayed_work(&twl->id_workaround_work);
+		schedule_delayed_work(&twl->id_workaround_work, HZ);
+	}
+}
+
 static void twl4030_usb_phy_init(struct twl4030_usb *twl)
 {
 	enum omap_musb_vbus_id_status status;
@@ -613,6 +647,8 @@ static int twl4030_usb_probe(struct platform_device *pdev)
 	/* init spinlock for workqueue */
 	spin_lock_init(&twl->lock);
 
+	INIT_DELAYED_WORK(&twl->id_workaround_work, twl4030_id_workaround_work);
+
 	err = twl4030_usb_ldo_init(twl);
 	if (err) {
 		dev_err(&pdev->dev, "ldo init failed\n");
@@ -653,6 +689,7 @@ static int __exit twl4030_usb_remove(struct platform_device *pdev)
 	struct twl4030_usb *twl = platform_get_drvdata(pdev);
 	int val;
 
+	cancel_delayed_work(&twl->id_workaround_work);
 	free_irq(twl->irq, twl);
 	device_remove_file(twl->dev, &dev_attr_vbus);
 
-- 
1.7.9.5

[PATCH 5/7] usb: otg: twl4030-usb: check if vbus is driven by twl itself

[Grazvydas Ignotas]

At least on pandora, STS_VBUS gets set even when VBUS is driven by twl
itself. Reporting VBUS in this case confuses OMAP musb glue and charger
driver, so check if OTG VBUS charge pump is on before reporting VBUS
event to avoid this problem.

Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
---
 drivers/usb/otg/twl4030-usb.c |   36 +++++++++++++++++++++++++++++++-----
 1 file changed, 31 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/otg/twl4030-usb.c b/drivers/usb/otg/twl4030-usb.c
index 2c1c27e..8f0d6cf 100644
--- a/drivers/usb/otg/twl4030-usb.c
+++ b/drivers/usb/otg/twl4030-usb.c
@@ -248,11 +248,31 @@ twl4030_usb_clear_bits(struct twl4030_usb *twl, u8 reg, u8 bits)
 
 /*-------------------------------------------------------------------------*/
 
+static bool twl4030_is_driving_vbus(struct twl4030_usb *twl)
+{
+	int ret;
+
+	ret = twl4030_usb_read(twl, PHY_CLK_CTRL_STS);
+	if (ret < 0 || !(ret & PHY_DPLL_CLK))
+		/*
+		 * if clocks are off, registers are not updated,
+		 * but we can assume we don't drive VBUS in this case
+		 */
+		return false;
+
+	ret = twl4030_usb_read(twl, ULPI_OTG_CTRL);
+	if (ret < 0)
+		return false;
+
+	return (ret & (ULPI_OTG_DRVVBUS | ULPI_OTG_CHRGVBUS)) ? true : false;
+}
+
 static enum omap_musb_vbus_id_status
 	twl4030_usb_linkstat(struct twl4030_usb *twl)
 {
 	int	status;
 	enum omap_musb_vbus_id_status linkstat = OMAP_MUSB_UNKNOWN;
+	bool	driving_vbus = false;
 
 	twl->vbus_supplied = false;
 
@@ -270,20 +290,26 @@ static enum omap_musb_vbus_id_status
 	if (status < 0)
 		dev_err(twl->dev, "USB link status err %d\n", status);
 	else if (status & (BIT(7) | BIT(2))) {
-		if (status & (BIT(7)))
-                        twl->vbus_supplied = true;
+		if (status & BIT(7)) {
+			driving_vbus = twl4030_is_driving_vbus(twl);
+			if (driving_vbus)
+				status &= ~BIT(7);
+		}
 
 		if (status & BIT(2))
 			linkstat = OMAP_MUSB_ID_GROUND;
-		else
+		else if (status & BIT(7)) {
 			linkstat = OMAP_MUSB_VBUS_VALID;
+			twl->vbus_supplied = true;
+		} else
+			linkstat = OMAP_MUSB_VBUS_OFF;
 	} else {
 		if (twl->linkstat != OMAP_MUSB_UNKNOWN)
 			linkstat = OMAP_MUSB_VBUS_OFF;
 	}
 
-	dev_dbg(twl->dev, "HW_CONDITIONS 0x%02x/%d; link %d\n",
-			status, status, linkstat);
+	dev_dbg(twl->dev, "HW_CONDITIONS 0x%02x; link %d, driving_vbus %d\n",
+		status, linkstat, driving_vbus);
 
 	/* REVISIT this assumes host and peripheral controllers
 	 * are registered, and that both are active...
-- 
1.7.9.5

[PATCH 6/7] usb: musb: omap2430: turn off vbus on cable disconnect

[Grazvydas Ignotas]

On USB_EVENT_ID event the musb glue enables VBUS by calling
omap2430_musb_set_vbus(musb, 1) that sets the session bit, but on
USB_EVENT_NONE reverse action is never made, and that breaks PM.

Disable VBUS unconditionally on USB_EVENT_NONE to be sure musb
session is ended on cable unplug so that PM works.

Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
---
 drivers/usb/musb/omap2430.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 2a39c11..d430677 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -296,6 +296,7 @@ static void omap_musb_set_mailbox(struct omap2430_glue *glue)
 			pm_runtime_put_autosuspend(dev);
 		}
 
+		omap2430_musb_set_vbus(musb, 0);
 		if (data->interface_type == MUSB_INTERFACE_UTMI) {
 			if (musb->xceiv->otg->set_vbus)
 				otg_set_vbus(musb->xceiv->otg, 0);
-- 
1.7.9.5

[PATCH 7/7] usb: musb: gadget: use platform callback to enable vbus

[Grazvydas Ignotas]

On some platform configurations (like OMAP3+twl4030) it's the platform
code that enables VBUS, not OTG transceiver, so call vbus platform
callback instead, it will then call the transceiver if needed.

This fixes a use case where USB cable is plugged first and gadget
driver is loaded later after that.

Signed-off-by: Grazvydas Ignotas <notasas-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/usb/musb/musb_gadget.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index be18537..19998e9 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -1972,9 +1972,8 @@ static int musb_gadget_start(struct usb_gadget *g,
 		goto err;
 	}
 
-	if ((musb->xceiv->last_event == USB_EVENT_ID)
-				&& otg->set_vbus)
-		otg_set_vbus(otg, 1);
+	if (musb->xceiv->last_event == USB_EVENT_ID)
+		musb_platform_set_vbus(musb, 1);
 
 	hcd->self.uses_pio_for_control = 1;
 
-- 
1.7.9.5

--
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 4/7] usb: otg: twl4030-usb: poll for ID disconnect

[Michael Trimarchi]

Hi

just one comment.

On 10/03/13 02:07, Grazvydas Ignotas wrote:
> On pandora, STS_USB interrupt doesn't arrive on USB host cable disconnect
> for some reason while VBUS is driven by twl itself, but STS_HW_CONDITIONS
> is updated correctly. It does work fine when PHY is powered down though.
> To work around that we have to poll.
> 
> TI PSP kernels have similar workarounds, so (many?) more boards are likely
> affected.
> 
> Signed-off-by: Grazvydas Ignotas <notasas-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/usb/otg/twl4030-usb.c |   37 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/drivers/usb/otg/twl4030-usb.c b/drivers/usb/otg/twl4030-usb.c
> index 90a19ff..2c1c27e 100644
> --- a/drivers/usb/otg/twl4030-usb.c
> +++ b/drivers/usb/otg/twl4030-usb.c
> @@ -163,6 +163,8 @@ struct twl4030_usb {
>  	bool			vbus_supplied;
>  	u8			asleep;
>  	bool			irq_enabled;
> +
> +	struct delayed_work	id_workaround_work;
>  };
>  
>  /* internal define on top of container_of */
> @@ -412,6 +414,16 @@ static void twl4030_phy_resume(struct twl4030_usb *twl)
>  	__twl4030_phy_resume(twl);
>  	twl->asleep = 0;
>  	dev_dbg(twl->dev, "%s\n", __func__);
> +
> +	/*
> +	 * XXX When VBUS gets driven after musb goes to A mode,
> +	 * ID_PRES related interrupts no longer arrive, why?
> +	 * Register itself is updated fine though, so we must poll.
> +	 */
> +	if (twl->linkstat == OMAP_MUSB_ID_GROUND) {
> +		cancel_delayed_work(&twl->id_workaround_work);
> +		schedule_delayed_work(&twl->id_workaround_work, HZ);
> +	}
>  }
>  
>  static int twl4030_usb_ldo_init(struct twl4030_usb *twl)
> @@ -513,6 +525,28 @@ static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
>  	return IRQ_HANDLED;
>  }
>  
> +static void twl4030_id_workaround_work(struct work_struct *work)
> +{
> +	struct twl4030_usb *twl = container_of(work, struct twl4030_usb,
> +		id_workaround_work.work);
> +	enum omap_musb_vbus_id_status status_prev = twl->linkstat;
> +	enum omap_musb_vbus_id_status status;
> +
> +	status = twl4030_usb_linkstat(twl);
> +	if (status != status_prev) {
> +		dev_dbg(twl->dev, "handle missing status change: %d->%d\n",
> +			status_prev, status);
> +		twl->linkstat = status_prev;
> +		twl4030_usb_irq(0, twl);

As I understand from the subject this happen in Pandora board and
many boards can be affected.
do you need any protection between the worker and the irq when
the irq arrive as expected?

> +	}
> +
> +	/* don't schedule during sleep - irq works right then */
> +	if (status == OMAP_MUSB_ID_GROUND && !twl->asleep) {
> +		cancel_delayed_work(&twl->id_workaround_work);
> +		schedule_delayed_work(&twl->id_workaround_work, HZ);
> +	}
> +}
> +
>  static void twl4030_usb_phy_init(struct twl4030_usb *twl)
>  {
>  	enum omap_musb_vbus_id_status status;
> @@ -613,6 +647,8 @@ static int twl4030_usb_probe(struct platform_device *pdev)
>  	/* init spinlock for workqueue */
>  	spin_lock_init(&twl->lock);
>  
> +	INIT_DELAYED_WORK(&twl->id_workaround_work, twl4030_id_workaround_work);
> +
>  	err = twl4030_usb_ldo_init(twl);
>  	if (err) {
>  		dev_err(&pdev->dev, "ldo init failed\n");
> @@ -653,6 +689,7 @@ static int __exit twl4030_usb_remove(struct platform_device *pdev)
>  	struct twl4030_usb *twl = platform_get_drvdata(pdev);
>  	int val;
>  
> +	cancel_delayed_work(&twl->id_workaround_work);
>  	free_irq(twl->irq, twl);
>  	device_remove_file(twl->dev, &dev_attr_vbus);
>  
> 

Michael

--
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 4/7] usb: otg: twl4030-usb: poll for ID disconnect

[Grazvydas Ignotas]

On Sun, Mar 10, 2013 at 1:03 PM, Michael Trimarchi
<michael@amarulasolutions.com> wrote:
> Hi
>
> just one comment.
>
> On 10/03/13 02:07, Grazvydas Ignotas wrote:
>> On pandora, STS_USB interrupt doesn't arrive on USB host cable disconnect
>> for some reason while VBUS is driven by twl itself, but STS_HW_CONDITIONS
>> is updated correctly. It does work fine when PHY is powered down though.
>> To work around that we have to poll.
>>
>> TI PSP kernels have similar workarounds, so (many?) more boards are likely
>> affected.
>>
>> Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
>> ---
>>  drivers/usb/otg/twl4030-usb.c |   37 +++++++++++++++++++++++++++++++++++++
>>  1 file changed, 37 insertions(+)
>>
>> diff --git a/drivers/usb/otg/twl4030-usb.c b/drivers/usb/otg/twl4030-usb.c
>> index 90a19ff..2c1c27e 100644
>> --- a/drivers/usb/otg/twl4030-usb.c
>> +++ b/drivers/usb/otg/twl4030-usb.c
...
>> @@ -513,6 +525,28 @@ static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
>>       return IRQ_HANDLED;
>>  }
>>
>> +static void twl4030_id_workaround_work(struct work_struct *work)
>> +{
>> +     struct twl4030_usb *twl = container_of(work, struct twl4030_usb,
>> +             id_workaround_work.work);
>> +     enum omap_musb_vbus_id_status status_prev = twl->linkstat;
>> +     enum omap_musb_vbus_id_status status;
>> +
>> +     status = twl4030_usb_linkstat(twl);
>> +     if (status != status_prev) {
>> +             dev_dbg(twl->dev, "handle missing status change: %d->%d\n",
>> +                     status_prev, status);
>> +             twl->linkstat = status_prev;
>> +             twl4030_usb_irq(0, twl);
>
> As I understand from the subject this happen in Pandora board and
> many boards can be affected.
> do you need any protection between the worker and the irq when
> the irq arrive as expected?

Hmm I guess i do, I'll add some locking in v2, which I'll send after
collecting more feedback.

>
> Michael
>


-- 
Gražvydas
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/7] usb: otg: twl4030-usb: ignore duplicate events

[kishon]

Hi,

On Sunday 10 March 2013 06:37 AM, Grazvydas Ignotas wrote:
> In some rare cases we may get multiple interrupts that will generate
> duplicate omap_musb_mailbox() calls. This is a problem because each
> VBUS/ID event generates runtime_pm call in OMAP glue code, causing
> unbalanced gets or puts and breaking PM.

Did you actually observed multiple interrupts? Actually we thought 
debouncing should be handled in hardware.
>
> The same goes for initial state, glue already defaults to "no cable"
> state, so only bother it if we have VBUS or ID.
>
> Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
Reviewed-by: Kishon Vijay Abraham I <kishon@ti.com>

Thanks
Kishon

Re: [PATCH 6/7] usb: musb: omap2430: turn off vbus on cable disconnect

[kishon]

Hi,

On Sunday 10 March 2013 06:38 AM, Grazvydas Ignotas wrote:
> On USB_EVENT_ID event the musb glue enables VBUS by calling
> omap2430_musb_set_vbus(musb, 1) that sets the session bit, but on
> USB_EVENT_NONE reverse action is never made, and that breaks PM.
>
> Disable VBUS unconditionally on USB_EVENT_NONE to be sure musb
> session is ended on cable unplug so that PM works.
>
> Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
> ---
>   drivers/usb/musb/omap2430.c |    1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
> index 2a39c11..d430677 100644
> --- a/drivers/usb/musb/omap2430.c
> +++ b/drivers/usb/musb/omap2430.c
> @@ -296,6 +296,7 @@ static void omap_musb_set_mailbox(struct omap2430_glue *glue)
>   			pm_runtime_put_autosuspend(dev);
>   		}
>
> +		omap2430_musb_set_vbus(musb, 0);
>   		if (data->interface_type == MUSB_INTERFACE_UTMI) {
>   			if (musb->xceiv->otg->set_vbus)
>   				otg_set_vbus(musb->xceiv->otg, 0);
Shouldn't this otg_set_vbus be done inside omap2430_musb_set_vbus? Any 
idea why they were doing this only for UTMI?

Thanks
Kishon

Re: [PATCH 2/7] usb: otg: twl4030-usb: ignore duplicate events

[Grazvydas Ignotas]

On Tue, Mar 12, 2013 at 3:32 PM, kishon <kishon@ti.com> wrote:
> Hi,
>
>
> On Sunday 10 March 2013 06:37 AM, Grazvydas Ignotas wrote:
>>
>> In some rare cases we may get multiple interrupts that will generate
>> duplicate omap_musb_mailbox() calls. This is a problem because each
>> VBUS/ID event generates runtime_pm call in OMAP glue code, causing
>> unbalanced gets or puts and breaking PM.
>
>
> Did you actually observed multiple interrupts? Actually we thought
> debouncing should be handled in hardware.

I think I saw it a few times, although it might have been something
else. In any case the software check is very cheap and there is no
reason not to have it, IMO.

>
>>
>> The same goes for initial state, glue already defaults to "no cable"
>> state, so only bother it if we have VBUS or ID.
>>
>> Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
>
> Reviewed-by: Kishon Vijay Abraham I <kishon@ti.com>
>
> Thanks
> Kishon

-- 
Gražvydas
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/7] usb: musb: omap2430: turn off vbus on cable disconnect

[Grazvydas Ignotas]

On Tue, Mar 12, 2013 at 3:37 PM, kishon <kishon-l0cyMroinI0@public.gmane.org> wrote:
> Hi,
>
>
> On Sunday 10 March 2013 06:38 AM, Grazvydas Ignotas wrote:
>>
>> On USB_EVENT_ID event the musb glue enables VBUS by calling
>> omap2430_musb_set_vbus(musb, 1) that sets the session bit, but on
>> USB_EVENT_NONE reverse action is never made, and that breaks PM.
>>
>> Disable VBUS unconditionally on USB_EVENT_NONE to be sure musb
>> session is ended on cable unplug so that PM works.
>>
>> Signed-off-by: Grazvydas Ignotas <notasas-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>   drivers/usb/musb/omap2430.c |    1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
>> index 2a39c11..d430677 100644
>> --- a/drivers/usb/musb/omap2430.c
>> +++ b/drivers/usb/musb/omap2430.c
>> @@ -296,6 +296,7 @@ static void omap_musb_set_mailbox(struct omap2430_glue
>> *glue)
>>                         pm_runtime_put_autosuspend(dev);
>>                 }
>>
>> +               omap2430_musb_set_vbus(musb, 0);
>>                 if (data->interface_type == MUSB_INTERFACE_UTMI) {
>>                         if (musb->xceiv->otg->set_vbus)
>>                                 otg_set_vbus(musb->xceiv->otg, 0);
>
> Shouldn't this otg_set_vbus be done inside omap2430_musb_set_vbus? Any idea
> why they were doing this only for UTMI?

I would think so too, there is otg_set_vbus() in
omap2430_musb_set_vbus() for enable but not for disable.
I don't know history of this code and didn't want to break existing
functionality.

>
> Thanks
> Kishon



-- 
Gražvydas
--
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