[PATCH 0/3] check regulator_enable() return value

[PATCH 0/3] check regulator_enable() return value

[Fabio Baltieri]

Hello Felipe,

While testing your 'next' branch merged with today's next I got some new
warnings, caused by a recently introduced __must_check in:

c8801a8 regulator: core: Mark all get and enable calls as __must_check

These patches introduces a check for regulator_enable() return value in
all three affected USB phy drivers, and issue a dev_err() in case it
fails.

TWL4030 and TWL6030 patches has been build-tested only.

Thanks,
Fabio


Fabio Baltieri (3):
  usb: phy: ab8500-usb: check regulator_enable return value
  usb: phy: twl4030-usb: check regulator_enable return value
  usb: phy: twl6030-usb: check regulator_enable return value

 drivers/usb/phy/phy-ab8500-usb.c  | 12 +++++++++---
 drivers/usb/phy/phy-twl4030-usb.c | 18 +++++++++++++++---
 drivers/usb/phy/phy-twl6030-usb.c | 11 +++++++++--
 3 files changed, 33 insertions(+), 8 deletions(-)

-- 
1.8.1.3

[PATCH 1/3] usb: phy: ab8500-usb: check regulator_enable return value

[Fabio Baltieri]

Since regulator_enable() is going to be marked as __must_check in the
next merge window, always check regulator_enable() return value and
print a warning if it fails.

Cc: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Fabio Baltieri <fabio.baltieri@linaro.org>
---
 drivers/usb/phy/phy-ab8500-usb.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/phy/phy-ab8500-usb.c b/drivers/usb/phy/phy-ab8500-usb.c
index b8115b0..ac27ed2 100644
--- a/drivers/usb/phy/phy-ab8500-usb.c
+++ b/drivers/usb/phy/phy-ab8500-usb.c
@@ -170,7 +170,9 @@ static void ab8500_usb_regulator_enable(struct ab8500_usb *ab)
 {
 	int ret, volt;
 
-	regulator_enable(ab->v_ape);
+	ret = regulator_enable(ab->v_ape);
+	if (ret)
+		dev_err(ab->dev, "Failed to enable v-ape\n");
 
 	if (!is_ab8500_2p0_or_earlier(ab->ab8500)) {
 		ab->saved_v_ulpi = regulator_get_voltage(ab->v_ulpi);
@@ -188,7 +190,9 @@ static void ab8500_usb_regulator_enable(struct ab8500_usb *ab)
 					ret);
 	}
 
-	regulator_enable(ab->v_ulpi);
+	ret = regulator_enable(ab->v_ulpi);
+	if (ret)
+		dev_err(ab->dev, "Failed to enable vddulpivio18\n");
 
 	if (!is_ab8500_2p0_or_earlier(ab->ab8500)) {
 		volt = regulator_get_voltage(ab->v_ulpi);
@@ -197,7 +201,9 @@ static void ab8500_usb_regulator_enable(struct ab8500_usb *ab)
 					volt);
 	}
 
-	regulator_enable(ab->v_musb);
+	ret = regulator_enable(ab->v_musb);
+	if (ret)
+		dev_err(ab->dev, "Failed to enable musb_1v8\n");
 }
 
 static void ab8500_usb_regulator_disable(struct ab8500_usb *ab)
-- 
1.8.1.3

[PATCH 2/3] usb: phy: twl4030-usb: check regulator_enable return value

[Fabio Baltieri]

Since regulator_enable() is going to be marked as __must_check in the
next merge window, always check regulator_enable() return value and
print a warning if it fails.

Cc: Kalle Jokiniemi <kalle.jokiniemi@jollamobile.com>
Signed-off-by: Fabio Baltieri <fabio.baltieri@linaro.org>
---
 drivers/usb/phy/phy-twl4030-usb.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/phy/phy-twl4030-usb.c b/drivers/usb/phy/phy-twl4030-usb.c
index 3f9858f..13e17ae 100644
--- a/drivers/usb/phy/phy-twl4030-usb.c
+++ b/drivers/usb/phy/phy-twl4030-usb.c
@@ -384,9 +384,17 @@ static void __twl4030_phy_power(struct twl4030_usb *twl, int on)
 
 static void twl4030_phy_power(struct twl4030_usb *twl, int on)
 {
+	int ret;
+
 	if (on) {
-		regulator_enable(twl->usb3v1);
-		regulator_enable(twl->usb1v8);
+		ret = regulator_enable(twl->usb3v1);
+		if (ret)
+			dev_err(twl->dev, "Failed to enable usb3v1\n");
+
+		ret = regulator_enable(twl->usb1v8);
+		if (ret)
+			dev_err(twl->dev, "Failed to enable usb1v8\n");
+
 		/*
 		 * Disabling usb3v1 regulator (= writing 0 to VUSB3V1_DEV_GRP
 		 * in twl4030) resets the VUSB_DEDICATED2 register. This reset
@@ -395,7 +403,11 @@ static void twl4030_phy_power(struct twl4030_usb *twl, int on)
 		 * is re-activated. This ensures that VUSB3V1 is really active.
 		 */
 		twl_i2c_write_u8(TWL_MODULE_PM_RECEIVER, 0, VUSB_DEDICATED2);
-		regulator_enable(twl->usb1v5);
+
+		ret = regulator_enable(twl->usb1v5);
+		if (ret)
+			dev_err(twl->dev, "Failed to enable usb1v5\n");
+
 		__twl4030_phy_power(twl, 1);
 		twl4030_usb_write(twl, PHY_CLK_CTRL,
 				  twl4030_usb_read(twl, PHY_CLK_CTRL) |
-- 
1.8.1.3

Re: [PATCH 2/3] usb: phy: twl4030-usb: check regulator_enable return value

[Kalle Jokiniemi]

On Wed, 2013-04-03 at 16:02 +0200, Fabio Baltieri wrote:
> Since regulator_enable() is going to be marked as __must_check in the
> next merge window, always check regulator_enable() return value and
> print a warning if it fails.

Could go bananas with this and all kinds of recovery schemes, but nah..
Looks good to me. FWIW,

Reviewed-by: Kalle Jokiniemi <kalle.jokiniemi@jollamobile.com>



> 
> Cc: Kalle Jokiniemi <kalle.jokiniemi@jollamobile.com>
> Signed-off-by: Fabio Baltieri <fabio.baltieri@linaro.org>
> ---
>  drivers/usb/phy/phy-twl4030-usb.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/phy/phy-twl4030-usb.c b/drivers/usb/phy/phy-twl4030-usb.c
> index 3f9858f..13e17ae 100644
> --- a/drivers/usb/phy/phy-twl4030-usb.c
> +++ b/drivers/usb/phy/phy-twl4030-usb.c
> @@ -384,9 +384,17 @@ static void __twl4030_phy_power(struct twl4030_usb *twl, int on)
>  
>  static void twl4030_phy_power(struct twl4030_usb *twl, int on)
>  {
> +	int ret;
> +
>  	if (on) {
> -		regulator_enable(twl->usb3v1);
> -		regulator_enable(twl->usb1v8);
> +		ret = regulator_enable(twl->usb3v1);
> +		if (ret)
> +			dev_err(twl->dev, "Failed to enable usb3v1\n");
> +
> +		ret = regulator_enable(twl->usb1v8);
> +		if (ret)
> +			dev_err(twl->dev, "Failed to enable usb1v8\n");
> +
>  		/*
>  		 * Disabling usb3v1 regulator (= writing 0 to VUSB3V1_DEV_GRP
>  		 * in twl4030) resets the VUSB_DEDICATED2 register. This reset
> @@ -395,7 +403,11 @@ static void twl4030_phy_power(struct twl4030_usb *twl, int on)
>  		 * is re-activated. This ensures that VUSB3V1 is really active.
>  		 */
>  		twl_i2c_write_u8(TWL_MODULE_PM_RECEIVER, 0, VUSB_DEDICATED2);
> -		regulator_enable(twl->usb1v5);
> +
> +		ret = regulator_enable(twl->usb1v5);
> +		if (ret)
> +			dev_err(twl->dev, "Failed to enable usb1v5\n");
> +
>  		__twl4030_phy_power(twl, 1);
>  		twl4030_usb_write(twl, PHY_CLK_CTRL,
>  				  twl4030_usb_read(twl, PHY_CLK_CTRL) |

[PATCH 3/3] usb: phy: twl6030-usb: check regulator_enable return value

[Fabio Baltieri]

Since regulator_enable() is going to be marked as __must_check in the
next merge window, always check regulator_enable() return value and
print a warning if it fails.

Cc: Hema HK <hemahk@ti.com>
Signed-off-by: Fabio Baltieri <fabio.baltieri@linaro.org>
---
 drivers/usb/phy/phy-twl6030-usb.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/phy/phy-twl6030-usb.c b/drivers/usb/phy/phy-twl6030-usb.c
index 8cd6cf4..e841474 100644
--- a/drivers/usb/phy/phy-twl6030-usb.c
+++ b/drivers/usb/phy/phy-twl6030-usb.c
@@ -211,6 +211,7 @@ static irqreturn_t twl6030_usb_irq(int irq, void *_twl)
 	struct twl6030_usb *twl = _twl;
 	enum omap_musb_vbus_id_status status = OMAP_MUSB_UNKNOWN;
 	u8 vbus_state, hw_state;
+	int ret;
 
 	hw_state = twl6030_readb(twl, TWL6030_MODULE_ID0, STS_HW_CONDITIONS);
 
@@ -218,7 +219,10 @@ static irqreturn_t twl6030_usb_irq(int irq, void *_twl)
 						CONTROLLER_STAT1);
 	if (!(hw_state & STS_USB_ID)) {
 		if (vbus_state & VBUS_DET) {
-			regulator_enable(twl->usb3v3);
+			ret = regulator_enable(twl->usb3v3);
+			if (ret)
+				dev_err(twl->dev, "Failed to enable usb3v3\n");
+
 			twl->asleep = 1;
 			status = OMAP_MUSB_VBUS_VALID;
 			twl->linkstat = status;
@@ -245,12 +249,15 @@ static irqreturn_t twl6030_usbotg_irq(int irq, void *_twl)
 	struct twl6030_usb *twl = _twl;
 	enum omap_musb_vbus_id_status status = OMAP_MUSB_UNKNOWN;
 	u8 hw_state;
+	int ret;
 
 	hw_state = twl6030_readb(twl, TWL6030_MODULE_ID0, STS_HW_CONDITIONS);
 
 	if (hw_state & STS_USB_ID) {
+		ret = regulator_enable(twl->usb3v3);
+		if (ret)
+			dev_err(twl->dev, "Failed to enable usb3v3\n");
 
-		regulator_enable(twl->usb3v3);
 		twl->asleep = 1;
 		twl6030_writeb(twl, TWL_MODULE_USB, 0x1, USB_ID_INT_EN_HI_CLR);
 		twl6030_writeb(twl, TWL_MODULE_USB, 0x10, USB_ID_INT_EN_HI_SET);
-- 
1.8.1.3

Re: [PATCH 0/3] check regulator_enable() return value

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 607 bytes --]

Hi,

On Wed, Apr 03, 2013 at 04:02:24PM +0200, Fabio Baltieri wrote:
> While testing your 'next' branch merged with today's next I got some new
> warnings, caused by a recently introduced __must_check in:
> 
> c8801a8 regulator: core: Mark all get and enable calls as __must_check
> 
> These patches introduces a check for regulator_enable() return value in
> all three affected USB phy drivers, and issue a dev_err() in case it
> fails.
> 
> TWL4030 and TWL6030 patches has been build-tested only.

Sorry but I can't change my tree anymore, we can send these during
v3.10-rc.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 0/3] check regulator_enable() return value

[Fabio Baltieri]

Hi Felipe,

On Wed, Apr 03, 2013 at 05:06:22PM +0300, Felipe Balbi wrote:
> Hi,
> 
> On Wed, Apr 03, 2013 at 04:02:24PM +0200, Fabio Baltieri wrote:
> > While testing your 'next' branch merged with today's next I got some new
> > warnings, caused by a recently introduced __must_check in:
> > 
> > c8801a8 regulator: core: Mark all get and enable calls as __must_check
> > 
> > These patches introduces a check for regulator_enable() return value in
> > all three affected USB phy drivers, and issue a dev_err() in case it
> > fails.
> > 
> > TWL4030 and TWL6030 patches has been build-tested only.
> 
> Sorry but I can't change my tree anymore, we can send these during
> v3.10-rc.

Sure, I've no problem with that!

Thanks,
Fabio

-- 
Fabio Baltieri

Re: [PATCH 0/3] check regulator_enable() return value

[Greg KH]

On Wed, Apr 03, 2013 at 05:06:22PM +0300, Felipe Balbi wrote:
> Hi,
> 
> On Wed, Apr 03, 2013 at 04:02:24PM +0200, Fabio Baltieri wrote:
> > While testing your 'next' branch merged with today's next I got some new
> > warnings, caused by a recently introduced __must_check in:
> > 
> > c8801a8 regulator: core: Mark all get and enable calls as __must_check
> > 
> > These patches introduces a check for regulator_enable() return value in
> > all three affected USB phy drivers, and issue a dev_err() in case it
> > fails.
> > 
> > TWL4030 and TWL6030 patches has been build-tested only.
> 
> Sorry but I can't change my tree anymore, we can send these during
> v3.10-rc.

Really?  You are going to send me a tree that adds build warnings?

Please don't.

greg k-h

Re: [PATCH 0/3] check regulator_enable() return value

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 929 bytes --]

Hi,

On Wed, Apr 03, 2013 at 07:22:38AM -0700, Greg KH wrote:
> On Wed, Apr 03, 2013 at 05:06:22PM +0300, Felipe Balbi wrote:
> > Hi,
> > 
> > On Wed, Apr 03, 2013 at 04:02:24PM +0200, Fabio Baltieri wrote:
> > > While testing your 'next' branch merged with today's next I got some new
> > > warnings, caused by a recently introduced __must_check in:
> > > 
> > > c8801a8 regulator: core: Mark all get and enable calls as __must_check
> > > 
> > > These patches introduces a check for regulator_enable() return value in
> > > all three affected USB phy drivers, and issue a dev_err() in case it
> > > fails.
> > > 
> > > TWL4030 and TWL6030 patches has been build-tested only.
> > 
> > Sorry but I can't change my tree anymore, we can send these during
> > v3.10-rc.
> 
> Really?  You are going to send me a tree that adds build warnings?
> 
> Please don't.

alright, I'll merge these in.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]