| Message ID | 1362822405.8907.1.camel@phoenix (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Axel, On Sat, Mar 09, 2013 at 05:46:45PM +0800, Axel Lin wrote: > Since commit c8801a8e > "regulator: core: Mark all get and enable calls as __must_check", > we must check return value of regulator_enable() to silence below build warning. > > CC drivers/mfd/ab8500-gpadc.o > drivers/mfd/ab8500-gpadc.c: In function 'ab8500_gpadc_runtime_resume': > drivers/mfd/ab8500-gpadc.c:598:18: warning: ignoring return value of 'regulator_enable', declared with attribute warn_unused_result [-Wunused-result] > drivers/mfd/ab8500-gpadc.c: In function 'ab8500_gpadc_probe': > drivers/mfd/ab8500-gpadc.c:655:18: warning: ignoring return value of 'regulator_enable', declared with attribute warn_unused_result [-Wunused-result] > > Also convert to devm_regulator_get(), this fixes a missing regulator_put() call > in ab8500_gpadc_remove(). This is on the edge of a patch split for 2 separate fixes, but I applied it anyway. Cheers, Samuel.
On Sat, 09 Mar 2013, Axel Lin wrote: > Since commit c8801a8e > "regulator: core: Mark all get and enable calls as __must_check", > we must check return value of regulator_enable() to silence below build warning. > > CC drivers/mfd/ab8500-gpadc.o > drivers/mfd/ab8500-gpadc.c: In function 'ab8500_gpadc_runtime_resume': > drivers/mfd/ab8500-gpadc.c:598:18: warning: ignoring return value of 'regulator_enable', declared with attribute warn_unused_result [-Wunused-result] > drivers/mfd/ab8500-gpadc.c: In function 'ab8500_gpadc_probe': > drivers/mfd/ab8500-gpadc.c:655:18: warning: ignoring return value of 'regulator_enable', declared with attribute warn_unused_result [-Wunused-result] > > Also convert to devm_regulator_get(), this fixes a missing regulator_put() call > in ab8500_gpadc_remove(). > > Signed-off-by: Axel Lin <axel.lin@ingics.com> > --- > v2: > In the case of goto fail_enable, we should call regulator_put instead of > regulator_disable. However, the regulator_put call is also missed in > ab8500_gpadc_remove(). Thus a simpler fix is to convert regulator_get to > devm_regulator_get. > > drivers/mfd/ab8500-gpadc.c | 17 +++++++++++++---- > 1 file changed, 13 insertions(+), 4 deletions(-) > > diff --git a/drivers/mfd/ab8500-gpadc.c b/drivers/mfd/ab8500-gpadc.c > index b1f3561..5f341a5 100644 > --- a/drivers/mfd/ab8500-gpadc.c > +++ b/drivers/mfd/ab8500-gpadc.c > @@ -594,9 +594,12 @@ static int ab8500_gpadc_runtime_suspend(struct device *dev) > static int ab8500_gpadc_runtime_resume(struct device *dev) > { > struct ab8500_gpadc *gpadc = dev_get_drvdata(dev); > + int ret; > > - regulator_enable(gpadc->regu); > - return 0; > + ret = regulator_enable(gpadc->regu); > + if (ret) > + dev_err(dev, "Failed to enable vtvout LDO: %d\n", ret); > + return ret; > } > > static int ab8500_gpadc_runtime_idle(struct device *dev) > @@ -643,7 +646,7 @@ static int ab8500_gpadc_probe(struct platform_device *pdev) > } > > /* VTVout LDO used to power up ab8500-GPADC */ > - gpadc->regu = regulator_get(&pdev->dev, "vddadc"); > + gpadc->regu = devm_regulator_get(&pdev->dev, "vddadc"); > if (IS_ERR(gpadc->regu)) { > ret = PTR_ERR(gpadc->regu); > dev_err(gpadc->dev, "failed to get vtvout LDO\n"); > @@ -652,7 +655,11 @@ static int ab8500_gpadc_probe(struct platform_device *pdev) > > platform_set_drvdata(pdev, gpadc); > > - regulator_enable(gpadc->regu); > + ret = regulator_enable(gpadc->regu); > + if (ret) { > + dev_err(gpadc->dev, "Failed to enable vtvout LDO: %d\n", ret); > + goto fail_enable; > + } > > pm_runtime_set_autosuspend_delay(gpadc->dev, GPADC_AUDOSUSPEND_DELAY); > pm_runtime_use_autosuspend(gpadc->dev); > @@ -663,6 +670,8 @@ static int ab8500_gpadc_probe(struct platform_device *pdev) > list_add_tail(&gpadc->node, &ab8500_gpadc_list); > dev_dbg(gpadc->dev, "probe success\n"); > return 0; > + > +fail_enable: > fail_irq: > free_irq(gpadc->irq, gpadc); > fail: Actually I think this is the opposite of what we want. Please see: https://patchwork.kernel.org/patch/2147571/ I can't get hold of Jonas to ask him yet, as he is on parental leave.
diff --git a/drivers/mfd/ab8500-gpadc.c b/drivers/mfd/ab8500-gpadc.c index b1f3561..5f341a5 100644 --- a/drivers/mfd/ab8500-gpadc.c +++ b/drivers/mfd/ab8500-gpadc.c @@ -594,9 +594,12 @@ static int ab8500_gpadc_runtime_suspend(struct device *dev) static int ab8500_gpadc_runtime_resume(struct device *dev) { struct ab8500_gpadc *gpadc = dev_get_drvdata(dev); + int ret; - regulator_enable(gpadc->regu); - return 0; + ret = regulator_enable(gpadc->regu); + if (ret) + dev_err(dev, "Failed to enable vtvout LDO: %d\n", ret); + return ret; } static int ab8500_gpadc_runtime_idle(struct device *dev) @@ -643,7 +646,7 @@ static int ab8500_gpadc_probe(struct platform_device *pdev) } /* VTVout LDO used to power up ab8500-GPADC */ - gpadc->regu = regulator_get(&pdev->dev, "vddadc"); + gpadc->regu = devm_regulator_get(&pdev->dev, "vddadc"); if (IS_ERR(gpadc->regu)) { ret = PTR_ERR(gpadc->regu); dev_err(gpadc->dev, "failed to get vtvout LDO\n"); @@ -652,7 +655,11 @@ static int ab8500_gpadc_probe(struct platform_device *pdev) platform_set_drvdata(pdev, gpadc); - regulator_enable(gpadc->regu); + ret = regulator_enable(gpadc->regu); + if (ret) { + dev_err(gpadc->dev, "Failed to enable vtvout LDO: %d\n", ret); + goto fail_enable; + } pm_runtime_set_autosuspend_delay(gpadc->dev, GPADC_AUDOSUSPEND_DELAY); pm_runtime_use_autosuspend(gpadc->dev); @@ -663,6 +670,8 @@ static int ab8500_gpadc_probe(struct platform_device *pdev) list_add_tail(&gpadc->node, &ab8500_gpadc_list); dev_dbg(gpadc->dev, "probe success\n"); return 0; + +fail_enable: fail_irq: free_irq(gpadc->irq, gpadc); fail:
Since commit c8801a8e "regulator: core: Mark all get and enable calls as __must_check", we must check return value of regulator_enable() to silence below build warning. CC drivers/mfd/ab8500-gpadc.o drivers/mfd/ab8500-gpadc.c: In function 'ab8500_gpadc_runtime_resume': drivers/mfd/ab8500-gpadc.c:598:18: warning: ignoring return value of 'regulator_enable', declared with attribute warn_unused_result [-Wunused-result] drivers/mfd/ab8500-gpadc.c: In function 'ab8500_gpadc_probe': drivers/mfd/ab8500-gpadc.c:655:18: warning: ignoring return value of 'regulator_enable', declared with attribute warn_unused_result [-Wunused-result] Also convert to devm_regulator_get(), this fixes a missing regulator_put() call in ab8500_gpadc_remove(). Signed-off-by: Axel Lin <axel.lin@ingics.com> --- v2: In the case of goto fail_enable, we should call regulator_put instead of regulator_disable. However, the regulator_put call is also missed in ab8500_gpadc_remove(). Thus a simpler fix is to convert regulator_get to devm_regulator_get. drivers/mfd/ab8500-gpadc.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)