On Sun, 12 Sep 2021 14:41:01 +0800 Cai Huoqing <caihuoqing@xxxxxxxxx> 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@xxxxxxxxx> +CC Florian who is reworking some of the regulator handling in this driver. https://lore.kernel.org/linux-iio/20210908152525.2946785-1-florian.boor@xxxxxxxxxxxxxxxxx/ 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);
