diff mbox

[2.6.29-rc3-git,2/2] USB: disable twl4030 USB regulators when cable unplugged

Message ID 200902081053.00255.david-b@pacbell.net
State Changes Requested, archived
Delegated to: Felipe Balbi
Headers show

Commit Message

David Brownell Feb. 8, 2009, 6:52 p.m. UTC
From: Kalle Jokiniemi <kalle.jokiniemi@digia.com>

This patch disables LDO regulators VUSB1V5, VUSB1V8, and VUSB3V1
when the USB cable is unplugged, to eliminate that source of power
waste.  (Enabled LDOs consume power at all times.)

Signed-off-by: Kalle Jokiniemi <kalle.jokiniemi@digia.com>
Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
---
Depends on the twl4030 regulator driver, so I'm suggesting this
be merged (with that driver) through the regulator patch queue
to simplify things.

 drivers/usb/otg/twl4030-usb.c |   30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

--
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

Comments

Kalle Jokiniemi Feb. 10, 2009, 1:08 p.m. UTC | #1
On Sun, 2009-02-08 at 10:52 -0800, David Brownell wrote:
> From: Kalle Jokiniemi <kalle.jokiniemi@digia.com>
> 
> This patch disables LDO regulators VUSB1V5, VUSB1V8, and VUSB3V1
> when the USB cable is unplugged, to eliminate that source of power
> waste.  (Enabled LDOs consume power at all times.)

Please put this one on hold.

A bug has been discovered here. If the twl-regulator is not compiled in,
the twl4030-usb fails while trying to access the reg-fw interfaces (and
not checking error values). Also it's bit confusing how we get different
error values if reg_fw is not compiled (no error) vs. reg_fw is
compiled, but twl_regulator is not... (error)

Also problems with boot order were discovered.

I'll post a set that fixes this soon.

- Kalle


> 
> Signed-off-by: Kalle Jokiniemi <kalle.jokiniemi@digia.com>
> Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
> ---
> Depends on the twl4030 regulator driver, so I'm suggesting this
> be merged (with that driver) through the regulator patch queue
> to simplify things.
> 
>  drivers/usb/otg/twl4030-usb.c |   30 ++++++++++++++++++++++++------
>  1 file changed, 24 insertions(+), 6 deletions(-)
> 
> --- a/drivers/usb/otg/twl4030-usb.c
> +++ b/drivers/usb/otg/twl4030-usb.c
> @@ -34,6 +34,7 @@
>  #include <linux/delay.h>
>  #include <linux/usb/otg.h>
>  #include <linux/i2c/twl4030.h>
> +#include <linux/regulator/consumer.h>
>  
> 
>  /* Register defines */
> @@ -246,6 +247,11 @@ struct twl4030_usb {
>  	struct otg_transceiver	otg;
>  	struct device		*dev;
>  
> +	/* TWL4030 internal USB regulator supplies */
> +	struct regulator	*usb1v5;
> +	struct regulator	*usb1v8;
> +	struct regulator	*usb3v1;
> +
>  	/* for vbus reporting with irqs disabled */
>  	spinlock_t		lock;
>  
> @@ -434,6 +440,9 @@ static void twl4030_phy_power(struct twl
>  
>  	pwr = twl4030_usb_read(twl, PHY_PWR_CTRL);
>  	if (on) {
> +		regulator_enable(twl->usb1v5);
> +		regulator_enable(twl->usb1v8);
> +		regulator_enable(twl->usb3v1);
>  		pwr &= ~PHY_PWR_PHYPWD;
>  		WARN_ON(twl4030_usb_write_verify(twl, PHY_PWR_CTRL, pwr) < 0);
>  		twl4030_usb_write(twl, PHY_CLK_CTRL,
> @@ -443,6 +452,9 @@ static void twl4030_phy_power(struct twl
>  	} else  {
>  		pwr |= PHY_PWR_PHYPWD;
>  		WARN_ON(twl4030_usb_write_verify(twl, PHY_PWR_CTRL, pwr) < 0);
> +		regulator_disable(twl->usb1v5);
> +		regulator_disable(twl->usb1v8);
> +		regulator_disable(twl->usb3v1);
>  	}
>  }
>  
> @@ -480,16 +492,19 @@ static void twl4030_usb_ldo_init(struct 
>  	/* input to VUSB3V1 LDO is from VBAT, not VBUS */
>  	twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, 0x14, VUSB_DEDICATED1);
>  
> -	/* turn on 3.1V regulator */
> -	twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, 0x20, VUSB3V1_DEV_GRP);
> +	/* Initialize 3.1V regulator */
> +	twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, 0, VUSB3V1_DEV_GRP);
> +	twl->usb3v1 = regulator_get(twl->dev, "usb3v1");
>  	twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, 0, VUSB3V1_TYPE);
>  
> -	/* turn on 1.5V regulator */
> -	twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, 0x20, VUSB1V5_DEV_GRP);
> +	/* Initialize 1.5V regulator */
> +	twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, 0, VUSB1V5_DEV_GRP);
> +	twl->usb1v5 = regulator_get(twl->dev, "usb1v5");
>  	twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, 0, VUSB1V5_TYPE);
>  
> -	/* turn on 1.8V regulator */
> -	twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, 0x20, VUSB1V8_DEV_GRP);
> +	/* Initialize 1.8V regulator */
> +	twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, 0, VUSB1V8_DEV_GRP);
> +	twl->usb1v8 = regulator_get(twl->dev, "usb1v8");
>  	twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, 0, VUSB1V8_TYPE);
>  
>  	/* disable access to power configuration registers */
> @@ -688,6 +703,9 @@ static int __exit twl4030_usb_remove(str
>  	twl4030_usb_clear_bits(twl, POWER_CTRL, POWER_CTRL_OTG_ENAB);
>  
>  	twl4030_phy_power(twl, 0);
> +	regulator_put(twl->usb1v5);
> +	regulator_put(twl->usb1v8);
> +	regulator_put(twl->usb3v1);
>  
>  	kfree(twl);
>  
--
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
Kalle Jokiniemi Feb. 10, 2009, 2:06 p.m. UTC | #2
On Sun, 2009-02-08 at 10:52 -0800, David Brownell wrote:
> From: Kalle Jokiniemi <kalle.jokiniemi@digia.com>
> 
> This patch disables LDO regulators VUSB1V5, VUSB1V8, and VUSB3V1
> when the USB cable is unplugged, to eliminate that source of power
> waste.  (Enabled LDOs consume power at all times.)

Please put this on hold for a while. This won't work if we don't have
TWL regulator support enabled. Also it's missing error value checking.

I'll post new patch(es) soon.

- Kalle


> 
> Signed-off-by: Kalle Jokiniemi <kalle.jokiniemi@digia.com>
> Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
> ---
> Depends on the twl4030 regulator driver, so I'm suggesting this
> be merged (with that driver) through the regulator patch queue
> to simplify things.
> 
>  drivers/usb/otg/twl4030-usb.c |   30 ++++++++++++++++++++++++------
>  1 file changed, 24 insertions(+), 6 deletions(-)
> 
> --- a/drivers/usb/otg/twl4030-usb.c
> +++ b/drivers/usb/otg/twl4030-usb.c
> @@ -34,6 +34,7 @@
>  #include <linux/delay.h>
>  #include <linux/usb/otg.h>
>  #include <linux/i2c/twl4030.h>
> +#include <linux/regulator/consumer.h>
>  
> 
>  /* Register defines */
> @@ -246,6 +247,11 @@ struct twl4030_usb {
>  	struct otg_transceiver	otg;
>  	struct device		*dev;
>  
> +	/* TWL4030 internal USB regulator supplies */
> +	struct regulator	*usb1v5;
> +	struct regulator	*usb1v8;
> +	struct regulator	*usb3v1;
> +
>  	/* for vbus reporting with irqs disabled */
>  	spinlock_t		lock;
>  
> @@ -434,6 +440,9 @@ static void twl4030_phy_power(struct twl
>  
>  	pwr = twl4030_usb_read(twl, PHY_PWR_CTRL);
>  	if (on) {
> +		regulator_enable(twl->usb1v5);
> +		regulator_enable(twl->usb1v8);
> +		regulator_enable(twl->usb3v1);
>  		pwr &= ~PHY_PWR_PHYPWD;
>  		WARN_ON(twl4030_usb_write_verify(twl, PHY_PWR_CTRL, pwr) < 0);
>  		twl4030_usb_write(twl, PHY_CLK_CTRL,
> @@ -443,6 +452,9 @@ static void twl4030_phy_power(struct twl
>  	} else  {
>  		pwr |= PHY_PWR_PHYPWD;
>  		WARN_ON(twl4030_usb_write_verify(twl, PHY_PWR_CTRL, pwr) < 0);
> +		regulator_disable(twl->usb1v5);
> +		regulator_disable(twl->usb1v8);
> +		regulator_disable(twl->usb3v1);
>  	}
>  }
>  
> @@ -480,16 +492,19 @@ static void twl4030_usb_ldo_init(struct 
>  	/* input to VUSB3V1 LDO is from VBAT, not VBUS */
>  	twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, 0x14, VUSB_DEDICATED1);
>  
> -	/* turn on 3.1V regulator */
> -	twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, 0x20, VUSB3V1_DEV_GRP);
> +	/* Initialize 3.1V regulator */
> +	twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, 0, VUSB3V1_DEV_GRP);
> +	twl->usb3v1 = regulator_get(twl->dev, "usb3v1");
>  	twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, 0, VUSB3V1_TYPE);
>  
> -	/* turn on 1.5V regulator */
> -	twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, 0x20, VUSB1V5_DEV_GRP);
> +	/* Initialize 1.5V regulator */
> +	twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, 0, VUSB1V5_DEV_GRP);
> +	twl->usb1v5 = regulator_get(twl->dev, "usb1v5");
>  	twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, 0, VUSB1V5_TYPE);
>  
> -	/* turn on 1.8V regulator */
> -	twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, 0x20, VUSB1V8_DEV_GRP);
> +	/* Initialize 1.8V regulator */
> +	twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, 0, VUSB1V8_DEV_GRP);
> +	twl->usb1v8 = regulator_get(twl->dev, "usb1v8");
>  	twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, 0, VUSB1V8_TYPE);
>  
>  	/* disable access to power configuration registers */
> @@ -688,6 +703,9 @@ static int __exit twl4030_usb_remove(str
>  	twl4030_usb_clear_bits(twl, POWER_CTRL, POWER_CTRL_OTG_ENAB);
>  
>  	twl4030_phy_power(twl, 0);
> +	regulator_put(twl->usb1v5);
> +	regulator_put(twl->usb1v8);
> +	regulator_put(twl->usb3v1);
>  
>  	kfree(twl);
>  
--
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
Mark Brown Feb. 10, 2009, 3:19 p.m. UTC | #3
On Tue, Feb 10, 2009 at 03:08:14PM +0200, Kalle Jokiniemi wrote:

> A bug has been discovered here. If the twl-regulator is not compiled in,
> the twl4030-usb fails while trying to access the reg-fw interfaces (and

reg-fw is the regulator framework?

> not checking error values). Also it's bit confusing how we get different
> error values if reg_fw is not compiled (no error) vs. reg_fw is
> compiled, but twl_regulator is not... (error)

The general model is that if you've got regulator API support enabled
then you'll have the relevant regulator drivers enabled.  The support
for compiling out the core is intended to allow configurations that
don't use regulators at all.  This is a bit restrictive but works well
for most users.
--
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
David Brownell Feb. 10, 2009, 7:52 p.m. UTC | #4
On Tuesday 10 February 2009, Kalle Jokiniemi wrote:
> > This patch disables LDO regulators VUSB1V5, VUSB1V8, and VUSB3V1
> > when the USB cable is unplugged, to eliminate that source of power
> > waste.  (Enabled LDOs consume power at all times.)
> 
> Please put this on hold for a while. This won't work if we don't have
> TWL regulator support enabled.

So Kconfig should have TWL4030_USB depend on REGULATOR_TWL4030?


> Also it's missing error value checking. 

You mean like in the ldo_init() stuff?  Fair enough.

- Dave


> I'll post new patch(es) soon.
> 

--
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
Kalle Jokiniemi Feb. 11, 2009, 6:25 a.m. UTC | #5
On Tue, 2009-02-10 at 11:52 -0800, David Brownell wrote:
> On Tuesday 10 February 2009, Kalle Jokiniemi wrote:
> > > This patch disables LDO regulators VUSB1V5, VUSB1V8, and VUSB3V1
> > > when the USB cable is unplugged, to eliminate that source of power
> > > waste.  (Enabled LDOs consume power at all times.)
> > 
> > Please put this on hold for a while. This won't work if we don't have
> > TWL regulator support enabled.
> 
> So Kconfig should have TWL4030_USB depend on REGULATOR_TWL4030?

Yes.

> 
> 
> > Also it's missing error value checking. 
> 
> You mean like in the ldo_init() stuff?  Fair enough.

I mean, if regulator_get fails. 

- Kalle

> 
> - Dave
> 
> 
> > I'll post new patch(es) soon.
> > 
> 
--
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
Liam Girdwood Feb. 26, 2009, 10:16 p.m. UTC | #6
On Sun, 2009-02-08 at 10:52 -0800, David Brownell wrote:
> From: Kalle Jokiniemi <kalle.jokiniemi@digia.com>
> 
> This patch disables LDO regulators VUSB1V5, VUSB1V8, and VUSB3V1
> when the USB cable is unplugged, to eliminate that source of power
> waste.  (Enabled LDOs consume power at all times.)
> 
> Signed-off-by: Kalle Jokiniemi <kalle.jokiniemi@digia.com>
> Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
> ---
> Depends on the twl4030 regulator driver, so I'm suggesting this
> be merged (with that driver) through the regulator patch queue
> to simplify things.
> 
>  drivers/usb/otg/twl4030-usb.c |   30 ++++++++++++++++++++++++------
>  1 file changed, 24 insertions(+), 6 deletions(-)

Applied.

Thanks

Liam

--
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
David Brownell Feb. 26, 2009, 10:40 p.m. UTC | #7
On Thursday 26 February 2009, Liam Girdwood wrote:
> On Sun, 2009-02-08 at 10:52 -0800, David Brownell wrote:
> > From: Kalle Jokiniemi <kalle.jokiniemi@digia.com>
> > 
> > This patch disables LDO regulators VUSB1V5, VUSB1V8, and VUSB3V1
> > when the USB cable is unplugged, to eliminate that source of power
> > waste.  (Enabled LDOs consume power at all times.)
> > 
> > Signed-off-by: Kalle Jokiniemi <kalle.jokiniemi@digia.com>
> > Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
> > ---
> > Depends on the twl4030 regulator driver, so I'm suggesting this
> > be merged (with that driver) through the regulator patch queue
> > to simplify things.
> > 
> >  drivers/usb/otg/twl4030-usb.c |   30 ++++++++++++++++++++++++------
> >  1 file changed, 24 insertions(+), 6 deletions(-)
> 
> Applied.

Better suggestion:  grab Kalle's updated patch from Greg's USB
queue:

  http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/gregkh-04-usb/

patch name usb-twl-disable-vusb-regulators-when-cable-unplugged.patch

- Dave
--
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
Kalle Jokiniemi Feb. 27, 2009, 6:19 a.m. UTC | #8
On Thu, 2009-02-26 at 14:40 -0800, David Brownell wrote:
> On Thursday 26 February 2009, Liam Girdwood wrote:
> > On Sun, 2009-02-08 at 10:52 -0800, David Brownell wrote:
> > > From: Kalle Jokiniemi <kalle.jokiniemi@digia.com>
> > > 
> > > This patch disables LDO regulators VUSB1V5, VUSB1V8, and VUSB3V1
> > > when the USB cable is unplugged, to eliminate that source of power
> > > waste.  (Enabled LDOs consume power at all times.)
> > > 
> > > Signed-off-by: Kalle Jokiniemi <kalle.jokiniemi@digia.com>
> > > Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
> > > ---
> > > Depends on the twl4030 regulator driver, so I'm suggesting this
> > > be merged (with that driver) through the regulator patch queue
> > > to simplify things.
> > > 
> > >  drivers/usb/otg/twl4030-usb.c |   30 ++++++++++++++++++++++++------
> > >  1 file changed, 24 insertions(+), 6 deletions(-)
> > 
> > Applied.
> 
> Better suggestion:  grab Kalle's updated patch from Greg's USB
> queue:
> 
>   http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/gregkh-04-usb/
> 
> patch name usb-twl-disable-vusb-regulators-when-cable-unplugged.patch


Yep, this previous one lacked some error checks and a build dependency
to TWL_REGULATOR. 

- Kalle


> 
> - Dave
--
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
Liam Girdwood Feb. 27, 2009, 12:29 p.m. UTC | #9
On Fri, 2009-02-27 at 08:19 +0200, Kalle Jokiniemi wrote:
> On Thu, 2009-02-26 at 14:40 -0800, David Brownell wrote:
> > On Thursday 26 February 2009, Liam Girdwood wrote:
> > > On Sun, 2009-02-08 at 10:52 -0800, David Brownell wrote:
> > > > From: Kalle Jokiniemi <kalle.jokiniemi@digia.com>
> > > > 
> > > > This patch disables LDO regulators VUSB1V5, VUSB1V8, and VUSB3V1
> > > > when the USB cable is unplugged, to eliminate that source of power
> > > > waste.  (Enabled LDOs consume power at all times.)
> > > > 
> > > > Signed-off-by: Kalle Jokiniemi <kalle.jokiniemi@digia.com>
> > > > Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
> > > > ---
> > > > Depends on the twl4030 regulator driver, so I'm suggesting this
> > > > be merged (with that driver) through the regulator patch queue
> > > > to simplify things.
> > > > 
> > > >  drivers/usb/otg/twl4030-usb.c |   30 ++++++++++++++++++++++++------
> > > >  1 file changed, 24 insertions(+), 6 deletions(-)
> > > 
> > > Applied.
> > 
> > Better suggestion:  grab Kalle's updated patch from Greg's USB
> > queue:
> > 
> >   http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/gregkh-04-usb/
> > 
> > patch name usb-twl-disable-vusb-regulators-when-cable-unplugged.patch
> 
> 
> Yep, this previous one lacked some error checks and a build dependency
> to TWL_REGULATOR. 

I've now removed this patch from the regulator tree. I guess Greg will
take care of it via the USB tree now.

Thanks

Liam

--
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

--- a/drivers/usb/otg/twl4030-usb.c
+++ b/drivers/usb/otg/twl4030-usb.c
@@ -34,6 +34,7 @@ 
 #include <linux/delay.h>
 #include <linux/usb/otg.h>
 #include <linux/i2c/twl4030.h>
+#include <linux/regulator/consumer.h>
 
 
 /* Register defines */
@@ -246,6 +247,11 @@  struct twl4030_usb {
 	struct otg_transceiver	otg;
 	struct device		*dev;
 
+	/* TWL4030 internal USB regulator supplies */
+	struct regulator	*usb1v5;
+	struct regulator	*usb1v8;
+	struct regulator	*usb3v1;
+
 	/* for vbus reporting with irqs disabled */
 	spinlock_t		lock;
 
@@ -434,6 +440,9 @@  static void twl4030_phy_power(struct twl
 
 	pwr = twl4030_usb_read(twl, PHY_PWR_CTRL);
 	if (on) {
+		regulator_enable(twl->usb1v5);
+		regulator_enable(twl->usb1v8);
+		regulator_enable(twl->usb3v1);
 		pwr &= ~PHY_PWR_PHYPWD;
 		WARN_ON(twl4030_usb_write_verify(twl, PHY_PWR_CTRL, pwr) < 0);
 		twl4030_usb_write(twl, PHY_CLK_CTRL,
@@ -443,6 +452,9 @@  static void twl4030_phy_power(struct twl
 	} else  {
 		pwr |= PHY_PWR_PHYPWD;
 		WARN_ON(twl4030_usb_write_verify(twl, PHY_PWR_CTRL, pwr) < 0);
+		regulator_disable(twl->usb1v5);
+		regulator_disable(twl->usb1v8);
+		regulator_disable(twl->usb3v1);
 	}
 }
 
@@ -480,16 +492,19 @@  static void twl4030_usb_ldo_init(struct 
 	/* input to VUSB3V1 LDO is from VBAT, not VBUS */
 	twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, 0x14, VUSB_DEDICATED1);
 
-	/* turn on 3.1V regulator */
-	twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, 0x20, VUSB3V1_DEV_GRP);
+	/* Initialize 3.1V regulator */
+	twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, 0, VUSB3V1_DEV_GRP);
+	twl->usb3v1 = regulator_get(twl->dev, "usb3v1");
 	twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, 0, VUSB3V1_TYPE);
 
-	/* turn on 1.5V regulator */
-	twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, 0x20, VUSB1V5_DEV_GRP);
+	/* Initialize 1.5V regulator */
+	twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, 0, VUSB1V5_DEV_GRP);
+	twl->usb1v5 = regulator_get(twl->dev, "usb1v5");
 	twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, 0, VUSB1V5_TYPE);
 
-	/* turn on 1.8V regulator */
-	twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, 0x20, VUSB1V8_DEV_GRP);
+	/* Initialize 1.8V regulator */
+	twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, 0, VUSB1V8_DEV_GRP);
+	twl->usb1v8 = regulator_get(twl->dev, "usb1v8");
 	twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, 0, VUSB1V8_TYPE);
 
 	/* disable access to power configuration registers */
@@ -688,6 +703,9 @@  static int __exit twl4030_usb_remove(str
 	twl4030_usb_clear_bits(twl, POWER_CTRL, POWER_CTRL_OTG_ENAB);
 
 	twl4030_phy_power(twl, 0);
+	regulator_put(twl->usb1v5);
+	regulator_put(twl->usb1v8);
+	regulator_put(twl->usb3v1);
 
 	kfree(twl);