When booting -next on the u8500 based Snowball development board these
new warnings appeared. If applicable these should probably be filtered
though your -fixes branches.
arch/arm/mach-ux500/board-mop500.c: In function ‘mop500_prox_activate’:
arch/arm/mach-ux500/board-mop500.c:406:18: warning: ignoring return value of
‘regulator_enable’, declared with attribute warn_unused_result
[-Wunused-result]
Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/mach-ux500/board-mop500.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-ux500/board-mop500.c b/arch/arm/mach-ux500/board-mop500.c
index a15dd6b..9bc762a 100644
--- a/arch/arm/mach-ux500/board-mop500.c
+++ b/arch/arm/mach-ux500/board-mop500.c
@@ -403,8 +403,8 @@ static int mop500_prox_activate(struct device *dev)
"no regulator\n");
return PTR_ERR(prox_regulator);
}
- regulator_enable(prox_regulator);
- return 0;
+
+ return regulator_enable(prox_regulator);
}
static void mop500_prox_deactivate(struct device *dev)
--
1.7.10.4
drivers/mfd/ab8500-gpadc.c: In function ‘ab8500_gpadc_resume’:
drivers/mfd/ab8500-gpadc.c:911:18: warning: ignoring return value of
‘regulator_enable’, declared with attribute warn_unused_result
[-Wunused-result]
Cc: Samuel Ortiz <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/mfd/ab8500-gpadc.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/mfd/ab8500-gpadc.c b/drivers/mfd/ab8500-gpadc.c
index 5e65b28..7278e53 100644
--- a/drivers/mfd/ab8500-gpadc.c
+++ b/drivers/mfd/ab8500-gpadc.c
@@ -907,8 +907,11 @@ static int ab8500_gpadc_suspend(struct device *dev)
static int ab8500_gpadc_resume(struct device *dev)
{
struct ab8500_gpadc *gpadc = dev_get_drvdata(dev);
+ int ret;
- regulator_enable(gpadc->regu);
+ ret = regulator_enable(gpadc->regu);
+ if (ret < 0)
+ return ret;
pm_runtime_mark_last_busy(gpadc->dev);
pm_runtime_put_autosuspend(gpadc->dev);
--
1.7.10.4
This patch suppresses the warning below:
drivers/mmc/host/mmci.c: In function ‘mmci_set_ios’:
drivers/mmc/host/mmci.c:1165:20: warning: ignoring return value of
‘regulator_enable’, declared with attribute warn_unused_result
[-Wunused-result]
Cc: Russell King <[email protected]>
Cc: Chris Ball <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/mmc/host/mmci.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 375c109..f4f3038 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1130,6 +1130,7 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
struct variant_data *variant = host->variant;
u32 pwr = 0;
unsigned long flags;
+ int ret;
pm_runtime_get_sync(mmc_dev(mmc));
@@ -1161,8 +1162,12 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
break;
case MMC_POWER_ON:
if (!IS_ERR(mmc->supply.vqmmc) &&
- !regulator_is_enabled(mmc->supply.vqmmc))
- regulator_enable(mmc->supply.vqmmc);
+ !regulator_is_enabled(mmc->supply.vqmmc)) {
+ ret = regulator_enable(mmc->supply.vqmmc);
+ if (ret < 0)
+ dev_err(mmc_dev(mmc),
+ "failed to enable vqmmc regulator\n");
+ }
pwr |= MCI_PWR_ON;
break;
--
1.7.10.4
drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c:
In function ‘synaptics_rmi4_resume’:
drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c:1090:18:
warning: ignoring return value of ‘regulator_enable’, declared
with attribute warn_unused_result [-Wunused-result
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
Signed-off-by: Lee Jones <[email protected]>
---
drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
index fe667dd..c4d013d 100644
--- a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
+++ b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
@@ -1087,7 +1087,9 @@ static int synaptics_rmi4_resume(struct device *dev)
unsigned char intr_status;
struct synaptics_rmi4_data *rmi4_data = dev_get_drvdata(dev);
- regulator_enable(rmi4_data->regulator);
+ retval = regulator_enable(rmi4_data->regulator);
+ if (retval < 0)
+ return retval;
enable_irq(rmi4_data->i2c_client->irq);
rmi4_data->touch_stopped = false;
--
1.7.10.4
On Thu, May 02, 2013 at 17:48:10 +0200, Lee Jones wrote:
> drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c:
> In function ‘synaptics_rmi4_resume’:
> drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c:1090:18:
> warning: ignoring return value of ‘regulator_enable’, declared
> with attribute warn_unused_result [-Wunused-result
>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: [email protected]
> Signed-off-by: Lee Jones <[email protected]>
> ---
> drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
> index fe667dd..c4d013d 100644
> --- a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
> +++ b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
> @@ -1087,7 +1087,9 @@ static int synaptics_rmi4_resume(struct device *dev)
> unsigned char intr_status;
> struct synaptics_rmi4_data *rmi4_data = dev_get_drvdata(dev);
>
> - regulator_enable(rmi4_data->regulator);
> + retval = regulator_enable(rmi4_data->regulator);
> + if (retval < 0)
> + return retval;
Does it make sense to add a dev_err?
Otherwise, you can add my
Acked-by:srinidhi kasagar <[email protected]> for this series.
>
> enable_irq(rmi4_data->i2c_client->irq);
> rmi4_data->touch_stopped = false;
> --
> 1.7.10.4
>
On Fri, 03 May 2013, Srinidhi Kasagar wrote:
> On Thu, May 02, 2013 at 17:48:10 +0200, Lee Jones wrote:
> > drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c:
> > In function ‘synaptics_rmi4_resume’:
> > drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c:1090:18:
> > warning: ignoring return value of ‘regulator_enable’, declared
> > with attribute warn_unused_result [-Wunused-result
> >
> > Cc: Greg Kroah-Hartman <[email protected]>
> > Cc: [email protected]
> > Signed-off-by: Lee Jones <[email protected]>
> > ---
> > drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
> > index fe667dd..c4d013d 100644
> > --- a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
> > +++ b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
> > @@ -1087,7 +1087,9 @@ static int synaptics_rmi4_resume(struct device *dev)
> > unsigned char intr_status;
> > struct synaptics_rmi4_data *rmi4_data = dev_get_drvdata(dev);
> >
> > - regulator_enable(rmi4_data->regulator);
> > + retval = regulator_enable(rmi4_data->regulator);
> > + if (retval < 0)
> > + return retval;
> Does it make sense to add a dev_err?
Yes, I can do that.
> Otherwise, you can add my
> Acked-by:srinidhi kasagar <[email protected]> for this series.
Thanks.
> >
> > enable_irq(rmi4_data->i2c_client->irq);
> > rmi4_data->touch_stopped = false;
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On 2 May 2013 17:48, Lee Jones <[email protected]> wrote:
> This patch suppresses the warning below:
>
> drivers/mmc/host/mmci.c: In function ?mmci_set_ios?:
> drivers/mmc/host/mmci.c:1165:20: warning: ignoring return value of
> ?regulator_enable?, declared with attribute warn_unused_result
> [-Wunused-result]
>
> Cc: Russell King <[email protected]>
> Cc: Chris Ball <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>
> ---
> drivers/mmc/host/mmci.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index 375c109..f4f3038 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -1130,6 +1130,7 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> struct variant_data *variant = host->variant;
> u32 pwr = 0;
> unsigned long flags;
> + int ret;
>
> pm_runtime_get_sync(mmc_dev(mmc));
>
> @@ -1161,8 +1162,12 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> break;
> case MMC_POWER_ON:
> if (!IS_ERR(mmc->supply.vqmmc) &&
> - !regulator_is_enabled(mmc->supply.vqmmc))
I realize that we actually have a bug here (and in the MMC_POWER_OFF
mode as well).
We shall not use regulator_is_enabled API as a trigger to enable/fetch
a reference to the regulator, since it will only tell us if the
regulator is enabled - hw wise.
Instead we need a local variable in the mmci host struct keeping track
if we have enabled the regulator. Do you mind fix this up in this
patch as well since it is tightly coupled to the regulator handling!?
Otherwise it looks good to me.
Kind regards
Ulf Hansson
> - regulator_enable(mmc->supply.vqmmc);
> + !regulator_is_enabled(mmc->supply.vqmmc)) {
> + ret = regulator_enable(mmc->supply.vqmmc);
> + if (ret < 0)
> + dev_err(mmc_dev(mmc),
> + "failed to enable vqmmc regulator\n");
> + }
>
> pwr |= MCI_PWR_ON;
> break;
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
On Fri, 03 May 2013, Ulf Hansson wrote:
> On 2 May 2013 17:48, Lee Jones <[email protected]> wrote:
> > This patch suppresses the warning below:
> >
> > drivers/mmc/host/mmci.c: In function ‘mmci_set_ios’:
> > drivers/mmc/host/mmci.c:1165:20: warning: ignoring return value of
> > ‘regulator_enable’, declared with attribute warn_unused_result
> > [-Wunused-result]
> >
> > Cc: Russell King <[email protected]>
> > Cc: Chris Ball <[email protected]>
> > Signed-off-by: Lee Jones <[email protected]>
> > ---
> > drivers/mmc/host/mmci.c | 9 +++++++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> > index 375c109..f4f3038 100644
> > --- a/drivers/mmc/host/mmci.c
> > +++ b/drivers/mmc/host/mmci.c
> > @@ -1130,6 +1130,7 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> > struct variant_data *variant = host->variant;
> > u32 pwr = 0;
> > unsigned long flags;
> > + int ret;
> >
> > pm_runtime_get_sync(mmc_dev(mmc));
> >
> > @@ -1161,8 +1162,12 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> > break;
> > case MMC_POWER_ON:
> > if (!IS_ERR(mmc->supply.vqmmc) &&
> > - !regulator_is_enabled(mmc->supply.vqmmc))
>
> I realize that we actually have a bug here (and in the MMC_POWER_OFF
> mode as well).
>
> We shall not use regulator_is_enabled API as a trigger to enable/fetch
> a reference to the regulator, since it will only tell us if the
> regulator is enabled - hw wise.
> Instead we need a local variable in the mmci host struct keeping track
> if we have enabled the regulator. Do you mind fix this up in this
> patch as well since it is tightly coupled to the regulator handling!?
IMHO I think that should be taken care of in a separate patch. This
patch only touches the line containing regulator_is_enabled() to
encompass a multi-line comparison result.
Care to write that patch? I have so much on my TODO already. :|
> Otherwise it looks good to me.
>
> Kind regards
> Ulf Hansson
>
> > - regulator_enable(mmc->supply.vqmmc);
> > + !regulator_is_enabled(mmc->supply.vqmmc)) {
> > + ret = regulator_enable(mmc->supply.vqmmc);
> > + if (ret < 0)
> > + dev_err(mmc_dev(mmc),
> > + "failed to enable vqmmc regulator\n");
> > + }
> >
> > pwr |= MCI_PWR_ON;
> > break;
> >
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On 3 May 2013 10:16, Lee Jones <[email protected]> wrote:
> On Fri, 03 May 2013, Ulf Hansson wrote:
>
>> On 2 May 2013 17:48, Lee Jones <[email protected]> wrote:
>> > This patch suppresses the warning below:
>> >
>> > drivers/mmc/host/mmci.c: In function ‘mmci_set_ios’:
>> > drivers/mmc/host/mmci.c:1165:20: warning: ignoring return value of
>> > ‘regulator_enable’, declared with attribute warn_unused_result
>> > [-Wunused-result]
>> >
>> > Cc: Russell King <[email protected]>
>> > Cc: Chris Ball <[email protected]>
>> > Signed-off-by: Lee Jones <[email protected]>
>> > ---
>> > drivers/mmc/host/mmci.c | 9 +++++++--
>> > 1 file changed, 7 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
>> > index 375c109..f4f3038 100644
>> > --- a/drivers/mmc/host/mmci.c
>> > +++ b/drivers/mmc/host/mmci.c
>> > @@ -1130,6 +1130,7 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>> > struct variant_data *variant = host->variant;
>> > u32 pwr = 0;
>> > unsigned long flags;
>> > + int ret;
>> >
>> > pm_runtime_get_sync(mmc_dev(mmc));
>> >
>> > @@ -1161,8 +1162,12 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>> > break;
>> > case MMC_POWER_ON:
>> > if (!IS_ERR(mmc->supply.vqmmc) &&
>> > - !regulator_is_enabled(mmc->supply.vqmmc))
>>
>> I realize that we actually have a bug here (and in the MMC_POWER_OFF
>> mode as well).
>>
>> We shall not use regulator_is_enabled API as a trigger to enable/fetch
>> a reference to the regulator, since it will only tell us if the
>> regulator is enabled - hw wise.
>> Instead we need a local variable in the mmci host struct keeping track
>> if we have enabled the regulator. Do you mind fix this up in this
>> patch as well since it is tightly coupled to the regulator handling!?
>
> IMHO I think that should be taken care of in a separate patch. This
> patch only touches the line containing regulator_is_enabled() to
> encompass a multi-line comparison result.
>
> Care to write that patch? I have so much on my TODO already. :|
Sure, I can fix it.
I guess this patch will be going through Russell's patch tracker?
>
>> Otherwise it looks good to me.
>>
>> Kind regards
>> Ulf Hansson
>>
>> > - regulator_enable(mmc->supply.vqmmc);
>> > + !regulator_is_enabled(mmc->supply.vqmmc)) {
>> > + ret = regulator_enable(mmc->supply.vqmmc);
>> > + if (ret < 0)
>> > + dev_err(mmc_dev(mmc),
>> > + "failed to enable vqmmc regulator\n");
>> > + }
>> >
>> > pwr |= MCI_PWR_ON;
>> > break;
>> >
>
> --
> Lee Jones
> Linaro ST-Ericsson Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
On Thu, May 2, 2013 at 5:48 PM, Lee Jones <[email protected]> wrote:
> arch/arm/mach-ux500/board-mop500.c: In function ?mop500_prox_activate?:
> arch/arm/mach-ux500/board-mop500.c:406:18: warning: ignoring return value of
> ?regulator_enable?, declared with attribute warn_unused_result
> [-Wunused-result]
>
> Signed-off-by: Lee Jones <[email protected]>
Applied!
Thanks,
Linus Walleij
On Thu, May 2, 2013 at 5:48 PM, Lee Jones <[email protected]> wrote:
> drivers/mfd/ab8500-gpadc.c: In function ?ab8500_gpadc_resume?:
> drivers/mfd/ab8500-gpadc.c:911:18: warning: ignoring return value of
> ?regulator_enable?, declared with attribute warn_unused_result
> [-Wunused-result]
>
> Cc: Samuel Ortiz <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>
> ---
> drivers/mfd/ab8500-gpadc.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mfd/ab8500-gpadc.c b/drivers/mfd/ab8500-gpadc.c
> index 5e65b28..7278e53 100644
> --- a/drivers/mfd/ab8500-gpadc.c
> +++ b/drivers/mfd/ab8500-gpadc.c
> @@ -907,8 +907,11 @@ static int ab8500_gpadc_suspend(struct device *dev)
> static int ab8500_gpadc_resume(struct device *dev)
> {
> struct ab8500_gpadc *gpadc = dev_get_drvdata(dev);
> + int ret;
>
> - regulator_enable(gpadc->regu);
> + ret = regulator_enable(gpadc->regu);
> + if (ret < 0)
> + return ret;
>
> pm_runtime_mark_last_busy(gpadc->dev);
> pm_runtime_put_autosuspend(gpadc->dev);
> --
> 1.7.10.4
Acked-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
On Fri, 03 May 2013, Ulf Hansson wrote:
> On 3 May 2013 10:16, Lee Jones <[email protected]> wrote:
> > On Fri, 03 May 2013, Ulf Hansson wrote:
> >
> >> On 2 May 2013 17:48, Lee Jones <[email protected]> wrote:
> >> > This patch suppresses the warning below:
> >> >
> >> > drivers/mmc/host/mmci.c: In function ‘mmci_set_ios’:
> >> > drivers/mmc/host/mmci.c:1165:20: warning: ignoring return value of
> >> > ‘regulator_enable’, declared with attribute warn_unused_result
> >> > [-Wunused-result]
> >> >
> >> > Cc: Russell King <[email protected]>
> >> > Cc: Chris Ball <[email protected]>
> >> > Signed-off-by: Lee Jones <[email protected]>
> >> > ---
> >> > drivers/mmc/host/mmci.c | 9 +++++++--
> >> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> >> > index 375c109..f4f3038 100644
> >> > --- a/drivers/mmc/host/mmci.c
> >> > +++ b/drivers/mmc/host/mmci.c
> >> > @@ -1130,6 +1130,7 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> >> > struct variant_data *variant = host->variant;
> >> > u32 pwr = 0;
> >> > unsigned long flags;
> >> > + int ret;
> >> >
> >> > pm_runtime_get_sync(mmc_dev(mmc));
> >> >
> >> > @@ -1161,8 +1162,12 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> >> > break;
> >> > case MMC_POWER_ON:
> >> > if (!IS_ERR(mmc->supply.vqmmc) &&
> >> > - !regulator_is_enabled(mmc->supply.vqmmc))
> >>
> >> I realize that we actually have a bug here (and in the MMC_POWER_OFF
> >> mode as well).
> >>
> >> We shall not use regulator_is_enabled API as a trigger to enable/fetch
> >> a reference to the regulator, since it will only tell us if the
> >> regulator is enabled - hw wise.
> >> Instead we need a local variable in the mmci host struct keeping track
> >> if we have enabled the regulator. Do you mind fix this up in this
> >> patch as well since it is tightly coupled to the regulator handling!?
> >
> > IMHO I think that should be taken care of in a separate patch. This
> > patch only touches the line containing regulator_is_enabled() to
> > encompass a multi-line comparison result.
> >
> > Care to write that patch? I have so much on my TODO already. :|
>
> Sure, I can fix it.
>
> I guess this patch will be going through Russell's patch tracker?
Yeah, I'll queue this and another one I have in a bit.
> >> Otherwise it looks good to me.
> >>
> >> Kind regards
> >> Ulf Hansson
> >>
> >> > - regulator_enable(mmc->supply.vqmmc);
> >> > + !regulator_is_enabled(mmc->supply.vqmmc)) {
> >> > + ret = regulator_enable(mmc->supply.vqmmc);
> >> > + if (ret < 0)
> >> > + dev_err(mmc_dev(mmc),
> >> > + "failed to enable vqmmc regulator\n");
> >> > + }
> >> >
> >> > pwr |= MCI_PWR_ON;
> >> > break;
> >> >
> >
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Fri, 03 May 2013, Linus Walleij wrote:
> On Thu, May 2, 2013 at 5:48 PM, Lee Jones <[email protected]> wrote:
> > drivers/mfd/ab8500-gpadc.c: In function ‘ab8500_gpadc_resume’:
> > drivers/mfd/ab8500-gpadc.c:911:18: warning: ignoring return value of
> > ‘regulator_enable’, declared with attribute warn_unused_result
> > [-Wunused-result]
> >
> > Cc: Samuel Ortiz <[email protected]>
> > Signed-off-by: Lee Jones <[email protected]>
> > ---
> > drivers/mfd/ab8500-gpadc.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mfd/ab8500-gpadc.c b/drivers/mfd/ab8500-gpadc.c
> > index 5e65b28..7278e53 100644
> > --- a/drivers/mfd/ab8500-gpadc.c
> > +++ b/drivers/mfd/ab8500-gpadc.c
> > @@ -907,8 +907,11 @@ static int ab8500_gpadc_suspend(struct device *dev)
> > static int ab8500_gpadc_resume(struct device *dev)
> > {
> > struct ab8500_gpadc *gpadc = dev_get_drvdata(dev);
> > + int ret;
> >
> > - regulator_enable(gpadc->regu);
> > + ret = regulator_enable(gpadc->regu);
> > + if (ret < 0)
> > + return ret;
> >
> > pm_runtime_mark_last_busy(gpadc->dev);
> > pm_runtime_put_autosuspend(gpadc->dev);
>
> Acked-by: Linus Walleij <[email protected]>
As it has your Ack, I'm going to add an error print as per Srinidhi's
suggestion, test it again and apply this to my MFD tree.
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
drivers/mfd/ab8500-gpadc.c: In function ‘ab8500_gpadc_resume’:
drivers/mfd/ab8500-gpadc.c:911:18: warning: ignoring return value of
‘regulator_enable’, declared with attribute warn_unused_result
[-Wunused-result]
Cc: Samuel Ortiz <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
diff --git a/drivers/mfd/ab8500-gpadc.c b/drivers/mfd/ab8500-gpadc.c
index 5e65b28..13f7866 100644
--- a/drivers/mfd/ab8500-gpadc.c
+++ b/drivers/mfd/ab8500-gpadc.c
@@ -907,14 +907,17 @@ static int ab8500_gpadc_suspend(struct device *dev)
static int ab8500_gpadc_resume(struct device *dev)
{
struct ab8500_gpadc *gpadc = dev_get_drvdata(dev);
+ int ret;
- regulator_enable(gpadc->regu);
+ ret = regulator_enable(gpadc->regu);
+ if (ret)
+ dev_err(dev, "Failed to enable vtvout LDO: %d\n", ret);
pm_runtime_mark_last_busy(gpadc->dev);
pm_runtime_put_autosuspend(gpadc->dev);
mutex_unlock(&gpadc->ab8500_gpadc_lock);
- return 0;
+ return ret;
}
static int ab8500_gpadc_probe(struct platform_device *pdev)
On Fri, 03 May 2013, Lee Jones wrote:
> On Fri, 03 May 2013, Linus Walleij wrote:
>
> > On Thu, May 2, 2013 at 5:48 PM, Lee Jones <[email protected]> wrote:
> > > drivers/mfd/ab8500-gpadc.c: In function ‘ab8500_gpadc_resume’:
> > > drivers/mfd/ab8500-gpadc.c:911:18: warning: ignoring return value of
> > > ‘regulator_enable’, declared with attribute warn_unused_result
> > > [-Wunused-result]
> > >
> > > Cc: Samuel Ortiz <[email protected]>
> > > Signed-off-by: Lee Jones <[email protected]>
> > > ---
> > > drivers/mfd/ab8500-gpadc.c | 5 ++++-
> > > 1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/mfd/ab8500-gpadc.c b/drivers/mfd/ab8500-gpadc.c
> > > index 5e65b28..7278e53 100644
> > > --- a/drivers/mfd/ab8500-gpadc.c
> > > +++ b/drivers/mfd/ab8500-gpadc.c
> > > @@ -907,8 +907,11 @@ static int ab8500_gpadc_suspend(struct device *dev)
> > > static int ab8500_gpadc_resume(struct device *dev)
> > > {
> > > struct ab8500_gpadc *gpadc = dev_get_drvdata(dev);
> > > + int ret;
> > >
> > > - regulator_enable(gpadc->regu);
> > > + ret = regulator_enable(gpadc->regu);
> > > + if (ret < 0)
> > > + return ret;
> > >
> > > pm_runtime_mark_last_busy(gpadc->dev);
> > > pm_runtime_put_autosuspend(gpadc->dev);
> >
> > Acked-by: Linus Walleij <[email protected]>
>
> As it has your Ack, I'm going to add an error print as per Srinidhi's
> suggestion, test it again and apply this to my MFD tree.
Slight change of plan, as I've changed the semantics.
Would you be kind enough to see v2?
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Fri, May 3, 2013 at 1:15 PM, Lee Jones <[email protected]> wrote:
> drivers/mfd/ab8500-gpadc.c: In function ?ab8500_gpadc_resume?:
> drivers/mfd/ab8500-gpadc.c:911:18: warning: ignoring return value of
> ?regulator_enable?, declared with attribute warn_unused_result
> [-Wunused-result]
>
> Cc: Samuel Ortiz <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>
Acked-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
On Fri, 03 May 2013, Linus Walleij wrote:
> On Fri, May 3, 2013 at 1:15 PM, Lee Jones <[email protected]> wrote:
>
> > drivers/mfd/ab8500-gpadc.c: In function ‘ab8500_gpadc_resume’:
> > drivers/mfd/ab8500-gpadc.c:911:18: warning: ignoring return value of
> > ‘regulator_enable’, declared with attribute warn_unused_result
> > [-Wunused-result]
> >
> > Cc: Samuel Ortiz <[email protected]>
> > Signed-off-by: Lee Jones <[email protected]>
>
> Acked-by: Linus Walleij <[email protected]>
>
> Yours,
> Linus Walleij
Nice, thanks. I'm applying this now.
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c:
In function ‘synaptics_rmi4_resume’:
drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c:1090:18:
warning: ignoring return value of ‘regulator_enable’, declared
with attribute warn_unused_result [-Wunused-result
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
Acked-by: Srinidhi Kasagar <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
diff --git a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
index fe667dd..386362c 100644
--- a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
+++ b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
@@ -1087,7 +1087,11 @@ static int synaptics_rmi4_resume(struct device *dev)
unsigned char intr_status;
struct synaptics_rmi4_data *rmi4_data = dev_get_drvdata(dev);
- regulator_enable(rmi4_data->regulator);
+ retval = regulator_enable(rmi4_data->regulator);
+ if (retval) {
+ dev_err(dev, "Regulator enable failed (%d)\n", retval);
+ return retval;
+ }
enable_irq(rmi4_data->i2c_client->irq);
rmi4_data->touch_stopped = false;
On Fri, May 03, 2013 at 12:37:14PM +0530, Srinidhi Kasagar wrote:
> On Thu, May 02, 2013 at 17:48:10 +0200, Lee Jones wrote:
> > drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c:
> > In function ‘synaptics_rmi4_resume’:
> > drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c:1090:18:
> > warning: ignoring return value of ‘regulator_enable’, declared
> > with attribute warn_unused_result [-Wunused-result
> >
> > Cc: Greg Kroah-Hartman <[email protected]>
> > Cc: [email protected]
> > Signed-off-by: Lee Jones <[email protected]>
> > ---
> > drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
> > index fe667dd..c4d013d 100644
> > --- a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
> > +++ b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
> > @@ -1087,7 +1087,9 @@ static int synaptics_rmi4_resume(struct device *dev)
> > unsigned char intr_status;
> > struct synaptics_rmi4_data *rmi4_data = dev_get_drvdata(dev);
> >
> > - regulator_enable(rmi4_data->regulator);
> > + retval = regulator_enable(rmi4_data->regulator);
> > + if (retval < 0)
> > + return retval;
> Does it make sense to add a dev_err?
>
Is that a question?
regulator_enable() already prints some warnings. Probably it's not
going to fail and adding code that is duplicative or will never be
run is pointless.
regards,
dan carpenter
On Sun, May 05, 2013 at 16:18:55 +0200, Dan Carpenter wrote:
> On Fri, May 03, 2013 at 12:37:14PM +0530, Srinidhi Kasagar wrote:
> > On Thu, May 02, 2013 at 17:48:10 +0200, Lee Jones wrote:
> > > drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c:
> > > In function ‘synaptics_rmi4_resume’:
> > > drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c:1090:18:
> > > warning: ignoring return value of ‘regulator_enable’, declared
> > > with attribute warn_unused_result [-Wunused-result
> > >
> > > Cc: Greg Kroah-Hartman <[email protected]>
> > > Cc: [email protected]
> > > Signed-off-by: Lee Jones <[email protected]>
> > > ---
> > > drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
> > > index fe667dd..c4d013d 100644
> > > --- a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
> > > +++ b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
> > > @@ -1087,7 +1087,9 @@ static int synaptics_rmi4_resume(struct device *dev)
> > > unsigned char intr_status;
> > > struct synaptics_rmi4_data *rmi4_data = dev_get_drvdata(dev);
> > >
> > > - regulator_enable(rmi4_data->regulator);
> > > + retval = regulator_enable(rmi4_data->regulator);
> > > + if (retval < 0)
> > > + return retval;
> > Does it make sense to add a dev_err?
> >
>
> Is that a question?
>
> regulator_enable() already prints some warnings. Probably it's not
> going to fail and adding code that is duplicative or will never be
> run is pointless.
It has become a habit checking the return value and spit some errors.
And BTW, there are many drivers which does this way..Anyway if your
intention is to avoid them for the new drivers..I dont mind skipping
this..
regards/srinidhi
>
> regards,
> dan carpenter
>
On Mon, May 06, 2013 at 12:21:39PM +0530, Srinidhi Kasagar wrote:
> > > > - regulator_enable(rmi4_data->regulator);
> > > > + retval = regulator_enable(rmi4_data->regulator);
> > > > + if (retval < 0)
> > > > + return retval;
> > > Does it make sense to add a dev_err?
> > >
> >
> > Is that a question?
> >
> > regulator_enable() already prints some warnings. Probably it's not
> > going to fail and adding code that is duplicative or will never be
> > run is pointless.
>
> It has become a habit checking the return value and spit some errors.
> And BTW, there are many drivers which does this way..Anyway if your
> intention is to avoid them for the new drivers..I dont mind skipping
> this..
It's not like I'm trying to ban error messages. There isn't any
official policy on error messages.
regards,
dan carpenter
Hi Greg,
> drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c:
> In function ‘synaptics_rmi4_resume’:
> drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c:1090:18:
> warning: ignoring return value of ‘regulator_enable’, declared
> with attribute warn_unused_result [-Wunused-result
>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: [email protected]
> Signed-off-by: Lee Jones <[email protected]>
> ---
> drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
> index fe667dd..c4d013d 100644
> --- a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
> +++ b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
> @@ -1087,7 +1087,9 @@ static int synaptics_rmi4_resume(struct device *dev)
> unsigned char intr_status;
> struct synaptics_rmi4_data *rmi4_data = dev_get_drvdata(dev);
>
> - regulator_enable(rmi4_data->regulator);
> + retval = regulator_enable(rmi4_data->regulator);
> + if (retval < 0)
> + return retval;
>
> enable_irq(rmi4_data->i2c_client->irq);
> rmi4_data->touch_stopped = false;
Are you planning to take this into the v3.10 -rcs?
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Tue, May 14, 2013 at 11:46:31AM +0100, Lee Jones wrote:
> Hi Greg,
>
> > drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c:
> > In function ‘synaptics_rmi4_resume’:
> > drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c:1090:18:
> > warning: ignoring return value of ‘regulator_enable’, declared
> > with attribute warn_unused_result [-Wunused-result
> >
> > Cc: Greg Kroah-Hartman <[email protected]>
> > Cc: [email protected]
> > Signed-off-by: Lee Jones <[email protected]>
> > ---
> > drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
> > index fe667dd..c4d013d 100644
> > --- a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
> > +++ b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
> > @@ -1087,7 +1087,9 @@ static int synaptics_rmi4_resume(struct device *dev)
> > unsigned char intr_status;
> > struct synaptics_rmi4_data *rmi4_data = dev_get_drvdata(dev);
> >
> > - regulator_enable(rmi4_data->regulator);
> > + retval = regulator_enable(rmi4_data->regulator);
> > + if (retval < 0)
> > + return retval;
> >
> > enable_irq(rmi4_data->i2c_client->irq);
> > rmi4_data->touch_stopped = false;
>
> Are you planning to take this into the v3.10 -rcs?
Yes, I was, give me some time to catch up on my pending patch queue (644
patches and counting). Don't worry, it's not lost.
thanks,
greg k-h