2013-04-03 14:02:43

by Fabio Baltieri

[permalink] [raw]
Subject: [PATCH 0/3] check regulator_enable() return value

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


2013-04-03 14:02:56

by Fabio Baltieri

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

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 <[email protected]>
Signed-off-by: Fabio Baltieri <[email protected]>
---
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

2013-04-03 14:03:19

by Fabio Baltieri

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

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 <[email protected]>
Signed-off-by: Fabio Baltieri <[email protected]>
---
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

2013-04-03 14:03:08

by Fabio Baltieri

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

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 <[email protected]>
Signed-off-by: Fabio Baltieri <[email protected]>
---
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

2013-04-03 14:06:41

by Felipe Balbi

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

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


Attachments:
(No filename) (588.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2013-04-03 14:12:20

by Fabio Baltieri

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

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

2013-04-03 14:22:44

by Greg Kroah-Hartman

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

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

2013-04-03 15:21:49

by Kalle Jokiniemi

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

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 <[email protected]>



>
> Cc: Kalle Jokiniemi <[email protected]>
> Signed-off-by: Fabio Baltieri <[email protected]>
> ---
> 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) |

2013-04-03 15:44:25

by Felipe Balbi

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

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


Attachments:
(No filename) (900.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments