| Message ID | 20210912064101.539-1-caihuoqing@baidu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] iio: adc: ad799x: Add a single error handling block at the end of the function. | expand |
On Sun, 12 Sep 2021 14:41:01 +0800 Cai Huoqing <caihuoqing@baidu.com> wrote: > A single error handling block at the end of the function could > be brought in to make code a little more concise. > > Signed-off-by: Cai Huoqing <caihuoqing@baidu.com> +CC Florian who is reworking some of the regulator handling in this driver. https://lore.kernel.org/linux-iio/20210908152525.2946785-1-florian.boor@kernelconcepts.de/ One suggestion I made to Florian's v2 was to make the enabling of st->vref only occur it is actually present (rather than no there after making it optional). That is going to cause merge problems against this patch. @Florian, would you mind adding Cai Huoqing's patch to your series so that we get both your improvements and his on one go without conflicts? Either apply it as a precursor or rebase it on top of your patches. Thanks, Jonathan > --- > v1->v2: > Change the error path-error_disable_reg from "st->vref" to "st->reg". > > drivers/iio/adc/ad799x.c | 17 ++++++++++------- > 1 file changed, 10 insertions(+), 7 deletions(-) > > diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c > index 18bf8386d50a..d3dbc4c1e375 100644 > --- a/drivers/iio/adc/ad799x.c > +++ b/drivers/iio/adc/ad799x.c > @@ -891,20 +891,23 @@ static int __maybe_unused ad799x_resume(struct device *dev) > } > ret = regulator_enable(st->vref); > if (ret) { > - regulator_disable(st->reg); > dev_err(dev, "Unable to enable vref regulator\n"); > - return ret; > + goto error_disable_reg; > } > > /* resync config */ > ret = ad799x_update_config(st, st->config); > - if (ret) { > - regulator_disable(st->vref); > - regulator_disable(st->reg); > - return ret; > - } > + if (ret) > + goto error_disable_vref; > > return 0; > + > +error_disable_vref: > + regulator_disable(st->vref); > +error_disable_reg: > + regulator_disable(st->reg); > + > + return ret; > } > > static SIMPLE_DEV_PM_OPS(ad799x_pm_ops, ad799x_suspend, ad799x_resume);
diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c index 18bf8386d50a..d3dbc4c1e375 100644 --- a/drivers/iio/adc/ad799x.c +++ b/drivers/iio/adc/ad799x.c @@ -891,20 +891,23 @@ static int __maybe_unused ad799x_resume(struct device *dev) } ret = regulator_enable(st->vref); if (ret) { - regulator_disable(st->reg); dev_err(dev, "Unable to enable vref regulator\n"); - return ret; + goto error_disable_reg; } /* resync config */ ret = ad799x_update_config(st, st->config); - if (ret) { - regulator_disable(st->vref); - regulator_disable(st->reg); - return ret; - } + if (ret) + goto error_disable_vref; return 0; + +error_disable_vref: + regulator_disable(st->vref); +error_disable_reg: + regulator_disable(st->reg); + + return ret; } static SIMPLE_DEV_PM_OPS(ad799x_pm_ops, ad799x_suspend, ad799x_resume);
A single error handling block at the end of the function could be brought in to make code a little more concise. Signed-off-by: Cai Huoqing <caihuoqing@baidu.com> --- v1->v2: Change the error path-error_disable_reg from "st->vref" to "st->reg". drivers/iio/adc/ad799x.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-)