diff mbox

phy-twl4030-usb: initialize charging-related stuff via pm_runtime

Message ID 1471896551-13292-1-git-send-email-andreas@kemnade.info (mailing list archive)
State New, archived
Headers show

Commit Message

Andreas Kemnade Aug. 22, 2016, 8:09 p.m. UTC
twl4030_phy_power_on() initializes some bits which are required for
charging. As they are not set in twl4030_usb_runtime_resume()
a call to pm_runtime_get_sync() is not sufficient to enable charging.

This patch moves the initialization to twl4030_usb_runtime_resume()
so everything needed for charging is initialized upon
pm_runtime_get_sync().

That also gives improved possibilities to debug problems in that area
because the relevant parts can be checked separately. Charging can be
enabled without having the musb subsystem active.

As a side effect this hides some bugs in musb which causes
unbalanced calls to phy_power_off()/phy_power_on() so that
phy->power_count becomes -1.

The result is that e.g. the GTA04 phone (dm3730 + twl4030) works
finally as a usb gadget again and charging is working.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
I am not 100% sure that this patch works properly in all situations
without the bugfix
phy-twl4030-usb: better handle musb_mailbox() failure

 drivers/phy/phy-twl4030-usb.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Tony Lindgren Aug. 23, 2016, 10:59 p.m. UTC | #1
* Andreas Kemnade <andreas@kemnade.info> [160822 13:10]:
> --- a/drivers/phy/phy-twl4030-usb.c
> +++ b/drivers/phy/phy-twl4030-usb.c
> @@ -469,6 +469,11 @@ static int __maybe_unused twl4030_usb_runtime_resume(struct device *dev)
>  			  (PHY_CLK_CTRL_CLOCKGATING_EN |
>  			   PHY_CLK_CTRL_CLK32K_EN));
>  
> +	twl4030_i2c_access(twl, 1);
> +	twl4030_usb_set_mode(twl, twl->usb_mode);
> +	if (twl->usb_mode == T2_USB_MODE_ULPI)
> +		twl4030_i2c_access(twl, 0);
> +	mdelay(50);
>  	return 0;
>  }
>  

Can you check for some register instead of the mdelay?

Other than that, works for me for my tests.

Regards,

Tony
--
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
Andreas Kemnade Aug. 24, 2016, 8:04 a.m. UTC | #2
On Tue, 23 Aug 2016 15:59:06 -0700
Tony Lindgren <tony@atomide.com> wrote:

> * Andreas Kemnade <andreas@kemnade.info> [160822 13:10]:
> > --- a/drivers/phy/phy-twl4030-usb.c
> > +++ b/drivers/phy/phy-twl4030-usb.c
> > @@ -469,6 +469,11 @@ static int __maybe_unused
> > twl4030_usb_runtime_resume(struct device *dev)
> > (PHY_CLK_CTRL_CLOCKGATING_EN | PHY_CLK_CTRL_CLK32K_EN));
> >  
> > +	twl4030_i2c_access(twl, 1);
> > +	twl4030_usb_set_mode(twl, twl->usb_mode);
> > +	if (twl->usb_mode == T2_USB_MODE_ULPI)
> > +		twl4030_i2c_access(twl, 0);
> > +	mdelay(50);
> >  	return 0;
> >  }
> >  
> 
> Can you check for some register instead of the mdelay?
> 
I am quoting the TPS65950 TRM here (page 443):

> "The software must set the POWER_CTRL[5] OTG_EN bit to 1 at least 50
> ms before forcing the BCIMFSTS4[2] USBFASTMCHG bit to 1."

So the answer is: no

Regards,
Andreas
--
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
Tony Lindgren Aug. 24, 2016, 2:46 p.m. UTC | #3
* Andreas Kemnade <andreas@kemnade.info> [160824 01:05]:
> On Tue, 23 Aug 2016 15:59:06 -0700
> Tony Lindgren <tony@atomide.com> wrote:
> 
> > * Andreas Kemnade <andreas@kemnade.info> [160822 13:10]:
> > > --- a/drivers/phy/phy-twl4030-usb.c
> > > +++ b/drivers/phy/phy-twl4030-usb.c
> > > @@ -469,6 +469,11 @@ static int __maybe_unused
> > > twl4030_usb_runtime_resume(struct device *dev)
> > > (PHY_CLK_CTRL_CLOCKGATING_EN | PHY_CLK_CTRL_CLK32K_EN));
> > >  
> > > +	twl4030_i2c_access(twl, 1);
> > > +	twl4030_usb_set_mode(twl, twl->usb_mode);
> > > +	if (twl->usb_mode == T2_USB_MODE_ULPI)
> > > +		twl4030_i2c_access(twl, 0);
> > > +	mdelay(50);
> > >  	return 0;
> > >  }
> > >  
> > 
> > Can you check for some register instead of the mdelay?
> > 
> I am quoting the TPS65950 TRM here (page 443):
> 
> > "The software must set the POWER_CTRL[5] OTG_EN bit to 1 at least 50
> > ms before forcing the BCIMFSTS4[2] USBFASTMCHG bit to 1."
> 
> So the answer is: no

Oh OK, can you please add a comment for that to the code for
the delay?

Also, since we're using threaded irq msleep() should work here
instead of mdelay()?

Regards,

Tony
--
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
diff mbox

Patch

diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
index 536480c..ed98cb6 100644
--- a/drivers/phy/phy-twl4030-usb.c
+++ b/drivers/phy/phy-twl4030-usb.c
@@ -469,6 +469,11 @@  static int __maybe_unused twl4030_usb_runtime_resume(struct device *dev)
 			  (PHY_CLK_CTRL_CLOCKGATING_EN |
 			   PHY_CLK_CTRL_CLK32K_EN));
 
+	twl4030_i2c_access(twl, 1);
+	twl4030_usb_set_mode(twl, twl->usb_mode);
+	if (twl->usb_mode == T2_USB_MODE_ULPI)
+		twl4030_i2c_access(twl, 0);
+	mdelay(50);
 	return 0;
 }
 
@@ -489,11 +494,6 @@  static int twl4030_phy_power_on(struct phy *phy)
 
 	dev_dbg(twl->dev, "%s\n", __func__);
 	pm_runtime_get_sync(twl->dev);
-	twl4030_i2c_access(twl, 1);
-	twl4030_usb_set_mode(twl, twl->usb_mode);
-	if (twl->usb_mode == T2_USB_MODE_ULPI)
-		twl4030_i2c_access(twl, 0);
-	twl->linkstat = MUSB_UNKNOWN;
 	schedule_delayed_work(&twl->id_workaround_work, HZ);
 
 	return 0;